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

AKASHI Takahiro
2021-10-11T07:41:48Z

On Sun, Oct 10, 2021 at 08:14:30AM -0600, Simon Glass wrote:
> 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.

Sure.

> > 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.

I will revisit the issue because this hack may be linked to one of my patches
in my repository which is not posted yet.
You can ignore it for now.

-Takahiro Akashi

> > + * 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