Re: [PATCH v4 08/14] efi_loader: net: set EFI bootdevice device path to HTTP when loaded from wget

[Heinrich Schuchardt <heinrich.schuchardt@canonical.com>]

On 18.11.24 22:09, Adriano Cordova wrote:
> Set the device path of the efi boot device to an HTTP device path
> (as formed by efi_dp_from_http) when the next boot stage is loaded
> using wget (i.e., when wget is used with wget_info.set_bootdev=1).
> 
> When loaded from HTTP, the device path should account for it so that
> the next boot stage is aware (e.g. grub only loads its http stack if
> it itself was loaded from http, and it checks this from its device path).
> 
> Signed-off-by: Adriano Cordova <adrianox@gmail.com>
> ---
> 
> (no changes since v2)
> 
>   include/efi_loader.h             |  6 +++++
>   lib/efi_loader/efi_device_path.c |  2 +-
>   lib/efi_loader/efi_net.c         | 38 +++++++++++++++++++++++++++++++-
>   net/lwip/wget.c                  |  1 +
>   net/wget.c                       |  1 +
>   5 files changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 96b204dfc3..f49f8e6be0 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -126,6 +126,10 @@ static inline void efi_set_bootdev(const char *dev, const char *devnr,
>   #endif
>   
>   #if CONFIG_IS_ENABLED(NETDEVICES) && CONFIG_IS_ENABLED(EFI_LOADER)
> +/* Call this to update the current device path of the efi net device */
> +void efi_net_set_dp(const char *dev, const char *server);
> +/* Call this to get the current device path of the efi net device */
> +void efi_net_get_dp(void **dp);
>   void efi_net_get_addr(struct efi_ipv4_address *ip,
>   		      struct efi_ipv4_address *mask,
>   		      struct efi_ipv4_address *gw);
> @@ -133,6 +137,8 @@ void efi_net_set_addr(struct efi_ipv4_address *ip,
>   		      struct efi_ipv4_address *mask,
>   		      struct efi_ipv4_address *gw);
>   #else
> +static inline void efi_net_set_dp(const char *dev, const char *server) { }
> +static inline void efi_net_get_dp(void **dp) { }
>   static inline void efi_net_get_addr(struct efi_ipv4_address *ip,
>   				     struct efi_ipv4_address *mask,
>   				     struct efi_ipv4_address *gw) { }
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index 9ee03062ac..5fae3fbb5a 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -1165,7 +1165,7 @@ efi_status_t efi_dp_from_name(const char *dev, const char *devnr,
>   		dp = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
>   				     (uintptr_t)image_addr, image_size);
>   	} else if (IS_ENABLED(CONFIG_NETDEVICES) && !strcmp(dev, "Net")) {
> -		dp = efi_dp_from_eth();
> +		efi_net_get_dp((void **)&dp);
>   	} else if (!strcmp(dev, "Uart")) {
>   		dp = efi_dp_from_uart();
>   	} else {
> diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
> index 3f0b571cc6..2207caf6fe 100644
> --- a/lib/efi_loader/efi_net.c
> +++ b/lib/efi_loader/efi_net.c
> @@ -16,6 +16,7 @@
>    */
>   
>   #include <efi_loader.h>
> +#include <dm.h>
>   #include <malloc.h>
>   #include <vsprintf.h>
>   #include <net.h>
> @@ -31,6 +32,7 @@ static size_t *receive_lengths;
>   static int rx_packet_idx;
>   static int rx_packet_num;
>   static struct efi_net_obj *netobj;

The role of net_dp deserves a description.

> +static struct efi_device_path *net_dp;
>   
>   /*
>    * The notification function of this event is called in every timer cycle
> @@ -902,8 +904,10 @@ efi_status_t efi_net_register(void)
>   			     &netobj->net);
>   	if (r != EFI_SUCCESS)
>   		goto failure_to_add_protocol;
> +	if (!net_dp)
> +		efi_net_set_dp("Net", NULL);
>   	r = efi_add_protocol(&netobj->header, &efi_guid_device_path,
> -			     efi_dp_from_eth());
> +			     net_dp);
>   	if (r != EFI_SUCCESS)
>   		goto failure_to_add_protocol;
>   	r = efi_add_protocol(&netobj->header, &efi_pxe_base_code_protocol_guid,
> @@ -999,6 +1003,38 @@ out_of_resources:
>   	return EFI_OUT_OF_RESOURCES;
>   }
>   
> +/**
> + * efi_net_set_dp() - set device path of efi net device
> + *
> + * This gets called to update the device path when a new boot
> + * file is dowloaded
> + *
> + * @dev:	dev to set the device path from
> + * @server:	remote server address
> + */
> +void efi_net_set_dp(const char *dev, const char *server)
> +{
> +	if (!strcmp(dev, "Net"))
> +		net_dp = efi_dp_from_eth();
> +	else if (!strcmp(dev, "Http"))
> +		net_dp = efi_dp_from_http(server);
> +}
> +
> +/**
> + * efi_net_get_dp() - get device path of efi net device
> + *
> + * Produce a copy of the current device path
> + *
> + * @dp:		copy of the current device path
> + */
> +void efi_net_get_dp(void **dp)

We know that dp must point to a device-path. Please, do not use void**.

efi_device_path** should be fine.

Best regards

Heinrich

> +{
> +	if (!net_dp)
> +		efi_net_set_dp("Net", NULL);
> +	if (dp && net_dp)
> +		*dp = efi_dp_dup(net_dp);
> +}
> +
>   /**
>    * efi_net_get_addr() - get IP address information
>    *
> diff --git a/net/lwip/wget.c b/net/lwip/wget.c
> index 53c3b169e0..5c44e324be 100644
> --- a/net/lwip/wget.c
> +++ b/net/lwip/wget.c
> @@ -217,6 +217,7 @@ static void httpc_result_cb(void *arg, httpc_result_t httpc_result,
>   	print_size(rx_content_len / elapsed * 1000, "/s)\n");
>   	printf("Bytes transferred = %lu (%lx hex)\n", ctx->size, ctx->size);
>   	if (wget_info->set_bootdev) {
> +		efi_net_set_dp("Http", ctx->server_name);
>   		efi_set_bootdev("Net", "", ctx->path, map_sysmem(ctx->saved_daddr, 0),
>   				rx_content_len);
>   	}
> diff --git a/net/wget.c b/net/wget.c
> index 1166aa5aa6..a1328990d2 100644
> --- a/net/wget.c
> +++ b/net/wget.c
> @@ -442,6 +442,7 @@ static void wget_handler(uchar *pkt, u16 dport,
>   		net_set_state(wget_loop_state);
>   		wget_info->file_size = net_boot_file_size;
>   		if (wget_info->method == WGET_HTTP_METHOD_GET && wget_info->set_bootdev) {
> +			efi_net_set_dp("Http", NULL);
>   			efi_set_bootdev("Net", "", image_url,
>   					map_sysmem(image_load_addr, 0),
>   					net_boot_file_size);