| Message ID | 20210305065120.11355-1-shawn.guo@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: efi: add check for broken efi poweroff | expand |
+ Maximilian On Fri, Mar 05, 2021 at 08:01:02AM +0100, Ard Biesheuvel wrote: > On Fri, 5 Mar 2021 at 07:51, Shawn Guo <shawn.guo@linaro.org> wrote: > > > > Poweroff via UEFI Runtime Services doesn't always work on every single > > arm64 machine. For example, on Lenovo Flex 5G laptop, it results in > > a system reboot rather than shutdown. Add a DMI check to keep such > > system stay with the original poweroff method (PSCI). > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > What is the point of using EFI runtime services on this machine if > poweroff doesn't work either? Can't we just boot this thing with > efi=noruntime? Ard, With Maximilian playing ACPI kernel on Microsoft Surface Pro X, this ResetSystem service issue triggers more discussion and testing [1]. Maximilian tested it with uefi-test-runner and reported that ResetSystem actually works [2]. Looking at the kernel dump, I'm wondering if it's because that kernel calls into the services with assuming they are in virtual addressing mode, while actually they are in flat physical mode instead, due to that SetVirtualAddressMap() call is skipped (efi_novamap). Shawn [1] https://github.com/Sonicadvance1/linux/issues/27#issuecomment-836103896 [2] https://github.com/Sonicadvance1/linux/issues/27#issuecomment-837184892
On Mon, 17 May 2021 at 02:59, Shawn Guo <shawn.guo@linaro.org> wrote: > > + Maximilian > > On Fri, Mar 05, 2021 at 08:01:02AM +0100, Ard Biesheuvel wrote: > > On Fri, 5 Mar 2021 at 07:51, Shawn Guo <shawn.guo@linaro.org> wrote: > > > > > > Poweroff via UEFI Runtime Services doesn't always work on every single > > > arm64 machine. For example, on Lenovo Flex 5G laptop, it results in > > > a system reboot rather than shutdown. Add a DMI check to keep such > > > system stay with the original poweroff method (PSCI). > > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > > > What is the point of using EFI runtime services on this machine if > > poweroff doesn't work either? Can't we just boot this thing with > > efi=noruntime? > > Ard, > > With Maximilian playing ACPI kernel on Microsoft Surface Pro X, this > ResetSystem service issue triggers more discussion and testing [1]. > Maximilian tested it with uefi-test-runner and reported that ResetSystem > actually works [2]. > > Looking at the kernel dump, I'm wondering if it's because that kernel > calls into the services with assuming they are in virtual addressing > mode, while actually they are in flat physical mode instead, due to > that SetVirtualAddressMap() call is skipped (efi_novamap). > That looks like a firmware bug. Boot with efi=debug to figure out whether the faulting address is a physical address that falls inside a EfiRuntimeServicesData region.
On Tue, May 18, 2021 at 09:44:12AM +0200, Ard Biesheuvel wrote: > On Mon, 17 May 2021 at 02:59, Shawn Guo <shawn.guo@linaro.org> wrote: > > > > + Maximilian > > > > On Fri, Mar 05, 2021 at 08:01:02AM +0100, Ard Biesheuvel wrote: > > > On Fri, 5 Mar 2021 at 07:51, Shawn Guo <shawn.guo@linaro.org> wrote: > > > > > > > > Poweroff via UEFI Runtime Services doesn't always work on every single > > > > arm64 machine. For example, on Lenovo Flex 5G laptop, it results in > > > > a system reboot rather than shutdown. Add a DMI check to keep such > > > > system stay with the original poweroff method (PSCI). > > > > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > > > > > What is the point of using EFI runtime services on this machine if > > > poweroff doesn't work either? Can't we just boot this thing with > > > efi=noruntime? > > > > Ard, > > > > With Maximilian playing ACPI kernel on Microsoft Surface Pro X, this > > ResetSystem service issue triggers more discussion and testing [1]. > > Maximilian tested it with uefi-test-runner and reported that ResetSystem > > actually works [2]. > > > > Looking at the kernel dump, I'm wondering if it's because that kernel > > calls into the services with assuming they are in virtual addressing > > mode, while actually they are in flat physical mode instead, due to > > that SetVirtualAddressMap() call is skipped (efi_novamap). > > > > That looks like a firmware bug. Boot with efi=debug to figure out > whether the faulting address is a physical address that falls inside a > EfiRuntimeServicesData region. Last time when I was seeing reboot/poweroff broken on Flex 5G, I did not capture any kernel dumps. I will retry with efi=debug and see if I can get more information. In the meantime, could you help me understand if EFI must be running in virtual address mode when kernel is calling into the services, or it should work no matter EFI is running in virtual or physical address mode? Thanks! Shawn
On Wed, 19 May 2021 at 16:05, Shawn Guo <shawn.guo@linaro.org> wrote: > > On Tue, May 18, 2021 at 09:44:12AM +0200, Ard Biesheuvel wrote: > > On Mon, 17 May 2021 at 02:59, Shawn Guo <shawn.guo@linaro.org> wrote: > > > > > > + Maximilian > > > > > > On Fri, Mar 05, 2021 at 08:01:02AM +0100, Ard Biesheuvel wrote: > > > > On Fri, 5 Mar 2021 at 07:51, Shawn Guo <shawn.guo@linaro.org> wrote: > > > > > > > > > > Poweroff via UEFI Runtime Services doesn't always work on every single > > > > > arm64 machine. For example, on Lenovo Flex 5G laptop, it results in > > > > > a system reboot rather than shutdown. Add a DMI check to keep such > > > > > system stay with the original poweroff method (PSCI). > > > > > > > > > > Signed-off-by: Shawn Guo <shawn.guo@linaro.org> > > > > > > > > What is the point of using EFI runtime services on this machine if > > > > poweroff doesn't work either? Can't we just boot this thing with > > > > efi=noruntime? > > > > > > Ard, > > > > > > With Maximilian playing ACPI kernel on Microsoft Surface Pro X, this > > > ResetSystem service issue triggers more discussion and testing [1]. > > > Maximilian tested it with uefi-test-runner and reported that ResetSystem > > > actually works [2]. > > > > > > Looking at the kernel dump, I'm wondering if it's because that kernel > > > calls into the services with assuming they are in virtual addressing > > > mode, while actually they are in flat physical mode instead, due to > > > that SetVirtualAddressMap() call is skipped (efi_novamap). > > > > > > > That looks like a firmware bug. Boot with efi=debug to figure out > > whether the faulting address is a physical address that falls inside a > > EfiRuntimeServicesData region. > > Last time when I was seeing reboot/poweroff broken on Flex 5G, I did not > capture any kernel dumps. I will retry with efi=debug and see if I can > get more information. > > In the meantime, could you help me understand if EFI must be running in > virtual address mode when kernel is calling into the services, or it > should work no matter EFI is running in virtual or physical address > mode? Thanks! > There are really three scenarios to consider here: - SetVirtualAddressMap() is never called - SetVirtualAddressMap() is called with a 1:1 mapping for all EFI_MEMORY_RUNTIME regions - SetVirtualAddressMap() is called with an virtual remapping for all EFI_MEMORY_RUNTIME regions. Firmware that is working correctly should not care about which scenario it is running under. All runtime services should simply work as expected. However, there are numerous examples of UEFI in the field (both x86 and ARM), where only the third scenario works, or where the second one is needed to work around problems that occur when using the first one.
diff --git a/arch/arm64/kernel/efi.c b/arch/arm64/kernel/efi.c index fa02efb28e88..8ae0002c72f1 100644 --- a/arch/arm64/kernel/efi.c +++ b/arch/arm64/kernel/efi.c @@ -7,6 +7,7 @@ * Copyright (C) 2013, 2014 Linaro Ltd. */ +#include <linux/dmi.h> #include <linux/efi.h> #include <linux/init.h> @@ -113,12 +114,26 @@ int __init efi_set_mapping_permissions(struct mm_struct *mm, set_permissions, md); } +static const struct dmi_system_id efi_reboot_broken_table[] = { + { + .ident = "Lenovo Flex 5G", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "LENOVO"), + DMI_MATCH(DMI_PRODUCT_FAMILY, "Flex 5G"), + }, + }, + { } /* terminator */ +}; + /* * UpdateCapsule() depends on the system being shutdown via * ResetSystem(). */ bool efi_poweroff_required(void) { + if (dmi_check_system(efi_reboot_broken_table)) + return false; + return efi_enabled(EFI_RUNTIME_SERVICES); }
Poweroff via UEFI Runtime Services doesn't always work on every single arm64 machine. For example, on Lenovo Flex 5G laptop, it results in a system reboot rather than shutdown. Add a DMI check to keep such system stay with the original poweroff method (PSCI). Signed-off-by: Shawn Guo <shawn.guo@linaro.org> --- arch/arm64/kernel/efi.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)