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

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

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