[PATCH v2 0/4] perf/arm_cspmu: Miscellaneous improvements

[PATCH v2 0/4] perf/arm_cspmu: Miscellaneous improvements

[Robin Murphy]

Hi all,

These are a few quality-of-life improvements for CoreSight PMU that
I've accumulated based on things which have come up in practical usage.
There's no real dependency between any of the individual patches, it
just felt right to send them as a series.

v1: https://lore.kernel.org/linux-perf-users/cover.1783605641.git.robin.murphy@arm.com/
v2: Fix valid nits from Sashiko review, add Ilkka's R-b.

Thanks,
Robin.


Robin Murphy (4):
  ACPI/APMT: Use stable device ID
  perf/arm_cspmu: Improve APMT-based PMU naming
  perf/arm_cspmu: Improve sub-module error reporting
  perf/arm_cspmu: Make IRQ more optional

 drivers/acpi/arm64/apmt.c          |  6 ++--
 drivers/perf/arm_cspmu/arm_cspmu.c | 48 ++++++++++++++++++------------
 2 files changed, 33 insertions(+), 21 deletions(-)

-- 
2.54.0.dirty

[PATCH v2 1/4] ACPI/APMT: Use stable device ID

[Robin Murphy]

The APMT node format includes a unique identifier, so we can use this as
the platform device ID to give userspace stable and identifiable device
names, rather than auto numbering dependent on how the table is parsed.

Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Hanjun Guo <guohanjun@huawei.com>
Cc: Sudeep Holla <sudeep.holla@kernel.org>
Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>

---
v2: Handle and warn about unexpected signed overflow
---
 drivers/acpi/arm64/apmt.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/arm64/apmt.c b/drivers/acpi/arm64/apmt.c
index bb010f6164e5..91fcdd289e63 100644
--- a/drivers/acpi/arm64/apmt.c
+++ b/drivers/acpi/arm64/apmt.c
@@ -76,10 +76,12 @@ static int __init apmt_add_platform_device(struct acpi_apmt_node *node,
 					   struct fwnode_handle *fwnode)
 {
 	struct platform_device *pdev;
-	int ret, count;
+	int ret, count, uid = node->id & INT_MAX;
 	struct resource res[DEV_MAX_RESOURCE_COUNT];
 
-	pdev = platform_device_alloc(DEV_NAME, PLATFORM_DEVID_AUTO);
+	if (uid != node->id)
+		pr_warn("Unexpectedly large UID 0x%x, truncated to 0x%x\n", node->id, uid);
+	pdev = platform_device_alloc(DEV_NAME, uid);
 	if (!pdev)
 		return -ENOMEM;
 
-- 
2.54.0.dirty

[PATCH v2 2/4] perf/arm_cspmu: Improve APMT-based PMU naming

[Robin Murphy]

On ACPI systems, it has not actually been possible for userspace to
reliably tell which PMU corresponds to which APMT entry for types
other than "ACPI device" - the evidence trail only leads from the
arbitrarily-numbered PMU device to its arbitrarily-numbered parent
platform device that has no distinguishing features either.

While we've now improved the platform device creation to associate the
actual APMT unique ID, we may as well also tweak the PMU devices to
substitute the arbitrary number with a different arbitrary number that
might be more directly meaningful based on the APMT definitions.

We don't have an equivalent for Devicetree, but in that case the
platform devices are at least identifiable via their sysfs-visible
of_node.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>

---
v2: Actually use the instance-based ID where different from node->id
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 35 +++++++++++++++++-------------
 1 file changed, 20 insertions(+), 15 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 80fb314d5135..8c2dda17d73e 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -250,38 +250,43 @@ static const char *arm_cspmu_get_name(const struct arm_cspmu *cspmu)
 	struct device *dev;
 	struct acpi_apmt_node *apmt_node;
 	u8 pmu_type;
-	char *name;
 	char acpi_hid_string[ACPI_ID_LEN] = { 0 };
-	static atomic_t pmu_idx[ACPI_APMT_NODE_TYPE_COUNT] = { 0 };
+	static atomic_t pmu_idx;
+	u32 id;
 
 	dev = cspmu->dev;
 	apmt_node = arm_cspmu_apmt_node(dev);
 	if (!apmt_node)
 		return devm_kasprintf(dev, GFP_KERNEL, PMUNAME "_%u",
-				      atomic_fetch_inc(&pmu_idx[0]));
+				      atomic_fetch_inc(&pmu_idx));
 
 	pmu_type = apmt_node->type;
-
-	if (pmu_type >= ACPI_APMT_NODE_TYPE_COUNT) {
+	switch (pmu_type) {
+	default:
 		dev_err(dev, "unsupported PMU type-%u\n", pmu_type);
 		return NULL;
-	}
-
-	if (pmu_type == ACPI_APMT_NODE_TYPE_ACPI) {
+	case ACPI_APMT_NODE_TYPE_ACPI:
 		memcpy(acpi_hid_string,
 			&apmt_node->inst_primary,
 			sizeof(apmt_node->inst_primary));
-		name = devm_kasprintf(dev, GFP_KERNEL, "%s_%s_%s_%u", PMUNAME,
+		return devm_kasprintf(dev, GFP_KERNEL, "%s_%s_%s_%u", PMUNAME,
 				      arm_cspmu_type_str[pmu_type],
 				      acpi_hid_string,
 				      apmt_node->inst_secondary);
-	} else {
-		name = devm_kasprintf(dev, GFP_KERNEL, "%s_%s_%d", PMUNAME,
-				      arm_cspmu_type_str[pmu_type],
-				      atomic_fetch_inc(&pmu_idx[pmu_type]));
-	}
+	case ACPI_APMT_NODE_TYPE_MC:
+		id = apmt_node->id;
+		break;
+	case ACPI_APMT_NODE_TYPE_SMMU:
+	case ACPI_APMT_NODE_TYPE_PCIE_ROOT:
+		id = apmt_node->inst_primary;
+		break;
+	case ACPI_APMT_NODE_TYPE_CACHE:
+		id = apmt_node->inst_secondary;
+		break;
+	};
 
-	return name;
+	return devm_kasprintf(dev, GFP_KERNEL, "%s_%s_%u", PMUNAME,
+			      arm_cspmu_type_str[pmu_type], id);
 }
 
 static ssize_t arm_cspmu_cpumask_show(struct device *dev,
-- 
2.54.0.dirty

[PATCH v2 3/4] perf/arm_cspmu: Improve sub-module error reporting

[Robin Murphy]

When waiting for a sub-module to register, we return a bare
-EPROBE_DEFER that ends up showing the end user:

  platform arm-cs-arch-pmu.1: deferred probe pending (no reason)

wherein it's not necessarily clear that they might need to take some
action to ensure the appropriate module is available to load. Let's use
dev_err_probe() here so we can show exactly what we're waiting for.

Similarly, in the case where something's gone horribly wrong with an
already-registered module, we can use dev_WARN() to standardise the
device/driver attribution rather than just open-coding "arm_cspmu".

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>

---
v2: No change
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index 8c2dda17d73e..caad9a36651d 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -437,13 +437,15 @@ static int arm_cspmu_init_impl_ops(struct arm_cspmu *cspmu)
 				if (ret)
 					module_put(match->module);
 			} else {
-				WARN(1, "arm_cspmu failed to get module: %s\n",
+				dev_WARN(cspmu->dev, "Failed to get module: %s\n",
 					match->module_name);
 				ret = -EINVAL;
 			}
 		} else {
 			request_module_nowait(match->module_name);
-			ret = -EPROBE_DEFER;
+			ret = dev_err_probe(cspmu->dev, -EPROBE_DEFER,
+					    "Waiting for module %s to load\n",
+					    match->module_name);
 		}
 
 		mutex_unlock(&arm_cspmu_lock);
-- 
2.54.0.dirty

[PATCH v2 4/4] perf/arm_cspmu: Make IRQ more optional

[Robin Murphy]

If we have 64-bit counters, we can reasonably assume we'll never have to
handle an overflow before the end of the universe (since we're a system
PMU with no sampling). Thus even if firmware does specify an IRQ, we can
still continue in the event of being unable to request it. This can help
systems where IRQs cannot be claimed exclusively, or are broken in other
ways.

Reviewed-by: Ilkka Koskinen <ilkka@os.amperecomputing.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>

---
v2: No change
---
 drivers/perf/arm_cspmu/arm_cspmu.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/perf/arm_cspmu/arm_cspmu.c b/drivers/perf/arm_cspmu/arm_cspmu.c
index caad9a36651d..ad82de166dd1 100644
--- a/drivers/perf/arm_cspmu/arm_cspmu.c
+++ b/drivers/perf/arm_cspmu/arm_cspmu.c
@@ -1256,8 +1256,11 @@ static int arm_cspmu_device_probe(struct platform_device *pdev)
 		return ret;
 
 	ret = arm_cspmu_request_irq(cspmu);
-	if (ret)
-		return ret;
+	if (ret) {
+		if (counter_size(cspmu) < 64)
+			return ret;
+		dev_info(cspmu->dev, "Continuing without IRQ\n");
+	}
 
 	ret = arm_cspmu_get_cpus(cspmu);
 	if (ret)
-- 
2.54.0.dirty