[RFC] Hardware Breakpoint support for systemtap translator

[RFC] Hardware Breakpoint support for systemtap translator

[Prerna Saxena]

[-- Attachment #1: Type: text/plain, Size: 952 bytes --]

Hi,
The attached set patches enable systemtap translator to probe kernel 
hardware breakpoints on x86.
It introduces the following language constructs :

1. probe kernel.data(ADDRESS).write     {....}   : To probe writes at 
the given address
2. probe kernel.data(ADDRESS).rw         {....}   : To probe read & 
write access to the given address.
3. probe kernel.data("SYMBOL").write   {....}
4. probe kernel.data("SYMBOL").rw       {....}   : Similar to 1,2, but 
using a symbol name as argument.

These are agnostic to dwarf/debuginfo and depend on the hardware 
breakpoint infrastructure for resolving symbols.

Things that remain to be done :
1. Translator support for probing hardware breakpoints on other 
architectures (ppc/s390 etc)
2. Implementation of userspace hardware breakpoints.

I'm still testing it...Looking for comments and feedback :-)

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India 


[-- Attachment #2: hwbkpt-1.patch --]
[-- Type: text/x-diff, Size: 13523 bytes --]

Index: git-3-july/tapsets.cxx
===================================================================
--- git-3-july.orig/tapsets.cxx
+++ git-3-july/tapsets.cxx
@@ -4929,8 +4929,323 @@ kprobe_builder::build(systemtap_session 
     }
 }
 
+// ------------------------------------------------------------------------
+//  Hardware breakpoint based probes.
+// ------------------------------------------------------------------------
+
+static const string TOK_HWBKPT("data");
+static const string TOK_HWBKPT_WRITE("write");
+static const string TOK_HWBKPT_RW("rw");
+
+#define HWBKPT_READ 0
+#define HWBKPT_WRITE 1
+#define HWBKPT_RW 2
+struct hwbkpt_derived_probe: public derived_probe
+{
+  hwbkpt_derived_probe (probe *base,
+                        probe_point *location,
+                        int64_t addr,
+			string symname,
+			bool has_only_read_access,
+			bool has_only_write_access,
+			bool has_rw_access
+                        );
+  Dwarf_Addr hwbkpt_addr;
+  string symbol_name;
+  unsigned int hwbkpt_access;
+
+  void printsig (std::ostream &o) const;
+  void join_group (systemtap_session& s);
+};
+
+struct hwbkpt_derived_probe_group: public derived_probe_group
+{
+private:
+
+  multimap<string, hwbkpt_derived_probe*> probes_by_module;
+  typedef map<string, hwbkpt_derived_probe*>::iterator p_b_m_iterator;
 
 
+public:
+  void enroll (hwbkpt_derived_probe* probe);
+  void emit_module_decls (systemtap_session& s);
+  void emit_module_init (systemtap_session& s);
+  void emit_module_exit (systemtap_session& s);
+};
+
+hwbkpt_derived_probe::hwbkpt_derived_probe (probe *base,
+					    probe_point *location,
+					    int64_t addr,
+					    string symname,
+					    bool has_only_read_access,
+					    bool has_only_write_access,
+					    bool has_rw_access
+					    ):
+  derived_probe (base, location),
+  hwbkpt_addr (addr),
+  symbol_name (symname)
+{
+  this->tok = base->tok;
+
+  // Expansion of $target variables in the probe body produces an error
+  // during translate phase
+  vector<probe_point::component*> comps;
+  comps.push_back (new probe_point::component(TOK_KERNEL));
+
+  if (hwbkpt_addr)
+	  comps.push_back (new probe_point::component (TOK_HWBKPT, new literal_number(hwbkpt_addr)));
+  else
+	if (symbol_name.size())
+	  comps.push_back (new probe_point::component (TOK_HWBKPT, new literal_string(symbol_name)));
+
+  if (has_only_read_access)
+	this->hwbkpt_access = HWBKPT_READ ;
+//TODO add code for comps.push_back for read, since this flag is not for x86
+
+  else
+	{
+	  if (has_only_write_access)
+		{
+		  this->hwbkpt_access = HWBKPT_WRITE ;
+	  	  comps.push_back (new probe_point::component(TOK_HWBKPT_WRITE));
+		}
+	  else
+		{
+		  this->hwbkpt_access = HWBKPT_RW ;
+	  	  comps.push_back (new probe_point::component(TOK_HWBKPT_RW));
+		}
+	}
+
+  this->sole_location()->components = comps;
+}
+
+void hwbkpt_derived_probe::printsig (ostream& o) const
+{
+  sole_location()->print (o);
+  o << " /* " << " Address = " << hwbkpt_addr << "*/";
+  o << " /* " << " Symbol  = " << symbol_name << "*/";
+  switch (hwbkpt_access)
+	{
+	  case HWBKPT_READ:
+  		o << " /* " << " Access = READ " << "*/";
+	  case HWBKPT_WRITE:
+  		o << " /* " << " Access = WRITE " << "*/";
+	  case HWBKPT_RW:
+  		o << " /* " << " Access = RW " << "*/";
+	}
+  printsig_nested (o);
+}
+
+void hwbkpt_derived_probe::join_group (systemtap_session& s)
+{
+  if (! s.hwbkpt_derived_probes)
+        s.hwbkpt_derived_probes = new hwbkpt_derived_probe_group ();
+  s.hwbkpt_derived_probes->enroll (this);
+}
+
+void hwbkpt_derived_probe_group::enroll (hwbkpt_derived_probe* p)
+{
+  string hwbkpt_key;
+  if (p->hwbkpt_addr)
+	{
+	  char hwbkpt_addr_str[64];
+	  sprintf(hwbkpt_addr_str, "%u",int(p->hwbkpt_addr));
+	  hwbkpt_key=hwbkpt_addr_str;
+	}
+  else
+	  hwbkpt_key=p->symbol_name;
+  switch (p->hwbkpt_access)
+	{
+	  case HWBKPT_READ:
+		hwbkpt_key = string("R_") + hwbkpt_key;
+	  case HWBKPT_WRITE:
+		hwbkpt_key = string("W_") + hwbkpt_key;
+	  case HWBKPT_RW:
+		hwbkpt_key = string("RW") + hwbkpt_key;
+	}
+
+  probes_by_module.insert (make_pair (hwbkpt_key, p));
+}
+
+void
+hwbkpt_derived_probe_group::emit_module_decls (systemtap_session& s)
+{
+  if (probes_by_module.empty()) return;
+
+  s.op->newline() << "/* ---- hwbkpt-based probes ---- */";
+  s.op->newline() << "#include <asm/hw_breakpoint.h>";
+  s.op->newline();
+
+// Define macros for access type
+  s.op->newline() << "#define HWBKPT_READ 0";
+  s.op->newline() << "#define HWBKPT_WRITE 1";
+  s.op->newline() << "#define HWBKPT_RW 2";
+
+  // Forward declare the master entry functions
+  s.op->newline() << "static int enter_hwbkpt_probe (struct hw_breakpoint *inst,";
+  s.op->line() << " struct pt_regs *regs);";
+
+  // Emit the actual probe list.
+
+  s.op->newline() << "static struct hw_breakpoint ";
+  s.op->newline() << "stap_hwbkpt_probe_array[" << probes_by_module.size() << "];";
+
+  s.op->newline() << "static struct stap_hwbkpt_probe {";
+  s.op->newline() << "unsigned registered_p:1;";
+
+  // Probe point & Symbol Names are mostly small and uniform enough
+  // to justify putting char[MAX]'s into  the array instead of
+  // relocated char*'s.
+
+  size_t pp_name_max = 0 , symbol_name_max = 0;
+  size_t pp_name_tot = 0 , symbol_name_tot = 0;
+  for (p_b_m_iterator it = probes_by_module.begin(); it != probes_by_module.end(); it++)
+    {
+      hwbkpt_derived_probe* p = it->second;
+#define DOIT(var,expr) do {                             \
+        size_t var##_size = (expr) + 1;                 \
+        var##_max = max (var##_max, var##_size);        \
+        var##_tot += var##_size; } while (0)
+      DOIT(pp_name, lex_cast_qstring(*p->sole_location()).size());
+      DOIT(symbol_name, p->symbol_name.size());
+#undef DOIT
+    }
+
+#define CALCIT(var)                                                     \
+  s.op->newline() << "const char " << #var << "[" << var##_name_max << "] ;";
+  CALCIT(pp);
+  CALCIT(symbol);
+#undef CALCIT
+
+  s.op->newline() << "const unsigned long address;";
+  s.op->newline() << "unsigned int atype;";
+  s.op->newline() << "void (* const ph) (struct context*);";
+  s.op->newline() << "} stap_hwbkpt_probes[] = {";
+  s.op->indent(1);
+
+  for (p_b_m_iterator it = probes_by_module.begin(); it != probes_by_module.end(); it++)
+    {
+      hwbkpt_derived_probe* p = it->second;
+      s.op->newline() << "{";
+      if (p->symbol_name.size())
+      s.op->line() << " .address=(unsigned long)0x0" << "ULL,";
+      else
+      s.op->line() << " .address=(unsigned long)0x" << hex << p->hwbkpt_addr << dec << "ULL,";
+      s.op->line() << " .atype=" << p->hwbkpt_access << ",";
+      s.op->line() << " .pp=" << lex_cast_qstring (*p->sole_location()) << ",";
+      s.op->line() << " .symbol=\"" << p->symbol_name << "\",";
+      s.op->line() << " .ph=&" << p->name << "";
+      s.op->line() << " },";
+    }
+  s.op->newline(-1) << "};";
+
+  // Emit the hwbkpt callback function
+  s.op->newline();
+  s.op->newline() << "static int enter_hwbkpt_probe (struct hw_breakpoint *inst,";
+  s.op->line() << " struct pt_regs *regs) {";
+  s.op->newline(1) << "int hwbkpt_probe_idx = ((uintptr_t)inst-(uintptr_t)stap_hwbkpt_probe_array)/sizeof(struct hw_breakpoint);";
+
+  // Check that the index is plausible
+  s.op->newline() << "struct stap_hwbkpt_probe *sdp = &stap_hwbkpt_probes[";
+  s.op->line() << "((hwbkpt_probe_idx >= 0 && hwbkpt_probe_idx < " << probes_by_module.size() << ")?";
+  s.op->line() << "hwbkpt_probe_idx:0)"; // NB: at least we avoid memory corruption
+  // XXX: it would be nice to give a more verbose error though; BUG_ON later?
+  s.op->line() << "];";
+  common_probe_entryfn_prologue (s.op, "STAP_SESSION_RUNNING", "sdp->pp");
+  s.op->newline() << "c->regs = regs;";
+  s.op->newline() << "(*sdp->ph) (c);";
+  common_probe_entryfn_epilogue (s.op);
+  s.op->newline() << "return 0;";
+  s.op->newline(-1) << "}";
+}
+
+
+void
+hwbkpt_derived_probe_group::emit_module_init (systemtap_session& s)
+{
+  s.op->newline() << "for (i=0; i<" << probes_by_module.size() << "; i++) {";
+  s.op->newline(1) << "struct stap_hwbkpt_probe *sdp = & stap_hwbkpt_probes[i];";
+  s.op->newline() << "struct hw_breakpoint *hp = & stap_hwbkpt_probe_array[i];";
+  s.op->newline() << "void *addr = (void *) sdp->address;";
+  s.op->newline() << "const char *hwbkpt_symbol_name = addr ? NULL : sdp->symbol;";
+  s.op->newline() << "#ifdef CONFIG_X86";
+  s.op->newline() << "	hp->info.name = (char *) hwbkpt_symbol_name;";
+  s.op->newline() << "	hp->info.address = (unsigned long) addr;";
+  s.op->newline() << "  hp->info.len = HW_BREAKPOINT_LEN_1;";
+  s.op->newline() << "  switch(sdp->atype)";
+  s.op->newline() << "	{";
+  s.op->newline() << "    case HWBKPT_WRITE:";
+  s.op->newline() << "		  hp->info.type = HW_BREAKPOINT_WRITE;";
+  s.op->newline() << "    case HWBKPT_RW:";
+  s.op->newline() << "		  hp->info.type = HW_BREAKPOINT_RW;";
+  s.op->newline() << "  }";
+  s.op->newline() << "#endif /*CONFIG_X86*/";
+  s.op->newline() << "hp->triggered = (void *)&enter_hwbkpt_probe;";
+  s.op->newline() << "probe_point = sdp->pp;"; // for error messages
+  s.op->newline() << "rc = register_kernel_hw_breakpoint(hp);";
+  s.op->newline() << "if (rc < 0) {";
+  s.op->newline() << "    sdp->registered_p = 0;";
+  s.op->newline(1) << "	  _stp_warn(\" Hwbkpt Probe %s : Registration error (rc = %d), Addr = %u, name = %s\",probe_point,rc,hp->info.address,hp->info.name);";
+  s.op->newline(-1) << "}";
+  s.op->newline() << "else sdp->registered_p = 1;";
+  s.op->newline(-1) << "}"; // for loop
+}
+
+void
+hwbkpt_derived_probe_group::emit_module_exit (systemtap_session& s)
+{
+  //Unregister hwbkpt probes by batch interfaces.
+  s.op->newline() << "for (i=0; i<" << probes_by_module.size() << "; i++) {";
+  s.op->newline(1) << "struct stap_hwbkpt_probe *sdp = & stap_hwbkpt_probes[i];";
+  s.op->newline() << "struct hw_breakpoint *hp = & stap_hwbkpt_probe_array[i];";
+  s.op->newline() << "if (! sdp->registered_p) continue;";
+  s.op->newline() << " unregister_kernel_hw_breakpoint(hp);";
+  s.op->newline() << "sdp->registered_p = 0;";
+  s.op->newline(-1) << "}";
+}
+
+struct hwbkpt_builder: public derived_probe_builder
+{
+  hwbkpt_builder() {}
+  virtual void build(systemtap_session & sess,
+		     probe * base,
+		     probe_point * location,
+		     literal_map_t const & parameters,
+		     vector<derived_probe *> & finished_results);
+};
+
+void
+hwbkpt_builder::build(systemtap_session & sess,
+		      probe * base,
+		      probe_point * location,
+		      literal_map_t const & parameters,
+		      vector<derived_probe *> & finished_results)
+{
+  string symbol_str_val;
+  int64_t hwbkpt_address;
+  bool has_addr, has_symbol_str, has_write, has_rw;
+
+  has_addr = get_param (parameters, TOK_HWBKPT, hwbkpt_address);
+  has_symbol_str = get_param (parameters, TOK_HWBKPT, symbol_str_val);
+  has_write = (parameters.find(TOK_HWBKPT_WRITE) != parameters.end());
+  has_rw = (parameters.find(TOK_HWBKPT_RW) != parameters.end());
+
+  if (has_addr)
+      finished_results.push_back (new hwbkpt_derived_probe (base,
+							    location,
+							    hwbkpt_address,
+							    "",0,
+							    has_write,
+							    has_rw));
+  else // has symbol_str
+      finished_results.push_back (new hwbkpt_derived_probe (base,
+							    location,
+							    0,
+							    symbol_str_val,0,
+							    has_write,
+							    has_rw));
+}
+
 // ------------------------------------------------------------------------
 // statically inserted kernel-tracepoint derived probes
 // ------------------------------------------------------------------------
@@ -5487,7 +5802,7 @@ tracepoint_derived_probe_group::emit_mod
         }
       s.op->line() << ") {";
       s.op->indent(1);
-      common_probe_entryfn_prologue (s.op, "STAP_SESSION_RUNNING", 
+      common_probe_entryfn_prologue (s.op, "STAP_SESSION_RUNNING",
                                      lex_cast_qstring (*p->sole_location()));
       s.op->newline() << "c->marker_name = "
                       << lex_cast_qstring (p->tracepoint_name)
@@ -5732,7 +6047,6 @@ tracepoint_builder::init_dw(systemtap_se
   return true;
 }
 
-
 void
 tracepoint_builder::build(systemtap_session& s,
                           probe *base, probe_point *location,
@@ -5750,7 +6064,6 @@ tracepoint_builder::build(systemtap_sess
 }
 
 
-
 // ------------------------------------------------------------------------
 //  Standard tapset registry.
 // ------------------------------------------------------------------------
@@ -5798,6 +6111,16 @@ register_standard_tapsets(systemtap_sess
      ->bind_num(TOK_MAXACTIVE)->bind(new kprobe_builder());
   s.pattern_root->bind(TOK_KPROBE)->bind_num(TOK_STATEMENT)
       ->bind(TOK_ABSOLUTE)->bind(new kprobe_builder());
+
+  //Hwbkpt based probe
+  s.pattern_root->bind(TOK_KERNEL)->bind_num(TOK_HWBKPT)
+	->bind(TOK_HWBKPT_WRITE)->bind(new hwbkpt_builder());
+  s.pattern_root->bind(TOK_KERNEL)->bind_str(TOK_HWBKPT)
+	->bind(TOK_HWBKPT_WRITE)->bind(new hwbkpt_builder());
+  s.pattern_root->bind(TOK_KERNEL)->bind_num(TOK_HWBKPT)
+	->bind(TOK_HWBKPT_RW)->bind(new hwbkpt_builder());
+  s.pattern_root->bind(TOK_KERNEL)->bind_str(TOK_HWBKPT)
+	->bind(TOK_HWBKPT_RW)->bind(new hwbkpt_builder());
 }
 
 
@@ -5824,6 +6147,7 @@ all_session_groups(systemtap_session& s)
   DOONE(mark);
   DOONE(tracepoint);
   DOONE(kprobe);
+  DOONE(hwbkpt);
   DOONE(hrtimer);
   DOONE(perfmon);
   DOONE(procfs);

[-- Attachment #3: hwbkpt-2.patch --]
[-- Type: text/x-diff, Size: 851 bytes --]

Index: git-3-july/session.h
===================================================================
--- git-3-july.orig/session.h
+++ git-3-july/session.h
@@ -31,6 +31,7 @@ struct derived_probe;
 struct be_derived_probe_group;
 struct dwarf_derived_probe_group;
 struct kprobe_derived_probe_group;
+struct hwbkpt_derived_probe_group;
 struct uprobe_derived_probe_group;
 struct utrace_derived_probe_group;
 struct itrace_derived_probe_group;
@@ -173,6 +174,7 @@ struct systemtap_session
   be_derived_probe_group* be_derived_probes;
   dwarf_derived_probe_group* dwarf_derived_probes;
   kprobe_derived_probe_group* kprobe_derived_probes;
+  hwbkpt_derived_probe_group* hwbkpt_derived_probes;
   uprobe_derived_probe_group* uprobe_derived_probes;
   utrace_derived_probe_group* utrace_derived_probes;
   itrace_derived_probe_group* itrace_derived_probes;

[-- Attachment #4: hwbkpt-3.patch --]
[-- Type: text/x-diff, Size: 424 bytes --]

Index: git-3-july/elaborate.cxx
===================================================================
--- git-3-july.orig/elaborate.cxx
+++ git-3-july/elaborate.cxx
@@ -1457,6 +1457,7 @@ systemtap_session::systemtap_session ():
   be_derived_probes(0),
   dwarf_derived_probes(0),
   kprobe_derived_probes(0),
+  hwbkpt_derived_probes(0),
   uprobe_derived_probes(0),
   utrace_derived_probes(0),
   itrace_derived_probes(0),

Re: [RFC] Hardware Breakpoint support for systemtap translator

[Prerna Saxena]

Prerna Saxena wrote:
> Hi,
> The attached set patches enable systemtap translator to probe kernel 
> hardware breakpoints on x86.
> It introduces the following language constructs :
>
> 1. probe kernel.data(ADDRESS).write     {....}   : To probe writes at 
> the given address
> 2. probe kernel.data(ADDRESS).rw         {....}   : To probe read & 
> write access to the given address.
> 3. probe kernel.data("SYMBOL").write   {....}
> 4. probe kernel.data("SYMBOL").rw       {....}   : Similar to 1,2, but 
> using a symbol name as argument.
>
> These are agnostic to dwarf/debuginfo and depend on the hardware 
> breakpoint infrastructure for resolving symbols.
>
> Things that remain to be done :
> 1. Translator support for probing hardware breakpoints on other 
> architectures (ppc/s390 etc)
> 2. Implementation of userspace hardware breakpoints.
>
> I'm still testing it...Looking for comments and feedback :-)
>
Forgot to mention that the underlying kernel infrastructure is in -tip. 
Would advise ppl to try these patches on systems running -tip kernel 
only :-)

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India 

Re: [RFC] Hardware Breakpoint support for systemtap translator

[Frank Ch. Eigler]

Prerna Saxena <prerna@linux.vnet.ibm.com> writes:

> [...]
> The attached set patches enable systemtap translator to probe kernel
> hardware breakpoints on x86.

Great.

> It introduces the following language constructs :
>
> 1. probe kernel.data(ADDRESS).write     {....}   : To probe writes at
> the given address
> 2. probe kernel.data(ADDRESS).rw         {....}   : To probe read &
> write access to the given address.
> 3. probe kernel.data("SYMBOL").write   {....}
> 4. probe kernel.data("SYMBOL").rw       {....}   : Similar to 1,2, but
> using a symbol name as argument.

These symbols are in the unified kernel+module namespace, right?


> [...]
> Index: git-3-july/tapsets.cxx
> +struct hwbkpt_derived_probe_group: public derived_probe_group
> +{
> +private:
> +
> +  multimap<string, hwbkpt_derived_probe*> probes_by_module;
> +  typedef map<string, hwbkpt_derived_probe*>::iterator p_b_m_iterator;

Do you need this multimap stuff?  In dwarfy ones we use them to
separate derived_probes by module, so that they can be treated as a
group at run time (as modules come and go).  But in your case, you
just iterate through the whole suite, so why not a plain vector<> or
set<> ?

> [...]
> +void hwbkpt_derived_probe::printsig (ostream& o) const
> +{
> +  sole_location()->print (o);
> +  o << " /* " << " Address = " << hwbkpt_addr << "*/";
> +  o << " /* " << " Symbol  = " << symbol_name << "*/";
> +  switch (hwbkpt_access)
> +	{
> +	  case HWBKPT_READ:
> +  		o << " /* " << " Access = READ " << "*/";
> +	  case HWBKPT_WRITE:
> +  		o << " /* " << " Access = WRITE " << "*/";
> +	  case HWBKPT_RW:
> +  		o << " /* " << " Access = RW " << "*/";
> +	}
> +  printsig_nested (o);
> +}

Is this extra /* .. */ verbosity necessary?  I'd think the
sole_location() printing will spell out the probe point in all the
same detail.  Dwarfy ones have extra data there to provide addresses
etc. that systemtap computed.  That does not seem to apply here.


> +void hwbkpt_derived_probe_group::enroll (hwbkpt_derived_probe* p)
> +{
> +  string hwbkpt_key;
> +  if (p->hwbkpt_addr)
> +	{
> +	  char hwbkpt_addr_str[64];
> +	  sprintf(hwbkpt_addr_str, "%u",int(p->hwbkpt_addr));
> +	  hwbkpt_key=hwbkpt_addr_str;
> +	}
> +  else
> +	  hwbkpt_key=p->symbol_name;
> +  switch (p->hwbkpt_access)
> +	{
> +	  case HWBKPT_READ:
> +		hwbkpt_key = string("R_") + hwbkpt_key;
> +	  case HWBKPT_WRITE:
> +		hwbkpt_key = string("W_") + hwbkpt_key;
> +	  case HWBKPT_RW:
> +		hwbkpt_key = string("RW") + hwbkpt_key;
> +	}
> +
> +  probes_by_module.insert (make_pair (hwbkpt_key, p));
> +}

What is the need for this?  (Was it solely to use the multimap<> ?)


> +void
> +hwbkpt_derived_probe_group::emit_module_decls (systemtap_session& s)
> +{
> +  if (probes_by_module.empty()) return;
> +
> +  s.op->newline() << "/* ---- hwbkpt-based probes ---- */";
> +  s.op->newline() << "#include <asm/hw_breakpoint.h>";
> +  s.op->newline();

It would be useful to make this break compilation more abruptly
if hw-breakpoint support for the current architecture is not available.
Whether a macro comes from a runtime/autoconf* or from a kernel header,
consider adding a ...

#ifndef HAVE_HW_BREAKPOINTS
#error "need hw breakpoints"
#endif

... at the top.


> [...]
> +void
> +hwbkpt_derived_probe_group::emit_module_init (systemtap_session& s)
> +{
> [...]
> +  s.op->newline() << "#ifdef CONFIG_X86";
> +  s.op->newline() << "	hp->info.name = (char *) hwbkpt_symbol_name;";
> +  s.op->newline() << "	hp->info.address = (unsigned long) addr;";

... and then you wouldn't need CONFIG_X86 here.

> +  s.op->newline() << "    case HWBKPT_WRITE:";
> +  s.op->newline() << "		  hp->info.type = HW_BREAKPOINT_WRITE;";

Is there some reason stap doesn't just emit the hw_breakpoint.h enum
names directly, a la HW_BREAKPOINT_WRITE, instead of the handmade
HWBKPT_WRITE / literal-numbers?


> +void
> +hwbkpt_derived_probe_group::emit_module_exit (systemtap_session& s)
> +{
> +  //Unregister hwbkpt probes by batch interfaces.
> +  s.op->newline() << "for (i=0; i<" << probes_by_module.size() << "; i++) {";
> +  s.op->newline(1) << "struct stap_hwbkpt_probe *sdp = & stap_hwbkpt_probes[i];";
> +  s.op->newline() << "struct hw_breakpoint *hp = & stap_hwbkpt_probe_array[i];";
> +  s.op->newline() << "if (! sdp->registered_p) continue;";
> +  s.op->newline() << " unregister_kernel_hw_breakpoint(hp);";
> +  s.op->newline() << "sdp->registered_p = 0;";
> +  s.op->newline(-1) << "}";
> +}

Actually this is not a batch unregistration (one call unregistering
many hw-breakpoints) at all; the comment should be fixed.


> Index: git-3-july/elaborate.cxx
> ===================================================================
> --- git-3-july.orig/elaborate.cxx
> +++ git-3-july/elaborate.cxx
> @@ -1457,6 +1457,7 @@ systemtap_session::systemtap_session ():
>    be_derived_probes(0),
>    dwarf_derived_probes(0),
>    kprobe_derived_probes(0),
> +  hwbkpt_derived_probes(0),
>    uprobe_derived_probes(0),
>    utrace_derived_probes(0),
>    itrace_derived_probes(0),

Thank you for remembering to clear that new pointer!


- FChE

Re: [RFC] Hardware Breakpoint support for systemtap translator

[Roland McGrath]

> 1. probe kernel.data(ADDRESS).write     {....}   : To probe writes at 
> the given address
> 2. probe kernel.data(ADDRESS).rw         {....}   : To probe read & 
> write access to the given address.
> 3. probe kernel.data("SYMBOL").write   {....}
> 4. probe kernel.data("SYMBOL").rw       {....}   : Similar to 1,2, but 
> using a symbol name as argument.

That is a nice start!  But what about dynamic uses?
i.e.

	probe kernel.function("foobar")
	{
	  w1 = watch $foo->bar
	}

	probe kernel.function.return("foobar")
	{
	  unwatch w1
	}

	probe watch.w1
	{
	  val = $$watch.baz # i.e. $foo->bar.baz
	}

or probably some entirely different syntax.  But I think you get the idea:
enable/disable dynamically-discovered addresses to trigger watchpoint
probes of some sort.  I think it would be really nice if the watch probe
could somehow be embedded in the enabling probe's context so it can use
$bar there and have those address translations taken at enable-time.

Also, what about specifying access-size for the watchpoint, on machines
where that can be done (i.e. x86)?


Thanks,
Roland

Re: [RFC] Hardware Breakpoint support for systemtap translator

[Prerna Saxena]

[-- Attachment #1: Type: text/plain, Size: 7168 bytes --]

Hi Frank,
Thanks for the feedback..Here's the updated set of patches based on your 
suggestions.
These only support x86 at present, and can be tested against the -tip 
kernel.

Frank Ch. Eigler wrote:
> Prerna Saxena <prerna@linux.vnet.ibm.com> writes:
>
>   
>> [...]
>> The attached set patches enable systemtap translator to probe kernel
>> hardware breakpoints on x86.
>>     
>
> Great.
>
>   
>> It introduces the following language constructs :
>>
>> 1. probe kernel.data(ADDRESS).write     {....}   : To probe writes at
>> the given address
>> 2. probe kernel.data(ADDRESS).rw         {....}   : To probe read &
>> write access to the given address.
>> 3. probe kernel.data("SYMBOL").write   {....}
>> 4. probe kernel.data("SYMBOL").rw       {....}   : Similar to 1,2, but
>> using a symbol name as argument.
>>     
>
> These symbols are in the unified kernel+module namespace, right?
>   
Yes.
>
>   
>> [...]
>> Index: git-3-july/tapsets.cxx
>> +struct hwbkpt_derived_probe_group: public derived_probe_group
>> +{
>> +private:
>> +
>> +  multimap<string, hwbkpt_derived_probe*> probes_by_module;
>> +  typedef map<string, hwbkpt_derived_probe*>::iterator p_b_m_iterator;
>>     
>
> Do you need this multimap stuff?  In dwarfy ones we use them to
> separate derived_probes by module, so that they can be treated as a
> group at run time (as modules come and go).  But in your case, you
> just iterate through the whole suite, so why not a plain vector<> or
> set<> ?
>
>   
Agree, using multimaps here is an overkill. The latest set of patches 
(attached) implement this as a vector of probes.
>> [...]
>> +void hwbkpt_derived_probe::printsig (ostream& o) const
>> +{
>> +  sole_location()->print (o);
>> +  o << " /* " << " Address = " << hwbkpt_addr << "*/";
>> +  o << " /* " << " Symbol  = " << symbol_name << "*/";
>> +  switch (hwbkpt_access)
>> +	{
>> +	  case HWBKPT_READ:
>> +  		o << " /* " << " Access = READ " << "*/";
>> +	  case HWBKPT_WRITE:
>> +  		o << " /* " << " Access = WRITE " << "*/";
>> +	  case HWBKPT_RW:
>> +  		o << " /* " << " Access = RW " << "*/";
>> +	}
>> +  printsig_nested (o);
>> +}
>>     
>
> Is this extra /* .. */ verbosity necessary?  I'd think the
> sole_location() printing will spell out the probe point in all the
> same detail.  Dwarfy ones have extra data there to provide addresses
> etc. that systemtap computed.  That does not seem to apply here.
>   
Thanks for pointing this out. You are right, systemtap translator doesnt 
compute any addresses for kernel hardware breakpoints so this isnt needed.
This verbosity would be required while setting userspace hardware 
breakpoints, if symbol-name arguments are supported. (coz one would need 
dwarf to lookup addresses for symbolic arguments)
Removed for now..
>
>   
>> +void hwbkpt_derived_probe_group::enroll (hwbkpt_derived_probe* p)
>> +{
>> +  string hwbkpt_key;
>> +  if (p->hwbkpt_addr)
>> +	{
>> +	  char hwbkpt_addr_str[64];
>> +	  sprintf(hwbkpt_addr_str, "%u",int(p->hwbkpt_addr));
>> +	  hwbkpt_key=hwbkpt_addr_str;
>> +	}
>> +  else
>> +	  hwbkpt_key=p->symbol_name;
>> +  switch (p->hwbkpt_access)
>> +	{
>> +	  case HWBKPT_READ:
>> +		hwbkpt_key = string("R_") + hwbkpt_key;
>> +	  case HWBKPT_WRITE:
>> +		hwbkpt_key = string("W_") + hwbkpt_key;
>> +	  case HWBKPT_RW:
>> +		hwbkpt_key = string("RW") + hwbkpt_key;
>> +	}
>> +
>> +  probes_by_module.insert (make_pair (hwbkpt_key, p));
>> +}
>>     
>
> What is the need for this?  (Was it solely to use the multimap<> ?)
>
>
>   
Yes, this was for multimap implementation. Cleaned up now. :-)
>> +void
>> +hwbkpt_derived_probe_group::emit_module_decls (systemtap_session& s)
>> +{
>> +  if (probes_by_module.empty()) return;
>> +
>> +  s.op->newline() << "/* ---- hwbkpt-based probes ---- */";
>> +  s.op->newline() << "#include <asm/hw_breakpoint.h>";
>> +  s.op->newline();
>>     
>
> It would be useful to make this break compilation more abruptly
> if hw-breakpoint support for the current architecture is not available.
> Whether a macro comes from a runtime/autoconf* or from a kernel header,
> consider adding a ...
>
> #ifndef HAVE_HW_BREAKPOINTS
> #error "need hw breakpoints"
> #endif
>
> ... at the top.
>   
Nice suggestion, I've included this check now.
>
>   
>> [...]
>> +void
>> +hwbkpt_derived_probe_group::emit_module_init (systemtap_session& s)
>> +{
>> [...]
>> +  s.op->newline() << "#ifdef CONFIG_X86";
>> +  s.op->newline() << "	hp->info.name = (char *) hwbkpt_symbol_name;";
>> +  s.op->newline() << "	hp->info.address = (unsigned long) addr;";
>>     
>
> ... and then you wouldn't need CONFIG_X86 here.
>   
Hardware Breakpoint functionality varies widely across architectures. On 
x86, hardware flags WRITE/ RW access to a given address, while on ppc, 
probing of READ / WRITE on a given address is flagged.
As of now, probing just READ access to a given address is not supported 
by x86 but is available on power architecture.
Morover, lengths of data addresses that can be probed also differ widely 
across arch. Hence the systemtap-generated module needs to make 
arch-dependent choices at runtime.
>   
>> +  s.op->newline() << "    case HWBKPT_WRITE:";
>> +  s.op->newline() << "		  hp->info.type = HW_BREAKPOINT_WRITE;";
>>     
>
> Is there some reason stap doesn't just emit the hw_breakpoint.h enum
> names directly, a la HW_BREAKPOINT_WRITE, instead of the handmade
> HWBKPT_WRITE / literal-numbers?
>
>   
As explained above, hardware breakpoint for a specific type of access 
that can be set up for a given address varies across architectures. This 
approach enables the stap module to decide at runtime if the requested 
access(read/write/rw) can be supported by the platform.
>   
>> +void
>> +hwbkpt_derived_probe_group::emit_module_exit (systemtap_session& s)
>> +{
>> +  //Unregister hwbkpt probes by batch interfaces.
>> +  s.op->newline() << "for (i=0; i<" << probes_by_module.size() << "; i++) {";
>> +  s.op->newline(1) << "struct stap_hwbkpt_probe *sdp = & stap_hwbkpt_probes[i];";
>> +  s.op->newline() << "struct hw_breakpoint *hp = & stap_hwbkpt_probe_array[i];";
>> +  s.op->newline() << "if (! sdp->registered_p) continue;";
>> +  s.op->newline() << " unregister_kernel_hw_breakpoint(hp);";
>> +  s.op->newline() << "sdp->registered_p = 0;";
>> +  s.op->newline(-1) << "}";
>> +}
>>     
>
> Actually this is not a batch unregistration (one call unregistering
> many hw-breakpoints) at all; the comment should be fixed.
>
>
>   
Done, thanks for pointing it out :-)
>> Index: git-3-july/elaborate.cxx
>> ===================================================================
>> --- git-3-july.orig/elaborate.cxx
>> +++ git-3-july/elaborate.cxx
>> @@ -1457,6 +1457,7 @@ systemtap_session::systemtap_session ():
>>    be_derived_probes(0),
>>    dwarf_derived_probes(0),
>>    kprobe_derived_probes(0),
>> +  hwbkpt_derived_probes(0),
>>    uprobe_derived_probes(0),
>>    utrace_derived_probes(0),
>>    itrace_derived_probes(0),
>>     
>
> Thank you for remembering to clear that new pointer!
>
>
> - FChE
>   
Awaiting feedback / comments,

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India 


[-- Attachment #2: hwbkpt-1.patch --]
[-- Type: text/x-diff, Size: 13192 bytes --]

Index: git-3-july/tapsets.cxx
===================================================================
--- git-3-july.orig/tapsets.cxx
+++ git-3-july/tapsets.cxx
@@ -4929,7 +4929,302 @@ kprobe_builder::build(systemtap_session 
     }
 }
 
+// ------------------------------------------------------------------------
+//  Hardware breakpoint based probes.
+// ------------------------------------------------------------------------
+
+static const string TOK_HWBKPT("data");
+static const string TOK_HWBKPT_WRITE("write");
+static const string TOK_HWBKPT_RW("rw");
+
+#define HWBKPT_READ 0
+#define HWBKPT_WRITE 1
+#define HWBKPT_RW 2
+struct hwbkpt_derived_probe: public derived_probe
+{
+  hwbkpt_derived_probe (probe *base,
+                        probe_point *location,
+                        int64_t addr,
+			string symname,
+			bool has_only_read_access,
+			bool has_only_write_access,
+			bool has_rw_access
+                        );
+  Dwarf_Addr hwbkpt_addr;
+  string symbol_name;
+  unsigned int hwbkpt_access;
+
+  void printsig (std::ostream &o) const;
+  void join_group (systemtap_session& s);
+};
+
+struct hwbkpt_derived_probe_group: public derived_probe_group
+{
+private:
+
+  vector<hwbkpt_derived_probe*> hwbkpt_probes_vector;
+  typedef vector<hwbkpt_derived_probe*>::iterator h_p_v_iterator;
+
+public:
+  void enroll (hwbkpt_derived_probe* probe);
+  void emit_module_decls (systemtap_session& s);
+  void emit_module_init (systemtap_session& s);
+  void emit_module_exit (systemtap_session& s);
+};
+
+hwbkpt_derived_probe::hwbkpt_derived_probe (probe *base,
+					    probe_point *location,
+					    int64_t addr,
+					    string symname,
+					    bool has_only_read_access,
+					    bool has_only_write_access,
+					    bool has_rw_access
+					    ):
+  derived_probe (base, location),
+  hwbkpt_addr (addr),
+  symbol_name (symname)
+{
+  this->tok = base->tok;
+
+  vector<probe_point::component*> comps;
+  comps.push_back (new probe_point::component(TOK_KERNEL));
+
+  if (hwbkpt_addr)
+	  comps.push_back (new probe_point::component (TOK_HWBKPT, new literal_number(hwbkpt_addr)));
+  else
+	if (symbol_name.size())
+	  comps.push_back (new probe_point::component (TOK_HWBKPT, new literal_string(symbol_name)));
+
+  if (has_only_read_access)
+	this->hwbkpt_access = HWBKPT_READ ;
+//TODO add code for comps.push_back for read, since this flag is not for x86
+
+  else
+	{
+	  if (has_only_write_access)
+		{
+		  this->hwbkpt_access = HWBKPT_WRITE ;
+	  	  comps.push_back (new probe_point::component(TOK_HWBKPT_WRITE));
+		}
+	  else
+		{
+		  this->hwbkpt_access = HWBKPT_RW ;
+	  	  comps.push_back (new probe_point::component(TOK_HWBKPT_RW));
+		}
+	}
+
+  this->sole_location()->components = comps;
+}
+
+void hwbkpt_derived_probe::printsig (ostream& o) const
+{
+  sole_location()->print (o);
+  printsig_nested (o);
+}
+
+void hwbkpt_derived_probe::join_group (systemtap_session& s)
+{
+  if (! s.hwbkpt_derived_probes)
+        s.hwbkpt_derived_probes = new hwbkpt_derived_probe_group ();
+  s.hwbkpt_derived_probes->enroll (this);
+}
+
+void hwbkpt_derived_probe_group::enroll (hwbkpt_derived_probe* p)
+{
+  hwbkpt_probes_vector.push_back (p);
+}
+
+void
+hwbkpt_derived_probe_group::emit_module_decls (systemtap_session& s)
+{
+  if (hwbkpt_probes_vector.empty()) return;
+
+  s.op->newline() << "/* ---- hwbkpt-based probes ---- */";
+
+  // Warn of misconfigured kernels
+  s.op->newline() << "#ifndef CONFIG_HAVE_HW_BREAKPOINT";
+  s.op->newline() << "#error \"Need CONFIG_HAVE_HW_BREAKPOINT!\"";
+  s.op->newline() << "#endif";
+
+  s.op->newline() << "#include <asm/hw_breakpoint.h>";
+  s.op->newline();
+
+// Define macros for access type
+  s.op->newline() << "#define HWBKPT_READ 0";
+  s.op->newline() << "#define HWBKPT_WRITE 1";
+  s.op->newline() << "#define HWBKPT_RW 2";
+
+  // Forward declare the master entry functions
+  s.op->newline() << "static int enter_hwbkpt_probe (struct hw_breakpoint *inst,";
+  s.op->line() << " struct pt_regs *regs);";
+
+  // Emit the actual probe list.
+
+  s.op->newline() << "static struct hw_breakpoint ";
+  s.op->newline() << "stap_hwbkpt_probe_array[" << hwbkpt_probes_vector.size() << "];";
+
+  s.op->newline() << "static struct stap_hwbkpt_probe {";
+  s.op->newline() << "int registered_p:1;";
+// registered_p = -1 signifies an invalid probe that must not be registered
+// registered_p =  0 signifies a probe that failed registration
+// registered_p =  1 signifies a probe that got registered successfully
+
+  // Probe point & Symbol Names are mostly small and uniform enough
+  // to justify putting char[MAX]'s into  the array instead of
+  // relocated char*'s.
+
+  size_t pp_name_max = 0 , symbol_name_max = 0;
+  size_t pp_name_tot = 0 , symbol_name_tot = 0;
+  for (unsigned int it = 0; it < hwbkpt_probes_vector.size(); it++)
+    {
+      hwbkpt_derived_probe* p = hwbkpt_probes_vector.at(it);
+#define DOIT(var,expr) do {                             \
+        size_t var##_size = (expr) + 1;                 \
+        var##_max = max (var##_max, var##_size);        \
+        var##_tot += var##_size; } while (0)
+      DOIT(pp_name, lex_cast_qstring(*p->sole_location()).size());
+      DOIT(symbol_name, p->symbol_name.size());
+#undef DOIT
+    }
+
+#define CALCIT(var)                                                     \
+  s.op->newline() << "const char " << #var << "[" << var##_name_max << "] ;";
+  CALCIT(pp);
+  CALCIT(symbol);
+#undef CALCIT
+
+  s.op->newline() << "const unsigned long address;";
+  s.op->newline() << "uint8_t atype;";
+  s.op->newline() << "void (* const ph) (struct context*);";
+  s.op->newline() << "} stap_hwbkpt_probes[] = {";
+  s.op->indent(1);
+
+  for (unsigned int it = 0; it < hwbkpt_probes_vector.size(); it++)
+    {
+      hwbkpt_derived_probe* p = hwbkpt_probes_vector.at(it);
+      s.op->newline() << "{";
+      if (p->symbol_name.size())
+      s.op->line() << " .address=(unsigned long)0x0" << "ULL,";
+      else
+      s.op->line() << " .address=(unsigned long)0x" << hex << p->hwbkpt_addr << dec << "ULL,";
+      s.op->line() << " .atype=" << p->hwbkpt_access << ",";
+      s.op->line() << " .pp=" << lex_cast_qstring (*p->sole_location()) << ",";
+      s.op->line() << " .symbol=\"" << p->symbol_name << "\",";
+      s.op->line() << " .ph=&" << p->name << "";
+      s.op->line() << " },";
+    }
+  s.op->newline(-1) << "};";
+
+  // Emit the hwbkpt callback function
+  s.op->newline();
+  s.op->newline() << "static int enter_hwbkpt_probe (struct hw_breakpoint *inst,";
+  s.op->line() << " struct pt_regs *regs) {";
+  s.op->newline(1) << "int hwbkpt_probe_idx = ((uintptr_t)inst-(uintptr_t)stap_hwbkpt_probe_array)/sizeof(struct hw_breakpoint);";
+
+  // Check that the index is plausible
+  s.op->newline() << "struct stap_hwbkpt_probe *sdp = &stap_hwbkpt_probes[";
+  s.op->line() << "((hwbkpt_probe_idx >= 0 && hwbkpt_probe_idx < " << hwbkpt_probes_vector.size() << ")?";
+  s.op->line() << "hwbkpt_probe_idx:0)"; // NB: at least we avoid memory corruption
+  s.op->line() << "];";
+  common_probe_entryfn_prologue (s.op, "STAP_SESSION_RUNNING", "sdp->pp");
+  s.op->newline() << "c->regs = regs;";
+  s.op->newline() << "(*sdp->ph) (c);";
+  common_probe_entryfn_epilogue (s.op);
+  s.op->newline() << "return 0;";
+  s.op->newline(-1) << "}";
+}
+
+void
+hwbkpt_derived_probe_group::emit_module_init (systemtap_session& s)
+{
+  s.op->newline() << "for (i=0; i<" << hwbkpt_probes_vector.size() << "; i++) {";
+  s.op->newline(1) << "struct stap_hwbkpt_probe *sdp = & stap_hwbkpt_probes[i];";
+  s.op->newline() << "struct hw_breakpoint *hp = & stap_hwbkpt_probe_array[i];";
+  s.op->newline() << "void *addr = (void *) sdp->address;";
+  s.op->newline() << "const char *hwbkpt_symbol_name = addr ? NULL : sdp->symbol;";
+  s.op->newline() << "	hp->info.name = (char *) hwbkpt_symbol_name;";
+  s.op->newline() << "	hp->info.address = (unsigned long) addr;";
+  s.op->newline() << "#ifdef CONFIG_X86";
+  s.op->newline() << "  hp->info.len = HW_BREAKPOINT_LEN_1;";
+  s.op->newline() << "  switch(sdp->atype)";
+  s.op->newline() << "	{";
+  s.op->newline() << "    case HWBKPT_READ:";
+  s.op->newline() << "  	  _stp_warn(\"Probe %s registration skipped : READ-ONLY hardware breakpoints not supported on x86\",sdp->pp);";
+  s.op->newline() << "		  sdp->registered_p = -1;";
+  s.op->newline() << "    case HWBKPT_WRITE:";
+  s.op->newline() << "		hp->info.type = HW_BREAKPOINT_WRITE;";
+  s.op->newline() << "    case HWBKPT_RW:";
+  s.op->newline() << "		hp->info.type = HW_BREAKPOINT_RW;";
+  s.op->newline() << "  }";
+  s.op->newline() << "#endif /*CONFIG_X86*/";
+  s.op->newline() << "hp->triggered = (void *)&enter_hwbkpt_probe;";
+  s.op->newline() << "probe_point = sdp->pp;"; // for error messages
+  s.op->newline() << "if (sdp->registered_p >= 0)";
+  s.op->newline() << "{";
+  s.op->newline() << "   rc = register_kernel_hw_breakpoint(hp);";
+  s.op->newline() << "   if (rc < 0) {";
+  s.op->newline() << "      sdp->registered_p = 0;";
+  s.op->newline(1) << "	    _stp_warn(\" Hwbkpt Probe %s : Registration error (rc = %d), Addr = %u, name = %s\",probe_point,rc,hp->info.address,hp->info.name);";
+  s.op->newline(-1) << "  }";
+  s.op->newline() << "    else sdp->registered_p = 1;";
+  s.op->newline() << "}";
+  s.op->newline(-1) << "}"; // for loop
+}
 
+void
+hwbkpt_derived_probe_group::emit_module_exit (systemtap_session& s)
+{
+  //Unregister hwbkpt probes.
+  s.op->newline() << "for (i=0; i<" << hwbkpt_probes_vector.size() << "; i++) {";
+  s.op->newline(1) << "struct stap_hwbkpt_probe *sdp = & stap_hwbkpt_probes[i];";
+  s.op->newline() << "struct hw_breakpoint *hp = & stap_hwbkpt_probe_array[i];";
+  s.op->newline() << "if (! sdp->registered_p) continue;";
+  s.op->newline() << " unregister_kernel_hw_breakpoint(hp);";
+  s.op->newline() << "sdp->registered_p = 0;";
+  s.op->newline(-1) << "}";
+}
+
+struct hwbkpt_builder: public derived_probe_builder
+{
+  hwbkpt_builder() {}
+  virtual void build(systemtap_session & sess,
+		     probe * base,
+		     probe_point * location,
+		     literal_map_t const & parameters,
+		     vector<derived_probe *> & finished_results);
+};
+
+void
+hwbkpt_builder::build(systemtap_session & sess,
+		      probe * base,
+		      probe_point * location,
+		      literal_map_t const & parameters,
+		      vector<derived_probe *> & finished_results)
+{
+  string symbol_str_val;
+  int64_t hwbkpt_address;
+  bool has_addr, has_symbol_str, has_write, has_rw;
+
+  has_addr = get_param (parameters, TOK_HWBKPT, hwbkpt_address);
+  has_symbol_str = get_param (parameters, TOK_HWBKPT, symbol_str_val);
+  has_write = (parameters.find(TOK_HWBKPT_WRITE) != parameters.end());
+  has_rw = (parameters.find(TOK_HWBKPT_RW) != parameters.end());
+
+  if (has_addr)
+      finished_results.push_back (new hwbkpt_derived_probe (base,
+							    location,
+							    hwbkpt_address,
+							    "",0,
+							    has_write,
+							    has_rw));
+  else // has symbol_str
+      finished_results.push_back (new hwbkpt_derived_probe (base,
+							    location,
+							    0,
+							    symbol_str_val,0,
+							    has_write,
+							    has_rw));
+}
 
 // ------------------------------------------------------------------------
 // statically inserted kernel-tracepoint derived probes
@@ -5487,7 +5782,7 @@ tracepoint_derived_probe_group::emit_mod
         }
       s.op->line() << ") {";
       s.op->indent(1);
-      common_probe_entryfn_prologue (s.op, "STAP_SESSION_RUNNING", 
+      common_probe_entryfn_prologue (s.op, "STAP_SESSION_RUNNING",
                                      lex_cast_qstring (*p->sole_location()));
       s.op->newline() << "c->marker_name = "
                       << lex_cast_qstring (p->tracepoint_name)
@@ -5732,7 +6027,6 @@ tracepoint_builder::init_dw(systemtap_se
   return true;
 }
 
-
 void
 tracepoint_builder::build(systemtap_session& s,
                           probe *base, probe_point *location,
@@ -5750,7 +6044,6 @@ tracepoint_builder::build(systemtap_sess
 }
 
 
-
 // ------------------------------------------------------------------------
 //  Standard tapset registry.
 // ------------------------------------------------------------------------
@@ -5798,6 +6091,16 @@ register_standard_tapsets(systemtap_sess
      ->bind_num(TOK_MAXACTIVE)->bind(new kprobe_builder());
   s.pattern_root->bind(TOK_KPROBE)->bind_num(TOK_STATEMENT)
       ->bind(TOK_ABSOLUTE)->bind(new kprobe_builder());
+
+  //Hwbkpt based probe
+  s.pattern_root->bind(TOK_KERNEL)->bind_num(TOK_HWBKPT)
+	->bind(TOK_HWBKPT_WRITE)->bind(new hwbkpt_builder());
+  s.pattern_root->bind(TOK_KERNEL)->bind_str(TOK_HWBKPT)
+	->bind(TOK_HWBKPT_WRITE)->bind(new hwbkpt_builder());
+  s.pattern_root->bind(TOK_KERNEL)->bind_num(TOK_HWBKPT)
+	->bind(TOK_HWBKPT_RW)->bind(new hwbkpt_builder());
+  s.pattern_root->bind(TOK_KERNEL)->bind_str(TOK_HWBKPT)
+	->bind(TOK_HWBKPT_RW)->bind(new hwbkpt_builder());
 }
 
 
@@ -5824,6 +6127,7 @@ all_session_groups(systemtap_session& s)
   DOONE(mark);
   DOONE(tracepoint);
   DOONE(kprobe);
+  DOONE(hwbkpt);
   DOONE(hrtimer);
   DOONE(perfmon);
   DOONE(procfs);

[-- Attachment #3: hwbkpt-2.patch --]
[-- Type: text/x-diff, Size: 851 bytes --]

Index: git-3-july/session.h
===================================================================
--- git-3-july.orig/session.h
+++ git-3-july/session.h
@@ -31,6 +31,7 @@ struct derived_probe;
 struct be_derived_probe_group;
 struct dwarf_derived_probe_group;
 struct kprobe_derived_probe_group;
+struct hwbkpt_derived_probe_group;
 struct uprobe_derived_probe_group;
 struct utrace_derived_probe_group;
 struct itrace_derived_probe_group;
@@ -173,6 +174,7 @@ struct systemtap_session
   be_derived_probe_group* be_derived_probes;
   dwarf_derived_probe_group* dwarf_derived_probes;
   kprobe_derived_probe_group* kprobe_derived_probes;
+  hwbkpt_derived_probe_group* hwbkpt_derived_probes;
   uprobe_derived_probe_group* uprobe_derived_probes;
   utrace_derived_probe_group* utrace_derived_probes;
   itrace_derived_probe_group* itrace_derived_probes;

[-- Attachment #4: hwbkpt-3.patch --]
[-- Type: text/x-diff, Size: 424 bytes --]

Index: git-3-july/elaborate.cxx
===================================================================
--- git-3-july.orig/elaborate.cxx
+++ git-3-july/elaborate.cxx
@@ -1457,6 +1457,7 @@ systemtap_session::systemtap_session ():
   be_derived_probes(0),
   dwarf_derived_probes(0),
   kprobe_derived_probes(0),
+  hwbkpt_derived_probes(0),
   uprobe_derived_probes(0),
   utrace_derived_probes(0),
   itrace_derived_probes(0),

Re: [RFC] Hardware Breakpoint support for systemtap translator

[Prerna Saxena]

Hi Roland,

Roland McGrath wrote:
>> 1. probe kernel.data(ADDRESS).write     {....}   : To probe writes at 
>> the given address
>> 2. probe kernel.data(ADDRESS).rw         {....}   : To probe read & 
>> write access to the given address.
>> 3. probe kernel.data("SYMBOL").write   {....}
>> 4. probe kernel.data("SYMBOL").rw       {....}   : Similar to 1,2, but 
>> using a symbol name as argument.
>>     
>
> That is a nice start!  But what about dynamic uses?
> i.e.
>
> 	probe kernel.function("foobar")
> 	{
> 	  w1 = watch $foo->bar
> 	}
>
> 	probe kernel.function.return("foobar")
> 	{
> 	  unwatch w1
> 	}
>
> 	probe watch.w1
> 	{
> 	  val = $$watch.baz # i.e. $foo->bar.baz
> 	}
>
> or probably some entirely different syntax.  But I think you get the idea:
> enable/disable dynamically-discovered addresses to trigger watchpoint
> probes of some sort.  I think it would be really nice if the watch probe
> could somehow be embedded in the enabling probe's context so it can use
> $bar there and have those address translations taken at enable-time.
>
>   

Enabling systemtap to set hardware breakpoints at runtime would be a 
very useful feature to have, but it presents a problem. Kprobe handlers 
get executed in exception context, and it is not possible to register a 
hardware breakpoint from within exception context. I'm wondering how it 
could be implemented..

> Also, what about specifying access-size for the watchpoint, on machines
> where that can be done (i.e. x86)?
>   

Thats a nice suggestion, I'll add this !

>
> Thanks,
> Roland
>   
Thanks,

-- 
Prerna Saxena

Linux Technology Centre,
IBM Systems and Technology Lab,
Bangalore, India 

Re: [RFC] Hardware Breakpoint support for systemtap translator

[Roland McGrath]

> Enabling systemtap to set hardware breakpoints at runtime would be a 
> very useful feature to have, but it presents a problem. Kprobe handlers 
> get executed in exception context, and it is not possible to register a 
> hardware breakpoint from within exception context. I'm wondering how it 
> could be implemented..

Hmm.  That seems like a general systemtap issue for a variety of features
down the line.  There are other things for which dynamic setup can only be
done from a safer process context, such as dynamically enabling various
sorts of user probes.


Thanks,
Roland