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

On Wed, Oct 11, 2017 at 10:59 AM, Alexander Graf agraf@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@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