[PATCH] perf: arm_spe: Support heterogeneous ACPI systems

[PATCH] perf: arm_spe: Support heterogeneous ACPI systems

[Johnny.Fan]

From: "Johnny.Fan" <Johnny.Fan@cixtech.com>

The ACPI MADT describes the SPE overflow interrupt on a per-CPU basis
in the GICC structure, where a CPU that does not implement SPE simply
reports 0. Heterogeneous systems therefore only report the interrupt
on the CPUs that implement SPE, but arm_pmu_acpi.c rejects this as a
non-homogeneous mapping and never creates the SPE platform device.

Treat the SPE interrupt as optional when walking the MADT: skip the
GICCs that report 0 and only require that the CPUs which do report it
agree on the number.

Note that the CPUs reporting the interrupt must still share a single
GSI, as the platform device only exposes one IRQ resource. A system
that assigns a different PPI per cluster cannot be described by this
single-device model.

The CPUs without SPE are trimmed from the PMU supported CPUs by
probing PMSVer on each CPU, both at probe time (for online CPUs) and
from the hotplug callback (for CPUs that were offline at probe), as
ACPI conveys no affinity information and the PPI otherwise defaults
to all possible CPUs.

Tested on a CIX sky1 SoC with heterogeneous SPE support.

Signed-off-by: Johnny.Fan <Johnny.Fan@cixtech.com>

Reviewed-by: Fugang Duan <fugang.duan@cixtech.com>
---
 drivers/perf/arm_pmu_acpi.c | 34 +++++++++++++++++++++++-------
 drivers/perf/arm_spe_pmu.c  | 41 ++++++++++++++++++++++++++++++++++---
 2 files changed, 65 insertions(+), 10 deletions(-)

diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
index ca6bf8f86cfe..4ece1070827b 100644
--- a/drivers/perf/arm_pmu_acpi.c
+++ b/drivers/perf/arm_pmu_acpi.c
@@ -70,7 +70,8 @@ static void arm_pmu_acpi_unregister_irq(int cpu)
 
 static int __maybe_unused
 arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len,
-			     u16 (*parse_gsi)(struct acpi_madt_generic_interrupt *))
+			     u16 (*parse_gsi)(struct acpi_madt_generic_interrupt *),
+			     bool optional)
 {
 	int cpu, this_hetid, hetid, irq, ret;
 	u16 this_gsi = 0, gsi = 0;
@@ -87,7 +88,9 @@ arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len,
 
 	/*
 	 * Sanity check all the GICC tables for the same interrupt
-	 * number. For now, only support homogeneous ACPI machines.
+	 * number. For now, only support homogeneous ACPI machines,
+	 * unless the interrupt is optional, in which case a CPU that
+	 * does not implement the feature simply reports 0.
 	 */
 	for_each_possible_cpu(cpu) {
 		struct acpi_madt_generic_interrupt *gicc;
@@ -97,6 +100,20 @@ arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len,
 			return gsi ? -ENXIO : 0;
 
 		this_gsi = parse_gsi(gicc);
+
+		if (optional) {
+			if (!this_gsi)
+				continue;
+
+			if (gsi && gsi != this_gsi) {
+				pr_warn("ACPI: %s: CPU%d: inconsistent interrupt %u/%u\n",
+					pdev->name, cpu, gsi, this_gsi);
+				return -ENXIO;
+			}
+			gsi = this_gsi;
+			continue;
+		}
+
 		this_hetid = find_acpi_cpu_topology_hetero_id(cpu);
 		if (!gsi) {
 			hetid = this_hetid;
@@ -107,7 +124,7 @@ arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len,
 		}
 	}
 
-	if (!this_gsi)
+	if (!gsi)
 		return 0;
 
 	irq = acpi_register_gsi(NULL, gsi, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_HIGH);
@@ -146,13 +163,16 @@ static u16 arm_spe_parse_gsi(struct acpi_madt_generic_interrupt *gicc)
 
 /*
  * For lack of a better place, hook the normal PMU MADT walk
- * and create a SPE device if we detect a recent MADT with
- * a homogeneous PPI mapping.
+ * and create a SPE device if we detect a recent MADT with a PPI
+ * mapping. The SPE overflow interrupt is described per CPU, so a
+ * GICC reporting 0 simply means that the CPU does not implement
+ * SPE; the arm_spe_pmu driver trims such CPUs from its supported
+ * CPUs using PMSVer.
  */
 static void arm_spe_acpi_register_device(void)
 {
 	int ret = arm_acpi_register_pmu_device(&spe_dev, ACPI_MADT_GICC_SPE,
-					       arm_spe_parse_gsi);
+					       arm_spe_parse_gsi, true);
 	if (ret)
 		pr_warn("ACPI: SPE: Unable to register device\n");
 }
@@ -185,7 +205,7 @@ static u16 arm_trbe_parse_gsi(struct acpi_madt_generic_interrupt *gicc)
 static void arm_trbe_acpi_register_device(void)
 {
 	int ret = arm_acpi_register_pmu_device(&trbe_dev, ACPI_MADT_GICC_TRBE,
-					       arm_trbe_parse_gsi);
+					       arm_trbe_parse_gsi, false);
 	if (ret)
 		pr_warn("ACPI: TRBE: Unable to register device\n");
 }
diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
index b70f3fedf028..6f1bacc78b1c 100644
--- a/drivers/perf/arm_spe_pmu.c
+++ b/drivers/perf/arm_spe_pmu.c
@@ -1102,6 +1102,21 @@ static void arm_spe_pmu_perf_destroy(struct arm_spe_pmu *spe_pmu)
 	perf_pmu_unregister(&spe_pmu->pmu);
 }
 
+static unsigned int arm_spe_pmu_pmsver(void)
+{
+	return cpuid_feature_extract_unsigned_field(read_cpuid(ID_AA64DFR0_EL1),
+						    ID_AA64DFR0_EL1_PMSVer_SHIFT);
+}
+
+static void __arm_spe_pmu_trim_supported_cpus(void *info)
+{
+	struct arm_spe_pmu *spe_pmu = info;
+
+	if (!arm_spe_pmu_pmsver())
+		cpumask_clear_cpu(smp_processor_id(),
+				  &spe_pmu->supported_cpus);
+}
+
 static void __arm_spe_pmu_dev_probe(void *info)
 {
 	int fld;
@@ -1109,8 +1124,7 @@ static void __arm_spe_pmu_dev_probe(void *info)
 	struct arm_spe_pmu *spe_pmu = info;
 	struct device *dev = &spe_pmu->pdev->dev;
 
-	fld = cpuid_feature_extract_unsigned_field(read_cpuid(ID_AA64DFR0_EL1),
-						   ID_AA64DFR0_EL1_PMSVer_SHIFT);
+	fld = arm_spe_pmu_pmsver();
 	if (!fld) {
 		dev_err(dev,
 			"unsupported ID_AA64DFR0_EL1.PMSVer [%d] on CPU %d\n",
@@ -1275,6 +1289,12 @@ static int arm_spe_pmu_cpu_startup(unsigned int cpu, struct hlist_node *node)
 	if (!cpumask_test_cpu(cpu, &spe_pmu->supported_cpus))
 		return 0;
 
+	/* The CPU was offline during probe; trim it if it lacks SPE */
+	if (!arm_spe_pmu_pmsver()) {
+		cpumask_clear_cpu(cpu, &spe_pmu->supported_cpus);
+		return 0;
+	}
+
 	__arm_spe_pmu_setup_one(spe_pmu);
 	return 0;
 }
@@ -1331,7 +1351,7 @@ static int arm_spe_pmu_irq_probe(struct arm_spe_pmu *spe_pmu)
 {
 	struct platform_device *pdev = spe_pmu->pdev;
 	const struct cpumask *affinity;
-	int irq;
+	int cpu, irq;
 
 	irq = platform_get_irq_affinity(pdev, 0, &affinity);
 	if (irq < 0)
@@ -1344,6 +1364,21 @@ static int arm_spe_pmu_irq_probe(struct arm_spe_pmu *spe_pmu)
 
 	cpumask_copy(&spe_pmu->supported_cpus, affinity);
 
+	/*
+	 * Heterogeneous systems may only implement SPE on a subset of the
+	 * CPUs covered by the interrupt affinity (ACPI in particular has
+	 * no affinity information and falls back to cpu_possible_mask).
+	 * SPE is self-describing through PMSVer, so drop the online CPUs
+	 * that do not implement it. CPUs that are offline at probe time
+	 * are checked when they come up.
+	 */
+	for_each_cpu(cpu, &spe_pmu->supported_cpus) {
+		if (cpu_online(cpu))
+			smp_call_function_single(cpu,
+						 __arm_spe_pmu_trim_supported_cpus,
+						 spe_pmu, 1);
+	}
+
 	spe_pmu->irq = irq;
 	return 0;
 }
-- 
2.55.0

Re: [PATCH] perf: arm_spe: Support heterogeneous ACPI systems

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] perf: arm_spe: missing heterogeneity check for optional PMUs
- [Medium] perf/arm_spe: spurious probe failure due to CPU hotplug race
--

commit 9ac6f1b22e8955b2543af029311236c9a14923ec
Author: Johnny.Fan <Johnny.Fan@cixtech.com>

perf: arm_spe: Support heterogeneous ACPI systems

The ACPI MADT describes the SPE overflow interrupt on a per-CPU basis
in the GICC structure, where a CPU that does not implement SPE simply
reports 0. Heterogeneous systems therefore only report the interrupt
on the CPUs that implement SPE, but arm_pmu_acpi.c rejects this as a
non-homogeneous mapping and never creates the SPE platform device.

> diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
> index ca6bf8f86cfe9..4ece1070827b9 100644
> --- a/drivers/perf/arm_pmu_acpi.c
> +++ b/drivers/perf/arm_pmu_acpi.c
[ ... ]
> @@ -97,6 +100,20 @@ arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len,
>  			return gsi ? -ENXIO : 0;
>  
>  		this_gsi = parse_gsi(gicc);
> +
> +		if (optional) {
> +			if (!this_gsi)
> +				continue;
> +
> +			if (gsi && gsi != this_gsi) {
> +				pr_warn("ACPI: %s: CPU%d: inconsistent interrupt %u/%u\n",
> +					pdev->name, cpu, gsi, this_gsi);
> +				return -ENXIO;
> +			}
> +			gsi = this_gsi;
> +			continue;
> +		}
> +
>  		this_hetid = find_acpi_cpu_topology_hetero_id(cpu);
>  		if (!gsi) {
>  			hetid = this_hetid;

[Severity: High]
Does skipping find_acpi_cpu_topology_hetero_id() allow grouping CPUs from
different microarchitectural clusters into a single PMU device?

The SPE PMU driver (arm_spe_pmu_dev_init()) assumes a single PMU device is
homogeneous. It probes SPE parameters (e.g., align, max_record_sz, features)
on a single CPU via smp_call_function_any() and applies them to all CPUs in
spe_pmu->supported_cpus.

If heterogeneous CPUs with different SPE capabilities are grouped together,
scheduling a perf event on a CPU from a different cluster could program the
hardware with mismatched constraints or feature flags. Could the this_hetid
check be preserved for the CPUs that actually implement the optional PMU?

> diff --git a/drivers/perf/arm_spe_pmu.c b/drivers/perf/arm_spe_pmu.c
> index 604ae6ba3b076..c1ff875be9577 100644
> --- a/drivers/perf/arm_spe_pmu.c
> +++ b/drivers/perf/arm_spe_pmu.c
[ ... ]
> @@ -1277,6 +1291,12 @@ static int arm_spe_pmu_cpu_startup(unsigned int cpu, struct hlist_node *node)
>  	if (!cpumask_test_cpu(cpu, &spe_pmu->supported_cpus))
>  		return 0;
>  
> +	/* The CPU was offline during probe; trim it if it lacks SPE */
> +	if (!arm_spe_pmu_pmsver()) {
> +		cpumask_clear_cpu(cpu, &spe_pmu->supported_cpus);
> +		return 0;
> +	}
> +
>  	__arm_spe_pmu_setup_one(spe_pmu);

[Severity: High]
Is it possible for the newly online CPU to have a different version of SPE
than what was cached during the initial probe?

This startup check only verifies that the CPU has some version of SPE
(checking pmsver != 0), but does not verify that the SPE parameters match
the ones cached from the initial probe.

[ ... ]
> @@ -1346,6 +1366,21 @@ static int arm_spe_pmu_irq_probe(struct arm_spe_pmu *spe_pmu)
>  
>  	cpumask_copy(&spe_pmu->supported_cpus, affinity);
>  
> +	/*
> +	 * Heterogeneous systems may only implement SPE on a subset of the
> +	 * CPUs covered by the interrupt affinity (ACPI in particular has
> +	 * no affinity information and falls back to cpu_possible_mask).
> +	 * SPE is self-describing through PMSVer, so drop the online CPUs
> +	 * that do not implement it. CPUs that are offline at probe time
> +	 * are checked when they come up.
> +	 */
> +	for_each_cpu(cpu, &spe_pmu->supported_cpus) {
> +		if (cpu_online(cpu))
> +			smp_call_function_single(cpu,
> +						 __arm_spe_pmu_trim_supported_cpus,
> +						 spe_pmu, 1);
> +	}
> +
>  	spe_pmu->irq = irq;
>  	return 0;
>  }

[Severity: Medium]
Could this cpu_online(cpu) check without holding cpus_read_lock() lead to a
race condition with CPU hotplug?

If an offline CPU lacking SPE comes online immediately after this loop, it
could remain in supported_cpus because the hotplug callback is not yet
registered.

Later, arm_spe_pmu_dev_init() consumes this mask:

	ret = smp_call_function_any(mask,  __arm_spe_pmu_dev_probe, spe_pmu, 1);
	if (ret || !(spe_pmu->features & SPE_PMU_FEAT_DEV_PROBED))
		return -ENXIO;

This could select the newly online CPU that lacks SPE, causing the driver
probe to spuriously fail.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015062908.3565761-1-johnny.fan@cixtech.com?part=1

Re: [PATCH] perf: arm_spe: Support heterogeneous ACPI systems

[Marc Zyngier]

On Thu, 15 Oct 2026 07:29:08 +0100,
"Johnny.Fan" <johnny.fan@cixtech.com> wrote:
> 
> From: "Johnny.Fan" <Johnny.Fan@cixtech.com>
> 
> The ACPI MADT describes the SPE overflow interrupt on a per-CPU basis
> in the GICC structure, where a CPU that does not implement SPE simply
> reports 0. Heterogeneous systems therefore only report the interrupt
> on the CPUs that implement SPE, but arm_pmu_acpi.c rejects this as a
> non-homogeneous mapping and never creates the SPE platform device.
> 
> Treat the SPE interrupt as optional when walking the MADT: skip the
> GICCs that report 0 and only require that the CPUs which do report it
> agree on the number.
> 
> Note that the CPUs reporting the interrupt must still share a single
> GSI, as the platform device only exposes one IRQ resource. A system
> that assigns a different PPI per cluster cannot be described by this
> single-device model.
> 
> The CPUs without SPE are trimmed from the PMU supported CPUs by
> probing PMSVer on each CPU, both at probe time (for online CPUs) and
> from the hotplug callback (for CPUs that were offline at probe), as
> ACPI conveys no affinity information and the PPI otherwise defaults
> to all possible CPUs.

This is a bit odd: at the beginning, you are stating that the SPE
interrupt being 0 is an indication of SPE not being supported. Then
you check for PMSVer for the same properly. This seems pretty wrong.

It should be possible to build the mask of SPE-enabled CPUs without
IPI-ing the other CPUs, and only rely on the firmware table.

> 
> Tested on a CIX sky1 SoC with heterogeneous SPE support.
> 
> Signed-off-by: Johnny.Fan <Johnny.Fan@cixtech.com>
> 
> Reviewed-by: Fugang Duan <fugang.duan@cixtech.com>
> ---
>  drivers/perf/arm_pmu_acpi.c | 34 +++++++++++++++++++++++-------
>  drivers/perf/arm_spe_pmu.c  | 41 ++++++++++++++++++++++++++++++++++---
>  2 files changed, 65 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
> index ca6bf8f86cfe..4ece1070827b 100644
> --- a/drivers/perf/arm_pmu_acpi.c
> +++ b/drivers/perf/arm_pmu_acpi.c
> @@ -70,7 +70,8 @@ static void arm_pmu_acpi_unregister_irq(int cpu)
>  
>  static int __maybe_unused
>  arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len,
> -			     u16 (*parse_gsi)(struct acpi_madt_generic_interrupt *))
> +			     u16 (*parse_gsi)(struct acpi_madt_generic_interrupt *),
> +			     bool optional)
>  {
>  	int cpu, this_hetid, hetid, irq, ret;
>  	u16 this_gsi = 0, gsi = 0;
> @@ -87,7 +88,9 @@ arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len,
>  
>  	/*
>  	 * Sanity check all the GICC tables for the same interrupt
> -	 * number. For now, only support homogeneous ACPI machines.
> +	 * number. For now, only support homogeneous ACPI machines,
> +	 * unless the interrupt is optional, in which case a CPU that
> +	 * does not implement the feature simply reports 0.
>  	 */
>  	for_each_possible_cpu(cpu) {
>  		struct acpi_madt_generic_interrupt *gicc;
> @@ -97,6 +100,20 @@ arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len,
>  			return gsi ? -ENXIO : 0;
>  
>  		this_gsi = parse_gsi(gicc);
> +
> +		if (optional) {
> +			if (!this_gsi)
> +				continue;
> +
> +			if (gsi && gsi != this_gsi) {
> +				pr_warn("ACPI: %s: CPU%d: inconsistent interrupt %u/%u\n",
> +					pdev->name, cpu, gsi, this_gsi);
> +				return -ENXIO;
> +			}
> +			gsi = this_gsi;
> +			continue;
> +		}
> +

This "optional" parameter feels very wrong. We now have an
infrastructure to get a set of affinities for an interrupt (see
5324fe21ba9b7 and following patches). What this needs is ACPI to be
plumbed into this.

Unfortunately, ACPI is awkward and very badly abstracted in this area,
but that doesn't mean we should make the driver code more bizarre.

Treating SPE and TRBE differently is just as odd, and there is no
reason to do other than suiting your particular platform.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH] perf: arm_spe: Support heterogeneous ACPI systems

[Johnny Fan]

On Thu, Oct 15, 2026 at 11:49:24AM +0100, Marc Zyngier wrote:
> This is a bit odd: at the beginning, you are stating that the SPE
> interrupt being 0 is an indication of SPE not being supported. Then
> you check for PMSVer for the same properly. This seems pretty wrong.
> 
> It should be possible to build the mask of SPE-enabled CPUs without
> IPI-ing the other CPUs, and only rely on the firmware table.

Agreed. The MADT walk already tells us which CPUs implement SPE, so the
whole PMSVer probing (including the hotplug compensation for CPUs that
were offline at probe time) can go away: the driver already consumes
platform_get_irq_affinity(), so once the mask comes from the GICC
entries it needs no changes at all.

> This "optional" parameter feels very wrong. We now have an
> infrastructure to get a set of affinities for an interrupt (see
> 5324fe21ba9b7 and following patches). What this needs is ACPI to be
> plumbed into this.
> 
> Unfortunately, ACPI is awkward and very badly abstracted in this area,
> but that doesn't mean we should make the driver code more bizarre.
> 
> Treating SPE and TRBE differently is just as odd, and there is no
> reason to do other than suiting your particular platform.

Also agreed -- the TRBE overflow interrupt has the same per-CPU
semantics in the GICC, so both devices can share one code path: build
the mask of CPUs reporting a non-zero interrupt, and require a single
GSI across that mask. The current hetid check is replaced by the GSI
consistency check. 

The part I'm unsure about is how to convey the MADT-derived mask to
platform_get_irq_affinity() for these devices: they are synthesized by
arm_pmu_acpi.c without an fwnode, so neither the of_node path nor the
_CRS-based ACPI path can reach them. Would you rather see the mask
registered with the GIC domain keyed by GSI, with a fallback in
get_irq_affinity() that resolves by IRQ number, or something simpler at
the platform_device level?

Happy to implement whichever shape you prefer.

Thanks,

Johnny