Re: [kvm-unit-tests PATCH v3 02/10] arm64: efi: initialise SCTLR_ELx fully

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

Hi Joey,

On 9/25/25 4:19 PM, Joey Gouly wrote:
> Don't rely on the value of SCTLR_ELx when booting via EFI.
>
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> ---
>  lib/arm/asm/setup.h   |  6 ++++++
>  lib/arm/setup.c       |  3 +++
>  lib/arm64/processor.c | 12 ++++++++++++
>  3 files changed, 21 insertions(+)
>
> diff --git a/lib/arm/asm/setup.h b/lib/arm/asm/setup.h
> index 9f8ef82e..4e60d552 100644
> --- a/lib/arm/asm/setup.h
> +++ b/lib/arm/asm/setup.h
> @@ -28,6 +28,12 @@ void setup(const void *fdt, phys_addr_t freemem_start);
>  
>  #include <efi.h>
>  
> +#ifdef __aarch64__
> +void setup_efi_sctlr(void);
> +#else
> +static inline void setup_efi_sctlr(void) {}
> +#endif
> +
>  efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo);
>  
>  #endif
> diff --git a/lib/arm/setup.c b/lib/arm/setup.c
> index 67b5db07..0aaa1d3a 100644
> --- a/lib/arm/setup.c
> +++ b/lib/arm/setup.c
> @@ -349,6 +349,9 @@ efi_status_t setup_efi(efi_bootinfo_t *efi_bootinfo)
>  {
>  	efi_status_t status;
>  
> +
spurious line
> +	setup_efi_sctlr();
> +
>  	exceptions_init();
>  
>  	memregions_init(arm_mem_regions, NR_MEM_REGIONS);
> diff --git a/lib/arm64/processor.c b/lib/arm64/processor.c
> index eb93fd7c..edc0ad87 100644
> --- a/lib/arm64/processor.c
> +++ b/lib/arm64/processor.c
> @@ -8,6 +8,7 @@
>  #include <libcflat.h>
>  #include <asm/ptrace.h>
>  #include <asm/processor.h>
> +#include <asm/setup.h>
>  #include <asm/thread_info.h>
>  
>  static const char *vector_names[] = {
> @@ -271,3 +272,14 @@ bool __mmu_enabled(void)
>  {
>  	return read_sysreg(sctlr_el1) & SCTLR_EL1_M;
>  }
> +
> +#ifdef CONFIG_EFI
> +
> +void setup_efi_sctlr(void)
> +{
> +	// EFI exits boot services with SCTLR_ELx.M=1, so keep
> +	// the MMU enabled.
I don't really understand the comment, if MMU was enabled why are we
mandated to set M bit again?

Eric
> +	write_sysreg(INIT_SCTLR_EL1_MMU_OFF | SCTLR_EL1_M, sctlr_el1);
> +}
> +
> +#endif