Re: [PATCH 6.6 v2 3/6] arm64: Fix early handling of FEAT_E2H0 not being implemented

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Writes to control-plane system registers are not immediately followed by an `isb()`.
--

--- Patch [3]: [PATCH 6.6 v2 3/6] arm64: Fix early handling of FEAT_E2H0 not being implemented ---
commit 2b993fe86ae9a0ae8ce53be35446a4e7f0a66876
Author: Marc Zyngier <maz@kernel.org>

arm64: Fix early handling of FEAT_E2H0 not being implemented

Commit 3944382fa6f2 introduced checks for FEAT_E2H0 that were incorrect and
executed too late. This commit fixes the check for ID_AA64MMFR4_EL1.E2H0
and sets the HCR_EL2.E2H bit early to allow proper initialization for hVHE
mode as a nested guest.

Link: https://lore.kernel.org/r/20240321115414.3169115-1-maz@kernel.org

> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index e32c8dd0b17a7..e0e710b36da37 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
[ ... ]
> @@ -588,22 +603,10 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
>  
>  	mov_q	x1, INIT_SCTLR_EL1_MMU_OFF
>  
[ ... ]
>  	mrs	x0, hcr_el2
>  	and	x0, x0, #HCR_E2H
>  	cbz	x0, 2f
> -1:
> +
>  	/* Set a sane SCTLR_EL1, the VHE way */
>  	pre_disable_mmu_workaround
>  	msr_s	SYS_SCTLR_EL12, x1

[Severity: High]
This isn't a bug introduced by this patch, but shouldn't writes to
control-plane system registers be immediately followed by an isb()
instruction?

Looking at init_el2() in arch/arm64/kernel/head.S, as well as __init_el2_hcrx()
and __init_el2_timers() in arch/arm64/include/asm/el2_setup.h, there are
several locations where registers like SYS_SCTLR_EL12, SYS_HCRX_EL2, and
cnthctl_el2 are written without an immediate context synchronization event.

For example, right after the code shown in the diff above, SYS_SCTLR_EL12
is updated:

	msr_s	SYS_SCTLR_EL12, x1
	mov	x2, #BOOT_CPU_FLAG_E2H
	b	3f

Could executing the subsequent branch or ALU instructions under an
architecturally undefined pipeline state cause unpredictable behavior or
unexpected traps during early boot?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708225124.4130846-1-coltonlewis@google.com?part=3