[PATCH v3] powerpc/perf: Set cpumode flags using sample address

Athira Rajeev atrajeev at linux.vnet.ibm.com
Fri May 31 19:48:12 AEST 2024



> On 28 May 2024, at 9:33 AM, Anjali K <anjalik at linux.ibm.com> wrote:
> 
> Currently in some cases, when the sampled instruction address register
> latches to a specific address during sampling, the privilege bits captured
> in the sampled event register are incorrect.
> For example, a snippet from the perf report on a power10 system is:
> Overhead  Address             Command       Shared Object      Symbol
> ........  ..................  ............  .................  .......................
>     2.41%  0x7fff9f94a02c      null_syscall  [unknown]          [k] 0x00007fff9f94a02c
>     2.20%  0x7fff9f94a02c      null_syscall  libc.so.6          [.] syscall
> 
> perf_get_misc_flags() function looks at the privilege bits to return the
> corresponding flags to be used for the address symbol and these privilege
> bit details are read from the sampled event register. In the above snippet,
> address "0x00007fff9f94a02c" is shown as "k" (kernel) due to the incorrect
> privilege bits captured in the sampled event register.
> To address this case check whether the sampled address is in the kernel
> area. Since this is specific to the latest platform, a new pmu flag is
> added called "PPMU_P10" and is used to contain the proposed fix.
> PPMU_P10_DD1 marked events are also included under PPMU_P10, hence
> remove the code specific to PPMU_P10_DD1 marked events.
> 
> Signed-off-by: Anjali K <anjalik at linux.ibm.com>

Reviewed-by: Athira Rajeev <atrajeev at linux.vnet.ibm.com <mailto:atrajeev at linux.vnet.ibm.com>>

Thanks
Athira Rajeev
> ---
> Changelog:
> v2->v3:
> Addressed review comments from Michael Ellerman to change the commit 
> message and update the values of PPMU_P10 and PPMU_HAS_ATTR_CONFIG1 flags
> to move the PPMU_P10 flag near the PPMU_P10_DD1 flag
> Rebased on linux 6.10-rc1
> v2 patch link:
> https://lore.kernel.org/linuxppc-dev/20240517094607.422166-1-anjalik@linux.ibm.com/
> 
> v1->v2:
> Fixed the build warning reported by the kernel test bot 
> Added a new flag PPMU_P10 and used it instead of PPMU_ARCH_31 to restrict
> the changes to the current platform (Power10)
> v1 patch link:
> https://lore.kernel.org/linuxppc-dev/20240511075344.1393631-1-anjalik@linux.ibm.com/
> 
> arch/powerpc/include/asm/perf_event_server.h |  3 +-
> arch/powerpc/perf/core-book3s.c              | 45 +++++++++-----------
> arch/powerpc/perf/power10-pmu.c              |  3 +-
> 3 files changed, 23 insertions(+), 28 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/perf_event_server.h b/arch/powerpc/include/asm/perf_event_server.h
> index e2221d29fdf9..5995614e9062 100644
> --- a/arch/powerpc/include/asm/perf_event_server.h
> +++ b/arch/powerpc/include/asm/perf_event_server.h
> @@ -89,7 +89,8 @@ struct power_pmu {
> #define PPMU_NO_SIAR 0x00000100 /* Do not use SIAR */
> #define PPMU_ARCH_31 0x00000200 /* Has MMCR3, SIER2 and SIER3 */
> #define PPMU_P10_DD1 0x00000400 /* Is power10 DD1 processor version */
> -#define PPMU_HAS_ATTR_CONFIG1 0x00000800 /* Using config1 attribute */
> +#define PPMU_P10 0x00000800 /* For power10 pmu */
> +#define PPMU_HAS_ATTR_CONFIG1 0x00001000 /* Using config1 attribute */
> 
> /*
>  * Values for flags to get_alternatives()
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 6b5f8a94e7d8..42867469752d 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -266,31 +266,12 @@ static inline u32 perf_flags_from_msr(struct pt_regs *regs)
> static inline u32 perf_get_misc_flags(struct pt_regs *regs)
> {
> bool use_siar = regs_use_siar(regs);
> - unsigned long mmcra = regs->dsisr;
> - int marked = mmcra & MMCRA_SAMPLE_ENABLE;
> + unsigned long siar;
> + unsigned long addr;
> 
> if (!use_siar)
> return perf_flags_from_msr(regs);
> 
> - /*
> - * Check the address in SIAR to identify the
> - * privilege levels since the SIER[MSR_HV, MSR_PR]
> - * bits are not set for marked events in power10
> - * DD1.
> - */
> - if (marked && (ppmu->flags & PPMU_P10_DD1)) {
> - unsigned long siar = mfspr(SPRN_SIAR);
> - if (siar) {
> - if (is_kernel_addr(siar))
> - return PERF_RECORD_MISC_KERNEL;
> - return PERF_RECORD_MISC_USER;
> - } else {
> - if (is_kernel_addr(regs->nip))
> - return PERF_RECORD_MISC_KERNEL;
> - return PERF_RECORD_MISC_USER;
> - }
> - }
> -
> /*
> * If we don't have flags in MMCRA, rather than using
> * the MSR, we intuit the flags from the address in
> @@ -298,19 +279,31 @@ static inline u32 perf_get_misc_flags(struct pt_regs *regs)
> * results
> */
> if (ppmu->flags & PPMU_NO_SIPR) {
> - unsigned long siar = mfspr(SPRN_SIAR);
> + siar = mfspr(SPRN_SIAR);
> if (is_kernel_addr(siar))
> return PERF_RECORD_MISC_KERNEL;
> return PERF_RECORD_MISC_USER;
> }
> 
> /* PR has priority over HV, so order below is important */
> - if (regs_sipr(regs))
> - return PERF_RECORD_MISC_USER;
> -
> - if (regs_sihv(regs) && (freeze_events_kernel != MMCR0_FCHV))
> + if (regs_sipr(regs)) {
> + if (!(ppmu->flags & PPMU_P10))
> + return PERF_RECORD_MISC_USER;
> + } else if (regs_sihv(regs) && (freeze_events_kernel != MMCR0_FCHV))
> return PERF_RECORD_MISC_HYPERVISOR;
> 
> + /*
> + * Check the address in SIAR to identify the
> + * privilege levels since the SIER[MSR_HV, MSR_PR]
> + * bits are not set correctly in power10 sometimes
> + */
> + if (ppmu->flags & PPMU_P10) {
> + siar = mfspr(SPRN_SIAR);
> + addr = siar ? siar : regs->nip;
> + if (!is_kernel_addr(addr))
> + return PERF_RECORD_MISC_USER;
> + }
> +
> return PERF_RECORD_MISC_KERNEL;
> }
> 
> diff --git a/arch/powerpc/perf/power10-pmu.c b/arch/powerpc/perf/power10-pmu.c
> index 62a68b6b2d4b..bb57b7cfe640 100644
> --- a/arch/powerpc/perf/power10-pmu.c
> +++ b/arch/powerpc/perf/power10-pmu.c
> @@ -593,7 +593,8 @@ static struct power_pmu power10_pmu = {
> .get_mem_weight = isa207_get_mem_weight,
> .disable_pmc = isa207_disable_pmc,
> .flags = PPMU_HAS_SIER | PPMU_ARCH_207S |
> -  PPMU_ARCH_31 | PPMU_HAS_ATTR_CONFIG1,
> +  PPMU_ARCH_31 | PPMU_HAS_ATTR_CONFIG1 |
> +  PPMU_P10,
> .n_generic = ARRAY_SIZE(power10_generic_events),
> .generic_events = power10_generic_events,
> .cache_events = &power10_cache_events,
> 
> base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
> -- 
> 2.39.3
> 



More information about the Linuxppc-dev mailing list