lkml.org 
[lkml]   [2009]   [Jul]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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



\
 
 \ /
  Last update: 2009-07-25 04:39    [from the cache]
©2003-2011 Jasper Spaans