Re: [PATCH RFC 6/6] arm64: dts: apple: t8103: Add PSCI and CPU idle states

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] State leak (preempt_count) and corruption (saved_ttbr0) in `arm64_efi_psci_call` during CPU/System suspend.

Pre-existing issues:
- [High] Missing Context Synchronization Events (ISB) after writes to control-plane system registers in user access helpers.
--

--- Patch [6]: [PATCH RFC 6/6] arm64: dts: apple: t8103: Add PSCI and CPU idle states ---
commit 4618cadb25b8f3f6d74cc463901a493df3b5128c
Author: Sven Peter <sven@kernel.org>

arm64: dts: apple: t8103: Add PSCI and CPU idle states
    
Switch the base M1 from spin-table to PSCI and also add two basic idle
states that we support for now.

> --- a/arch/arm64/boot/dts/apple/t8103.dtsi
> +++ b/arch/arm64/boot/dts/apple/t8103.dtsi

[ ... ]

> @@ -185,6 +198,28 @@ l2_cache_1: l2-cache-1 {
>  			cache-unified;
>  			cache-size = <0xc00000>;
>  		};
> +
> +		idle-states {
> +			entry-method = "psci";
> +
> +			cpu_retention_wfi: cpu-retention-wfi {
> +				compatible = "arm,idle-state";
> +				idle-state-name = "wfi";
> +				arm,psci-suspend-param = <0x00000000>;
> +				entry-latency-us = <1>;
> +				exit-latency-us = <1>;
> +				min-residency-us = <1>;
> +			};
> +
> +			cpu_deep_wfi: cpu-deep-wfi {
> +				compatible = "arm,idle-state";
> +				idle-state-name = "deep-wfi";
> +				arm,psci-suspend-param = <0x00000001>;
> +				entry-latency-us = <5>;
> +				exit-latency-us = <5>;
> +				min-residency-us = <10000>;
> +			};

[Severity: High]
Does adding this deep suspend state expose a state leak in the EFI PSCI
wrapper?

When the CPU enters deep sleep via CPU_SUSPEND or SYSTEM_SUSPEND, it uses
psci_suspend_finisher which invokes arm64_efi_psci_call() in
arch/arm64/kernel/efi.c. This wrapper calls efi_virtmap_load() (which
disables preemption and swaps ttbr0 to efi_mm) and uaccess_ttbr0_enable().

On success, the firmware powers down the CPU and never returns to the caller
of arm64_efi_psci_call(). Upon waking, the hardware resumes execution at
pa_cpu_resume, restoring the pre-suspend CPU registers and returning directly
to cpu_suspend().

Since the return path of arm64_efi_psci_call() is bypassed,
uaccess_ttbr0_disable() and efi_virtmap_unload() are never executed. Can this
leak the preempt_count increment and leave the saved_ttbr0 page table
reference corrupted, leading to scheduling-while-atomic panics or deadlocks
upon resume?


[Severity: High]
This is a pre-existing issue, but the EFI PSCI path used by this idle state
relies on __uaccess_ttbr0_enable() and __uaccess_ttbr0_disable() in
arch/arm64/include/asm/uaccess.h, which appear to be missing required
synchronization.

In __uaccess_ttbr0_disable(), the code performs sequential writes to control
plane registers without an intervening isb():

    write_sysreg(ttbr - RESERVED_SWAPPER_OFFSET, ttbr0_el1);
    /* Set reserved ASID */
    write_sysreg(ttbr, ttbr1_el1);
    isb();

Similarly, __uaccess_ttbr0_enable() writes to ttbr1_el1 and ttbr0_el1
sequentially before the isb().

Architectural rules require an isb() immediately after each write to a
control-plane system register before any subsequent write or read-back. Does
the lack of an intervening isb() here force the pipeline to observe
architecturally undefined state?

> +		};
>  	};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-efi-psci-v1-0-9efb3abf0e4c@kernel.org?part=6