On Tue, Aug 09, 2022, Alexandru Elisei wrote:
> Hi,
>
> Adding Sean and Zixuan, as they were involved in the initial x86 UEFI
> support.
>
> This version of the UEFI support for arm64 jumps to lib/efi.c::efi_main
> after performing the relocation. I'll post an abbreviated/simplified
> version of efi_main() for reference:
>
> efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
> {
> /* Get image, cmdline and memory map parameters from UEFI */
>
> efi_exit_boot_services(handle, &efi_bootinfo.mem_map);
>
> /* Set up arch-specific resources */
> setup_efi(&efi_bootinfo);
>
> /* Run the test case */
> ret = main(__argc, __argv, __environ);
>
> /* Shutdown the guest VM */
> efi_exit(ret);
>
> /* Unreachable */
> return EFI_UNSUPPORTED;
> }
>
> Note that the assumption that efi_main() makes is that setup_efi() doesn't
> change the stack from the stack that the UEFI implementation allocated, in
> order for setup_efi() to be able to return to efi_main().
On the x86 side, efi_main() now runs with a KUT-controlled stack since commit
d316d12a ("x86: efi: Provide a stack within testcase memory")
> If we want to keep the UEFI allocated stack, then both mechanism must be
> forbidden when running under UEFI. I dislike this idea, because those two
> mechanisms allow kvm-unit-tests to run tests which otherwise wouldn't have
> been possible with a normal operating system, which, except for the early
> boot code, runs with the MMU enabled.
Agreed. IMO, KUT should stop using UEFI-controlled data as early as possible.
The original x86 behavior was effectively a temporary solution to get UEFI working
without needing to simultaneously rework the common early boot flows.
Side topic, I think the x86 code now has a benign bug. The old code contained an
adjustment to RSP to undo some stack shenanigans (can't figure out why those
shenanigans exist), but now the adjustment happens on the KUT stack, which doesn't
need to be fixed up.
It's a moot point since efi_main() should never return, but it looks odd. And it
seems like KUT should intentionally explode if efi_main() returns, e.g. do this
over two patches:
diff --git a/x86/efi/crt0-efi-x86_64.S b/x86/efi/crt0-efi-x86_64.S
index 1708ed55..e62891bc 100644
--- a/x86/efi/crt0-efi-x86_64.S
+++ b/x86/efi/crt0-efi-x86_64.S
@@ -62,10 +62,7 @@ _start:
lea stacktop(%rip), %rsp
call efi_main
- addq $8, %rsp
-
-.exit:
- ret
+ ud2
// hand-craft a dummy .reloc section so EFI knows it's a relocatable executable:
