Re: [kvm-unit-tests PATCH v3 00/27] EFI and ACPI support for arm64

[Sean Christopherson <seanjc@google.com>]

On Wed, Aug 10, 2022, Alexandru Elisei wrote:
> Hi Sean,
> 
> On Tue, Aug 09, 2022 at 03:29:58PM +0000, Sean Christopherson wrote:
> > On Tue, Aug 09, 2022, Alexandru Elisei wrote:
> > > 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.
> 
> Yes, this is also what I am thinking, the stack is poorly specified in the
> specification because the specification doesn't expect an application to
> keep using it after calling EFI_BOOT_SERVICES.Exit(). Plus, using the UEFI
> allocated stack makes the test less reproducible, as even EDK2 today
> diverges from the spec wrt the stack, and other UEFI implementations might
> do things differently. And with just like all software, there might be bugs
> in the firmware. IMO, the more control kvm-unit-tests has over its
> resources, the more robust the tests are.
> 
> What I was thinking is rewriting efi_main to return setup_efi(),
> something like this:
> 
> void 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, not expected to return. */
>         setup_efi(&efi_bootinfo);
> }
> 
> Which would allow all architectures to change their environment as they see
> fit, as setup_efi() is not expected to return. Architectures would have to
> be made aware of the efi_exit() function though.

And they'd also have to invoke the test's main().  Why can't ARM switch to a KUT
stack before calling efi_main()?  The stack is a rather special case, I don't think
it's unreasonable to require architectures to handle that before efi_main().