[U-Boot] [PATCH v2 3/4] efi_loader: use efi_start_image() for bootefi
Jonathan Gray
jsg at jsg.id.au
Thu Jul 18 09:16:44 UTC 2019
On Thu, Jul 18, 2019 at 10:39:57AM +0200, Mark Kettenis wrote:
> > Date: Thu, 18 Jul 2019 16:00:16 +1000
> > From: Jonathan Gray <jsg at jsg.id.au>
> >
> > On Fri, Feb 08, 2019 at 07:46:49PM +0100, Heinrich Schuchardt wrote:
> > > Remove the duplicate code in efi_do_enter() and use efi_start_image() to
> > > start the image invoked by the bootefi command.
> > >
> > > Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > > ---
> > > v2
> > > use EFI_CALL
> >
> > This commit broke booting OpenBSD/armv7 kernels on mx6cuboxi with U-Boot
> > releases after 2019.01. 2019.04 works if this commit is reverted. With
> > 2019.07 there are conflicts trying to revert it and it is still broken
> > as released.
> >
> > f69d63fae281ba98c3d063097cf4e95d17f3754d is the first bad commit
> > commit f69d63fae281ba98c3d063097cf4e95d17f3754d
> > Author: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > Date: Wed Dec 26 13:28:09 2018 +0100
> >
> > efi_loader: use efi_start_image() for bootefi
> >
> > Remove the duplicate code in efi_do_enter() and use efi_start_image() to
> > start the image invoked by the bootefi command.
> >
> > Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> >
> > cmd/bootefi.c | 22 +---------------------
> > include/efi_loader.h | 4 ++++
> > lib/efi_loader/efi_boottime.c | 6 +++---
> > 3 files changed, 8 insertions(+), 24 deletions(-)
>
> Hi Jonathan,
>
> With this commit the OpenBSD bootloader (an EFI application) still
> boots, but the loaded OpenBSD kernel doesn't isn't it?
Yes, when it fails the last thing on serial is:
## Starting EFI application at 12000000 ...
>> OpenBSD/armv7 BOOTARM 1.3
boot>
booting sd0a:/bsd: 4572484+689312+238360+561608 [298268+120+314400+278666]=0x0
>
> I bet the problem here is that efi_start_image() sets
> efi_is_direct_boot to false, which means that efi_exit_caches() which
> runs as a result of calling ExitBootServices() no longer
> flushes/disables the caches on 32-bit ARM.
Indeed, removing 'efi_is_direct_boot = false;' from
efi_start_image() allows me to boot multiuser on cubox with 2019.07.
>
> We have been here before. It really isn't possible to flush/disable
> the L2 cache on most 32-bit ARM implementations without SoC-specific
> code. And having such code in the early kernel bootstrap code isn't
> really possible.
>
> The comments in the code suggest that loading an EFI Linux kernel
> directly from U-Boot somehow works without flushing the caches. But I
> don't understand how. But I'm pretty sure this change break booting
> non-EFI Linux kernels through grub on 32-bit ARM platforms as well.
>
> The UEFI spec has the following text (UEFI 2.5 section 2.3.5 "AArch32
> Platforms"):
>
> Implementations of boot services will enable architecturally
> manageable caches and TLBs i.e., those that can be managed directly
> using CP15 operations using mechanisms and procedures defined in the
> ARM Architecture Reference Manual. They should not enable caches
> requiring platform information to manage or invoke non-architectural
> cache/TLB lockdown mechanisms
>
> This suggests that the real problem here is that U-Boot enables the L2
> cache on i.MX6 which violates the spec. But things will probably run
> notably slower without the L2 cache enabled. Maybe the answer is to
> just flush/disable the L2 cache when ExitBootServices() is called?
>
>
> > > ---
> > > cmd/bootefi.c | 22 +---------------------
> > > include/efi_loader.h | 4 ++++
> > > lib/efi_loader/efi_boottime.c | 6 +++---
> > > 3 files changed, 8 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > > index 7f9913c0ee..a2d38256e9 100644
> > > --- a/cmd/bootefi.c
> > > +++ b/cmd/bootefi.c
> > > @@ -133,20 +133,6 @@ done:
> > > return ret;
> > > }
> > >
> > > -static efi_status_t efi_do_enter(
> > > - efi_handle_t image_handle, struct efi_system_table *st,
> > > - EFIAPI efi_status_t (*entry)(
> > > - efi_handle_t image_handle,
> > > - struct efi_system_table *st))
> > > -{
> > > - efi_status_t ret = EFI_LOAD_ERROR;
> > > -
> > > - if (entry)
> > > - ret = entry(image_handle, st);
> > > - st->boottime->exit(image_handle, ret, 0, NULL);
> > > - return ret;
> > > -}
> > > -
> > > /*
> > > * efi_carve_out_dt_rsv() - Carve out DT reserved memory ranges
> > > *
> > > @@ -315,13 +301,7 @@ static efi_status_t do_bootefi_exec(void *efi,
> > >
> > > /* Call our payload! */
> > > debug("%s: Jumping to 0x%p\n", __func__, image_obj->entry);
> > > -
> > > - if (setjmp(&image_obj->exit_jmp)) {
> > > - ret = image_obj->exit_status;
> > > - goto err_prepare;
> > > - }
> > > -
> > > - ret = efi_do_enter(&image_obj->header, &systab, image_obj->entry);
> > > + ret = EFI_CALL(efi_start_image(&image_obj->header, NULL, NULL));
> > >
> > > err_prepare:
> > > /* image has returned, loaded-image obj goes *poof*: */
> > > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > > index 3ce43f7a6f..512880ab8f 100644
> > > --- a/include/efi_loader.h
> > > +++ b/include/efi_loader.h
> > > @@ -320,6 +320,10 @@ efi_status_t efi_create_handle(efi_handle_t *handle);
> > > void efi_delete_handle(efi_handle_t obj);
> > > /* Call this to validate a handle and find the EFI object for it */
> > > struct efi_object *efi_search_obj(const efi_handle_t handle);
> > > +/* Start image */
> > > +efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
> > > + efi_uintn_t *exit_data_size,
> > > + u16 **exit_data);
> > > /* Find a protocol on a handle */
> > > efi_status_t efi_search_protocol(const efi_handle_t handle,
> > > const efi_guid_t *protocol_guid,
> > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > > index 7a61a905f4..6c4e2f82ba 100644
> > > --- a/lib/efi_loader/efi_boottime.c
> > > +++ b/lib/efi_loader/efi_boottime.c
> > > @@ -1770,9 +1770,9 @@ error:
> > > *
> > > * Return: status code
> > > */
> > > -static efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
> > > - efi_uintn_t *exit_data_size,
> > > - u16 **exit_data)
> > > +efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle,
> > > + efi_uintn_t *exit_data_size,
> > > + u16 **exit_data)
> > > {
> > > struct efi_loaded_image_obj *image_obj =
> > > (struct efi_loaded_image_obj *)image_handle;
> > > --
> > > 2.20.1
> > >
> > > _______________________________________________
> > > U-Boot mailing list
> > > U-Boot at lists.denx.de
> > > https://lists.denx.de/listinfo/u-boot
> > _______________________________________________
> > U-Boot mailing list
> > U-Boot at lists.denx.de
> > https://lists.denx.de/listinfo/u-boot
More information about the U-Boot
mailing list