On 1/30/19 11:46 AM, Alexander Graf wrote:
> While discussing something compeltely different, Ard pointed out
> that it might be legal to omit calling SetVirtualAddressMap altogether.
>
> There is even a patch on the Linux Kernel Mailing List that implements
> such behavior by now:
>
> https://patchwork.kernel.org/patch/10782393/
>
> While that sounds great, we currently rely on the SetVirtualAddressMap
> call to remove all references to code that would not workoutside of
> boot services.
>
> So let's patch out those bits already on the call to ExitBootServices,
> so that we can successfully run even when an OS chooses to omit
> any call to SetVirtualAddressMap.
>
> Reported-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> Signed-off-by: Alexander Graf <ag...@suse.de>
>
> ---
>
This patch series leads to a Python test error on qemu_arm64_defconfig:
if m != 0:
> raise Exception('Reset failed during the EFI selftest')
E Exception: Reset failed during the EFI selftest
test/py/tests/test_efi_selftest.py:25: Exception
<snip />
The output looks like:
Preparing for reset. Press any key...
U-Boot 2019.01-00458-gada5205aef (Jan 30 2019 - 19:47:02 +0100)
DRAM:
<snip />
Please, adjust test_efi_selftest().
Best regards
Heinrich
> v1 -> v2:
>
> - Add missing icache invalidation
>
> v2 -> v3:
>
> - Add link to upstream Linux patch
> ---
> include/efi_loader.h | 2 ++
> lib/efi_loader/efi_boottime.c | 1 +
> lib/efi_loader/efi_runtime.c | 29 ++++++++++++++++++++---------
> 3 files changed, 23 insertions(+), 9 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 9dd933dae7..2a40b09693 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -310,6 +310,8 @@ void efi_save_gd(void);
> void efi_restore_gd(void);
> /* Call this to relocate the runtime section to an address space */
> void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map);
> +/* Call this when we start to live in a runtime only world */
> +void efi_runtime_detach(ulong offset);
> /* Call this to set the current device name */
> void efi_set_bootdev(const char *dev, const char *devnr, const char *path);
> /* Add a new object to the object list. */
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index fc26d6adc1..8cb2979bab 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1917,6 +1917,7 @@ static efi_status_t EFIAPI
> efi_exit_boot_services(efi_handle_t image_handle,
> bootm_disable_interrupts();
>
> /* Disable boot time services */
> + efi_runtime_detach((ulong)gd->relocaddr);
> systab.con_in_handle = NULL;
> systab.con_in = NULL;
> systab.con_out_handle = NULL;
> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> index 636dfdab39..17d22d429e 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -276,16 +276,12 @@ struct efi_runtime_detach_list_struct {
> void *patchto;
> };
>
> -static const struct efi_runtime_detach_list_struct efi_runtime_detach_list[]
> = {
> +static struct efi_runtime_detach_list_struct efi_runtime_detach_list[] = {
> {
> /* do_reset is gone */
> .ptr = &efi_runtime_services.reset_system,
> .patchto = efi_reset_system,
> }, {
> - /* invalidate_*cache_all are gone */
> - .ptr = &efi_runtime_services.set_virtual_address_map,
> - .patchto = &efi_unimplemented,
> - }, {
> /* RTC accessors are gone */
> .ptr = &efi_runtime_services.get_time,
> .patchto = &efi_get_time,
> @@ -328,7 +324,15 @@ static bool efi_runtime_tobedetached(void *p)
> return false;
> }
>
> -static void efi_runtime_detach(ulong offset)
> +/**
> + * efi_runtime_detach() - Remove any dependency on non-runtime sections
> + *
> + * This function patches all remaining code to be self-sufficient inside
> + * runtime sections. Any calls to non-runtime will be removed after this.
> + *
> + * @offset: relocaddr for pre-set_v_a_space, offset to VA after
> + */
> +__efi_runtime void efi_runtime_detach(ulong offset)
> {
> int i;
> ulong patchoff = offset - (ulong)gd->relocaddr;
> @@ -344,6 +348,8 @@ static void efi_runtime_detach(ulong offset)
>
> /* Update CRC32 */
> efi_update_table_header_crc32(&efi_runtime_services.hdr);
> +
> + invalidate_icache_all();
> }
>
> /* Relocate EFI runtime to uboot_reloc_base = offset */
> @@ -430,19 +436,25 @@ void efi_runtime_relocate(ulong offset, struct
> efi_mem_desc *map)
> * @virtmap: virtual address mapping information
> * Return: status code
> */
> -static efi_status_t EFIAPI efi_set_virtual_address_map(
> +static __efi_runtime efi_status_t EFIAPI efi_set_virtual_address_map(
> unsigned long memory_map_size,
> unsigned long descriptor_size,
> uint32_t descriptor_version,
> struct efi_mem_desc *virtmap)
> {
> + static __efi_runtime_data bool is_patched;
> int n = memory_map_size / descriptor_size;
> int i;
> int rt_code_sections = 0;
>
> + if (is_patched)
> + return EFI_INVALID_PARAMETER;
> +
> EFI_ENTRY("%lx %lx %x %p", memory_map_size, descriptor_size,
> descriptor_version, virtmap);
>
> + is_patched = true;
> +
> /*
> * TODO:
> * Further down we are cheating. While really we should implement
> @@ -514,8 +526,7 @@ static efi_status_t EFIAPI efi_set_virtual_address_map(
> map->physical_start + gd->relocaddr;
>
> efi_runtime_relocate(new_offset, map);
> - /* Once we're virtual, we can no longer handle
> - complex callbacks */
> + /* We need to repatch callbacks for their new VA */
> efi_runtime_detach(new_offset);
> return EFI_EXIT(EFI_SUCCESS);
> }
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
