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;
 }
 
-- 
2.42.0

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