[U-Boot] [U-Boot,6/8] efi_loader: Pass file path to payload

[Alexander Graf]

On 08/13/2018 09:21 PM, Heinrich Schuchardt wrote:
> On 04/11/2016 04:16 PM, Alexander Graf wrote:
>> The payload gets information on where it got loaded from. This includes
>> the device as well as file path.
>>
>> So far we've treated both as the same thing and always gave it the device
>> name. However, in some situations grub2 actually wants to find its loading
>> path to find its configuration file.
>>
>> So let's split the two semantically separte bits into separate structs and
>> pass the loaded file name into our payload when we load it using "load".
>>
>> Signed-off-by: Alexander Graf <agraf at suse.de>
>> ---
>>   cmd/bootefi.c        | 32 +++++++++++++++++++++++++-------
>>   cmd/fs.c             |  3 ++-
>>   include/efi_loader.h |  5 +++--
>>   3 files changed, 30 insertions(+), 10 deletions(-)
>>
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index 0d09aa1..adcf645 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -34,17 +34,30 @@ static struct efi_device_path_file_path bootefi_image_path[] = {
>>   	}
>>   };
>>   
>> +static struct efi_device_path_file_path bootefi_device_path[] = {
>> +	{
>> +		.dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE,
>> +		.dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH,
>> +		.dp.length = sizeof(bootefi_image_path[0]),
>> +		.str = { 'b','o','o','t','e','f','i' },
>> +	}, {
>> +		.dp.type = DEVICE_PATH_TYPE_END,
>> +		.dp.sub_type = DEVICE_PATH_SUB_TYPE_END,
>> +		.dp.length = sizeof(bootefi_image_path[0]),
>> +	}
>> +};
>> +
>>   static efi_status_t bootefi_open_dp(void *handle, efi_guid_t *protocol,
>>   			void **protocol_interface, void *agent_handle,
>>   			void *controller_handle, uint32_t attributes)
>>   {
>> -	*protocol_interface = bootefi_image_path;
>> +	*protocol_interface = bootefi_device_path;
>>   	return EFI_SUCCESS;
>>   }
>>   
>>   /* The EFI loaded_image interface for the image executed via "bootefi" */
>>   static struct efi_loaded_image loaded_image_info = {
>> -	.device_handle = bootefi_image_path,
>> +	.device_handle = bootefi_device_path,
>>   	.file_path = bootefi_image_path,
>>   };
>>   
>> @@ -63,7 +76,7 @@ static struct efi_object loaded_image_info_obj = {
>>   		{
>>   			/*
>>   			 * When asking for the device path interface, return
>> -			 * bootefi_image_path
>> +			 * bootefi_device_path
>>   			 */
>>   			.guid = &efi_guid_device_path,
>>   			.open = &bootefi_open_dp,
>> @@ -73,11 +86,11 @@ static struct efi_object loaded_image_info_obj = {
>>   
>>   /* The EFI object struct for the device the "bootefi" image was loaded from */
>>   static struct efi_object bootefi_device_obj = {
>> -	.handle = bootefi_image_path,
>> +	.handle = bootefi_device_path,
>>   	.protocols = {
>>   		{
>>   			/* When asking for the device path interface, return
>> -			 * bootefi_image_path */
>> +			 * bootefi_device_path */
>>   			.guid = &efi_guid_device_path,
>>   			.open = &bootefi_open_dp,
>>   		}
>> @@ -192,7 +205,7 @@ U_BOOT_CMD(
>>   	bootefi_help_text
>>   );
>>   
>> -void efi_set_bootdev(const char *dev, const char *devnr)
>> +void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
>>   {
>>   	__maybe_unused struct blk_desc *desc;
>>   	char devname[16] = { 0 }; /* dp->str is u16[16] long */
>> @@ -217,7 +230,12 @@ void efi_set_bootdev(const char *dev, const char *devnr)
>>   	if (colon)
>>   		*colon = '\0';
>>   
>> -	/* Patch the bootefi_image_path to the target device */
>> +	/* Patch bootefi_device_path to the target device */
>> +	memset(bootefi_device_path[0].str, 0, sizeof(bootefi_device_path[0].str));
>> +	ascii2unicode(bootefi_device_path[0].str, devname);
>> +
>> +	/* Patch bootefi_image_path to the target file path */
>>   	memset(bootefi_image_path[0].str, 0, sizeof(bootefi_image_path[0].str));
>> +	snprintf(devname, sizeof(devname), "%s", path);
>>   	ascii2unicode(bootefi_image_path[0].str, devname);
>>   }
>> diff --git a/cmd/fs.c b/cmd/fs.c
>> index be8f289..abfe5be 100644
>> --- a/cmd/fs.c
>> +++ b/cmd/fs.c
>> @@ -27,7 +27,8 @@ U_BOOT_CMD(
>>   static int do_load_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
>>   				char * const argv[])
>>   {
>> -	efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "");
>> +	efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
>> +			(argc > 4) ? argv[4] : "");
> Hi Alex,
>
> efi_set_bootdev is used here to set both the device from which a file
> was loaded and the filename used as image_path.
>
> Why should we assume that the last file loaded is the EFI executable and
> not the device tree (or any other file)?

The image path logic goes hand in hand with the distro boot logic which 
tries to always load the efi binary last. If you found a case where it 
doesn't, let's fix that up please.

> Shouldn't we at least check that this file is an EFI executable or driver?

Well, that would only be a marginal improvement, no? And in fact I'm not 
sure I'd be very happy to clutter the load code with such logic.

The "real" fix would be to create a LoadedImage object for every "load" 
call and then just search for the one that starts at the bootefi load 
address. But I'm also not sure people who don't want to run any efi 
payloads want to pay the memory penalty that incurs.


Alex