Re: [PATCH 1/2] powerpc/perf: Expose instruction and data address registers as part of extended regs

On 08-Sep-2021, at 10:47 AM, Michael Ellerman [off-list ref] wrote:

Athira Rajeev [off-list ref] writes:

quoted

Patch adds support to include Sampled Instruction Address Register
(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.

Signed-off-by: Athira Rajeev <redacted>
---
arch/powerpc/include/uapi/asm/perf_regs.h | 12 +++++++-----
arch/powerpc/perf/perf_regs.c             |  4 ++++
2 files changed, 11 insertions(+), 5 deletions(-)

...

quoted

diff --git a/arch/powerpc/perf/perf_regs.c b/arch/powerpc/perf/perf_regs.c
index b931eed..51d31b6 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);
	default: return 0;
	}

This file is built for all powerpc configs that have PERF_EVENTS. Which
includes CPUs that don't have SDAR or SIAR.

Don't we need checks in perf_reg_value() like we do for SIER?

Hi Michael,

Thanks for the review.

SIER is part of PERF_REG_PMU_MASK and hence check is needed to see if platform supports SIER.
Incase of extended regs, they are part of PERF_REG_EXTENDED_MASK and this mask is
filled with supported registers while registering the PMU ( ie during init_power9/10_pmu ). So these registers will be added
only for supported platforms. The validity of extended mask is also done in PMU common code 
( In kernel/events/core.c with PERF_REG_EXTENDED_MASK check ). So an unsupported platform requesting for extended
registers won’t get it.

Thanks
Athira

I guess we already got this wrong when we added the Power10 registers,
SIER2/3 etc.

cheers