Subject: Re: [V2 3/4] powerpc/perf: Expose instruction and data address registers as part of extended regs

From: Daniel Axtens <[email-masked]>
Date: Fri, 01 Oct 2021 16:40:26 +1000

Athira Rajeev <[email-masked]> writes:

> Patch adds support to include Sampled Instruction Address Register

This is a nit and doesn't require a new revision, but I think this
should read "Include Sampled Instruction Address ...", not "Patch adds
support to include Sampled Instruction ..." - see
https://www.kernel.org/doc/html/v5.11/process/submitting-patches.html#describe-your-changes

> (SIAR) and Sampled Data Address Register (SDAR) SPRs as part of extended
> registers. Update the definition of PERF_REG_PMU_MASK_300/31 and
> PERF_REG_EXTENDED_MAX to include these SPR's.

> diff --git a/arch/powerpc/perf/perf_regs.c b/arch/powerpc/perf/perf_regs.c
> index b931eed482c9..51d31b65e423 100644
> --- a/arch/powerpc/perf/perf_regs.c
> +++ b/arch/powerpc/perf/perf_regs.c
> @@ -90,7 +90,11 @@ static u64 get_ext_regs_value(int idx)
>  		return mfspr(SPRN_SIER2);
>  	case PERF_REG_POWERPC_SIER3:
>  		return mfspr(SPRN_SIER3);
> +	case PERF_REG_POWERPC_SDAR:
> +		return mfspr(SPRN_SDAR);
>  #endif
> +	case PERF_REG_POWERPC_SIAR:
> +		return mfspr(SPRN_SIAR);

I was initially confused about why SIAR was outside the CONFIG_PPC64
block and SDAR was inside. But it turns out that SIAR is also defined
for a 32 bit platform, so that makes sense.

I'm not an expert on how the perf subsystem works, but this all seems
consistent with the surrounding code and it seems to do what the commit
message says, so on that limited basis:

Reviewed-by: Daniel Axtens <[email-masked]>

Kind regards,
Daniel