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().
