public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* [PATCH]Systemtap IA64 Runtime support
@ 2005-10-27 19:28 Keshavamurthy Anil S
  2005-10-27 20:53 ` Roland McGrath
  0 siblings, 1 reply; 9+ messages in thread
From: Keshavamurthy Anil S @ 2005-10-27 19:28 UTC (permalink / raw)
  To: systemtap; +Cc: anil.s.keshavamurthy, bibo.mao, yanmin.zhang, rohit.seth

[PATCH]: IA64 Runtime support patch
This patch adds Systemtap Runtime support for IA64 Architecture.

This patch has been tested on the following kernels
1)2.6.14-rc4, 
2)2.6.14-rc4 + Kprobes RCU scalability patches and
3)RHEL4 U2 Kernel 

The following tests has passed on Ia64 with the below patch.
1) Probes on function entry points
2) probes on function returns
3) Printing registers dump
4) Printing stack backtrace
5) Printing function arguments

The below patch applies cleanly onto Systemtap snapshot
systemtap-20051008.tar.bz2

Please apply.

========================================================================
 runtime/copy.c          |   12 ++++
 runtime/current.c       |   41 ++++++++++++++
 runtime/loc2c-runtime.h |  133 +++++++++++++++++++++++++++++++++++++++++++-----
 runtime/regs.h          |   14 ++++-
 runtime/runtime.h       |   31 ++++++++---
 runtime/stack.c         |   65 +++++++++++++++++++++++
 runtime/sym.c           |   16 ++++-
 tapsets.cxx             |   13 ++++
 translate.cxx           |    3 -
 9 files changed, 302 insertions(+), 26 deletions(-)

Index: src/runtime/copy.c
===================================================================
--- src.orig/runtime/copy.c
+++ src/runtime/copy.c
@@ -1,3 +1,12 @@
+/* Copy from user space functions
+ * Copyright (C) 2005 Intel Corporation.
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
+
 #ifndef _COPY_C_ /* -*- linux-c -*- */
 #define _COPY_C_
 
@@ -77,6 +86,9 @@ do {									   \
 #elif defined (__powerpc64__)
 #define __stp_strncpy_from_user(dst,src,count,res) \
 	do { res = __strncpy_from_user(dst, src, count); } while(0)
+#elif defined (__ia64__)
+#define __stp_strncpy_from_user(dst,src,count,res) \
+	do { res = __strncpy_from_user(dst, src, count); } while(0)
 #endif
 
 /** Copy a NULL-terminated string from userspace.
Index: src/runtime/current.c
===================================================================
--- src.orig/runtime/current.c
+++ src/runtime/current.c
@@ -1,3 +1,12 @@
+/* Functions to access the members of pt_regs struct
+ * Copyright (C) 2005 Intel Corporation.
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
+
 #ifndef _CURRENT_C_ /* -*- linux-c -*- */
 #define _CURRENT_C_
 
@@ -31,6 +40,8 @@ unsigned long _stp_ret_addr (struct pt_r
 	return regs->esp;
 #elif defined (__powerpc64__)
 	return REG_LINK(regs);
+#elif defined (__ia64__)
+	return regs->b0;
 #else
 	#error Unimplemented architecture
 #endif
@@ -99,6 +110,36 @@ void _stp_sprint_regs(String str, struct
         _stp_sprintf(str,"CR2: %016lx CR3: %016lx CR4: %016lx\n", cr2, cr3, cr4);
 }
 
+#elif defined (__ia64__)
+void _stp_sprint_regs(String str, struct pt_regs * regs)
+{
+     unsigned long ip = regs->cr_iip + ia64_psr(regs)->ri;
+
+	_stp_sprintf(str, "\nPid: %d, CPU %d, comm: %20s\n", current->pid,
+		smp_processor_id(), current->comm);
+	_stp_sprintf(str, "psr : %016lx ifs : %016lx ip  : [<%016lx>]  \n",
+		regs->cr_ipsr, regs->cr_ifs, ip);
+	_stp_sprintf(str, "unat: %016lx pfs : %016lx rsc : %016lx\n",
+		regs->ar_unat, regs->ar_pfs, regs->ar_rsc);
+	_stp_sprintf(str, "rnat: %016lx bsps: %016lx pr  : %016lx\n",
+		regs->ar_rnat, regs->ar_bspstore, regs->pr);
+	_stp_sprintf(str, "ldrs: %016lx ccv : %016lx fpsr: %016lx\n",
+		regs->loadrs, regs->ar_ccv, regs->ar_fpsr);
+	_stp_sprintf(str, "csd : %016lx ssd : %016lx\n",
+		regs->ar_csd, regs->ar_ssd);
+	_stp_sprintf(str, "b0  : %016lx b6  : %016lx b7  : %016lx\n",
+		regs->b0, regs->b6, regs->b7);
+	_stp_sprintf(str, "f6  : %05lx%016lx f7  : %05lx%016lx\n",
+		regs->f6.u.bits[1], regs->f6.u.bits[0],
+		regs->f7.u.bits[1], regs->f7.u.bits[0]);
+	_stp_sprintf(str, "f8  : %05lx%016lx f9  : %05lx%016lx\n",
+		regs->f8.u.bits[1], regs->f8.u.bits[0],
+		regs->f9.u.bits[1], regs->f9.u.bits[0]);
+	_stp_sprintf(str, "f10 : %05lx%016lx f11 : %05lx%016lx\n",
+		regs->f10.u.bits[1], regs->f10.u.bits[0],
+		regs->f11.u.bits[1], regs->f11.u.bits[0]);
+}
+
 #elif defined (__i386__)
 
 /** Write the registers to a string.
Index: src/runtime/loc2c-runtime.h
===================================================================
--- src.orig/runtime/loc2c-runtime.h
+++ src/runtime/loc2c-runtime.h
@@ -1,4 +1,11 @@
-/* target operations */
+/* target operations
+ * Copyright (C) 2005 Intel Corporation.
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
 
 #include <linux/types.h>
 #define intptr_t long
@@ -50,6 +57,75 @@
 #define dwarf_register_6(regs)	regs->esi
 #define dwarf_register_7(regs)	regs->edi
 
+#elif defined __ia64__
+#undef fetch_register
+#undef store_register
+
+struct ia64_stap_get_arbsp_param {
+	unsigned long ip;
+	unsigned long *address;
+};
+
+inline void ia64_stap_get_arbsp(struct unw_frame_info *info, void *arg)
+{
+	unsigned long ip;
+	struct ia64_stap_get_arbsp_param *lp = arg;
+
+	do {
+		unw_get_ip(info, &ip);
+		if (ip == 0)
+			break;
+		if (ip == lp->ip) {
+			unw_get_bsp(info, (unsigned long*)&lp->address);
+			return;
+		}
+	} while (unw_unwind(info) >= 0);
+	lp->address = 0;
+}
+
+inline intptr_t ia64_fetch_register(int regno, struct pt_regs *pt_regs)
+{
+	struct ia64_stap_get_arbsp_param pa;
+
+	if (regno < 32 || regno > 127)
+		return 0;
+
+	pa.ip = pt_regs->cr_iip;
+	unw_init_running(ia64_stap_get_arbsp, &pa);
+	if (pa.address == 0)
+		return 0;
+
+	return *ia64_rse_skip_regs(pa.address, regno-32);
+}
+
+inline void ia64_store_register(int regno,
+		struct pt_regs *pt_regs,
+		unsigned long value)
+{
+	struct ia64_stap_get_arbsp_param pa;
+	unsigned long rsc_save = 0;
+
+	if (regno < 32 || regno > 127)
+		return;
+
+	pa.ip = pt_regs->cr_iip;
+	unw_init_running(ia64_stap_get_arbsp, &pa);
+	if (pa.address == 0)
+		return;
+	*ia64_rse_skip_regs(pa.address, regno-32) = value;
+	//Invalidate all stacked registers outside the current frame
+	asm volatile( "mov %0=ar.rsc;;\n\t"
+			"mov ar.rsc=0;;\n\t"
+			"{\n\tloadrs;;\n\t\n\t\n\t}\n\t"
+			"mov ar.rsc=%1\n\t"
+			:"=r" (rsc_save):"r" (rsc_save):"memory");
+
+	return;
+}
+
+#define fetch_register(regno)		ia64_fetch_register(regno, c->regs)
+#define store_register(regno, value)	ia64_store_register(regno, c->regs, value)
+
 #elif defined __x86_64__
 
 #define dwarf_register_0(regs)	regs->rax
@@ -103,16 +179,47 @@
     int _bad = 0;							      \
     switch (size)							      \
       {									      \
-      case 1: __put_user_asm(((u8)(value)),addr,_bad,"b","b","iq",1); break;  \
-      case 2: __put_user_asm(((u16)(value)),addr,_bad,"w","w","ir",1); break; \
-      case 4: __put_user_asm(((u32)(value)),addr,_bad,"l","k","ir",1); break; \
-      case 8: __put_user_asm(((u64)(value)),addr,_bad,"q","","ir",1); break;  \
+      case 1: __put_user_asm(((u8)(value),addr,_bad,"b","b","iq",1); break;   \
+      case 2: __put_user_asm(((u16)(value),addr,_bad,"w","w","ir",1); break;  \
+      case 4: __put_user_asm(((u32)(value),addr,_bad,"l","k","ir",1); break;  \
+      case 8: __put_user_asm(((u64)(value),addr,_bad,"q","","ir",1); break;   \
       default: __put_user_bad();					      \
       }									      \
     if (_bad)								      \
       goto deref_fault;							      \
   })
 
+#elif defined __ia64__								
+#define deref(size, addr)						\
+  ({									\
+     int _bad = 0;							\
+     intptr_t _v=0;							\
+	switch (size){							\
+	case 1: __get_user_size(_v, addr, 1, _bad); break; 		\
+	case 2: __get_user_size(_v, addr, 2, _bad); break;  		\
+	case 4: __get_user_size(_v, addr, 4, _bad); break;  		\
+	case 8: __get_user_size(_v, addr, 8, _bad); break;  		\
+	default: __get_user_unknown(); break;				\
+	}								\
+    if (_bad)  								\
+	goto deref_fault;						\
+     _v;								\
+   })
+
+#define store_deref(size, addr, value)					\
+  ({									\
+    int _bad=0;								\
+	switch (size){							\
+	case 1: __put_user_size(value, addr, 1, _bad); break;		\
+	case 2: __put_user_size(value, addr, 2, _bad); break;		\
+	case 4: __put_user_size(value, addr, 4, _bad); break;		\
+	case 8: __put_user_size(value, addr, 8, _bad); break;		\
+	default: __put_user_unknown(); break;				\
+	}								\
+    if (_bad)								\
+	   goto deref_fault;						\
+   })
+
 #elif defined __powerpc64__
 
 #define deref(size, addr)						      \
@@ -137,10 +244,10 @@
     int _bad = 0;							      \
     switch (size)							      \
       {									      \
-      case 1: __put_user_asm(((u8)(value)),addr,_bad,"stb",1); break;	      \
-      case 2: __put_user_asm(((u16)(value)),addr,_bad,"sth",1); break;	      \
-      case 4: __put_user_asm(((u32)(value)),addr,_bad,"stw",1); break;	      \
-      case 8: __put_user_asm(((u64)(value)),addr,_bad,"std",1); break;	      \
+      case 1: __put_user_asm(((u8)(value),addr,_bad,"stb",1); break;   	      \
+      case 2: __put_user_asm(((u16)(value),addr,_bad,"sth",1); break;  	      \
+      case 4: __put_user_asm(((u32)(value),addr,_bad,"stw",1); break;  	      \
+      case 8: __put_user_asm(((u64)(value),addr,_bad,"std",1); break; 	      \
       default: __put_user_bad();					      \
       }									      \
     if (_bad)								      \
@@ -171,10 +278,10 @@
     int _bad = 0;							      \
     switch (size)							      \
       {									      \
-      case 1: __put_user_asm(((u8)(value)),addr,_bad,"stb"); break;	      \
-      case 2: __put_user_asm(((u16)(value)),addr,_bad,"sth"); break;	      \
-      case 4: __put_user_asm(((u32)(value)),addr,_bad,"stw"); break;	      \
-      case 8: __put_user_asm2(((u64)(value)),addr,_bad); break;		      \
+      case 1: __put_user_asm(((u8)(value),addr,_bad,"stb"); break;   	      \
+      case 2: __put_user_asm(((u16)(value),addr,_bad,"sth"); break;  	      \
+      case 4: __put_user_asm(((u32)(value),addr,_bad,"stw"); break;  	      \
+      case 8: __put_user_asm2(((u64)(value),addr,_bad); break;		      \
       default: __put_user_bad();					      \
       }									      \
     if (_bad)								      \
Index: src/runtime/regs.h
===================================================================
--- src.orig/runtime/regs.h
+++ src/runtime/regs.h
@@ -1,7 +1,15 @@
+/* common register includes used in multiple modules
+ * Copyright (C) 2005 Intel Corporation.
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
+
 #ifndef _REGS_H_ /* -*- linux-c -*- */
 #define _REGS_H_
 
-/* common register includes used in multiple modules */
 
 #ifdef __x86_64__
 
@@ -13,6 +21,10 @@
 #define REG_IP(regs) regs->eip
 #define REG_SP(regs) regs->esp
 
+#elif defined (__ia64__)
+#define REG_IP(regs)    ((regs)->cr_iip +ia64_psr(regs)->ri)
+#define REG_SP(regs)    ((regs)->r12)
+
 #elif defined (__powerpc64__)
 
 #define REG_IP(regs) regs->nip
Index: src/runtime/runtime.h
===================================================================
--- src.orig/runtime/runtime.h
+++ src/runtime/runtime.h
@@ -1,5 +1,6 @@
 /* main header file
  * Copyright (C) 2005 Red Hat Inc.
+ * Copyright (C) 2005 Intel Corporation.
  *
  * This file is part of systemtap, and is free software.  You can
  * redistribute it and/or modify it under the terms of the GNU General
@@ -64,15 +65,15 @@ static struct
 
 
 /************* Module Stuff ********************/
-static int (*_stp_kta)(unsigned long addr);
-static const char * (*_stp_kallsyms_lookup)(unsigned long addr,
+int (*_stp_kta)(unsigned long addr);
+const char * (*_stp_kallsyms_lookup)(unsigned long addr,
 					    unsigned long *symbolsize,
 					    unsigned long *offset,
 					    char **modname, char *namebuf);
 
 
 /* This implementation is used if stap_[num_]symbols are available. */
-static const char * _stp_kallsyms_lookup_tabled (unsigned long addr,
+const char * _stp_kallsyms_lookup_tabled (unsigned long addr,
 						 unsigned long *symbolsize,
 						 unsigned long *offset,
 						 char **modname,
@@ -81,7 +82,6 @@ static const char * _stp_kallsyms_lookup
   unsigned begin = 0;
   unsigned end = stap_num_symbols;
   /*const*/ struct stap_symbol* s;
-
   /* binary search on index [begin,end) */
   do
     {
@@ -121,15 +121,32 @@ static const char * _stp_kallsyms_lookup
 
 
 
+#ifdef __ia64__	
+  struct fnptr func_entry, *pfunc_entry;
+#endif
 int init_module (void)
 {
+
   _stp_kta = (int (*)(unsigned long))kallsyms_lookup_name("__kernel_text_address");
 
-  if (stap_num_symbols > 0)
-    _stp_kallsyms_lookup = & _stp_kallsyms_lookup_tabled;
-  else
+  if (stap_num_symbols > 0) {
+	_stp_kallsyms_lookup = & _stp_kallsyms_lookup_tabled;
+  } else {
+#ifdef __ia64__
+/* Refer to Ia-64 Softeware Conventions and Runtime Architecture Guide
+ * Chapter 8.3 about indirect call, A function pointer must pointer to
+ * a descriptor that contains both the address of the function entry 
+ * point and the gp register value for the target function.
+ */
+        func_entry.gp = ((struct fnptr *) kallsyms_lookup_name)->gp;
+        func_entry.ip = kallsyms_lookup_name("kallsyms_lookup");
+        _stp_kallsyms_lookup = (const char * (*)(unsigned long,unsigned long *,unsigned long *,char **,char *))&func_entry;
+
+#else
     _stp_kallsyms_lookup = (const char * (*)(unsigned long,unsigned long *,unsigned long *,char **,char *))
       kallsyms_lookup_name("kallsyms_lookup");
+#endif
+	}
 
   return _stp_transport_init();
 }
Index: src/runtime/stack.c
===================================================================
--- src.orig/runtime/stack.c
+++ src/runtime/stack.c
@@ -1,3 +1,12 @@
+/* Stack tracing functions
+ * Copyright (C) 2005 Intel Corporation.
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
+
 #ifndef _STACK_C_ /* -*- linux-c -*- */
 #define _STACK_C_
 
@@ -38,6 +47,62 @@ static void __stp_stack_sprint (String s
 	}
 }
 
+#elif defined (__ia64__)
+struct dump_para{
+  unsigned long *sp;
+  String str;
+};
+
+static void __stp_show_stack_sym(struct unw_frame_info *info, void *arg)
+{
+   unsigned long ip, skip=1;
+   String str = ((struct dump_para*)arg)->str;
+   struct pt_regs *regs = container_of(((struct dump_para*)arg)->sp, struct pt_regs, r12);
+
+	do {
+		unw_get_ip(info, &ip);
+		if (ip == 0) break;
+                if (skip){
+			if (ip == REG_IP(regs))
+				skip = 0;
+                        else continue;
+                }
+		_stp_string_cat(str, " ");
+		_stp_symbol_sprint(str, ip);
+		_stp_string_cat (str, "\n");
+        } while (unw_unwind(info) >= 0);
+}
+
+static void __stp_show_stack_addr(struct unw_frame_info *info, void *arg)
+{
+   unsigned long ip, skip=1;
+   String str = ((struct dump_para*)arg)->str;
+   struct pt_regs *regs = container_of(((struct dump_para*)arg)->sp, struct pt_regs, r12);	
+
+	do {
+		unw_get_ip(info, &ip);
+		if (ip == 0) break;
+		if (skip){
+			if (ip == REG_IP(regs))
+				skip = 0;
+			continue;
+		}
+		_stp_sprintf (str, "%lx ", ip);
+	} while (unw_unwind(info) >= 0);
+}
+
+static void __stp_stack_sprint (String str, unsigned long *stack, int verbose, int levels)
+{
+  struct dump_para para;
+
+	para.str = str;
+	para.sp  = stack; 
+	if (verbose)
+	    unw_init_running(__stp_show_stack_sym, &para);
+        else
+	    unw_init_running(__stp_show_stack_addr, &para);
+}
+
 #elif  defined (__i386__)
 
 static inline int valid_stack_ptr(struct thread_info *tinfo, void *p)
Index: src/runtime/sym.c
===================================================================
--- src.orig/runtime/sym.c
+++ src/runtime/sym.c
@@ -1,3 +1,12 @@
+/* Symbolic lookup functions
+ * Copyright (C) 2005 Intel Corporation.
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
+
 #ifndef _SYM_C_ /* -*- linux-c -*- */
 #define _SYM_C_
 
@@ -22,13 +31,12 @@ String _stp_symbol_sprint (String str, u
         const char *name;
         unsigned long offset, size;
         char namebuf[KSYM_NAME_LEN+1];
-
+	
         name = _stp_kallsyms_lookup(address, &size, &offset, &modname, namebuf);
 
-	_stp_sprintf (str, "0x%lx", address);
-
 	if (name) {		
-		if (modname)
+		_stp_sprintf (str, "0x%lx", address);
+		if (modname && *modname)
 			_stp_sprintf (str, " : %s+%#lx/%#lx [%s]", name, offset, size, modname);
 		else
 			_stp_sprintf (str, " : %s+%#lx/%#lx", name, offset, size);
Index: src/tapsets.cxx
===================================================================
--- src.orig/tapsets.cxx
+++ src/tapsets.cxx
@@ -1,5 +1,6 @@
 // tapset resolution
 // Copyright (C) 2005 Red Hat Inc.
+// Copyright (C) 2005 Intel Corporation.
 //
 // This file is part of systemtap, and is free software.  You can
 // redistribute it and/or modify it under the terms of the GNU General
@@ -1760,6 +1761,17 @@ query_func_info (Dwarf_Addr entrypc,
 	}
       else
 	{
+#ifdef __ia64__
+	// In IA64 platform function probe point is set at its
+	// entry point rather than prologue end pointer, and in
+	// IA64 prologue end point also is not computed in such way
+	if (q->sess.verbose)
+	   clog << "querying entrypc of function '"
+		<< fi.name << "'" << endl;
+	   query_statement (fi.name, fi.decl_file, fi.decl_line, 
+		&fi.die, entrypc, q);
+
+#else
 	  if (q->sess.verbose)
 	    clog << "querying prologue-end of function '" 
 		 << fi.name << "'" << endl;
@@ -1770,6 +1782,7 @@ query_func_info (Dwarf_Addr entrypc,
 
 	  query_statement (fi.name, fi.decl_file, fi.decl_line, 
 			   &fi.die, fi.prologue_end, q);
+#endif
 	}
     }
   catch (semantic_error &e)
Index: src/translate.cxx
===================================================================
--- src.orig/translate.cxx
+++ src/translate.cxx
@@ -1,5 +1,6 @@
 // translation pass
 // Copyright (C) 2005 Red Hat Inc.
+// Copyright (C) 2005 Intel Corporation
 //
 // This file is part of systemtap, and is free software.  You can
 // redistribute it and/or modify it under the terms of the GNU General
@@ -2452,7 +2453,7 @@ translate_pass (systemtap_session& s)
       s.op->newline() << "#define MAXNESTING 30";
       s.op->newline() << "#endif";
       s.op->newline() << "#ifndef MAXSTRINGLEN";
-      s.op->newline() << "#define MAXSTRINGLEN 128";
+      s.op->newline() << "#define MAXSTRINGLEN 256";
       s.op->newline() << "#endif";
       s.op->newline() << "#ifndef MAXTRYLOCK";
       s.op->newline() << "#define MAXTRYLOCK 20";

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH]Systemtap IA64 Runtime support
  2005-10-27 19:28 [PATCH]Systemtap IA64 Runtime support Keshavamurthy Anil S
@ 2005-10-27 20:53 ` Roland McGrath
  2005-10-27 23:19   ` Keshavamurthy Anil S
  0 siblings, 1 reply; 9+ messages in thread
From: Roland McGrath @ 2005-10-27 20:53 UTC (permalink / raw)
  To: Keshavamurthy Anil S; +Cc: systemtap, bibo.mao, yanmin.zhang, rohit.seth

Please clean up your patches so they don't touch unrelated lines with
random whitespace changes.

Don't use unadorned "inline", that defines globals.  For inlines in a
header for the kernel, use "static inline".  

For the ia64 register accessors, is it really advantageous to inline those?
Perhaps the functions should be in the runtime proper rather than inlined
by the macros.


Thanks,
Roland

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH]Systemtap IA64 Runtime support
  2005-10-27 20:53 ` Roland McGrath
@ 2005-10-27 23:19   ` Keshavamurthy Anil S
  2005-10-28 18:18     ` Frank Ch. Eigler
  0 siblings, 1 reply; 9+ messages in thread
From: Keshavamurthy Anil S @ 2005-10-27 23:19 UTC (permalink / raw)
  To: Roland McGrath
  Cc: Keshavamurthy Anil S, systemtap, bibo.mao, yanmin.zhang, rohit.seth

On Thu, Oct 27, 2005 at 01:53:43PM -0700, Roland McGrath wrote:
> Please clean up your patches so they don't touch unrelated lines with
> random whitespace changes.
> 
> Don't use unadorned "inline", that defines globals.  For inlines in a
> header for the kernel, use "static inline".  
> 
> For the ia64 register accessors, is it really advantageous to inline those?
> Perhaps the functions should be in the runtime proper rather than inlined
> by the macros.

Hi Roland,
	All the above are very valid feedback. Here is the modified patch.
And please let me know if you find any thing wrong and thanks for your review.

========================================================================
 runtime/copy.c          |   12 +++++++
 runtime/current.c       |   41 ++++++++++++++++++++++++
 runtime/loc2c-runtime.h |   47 +++++++++++++++++++++++++++-
 runtime/regs.c          |   80 ++++++++++++++++++++++++++++++++++++++++++++++++
 runtime/regs.h          |   14 +++++++-
 runtime/runtime.h       |   23 ++++++++++++-
 runtime/stack.c         |   65 +++++++++++++++++++++++++++++++++++++++
 runtime/sym.c           |   14 ++++++--
 tapsets.cxx             |   13 +++++++
 translate.cxx           |    4 +-
 10 files changed, 304 insertions(+), 9 deletions(-)

Index: src/runtime/copy.c
===================================================================
--- src.orig/runtime/copy.c
+++ src/runtime/copy.c
@@ -1,3 +1,12 @@
+/* Copy from user space functions
+ * Copyright (C) 2005 Intel Corporation.
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
+
 #ifndef _COPY_C_ /* -*- linux-c -*- */
 #define _COPY_C_
 
@@ -77,6 +86,9 @@ do {									   \
 #elif defined (__powerpc64__)
 #define __stp_strncpy_from_user(dst,src,count,res) \
 	do { res = __strncpy_from_user(dst, src, count); } while(0)
+#elif defined (__ia64__)
+#define __stp_strncpy_from_user(dst,src,count,res) \
+	do { res = __strncpy_from_user(dst, src, count); } while(0)
 #endif
 
 /** Copy a NULL-terminated string from userspace.
Index: src/runtime/current.c
===================================================================
--- src.orig/runtime/current.c
+++ src/runtime/current.c
@@ -1,3 +1,12 @@
+/* Functions to access the members of pt_regs struct
+ * Copyright (C) 2005 Intel Corporation.
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
+
 #ifndef _CURRENT_C_ /* -*- linux-c -*- */
 #define _CURRENT_C_
 
@@ -31,6 +40,8 @@ unsigned long _stp_ret_addr (struct pt_r
 	return regs->esp;
 #elif defined (__powerpc64__)
 	return REG_LINK(regs);
+#elif defined (__ia64__)
+	return regs->b0;
 #else
 	#error Unimplemented architecture
 #endif
@@ -99,6 +110,36 @@ void _stp_sprint_regs(String str, struct
         _stp_sprintf(str,"CR2: %016lx CR3: %016lx CR4: %016lx\n", cr2, cr3, cr4);
 }
 
+#elif defined (__ia64__)
+void _stp_sprint_regs(String str, struct pt_regs * regs)
+{
+     unsigned long ip = regs->cr_iip + ia64_psr(regs)->ri;
+
+	_stp_sprintf(str, "\nPid: %d, CPU %d, comm: %20s\n", current->pid,
+		smp_processor_id(), current->comm);
+	_stp_sprintf(str, "psr : %016lx ifs : %016lx ip  : [<%016lx>]  \n",
+		regs->cr_ipsr, regs->cr_ifs, ip);
+	_stp_sprintf(str, "unat: %016lx pfs : %016lx rsc : %016lx\n",
+		regs->ar_unat, regs->ar_pfs, regs->ar_rsc);
+	_stp_sprintf(str, "rnat: %016lx bsps: %016lx pr  : %016lx\n",
+		regs->ar_rnat, regs->ar_bspstore, regs->pr);
+	_stp_sprintf(str, "ldrs: %016lx ccv : %016lx fpsr: %016lx\n",
+		regs->loadrs, regs->ar_ccv, regs->ar_fpsr);
+	_stp_sprintf(str, "csd : %016lx ssd : %016lx\n",
+		regs->ar_csd, regs->ar_ssd);
+	_stp_sprintf(str, "b0  : %016lx b6  : %016lx b7  : %016lx\n",
+		regs->b0, regs->b6, regs->b7);
+	_stp_sprintf(str, "f6  : %05lx%016lx f7  : %05lx%016lx\n",
+		regs->f6.u.bits[1], regs->f6.u.bits[0],
+		regs->f7.u.bits[1], regs->f7.u.bits[0]);
+	_stp_sprintf(str, "f8  : %05lx%016lx f9  : %05lx%016lx\n",
+		regs->f8.u.bits[1], regs->f8.u.bits[0],
+		regs->f9.u.bits[1], regs->f9.u.bits[0]);
+	_stp_sprintf(str, "f10 : %05lx%016lx f11 : %05lx%016lx\n",
+		regs->f10.u.bits[1], regs->f10.u.bits[0],
+		regs->f11.u.bits[1], regs->f11.u.bits[0]);
+}
+
 #elif defined (__i386__)
 
 /** Write the registers to a string.
Index: src/runtime/loc2c-runtime.h
===================================================================
--- src.orig/runtime/loc2c-runtime.h
+++ src/runtime/loc2c-runtime.h
@@ -1,4 +1,11 @@
-/* target operations */
+/* target operations
+ * Copyright (C) 2005 Intel Corporation.
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
 
 #include <linux/types.h>
 #define intptr_t long
@@ -50,6 +57,13 @@
 #define dwarf_register_6(regs)	regs->esi
 #define dwarf_register_7(regs)	regs->edi
 
+#elif defined __ia64__
+#undef fetch_register
+#undef store_register
+
+#define fetch_register(regno)		ia64_fetch_register(regno, c->regs)
+#define store_register(regno, value)	ia64_store_register(regno, c->regs, value)
+
 #elif defined __x86_64__
 
 #define dwarf_register_0(regs)	regs->rax
@@ -113,6 +127,37 @@
       goto deref_fault;							      \
   })
 
+#elif defined __ia64__								
+#define deref(size, addr)						\
+  ({									\
+     int _bad = 0;							\
+     intptr_t _v=0;							\
+	switch (size){							\
+	case 1: __get_user_size(_v, addr, 1, _bad); break; 		\
+	case 2: __get_user_size(_v, addr, 2, _bad); break;  		\
+	case 4: __get_user_size(_v, addr, 4, _bad); break;  		\
+	case 8: __get_user_size(_v, addr, 8, _bad); break;  		\
+	default: __get_user_unknown(); break;				\
+	}								\
+    if (_bad)  								\
+	goto deref_fault;						\
+     _v;								\
+   })
+
+#define store_deref(size, addr, value)					\
+  ({									\
+    int _bad=0;								\
+	switch (size){							\
+	case 1: __put_user_size(value, addr, 1, _bad); break;		\
+	case 2: __put_user_size(value, addr, 2, _bad); break;		\
+	case 4: __put_user_size(value, addr, 4, _bad); break;		\
+	case 8: __put_user_size(value, addr, 8, _bad); break;		\
+	default: __put_user_unknown(); break;				\
+	}								\
+    if (_bad)								\
+	   goto deref_fault;						\
+   })
+
 #elif defined __powerpc64__
 
 #define deref(size, addr)						      \
Index: src/runtime/regs.c
===================================================================
--- /dev/null
+++ src/runtime/regs.c
@@ -0,0 +1,80 @@
+/* register access functions
+ * Copyright (C) 2005 Intel Corporation.
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
+
+#ifndef _REG_C_ /* -*- linux-c -*- */
+#define _REG_C_
+
+#if defined __ia64__
+
+struct ia64_stap_get_arbsp_param {
+	unsigned long ip;
+	unsigned long *address;
+};
+
+static void ia64_stap_get_arbsp(struct unw_frame_info *info, void *arg)
+{
+	unsigned long ip;
+	struct ia64_stap_get_arbsp_param *lp = arg;
+
+	do {
+		unw_get_ip(info, &ip);
+		if (ip == 0)
+			break;
+		if (ip == lp->ip) {
+			unw_get_bsp(info, (unsigned long*)&lp->address);
+			return;
+		}
+	} while (unw_unwind(info) >= 0);
+	lp->address = 0;
+}
+
+static long ia64_fetch_register(int regno, struct pt_regs *pt_regs)
+{
+	struct ia64_stap_get_arbsp_param pa;
+
+	if (regno < 32 || regno > 127)
+		return 0;
+
+	pa.ip = pt_regs->cr_iip;
+	unw_init_running(ia64_stap_get_arbsp, &pa);
+	if (pa.address == 0)
+		return 0;
+
+	return *ia64_rse_skip_regs(pa.address, regno-32);
+}
+
+static void ia64_store_register(int regno,
+		struct pt_regs *pt_regs,
+		unsigned long value)
+{
+	struct ia64_stap_get_arbsp_param pa;
+	unsigned long rsc_save = 0;
+
+	if (regno < 32 || regno > 127)
+		return;
+
+	pa.ip = pt_regs->cr_iip;
+	unw_init_running(ia64_stap_get_arbsp, &pa);
+	if (pa.address == 0)
+		return;
+	*ia64_rse_skip_regs(pa.address, regno-32) = value;
+	//Invalidate all stacked registers outside the current frame
+	asm volatile( "mov %0=ar.rsc;;\n\t"
+			"mov ar.rsc=0;;\n\t"
+			"{\n\tloadrs;;\n\t\n\t\n\t}\n\t"
+			"mov ar.rsc=%1\n\t"
+			:"=r" (rsc_save):"r" (rsc_save):"memory");
+
+	return;
+}
+
+#endif /* if defined __ia64__ */
+
+
+#endif /* _REG_C_ */
Index: src/runtime/regs.h
===================================================================
--- src.orig/runtime/regs.h
+++ src/runtime/regs.h
@@ -1,7 +1,15 @@
+/* common register includes used in multiple modules
+ * Copyright (C) 2005 Intel Corporation.
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
+
 #ifndef _REGS_H_ /* -*- linux-c -*- */
 #define _REGS_H_
 
-/* common register includes used in multiple modules */
 
 #ifdef __x86_64__
 
@@ -13,6 +21,10 @@
 #define REG_IP(regs) regs->eip
 #define REG_SP(regs) regs->esp
 
+#elif defined (__ia64__)
+#define REG_IP(regs)    ((regs)->cr_iip +ia64_psr(regs)->ri)
+#define REG_SP(regs)    ((regs)->r12)
+
 #elif defined (__powerpc64__)
 
 #define REG_IP(regs) regs->nip
Index: src/runtime/runtime.h
===================================================================
--- src.orig/runtime/runtime.h
+++ src/runtime/runtime.h
@@ -1,5 +1,6 @@
 /* main header file
  * Copyright (C) 2005 Red Hat Inc.
+ * Copyright (C) 2005 Intel Corporation.
  *
  * This file is part of systemtap, and is free software.  You can
  * redistribute it and/or modify it under the terms of the GNU General
@@ -121,15 +122,31 @@ static const char * _stp_kallsyms_lookup
 
 
 
+#ifdef __ia64__	
+  struct fnptr func_entry, *pfunc_entry;
+#endif
 int init_module (void)
 {
   _stp_kta = (int (*)(unsigned long))kallsyms_lookup_name("__kernel_text_address");
 
-  if (stap_num_symbols > 0)
-    _stp_kallsyms_lookup = & _stp_kallsyms_lookup_tabled;
-  else
+  if (stap_num_symbols > 0) {
+	_stp_kallsyms_lookup = & _stp_kallsyms_lookup_tabled;
+  } else {
+#ifdef __ia64__
+/* Refer to Ia-64 Softeware Conventions and Runtime Architecture Guide
+ * Chapter 8.3 about indirect call, A function pointer must pointer to
+ * a descriptor that contains both the address of the function entry 
+ * point and the gp register value for the target function.
+ */
+        func_entry.gp = ((struct fnptr *) kallsyms_lookup_name)->gp;
+        func_entry.ip = kallsyms_lookup_name("kallsyms_lookup");
+        _stp_kallsyms_lookup = (const char * (*)(unsigned long,unsigned long *,unsigned long *,char **,char *))&func_entry;
+
+#else
     _stp_kallsyms_lookup = (const char * (*)(unsigned long,unsigned long *,unsigned long *,char **,char *))
       kallsyms_lookup_name("kallsyms_lookup");
+#endif
+	}
 
   return _stp_transport_init();
 }
Index: src/runtime/stack.c
===================================================================
--- src.orig/runtime/stack.c
+++ src/runtime/stack.c
@@ -1,3 +1,12 @@
+/* Stack tracing functions
+ * Copyright (C) 2005 Intel Corporation.
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
+
 #ifndef _STACK_C_ /* -*- linux-c -*- */
 #define _STACK_C_
 
@@ -38,6 +47,62 @@ static void __stp_stack_sprint (String s
 	}
 }
 
+#elif defined (__ia64__)
+struct dump_para{
+  unsigned long *sp;
+  String str;
+};
+
+static void __stp_show_stack_sym(struct unw_frame_info *info, void *arg)
+{
+   unsigned long ip, skip=1;
+   String str = ((struct dump_para*)arg)->str;
+   struct pt_regs *regs = container_of(((struct dump_para*)arg)->sp, struct pt_regs, r12);
+
+	do {
+		unw_get_ip(info, &ip);
+		if (ip == 0) break;
+                if (skip){
+			if (ip == REG_IP(regs))
+				skip = 0;
+                        else continue;
+                }
+		_stp_string_cat(str, " ");
+		_stp_symbol_sprint(str, ip);
+		_stp_string_cat (str, "\n");
+        } while (unw_unwind(info) >= 0);
+}
+
+static void __stp_show_stack_addr(struct unw_frame_info *info, void *arg)
+{
+   unsigned long ip, skip=1;
+   String str = ((struct dump_para*)arg)->str;
+   struct pt_regs *regs = container_of(((struct dump_para*)arg)->sp, struct pt_regs, r12);	
+
+	do {
+		unw_get_ip(info, &ip);
+		if (ip == 0) break;
+		if (skip){
+			if (ip == REG_IP(regs))
+				skip = 0;
+			continue;
+		}
+		_stp_sprintf (str, "%lx ", ip);
+	} while (unw_unwind(info) >= 0);
+}
+
+static void __stp_stack_sprint (String str, unsigned long *stack, int verbose, int levels)
+{
+  struct dump_para para;
+
+	para.str = str;
+	para.sp  = stack; 
+	if (verbose)
+	    unw_init_running(__stp_show_stack_sym, &para);
+        else
+	    unw_init_running(__stp_show_stack_addr, &para);
+}
+
 #elif  defined (__i386__)
 
 static inline int valid_stack_ptr(struct thread_info *tinfo, void *p)
Index: src/runtime/sym.c
===================================================================
--- src.orig/runtime/sym.c
+++ src/runtime/sym.c
@@ -1,3 +1,12 @@
+/* Symbolic lookup functions
+ * Copyright (C) 2005 Intel Corporation.
+ *
+ * This file is part of systemtap, and is free software.  You can
+ * redistribute it and/or modify it under the terms of the GNU General
+ * Public License (GPL); either version 2, or (at your option) any
+ * later version.
+ */
+
 #ifndef _SYM_C_ /* -*- linux-c -*- */
 #define _SYM_C_
 
@@ -25,10 +34,9 @@ String _stp_symbol_sprint (String str, u
 
         name = _stp_kallsyms_lookup(address, &size, &offset, &modname, namebuf);
 
-	_stp_sprintf (str, "0x%lx", address);
-
 	if (name) {		
-		if (modname)
+		_stp_sprintf (str, "0x%lx", address);
+		if (modname && *modname)
 			_stp_sprintf (str, " : %s+%#lx/%#lx [%s]", name, offset, size, modname);
 		else
 			_stp_sprintf (str, " : %s+%#lx/%#lx", name, offset, size);
Index: src/tapsets.cxx
===================================================================
--- src.orig/tapsets.cxx
+++ src/tapsets.cxx
@@ -1,5 +1,6 @@
 // tapset resolution
 // Copyright (C) 2005 Red Hat Inc.
+// Copyright (C) 2005 Intel Corporation.
 //
 // This file is part of systemtap, and is free software.  You can
 // redistribute it and/or modify it under the terms of the GNU General
@@ -1760,6 +1761,17 @@ query_func_info (Dwarf_Addr entrypc,
 	}
       else
 	{
+#ifdef __ia64__
+	// In IA64 platform function probe point is set at its
+	// entry point rather than prologue end pointer, and in
+	// IA64 prologue end point also is not computed in such way
+	if (q->sess.verbose)
+	   clog << "querying entrypc of function '"
+		<< fi.name << "'" << endl;
+	   query_statement (fi.name, fi.decl_file, fi.decl_line, 
+		&fi.die, entrypc, q);
+
+#else
 	  if (q->sess.verbose)
 	    clog << "querying prologue-end of function '" 
 		 << fi.name << "'" << endl;
@@ -1770,6 +1782,7 @@ query_func_info (Dwarf_Addr entrypc,
 
 	  query_statement (fi.name, fi.decl_file, fi.decl_line, 
 			   &fi.die, fi.prologue_end, q);
+#endif
 	}
     }
   catch (semantic_error &e)
Index: src/translate.cxx
===================================================================
--- src.orig/translate.cxx
+++ src/translate.cxx
@@ -1,5 +1,6 @@
 // translation pass
 // Copyright (C) 2005 Red Hat Inc.
+// Copyright (C) 2005 Intel Corporation
 //
 // This file is part of systemtap, and is free software.  You can
 // redistribute it and/or modify it under the terms of the GNU General
@@ -2452,7 +2453,7 @@ translate_pass (systemtap_session& s)
       s.op->newline() << "#define MAXNESTING 30";
       s.op->newline() << "#endif";
       s.op->newline() << "#ifndef MAXSTRINGLEN";
-      s.op->newline() << "#define MAXSTRINGLEN 128";
+      s.op->newline() << "#define MAXSTRINGLEN 256";
       s.op->newline() << "#endif";
       s.op->newline() << "#ifndef MAXTRYLOCK";
       s.op->newline() << "#define MAXTRYLOCK 20";
@@ -2477,6 +2478,7 @@ translate_pass (systemtap_session& s)
       s.op->newline() << "#include \"runtime.h\"";
       s.op->newline() << "#include \"current.c\"";
       s.op->newline() << "#include \"stack.c\"";
+      s.op->newline() << "#include \"regs.c\"";
       s.op->newline() << "#include <linux/string.h>";
       s.op->newline() << "#include <linux/timer.h>";
       s.op->newline() << "#endif";

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH]Systemtap IA64 Runtime support
  2005-10-27 23:19   ` Keshavamurthy Anil S
@ 2005-10-28 18:18     ` Frank Ch. Eigler
  2005-10-28 19:27       ` Keshavamurthy Anil S
  0 siblings, 1 reply; 9+ messages in thread
From: Frank Ch. Eigler @ 2005-10-28 18:18 UTC (permalink / raw)
  To: Keshavamurthy Anil S; +Cc: systemtap, bibo.mao, yanmin.zhang, rohit.seth


anil.s.keshavamurthy wrote:

> [...]
> --- src.orig/runtime/copy.c
> +++ src/runtime/copy.c
> @@ -1,3 +1,12 @@
> +/* Copy from user space functions
> + * Copyright (C) 2005 Intel Corporation.
> + *
> + * This file is part of systemtap, and is free software.  You can
> + * redistribute it and/or modify it under the terms of the GNU General
> + * Public License (GPL); either version 2, or (at your option) any
> + * later version.
> + */

(There seems to have been an oversight in attaching Red Hat copyright
notices to some of the runtime files.)


>  #elif defined (__powerpc64__)
>  #define __stp_strncpy_from_user(dst,src,count,res) \
>  	do { res = __strncpy_from_user(dst, src, count); } while(0)
> +#elif defined (__ia64__)
> +#define __stp_strncpy_from_user(dst,src,count,res) \
> +	do { res = __strncpy_from_user(dst, src, count); } while(0)
>  #endif

Why not  "#elif defined (__powerpc64__) || defined (__ia64__)?


> [...]
> @@ -25,10 +34,9 @@ String _stp_symbol_sprint (String str, u
> -	_stp_sprintf (str, "0x%lx", address);
> -
>  	if (name) {		
> -		if (modname)
> +		_stp_sprintf (str, "0x%lx", address);

Why is this part now conditional on <name> ?

> [...]
> +	// In IA64 platform function probe point is set at its
> +	// entry point rather than prologue end pointer, and in
> +	// IA64 prologue end point also is not computed in such way

I probably missed the discussion, but why exactly is this?  Are all
the parameters available at the debuginfo-identified location list
right at this entry address?


> [...]
> -      s.op->newline() << "#define MAXSTRINGLEN 128";
> +      s.op->newline() << "#define MAXSTRINGLEN 256";

Why?


- FChE

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH]Systemtap IA64 Runtime support
  2005-10-28 18:18     ` Frank Ch. Eigler
@ 2005-10-28 19:27       ` Keshavamurthy Anil S
  2005-10-28 19:33         ` Frank Ch. Eigler
  0 siblings, 1 reply; 9+ messages in thread
From: Keshavamurthy Anil S @ 2005-10-28 19:27 UTC (permalink / raw)
  To: Frank Ch. Eigler
  Cc: Keshavamurthy Anil S, systemtap, bibo.mao, yanmin.zhang, rohit.seth

On Fri, Oct 28, 2005 at 02:18:12PM -0400, Frank Ch. Eigler wrote:
Hi Frank,
	Thanks for taking a look at my code, I have inlined my replies to
your comments. 

I will refresh my patch and send it again once you confirm my comments.

Thanks,
Anil

> 
> anil.s.keshavamurthy wrote:
> 
> > [...]
> > --- src.orig/runtime/copy.c
> > +++ src/runtime/copy.c
> > @@ -1,3 +1,12 @@
> > +/* Copy from user space functions
> > + * Copyright (C) 2005 Intel Corporation.
> > + *
> > + * This file is part of systemtap, and is free software.  You can
> > + * redistribute it and/or modify it under the terms of the GNU General
> > + * Public License (GPL); either version 2, or (at your option) any
> > + * later version.
> > + */
> (There seems to have been an oversight in attaching Red Hat copyright
> notices to some of the runtime files.)

ARe you asking me to remove the above lines in all the files?
The reason I copied the above was that some files had this GPL statement and
some files didn't.
Also are you saying not keep even "Copyright (C) 2005 Intel Corporation"


> 
> 
> >  #elif defined (__powerpc64__)
> >  #define __stp_strncpy_from_user(dst,src,count,res) \
> >  	do { res = __strncpy_from_user(dst, src, count); } while(0)
> > +#elif defined (__ia64__)
> > +#define __stp_strncpy_from_user(dst,src,count,res) \
> > +	do { res = __strncpy_from_user(dst, src, count); } while(0)
> >  #endif
> 
> Why not  "#elif defined (__powerpc64__) || defined (__ia64__)?
No problem. I can do this.

> 
> 
> > [...]
> > @@ -25,10 +34,9 @@ String _stp_symbol_sprint (String str, u
> > -	_stp_sprintf (str, "0x%lx", address);
> > -
> >  	if (name) {		
> > -		if (modname)
> > +		_stp_sprintf (str, "0x%lx", address);
> 
> Why is this part now conditional on <name> ?
Oh well..Initially I had thought of dumping the address only if it has a
corresponding symbol name. I think original code was right where it prints
the address irrespective of its symbol name. I will revert back.

> 
> > [...]
> > +	// In IA64 platform function probe point is set at its
> > +	// entry point rather than prologue end pointer, and in
> > +	// IA64 prologue end point also is not computed in such way
> 
> I probably missed the discussion, but why exactly is this?  Are all
> the parameters available at the debuginfo-identified location list
> right at this entry address?
On Ia64 platform, I found probe address of function entry translated by
systemtap was one bundle more than the actual address. However, when
queried from entrypc rather than prolouge-end of the function we got the
right address translation. Let me know if you are ok with this change and
I will reword my comments.

> 
> 
> > [...]
> > -      s.op->newline() << "#define MAXSTRINGLEN 128";
> > +      s.op->newline() << "#define MAXSTRINGLEN 256";
> 
> Why?
Humm..I think this can be reverted back.



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH]Systemtap IA64 Runtime support
  2005-10-28 19:27       ` Keshavamurthy Anil S
@ 2005-10-28 19:33         ` Frank Ch. Eigler
  2005-10-28 20:01           ` Keshavamurthy Anil S
  2005-10-28 20:03           ` Roland McGrath
  0 siblings, 2 replies; 9+ messages in thread
From: Frank Ch. Eigler @ 2005-10-28 19:33 UTC (permalink / raw)
  To: Keshavamurthy Anil S; +Cc: systemtap, bibo.mao, yanmin.zhang, rohit.seth

Hi -


> > (There seems to have been an oversight in attaching Red Hat copyright
> > notices to some of the runtime files.)
>
> [...]  The reason I copied the above was that some files had this
> GPL statement and some files didn't.  Also are you saying not keep
> even "Copyright (C) 2005 Intel Corporation"

Not at all - just that Red Hat should have already been there too.  If
you were so kind as to add it, that would be great.


> > > [...]
> > > +	// In IA64 platform function probe point is set at its
> > > +	// entry point rather than prologue end pointer, and in
> > > +	// IA64 prologue end point also is not computed in such way
> > 
> > I probably missed the discussion, but why exactly is this?  Are all
> > the parameters available at the debuginfo-identified location list
> > right at this entry address?
>
> On Ia64 platform, I found probe address of function entry translated by
> systemtap was one bundle more than the actual address. However, when
> queried from entrypc rather than prolouge-end of the function we got the
> right address translation. [...]

I guess it comes down testing it - whichever works should stay.  How
many of the pass-4 / pass-5 tests have you been able to run?  Also,
have you tested systemtap on a plain x86 after your changes, just to
make sure that the base port was not affected?

Do you already have physical CVS write privileges?  If so, you could
commit.


- FChE

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH]Systemtap IA64 Runtime support
  2005-10-28 19:33         ` Frank Ch. Eigler
@ 2005-10-28 20:01           ` Keshavamurthy Anil S
  2005-10-28 20:06             ` Frank Ch. Eigler
  2005-10-28 20:03           ` Roland McGrath
  1 sibling, 1 reply; 9+ messages in thread
From: Keshavamurthy Anil S @ 2005-10-28 20:01 UTC (permalink / raw)
  To: Frank Ch. Eigler
  Cc: Keshavamurthy Anil S, systemtap, bibo.mao, yanmin.zhang, rohit.seth

On Fri, Oct 28, 2005 at 03:33:32PM -0400, Frank Ch. Eigler wrote:
> Not at all - just that Red Hat should have already been there too.  If
> you were so kind as to add it, that would be great.
Sure, not a problem, I will add it.

> I guess it comes down testing it - whichever works should stay.  How
> many of the pass-4 / pass-5 tests have you been able to run?  Also,
> have you tested systemtap on a plain x86 after your changes, just to
> make sure that the base port was not affected?

Almost all of my changes are under #if defined (__ia64__) but definitely
before check in, I will make sure I test it on i386 once again.

BTW, I tested manually by writing some tap scripts, which covered
most of the stuff like, printing function args, stack dump, function probe,
return probe etc. Also with the make check, it reported 2 of 103 tests failed.

Can you please elaborate if there are any other standard tests 
that I should be running?

> 
> Do you already have physical CVS write privileges?  If so, you could
> commit.
Yes, I have and will check it in.

thanks,
Anil

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH]Systemtap IA64 Runtime support
  2005-10-28 19:33         ` Frank Ch. Eigler
  2005-10-28 20:01           ` Keshavamurthy Anil S
@ 2005-10-28 20:03           ` Roland McGrath
  1 sibling, 0 replies; 9+ messages in thread
From: Roland McGrath @ 2005-10-28 20:03 UTC (permalink / raw)
  To: Frank Ch. Eigler
  Cc: Keshavamurthy Anil S, systemtap, bibo.mao, yanmin.zhang, rohit.seth

> > On Ia64 platform, I found probe address of function entry translated by
> > systemtap was one bundle more than the actual address. However, when
> > queried from entrypc rather than prolouge-end of the function we got the
> > right address translation. [...]
> 
> I guess it comes down testing it - whichever works should stay.  

We should figure out what the issues are, as well as working around them.
There have been DWARF consumer and GCC issues exposed by the other platforms.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH]Systemtap IA64 Runtime support
  2005-10-28 20:01           ` Keshavamurthy Anil S
@ 2005-10-28 20:06             ` Frank Ch. Eigler
  0 siblings, 0 replies; 9+ messages in thread
From: Frank Ch. Eigler @ 2005-10-28 20:06 UTC (permalink / raw)
  To: Keshavamurthy Anil S; +Cc: systemtap, bibo.mao, yanmin.zhang, rohit.seth

Hi -

> [...]  BTW, I tested manually by writing some tap scripts, which
> covered most of the stuff like, printing function args, stack dump,
> function probe, return probe etc. Also with the make check, it
> reported 2 of 103 tests failed.

OK.  (Which ones failed?)

> Can you please elaborate if there are any other standard tests 
> that I should be running?

You might give the dejagnu test suite under /tests/testsuite a try.


- FChE

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2005-10-28 20:06 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-10-27 19:28 [PATCH]Systemtap IA64 Runtime support Keshavamurthy Anil S
2005-10-27 20:53 ` Roland McGrath
2005-10-27 23:19   ` Keshavamurthy Anil S
2005-10-28 18:18     ` Frank Ch. Eigler
2005-10-28 19:27       ` Keshavamurthy Anil S
2005-10-28 19:33         ` Frank Ch. Eigler
2005-10-28 20:01           ` Keshavamurthy Anil S
2005-10-28 20:06             ` Frank Ch. Eigler
2005-10-28 20:03           ` Roland McGrath

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).