[PATCH 1/2] efi: remove error in efi_disk_probe

[Heinrich Schuchardt]

On 3/8/23 14:26, Patrick Delaunay wrote:
> EFI has no reason to block the dm core device_probe() in the callback
> efi_disk_probe() registered with EVT_DM_POST_PROBE.
>
> This patch avoids to have error in DM core on device_probe()
>
>    ret = device_notify(dev, EVT_DM_POST_PROBE);
>
> only because EFI is not able to create its instance/handle.

This should only occur if we are out of memory or if you call
efi_disk_probe() twice for the same device.


>
> For example on usb start, when the SAME KEY (PID/VID) is present on
> 2 ports of the USB HUB, the 2nd key have the same EFI device path
> with the call stack:

We need the HUB device with its USB port in the device path.

The way we currently create device paths is not good. We should traverse
the dm tree to the root and create a node for each dm device. The code
code for creating the individual nodes should be moved to uclasses.

@Simon: is that ok for you?

>
> efi_disk_probe()
> 	efi_disk_create_raw()
> 		efi_disk_add_dev()
> 			efi_install_multiple_protocol_interfaces()
> 				EFI_ALREADY_STARTED

If we create the same device path for two USB devices, this is a bug we
must fix.

>
> In case of error in probe, the 2nd key is unbound and deactivated for
> the next usb commands even if the limitation is only for EFI.
>
> This patch removes any return error in probe event callback;
> if something occurs in EFI registration, the device is still probed.
>
> Signed-off-by: Patrick Delaunay <patrick.delaunay at foss.st.com>
> ---
>
>   lib/efi_loader/efi_disk.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index d2256713a8e7..8d53ba3bd27e 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -677,14 +677,18 @@ int efi_disk_probe(void *ctx, struct event *event)
>   	desc = dev_get_uclass_plat(dev);
>   	if (desc->uclass_id != UCLASS_EFI_LOADER) {
>   		ret = efi_disk_create_raw(dev, agent_handle);
> -		if (ret)
> -			return -1;
> +		if (ret) {
> +			log_err("efi_disk_create_raw %s failed (%d)\n",
> +				dev->name, ret);

This isn't a message a non-developer can easily understand.

> +			return 0;
> +		}
>   	}
>
>   	device_foreach_child(child, dev) {
>   		ret = efi_disk_create_part(child, agent_handle);
>   		if (ret)
> -			return -1;
> +			log_err("efi_disk_create_part %s failed (%d)\n",

ditto.

Best regards

Heinrich

> +				dev->name, ret);
>   	}
>
>   	return 0;