On Sat, 21 Dec 2024 at 10:21, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

Hi Simon, I don't see any of the requested changes from v2 on this one.

Ah you changed it to static instead. That's fine we can move it to the dp helpers. if someone else needs it.

Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org

Cheers /Ilias

On Thu, 19 Dec 2024 at 04:39, Simon Glass sjg@chromium.org wrote:

...

Move this code into a function so it can be called from elsewhere.

Signed-off-by: Simon Glass sjg@chromium.org

Changes in v3:

• Make calculate_paths() static and add a comment

lib/efi_loader/efi_bootbin.c | 85 ++++++++++++++++++++++++------------ 1 file changed, 57 insertions(+), 28 deletions(-)

diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index b677bbc3124..5016ba7e225 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -44,12 +44,64 @@ void efi_clear_bootdev(void) image_size = 0; }

+/**

• • calculate_paths() - Calculate the device and image patch given a device
• • @dev: device, e.g. "MMC"
• • @devnr: number of the device, e.g. "1:2"
• • @path: path to file loaded
• • @device_pathp: returns EFI device path
• • @image_pathp: returns EFI image path
• • Return EFI_SUCCESS on success, else error code
• */

+static efi_status_t calculate_paths(const char *dev, const char *devnr,

•                               const char *path,
  

•                               struct efi_device_path **device_pathp,
  

•                               struct efi_device_path **image_pathp)
  

+{

•   struct efi_device_path *image, *device;
  

•   efi_status_t ret;
  

+#if IS_ENABLED(CONFIG_NETDEVICES)

•   if (!strcmp(dev, "Net") || !strcmp(dev, "Http")) {
  

•           ret = efi_net_set_dp(dev, devnr);
  

•           if (ret != EFI_SUCCESS)
  

•                   return ret;
  

•   }
  

+#endif

•   ret = efi_dp_from_name(dev, devnr, path, &device, &image);
  

•   if (ret != EFI_SUCCESS)
  

•           return ret;
  

•   *device_pathp = device;
  

•   if (image) {
  

•           /* FIXME: image should not contain device */
  

•           struct efi_device_path *image_tmp = image;
  

•           efi_dp_split_file_path(image, &device, &image);
  

•           efi_free_pool(image_tmp);
  

•   }
  

•   *image_pathp = image;
  

•   log_debug("- boot device %pD\n", device);
  

•   if (image)
  

•           log_debug("- image %pD\n", image);
  

•   return EFI_SUCCESS;
  

+}

/**

• efi_set_bootdev() - set boot device
• This function is called when a file is loaded, e.g. via the 'load' command.
• We use the path to this file to inform the UEFI binary about the boot device.

• • For a valid image, it sets:
• • • image_addr to the provided buffer
• • • image_size to the provided buffer_size
• • • bootefi_device_path to the EFI device-path
• • • bootefi_image_path to the EFI image-path
• • @dev: device, e.g. "MMC"
  • @devnr: number of the device, e.g. "1:2"
  • @path: path to file loaded

@@ -59,7 +111,6 @@ void efi_clear_bootdev(void) void efi_set_bootdev(const char *dev, const char *devnr, const char *path, void *buffer, size_t buffer_size) {

•   struct efi_device_path *device, *image;
    efi_status_t ret;
  
    log_debug("dev=%s, devnr=%s, path=%s, buffer=%p, size=%zx\n", dev,
  

@@ -93,34 +144,12 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path, image_addr = buffer; image_size = buffer_size;

-#if IS_ENABLED(CONFIG_NETDEVICES)

•   if (!strcmp(dev, "Net") || !strcmp(dev, "Http")) {
  

•           ret = efi_net_set_dp(dev, devnr);
  

•           if (ret != EFI_SUCCESS)
  

•                   goto error;
  

•   }
  

-#endif

•   ret = efi_dp_from_name(dev, devnr, path, &device, &image);
  

•   if (ret != EFI_SUCCESS)
  

•           goto error;
  

•   bootefi_device_path = device;
  

•   if (image) {
  

•           /* FIXME: image should not contain device */
  

•           struct efi_device_path *image_tmp = image;
  

•           efi_dp_split_file_path(image, &device, &image);
  

•           efi_free_pool(image_tmp);
  

•   ret = calculate_paths(dev, devnr, path, &bootefi_device_path,
  

•                         &bootefi_image_path);
  

•   if (ret) {
  

•           log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
  

•           efi_clear_bootdev();
    }
  

•   bootefi_image_path = image;
  

•   log_debug("- boot device %pD\n", device);
  

•   if (image)
  

•           log_debug("- image %pD\n", image);
  

•   return;
  

-error:

•   log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
  

•   efi_clear_bootdev();
  

}

/**

2.34.1

Hi Heinrich,

On Sun, 22 Dec 2024 at 05:35, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

On 12/19/24 03:38, Simon Glass wrote:

...

Move this code into a function so it can be called from elsewhere.

Signed-off-by: Simon Glass sjg@chromium.org

Changes in v3:

• Make calculate_paths() static and add a comment

  lib/efi_loader/efi_bootbin.c | 85 ++++++++++++++++++++++++------------ 1 file changed, 57 insertions(+), 28 deletions(-)

diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index b677bbc3124..5016ba7e225 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -44,12 +44,64 @@ void efi_clear_bootdev(void) image_size = 0; }

+/**

• • calculate_paths() - Calculate the device and image patch given a device

%s/patch/path/

The description makes no sense. You cannot derive an image device path from a device.

I'll change it to 'strings'

...

• • @dev: device, e.g. "MMC"
• • @devnr: number of the device, e.g. "1:2"
• • @path: path to file loaded
• • @device_pathp: returns EFI device path
• • @image_pathp: returns EFI image path
• • Return EFI_SUCCESS on success, else error code

This would not work with Sphinx

%s/Return/Return:/

...

• */

+static efi_status_t calculate_paths(const char *dev, const char *devnr,

•                             const char *path,
  

•                             struct efi_device_path **device_pathp,
  

•                             struct efi_device_path **image_pathp)
  

+{

• struct efi_device_path *image, *device;
  

• efi_status_t ret;
  

+#if IS_ENABLED(CONFIG_NETDEVICES)

Please, avoid #if. The include needs to be fixed.

...

• if (!strcmp(dev, "Net") || !strcmp(dev, "Http")) {
  

•         ret = efi_net_set_dp(dev, devnr);
  

•         if (ret != EFI_SUCCESS)
  

•                 return ret;
  

• }
  

This does not fit to the function description. It has nothing to do with calculating device-paths.

That code was added just recently, reviewed by you, so I will let you clean it up. It is perpetuating the globals that this series is (was) resolving.

Anyway, the naming is correct, once that is fixed.

...

+#endif

• ret = efi_dp_from_name(dev, devnr, path, &device, &image);
  

• if (ret != EFI_SUCCESS)
  

•         return ret;
  

• *device_pathp = device;
  

• if (image) {
  

•         /* FIXME: image should not contain device */
  

We could change efi_dp_from_name() but then we would have to concatenate paths in create_lo_dp_part().

This does not deserve a FIXME label.

This is copied from the existing code. Please check the patch before requesting unrelated changes.

Best regards

Heinrich

...

•         struct efi_device_path *image_tmp = image;
  

•         efi_dp_split_file_path(image, &device, &image);
  

•         efi_free_pool(image_tmp);
  

• }
  

• *image_pathp = image;
  

• log_debug("- boot device %pD\n", device);
  

• if (image)
  

•         log_debug("- image %pD\n", image);
  

• return EFI_SUCCESS;
  

+}

• /**
  • efi_set_bootdev() - set boot device
  • This function is called when a file is loaded, e.g. via the 'load' command.
  • We use the path to this file to inform the UEFI binary about the boot device.
• • For a valid image, it sets:
• • • image_addr to the provided buffer
• • • image_size to the provided buffer_size
• • • bootefi_device_path to the EFI device-path
• • • bootefi_image_path to the EFI image-path
• • @dev: device, e.g. "MMC"
  • @devnr: number of the device, e.g. "1:2"
  • @path: path to file loaded

@@ -59,7 +111,6 @@ void efi_clear_bootdev(void) void efi_set_bootdev(const char *dev, const char *devnr, const char *path, void *buffer, size_t buffer_size) {

• struct efi_device_path *device, *image;
  efi_status_t ret;
  
  log_debug("dev=%s, devnr=%s, path=%s, buffer=%p, size=%zx\n", dev,
  

@@ -93,34 +144,12 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path, image_addr = buffer; image_size = buffer_size;

-#if IS_ENABLED(CONFIG_NETDEVICES)

• if (!strcmp(dev, "Net") || !strcmp(dev, "Http")) {
  

•         ret = efi_net_set_dp(dev, devnr);
  

•         if (ret != EFI_SUCCESS)
  

•                 goto error;
  

• }
  

-#endif

• ret = efi_dp_from_name(dev, devnr, path, &device, &image);
  

• if (ret != EFI_SUCCESS)
  

•         goto error;
  

• bootefi_device_path = device;
  

• if (image) {
  

•         /* FIXME: image should not contain device */
  

•         struct efi_device_path *image_tmp = image;
  

•         efi_dp_split_file_path(image, &device, &image);
  

•         efi_free_pool(image_tmp);
  

• ret = calculate_paths(dev, devnr, path, &bootefi_device_path,
  

•                       &bootefi_image_path);
  

• if (ret) {
  

•         log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
  

•         efi_clear_bootdev();
  }
  

• bootefi_image_path = image;
  

• log_debug("- boot device %pD\n", device);
  

• if (image)
  

•         log_debug("- image %pD\n", image);
  

• return;
  

-error:

• log_debug("- efi_dp_from_name() failed, err=%lx\n", ret);
  

• efi_clear_bootdev();
  

  }

  /**

Regards, Simon

On 12/19/24 03:38, Simon Glass wrote:

This function is not called from outside this file and has no entry in the header file, so mark it static.

Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org

Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de

---

(no changes since v1)

lib/efi_loader/efi_bootbin.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index 5016ba7e225..051d86ef993 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -159,7 +159,7 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path,

• @source_size: size of the UEFI image
• Return: status code

*/ -efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) +static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) { efi_handle_t mem_handle = NULL, handle; struct efi_device_path *file_path = NULL;

On Thu, 19 Dec 2024 at 04:39, Simon Glass sjg@chromium.org wrote:

Provide these globals as parameters to this function, on the way to making it possible to start an image without relying on the globals.

Signed-off-by: Simon Glass sjg@chromium.org

(no changes since v2)

Changes in v2:

• Adjust argument ordering for efi_run_image()

lib/efi_loader/efi_bootbin.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index 051d86ef993..b42ff3cb4e0 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -157,9 +157,13 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path,

• @source_buffer: memory address of the UEFI image
• @source_size: size of the UEFI image

• • @device: EFI device-path
• • @image: EFI image-path
  • Return: status code
  */

-static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) +static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size,

•                             struct efi_device_path *device,
  

•                             struct efi_device_path *image)
  

{ efi_handle_t mem_handle = NULL, handle; struct efi_device_path *file_path = NULL; @@ -167,7 +171,7 @@ static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) efi_status_t ret; u16 *load_options;

•   if (!bootefi_device_path || !bootefi_image_path) {
  

•   if (!device || !image) {
            log_debug("Not loaded from disk\n");
            /*
             * Special case for efi payload not loaded from disk,
  

@@ -188,9 +192,8 @@ static efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size) goto out; msg_path = file_path; } else {

•           file_path = efi_dp_concat(bootefi_device_path,
  

•                                     bootefi_image_path, 0);
  

•           msg_path = bootefi_image_path;
  

•           file_path = efi_dp_concat(device, image, 0);
  

•           msg_path = image;
            log_debug("Loaded from disk\n");
    }
  
  

@@ -227,7 +230,7 @@ out: /**

• efi_binary_run() - run loaded UEFI image

• • @image: memory address of the UEFI image

• • @image_ptr: memory address of the UEFI image
  • @size: size of the UEFI image
  • @fdt: device-tree

@@ -236,7 +239,7 @@ out:

• Return: status code

*/ -efi_status_t efi_binary_run(void *image, size_t size, void *fdt) +efi_status_t efi_binary_run(void *image_ptr, size_t size, void *fdt) { efi_status_t ret;

@@ -252,5 +255,6 @@ efi_status_t efi_binary_run(void *image, size_t size, void *fdt) if (ret != EFI_SUCCESS) return ret;

•   return efi_run_image(image, size);
  

•   return efi_run_image(image_ptr, size, bootefi_device_path,
  

•                        bootefi_image_path);
  

}

https://lore.kernel.org/u-boot/CAC_iWj+5-uoXq9BanaOSxedVgbBYNFQ2AZVqka+J7iwM...

I thought you said the _ptr was dropped.

Anyway Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org

We can drop it later or in v3 if one is needed

Thanks /Ilias

-- 2.34.1

On 12/19/24 03:38, Simon Glass wrote:

This uses a few global variables at present. With the bootflow we have the required parameters, so add a function which accepts these. Update the existing function to call the new one with the globals.

• Rename efi_binary_run_() to _efi_binary_run()

Signed-off-by: Simon Glass sjg@chromium.org

(no changes since v1)

lib/efi_loader/efi_bootbin.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index b42ff3cb4e0..f7aedeaab8a 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -228,18 +228,22 @@ out: }

/**

• • efi_binary_run() - run loaded UEFI image

• • efi_binary_run_() - run loaded UEFI image
  • @image_ptr: memory address of the UEFI image
  • @size: size of the UEFI image
  • @fdt: device-tree
• • @device: EFI device-path
• • @image: EFI image-path
  • Execute an EFI binary image loaded at @image.
  • @size may be zero if the binary is loaded with U-Boot load command.
  • Return: status code
  */

-efi_status_t efi_binary_run(void *image_ptr, size_t size, void *fdt) +efi_status_t _efi_binary_run(void *image_ptr, size_t size, void *fdt,

• 	     struct efi_device_path *device,
  

• 	     struct efi_device_path *image)
  

A non-static function starting with underscore? Please, avoid this.

Best regards

Heinrich

{ efi_status_t ret;

@@ -255,6 +259,23 @@ efi_status_t efi_binary_run(void *image_ptr, size_t size, void *fdt) if (ret != EFI_SUCCESS) return ret;

• return efi_run_image(image_ptr, size, bootefi_device_path,

• 	     bootefi_image_path);
  

• return efi_run_image(image_ptr, size, device, image);

+}

+/**

• • efi_binary_run() - run loaded UEFI image
• • @image: memory address of the UEFI image
• • @size: size of the UEFI image
• • @fdt: device-tree
• • Execute an EFI binary image loaded at @image.
• • @size may be zero if the binary is loaded with U-Boot load command.
• • Return: status code
• */

+efi_status_t efi_binary_run(void *image, size_t size, void *fdt) +{

• return _efi_binary_run(image, size, fdt, bootefi_device_path,

• 	       bootefi_image_path);
  

  }

Hi guys,

On Sun, 22 Dec 2024 at 06:17, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

Hi Heinrich

On Sun, 22 Dec 2024 at 14:45, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 12/19/24 03:38, Simon Glass wrote:

...

This uses a few global variables at present. With the bootflow we have the required parameters, so add a function which accepts these. Update the existing function to call the new one with the globals.

• Rename efi_binary_run_() to _efi_binary_run()

Signed-off-by: Simon Glass sjg@chromium.org

(no changes since v1)

lib/efi_loader/efi_bootbin.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index b42ff3cb4e0..f7aedeaab8a 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -228,18 +228,22 @@ out: }

/**

• • efi_binary_run() - run loaded UEFI image

• • efi_binary_run_() - run loaded UEFI image
  • @image_ptr: memory address of the UEFI image
  • @size: size of the UEFI image
  • @fdt: device-tree
• • @device: EFI device-path
• • @image: EFI image-path
  • Execute an EFI binary image loaded at @image.
  • @size may be zero if the binary is loaded with U-Boot load command.
  • Return: status code
  */

-efi_status_t efi_binary_run(void *image_ptr, size_t size, void *fdt) +efi_status_t _efi_binary_run(void *image_ptr, size_t size, void *fdt,

•                      struct efi_device_path *device,
  

•                      struct efi_device_path *image)
  

A non-static function starting with underscore? Please, avoid this.

I was the one who requested this, the v1 had it as efi_binary_run_, but i a dont mind any of those

Well I'm not changing it again :-) Heinrich you are free to send a patch later.

Regards, Simon

Hi Mark,

On Thu, 9 Jan 2025 at 08:31, Mark Kettenis mark.kettenis@xs4all.nl wrote:

...

From: Simon Glass sjg@chromium.org Date: Thu, 9 Jan 2025 08:01:34 -0700

Hi guys,

On Sun, 22 Dec 2024 at 06:17, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

...

Hi Heinrich

On Sun, 22 Dec 2024 at 14:45, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

...

On 12/19/24 03:38, Simon Glass wrote:

...

This uses a few global variables at present. With the bootflow we have the required parameters, so add a function which accepts these. Update the existing function to call the new one with the globals.

• Rename efi_binary_run_() to _efi_binary_run()

Signed-off-by: Simon Glass sjg@chromium.org

(no changes since v1)

lib/efi_loader/efi_bootbin.c | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-)

diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index b42ff3cb4e0..f7aedeaab8a 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -228,18 +228,22 @@ out: }

/**

• • efi_binary_run() - run loaded UEFI image

• • efi_binary_run_() - run loaded UEFI image
  • @image_ptr: memory address of the UEFI image
  • @size: size of the UEFI image
  • @fdt: device-tree
• • @device: EFI device-path
• • @image: EFI image-path
  • Execute an EFI binary image loaded at @image.
  • @size may be zero if the binary is loaded with U-Boot load command.
  • Return: status code
  */

-efi_status_t efi_binary_run(void *image_ptr, size_t size, void *fdt) +efi_status_t _efi_binary_run(void *image_ptr, size_t size, void *fdt,

•                      struct efi_device_path *device,
  

•                      struct efi_device_path *image)
  

A non-static function starting with underscore? Please, avoid this.

I was the one who requested this, the v1 had it as efi_binary_run_, but i a dont mind any of those

Well I'm not changing it again :-) Heinrich you are free to send a patch later.

Well, you shouldn't commit a diff where the function starts with an underscore but the comment documenting the function ends with an underscore.

Oh dear...perhaps Heinrich can fix it up / decide when applying.

Regards, Simon

Hi Heinrich,

On Sun, 22 Dec 2024 at 05:55, Heinrich Schuchardt xypron.glpk@gmx.de wrote:

On 12/19/24 03:38, Simon Glass wrote:

...

Rather than setting up the global variables and then making the call, pass them into function directly. This cleans up the code and makes it all a bit easier to understand.

Signed-off-by: Simon Glass sjg@chromium.org

(no changes since v2)

Changes in v2:

• Use efi_loader tag instead of efi
• Drop unnecessary path removal
• Fix 'require' typo
• Move calculation of dev-name into a separate function

Please, split the patch into logical units.

What do you mean? Are you wanting me to have this patch and then four more after it to make the changes listed above? If so, why?

...

boot/bootmeth_efi.c | 49 +------------------------ include/efi_loader.h | 10 +++++ lib/efi_loader/efi_bootbin.c | 71 ++++++++++++++++++++++++++++++++++++ 3 files changed, 83 insertions(+), 47 deletions(-)

diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index f836aa655f5..fa7f66c9a0b 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -52,40 +52,6 @@ static bool bootmeth_uses_network(struct bootflow *bflow) device_get_uclass_id(media) == UCLASS_ETH; }

-static void set_efi_bootdev(struct blk_desc *desc, struct bootflow *bflow) -{

• const struct udevice *media_dev;
  

• int size = bflow->size;
  

• const char *dev_name;
  

• char devnum_str[9];
  

• char dirname[200];
  

• char *last_slash;
  

• /*
  

•  * This is a horrible hack to tell EFI about this boot device. Once we
  

•  * unify EFI with the rest of U-Boot we can clean this up. The same hack
  

•  * exists in multiple places, e.g. in the fs, tftp and load commands.
  

•  *
  

•  * Once we can clean up the EFI code to make proper use of driver model,
  

•  * this can go away.
  

•  */
  

• media_dev = dev_get_parent(bflow->dev);
  

• snprintf(devnum_str, sizeof(devnum_str), "%x:%x",
  

•          desc ? desc->devnum : dev_seq(media_dev),
  

•          bflow->part);
  

• strlcpy(dirname, bflow->fname, sizeof(dirname));
  

• last_slash = strrchr(dirname, '/');
  

• if (last_slash)
  

•         *last_slash = '\0';
  

• dev_name = device_get_uclass_id(media_dev) == UCLASS_MASS_STORAGE ?
  

•          "usb" : blk_get_uclass_name(device_get_uclass_id(media_dev));
  

• log_debug("setting bootdev %s, %s, %s, %p, %x\n",
  

•           dev_name, devnum_str, bflow->fname, bflow->buf, size);
  

• efi_set_bootdev(dev_name, devnum_str, bflow->fname, bflow->buf, size);
  

-}

• static int efiload_read_file(struct bootflow *bflow, ulong addr) { struct blk_desc *desc = NULL;

@@ -103,8 +69,6 @@ static int efiload_read_file(struct bootflow *bflow, ulong addr) return log_msg_ret("read", ret); bflow->buf = map_sysmem(addr, bflow->size);

• set_efi_bootdev(desc, bflow);
  

• return 0;
  

  }

@@ -342,17 +306,8 @@ static int distro_efi_boot(struct udevice *dev, struct bootflow *bflow) fdt = env_get_hex("fdt_addr_r", 0); }

• if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) {
  

•         log_debug("Booting with built-in fdt\n");
  

•         if (efi_binary_run(map_sysmem(kernel, 0), bflow->size,
  

•                            EFI_FDT_USE_INTERNAL))
  

•                 return log_msg_ret("run", -EINVAL);
  

• } else {
  

•         log_debug("Booting with external fdt\n");
  

•         if (efi_binary_run(map_sysmem(kernel, 0), bflow->size,
  

•                            map_sysmem(fdt, 0)))
  

•                 return log_msg_ret("run", -EINVAL);
  

• }
  

• if (efi_bootflow_run(bflow))
  

•         return log_msg_ret("run", -EINVAL);
  
  return 0;
  

  }

diff --git a/include/efi_loader.h b/include/efi_loader.h index 9afbec35ebf..a9325b1a9d0 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -20,6 +20,7 @@ #include <linux/oid_registry.h>

struct blk_desc; +struct bootflow; struct jmp_buf_data;

#if CONFIG_IS_ENABLED(EFI_LOADER) @@ -578,6 +579,15 @@ efi_status_t efi_install_fdt(void *fdt); efi_status_t do_bootefi_exec(efi_handle_t handle, void *load_options); /* Run loaded UEFI image with given fdt */ efi_status_t efi_binary_run(void *image, size_t size, void *fdt);

+/**

• • efi_bootflow_run() - Run a bootflow containing an EFI application
• • @bootflow: Bootflow to run
• • Return: Status code, something went wrong
• */

+efi_status_t efi_bootflow_run(struct bootflow *bootflow);

• /* Initialize variable services */ efi_status_t efi_init_variables(void); /* Notify ExitBootServices() is called */

diff --git a/lib/efi_loader/efi_bootbin.c b/lib/efi_loader/efi_bootbin.c index 8332993f421..b5d15a723b4 100644 --- a/lib/efi_loader/efi_bootbin.c +++ b/lib/efi_loader/efi_bootbin.c @@ -6,13 +6,16 @@

#define LOG_CATEGORY LOGC_EFI

+#include <bootflow.h> #include <charset.h> +#include <dm.h> #include <efi.h> #include <efi_loader.h> #include <env.h> #include <image.h> #include <log.h> #include <malloc.h> +#include <mapmem.h>

static struct efi_device_path *bootefi_image_path; static struct efi_device_path *bootefi_device_path; @@ -285,3 +288,71 @@ out: return ret; }

+/**

• • calc_dev_name() - Calculate the device name to give to EFI
• • If not supported, this shows an error.
• • Return name, or NULL if not supported
• */

+static const char *calc_dev_name(struct bootflow *bflow) +{

• const struct udevice *media_dev;
  

• media_dev = dev_get_parent(bflow->dev);
  

• if (!bflow->blk) {
  

•         log_err("Cannot boot EFI app on media '%s'\n",
  

•                 dev_get_uclass_name(media_dev));
  

•         return NULL;
  

• }
  

• if (device_get_uclass_id(media_dev) == UCLASS_MASS_STORAGE)
  

•         return "usb";
  

• return blk_get_uclass_name(device_get_uclass_id(media_dev));
  

+}

+efi_status_t efi_bootflow_run(struct bootflow *bflow) +{

• struct efi_device_path *device, *image;
  

• const struct udevice *media_dev;
  

• struct blk_desc *desc = NULL;
  

• const char *dev_name;
  

• char devnum_str[9];
  

• efi_status_t ret;
  

• void *fdt;
  

• media_dev = dev_get_parent(bflow->dev);
  

• if (bflow->blk) {
  

•         desc = dev_get_uclass_plat(bflow->blk);
  

Where do you handle network devices?

That's not supported with bootstd yet. See the last patch in this series[1]. Also [2]

...

•         snprintf(devnum_str, sizeof(devnum_str), "%x:%x",
  

•                  desc ? desc->devnum : dev_seq(media_dev), bflow->part);
  

• } else {
  

•         *devnum_str = '\0';
  

• }
  

• dev_name = calc_dev_name(bflow);
  

• log_debug("dev_name '%s' devnum_str '%s' fname '%s' media_dev '%s'\n",
  

•           dev_name, devnum_str, bflow->fname, media_dev->name);
  

• if (!dev_name)
  

•         return EFI_UNSUPPORTED;
  

• ret = calculate_paths(dev_name, devnum_str, bflow->fname, &device,
  

•                       &image);
  

• if (ret)
  

•         return ret;
  

• if (bflow->flags & BOOTFLOWF_USE_BUILTIN_FDT) {
  

•         log_debug("Booting with built-in fdt\n");
  

•         fdt = EFI_FDT_USE_INTERNAL;
  

• } else {
  

•         log_debug("Booting with external fdt\n");
  

•         fdt = map_sysmem(bflow->fdt_addr, 0);
  

• }
  

• ret = _efi_binary_run(bflow->buf, bflow->size, fdt, device, image);
  

• return ret;
  

+}

- Simon

[1] https://patchwork.ozlabs.org/project/uboot/patch/20241219023832.1098689-8-sj... [2] https://patchwork.ozlabs.org/project/uboot/list/?series=435516