[U-Boot] [PATCH v2 3/4] efi_loader: use efi_start_image() for bootefi
Heinrich Schuchardt
xypron.glpk at gmx.de
Thu Jul 18 17:33:53 UTC 2019
On 7/18/19 11:16 AM, Jonathan Gray wrote:
> 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
Thanks for your investigation.
To which spec are you relating?
UEFI 2.8 spec, chapter 2.3.5 AArch32 Platforms:
The core will be configured as follows (common across all processor
architecture revisions): Instruction and Data caches enabled
GRUB is invalidating the cache before starting Linux by calling
grub_arch_sync_caches ((void *) linux_addr, linux_size);.
I think the BSD bootloader will also have to clear caches after loading
the main program.
@Alex:
When should we call clean_up_before_linux()?
In the first StartImage() call?
Always in ExitBootServices()?
Best regards
Heinrich
>> 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