On 25.12.18 09:26, Heinrich Schuchardt wrote:
Refactor the switch from supervisor to hypervisor to a new function called at the beginning of do_bootefi().
Why?
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
With this patch I am just moving around the switch from supervisor to hypervisor mode within the EFI subsystem. Similar switching also occurs in all other boot commands (cf. arch/arm/lib/bootm.c).
Why are we running the U-Boot console in supervisor mode at all? Wouldn't it be advisable for security reasons to switch to hypervisor mode before entering the console?
Best regards
Heinrich
cmd/bootefi.c | 109 ++++++++++++++++++++++---------------------------- 1 file changed, 47 insertions(+), 62 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 1aaf5319e13..277d4863953 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -31,9 +31,9 @@ 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; +struct jmp_buf_data hyp_jmp;
/* Initialize and populate EFI object list */ efi_status_t efi_init_obj_list(void) @@ -122,6 +122,50 @@ void __weak allow_unaligned(void) { }
+/**
- entry_hyp() - entry point when switching to hypervisor mode
- */
+static void entry_hyp(void)
Potentially unused function depending on config settings?
+{
- dcache_enable();
- debug("Reached HYP mode\n");
HYP is a 32bit ARM term. AArch64 does not know what HYP is - there it's EL2.
Also keep in mind that this is not 100% ARM specific. We might see something along the lines of this logic on RISC-V eventually too.
- longjmp(&hyp_jmp, 1);
+}
+/**
- leave_supervisor() - switch to hypervisor mode
- */
+static void leave_supervisor(void)
That's not necessarily what this function does. It may switch from EL1->EL2 or from EL3->EL2.
+{ +#ifdef CONFIG_ARMV7_NONSEC
- static bool is_nonsec;
- if (armv7_boot_nonsec() && !is_nonsec) {
if (setjmp(&hyp_jmp))return;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);- }
+#endif
+#ifdef CONFIG_ARM64
- /* 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();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);- }
+#endif
- return;
+}
/*
- Set the load options of an image from an environment variable.
@@ -233,34 +277,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 +456,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 +541,8 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) /* Allow unaligned memory access */ allow_unaligned();
- leave_supervisor();
This means you're potentially executing object initialization code in a mode it wasn't meant to be run in. Is that a smart thing to do? It's definitely more than just refactoring.
If you just find the do_bootefi_exec() function too hard to read, refactoring it into calling an arch_efi_do_enter() call is probably smarter. Then aarch64 and 32bit arm can just provide their own "get be into target mode" functions within their own arch directories.
Alex
- /* Initialize EFI drivers */ r = efi_init_obj_list(); if (r != EFI_SUCCESS) {