[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <663344cc-9be4-40c2-9708-6b765af8f6e8@linux.intel.com>
Date: Thu, 19 Jun 2025 20:26:33 +0800
From: "Mi, Dapeng" <dapeng1.mi@...ux.intel.com>
To: "Liang, Kan" <kan.liang@...ux.intel.com>,
Peter Zijlstra <peterz@...radead.org>, Mark Rutland <mark.rutland@....com>
Cc: 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 6/19/2025 7:11 PM, Liang, Kan wrote:
>
> 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;
This is still a bitmap, right? Is it enough for ARM?
> + __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
Powered by blists - more mailing lists