On Mon, 23 Dec 2019 at 12:49, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > > Hi Ard, > > On 22-12-2019 13:02, Ard Biesheuvel wrote: > > On Sat, 21 Dec 2019 at 22:22, Hans de Goede <hdegoede@xxxxxxxxxx> wrote: > >> > >> Hi Ard, > >> > >> On 18-12-2019 18:01, Ard Biesheuvel wrote: > >>> We use special wrapper routines to invoke firmware services in the > >>> native case as well as the mixed mode case. For mixed mode, the need > >>> is obvious, but for the native cases, we can simply rely on the > >>> compiler to generate the indirect call, given that GCC now has > >>> support for the MS calling convention (and has had it for quite some > >>> time now). Note that on i386, the decompressor and the EFI stub are not > >>> built with -mregparm=3 like the rest of the i386 kernel, so we can > >>> safely allow the compiler to emit the indirect calls here as well. > >>> > >>> So drop all the wrappers and indirection, and switch to either native > >>> calls, or direct calls into the thunk routine for mixed mode. > >>> > >>> Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx> > >> > >> I'm afraid that this patch breaks the boot on one of my machines. > >> > >> Specifically this patch breaks my GDP pocket machine. This is a Cherry > >> Trail device with a 64 UEFI running a 64 bit kernel build. > >> > >> As soon as I cherry pick this patch into my personal 5.5.0-rc2 based > >> tree, the GPD pocket stops booting and it stop so early on that I get 0 > >> debug output. I guess I could try adding a few pr_efi_err calls > >> and see if those still do something. > >> > >> I noticed that you have made some changes to this patch, I've > >> tried updating it to the version from your efistub-x86-cleanup-v3 > >> branch, commit id a37d90a2c570a25926fd1645482cb9f3c1d042a0 > >> and I have also cherry-picked the latest version of all preceding > >> commits, unfortunately even with the new version, the GPD pocket > >> still hangs at boot. > >> > >> Unfortunately the nature of this patch makes it hard to figure > >> out the root cause of this issue... > >> > >> I've also tried another Cherry Trail device with 64 bit UEFI and > >> that does not suffer from this problem. > >> > > > > Thanks Hans. > > > > There are a number of things that change in the way the calls are > > made, but the most obvious thing to check is whether the stack needs > > to be aligned, since that is no longer being done. > > > > If you have time to experiment a bit more, could you check whether > > doing 'and $~0xf %rsp' before 'call efi_main' in the .S stub code for > > x86_64 makes a difference? > > Ok, so I made this change on top of a37d90a2c570a25926fd1645482cb9f3c1d042a0 > (the last "efi/libstub/x86: avoid thunking for native firmware calls" version > I tried before reporting this problem) : > > --- a/arch/x86/boot/compressed/head_64.S > +++ b/arch/x86/boot/compressed/head_64.S > @@ -464,6 +464,7 @@ SYM_FUNC_START(efi_pe_entry) > > handover_entry: > movq efi_config(%rip), %rdi > + and $~0xf, %rsp > call efi_main > movq %rax,%rsi > cmpq $0,%rax > > And that does the trick, the GPD pocket boots successfully with commit > a37d90a2c570a25926fd1645482cb9f3c1d042a0 + the above change. > > So it looks like your first hunch on how to fix this is correct :) > Excellent! Thanks for testing. I'll fold this in.
