Re: [Patch 01/12] Prepare the code for Hardware Breakpoint interfaces

["K.Prasad" <prasad@linux.vnet.ibm.com>]

On Thu, May 21, 2009 at 09:16:38AM -0700, David Daney wrote:
> K.Prasad wrote:
> [...]
>> +struct hw_breakpoint {
>> +	void (*triggered)(struct hw_breakpoint *, struct pt_regs *);
>> +	struct arch_hw_breakpoint info;
>> +};
>> +/*
>> + * len and type values are defined in include/asm/hw_breakpoint.h.
>> + * Available values vary according to the architecture.  On i386 the
>> + * possibilities are:
>> + *
>> + *	HW_BREAKPOINT_LEN_1
>> + *	HW_BREAKPOINT_LEN_2
>> + *	HW_BREAKPOINT_LEN_4
>> + *	HW_BREAKPOINT_RW
>> + *	HW_BREAKPOINT_READ
>> + *
>> + * On other architectures HW_BREAKPOINT_LEN_8 may be available, and the
>> + * 1-, 2-, and 4-byte lengths may be unavailable.  There also may be
>> + * HW_BREAKPOINT_WRITE.  You can use #ifdef to check at compile time.
>> + */
>> +
>
> I question weather having all these symbols for lengths is the proper  
> approach.
>
> On mips we would currently have:
>
> HW_BREAKPOINT_LEN_8
> HW_BREAKPOINT_LEN_16
> HW_BREAKPOINT_LEN_32
> HW_BREAKPOINT_LEN_64
> HW_BREAKPOINT_LEN_128
> HW_BREAKPOINT_LEN_256
> HW_BREAKPOINT_LEN_512
> HW_BREAKPOINT_LEN_1024
> HW_BREAKPOINT_LEN_2048
>
> If we were to use a debug agent hooked into the MIPS EJTAG debugger
> support registers, 63 different even powers of 2 are valid lengths.
>
> Determining the range of allowed breakpoint lengths, converting back
> and forth between numeric values that are likely to be used in a
> debugger, and these symbolic values that the proposed kernel interface
> would use, could be a little ugly.
>
> Have you thought about passing just the raw length?  And perhaps
> having:
>
> HW_BREAKPOINT_LEN_MASK that would have a bit set for each log2 of a
> legal length?
>

As explained to you here: http://lkml.org/lkml/2008/12/16/553/, defining
every possible length of the HW Breakpoint works for x86, but may not be
suitable for MIPS.

As you might have seen, the HW_BREAKPOINT_LEN_* values are defined in
x86-specific files and will be compared against 'len' field in
arch-specific 'struct arch_hw_breakpoint', for the reason that these
attributes are not valid for all architectures and have to be defined
for each processor in their own way.

Defining a HW_BREAKPOINT_LEN_MASK mask and validation of the input
length to check if it is a valid power of 2 can still be done for MIPS
in the arch-specific files and I don't see any part of the generic
interface being a hurdle during its implementation. Let me know if you
think there's any.

> Or perhaps add a function to the interface that would validate the
> length?
> 

arch_validate_hwbkpt_settings() invoked during
register_<kernel>/<user>_hw_breakpoint() is that function which
validates the length.

> David Daney

Thanks,
K.Prasad