[U-Boot] [RFC 2/6] efi: add RuntimeServicesSupported variable

[Heinrich Schuchardt]

On 6/13/19 11:17 AM, Heinrich Schuchardt wrote:
>
>
> On 6/13/19 9:06 AM, AKASHI Takahiro wrote:
>> On Thu, Jun 13, 2019 at 07:56:19AM +0200, Heinrich Schuchardt wrote:
>>>
>>>
>>> On 6/5/19 6:21 AM, AKASHI Takahiro wrote:
>>>> This variable is defined in UEFI specification 2.8, section 8.1.
>>>> Its value should be updated whenever we add any usable runtime services
>>>> function.
>>>
>>> It is also required by the EBBR specification.
>>>
>>>>
>>>> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
>>>> ---
>>>>  include/efi_api.h            | 15 +++++++++++++++
>>>>  include/efi_loader.h         |  3 +++
>>>>  lib/efi_loader/efi_runtime.c | 28 ++++++++++++++++++++++++++++
>>>>  lib/efi_loader/efi_setup.c   |  5 +++++
>>>>  4 files changed, 51 insertions(+)
>>>>
>>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>>> index 65584dd2d82a..d7d95edd4dfc 100644
>>>> --- a/include/efi_api.h
>>>> +++ b/include/efi_api.h
>>>> @@ -213,6 +213,21 @@ struct efi_capsule_header {
>>>>  	u32 capsule_image_size;
>>>>  };
>>>>
>>>> +#define EFI_RT_SUPPORTED_GET_TIME			0x0001
>>>> +#define EFI_RT_SUPPORTED_SET_TIME			0x0002
>>>> +#define EFI_RT_SUPPORTED_GET_WAKEUP_TIME		0x0004
>>>> +#define EFI_RT_SUPPORTED_SET_WAKEUP_TIME		0x0008
>>>> +#define EFI_RT_SUPPORTED_GET_VARIABLE			0x0010
>>>> +#define EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME		0x0020
>>>> +#define EFI_RT_SUPPORTED_SET_VARIABLE			0x0040
>>>> +#define EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP	0x0080
>>>> +#define EFI_RT_SUPPORTED_CONVERT_POINTER		0x0100
>>>> +#define EFI_RT_SUPPORTED_GET_NEXT_HIGH_MONOTONIC_COUNT	0x0200
>>>> +#define EFI_RT_SUPPORTED_RESET_SYSTEM			0x0400
>>>> +#define EFI_RT_SUPPORTED_UPDATE_CAPSULE			0x0800
>>>> +#define EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES	0x1000
>>>> +#define EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO		0x2000
>>>> +
>>>>  struct efi_runtime_services {
>>>>  	struct efi_table_hdr hdr;
>>>>  	efi_status_t (EFIAPI *get_time)(struct efi_time *time,
>>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>>> index 23ce73226762..7bd8002e303e 100644
>>>> --- a/include/efi_loader.h
>>>> +++ b/include/efi_loader.h
>>>> @@ -573,6 +573,9 @@ static inline int guidcmp(const efi_guid_t *g1, const efi_guid_t *g2)
>>>>  #define __efi_runtime_data __attribute__ ((section (".data.efi_runtime")))
>>>>  #define __efi_runtime __attribute__ ((section (".text.efi_runtime")))
>>>>
>>>> +/* Indicate supported runtime services */
>>>> +efi_status_t efi_init_runtime_supported(void);
>>>> +
>>>>  /* Update CRC32 in table header */
>>>>  void __efi_runtime efi_update_table_header_crc32(struct efi_table_hdr *table);
>>>>
>>>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
>>>> index 60442cb21d37..cf202bb9ec07 100644
>>>> --- a/lib/efi_loader/efi_runtime.c
>>>> +++ b/lib/efi_loader/efi_runtime.c
>>>> @@ -89,6 +89,34 @@ struct elf_rela {
>>>>   * handle a good number of runtime callbacks
>>>>   */
>>>>
>>>> +efi_status_t efi_init_runtime_supported(void)
>>>> +{
>>>> +	u16 efi_runtime_services_supported;
>>>> +
>>>> +	/*
>>>> +	 * This value must be synced with efi_runtime_detach_list
>>>> +	 * as well as efi_runtime_services.
>>>> +	 */
>>>> +	efi_runtime_services_supported = EFI_RT_SUPPORTED_RESET_SYSTEM;
>>>
>>> This support is system dependent, e.g. on RK3288 systems it does not exist.
>>
>> efi_reset_system() is defined as a weak function and so
>> there is no easy way to determine whether this API is supported or not.
>>
>>>> +#ifdef CONFIG_EFI_GET_TIME
>>>> +	efi_runtime_services_supported |= EFI_RT_SUPPORTED_GET_TIME;
>>>
>>> We do not support this at runtime.
>>
>> Okay, I will drop it.
>>
>>>> +#endif
>>>> +#ifdef CONFIG_EFI_SET_TIME
>>>> +	efi_runtime_services_supported |= EFI_RT_SUPPORTED_SET_TIME;
>>>
>>> We do not support this at runtime.
>>
>> ditto
>>
>>>> +#endif
>>>> +#ifdef CONFIG_EFI_RUNTIME_SET_VIRTUAL_ADDRESS_MAP
>>>> +	efi_runtime_services_supported |=
>>>> +				EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP;
>>>
>>> Our support is incomplete as we have not implemented ConvertPointer().
>>
>> Incomplete?
>> My patch#3 adds ConvertPointer().
>
> That is a later patch. So I did not consider it for this patch.
> Furthermore the implementation of ConvertPointer depends on
> CONFIG_EFI_RUNTIME_CONVERT_POINTER.

EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP and
EFI_RT_SUPPORTED_CONVERT_POINTER are separate values.

You are right in switching EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP on here.

Best regards

Heinrich

>>
>> -Takahiro Akashi
>>
>>> For unsupported services we will have to change the return value to
>>> EFI_UNSUPPORTED. But that will be a separate patch.
>>>
>>> Best regards
>>>
>>> Heinrich
>>>
>>>> +#endif
>>>> +
>>>> +	return EFI_CALL(efi_set_variable(L"RuntimeServicesSupported",
>>>> +					 &efi_global_variable_guid,
>>>> +					 EFI_VARIABLE_BOOTSERVICE_ACCESS |
>>>> +					 EFI_VARIABLE_RUNTIME_ACCESS,
>>>> +					 sizeof(efi_runtime_services_supported),
>>>> +					 &efi_runtime_services_supported));
>>>> +}
>>>> +
>>>>  /**
>>>>   * efi_update_table_header_crc32() - Update crc32 in table header
>>>>   *
>>>> diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
>>>> index 45d6aca051f3..75fa344060d5 100644
>>>> --- a/lib/efi_loader/efi_setup.c
>>>> +++ b/lib/efi_loader/efi_setup.c
>>>> @@ -123,6 +123,11 @@ efi_status_t efi_init_obj_list(void)
>>>>  	if (ret != EFI_SUCCESS)
>>>>  		goto out;
>>>>
>>>> +	/* Indicate supported runtime services */
>>>> +	ret = efi_init_runtime_supported();
>>>> +	if (ret != EFI_SUCCESS)
>>>> +		goto out;
>>>> +
>>>>  	/* Initialize system table */
>>>>  	ret = efi_initialize_system_table();
>>>>  	if (ret != EFI_SUCCESS)
>>>>
>>
>