Re: [PATCH 4/6] perf/tool: Parse non-cpu pmu capabilities

Ravi Bangoria
2022-04-26T11:54:02Z

On 26-Apr-22 4:07 PM, Robert Richter wrote:
> On 25.04.22 10:13:21, Ravi Bangoria wrote:
>
>> diff --git a/tools/perf/Documentation/perf.data-file-format.txt b/tools/perf/Documentation/perf.data-file-format.txt
>> index f56d0e0fbff6..dea3acb36558 100644
>> --- a/tools/perf/Documentation/perf.data-file-format.txt
>> +++ b/tools/perf/Documentation/perf.data-file-format.txt
>> @@ -435,6 +435,24 @@ struct {
>> } [nr_pmu];
>> };
>>
>> + HEADER_PMU_CAPS = 32,
>> +
>> + List of pmu capabilities (except cpu pmu which is already
>> + covered by HEADER_CPU_PMU_CAPS)
>> +
>> +struct {
>> + u32 nr_pmus;
>> + struct {
>> + u8 core_type; /* For hybrid topology */
>> + char pmu_name[];
>> + u16 nr_caps;
>> + struct {
>> + char name[];
>> + char value[];
>> + } [nr_caps];
>> + } [nr_pmus];
>> +};
>> +
>
> This looks quite a bit complex and special.
>
> Why not just reusing struct nr_cpu_pmu_caps (id 28)? Rename it and
> introduce macros for backwards compatability if needed.

No. HEADER_CPU_PMU_CAPS (id 28) is designed only for CPU pmu and can
not save more than one pmu data. A designed proposed in this patch
is generic and can be used to save capabilities of multiple pmus
within single header. Something like:

struct {
nr_pmus=2,

[0] = struct {
core_type = 0,
pmu_name = "ibs_fetch"
nr_caps = 2,
[0] = { .name = "cap1", .value = "value1" }
[1] = { .name = "cap2", .value = "value2" }
},

[1] = struct {
core_type = 0,
pmu_name = "ibs_op"
nr_caps = 3,
[0] = { .name = "cap1", .value = "value1" }
[1] = { .name = "cap2", .value = "value2" }
[2] = { .name = "cap3", .value = "value3" }
},
}

>
> pmu_name is already encoded in sysfs and core_type could be a caps
> value?

pmu_name is needed to map capability with pmu. And core_type is needed
when same pmu is supported by multiple core types (like P-core / E-core
on Intel platform) but has different capabilities.

Thanks,
Ravi