[PATCH v4 3/7] efi_loader: Update efi_run_image() to accept image and device path

[Heinrich Schuchardt]

On 1/9/25 16:02, Simon Glass 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 at chromium.org>
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.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 428991df88f..e3ac27b03fa 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)

Please, use parameter names that make it clear that we are dealing with
device paths, e.g. dp_dev and dp_img.

>   {
>   	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)

This is an unrelated change.
What is the point of adding a _ptr suffix here?
We are only dealing with pointers in the EFI sub-system and keep sandbox
virtual addresses out.

Best regards

Heinrich

>   {
>   	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);
>   }