lkml.org 
[lkml]   [2009]   [Jul]   [21]   [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:08:03PM -0400, Frederic Weisbecker wrote:
    > To define a kernel hardware breakpoint, one need to define the
    > address, type and length of the breakpoint using arch specific
    > operations and then register it using a core helper.
    >
    > The first stage is truly not scalable with respect to the number of
    > archictures, because for each of them that support hardware
    > breakpoints, we would need a seperate specific field definition for
    > the breakpoint.
    >
    > However, the supported breakpoint functionalities may be very different
    > between architectures.
    > Then this new API tries to compose with the following constraints:
    >
    > - a given architecture may perhaps not support the triggering on one
    > of the usual memory access (read-write/read/write/execute)
    >
    > - a given architecture may perhaps not support the ability to trigger
    > a breakpoint only on specific memory access size lower than the word
    > size for this arch.
    >
    > - a given architecture may perhaps not support breakpoints on addresses
    > range.
    >
    > The new API changes the following prototype for a kernel breakpoint
    > registration:
    >
    > int register_kernel_hw_breakpoint(struct hw_breakpoint *bp)
    >
    > into:
    >
    > int register_kernel_hw_breakpoint(struct hw_breakpoint *bp,
    > unsigned long addr,
    > int len, enum breakpoint_type type)

    It is not clear how adding these new parameters to the interface would
    help it become generic, as opposed to moving them to 'struct
    hw_breakpoint'.

    It would make the usage cumbersome of some architectures - say for
    instance the PPC64 which always has a breakpoint length of 8 bytes. So
    the user needs to specify either '8' always or '0' to indicate variable
    length not supported (but it is counter-intuitive..may be interpreted as
    zero-length).

    >
    > The choice of passing the breakpoint settings as parameters of the
    > registration helper and not by adding generic fields into the breakpoint
    > structure is motivated by the need of a very specific per arch
    > representation of the breakpoint:
    >
    > - the arch may only need an address, but could also need a couple for
    > breakpoints in ranges.
    > - the type is subject to arch interpretation (values of debug registers)
    > - the length too.
    >
    > Then, to get back these values from a generic breakpoint structure that have
    > specific encodings into the arch fields, this API comes along with abstract
    > accessors which implementation is arch specific:
    >
    > - type hw_breakpoint_type(bp)
    > - addr hw_breakpoint_addr(bp)
    >
    > However, open debates come along this RFC patch:
    >
    > - the address could be a generic field in struct hw_breakpoint. If we
    > are dealing with a range breakpoint, then we would just need to
    > compute addr + length to get the end of the range.
    >
    > - the length and type could also be generic fields of
    > struct hw_breakpoint. It would then be up to the arch to get a
    > translation between such generic values and per arch needs.
    >

    While the issues have been enumerated above, the patchset only pushes
    the issue into a different domain i.e. make the user determine if a
    breakpoint type or len is supported in a given architecture vs the existing
    implementation in which the user determines if a constant pertaining to
    a given len/type is defined. But the accessor-routines
    hw_breakpoint_type() and hw_breakpoint_addr() make it much easier to use
    and is a good addition.

    To make the usage much easier, I would see a combination of the
    following:

    - Define constants/enums for length and type that are common to all
    architectures.
    - Define accessor routines that help determine if a given type/len is
    supported on the host processor.
    - Move fields such as address, len and type to generic breakpoint
    structure (if it still matters despite the two changes above).

    Let me know what you think.

    Thanks,
    K.Prasad



    \
     
     \ /
      Last update: 2009-07-21 13:17    [W:0.034 / U:29.760 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site