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-2014 Jasper Spaans. hosted at Digital Ocean