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

[Christophe Leroy <christophe.leroy@csgroup.eu>]

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@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

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