diff mbox series

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

Message ID 20211001050228.55183-40-takahiro.akashi@linaro.org
State Superseded
Headers show
Series efi_loader: more tightly integrate UEFI disks to device model | expand

Commit Message

AKASHI Takahiro Oct. 1, 2021, 5:02 a.m. UTC 
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(-)

-- 
2.33.0

Comments

Simon Glass Oct. 10, 2021, 2:14 p.m. UTC | #1 
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
AKASHI Takahiro Oct. 11, 2021, 7:41 a.m. UTC | #2 
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
diff mbox series

Patch

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