From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jonathan Gray Date: Thu, 18 Jul 2019 19:16:44 +1000 Subject: [U-Boot] [PATCH v2 3/4] efi_loader: use efi_start_image() for bootefi In-Reply-To: <5438c1db9647d8f4@bloch.sibelius.xs4all.nl> References: <20190208184650.22765-1-xypron.glpk@gmx.de> <20190208184650.22765-4-xypron.glpk@gmx.de> <20190718060016.GA73593@largo.jsg.id.au> <5438c1db9647d8f4@bloch.sibelius.xs4all.nl> Message-ID: <20190718091644.GA18133@largo.jsg.id.au> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 > > > > 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 > > > --- > > > 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 > > 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 > > > > 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