On 32-bit, perf_read_regs() sets regs->result to 0, so regs_use_siar() is always false and every sample is attributed to the interrupted PC.
The MPC7450 family's SIAR holds the address of the last instruction to complete before the performance monitor exception is generated (MPC7450UM Rev. 5, section 11.3.7). The exception can be generated with MSR[EE] = 0 and is taken once EE is set again (section 11.2), so when interrupts are disabled the interrupted PC is wherever they are re-enabled, while SIAR still points into the disabled region. On a 7448, sampling cycles system-wide with --intr-regs=nip during dd, perf bench sched pipe and find, 46% of the kernel samples have SIAR and the interrupted PC in different functions. 13270 samples have their PC on the instruction after the mtmsr that restores interrupts in ata_scsi_queuecmd(), and 11661 of those have SIAR in udelay(). With interrupts enabled SIAR helps less. Sampling a 32-instruction loop with one store and one branch with -c 10007, 99.8% of the samples land at: event interrupted PC SIAR r122 (branches) br + 13 br + 9 r014 (stores) stw + 21 stw + 17 siar_valid() already returns true on 32-bit. Use SIAR for PMU exceptions, and take the user/kernel flags from it too. Move regs_use_siar() above the 32-bit block so that perf_get_misc_flags() can call it. Signed-off-by: Matt Turner <[email protected]> --- arch/powerpc/perf/core-book3s.c | 55 +++++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 19 deletions(-) diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c index 10ded865583c..e4c8ecddccbd 100644 --- a/arch/powerpc/perf/core-book3s.c +++ b/arch/powerpc/perf/core-book3s.c @@ -75,6 +75,20 @@ static struct power_pmu *ppmu; */ static unsigned int freeze_events_kernel = MMCR0_FCS; +static bool regs_use_siar(struct pt_regs *regs) +{ + /* + * When we take a performance monitor exception the regs are setup + * using perf_read_regs() which overloads some fields, in particular + * regs->result to tell us whether to use SIAR. + * + * However if the regs are from another exception, eg. a syscall, then + * they have not been setup using perf_read_regs() and so regs->result + * is something random. + */ + return ((TRAP(regs) == INTERRUPT_PERFMON) && regs->result); +} + /* * 32-bit doesn't have MMCRA but does have an MMCR2, * and a few other names are different. @@ -108,13 +122,30 @@ static inline unsigned long perf_ip_adjust(struct pt_regs *regs) return 0; } static inline void perf_get_data_addr(struct perf_event *event, struct pt_regs *regs, u64 *addrp) { } -static inline u32 perf_get_misc_flags(struct pt_regs *regs) -{ - return 0; -} + +/* + * The MPC7450 family's SIAR holds the address of the last instruction to + * complete before the performance monitor exception was generated, so for + * a PMU exception it is closer to the event than the interrupted PC. + */ static inline void perf_read_regs(struct pt_regs *regs) { - regs->result = 0; + regs->result = 1; +} + +static inline u32 perf_get_misc_flags(struct pt_regs *regs) +{ + unsigned long siar; + + if (!regs_use_siar(regs)) + return 0; + + siar = mfspr(SPRN_SIAR); + if (!siar) + return 0; + + return is_kernel_addr(siar) ? PERF_RECORD_MISC_KERNEL : + PERF_RECORD_MISC_USER; } static inline int siar_valid(struct pt_regs *regs) @@ -163,20 +194,6 @@ unsigned long get_pmcs_ext_regs(int idx) return cpuhw->pmcs[idx]; } -static bool regs_use_siar(struct pt_regs *regs) -{ - /* - * When we take a performance monitor exception the regs are setup - * using perf_read_regs() which overloads some fields, in particular - * regs->result to tell us whether to use SIAR. - * - * However if the regs are from another exception, eg. a syscall, then - * they have not been setup using perf_read_regs() and so regs->result - * is something random. - */ - return ((TRAP(regs) == INTERRUPT_PERFMON) && regs->result); -} - /* * Things that are specific to 64-bit implementations. */ -- 2.55.0
