Re: [PATCH 13/13] arm64: dts: apple: t8140: 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: t8140: missing aic affinities node for pmu-e and pmu-p
--

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

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

This commit adds the pmu-e and pmu-p device nodes for the efficiency and
performance core PMUs to the Apple t8140 device tree.

> diff --git a/arch/arm64/boot/dts/apple/t8140.dtsi b/arch/arm64/boot/dts/apple/t8140.dtsi
> index 150d91bdf6fcc..6e6fd5754406b 100644
> --- a/arch/arm64/boot/dts/apple/t8140.dtsi
> +++ b/arch/arm64/boot/dts/apple/t8140.dtsi
> @@ -136,6 +136,18 @@ l2_cache_1: l2-cache-1 {
>  		};
>  	};
>  
> +	pmu-e {
> +		compatible = "apple,tahiti-e-pmu";
> +		interrupt-parent = <&aic>;
> +		interrupts = <AIC_FIQ AIC_CPU_PMU_E IRQ_TYPE_LEVEL_HIGH>;
> +	};
> +
> +	pmu-p {
> +		compatible = "apple,tahiti-p-pmu";
> +		interrupt-parent = <&aic>;
> +		interrupts = <AIC_FIQ AIC_CPU_PMU_P IRQ_TYPE_LEVEL_HIGH>;
> +	};

[Severity: High]
Does adding pmu-e and pmu-p also require an affinities child node under the
aic interrupt controller in arch/arm64/boot/dts/apple/t8140.dtsi (matching
t8122.dtsi and t8132.dtsi)?

Currently, the aic node in t8140.dtsi does not define an affinities subnode:

	aic: interrupt-controller@301000000 {
		compatible = "apple,t8140-aic3", "apple,t8122-aic3";
		reg = <0x3 0x01000000 0x0 0x1cc000>,
		      <0x3 0x01040000 0x0 0x4000>;
		reg-names = "core", "event";
		#interrupt-cells = <3>;
		interrupt-controller;
		power-domains = <&ps_aic>;
	};

Without the affinities node, aic_of_ic_init() in
drivers/irqchip/irq-apple-aic.c leaves aic_irqc->fiq_aff[AIC_CPU_PMU_E] and
aic_irqc->fiq_aff[AIC_CPU_PMU_P] as NULL. When arm_pmu_device_probe() ->
pmu_parse_irqs() -> platform_get_irq_affinity() queries the interrupt
affinity, aic_irq_get_fwspec_info() falls back to cpu_possible_mask for both
PMUs:

	if (aic_irqc->fiq_aff[intid])
		mask = &aic_irqc->fiq_aff[intid]->aff;
	else
		mask = cpu_possible_mask;

This causes pmu_parse_percpu_irq() to populate pmu->supported_cpus with all
CPUs (both E-cores and P-cores) on both PMU instances, breaking heterogeneous
CPU filtering in armpmu_filter() and armpmu_event_init().

In addition, because aic_irq_domain_translate() merges AIC_CPU_PMU_E onto
AIC_CPU_PMU_P so both PMUs share the same Linux virq, the second PMU to probe
sees armpmu_count_irq_users(affinity, irq) != 0 in armpmu_request_irq()
(drivers/perf/arm_pmu.c) and skips registering its per-CPU interrupt handler:

	} else if (armpmu_count_irq_users(affinity, irq) == 0) {
		err = request_percpu_nmi(irq, handler, "arm-pmu", affinity, pcpu_armpmu);

		/* If cannot get an NMI, get a normal interrupt */
		if (err) {
			err = request_percpu_irq_affinity(irq, handler, "arm-pmu",
							  affinity, pcpu_armpmu);
			irq_ops = &percpu_pmuirq_ops;
		} else {
			has_nmi = true;
			irq_ops = &percpu_pmunmi_ops;
		}
	} else {
		/* Per cpudevid irq was already requested by another CPU */
		irq_ops = armpmu_find_irq_ops(affinity, irq);

When a P-core PMU counter overflow FIQ fires, handle_percpu_devid_irq()
dispatches it to pmu-e's handler instead of pmu-p's, leaving the P-core
counter unhandled and triggering an FIQ storm.

Should the aic node in t8140.dtsi include an affinities subnode mapping
AIC_CPU_PMU_E to <&cpu_e0>, <&cpu_e1>, <&cpu_e2>, <&cpu_e3> and AIC_CPU_PMU_P
to <&cpu_p0>, <&cpu_p1>?

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