> On 24-Jul-2020, at 5:56 PM, Ravi Bangoria <ravi.bangoria@xxxxxxxxxxxxx> wrote:
>
> Hi Athira,
>
>> +/* Function to return the extended register values */
>> +static u64 get_ext_regs_value(int idx)
>> +{
>> + switch (idx) {
>> + case PERF_REG_POWERPC_MMCR0:
>> + return mfspr(SPRN_MMCR0);
>> + case PERF_REG_POWERPC_MMCR1:
>> + return mfspr(SPRN_MMCR1);
>> + case PERF_REG_POWERPC_MMCR2:
>> + return mfspr(SPRN_MMCR2);
>> + default: return 0;
>> + }
>> +}
>> +
>> u64 perf_reg_value(struct pt_regs *regs, int idx)
>> {
>> - if (WARN_ON_ONCE(idx >= PERF_REG_POWERPC_MAX))
>> - return 0;
>> + u64 PERF_REG_EXTENDED_MAX;
>
> PERF_REG_EXTENDED_MAX should be initialized. otherwise ...
>
>> +
>> + if (cpu_has_feature(CPU_FTR_ARCH_300))
>> + PERF_REG_EXTENDED_MAX = PERF_REG_MAX_ISA_300;
>> if (idx == PERF_REG_POWERPC_SIER &&
>> (IS_ENABLED(CONFIG_FSL_EMB_PERF_EVENT) ||
>> @@ -85,6 +103,16 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
>> IS_ENABLED(CONFIG_PPC32)))
>> return 0;
>> + if (idx >= PERF_REG_POWERPC_MAX && idx < PERF_REG_EXTENDED_MAX)
>> + return get_ext_regs_value(idx);
>
> On non p9/p10 machine, PERF_REG_EXTENDED_MAX may contain random value which will
> allow user to pass this if condition unintentionally.
>
> Neat: PERF_REG_EXTENDED_MAX is a local variable so it should be in lowercase.
> Any specific reason to define it in capital?
Hi Ravi
There is no specific reason. I will include both these changes in next version
Thanks
Athira Rajeev
>
> Ravi
