linux-kernel - Re: [PATCH] arm64: fix cleared E0POE bit after cpu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aV4uDW2p+WBCitwl@e129823.arm.com>
Date: Wed, 7 Jan 2026 09:57:33 +0000
From: Yeoreum Yun <yeoreum.yun@....com>
To: Kevin Brodsky <kevin.brodsky@....com>
Cc: linux-pm@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-kernel@...r.kernel.org, rafael@...nel.org, pavel@...nel.org,
	catalin.marinas@....com, will@...nel.org, anshuman.khandual@....com,
	ryan.roberts@....com, yang@...amperecomputing.com,
	joey.gouly@....com
Subject: Re: [PATCH] arm64: fix cleared E0POE bit after cpu_suspend()/resume()

Hi Kevin,

> On 05/01/2026 21:07, Yeoreum Yun wrote:
> > TCR2_ELx.E0POE is set during smp_init().
> > However, this bit is not reprogrammed when the CPU enters suspension and
> > later resumes via cpu_resume(), as __cpu_setup() does not re-enable E0POE
> > and there is no save/restore logic for the TCR2_ELx system register.
> >
> > As a result, the E0POE feature no longer works after cpu_resume().
> >
> > To address this, save and restore TCR2_EL1 in the cpu_suspend()/cpu_resume()
> > path, rather than adding related logic to __cpu_setup(), taking into account
> > possible future extensions of the TCR2_ELx feature.
>
> This is a very good catch!
>
> > Fixes: bf83dae90fbc ("arm64: enable the Permission Overlay Extension for EL0")
>
> We should also Cc: stable@...r.kernel.org as this should be backported
> to stable kernels that support POE.
>

Okay. I'll add it for next.

[...]
> > @@ -97,8 +97,11 @@ SYM_FUNC_START(cpu_do_suspend)
> >  	mrs	x9, mdscr_el1
> >  	mrs	x10, oslsr_el1
> >  	mrs	x11, sctlr_el1
> > -	get_this_cpu_offset x12
> > -	mrs	x13, sp_el0
> > +alternative_if ARM64_HAS_TCR2
> > +	mrs	x12, REG_TCR2_EL1
> > +alternative_else_nop_endif
> > +	get_this_cpu_offset x13
> > +	mrs	x14, sp_el0
> >  	stp	x2, x3, [x0]
> >  	stp	x4, x5, [x0, #16]
> >  	stp	x6, x7, [x0, #32]
> > @@ -109,7 +112,7 @@ SYM_FUNC_START(cpu_do_suspend)
> >  	 * Save x18 as it may be used as a platform register, e.g. by shadow
> >  	 * call stack.
> >  	 */
> > -	str	x18, [x0, #96]
> > +	stp	x14, x18, [x0, #96]
>
> If TCR2_EL1 isn't supported, we store and reload an unused arbitrary
> value. I think it'd be better to make it all conditional and add it at
> the end, something like:
>
> ï¿½ ï¿½ alternative_if ARM64_HAS_TCR2
> ï¿½ ï¿½ ï¿½ ï¿½ mrsï¿½ ï¿½ x2, REG_TCR2_EL1
> ï¿½ ï¿½ ï¿½ ï¿½ strï¿½ ï¿½ x2, [x0, #104]
> ï¿½ ï¿½ alternative_else_nop_endif
>
> Same idea on the resume path. This also avoids the noise of renaming
> existing registers.

IMHO, I think it would be better to sustain the change since
it seems more simpler to maintain  and x12 is temporary regsiter
so leaking whatever was in x12 does not really feel like a concern...

Am I missing something?

Thanks!

--
Sincerely,
Yeoreum Yun