Re: [PATCH v7] arm64: perf: Add support caps in sysfs

[Will Deacon <will@kernel.org>]

On Sat, Sep 12, 2020 at 04:07:22PM +0800, Shaokun Zhang wrote:
> ARMv8.4-PMU introduces the PMMIR_EL1 registers and some new PMU events,
> like STALL_SLOT etc, are related to it. Let's add a caps directory to
> /sys/bus/event_source/devices/armv8_pmuv3_0/ and support slots from
> PMMIR_EL1 registers in this entry. The user programs can get the slots
> from sysfs directly.
> 
> /sys/bus/event_source/devices/armv8_pmuv3_0/caps/slots is exposed
> through sysfs. Both ARMv8.4-PMU and STALL_SLOT event are implemented,
> it returns the slots from PMMIR_EL1, otherwise it will return 0.
> 
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Shaokun Zhang <zhangshaokun@hisilicon.com>
> ---
> ChangeLog in v7:
>     * If this feature is not supported, return 0.

[...]

>  arch/arm64/include/asm/perf_event.h |   3 ++
>  arch/arm64/include/asm/sysreg.h     |   2 +
>  arch/arm64/kernel/perf_event.c      | 103 ++++++++++++++++++++++++------------
>  include/linux/perf/arm_pmu.h        |   3 ++
>  4 files changed, 78 insertions(+), 33 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/perf_event.h b/arch/arm64/include/asm/perf_event.h
> index 2c2d7dbe8a02..60731f602d3e 100644
> --- a/arch/arm64/include/asm/perf_event.h
> +++ b/arch/arm64/include/asm/perf_event.h
> @@ -236,6 +236,9 @@
>  #define ARMV8_PMU_USERENR_CR	(1 << 2) /* Cycle counter can be read at EL0 */
>  #define ARMV8_PMU_USERENR_ER	(1 << 3) /* Event counter can be read at EL0 */
>  
> +/* PMMIR_EL1.SLOTS mask */
> +#define ARMV8_PMU_SLOTS_MASK	0xff
> +
>  #ifdef CONFIG_PERF_EVENTS
>  struct pt_regs;
>  extern unsigned long perf_instruction_pointer(struct pt_regs *regs);
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 554a7e8ecb07..921773adff5e 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -321,6 +321,8 @@
>  #define SYS_PMINTENSET_EL1		sys_reg(3, 0, 9, 14, 1)
>  #define SYS_PMINTENCLR_EL1		sys_reg(3, 0, 9, 14, 2)
>  
> +#define SYS_PMMIR_EL1			sys_reg(3, 0, 9, 14, 6)
> +
>  #define SYS_MAIR_EL1			sys_reg(3, 0, 10, 2, 0)
>  #define SYS_AMAIR_EL1			sys_reg(3, 0, 10, 3, 0)
>  
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index 462f9a9cc44b..953d92145908 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -302,6 +302,28 @@ static struct attribute_group armv8_pmuv3_format_attr_group = {
>  	.attrs = armv8_pmuv3_format_attrs,
>  };
>  
> +static ssize_t slots_show(struct device *dev, struct device_attribute *attr,
> +			  char *page)
> +{
> +	struct pmu *pmu = dev_get_drvdata(dev);
> +	struct arm_pmu *cpu_pmu = container_of(pmu, struct arm_pmu, pmu);
> +	int slots = cpu_pmu->reg_pmmir & ARMV8_PMU_SLOTS_MASK;
> +
> +	return snprintf(page, PAGE_SIZE, "0x%02x\n", slots);

I'm a little bit nervous about %02x here, as future versions of the
architecture could extend PMMIR.SLOTS and parsers would have a bit of a
rotten time dealing with the new immediates. Why don't we just make it
%08x instead, and have 'int slots' be 'u32 slots'?

Otherwise patch looks fine, thanks. Happy to apply a new version with that
change.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel