[PATCH v3 3/3] powerpc/perf: Fix the check for SIAR value

[kajoljain]

On 8/18/21 6:58 PM, Christophe Leroy wrote:
> 
> 
> Le 18/08/2021 à 15:19, Kajol Jain a écrit :
>> Incase of random sampling, there can be scenarios where
>> Sample Instruction Address Register(SIAR) may not latch
>> to the sampled instruction and could result in
>> the value of 0. In these scenarios it is preferred to
>> return regs->nip. These corner cases are seen in the
>> previous generation (p9) also.
>>
>> Patch adds the check for SIAR value along with use_siar and
>> siar_valid checks so that the function will return regs->nip
>> incase SIAR is zero.
>>
>> Patch drops the code under PPMU_P10_DD1 flag check
>> which handles SIAR 0 case only for Power10 DD1.
>>
>> Fixes: 2ca13a4cc56c9 ("powerpc/perf: Use regs->nip when SIAR is zero")
>> Signed-off-by: Kajol Jain <kjain at linux.ibm.com>
>> ---
>>
>> Changelog:
>> - Drop adding new ternary condition to check siar value.
>> - Remove siar check specific for PPMU_P10_DD1 and add
>>    it along with common checks as suggested by Christophe Leroy
>>    and Michael Ellermen
>>
>>   arch/powerpc/perf/core-book3s.c | 7 +------
>>   1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
>> index 23ec89a59893..55efbba7572b 100644
>> --- a/arch/powerpc/perf/core-book3s.c
>> +++ b/arch/powerpc/perf/core-book3s.c
>> @@ -2254,12 +2254,7 @@ unsigned long perf_instruction_pointer(struct pt_regs *regs)
>>       bool use_siar = regs_use_siar(regs);
>>       unsigned long siar = mfspr(SPRN_SIAR);
>>   -    if (ppmu && (ppmu->flags & PPMU_P10_DD1)) {
>> -        if (siar)
>> -            return siar;
>> -        else
>> -            return regs->nip;
>> -    } else if (use_siar && siar_valid(regs))
>> +    if (use_siar && siar_valid(regs) && siar)
> 
> You can probably now do
> 
> +    if (regs_use_siar(regs) && siar_valid(regs) && siar)
> 
> and remove use_siar

Hi Christophe,
     I will update it. Thanks for pointing it.

Thanks,
Kajol Jain

> 
>>           return siar + perf_ip_adjust(regs);
>>       else
>>           return regs->nip;
>>