[PATCH 12/16] efi_loader: efi_net: add snp_owner field to efi_net_obj struct

[Heinrich Schuchardt]

On 11.03.25 17:47, Adriano Cordova wrote:
> Two efi_net_obj's could share the same efi_simple_network_protocol interface.
> For example, say a working U-Boot net device is registered in the EFI
> subsystem, this will create an instance of an efi simple network protocol.
> Then another handle, say handle A, could be created using this same efi simple
> network protocol interface, and connect controller could be called. A U-Boot
> udevice would be created for handle A and it would be registered in the EFI
> subsystem as an efi_net_obj with the same simple network protocol interface as
> the efi_net_obj we started with.

Why do you think it would be problematic if some new udevice is created
which via efi_net_driver could call into the driver of the physical udevice?

What we should avoid is having multiple handles pointing to the same
udevice.

>
> Signed-off-by: Adriano Cordova <adriano.cordova at canonical.com>
> ---
>   lib/efi_loader/efi_net.c | 51 +++++++++++++++++++++++++---------------
>   1 file changed, 32 insertions(+), 19 deletions(-)
>
> diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
> index 5a00e7a570c..3ad51d343da 100644
> --- a/lib/efi_loader/efi_net.c
> +++ b/lib/efi_loader/efi_net.c
> @@ -110,6 +110,7 @@ struct efi_net_obj {
>   	struct efi_event *wait_for_packet;
>   	struct efi_event *network_timer_event;
>   	int efi_seq_num;
> +	bool snp_owner;

Building the documentation fails because a description for the new
element snp_owner is missing.

Best regards

Heinrich

>   };
>
>   static int curr_efi_net_obj;
> @@ -134,15 +135,17 @@ static bool efi_netobj_is_active(struct efi_net_obj *netobj)
>    *
>    *
>    * @snp:	pointer to the simple network protocol
> + * @bool:	snp owner
>    * Return:	pointer to efi_net_obj, NULL on error
>    */
> -static struct efi_net_obj *efi_netobj_from_snp(struct efi_simple_network *snp)
> +static struct efi_net_obj *efi_netobj_from_snp(struct efi_simple_network *snp,
> +					       bool snp_owner)
>   {
>   	int i;
>
>   	for (i = 0; i < MAX_EFI_NET_OBJS; i++) {
> -		if (net_objs[i] && net_objs[i]->net == snp) {
> -			// Do not register duplicate devices
> +		if (net_objs[i] && net_objs[i]->net == snp &&
> +		    (!snp_owner || net_objs[i]->snp_owner)) {
>   			return net_objs[i];
>   		}
>   	}
> @@ -171,7 +174,7 @@ static efi_status_t EFIAPI efi_net_start(struct efi_simple_network *this)
>   		goto out;
>   	}
>
> -	nt = efi_netobj_from_snp(this);
> +	nt = efi_netobj_from_snp(this, true);
>
>   	if (this->mode->state != EFI_NETWORK_STOPPED) {
>   		ret = EFI_ALREADY_STARTED;
> @@ -207,7 +210,7 @@ static efi_status_t EFIAPI efi_net_stop(struct efi_simple_network *this)
>   		goto out;
>   	}
>
> -	nt = efi_netobj_from_snp(this);
> +	nt = efi_netobj_from_snp(this, true);
>
>   	if (this->mode->state == EFI_NETWORK_STOPPED) {
>   		ret = EFI_NOT_STARTED;
> @@ -250,7 +253,7 @@ static efi_status_t EFIAPI efi_net_initialize(struct efi_simple_network *this,
>   		r = EFI_INVALID_PARAMETER;
>   		goto out;
>   	}
> -	nt = efi_netobj_from_snp(this);
> +	nt = efi_netobj_from_snp(this, true);
>
>   	switch (this->mode->state) {
>   	case EFI_NETWORK_INITIALIZED:
> @@ -347,7 +350,7 @@ static efi_status_t EFIAPI efi_net_shutdown(struct efi_simple_network *this)
>   		ret = EFI_INVALID_PARAMETER;
>   		goto out;
>   	}
> -	nt = efi_netobj_from_snp(this);
> +	nt = efi_netobj_from_snp(this, true);
>
>   	switch (this->mode->state) {
>   	case EFI_NETWORK_INITIALIZED:
> @@ -553,7 +556,7 @@ static efi_status_t EFIAPI efi_net_get_status(struct efi_simple_network *this,
>   		goto out;
>   	}
>
> -	nt = efi_netobj_from_snp(this);
> +	nt = efi_netobj_from_snp(this, true);
>
>   	switch (this->mode->state) {
>   	case EFI_NETWORK_STOPPED:
> @@ -614,7 +617,7 @@ static efi_status_t EFIAPI efi_net_transmit
>   		goto out;
>   	}
>
> -	nt = efi_netobj_from_snp(this);
> +	nt = efi_netobj_from_snp(this, true);
>
>   	/* We do not support jumbo packets */
>   	if (buffer_size > PKTSIZE_ALIGN) {
> @@ -712,7 +715,7 @@ static efi_status_t EFIAPI efi_net_receive
>   		goto out;
>   	}
>
> -	nt = efi_netobj_from_snp(this);
> +	nt = efi_netobj_from_snp(this, true);
>
>   	switch (this->mode->state) {
>   	case EFI_NETWORK_STOPPED:
> @@ -870,7 +873,7 @@ static void EFIAPI efi_network_timer_notify(struct efi_event *event,
>   	if (!this || this->mode->state != EFI_NETWORK_INITIALIZED)
>   		goto out;
>
> -	nt = efi_netobj_from_snp(this);
> +	nt = efi_netobj_from_snp(this, true);
>   	curr_efi_net_obj = nt->efi_seq_num;
>
>   	// The following only happens if the net obj was removed but the event
> @@ -1205,7 +1208,7 @@ static int efi_netobj_init(struct efi_net_obj *netobj)
>
>   	dev = netobj->dev;
>
> -	if (efi_netobj_is_active(netobj))
> +	if (efi_netobj_is_active(netobj) || !netobj->snp_owner)
>   		goto set_timers;
>
>   	if (!netobj->net_mode)
> @@ -1417,6 +1420,7 @@ struct efi_net_obj *efi_netobj_alloc(efi_handle_t handle,
>   			free(netobj->net->mode);
>   		free(netobj->net);
>   	}
> +	netobj->net = NULL;
>
>   	if (handle) {
>   		netobj->handle = handle;
> @@ -1429,6 +1433,8 @@ struct efi_net_obj *efi_netobj_alloc(efi_handle_t handle,
>   		}
>   	}
>
> +	netobj->snp_owner = efi_netobj_from_snp(net, true) ? false : true;
> +
>   	if (net) {
>   		netobj->net = net;
>   		netobj->net_mode = net->mode;
> @@ -1556,14 +1562,9 @@ int efi_net_unregister(void *ctx, struct event *event)
>   		return -1;
>   	}
>
> +	interface = NULL;
>   	if (drv != dev->driver) {
>   		ret = EFI_CALL(efi_disconnect_controller(netobj->handle, NULL, NULL));
> -		if (ret != EFI_SUCCESS)
> -			return -1;
> -		ret = EFI_CALL(efi_close_event(netobj->wait_for_packet));
> -		if (ret != EFI_SUCCESS)
> -			return -1;
> -		ret = EFI_CALL(efi_close_event(netobj->network_timer_event));
>   		if (ret != EFI_SUCCESS)
>   			return -1;
>
> @@ -1581,6 +1582,15 @@ int efi_net_unregister(void *ctx, struct event *event)
>   		}
>   	}
>
> +	if (netobj->snp_owner) {
> +		ret = EFI_CALL(efi_close_event(netobj->wait_for_packet));
> +		if (ret != EFI_SUCCESS)
> +			return -1;
> +		ret = EFI_CALL(efi_close_event(netobj->network_timer_event));
> +		if (ret != EFI_SUCCESS)
> +			return -1;
> +	}
> +
>   #if IS_ENABLED(CONFIG_EFI_IP4_CONFIG2_PROTOCOL)
>   	if (netobj->ip4_config2) {
>   		r = efi_ipconfig_unregister(netobj->handle, netobj->ip4_config2);
> @@ -1603,8 +1613,11 @@ int efi_net_unregister(void *ctx, struct event *event)
>   		ret = efi_delete_handle(netobj->handle);
>   		if (ret != EFI_SUCCESS)
>   			return -1;
> +	}
> +
> +	efi_free_pool(interface);
>
> -		efi_free_pool(interface);
> +	if (netobj->snp_owner) {
>   		if (netobj->net)
>   			free(netobj->net);
>   		if (netobj->net_mode)