[PATCH v4 00/12] efi_loader: more tightly integrate UEFI disks to driver model

[PATCH v4 00/12] efi_loader: more tightly integrate UEFI disks to driver model

[AKASHI Takahiro]

With this patch set[1] applied, UEFI subsystem maintains a list of its
disk objects dynamically at runtime based on block device's probing.
(See "issues" and "prerequisite" below.)

[1] https://github.com/t-akashi/u-boot/tree/efi/dm_disk

For instance,
=> dm tree
 Class     Index  Probed  Driver                Name
-----------------------------------------------------------
 root          0  [ + ]   root_driver           root_driver
 ...
 pci           0  [ + ]   pci_generic_ecam      |-- pcie@10000000
 ...
 ahci          0  [   ]   ahci_pci              |   |-- ahci_pci
 scsi          0  [   ]   ahci_scsi             |   |   `-- ahci_scsi
 usb           0  [   ]   xhci_pci              |   `-- xhci_pci
 ...
=> efi devices
Missing RNG device for EFI_RNG_PROTOCOL
No EFI system partition
Unable to find TPMv2 device
Device           Device Path
================ ====================
000000013eee88d0 /VenHw(..)
000000013ffeb798 /VenHw(..)/Uart(0,0,D,D)
000000013eeeb810 /VenHw(..)/MAC(525252525252,1)
=> scsi rescan
 ...
=> efi devices
Device           Device Path
================ ====================
000000013eee88d0 /VenHw(..)
000000013ffeb798 /VenHw(..)/Uart(0,0,D,D)
000000013eeeb810 /VenHw(..)/MAC(525252525252,1)
000000013eefb730 /VenHw(..)/Scsi(0,0)
000000013eefb840 /VenHw(..)/Scsi(0,0)/HD(1,GPT,ce86c5a7-b32a-488f-a346-88fe698e0edc,0x22,0x4c2a)
000000013eefbc80 /VenHw(..)/Scsi(0,0)/HD(2,GPT,aa80aab9-33e6-42b6-b5db-def2cb8d7844,0x5000,0x1a800)
=> usb start
 ...
=> efi devices
Device           Device Path
================ ====================
000000013eee88d0 /VenHw(..)
000000013ffeb798 /VenHw(..)/Uart(0,0,D,D)
000000013eeeb810 /VenHw(..)/MAC(525252525252,1)
000000013eefb730 /VenHw(..)/Scsi(0,0)
000000013eefb840 /VenHw(..)/Scsi(0,0)/HD(1,GPT,ce86c5a7-b32a-488f-a346-88fe698e0edc,0x22,0x4c2a)
000000013eefbc80 /VenHw(..)/Scsi(0,0)/HD(2,GPT,aa80aab9-33e6-42b6-b5db-def2cb8d7844,0x5000,0x1a800)
000000013ef01330 /VenHw(..)/UsbClass(0x0,0x0,0x9,0x0,0x3)/UsbClass(0x46f4,0x1,0x0,0x0,0x0)
000000013ef014b0 /VenHw(..)/UsbClass(0x0,0x0,0x9,0x0,0x3)/UsbClass(0x46f4,0x1,0x0,0x0,0x0)/HD(1,GPT,ce86c5a7-b32a-488f-a346-88fe698e0edc,0x22,0x4c2a)
000000013ef018f0 /VenHw(..)/UsbClass(0x0,0x0,0x9,0x0,0x3)/UsbClass(0x46f4,0x1,0x0,0x0,0x0)/HD(2,GPT,aa80aab9-33e6-42b6-b5db-def2cb8d7844,0x5000,0x1a800)
=> dm tree
 ...
 pci           0  [ + ]   pci_generic_ecam      |-- pcie@10000000
 ...
 ahci          0  [ + ]   ahci_pci              |   |-- ahci_pci
 scsi          0  [ + ]   ahci_scsi             |   |   `-- ahci_scsi
 blk           2  [ + ]   scsi_blk              |   |       `-- ahci_scsi.id0lun0
 partition     0  [ + ]   blk_partition         |   |           |-- ahci_scsi.id0lun0:1
 partition     1  [ + ]   blk_partition         |   |           `-- ahci_scsi.id0lun0:2
 usb           0  [ + ]   xhci_pci              |   `-- xhci_pci
 usb_hub       0  [ + ]   usb_hub               |       `-- usb_hub
 usb_mass_s    0  [ + ]   usb_mass_storage      |           |-- usb_mass_storage
 blk           3  [ + ]   usb_storage_blk       |           |   `-- usb_mass_storage.lun0
 partition     2  [ + ]   blk_partition         |           |       |-- usb_mass_storage.lun0:1
 partition     3  [ + ]   blk_partition         |           |       `-- usb_mass_storage.lun0:2
 ...

=> usb stop
stopping USB..
=> efi devices
Device           Device Path
================ ====================
000000013eee88d0 /VenHw(..)
000000013ffeb798 /VenHw(..)/Uart(0,0,D,D)
000000013eeeb810 /VenHw(..)/MAC(525252525252,1)
000000013eefb730 /VenHw(..)/Scsi(0,0)
000000013eefb840 /VenHw(..)/Scsi(0,0)/HD(1,GPT,ce86c5a7-b32a-488f-a346-88fe698e0edc,0x22,0x4c2a)
000000013eefbc80 /VenHw(..)/Scsi(0,0)/HD(2,GPT,aa80aab9-33e6-42b6-b5db-def2cb8d7844,0x5000,0x1a800)


Issues:
=======
* Some (UEFI-unrelated) UT tests still fail for a unknown reason.
  As I reported[2], this might happen due to U-Boot itself or UT framework.

* For removal case, we may need more consideration since removing handles
  unconditionally may end up breaking integrity of handles
  (as some may still be held and referenced to by a UEFI app).

[2] https://lists.denx.de/pipermail/u-boot/2022-April/481405.html


Prerequisite:
=============
* my patch, "disk: don't compile in partition support for spl/tpl if not
  really necessary"[3]

[3] https://lists.denx.de/pipermail/u-boot/2022-April/481531.html


Patchs:
=======
For easy understandings, patches may be categorized into separate groups
of changes.

Patch#1-#3: DM: add a new feature (DM tag)
Patch#4-#10: UEFI: dynamically create/remove efi_disk's for a raw disk
  and its partitions
Patch#11-#12: UEFI: use udevice read/write interfaces


Change history:
===============
v4 (Apr, 15, 2022)
* rebased on pre-2022.07-rc1
* removed already-merged patches
* added patch#1 (for better error code)
* merged v3's patch#19 to patch#7 to make bisect work
* split v3's patch#13 into patch#8(create) and patch#10(delete)
* move a function prototype out of "#ifdef CONFIG_PARTITIONS" (patch#4)
* move function prototypes out of "#ifdef CONFIG_PARTITIONS" (patch#11)

v3 (Mar 8, 2022)
* rebased on 2022.04-rc3
* rebased on v2 of Simon's event patch
  (hence removed his patch from this patch set)
* fix a spl-build error in mmc_blk_probe() (patch#3)
* fix a build warning in fsl_ata_probe()/sil_pci_probe() (patch#5)
* remove a problematic log message in efi_disk_probe() (patch#14)

v2 (Feb 10, 2022)
* add/revise an error message if device_probe() fails (patch#3,#5)
* fix a build error in sandbox_spl_defconfig (patch#8)
* fix warnings in 'make htmldocs' (patch#8,#9,#18)
* new commit: split efi_init_obj_list() (patch#14)
* new commit: simplify efi_capsule pytest (patch#21)

v1 (Feb 2, 2022)
* rebased on 2022.04-rc1
* drop patches that have already been merged
* modify a tag-range check with "tag >= DM_TAG_COUNT" (patch#9)
* move dmtag_list to GD (global data) (patch#9)
* add function descriptions and a document about DM tag feature (patch#9,10)
* add tests for DM tag support (patch#11)
* change 'depends on EVENT' to 'select EVENT' for EFI_LOADER (patch#14)
* migrate IF_TYPE_EFI to IF_TYPE_EFI_LOADER (patch#18)

RFCv2 (Dec 10, 2021)
* rebased on 2022-rc3
* re-order and merge some related commits into ones
* call device_probe() in MMC (not bind, but) probe hook (patch#5)
* fix a wrong name of variable (patch#7)
* add patch#9
* invoke device_probe() for virtio devices (patch#10)
* add DM event notification (from Simon) (patch#11)
* add DM tag support (patch#12)
* move UCLASS_PARTITION driver under disk/ (patch#13)
* create partition's dp using its parent's. This change is necessary
  in particular for 'efi_blk' efi_disk (patch#13)
* modify the code so that we will use new features like tags and
  event notification (patch#13,15,16,20)
* rename new functions from blk_read/write() to dev_read/write()
  (patch#17,18)
* isolate changes in efi_driver from the rest (in efi_loader) (patch#19)
* drop the previous patch#22 ("efi_selftest: block device: adjust dp
  for a test") due to the fix in patch#13

RFC (Nov 16, 2021)
* initial RFC

AKASHI Takahiro (12):
  dm: tag: change ENOSPC to ENOMEM
  dm: tag: add some document
  test: dm: add tests for tag support
  dm: disk: add UCLASS_PARTITION
  dm: blk: add a device-probe hook for scanning disk partitions
  efi_loader: split efi_init_obj_list() into two stages
  efi_loader: disk: a helper function to create efi_disk objects from
    udevice
  efi_loader: disk: not create BLK device for BLK(IF_TYPE_EFI_LOADER)
    devices
  efi_loader: disk: a helper function to delete efi_disk objects
  efi_loader: disk: not delete BLK device for BLK(IF_TYPE_EFI_LOADER)
    devices
  dm: disk: add read/write interfaces with udevice
  efi_loader: disk: use udevice instead of blk_desc

 common/board_r.c                    |   2 +-
 common/main.c                       |   7 +-
 disk/Makefile                       |   3 +
 disk/disk-uclass.c                  | 247 ++++++++++++++++++++++
 doc/develop/driver-model/design.rst |  20 ++
 drivers/block/blk-uclass.c          |   4 +
 drivers/core/tag.c                  |   4 +-
 include/dm/uclass-id.h              |   1 +
 include/efi_loader.h                |   6 +-
 include/part.h                      |  17 ++
 lib/efi_driver/efi_block_device.c   |  34 +--
 lib/efi_loader/Kconfig              |   3 +
 lib/efi_loader/efi_disk.c           | 314 +++++++++++++++++++++-------
 lib/efi_loader/efi_setup.c          |  62 ++++--
 test/dm/Makefile                    |   1 +
 test/dm/tag.c                       |  84 ++++++++
 16 files changed, 685 insertions(+), 124 deletions(-)
 create mode 100644 disk/disk-uclass.c
 create mode 100644 test/dm/tag.c

-- 
2.33.0

[PATCH v4 01/12] dm: tag: change ENOSPC to ENOMEM

[AKASHI Takahiro]

ENOMEM is a more common error code for memory starvation.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 drivers/core/tag.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/core/tag.c b/drivers/core/tag.c
index 6829bcd8806c..22999193a5a3 100644
--- a/drivers/core/tag.c
+++ b/drivers/core/tag.c
@@ -29,7 +29,7 @@ int dev_tag_set_ptr(struct udevice *dev, enum dm_tag_t tag, void *ptr)
 
 	node = calloc(sizeof(*node), 1);
 	if (!node)
-		return -ENOSPC;
+		return -ENOMEM;
 
 	node->dev = dev;
 	node->tag = tag;
@@ -53,7 +53,7 @@ int dev_tag_set_val(struct udevice *dev, enum dm_tag_t tag, ulong val)
 
 	node = calloc(sizeof(*node), 1);
 	if (!node)
-		return -ENOSPC;
+		return -ENOMEM;
 
 	node->dev = dev;
 	node->tag = tag;
-- 
2.33.0

[PATCH v4 02/12] dm: tag: add some document

[AKASHI Takahiro]

Some basic stuff about tag support is explained under
doc/devlop/driver-model.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 doc/develop/driver-model/design.rst | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/doc/develop/driver-model/design.rst b/doc/develop/driver-model/design.rst
index b0e6337030a1..3e88dc40e6fd 100644
--- a/doc/develop/driver-model/design.rst
+++ b/doc/develop/driver-model/design.rst
@@ -1042,6 +1042,26 @@ data structure might be worthwhile in some rare cases, once we understand
 what the bottlenecks are.
 
 
+Tag Support
+-----------
+
+It is sometimes useful for a subsystem to associate its own private
+data (or object) to a DM device, i.e. struct udevice, to support
+additional features.
+
+Tag support in driver model will give us the ability to do so dynamically
+instead of modifying "udevice" data structure. In the initial release, we
+will support two type of attributes:
+- a pointer with dm_tag_set_ptr(), and
+- an unsigned long with dm_tag_set_val()
+
+For example, UEFI subsystem utilizes the feature to maintain efi_disk
+objects depending on linked udevice's lifecycle.
+
+While the current implementation is quite simple, it will get evolved
+as the feature is more extensively used in U-Boot subsystems.
+
+
 Changes since v1
 ----------------
 
-- 
2.33.0

[PATCH v4 03/12] test: dm: add tests for tag support

[AKASHI Takahiro]

The new test covers all tag-related interfaces.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 test/dm/Makefile |  1 +
 test/dm/tag.c    | 84 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)
 create mode 100644 test/dm/tag.c

diff --git a/test/dm/Makefile b/test/dm/Makefile
index d46552fbf320..dc3177dbb7f4 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -102,6 +102,7 @@ obj-y += syscon.o
 obj-$(CONFIG_RESET_SYSCON) += syscon-reset.o
 obj-$(CONFIG_SYSINFO) += sysinfo.o
 obj-$(CONFIG_SYSINFO_GPIO) += sysinfo-gpio.o
+obj-$(CONFIG_UT_DM) += tag.o
 obj-$(CONFIG_TEE) += tee.o
 obj-$(CONFIG_TIMER) += timer.o
 obj-$(CONFIG_DM_USB) += usb.o
diff --git a/test/dm/tag.c b/test/dm/tag.c
new file mode 100644
index 000000000000..8289954e7c26
--- /dev/null
+++ b/test/dm/tag.c
@@ -0,0 +1,84 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  DM tag test
+ *
+ *  Copyright (c) 2021 Linaro Limited
+ *                      Author: AKASHI Takahiro
+ */
+
+#include <common.h>
+#include <dm/tag.h>
+#include <dm/test.h> /* DM_TEST() */
+#include <test/test.h> /* struct unit_test_state */
+#include <test/ut.h> /* assertions */
+
+/*
+ * Test dm_tag_ptr() API
+ */
+static int dm_test_tag_ptr(struct unit_test_state *uts)
+{
+	ulong val;
+	void *ptr = NULL;
+
+	ut_assertok(dev_tag_set_ptr(uts->root, DM_TAG_EFI, &val));
+
+	ut_assertok(dev_tag_get_ptr(uts->root, DM_TAG_EFI, &ptr));
+
+	ut_asserteq_ptr(&val, ptr);
+
+	ut_assertok(dev_tag_del(uts->root, DM_TAG_EFI));
+
+	return 0;
+}
+
+DM_TEST(dm_test_tag_ptr, 0);
+
+/*
+ * Test dm_tag_val() API
+ */
+static int dm_test_tag_val(struct unit_test_state *uts)
+{
+	ulong val1 = 0x12345678, val2 = 0;
+
+	ut_assertok(dev_tag_set_val(uts->root, DM_TAG_EFI, val1));
+
+	ut_assertok(dev_tag_get_val(uts->root, DM_TAG_EFI, &val2));
+
+	ut_asserteq_64(val1, val2);
+
+	ut_assertok(dev_tag_del(uts->root, DM_TAG_EFI));
+
+	return 0;
+}
+
+DM_TEST(dm_test_tag_val, 0);
+
+/*
+ * Test against an invalid tag
+ */
+static int dm_test_tag_inval(struct unit_test_state *uts)
+{
+	ulong val;
+
+	ut_asserteq(-EINVAL, dev_tag_set_ptr(uts->root, DM_TAG_COUNT, &val));
+
+	return 0;
+}
+
+DM_TEST(dm_test_tag_inval, 0);
+
+/*
+ * Test dm_tag_del_all() AP:
+ */
+static int dm_test_tag_del_all(struct unit_test_state *uts)
+{
+	ulong val;
+
+	ut_assertok(dev_tag_set_ptr(uts->root, DM_TAG_EFI, &val));
+
+	ut_assertok(dev_tag_del_all(uts->root));
+
+	return 0;
+}
+
+DM_TEST(dm_test_tag_del_all, 0);
-- 
2.33.0

[PATCH v4 04/12] dm: disk: add UCLASS_PARTITION

[AKASHI Takahiro]

NOTE: probably we have to update config dependencies,
in particular, SPL/TPL_PRINTF?

With this new function, UCLASS_PARTITION devices will be created as
child nodes of UCLASS_BLK device.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 disk/Makefile          |   3 +
 disk/disk-uclass.c     | 153 +++++++++++++++++++++++++++++++++++++++++
 include/dm/uclass-id.h |   1 +
 include/part.h         |  10 +++
 4 files changed, 167 insertions(+)
 create mode 100644 disk/disk-uclass.c

diff --git a/disk/Makefile b/disk/Makefile
index 5ca10c55762b..ec148832b330 100644
--- a/disk/Makefile
+++ b/disk/Makefile
@@ -6,6 +6,9 @@
 #ccflags-y += -DET_DEBUG -DDEBUG
 
 obj-$(CONFIG_$(SPL_TPL_)PARTITIONS)  += part.o
+ifdef CONFIG_$(SPL_TPL_)BLK
+obj-$(CONFIG_$(SPL_TPL_)PARTITIONS)  += disk-uclass.o
+endif
 obj-$(CONFIG_$(SPL_TPL_)MAC_PARTITION)   += part_mac.o
 obj-$(CONFIG_$(SPL_TPL_)DOS_PARTITION)   += part_dos.o
 obj-$(CONFIG_$(SPL_TPL_)ISO_PARTITION)   += part_iso.o
diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c
new file mode 100644
index 000000000000..4918a2f72d1e
--- /dev/null
+++ b/disk/disk-uclass.c
@@ -0,0 +1,153 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  Software partition device (UCLASS_PARTITION)
+ *
+ *  Copyright (c) 2021 Linaro Limited
+ *			Author: AKASHI Takahiro
+ */
+
+#define LOG_CATEGORY UCLASS_PARTITION
+
+#include <blk.h>
+#include <dm.h>
+#include <log.h>
+#include <part.h>
+#include <vsprintf.h>
+#include <dm/device-internal.h>
+#include <dm/lists.h>
+
+int part_create_block_devices(struct udevice *blk_dev)
+{
+	int part, count;
+	struct blk_desc *desc = dev_get_uclass_plat(blk_dev);
+	struct disk_partition info;
+	struct disk_part *part_data;
+	char devname[32];
+	struct udevice *dev;
+	int ret;
+
+	if (!CONFIG_IS_ENABLED(PARTITIONS) ||
+	    !CONFIG_IS_ENABLED(HAVE_BLOCK_DEVICE))
+		return 0;
+
+	if (device_get_uclass_id(blk_dev) != UCLASS_BLK)
+		return 0;
+
+	/* Add devices for each partition */
+	for (count = 0, part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
+		if (part_get_info(desc, part, &info))
+			continue;
+		snprintf(devname, sizeof(devname), "%s:%d", blk_dev->name,
+			 part);
+
+		ret = device_bind_driver(blk_dev, "blk_partition",
+					 strdup(devname), &dev);
+		if (ret)
+			return ret;
+
+		part_data = dev_get_uclass_plat(dev);
+		part_data->partnum = part;
+		part_data->gpt_part_info = info;
+		count++;
+
+		ret = device_probe(dev);
+		if (ret) {
+			debug("Can't probe\n");
+			count--;
+			device_unbind(dev);
+
+			continue;
+		}
+	}
+	debug("%s: %d partitions found in %s\n", __func__, count,
+	      blk_dev->name);
+
+	return 0;
+}
+
+static ulong blk_part_read(struct udevice *dev, lbaint_t start,
+			   lbaint_t blkcnt, void *buffer)
+{
+	struct udevice *parent;
+	struct disk_part *part;
+	const struct blk_ops *ops;
+
+	parent = dev_get_parent(dev);
+	ops = blk_get_ops(parent);
+	if (!ops->read)
+		return -ENOSYS;
+
+	part = dev_get_uclass_plat(dev);
+	if (start >= part->gpt_part_info.size)
+		return 0;
+
+	if ((start + blkcnt) > part->gpt_part_info.size)
+		blkcnt = part->gpt_part_info.size - start;
+	start += part->gpt_part_info.start;
+
+	return ops->read(parent, start, blkcnt, buffer);
+}
+
+static ulong blk_part_write(struct udevice *dev, lbaint_t start,
+			    lbaint_t blkcnt, const void *buffer)
+{
+	struct udevice *parent;
+	struct disk_part *part;
+	const struct blk_ops *ops;
+
+	parent = dev_get_parent(dev);
+	ops = blk_get_ops(parent);
+	if (!ops->write)
+		return -ENOSYS;
+
+	part = dev_get_uclass_plat(dev);
+	if (start >= part->gpt_part_info.size)
+		return 0;
+
+	if ((start + blkcnt) > part->gpt_part_info.size)
+		blkcnt = part->gpt_part_info.size - start;
+	start += part->gpt_part_info.start;
+
+	return ops->write(parent, start, blkcnt, buffer);
+}
+
+static ulong blk_part_erase(struct udevice *dev, lbaint_t start,
+			    lbaint_t blkcnt)
+{
+	struct udevice *parent;
+	struct disk_part *part;
+	const struct blk_ops *ops;
+
+	parent = dev_get_parent(dev);
+	ops = blk_get_ops(parent);
+	if (!ops->erase)
+		return -ENOSYS;
+
+	part = dev_get_uclass_plat(dev);
+	if (start >= part->gpt_part_info.size)
+		return 0;
+
+	if ((start + blkcnt) > part->gpt_part_info.size)
+		blkcnt = part->gpt_part_info.size - start;
+	start += part->gpt_part_info.start;
+
+	return ops->erase(parent, start, blkcnt);
+}
+
+static const struct blk_ops blk_part_ops = {
+	.read	= blk_part_read,
+	.write	= blk_part_write,
+	.erase	= blk_part_erase,
+};
+
+U_BOOT_DRIVER(blk_partition) = {
+	.name		= "blk_partition",
+	.id		= UCLASS_PARTITION,
+	.ops		= &blk_part_ops,
+};
+
+UCLASS_DRIVER(partition) = {
+	.id		= UCLASS_PARTITION,
+	.per_device_plat_auto	= sizeof(struct disk_part),
+	.name		= "partition",
+};
diff --git a/include/dm/uclass-id.h b/include/dm/uclass-id.h
index 0e26e1d13824..230b1ea528cf 100644
--- a/include/dm/uclass-id.h
+++ b/include/dm/uclass-id.h
@@ -83,6 +83,7 @@ enum uclass_id {
 	UCLASS_P2SB,		/* (x86) Primary-to-Sideband Bus */
 	UCLASS_PANEL,		/* Display panel, such as an LCD */
 	UCLASS_PANEL_BACKLIGHT,	/* Backlight controller for panel */
+	UCLASS_PARTITION,	/* Logical disk partition device */
 	UCLASS_PCH,		/* x86 platform controller hub */
 	UCLASS_PCI,		/* PCI bus */
 	UCLASS_PCI_EP,		/* PCI endpoint device */
diff --git a/include/part.h b/include/part.h
index 53cfbdd87671..907c227aa20d 100644
--- a/include/part.h
+++ b/include/part.h
@@ -287,6 +287,16 @@ part_get_info_by_dev_and_name_or_num(const char *dev_iface,
 }
 #endif
 
+struct udevice;
+/**
+ * part_create_block_devices - Create block devices for disk partitions
+ *
+ * Create UCLASS_PARTITION udevices for each of disk partitions in @parent
+ *
+ * @blk_dev:	Whole disk device
+ */
+int part_create_block_devices(struct udevice *blk_dev);
+
 /*
  * We don't support printing partition information in SPL and only support
  * getting partition information in a few cases.
-- 
2.33.0

[PATCH v4 05/12] dm: blk: add a device-probe hook for scanning disk partitions

[AKASHI Takahiro]

Now that all the block device drivers have enable a probe hook, we will
call part_create_block_devices() to enumerate all the partitions and
create associated udevices when a block device is detected.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 drivers/block/blk-uclass.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
index f1e4a8564679..791e26c06ebe 100644
--- a/drivers/block/blk-uclass.c
+++ b/drivers/block/blk-uclass.c
@@ -741,6 +741,10 @@ static int blk_post_probe(struct udevice *dev)
 		struct blk_desc *desc = dev_get_uclass_plat(dev);
 
 		part_init(desc);
+
+		if (desc->part_type != PART_TYPE_UNKNOWN &&
+		    part_create_block_devices(dev))
+			debug("*** creating partitions failed\n");
 	}
 
 	return 0;
-- 
2.33.0

[PATCH v4 06/12] efi_loader: split efi_init_obj_list() into two stages

[AKASHI Takahiro]

In the next commit, CONFIG_EFI_SETUP_EARLY will become mandated
in order to support dynamic enumeration of efi_disk objects.

This can, however, be problematic particularly in case of file-based
variable storage (efi_variable.c, default).
Non-volatile variables are to be restored from EFI system partition
by efi_init_variables() in efi_init_obj_list(). When efi_init_obj_list()
is called in board_init_r(), we don't know yet what disk devices
we have since none of device probing commands (say, scsi rescan) has not
been executed at that stage.

So in this commit, a preparatory change is made; efi_init_obj_list() is
broken into the two functions;
   * efi_init_early(), and
   * new efi_init_obj_list()

Only efi_init_early() will be called in board_init_r(), which allows
us to execute any of device probing commands, either though "preboot"
variable or normal command line, before calling efi_init_obj_list() which
is to be invoked at the first execution of an efi-related command
(or at efi_launch_capsules()) as used to be.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 common/board_r.c           |  2 +-
 common/main.c              |  7 +++--
 include/efi_loader.h       |  2 ++
 lib/efi_loader/efi_setup.c | 58 ++++++++++++++++++++++++++++++++------
 4 files changed, 57 insertions(+), 12 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c
index 8dc87ed2be4c..7d0281f5447e 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -795,7 +795,7 @@ static init_fnc_t init_sequence_r[] = {
 	initr_mem,
 #endif
 #ifdef CONFIG_EFI_SETUP_EARLY
-	(init_fnc_t)efi_init_obj_list,
+	efi_init_early,
 #endif
 	run_main_loop,
 };
diff --git a/common/main.c b/common/main.c
index 3f5214fd44b8..682f3359ea38 100644
--- a/common/main.c
+++ b/common/main.c
@@ -54,8 +54,11 @@ void main_loop(void)
 	if (IS_ENABLED(CONFIG_UPDATE_TFTP))
 		update_tftp(0UL, NULL, NULL);
 
-	if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY))
-		efi_launch_capsules();
+	if (IS_ENABLED(CONFIG_EFI_CAPSULE_ON_DISK_EARLY)) {
+		/* efi_init_early() already called */
+		if (efi_init_obj_list() == EFI_SUCCESS)
+			efi_launch_capsules();
+	}
 
 	s = bootdelay_process();
 	if (cli_process_fdt(&s))
diff --git a/include/efi_loader.h b/include/efi_loader.h
index af36639ec6a7..3dedb2c5299d 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -493,6 +493,8 @@ struct efi_register_notify_event {
 /* List of all events registered by RegisterProtocolNotify() */
 extern struct list_head efi_register_notify_events;
 
+/* called at pre-initialization */
+int efi_init_early(void);
 /* Initialize efi execution environment */
 efi_status_t efi_init_obj_list(void);
 /* Install device tree */
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index eee54e48784f..de2f34bab537 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -174,20 +174,18 @@ static efi_status_t efi_init_os_indications(void)
 				    &os_indications_supported, false);
 }
 
-
 /**
- * efi_init_obj_list() - Initialize and populate EFI object list
+ * __efi_init_early() - handle initialization at early stage
+ *
+ * This function is called in efi_init_obj_list() only if
+ * !CONFIG_EFI_SETUP_EARLY.
  *
  * Return:	status code
  */
-efi_status_t efi_init_obj_list(void)
+static efi_status_t __efi_init_early(void)
 {
 	efi_status_t ret = EFI_SUCCESS;
 
-	/* Initialize once only */
-	if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
-		return efi_obj_list_initialized;
-
 	/* Allow unaligned memory access */
 	allow_unaligned();
 
@@ -202,9 +200,51 @@ efi_status_t efi_init_obj_list(void)
 
 #ifdef CONFIG_PARTITIONS
 	ret = efi_disk_register();
-	if (ret != EFI_SUCCESS)
-		goto out;
 #endif
+out:
+	return ret;
+}
+
+/**
+ * efi_init_early() - handle initialization at early stage
+ *
+ * external version of __efi_init_early(); expected to be called in
+ * board_init_r().
+ *
+ * Return:	status code
+ */
+int efi_init_early(void)
+{
+	efi_status_t ret;
+
+	ret = __efi_init_early();
+	if (ret != EFI_SUCCESS) {
+		/* never re-init UEFI subsystem */
+		efi_obj_list_initialized = ret;
+		return -1;
+	}
+	return 0;
+}
+
+/**
+ * efi_init_obj_list() - Initialize and populate EFI object list
+ *
+ * Return:	status code
+ */
+efi_status_t efi_init_obj_list(void)
+{
+	efi_status_t ret = EFI_SUCCESS;
+
+	/* Initialize once only */
+	if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
+		return efi_obj_list_initialized;
+
+	if (!IS_ENABLED(CONFIG_EFI_SETUP_EARLY)) {
+		ret = __efi_init_early();
+		if (ret != EFI_SUCCESS)
+			goto out;
+	}
+
 	if (IS_ENABLED(CONFIG_EFI_RNG_PROTOCOL)) {
 		ret = efi_rng_register();
 		if (ret != EFI_SUCCESS)
-- 
2.33.0

[PATCH v4 07/12] efi_loader: disk: a helper function to create efi_disk objects from udevice

[AKASHI Takahiro]

Add efi_disk_probe() function.
This function creates an efi_disk object for a raw disk device (UCLASS_BLK)
and additional objects for related partitions (UCLASS_PARTITION).

So this function is expected to be called through driver model's "probe"
interface every time one raw disk device is detected and activated.
We assume that partition devices (UCLASS_PARTITION) have been created
when this function is invoked.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 include/efi_loader.h              |   4 +-
 lib/efi_driver/efi_block_device.c |  34 ++---
 lib/efi_loader/Kconfig            |   3 +
 lib/efi_loader/efi_disk.c         | 201 +++++++++++++++++++-----------
 lib/efi_loader/efi_setup.c        |   4 +-
 5 files changed, 143 insertions(+), 103 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 3dedb2c5299d..f946e74c93cf 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -525,8 +525,8 @@ void efi_carve_out_dt_rsv(void *fdt);
 void efi_try_purge_kaslr_seed(void *fdt);
 /* Called by bootefi to make console interface available */
 efi_status_t efi_console_register(void);
-/* Called by bootefi to make all disk storage accessible as EFI objects */
-efi_status_t efi_disk_register(void);
+/* Called by efi_init_obj_list() to initialize efi_disks */
+efi_status_t efi_disk_init(void);
 /* Called by efi_init_obj_list() to install EFI_RNG_PROTOCOL */
 efi_status_t efi_rng_register(void);
 /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
diff --git a/lib/efi_driver/efi_block_device.c b/lib/efi_driver/efi_block_device.c
index 04cb3ef0d4e5..5baa6f87a375 100644
--- a/lib/efi_driver/efi_block_device.c
+++ b/lib/efi_driver/efi_block_device.c
@@ -35,6 +35,7 @@
 #include <malloc.h>
 #include <dm/device-internal.h>
 #include <dm/root.h>
+#include <dm/tag.h>
 
 /*
  * EFI attributes of the udevice handled by this driver.
@@ -106,25 +107,6 @@ static ulong efi_bl_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
 	return blkcnt;
 }
 
-/**
- * Create partions for the block device.
- *
- * @handle:	EFI handle of the block device
- * @dev:	udevice of the block device
- * Return:	number of partitions created
- */
-static int efi_bl_bind_partitions(efi_handle_t handle, struct udevice *dev)
-{
-	struct blk_desc *desc;
-	const char *if_typename;
-
-	desc = dev_get_uclass_plat(dev);
-	if_typename = blk_get_if_type_name(desc->if_type);
-
-	return efi_disk_create_partitions(handle, desc, if_typename,
-					  desc->devnum, dev->name);
-}
-
 /**
  * Create a block device for a handle
  *
@@ -139,7 +121,6 @@ static int efi_bl_bind(efi_handle_t handle, void *interface)
 	char *name;
 	struct efi_object *obj = efi_search_obj(handle);
 	struct efi_block_io *io = interface;
-	int disks;
 	struct efi_blk_plat *plat;
 
 	EFI_PRINT("%s: handle %p, interface %p\n", __func__, handle, io);
@@ -173,15 +154,20 @@ 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().
+	 */
+	ret = dev_tag_set_ptr(bdev, DM_TAG_EFI, handle);
+	if (ret)
+		/* FIXME: cleanup for bdev */
+		return ret;
+
 	ret = device_probe(bdev);
 	if (ret)
 		return ret;
 	EFI_PRINT("%s: block device '%s' created\n", __func__, bdev->name);
 
-	/* Create handles for the partions of the block device */
-	disks = efi_bl_bind_partitions(handle, bdev);
-	EFI_PRINT("Found %d partitions\n", disks);
-
 	return 0;
 }
 
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 559b95a599b9..7dc24fbf0aa9 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -14,6 +14,8 @@ config EFI_LOADER
 	depends on DM_ETH || !NET
 	depends on !EFI_APP
 	default y if !ARM || SYS_CPU = armv7 || SYS_CPU = armv8
+	select DM_EVENT
+	select EVENT_DYNAMIC
 	select LIB_UUID
 	select PARTITION_UUIDS
 	select HAVE_BLOCK_DEVICE
@@ -40,6 +42,7 @@ config CMD_BOOTEFI_BOOTMGR
 
 config EFI_SETUP_EARLY
 	bool
+	default y
 
 choice
 	prompt "Store for non-volatile UEFI variables"
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index c905c12abc2f..d4a0edb458b8 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -10,6 +10,9 @@
 #include <common.h>
 #include <blk.h>
 #include <dm.h>
+#include <dm/device-internal.h>
+#include <dm/tag.h>
+#include <event.h>
 #include <efi_loader.h>
 #include <fs.h>
 #include <log.h>
@@ -487,103 +490,153 @@ error:
 	return ret;
 }
 
-/**
- * efi_disk_create_partitions() - create handles and protocols for partitions
+/*
+ * Create a handle for a whole raw disk
+ *
+ * @dev		uclass device (UCLASS_BLK)
  *
- * Create handles and protocols for the partitions of a block device.
+ * Create an efi_disk object which is associated with @dev.
+ * The type of @dev must be UCLASS_BLK.
  *
- * @parent:		handle of the parent disk
- * @desc:		block device
- * @if_typename:	interface type
- * @diskid:		device number
- * @pdevname:		device name
- * Return:		number of partitions created
+ * @return	0 on success, -1 otherwise
  */
-int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
-			       const char *if_typename, int diskid,
-			       const char *pdevname)
+static int efi_disk_create_raw(struct udevice *dev)
 {
-	int disks = 0;
-	char devname[32] = { 0 }; /* dp->str is u16[32] long */
-	int part;
-	struct efi_device_path *dp = NULL;
+	struct efi_disk_obj *disk;
+	struct blk_desc *desc;
+	const char *if_typename;
+	int diskid;
 	efi_status_t ret;
-	struct efi_handler *handler;
 
-	/* Get the device path of the parent */
-	ret = efi_search_protocol(parent, &efi_guid_device_path, &handler);
-	if (ret == EFI_SUCCESS)
-		dp = handler->protocol_interface;
-
-	/* Add devices for each partition */
-	for (part = 1; part <= MAX_SEARCH_PARTITIONS; part++) {
-		struct disk_partition info;
-
-		if (part_get_info(desc, part, &info))
-			continue;
-		snprintf(devname, sizeof(devname), "%s:%x", pdevname,
-			 part);
-		ret = efi_disk_add_dev(parent, dp, if_typename, desc, diskid,
-				       &info, part, NULL);
-		if (ret != EFI_SUCCESS) {
-			log_err("Adding partition %s failed\n", pdevname);
-			continue;
-		}
-		disks++;
+	desc = dev_get_uclass_plat(dev);
+	if_typename = blk_get_if_type_name(desc->if_type);
+	diskid = desc->devnum;
+
+	ret = efi_disk_add_dev(NULL, NULL, if_typename, desc,
+			       diskid, NULL, 0, &disk);
+	if (ret != EFI_SUCCESS) {
+		if (ret == EFI_NOT_READY)
+			log_notice("Disk %s not ready\n", dev->name);
+		else
+			log_err("Adding disk for %s failed\n", dev->name);
+
+		return -1;
+	}
+	if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) {
+		efi_free_pool(disk->dp);
+		efi_delete_handle(&disk->header);
+
+		return -1;
 	}
 
-	return disks;
+	return 0;
 }
 
-/**
- * efi_disk_register() - register block devices
- *
- * U-Boot doesn't have a list of all online disk devices. So when running our
- * EFI payload, we scan through all of the potentially available ones and
- * store them in our object pool.
+/*
+ * Create a handle for a disk partition
  *
- * This function is called in efi_init_obj_list().
+ * @dev		uclass device (UCLASS_PARTITION)
  *
- * TODO(sjg@chromium.org): Actually with CONFIG_BLK, U-Boot does have this.
- * Consider converting the code to look up devices as needed. The EFI device
- * could be a child of the UCLASS_BLK block device, perhaps.
+ * Create an efi_disk object which is associated with @dev.
+ * The type of @dev must be UCLASS_PARTITION.
  *
- * Return:	status code
+ * @return	0 on success, -1 otherwise
  */
-efi_status_t efi_disk_register(void)
+static int efi_disk_create_part(struct udevice *dev)
 {
+	efi_handle_t parent;
+	struct blk_desc *desc;
+	const char *if_typename;
+	struct disk_part *part_data;
+	struct disk_partition *info;
+	unsigned int part;
+	int diskid;
+	struct efi_handler *handler;
+	struct efi_device_path *dp_parent;
 	struct efi_disk_obj *disk;
-	int disks = 0;
 	efi_status_t ret;
+
+	if (dev_tag_get_ptr(dev_get_parent(dev), DM_TAG_EFI, (void **)&parent))
+		return -1;
+
+	desc = dev_get_uclass_plat(dev_get_parent(dev));
+	if_typename = blk_get_if_type_name(desc->if_type);
+	diskid = desc->devnum;
+
+	part_data = dev_get_uclass_plat(dev);
+	part = part_data->partnum;
+	info = &part_data->gpt_part_info;
+
+	ret = efi_search_protocol(parent, &efi_guid_device_path, &handler);
+	if (ret != EFI_SUCCESS)
+		return -1;
+	dp_parent = (struct efi_device_path *)handler->protocol_interface;
+
+	ret = efi_disk_add_dev(parent, dp_parent, if_typename, desc, diskid,
+			       info, part, &disk);
+	if (ret != EFI_SUCCESS) {
+		log_err("Adding partition for %s failed\n", dev->name);
+		return -1;
+	}
+	if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) {
+		efi_free_pool(disk->dp);
+		efi_delete_handle(&disk->header);
+
+		return -1;
+	}
+
+	return 0;
+}
+
+/*
+ * Create efi_disk objects for a block device
+ *
+ * @dev		uclass device (UCLASS_BLK)
+ *
+ * Create efi_disk objects for partitions as well as a raw disk
+ * which is associated with @dev.
+ * The type of @dev must be UCLASS_BLK.
+ * This function is expected to be called at EV_PM_POST_PROBE.
+ *
+ * @return	0 on success, -1 otherwise
+ */
+static int efi_disk_probe(void *ctx, struct event *event)
+{
 	struct udevice *dev;
+	enum uclass_id id;
+	struct udevice *child;
+	int ret;
 
-	for (uclass_first_device_check(UCLASS_BLK, &dev); dev;
-	     uclass_next_device_check(&dev)) {
-		struct blk_desc *desc = dev_get_uclass_plat(dev);
-		const char *if_typename = blk_get_if_type_name(desc->if_type);
+	dev = event->data.dm.dev;
+	id = device_get_uclass_id(dev);
 
-		/* Add block device for the full device */
-		log_info("Scanning disk %s...\n", dev->name);
-		ret = efi_disk_add_dev(NULL, NULL, if_typename,
-					desc, desc->devnum, NULL, 0, &disk);
-		if (ret == EFI_NOT_READY) {
-			log_notice("Disk %s not ready\n", dev->name);
-			continue;
-		}
-		if (ret) {
-			log_err("ERROR: failure to add disk device %s, r = %lu\n",
-				dev->name, ret & ~EFI_ERROR_MASK);
-			continue;
-		}
-		disks++;
+	/* TODO: We won't support partitions in a partition */
+	if (id != UCLASS_BLK)
+		return 0;
+
+	ret = efi_disk_create_raw(dev);
+	if (ret)
+		return -1;
 
-		/* Partitions show up as block devices in EFI */
-		disks += efi_disk_create_partitions(
-					&disk->header, desc, if_typename,
-					desc->devnum, dev->name);
+	device_foreach_child(child, dev) {
+		ret = efi_disk_create_part(child);
+		if (ret)
+			return -1;
 	}
 
-	log_info("Found %d disks\n", disks);
+	return 0;
+}
+
+efi_status_t efi_disk_init(void)
+{
+	int ret;
+
+	ret = event_register("efi_disk add", EVT_DM_POST_PROBE,
+			     efi_disk_probe, NULL);
+	if (ret) {
+		log_err("Event registration for efi_disk add failed\n");
+		return EFI_OUT_OF_RESOURCES;
+	}
 
 	return EFI_SUCCESS;
 }
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index de2f34bab537..250eeb2fcd6b 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -198,9 +198,7 @@ static efi_status_t __efi_init_early(void)
 	if (ret != EFI_SUCCESS)
 		goto out;
 
-#ifdef CONFIG_PARTITIONS
-	ret = efi_disk_register();
-#endif
+	ret = efi_disk_init();
 out:
 	return ret;
 }
-- 
2.33.0

[PATCH v4 08/12] efi_loader: disk: not create BLK device for BLK(IF_TYPE_EFI_LOADER) devices

[AKASHI Takahiro]

When we create an efi_disk device with an UEFI application using driver
binding protocol, the 'efi_driver' framework tries to create
a corresponding block device(UCLASS_BLK/IF_TYPE_EFI). This will lead to
calling a PROBE callback, efi_disk_probe().
In this case, however, we don't need to create another "efi_disk" device
as we already have this device instance.

So we should avoid recursively invoke further processing in the callback
function.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 lib/efi_loader/efi_disk.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index d4a0edb458b8..037f8594a727 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -604,6 +604,7 @@ static int efi_disk_probe(void *ctx, struct event *event)
 {
 	struct udevice *dev;
 	enum uclass_id id;
+	struct blk_desc *desc;
 	struct udevice *child;
 	int ret;
 
@@ -614,9 +615,16 @@ static int efi_disk_probe(void *ctx, struct event *event)
 	if (id != UCLASS_BLK)
 		return 0;
 
-	ret = efi_disk_create_raw(dev);
-	if (ret)
-		return -1;
+	/*
+	 * 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_LOADER) {
+		ret = efi_disk_create_raw(dev);
+		if (ret)
+			return -1;
+	}
 
 	device_foreach_child(child, dev) {
 		ret = efi_disk_create_part(child);
-- 
2.33.0

[PATCH v4 09/12] efi_loader: disk: a helper function to delete efi_disk objects

[AKASHI Takahiro]

This function is expected to be called, in particular from dm's pre_remove
hook, when associated block devices no longer exist.

Add efi_disk_remove() function.
This function removes an efi_disk object for a raw disk device (UCLASS_BLK)
and related objects for its partitions (UCLASS_PARTITION).

So this function is expected to be called through driver model's "remove"
interface every time a raw disk device is to be disconnected.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 lib/efi_loader/efi_disk.c | 88 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 037f8594a727..940390ac8bcc 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -635,6 +635,87 @@ static int efi_disk_probe(void *ctx, struct event *event)
 	return 0;
 }
 
+/*
+ * Delete an efi_disk object for a whole raw disk
+ *
+ * @dev		uclass device (UCLASS_BLK)
+ *
+ * Delete an efi_disk object which is associated with @dev.
+ * The type of @dev must be UCLASS_BLK.
+ *
+ * @return	0 on success, -1 otherwise
+ */
+static int efi_disk_delete_raw(struct udevice *dev)
+{
+	efi_handle_t handle;
+	struct efi_disk_obj *diskobj;
+
+	if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle))
+		return -1;
+
+	diskobj = container_of(handle, struct efi_disk_obj, header);
+	efi_free_pool(diskobj->dp);
+
+	efi_delete_handle(handle);
+	dev_tag_del(dev, DM_TAG_EFI);
+
+	return 0;
+}
+
+/*
+ * Delete an efi_disk object for a disk partition
+ *
+ * @dev		uclass device (UCLASS_PARTITION)
+ *
+ * Delete an efi_disk object which is associated with @dev.
+ * The type of @dev must be UCLASS_PARTITION.
+ *
+ * @return	0 on success, -1 otherwise
+ */
+static int efi_disk_delete_part(struct udevice *dev)
+{
+	efi_handle_t handle;
+	struct efi_disk_obj *diskobj;
+
+	if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle))
+		return -1;
+
+	diskobj = container_of(handle, struct efi_disk_obj, header);
+
+	efi_free_pool(diskobj->dp);
+	efi_delete_handle(handle);
+	dev_tag_del(dev, DM_TAG_EFI);
+
+	return 0;
+}
+
+/*
+ * Delete an efi_disk object for a block device
+ *
+ * @dev		uclass device (UCLASS_BLK or UCLASS_PARTITION)
+ *
+ * Delete an efi_disk object which is associated with @dev.
+ * The type of @dev must be either UCLASS_BLK or UCLASS_PARTITION.
+ * This function is expected to be called at EV_PM_PRE_REMOVE.
+ *
+ * @return	0 on success, -1 otherwise
+ */
+static int efi_disk_remove(void *ctx, struct event *event)
+{
+	enum uclass_id id;
+	struct udevice *dev;
+
+	dev = event->data.dm.dev;
+	id = device_get_uclass_id(dev);
+
+	if (id == UCLASS_BLK)
+		return efi_disk_delete_raw(dev);
+	else if (id == UCLASS_PARTITION)
+		return efi_disk_delete_part(dev);
+	else
+		return 0;
+}
+
 efi_status_t efi_disk_init(void)
 {
 	int ret;
@@ -646,5 +727,12 @@ efi_status_t efi_disk_init(void)
 		return EFI_OUT_OF_RESOURCES;
 	}
 
+	ret = event_register("efi_disk del", EVT_DM_PRE_REMOVE,
+			     efi_disk_remove, NULL);
+	if (ret) {
+		log_err("Event registration for efi_disk del failed\n");
+		return EFI_OUT_OF_RESOURCES;
+	}
+
 	return EFI_SUCCESS;
 }
-- 
2.33.0

[PATCH v4 10/12] efi_loader: disk: not delete BLK device for BLK(IF_TYPE_EFI_LOADER) devices

[AKASHI Takahiro]

When we create an efi_disk device with an UEFI application using driver
binding protocol, the 'efi_driver' framework tries to create
a corresponding block device(UCLASS_BLK/IF_TYPE_EFI). This will lead to
calling a PROBE callback, efi_disk_probe().
In this case, however, we don't need to create another "efi_disk" device
as we already have this device instance.

So we should avoid recursively invoke further processing in the callback
function.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 lib/efi_loader/efi_disk.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 940390ac8bcc..34bb662e9cc0 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -648,13 +648,17 @@ static int efi_disk_probe(void *ctx, struct event *event)
 static int efi_disk_delete_raw(struct udevice *dev)
 {
 	efi_handle_t handle;
+	struct blk_desc *desc;
 	struct efi_disk_obj *diskobj;
 
 	if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **)&handle))
 		return -1;
 
-	diskobj = container_of(handle, struct efi_disk_obj, header);
-	efi_free_pool(diskobj->dp);
+	desc = dev_get_uclass_plat(dev);
+	if (desc->if_type != IF_TYPE_EFI_LOADER) {
+		diskobj = container_of(handle, struct efi_disk_obj, header);
+		efi_free_pool(diskobj->dp);
+	}
 
 	efi_delete_handle(handle);
 	dev_tag_del(dev, DM_TAG_EFI);
-- 
2.33.0

[PATCH v4 11/12] dm: disk: add read/write interfaces with udevice

[AKASHI Takahiro]

In include/blk.h, Simon suggested:
===>
/*
 * These functions should take struct udevice instead of struct blk_desc,
 * but this is convenient for migration to driver model. Add a 'd' prefix
 * to the function operations, so that blk_read(), etc. can be reserved for
 * functions with the correct arguments.
 */
unsigned long blk_dread(struct blk_desc *block_dev, lbaint_t start,
                        lbaint_t blkcnt, void *buffer);
unsigned long blk_dwrite(struct blk_desc *block_dev, lbaint_t start,
                         lbaint_t blkcnt, const void *buffer);
unsigned long blk_derase(struct blk_desc *block_dev, lbaint_t start,
                         lbaint_t blkcnt);
<===

So new interfaces are provided with this patch.

They are expected to be used everywhere in U-Boot at the end.
The exceptions are block device drivers, partition drivers and efi_disk
which should know details of blk_desc structure.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 disk/disk-uclass.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++
 include/part.h     |  7 ++++
 2 files changed, 101 insertions(+)

diff --git a/disk/disk-uclass.c b/disk/disk-uclass.c
index 4918a2f72d1e..72ff62ebf581 100644
--- a/disk/disk-uclass.c
+++ b/disk/disk-uclass.c
@@ -146,6 +146,100 @@ U_BOOT_DRIVER(blk_partition) = {
 	.ops		= &blk_part_ops,
 };
 
+/*
+ * BLOCK IO APIs
+ */
+static struct blk_desc *dev_get_blk(struct udevice *dev)
+{
+	struct blk_desc *block_dev;
+
+	switch (device_get_uclass_id(dev)) {
+	/*
+	 * We won't support UCLASS_BLK with dev_* interfaces.
+	 */
+	case UCLASS_PARTITION:
+		block_dev = dev_get_uclass_plat(dev_get_parent(dev));
+		break;
+	default:
+		block_dev = NULL;
+		break;
+	}
+
+	return block_dev;
+}
+
+unsigned long dev_read(struct udevice *dev, lbaint_t start,
+		       lbaint_t blkcnt, void *buffer)
+{
+	struct blk_desc *block_dev;
+	const struct blk_ops *ops;
+	struct disk_part *part;
+	lbaint_t start_in_disk;
+	ulong blks_read;
+
+	block_dev = dev_get_blk(dev);
+	if (!block_dev)
+		return -ENOSYS;
+
+	ops = blk_get_ops(dev);
+	if (!ops->read)
+		return -ENOSYS;
+
+	start_in_disk = start;
+	if (device_get_uclass_id(dev) == UCLASS_PARTITION) {
+		part = dev_get_uclass_plat(dev);
+		start_in_disk += part->gpt_part_info.start;
+	}
+
+	if (blkcache_read(block_dev->if_type, block_dev->devnum,
+			  start_in_disk, blkcnt, block_dev->blksz, buffer))
+		return blkcnt;
+	blks_read = ops->read(dev, start, blkcnt, buffer);
+	if (blks_read == blkcnt)
+		blkcache_fill(block_dev->if_type, block_dev->devnum,
+			      start_in_disk, blkcnt, block_dev->blksz, buffer);
+
+	return blks_read;
+}
+
+unsigned long dev_write(struct udevice *dev, lbaint_t start,
+			lbaint_t blkcnt, const void *buffer)
+{
+	struct blk_desc *block_dev;
+	const struct blk_ops *ops;
+
+	block_dev = dev_get_blk(dev);
+	if (!block_dev)
+		return -ENOSYS;
+
+	ops = blk_get_ops(dev);
+	if (!ops->write)
+		return -ENOSYS;
+
+	blkcache_invalidate(block_dev->if_type, block_dev->devnum);
+
+	return ops->write(dev, start, blkcnt, buffer);
+}
+
+unsigned long dev_erase(struct udevice *dev, lbaint_t start,
+			lbaint_t blkcnt)
+{
+	struct blk_desc *block_dev;
+	const struct blk_ops *ops;
+
+	block_dev = dev_get_blk(dev);
+	if (!block_dev)
+		return -ENOSYS;
+
+	ops = blk_get_ops(dev);
+	if (!ops->erase)
+		return -ENOSYS;
+
+	blkcache_invalidate(block_dev->if_type, block_dev->devnum);
+
+	return ops->erase(dev, start, blkcnt);
+}
+
 UCLASS_DRIVER(partition) = {
 	.id		= UCLASS_PARTITION,
 	.per_device_plat_auto	= sizeof(struct disk_part),
diff --git a/include/part.h b/include/part.h
index 907c227aa20d..36b76f00563f 100644
--- a/include/part.h
+++ b/include/part.h
@@ -297,6 +297,13 @@ struct udevice;
  */
 int part_create_block_devices(struct udevice *blk_dev);
 
+unsigned long dev_read(struct udevice *dev, lbaint_t start,
+		       lbaint_t blkcnt, void *buffer);
+unsigned long dev_write(struct udevice *dev, lbaint_t start,
+			lbaint_t blkcnt, const void *buffer);
+unsigned long dev_erase(struct udevice *dev, lbaint_t start,
+			lbaint_t blkcnt);
+
 /*
  * We don't support printing partition information in SPL and only support
  * getting partition information in a few cases.
-- 
2.33.0

[PATCH v4 12/12] efi_loader: disk: use udevice instead of blk_desc

[AKASHI Takahiro]

In most of all cases, we can avoid using blk_desc which is expected
to be private to udevice(UCLASS_BLK), that is, the data should not
be manipulated outside the device driver unless really needed.

Now efi_disk's internally use dev_read/write() interfaces.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 lib/efi_loader/efi_disk.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 34bb662e9cc0..8ef493cc972e 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -36,7 +36,7 @@ const efi_guid_t efi_system_partition_guid = PARTITION_SYSTEM_GUID;
  * @part:	partition
  * @volume:	simple file system protocol of the partition
  * @offset:	offset into disk for simple partition
- * @desc:	internal block device descriptor
+ * @dev:	associated DM device
  */
 struct efi_disk_obj {
 	struct efi_object header;
@@ -48,7 +48,7 @@ struct efi_disk_obj {
 	unsigned int part;
 	struct efi_simple_file_system_protocol *volume;
 	lbaint_t offset;
-	struct blk_desc *desc;
+	struct udevice *dev; /* TODO: move it to efi_object */
 };
 
 /**
@@ -83,14 +83,12 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
 			void *buffer, enum efi_disk_direction direction)
 {
 	struct efi_disk_obj *diskobj;
-	struct blk_desc *desc;
 	int blksz;
 	int blocks;
 	unsigned long n;
 
 	diskobj = container_of(this, struct efi_disk_obj, ops);
-	desc = (struct blk_desc *) diskobj->desc;
-	blksz = desc->blksz;
+	blksz = diskobj->media.block_size;
 	blocks = buffer_size / blksz;
 	lba += diskobj->offset;
 
@@ -102,9 +100,9 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
 		return EFI_BAD_BUFFER_SIZE;
 
 	if (direction == EFI_DISK_READ)
-		n = blk_dread(desc, lba, blocks, buffer);
+		n = dev_read(diskobj->dev, lba, blocks, buffer);
 	else
-		n = blk_dwrite(desc, lba, blocks, buffer);
+		n = dev_write(diskobj->dev, lba, blocks, buffer);
 
 	/* We don't do interrupts, so check for timers cooperatively */
 	efi_timer_check();
@@ -446,7 +444,6 @@ static efi_status_t efi_disk_add_dev(
 	diskobj->ops = block_io_disk_template;
 	diskobj->ifname = if_typename;
 	diskobj->dev_index = dev_index;
-	diskobj->desc = desc;
 
 	/* Fill in EFI IO Media info (for read/write callbacks) */
 	diskobj->media.removable_media = desc->removable;
@@ -522,6 +519,7 @@ static int efi_disk_create_raw(struct udevice *dev)
 
 		return -1;
 	}
+	disk->dev = dev;
 	if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) {
 		efi_free_pool(disk->dp);
 		efi_delete_handle(&disk->header);
@@ -578,6 +576,7 @@ static int efi_disk_create_part(struct udevice *dev)
 		log_err("Adding partition for %s failed\n", dev->name);
 		return -1;
 	}
+	disk->dev = dev;
 	if (dev_tag_set_ptr(dev, DM_TAG_EFI, &disk->header)) {
 		efi_free_pool(disk->dp);
 		efi_delete_handle(&disk->header);
-- 
2.33.0

Re: [PATCH v4 01/12] dm: tag: change ENOSPC to ENOMEM

[Heinrich Schuchardt]

On 4/15/22 09:15, AKASHI Takahiro wrote:
> ENOMEM is a more common error code for memory starvation.
>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

> ---
>   drivers/core/tag.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/core/tag.c b/drivers/core/tag.c
> index 6829bcd8806c..22999193a5a3 100644
> --- a/drivers/core/tag.c
> +++ b/drivers/core/tag.c
> @@ -29,7 +29,7 @@ int dev_tag_set_ptr(struct udevice *dev, enum dm_tag_t tag, void *ptr)
>
>   	node = calloc(sizeof(*node), 1);
>   	if (!node)
> -		return -ENOSPC;
> +		return -ENOMEM;
>
>   	node->dev = dev;
>   	node->tag = tag;
> @@ -53,7 +53,7 @@ int dev_tag_set_val(struct udevice *dev, enum dm_tag_t tag, ulong val)
>
>   	node = calloc(sizeof(*node), 1);
>   	if (!node)
> -		return -ENOSPC;
> +		return -ENOMEM;
>
>   	node->dev = dev;
>   	node->tag = tag;

Re: [PATCH v4 01/12] dm: tag: change ENOSPC to ENOMEM

[Ilias Apalodimas]

On Fri, 15 Apr 2022 at 10:59, Heinrich Schuchardt <xypron.glpk@gmx.de>
wrote:

> On 4/15/22 09:15, AKASHI Takahiro wrote:
> > ENOMEM is a more common error code for memory starvation.
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>
> > ---
> >   drivers/core/tag.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/core/tag.c b/drivers/core/tag.c
> > index 6829bcd8806c..22999193a5a3 100644
> > --- a/drivers/core/tag.c
> > +++ b/drivers/core/tag.c
> > @@ -29,7 +29,7 @@ int dev_tag_set_ptr(struct udevice *dev, enum dm_tag_t
> tag, void *ptr)
> >
> >       node = calloc(sizeof(*node), 1);
> >       if (!node)
> > -             return -ENOSPC;
> > +             return -ENOMEM;
> >
> >       node->dev = dev;
> >       node->tag = tag;
> > @@ -53,7 +53,7 @@ int dev_tag_set_val(struct udevice *dev, enum dm_tag_t
> tag, ulong val)
> >
> >       node = calloc(sizeof(*node), 1);
> >       if (!node)
> > -             return -ENOSPC;
> > +             return -ENOMEM;
> >
> >       node->dev = dev;
> >       node->tag = tag;
>


Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>