Re: [kvm-unit-tests PATCH v3 01/10] arm64: drop to EL1 if booted at EL2

[Eric Auger <eric.auger@redhat.com>]

Hi Joey,

On 9/25/25 4:19 PM, Joey Gouly wrote:
> EL2 is not currently supported, drop to EL1 to conitnue booting.
continue
>
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> ---
>  arm/cstart64.S         | 47 +++++++++++++++++++++++++++++++++++++++---
>  lib/arm64/asm/sysreg.h | 14 +++++++++++++
>  2 files changed, 58 insertions(+), 3 deletions(-)
>
> diff --git a/arm/cstart64.S b/arm/cstart64.S
> index 014c9c7b..79b93dd4 100644
> --- a/arm/cstart64.S
> +++ b/arm/cstart64.S
> @@ -15,6 +15,46 @@
>  #include <asm/thread_info.h>
>  #include <asm/sysreg.h>
>  
> +.macro init_el, tmp

init_el2? but also likely to fall back to EL1. At least add a doc comment to explain what the function does.

> +	mrs	\tmp, CurrentEL
> +	cmp	\tmp, CurrentEL_EL2
> +	b.ne	1f
> +	/* EL2 setup */
> +	mrs	\tmp, mpidr_el1
> +	msr	vmpidr_el2, \tmp
> +	mrs	\tmp, midr_el1
> +	msr	vpidr_el2, \tmp
> +	/* clear trap registers */
clear FGT registers if FGT feature supported
> +	mrs	\tmp, id_aa64mmfr0_el1
> +	ubfx	\tmp, \tmp, #ID_AA64MMFR0_EL1_FGT_SHIFT, #4
> +	cbz	\tmp, .Lskip_fgt_\@
> +	mov	\tmp, #0
> +	msr_s	SYS_HFGRTR_EL2, \tmp
> +	msr_s	SYS_HFGWTR_EL2, \tmp
> +	msr_s	SYS_HFGITR_EL2, \tmp
> +	mrs	\tmp, id_aa64mmfr0_el1
> +	ubfx	\tmp, \tmp, #ID_AA64MMFR0_EL1_FGT_SHIFT, #4
> +	cmp	\tmp, #ID_AA64MMFR0_EL1_FGT_FGT2
> +	bne	.Lskip_fgt_\@
> +	mov	\tmp, #0
> +	msr_s	SYS_HFGRTR2_EL2, \tmp
> +	msr_s	SYS_HFGWTR2_EL2, \tmp
> +	msr_s	SYS_HFGITR2_EL2, \tmp
> +.Lskip_fgt_\@:
> +	mov	\tmp, #0
> +	msr	cptr_el2, \tmp
> +	ldr	\tmp, =(INIT_HCR_EL2_EL1_ONLY)
> +	msr	hcr_el2, \tmp
> +	mov	\tmp, PSR_MODE_EL1t
> +	msr	spsr_el2, \tmp
> +	adrp	\tmp, 1f
> +	add	\tmp, \tmp, :lo12:1f
> +	msr	elr_el2, \tmp
> +	eret
> +1:
> +.endm

I read in the coverletter you chose to not reuse include/asm/el2_setup.h
which indeed would look overkill given the reduced scope that we target
here. However compared to init_el2_state we seem to do very few things
in the EL2 setup before switching to EL1. I would give a bit more info
in the commit msg about what you keep and what you dropped and why it is
reasonable (stage2, timers, debug at least).

> +
> +
>  #ifdef CONFIG_EFI
>  #include "efi/crt0-efi-aarch64.S"
>  #else
> @@ -56,15 +96,15 @@ start:
>  	add     x6, x6, :lo12:reloc_end
>  1:
>  	cmp	x5, x6
> -	b.hs	1f
> +	b.hs	reloc_done
>  	ldr	x7, [x5]			// r_offset
>  	ldr	x8, [x5, #16]			// r_addend
>  	add	x8, x8, x4			// val = base + r_addend
>  	str	x8, [x4, x7]			// base[r_offset] = val
>  	add	x5, x5, #24
>  	b	1b
> -
> -1:
> +reloc_done:
> +	init_el x4
>  	/* zero BSS */
>  	adrp	x4, bss
>  	add	x4, x4, :lo12:bss
> @@ -185,6 +225,7 @@ get_mmu_off:
>  
>  .globl secondary_entry
>  secondary_entry:
> +	init_el x0
>  	/* enable FP/ASIMD and SVE */
>  	mov	x0, #(3 << 20)
>  	orr	x0, x0, #(3 << 16)
> diff --git a/lib/arm64/asm/sysreg.h b/lib/arm64/asm/sysreg.h
> index e537bb46..ed776716 100644
> --- a/lib/arm64/asm/sysreg.h
> +++ b/lib/arm64/asm/sysreg.h
> @@ -77,6 +77,9 @@ asm(
>  #define ID_AA64ISAR0_EL1_RNDR_SHIFT	60
>  #define ID_AA64PFR1_EL1_MTE_SHIFT	8
>  
> +#define ID_AA64MMFR0_EL1_FGT_SHIFT	56
> +#define ID_AA64MMFR0_EL1_FGT_FGT2	0x2
> +
>  #define ICC_PMR_EL1			sys_reg(3, 0, 4, 6, 0)
>  #define ICC_SGI1R_EL1			sys_reg(3, 0, 12, 11, 5)
>  #define ICC_IAR1_EL1			sys_reg(3, 0, 12, 12, 0)
> @@ -113,6 +116,17 @@ asm(
>  #define SCTLR_EL1_TCF0_SHIFT	38
>  #define SCTLR_EL1_TCF0_MASK	GENMASK_ULL(39, 38)
>  
> +#define HCR_EL2_RW		_BITULL(31)
> +
> +#define INIT_HCR_EL2_EL1_ONLY	(HCR_EL2_RW)

I don't really understand the renaming

> +
> +#define SYS_HFGRTR_EL2		sys_reg(3, 4, 1, 1, 4)
> +#define SYS_HFGWTR_EL2		sys_reg(3, 4, 1, 1, 5)
> +#define SYS_HFGITR_EL2		sys_reg(3, 4, 1, 1, 6)
> +#define SYS_HFGRTR2_EL2		sys_reg(3, 4, 3, 1, 2)
> +#define SYS_HFGWTR2_EL2		sys_reg(3, 4, 3, 1, 3)
> +#define SYS_HFGITR2_EL2		sys_reg(3, 4, 3, 1, 7)
> +
>  #define INIT_SCTLR_EL1_MMU_OFF	\
>  			(SCTLR_EL1_ITD | SCTLR_EL1_SED | SCTLR_EL1_EOS | \
>  			 SCTLR_EL1_TSCXT | SCTLR_EL1_EIS | SCTLR_EL1_SPAN | \
Thanks

Eric