Re: [PATCH 11/13] arm64: dts: apple: t604x: Add PMU Device Nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] arm64: dts: apple: t6041: 4-cell PMU interrupts with 3-cell aic parent
--

commit d0216cee20998d9364fd46a4827e945211d4e88a
Author: Myria Sarvay <myrialsarvay@gmail.com>

arm64: dts: apple: t604x: Add PMU Device Nodes

This commit adds the pmu-e and pmu-p device tree nodes for the Apple M4
Pro/Max (t6041) SoC.

> diff --git a/arch/arm64/boot/dts/apple/t6041.dtsi b/arch/arm64/boot/dts/apple/t6041.dtsi
> index 9110d05c896da..2c72972d12192 100644
> --- a/arch/arm64/boot/dts/apple/t6041.dtsi
> +++ b/arch/arm64/boot/dts/apple/t6041.dtsi
> @@ -314,6 +314,18 @@ l2_cache_2: l2-cache-2 {
>  		};
>  	};
>  
> +	pmu-e {
> +		compatible = "apple,brava-e-pmu";
> +		interrupt-parent = <&aic>;
> +		interrupts = <AIC_FIQ 0 AIC_CPU_PMU_E IRQ_TYPE_LEVEL_HIGH>;
> +	};
> +
> +	pmu-p {
> +		compatible = "apple,brava-p-pmu";
> +		interrupt-parent = <&aic>;
> +		interrupts = <AIC_FIQ 0 AIC_CPU_PMU_P IRQ_TYPE_LEVEL_HIGH>;
> +	};

[Severity: High]
Should the interrupts properties in pmu-e and pmu-p use 3 cells instead of 4?

In arch/arm64/boot/dts/apple/t6041.dtsi, the parent aic controller declares
3 interrupt cells:

	aic: interrupt-controller@502400000 {
		compatible = "apple,t6041-aic3", "apple,t8122-aic3";
		...
		#interrupt-cells = <3>;

When arm_pmu_device_probe() -> pmu_parse_irqs() ->
platform_get_irq_affinity() -> of_irq_parse_one() parses these 4-cell
specifiers against the 3-cell parent, only the first 3 cells are read into
fwspec->param[0..2] with fwspec->param_count = 3.

In drivers/irqchip/irq-apple-aic.c, both aic_irq_get_fwspec_info() and
aic_irq_domain_translate() then read param[1] (which is 0, matching
AIC_TMR_HV_PHYS) as the FIQ index and param[2] (AIC_CPU_PMU_E or
AIC_CPU_PMU_P) as the trigger type:

	if (fwspec->param_count == 3)
		intid = fwspec->param[1];
	else
		intid = fwspec->param[2];

and:

	args = &fwspec->param[1];

	if (fwspec->param_count == 4) {
		die = args[0];
		args++;
	}

As a result, the PMU affinity masks at AIC_CPU_PMU_E and AIC_CPU_PMU_P are
missed, and interrupt translation or registration fails for both PMU nodes
(returning -ENOENT in EL1, or colliding with the hyp-phys timer FIQ / failing
aic_fiq_set_type() with -EINVAL in EL2). When platform_get_irq_affinity()
fails in pmu_parse_irqs(), it triggers WARN_ON(irq <= 0) and leaves
pmu->supported_cpus empty, causing armpmu_register() to pass nr_cpu_ids to
topology_core_has_smt() and read past the end of cpu_topology[].

Should these be:

	interrupts = <AIC_FIQ AIC_CPU_PMU_E IRQ_TYPE_LEVEL_HIGH>;

and:

	interrupts = <AIC_FIQ AIC_CPU_PMU_P IRQ_TYPE_LEVEL_HIGH>;

matching the timer node in t6041.dtsi and the PMU nodes in t8132.dtsi and
t8140.dtsi?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-m3-plus-pmu-events-v1-0-a8eb6c1139fe@gmail.com?part=11