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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <3f1c9a9e-cb63-47ff-a5e9-06555fa6cc9a@linux.intel.com>
Date: Thu, 19 Jun 2025 10:27:21 -0400
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: "Mi, Dapeng" <dapeng1.mi@...ux.intel.com>,
 Mark Rutland <mark.rutland@....com>, mingo@...hat.com, acme@...nel.org,
 namhyung@...nel.org, tglx@...utronix.de, dave.hansen@...ux.intel.com,
 irogers@...gle.com, adrian.hunter@...el.com, jolsa@...nel.org,
 alexander.shishkin@...ux.intel.com, linux-kernel@...r.kernel.org,
 ak@...ux.intel.com, zide.chen@...el.com
Subject: Re: [RFC PATCH 00/12] Support vector and more extended registers in
 perf



On 2025-06-19 9:38 a.m., Peter Zijlstra wrote:
> On Thu, Jun 19, 2025 at 07:11:23AM -0400, Liang, Kan wrote:
> 
>> @@ -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;
> 
> This limits things to max 16 predicate registers. ARM will fully fill
> that with present hardware.

I think I can use __u32 for predicate registers.
It means we need one more u64 for the qwords. It should not be a problem.

> 
>> +	__u16 sample_simd_reg_qwords;
>> +	__u64 sample_simd_reg_intr;
>> +	__u64 sample_simd_reg_user;
> 
> I would perhaps make this vec_reg.

Sure.

> 
>>  };
>>
>>  /*
>> @@ -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
> 
> Instead of using sample_simd_reg_enabled here I would perhaps extend
> perf_sample_regs_abi. The current abi word is woefully underused.
> 

Yes. Now I think the abi is used like a version number. I guess I can
add PERF_SAMPLE_REGS_ABI_SIMD and change it to a bitmap.
There should be no impact on the existing tool, since version and bitmap
are the same for 1 and 2.
 enum perf_sample_regs_abi {
-       PERF_SAMPLE_REGS_ABI_NONE               = 0,
-       PERF_SAMPLE_REGS_ABI_32                 = 1,
-       PERF_SAMPLE_REGS_ABI_64                 = 2,
+       PERF_SAMPLE_REGS_ABI_NONE               = 0x0,
+       PERF_SAMPLE_REGS_ABI_32                 = 0x1,
+       PERF_SAMPLE_REGS_ABI_64                 = 0x2,
+       PERF_SAMPLE_REGS_ABI_SIMD               = 0x4,
 };

> Also, realistically, what you want to look at here is:
> 
>   sample_simd_{pred,vec}_reg_user;
> 
> If those are empty, there will be no registers.

Sure. But I will still keep the sample_simd_reg_enabled, since it can
explicitly tell if the new format is used.

Thanks,
Kan

> 
>> +	 *	} && PERF_SAMPLE_REGS_USER
>>  	 *
>>  	 *	{ u64			size;
>>  	 *	  char			data[size];
>