[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z0gn1N3IsP8r3gTA@hovoldconsulting.com>
Date: Thu, 28 Nov 2024 09:20:36 +0100
From: Johan Hovold <johan@...nel.org>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: Bjorn Andersson <andersson@...nel.org>,
Ricardo Salveti <ricardo@...ndries.io>,
Marc Zyngier <maz@...nel.org>, linux-efi@...r.kernel.org,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: UEFI EBS() failures on Lenovo T14s
Hi Ard,
We've run into a buggy UEFI implementation on the Qualcomm Snapdragon
based Lenovo ThinkPad T14s where ExitBootServices() often fails.
One bootloader entry may fail to start almost consistently (once in a
while it may start), while a second entry may always work even when the
kernel, dtb and initramfs images are copies of the failing entry on the
same ESP.
This can be worked around by starting and exiting a UEFI shell from the
bootloader or by starting the bootloader manually via the Boot Menu
(F12) before starting the kernel.
Notably starting the kernel automatically from the shell startup.nsh
does not work, while calling the same script manually works.
Based on your comments to a similar report for an older Snapdragon based
Lenovo UEFI implementation [1], I discovered that allocating an event
before calling ExitBootServices() can make the call succeed. There is
often no need to actually signal the event group, but the event must
remain allocated (i.e. CloseEvent() must not be called).
(Raising TPL or disabling interrupts does not seem to help.)
Also with the event signalling, ExitBootServices() sometimes fails when
starting the kernel automatically from a shell startup.nsh, while
systemd-boot seems to always work. This was only observed after removing
some efi_printk() used during the experiments from the stub...
Something is obviously really broken here, but do you have any
suggestions about what could be the cause of this as further input to
Qualcomm (and Lenovo) as they try to fix this?
For completeness, the first call to ExitBootServices() often fails also
on the x1e80100 reference design (CRD), and Qualcomm appears to have
been the ones providing the current retry implementation:
fc07716ba803 ("efi/libstub: Introduce ExitBootServices helper")
as this was needed to prevent similar boot failures with older Qualcomm
UEFI fw.
Marc is also hitting something like this on the Qualcomm X1E devkit
(i.e. with firmware that should not have any modifications from Lenovo).
I'm attaching the patch that allows me to boot kernels on the T14s
below.
Johan
[1] https://bugzilla.kernel.org/show_bug.cgi?id=216375#c10
>From 4ceece4e15e87d0949110b835b867e7cea1bd658 Mon Sep 17 00:00:00 2001
From: Johan Hovold <johan+linaro@...nel.org>
Date: Wed, 27 Nov 2024 16:05:37 +0100
Subject: [PATCH] hack: efi/libstub: fix t14s exit_boot_services() failure
The UEFI firmware on the Lenovo ThinkPad T14s is broken and
ExitBootServices() often fails and prevents the kernel from starting:
EFI stub: Exiting boot services...
EFI stub: Exit boot services failed.
One bootloader entry may fail to start almost consistently (once in a
while it may start), while a second entry may always work even when the
kernel, dtb and initramfs images are copies of the failing entry on the
same ESP.
This can be worked around by starting and exiting a UEFI shell from the
bootloader or by starting the bootloader manually via the Boot Menu
(F12) before starting the kernel.
Notably starting the kernel automatically from the shell startup.nsh
does not work, while calling the same script manually works.
Experiments have revealed that allocating an event before calling
ExitBootServices() can make the call succeed. There is often no need to
actually signal the event group, but the event must remain allocated
(i.e. CloseEvent() must not be called).
Also with the event signalling, ExitBootServices() sometimes fails when
starting the kernel automatically from a shell startup.nsh, while
systemd-boot seems to always work. This was only observed after removing
some efi_printk() used during the experiments from the stub...
Signed-off-by: Johan Hovold <johan+linaro@...nel.org>
---
.../firmware/efi/libstub/efi-stub-helper.c | 20 +++++++++++++++++++
drivers/firmware/efi/libstub/efistub.h | 4 ++--
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
index de659f6a815f..f228895bf090 100644
--- a/drivers/firmware/efi/libstub/efi-stub-helper.c
+++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
@@ -409,6 +409,9 @@ char *efi_convert_cmdline(efi_loaded_image_t *image, int *cmd_line_len)
return (char *)cmdline_addr;
}
+#define EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES \
+ EFI_GUID(0x8be0e274, 0x3970, 0x4b44, 0x80, 0xc5, 0x1a, 0xb9, 0x50, 0x2f, 0x3b, 0xfc)
+
/**
* efi_exit_boot_services() - Exit boot services
* @handle: handle of the exiting image
@@ -429,10 +432,26 @@ efi_status_t efi_exit_boot_services(void *handle, void *priv,
{
struct efi_boot_memmap *map;
efi_status_t status;
+ efi_guid_t guid = EFI_EVENT_GROUP_BEFORE_EXIT_BOOT_SERVICES;
+ efi_event_t event;
if (efi_disable_pci_dma)
efi_pci_disable_bridge_busmaster();
+ status = efi_bs_call(create_event_ex, 0, 0, NULL, NULL, &guid, &event);
+ if (status == EFI_SUCCESS) {
+ status = efi_bs_call(signal_event, event);
+ if (status != EFI_SUCCESS)
+ efi_err("%s - signal event failed: %02lx\n", __func__, status);
+#if 0
+ status = efi_bs_call(close_event, event);
+ if (status != EFI_SUCCESS)
+ efi_err("%s - close event failed: %02lx\n", __func__, status);
+#endif
+ } else {
+ efi_err("%s - create event ex failed: %02lx\n", __func__, status);
+ }
+
status = efi_get_memory_map(&map, true);
if (status != EFI_SUCCESS)
return status;
@@ -446,6 +465,7 @@ efi_status_t efi_exit_boot_services(void *handle, void *priv,
status = efi_bs_call(exit_boot_services, handle, map->map_key);
if (status == EFI_INVALID_PARAMETER) {
+ //efi_err("Exit boot services failed: %lx\n", status);
/*
* The memory map changed between efi_get_memory_map() and
* exit_boot_services(). Per the UEFI Spec v2.6, Section 6.4:
diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
index 685098f9626f..e3f710823a29 100644
--- a/drivers/firmware/efi/libstub/efistub.h
+++ b/drivers/firmware/efi/libstub/efistub.h
@@ -272,7 +272,7 @@ union efi_boot_services {
efi_status_t (__efiapi *wait_for_event)(unsigned long,
efi_event_t *,
unsigned long *);
- void *signal_event;
+ efi_status_t (__efiapi *signal_event)(efi_event_t);
efi_status_t (__efiapi *close_event)(efi_event_t);
void *check_event;
void *install_protocol_interface;
@@ -322,7 +322,7 @@ union efi_boot_services {
void *calculate_crc32;
void (__efiapi *copy_mem)(void *, const void *, unsigned long);
void (__efiapi *set_mem)(void *, unsigned long, unsigned char);
- void *create_event_ex;
+ efi_status_t (__efiapi *create_event_ex)(u32, int, void *, void *, void *, efi_event_t *);
};
struct {
efi_table_hdr_t hdr;
--
2.45.2
Powered by blists - more mailing lists