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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] perf: arm_pmu_acpi: GSI leak in arm_acpi_register_group() when shared
--

commit 07d4d2d60385c80c0a1df910cfddbd2fb128123f
Author: Johnny.Fan <Johnny.Fan@cixtech.com>

perf: arm_spe: Support heterogeneous ACPI systems

This commit updates ACPI registration for ARM SPE and TRBE to group CPUs by
hetero ID and interrupt number, registering one platform device per group
and passing the group's CPU mask via platform data.

> diff --git a/drivers/perf/arm_pmu_acpi.c b/drivers/perf/arm_pmu_acpi.c
> index ca6bf8f86cfe9..fdf39673648f3 100644
> --- a/drivers/perf/arm_pmu_acpi.c
> +++ b/drivers/perf/arm_pmu_acpi.c
[ ... ]
> +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);
> +
> +	return ret;
> +}
> +
> +static int __maybe_unused
> +arm_acpi_register_pmu_devices(const char *name, u8 len,
> +			      u16 (*parse_gsi)(struct acpi_madt_generic_interrupt *))
> +{
[ ... ]
> +	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++;
> +		}
> +	}

[Severity: Medium]
Can this leak the GSI mapping if all groups sharing an interrupt fail to
register their platform devices?

In drivers/perf/arm_pmu_acpi.c, arm_acpi_register_pmu_devices() calls
arm_acpi_gsi_is_shared(), which checks whether any other entry in groups[]
has the same gsi, regardless of whether any of those groups actually
succeeded in registering a platform device.

When two or more groups share the same gsi, arm_acpi_gsi_is_shared() returns
true for every group in that set. If the first group creates the mapping via
acpi_register_gsi() in arm_acpi_register_group() and then fails in
platform_device_alloc(), platform_device_add_resources(),
platform_device_add_data(), or platform_device_add(), the err_unreg path sees
gsi_shared as true and skips acpi_unregister_gsi(group->gsi). If the
remaining groups sharing that gsi also fail during registration, none of them
will call acpi_unregister_gsi(group->gsi) either, leaving the GSI mapping
registered with no platform device using it.

Would it make sense to track whether another group sharing the gsi has
already registered successfully, or clean up the mapping if all groups
sharing that gsi fail?

[ ... ]

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