diff mbox series

[U-Boot,3/4] efi_loader: implement ConvertPointer()

Message ID 20190727210004.6379-4-xypron.glpk@gmx.de
State Superseded, archived
Delegated to: Heinrich Schuchardt
Headers show
Series efi_loader: implement ConvertPointer() | expand

Commit Message

Heinrich Schuchardt July 27, 2019, 9 p.m. UTC 
Implement the ConvertPointer() runtime service.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_loader/efi_runtime.c | 75 ++++++++++++++++++++++++++++++++++--
 1 file changed, 71 insertions(+), 4 deletions(-)

--
2.20.1

Comments

AKASHI Takahiro July 29, 2019, 1:37 a.m. UTC | #1 
On Sat, Jul 27, 2019 at 11:00:03PM +0200, Heinrich Schuchardt wrote:
> Implement the ConvertPointer() runtime service.

I already submitted a similar patch:
https://lists.denx.de/pipermail/u-boot/2019-June/371773.html

Anyhow,

> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_loader/efi_runtime.c | 75 ++++++++++++++++++++++++++++++++++--
>  1 file changed, 71 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
> index a8f0b5eae3..2286e847a7 100644
> --- a/lib/efi_loader/efi_runtime.c
> +++ b/lib/efi_loader/efi_runtime.c
> @@ -81,6 +81,10 @@ struct elf_rela {
>  	long addend;
>  };
> 
> +static __efi_runtime_data struct efi_mem_desc *efi_virtmap;
> +static __efi_runtime_data efi_uintn_t efi_descriptor_count;
> +static __efi_runtime_data efi_uintn_t efi_descriptor_size;
> +
>  /*
>   * EFI runtime code lives in two stages. In the first stage, U-Boot and an EFI
>   * payload are running concurrently at the same time. In this mode, we can
> @@ -89,7 +93,9 @@ struct elf_rela {
> 
>  efi_status_t efi_init_runtime_supported(void)
>  {
> -	u16 efi_runtime_services_supported = 0;
> +	u16 efi_runtime_services_supported =
> +				EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP ||

"||" -> "|"

> +				EFI_RT_SUPPORTED_CONVERT_POINTER;
> 
>  	/*
>  	 * This value must be synced with efi_runtime_detach_list
> @@ -98,8 +104,7 @@ efi_status_t efi_init_runtime_supported(void)
>  #ifdef CONFIG_EFI_HAVE_RUNTIME_RESET
>  	efi_runtime_services_supported |= EFI_RT_SUPPORTED_RESET_SYSTEM;
>  #endif
> -	efi_runtime_services_supported |=
> -				EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP;
> +
>  	return EFI_CALL(efi_set_variable(L"RuntimeServicesSupported",
>  					 &efi_global_variable_guid,
>  					 EFI_VARIABLE_BOOTSERVICE_ACCESS |
> @@ -454,6 +459,58 @@ static __efi_runtime efi_status_t EFIAPI efi_convert_pointer_runtime(
>  	return EFI_UNSUPPORTED;
>  }
> 
> +/**
> + * efi_convert_pointer_runtime() - convert from physical to virtual pointer
> + *
> + * This function implements the ConvertPointer() runtime service until
> + * the first call to SetVirtualAddressMap().
> + *
> + * See the Unified Extensible Firmware Interface (UEFI) specification for
> + * details.
> + *
> + * @debug_disposition:	indicates if pointer may be converted to NULL
> + * @address:		pointer to be converted
> + * Return:		status code EFI_UNSUPPORTED
> + */
> +static __efi_runtime efi_status_t EFIAPI efi_convert_pointer(
> +			efi_uintn_t debug_disposition, void **address)
> +{
> +	efi_physical_addr_t addr = (uintptr_t)*address;
> +	efi_uintn_t i;
> +	efi_status_t ret = EFI_NOT_FOUND;
> +
> +	EFI_ENTRY("%zu %p", debug_disposition, address);
> +
> +	if (!efi_virtmap) {
> +		ret = EFI_UNSUPPORTED;
> +		goto out;
> +	}
> +
> +	if (!address) {
> +		ret = EFI_INVALID_PARAMETER;
> +		goto out;
> +	}
> +
> +	for (i = 0; i < efi_descriptor_count; i++) {
> +		struct efi_mem_desc *map = (void *)efi_virtmap +
> +					   (efi_descriptor_size * i);
> +
> +		if (addr >= map->physical_start &&
> +		    (addr < map->physical_start
> +			    + (map->num_pages << EFI_PAGE_SHIFT))) {
> +			*address = (void *)(uintptr_t)
> +				   (addr + map->virtual_start -
> +				    map->physical_start);
> +
> +			ret = EFI_SUCCESS;
> +			break;
> +		}
> +	}
> +
> +out:
> +	return EFI_EXIT(ret);
> +}
> +
>  static __efi_runtime void efi_relocate_runtime_table(ulong offset)
>  {
>  	ulong patchoff;
> @@ -480,6 +537,12 @@ static __efi_runtime void efi_relocate_runtime_table(ulong offset)
>  	 */
>  	efi_runtime_services.convert_pointer = &efi_convert_pointer_runtime;
> 
> +	/*
> +	 * TODO: Update UEFI variable RuntimeServicesSupported removing flags
> +	 * EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP and
> +	 * EFI_RT_SUPPORTED_CONVERT_POINTER as required by the UEFI spec 2.8.
> +	 */
> +
>  	/* Update CRC32 */
>  	efi_update_table_header_crc32(&efi_runtime_services.hdr);
>  }
> @@ -584,6 +647,10 @@ static efi_status_t EFIAPI efi_set_virtual_address_map(
>  	EFI_ENTRY("%zx %zx %x %p", memory_map_size, descriptor_size,
>  		  descriptor_version, virtmap);
> 
> +	efi_virtmap = virtmap;
> +	efi_descriptor_size = descriptor_size;
> +	efi_descriptor_count = n;
> +

As I mentioned in the patch above,

!        /* Rebind mmio pointers */
!                for (i = 0; i < n; i++) {
!       ...

This part of code in efi_set_virtual_address_map() is fragile as 
entries in efi_runtime_mmio belong to memory allocated by calloc(),
which means that they are no longer accessible at runtime.

-Takahiro Akashi

>  	/*
>  	 * TODO:
>  	 * Further down we are cheating. While really we should implement
> @@ -800,7 +867,7 @@ struct efi_runtime_services __efi_runtime_data efi_runtime_services = {
>  	.get_wakeup_time = (void *)&efi_unimplemented,
>  	.set_wakeup_time = (void *)&efi_unimplemented,
>  	.set_virtual_address_map = &efi_set_virtual_address_map,
> -	.convert_pointer = (void *)&efi_unimplemented,
> +	.convert_pointer = efi_convert_pointer,
>  	.get_variable = efi_get_variable,
>  	.get_next_variable_name = efi_get_next_variable_name,
>  	.set_variable = efi_set_variable,
> --
> 2.20.1
>
Heinrich Schuchardt July 29, 2019, 5:51 a.m. UTC | #2 
On 7/29/19 3:37 AM, AKASHI Takahiro wrote:
> On Sat, Jul 27, 2019 at 11:00:03PM +0200, Heinrich Schuchardt wrote:
>> Implement the ConvertPointer() runtime service.

Thank you for reviewing.

>
> I already submitted a similar patch:
> https://lists.denx.de/pipermail/u-boot/2019-June/371773.html

Unfortunately I do not see any follow up version of the patch after I
started reviewing.

>
> Anyhow,

Should I put a
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> or a
Suggested-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
before my signature?

>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>   lib/efi_loader/efi_runtime.c | 75 ++++++++++++++++++++++++++++++++++--
>>   1 file changed, 71 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
>> index a8f0b5eae3..2286e847a7 100644
>> --- a/lib/efi_loader/efi_runtime.c
>> +++ b/lib/efi_loader/efi_runtime.c
>> @@ -81,6 +81,10 @@ struct elf_rela {
>>   	long addend;
>>   };
>>
>> +static __efi_runtime_data struct efi_mem_desc *efi_virtmap;
>> +static __efi_runtime_data efi_uintn_t efi_descriptor_count;
>> +static __efi_runtime_data efi_uintn_t efi_descriptor_size;
>> +
>>   /*
>>    * EFI runtime code lives in two stages. In the first stage, U-Boot and an EFI
>>    * payload are running concurrently at the same time. In this mode, we can
>> @@ -89,7 +93,9 @@ struct elf_rela {
>>
>>   efi_status_t efi_init_runtime_supported(void)
>>   {
>> -	u16 efi_runtime_services_supported = 0;
>> +	u16 efi_runtime_services_supported =
>> +				EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP ||
>
> "||" -> "|"

I will fix this?

>
>> +				EFI_RT_SUPPORTED_CONVERT_POINTER;
>>
>>   	/*
>>   	 * This value must be synced with efi_runtime_detach_list
>> @@ -98,8 +104,7 @@ efi_status_t efi_init_runtime_supported(void)
>>   #ifdef CONFIG_EFI_HAVE_RUNTIME_RESET
>>   	efi_runtime_services_supported |= EFI_RT_SUPPORTED_RESET_SYSTEM;
>>   #endif
>> -	efi_runtime_services_supported |=
>> -				EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP;
>> +
>>   	return EFI_CALL(efi_set_variable(L"RuntimeServicesSupported",
>>   					 &efi_global_variable_guid,
>>   					 EFI_VARIABLE_BOOTSERVICE_ACCESS |
>> @@ -454,6 +459,58 @@ static __efi_runtime efi_status_t EFIAPI efi_convert_pointer_runtime(
>>   	return EFI_UNSUPPORTED;
>>   }
>>
>> +/**
>> + * efi_convert_pointer_runtime() - convert from physical to virtual pointer
>> + *
>> + * This function implements the ConvertPointer() runtime service until
>> + * the first call to SetVirtualAddressMap().
>> + *
>> + * See the Unified Extensible Firmware Interface (UEFI) specification for
>> + * details.
>> + *
>> + * @debug_disposition:	indicates if pointer may be converted to NULL
>> + * @address:		pointer to be converted
>> + * Return:		status code EFI_UNSUPPORTED
>> + */
>> +static __efi_runtime efi_status_t EFIAPI efi_convert_pointer(
>> +			efi_uintn_t debug_disposition, void **address)
>> +{
>> +	efi_physical_addr_t addr = (uintptr_t)*address;
>> +	efi_uintn_t i;
>> +	efi_status_t ret = EFI_NOT_FOUND;
>> +
>> +	EFI_ENTRY("%zu %p", debug_disposition, address);
>> +
>> +	if (!efi_virtmap) {
>> +		ret = EFI_UNSUPPORTED;
>> +		goto out;
>> +	}
>> +
>> +	if (!address) {
>> +		ret = EFI_INVALID_PARAMETER;
>> +		goto out;
>> +	}
>> +
>> +	for (i = 0; i < efi_descriptor_count; i++) {
>> +		struct efi_mem_desc *map = (void *)efi_virtmap +
>> +					   (efi_descriptor_size * i);
>> +
>> +		if (addr >= map->physical_start &&
>> +		    (addr < map->physical_start
>> +			    + (map->num_pages << EFI_PAGE_SHIFT))) {
>> +			*address = (void *)(uintptr_t)
>> +				   (addr + map->virtual_start -
>> +				    map->physical_start);
>> +
>> +			ret = EFI_SUCCESS;
>> +			break;
>> +		}
>> +	}
>> +
>> +out:
>> +	return EFI_EXIT(ret);
>> +}
>> +
>>   static __efi_runtime void efi_relocate_runtime_table(ulong offset)
>>   {
>>   	ulong patchoff;
>> @@ -480,6 +537,12 @@ static __efi_runtime void efi_relocate_runtime_table(ulong offset)
>>   	 */
>>   	efi_runtime_services.convert_pointer = &efi_convert_pointer_runtime;
>>
>> +	/*
>> +	 * TODO: Update UEFI variable RuntimeServicesSupported removing flags
>> +	 * EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP and
>> +	 * EFI_RT_SUPPORTED_CONVERT_POINTER as required by the UEFI spec 2.8.
>> +	 */
>> +
>>   	/* Update CRC32 */
>>   	efi_update_table_header_crc32(&efi_runtime_services.hdr);
>>   }
>> @@ -584,6 +647,10 @@ static efi_status_t EFIAPI efi_set_virtual_address_map(
>>   	EFI_ENTRY("%zx %zx %x %p", memory_map_size, descriptor_size,
>>   		  descriptor_version, virtmap);
>>
>> +	efi_virtmap = virtmap;
>> +	efi_descriptor_size = descriptor_size;
>> +	efi_descriptor_count = n;
>> +
>
> As I mentioned in the patch above,
>
> !        /* Rebind mmio pointers */
> !                for (i = 0; i < n; i++) {
> !       ...
>
> This part of code in efi_set_virtual_address_map() is fragile as
> entries in efi_runtime_mmio belong to memory allocated by calloc(),
> which means that they are no longer accessible at runtime.

This should be addressed in a separate patch.

Best regards

Heinrich

>
> -Takahiro Akashi
>
>>   	/*
>>   	 * TODO:
>>   	 * Further down we are cheating. While really we should implement
>> @@ -800,7 +867,7 @@ struct efi_runtime_services __efi_runtime_data efi_runtime_services = {
>>   	.get_wakeup_time = (void *)&efi_unimplemented,
>>   	.set_wakeup_time = (void *)&efi_unimplemented,
>>   	.set_virtual_address_map = &efi_set_virtual_address_map,
>> -	.convert_pointer = (void *)&efi_unimplemented,
>> +	.convert_pointer = efi_convert_pointer,
>>   	.get_variable = efi_get_variable,
>>   	.get_next_variable_name = efi_get_next_variable_name,
>>   	.set_variable = efi_set_variable,
>> --
>> 2.20.1
>>
>
diff mbox series

Patch

diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
index a8f0b5eae3..2286e847a7 100644
--- a/lib/efi_loader/efi_runtime.c
+++ b/lib/efi_loader/efi_runtime.c
@@ -81,6 +81,10 @@  struct elf_rela {
 	long addend;
 };

+static __efi_runtime_data struct efi_mem_desc *efi_virtmap;
+static __efi_runtime_data efi_uintn_t efi_descriptor_count;
+static __efi_runtime_data efi_uintn_t efi_descriptor_size;
+
 /*
  * EFI runtime code lives in two stages. In the first stage, U-Boot and an EFI
  * payload are running concurrently at the same time. In this mode, we can
@@ -89,7 +93,9 @@  struct elf_rela {

 efi_status_t efi_init_runtime_supported(void)
 {
-	u16 efi_runtime_services_supported = 0;
+	u16 efi_runtime_services_supported =
+				EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP ||
+				EFI_RT_SUPPORTED_CONVERT_POINTER;

 	/*
 	 * This value must be synced with efi_runtime_detach_list
@@ -98,8 +104,7 @@  efi_status_t efi_init_runtime_supported(void)
 #ifdef CONFIG_EFI_HAVE_RUNTIME_RESET
 	efi_runtime_services_supported |= EFI_RT_SUPPORTED_RESET_SYSTEM;
 #endif
-	efi_runtime_services_supported |=
-				EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP;
+
 	return EFI_CALL(efi_set_variable(L"RuntimeServicesSupported",
 					 &efi_global_variable_guid,
 					 EFI_VARIABLE_BOOTSERVICE_ACCESS |
@@ -454,6 +459,58 @@  static __efi_runtime efi_status_t EFIAPI efi_convert_pointer_runtime(
 	return EFI_UNSUPPORTED;
 }

+/**
+ * efi_convert_pointer_runtime() - convert from physical to virtual pointer
+ *
+ * This function implements the ConvertPointer() runtime service until
+ * the first call to SetVirtualAddressMap().
+ *
+ * See the Unified Extensible Firmware Interface (UEFI) specification for
+ * details.
+ *
+ * @debug_disposition:	indicates if pointer may be converted to NULL
+ * @address:		pointer to be converted
+ * Return:		status code EFI_UNSUPPORTED
+ */
+static __efi_runtime efi_status_t EFIAPI efi_convert_pointer(
+			efi_uintn_t debug_disposition, void **address)
+{
+	efi_physical_addr_t addr = (uintptr_t)*address;
+	efi_uintn_t i;
+	efi_status_t ret = EFI_NOT_FOUND;
+
+	EFI_ENTRY("%zu %p", debug_disposition, address);
+
+	if (!efi_virtmap) {
+		ret = EFI_UNSUPPORTED;
+		goto out;
+	}
+
+	if (!address) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	for (i = 0; i < efi_descriptor_count; i++) {
+		struct efi_mem_desc *map = (void *)efi_virtmap +
+					   (efi_descriptor_size * i);
+
+		if (addr >= map->physical_start &&
+		    (addr < map->physical_start
+			    + (map->num_pages << EFI_PAGE_SHIFT))) {
+			*address = (void *)(uintptr_t)
+				   (addr + map->virtual_start -
+				    map->physical_start);
+
+			ret = EFI_SUCCESS;
+			break;
+		}
+	}
+
+out:
+	return EFI_EXIT(ret);
+}
+
 static __efi_runtime void efi_relocate_runtime_table(ulong offset)
 {
 	ulong patchoff;
@@ -480,6 +537,12 @@  static __efi_runtime void efi_relocate_runtime_table(ulong offset)
 	 */
 	efi_runtime_services.convert_pointer = &efi_convert_pointer_runtime;

+	/*
+	 * TODO: Update UEFI variable RuntimeServicesSupported removing flags
+	 * EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP and
+	 * EFI_RT_SUPPORTED_CONVERT_POINTER as required by the UEFI spec 2.8.
+	 */
+
 	/* Update CRC32 */
 	efi_update_table_header_crc32(&efi_runtime_services.hdr);
 }
@@ -584,6 +647,10 @@  static efi_status_t EFIAPI efi_set_virtual_address_map(
 	EFI_ENTRY("%zx %zx %x %p", memory_map_size, descriptor_size,
 		  descriptor_version, virtmap);

+	efi_virtmap = virtmap;
+	efi_descriptor_size = descriptor_size;
+	efi_descriptor_count = n;
+
 	/*
 	 * TODO:
 	 * Further down we are cheating. While really we should implement
@@ -800,7 +867,7 @@  struct efi_runtime_services __efi_runtime_data efi_runtime_services = {
 	.get_wakeup_time = (void *)&efi_unimplemented,
 	.set_wakeup_time = (void *)&efi_unimplemented,
 	.set_virtual_address_map = &efi_set_virtual_address_map,
-	.convert_pointer = (void *)&efi_unimplemented,
+	.convert_pointer = efi_convert_pointer,
 	.get_variable = efi_get_variable,
 	.get_next_variable_name = efi_get_next_variable_name,
 	.set_variable = efi_set_variable,