Re: [PATCH 6.6 v3 2/6] arm64: Treat HCR_EL2.E2H as RES1 when ID_AA64MMFR4_EL1.E2H0 is negative

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The check for ID_AA64MMFR4_EL1.E2H0 < 0 is fundamentally broken because it tests a zeroed bit, rendering the new VHE-only boot path completely unreachable.
--

--- Patch [2]: [PATCH 6.6 v3 2/6] arm64: Treat HCR_EL2.E2H as RES1 when ID_AA64MMFR4_EL1.E2H0 is negative ---
commit e7cc4eff9449a99298c29ed34077323ad01efaea
Author: Marc Zyngier <maz@kernel.org>

arm64: Treat HCR_EL2.E2H as RES1 when ID_AA64MMFR4_EL1.E2H0 is negative

This commit updates the init_el2 boot path to check ID_AA64MMFR4_EL1.E2H0
before evaluating HCR_EL2.E2H. It aims to support compliant CPUs that
advertise VHE-only mode with a negative E2H0 value, avoiding a boot path
that erroneously attempts to clear HCR_EL2.E2H.

Link: https://lore.kernel.org/r/20240122181344.258974-8-maz@kernel.org

> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 6517bf2644a08..e32c8dd0b17a7 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -589,25 +589,32 @@ SYM_INNER_LABEL(init_el2, SYM_L_LOCAL)
>  	mov_q	x1, INIT_SCTLR_EL1_MMU_OFF
>  
>  	/*
> -	 * Fruity CPUs seem to have HCR_EL2.E2H set to RES1,
> -	 * making it impossible to start in nVHE mode. Is that
> -	 * compliant with the architecture? Absolutely not!
> +	 * Compliant CPUs advertise their VHE-onlyness with
> +	 * ID_AA64MMFR4_EL1.E2H0 < 0. HCR_EL2.E2H can be
> +	 * RES1 in that case.
> +	 *
> +	 * Fruity CPUs seem to have HCR_EL2.E2H set to RES1, but
> +	 * don't advertise it (they predate this relaxation).
>  	 */
> +	mrs_s	x0, SYS_ID_AA64MMFR4_EL1
> +	ubfx	x0, x0, #ID_AA64MMFR4_EL1_E2H0_SHIFT, #ID_AA64MMFR4_EL1_E2H0_WIDTH
> +	tbnz	x0, #(ID_AA64MMFR4_EL1_E2H0_SHIFT + ID_AA64MMFR4_EL1_E2H0_WIDTH - 1), 1f

[Severity: High]
Since ubfx in init_el2 extracts the field and places it at the bottom of x0
while zeroing the upper bits, will this tbnz instruction always fail?

It appears tbnz is testing the original bit position (SHIFT + WIDTH - 1)
rather than the new most significant bit position of the extracted field
(WIDTH - 1). Because ubfx zeroes out the original bit position, testing it
here in arch/arm64/kernel/head.S would unconditionally fall through.

Does this mean the new VHE-only boot path remains completely unreachable on
compliant hardware where ID_AA64MMFR4_EL1.E2H0 is negative?

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