linux-kernel - Re: [PATCH v2 2/6] arm64: initialise SCTLR2

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: <aJpDQ71/f9zdwXbf@e129823.arm.com>
Date: Mon, 11 Aug 2025 20:23:47 +0100
From: Yeoreum Yun <yeoreum.yun@....com>
To: Marc Zyngier <maz@...nel.org>
Cc: catalin.marinas@....com, will@...nel.org, broonie@...nel.org,
	oliver.upton@...ux.dev, anshuman.khandual@....com, robh@...nel.org,
	james.morse@....com, mark.rutland@....com, joey.gouly@....com,
	Dave.Martin@....com, ahmed.genidi@....com, kevin.brodsky@....com,
	scott@...amperecomputing.com, mbenes@...e.cz,
	james.clark@...aro.org, frederic@...nel.org, rafael@...nel.org,
	pavel@...nel.org, ryan.roberts@....com, suzuki.poulose@....com,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	linux-pm@...r.kernel.org, kvmarm@...ts.linux.dev
Subject: Re: [PATCH v2 2/6] arm64: initialise SCTLR2_ELx register at boot time

Hi Marc,

> [dropping ry111@...111.site, which bounces]
>
> On Mon, 11 Aug 2025 17:33:36 +0100,
> Yeoreum Yun <yeoreum.yun@....com> wrote:
> >
> > add initialisation for SCTRL2_ELx register at boot time.
>
> Again, please expand.

Okay.


[...]
> >
> > +.macro __init_el2_sctlr2
>
> Writing this as __init_sctlr2_el2 would read vastly better (yes, I
> know most macros in this file are similarly braindead).
>
> > +	.macro init_sctlr2_elx, el, tmp
> > +	mrs_s	\tmp, SYS_ID_AA64MMFR3_EL1
> > +	ubfx	\tmp, \tmp, #ID_AA64MMFR3_EL1_SCTLRX_SHIFT, #4
> > +	cbz	\tmp, .Lskip_sctlr2_\@
> > +	.if	\el == 2
> > +	mov_q	\tmp, INIT_SCTLR2_EL2
> > +	msr_s	SYS_SCTLR_EL2, \tmp
> > +	.else
> > +	mov_q	\tmp, INIT_SCTLR2_EL1
> > +	.if	\el == 12
> > +	msr_s	SYS_SCTLR_EL12, \tmp
> > +	.else
> > +	msr_s	SYS_SCTLR_EL1, \tmp
> > +	.endif
>
> I don't think this is the correct place for this macro.
> asm/assembler.h seems more suitable, and already has that sort of
> things.

Thanks for letting me know the proper place.
I'll rewrite it on there.

>
> > +	.endif
> > +.Lskip_sctlr2_\@:
> > +	.endm
> >  #else
> >
> >  #include <linux/bitfield.h>
> > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > index ca04b338cb0d..0dff7593e50b 100644
> > --- a/arch/arm64/kernel/head.S
> > +++ b/arch/arm64/kernel/head.S
> > @@ -276,6 +276,7 @@ SYM_INNER_LABEL(init_el1, SYM_L_LOCAL)
> >  	mov_q	x0, INIT_SCTLR_EL1_MMU_OFF
> >  	pre_disable_mmu_workaround
> >  	msr	sctlr_el1, x0
> > +	init_sctlr2_elx	1, x0
> >  	isb
> >  	mov_q	x0, INIT_PSTATE_EL1
> >  	msr	spsr_el1, x0
> > @@ -298,7 +299,6 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
> >  	msr	sctlr_el2, x0
> >  	isb
> >  0:
> > -
> >  	init_el2_hcr	HCR_HOST_NVHE_FLAGS
> >  	init_el2_state
> >
> > @@ -315,12 +315,15 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
> >
> >  	/* Set a sane SCTLR_EL1, the VHE way */
> >  	msr_s	SYS_SCTLR_EL12, x1
> > +	init_sctlr2_elx	12, x2
> >  	mov	x2, #BOOT_CPU_FLAG_E2H
> >  	b	3f
> >
> >  2:
> >  	msr	sctlr_el1, x1
> > +	init_sctlr2_elx	1, x2
> >  	mov	x2, xzr
> > +
> >  3:
> >  	mov	x0, #INIT_PSTATE_EL1
> >  	msr	spsr_el2, x0
>
> This is missing something: you should resynchronise SCTLR2_EL2 from
> SCTLR2_EL1 in __finalise_el2, rather than relying on whatever you've
> set in __init_el2_sctlr2.

Thanks. I'll add missing on enter_vhe().

--
Sincerely,
Yeoreum Yun