perf/arm_cspmu: Miscellaneous improvements

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 <[email protected]>

Cc Hanjun Guo <[email protected]>

Cc Sudeep Holla <[email protected]>

Signed-off-by Robin Murphy <[email protected]>

---
  drivers/acpi/arm64/apmt.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

drivers/acpi/arm64/apmt.c +0 -0

index bb010f6164e5..ba48fcb26d52 100644

drivers/acpi/arm64/apmt.c +0 -0

  	if (!pdev)
  		return -ENOMEM;
 

Quoted 3 lines · 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.

Quoted 9 lines

Cc Lorenzo Pieralisi <[email protected]>

Cc Hanjun Guo <[email protected]>

Cc Sudeep Holla <[email protected]>

Signed-off-by Robin Murphy <[email protected]>

---
  drivers/acpi/arm64/apmt.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

drivers/acpi/arm64/apmt.c static int __init apmt_add_platform_device(struct · line 83

Quoted 10 lines

drivers/acpi/arm64/apmt.c +1 -0

index bb010f6164e5..ba48fcb26d52 100644
@@ -79,7 +79,7 @@ static int __init apmt_add_platform_device(struct 
 acpi_apmt_node *node,
  	int ret, count;
  	struct resource res[DEV_MAX_RESOURCE_COUNT];
  -	pdev = platform_device_alloc(DEV_NAME, PLATFORM_DEVID_AUTO);
+	pdev = platform_device_alloc(DEV_NAME, node->id);

Per the Sashiko comment, while I really wouldn't expect it to be an issue in practice, I guess we could explicitly check and truncate node->id to INT_MAX (probably with some degree of warning) here just in case.

Cheers,
Robin.

Quoted 3 lines

drivers/acpi/arm64/apmt.c +0 -0

  	if (!pdev)
  		return -ENOMEM;

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 <[email protected]>

---
  drivers/perf/arm_cspmu/arm_cspmu.c | 36 +++++++++++++++++-------------
  1 file changed, 21 insertions(+), 15 deletions(-)

drivers/perf/arm_cspmu/arm_cspmu.c +13 -14

index 80fb314d5135..0570be74d11f 100644
@@ -250,38 +250,44 @@ 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;

drivers/perf/arm_cspmu/arm_cspmu.c +0 -0

  }
 
  static ssize_t arm_cspmu_cpumask_show(struct device *dev,

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

Signed-off-by Robin Murphy <[email protected]>

---
drivers/perf/arm_cspmu/arm_cspmu.c | 6 ++++--

1 file changed, 4 insertions(+), 2 deletions(-)

drivers/perf/arm_cspmu/arm_cspmu.c +4 -2

index 0570be74d11f..0b26a1588eba 100644
@@ -438,13 +438,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);

Signature · 1 line

2.54.0.dirty

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.

Signed-off-by Robin Murphy <[email protected]>

---
drivers/perf/arm_cspmu/arm_cspmu.c | 7 +++++--

1 file changed, 5 insertions(+), 2 deletions(-)

drivers/perf/arm_cspmu/arm_cspmu.c +5 -2

index 0b26a1588eba..4ba47bccc99c 100644
@@ -1257,8 +1257,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)

Signature · 1 line

2.54.0.dirty