linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] arm64: perf: Add more support on caps under sysfs
@ 2021-05-21  7:25 Shaokun Zhang
  2021-05-24 17:18 ` Robin Murphy
  0 siblings, 1 reply; 3+ messages in thread
From: Shaokun Zhang @ 2021-05-21  7:25 UTC (permalink / raw)
  To: linux-arm-kernel; +Cc: Shaokun Zhang, Will Deacon, Mark Rutland

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@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Shaokun Zhang <zhangshaokun@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;
+	default:
+		val = 0;
+	}
+
+	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,
 };
 
-- 
2.7.4


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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] arm64: perf: Add more support on caps under sysfs
  2021-05-21  7:25 [PATCH] arm64: perf: Add more support on caps under sysfs Shaokun Zhang
@ 2021-05-24 17:18 ` Robin Murphy
  2021-05-25  2:13   ` Shaokun Zhang
  0 siblings, 1 reply; 3+ messages in thread
From: Robin Murphy @ 2021-05-24 17:18 UTC (permalink / raw)
  To: Shaokun Zhang, linux-arm-kernel; +Cc: Will Deacon, Mark Rutland

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@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Signed-off-by: Shaokun Zhang <zhangshaokun@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,
>   };
>   
> 

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] arm64: perf: Add more support on caps under sysfs
  2021-05-24 17:18 ` Robin Murphy
@ 2021-05-25  2:13   ` Shaokun Zhang
  0 siblings, 0 replies; 3+ messages in thread
From: Shaokun Zhang @ 2021-05-25  2:13 UTC (permalink / raw)
  To: Robin Murphy, linux-arm-kernel; +Cc: Will Deacon, Mark Rutland

Hi Robin,

On 2021/5/25 1:18, Robin Murphy wrote:
> 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@kernel.org>
>> Cc: Mark Rutland <mark.rutland@arm.com>
>> Signed-off-by: Shaokun Zhang <zhangshaokun@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);
> 

Good catch, I will follow it in next version.

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

To be honest, I'm also not sure about it and I checked it in the document that
For BUS_WIDTH and BUS_SLOTS, if it is not from Armv8.7, it shall be Reserved, RAZ
and I returned 0 including other values from Armv8.7.

> 
>> +    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.

When talked about SLOTS field [1], Will suggested to get rid of visible hook and
return zero if the CPUs without it.

[1]https://www.spinics.net/lists/arm-kernel/msg824289.html

Thanks,
Shaokun

> 
> 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,
>>   };
>>  
> .

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-05-25  3:40 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-21  7:25 [PATCH] arm64: perf: Add more support on caps under sysfs Shaokun Zhang
2021-05-24 17:18 ` Robin Murphy
2021-05-25  2:13   ` Shaokun Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).