From: Hans de Goede <hdegoede@redhat.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
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: Mon, 23 Dec 2019 12:49:48 +0100 [thread overview]
Message-ID: <c82d02c3-c6e7-70af-37ad-5a8209ab8f22@redhat.com> (raw)
In-Reply-To: <CAKv+Gu8gWLw5-LkKwU0reUcreLyC8s1hQSzrR__-myuHrdVr7Q@mail.gmail.com>
Hi Ard,
On 22-12-2019 13:02, Ard Biesheuvel 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?
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 :)
Regards,
Hans
next prev parent reply other threads:[~2019-12-23 11:49 UTC|newest]
Thread overview: 87+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-12-18 17:01 [PATCH v2 00/21] efi/x86: confine type unsafe casting to mixed mode Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 01/21] efi/libstub: remove unused __efi_call_early() macro Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 02/21] efi/x86: rename efi_is_native() to efi_is_mixed() Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 03/21] efi/libstub: use a helper to iterate over a EFI handle array Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 04/21] efi/libstub: extend native protocol definitions with mixed_mode aliases Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 05/21] efi/libstub: distinguish between native/mixed not 32/64 bit Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 06/21] efi/libstub/x86: use mixed mode helpers to populate efi_config Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 07/21] efi/libstub: drop explicit 32/64-bit protocol definitions Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 08/21] efi/libstub: use stricter typing for firmware function pointers Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 09/21] efi/libstub: annotate firmware routines as __efiapi Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 10/21] efi/libstub/x86: avoid thunking for native firmware calls Ard Biesheuvel
2019-12-21 21:22 ` Hans de Goede
2019-12-22 12:02 ` Ard Biesheuvel
2019-12-22 12:37 ` Ard Biesheuvel
2019-12-22 12:46 ` Andy Lutomirski
2019-12-22 15:29 ` Ard Biesheuvel
2019-12-22 21:12 ` Arvind Sankar
2019-12-22 21:25 ` Ard Biesheuvel
2019-12-23 11:49 ` Hans de Goede [this message]
2019-12-23 12:00 ` Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 11/21] efi/libstub: get rid of 'sys_table_arg' macro parameter Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 12/21] efi/libstub: unify the efi_char16_printk implementations Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 13/21] efi/libstub/x86: drop __efi_early() export of efi_config struct Ard Biesheuvel
2019-12-24 19:34 ` Hans de Goede
2019-12-25 14:42 ` Ard Biesheuvel
2019-12-27 22:44 ` Hans de Goede
2019-12-27 22:51 ` Ard Biesheuvel
2019-12-31 23:04 ` Arvind Sankar
2020-01-01 18:13 ` Ard Biesheuvel
2020-01-01 19:08 ` Arvind Sankar
2020-01-02 7:33 ` Ard Biesheuvel
2020-01-02 14:06 ` Arvind Sankar
2020-01-02 15:20 ` Ard Biesheuvel
2020-01-02 15:51 ` Arvind Sankar
2020-01-02 15:58 ` Ard Biesheuvel
2020-01-02 16:28 ` Ard Biesheuvel
2020-01-02 16:59 ` Ard Biesheuvel
2020-01-02 17:26 ` Arvind Sankar
2020-01-02 17:30 ` Ard Biesheuvel
2020-01-02 17:41 ` Arvind Sankar
2020-01-02 17:48 ` Ard Biesheuvel
2020-01-02 18:10 ` Arvind Sankar
2020-01-02 18:38 ` Ard Biesheuvel
2020-01-03 14:16 ` Arvind Sankar
2020-01-03 14:23 ` Ard Biesheuvel
2020-01-02 18:38 ` Arvind Sankar
2020-01-02 16:59 ` Arvind Sankar
2020-01-02 17:03 ` Ard Biesheuvel
2020-01-02 17:21 ` Arvind Sankar
2019-12-18 17:01 ` [PATCH v2 14/21] efi/libstub: drop sys_table_arg from printk routines Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 15/21] efi/libstub: remove 'sys_table_arg' from all function prototypes Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 16/21] efi/libstub: drop protocol argument from efi_call_proto() macro Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 17/21] efi/libstub: drop 'table' argument from efi_table_attr() macro Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 18/21] efi/libstub: use 'func' not 'f' as macro parameter Ard Biesheuvel
2019-12-31 16:51 ` Arvind Sankar
2019-12-31 17:06 ` Ard Biesheuvel
2019-12-31 17:36 ` Arvind Sankar
2019-12-18 17:01 ` [PATCH v2 19/21] efi/libstub: tidy up types and names of global cmdline variables Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 20/21] efi/libstub: import type definitions for creating and signalling events Ard Biesheuvel
2019-12-18 17:01 ` [PATCH v2 21/21] efi: Allow disabling PCI busmastering on bridges during boot Ard Biesheuvel
2019-12-19 2:50 ` Andy Lutomirski
2019-12-19 13:17 ` Ard Biesheuvel
2019-12-19 20:04 ` Matthew Garrett
2019-12-19 20:04 ` Matthew Garrett
2019-12-20 7:06 ` Ard Biesheuvel
2019-12-20 7:17 ` Andy Lutomirski
2019-12-20 8:11 ` Ard Biesheuvel
2019-12-20 19:41 ` Arvind Sankar
2020-01-02 14:46 ` Laszlo Ersek
2020-01-02 15:40 ` Ard Biesheuvel
2019-12-20 20:43 ` Matthew Garrett
2019-12-21 16:44 ` Ard Biesheuvel
2019-12-21 21:24 ` Matthew Garrett
2019-12-21 22:54 ` Arvind Sankar
2019-12-23 14:02 ` Ard Biesheuvel
2019-12-23 15:46 ` Arvind Sankar
2019-12-23 15:58 ` Ard Biesheuvel
2019-12-23 16:12 ` Arvind Sankar
2019-12-23 20:57 ` Matthew Garrett
2020-02-06 14:30 ` Hans de Goede
2020-02-06 14:35 ` Ard Biesheuvel
2020-03-04 10:38 ` Hans de Goede
2020-03-04 18:26 ` Ard Biesheuvel
2020-03-04 18:49 ` Hans de Goede
2020-03-04 21:59 ` Ard Biesheuvel
2019-12-19 11:12 ` [PATCH v2 00/21] efi/x86: confine type unsafe casting to mixed mode Hans de Goede
2019-12-19 13:22 ` Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=c82d02c3-c6e7-70af-37ad-5a8209ab8f22@redhat.com \
--to=hdegoede@redhat.com \
--cc=ard.biesheuvel@linaro.org \
--cc=ardb@kernel.org \
--cc=linux-efi@vger.kernel.org \
--cc=luto@kernel.org \
--cc=matthewgarrett@google.com \
--cc=mingo@kernel.org \
--cc=nivedita@alum.mit.edu \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).