Re: [PATCH v2 10/21] efi/libstub/x86: avoid thunking for native firmware calls

From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: Hans de Goede <hdegoede@redhat.com>
Cc: Ard Biesheuvel <ardb@kernel.org>,
	linux-efi <linux-efi@vger.kernel.org>,
	Matthew Garrett <matthewgarrett@google.com>,
	Ingo Molnar <mingo@kernel.org>, Andy Lutomirski <luto@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Arvind Sankar <nivedita@alum.mit.edu>
Subject: Re: [PATCH v2 10/21] efi/libstub/x86: avoid thunking for native firmware calls
Date: Sun, 22 Dec 2019 13:37:14 +0100	[thread overview]
Message-ID: <CAKv+Gu8MrFpSHJOnJkDt-jig4nLxXStTrMKTMj2377xwr4DM7w@mail.gmail.com> (raw)
In-Reply-To: <CAKv+Gu8gWLw5-LkKwU0reUcreLyC8s1hQSzrR__-myuHrdVr7Q@mail.gmail.com>

On Sun, 22 Dec 2019 at 13:02, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Sat, 21 Dec 2019 at 22:22, Hans de Goede <hdegoede@redhat.com> 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@kernel.org>
> >
> > 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?

... or in case that does not help, I have updated my -v3 branch to
split the removal of the thunking sequence for x86_64 in several
discrete steps.