[U-Boot] [PATCH v4 1/1] efi_loader: Patch non-runtime code out at ExitBootServices already

Ard Biesheuvel ard.biesheuvel at linaro.org
Mon Mar 11 08:03:52 UTC 2019


On Mon, 11 Mar 2019 at 01:16, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> From: Alexander Graf <agraf at suse.de>
>
> 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 work outside 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.biesheuvel at linaro.org>
> Signed-off-by: Alexander Graf <agraf at suse.de>
>
> OpenBSD is not calling SetVirtualAddressMap on ARM 32 bit.
>

I take it this means that OpenBSD does not support runtime services at
all on 32-bit ARM?


> Adjust selftest: expect 'U-Boot' instead of 'resetting'.
>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
> v4
>         Adjust selftest: expect 'U-Boot' instead of 'resetting'.
>         (by fault sent as v2)
> v3
>         Add link to upstream Linux patch
> v2
>         Add missing icache invalidation
> ---
>  include/efi_loader.h               |  2 ++
>  lib/efi_loader/efi_boottime.c      |  1 +
>  lib/efi_loader/efi_runtime.c       | 29 ++++++++++++++++++++---------
>  test/py/tests/test_efi_selftest.py |  4 ++--
>  4 files changed, 25 insertions(+), 11 deletions(-)
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 512880ab8fb..82db7775c72 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 bd8b8a17ae7..233bca78e0a 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1967,6 +1967,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 636dfdab39d..17d22d429e2 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -276,15 +276,11 @@ 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,
> @@ -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);
>                 }
> diff --git a/test/py/tests/test_efi_selftest.py b/test/py/tests/test_efi_selftest.py
> index 36b35ee536b..6f5b2b8f406 100644
> --- a/test/py/tests/test_efi_selftest.py
> +++ b/test/py/tests/test_efi_selftest.py
> @@ -20,7 +20,7 @@ def test_efi_selftest(u_boot_console):
>         if m != 0:
>                 raise Exception('Failures occurred during the EFI selftest')
>         u_boot_console.run_command(cmd='', wait_for_echo=False, wait_for_prompt=False);
> -       m = u_boot_console.p.expect(['resetting', 'U-Boot'])
> +       m = u_boot_console.p.expect(['U-Boot'])
>         if m != 0:
>                 raise Exception('Reset failed during the EFI selftest')
>         u_boot_console.restart_uboot();
> @@ -46,7 +46,7 @@ def test_efi_selftest_watchdog_reboot(u_boot_console):
>         assert '\'watchdog reboot\'' in output
>         u_boot_console.run_command(cmd='setenv efi_selftest watchdog reboot')
>         u_boot_console.run_command(cmd='bootefi selftest', wait_for_prompt=False)
> -       m = u_boot_console.p.expect(['resetting', 'U-Boot'])
> +       m = u_boot_console.p.expect(['U-Boot'])
>         if m != 0:
>                 raise Exception('Reset failed in \'watchdog reboot\' test')
>         u_boot_console.restart_uboot();
> --
> 2.20.1
>


More information about the U-Boot mailing list