[lkml]   [2009]   [Jul]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
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

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

> 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

- Define constants/enums for length and type that are common to all
- 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.


 \ /
  Last update: 2009-07-21 13:17    [W:0.302 / U:0.784 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site