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

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@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++;

  /*