lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 5 May 2020 16:01:15 +0530 From: Ravi Bangoria <ravi.bangoria@...ux.ibm.com> To: Anju T Sudhakar <anju@...ux.vnet.ibm.com> Cc: linuxppc-dev@...ts.ozlabs.org, mpe@...erman.id.au, acme@...nel.org, linux-kernel@...r.kernel.org, maddy@...ux.vnet.ibm.com, jolsa@...nel.org, Ravi Bangoria <ravi.bangoria@...ux.ibm.com> Subject: Re: [PATCH 2/2] powerpc/perf: Add support for outputting extended regs in perf intr_regs Hi Anju, Minor neats... > /* > diff --git a/arch/powerpc/include/uapi/asm/perf_regs.h b/arch/powerpc/include/uapi/asm/perf_regs.h > index f599064dd8dc..604b831378fe 100644 > --- a/arch/powerpc/include/uapi/asm/perf_regs.h > +++ b/arch/powerpc/include/uapi/asm/perf_regs.h > @@ -48,6 +48,17 @@ enum perf_event_powerpc_regs { > PERF_REG_POWERPC_DSISR, > PERF_REG_POWERPC_SIER, > PERF_REG_POWERPC_MMCRA, > - PERF_REG_POWERPC_MAX, > + /* Extended registers */ > + PERF_REG_POWERPC_MMCR0, > + PERF_REG_POWERPC_MMCR1, > + PERF_REG_POWERPC_MMCR2, > + PERF_REG_EXTENDED_MAX, > + /* Max regs without the extended regs */ > + PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1, > }; > + > +#define PERF_REG_PMU_MASK ((1ULL << PERF_REG_POWERPC_MAX) - 1) Would it make sense to reuse PERF_REG_MASK? Userspace code already uses that name for the same expression. > +#define PERF_REG_EXTENDED_MASK (((1ULL << (PERF_REG_EXTENDED_MAX)) \ > + - 1) - PERF_REG_PMU_MASK) You don't need parenthesis in (PERF_REG_EXTENDED_MAX). Also, better to keep that `- 1)` in first line. > + > #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */ > diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c > index 3dcfecf858f3..f56b77800a7b 100644 > --- a/arch/powerpc/perf/core-book3s.c > +++ b/arch/powerpc/perf/core-book3s.c > @@ -2276,6 +2276,7 @@ int register_power_pmu(struct power_pmu *pmu) > > power_pmu.attr_groups = ppmu->attr_groups; > > + power_pmu.capabilities |= (ppmu->capabilities & PERF_PMU_CAP_EXTENDED_REGS); > #ifdef MSR_HV > /* > * Use FCHV to ignore kernel events if MSR.HV is set. > diff --git a/arch/powerpc/perf/perf_regs.c b/arch/powerpc/perf/perf_regs.c > index a213a0aa5d25..57aa02568caf 100644 > --- a/arch/powerpc/perf/perf_regs.c > +++ b/arch/powerpc/perf/perf_regs.c > @@ -15,7 +15,8 @@ > > #define PT_REGS_OFFSET(id, r) [id] = offsetof(struct pt_regs, r) > > -#define REG_RESERVED (~((1ULL << PERF_REG_POWERPC_MAX) - 1)) > +#define REG_RESERVED (~(PERF_REG_EXTENDED_MASK) & \ > + (~((1ULL << PERF_REG_POWERPC_MAX) - 1))) Can we reuse PERF_REG_PMU_MASK here and simplify it to: #define REG_RESERVED (~(PERF_REG_EXTENDED_MASK | PERF_REG_PMU_MASK)) > > static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = { > PT_REGS_OFFSET(PERF_REG_POWERPC_R0, gpr[0]), > @@ -69,10 +70,22 @@ static unsigned int pt_regs_offset[PERF_REG_POWERPC_MAX] = { > PT_REGS_OFFSET(PERF_REG_POWERPC_MMCRA, dsisr), > }; > > +/* Function to return the extended register values */ > +static u64 get_ext_regs_value(int idx) > +{ > + switch (idx) { > + case PERF_REG_POWERPC_MMCR0: > + return mfspr(SPRN_MMCR0); > + case PERF_REG_POWERPC_MMCR1: > + return mfspr(SPRN_MMCR1); > + case PERF_REG_POWERPC_MMCR2: > + return mfspr(SPRN_MMCR2); Unnecessary tabs. [...] > diff --git a/tools/arch/powerpc/include/uapi/asm/perf_regs.h b/tools/arch/powerpc/include/uapi/asm/perf_regs.h > index f599064dd8dc..d66953294c73 100644 > --- a/tools/arch/powerpc/include/uapi/asm/perf_regs.h > +++ b/tools/arch/powerpc/include/uapi/asm/perf_regs.h > @@ -48,6 +48,17 @@ enum perf_event_powerpc_regs { > PERF_REG_POWERPC_DSISR, > PERF_REG_POWERPC_SIER, > PERF_REG_POWERPC_MMCRA, > - PERF_REG_POWERPC_MAX, > + /* Extended arch registers */ > + PERF_REG_POWERPC_MMCR0, > + PERF_REG_POWERPC_MMCR1, > + PERF_REG_POWERPC_MMCR2, > + PERF_REG_EXTENDED_MAX, > + /* Max regs without extended arch regs */ > + PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1, > + Unnecesasy line. > }; > +#define PERF_REG_PMU_MASK ((1ULL << PERF_REG_POWERPC_MAX) - 1) > +#define PERF_REG_EXTENDED_MASK (((1ULL << (PERF_REG_EXTENDED_MAX))\ > + - 1) - PERF_REG_PMU_MASK) > + > #endif /* _UAPI_ASM_POWERPC_PERF_REGS_H */ > diff --git a/tools/perf/arch/powerpc/include/perf_regs.h b/tools/perf/arch/powerpc/include/perf_regs.h > index e18a3556f5e3..f7bbdb816f88 100644 > --- a/tools/perf/arch/powerpc/include/perf_regs.h > +++ b/tools/perf/arch/powerpc/include/perf_regs.h > @@ -64,7 +64,11 @@ static const char *reg_names[] = { > [PERF_REG_POWERPC_DAR] = "dar", > [PERF_REG_POWERPC_DSISR] = "dsisr", > [PERF_REG_POWERPC_SIER] = "sier", > - [PERF_REG_POWERPC_MMCRA] = "mmcra" > + [PERF_REG_POWERPC_MMCRA] = "mmcra", > + [PERF_REG_POWERPC_MMCR0] = "mmcr0", > + [PERF_REG_POWERPC_MMCR1] = "mmcr1", > + [PERF_REG_POWERPC_MMCR2] = "mmcr2", > + Unnecesasy line. Apart from those, for the series: Reviewed-and-Tested-by: Ravi Bangoria <ravi.bangoria@...ux.ibm.com>
Powered by blists - more mailing lists