[RFC] efi_driver: fix a parent issue in efi-created block devices

[AKASHI Takahiro]

Please ignore the following RFC since I accidentally posted the wrong patch.

-Takahiro Akashi

On Thu, Jul 27, 2023 at 09:29:55AM +0900, AKASHI Takahiro wrote:
> An EFI application may create an EFI block device (EFI_BLOCK_IO_PROTOCOL) in
> EFI world, which in turn generates a corresponding U-Boot block device based on
> U-Boot's Driver Model.
> The latter device, however, doesn't work as U-Boot proper block device
> due to an issue in efi_driver's implementation. We saw discussions in the past,
> most recently in [1].
> 
>   [1] https://lists.denx.de/pipermail/u-boot/2023-July/522565.html
> 
> This RFC patch tries to address (part of) the issue.
> If it is considered acceptable, I will create a formal patch.
> 
> Withtout this patch,
> ===8<===
> => env set efi_selftest 'block device'
> => bootefi selftest
>  ...
> 
> Summary: 0 failures
> 
> => dm tree
>  Class     Index  Probed  Driver                Name
> -----------------------------------------------------------
>  root          0  [ + ]   root_driver           root_driver
>  ...
>  bootmeth      7  [   ]   vbe_simple            |   `-- vbe_simple
>  blk           0  [ + ]   efi_blk               `-- efiblk#0
>  partition     0  [ + ]   blk_partition             `-- efiblk#0:1
> => ls efiloader 0:1
> ** Bad device specification efiloader 0 **
> Couldn't find partition efiloader 0:1
> ===>8===
> 
> With this patch applied, efiblk#0(:1) now gets accessible.
> 
> ===8<===
> => env set efi_selftest 'block device'
> => bootefi selftest
>  ...
> 
> Summary: 0 failures
> 
> => dm tree
>  Class     Index  Probed  Driver                Name
> -----------------------------------------------------------
>  root          0  [ + ]   root_driver           root_driver
>  ...
>  bootmeth      7  [   ]   vbe_simple            |   `-- vbe_simple
>  efi           0  [ + ]   EFI block driver      `-- /VenHw(dbca4c98-6cb0-694d-0872-819c650cb7b8)
>  blk           0  [ + ]   efi_blk                   `-- efiblk#0
>  partition     0  [ + ]   blk_partition                 `-- efiblk#0:1
> => ls efiloader 0:1
>        13   hello.txt
>         7   u-boot.txt
> 
> 2 file(s), 0 dir(s)
> ===>8===
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
>  include/efi_driver.h                         |  2 +-
>  lib/efi_driver/efi_block_device.c            | 17 ++++++++++++-----
>  lib/efi_driver/efi_uclass.c                  |  8 +++++++-
>  lib/efi_selftest/efi_selftest_block_device.c |  2 ++
>  4 files changed, 22 insertions(+), 7 deletions(-)
> 
> diff --git a/include/efi_driver.h b/include/efi_driver.h
> index 63a95e4cf800..ed66796f3519 100644
> --- a/include/efi_driver.h
> +++ b/include/efi_driver.h
> @@ -42,7 +42,7 @@ struct efi_driver_ops {
>  	const efi_guid_t *child_protocol;
>  	efi_status_t (*init)(struct efi_driver_binding_extended_protocol *this);
>  	efi_status_t (*bind)(struct efi_driver_binding_extended_protocol *this,
> -			     efi_handle_t handle, void *interface);
> +			     efi_handle_t handle, void *interface, char *name);
>  };
>  
>  #endif /* _EFI_DRIVER_H */
> diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c
> index add00eeebbea..43b7ed7c973c 100644
> --- a/lib/efi_driver/efi_block_device.c
> +++ b/lib/efi_driver/efi_block_device.c
> @@ -115,9 +115,9 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>   * Return:	status code
>   */
>  static efi_status_t
> -efi_bl_create_block_device(efi_handle_t handle, void *interface)
> +efi_bl_create_block_device(efi_handle_t handle, void *interface, struct udevice *parent)
>  {
> -	struct udevice *bdev = NULL, *parent = dm_root();
> +	struct udevice *bdev = NULL;
>  	efi_status_t ret;
>  	int devnum;
>  	char *name;
> @@ -181,7 +181,7 @@ err:
>   */
>  static efi_status_t efi_bl_bind(
>  			struct efi_driver_binding_extended_protocol *this,
> -			efi_handle_t handle, void *interface)
> +			efi_handle_t handle, void *interface, char *name)
>  {
>  	efi_status_t ret = EFI_SUCCESS;
>  	struct efi_object *obj = efi_search_obj(handle);
> @@ -191,8 +191,15 @@ static efi_status_t efi_bl_bind(
>  	if (!obj || !interface)
>  		return EFI_INVALID_PARAMETER;
>  
> -	if (!handle->dev)
> -		ret = efi_bl_create_block_device(handle, interface);
> +	if (!handle->dev) {
> +		struct udevice *parent;
> +
> +		ret = device_bind_driver(dm_root(), "EFI block driver", name, &parent);
> +		if (!ret)
> +			ret = efi_bl_create_block_device(handle, interface, parent);
> +		else
> +			ret = EFI_DEVICE_ERROR;
> +	}
>  
>  	return ret;
>  }
> diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
> index 45f935198874..bf669742783e 100644
> --- a/lib/efi_driver/efi_uclass.c
> +++ b/lib/efi_driver/efi_uclass.c
> @@ -145,7 +145,13 @@ static efi_status_t EFIAPI efi_uc_start(
>  	ret = check_node_type(controller_handle);
>  	if (ret != EFI_SUCCESS)
>  		goto err;
> -	ret = bp->ops->bind(bp, controller_handle, interface);
> +
> +	struct efi_handler *handler;
> +	char tmpname[256] = "AppName";
> +	ret = efi_search_protocol(controller_handle, &efi_guid_device_path,
> +				  &handler);
> +	snprintf(tmpname, 256, "%pD", handler->protocol_interface);
> +	ret = bp->ops->bind(bp, controller_handle, interface, strdup(tmpname));
>  	if (ret == EFI_SUCCESS)
>  		goto out;
>  
> diff --git a/lib/efi_selftest/efi_selftest_block_device.c b/lib/efi_selftest/efi_selftest_block_device.c
> index a367e8b89d17..0ab8e4590dfe 100644
> --- a/lib/efi_selftest/efi_selftest_block_device.c
> +++ b/lib/efi_selftest/efi_selftest_block_device.c
> @@ -248,6 +248,7 @@ static int teardown(void)
>  {
>  	efi_status_t r = EFI_ST_SUCCESS;
>  
> +#if 0 /* Temporarily out for confirmation */
>  	if (disk_handle) {
>  		r = boottime->uninstall_protocol_interface(disk_handle,
>  							   &guid_device_path,
> @@ -273,6 +274,7 @@ static int teardown(void)
>  			return EFI_ST_FAILURE;
>  		}
>  	}
> +#endif
>  	return r;
>  }
>  
> -- 
> 2.41.0
>