Re: [U-Boot] [RFC 3/8] efi_loader: bootmgr: return pointer and size of buffer in loading

On Thu, Mar 21, 2019 at 12:41:06PM +0100, Heinrich Schuchardt wrote:

On 3/5/19 6:53 AM, AKASHI Takahiro wrote:

...

We need to know the size of image loaded so as to be able to use load_image() API at do_bootefi_exec() in a later patch.

So change the interface of efi_bootmgr_load().

Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org

include/efi_loader.h | 5 +++-- lib/efi_loader/efi_bootmgr.c | 41 +++++++++++++++++++++--------------- 2 files changed, 27 insertions(+), 19 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h index 47a51ddc9406..3f51116155ad 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -564,8 +564,9 @@ struct efi_load_option {

void efi_deserialize_load_option(struct efi_load_option *lo, u8 *data); unsigned long efi_serialize_load_option(struct efi_load_option *lo, u8 **data); -void *efi_bootmgr_load(struct efi_device_path **device_path,

•        struct efi_device_path **file_path);
  

+efi_status_t efi_bootmgr_load(struct efi_device_path **device_path,

• 	      struct efi_device_path **file_path,
  

• 	      void **image, efi_uintn_t *size);
  
  

#else /* CONFIG_IS_ENABLED(EFI_LOADER) */

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c index 1575c5c09e46..38f3fa15f6ef 100644 --- a/lib/efi_loader/efi_bootmgr.c +++ b/lib/efi_loader/efi_bootmgr.c @@ -120,14 +120,17 @@ static void *get_var(u16 *name, const efi_guid_t *vendor,

• if successful. This checks that the EFI_LOAD_OPTION is active (enabled)
• and that the specified file to boot exists.

*/ -static void *try_load_entry(uint16_t n, struct efi_device_path **device_path,

• 	    struct efi_device_path **file_path)
  

+static efi_status_t try_load_entry(u16 n,

• 		   struct efi_device_path **device_path,
  

• 		   struct efi_device_path **file_path,
  

• 		   void **image, efi_uintn_t *image_size)
  

{ struct efi_load_option lo; u16 varname[] = L"Boot0000"; u16 hexmap[] = L"0123456789ABCDEF";

• void *load_option, *image = NULL;

• void *load_option; efi_uintn_t size;

• efi_status_t ret;

  varname[4] = hexmap[(n & 0xf000) >> 12]; varname[5] = hexmap[(n & 0x0f00) >> 8];

@@ -136,18 +139,17 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path,

load_option = get_var(varname, &efi_global_variable_guid, &size); if (!load_option)

• return NULL;
  

• return EFI_LOAD_ERROR;
  
  

  efi_deserialize_load_option(&lo, load_option);

  if (lo.attributes & LOAD_OPTION_ACTIVE) { u32 attributes;

• efi_status_t ret;
  
  

  debug("%s: trying to load "%ls" from %pD\n", __func__, lo.label, lo.file_path);

• ret = efi_load_image_from_path(lo.file_path, &image, &size);
  

• ret = efi_load_image_from_path(lo.file_path, image, image_size);
  
  

This call to efi_load_image_from_path() leads to duplication of logic.

Why don't we simply call EFI_CALL(efi_load_image())) here and if it succeeds return from efi_bootmgr_load()?

Make sense. It will make do_bootefi_exec() simpler. This will also give us a reason why we have do_efibootmgr() apart from do_boot_efi().

Thanks, -Takahiro Akashi

Best regards

Heinrich

...

if (ret != EFI_SUCCESS)
	goto error;

@@ -164,12 +166,14 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path,

printf("Booting: %ls\n", lo.label);
efi_dp_split_file_path(lo.file_path, device_path, file_path);

• } else {

• ret = EFI_LOAD_ERROR;
  

  }

error: free(load_option);

• return image;

• return ret;

}

/* @@ -177,12 +181,12 @@ error:

• EFI variable, the available load-options, finding and returning
• the first one that can be loaded successfully.

*/ -void *efi_bootmgr_load(struct efi_device_path **device_path,

•        struct efi_device_path **file_path)
  

+efi_status_t efi_bootmgr_load(struct efi_device_path **device_path,

• 	      struct efi_device_path **file_path,
  

• 	      void **image, efi_uintn_t *image_size)
  

{ u16 bootnext, *bootorder; efi_uintn_t size;

• void *image = NULL; int i, num; efi_status_t ret;

@@ -201,9 +205,9 @@ void *efi_bootmgr_load(struct efi_device_path **device_path, (efi_guid_t *)&efi_global_variable_guid, 0, 0, &bootnext)); if (ret == EFI_SUCCESS) {

• 	image = try_load_entry(bootnext,
  

• 			       device_path, file_path);
  

• 	if (image)
  

• 	ret = try_load_entry(bootnext, device_path, file_path,
  

• 			     image, image_size);
  

• 	if (ret == EFI_SUCCESS)
  	goto error;
  

  } }

@@ -212,19 +216,22 @@ void *efi_bootmgr_load(struct efi_device_path **device_path, bootorder = get_var(L"BootOrder", &efi_global_variable_guid, &size); if (!bootorder) { printf("BootOrder not defined\n");

• ret = EFI_NOT_FOUND;
  

  goto error; }

  num = size / sizeof(uint16_t); for (i = 0; i < num; i++) {

• debug("%s: trying to load Boot%04X\n", __func__, bootorder[i]);
  

• image = try_load_entry(bootorder[i], device_path, file_path);
  

• if (image)
  

• debug("%s: trying to load Boot%04X\n", __func__,
  

•       bootorder[i]);
  

• ret = try_load_entry(bootorder[i], device_path, file_path,
  

• 		     image, image_size);
  

• if (ret == EFI_SUCCESS)
  break;
  

  }

  free(bootorder);

error:

• return image;

• return ret;

}