[U-Boot] [PATCH 6/9] efi_loader: Add runtime services

[Alexander Graf <agraf@suse.de>]

On 26.12.15 19:33, Leif Lindholm wrote:
> On Tue, Dec 22, 2015 at 02:57:53PM +0100, Alexander Graf wrote:
>> After booting has finished, EFI allows firmware to still interact with the OS
>> using the "runtime services". These callbacks live in a separate address space,
>> since they are available long after U-Boot has been overwritten by the OS.
>>
>> However, since U-Boot has no notion of RTS, we just create an extremely minimal
>> RTS stub that just declares all functions as unsupported. We could in the future
>> map U-boot environment variables to EFI variables here.
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>  arch/arm/cpu/armv8/u-boot.lds |  8 ++++++
>>  arch/arm/cpu/u-boot.lds       | 13 ++++++++++
>>  arch/arm/lib/sections.c       |  2 ++
>>  include/efi_loader.h          |  3 +++
>>  lib/efi_loader/efi_runtime.c  | 59 +++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 85 insertions(+)
>>  create mode 100644 lib/efi_loader/efi_runtime.c
>>
>> diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
>> index 4c12222..7c5b032 100644
>> --- a/arch/arm/cpu/armv8/u-boot.lds
>> +++ b/arch/arm/cpu/armv8/u-boot.lds
>> @@ -42,6 +42,14 @@ SECTIONS
>>  
>>  	. = ALIGN(8);
>>  
>> +	.efi_runtime : {
>> +                __efi_runtime_start = .;
>> +		*(efi_runtime)
>> +                __efi_runtime_stop = .;
>> +	}
>> +
>> +	. = ALIGN(8);
>> +
>>  	.image_copy_end :
>>  	{
>>  		*(.__image_copy_end)
>> diff --git a/arch/arm/cpu/u-boot.lds b/arch/arm/cpu/u-boot.lds
>> index d48a905..b5198d0 100644
>> --- a/arch/arm/cpu/u-boot.lds
>> +++ b/arch/arm/cpu/u-boot.lds
>> @@ -89,6 +89,19 @@ SECTIONS
>>  
>>  	. = ALIGN(4);
>>  
>> +	.__efi_runtime_start : {
>> +		*(.__efi_runtime_start)
>> +	}
>> +
>> +	.efi_runtime : {
>> +		*(efi_runtime)
>> +	}
>> +
>> +	.__efi_runtime_stop : {
>> +		*(.__efi_runtime_stop)
>> +	}
>> +	. = ALIGN(4);
>> +
>>  	.image_copy_end :
>>  	{
>>  		*(.__image_copy_end)
>> diff --git a/arch/arm/lib/sections.c b/arch/arm/lib/sections.c
>> index a1205c3..21b3066 100644
>> --- a/arch/arm/lib/sections.c
>> +++ b/arch/arm/lib/sections.c
>> @@ -27,4 +27,6 @@ char __rel_dyn_start[0] __attribute__((section(".__rel_dyn_start")));
>>  char __rel_dyn_end[0] __attribute__((section(".__rel_dyn_end")));
>>  char __secure_start[0] __attribute__((section(".__secure_start")));
>>  char __secure_end[0] __attribute__((section(".__secure_end")));
>> +char __efi_runtime_start[0] __attribute__((section(".__efi_runtime_start")));
>> +char __efi_runtime_stop[0] __attribute__((section(".__efi_runtime_stop")));
>>  char _end[0] __attribute__((section(".__end")));
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index 7fb2106..af1c88f 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -39,6 +39,7 @@
>>  
>>  #define EFI_EXIT(ret) efi_exit_func(ret);
>>  
>> +extern const struct efi_runtime_services efi_runtime_services;
>>  extern struct efi_system_table systab;
>>  
>>  extern const struct efi_simple_text_output_protocol efi_con_out;
>> @@ -49,6 +50,8 @@ extern const efi_guid_t efi_guid_console_control;
>>  extern const efi_guid_t efi_guid_device_path;
>>  extern const efi_guid_t efi_guid_loaded_image;
>>  
>> +extern unsigned int __efi_runtime_start, __efi_runtime_stop;
>> +
>>  struct efi_class_map {
>>  	const efi_guid_t *guid;
>>  	const void *interface;
>> diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c
>> new file mode 100644
>> index 0000000..214e1f5
>> --- /dev/null
>> +++ b/lib/efi_loader/efi_runtime.c
>> @@ -0,0 +1,59 @@
>> +/*
>> + *  EFI application runtime services
>> + *
>> + *  Copyright (c) 2015 Alexander Graf
>> + *
>> + *  This library is free software; you can redistribute it and/or
>> + *  modify it under the terms of the GNU Lesser General Public
>> + *  License as published by the Free Software Foundation; either
>> + *  version 2.1 of the License, or (at your option) any later version.
>> + *
>> + *  This library is distributed in the hope that it will be useful,
>> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>> + *  Lesser General Public License for more details.
>> + *
>> + *  You should have received a copy of the GNU Lesser General Public
>> + *  License along with this library; if not, write to the Free Software
>> + *  Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
>> + *
>> + *  SPDX-License-Identifier:     LGPL-2.1+
>> + */
>> +
>> +#include <common.h>
>> +#include <efi_loader.h>
>> +
>> +/*
>> + * EFI Runtime code is still alive when U-Boot is long overwritten. To isolate
>> + * this code from the rest, we put it into a special section.
>> + *
>> + *        !!WARNING!!
>> + *
>> + * This means that we can not rely on any code outside of this file at runtime.
>> + * Please keep it fully self-contained.
>> + */
>> +asm(".section efi_runtime,\"a\"");
>> +
>> +static efi_status_t efi_unimplemented(void)
>> +{
>> +	return EFI_UNSUPPORTED;
> 
> Again, EFI_UNSUPPORTED is not necessarily a valid return value for all
> runtime services.
> 
>> +}
>> +
>> +const struct efi_runtime_services efi_runtime_services = {
>> +	.hdr = {
>> +		.signature = EFI_RUNTIME_SERVICES_SIGNATURE,
>> +		.revision = EFI_RUNTIME_SERVICES_REVISION,
>> +		.headersize = sizeof(struct efi_table_hdr),
>> +	},
>> +	.get_time = (void *)&efi_unimplemented,
> 
> EFI_DEVICE_ERROR
> 
>> +	.set_time = (void *)&efi_unimplemented,
> 
> EFI_DEVICE_ERROR
> 
>> +	.get_wakeup_time = (void *)&efi_unimplemented,
>> +	.set_wakeup_time = (void *)&efi_unimplemented,
> 
> Both of these are fine, and correct, to return EFI_UNSUPPORTED.
> 
>> +	.set_virtual_address_map = (void *)&efi_unimplemented,
>> +	.convert_pointer = (void *)&efi_unimplemented,
> 
> There really isn't a way to gracefully decline these two functions.
> All valid error codes refer to invalid inputs.

Ok, changing to EFI_INVALID_PARAMETER then :).

> 
>> +	.get_variable = (void *)&efi_unimplemented,
> 
> EFI_DEVICE_ERROR would probably be the closest thing to a correct
> return code in this instance.
> 
>> +	.get_next_variable = (void *)&efi_unimplemented,
> 
> (get_next_variable_name?)

git blame says it's Simon's fault :).

867a6ac8 (Simon Glass    2015-07-31 09:31:36 -0600 170)
efi_status_t (EFIAPI *get_next_variable)(
867a6ac8 (Simon Glass    2015-07-31 09:31:36 -0600 171)
        unsigned long *variable_name_size,
867a6ac8 (Simon Glass    2015-07-31 09:31:36 -0600 172)
        s16 *variable_name, efi_guid_t *vendor);

> Again, EFI_DEVICE_ERROR, is probably the least wrong return value.
> 
>> +	.set_variable = (void *)&efi_unimplemented,
> 
> EFI_DEVICE_ERROR
> 
>> +	.get_next_high_mono_count = (void *)&efi_unimplemented,
> 
> EFI_DEVICE_ERROR
> 

Ok, fixed all of the above.

>> +	.reset_system = (void *)&efi_unimplemented,
> 
> "The ResetSystem() function does not return."

Hrm, I think returning EFI_UNSUPPORTED is still better than while(1) {
}. With the return an OS at least has the chance to fix things up itself.


Alex