[U-Boot] [PATCH 1/1] efi_loader: re-enable GRUB workaround on 32bit ARM
GRUB on ARM 32bit prior to version 2.04 lacks proper handling of caches. In U-Boot v2019.04 a workaround for this was inadvertently removed.
The workaround is currently also needed for booting on systems with caches that cannot be managed via CP15 (e.g. with an i.MX6 CPU).
Re-enable the workaround and make it customizable.
Fixes: f69d63fae281 ("efi_loader: use efi_start_image() for bootefi") Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/efi_loader/Kconfig | 8 ++++++++ lib/efi_loader/efi_boottime.c | 28 +++++++++++++--------------- 2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index a7f2c68fa9..c7027a9676 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -107,4 +107,12 @@ config EFI_HAVE_RUNTIME_RESET default y depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || SYSRESET_X86
+config EFI_GRUB_ARM32_WORKAROUND + bool "Workaround for GRUB on 32bit ARM" + default y + depends on ARM && !ARM64 + help + GRUB prior to version 2.04 requires U-Boot to disable caches. This + workaround currently is also needed on systems with caches that + cannot be managed via CP15. endif diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 4f6e8d1679..f75ca1a67c 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -39,14 +39,6 @@ LIST_HEAD(efi_register_notify_events); /* Handle of the currently executing image */ static efi_handle_t current_image;
-/* - * If we're running on nasty systems (32bit ARM booting into non-EFI Linux) - * we need to do trickery with caches. Since we don't want to break the EFI - * aware boot path, only apply hacks when loading exiting directly (breaking - * direct Linux EFI booting along the way - oh well). - */ -static bool efi_is_direct_boot = true; - #ifdef CONFIG_ARM /* * The "gd" pointer lives in a register on ARM and AArch64 that we declare @@ -1911,13 +1903,21 @@ error: */ static void efi_exit_caches(void) { -#if defined(CONFIG_ARM) && !defined(CONFIG_ARM64) +#if defined(CONFIG_EFI_GRUB_ARM32_WORKAROUND) /* - * Grub on 32bit ARM needs to have caches disabled before jumping into - * a zImage, but does not know of all cache layers. Give it a hand. + * Boooting Linux via GRUB prior to version 2.04 fails on 32bit ARM if + * caches are enabled. + * + * TODO: + * According to the UEFI spec caches that can be managed via CP15 + * operations should be enabled. Caches requiring platform information + * to manage should be disabled. This should not happen in + * ExitBootServices() but before invoking any UEFI binary is invoked. + * + * We want to keep the current workaround while GRUB prior to version + * 2.04 is still in use. */ - if (efi_is_direct_boot) - cleanup_before_linux(); + cleanup_before_linux(); #endif }
@@ -2893,8 +2893,6 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, if (ret != EFI_SUCCESS) return EFI_EXIT(EFI_INVALID_PARAMETER);
- efi_is_direct_boot = false; - image_obj->exit_data_size = exit_data_size; image_obj->exit_data = exit_data;
-- 2.20.1
On 27.07.19 10:02, Heinrich Schuchardt wrote:
GRUB on ARM 32bit prior to version 2.04 lacks proper handling of caches. In U-Boot v2019.04 a workaround for this was inadvertently removed.
The workaround is currently also needed for booting on systems with caches that cannot be managed via CP15 (e.g. with an i.MX6 CPU).
Re-enable the workaround and make it customizable.
Fixes: f69d63fae281 ("efi_loader: use efi_start_image() for bootefi") Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_loader/Kconfig | 8 ++++++++ lib/efi_loader/efi_boottime.c | 28 +++++++++++++--------------- 2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index a7f2c68fa9..c7027a9676 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -107,4 +107,12 @@ config EFI_HAVE_RUNTIME_RESET default y depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || SYSRESET_X86
+config EFI_GRUB_ARM32_WORKAROUND
- bool "Workaround for GRUB on 32bit ARM"
- default y
- depends on ARM && !ARM64
- help
GRUB prior to version 2.04 requires U-Boot to disable caches. Thisworkaround currently is also needed on systems with caches thatcannot be managed via CP15.diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 4f6e8d1679..f75ca1a67c 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -39,14 +39,6 @@ LIST_HEAD(efi_register_notify_events); /* Handle of the currently executing image */ static efi_handle_t current_image;
-/*
- If we're running on nasty systems (32bit ARM booting into non-EFI Linux)
- we need to do trickery with caches. Since we don't want to break the EFI
- aware boot path, only apply hacks when loading exiting directly (breaking
- direct Linux EFI booting along the way - oh well).
- */
-static bool efi_is_direct_boot = true;
By removing the toggling, you now couple a U-Boot configuration variant to a specific grub version. New grub versions will run Linux using the EFI stub. So enabling CONFIG_EFI_GRUB_ARM32_WORKAROUND means only old versions work, but new grub versions break.
We really should treat this like any other erratum and make it as runtime detected as possible, so that we don't block people from making forward progress towards a the "sane" boot path.
Btw, has there been any solution for the non-CP15 caches? They would still be broken with this applied, as payloads can rely on CP15 caches enabled (for unaligned accesses), but non-CP15 caches disabled, right?
Thanks,
Alex
- #ifdef CONFIG_ARM /*
- The "gd" pointer lives in a register on ARM and AArch64 that we declare
@@ -1911,13 +1903,21 @@ error: */ static void efi_exit_caches(void) { -#if defined(CONFIG_ARM) && !defined(CONFIG_ARM64) +#if defined(CONFIG_EFI_GRUB_ARM32_WORKAROUND) /*
* Grub on 32bit ARM needs to have caches disabled before jumping into* a zImage, but does not know of all cache layers. Give it a hand.
* Boooting Linux via GRUB prior to version 2.04 fails on 32bit ARM if* caches are enabled.** TODO:* According to the UEFI spec caches that can be managed via CP15* operations should be enabled. Caches requiring platform information* to manage should be disabled. This should not happen in* ExitBootServices() but before invoking any UEFI binary is invoked.** We want to keep the current workaround while GRUB prior to version* 2.04 is still in use.
- if (efi_is_direct_boot)
cleanup_before_linux();
- cleanup_before_linux(); #endif }
@@ -2893,8 +2893,6 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, if (ret != EFI_SUCCESS) return EFI_EXIT(EFI_INVALID_PARAMETER);
- efi_is_direct_boot = false;
- image_obj->exit_data_size = exit_data_size; image_obj->exit_data = exit_data;
-- 2.20.1
On 7/27/19 11:58 AM, Alexander Graf wrote:
On 27.07.19 10:02, Heinrich Schuchardt wrote:
GRUB on ARM 32bit prior to version 2.04 lacks proper handling of caches. In U-Boot v2019.04 a workaround for this was inadvertently removed.
The workaround is currently also needed for booting on systems with caches that cannot be managed via CP15 (e.g. with an i.MX6 CPU).
Re-enable the workaround and make it customizable.
Fixes: f69d63fae281 ("efi_loader: use efi_start_image() for bootefi") Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_loader/Kconfig | 8 ++++++++ lib/efi_loader/efi_boottime.c | 28 +++++++++++++--------------- 2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index a7f2c68fa9..c7027a9676 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -107,4 +107,12 @@ config EFI_HAVE_RUNTIME_RESET default y depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || SYSRESET_X86
+config EFI_GRUB_ARM32_WORKAROUND + bool "Workaround for GRUB on 32bit ARM" + default y + depends on ARM && !ARM64 + help + GRUB prior to version 2.04 requires U-Boot to disable caches. This + workaround currently is also needed on systems with caches that + cannot be managed via CP15. endif diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 4f6e8d1679..f75ca1a67c 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -39,14 +39,6 @@ LIST_HEAD(efi_register_notify_events); /* Handle of the currently executing image */ static efi_handle_t current_image;
-/*
- If we're running on nasty systems (32bit ARM booting into non-EFI
Linux)
- we need to do trickery with caches. Since we don't want to break
the EFI
- aware boot path, only apply hacks when loading exiting directly
(breaking
- direct Linux EFI booting along the way - oh well).
- */
-static bool efi_is_direct_boot = true;
By removing the toggling, you now couple a U-Boot configuration variant to a specific grub version. New grub versions will run Linux using the EFI stub. So enabling CONFIG_EFI_GRUB_ARM32_WORKAROUND means only old versions work, but new grub versions break.
No, the Wandboard boots fine with GRUB 2.04 and Linux 4.19.55. Do you have any negative test results?
We really should treat this like any other erratum and make it as runtime detected as possible, so that we don't block people from making forward progress towards a the "sane" boot path.
It is not possible to determine at runtime which assumptions a loaded EFI binary makes about the cache. Your original logic counting the number of StartImage() invocations cannot detect this. This is why I added a customizing option.
Btw, has there been any solution for the non-CP15 caches? They would still be broken with this applied, as payloads can rely on CP15 caches enabled (for unaligned accesses), but non-CP15 caches disabled, right?
No, this will need future work. With CONFIG_EFI_GRUB_ARM32_WORKAROUND=y all caches are disabled.
On i.MX6 CONFIG_SYS_L2CACHE_OFF=y selectively disables the non-CP15 cache. So here CONFIG_EFI_GRUB_ARM32_WORKAROUND=n and CONFIG_SYS_L2CACHE_OFF=y result in compliance with the UEFI spec.
Best regards
Heinrich
Thanks,
Alex
#ifdef CONFIG_ARM /* * The "gd" pointer lives in a register on ARM and AArch64 that we declare @@ -1911,13 +1903,21 @@ error: */ static void efi_exit_caches(void) { -#if defined(CONFIG_ARM) && !defined(CONFIG_ARM64) +#if defined(CONFIG_EFI_GRUB_ARM32_WORKAROUND) /* - * Grub on 32bit ARM needs to have caches disabled before jumping into - * a zImage, but does not know of all cache layers. Give it a hand. + * Boooting Linux via GRUB prior to version 2.04 fails on 32bit ARM if + * caches are enabled. + * + * TODO: + * According to the UEFI spec caches that can be managed via CP15 + * operations should be enabled. Caches requiring platform information + * to manage should be disabled. This should not happen in + * ExitBootServices() but before invoking any UEFI binary is invoked. + * + * We want to keep the current workaround while GRUB prior to version + * 2.04 is still in use. */ - if (efi_is_direct_boot) - cleanup_before_linux(); + cleanup_before_linux(); #endif }
@@ -2893,8 +2893,6 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, if (ret != EFI_SUCCESS) return EFI_EXIT(EFI_INVALID_PARAMETER);
- efi_is_direct_boot = false;
image_obj->exit_data_size = exit_data_size; image_obj->exit_data = exit_data;
-- 2.20.1
On Sun, Jul 28, 2019 at 10:13:24AM +0200, Heinrich Schuchardt wrote:
On 7/27/19 11:58 AM, Alexander Graf wrote:
On 27.07.19 10:02, Heinrich Schuchardt wrote:
GRUB on ARM 32bit prior to version 2.04 lacks proper handling of caches. In U-Boot v2019.04 a workaround for this was inadvertently removed.
The workaround is currently also needed for booting on systems with caches that cannot be managed via CP15 (e.g. with an i.MX6 CPU).
Re-enable the workaround and make it customizable.
Fixes: f69d63fae281 ("efi_loader: use efi_start_image() for bootefi") Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_loader/Kconfig | 8 ++++++++ lib/efi_loader/efi_boottime.c | 28 +++++++++++++--------------- 2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index a7f2c68fa9..c7027a9676 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -107,4 +107,12 @@ config EFI_HAVE_RUNTIME_RESET default y depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || SYSRESET_X86
+config EFI_GRUB_ARM32_WORKAROUND + bool "Workaround for GRUB on 32bit ARM" + default y + depends on ARM && !ARM64 + help + GRUB prior to version 2.04 requires U-Boot to disable caches. This + workaround currently is also needed on systems with caches that + cannot be managed via CP15. endif diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 4f6e8d1679..f75ca1a67c 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -39,14 +39,6 @@ LIST_HEAD(efi_register_notify_events); /* Handle of the currently executing image */ static efi_handle_t current_image;
-/*
- If we're running on nasty systems (32bit ARM booting into non-EFI
Linux)
- we need to do trickery with caches. Since we don't want to break
the EFI
- aware boot path, only apply hacks when loading exiting directly
(breaking
- direct Linux EFI booting along the way - oh well).
- */
-static bool efi_is_direct_boot = true;
By removing the toggling, you now couple a U-Boot configuration variant to a specific grub version. New grub versions will run Linux using the EFI stub. So enabling CONFIG_EFI_GRUB_ARM32_WORKAROUND means only old versions work, but new grub versions break.
No, the Wandboard boots fine with GRUB 2.04 and Linux 4.19.55. Do you have any negative test results?
We really should treat this like any other erratum and make it as runtime detected as possible, so that we don't block people from making forward progress towards a the "sane" boot path.
It is not possible to determine at runtime which assumptions a loaded EFI binary makes about the cache. Your original logic counting the number of StartImage() invocations cannot detect this. This is why I added a customizing option.
Btw, has there been any solution for the non-CP15 caches? They would still be broken with this applied, as payloads can rely on CP15 caches enabled (for unaligned accesses), but non-CP15 caches disabled, right?
No, this will need future work. With CONFIG_EFI_GRUB_ARM32_WORKAROUND=y all caches are disabled.
On i.MX6 CONFIG_SYS_L2CACHE_OFF=y selectively disables the non-CP15 cache. So here CONFIG_EFI_GRUB_ARM32_WORKAROUND=n and CONFIG_SYS_L2CACHE_OFF=y result in compliance with the UEFI spec.
Where did all of this end up, in terms of what combinations of things do / don't work, on current master (or v2019.10-rc2) ? Thanks all!
On 8/19/19 9:20 PM, Tom Rini wrote:
On Sun, Jul 28, 2019 at 10:13:24AM +0200, Heinrich Schuchardt wrote:
On 7/27/19 11:58 AM, Alexander Graf wrote:
On 27.07.19 10:02, Heinrich Schuchardt wrote:
GRUB on ARM 32bit prior to version 2.04 lacks proper handling of caches. In U-Boot v2019.04 a workaround for this was inadvertently removed.
The workaround is currently also needed for booting on systems with caches that cannot be managed via CP15 (e.g. with an i.MX6 CPU).
Re-enable the workaround and make it customizable.
Fixes: f69d63fae281 ("efi_loader: use efi_start_image() for bootefi") Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/efi_loader/Kconfig | 8 ++++++++ lib/efi_loader/efi_boottime.c | 28 +++++++++++++--------------- 2 files changed, 21 insertions(+), 15 deletions(-)
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index a7f2c68fa9..c7027a9676 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -107,4 +107,12 @@ config EFI_HAVE_RUNTIME_RESET default y depends on ARCH_BCM283X || FSL_LAYERSCAPE || PSCI_RESET || SYSRESET_X86
+config EFI_GRUB_ARM32_WORKAROUND + bool "Workaround for GRUB on 32bit ARM" + default y + depends on ARM && !ARM64 + help + GRUB prior to version 2.04 requires U-Boot to disable caches. This + workaround currently is also needed on systems with caches that + cannot be managed via CP15. endif diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 4f6e8d1679..f75ca1a67c 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -39,14 +39,6 @@ LIST_HEAD(efi_register_notify_events); /* Handle of the currently executing image */ static efi_handle_t current_image;
-/*
- If we're running on nasty systems (32bit ARM booting into non-EFI
Linux)
- we need to do trickery with caches. Since we don't want to break
the EFI
- aware boot path, only apply hacks when loading exiting directly
(breaking
- direct Linux EFI booting along the way - oh well).
- */
-static bool efi_is_direct_boot = true;
By removing the toggling, you now couple a U-Boot configuration variant to a specific grub version. New grub versions will run Linux using the EFI stub. So enabling CONFIG_EFI_GRUB_ARM32_WORKAROUND means only old versions work, but new grub versions break.
No, the Wandboard boots fine with GRUB 2.04 and Linux 4.19.55. Do you have any negative test results?
We really should treat this like any other erratum and make it as runtime detected as possible, so that we don't block people from making forward progress towards a the "sane" boot path.
It is not possible to determine at runtime which assumptions a loaded EFI binary makes about the cache. Your original logic counting the number of StartImage() invocations cannot detect this. This is why I added a customizing option.
Btw, has there been any solution for the non-CP15 caches? They would still be broken with this applied, as payloads can rely on CP15 caches enabled (for unaligned accesses), but non-CP15 caches disabled, right?
No, this will need future work. With CONFIG_EFI_GRUB_ARM32_WORKAROUND=y all caches are disabled.
On i.MX6 CONFIG_SYS_L2CACHE_OFF=y selectively disables the non-CP15 cache. So here CONFIG_EFI_GRUB_ARM32_WORKAROUND=n and CONFIG_SYS_L2CACHE_OFF=y result in compliance with the UEFI spec.
Where did all of this end up, in terms of what combinations of things do / don't work, on current master (or v2019.10-rc2) ? Thanks all!
The patch is merged with CONFIG_EFI_GRUB_ARM32_WORKAROUND=y as default. I have tested
* Wandboard (i.MX6) with Debian Buster * Orange Pi PC (Allwinner H3) with Debian Buster * BananaPi (Allwinner A20) with OpenBSD and Debian Buster
and found no issues.
Best regards
Heinrich
participants (3)
-
Alexander Graf -
Heinrich Schuchardt -
Tom Rini