Re: [RFC 20/22] efi_driver: align with efi_disk-dm integration

Simon Glass
2021-10-10T14:17:10Z

Hi Takahiro,

On Thu, 30 Sept 2021 at 23:05, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>

Can you please add a commit message as I am not sure what this patch is doing.

> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> ---
> lib/efi_driver/efi_block_device.c | 6 ++++++
> lib/efi_loader/efi_device_path.c | 29 +++++++++++++++++++++++++++++
> lib/efi_loader/efi_disk.c | 12 +++++++++++-
> 3 files changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c
> index 0937e3595a43..b6afa939e1d1 100644
> --- a/lib/efi_driver/efi_block_device.c
> +++ b/lib/efi_driver/efi_block_device.c
> @@ -173,6 +173,12 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
> plat->handle = handle;
> plat->io = interface;
>
> + /*
> + * FIXME: necessary because we won't do almost nothing in
> + * efi_disk_create() when called from device_probe().
> + */
> + bdev->efi_obj = handle;
> +
> ret = device_probe(bdev);
> if (ret)
> return ret;
> diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
> index cbdb466da41c..36c77bce9a05 100644
> --- a/lib/efi_loader/efi_device_path.c
> +++ b/lib/efi_loader/efi_device_path.c
> @@ -628,6 +628,35 @@ __maybe_unused static void *dp_fill(void *buf, struct udevice *dev)
> return &dp->vendor_data[1];
> }
> #endif
> +#ifdef CONFIG_EFI_LOADER
> + /*
> + * FIXME: conflicting with CONFIG_SANDBOX

In what way? If it is just a test we should be able to update the test
to avoid the conflict.

> + * This case is necessary to support efi_disk's created by
> + * efi_driver (and efi_driver_binding_protocol).
> + * TODO:
> + * The best way to work around here is to create efi_root as
> + * udevice and put all efi_driver objects under it.
> + */
> + case UCLASS_ROOT: {
> + struct efi_device_path_vendor *dp;
> + struct blk_desc *desc = dev_get_uclass_plat(dev);
> + /* FIXME: guid_vendor used in selftest_block_device */
> + static efi_guid_t guid_vendor =
> + EFI_GUID(0xdbca4c98, 0x6cb0, 0x694d,
> + 0x08, 0x72, 0x81, 0x9c, 0x65, 0x0c, 0xb7, 0xb8);
> +
> +
> + dp_fill(buf, dev->parent);
> + dp = buf;
> + ++dp;
> + dp->dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
> + dp->dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
> + dp->dp.length = sizeof(*dp) + 1;
> + memcpy(&dp->guid, &guid_vendor, sizeof(efi_guid_t));
> + dp->vendor_data[0] = desc->devnum;
> + return &dp->vendor_data[1];
> + }
> +#endif
> #ifdef CONFIG_VIRTIO_BLK
> case UCLASS_VIRTIO: {
> struct efi_device_path_vendor *dp;
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index dfd06dd31e4a..e7cf1567929b 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -562,11 +562,21 @@ static int efi_disk_create_part(struct udevice *dev)
> int efi_disk_create(struct udevice *dev)
> {
> enum uclass_id id;
> + struct blk_desc *desc;
>
> id = device_get_uclass_id(dev);
>
> - if (id == UCLASS_BLK)
> + if (id == UCLASS_BLK) {
> + /*
> + * avoid creating duplicated objects now that efi_driver
> + * has already created an efi_disk at this moment.
> + */
> + desc = dev_get_uclass_plat(dev);
> + if (desc->if_type == IF_TYPE_EFI)
> + return 0;
> +
> return efi_disk_create_raw(dev);
> + }
>
> if (id == UCLASS_PARTITION)
> return efi_disk_create_part(dev);
> --
> 2.33.0
>

Regards,
Simon