[PATCH v2] perf: arm_spe: Support heterogeneous ACPI systems
Jeremy Linton
jeremy.linton at arm.com
Tue Oct 27 15:37:39 PDT 2026
Hi,
This largely looks reasonable, some comments inline.
On 10/20/26 9:03 PM, Johnny.Fan wrote:
> From: "Johnny.Fan" <Johnny.Fan at cixtech.com>
>
> On heterogeneous ACPI systems, not all CPUs implement SPE, and the
> ones that do may use different PPIs. The MADT describes this per
> GICC entry: a CPU without SPE reports a zero SPE overflow interrupt,
> and the PPTT hetero id identifies CPUs of the same implementation.
>
> The current code registers a single platform device and gives up if
> the GICC entries disagree on either the hetero id or the interrupt,
> so SPE is unavailable on such systems even though the firmware
> describes it.
>
> Group the CPUs by (hetero id, SPE interrupt) and register one
> platform device per group. A GSI may be shared between groups with
> non-overlapping CPU masks, so only unregister the GSI mapping when
> no other group uses the same interrupt, as the mapping is not
> reference counted.
>
> These synthetic devices have no fwnode, so the firmware-described
> IRQ affinity cannot tell which CPUs implement the feature. Pass the
> CPU mask as platform data and have the drivers prefer it over the
> IRQ affinity when present.
>
> Apply the same treatment to TRBE, which shares this registration
> code. TRBE must also pass a stable copy of the mask to
> request_percpu_irq_affinity(), since supported_cpus is pruned as
> CPUs are probed.
>
> Tested on a CIX sky1 SoC with heterogeneous SPE support.
>
> Signed-off-by: Johnny.Fan <Johnny.Fan at cixtech.com>
> Reviewed-by: Fugang Duan <fugang.duan at cixtech.com>
> ---
> drivers/hwtracing/coresight/coresight-trbe.c | 22 +-
> drivers/perf/arm_pmu_acpi.c | 211 ++++++++++++-------
It makes sense to split the device creation portion in arm_pmu_acpi.c
into its own patch separate from the consumption IMHO.
> drivers/perf/arm_spe_pmu.c | 10 +-
> include/linux/perf/arm_pmu.h | 10 +
> 4 files changed, 176 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/hwtracing/coresight/coresight-trbe.c b/drivers/hwtracing/coresight/coresight-trbe.c
> index c7cbca45f2de..0c4bc8fbcaf3 100644
> --- a/drivers/hwtracing/coresight/coresight-trbe.c
> +++ b/drivers/hwtracing/coresight/coresight-trbe.c
> @@ -158,6 +158,7 @@ struct trbe_drvdata {
> struct hlist_node hotplug_node;
> int irq;
> cpumask_t supported_cpus;
> + cpumask_t irq_affinity;
> enum cpuhp_state trbe_online;
> struct platform_device *pdev;
> };
> @@ -1533,6 +1534,8 @@ static void arm_trbe_remove_cpuhp(struct trbe_drvdata *drvdata)
> static int arm_trbe_probe_irq(struct platform_device *pdev,
> struct trbe_drvdata *drvdata)
> {
> + const struct arm_pmu_acpi_dev_data *acpi_data =
> + dev_get_platdata(&pdev->dev);
> const struct cpumask *affinity;
> int ret;
>
> @@ -1547,14 +1550,27 @@ static int arm_trbe_probe_irq(struct platform_device *pdev,
> return -EINVAL;
> }
>
> - cpumask_copy(&drvdata->supported_cpus, affinity);
> + /*
> + * The ACPI path has no fwnode describing which CPUs implement TRBE,
> + * so it passes the mask explicitly. DT devices still use the
> + * firmware-described IRQ affinity.
> + */
> + cpumask_copy(&drvdata->supported_cpus,
> + acpi_data ? &acpi_data->cpus : affinity);
> +
> + /*
> + * supported_cpus gets pruned as CPUs are probed, so keep a stable
> + * copy for the IRQ action.
> + */
> + cpumask_copy(&drvdata->irq_affinity, &drvdata->supported_cpus);
>
> drvdata->handle = alloc_percpu(struct perf_output_handle *);
> if (!drvdata->handle)
> return -ENOMEM;
>
> - ret = request_percpu_irq_affinity(drvdata->irq, arm_trbe_irq_handler, DRVNAME,
> - affinity, drvdata->handle);
> + ret = request_percpu_irq_affinity(drvdata->irq, arm_trbe_irq_handler,
> + DRVNAME, &drvdata->irq_affinity,
> + drvdata->handle);
> if (ret) {
> free_percpu(drvdata->handle);
> return ret;
> diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
> index ca6bf8f86cfe..fdf39673648f 100644
> --- a/drivers/perf/arm_pmu_acpi.c
> +++ b/drivers/perf/arm_pmu_acpi.c
> @@ -12,6 +12,8 @@
> #include <linux/irqdesc.h>
> #include <linux/percpu.h>
> #include <linux/perf/arm_pmu.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
>
> #include <asm/cpu.h>
> #include <asm/cputype.h>
> @@ -68,91 +70,165 @@ static void arm_pmu_acpi_unregister_irq(int cpu)
> acpi_unregister_gsi(gsi);
> }
>
> -static int __maybe_unused
> -arm_acpi_register_pmu_device(struct platform_device *pdev, u8 len,
> - u16 (*parse_gsi)(struct acpi_madt_generic_interrupt *))
> +struct arm_pmu_acpi_group {
> + int hetid;
> + u16 gsi;
> + struct cpumask cpus;
> +};
> +
> +static int arm_acpi_find_group(struct arm_pmu_acpi_group *groups, int nr_groups,
> + int hetid, u16 gsi)
> {
> - int cpu, this_hetid, hetid, irq, ret;
> - u16 this_gsi = 0, gsi = 0;
> + int i;
>
> - /*
> - * Ensure that platform device must have IORESOURCE_IRQ
> - * resource to hold gsi interrupt.
> - */
> - if (pdev->num_resources != 1)
> - return -ENXIO;
> + for (i = 0; i < nr_groups; i++) {
> + if (groups[i].hetid == hetid && groups[i].gsi == gsi)
> + return i;
> + }
> +
> + return -1;
> +}
> +
> +static bool arm_acpi_gsi_is_shared(struct arm_pmu_acpi_group *groups,
> + int nr_groups, int idx)
> +{
> + int i;
> +
> + for (i = 0; i < nr_groups; i++) {
> + if (i != idx && groups[i].gsi == groups[idx].gsi)
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static int arm_acpi_register_group(struct arm_pmu_acpi_group *group,
> + const char *name, bool gsi_shared)
> +{
> + struct arm_pmu_acpi_dev_data data = { };
> + struct platform_device *pdev;
> + struct resource res;
> + int irq, ret;
> +
> + irq = acpi_register_gsi(NULL, group->gsi, ACPI_LEVEL_SENSITIVE,
> + ACPI_ACTIVE_HIGH);
> + if (irq < 0)
> + return irq;
> +
> + pdev = platform_device_alloc(name, PLATFORM_DEVID_AUTO);
> + if (!pdev) {
> + ret = -ENOMEM;
> + goto err_unreg;
> + }
>
> - if (pdev->resource[0].flags != IORESOURCE_IRQ)
> - return -ENXIO;
> + res = DEFINE_RES_IRQ(irq);
> + data.cpus = group->cpus;
>
> + ret = platform_device_add_resources(pdev, &res, 1);
> + if (ret)
> + goto err_put;
> +
> + ret = platform_device_add_data(pdev, &data, sizeof(data));
> + if (ret)
> + goto err_put;
> +
> + ret = platform_device_add(pdev);
> + if (!ret)
> + return 0;
> +
> +err_put:
> + platform_device_put(pdev);
> +err_unreg:
> /*
> - * Sanity check all the GICC tables for the same interrupt
> - * number. For now, only support homogeneous ACPI machines.
> + * The GSI mapping is not reference counted, so only drop it if no
> + * other group shares it.
> */
> + if (!gsi_shared)
> + acpi_unregister_gsi(group->gsi);
This is a bit odd, if whatever fails, and gets here happens on all the
paths associated with the shared GSI, then that GSI is never
unregistered and the resource is leaked.
> +
> + return ret;
> +}
> +
> +static int __maybe_unused
> +arm_acpi_register_pmu_devices(const char *name, u8 len,
> + u16 (*parse_gsi)(struct acpi_madt_generic_interrupt *))
> +{
> + struct arm_pmu_acpi_group *groups;
> + int cpu, i, nr_cpus = 0, nr_groups = 0, nr_devices = 0;
> + int last_err = 0, ret;
> +
> for_each_possible_cpu(cpu) {
> struct acpi_madt_generic_interrupt *gicc;
>
> gicc = acpi_cpu_get_madt_gicc(cpu);
> if (gicc->header.length < len)
> - return gsi ? -ENXIO : 0;
> -
> - this_gsi = parse_gsi(gicc);
> - this_hetid = find_acpi_cpu_topology_hetero_id(cpu);
> - if (!gsi) {
> - hetid = this_hetid;
> - gsi = this_gsi;
> - } else if (hetid != this_hetid || gsi != this_gsi) {
> - pr_warn("ACPI: %s: must be homogeneous\n", pdev->name);
> - return -ENXIO;
> - }
> + continue;
This continue seems like a mistake, if the firmware has provided
unreliable tables we should abort all device creation, even if that
means some subset of the SPE devices which appear to be correct aren't
created.
> + if (parse_gsi(gicc))
> + nr_cpus++;
> }
>
> - if (!this_gsi)
> + if (!nr_cpus)
> return 0;
>
> - irq = acpi_register_gsi(NULL, gsi, ACPI_LEVEL_SENSITIVE, ACPI_ACTIVE_HIGH);
> - if (irq < 0) {
> - pr_warn("ACPI: %s Unable to register interrupt: %d\n", pdev->name, gsi);
> - return -ENXIO;
> - }
> + groups = kcalloc(nr_cpus, sizeof(*groups), GFP_KERNEL);
And this seemed a bit chunky because in theory this could be NR_CPUS,
which made me initially say, skip counting the cores and just allocate
the max, then I realized its close to having kcalloc() failures if there
are a lot of cores in the machine since this allocation becomes multiple
MB. I think we are creating a future headache and need something smarter
than counting the number of cores with valid SPE/TRBE interrupts.
> + if (!groups)
> + return -ENOMEM;
>
> - pdev->resource[0].start = irq;
> - ret = platform_device_register(pdev);
> - if (ret)
> - acpi_unregister_gsi(gsi);
> + for_each_possible_cpu(cpu) {
> + struct acpi_madt_generic_interrupt *gicc;
> + int hetid;
> + u16 gsi;
>
> - return ret;
> -}
> + gicc = acpi_cpu_get_madt_gicc(cpu);
> + if (gicc->header.length < len)
> + continue;
same continue issue here as above>
> -#if IS_ENABLED(CONFIG_ARM_SPE_PMU)
> -static struct resource spe_resources[] = {
> - {
> - /* irq */
> - .flags = IORESOURCE_IRQ,
> + gsi = parse_gsi(gicc);
> + if (!gsi)
> + continue;
> +
> + hetid = find_acpi_cpu_topology_hetero_id(cpu);
If groups were say a linked list (or something else dynamic) you could
just use the failure case here to allocate the next entry and simply
skip the first loop iteration above counting cpus.
> + i = arm_acpi_find_group(groups, nr_groups, hetid, gsi);
> + if (i < 0) {
> + i = nr_groups++;
> + groups[i].hetid = hetid;
> + groups[i].gsi = gsi;
> + }
> +
> + cpumask_set_cpu(cpu, &groups[i].cpus);
> }
> -};
>
> -static struct platform_device spe_dev = {
> - .name = ARMV8_SPE_PDEV_NAME,
> - .id = -1,
> - .resource = spe_resources,
> - .num_resources = ARRAY_SIZE(spe_resources)
> -};
> + for (i = 0; i < nr_groups; i++) {
> + ret = arm_acpi_register_group(&groups[i], name,
> + arm_acpi_gsi_is_shared(groups,
> + nr_groups, i));
> + if (ret) {
> + pr_warn("ACPI: %s: %*pbl: add device failed (%d)\n",
> + name, cpumask_pr_args(&groups[i].cpus), ret);
> + last_err = ret;
> + } else {
> + nr_devices++;
> + }
> + }
> +
> + kfree(groups);
> +
> + return nr_devices ? 0 : last_err;
> +}
>
> +#if IS_ENABLED(CONFIG_ARM_SPE_PMU)
> static u16 arm_spe_parse_gsi(struct acpi_madt_generic_interrupt *gicc)
> {
> return gicc->spe_interrupt;
> }
>
> -/*
> - * 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.
> - */
> 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);
> + int ret;
> +
> + ret = arm_acpi_register_pmu_devices(ARMV8_SPE_PDEV_NAME,
> + ACPI_MADT_GICC_SPE,
> + arm_spe_parse_gsi);
> if (ret)
> pr_warn("ACPI: SPE: Unable to register device\n");
> }
> @@ -163,20 +239,6 @@ static inline void arm_spe_acpi_register_device(void)
> #endif /* CONFIG_ARM_SPE_PMU */
>
> #if IS_ENABLED(CONFIG_CORESIGHT_TRBE)
> -static struct resource trbe_resources[] = {
> - {
> - /* irq */
> - .flags = IORESOURCE_IRQ,
> - }
> -};
> -
> -static struct platform_device trbe_dev = {
> - .name = ARMV8_TRBE_PDEV_NAME,
> - .id = -1,
> - .resource = trbe_resources,
> - .num_resources = ARRAY_SIZE(trbe_resources)
> -};
> -
> static u16 arm_trbe_parse_gsi(struct acpi_madt_generic_interrupt *gicc)
> {
> return gicc->trbe_interrupt;
> @@ -184,8 +246,11 @@ 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);
> + int ret;
> +
> + ret = arm_acpi_register_pmu_devices(ARMV8_TRBE_PDEV_NAME,
> + ACPI_MADT_GICC_TRBE,
> + arm_trbe_parse_gsi);
> 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..712b7f187301 100644
> --- a/drivers/perf/arm_spe_pmu.c
> +++ b/drivers/perf/arm_spe_pmu.c
> @@ -1330,6 +1330,8 @@ static void arm_spe_pmu_dev_teardown(struct arm_spe_pmu *spe_pmu)
> static int arm_spe_pmu_irq_probe(struct arm_spe_pmu *spe_pmu)
> {
> struct platform_device *pdev = spe_pmu->pdev;
> + const struct arm_pmu_acpi_dev_data *acpi_data =
> + dev_get_platdata(&pdev->dev);
> const struct cpumask *affinity;
> int irq;
>
> @@ -1342,7 +1344,13 @@ static int arm_spe_pmu_irq_probe(struct arm_spe_pmu *spe_pmu)
> return -EINVAL;
> }
>
> - cpumask_copy(&spe_pmu->supported_cpus, affinity);
> + /*
> + * The ACPI path has no fwnode describing which CPUs implement SPE,
> + * so it passes the mask explicitly. DT devices still use the
> + * firmware-described IRQ affinity.
> + */
> + cpumask_copy(&spe_pmu->supported_cpus,
> + acpi_data ? &acpi_data->cpus : affinity);
>
> spe_pmu->irq = irq;
> return 0;
> diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
> index 02d2c7f45b52..7f1c03c55025 100644
> --- a/include/linux/perf/arm_pmu.h
> +++ b/include/linux/perf/arm_pmu.h
> @@ -8,6 +8,7 @@
> #ifndef __ARM_PMU_H__
> #define __ARM_PMU_H__
>
> +#include <linux/cpumask.h>
> #include <linux/interrupt.h>
> #include <linux/perf_event.h>
> #include <linux/platform_device.h>
> @@ -199,6 +200,15 @@ void armpmu_free_irq(struct arm_pmu * __percpu *armpmu, int irq, int cpu);
> #define ARMV8_SPE_PDEV_NAME "arm,spe-v1"
> #define ARMV8_TRBE_PDEV_NAME "arm,trbe"
>
> +/*
> + * Per-device CPU mask passed by the ACPI MADT walk. The synthetic
> + * platform devices have no fwnode, so the IRQ affinity cannot describe
> + * which CPUs actually implement the feature.
> + */
> +struct arm_pmu_acpi_dev_data {
> + struct cpumask cpus;
> +};
> +
> /* Why does everything I do descend into this? */
> #define __GEN_PMU_FORMAT_ATTR(cfg, lo, hi) \
> (lo) == (hi) ? #cfg ":" #lo "\n" : #cfg ":" #lo "-" #hi
Thanks for working on this.
Jeremy
More information about the linux-arm-kernel
mailing list