| Message ID | 20240709095506.9691-2-yangtiezhu@loongson.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw_breakpoint: Save privilege of access control via ptrace | expand |
On Tue, Jul 09, 2024 at 05:55:04PM +0800, Tiezhu Yang wrote: > When developing hardware watchpoint on LoongArch, we want to set the > same privilege passed by the ptrace user data, but there is no bridge > to save this value like bp_addr, bp_type and bp_len. This is a common > issue for the archs which have privilege level of breakpoint. > > Add a member "bp_priv" which lives in a union on config3 at the end > of the uapi struct perf_event_attr to make a bridge between ptrace > and hardware breakpoint. > > This is preparation for later patch on some archs such as ARM, ARM64 > and LoongArch which have privilege level of breakpoint. > > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> > --- > include/uapi/linux/perf_event.h | 5 ++++- > kernel/events/hw_breakpoint.c | 1 + > 2 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h > index 3a64499b0f5d..abe8da7a1f60 100644 > --- a/include/uapi/linux/perf_event.h > +++ b/include/uapi/linux/perf_event.h > @@ -521,7 +521,10 @@ struct perf_event_attr { > */ > __u64 sig_data; > > - __u64 config3; /* extension of config2 */ > + union { > + __u8 bp_priv; /* privilege level of breakpoint */ > + __u64 config3; /* extension of config2 */ > + }; > }; > > /* > diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c > index 6c2cb4e4f48d..3ad16b226e4f 100644 > --- a/kernel/events/hw_breakpoint.c > +++ b/kernel/events/hw_breakpoint.c > @@ -754,6 +754,7 @@ static void hw_breakpoint_copy_attr(struct perf_event_attr *to, > to->bp_addr = from->bp_addr; > to->bp_type = from->bp_type; > to->bp_len = from->bp_len; > + to->bp_priv = from->bp_priv; > to->disabled = from->disabled; > } Sorry, but I still don't see why we should change uapi for this. As I have said multiple times, this is unnecessary. Will
On 07/09/2024 06:03 PM, Will Deacon wrote: > On Tue, Jul 09, 2024 at 05:55:04PM +0800, Tiezhu Yang wrote: >> When developing hardware watchpoint on LoongArch, we want to set the >> same privilege passed by the ptrace user data, but there is no bridge >> to save this value like bp_addr, bp_type and bp_len. This is a common >> issue for the archs which have privilege level of breakpoint. >> >> Add a member "bp_priv" which lives in a union on config3 at the end >> of the uapi struct perf_event_attr to make a bridge between ptrace >> and hardware breakpoint. >> >> This is preparation for later patch on some archs such as ARM, ARM64 >> and LoongArch which have privilege level of breakpoint. >> >> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> >> --- >> include/uapi/linux/perf_event.h | 5 ++++- >> kernel/events/hw_breakpoint.c | 1 + >> 2 files changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h >> index 3a64499b0f5d..abe8da7a1f60 100644 >> --- a/include/uapi/linux/perf_event.h >> +++ b/include/uapi/linux/perf_event.h >> @@ -521,7 +521,10 @@ struct perf_event_attr { >> */ >> __u64 sig_data; >> >> - __u64 config3; /* extension of config2 */ >> + union { >> + __u8 bp_priv; /* privilege level of breakpoint */ >> + __u64 config3; /* extension of config2 */ >> + }; >> }; >> >> /* >> diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c >> index 6c2cb4e4f48d..3ad16b226e4f 100644 >> --- a/kernel/events/hw_breakpoint.c >> +++ b/kernel/events/hw_breakpoint.c >> @@ -754,6 +754,7 @@ static void hw_breakpoint_copy_attr(struct perf_event_attr *to, >> to->bp_addr = from->bp_addr; >> to->bp_type = from->bp_type; >> to->bp_len = from->bp_len; >> + to->bp_priv = from->bp_priv; >> to->disabled = from->disabled; >> } > > Sorry, but I still don't see why we should change uapi for this. As I > have said multiple times, this is unnecessary. Thanks for your review, I see your point but let us wait for more comments from perf people. Thanks, Tiezhu
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h index 3a64499b0f5d..abe8da7a1f60 100644 --- a/include/uapi/linux/perf_event.h +++ b/include/uapi/linux/perf_event.h @@ -521,7 +521,10 @@ struct perf_event_attr { */ __u64 sig_data; - __u64 config3; /* extension of config2 */ + union { + __u8 bp_priv; /* privilege level of breakpoint */ + __u64 config3; /* extension of config2 */ + }; }; /* diff --git a/kernel/events/hw_breakpoint.c b/kernel/events/hw_breakpoint.c index 6c2cb4e4f48d..3ad16b226e4f 100644 --- a/kernel/events/hw_breakpoint.c +++ b/kernel/events/hw_breakpoint.c @@ -754,6 +754,7 @@ static void hw_breakpoint_copy_attr(struct perf_event_attr *to, to->bp_addr = from->bp_addr; to->bp_type = from->bp_type; to->bp_len = from->bp_len; + to->bp_priv = from->bp_priv; to->disabled = from->disabled; }
When developing hardware watchpoint on LoongArch, we want to set the same privilege passed by the ptrace user data, but there is no bridge to save this value like bp_addr, bp_type and bp_len. This is a common issue for the archs which have privilege level of breakpoint. Add a member "bp_priv" which lives in a union on config3 at the end of the uapi struct perf_event_attr to make a bridge between ptrace and hardware breakpoint. This is preparation for later patch on some archs such as ARM, ARM64 and LoongArch which have privilege level of breakpoint. Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> --- include/uapi/linux/perf_event.h | 5 ++++- kernel/events/hw_breakpoint.c | 1 + 2 files changed, 5 insertions(+), 1 deletion(-)