[U-Boot,1/1] efi_loader: remove efi_exit_caches()
diff mbox series

Message ID 20190719182545.15662-1-xypron.glpk@gmx.de
State Rejected, archived
Delegated to: Heinrich Schuchardt
Headers show
Series
  • [U-Boot,1/1] efi_loader: remove efi_exit_caches()
Related show

Commit Message

Heinrich Schuchardt July 19, 2019, 6:25 p.m. UTC
In GRUB before 2.04 a bug existed which did not allow booting some ARM32
boards if U-Boot did not disable caches, cf.
https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html

In ExitBootServices() we were disabling the caches by calling
cleanup_before_linux(). This workaround is not needed anymore.

The UEFI spec requires that caches are enabled but architecturally defined
caches should be disabled. But this requirement has to be fulfilled when
invoking StartImage() and not after calling ExitBootServices(). So there is
no reason for calling cleanup_before_linux() here.

Since commit f69d63fae281 ("efi_loader: use efi_start_image() for
bootefi"), i.e. v2019.04, we have not been calling efi_exit_caches()
anymore which in turn would call cleanup_before_linux().

Remove the obsolete function efi_exit_caches().

Suggested-by: Alexander Graf <agraf@csgraf.de>
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_boottime.c | 28 ----------------------------
 1 file changed, 28 deletions(-)

--
2.20.1

Comments

Peter Robinson July 19, 2019, 7:06 p.m. UTC | #1
On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
> boards if U-Boot did not disable caches, cf.
> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
>
> In ExitBootServices() we were disabling the caches by calling
> cleanup_before_linux(). This workaround is not needed anymore.

Do we want to remove this straight away? A lot of distributions will
take time to move to grub 2.04 because it's been a long time between
grub releases so they'll have quite a patch delta to re-align to the
new release. Fedora for example will rebase to grub 2.04 in Fedora 32
which will start development end of August but won't be released until
next year.

> The UEFI spec requires that caches are enabled but architecturally defined
> caches should be disabled. But this requirement has to be fulfilled when
> invoking StartImage() and not after calling ExitBootServices(). So there is
> no reason for calling cleanup_before_linux() here.
>
> Since commit f69d63fae281 ("efi_loader: use efi_start_image() for
> bootefi"), i.e. v2019.04, we have not been calling efi_exit_caches()
> anymore which in turn would call cleanup_before_linux().
>
> Remove the obsolete function efi_exit_caches().
>
> Suggested-by: Alexander Graf <agraf@csgraf.de>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_boottime.c | 28 ----------------------------
>  1 file changed, 28 deletions(-)
>
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 4f6e8d1679..f84ee24416 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
> @@ -1906,21 +1898,6 @@ error:
>         return EFI_EXIT(ret);
>  }
>
> -/**
> - * efi_exit_caches() - fix up caches for EFI payloads if necessary
> - */
> -static void efi_exit_caches(void)
> -{
> -#if defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
> -       /*
> -        * 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.
> -        */
> -       if (efi_is_direct_boot)
> -               cleanup_before_linux();
> -#endif
> -}
> -
>  /**
>   * efi_exit_boot_services() - stop all boot services
>   * @image_handle: handle of the loaded image
> @@ -1990,9 +1967,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
>         /* Patch out unsupported runtime function */
>         efi_runtime_detach();
>
> -       /* Fix up caches for EFI payloads if necessary */
> -       efi_exit_caches();
> -
>         /* This stops all lingering devices */
>         bootm_disable_interrupts();
>
> @@ -2893,8 +2867,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
>
Jonathan Gray July 19, 2019, 11:59 p.m. UTC | #2
On Fri, Jul 19, 2019 at 08:25:45PM +0200, Heinrich Schuchardt wrote:
> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
> boards if U-Boot did not disable caches, cf.
> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
> 
> In ExitBootServices() we were disabling the caches by calling
> cleanup_before_linux(). This workaround is not needed anymore.

It is required for at least OpenBSD, FreeBSD and NetBSD.
Which are all now forced to carry patches to be able to use U-Boot.

> 
> The UEFI spec requires that caches are enabled but architecturally defined
> caches should be disabled. But this requirement has to be fulfilled when
> invoking StartImage() and not after calling ExitBootServices(). So there is
> no reason for calling cleanup_before_linux() here.
> 
> Since commit f69d63fae281 ("efi_loader: use efi_start_image() for
> bootefi"), i.e. v2019.04, we have not been calling efi_exit_caches()
> anymore which in turn would call cleanup_before_linux().
> 
> Remove the obsolete function efi_exit_caches().
> 
> Suggested-by: Alexander Graf <agraf@csgraf.de>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_boottime.c | 28 ----------------------------
>  1 file changed, 28 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 4f6e8d1679..f84ee24416 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
> @@ -1906,21 +1898,6 @@ error:
>  	return EFI_EXIT(ret);
>  }
> 
> -/**
> - * efi_exit_caches() - fix up caches for EFI payloads if necessary
> - */
> -static void efi_exit_caches(void)
> -{
> -#if defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
> -	/*
> -	 * 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.
> -	 */
> -	if (efi_is_direct_boot)
> -		cleanup_before_linux();
> -#endif
> -}
> -
>  /**
>   * efi_exit_boot_services() - stop all boot services
>   * @image_handle: handle of the loaded image
> @@ -1990,9 +1967,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
>  	/* Patch out unsupported runtime function */
>  	efi_runtime_detach();
> 
> -	/* Fix up caches for EFI payloads if necessary */
> -	efi_exit_caches();
> -
>  	/* This stops all lingering devices */
>  	bootm_disable_interrupts();
> 
> @@ -2893,8 +2867,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
>
Heinrich Schuchardt July 20, 2019, 4:19 a.m. UTC | #3
On 7/20/19 1:59 AM, Jonathan Gray wrote:
> On Fri, Jul 19, 2019 at 08:25:45PM +0200, Heinrich Schuchardt wrote:
>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
>> boards if U-Boot did not disable caches, cf.
>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
>>
>> In ExitBootServices() we were disabling the caches by calling
>> cleanup_before_linux(). This workaround is not needed anymore.
>
> It is required for at least OpenBSD, FreeBSD and NetBSD.
> Which are all now forced to carry patches to be able to use U-Boot.

As described below this code does not remove any functionality that was
active in v2019.04 or v2019.07.

The logic in pre-v2019.04 to change cache status in ExitBootImage() has
no relation to the UEFI spec. If we do anything it has to be at an
earlier stage.

I am able to reproduce that OpenBSD on ARM32 is broken with current
U-Boot. So some further work is needed.

My testing results on a Banana Pi (an Allwinner A20 system) were as follows:

Without any change the Banana Pi hangs on

boot>
booting sd0a:/bsd: 4519648+675728+235352+561868
[296447+107+311232+275275]=0x68f5b0
EHCI failed to shut down host controller.

CONFIG_SYS_L2CACHE_OFF=y has no effect on Allwinner CPUs (mach-sunxi) so
the Banana Pi keeps hanging.

When cleanup_before_linux() is called it boots into OpenBSD.

cleanup_before_linux() disables all caches and this contradicts the UEFI
specification.

Best regards

Heinrich

>
>>
>> The UEFI spec requires that caches are enabled but architecturally defined
>> caches should be disabled. But this requirement has to be fulfilled when
>> invoking StartImage() and not after calling ExitBootServices(). So there is
>> no reason for calling cleanup_before_linux() here.
>>
>> Since commit f69d63fae281 ("efi_loader: use efi_start_image() for
>> bootefi"), i.e. v2019.04, we have not been calling efi_exit_caches()
>> anymore which in turn would call cleanup_before_linux().
>>
>> Remove the obsolete function efi_exit_caches().
>>
>> Suggested-by: Alexander Graf <agraf@csgraf.de>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>  lib/efi_loader/efi_boottime.c | 28 ----------------------------
>>  1 file changed, 28 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>> index 4f6e8d1679..f84ee24416 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
>> @@ -1906,21 +1898,6 @@ error:
>>  	return EFI_EXIT(ret);
>>  }
>>
>> -/**
>> - * efi_exit_caches() - fix up caches for EFI payloads if necessary
>> - */
>> -static void efi_exit_caches(void)
>> -{
>> -#if defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
>> -	/*
>> -	 * 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.
>> -	 */
>> -	if (efi_is_direct_boot)
>> -		cleanup_before_linux();
>> -#endif
>> -}
>> -
>>  /**
>>   * efi_exit_boot_services() - stop all boot services
>>   * @image_handle: handle of the loaded image
>> @@ -1990,9 +1967,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
>>  	/* Patch out unsupported runtime function */
>>  	efi_runtime_detach();
>>
>> -	/* Fix up caches for EFI payloads if necessary */
>> -	efi_exit_caches();
>> -
>>  	/* This stops all lingering devices */
>>  	bootm_disable_interrupts();
>>
>> @@ -2893,8 +2867,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
>>
>
Heinrich Schuchardt July 20, 2019, 4:31 a.m. UTC | #4
On 7/19/19 9:06 PM, Peter Robinson wrote:
> On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
>> boards if U-Boot did not disable caches, cf.
>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
>>
>> In ExitBootServices() we were disabling the caches by calling
>> cleanup_before_linux(). This workaround is not needed anymore.
>
> Do we want to remove this straight away? A lot of distributions will
> take time to move to grub 2.04 because it's been a long time between
> grub releases so they'll have quite a patch delta to re-align to the
> new release. Fedora for example will rebase to grub 2.04 in Fedora 32
> which will start development end of August but won't be released until
> next year.

As described below this code does not remove any functionality that was
active in U-Boot v2019.04 or v2019.07.

I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
stopping GRUB 2.04 from being made available for stable Fedora releases.

Best regards

Heinrich

>
>> The UEFI spec requires that caches are enabled but architecturally defined
>> caches should be disabled. But this requirement has to be fulfilled when
>> invoking StartImage() and not after calling ExitBootServices(). So there is
>> no reason for calling cleanup_before_linux() here.
>>
>> Since commit f69d63fae281 ("efi_loader: use efi_start_image() for
>> bootefi"), i.e. v2019.04, we have not been calling efi_exit_caches()
>> anymore which in turn would call cleanup_before_linux().
>>
>> Remove the obsolete function efi_exit_caches().
>>
>> Suggested-by: Alexander Graf <agraf@csgraf.de>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>  lib/efi_loader/efi_boottime.c | 28 ----------------------------
>>  1 file changed, 28 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>> index 4f6e8d1679..f84ee24416 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
>> @@ -1906,21 +1898,6 @@ error:
>>         return EFI_EXIT(ret);
>>  }
>>
>> -/**
>> - * efi_exit_caches() - fix up caches for EFI payloads if necessary
>> - */
>> -static void efi_exit_caches(void)
>> -{
>> -#if defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
>> -       /*
>> -        * 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.
>> -        */
>> -       if (efi_is_direct_boot)
>> -               cleanup_before_linux();
>> -#endif
>> -}
>> -
>>  /**
>>   * efi_exit_boot_services() - stop all boot services
>>   * @image_handle: handle of the loaded image
>> @@ -1990,9 +1967,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
>>         /* Patch out unsupported runtime function */
>>         efi_runtime_detach();
>>
>> -       /* Fix up caches for EFI payloads if necessary */
>> -       efi_exit_caches();
>> -
>>         /* This stops all lingering devices */
>>         bootm_disable_interrupts();
>>
>> @@ -2893,8 +2867,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
>>
>
Peter Robinson July 20, 2019, 10:22 a.m. UTC | #5
> On 7/19/19 9:06 PM, Peter Robinson wrote:
> > On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>
> >> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
> >> boards if U-Boot did not disable caches, cf.
> >> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
> >>
> >> In ExitBootServices() we were disabling the caches by calling
> >> cleanup_before_linux(). This workaround is not needed anymore.
> >
> > Do we want to remove this straight away? A lot of distributions will
> > take time to move to grub 2.04 because it's been a long time between
> > grub releases so they'll have quite a patch delta to re-align to the
> > new release. Fedora for example will rebase to grub 2.04 in Fedora 32
> > which will start development end of August but won't be released until
> > next year.
>
> As described below this code does not remove any functionality that was
> active in U-Boot v2019.04 or v2019.07.
>
> I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
> stopping GRUB 2.04 from being made available for stable Fedora releases.

The maintainers believe that it's too intrusive to land now and they
want maximum testing time before it gets to stable users, funnily
enough people don't like it when their machines cease to boot.

> >> The UEFI spec requires that caches are enabled but architecturally defined
> >> caches should be disabled. But this requirement has to be fulfilled when
> >> invoking StartImage() and not after calling ExitBootServices(). So there is
> >> no reason for calling cleanup_before_linux() here.
> >>
> >> Since commit f69d63fae281 ("efi_loader: use efi_start_image() for
> >> bootefi"), i.e. v2019.04, we have not been calling efi_exit_caches()
> >> anymore which in turn would call cleanup_before_linux().
> >>
> >> Remove the obsolete function efi_exit_caches().
> >>
> >> Suggested-by: Alexander Graf <agraf@csgraf.de>
> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> >> ---
> >>  lib/efi_loader/efi_boottime.c | 28 ----------------------------
> >>  1 file changed, 28 deletions(-)
> >>
> >> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> >> index 4f6e8d1679..f84ee24416 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
> >> @@ -1906,21 +1898,6 @@ error:
> >>         return EFI_EXIT(ret);
> >>  }
> >>
> >> -/**
> >> - * efi_exit_caches() - fix up caches for EFI payloads if necessary
> >> - */
> >> -static void efi_exit_caches(void)
> >> -{
> >> -#if defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
> >> -       /*
> >> -        * 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.
> >> -        */
> >> -       if (efi_is_direct_boot)
> >> -               cleanup_before_linux();
> >> -#endif
> >> -}
> >> -
> >>  /**
> >>   * efi_exit_boot_services() - stop all boot services
> >>   * @image_handle: handle of the loaded image
> >> @@ -1990,9 +1967,6 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
> >>         /* Patch out unsupported runtime function */
> >>         efi_runtime_detach();
> >>
> >> -       /* Fix up caches for EFI payloads if necessary */
> >> -       efi_exit_caches();
> >> -
> >>         /* This stops all lingering devices */
> >>         bootm_disable_interrupts();
> >>
> >> @@ -2893,8 +2867,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
> >>
> >
>
Heinrich Schuchardt July 20, 2019, 11:23 a.m. UTC | #6
On 7/20/19 1:59 AM, Jonathan Gray wrote:
> On Fri, Jul 19, 2019 at 08:25:45PM +0200, Heinrich Schuchardt wrote:
>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
>> boards if U-Boot did not disable caches, cf.
>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
>>
>> In ExitBootServices() we were disabling the caches by calling
>> cleanup_before_linux(). This workaround is not needed anymore.
>
> It is required for at least OpenBSD, FreeBSD and NetBSD.
> Which are all now forced to carry patches to be able to use U-Boot.

Please, compare function run_loadfile() in files
sys/arch/arm64/stand/efiboot/exec.c and
sys/arch/armv7/stand/efiboot/exec.c of openbsd/src.

On arm64 after loading the kernel you flush the data and instruction
caches. On armv7 you don't. As the UEFI spec requires caches to be
switched on this is bound to fail. You should be good if you flush
caches like you do on arm64.

You can find the relevant code for GRUB in
grub-core/kern/arm/cache.S and
grub-core/kern/arm/cache.c

After GRUB loads the Linux kernel it calls
grub_arch_sync_caches ((void *) linux_addr, linux_size);
in function linux_boot().

FreeBSD simply calls LoadImage() and StartImage() when booting and
leaves the cleanup of caches to the firmware (i.e. EDK2 or U-Boot).

https://github.com/freebsd/freebsd/blob/master/stand/efi/boot1/boot1.c#L133

Best regards

Heinrich
Heinrich Schuchardt July 20, 2019, 4:19 p.m. UTC | #7
On 7/20/19 12:22 PM, Peter Robinson wrote:
>> On 7/19/19 9:06 PM, Peter Robinson wrote:
>>> On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>
>>>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
>>>> boards if U-Boot did not disable caches, cf.
>>>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
>>>>
>>>> In ExitBootServices() we were disabling the caches by calling
>>>> cleanup_before_linux(). This workaround is not needed anymore.
>>>
>>> Do we want to remove this straight away? A lot of distributions will
>>> take time to move to grub 2.04 because it's been a long time between
>>> grub releases so they'll have quite a patch delta to re-align to the
>>> new release. Fedora for example will rebase to grub 2.04 in Fedora 32
>>> which will start development end of August but won't be released until
>>> next year.
>>
>> As described below this code does not remove any functionality that was
>> active in U-Boot v2019.04 or v2019.07.
>>
>> I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
>> stopping GRUB 2.04 from being made available for stable Fedora releases.
>
> The maintainers believe that it's too intrusive to land now and they
> want maximum testing time before it gets to stable users, funnily
> enough people don't like it when their machines cease to boot.

Why should anybody's machines cease to boot?

If Fedora does not role out a new U-Boot they are fine. If Fedora roles
out a new U-Boot they should role out a matching GRUB and they are fine too.

The venturous who build their own U-Boot should know how to role back
their system if needed.

Best regards

Heinrich
Peter Robinson July 21, 2019, 9:46 a.m. UTC | #8
> >>> On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>>>
> >>>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
> >>>> boards if U-Boot did not disable caches, cf.
> >>>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
> >>>>
> >>>> In ExitBootServices() we were disabling the caches by calling
> >>>> cleanup_before_linux(). This workaround is not needed anymore.
> >>>
> >>> Do we want to remove this straight away? A lot of distributions will
> >>> take time to move to grub 2.04 because it's been a long time between
> >>> grub releases so they'll have quite a patch delta to re-align to the
> >>> new release. Fedora for example will rebase to grub 2.04 in Fedora 32
> >>> which will start development end of August but won't be released until
> >>> next year.
> >>
> >> As described below this code does not remove any functionality that was
> >> active in U-Boot v2019.04 or v2019.07.
> >>
> >> I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
> >> stopping GRUB 2.04 from being made available for stable Fedora releases.
> >
> > The maintainers believe that it's too intrusive to land now and they
> > want maximum testing time before it gets to stable users, funnily
> > enough people don't like it when their machines cease to boot.
>
> Why should anybody's machines cease to boot?
>
> If Fedora does not role out a new U-Boot they are fine. If Fedora roles
> out a new U-Boot they should role out a matching GRUB and they are fine too.
>
> The venturous who build their own U-Boot should know how to role back
> their system if needed.

You've clearly never maintained a distribution across 1000s of device
types and 100s of thousands of users.

We will be shipping Fedora 31 with U-Boot 2019.10 and the current
version of grub that the maintainers wish to support, if that requires
me to revert a number of your changes I will, which will be an
inconvenience and probably take more time than I have spare but I will
survive. I find it strange you fix one OS only to break another. How
will this work for users that want to boot a newly released device
which has recently added U-Boot support to an already released stable
OS?

If you wish to actively break currently working use cases that's your
prerogative that is your choice but I find breaking currently working
use cases without a reasonable window to migrate dependencies actively
hostile which has tended to not be the way U-Boot has worked in the
past for such things as DM, so breaking a interface to the way OSes
boot IMO is even worse.

Peter
Heinrich Schuchardt July 21, 2019, 7:42 p.m. UTC | #9
On 7/21/19 11:46 AM, Peter Robinson wrote:
>>>>> On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>>>
>>>>>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
>>>>>> boards if U-Boot did not disable caches, cf.
>>>>>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
>>>>>>
>>>>>> In ExitBootServices() we were disabling the caches by calling
>>>>>> cleanup_before_linux(). This workaround is not needed anymore.
>>>>>
>>>>> Do we want to remove this straight away? A lot of distributions will
>>>>> take time to move to grub 2.04 because it's been a long time between
>>>>> grub releases so they'll have quite a patch delta to re-align to the
>>>>> new release. Fedora for example will rebase to grub 2.04 in Fedora 32
>>>>> which will start development end of August but won't be released until
>>>>> next year.
>>>>
>>>> As described below this code does not remove any functionality that was
>>>> active in U-Boot v2019.04 or v2019.07.
>>>>
>>>> I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
>>>> stopping GRUB 2.04 from being made available for stable Fedora releases.
>>>
>>> The maintainers believe that it's too intrusive to land now and they
>>> want maximum testing time before it gets to stable users, funnily
>>> enough people don't like it when their machines cease to boot.
>>
>> Why should anybody's machines cease to boot?
>>
>> If Fedora does not role out a new U-Boot they are fine. If Fedora roles
>> out a new U-Boot they should role out a matching GRUB and they are fine too.
>>
>> The venturous who build their own U-Boot should know how to role back
>> their system if needed.
>
> You've clearly never maintained a distribution across 1000s of device
> types and 100s of thousands of users.
>
> We will be shipping Fedora 31 with U-Boot 2019.10 and the current
> version of grub that the maintainers wish to support, if that requires

You do not give any clue which version of GRUB will be shipped with
Fedora 31. Could you, please, clarify this. A link to the Fedora repo
would be helpful. Thanks.

Best regards

Heinrich

> me to revert a number of your changes I will, which will be an
> inconvenience and probably take more time than I have spare but I will
> survive. I find it strange you fix one OS only to break another. How
> will this work for users that want to boot a newly released device
> which has recently added U-Boot support to an already released stable
> OS?
>
> If you wish to actively break currently working use cases that's your
> prerogative that is your choice but I find breaking currently working
> use cases without a reasonable window to migrate dependencies actively
> hostile which has tended to not be the way U-Boot has worked in the
> past for such things as DM, so breaking a interface to the way OSes
> boot IMO is even worse.
>
> Peter
>
Tom Rini July 22, 2019, 1:36 p.m. UTC | #10
On Sun, Jul 21, 2019 at 10:46:24AM +0100, Peter Robinson wrote:
> > >>> On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > >>>>
> > >>>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
> > >>>> boards if U-Boot did not disable caches, cf.
> > >>>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
> > >>>>
> > >>>> In ExitBootServices() we were disabling the caches by calling
> > >>>> cleanup_before_linux(). This workaround is not needed anymore.
> > >>>
> > >>> Do we want to remove this straight away? A lot of distributions will
> > >>> take time to move to grub 2.04 because it's been a long time between
> > >>> grub releases so they'll have quite a patch delta to re-align to the
> > >>> new release. Fedora for example will rebase to grub 2.04 in Fedora 32
> > >>> which will start development end of August but won't be released until
> > >>> next year.
> > >>
> > >> As described below this code does not remove any functionality that was
> > >> active in U-Boot v2019.04 or v2019.07.
> > >>
> > >> I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
> > >> stopping GRUB 2.04 from being made available for stable Fedora releases.
> > >
> > > The maintainers believe that it's too intrusive to land now and they
> > > want maximum testing time before it gets to stable users, funnily
> > > enough people don't like it when their machines cease to boot.
> >
> > Why should anybody's machines cease to boot?
> >
> > If Fedora does not role out a new U-Boot they are fine. If Fedora roles
> > out a new U-Boot they should role out a matching GRUB and they are fine too.
> >
> > The venturous who build their own U-Boot should know how to role back
> > their system if needed.
> 
> You've clearly never maintained a distribution across 1000s of device
> types and 100s of thousands of users.
> 
> We will be shipping Fedora 31 with U-Boot 2019.10 and the current
> version of grub that the maintainers wish to support, if that requires
> me to revert a number of your changes I will, which will be an
> inconvenience and probably take more time than I have spare but I will
> survive. I find it strange you fix one OS only to break another. How
> will this work for users that want to boot a newly released device
> which has recently added U-Boot support to an already released stable
> OS?
> 
> If you wish to actively break currently working use cases that's your
> prerogative that is your choice but I find breaking currently working
> use cases without a reasonable window to migrate dependencies actively
> hostile which has tended to not be the way U-Boot has worked in the
> past for such things as DM, so breaking a interface to the way OSes
> boot IMO is even worse.

OK, we have a problem here.  A better example than DM would be the
various work-arounds we have (or carried for ages) to allow using newer
U-Boot with various old and broken kernels.  So no, we need to keep this
work-around for a long while.  What's the EOL date for any Linux
distribution that uses this broken grub?  The first U-Boot release post
that EOL date is when we can drop this particular bit of work-around
code.
Peter Robinson July 22, 2019, 3:10 p.m. UTC | #11
On Sun, Jul 21, 2019 at 8:42 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 7/21/19 11:46 AM, Peter Robinson wrote:
> >>>>> On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>>>>>
> >>>>>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
> >>>>>> boards if U-Boot did not disable caches, cf.
> >>>>>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
> >>>>>>
> >>>>>> In ExitBootServices() we were disabling the caches by calling
> >>>>>> cleanup_before_linux(). This workaround is not needed anymore.
> >>>>>
> >>>>> Do we want to remove this straight away? A lot of distributions will
> >>>>> take time to move to grub 2.04 because it's been a long time between
> >>>>> grub releases so they'll have quite a patch delta to re-align to the
> >>>>> new release. Fedora for example will rebase to grub 2.04 in Fedora 32
> >>>>> which will start development end of August but won't be released until
> >>>>> next year.
> >>>>
> >>>> As described below this code does not remove any functionality that was
> >>>> active in U-Boot v2019.04 or v2019.07.
> >>>>
> >>>> I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
> >>>> stopping GRUB 2.04 from being made available for stable Fedora releases.
> >>>
> >>> The maintainers believe that it's too intrusive to land now and they
> >>> want maximum testing time before it gets to stable users, funnily
> >>> enough people don't like it when their machines cease to boot.
> >>
> >> Why should anybody's machines cease to boot?
> >>
> >> If Fedora does not role out a new U-Boot they are fine. If Fedora roles
> >> out a new U-Boot they should role out a matching GRUB and they are fine too.
> >>
> >> The venturous who build their own U-Boot should know how to role back
> >> their system if needed.
> >
> > You've clearly never maintained a distribution across 1000s of device
> > types and 100s of thousands of users.
> >
> > We will be shipping Fedora 31 with U-Boot 2019.10 and the current
> > version of grub that the maintainers wish to support, if that requires
>
> You do not give any clue which version of GRUB will be shipped with
> Fedora 31. Could you, please, clarify this. A link to the Fedora repo
> would be helpful. Thanks.

It's 2.02 and a whole of lot of patches, like the rest of the world
seems to ship. The git repo is here:
https://github.com/rhboot/grub2/tree/fedora-31
Peter Robinson July 22, 2019, 3:15 p.m. UTC | #12
On Mon, Jul 22, 2019 at 2:36 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Sun, Jul 21, 2019 at 10:46:24AM +0100, Peter Robinson wrote:
> > > >>> On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > >>>>
> > > >>>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
> > > >>>> boards if U-Boot did not disable caches, cf.
> > > >>>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
> > > >>>>
> > > >>>> In ExitBootServices() we were disabling the caches by calling
> > > >>>> cleanup_before_linux(). This workaround is not needed anymore.
> > > >>>
> > > >>> Do we want to remove this straight away? A lot of distributions will
> > > >>> take time to move to grub 2.04 because it's been a long time between
> > > >>> grub releases so they'll have quite a patch delta to re-align to the
> > > >>> new release. Fedora for example will rebase to grub 2.04 in Fedora 32
> > > >>> which will start development end of August but won't be released until
> > > >>> next year.
> > > >>
> > > >> As described below this code does not remove any functionality that was
> > > >> active in U-Boot v2019.04 or v2019.07.
> > > >>
> > > >> I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
> > > >> stopping GRUB 2.04 from being made available for stable Fedora releases.
> > > >
> > > > The maintainers believe that it's too intrusive to land now and they
> > > > want maximum testing time before it gets to stable users, funnily
> > > > enough people don't like it when their machines cease to boot.
> > >
> > > Why should anybody's machines cease to boot?
> > >
> > > If Fedora does not role out a new U-Boot they are fine. If Fedora roles
> > > out a new U-Boot they should role out a matching GRUB and they are fine too.
> > >
> > > The venturous who build their own U-Boot should know how to role back
> > > their system if needed.
> >
> > You've clearly never maintained a distribution across 1000s of device
> > types and 100s of thousands of users.
> >
> > We will be shipping Fedora 31 with U-Boot 2019.10 and the current
> > version of grub that the maintainers wish to support, if that requires
> > me to revert a number of your changes I will, which will be an
> > inconvenience and probably take more time than I have spare but I will
> > survive. I find it strange you fix one OS only to break another. How
> > will this work for users that want to boot a newly released device
> > which has recently added U-Boot support to an already released stable
> > OS?
> >
> > If you wish to actively break currently working use cases that's your
> > prerogative that is your choice but I find breaking currently working
> > use cases without a reasonable window to migrate dependencies actively
> > hostile which has tended to not be the way U-Boot has worked in the
> > past for such things as DM, so breaking a interface to the way OSes
> > boot IMO is even worse.
>
> OK, we have a problem here.  A better example than DM would be the
> various work-arounds we have (or carried for ages) to allow using newer
> U-Boot with various old and broken kernels.  So no, we need to keep this
> work-around for a long while.  What's the EOL date for any Linux
> distribution that uses this broken grub?  The first U-Boot release post
> that EOL date is when we can drop this particular bit of work-around
> code.

Well Fedora 31 would be EOL around Nov/Dec 2020, and while I know the
CentOS team uses the Fedora U-Boot but I believe they boot their
arm-32 instances with extlinux to date. I can't speak for any of the
other distros in this regard.

Peter
Heinrich Schuchardt July 22, 2019, 6:51 p.m. UTC | #13
On 7/22/19 3:36 PM, Tom Rini wrote:
> On Sun, Jul 21, 2019 at 10:46:24AM +0100, Peter Robinson wrote:
>>>>>> On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>>>>
>>>>>>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
>>>>>>> boards if U-Boot did not disable caches, cf.
>>>>>>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
>>>>>>>
>>>>>>> In ExitBootServices() we were disabling the caches by calling
>>>>>>> cleanup_before_linux(). This workaround is not needed anymore.
>>>>>>
>>>>>> Do we want to remove this straight away? A lot of distributions will
>>>>>> take time to move to grub 2.04 because it's been a long time between
>>>>>> grub releases so they'll have quite a patch delta to re-align to the
>>>>>> new release. Fedora for example will rebase to grub 2.04 in Fedora 32
>>>>>> which will start development end of August but won't be released until
>>>>>> next year.
>>>>>
>>>>> As described below this code does not remove any functionality that was
>>>>> active in U-Boot v2019.04 or v2019.07.
>>>>>
>>>>> I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
>>>>> stopping GRUB 2.04 from being made available for stable Fedora releases.
>>>>
>>>> The maintainers believe that it's too intrusive to land now and they
>>>> want maximum testing time before it gets to stable users, funnily
>>>> enough people don't like it when their machines cease to boot.
>>>
>>> Why should anybody's machines cease to boot?
>>>
>>> If Fedora does not role out a new U-Boot they are fine. If Fedora roles
>>> out a new U-Boot they should role out a matching GRUB and they are fine too.
>>>
>>> The venturous who build their own U-Boot should know how to role back
>>> their system if needed.
>>
>> You've clearly never maintained a distribution across 1000s of device
>> types and 100s of thousands of users.
>>
>> We will be shipping Fedora 31 with U-Boot 2019.10 and the current
>> version of grub that the maintainers wish to support, if that requires
>> me to revert a number of your changes I will, which will be an
>> inconvenience and probably take more time than I have spare but I will
>> survive. I find it strange you fix one OS only to break another. How
>> will this work for users that want to boot a newly released device
>> which has recently added U-Boot support to an already released stable
>> OS?
>>
>> If you wish to actively break currently working use cases that's your
>> prerogative that is your choice but I find breaking currently working
>> use cases without a reasonable window to migrate dependencies actively
>> hostile which has tended to not be the way U-Boot has worked in the
>> past for such things as DM, so breaking a interface to the way OSes
>> boot IMO is even worse.
>
> OK, we have a problem here.  A better example than DM would be the
> various work-arounds we have (or carried for ages) to allow using newer
> U-Boot with various old and broken kernels.  So no, we need to keep this
> work-around for a long while.  What's the EOL date for any Linux
> distribution that uses this broken grub?  The first U-Boot release post
> that EOL date is when we can drop this particular bit of work-around
> code.
>

The current behavior contradicts the UEFI spec. Our target is to
implement a UEFI compliant firmware.

If OpenBSD does not change their broken boot loader will we never
correct U-Boot? That would not make sense to me.

Old distros tend not to to update their U-Boot release. E.g. Debian
Stretch is still on U-Boot v2016.11. So why would you care about its EOL?

I could agree if you suggested that we should re-enable the workaround
until the major distros use a compatible GRUB in their stable release
like Debian Buster does.

Let's be proactive and analyze if anything is missing in Fedora Rawhide
GRUB. If yes, we should contact the maintainer and clarify which
adjustments are possible until the Fedora 31 release.

Best regards

Heinrich
Mark Kettenis July 22, 2019, 6:59 p.m. UTC | #14
> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Date: Sat, 20 Jul 2019 13:23:13 +0200
> 
> On 7/20/19 1:59 AM, Jonathan Gray wrote:
> > On Fri, Jul 19, 2019 at 08:25:45PM +0200, Heinrich Schuchardt wrote:
> >> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
> >> boards if U-Boot did not disable caches, cf.
> >> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
> >>
> >> In ExitBootServices() we were disabling the caches by calling
> >> cleanup_before_linux(). This workaround is not needed anymore.
> >
> > It is required for at least OpenBSD, FreeBSD and NetBSD.
> > Which are all now forced to carry patches to be able to use U-Boot.
> 
> Please, compare function run_loadfile() in files
> sys/arch/arm64/stand/efiboot/exec.c and
> sys/arch/armv7/stand/efiboot/exec.c of openbsd/src.
> 
> On arm64 after loading the kernel you flush the data and instruction
> caches. On armv7 you don't. As the UEFI spec requires caches to be
> switched on this is bound to fail. You should be good if you flush
> caches like you do on arm64.
> 
> You can find the relevant code for GRUB in
> grub-core/kern/arm/cache.S and
> grub-core/kern/arm/cache.c
> 
> After GRUB loads the Linux kernel it calls
> grub_arch_sync_caches ((void *) linux_addr, linux_size);
> in function linux_boot().

Thanks for the pointer.  I added code to OpenBSD's BOOTARM.EFI to
disable the caches and MMU such that the kernel ends up being started
in the same state as before.

That still leaves us with the L2 cache issue though.  A U-Boot with
CONFIG_SYS_L2CACHE_OFF set works, whereas without it doesn't.
Tom Rini July 22, 2019, 7:02 p.m. UTC | #15
On Mon, Jul 22, 2019 at 08:51:41PM +0200, Heinrich Schuchardt wrote:
> On 7/22/19 3:36 PM, Tom Rini wrote:
> >On Sun, Jul 21, 2019 at 10:46:24AM +0100, Peter Robinson wrote:
> >>>>>>On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>>>>>>
> >>>>>>>In GRUB before 2.04 a bug existed which did not allow booting some ARM32
> >>>>>>>boards if U-Boot did not disable caches, cf.
> >>>>>>>https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
> >>>>>>>
> >>>>>>>In ExitBootServices() we were disabling the caches by calling
> >>>>>>>cleanup_before_linux(). This workaround is not needed anymore.
> >>>>>>
> >>>>>>Do we want to remove this straight away? A lot of distributions will
> >>>>>>take time to move to grub 2.04 because it's been a long time between
> >>>>>>grub releases so they'll have quite a patch delta to re-align to the
> >>>>>>new release. Fedora for example will rebase to grub 2.04 in Fedora 32
> >>>>>>which will start development end of August but won't be released until
> >>>>>>next year.
> >>>>>
> >>>>>As described below this code does not remove any functionality that was
> >>>>>active in U-Boot v2019.04 or v2019.07.
> >>>>>
> >>>>>I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
> >>>>>stopping GRUB 2.04 from being made available for stable Fedora releases.
> >>>>
> >>>>The maintainers believe that it's too intrusive to land now and they
> >>>>want maximum testing time before it gets to stable users, funnily
> >>>>enough people don't like it when their machines cease to boot.
> >>>
> >>>Why should anybody's machines cease to boot?
> >>>
> >>>If Fedora does not role out a new U-Boot they are fine. If Fedora roles
> >>>out a new U-Boot they should role out a matching GRUB and they are fine too.
> >>>
> >>>The venturous who build their own U-Boot should know how to role back
> >>>their system if needed.
> >>
> >>You've clearly never maintained a distribution across 1000s of device
> >>types and 100s of thousands of users.
> >>
> >>We will be shipping Fedora 31 with U-Boot 2019.10 and the current
> >>version of grub that the maintainers wish to support, if that requires
> >>me to revert a number of your changes I will, which will be an
> >>inconvenience and probably take more time than I have spare but I will
> >>survive. I find it strange you fix one OS only to break another. How
> >>will this work for users that want to boot a newly released device
> >>which has recently added U-Boot support to an already released stable
> >>OS?
> >>
> >>If you wish to actively break currently working use cases that's your
> >>prerogative that is your choice but I find breaking currently working
> >>use cases without a reasonable window to migrate dependencies actively
> >>hostile which has tended to not be the way U-Boot has worked in the
> >>past for such things as DM, so breaking a interface to the way OSes
> >>boot IMO is even worse.
> >
> >OK, we have a problem here.  A better example than DM would be the
> >various work-arounds we have (or carried for ages) to allow using newer
> >U-Boot with various old and broken kernels.  So no, we need to keep this
> >work-around for a long while.  What's the EOL date for any Linux
> >distribution that uses this broken grub?  The first U-Boot release post
> >that EOL date is when we can drop this particular bit of work-around
> >code.
> 
> The current behavior contradicts the UEFI spec. Our target is to
> implement a UEFI compliant firmware.

Our target is to be both compliant and functional, and functional wins.

> If OpenBSD does not change their broken boot loader will we never
> correct U-Boot? That would not make sense to me.

I'm sorry, I don't see the connection.  It's not "GRUB won't change to
be compliant" it's "GRUB changed things but it's going to take a long
while for that to be deployed out".

> Old distros tend not to to update their U-Boot release. E.g. Debian
> Stretch is still on U-Boot v2016.11. So why would you care about its EOL?

Because I want to make it really hard for people to end up in a "does
not boot now" mode.

> I could agree if you suggested that we should re-enable the workaround
> until the major distros use a compatible GRUB in their stable release
> like Debian Buster does.
> 
> Let's be proactive and analyze if anything is missing in Fedora Rawhide
> GRUB. If yes, we should contact the maintainer and clarify which
> adjustments are possible until the Fedora 31 release.

I don't want it to be "just latest stable has it".  I want it to be
"supported releases have it".

Why?  It's the user-friendly behavior.  You will find people that
upgrade their U-Boot because they're trying something but still have an
older distro.  Or people that are bringing up a new board and testing it
out with an older distro.
Heinrich Schuchardt July 22, 2019, 7:07 p.m. UTC | #16
On 7/22/19 9:02 PM, Tom Rini wrote:
> On Mon, Jul 22, 2019 at 08:51:41PM +0200, Heinrich Schuchardt wrote:
>> On 7/22/19 3:36 PM, Tom Rini wrote:
>>> On Sun, Jul 21, 2019 at 10:46:24AM +0100, Peter Robinson wrote:
>>>>>>>> On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>>>>>>
>>>>>>>>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
>>>>>>>>> boards if U-Boot did not disable caches, cf.
>>>>>>>>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
>>>>>>>>>
>>>>>>>>> In ExitBootServices() we were disabling the caches by calling
>>>>>>>>> cleanup_before_linux(). This workaround is not needed anymore.
>>>>>>>>
>>>>>>>> Do we want to remove this straight away? A lot of distributions will
>>>>>>>> take time to move to grub 2.04 because it's been a long time between
>>>>>>>> grub releases so they'll have quite a patch delta to re-align to the
>>>>>>>> new release. Fedora for example will rebase to grub 2.04 in Fedora 32
>>>>>>>> which will start development end of August but won't be released until
>>>>>>>> next year.
>>>>>>>
>>>>>>> As described below this code does not remove any functionality that was
>>>>>>> active in U-Boot v2019.04 or v2019.07.
>>>>>>>
>>>>>>> I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
>>>>>>> stopping GRUB 2.04 from being made available for stable Fedora releases.
>>>>>>
>>>>>> The maintainers believe that it's too intrusive to land now and they
>>>>>> want maximum testing time before it gets to stable users, funnily
>>>>>> enough people don't like it when their machines cease to boot.
>>>>>
>>>>> Why should anybody's machines cease to boot?
>>>>>
>>>>> If Fedora does not role out a new U-Boot they are fine. If Fedora roles
>>>>> out a new U-Boot they should role out a matching GRUB and they are fine too.
>>>>>
>>>>> The venturous who build their own U-Boot should know how to role back
>>>>> their system if needed.
>>>>
>>>> You've clearly never maintained a distribution across 1000s of device
>>>> types and 100s of thousands of users.
>>>>
>>>> We will be shipping Fedora 31 with U-Boot 2019.10 and the current
>>>> version of grub that the maintainers wish to support, if that requires
>>>> me to revert a number of your changes I will, which will be an
>>>> inconvenience and probably take more time than I have spare but I will
>>>> survive. I find it strange you fix one OS only to break another. How
>>>> will this work for users that want to boot a newly released device
>>>> which has recently added U-Boot support to an already released stable
>>>> OS?
>>>>
>>>> If you wish to actively break currently working use cases that's your
>>>> prerogative that is your choice but I find breaking currently working
>>>> use cases without a reasonable window to migrate dependencies actively
>>>> hostile which has tended to not be the way U-Boot has worked in the
>>>> past for such things as DM, so breaking a interface to the way OSes
>>>> boot IMO is even worse.
>>>
>>> OK, we have a problem here.  A better example than DM would be the
>>> various work-arounds we have (or carried for ages) to allow using newer
>>> U-Boot with various old and broken kernels.  So no, we need to keep this
>>> work-around for a long while.  What's the EOL date for any Linux
>>> distribution that uses this broken grub?  The first U-Boot release post
>>> that EOL date is when we can drop this particular bit of work-around
>>> code.
>>
>> The current behavior contradicts the UEFI spec. Our target is to
>> implement a UEFI compliant firmware.
>
> Our target is to be both compliant and functional, and functional wins.
>
>> If OpenBSD does not change their broken boot loader will we never
>> correct U-Boot? That would not make sense to me.
>
> I'm sorry, I don't see the connection.  It's not "GRUB won't change to
> be compliant" it's "GRUB changed things but it's going to take a long
> while for that to be deployed out".
>
>> Old distros tend not to to update their U-Boot release. E.g. Debian
>> Stretch is still on U-Boot v2016.11. So why would you care about its EOL?
>
> Because I want to make it really hard for people to end up in a "does
> not boot now" mode.
>
>> I could agree if you suggested that we should re-enable the workaround
>> until the major distros use a compatible GRUB in their stable release
>> like Debian Buster does.
>>
>> Let's be proactive and analyze if anything is missing in Fedora Rawhide
>> GRUB. If yes, we should contact the maintainer and clarify which
>> adjustments are possible until the Fedora 31 release.
>
> I don't want it to be "just latest stable has it".  I want it to be
> "supported releases have it".

RHEL distros are supported for 10 years. Are you serious?

Best regards

Heinrich

>
> Why?  It's the user-friendly behavior.  You will find people that
> upgrade their U-Boot because they're trying something but still have an
> older distro.  Or people that are bringing up a new board and testing it
> out with an older distro.
>
Tom Rini July 22, 2019, 7:16 p.m. UTC | #17
On Mon, Jul 22, 2019 at 09:07:40PM +0200, Heinrich Schuchardt wrote:
> On 7/22/19 9:02 PM, Tom Rini wrote:
> >On Mon, Jul 22, 2019 at 08:51:41PM +0200, Heinrich Schuchardt wrote:
> >>On 7/22/19 3:36 PM, Tom Rini wrote:
> >>>On Sun, Jul 21, 2019 at 10:46:24AM +0100, Peter Robinson wrote:
> >>>>>>>>On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >>>>>>>>>
> >>>>>>>>>In GRUB before 2.04 a bug existed which did not allow booting some ARM32
> >>>>>>>>>boards if U-Boot did not disable caches, cf.
> >>>>>>>>>https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
> >>>>>>>>>
> >>>>>>>>>In ExitBootServices() we were disabling the caches by calling
> >>>>>>>>>cleanup_before_linux(). This workaround is not needed anymore.
> >>>>>>>>
> >>>>>>>>Do we want to remove this straight away? A lot of distributions will
> >>>>>>>>take time to move to grub 2.04 because it's been a long time between
> >>>>>>>>grub releases so they'll have quite a patch delta to re-align to the
> >>>>>>>>new release. Fedora for example will rebase to grub 2.04 in Fedora 32
> >>>>>>>>which will start development end of August but won't be released until
> >>>>>>>>next year.
> >>>>>>>
> >>>>>>>As described below this code does not remove any functionality that was
> >>>>>>>active in U-Boot v2019.04 or v2019.07.
> >>>>>>>
> >>>>>>>I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
> >>>>>>>stopping GRUB 2.04 from being made available for stable Fedora releases.
> >>>>>>
> >>>>>>The maintainers believe that it's too intrusive to land now and they
> >>>>>>want maximum testing time before it gets to stable users, funnily
> >>>>>>enough people don't like it when their machines cease to boot.
> >>>>>
> >>>>>Why should anybody's machines cease to boot?
> >>>>>
> >>>>>If Fedora does not role out a new U-Boot they are fine. If Fedora roles
> >>>>>out a new U-Boot they should role out a matching GRUB and they are fine too.
> >>>>>
> >>>>>The venturous who build their own U-Boot should know how to role back
> >>>>>their system if needed.
> >>>>
> >>>>You've clearly never maintained a distribution across 1000s of device
> >>>>types and 100s of thousands of users.
> >>>>
> >>>>We will be shipping Fedora 31 with U-Boot 2019.10 and the current
> >>>>version of grub that the maintainers wish to support, if that requires
> >>>>me to revert a number of your changes I will, which will be an
> >>>>inconvenience and probably take more time than I have spare but I will
> >>>>survive. I find it strange you fix one OS only to break another. How
> >>>>will this work for users that want to boot a newly released device
> >>>>which has recently added U-Boot support to an already released stable
> >>>>OS?
> >>>>
> >>>>If you wish to actively break currently working use cases that's your
> >>>>prerogative that is your choice but I find breaking currently working
> >>>>use cases without a reasonable window to migrate dependencies actively
> >>>>hostile which has tended to not be the way U-Boot has worked in the
> >>>>past for such things as DM, so breaking a interface to the way OSes
> >>>>boot IMO is even worse.
> >>>
> >>>OK, we have a problem here.  A better example than DM would be the
> >>>various work-arounds we have (or carried for ages) to allow using newer
> >>>U-Boot with various old and broken kernels.  So no, we need to keep this
> >>>work-around for a long while.  What's the EOL date for any Linux
> >>>distribution that uses this broken grub?  The first U-Boot release post
> >>>that EOL date is when we can drop this particular bit of work-around
> >>>code.
> >>
> >>The current behavior contradicts the UEFI spec. Our target is to
> >>implement a UEFI compliant firmware.
> >
> >Our target is to be both compliant and functional, and functional wins.
> >
> >>If OpenBSD does not change their broken boot loader will we never
> >>correct U-Boot? That would not make sense to me.
> >
> >I'm sorry, I don't see the connection.  It's not "GRUB won't change to
> >be compliant" it's "GRUB changed things but it's going to take a long
> >while for that to be deployed out".
> >
> >>Old distros tend not to to update their U-Boot release. E.g. Debian
> >>Stretch is still on U-Boot v2016.11. So why would you care about its EOL?
> >
> >Because I want to make it really hard for people to end up in a "does
> >not boot now" mode.
> >
> >>I could agree if you suggested that we should re-enable the workaround
> >>until the major distros use a compatible GRUB in their stable release
> >>like Debian Buster does.
> >>
> >>Let's be proactive and analyze if anything is missing in Fedora Rawhide
> >>GRUB. If yes, we should contact the maintainer and clarify which
> >>adjustments are possible until the Fedora 31 release.
> >
> >I don't want it to be "just latest stable has it".  I want it to be
> >"supported releases have it".
> 
> RHEL distros are supported for 10 years. Are you serious?

RHEL doesn't support 32bit ARM, which is the problem code path in
question, so we're good there.  Otherwise, depending on the overhead of
maintaining a work-around, yes.  We kept one around for sunxi until it
was very clear that yes, really, there was no one around using that
particular kernel anymore.  And people do expect to be able to throw
new hardware at old distros and U-Boot falls on the "needs to work with
the old SW" side of the line and GRUB falls on the "it's the distro, use
what's there" side.
Mark Kettenis July 22, 2019, 7:29 p.m. UTC | #18
> Date: Mon, 22 Jul 2019 09:36:38 -0400
> From: Tom Rini <trini@konsulko.com>
> 
> On Sun, Jul 21, 2019 at 10:46:24AM +0100, Peter Robinson wrote:
> > > >>> On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > >>>>
> > > >>>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
> > > >>>> boards if U-Boot did not disable caches, cf.
> > > >>>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
> > > >>>>
> > > >>>> In ExitBootServices() we were disabling the caches by calling
> > > >>>> cleanup_before_linux(). This workaround is not needed anymore.
> > > >>>
> > > >>> Do we want to remove this straight away? A lot of distributions will
> > > >>> take time to move to grub 2.04 because it's been a long time between
> > > >>> grub releases so they'll have quite a patch delta to re-align to the
> > > >>> new release. Fedora for example will rebase to grub 2.04 in Fedora 32
> > > >>> which will start development end of August but won't be released until
> > > >>> next year.
> > > >>
> > > >> As described below this code does not remove any functionality that was
> > > >> active in U-Boot v2019.04 or v2019.07.
> > > >>
> > > >> I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
> > > >> stopping GRUB 2.04 from being made available for stable Fedora releases.
> > > >
> > > > The maintainers believe that it's too intrusive to land now and they
> > > > want maximum testing time before it gets to stable users, funnily
> > > > enough people don't like it when their machines cease to boot.
> > >
> > > Why should anybody's machines cease to boot?
> > >
> > > If Fedora does not role out a new U-Boot they are fine. If Fedora roles
> > > out a new U-Boot they should role out a matching GRUB and they are fine too.
> > >
> > > The venturous who build their own U-Boot should know how to role back
> > > their system if needed.
> > 
> > You've clearly never maintained a distribution across 1000s of device
> > types and 100s of thousands of users.
> > 
> > We will be shipping Fedora 31 with U-Boot 2019.10 and the current
> > version of grub that the maintainers wish to support, if that requires
> > me to revert a number of your changes I will, which will be an
> > inconvenience and probably take more time than I have spare but I will
> > survive. I find it strange you fix one OS only to break another. How
> > will this work for users that want to boot a newly released device
> > which has recently added U-Boot support to an already released stable
> > OS?
> > 
> > If you wish to actively break currently working use cases that's your
> > prerogative that is your choice but I find breaking currently working
> > use cases without a reasonable window to migrate dependencies actively
> > hostile which has tended to not be the way U-Boot has worked in the
> > past for such things as DM, so breaking a interface to the way OSes
> > boot IMO is even worse.
> 
> OK, we have a problem here.  A better example than DM would be the
> various work-arounds we have (or carried for ages) to allow using newer
> U-Boot with various old and broken kernels.  So no, we need to keep this
> work-around for a long while.  What's the EOL date for any Linux
> distribution that uses this broken grub?  The first U-Boot release post
> that EOL date is when we can drop this particular bit of work-around
> code.

Hi Tom,

Things are a bit more complicated.  The breakage was introduced in
2019.04 as part of some efi_loader refacoring.  Further refactoring in
2019.07 makes a simple revert impossible.

I committed code to the OpenBSD/armv7 bootloader today that disables
the architecturally defined caches and MMU.  However, that isn't
enough to fix the breakage on all boards as some board configurations
enable the non-architectural L2 cache.  I believe that even with grub
2.04 bootling a Linux kernel is broken on boards with such an L2 cache
as well.

If the L2 cache issue is addressed, OpenBSD will be fine.  OpenBSD 6.5
shipped with a U-Boot 2019.01 package, and OpenBSD 6.6 will have the
fixed bootloader.

I'm not sure how to fix the L2 cache issue though.  I see two options:

1. Enable CONFIG_SYS_L2CACHE_OFF on all 32-bit ARM boards if the EFI
   loader is enabled.  Some board maintainers could object though
   since this probably makes U-Boot a bit slower.  And it would be bad
   if that would lead them towards disabling the EFI loader.

2. Disable the L2 cache whenever the EFI loader is first entered.  I'm
   not sure if this is possible while keeping the caches enabled.

Cheers,

Mark
Pablo Sebastián Greco July 22, 2019, 9:49 p.m. UTC | #19
El 22/7/19 a las 12:15, Peter Robinson escribió:
> On Mon, Jul 22, 2019 at 2:36 PM Tom Rini <trini@konsulko.com> wrote:
>> On Sun, Jul 21, 2019 at 10:46:24AM +0100, Peter Robinson wrote:
>>>>>>> On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>>>>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
>>>>>>>> boards if U-Boot did not disable caches, cf.
>>>>>>>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
>>>>>>>>
>>>>>>>> In ExitBootServices() we were disabling the caches by calling
>>>>>>>> cleanup_before_linux(). This workaround is not needed anymore.
>>>>>>> Do we want to remove this straight away? A lot of distributions will
>>>>>>> take time to move to grub 2.04 because it's been a long time between
>>>>>>> grub releases so they'll have quite a patch delta to re-align to the
>>>>>>> new release. Fedora for example will rebase to grub 2.04 in Fedora 32
>>>>>>> which will start development end of August but won't be released until
>>>>>>> next year.
>>>>>> As described below this code does not remove any functionality that was
>>>>>> active in U-Boot v2019.04 or v2019.07.
>>>>>>
>>>>>> I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
>>>>>> stopping GRUB 2.04 from being made available for stable Fedora releases.
>>>>> The maintainers believe that it's too intrusive to land now and they
>>>>> want maximum testing time before it gets to stable users, funnily
>>>>> enough people don't like it when their machines cease to boot.
>>>> Why should anybody's machines cease to boot?
>>>>
>>>> If Fedora does not role out a new U-Boot they are fine. If Fedora roles
>>>> out a new U-Boot they should role out a matching GRUB and they are fine too.
>>>>
>>>> The venturous who build their own U-Boot should know how to role back
>>>> their system if needed.
>>> You've clearly never maintained a distribution across 1000s of device
>>> types and 100s of thousands of users.
>>>
>>> We will be shipping Fedora 31 with U-Boot 2019.10 and the current
>>> version of grub that the maintainers wish to support, if that requires
>>> me to revert a number of your changes I will, which will be an
>>> inconvenience and probably take more time than I have spare but I will
>>> survive. I find it strange you fix one OS only to break another. How
>>> will this work for users that want to boot a newly released device
>>> which has recently added U-Boot support to an already released stable
>>> OS?
>>>
>>> If you wish to actively break currently working use cases that's your
>>> prerogative that is your choice but I find breaking currently working
>>> use cases without a reasonable window to migrate dependencies actively
>>> hostile which has tended to not be the way U-Boot has worked in the
>>> past for such things as DM, so breaking a interface to the way OSes
>>> boot IMO is even worse.
>> OK, we have a problem here.  A better example than DM would be the
>> various work-arounds we have (or carried for ages) to allow using newer
>> U-Boot with various old and broken kernels.  So no, we need to keep this
>> work-around for a long while.  What's the EOL date for any Linux
>> distribution that uses this broken grub?  The first U-Boot release post
>> that EOL date is when we can drop this particular bit of work-around
>> code.
> Well Fedora 31 would be EOL around Nov/Dec 2020, and while I know the
> CentOS team uses the Fedora U-Boot but I believe they boot their
> arm-32 instances with extlinux to date. I can't speak for any of the
> other distros in this regard.
Well, that is true for CentOS7, but for CentOS8 the plan is to boot 
using EFI also.
> Peter
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot


Pablo.
Pablo Sebastián Greco July 22, 2019, 9:53 p.m. UTC | #20
El 22/7/19 a las 12:15, Peter Robinson escribió:
> On Mon, Jul 22, 2019 at 2:36 PM Tom Rini <trini@konsulko.com> wrote:
>> On Sun, Jul 21, 2019 at 10:46:24AM +0100, Peter Robinson wrote:
>>>>>>> On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>>>>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
>>>>>>>> boards if U-Boot did not disable caches, cf.
>>>>>>>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
>>>>>>>>
>>>>>>>> In ExitBootServices() we were disabling the caches by calling
>>>>>>>> cleanup_before_linux(). This workaround is not needed anymore.
>>>>>>> Do we want to remove this straight away? A lot of distributions will
>>>>>>> take time to move to grub 2.04 because it's been a long time between
>>>>>>> grub releases so they'll have quite a patch delta to re-align to the
>>>>>>> new release. Fedora for example will rebase to grub 2.04 in Fedora 32
>>>>>>> which will start development end of August but won't be released until
>>>>>>> next year.
>>>>>> As described below this code does not remove any functionality that was
>>>>>> active in U-Boot v2019.04 or v2019.07.
>>>>>>
>>>>>> I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
>>>>>> stopping GRUB 2.04 from being made available for stable Fedora releases.
>>>>> The maintainers believe that it's too intrusive to land now and they
>>>>> want maximum testing time before it gets to stable users, funnily
>>>>> enough people don't like it when their machines cease to boot.
>>>> Why should anybody's machines cease to boot?
>>>>
>>>> If Fedora does not role out a new U-Boot they are fine. If Fedora roles
>>>> out a new U-Boot they should role out a matching GRUB and they are fine too.
>>>>
>>>> The venturous who build their own U-Boot should know how to role back
>>>> their system if needed.
>>> You've clearly never maintained a distribution across 1000s of device
>>> types and 100s of thousands of users.
>>>
>>> We will be shipping Fedora 31 with U-Boot 2019.10 and the current
>>> version of grub that the maintainers wish to support, if that requires
>>> me to revert a number of your changes I will, which will be an
>>> inconvenience and probably take more time than I have spare but I will
>>> survive. I find it strange you fix one OS only to break another. How
>>> will this work for users that want to boot a newly released device
>>> which has recently added U-Boot support to an already released stable
>>> OS?
>>>
>>> If you wish to actively break currently working use cases that's your
>>> prerogative that is your choice but I find breaking currently working
>>> use cases without a reasonable window to migrate dependencies actively
>>> hostile which has tended to not be the way U-Boot has worked in the
>>> past for such things as DM, so breaking a interface to the way OSes
>>> boot IMO is even worse.
>> OK, we have a problem here.  A better example than DM would be the
>> various work-arounds we have (or carried for ages) to allow using newer
>> U-Boot with various old and broken kernels.  So no, we need to keep this
>> work-around for a long while.  What's the EOL date for any Linux
>> distribution that uses this broken grub?  The first U-Boot release post
>> that EOL date is when we can drop this particular bit of work-around
>> code.
> Well Fedora 31 would be EOL around Nov/Dec 2020, and while I know the
> CentOS team uses the Fedora U-Boot but I believe they boot their
> arm-32 instances with extlinux to date. I can't speak for any of the
> other distros in this regard.
Well, that is true for CentOS7, but for CentOS8 the plan is to boot 
using EFI also.
>
> Peter
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> https://lists.denx.de/listinfo/u-boot


Pablo.
Tom Rini July 22, 2019, 10:12 p.m. UTC | #21
On Mon, Jul 22, 2019 at 09:29:21PM +0200, Mark Kettenis wrote:
> > Date: Mon, 22 Jul 2019 09:36:38 -0400
> > From: Tom Rini <trini@konsulko.com>
> > 
> > On Sun, Jul 21, 2019 at 10:46:24AM +0100, Peter Robinson wrote:
> > > > >>> On Fri, Jul 19, 2019 at 7:28 PM Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > > > >>>>
> > > > >>>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
> > > > >>>> boards if U-Boot did not disable caches, cf.
> > > > >>>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
> > > > >>>>
> > > > >>>> In ExitBootServices() we were disabling the caches by calling
> > > > >>>> cleanup_before_linux(). This workaround is not needed anymore.
> > > > >>>
> > > > >>> Do we want to remove this straight away? A lot of distributions will
> > > > >>> take time to move to grub 2.04 because it's been a long time between
> > > > >>> grub releases so they'll have quite a patch delta to re-align to the
> > > > >>> new release. Fedora for example will rebase to grub 2.04 in Fedora 32
> > > > >>> which will start development end of August but won't be released until
> > > > >>> next year.
> > > > >>
> > > > >> As described below this code does not remove any functionality that was
> > > > >> active in U-Boot v2019.04 or v2019.07.
> > > > >>
> > > > >> I can see nothing in https://fedoraproject.org/wiki/Updates_Policy
> > > > >> stopping GRUB 2.04 from being made available for stable Fedora releases.
> > > > >
> > > > > The maintainers believe that it's too intrusive to land now and they
> > > > > want maximum testing time before it gets to stable users, funnily
> > > > > enough people don't like it when their machines cease to boot.
> > > >
> > > > Why should anybody's machines cease to boot?
> > > >
> > > > If Fedora does not role out a new U-Boot they are fine. If Fedora roles
> > > > out a new U-Boot they should role out a matching GRUB and they are fine too.
> > > >
> > > > The venturous who build their own U-Boot should know how to role back
> > > > their system if needed.
> > > 
> > > You've clearly never maintained a distribution across 1000s of device
> > > types and 100s of thousands of users.
> > > 
> > > We will be shipping Fedora 31 with U-Boot 2019.10 and the current
> > > version of grub that the maintainers wish to support, if that requires
> > > me to revert a number of your changes I will, which will be an
> > > inconvenience and probably take more time than I have spare but I will
> > > survive. I find it strange you fix one OS only to break another. How
> > > will this work for users that want to boot a newly released device
> > > which has recently added U-Boot support to an already released stable
> > > OS?
> > > 
> > > If you wish to actively break currently working use cases that's your
> > > prerogative that is your choice but I find breaking currently working
> > > use cases without a reasonable window to migrate dependencies actively
> > > hostile which has tended to not be the way U-Boot has worked in the
> > > past for such things as DM, so breaking a interface to the way OSes
> > > boot IMO is even worse.
> > 
> > OK, we have a problem here.  A better example than DM would be the
> > various work-arounds we have (or carried for ages) to allow using newer
> > U-Boot with various old and broken kernels.  So no, we need to keep this
> > work-around for a long while.  What's the EOL date for any Linux
> > distribution that uses this broken grub?  The first U-Boot release post
> > that EOL date is when we can drop this particular bit of work-around
> > code.
> 
> Hi Tom,
> 
> Things are a bit more complicated.  The breakage was introduced in
> 2019.04 as part of some efi_loader refacoring.  Further refactoring in
> 2019.07 makes a simple revert impossible.
> 
> I committed code to the OpenBSD/armv7 bootloader today that disables
> the architecturally defined caches and MMU.  However, that isn't
> enough to fix the breakage on all boards as some board configurations
> enable the non-architectural L2 cache.  I believe that even with grub
> 2.04 bootling a Linux kernel is broken on boards with such an L2 cache
> as well.
> 
> If the L2 cache issue is addressed, OpenBSD will be fine.  OpenBSD 6.5
> shipped with a U-Boot 2019.01 package, and OpenBSD 6.6 will have the
> fixed bootloader.
> 
> I'm not sure how to fix the L2 cache issue though.  I see two options:
> 
> 1. Enable CONFIG_SYS_L2CACHE_OFF on all 32-bit ARM boards if the EFI
>    loader is enabled.  Some board maintainers could object though
>    since this probably makes U-Boot a bit slower.  And it would be bad
>    if that would lead them towards disabling the EFI loader.
> 
> 2. Disable the L2 cache whenever the EFI loader is first entered.  I'm
>    not sure if this is possible while keeping the caches enabled.

Thanks for explaining the background issues here, I was unaware.  Would
it be possible to introduce a work-around in the new code paths such
that we continue to work around this issue?  But perhaps we need to
confirm what you suspect is still not functional and get a fix for that?
As yes, we do want to be spec compliant, but we also need to function in
the real world, so to speak.  And we also cannot disable L2 cache
globally and lead to EFI loader support being disabled by most boards
out of the box, which is not the intended situation.
Heinrich Schuchardt July 23, 2019, 6:18 a.m. UTC | #22
On 7/22/19 8:59 PM, Mark Kettenis wrote:
>> From: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> Date: Sat, 20 Jul 2019 13:23:13 +0200
>>
>> On 7/20/19 1:59 AM, Jonathan Gray wrote:
>>> On Fri, Jul 19, 2019 at 08:25:45PM +0200, Heinrich Schuchardt wrote:
>>>> In GRUB before 2.04 a bug existed which did not allow booting some ARM32
>>>> boards if U-Boot did not disable caches, cf.
>>>> https://lists.linaro.org/pipermail/cross-distro/2019-July/000933.html
>>>>
>>>> In ExitBootServices() we were disabling the caches by calling
>>>> cleanup_before_linux(). This workaround is not needed anymore.
>>>
>>> It is required for at least OpenBSD, FreeBSD and NetBSD.
>>> Which are all now forced to carry patches to be able to use U-Boot.
>>
>> Please, compare function run_loadfile() in files
>> sys/arch/arm64/stand/efiboot/exec.c and
>> sys/arch/armv7/stand/efiboot/exec.c of openbsd/src.
>>
>> On arm64 after loading the kernel you flush the data and instruction
>> caches. On armv7 you don't. As the UEFI spec requires caches to be
>> switched on this is bound to fail. You should be good if you flush
>> caches like you do on arm64.
>>
>> You can find the relevant code for GRUB in
>> grub-core/kern/arm/cache.S and
>> grub-core/kern/arm/cache.c
>>
>> After GRUB loads the Linux kernel it calls
>> grub_arch_sync_caches ((void *) linux_addr, linux_size);
>> in function linux_boot().
>
> Thanks for the pointer.  I added code to OpenBSD's BOOTARM.EFI to
> disable the caches and MMU such that the kernel ends up being started
> in the same state as before.
>
> That still leaves us with the L2 cache issue though.  A U-Boot with
> CONFIG_SYS_L2CACHE_OFF set works, whereas without it doesn't.
>

I just tested on Wandboard which is an i.MX6 system with a
architecturally defined L2 cache. The operating system is Debian Buster
which has a GRUB 2.02 but where the EFI path has been patched like in
GRUB 2.04.

It will only boot via GRUB if either CONFIG_SYS_L2CACHE_OFF=y or
cleanup_before_linux() is called (in efi_exit_caches()).

As current GRUB calls StartImage() itself the original logic implemented
by Alex not to call cleanup_before_linux() if a payload has called
StartImage() does not work.

@Alex:
Given this finding and the discussion in this thread could you, please,
revisit your review comment in
https://lists.denx.de/pipermail/u-boot/2019-July/377339.html
[RFC 1/1] efi_loader: usage of cleanup_before_linux()

I think short term we should always call cleanup_before_linux() and long
term we should move to a logic switching of only architecturally defined
caches in efi_init_obj_list(), i.e. before loading any EFI binary.

Best regards

Heinrich

Patch
diff mbox series

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 4f6e8d1679..f84ee24416 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
@@ -1906,21 +1898,6 @@  error:
 	return EFI_EXIT(ret);
 }

-/**
- * efi_exit_caches() - fix up caches for EFI payloads if necessary
- */
-static void efi_exit_caches(void)
-{
-#if defined(CONFIG_ARM) && !defined(CONFIG_ARM64)
-	/*
-	 * 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.
-	 */
-	if (efi_is_direct_boot)
-		cleanup_before_linux();
-#endif
-}
-
 /**
  * efi_exit_boot_services() - stop all boot services
  * @image_handle: handle of the loaded image
@@ -1990,9 +1967,6 @@  static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
 	/* Patch out unsupported runtime function */
 	efi_runtime_detach();

-	/* Fix up caches for EFI payloads if necessary */
-	efi_exit_caches();
-
 	/* This stops all lingering devices */
 	bootm_disable_interrupts();

@@ -2893,8 +2867,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;