Messages in this thread | | | Date | Sat, 25 Jul 2009 04:37:25 +0200 | From | Frederic Weisbecker <> | Subject | Re: [RFC][PATCH 1/5] hw-breakpoints: Make kernel breakpoints API truly generic |
| |
On Mon, Jul 20, 2009 at 01:27:48PM -0400, Mathieu Desnoyers wrote: > * Frederic Weisbecker (fweisbec@gmail.com) wrote: > [...] > > diff --git a/kernel/hw_breakpoint.c b/kernel/hw_breakpoint.c > > index c1f64e6..015fec6 100644 > > --- a/kernel/hw_breakpoint.c > > +++ b/kernel/hw_breakpoint.c > > @@ -297,15 +297,21 @@ EXPORT_SYMBOL_GPL(unregister_user_hw_breakpoint); > > /** > > * register_kernel_hw_breakpoint - register a hardware breakpoint for kernel space > > * @bp: the breakpoint structure to register > > - * > > - * @bp.info->name or @bp.info->address, @bp.info->len, @bp.info->type and > > + * @addr: the address where we want to set the breakpoint > > + * @len: length of the value in memory to break in > > + * @type: the type of the breakpoint (read/write/execute) > > * @bp->triggered must be set properly before invocation > > Hi Frederic, > > I think one of the great addition in this patchset is to allow using > breakpoints from arch-agnostic code. > > It becomes important to document the error values which can be returned > by register_kernel_hw_breakpoint, so this will serve as guidelines for > architecture-specific arch_fill_hw_breakpoint() implementation. This > will become increasingly important, as this abstraction layer will > basically be responsible for either: > > - Finding the best support the architecture can provide for a given hw > breakpoint.
Indeed, and that's the biggest problem it has to face because supported hardware breakpoint features are very differents from one architecture to another.
> - Failing with an explicit error value telling the in-kernel user why it > failed (e.g. if it must use a fallback, or return the error to the > user).
Yeah, nice point, I'll send another iteration which better documents the error return values, at least once I get a mostly agreed core implementation :-)
> Maybe we should think of a more flexible breakpoint type mapping too, > e.g.: > > monitor _strictly_ execute operation on address 0x... > -> would fail if the architecture does not support execution access > monitoring > monitor (at least) execute operations on address 0x... > -> would be allowed to use a more general monitor (e.g. RWX) if the > architecture does not support "execute only" monitor. > > (same for read and write) > > Mathieu
Well, I'm not sure the problem mostly resides in the hardware implementation of strict exec breakpoint types. But I guess your point is not limiting to that. Yeah for example, x86 doesn't support read-only breakpoints. But I guess that can be simulated using software artifacts, for example using READ-WRITE breakpoints + the x86 decoder API, recently submitted by Masami, to find the nature of the current instruction.
Anyway, your point is indeed important: return common error values for unsupported breakpoint operations.
Thanks.
> > > * > > */ > > -int register_kernel_hw_breakpoint(struct hw_breakpoint *bp) > > +int register_kernel_hw_breakpoint(struct hw_breakpoint *bp, unsigned long addr, > > + int len, enum breakpoint_type type) > > { > > int rc; > > > > + rc = arch_fill_hw_breakpoint(bp, addr, len, type); > > + if (rc) > > + return rc; > > + > > rc = arch_validate_hwbkpt_settings(bp, NULL); > > if (rc) > > return rc; > -- > Mathieu Desnoyers > OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68
| |