On 08-Jul-2020, at 5:34 PM, Michael Ellerman <mpe@ellerman.id.au> = wrote:

Athira Rajeev <atrajeev@linux.vnet.ibm.com> writes:
Include capability flag = `PERF_PMU_CAP_EXTENDED_REGS` for power10
and expose MMCR3, = SIER2, SIER3 registers as part of extended regs.
Also = introduce `PERF_REG_PMU_MASK_31` to define extended mask
value at runtime for power10

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
arch/powerpc/include/uapi/asm/perf_regs.h =       | 6 ++++++
arch/powerpc/perf/perf_regs.c =             &n= bsp;     | 10 +++++++++-
arch/powerpc/perf/power10-pmu.c =             &n= bsp;   | 6 ++++++
tools/arch/powerpc/include/uapi/asm/perf_regs.h | 6 = ++++++
tools/perf/arch/powerpc/include/perf_regs.h =     | 3 +++
tools/perf/arch/powerpc/util/perf_regs.c =        | 6 ++++++

Please split into a kernel patch and a tools patch. And cc = the tools people.

Ok sure

6 files changed, 36 insertions(+), 1 = deletion(-)

diff --git = a/arch/powerpc/include/uapi/asm/perf_regs.h = b/arch/powerpc/include/uapi/asm/perf_regs.h
index = 485b1d5..020b51c 100644
--- = a/arch/powerpc/include/uapi/asm/perf_regs.h
+++ = b/arch/powerpc/include/uapi/asm/perf_regs.h
@@ -52,6 +52,9 = @@ enum perf_event_powerpc_regs {
= PERF_REG_POWERPC_MMCR0,
= PERF_REG_POWERPC_MMCR1,
= PERF_REG_POWERPC_MMCR2,
+ = PERF_REG_POWERPC_MMCR3,
+ = PERF_REG_POWERPC_SIER2,
+ = PERF_REG_POWERPC_SIER3,
/* Max = regs without the extended regs */
= PERF_REG_POWERPC_MAX =3D PERF_REG_POWERPC_MMCRA + 1,
};
@@ -62,4 +65,7 @@ enum = perf_event_powerpc_regs {
#define PERF_REG_PMU_MASK_300 = (((1ULL << (PERF_REG_POWERPC_MMCR2 + 1)) - 1) \
= = = = - PERF_REG_PMU_MASK)

+/* = PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_31 */
+#define= PERF_REG_PMU_MASK_31 (((1ULL << (PERF_REG_POWERPC_SIER3 + 1)) - 1) \
+ = = = = - PERF_REG_PMU_MASK)

Wrapping that provides no = benefit, just let it be long.

Ok,

#endif = /* _UAPI_ASM_POWERPC_PERF_REGS_H */
diff --git = a/arch/powerpc/perf/perf_regs.c b/arch/powerpc/perf/perf_regs.c
index c8a7e8c..c969935 100644
--- = a/arch/powerpc/perf/perf_regs.c
+++ = b/arch/powerpc/perf/perf_regs.c
@@ -81,6 +81,12 @@ static = u64 get_ext_regs_value(int idx)
return = mfspr(SPRN_MMCR1);
case PERF_REG_POWERPC_MMCR2:
= = return mfspr(SPRN_MMCR2);
+ case = PERF_REG_POWERPC_MMCR3:
+ return mfspr(SPRN_MMCR3);
+ = case PERF_REG_POWERPC_SIER2:
+ return = mfspr(SPRN_SIER2);
+ case PERF_REG_POWERPC_SIER3:
+ = = = return mfspr(SPRN_SIER3);

Indentation is wrong.

default: = return 0;
}
}
@@ = -89,7 +95,9 @@ u64 perf_reg_value(struct pt_regs *regs, int idx)
{
u64 PERF_REG_EXTENDED_MAX;

- if = (cpu_has_feature(CPU_FTR_ARCH_300))
+ if = (cpu_has_feature(CPU_FTR_ARCH_31))
+ = PERF_REG_EXTENDED_MAX =3D PERF_REG_POWERPC_SIER3 + 1;

There's no way to know if that's correct other than going = back to the
header to = look at the list of values.

So instead you should define it in the header, next to the = other values,
with a = meaningful name, like PERF_REG_MAX_ISA_31 or something.

+ else if = (cpu_has_feature(CPU_FTR_ARCH_300))
= PERF_REG_EXTENDED_MAX =3D PERF_REG_POWERPC_MMCR2 + 1;

Same.

Ok, will make this change