Re: [PATCH 4/4] efi: add helper functions to insert pmem node for DT fixup

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

On 10/25/24 13:14, Sughosh Ganu wrote:
> The EFI HTTP boot puts the iso installer image at some location in
> memory which needs to be reserved in the devicetree as persistent
> memory (pmem). Add helper functions which add this pmem node when the
> EFI_DT_FIXUP protocol's fixup callback is invoked.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
> ---
>   boot/image-fdt.c             |  9 +++++++++
>   include/efi_loader.h         | 17 +++++++++++++++++
>   lib/efi_loader/efi_bootmgr.c | 21 +++++++++++++++++++++
>   lib/efi_loader/efi_helper.c  | 12 ++++++++++++
>   4 files changed, 59 insertions(+)
>
> diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> index 8eda521693..b39e81ad30 100644
> --- a/boot/image-fdt.c
> +++ b/boot/image-fdt.c
> @@ -11,6 +11,7 @@
>   #include <command.h>
>   #include <fdt_support.h>
>   #include <fdtdec.h>
> +#include <efi_loader.h>
>   #include <env.h>
>   #include <errno.h>
>   #include <image.h>
> @@ -648,6 +649,14 @@ int image_setup_libfdt(struct bootm_headers *images, void *blob, bool lmb)
>   	if (!ft_verify_fdt(blob))
>   		goto err;
>
> +	if (CONFIG_IS_ENABLED(EFI_HTTP_BOOT)) {
> +		fdt_ret = fdt_efi_pmem_setup(blob);

I can see no reason why pmem setup should depend on HTTP boot.

It should be possible to pass a memory block device to the kernel no
matter how it was created.

Best regards

Heinrich

> +		if (fdt_ret) {
> +			printf("ERROR: HTTP boot pmem fixup failed\n");
> +			goto err;
> +		}
> +	}
> +
>   	/* after here we are using a livetree */
>   	if (!of_live_active() && CONFIG_IS_ENABLED(EVENT)) {
>   		struct event_ft_fixup fixup;
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index d450e304c6..031de18746 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -748,6 +748,15 @@ bool efi_varname_is_load_option(u16 *var_name16, int *index);
>   efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf,
>   				    efi_guid_t *guid);
>
> +/**
> + * fdt_efi_pmem_setup() - Setup the pmem node in the devicetree
> + *
> + * @fdt:	Pointer to the devicetree
> + *
> + * Return:	0 on success, negative on failure
> + */
> +int fdt_efi_pmem_setup(void *fdt);
> +
>   /**
>    * efi_size_in_pages() - convert size in bytes to size in pages
>    *
> @@ -964,6 +973,14 @@ efi_status_t efi_set_load_options(efi_handle_t handle,
>   				  void *load_options);
>   efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options);
>
> +/**
> + * efi_bootmgr_pmem_setup() - Put a pmem node for UEFI HTTP installers
> + *
> + * @fdt:	Pointer to the DT blob
> + * Return:	status code
> + */
> +efi_status_t efi_bootmgr_pmem_setup(void *fdt);
> +
>   /**
>    * struct efi_image_regions - A list of memory regions
>    *
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 16f75555f6..1d9246be61 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -41,6 +41,8 @@ struct uridp_context {
>   	efi_handle_t mem_handle;
>   };
>
> +static struct uridp_context *uctx;
> +
>   const efi_guid_t efi_guid_bootmenu_auto_generated =
>   		EFICONFIG_AUTO_GENERATED_ENTRY_GUID;
>
> @@ -423,6 +425,7 @@ efi_status_t efi_bootmgr_release_uridp(struct uridp_context *ctx)
>
>   	efi_free_pool(ctx->loaded_dp);
>   	free(ctx);
> +	uctx = NULL;
>
>   	return ret == EFI_SUCCESS ? ret2 : ret;
>   }
> @@ -443,6 +446,23 @@ static void EFIAPI efi_bootmgr_http_return(struct efi_event *event,
>   	EFI_EXIT(ret);
>   }
>
> +/**
> + * efi_bootmgr_pmem_setup() - Put a pmem node for UEFI HTTP installers
> + *
> + * @fdt:	Pointer to the DT blob
> + * Return:	status code
> + */
> +efi_status_t efi_bootmgr_pmem_setup(void *fdt)
> +{
> +	if (!uctx) {
> +		log_warning("No EFI HTTP boot context found\n");
> +		return EFI_SUCCESS;
> +	}
> +
> +	return !fdt_fixup_pmem_region(fdt, uctx->image_addr, uctx->image_size) ?
> +		EFI_SUCCESS : EFI_INVALID_PARAMETER;
> +}
> +
>   /**
>    * try_load_from_uri_path() - Handle the URI device path
>    *
> @@ -472,6 +492,7 @@ static efi_status_t try_load_from_uri_path(struct efi_device_path_uri *uridp,
>   	if (!ctx)
>   		return EFI_OUT_OF_RESOURCES;
>
> +	uctx = ctx;
>   	s = env_get("loadaddr");
>   	if (!s) {
>   		log_err("Error: loadaddr is not set\n");
> diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
> index 00167bd2a1..33cd8b9a50 100644
> --- a/lib/efi_loader/efi_helper.c
> +++ b/lib/efi_loader/efi_helper.c
> @@ -242,6 +242,18 @@ int efi_unlink_dev(efi_handle_t handle)
>   	return 0;
>   }
>
> +/**
> + * fdt_efi_pmem_setup() - Setup the pmem node in the devicetree
> + *
> + * @fdt:	Pointer to the devicetree
> + *
> + * Return:	0 on success, negative on failure
> + */
> +int fdt_efi_pmem_setup(void *fdt)
> +{
> +	return efi_bootmgr_pmem_setup(fdt) == EFI_SUCCESS ? 0 : -1;
> +}
> +
>   static int u16_tohex(u16 c)
>   {
>   	if (c >= '0' && c <= '9')