[U-Boot] [PATCH 10/11] efi_loader: Add mem-mapped for fallback

[Rob Clark]

On Wed, Oct 11, 2017 at 10:59 AM, Alexander Graf <agraf at suse.de> wrote:
>
>
> On 10.10.17 14:23, Rob Clark wrote:
>> When we don't have a real device/image path, such as 'bootefi hello',
>> construct a mem-mapped device-path.
>>
>> This fixes 'bootefi hello' after devicepath refactoring.
>>
>> Fixes: 95c5553ea2 ("efi_loader: refactor boot device and loaded_image handling")
>> Signed-off-by: Rob Clark <robdclark at gmail.com>
>> ---
>>  cmd/bootefi.c                            | 23 +++++++++++++++++++++++
>>  include/efi_api.h                        |  8 ++++++++
>>  include/efi_loader.h                     |  3 +++
>>  lib/efi_loader/efi_device_path.c         | 24 ++++++++++++++++++++++++
>>  lib/efi_loader/efi_device_path_to_text.c |  9 +++++++++
>>  5 files changed, 67 insertions(+)
>>
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index 24958ada46..18176a1266 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -128,6 +128,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt,
>>  {
>>       struct efi_loaded_image loaded_image_info = {};
>>       struct efi_object loaded_image_info_obj = {};
>> +     struct efi_device_path *memdp = NULL;
>>       ulong ret;
>>
>>       ulong (*entry)(void *image_handle, struct efi_system_table *st)
>> @@ -136,6 +137,20 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt,
>>       const efi_guid_t fdt_guid = EFI_FDT_GUID;
>>       bootm_headers_t img = { 0 };
>>
>> +     /*
>> +      * Special case for efi payload not loaded from disk, such as
>> +      * 'bootefi hello' or for example payload loaded directly into
>> +      * memory via jtag/etc:
>> +      */
>> +     if (!device_path && !image_path) {
>> +             printf("WARNING: using memory device/image path, this may confuse some payloads!\n");
>> +             /* actual addresses filled in after efi_load_pe() */
>> +             memdp = efi_dp_from_mem(0, 0, 0);
>> +             device_path = image_path = memdp;
>> +     } else {
>> +             assert(device_path && image_path);
>> +     }
>> +
>>       /* Initialize and populate EFI object list */
>>       if (!efi_obj_list_initalized)
>>               efi_init_obj_list();
>> @@ -182,6 +197,14 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt,
>>               goto exit;
>>       }
>>
>> +     if (memdp) {
>> +             struct efi_device_path_memory *mdp = (void *)memdp;
>> +             mdp->memory_type = loaded_image_info.image_code_type;
>> +             mdp->start_address = (uintptr_t)loaded_image_info.image_base;
>> +             mdp->end_address = mdp->start_address +
>> +                             loaded_image_info.image_size;
>> +     }
>> +
>
> memdp gets leaked after bootefi is done. Putting it on the stack would
> at least remove that problem ;). We currently expect to only return from
> bootefi when a payload was successfully quit.
>

dp's that aren't allocated from pool are a bad idea, in some cases
they get free'd by the payload.  (Well not really in this particular
case but it feels like a bad idea to mix/match how we allocate dp's..
also, it needs an /End node.)  I guess it isn't such a critical leak,
but the right solution would be to efi_free_pool() it..

BR,
-R