> On 01-Oct-2021, at 11:50 AM, Daniel Axtens <d...@axtens.net> wrote:
>
> Hi Athira,
>
>> PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 defines the mask
>> value for extended registers. Current definition of these mask values
>> uses hex constant and does not use registers by name, making it less
>> readable. Patch refactor the macro values in perf tools side header file
>> by or'ing together the actual register value constants.
>>
> This looks like a good simplification.
>
>> -/* Exclude MMCR3, SIER2, SIER3 for CPU_FTR_ARCH_300 */
>> -#define PERF_EXCLUDE_REG_EXT_300 (7ULL << PERF_REG_POWERPC_MMCR3)
>
> This file is uAPI - are we allowed to remove a define? Could a program
> built against these headers now fail to compile because we've removed it?
Hi Daniel,
Thanks for the review.
My bad, you are right. I will correct this in version3 by retaining this define
and refactoring the macro.
>
>> -
>> /*
>> * PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_300
>> * includes 9 SPRS from MMCR0 to PMC6 excluding the
>> - * unsupported SPRS in PERF_EXCLUDE_REG_EXT_300.
>> + * unsupported SPRS MMCR3, SIER2 and SIER3.
>> */
>> -#define PERF_REG_PMU_MASK_300 ((0xfffULL << PERF_REG_POWERPC_MMCR0) -
>> PERF_EXCLUDE_REG_EXT_300)
>> +#define PERF_REG_PMU_MASK_300 \
>> + ((1ul << PERF_REG_POWERPC_MMCR0) | (1ul << PERF_REG_POWERPC_MMCR1) | \
>> + (1ul << PERF_REG_POWERPC_MMCR2) | (1ul << PERF_REG_POWERPC_PMC1) | \
>> + (1ul << PERF_REG_POWERPC_PMC2) | (1ul << PERF_REG_POWERPC_PMC3) | \
>> + (1ul << PERF_REG_POWERPC_PMC4) | (1ul << PERF_REG_POWERPC_PMC5) | \
>> + (1ul << PERF_REG_POWERPC_PMC6))
>>
>> /*
>> * PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_31
>> * includes 12 SPRs from MMCR0 to PMC6.
>> */
>> -#define PERF_REG_PMU_MASK_31 (0xfffULL << PERF_REG_POWERPC_MMCR0)
>> +#define PERF_REG_PMU_MASK_31 \
>> + (PERF_REG_PMU_MASK_300 | (1ul << PERF_REG_POWERPC_MMCR3) | \
>> + (1ul << PERF_REG_POWERPC_SIER2) | (1ul << PERF_REG_POWERPC_SIER3))
>>
>> -#define PERF_REG_EXTENDED_MAX (PERF_REG_POWERPC_PMC6 + 1)
>
> Likewise for this define?
>
> I think this might also be an issue for some of your other patches which
> change an include/uapi/ file.
Though I am removing PERF_REG_EXTENDED_MAX define from end of this uapi file,
it is moved along with enum definition of perf_event_powerpc_regs.
So we should be good with moving this define from this place.
Thanks
Athira
>
> Kind regards,
> Daniel
>
>> --
>> 2.30.1 (Apple Git-130)
