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

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 <redacted>
---
 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

On 2021-05-21 08:25, Shaokun Zhang wrote:

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

Hi Robin,

On 2021/5/25 1:18, Robin Murphy wrote:

On 2021-05-21 08:25, Shaokun Zhang wrote:

quoted

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

quoted

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

quoted

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