Patch set adds PMU registers namely Sampled Instruction Address Register
(SIAR) and Sampled Data Address Register (SDAR) as part of extended regs
in PowerPC. These registers provides the instruction/data address and
adding these to extended regs helps in debug purposes.
Patch 1/4 and 2/4 refactors the existing macro definition of
PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 to make it more
readable.
Patch 3/4 adds SIAR and SDAR as part of the extended regs mask.
Patch 4/4 includes perf tools side changes to add the SPRs to
sample_reg_mask to use with -I? option.
Changelog:
Change from v1 -> v2:
Addressed review comments from Michael Ellerman
- Refactored the perf reg extended mask value macros for
PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 to
make it more readable. Also moved PERF_REG_EXTENDED_MAX
along with enum definition similar to PERF_REG_POWERPC_MAX.
Athira Rajeev (4):
powerpc/perf: Refactor the code definition of perf reg extended mask
tools/perf: Refactor the code definition of perf reg extended mask in
tools side header file
powerpc/perf: Expose instruction and data address registers as part of
extended regs
tools/perf: Add perf tools support to expose instruction and data
address registers as part of extended regs
arch/powerpc/include/uapi/asm/perf_regs.h | 28 ++++++++++++-------
arch/powerpc/perf/perf_regs.c | 4 +++
.../arch/powerpc/include/uapi/asm/perf_regs.h | 28 ++++++++++++-------
tools/perf/arch/powerpc/include/perf_regs.h | 2 ++
tools/perf/arch/powerpc/util/perf_regs.c | 2 ++
5 files changed, 44 insertions(+), 20 deletions(-)
--
2.30.1 (Apple Git-130)
PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 defines the mask
value for extended registers. Current definition of these mask values
uses hex constant and does not use registers by name, making it less
readable. Patch refactor the macro values by or'ing together the actual
register value constants. Also include PERF_REG_EXTENDED_MAX as
part of enum definition.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Athira Rajeev <redacted>
---
arch/powerpc/include/uapi/asm/perf_regs.h | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
Patch enables presenting of Sampled Instruction Address Register (SIAR)
and Sampled Data Address Register (SDAR) SPRs as part of extended regsiters
for perf tool. Add these SPR's to sample_reg_mask in the tool side (to use
with -I? option).
Signed-off-by: Athira Rajeev <redacted>
---
tools/arch/powerpc/include/uapi/asm/perf_regs.h | 11 +++++++----
tools/perf/arch/powerpc/include/perf_regs.h | 2 ++
tools/perf/arch/powerpc/util/perf_regs.c | 2 ++
3 files changed, 11 insertions(+), 4 deletions(-)
@@ -61,17 +61,19 @@ enum perf_event_powerpc_regs {PERF_REG_POWERPC_PMC4,PERF_REG_POWERPC_PMC5,PERF_REG_POWERPC_PMC6,+PERF_REG_POWERPC_SDAR,+PERF_REG_POWERPC_SIAR,/* Max mask value for interrupt regs w/o extended regs */PERF_REG_POWERPC_MAX=PERF_REG_POWERPC_MMCRA+1,/* Max mask value for interrupt regs including extended regs */-PERF_REG_EXTENDED_MAX=PERF_REG_POWERPC_PMC6+1,+PERF_REG_EXTENDED_MAX=PERF_REG_POWERPC_SIAR+1,};#define PERF_REG_PMU_MASK ((1ULL << PERF_REG_POWERPC_MAX) - 1)/**PERF_REG_EXTENDED_MASKvalueforCPU_FTR_ARCH_300-*includes9SPRSfromMMCR0toPMC6excludingthe+*includes11SPRSfromMMCR0toSIARexcludingthe*unsupportedSPRSMMCR3,SIER2andSIER3.*/#define PERF_REG_PMU_MASK_300 \
PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 defines the mask
value for extended registers. Current definition of these mask values
uses hex constant and does not use registers by name, making it less
readable. Patch refactor the macro values in perf tools side header file
by or'ing together the actual register value constants.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Athira Rajeev <redacted>
---
.../arch/powerpc/include/uapi/asm/perf_regs.h | 21 ++++++++++++-------
1 file changed, 13 insertions(+), 8 deletions(-)
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 | 11 +++++++----
arch/powerpc/perf/perf_regs.c | 4 ++++
2 files changed, 11 insertions(+), 4 deletions(-)
@@ -61,17 +61,19 @@ enum perf_event_powerpc_regs {PERF_REG_POWERPC_PMC4,PERF_REG_POWERPC_PMC5,PERF_REG_POWERPC_PMC6,+PERF_REG_POWERPC_SDAR,+PERF_REG_POWERPC_SIAR,/* Max mask value for interrupt regs w/o extended regs */PERF_REG_POWERPC_MAX=PERF_REG_POWERPC_MMCRA+1,/* Max mask value for interrupt regs including extended regs */-PERF_REG_EXTENDED_MAX=PERF_REG_POWERPC_PMC6+1,+PERF_REG_EXTENDED_MAX=PERF_REG_POWERPC_SIAR+1,};#define PERF_REG_PMU_MASK ((1ULL << PERF_REG_POWERPC_MAX) - 1)/**PERF_REG_EXTENDED_MASKvalueforCPU_FTR_ARCH_300-*includes9SPRSfromMMCR0toPMC6excludingthe+*includes11SPRSfromMMCR0toSIARexcludingthe*unsupportedSPRSMMCR3,SIER2andSIER3.*/#define PERF_REG_PMU_MASK_300 \
From: Daniel Axtens <hidden> Date: 2021-10-01 06:20:54
Hi Athira,
PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 defines the mask
value for extended registers. Current definition of these mask values
uses hex constant and does not use registers by name, making it less
readable. Patch refactor the macro values in perf tools side header file
by or'ing together the actual register value constants.
(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.
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 <redacted>
Kind regards,
Daniel
On 01-Oct-2021, at 11:50 AM, Daniel Axtens [off-list ref] wrote:
Hi Athira,
quoted
PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 defines the mask
value for extended registers. Current definition of these mask values
uses hex constant and does not use registers by name, making it less
readable. Patch refactor the macro values in perf tools side header file
by or'ing together the actual register value constants.
This file is uAPI - are we allowed to remove a define? Could a program
built against these headers now fail to compile because we've removed it?
Hi Daniel,
Thanks for the review.
My bad, you are right. I will correct this in version3 by retaining this define and refactoring the macro.
quoted
-
/*
* PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_300
* includes 9 SPRS from MMCR0 to PMC6 excluding the
- * unsupported SPRS in PERF_EXCLUDE_REG_EXT_300.
+ * unsupported SPRS MMCR3, SIER2 and SIER3.
*/
-#define PERF_REG_PMU_MASK_300 ((0xfffULL << PERF_REG_POWERPC_MMCR0) - PERF_EXCLUDE_REG_EXT_300)
+#define PERF_REG_PMU_MASK_300 \
+ ((1ul << PERF_REG_POWERPC_MMCR0) | (1ul << PERF_REG_POWERPC_MMCR1) | \
+ (1ul << PERF_REG_POWERPC_MMCR2) | (1ul << PERF_REG_POWERPC_PMC1) | \
+ (1ul << PERF_REG_POWERPC_PMC2) | (1ul << PERF_REG_POWERPC_PMC3) | \
+ (1ul << PERF_REG_POWERPC_PMC4) | (1ul << PERF_REG_POWERPC_PMC5) | \
+ (1ul << PERF_REG_POWERPC_PMC6))
/*
* PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_31
* includes 12 SPRs from MMCR0 to PMC6.
*/
-#define PERF_REG_PMU_MASK_31 (0xfffULL << PERF_REG_POWERPC_MMCR0)
+#define PERF_REG_PMU_MASK_31 \
+ (PERF_REG_PMU_MASK_300 | (1ul << PERF_REG_POWERPC_MMCR3) | \
+ (1ul << PERF_REG_POWERPC_SIER2) | (1ul << PERF_REG_POWERPC_SIER3))
-#define PERF_REG_EXTENDED_MAX (PERF_REG_POWERPC_PMC6 + 1)
Likewise for this define?
I think this might also be an issue for some of your other patches which
change an include/uapi/ file.
Though I am removing PERF_REG_EXTENDED_MAX define from end of this uapi file, it is moved along with enum definition of perf_event_powerpc_regs.
So we should be good with moving this define from this place.
Thanks
Athira
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2021-10-01 11:30:40
Daniel Axtens [off-list ref] writes:
Hi Athira,
quoted
PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 defines the mask
value for extended registers. Current definition of these mask values
uses hex constant and does not use registers by name, making it less
readable. Patch refactor the macro values in perf tools side header file
by or'ing together the actual register value constants.
This file is uAPI - are we allowed to remove a define? Could a program
built against these headers now fail to compile because we've removed it?
Yeah that's true.
In this case I think I'd rather we remove it though, because:
- it was never meant to be part of the uapi, it was just meant for use
in the construction of PERF_REG_PMU_MASK_300, and is no longer needed
for that.
- it's only been in the header since v5.12, so I think the chance of
anything using it is essentially zero.
cheers
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2021-10-05 05:53:35
Athira Rajeev [off-list ref] writes:
quoted hunk
PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 defines the mask
value for extended registers. Current definition of these mask values
uses hex constant and does not use registers by name, making it less
readable. Patch refactor the macro values by or'ing together the actual
register value constants. Also include PERF_REG_EXTENDED_MAX as
part of enum definition.
Suggested-by: Michael Ellerman <mpe@ellerman.id.au>
Signed-off-by: Athira Rajeev <redacted>
---
arch/powerpc/include/uapi/asm/perf_regs.h | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
@@ -61,27 +61,32 @@ enum perf_event_powerpc_regs {PERF_REG_POWERPC_PMC4,PERF_REG_POWERPC_PMC5,PERF_REG_POWERPC_PMC6,-/* Max regs without the extended regs */+/* Max mask value for interrupt regs w/o extended regs */PERF_REG_POWERPC_MAX=PERF_REG_POWERPC_MMCRA+1,+/* Max mask value for interrupt regs including extended regs */+PERF_REG_EXTENDED_MAX=PERF_REG_POWERPC_PMC6+1,};#define PERF_REG_PMU_MASK ((1ULL << PERF_REG_POWERPC_MAX) - 1)-/* Exclude MMCR3, SIER2, SIER3 for CPU_FTR_ARCH_300 */-#define PERF_EXCLUDE_REG_EXT_300 (7ULL << PERF_REG_POWERPC_MMCR3)-/**PERF_REG_EXTENDED_MASKvalueforCPU_FTR_ARCH_300*includes9SPRSfromMMCR0toPMC6excludingthe-*unsupportedSPRSinPERF_EXCLUDE_REG_EXT_300.+*unsupportedSPRSMMCR3,SIER2andSIER3.*/-#define PERF_REG_PMU_MASK_300 ((0xfffULL << PERF_REG_POWERPC_MMCR0) - PERF_EXCLUDE_REG_EXT_300)+#define PERF_REG_PMU_MASK_300 \+((1ul<<PERF_REG_POWERPC_MMCR0)|(1ul<<PERF_REG_POWERPC_MMCR1)|\+(1ul<<PERF_REG_POWERPC_MMCR2)|(1ul<<PERF_REG_POWERPC_PMC1)|\+(1ul<<PERF_REG_POWERPC_PMC2)|(1ul<<PERF_REG_POWERPC_PMC3)|\+(1ul<<PERF_REG_POWERPC_PMC4)|(1ul<<PERF_REG_POWERPC_PMC5)|\+(1ul<<PERF_REG_POWERPC_PMC6))
These all need to be unsigned long long. Otherwise when building on big
endian (which defaults to 32-bit), we see errors such as:
In file included from /home/michael/linux/tools/perf/arch/powerpc/include/perf_regs.h:7:0,
from arch/powerpc/util/../../../util/perf_regs.h:30,
from arch/powerpc/util/perf_regs.c:7:
arch/powerpc/util/perf_regs.c: In function ‘arch__intr_reg_mask’:
/home/michael/linux/tools/arch/powerpc/include/uapi/asm/perf_regs.h:78:8: error: left shift count >= width of type [-Werror=shift-count-overflow]
((1ul << PERF_REG_POWERPC_MMCR0) | (1ul << PERF_REG_POWERPC_MMCR1) | \
^
arch/powerpc/util/perf_regs.c:206:19: note: in expansion of macro ‘PERF_REG_PMU_MASK_300’
extended_mask = PERF_REG_PMU_MASK_300;
^
cheers