lkml.org 
[lkml]   [2017]   [Jul]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH V2 1/4] hw_breakpoint: Add step_needed event attribute
From
Date


On Tuesday 25 July 2017 06:57 PM, Will Deacon wrote:
> On Fri, Jul 07, 2017 at 05:33:57PM +0530, Pratyush Anand wrote:
>> Architecture like ARM64 currently allows to use default hw breakpoint
>> single step handler only to perf. However, some other users like few
>> systemtap tests or kernel test in
>> samples/hw_breakpoint/data_breakpoint.c can also work with default step
>> handler implementation. At the same time, some other like GDB/ptrace may
>> implement their own step handler.
>>
>> Therefore, this patch introduces a new perf_event_attr bit field, so
>> that arch specific code(specially on arm64) can make a decision to
>> enable single stepping.
>>
>> Any architecture which is not using this field will not have any
>> side effect.
>>
>> Signed-off-by: Pratyush Anand <panand@redhat.com>
>> ---
>> include/linux/hw_breakpoint.h | 6 ++++++
>> include/uapi/linux/perf_event.h | 3 ++-
>> kernel/events/core.c | 2 ++
>> tools/include/uapi/linux/perf_event.h | 3 ++-
>> 4 files changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/hw_breakpoint.h b/include/linux/hw_breakpoint.h
>> index 0464c85e63fd..6173ae048cbc 100644
>> --- a/include/linux/hw_breakpoint.h
>> +++ b/include/linux/hw_breakpoint.h
>> @@ -38,6 +38,12 @@ static inline int hw_breakpoint_type(struct perf_event *bp)
>> return bp->attr.bp_type;
>> }
>>
>> +static inline bool
>> +hw_breakpoint_needs_single_step(struct perf_event *bp)
>> +{
>> + return bp->attr.step_needed;
>> +}
>> +
>> static inline unsigned long hw_breakpoint_len(struct perf_event *bp)
>> {
>> return bp->attr.bp_len;
>> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
>> index b1c0b187acfe..00935808de0d 100644
>> --- a/include/uapi/linux/perf_event.h
>> +++ b/include/uapi/linux/perf_event.h
>> @@ -345,7 +345,8 @@ struct perf_event_attr {
>> context_switch : 1, /* context switch data */
>> write_backward : 1, /* Write ring buffer from end to beginning */
>> namespaces : 1, /* include namespaces data */
>> - __reserved_1 : 35;
>> + step_needed : 1, /* Use arch step handler */
>> + __reserved_1 : 34;
>
> This needs documenting properly, as I really have no idea how userspace is
> going to use it sensibley, especially as you silently overwrite it in some
> cases below.

I too had thought to put it under include/linux/perf_event.h : struct
perf_event. But, see hw_break_module_init() which does not have knowledge of
this structure, and we need to have some way so that none-perf kernel module
implementation can tell that it needs default arch step handler.

Do you see any alternative?

--
Regards
Pratyush

\
 
 \ /
  Last update: 2017-07-26 07:42    [W:0.066 / U:0.296 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site