| Message ID | 20200917171808.18297-1-xypron.glpk@gmx.de |
|---|---|
| State | Accepted |
| Commit | 4b71f6dc4e2a6d98f24120dfc2f88d37c37d35e8 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [1/1] efi_loader: QEMU CONFIG_EFI_GRUB_ARM32_WORKAROUND=n | expand |
On Thu, Sep 17, 2020 at 07:18:08PM +0200, Heinrich Schuchardt wrote: > CONFIG_EFI_GRUB_ARM32 is only needed for architectures with caches that are > not managed via CP15 (or for some outdated buggy versions of GRUB). It > makes more sense to disable the setting per architecture than per defconfig. > > Move QEMU's CONFIG_EFI_GRUB_ARM32_WORKAROUND=n from defconfig to Kconfig. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > configs/qemu_arm_defconfig | 1 - > lib/efi_loader/Kconfig | 1 + > 2 files changed, 1 insertion(+), 1 deletion(-) Why? Are we about to move this to be a short list of "default y if ..." instead? Otherwise no, this is the level of detail that should be in a defconfig I think and not Kconfig.
On 9/17/20 7:47 PM, Tom Rini wrote: > On Thu, Sep 17, 2020 at 07:18:08PM +0200, Heinrich Schuchardt wrote: > >> CONFIG_EFI_GRUB_ARM32 is only needed for architectures with caches that are >> not managed via CP15 (or for some outdated buggy versions of GRUB). It >> makes more sense to disable the setting per architecture than per defconfig. >> >> Move QEMU's CONFIG_EFI_GRUB_ARM32_WORKAROUND=n from defconfig to Kconfig. >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >> --- >> configs/qemu_arm_defconfig | 1 - >> lib/efi_loader/Kconfig | 1 + >> 2 files changed, 1 insertion(+), 1 deletion(-) > > Why? Are we about to move this to be a short list of "default y if ..." > instead? Otherwise no, this is the level of detail that should be in a > defconfig I think and not Kconfig. > Hello Tom, yes, in the long run I want to turn this into "default y if". But I first need to a list of all ARCH_* which have a cache which is not controlled via CP15. Best regards Heinrich
On Thu, Sep 17, 2020 at 08:03:24PM +0200, Heinrich Schuchardt wrote: > On 9/17/20 7:47 PM, Tom Rini wrote: > > On Thu, Sep 17, 2020 at 07:18:08PM +0200, Heinrich Schuchardt wrote: > > > >> CONFIG_EFI_GRUB_ARM32 is only needed for architectures with caches that are > >> not managed via CP15 (or for some outdated buggy versions of GRUB). It > >> makes more sense to disable the setting per architecture than per defconfig. > >> > >> Move QEMU's CONFIG_EFI_GRUB_ARM32_WORKAROUND=n from defconfig to Kconfig. > >> > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > >> --- > >> configs/qemu_arm_defconfig | 1 - > >> lib/efi_loader/Kconfig | 1 + > >> 2 files changed, 1 insertion(+), 1 deletion(-) > > > > Why? Are we about to move this to be a short list of "default y if ..." > > instead? Otherwise no, this is the level of detail that should be in a > > defconfig I think and not Kconfig. > > > > Hello Tom, > > yes, in the long run I want to turn this into "default y if". But I > first need to a list of all ARCH_* which have a cache which is not > controlled via CP15. Isn't the bigger problem / list "needs to support old / buggy GRUB" ?
Am 17. September 2020 20:53:20 MESZ schrieb Tom Rini <trini@konsulko.com>: >On Thu, Sep 17, 2020 at 08:03:24PM +0200, Heinrich Schuchardt wrote: >> On 9/17/20 7:47 PM, Tom Rini wrote: >> > On Thu, Sep 17, 2020 at 07:18:08PM +0200, Heinrich Schuchardt >wrote: >> > >> >> CONFIG_EFI_GRUB_ARM32 is only needed for architectures with caches >that are >> >> not managed via CP15 (or for some outdated buggy versions of >GRUB). It >> >> makes more sense to disable the setting per architecture than per >defconfig. >> >> >> >> Move QEMU's CONFIG_EFI_GRUB_ARM32_WORKAROUND=n from defconfig to >Kconfig. >> >> >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >> >> --- >> >> configs/qemu_arm_defconfig | 1 - >> >> lib/efi_loader/Kconfig | 1 + >> >> 2 files changed, 1 insertion(+), 1 deletion(-) >> > >> > Why? Are we about to move this to be a short list of "default y if >..." >> > instead? Otherwise no, this is the level of detail that should be >in a >> > defconfig I think and not Kconfig. >> > >> >> Hello Tom, >> >> yes, in the long run I want to turn this into "default y if". But I >> first need to a list of all ARCH_* which have a cache which is not >> controlled via CP15. > >Isn't the bigger problem / list "needs to support old / buggy GRUB" ? There are no operating systems distributing 2021 U-Boot with 2018 GRUB. Best regards Heinrich
On Fri, Sep 18, 2020 at 06:07:42AM +0200, Heinrich Schuchardt wrote: > Am 17. September 2020 20:53:20 MESZ schrieb Tom Rini <trini@konsulko.com>: > >On Thu, Sep 17, 2020 at 08:03:24PM +0200, Heinrich Schuchardt wrote: > >> On 9/17/20 7:47 PM, Tom Rini wrote: > >> > On Thu, Sep 17, 2020 at 07:18:08PM +0200, Heinrich Schuchardt > >wrote: > >> > > >> >> CONFIG_EFI_GRUB_ARM32 is only needed for architectures with caches > >that are > >> >> not managed via CP15 (or for some outdated buggy versions of > >GRUB). It > >> >> makes more sense to disable the setting per architecture than per > >defconfig. > >> >> > >> >> Move QEMU's CONFIG_EFI_GRUB_ARM32_WORKAROUND=n from defconfig to > >Kconfig. > >> >> > >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > >> >> --- > >> >> configs/qemu_arm_defconfig | 1 - > >> >> lib/efi_loader/Kconfig | 1 + > >> >> 2 files changed, 1 insertion(+), 1 deletion(-) > >> > > >> > Why? Are we about to move this to be a short list of "default y if > >..." > >> > instead? Otherwise no, this is the level of detail that should be > >in a > >> > defconfig I think and not Kconfig. > >> > > >> > >> Hello Tom, > >> > >> yes, in the long run I want to turn this into "default y if". But I > >> first need to a list of all ARCH_* which have a cache which is not > >> controlled via CP15. > > > >Isn't the bigger problem / list "needs to support old / buggy GRUB" ? > > There are no operating systems distributing 2021 U-Boot with 2018 GRUB. Then we should remove default y there after v2020.10 I think, and get the distro folks to ack/review that. I don't think there's any cases of "SoC doesn't use CP15 for cache" and "SoC uses GRUB".
On 9/18/20 1:28 PM, Tom Rini wrote: > On Fri, Sep 18, 2020 at 06:07:42AM +0200, Heinrich Schuchardt wrote: >> Am 17. September 2020 20:53:20 MESZ schrieb Tom Rini <trini@konsulko.com>: >>> On Thu, Sep 17, 2020 at 08:03:24PM +0200, Heinrich Schuchardt wrote: >>>> On 9/17/20 7:47 PM, Tom Rini wrote: >>>>> On Thu, Sep 17, 2020 at 07:18:08PM +0200, Heinrich Schuchardt >>> wrote: >>>>> >>>>>> CONFIG_EFI_GRUB_ARM32 is only needed for architectures with caches >>> that are >>>>>> not managed via CP15 (or for some outdated buggy versions of >>> GRUB). It >>>>>> makes more sense to disable the setting per architecture than per >>> defconfig. >>>>>> >>>>>> Move QEMU's CONFIG_EFI_GRUB_ARM32_WORKAROUND=n from defconfig to >>> Kconfig. >>>>>> >>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >>>>>> --- >>>>>> configs/qemu_arm_defconfig | 1 - >>>>>> lib/efi_loader/Kconfig | 1 + >>>>>> 2 files changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> Why? Are we about to move this to be a short list of "default y if >>> ..." >>>>> instead? Otherwise no, this is the level of detail that should be >>> in a >>>>> defconfig I think and not Kconfig. >>>>> >>>> >>>> Hello Tom, >>>> >>>> yes, in the long run I want to turn this into "default y if". But I >>>> first need to a list of all ARCH_* which have a cache which is not >>>> controlled via CP15. >>> >>> Isn't the bigger problem / list "needs to support old / buggy GRUB" ? >> >> There are no operating systems distributing 2021 U-Boot with 2018 GRUB. > > Then we should remove default y there after v2020.10 I think, and get > the distro folks to ack/review that. I don't think there's any cases of > "SoC doesn't use CP15 for cache" and "SoC uses GRUB". > How about l2_cache_disable() in arch/arm/mach-kirkwood/cache.c? Best regards Heinrich
On Fri, Sep 18, 2020 at 07:14:02PM +0200, Heinrich Schuchardt wrote: > On 9/18/20 1:28 PM, Tom Rini wrote: > > On Fri, Sep 18, 2020 at 06:07:42AM +0200, Heinrich Schuchardt wrote: > >> Am 17. September 2020 20:53:20 MESZ schrieb Tom Rini <trini@konsulko.com>: > >>> On Thu, Sep 17, 2020 at 08:03:24PM +0200, Heinrich Schuchardt wrote: > >>>> On 9/17/20 7:47 PM, Tom Rini wrote: > >>>>> On Thu, Sep 17, 2020 at 07:18:08PM +0200, Heinrich Schuchardt > >>> wrote: > >>>>> > >>>>>> CONFIG_EFI_GRUB_ARM32 is only needed for architectures with caches > >>> that are > >>>>>> not managed via CP15 (or for some outdated buggy versions of > >>> GRUB). It > >>>>>> makes more sense to disable the setting per architecture than per > >>> defconfig. > >>>>>> > >>>>>> Move QEMU's CONFIG_EFI_GRUB_ARM32_WORKAROUND=n from defconfig to > >>> Kconfig. > >>>>>> > >>>>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > >>>>>> --- > >>>>>> configs/qemu_arm_defconfig | 1 - > >>>>>> lib/efi_loader/Kconfig | 1 + > >>>>>> 2 files changed, 1 insertion(+), 1 deletion(-) > >>>>> > >>>>> Why? Are we about to move this to be a short list of "default y if > >>> ..." > >>>>> instead? Otherwise no, this is the level of detail that should be > >>> in a > >>>>> defconfig I think and not Kconfig. > >>>>> > >>>> > >>>> Hello Tom, > >>>> > >>>> yes, in the long run I want to turn this into "default y if". But I > >>>> first need to a list of all ARCH_* which have a cache which is not > >>>> controlled via CP15. > >>> > >>> Isn't the bigger problem / list "needs to support old / buggy GRUB" ? > >> > >> There are no operating systems distributing 2021 U-Boot with 2018 GRUB. > > > > Then we should remove default y there after v2020.10 I think, and get > > the distro folks to ack/review that. I don't think there's any cases of > > "SoC doesn't use CP15 for cache" and "SoC uses GRUB". > > How about l2_cache_disable() in arch/arm/mach-kirkwood/cache.c? What's the board that uses that and where "SoC uses GRUB" comes in to play. What are the board(s) there we're talking about?
> From: Heinrich Schuchardt <xypron.glpk@gmx.de> > Date: Thu, 17 Sep 2020 20:03:24 +0200 > > On 9/17/20 7:47 PM, Tom Rini wrote: > > On Thu, Sep 17, 2020 at 07:18:08PM +0200, Heinrich Schuchardt wrote: > > > >> CONFIG_EFI_GRUB_ARM32 is only needed for architectures with caches that are > >> not managed via CP15 (or for some outdated buggy versions of GRUB). It > >> makes more sense to disable the setting per architecture than per defconfig. > >> > >> Move QEMU's CONFIG_EFI_GRUB_ARM32_WORKAROUND=n from defconfig to Kconfig. > >> > >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > >> --- > >> configs/qemu_arm_defconfig | 1 - > >> lib/efi_loader/Kconfig | 1 + > >> 2 files changed, 1 insertion(+), 1 deletion(-) > > > > Why? Are we about to move this to be a short list of "default y if ..." > > instead? Otherwise no, this is the level of detail that should be in a > > defconfig I think and not Kconfig. > > > > Hello Tom, > > yes, in the long run I want to turn this into "default y if". But I > first need to a list of all ARCH_* which have a cache which is not > controlled via CP15. It is fairly standard for SoCs with Cortex-A9 cores. Maybe some SoCs with Cortex-A8 are affected as well. Probably the list of affected SoCs is those that implement v7_outer_cache_enable(). However, since other 32-bit ARM UEFI implementation are generally not unavailable, I'm not sure disabling this workaround on other SoCs is feasable since the code oath for booting OSes without the MMU enabled may not be very well tested.
On 19.09.20 13:42, Mark Kettenis wrote: >> From: Heinrich Schuchardt <xypron.glpk@gmx.de> >> Date: Thu, 17 Sep 2020 20:03:24 +0200 >> >> On 9/17/20 7:47 PM, Tom Rini wrote: >>> On Thu, Sep 17, 2020 at 07:18:08PM +0200, Heinrich Schuchardt wrote: >>> >>>> CONFIG_EFI_GRUB_ARM32 is only needed for architectures with caches that are >>>> not managed via CP15 (or for some outdated buggy versions of GRUB). It >>>> makes more sense to disable the setting per architecture than per defconfig. >>>> >>>> Move QEMU's CONFIG_EFI_GRUB_ARM32_WORKAROUND=n from defconfig to Kconfig. >>>> >>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >>>> --- >>>> configs/qemu_arm_defconfig | 1 - >>>> lib/efi_loader/Kconfig | 1 + >>>> 2 files changed, 1 insertion(+), 1 deletion(-) >>> >>> Why? Are we about to move this to be a short list of "default y if ..." >>> instead? Otherwise no, this is the level of detail that should be in a >>> defconfig I think and not Kconfig. >>> >> >> Hello Tom, >> >> yes, in the long run I want to turn this into "default y if". But I >> first need to a list of all ARCH_* which have a cache which is not >> controlled via CP15. > > It is fairly standard for SoCs with Cortex-A9 cores. Maybe some SoCs > with Cortex-A8 are affected as well. Probably the list of affected > SoCs is those that implement v7_outer_cache_enable(). > git grep -n v7_outer_cache_enable yields arch/arm/cpu/armv7/cache_v7.c:210:__weak void v7_outer_cache_enable(void) {} arch/arm/include/asm/armv7.h:128:void v7_outer_cache_enable(void); arch/arm/mach-imx/cache.c:83:void v7_outer_cache_enable(void) arch/arm/mach-mvebu/cpu.c:658:void v7_outer_cache_enable(void) arch/arm/mach-omap2/omap3/board.c:433:void v7_outer_cache_enable(void) arch/arm/mach-omap2/omap4/hwinit.c:166:void v7_outer_cache_enable(void) arch/arm/mach-s5pc1xx/cache.c:27:void v7_outer_cache_enable(void) arch/arm/mach-socfpga/misc.c:69:void v7_outer_cache_enable(void) arch/arm/mach-uniphier/arm32/cache-uniphier.c:277:void v7_outer_cache_enable(void) arch/arm/mach-kirkwood/cache.c has an architecture specific implementation of l2_cache_disable(). > However, since other 32-bit ARM UEFI implementation are generally not > unavailable, I'm not sure disabling this workaround on other SoCs is > feasable since the code oath for booting OSes without the MMU enabled > may not be very well tested. > According to the UEFI spec we have to fulfill the following requirements when handing over to the OS on ARM 32bit: - MMU enabled - Instruction and data caches enabled - Access flag disabled - Translation remap disabled - Fast Context Switch Extension (FCSE) must be disabled - caches requiring platform information to manage or invoke non-architectural cache/TLB lockdown mechanisms disabled With the GRUB workaround enabled we call cleanup_before_linux(). According to README.arm_caches this shall disable the MMU. Mark, I do not understand your concerns regarding a disabled MMU when *not* calling cleanup_before_linux(). Best regards Heinrich
diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig index 1ffa727e63..278d8f41f4 100644 --- a/configs/qemu_arm_defconfig +++ b/configs/qemu_arm_defconfig @@ -56,4 +56,3 @@ CONFIG_USB=y CONFIG_DM_USB=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_PCI=y -# CONFIG_EFI_GRUB_ARM32_WORKAROUND is not set diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig index bad1a29ba8..ab42f3ba75 100644 --- a/lib/efi_loader/Kconfig +++ b/lib/efi_loader/Kconfig @@ -164,6 +164,7 @@ config EFI_HAVE_RUNTIME_RESET config EFI_GRUB_ARM32_WORKAROUND bool "Workaround for GRUB on 32bit ARM" + default n if ARCH_QEMU default y depends on ARM && !ARM64 help
CONFIG_EFI_GRUB_ARM32 is only needed for architectures with caches that are not managed via CP15 (or for some outdated buggy versions of GRUB). It makes more sense to disable the setting per architecture than per defconfig. Move QEMU's CONFIG_EFI_GRUB_ARM32_WORKAROUND=n from defconfig to Kconfig. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- configs/qemu_arm_defconfig | 1 - lib/efi_loader/Kconfig | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) -- 2.28.0