[U-Boot] [PATCH 0/4] efi_loader: fix disk objects + device-paths

[U-Boot] [PATCH 0/4] efi_loader: fix disk objects + device-paths

[Rob Clark]

Currently u-boot completely gets block-io objects (and their associated
device-paths) wrong, which prevents grub2 from figuring out the boot
device+partition, which prevents it from automatically finding it's
grub.cfg.

Currently efi_loader creates a block-io object per disk, for example on
a board with 3 partitions on an sd-card and 4 partitions on a usb disk:

  /File(sdhci at 07864000.blk)/EndEntire
  /File(usb_mass_storage.lun0)/EndEntire

at least grub manages to match up the loaded_image device-path, but it
cannot figure out which partition it was on.  So you end up with
prefix=(hd1)/EFI/fedora instead of prefix=(hd1,msdos1)/EFI/fedora.
With this patchset, you end up with:

  /ACPI(133741d0,0)/UnknownMessaging(1d)/EndEntire
  /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(0,800,64000,0000000000000000,1,0)/EndEntire
  /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,200000,0000000000000000,1,0)/EndEntire
  /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,0000000000000000,1,0)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(0,800,60000,0000000000000000,1,0)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,0000000000000000,1,0)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,0000000000000000,1,0)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,0000000000000000,1,0)/EndEntire

(Note that "UnknownMessaging(1d)" is actually the MMC node in the device-
path, but grub wasn't updated to print out some of the newer device-path
sub-types.)

Since we use DM to generate the device-paths from u-boot's device
hierarchy, legacy devices will still be broken.  I kept them working
as they did previously and tried to minimize the ifdeffery, but things
will get much cleaner when legacy support is removed.

Peter Jones (1):
  efi: add some more device path structures

Rob Clark (3):
  efi_loader: add udevice to EFI device-path mapping
  efi_loader: make disk objects for partitions
  efi_loader: use efi_devpath to get correct boot device-path

 cmd/bootefi.c                |  31 ++++++--
 include/efi_api.h            |  49 +++++++++++-
 include/efi_loader.h         |   9 +++
 lib/efi_loader/Makefile      |   1 +
 lib/efi_loader/efi_devpath.c | 175 +++++++++++++++++++++++++++++++++++++++++++
 lib/efi_loader/efi_disk.c    |  73 +++++++++++++-----
 6 files changed, 311 insertions(+), 27 deletions(-)
 create mode 100644 lib/efi_loader/efi_devpath.c

-- 
2.13.0

[U-Boot] [PATCH 1/4] efi: add some more device path structures

[Rob Clark]

From: Peter Jones <pjones@redhat.com>

Signed-off-by: Peter Jones <pjones@redhat.com>
---
 include/efi_api.h | 49 +++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 47 insertions(+), 2 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index f071b36b53..e1b3f11c6d 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -275,22 +275,67 @@ struct efi_mac_addr {
 	u8 addr[32];
 };
 
+#define DEVICE_PATH_TYPE_ACPI_DEVICE		0x02
+#define DEVICE_PATH_SUB_TYPE_ACPI_DEVICE	0x01
+
+#define EFI_PNP_ID(ID)				(u32)(((ID) << 16) | 0x41D0)
+#define EISA_PNP_ID(ID)				EFI_PNP_ID(ID)
+
+struct efi_device_path_acpi_path {
+	struct efi_device_path dp;
+	u32 hid;
+	u32 uid;
+} __packed;
+
 #define DEVICE_PATH_TYPE_MESSAGING_DEVICE	0x03
+#  define DEVICE_PATH_SUB_TYPE_MSG_USB		0x05
 #  define DEVICE_PATH_SUB_TYPE_MSG_MAC_ADDR	0x0b
+#  define DEVICE_PATH_SUB_TYPE_MSG_SD		0x1a
+#  define DEVICE_PATH_SUB_TYPE_MSG_MMC		0x1d
+
+struct efi_device_path_usb {
+	struct efi_device_path dp;
+	u8 parent_port_number;
+	u8 usb_interface;
+} __packed;
 
 struct efi_device_path_mac_addr {
 	struct efi_device_path dp;
 	struct efi_mac_addr mac;
 	u8 if_type;
-};
+} __packed;
+
+struct efi_device_path_sd_mmc_path {
+	struct efi_device_path dp;
+	u8 slot_number;
+} __packed;
 
 #define DEVICE_PATH_TYPE_MEDIA_DEVICE		0x04
+#  define DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH	0x01
+#  define DEVICE_PATH_SUB_TYPE_CDROM_PATH	0x02
 #  define DEVICE_PATH_SUB_TYPE_FILE_PATH	0x04
 
+struct efi_device_path_hard_drive_path {
+	struct efi_device_path dp;
+	u32 partition_number;
+	u64 partition_start;
+	u64 partition_end;
+	u8 partition_signature[16];
+	u8 partmap_type;
+	u8 signature_type;
+} __packed;
+
+struct efi_device_path_cdrom_path {
+	struct efi_device_path dp;
+	u32 boot_entry;
+	u64 partition_start;
+	u64 partition_end;
+} __packed;
+
 struct efi_device_path_file_path {
 	struct efi_device_path dp;
 	u16 str[32];
-};
+} __packed;
 
 #define BLOCK_IO_GUID \
 	EFI_GUID(0x964e5b21, 0x6459, 0x11d2, \
-- 
2.13.0

[U-Boot] [PATCH 2/4] efi_loader: add udevice to EFI device-path mapping

[Rob Clark]

Initially just supports "disk" type devices, but a real UEFI
implementation would expose input/output/display/etc with device-paths
as well, so we may eventually want to expand this for other uses.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 include/efi_loader.h         |   9 +++
 lib/efi_loader/Makefile      |   1 +
 lib/efi_loader/efi_devpath.c | 175 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 185 insertions(+)
 create mode 100644 lib/efi_loader/efi_devpath.c

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 812ec10e37..97d5e3d13d 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -145,6 +145,15 @@ extern void *efi_bounce_buffer;
 #define EFI_LOADER_BOUNCE_BUFFER_SIZE (64 * 1024 * 1024)
 #endif
 
+/*
+ * u-boot to EFI device mapping:
+ *
+ * TODO extend this for GOP and various other input/output
+ * devices which should also have an EFI devicepath?
+ */
+struct efi_device_path *efi_dp_from_dev(struct udevice *dev);
+struct efi_device_path *efi_dp_from_part(struct blk_desc *desc, int part);
+
 /* Convert strings from normal C strings to uEFI strings */
 static inline void ascii2unicode(u16 *unicode, const char *ascii)
 {
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 9c67367df4..e191a3280b 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -22,3 +22,4 @@ obj-$(CONFIG_DM_VIDEO) += efi_gop.o
 obj-$(CONFIG_PARTITIONS) += efi_disk.o
 obj-$(CONFIG_NET) += efi_net.o
 obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
+obj-$(CONFIG_DM) += efi_devpath.o
diff --git a/lib/efi_loader/efi_devpath.c b/lib/efi_loader/efi_devpath.c
new file mode 100644
index 0000000000..16b8edf899
--- /dev/null
+++ b/lib/efi_loader/efi_devpath.c
@@ -0,0 +1,175 @@
+/*
+ * EFI device path from u-boot device-model mapping
+ *
+ * (C) Copyright 2017 Rob Clark
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#include <common.h>
+#include <blk.h>
+#include <dm.h>
+#include <usb.h>
+#include <mmc.h>
+#include <efi_loader.h>
+#include <inttypes.h>
+#include <part.h>
+#include <malloc.h>
+
+/* template END node: */
+const static struct efi_device_path END = {
+	.type     = DEVICE_PATH_TYPE_END,
+	.sub_type = DEVICE_PATH_SUB_TYPE_END,
+	.length   = sizeof(END),
+};
+
+/* template ROOT node, a fictional ACPI PNP device: */
+const static struct efi_device_path_acpi_path ROOT = {
+	.dp = {
+		.type     = DEVICE_PATH_TYPE_ACPI_DEVICE,
+		.sub_type = DEVICE_PATH_SUB_TYPE_ACPI_DEVICE,
+		.length   = sizeof(ROOT),
+	},
+	.hid = EISA_PNP_ID(0x1337),
+	.uid = 0,
+};
+
+
+/* size of device-path not including END node for device and all parents
+ * up to the root device.
+ */
+static unsigned dp_size(struct udevice *dev)
+{
+	if (!dev || !dev->driver)
+		return sizeof(ROOT);
+
+	switch (dev->driver->id) {
+	case UCLASS_ROOT:
+	case UCLASS_SIMPLE_BUS:
+		/* stop traversing parents at this point: */
+		return sizeof(ROOT);
+	case UCLASS_MMC:
+		return dp_size(dev->parent) + sizeof(struct efi_device_path_sd_mmc_path);
+	case UCLASS_MASS_STORAGE:
+	case UCLASS_USB_HUB:
+		return dp_size(dev->parent) + sizeof(struct efi_device_path_usb);
+	default:
+		/* just skip over unknown classes: */
+		return dp_size(dev->parent);
+	}
+}
+
+static void *dp_fill(void *buf, struct udevice *dev)
+{
+	if (!dev || !dev->driver)
+		return buf;
+
+	switch (dev->driver->id) {
+	case UCLASS_ROOT:
+	case UCLASS_SIMPLE_BUS: {
+		/* stop traversing parents at this point: */
+		struct efi_device_path_acpi_path *adp = buf;
+		*adp = ROOT;
+		return &adp[1];
+	}
+	case UCLASS_MMC: {
+		struct efi_device_path_sd_mmc_path *sddp =
+			dp_fill(buf, dev->parent);
+		struct mmc *mmc = mmc_get_mmc_dev(dev);
+		struct blk_desc *desc = mmc_get_blk_desc(mmc);
+
+		sddp->dp.type     = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
+		sddp->dp.sub_type = (desc->if_type == IF_TYPE_MMC) ?
+			DEVICE_PATH_SUB_TYPE_MSG_MMC :
+			DEVICE_PATH_SUB_TYPE_MSG_SD;
+		sddp->dp.length   = sizeof(*sddp);
+		sddp->slot_number = 0;  // XXX ???
+
+		return &sddp[1];
+	}
+	case UCLASS_MASS_STORAGE:
+	case UCLASS_USB_HUB: {
+		struct efi_device_path_usb *udp =
+			dp_fill(buf, dev->parent);
+
+		udp->dp.type     = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
+		udp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_USB;
+		udp->dp.length   = sizeof(*udp);
+		udp->parent_port_number = 0; // XXX ???
+		udp->usb_interface = 0; // XXX ???
+
+		return &udp[1];
+	}
+	default:
+		debug("unhandled device class: %s (%u)\n",
+			dev->name, dev->driver->id);
+		return dp_fill(buf, dev->parent);
+	}
+}
+
+/* Construct a device-path from a device: */
+struct efi_device_path *efi_dp_from_dev(struct udevice *dev)
+{
+	void *buf, *start;
+
+	start = buf = calloc(1, dp_size(dev) + sizeof(END));
+	buf = dp_fill(buf, dev);
+	*((struct efi_device_path *)buf) = END;
+
+	return start;
+}
+
+/* Construct a device-path from a partition on a blk device: */
+struct efi_device_path *efi_dp_from_part(struct blk_desc *desc, int part)
+{
+	disk_partition_t info;
+	unsigned dpsize;
+	void *buf, *start;
+
+	dpsize = dp_size(desc->bdev->parent) + sizeof(END);
+
+	if (desc->part_type == PART_TYPE_ISO) {
+		dpsize += sizeof(struct efi_device_path_cdrom_path);
+	} else {
+		dpsize += sizeof(struct efi_device_path_hard_drive_path);
+	}
+
+	start = buf = calloc(1, dpsize);
+
+	buf = dp_fill(buf, desc->bdev->parent);
+
+	part_get_info(desc, part, &info);
+
+	if (desc->part_type == PART_TYPE_ISO) {
+		struct efi_device_path_cdrom_path *cddp = buf;
+
+		cddp->boot_entry = part - 1;
+		cddp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
+		cddp->dp.sub_type = DEVICE_PATH_SUB_TYPE_CDROM_PATH;
+		cddp->dp.length = sizeof (*cddp);
+		cddp->partition_start = info.start;
+		cddp->partition_end = info.size;
+
+		buf = &cddp[1];
+	} else {
+		struct efi_device_path_hard_drive_path *hddp = buf;
+
+		hddp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
+		hddp->dp.sub_type = DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH;
+		hddp->dp.length = sizeof (*hddp);
+		hddp->partition_number = part - 1;
+		hddp->partition_start = info.start;
+		hddp->partition_end = info.size;
+		if (desc->part_type == PART_TYPE_EFI)
+			hddp->partmap_type = 2;
+		else
+			hddp->partmap_type = 1;
+		hddp->signature_type = 0;
+
+		buf = &hddp[1];
+	}
+
+	*((struct efi_device_path *)buf) = END;
+
+	return start;
+}
-- 
2.13.0

[U-Boot] [PATCH 2/4] efi_loader: add udevice to EFI device-path mapping

[Rob Clark]

So the static bootefi_device_obj is making things slightly awkward for
efi_load_image() from a file-path.  And really it should just go away,
and instead we should plug in the appropriate diskobj (or netobj) to
the loaded_image_obj at boot time.  Also we should nuke
bootefi_device_path.  And since we need to construct a new
loaded_image_obj in efi_load_image(), probably split out a helper to
fill that out properly and plug in the correct boot device-path, etc,
etc, so we don't have too many different places constructing the same
sort of object and forgetting to install some protocols in one place
or another.

And since there are a lot of places we need to map to device-path and
back, I'm starting to thing the sane way to do all this without
breaking legacy (!CONFIG_DM) is to introduce a efi_device_path.c and
efi_device_path_legacy.c.  Move all the hacky stuff of current
devicepath construction into efi_device_path_legacy.c.  Add some
device-path parsing/matching stuff to efi_device_path_util.c (which
probably just be efi_device_path_to_text.c renamed and then spiffed
out with some more device-path helpers), which would be shared in
common in legacy and CONFIG_DM cases.

Sound semi-reasonable?  I'm not sure if this intersects too badly with
other stuff Heinrich is working on?

(Also, small logistical question.. anyone know how to do
"obj-$(!CONFIG_DM) += efi_boot_device_legacy.o"?)

BR,
-R

On Fri, Jul 21, 2017 at 2:43 PM, Rob Clark <robdclark@gmail.com> wrote:
> Initially just supports "disk" type devices, but a real UEFI
> implementation would expose input/output/display/etc with device-paths
> as well, so we may eventually want to expand this for other uses.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  include/efi_loader.h         |   9 +++
>  lib/efi_loader/Makefile      |   1 +
>  lib/efi_loader/efi_devpath.c | 175 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 185 insertions(+)
>  create mode 100644 lib/efi_loader/efi_devpath.c
>
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 812ec10e37..97d5e3d13d 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -145,6 +145,15 @@ extern void *efi_bounce_buffer;
>  #define EFI_LOADER_BOUNCE_BUFFER_SIZE (64 * 1024 * 1024)
>  #endif
>
> +/*
> + * u-boot to EFI device mapping:
> + *
> + * TODO extend this for GOP and various other input/output
> + * devices which should also have an EFI devicepath?
> + */
> +struct efi_device_path *efi_dp_from_dev(struct udevice *dev);
> +struct efi_device_path *efi_dp_from_part(struct blk_desc *desc, int part);
> +
>  /* Convert strings from normal C strings to uEFI strings */
>  static inline void ascii2unicode(u16 *unicode, const char *ascii)
>  {
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index 9c67367df4..e191a3280b 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -22,3 +22,4 @@ obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>  obj-$(CONFIG_PARTITIONS) += efi_disk.o
>  obj-$(CONFIG_NET) += efi_net.o
>  obj-$(CONFIG_GENERATE_SMBIOS_TABLE) += efi_smbios.o
> +obj-$(CONFIG_DM) += efi_devpath.o
> diff --git a/lib/efi_loader/efi_devpath.c b/lib/efi_loader/efi_devpath.c
> new file mode 100644
> index 0000000000..16b8edf899
> --- /dev/null
> +++ b/lib/efi_loader/efi_devpath.c
> @@ -0,0 +1,175 @@
> +/*
> + * EFI device path from u-boot device-model mapping
> + *
> + * (C) Copyright 2017 Rob Clark
> + *
> + * SPDX-License-Identifier:    GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <blk.h>
> +#include <dm.h>
> +#include <usb.h>
> +#include <mmc.h>
> +#include <efi_loader.h>
> +#include <inttypes.h>
> +#include <part.h>
> +#include <malloc.h>
> +
> +/* template END node: */
> +const static struct efi_device_path END = {
> +       .type     = DEVICE_PATH_TYPE_END,
> +       .sub_type = DEVICE_PATH_SUB_TYPE_END,
> +       .length   = sizeof(END),
> +};
> +
> +/* template ROOT node, a fictional ACPI PNP device: */
> +const static struct efi_device_path_acpi_path ROOT = {
> +       .dp = {
> +               .type     = DEVICE_PATH_TYPE_ACPI_DEVICE,
> +               .sub_type = DEVICE_PATH_SUB_TYPE_ACPI_DEVICE,
> +               .length   = sizeof(ROOT),
> +       },
> +       .hid = EISA_PNP_ID(0x1337),
> +       .uid = 0,
> +};
> +
> +
> +/* size of device-path not including END node for device and all parents
> + * up to the root device.
> + */
> +static unsigned dp_size(struct udevice *dev)
> +{
> +       if (!dev || !dev->driver)
> +               return sizeof(ROOT);
> +
> +       switch (dev->driver->id) {
> +       case UCLASS_ROOT:
> +       case UCLASS_SIMPLE_BUS:
> +               /* stop traversing parents at this point: */
> +               return sizeof(ROOT);
> +       case UCLASS_MMC:
> +               return dp_size(dev->parent) + sizeof(struct efi_device_path_sd_mmc_path);
> +       case UCLASS_MASS_STORAGE:
> +       case UCLASS_USB_HUB:
> +               return dp_size(dev->parent) + sizeof(struct efi_device_path_usb);
> +       default:
> +               /* just skip over unknown classes: */
> +               return dp_size(dev->parent);
> +       }
> +}
> +
> +static void *dp_fill(void *buf, struct udevice *dev)
> +{
> +       if (!dev || !dev->driver)
> +               return buf;
> +
> +       switch (dev->driver->id) {
> +       case UCLASS_ROOT:
> +       case UCLASS_SIMPLE_BUS: {
> +               /* stop traversing parents at this point: */
> +               struct efi_device_path_acpi_path *adp = buf;
> +               *adp = ROOT;
> +               return &adp[1];
> +       }
> +       case UCLASS_MMC: {
> +               struct efi_device_path_sd_mmc_path *sddp =
> +                       dp_fill(buf, dev->parent);
> +               struct mmc *mmc = mmc_get_mmc_dev(dev);
> +               struct blk_desc *desc = mmc_get_blk_desc(mmc);
> +
> +               sddp->dp.type     = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
> +               sddp->dp.sub_type = (desc->if_type == IF_TYPE_MMC) ?
> +                       DEVICE_PATH_SUB_TYPE_MSG_MMC :
> +                       DEVICE_PATH_SUB_TYPE_MSG_SD;
> +               sddp->dp.length   = sizeof(*sddp);
> +               sddp->slot_number = 0;  // XXX ???
> +
> +               return &sddp[1];
> +       }
> +       case UCLASS_MASS_STORAGE:
> +       case UCLASS_USB_HUB: {
> +               struct efi_device_path_usb *udp =
> +                       dp_fill(buf, dev->parent);
> +
> +               udp->dp.type     = DEVICE_PATH_TYPE_MESSAGING_DEVICE;
> +               udp->dp.sub_type = DEVICE_PATH_SUB_TYPE_MSG_USB;
> +               udp->dp.length   = sizeof(*udp);
> +               udp->parent_port_number = 0; // XXX ???
> +               udp->usb_interface = 0; // XXX ???
> +
> +               return &udp[1];
> +       }
> +       default:
> +               debug("unhandled device class: %s (%u)\n",
> +                       dev->name, dev->driver->id);
> +               return dp_fill(buf, dev->parent);
> +       }
> +}
> +
> +/* Construct a device-path from a device: */
> +struct efi_device_path *efi_dp_from_dev(struct udevice *dev)
> +{
> +       void *buf, *start;
> +
> +       start = buf = calloc(1, dp_size(dev) + sizeof(END));
> +       buf = dp_fill(buf, dev);
> +       *((struct efi_device_path *)buf) = END;
> +
> +       return start;
> +}
> +
> +/* Construct a device-path from a partition on a blk device: */
> +struct efi_device_path *efi_dp_from_part(struct blk_desc *desc, int part)
> +{
> +       disk_partition_t info;
> +       unsigned dpsize;
> +       void *buf, *start;
> +
> +       dpsize = dp_size(desc->bdev->parent) + sizeof(END);
> +
> +       if (desc->part_type == PART_TYPE_ISO) {
> +               dpsize += sizeof(struct efi_device_path_cdrom_path);
> +       } else {
> +               dpsize += sizeof(struct efi_device_path_hard_drive_path);
> +       }
> +
> +       start = buf = calloc(1, dpsize);
> +
> +       buf = dp_fill(buf, desc->bdev->parent);
> +
> +       part_get_info(desc, part, &info);
> +
> +       if (desc->part_type == PART_TYPE_ISO) {
> +               struct efi_device_path_cdrom_path *cddp = buf;
> +
> +               cddp->boot_entry = part - 1;
> +               cddp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
> +               cddp->dp.sub_type = DEVICE_PATH_SUB_TYPE_CDROM_PATH;
> +               cddp->dp.length = sizeof (*cddp);
> +               cddp->partition_start = info.start;
> +               cddp->partition_end = info.size;
> +
> +               buf = &cddp[1];
> +       } else {
> +               struct efi_device_path_hard_drive_path *hddp = buf;
> +
> +               hddp->dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
> +               hddp->dp.sub_type = DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH;
> +               hddp->dp.length = sizeof (*hddp);
> +               hddp->partition_number = part - 1;
> +               hddp->partition_start = info.start;
> +               hddp->partition_end = info.size;
> +               if (desc->part_type == PART_TYPE_EFI)
> +                       hddp->partmap_type = 2;
> +               else
> +                       hddp->partmap_type = 1;
> +               hddp->signature_type = 0;
> +
> +               buf = &hddp[1];
> +       }
> +
> +       *((struct efi_device_path *)buf) = END;
> +
> +       return start;
> +}
> --
> 2.13.0
>

[U-Boot] [PATCH 2/4] efi_loader: add udevice to EFI device-path mapping

[Alexander Graf]

On 25.07.17 15:57, Rob Clark wrote:
> So the static bootefi_device_obj is making things slightly awkward for
> efi_load_image() from a file-path.  And really it should just go away,
> and instead we should plug in the appropriate diskobj (or netobj) to
> the loaded_image_obj at boot time.  Also we should nuke
> bootefi_device_path.  And since we need to construct a new
> loaded_image_obj in efi_load_image(), probably split out a helper to
> fill that out properly and plug in the correct boot device-path, etc,
> etc, so we don't have too many different places constructing the same
> sort of object and forgetting to install some protocols in one place
> or another.
> 
> And since there are a lot of places we need to map to device-path and
> back, I'm starting to thing the sane way to do all this without
> breaking legacy (!CONFIG_DM) is to introduce a efi_device_path.c and
> efi_device_path_legacy.c.  Move all the hacky stuff of current
> devicepath construction into efi_device_path_legacy.c.  Add some
> device-path parsing/matching stuff to efi_device_path_util.c (which
> probably just be efi_device_path_to_text.c renamed and then spiffed
> out with some more device-path helpers), which would be shared in
> common in legacy and CONFIG_DM cases.
> 
> Sound semi-reasonable?  I'm not sure if this intersects too badly with

Sounds reasonable to me :).

> other stuff Heinrich is working on?

I don't know - I'll let him comment.

> 
> (Also, small logistical question.. anyone know how to do
> "obj-$(!CONFIG_DM) += efi_boot_device_legacy.o"?)

I would do

ifeq ($(CONFIG_DM),y)
   obj-y += efi_boot_device_dm.o
else
   obj-y += efi_boot_device_nodm.o
endif


Alex

[U-Boot] [PATCH 2/4] efi_loader: add udevice to EFI device-path mapping

[Heinrich Schuchardt]

On 07/25/2017 06:46 PM, Alexander Graf wrote:
> 
> 
> On 25.07.17 15:57, Rob Clark wrote:
>> So the static bootefi_device_obj is making things slightly awkward for
>> efi_load_image() from a file-path.  And really it should just go away,
>> and instead we should plug in the appropriate diskobj (or netobj) to
>> the loaded_image_obj at boot time.  Also we should nuke
>> bootefi_device_path.  And since we need to construct a new
>> loaded_image_obj in efi_load_image(), probably split out a helper to
>> fill that out properly and plug in the correct boot device-path, etc,
>> etc, so we don't have too many different places constructing the same
>> sort of object and forgetting to install some protocols in one place
>> or another.
>>
>> And since there are a lot of places we need to map to device-path and
>> back, I'm starting to thing the sane way to do all this without
>> breaking legacy (!CONFIG_DM) is to introduce a efi_device_path.c and
>> efi_device_path_legacy.c.  Move all the hacky stuff of current
>> devicepath construction into efi_device_path_legacy.c.  Add some
>> device-path parsing/matching stuff to efi_device_path_util.c (which
>> probably just be efi_device_path_to_text.c renamed and then spiffed
>> out with some more device-path helpers), which would be shared in
>> common in legacy and CONFIG_DM cases.
>>
>> Sound semi-reasonable?  I'm not sure if this intersects too badly with
> 
> Sounds reasonable to me :).
> 
>> other stuff Heinrich is working on?
> 
> I don't know - I'll let him comment.
> 

OpenProtocol should manage a lock table of type
EFI_OPEN_PROTOCOL_INFORMATION_ENTRY[] per protocol.
I am trying to implement this table which is also
needed by OpenProtocolInformation, CloseProtocol,
ConnectController, and DisconnectConntroller.

Best regards

Heinrich

>>
>> (Also, small logistical question.. anyone know how to do
>> "obj-$(!CONFIG_DM) += efi_boot_device_legacy.o"?)
> 
> I would do
> 
> ifeq ($(CONFIG_DM),y)
>   obj-y += efi_boot_device_dm.o
> else
>   obj-y += efi_boot_device_nodm.o
> endif
> 
> 
> Alex
> 

[U-Boot] [PATCH 3/4] efi_loader: make disk objects for partitions

[Rob Clark]

Normally a EFI application, like grub2, is going to expect to see
block-io devices for both the actual disk and for each partition
(with the disk object being parent of the partition objects).

Now instead of seeing devices like:

  /File(sdhci at 07864000.blk)/EndEntire
  /File(usb_mass_storage.lun0)/EndEntire

You see:

  /ACPI(133741d0,0)/UnknownMessaging(1d)/EndEntire
  /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(0,800,64000,0000000000000000,1,0)/EndEntire
  /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,200000,0000000000000000,1,0)/EndEntire
  /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,0000000000000000,1,0)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(0,800,60000,0000000000000000,1,0)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,0000000000000000,1,0)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,0000000000000000,1,0)/EndEntire
  /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,0000000000000000,1,0)/EndEntire

This is on a board with single USB disk and single sd-card.  The
UnknownMessaging(1d) node in the device-path is the MMC device,
but grub_efi_print_device_path() hasn't been updated yet for some
of the newer device-path sub-types.

This patch is somewhat based on a patch originally from Peter Jones,
but re-worked to use efi_devpath, so it doesn't much resemble the
original.

This depends on efi_devpath, which depends on CONFIG_DM, so legacy
devices will still get the old broken behavior.

There are a couple TODOs:
 1) how to find slot # for mmc device
 2) how to find parent port # and interface # for usb device

Also I don't seem to manage to get u-boot to recognize my usb-eth
dongle (config issue?) so I haven't been able to test the net-boot
path.. so it likely needs some work.

Cc: Peter Jones <pjones@redhat.com>
Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 lib/efi_loader/efi_disk.c | 73 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 53 insertions(+), 20 deletions(-)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 09697936b5..44f72c93e2 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -28,7 +28,7 @@ struct efi_disk_obj {
 	/* EFI Interface Media descriptor struct, referenced by ops */
 	struct efi_block_io_media media;
 	/* EFI device path to this block device */
-	struct efi_device_path_file_path *dp;
+	struct efi_device_path *dp;
 	/* Offset into disk for simple partitions */
 	lbaint_t offset;
 	/* Internal block device */
@@ -193,21 +193,47 @@ static const struct efi_block_io block_io_disk_template = {
 	.flush_blocks = &efi_disk_flush_blocks,
 };
 
+static struct efi_device_path *blk2dp(struct blk_desc *desc,
+		const char *name, unsigned part)
+{
+#ifdef CONFIG_DM
+	/* part==0 is the case for parent block device representing
+	 * entire disk, and part>=1 is for child per-partition
+	 * MEDIA_DEVICE objects:
+	 */
+	if (part == 0)
+		return efi_dp_from_dev(desc->bdev->parent);
+	return efi_dp_from_part(desc, part);
+#else
+	struct efi_device_path_file_path *dp = calloc(2, sizeof(*dp));
+
+	dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
+	dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
+	dp[0].dp.length = sizeof(*dp);
+	ascii2unicode(dp[0].str, name);
+
+	dp[1].dp.type = DEVICE_PATH_TYPE_END;
+	dp[1].dp.sub_type = DEVICE_PATH_SUB_TYPE_END;
+	dp[1].dp.length = sizeof(*dp);
+
+	return dp;
+#endif
+}
+
 static void efi_disk_add_dev(const char *name,
 			     const char *if_typename,
-			     const struct blk_desc *desc,
+			     struct blk_desc *desc,
 			     int dev_index,
-			     lbaint_t offset)
+			     lbaint_t offset,
+			     unsigned part)
 {
 	struct efi_disk_obj *diskobj;
-	struct efi_device_path_file_path *dp;
-	int objlen = sizeof(*diskobj) + (sizeof(*dp) * 2);
 
 	/* Don't add empty devices */
 	if (!desc->lba)
 		return;
 
-	diskobj = calloc(1, objlen);
+	diskobj = calloc(1, sizeof(*diskobj));
 
 	/* Fill in object data */
 	diskobj->parent.protocols[0].guid = &efi_block_io_guid;
@@ -228,18 +254,7 @@ static void efi_disk_add_dev(const char *name,
 	diskobj->media.io_align = desc->blksz;
 	diskobj->media.last_block = desc->lba - offset;
 	diskobj->ops.media = &diskobj->media;
-
-	/* Fill in device path */
-	dp = (void*)&diskobj[1];
-	diskobj->dp = dp;
-	dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
-	dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
-	dp[0].dp.length = sizeof(*dp);
-	ascii2unicode(dp[0].str, name);
-
-	dp[1].dp.type = DEVICE_PATH_TYPE_END;
-	dp[1].dp.sub_type = DEVICE_PATH_SUB_TYPE_END;
-	dp[1].dp.length = sizeof(*dp);
+	diskobj->dp = blk2dp(desc, name, part);
 
 	/* Hook up to the device list */
 	list_add_tail(&diskobj->parent.link, &efi_obj_list);
@@ -259,11 +274,16 @@ static int efi_disk_create_eltorito(struct blk_desc *desc,
 	if (desc->part_type != PART_TYPE_ISO)
 		return 0;
 
+#ifdef CONFIG_DM
+	/* add block device: */
+	efi_disk_add_dev(devname, if_typename, desc, diskid, 0, 0);
+#endif
+	/* ... and devices for each partition: */
 	while (!part_get_info(desc, part, &info)) {
 		snprintf(devname, sizeof(devname), "%s:%d", pdevname,
 			 part);
 		efi_disk_add_dev(devname, if_typename, desc, diskid,
-				 info.start);
+				 info.start, part);
 		part++;
 		disks++;
 	}
@@ -290,9 +310,22 @@ int efi_disk_register(void)
 	     uclass_next_device(&dev)) {
 		struct blk_desc *desc = dev_get_uclass_platdata(dev);
 		const char *if_typename = dev->driver->name;
+		disk_partition_t info;
+		int part = 1;
 
 		printf("Scanning disk %s...\n", dev->name);
-		efi_disk_add_dev(dev->name, if_typename, desc, desc->devnum, 0);
+
+#ifdef CONFIG_DM
+		/* add block device: */
+		efi_disk_add_dev(dev->name, if_typename, desc,
+				desc->devnum, 0, 0);
+#endif
+		/* ... and devices for each partition: */
+		while (!part_get_info(desc, part, &info)) {
+			efi_disk_add_dev(dev->name, if_typename, desc,
+					desc->devnum, 0, part);
+			part++;
+		}
 		disks++;
 
 		/*
-- 
2.13.0

[U-Boot] [U-Boot, 3/4] efi_loader: make disk objects for partitions

[Heinrich Schuchardt]

On 07/21/2017 08:43 PM, Rob Clark wrote:
> Normally a EFI application, like grub2, is going to expect to see

%s/a/an/

> block-io devices for both the actual disk and for each partition
> (with the disk object being parent of the partition objects).
> 
> Now instead of seeing devices like:
> 
>   /File(sdhci at 07864000.blk)/EndEntire
>   /File(usb_mass_storage.lun0)/EndEntire
> 
> You see:
> 
>   /ACPI(133741d0,0)/UnknownMessaging(1d)/EndEntire
>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(0,800,64000,0000000000000000,1,0)/EndEntire
>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,200000,0000000000000000,1,0)/EndEntire
>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,0000000000000000,1,0)/EndEntire
>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/EndEntire
>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(0,800,60000,0000000000000000,1,0)/EndEntire
>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,0000000000000000,1,0)/EndEntire
>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,0000000000000000,1,0)/EndEntire
>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,0000000000000000,1,0)/EndEntire
> 
> This is on a board with single USB disk and single sd-card.  The
> UnknownMessaging(1d) node in the device-path is the MMC device,
> but grub_efi_print_device_path() hasn't been updated yet for some
> of the newer device-path sub-types.
> 
> This patch is somewhat based on a patch originally from Peter Jones,
> but re-worked to use efi_devpath, so it doesn't much resemble the
> original.
> 
> This depends on efi_devpath, which depends on CONFIG_DM, so legacy
> devices will still get the old broken behavior.
> 
> There are a couple TODOs:
>  1) how to find slot # for mmc device
>  2) how to find parent port # and interface # for usb device
> 
> Also I don't seem to manage to get u-boot to recognize my usb-eth
> dongle (config issue?) so I haven't been able to test the net-boot
> path.. so it likely needs some work.
> 
> Cc: Peter Jones <pjones@redhat.com>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  lib/efi_loader/efi_disk.c | 73 ++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 53 insertions(+), 20 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 09697936b5..44f72c93e2 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -28,7 +28,7 @@ struct efi_disk_obj {
>  	/* EFI Interface Media descriptor struct, referenced by ops */
>  	struct efi_block_io_media media;
>  	/* EFI device path to this block device */
> -	struct efi_device_path_file_path *dp;
> +	struct efi_device_path *dp;
>  	/* Offset into disk for simple partitions */
>  	lbaint_t offset;
>  	/* Internal block device */
> @@ -193,21 +193,47 @@ static const struct efi_block_io block_io_disk_template = {
>  	.flush_blocks = &efi_disk_flush_blocks,
>  };
>  
> +static struct efi_device_path *blk2dp(struct blk_desc *desc,
> +		const char *name, unsigned part)
> +{
> +#ifdef CONFIG_DM
> +	/* part==0 is the case for parent block device representing
> +	 * entire disk, and part>=1 is for child per-partition
> +	 * MEDIA_DEVICE objects:
> +	 */
> +	if (part == 0)
> +		return efi_dp_from_dev(desc->bdev->parent);
> +	return efi_dp_from_part(desc, part);
> +#else
> +	struct efi_device_path_file_path *dp = calloc(2, sizeof(*dp));
> +
> +	dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
> +	dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
> +	dp[0].dp.length = sizeof(*dp);
> +	ascii2unicode(dp[0].str, name);
> +
> +	dp[1].dp.type = DEVICE_PATH_TYPE_END;
> +	dp[1].dp.sub_type = DEVICE_PATH_SUB_TYPE_END;
> +	dp[1].dp.length = sizeof(*dp);
> +
> +	return dp;
> +#endif
> +}
> +
>  static void efi_disk_add_dev(const char *name,
>  			     const char *if_typename,
> -			     const struct blk_desc *desc,
> +			     struct blk_desc *desc,
>  			     int dev_index,
> -			     lbaint_t offset)
> +			     lbaint_t offset,
> +			     unsigned part)
>  {
>  	struct efi_disk_obj *diskobj;
> -	struct efi_device_path_file_path *dp;
> -	int objlen = sizeof(*diskobj) + (sizeof(*dp) * 2);
>  
>  	/* Don't add empty devices */
>  	if (!desc->lba)
>  		return;
>  
> -	diskobj = calloc(1, objlen);
> +	diskobj = calloc(1, sizeof(*diskobj));
>  
>  	/* Fill in object data */
>  	diskobj->parent.protocols[0].guid = &efi_block_io_guid;

I am unable to apply this patch to the efi-next branch of
git://github.com/agraf/u-boot.git

Applying: efi_loader: make disk objects for partitions
error: patch failed: lib/efi_loader/efi_disk.c:193
error: lib/efi_loader/efi_disk.c: patch does not apply
Patch failed at 0001 efi_loader: make disk objects for partitions

Could you, please, resubmit your patch series rebased on efi-next.

You somehow forgot to CC the maintainer Alexander Graf in your patch.

Please, always use scripts/get_maintainer.pl to determine whom a patch
should be addressed to.

Best regards

Heinrich Schuchardt

> @@ -228,18 +254,7 @@ static void efi_disk_add_dev(const char *name,
>  	diskobj->media.io_align = desc->blksz;
>  	diskobj->media.last_block = desc->lba - offset;
>  	diskobj->ops.media = &diskobj->media;
> -
> -	/* Fill in device path */
> -	dp = (void*)&diskobj[1];
> -	diskobj->dp = dp;
> -	dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
> -	dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
> -	dp[0].dp.length = sizeof(*dp);
> -	ascii2unicode(dp[0].str, name);
> -
> -	dp[1].dp.type = DEVICE_PATH_TYPE_END;
> -	dp[1].dp.sub_type = DEVICE_PATH_SUB_TYPE_END;
> -	dp[1].dp.length = sizeof(*dp);
> +	diskobj->dp = blk2dp(desc, name, part);
>  
>  	/* Hook up to the device list */
>  	list_add_tail(&diskobj->parent.link, &efi_obj_list);
> @@ -259,11 +274,16 @@ static int efi_disk_create_eltorito(struct blk_desc *desc,
>  	if (desc->part_type != PART_TYPE_ISO)
>  		return 0;
>  
> +#ifdef CONFIG_DM
> +	/* add block device: */
> +	efi_disk_add_dev(devname, if_typename, desc, diskid, 0, 0);
> +#endif
> +	/* ... and devices for each partition: */
>  	while (!part_get_info(desc, part, &info)) {
>  		snprintf(devname, sizeof(devname), "%s:%d", pdevname,
>  			 part);
>  		efi_disk_add_dev(devname, if_typename, desc, diskid,
> -				 info.start);
> +				 info.start, part);
>  		part++;
>  		disks++;
>  	}
> @@ -290,9 +310,22 @@ int efi_disk_register(void)
>  	     uclass_next_device(&dev)) {
>  		struct blk_desc *desc = dev_get_uclass_platdata(dev);
>  		const char *if_typename = dev->driver->name;
> +		disk_partition_t info;
> +		int part = 1;
>  
>  		printf("Scanning disk %s...\n", dev->name);
> -		efi_disk_add_dev(dev->name, if_typename, desc, desc->devnum, 0);
> +
> +#ifdef CONFIG_DM
> +		/* add block device: */
> +		efi_disk_add_dev(dev->name, if_typename, desc,
> +				desc->devnum, 0, 0);
> +#endif
> +		/* ... and devices for each partition: */
> +		while (!part_get_info(desc, part, &info)) {
> +			efi_disk_add_dev(dev->name, if_typename, desc,
> +					desc->devnum, 0, part);
> +			part++;
> +		}
>  		disks++;
>  
>  		/*
> 

[U-Boot] [U-Boot, 3/4] efi_loader: make disk objects for partitions

[Rob Clark]

On Sun, Jul 23, 2017 at 6:25 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 07/21/2017 08:43 PM, Rob Clark wrote:
>> Normally a EFI application, like grub2, is going to expect to see
>
> %s/a/an/
>
>> block-io devices for both the actual disk and for each partition
>> (with the disk object being parent of the partition objects).
>>
>> Now instead of seeing devices like:
>>
>>   /File(sdhci at 07864000.blk)/EndEntire
>>   /File(usb_mass_storage.lun0)/EndEntire
>>
>> You see:
>>
>>   /ACPI(133741d0,0)/UnknownMessaging(1d)/EndEntire
>>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(0,800,64000,0000000000000000,1,0)/EndEntire
>>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(1,64800,200000,0000000000000000,1,0)/EndEntire
>>   /ACPI(133741d0,0)/UnknownMessaging(1d)/HD(2,264800,19a000,0000000000000000,1,0)/EndEntire
>>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/EndEntire
>>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(0,800,60000,0000000000000000,1,0)/EndEntire
>>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(1,61000,155000,0000000000000000,1,0)/EndEntire
>>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(2,20fa800,1bbf8800,0000000000000000,1,0)/EndEntire
>>   /ACPI(133741d0,0)/USB(0,0)/USB(0,0)/USB(0,0)/HD(3,1b6800,1f44000,0000000000000000,1,0)/EndEntire
>>
>> This is on a board with single USB disk and single sd-card.  The
>> UnknownMessaging(1d) node in the device-path is the MMC device,
>> but grub_efi_print_device_path() hasn't been updated yet for some
>> of the newer device-path sub-types.
>>
>> This patch is somewhat based on a patch originally from Peter Jones,
>> but re-worked to use efi_devpath, so it doesn't much resemble the
>> original.
>>
>> This depends on efi_devpath, which depends on CONFIG_DM, so legacy
>> devices will still get the old broken behavior.
>>
>> There are a couple TODOs:
>>  1) how to find slot # for mmc device
>>  2) how to find parent port # and interface # for usb device
>>
>> Also I don't seem to manage to get u-boot to recognize my usb-eth
>> dongle (config issue?) so I haven't been able to test the net-boot
>> path.. so it likely needs some work.
>>
>> Cc: Peter Jones <pjones@redhat.com>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  lib/efi_loader/efi_disk.c | 73 ++++++++++++++++++++++++++++++++++-------------
>>  1 file changed, 53 insertions(+), 20 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
>> index 09697936b5..44f72c93e2 100644
>> --- a/lib/efi_loader/efi_disk.c
>> +++ b/lib/efi_loader/efi_disk.c
>> @@ -28,7 +28,7 @@ struct efi_disk_obj {
>>       /* EFI Interface Media descriptor struct, referenced by ops */
>>       struct efi_block_io_media media;
>>       /* EFI device path to this block device */
>> -     struct efi_device_path_file_path *dp;
>> +     struct efi_device_path *dp;
>>       /* Offset into disk for simple partitions */
>>       lbaint_t offset;
>>       /* Internal block device */
>> @@ -193,21 +193,47 @@ static const struct efi_block_io block_io_disk_template = {
>>       .flush_blocks = &efi_disk_flush_blocks,
>>  };
>>
>> +static struct efi_device_path *blk2dp(struct blk_desc *desc,
>> +             const char *name, unsigned part)
>> +{
>> +#ifdef CONFIG_DM
>> +     /* part==0 is the case for parent block device representing
>> +      * entire disk, and part>=1 is for child per-partition
>> +      * MEDIA_DEVICE objects:
>> +      */
>> +     if (part == 0)
>> +             return efi_dp_from_dev(desc->bdev->parent);
>> +     return efi_dp_from_part(desc, part);
>> +#else
>> +     struct efi_device_path_file_path *dp = calloc(2, sizeof(*dp));
>> +
>> +     dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
>> +     dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
>> +     dp[0].dp.length = sizeof(*dp);
>> +     ascii2unicode(dp[0].str, name);
>> +
>> +     dp[1].dp.type = DEVICE_PATH_TYPE_END;
>> +     dp[1].dp.sub_type = DEVICE_PATH_SUB_TYPE_END;
>> +     dp[1].dp.length = sizeof(*dp);
>> +
>> +     return dp;
>> +#endif
>> +}
>> +
>>  static void efi_disk_add_dev(const char *name,
>>                            const char *if_typename,
>> -                          const struct blk_desc *desc,
>> +                          struct blk_desc *desc,
>>                            int dev_index,
>> -                          lbaint_t offset)
>> +                          lbaint_t offset,
>> +                          unsigned part)
>>  {
>>       struct efi_disk_obj *diskobj;
>> -     struct efi_device_path_file_path *dp;
>> -     int objlen = sizeof(*diskobj) + (sizeof(*dp) * 2);
>>
>>       /* Don't add empty devices */
>>       if (!desc->lba)
>>               return;
>>
>> -     diskobj = calloc(1, objlen);
>> +     diskobj = calloc(1, sizeof(*diskobj));
>>
>>       /* Fill in object data */
>>       diskobj->parent.protocols[0].guid = &efi_block_io_guid;
>
> I am unable to apply this patch to the efi-next branch of
> git://github.com/agraf/u-boot.git
>
> Applying: efi_loader: make disk objects for partitions
> error: patch failed: lib/efi_loader/efi_disk.c:193
> error: lib/efi_loader/efi_disk.c: patch does not apply
> Patch failed at 0001 efi_loader: make disk objects for partitions
>
> Could you, please, resubmit your patch series rebased on efi-next.

sorry, wasn't aware of efi-next tree..  I'll rebase soon.  I have some
more updates[1] (including some parts that I'd like to squash into
this patch) to get shim.efi and fallback.efi working, but I'll send a
new version of these patches when I have that working.

[1] https://github.com/robclark/u-boot/commits/wip-enough-uefi-for-shim
<-- very much work-in-progress

BR,
-R

> You somehow forgot to CC the maintainer Alexander Graf in your patch.
>
> Please, always use scripts/get_maintainer.pl to determine whom a patch
> should be addressed to.
>
> Best regards
>
> Heinrich Schuchardt
>
>> @@ -228,18 +254,7 @@ static void efi_disk_add_dev(const char *name,
>>       diskobj->media.io_align = desc->blksz;
>>       diskobj->media.last_block = desc->lba - offset;
>>       diskobj->ops.media = &diskobj->media;
>> -
>> -     /* Fill in device path */
>> -     dp = (void*)&diskobj[1];
>> -     diskobj->dp = dp;
>> -     dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE;
>> -     dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH;
>> -     dp[0].dp.length = sizeof(*dp);
>> -     ascii2unicode(dp[0].str, name);
>> -
>> -     dp[1].dp.type = DEVICE_PATH_TYPE_END;
>> -     dp[1].dp.sub_type = DEVICE_PATH_SUB_TYPE_END;
>> -     dp[1].dp.length = sizeof(*dp);
>> +     diskobj->dp = blk2dp(desc, name, part);
>>
>>       /* Hook up to the device list */
>>       list_add_tail(&diskobj->parent.link, &efi_obj_list);
>> @@ -259,11 +274,16 @@ static int efi_disk_create_eltorito(struct blk_desc *desc,
>>       if (desc->part_type != PART_TYPE_ISO)
>>               return 0;
>>
>> +#ifdef CONFIG_DM
>> +     /* add block device: */
>> +     efi_disk_add_dev(devname, if_typename, desc, diskid, 0, 0);
>> +#endif
>> +     /* ... and devices for each partition: */
>>       while (!part_get_info(desc, part, &info)) {
>>               snprintf(devname, sizeof(devname), "%s:%d", pdevname,
>>                        part);
>>               efi_disk_add_dev(devname, if_typename, desc, diskid,
>> -                              info.start);
>> +                              info.start, part);
>>               part++;
>>               disks++;
>>       }
>> @@ -290,9 +310,22 @@ int efi_disk_register(void)
>>            uclass_next_device(&dev)) {
>>               struct blk_desc *desc = dev_get_uclass_platdata(dev);
>>               const char *if_typename = dev->driver->name;
>> +             disk_partition_t info;
>> +             int part = 1;
>>
>>               printf("Scanning disk %s...\n", dev->name);
>> -             efi_disk_add_dev(dev->name, if_typename, desc, desc->devnum, 0);
>> +
>> +#ifdef CONFIG_DM
>> +             /* add block device: */
>> +             efi_disk_add_dev(dev->name, if_typename, desc,
>> +                             desc->devnum, 0, 0);
>> +#endif
>> +             /* ... and devices for each partition: */
>> +             while (!part_get_info(desc, part, &info)) {
>> +                     efi_disk_add_dev(dev->name, if_typename, desc,
>> +                                     desc->devnum, 0, part);
>> +                     part++;
>> +             }
>>               disks++;
>>
>>               /*
>>
>

[U-Boot] [PATCH 4/4] efi_loader: use efi_devpath to get correct boot device-path

[Rob Clark]

This way grub can properly match up the loaded_image device-path to a
partition device object, so it can locate it's grub.cfg.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 cmd/bootefi.c | 31 ++++++++++++++++++++++++++-----
 1 file changed, 26 insertions(+), 5 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 5030b7bd86..ea0d5dbc79 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -41,6 +41,11 @@ static struct efi_device_path_file_path bootefi_image_path[] = {
 	}
 };
 
+/* if we have CONFIG_DM we construct a proper device-path from the
+ * boot device, otherwise fallback to using bootefi_device_path.
+ */
+static struct efi_device_path *real_bootefi_device_path;
+
 static struct efi_device_path_file_path bootefi_device_path[] = {
 	{
 		.dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE,
@@ -58,13 +63,12 @@ static efi_status_t EFIAPI bootefi_open_dp(void *handle, efi_guid_t *protocol,
 			void **protocol_interface, void *agent_handle,
 			void *controller_handle, uint32_t attributes)
 {
-	*protocol_interface = bootefi_device_path;
+	*protocol_interface = real_bootefi_device_path;
 	return EFI_SUCCESS;
 }
 
 /* The EFI loaded_image interface for the image executed via "bootefi" */
 static struct efi_loaded_image loaded_image_info = {
-	.device_handle = bootefi_device_path,
 	.file_path = bootefi_image_path,
 };
 
@@ -93,7 +97,6 @@ static struct efi_object loaded_image_info_obj = {
 
 /* The EFI object struct for the device the "bootefi" image was loaded from */
 static struct efi_object bootefi_device_obj = {
-	.handle = bootefi_device_path,
 	.protocols = {
 		{
 			/* When asking for the device path interface, return
@@ -239,8 +242,6 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt)
 
 	if (!memcmp(bootefi_device_path[0].str, "N\0e\0t", 6))
 		loaded_image_info.device_handle = nethandle;
-	else
-		loaded_image_info.device_handle = bootefi_device_path;
 #endif
 #ifdef CONFIG_GENERATE_SMBIOS_TABLE
 	efi_smbios_register();
@@ -339,6 +340,18 @@ U_BOOT_CMD(
 	bootefi_help_text
 );
 
+#ifdef CONFIG_DM
+static int parse_partnum(const char *devnr)
+{
+	const char *str = strchr(devnr, ':');
+	if (str) {
+		str++;
+		return simple_strtoul(str, NULL, 16);
+	}
+	return 0;
+}
+#endif
+
 void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
 {
 	__maybe_unused struct blk_desc *desc;
@@ -376,9 +389,14 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
 	if (colon)
 		*colon = '\0';
 
+#ifdef CONFIG_DM
+	real_bootefi_device_path = efi_dp_from_part(desc, parse_partnum(devnr));
+#else
 	/* Patch bootefi_device_path to the target device */
+	real_bootefi_device_path = bootefi_device_path;
 	memset(bootefi_device_path[0].str, 0, sizeof(bootefi_device_path[0].str));
 	ascii2unicode(bootefi_device_path[0].str, devname);
+#endif
 
 	/* Patch bootefi_image_path to the target file path */
 	memset(bootefi_image_path[0].str, 0, sizeof(bootefi_image_path[0].str));
@@ -389,4 +407,7 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
 		snprintf(devname, sizeof(devname), "%s", path);
 	}
 	ascii2unicode(bootefi_image_path[0].str, devname);
+
+	loaded_image_info.device_handle = real_bootefi_device_path;
+	bootefi_device_obj.handle = real_bootefi_device_path;
 }
-- 
2.13.0