Re: [RFC PATCH 00/12] Support vector and more extended registers in perf

["Liang, Kan" <kan.liang@linux.intel.com>]

On 2025-06-18 8:41 p.m., Mi, Dapeng wrote:
> 
> On 6/18/2025 9:15 PM, Liang, Kan wrote:
>>
>> On 2025-06-18 8:28 a.m., Mi, Dapeng wrote:
>>>>>>> Not sure, it eats up a whole byte. Dapeng seemed to favour separate
>>>>>>> intr/user vector width (although I'm not quite sure what the use would
>>>>>>> be).
>>>>> The reason that I prefer to add 2 separate "words" item is that user could
>>>>> sample interrupt and user space SIMD regs (but with different bit-width)
>>>>> simultaneously in theory, like "--intr-regs=YMM0, --user-regs=XMM0".
>>>> I'm not sure why the user wants a different bit-width. The
>>>> --user-regs=XMM0" doesn't seem to provide more useful information.
>>>>
>>>> Anyway, I believe the tool can handle this case. The tool can always ask
>>>> YMM0 for both --intr-regs and --user-regs, but only output the XMM0 for
>>>> --user-regs. The only drawback is that the kernel may dump extra
>>>> information for the --user-regs. I don't think it's a big problem.
>>> If we intent to handle it in user space tools, I'm not sure if user space
>>> tool can easily know which records are from user space and filter out the
>>> SIMD regs from kernel space and how complicated would the change be. IMO,
>>> adding an extra u16 "words" would be much easier and won't consume too much
>>> memory.
>> The filter is always done in kernel for --user-regs. The only difference
>> is that the YMM (after filter) will be dumped to the perf.data. The tool
>> just show the XMM registers to the end user.
> 
> Ok. But there could be another case, user may want to sample some APX eGPRs
> in user space and sample SIMD regs in interrupt, like "--intr-regs=YMM0,
> --user-regs=R16", then we have to define 2 separate "words" fields.
> 

Not for eGPRs. It uses the regular GP regs space, which implies u64 for
a 64b kernel. The "words" fields is only for vector and predicate registers.

I've stated working on the V2. The new interface would be as below.

diff --git a/include/uapi/linux/perf_event.h
b/include/uapi/linux/perf_event.h
index 78a362b80027..f7b8971fa99d 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -382,6 +382,7 @@ enum perf_event_read_format {
 #define PERF_ATTR_SIZE_VER6			120	/* Add: aux_sample_size */
 #define PERF_ATTR_SIZE_VER7			128	/* Add: sig_data */
 #define PERF_ATTR_SIZE_VER8			136	/* Add: config3 */
+#define PERF_ATTR_SIZE_VER9			184	/* Add: sample_simd_regs */

 /*
  * 'struct perf_event_attr' contains various attributes that define
@@ -543,6 +544,24 @@ struct perf_event_attr {
 	__u64	sig_data;

 	__u64	config3; /* extension of config2 */
+
+
+	/*
+	 * Defines set of SIMD registers to dump on samples.
+	 * The sample_simd_req_enabled !=0 implies the
+	 * set of SIMD registers is used to config all SIMD registers.
+	 * If !sample_simd_req_enabled, sample_regs_XXX may be used to
+	 * config some SIMD registers on X86.
+	 */
+	union {
+		__u16 sample_simd_reg_enabled;
+		__u16 sample_simd_pred_reg_qwords;
+	};
+	__u16 sample_simd_pred_reg_intr;
+	__u16 sample_simd_pred_reg_user;
+	__u16 sample_simd_reg_qwords;
+	__u64 sample_simd_reg_intr;
+	__u64 sample_simd_reg_user;
 };

 /*
@@ -1016,7 +1035,15 @@ enum perf_event_type {
 	 *      } && PERF_SAMPLE_BRANCH_STACK
 	 *
 	 *	{ u64			abi; # enum perf_sample_regs_abi
-	 *	  u64			regs[weight(mask)]; } && PERF_SAMPLE_REGS_USER
+	 *	  u64			regs[weight(mask)];
+	 *	  struct {
+	 *	  	u16 nr_vectors;
+	 *	  	u16 vector_qwords;
+	 *	  	u16 nr_pred;
+	 *	  	u16 pred_qwords;
+	 *	  	u64 data[nr_vectors * vector_qwords + nr_pred * pred_qwords];
+	 *	  } && sample_simd_reg_enabled
+	 *	} && PERF_SAMPLE_REGS_USER
 	 *
 	 *	{ u64			size;
 	 *	  char			data[size];
@@ -1043,7 +1070,15 @@ enum perf_event_type {
 	 *	{ u64			data_src; } && PERF_SAMPLE_DATA_SRC
 	 *	{ u64			transaction; } && PERF_SAMPLE_TRANSACTION
 	 *	{ u64			abi; # enum perf_sample_regs_abi
-	 *	  u64			regs[weight(mask)]; } && PERF_SAMPLE_REGS_INTR
+	 *	  u64			regs[weight(mask)];
+	 *	  struct {
+	 *	  	u16 nr_vectors;
+	 *	  	u16 vector_qwords;
+	 *	  	u16 nr_pred;
+	 *	  	u16 pred_qwords;
+	 *	  	u64 data[nr_vectors * vector_qwords + nr_pred * pred_qwords];
+	 *	  } && sample_simd_reg_enabled
+	 *	} && PERF_SAMPLE_REGS_INTR
 	 *	{ u64			phys_addr;} && PERF_SAMPLE_PHYS_ADDR
 	 *	{ u64			cgroup;} && PERF_SAMPLE_CGROUP
 	 *	{ u64			data_page_size;} && PERF_SAMPLE_DATA_PAGE_SIZE


Thanks,
Kan