[PATCH] arm64: perf: Add more support on caps under sysfs

[Robin Murphy]

On 2021-05-21 08:25, Shaokun Zhang wrote:
> Armv8.7 has introduced BUS_SLOTS and BUS_WIDTH in PMMIR_EL1 register,
> add two entries in caps for bus_slots and bus_width under sysfs. It
> will return the true slots and width if the information is available,
> otherwise it will return 0.
> 
> Cc: Will Deacon <will at kernel.org>
> Cc: Mark Rutland <mark.rutland at arm.com>
> Signed-off-by: Shaokun Zhang <zhangshaokun at hisilicon.com>
> ---
>   arch/arm64/include/asm/perf_event.h |  5 +++
>   arch/arm64/kernel/perf_event.c      | 64 +++++++++++++++++++++++++++++++++++++
>   2 files changed, 69 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/perf_event.h b/arch/arm64/include/asm/perf_event.h
> index 60731f602d3e..4ef6f19331f9 100644
> --- a/arch/arm64/include/asm/perf_event.h
> +++ b/arch/arm64/include/asm/perf_event.h
> @@ -239,6 +239,11 @@
>   /* PMMIR_EL1.SLOTS mask */
>   #define ARMV8_PMU_SLOTS_MASK	0xff
>   
> +#define ARMV8_PMU_BUS_SLOTS_SHIFT 8
> +#define ARMV8_PMU_BUS_SLOTS_MASK 0xff
> +#define ARMV8_PMU_BUS_WIDTH_SHIFT 16
> +#define ARMV8_PMU_BUS_WIDTH_MASK 0xf
> +
>   #ifdef CONFIG_PERF_EVENTS
>   struct pt_regs;
>   extern unsigned long perf_instruction_pointer(struct pt_regs *regs);
> diff --git a/arch/arm64/kernel/perf_event.c b/arch/arm64/kernel/perf_event.c
> index f594957e29bd..f0847e4f48a9 100644
> --- a/arch/arm64/kernel/perf_event.c
> +++ b/arch/arm64/kernel/perf_event.c
> @@ -317,8 +317,72 @@ static ssize_t slots_show(struct device *dev, struct device_attribute *attr,
>   
>   static DEVICE_ATTR_RO(slots);
>   
> +static ssize_t bus_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);
> +	u32 bus_slots = (cpu_pmu->reg_pmmir >> ARMV8_PMU_BUS_SLOTS_SHIFT)
> +			& ARMV8_PMU_BUS_SLOTS_MASK;
> +
> +	return snprintf(page, PAGE_SIZE, "0x%08x\n", bus_slots);
> +}
> +
> +static DEVICE_ATTR_RO(bus_slots);
> +
> +static ssize_t bus_width_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);
> +	u32 bus_width = (cpu_pmu->reg_pmmir >> ARMV8_PMU_BUS_WIDTH_SHIFT)
> +			& ARMV8_PMU_BUS_WIDTH_MASK;
> +	u32 val;
> +
> +	switch (bus_width) {
> +	case 3:
> +		val = 4;
> +		break;
> +	case 4:
> +		val = 8;
> +		break;
> +	case 5:
> +		val = 16;
> +		break;
> +	case 6:
> +		val = 32;
> +		break;
> +	case 7:
> +		val = 64;
> +		break;
> +	case 8:
> +		val = 128;
> +		break;
> +	case 9:
> +		val = 256;
> +		break;
> +	case 10:
> +		val = 512;
> +		break;
> +	case 11:
> +		val = 1024;
> +		break;
> +	case 12:
> +		val = 2048;
> +		break;

Nit: the encoding is explicitly defined as "log2(number of bytes) plus 
one", so this might be neater as simply:

	val = 1 << (bus_width - 1);

I'm not really sure whether interpreting reserved values as 0 is any 
better or worse than interpreting them as implied :/

> +	default:
> +		val = 0;
> +	}

If the information is not available, wouldn't it make sense to just hide 
the attribute(s) entirely? Userspace should already have to cope with 
older kernels, so from that point of view it seems easier to only have 
two states of "not present" and "present and valid", without having to 
also consider a third "present but invalid" state.

Robin.

> +
> +	return snprintf(page, PAGE_SIZE, "0x%08x\n", val);
> +}
> +
> +static DEVICE_ATTR_RO(bus_width);
> +
>   static struct attribute *armv8_pmuv3_caps_attrs[] = {
>   	&dev_attr_slots.attr,
> +	&dev_attr_bus_slots.attr,
> +	&dev_attr_bus_width.attr,
>   	NULL,
>   };
>   
>