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

[Heinrich Schuchardt]

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.biesheuvel at linaro.org>
> Signed-off-by: Alexander Graf <agraf at 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);
>  		}
>