[U-Boot] [PATCH v2 1/1] efi_loader: refactor switch to hypervisor mode
Alexander Graf
agraf at suse.de
Wed Dec 26 21:19:06 UTC 2018
On 26.12.18 11:07, Heinrich Schuchardt wrote:
> Refactor the switch from supervisor to hypervisor to a new function called at
> the beginning of do_bootefi().
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
> v2
> Use weak function with implementation in arch directories
> ---
> arch/arm/cpu/armv7/Makefile | 1 +
> arch/arm/cpu/armv7/exception_level.c | 41 ++++++++++++++
> arch/arm/cpu/armv8/Makefile | 1 +
> arch/arm/cpu/armv8/exception_level.c | 40 ++++++++++++++
> cmd/bootefi.c | 81 +++++-----------------------
> include/bootm.h | 5 ++
> 6 files changed, 101 insertions(+), 68 deletions(-)
> create mode 100644 arch/arm/cpu/armv7/exception_level.c
> create mode 100644 arch/arm/cpu/armv8/exception_level.c
>
> diff --git a/arch/arm/cpu/armv7/Makefile b/arch/arm/cpu/armv7/Makefile
> index 4f4647c90ac..8c955d0d528 100644
> --- a/arch/arm/cpu/armv7/Makefile
> +++ b/arch/arm/cpu/armv7/Makefile
> @@ -14,6 +14,7 @@ obj-$(CONFIG_SYS_ARM_MPU) += mpu_v7r.o
>
> ifneq ($(CONFIG_SPL_BUILD),y)
> obj-$(CONFIG_EFI_LOADER) += sctlr.o
> +obj-$(CONFIG_ARMV7_NONSEC) += exception_level.o
> endif
>
> ifneq ($(CONFIG_SKIP_LOWLEVEL_INIT),y)
> diff --git a/arch/arm/cpu/armv7/exception_level.c b/arch/arm/cpu/armv7/exception_level.c
> new file mode 100644
> index 00000000000..e231fc94633
> --- /dev/null
> +++ b/arch/arm/cpu/armv7/exception_level.c
> @@ -0,0 +1,41 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Switching exception levels
Please make this a bit more verbose ;).
> + *
> + * Copyright (c) 2018 Heinrich Schuchardt
> + */
> +
> +#include <common.h>
> +#include <bootm.h>
> +#include <asm/armv7.h>
> +#include <asm/secure.h>
> +#include <asm/setjmp.h>
> +
> +struct jmp_buf_data hyp_jmp;
> +
> +/**
> + * entry_hyp() - entry point when switching to hypervisor mode
> + */
> +static void entry_hyp(void)
> +{
> + dcache_enable();
> + debug("Reached HYP mode\n");
The longjmp here wants documentation.
> + longjmp(&hyp_jmp, 1);
> +}
> +
> +/**
> + * leave_supervisor() - switch to hypervisor mode
> + */
> +void leave_supervisor(void)
The name is still incorrect. Please give it something more obvious.
Switch_to_payload_mode()? Whatever it is, we need a name that covers all
the cases.
> +{
> + static bool is_nonsec;
> +
> + if (armv7_boot_nonsec() && !is_nonsec) {
> + if (setjmp(&hyp_jmp))
> + return;
hyp_jmp is only used within this function's scope. Just pass it as
argument to entry_hyp() and make it a local variable.
> + dcache_disable(); /* flush cache before switch to HYP */
> + armv7_init_nonsec();
> + is_nonsec = true;
> + secure_ram_addr(_do_nonsec_entry)(entry_hyp, 0, 0, 0);
> + }
> +}
> diff --git a/arch/arm/cpu/armv8/Makefile b/arch/arm/cpu/armv8/Makefile
> index 52c8daa0496..8b0b887a696 100644
> --- a/arch/arm/cpu/armv8/Makefile
> +++ b/arch/arm/cpu/armv8/Makefile
> @@ -14,6 +14,7 @@ ifdef CONFIG_SPL_BUILD
> obj-$(CONFIG_ARMV8_SPL_EXCEPTION_VECTORS) += exceptions.o
> else
> obj-y += exceptions.o
> +obj-y += exception_level.o
> endif
> obj-y += cache.o
> obj-y += tlb.o
> diff --git a/arch/arm/cpu/armv8/exception_level.c b/arch/arm/cpu/armv8/exception_level.c
> new file mode 100644
> index 00000000000..ad8211bb85d
> --- /dev/null
> +++ b/arch/arm/cpu/armv8/exception_level.c
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Switching exception levels
See above.
> + *
> + * Copyright (c) 2018 Heinrich Schuchardt
> + */
> +
> +#include <common.h>
> +#include <bootm.h>
> +#include <asm/setjmp.h>
> +
> +static struct jmp_buf_data hyp_jmp;
Same comment here. Make it local please.
> +
> +/**
> + * entry_hyp() - entry point when switching to hypervisor mode
> + */
> +static void entry_hyp(void)
> +{
> + dcache_enable();
> + debug("Reached HYP mode\n");
Also wants documentation.
> + longjmp(&hyp_jmp, 1);
> +}
> +
> +/**
> + * leave_supervisor() - switch to hypervisor mode
> + */
> +void leave_supervisor(void)
> +{
> + /* On AArch64 we need to make sure we call our payload in < EL3 */
> + if (current_el() == 3) {
> + if (setjmp(&hyp_jmp))
> + return;
> + smp_kick_all_cpus();
This is definitely behavioral change. Please add that call in a separate
patch before or after this one.
> + dcache_disable(); /* flush cache before switch to EL2 */
> +
> + /* Move into EL2 and keep running there */
> + armv8_switch_to_el2(0, 0, 0, 0, (uintptr_t)entry_hyp,
> + ES_TO_AARCH64);
> + }
> +}
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 1aaf5319e13..804f527e730 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -5,8 +5,9 @@
> * Copyright (c) 2016 Alexander Graf
> */
>
> -#include <charset.h>
> #include <common.h>
> +#include <bootm.h>
> +#include <charset.h>
> #include <command.h>
> #include <dm.h>
> #include <efi_dxe.h>
> @@ -21,17 +22,11 @@
> #include <asm-generic/unaligned.h>
> #include <linux/linkage.h>
>
> -#ifdef CONFIG_ARMV7_NONSEC
> -#include <asm/armv7.h>
> -#include <asm/secure.h>
> -#endif
> -
> DECLARE_GLOBAL_DATA_PTR;
>
> #define OBJ_LIST_NOT_INITIALIZED 1
>
> static efi_status_t efi_obj_list_initialized = OBJ_LIST_NOT_INITIALIZED;
> -
> static struct efi_device_path *bootefi_image_path;
> static struct efi_device_path *bootefi_device_path;
>
> @@ -122,6 +117,15 @@ void __weak allow_unaligned(void)
> {
> }
>
> +/**
> + * leave_supervisor() - switch to hypervisor mode
> + *
> + * This routine is overridden by architectures requiring this feature.
> + */
> +void __weak leave_supervisor(void)
This function does not want to live in bootefi.c, but be defined in
bootm.h. Choose one or the other. Either it's efi specific, then its
prototype should be defined in efi_loader.h. Or it's generic, then
please move it to more generic bootm code.
And again, the name is wrong :).
Alex
> +{
> +}
> +
> /*
> * Set the load options of an image from an environment variable.
> *
> @@ -233,34 +237,6 @@ static efi_status_t efi_do_enter(
> return ret;
> }
>
> -#ifdef CONFIG_ARM64
> -static efi_status_t efi_run_in_el2(EFIAPI efi_status_t (*entry)(
> - efi_handle_t image_handle, struct efi_system_table *st),
> - efi_handle_t image_handle, struct efi_system_table *st)
> -{
> - /* Enable caches again */
> - dcache_enable();
> -
> - return efi_do_enter(image_handle, st, entry);
> -}
> -#endif
> -
> -#ifdef CONFIG_ARMV7_NONSEC
> -static bool is_nonsec;
> -
> -static efi_status_t efi_run_in_hyp(EFIAPI efi_status_t (*entry)(
> - efi_handle_t image_handle, struct efi_system_table *st),
> - efi_handle_t image_handle, struct efi_system_table *st)
> -{
> - /* Enable caches again */
> - dcache_enable();
> -
> - is_nonsec = true;
> -
> - return efi_do_enter(image_handle, st, entry);
> -}
> -#endif
> -
> /*
> * efi_carve_out_dt_rsv() - Carve out DT reserved memory ranges
> *
> @@ -440,39 +416,6 @@ static efi_status_t do_bootefi_exec(void *efi,
> goto err_prepare;
> }
>
> -#ifdef CONFIG_ARM64
> - /* On AArch64 we need to make sure we call our payload in < EL3 */
> - if (current_el() == 3) {
> - smp_kick_all_cpus();
> - dcache_disable(); /* flush cache before switch to EL2 */
> -
> - /* Move into EL2 and keep running there */
> - armv8_switch_to_el2((ulong)entry,
> - (ulong)&image_obj->header,
> - (ulong)&systab, 0, (ulong)efi_run_in_el2,
> - ES_TO_AARCH64);
> -
> - /* Should never reach here, efi exits with longjmp */
> - while (1) { }
> - }
> -#endif
> -
> -#ifdef CONFIG_ARMV7_NONSEC
> - if (armv7_boot_nonsec() && !is_nonsec) {
> - dcache_disable(); /* flush cache before switch to HYP */
> -
> - armv7_init_nonsec();
> - secure_ram_addr(_do_nonsec_entry)(
> - efi_run_in_hyp,
> - (uintptr_t)entry,
> - (uintptr_t)&image_obj->header,
> - (uintptr_t)&systab);
> -
> - /* Should never reach here, efi exits with longjmp */
> - while (1) { }
> - }
> -#endif
> -
> ret = efi_do_enter(&image_obj->header, &systab, entry);
>
> err_prepare:
> @@ -558,6 +501,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
> /* Allow unaligned memory access */
> allow_unaligned();
>
> + leave_supervisor();
> +
> /* Initialize EFI drivers */
> r = efi_init_obj_list();
> if (r != EFI_SUCCESS) {
> diff --git a/include/bootm.h b/include/bootm.h
> index 0501414e0dc..c64874c98b5 100644
> --- a/include/bootm.h
> +++ b/include/bootm.h
> @@ -80,4 +80,9 @@ int bootm_decomp_image(int comp, ulong load, ulong image_start, int type,
> */
> void board_quiesce_devices(void);
>
> +/**
> + * leave_supervisor() - switch to hypervisor mode
> + */
> +void leave_supervisor(void);
> +
> #endif
>
More information about the U-Boot
mailing list