[U-Boot] [RFC 1/6] efi_loader: runtime: make SetVirtualAddressMap configurable

[AKASHI Takahiro]

On Sat, Jun 15, 2019 at 09:46:02PM +0200, Heinrich Schuchardt wrote:
> On 6/5/19 6:21 AM, AKASHI Takahiro wrote:
> >OS does not always need to call SetVirtualAddressMap.
> >(Ard confirmed this on arm64 linux.)
> >So let this API configurable. If disabled, it will return EFI_UNSUPPORTED
> >as UEFI specification requires.
> 
> Currently we do not support this scenario. Alex's patch should go in first.
> 
> >
> >Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> >---
> >  lib/efi_loader/Kconfig       | 7 +++++++
> >  lib/efi_loader/efi_runtime.c | 8 ++++++++
> >  2 files changed, 15 insertions(+)
> >
> >diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> >index 8bf4b1754d06..bb9c7582b14d 100644
> >--- a/lib/efi_loader/Kconfig
> >+++ b/lib/efi_loader/Kconfig
> >@@ -44,6 +44,13 @@ config EFI_SET_TIME
> >  	  Provide the SetTime() runtime service at boottime. This service
> >  	  can be used by an EFI application to adjust the real time clock.
> >
> >+config EFI_RUNTIME_SET_VIRTUAL_ADDRESS_MAP
> >+	bool "runtime service: SetVirtualAddressMap"
> >+	default n
> >+	help
> >+	  Enable SetVirtualAddressMap runtime service. This API will be
> >+	  called by OS just before it enters into virtual address mode.
> >+
> >  config EFI_DEVICE_PATH_TO_TEXT
> >  	bool "Device path to text protocol"
> >  	default y
> >diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> >index 9c50955c9bd0..60442cb21d37 100644
> >--- a/lib/efi_loader/efi_runtime.c
> >+++ b/lib/efi_loader/efi_runtime.c
> >@@ -374,10 +374,12 @@ static const struct efi_runtime_detach_list_struct efi_runtime_detach_list[] = {
> >  		/* do_reset is gone */
> >  		.ptr = &efi_runtime_services.reset_system,
> >  		.patchto = efi_reset_system,
> >+#ifdef CONFIG_RUNTIME_SET_VIRTUAL_ADDRESS_MAP
> >  	}, {
> >  		/* invalidate_*cache_all are gone */
> >  		.ptr = &efi_runtime_services.set_virtual_address_map,
> >  		.patchto = &efi_unimplemented,
> >+#endif
> >  	}, {
> >  		/* RTC accessors are gone */
> >  		.ptr = &efi_runtime_services.get_time,
> >@@ -512,6 +514,7 @@ void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map)
> >          invalidate_icache_all();
> >  }
> >
> >+#ifdef CONFIG_RUNTIME_SET_VIRTUAL_ADDRESS_MAP
> >  /**
> >   * efi_set_virtual_address_map() - change from physical to virtual mapping
> >   *
> >@@ -619,6 +622,7 @@ static efi_status_t EFIAPI efi_set_virtual_address_map(
> >
> >  	return EFI_EXIT(EFI_INVALID_PARAMETER);
> >  }
> >+#endif /* CONFIG_RUNTIME_SET_VIRTUAL_ADDRESS_MAP */
> >
> >  /**
> >   * efi_add_runtime_mmio() - add memory-mapped IO region
> >@@ -796,7 +800,11 @@ struct efi_runtime_services __efi_runtime_data efi_runtime_services = {
> >  	.set_time = &efi_set_time_boottime,
> >  	.get_wakeup_time = (void *)&efi_unimplemented,
> >  	.set_wakeup_time = (void *)&efi_unimplemented,
> >+#ifdef CONFIG_RUNTIME_SET_VIRTUAL_ADDRESS_MAP
> >  	.set_virtual_address_map = &efi_set_virtual_address_map,
> >+#else
> >+	.set_virtual_address_map = (void *)&efi_unimplemented,
> 
> Depending on the ABI it is not save to use a function with another set
> of parameters.

I don't think it's a good practice, but there already are
a couple of precedents:
        get_wakeup_time
        set_wakeup_time
        get_next_high_mono_count, and
        convert_pointer

-Takahiro Akashi

> Best regards
> 
> Heinrich
> 
> >+#endif
> >  	.convert_pointer = (void *)&efi_invalid_parameter,
> >  	.get_variable = efi_get_variable,
> >  	.get_next_variable_name = efi_get_next_variable_name,
> >
>