[U-Boot] [PATCH 0/8] efi_loader: Support loading from El Torito isos

[U-Boot] [PATCH 0/8] efi_loader: Support loading from El Torito isos

[Alexander Graf]

Some distributions still provide .iso files for installation media.

To give us greatest flexibility, this patch set adds support for
El Torito booting with EFI payloads.

Please consider for 2016.05 still.


Thanks,

Alex

Alexander Graf (8):
  iso: Make little endian and 64bit safe
  iso: Start with partition 1
  iso: Allow 512 byte sector size
  efi_loader: Split drive add into function
  efi_loader: Add el torito support
  efi_loader: Pass file path to payload
  efi_loader: Increase path string to 32 characters
  distro: Enable iso partition code

 cmd/bootefi.c                    |  48 +++++++++++++---
 cmd/fs.c                         |   3 +-
 disk/part_iso.c                  |  50 +++++++++++-----
 disk/part_iso.h                  |  40 ++++++-------
 include/config_distro_defaults.h |   1 +
 include/efi_api.h                |   2 +-
 include/efi_loader.h             |   5 +-
 lib/efi_loader/efi_disk.c        | 121 +++++++++++++++++++++++++++------------
 8 files changed, 187 insertions(+), 83 deletions(-)

-- 
1.8.5.6

[U-Boot] [PATCH 1/8] iso: Make little endian and 64bit safe

[Alexander Graf]

The iso partition table implementation has a few endian and 64bit
problems. Clean it up a bit to become endian and bitness safe.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 disk/part_iso.c | 10 ++++------
 disk/part_iso.h | 40 ++++++++++++++++++++--------------------
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/disk/part_iso.c b/disk/part_iso.c
index 2114faf..b7a5381 100644
--- a/disk/part_iso.c
+++ b/disk/part_iso.c
@@ -58,11 +58,9 @@ int part_get_info_iso_verb(struct blk_desc *dev_desc, int part_num,
 				ppr->stand_ident, dev_desc->devnum, part_num);
 		return (-1);
 	}
-	lastsect= ((ppr->firstsek_LEpathtab1_LE & 0x000000ff)<<24) +
-		  ((ppr->firstsek_LEpathtab1_LE & 0x0000ff00)<< 8) +
-		  ((ppr->firstsek_LEpathtab1_LE & 0x00ff0000)>> 8) +
-		  ((ppr->firstsek_LEpathtab1_LE & 0xff000000)>>24) ;
-	info->blksz=ppr->secsize_BE; /* assuming same block size for all entries */
+	lastsect = le32_to_cpu(ppr->firstsek_LEpathtab1_LE);
+	/* assuming same block size for all entries */
+	info->blksz = be16_to_cpu(ppr->secsize_BE);
 	PRINTF(" Lastsect:%08lx\n",lastsect);
 	for(i=blkaddr;i<lastsect;i++) {
 		PRINTF("Reading block %d\n", i);
@@ -95,7 +93,7 @@ int part_get_info_iso_verb(struct blk_desc *dev_desc, int part_num,
 	chksum=0;
 	chksumbuf = (unsigned short *)tmpbuf;
 	for(i=0;i<0x10;i++)
-		chksum+=((chksumbuf[i] &0xff)<<8)+((chksumbuf[i] &0xff00)>>8);
+		chksum += le16_to_cpu(chksumbuf[i]);
 	if(chksum!=0) {
 		if(verb)
 			printf("** Checksum Error in booting catalog validation entry on %d:%d **\n",
diff --git a/disk/part_iso.h b/disk/part_iso.h
index dace0f8..045784f 100644
--- a/disk/part_iso.h
+++ b/disk/part_iso.h
@@ -29,8 +29,8 @@ typedef struct iso_pri_rec {
 	char					sysid[32];		/* system Identifier */
 	char					volid[32];		/* volume Identifier */
 	unsigned char zeros1[8];		/* unused */
-	unsigned long volsiz_LE;		/* volume size Little Endian */
-	unsigned long volsiz_BE;		/* volume size Big Endian */
+	unsigned int volsiz_LE;		/* volume size Little Endian */
+	unsigned int volsiz_BE;		/* volume size Big Endian */
 	unsigned char zeros2[32];		/* unused */
 	unsigned short setsize_LE;	/* volume set size LE */
 	unsigned short setsize_BE;	/* volume set size BE */
@@ -38,12 +38,12 @@ typedef struct iso_pri_rec {
 	unsigned short seqnum_BE;		/* volume sequence number BE */
 	unsigned short secsize_LE;	/* sector size LE */
 	unsigned short secsize_BE;	/* sector size BE */
-	unsigned long pathtablen_LE;/* Path Table size LE */
-	unsigned long pathtablen_BE;/* Path Table size BE */
-	unsigned long firstsek_LEpathtab1_LE; /* location of first occurrence of little endian type path table */
-	unsigned long firstsek_LEpathtab2_LE; /* location of optional occurrence of little endian type path table */
-	unsigned long firstsek_BEpathtab1_BE; /* location of first occurrence of big endian type path table */
-	unsigned long firstsek_BEpathtab2_BE; /* location of optional occurrence of big endian type path table */
+	unsigned int pathtablen_LE;/* Path Table size LE */
+	unsigned int pathtablen_BE;/* Path Table size BE */
+	unsigned int firstsek_LEpathtab1_LE; /* location of first occurrence of little endian type path table */
+	unsigned int firstsek_LEpathtab2_LE; /* location of optional occurrence of little endian type path table */
+	unsigned int firstsek_BEpathtab1_BE; /* location of first occurrence of big endian type path table */
+	unsigned int firstsek_BEpathtab2_BE; /* location of optional occurrence of big endian type path table */
 	unsigned char rootdir[34];	/* directory record for root dir */
 	char					volsetid[128];/* Volume set identifier */
 	char					pubid[128];		/* Publisher identifier */
@@ -67,8 +67,8 @@ typedef struct iso_sup_rec {
 	char					sysid[32];		/* system Identifier */
 	char					volid[32];		/* volume Identifier */
 	unsigned char zeros1[8];		/* unused */
-	unsigned long volsiz_LE;		/* volume size Little Endian */
-	unsigned long volsiz_BE;		/* volume size Big Endian */
+	unsigned int volsiz_LE;		/* volume size Little Endian */
+	unsigned int volsiz_BE;		/* volume size Big Endian */
 	unsigned char escapeseq[32];/* Escape sequences */
 	unsigned short setsize_LE;	/* volume set size LE */
 	unsigned short setsize_BE;	/* volume set size BE */
@@ -76,12 +76,12 @@ typedef struct iso_sup_rec {
 	unsigned short seqnum_BE;		/* volume sequence number BE */
 	unsigned short secsize_LE;	/* sector size LE */
 	unsigned short secsize_BE;	/* sector size BE */
-	unsigned long pathtablen_LE;/* Path Table size LE */
-	unsigned long pathtablen_BE;/* Path Table size BE */
-	unsigned long firstsek_LEpathtab1_LE; /* location of first occurrence of little endian type path table */
-	unsigned long firstsek_LEpathtab2_LE; /* location of optional occurrence of little endian type path table */
-	unsigned long firstsek_BEpathtab1_BE; /* location of first occurrence of big endian type path table */
-	unsigned long firstsek_BEpathtab2_BE; /* location of optional occurrence of big endian type path table */
+	unsigned int pathtablen_LE;/* Path Table size LE */
+	unsigned int pathtablen_BE;/* Path Table size BE */
+	unsigned int firstsek_LEpathtab1_LE; /* location of first occurrence of little endian type path table */
+	unsigned int firstsek_LEpathtab2_LE; /* location of optional occurrence of little endian type path table */
+	unsigned int firstsek_BEpathtab1_BE; /* location of first occurrence of big endian type path table */
+	unsigned int firstsek_BEpathtab2_BE; /* location of optional occurrence of big endian type path table */
 	unsigned char rootdir[34];	/* directory record for root dir */
 	char					volsetid[128];/* Volume set identifier */
 	char					pubid[128];		/* Publisher identifier */
@@ -104,10 +104,10 @@ typedef struct iso_part_rec {
 	unsigned char unused;
 	char					sysid[32];		 /* system Identifier */
 	char					volid[32];		/* volume partition Identifier */
-	unsigned long partloc_LE;		/* volume partition location LE */
-	unsigned long partloc_BE;		/* volume partition location BE */
-	unsigned long partsiz_LE;		/* volume partition size LE */
-	unsigned long partsiz_BE;		/* volume partition size BE */
+	unsigned int partloc_LE;		/* volume partition location LE */
+	unsigned int partloc_BE;		/* volume partition location BE */
+	unsigned int partsiz_LE;		/* volume partition size LE */
+	unsigned int partsiz_BE;		/* volume partition size BE */
 }iso_part_rec_t;
 
 
-- 
1.8.5.6

[U-Boot] [PATCH 2/8] iso: Start with partition 1

[Alexander Graf]

The generic partition code treats partition 0 as "whole disk". So
we should start with partition 1 as the first partition in the iso
partition table.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 disk/part_iso.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/disk/part_iso.c b/disk/part_iso.c
index b7a5381..5adb349 100644
--- a/disk/part_iso.c
+++ b/disk/part_iso.c
@@ -115,7 +115,7 @@ int part_get_info_iso_verb(struct blk_desc *dev_desc, int part_num,
 	}
 #endif
 	/* the validation entry seems to be ok, now search the "partition" */
-	entry_num=0;
+	entry_num=1;
 	offset=0x20;
 	strcpy((char *)info->type, "U-Boot");
 	switch(dev_desc->if_type) {
@@ -225,7 +225,7 @@ static int part_test_iso(struct blk_desc *dev_desc)
 {
 	disk_partition_t info;
 
-	return part_get_info_iso_verb(dev_desc, 0, &info, 0);
+	return part_get_info_iso_verb(dev_desc, 1, &info, 1);
 }
 
 U_BOOT_PART_TYPE(iso) = {
-- 
1.8.5.6

[U-Boot] [PATCH 3/8] iso: Allow 512 byte sector size

[Alexander Graf]

Real CD-ROMs are pretty obsolete these days. Usually people still keep
iso files around, but just put them on USB sticks or SD cards and expect
them to "just work".

To support this use case with El Torito images, add support for 512 byte
sector size to the iso parsing code.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 disk/part_iso.c | 36 +++++++++++++++++++++++++++++++-----
 1 file changed, 31 insertions(+), 5 deletions(-)

diff --git a/disk/part_iso.c b/disk/part_iso.c
index 5adb349..9f5c50c 100644
--- a/disk/part_iso.c
+++ b/disk/part_iso.c
@@ -26,6 +26,25 @@
 
 static unsigned char tmpbuf[CD_SECTSIZE];
 
+unsigned long iso_dread(struct blk_desc *block_dev, lbaint_t start,
+                        lbaint_t blkcnt, void *buffer)
+{
+	unsigned long ret;
+
+	if (block_dev->blksz == 512) {
+		/* Convert from 2048 to 512 sector size */
+		start *= 4;
+		blkcnt *= 4;
+	}
+
+	ret = blk_dread(block_dev, start, blkcnt, buffer);
+
+	if (block_dev->blksz == 512)
+		ret /= 4;
+
+	return ret;
+}
+
 /* only boot records will be listed as valid partitions */
 int part_get_info_iso_verb(struct blk_desc *dev_desc, int part_num,
 			   disk_partition_t *info, int verb)
@@ -39,12 +58,12 @@ int part_get_info_iso_verb(struct blk_desc *dev_desc, int part_num,
 	iso_val_entry_t *pve = (iso_val_entry_t *)tmpbuf;
 	iso_init_def_entry_t *pide;
 
-	if (dev_desc->blksz != CD_SECTSIZE)
+	if ((dev_desc->blksz != CD_SECTSIZE) && (dev_desc->blksz != 512))
 		return -1;
 
 	/* the first sector (sector 0x10) must be a primary volume desc */
 	blkaddr=PVD_OFFSET;
-	if (blk_dread(dev_desc, PVD_OFFSET, 1, (ulong *)tmpbuf) != 1)
+	if (iso_dread(dev_desc, PVD_OFFSET, 1, (ulong *)tmpbuf) != 1)
 		return -1;
 	if(ppr->desctype!=0x01) {
 		if(verb)
@@ -64,7 +83,7 @@ int part_get_info_iso_verb(struct blk_desc *dev_desc, int part_num,
 	PRINTF(" Lastsect:%08lx\n",lastsect);
 	for(i=blkaddr;i<lastsect;i++) {
 		PRINTF("Reading block %d\n", i);
-		if (blk_dread(dev_desc, i, 1, (ulong *)tmpbuf) != 1)
+		if (iso_dread(dev_desc, i, 1, (ulong *)tmpbuf) != 1)
 			return -1;
 		if(ppr->desctype==0x00)
 			break; /* boot entry found */
@@ -84,7 +103,7 @@ int part_get_info_iso_verb(struct blk_desc *dev_desc, int part_num,
 	}
 	bootaddr = get_unaligned_le32(pbr->pointer);
 	PRINTF(" Boot Entry at: %08lX\n",bootaddr);
-	if (blk_dread(dev_desc, bootaddr, 1, (ulong *)tmpbuf) != 1) {
+	if (iso_dread(dev_desc, bootaddr, 1, (ulong *)tmpbuf) != 1) {
 		if(verb)
 			printf ("** Can't read Boot Entry at %lX on %d:%d **\n",
 				bootaddr, dev_desc->devnum, part_num);
@@ -192,7 +211,14 @@ found:
 	}
 	newblkaddr = get_unaligned_le32(pide->rel_block_addr);
 	info->start=newblkaddr;
-	PRINTF(" part %d found @ %lx size %lx\n",part_num,newblkaddr,info->size);
+
+	if (dev_desc->blksz == 512) {
+		info->size *= 4;
+		info->start *= 4;
+		info->blksz = 512;
+	}
+
+	PRINTF(" part %d found @ %lx size %lx\n",part_num,info->start,info->size);
 	return 0;
 }
 
-- 
1.8.5.6

[U-Boot] [PATCH 4/8] efi_loader: Split drive add into function

[Alexander Graf]

The snippet of code to add a drive to our drive list needs to
get called from 2 places in the future. Split it into a separate
function.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 lib/efi_loader/efi_disk.c | 84 ++++++++++++++++++++++++++---------------------
 1 file changed, 47 insertions(+), 37 deletions(-)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index aaff947..e30e728 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -138,6 +138,52 @@ static const struct efi_block_io block_io_disk_template = {
 	.flush_blocks = &efi_disk_flush_blocks,
 };
 
+static void efi_disk_add_dev(char *name,
+			     const struct block_drvr *cur_drvr,
+			     const struct blk_desc *desc,
+			     int dev_index,
+			     lbaint_t offset)
+{
+	struct efi_disk_obj *diskobj;
+	struct efi_device_path_file_path *dp;
+	int objlen = sizeof(*diskobj) + (sizeof(*dp) * 2);
+
+	diskobj = calloc(1, objlen);
+
+	/* Fill in object data */
+	diskobj->parent.protocols[0].guid = &efi_block_io_guid;
+	diskobj->parent.protocols[0].open = efi_disk_open_block;
+	diskobj->parent.protocols[1].guid = &efi_guid_device_path;
+	diskobj->parent.protocols[1].open = efi_disk_open_dp;
+	diskobj->parent.handle = diskobj;
+	diskobj->ops = block_io_disk_template;
+	diskobj->ifname = cur_drvr->name;
+	diskobj->dev_index = dev_index;
+
+	/* Fill in EFI IO Media info (for read/write callbacks) */
+	diskobj->media.removable_media = desc->removable;
+	diskobj->media.media_present = 1;
+	diskobj->media.block_size = desc->blksz;
+	diskobj->media.io_align = desc->blksz;
+	diskobj->media.last_block = desc->lba;
+	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);
+
+	/* Hook up to the device list */
+	list_add_tail(&diskobj->parent.link, &efi_obj_list);
+}
+
 /*
  * 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
@@ -156,9 +202,6 @@ int efi_disk_register(void)
 		printf("Scanning disks on %s...\n", cur_drvr->name);
 		for (i = 0; i < 4; i++) {
 			struct blk_desc *desc;
-			struct efi_disk_obj *diskobj;
-			struct efi_device_path_file_path *dp;
-			int objlen = sizeof(*diskobj) + (sizeof(*dp) * 2);
 			char devname[16] = { 0 }; /* dp->str is u16[16] long */
 
 			desc = blk_get_dev(cur_drvr->name, i);
@@ -167,42 +210,9 @@ int efi_disk_register(void)
 			if (desc->type == DEV_TYPE_UNKNOWN)
 				continue;
 
-			diskobj = calloc(1, objlen);
-
-			/* Fill in object data */
-			diskobj->parent.protocols[0].guid = &efi_block_io_guid;
-			diskobj->parent.protocols[0].open = efi_disk_open_block;
-			diskobj->parent.protocols[1].guid = &efi_guid_device_path;
-			diskobj->parent.protocols[1].open = efi_disk_open_dp;
-			diskobj->parent.handle = diskobj;
-			diskobj->ops = block_io_disk_template;
-			diskobj->ifname = cur_drvr->name;
-			diskobj->dev_index = i;
-
-			/* Fill in EFI IO Media info (for read/write callbacks) */
-			diskobj->media.removable_media = desc->removable;
-			diskobj->media.media_present = 1;
-			diskobj->media.block_size = desc->blksz;
-			diskobj->media.io_align = desc->blksz;
-			diskobj->media.last_block = desc->lba;
-			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);
 			snprintf(devname, sizeof(devname), "%s%d",
 				 cur_drvr->name, i);
-			ascii2unicode(dp[0].str, devname);
-
-			dp[1].dp.type = DEVICE_PATH_TYPE_END;
-			dp[1].dp.sub_type = DEVICE_PATH_SUB_TYPE_END;
-			dp[1].dp.length = sizeof(*dp);
-
-			/* Hook up to the device list */
-			list_add_tail(&diskobj->parent.link, &efi_obj_list);
+			efi_disk_add_dev(devname, cur_drvr, desc, i, 0);
 			disks++;
 		}
 	}
-- 
1.8.5.6

[U-Boot] [PATCH 5/8] efi_loader: Add el torito support

[Alexander Graf]

When loading an el torito image, uEFI exposes said image as a raw
block device to the payload.

Let's do the same by creating new block devices with added offsets for
the respective el torito partitions.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 cmd/bootefi.c             | 14 ++++++++++++++
 lib/efi_loader/efi_disk.c | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 3add632..0d09aa1 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -194,12 +194,26 @@ U_BOOT_CMD(
 
 void efi_set_bootdev(const char *dev, const char *devnr)
 {
+	__maybe_unused struct blk_desc *desc;
 	char devname[16] = { 0 }; /* dp->str is u16[16] long */
 	char *colon;
 
 	/* Assemble the condensed device name we use in efi_disk.c */
 	snprintf(devname, sizeof(devname), "%s%s", dev, devnr);
 	colon = strchr(devname, ':');
+
+#ifdef CONFIG_ISO_PARTITION
+	/* For ISOs we create partition block devices */
+	desc = blk_get_dev(dev, simple_strtol(devnr, NULL, 10));
+	if (desc && (desc->type != DEV_TYPE_UNKNOWN) &&
+	    (desc->part_type == PART_TYPE_ISO)) {
+		if (!colon)
+			snprintf(devname, sizeof(devname), "%s%s:1", dev,
+				 devnr);
+		colon = NULL;
+	}
+#endif
+
 	if (colon)
 		*colon = '\0';
 
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index e30e728..b3d56a8 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -27,6 +27,8 @@ struct efi_disk_obj {
 	struct efi_block_io_media media;
 	/* EFI device path to this block device */
 	struct efi_device_path_file_path *dp;
+	/* Offset into disk for simple partitions */
+	lbaint_t offset;
 };
 
 static efi_status_t efi_disk_open_block(void *handle, efi_guid_t *protocol,
@@ -81,6 +83,7 @@ static efi_status_t EFIAPI efi_disk_rw_blocks(struct efi_block_io *this,
 		return EFI_EXIT(EFI_DEVICE_ERROR);
 	blksz = desc->blksz;
 	blocks = buffer_size / blksz;
+	lba += diskobj->offset;
 
 #ifdef DEBUG_EFI
 	printf("EFI: %s:%d blocks=%x lba=%"PRIx64" blksz=%x dir=%d\n", __func__,
@@ -159,6 +162,7 @@ static void efi_disk_add_dev(char *name,
 	diskobj->ops = block_io_disk_template;
 	diskobj->ifname = cur_drvr->name;
 	diskobj->dev_index = dev_index;
+	diskobj->offset = offset;
 
 	/* Fill in EFI IO Media info (for read/write callbacks) */
 	diskobj->media.removable_media = desc->removable;
@@ -184,6 +188,31 @@ static void efi_disk_add_dev(char *name,
 	list_add_tail(&diskobj->parent.link, &efi_obj_list);
 }
 
+static int efi_disk_create_eltorito(struct blk_desc *desc,
+				    const struct block_drvr *cur_drvr,
+				    int diskid)
+{
+	int disks = 0;
+#ifdef CONFIG_ISO_PARTITION
+	char devname[16] = { 0 }; /* dp->str is u16[16] long */
+	disk_partition_t info;
+	int part = 1;
+
+	if (desc->part_type != PART_TYPE_ISO)
+		return 0;
+
+	while (!part_get_info(desc, part, &info)) {
+		snprintf(devname, sizeof(devname), "%s%d:%d", cur_drvr->name,
+			 diskid, part);
+		efi_disk_add_dev(devname, cur_drvr, desc, diskid, info.start);
+		part++;
+		disks++;
+	}
+#endif
+
+	return disks;
+}
+
 /*
  * 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
@@ -214,6 +243,12 @@ int efi_disk_register(void)
 				 cur_drvr->name, i);
 			efi_disk_add_dev(devname, cur_drvr, desc, i, 0);
 			disks++;
+
+			/*
+			 * El Torito images show up as block devices
+			 * in an EFI world, so let's create them here
+			 */
+			disks += efi_disk_create_eltorito(desc, cur_drvr, i);
 		}
 	}
 	printf("Found %d disks\n", disks);
-- 
1.8.5.6

[U-Boot] [PATCH 6/8] efi_loader: Pass file path to payload

[Alexander Graf]

The payload gets information on where it got loaded from. This includes
the device as well as file path.

So far we've treated both as the same thing and always gave it the device
name. However, in some situations grub2 actually wants to find its loading
path to find its configuration file.

So let's split the two semantically separte bits into separate structs and
pass the loaded file name into our payload when we load it using "load".

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 cmd/bootefi.c        | 32 +++++++++++++++++++++++++-------
 cmd/fs.c             |  3 ++-
 include/efi_loader.h |  5 +++--
 3 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 0d09aa1..adcf645 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -34,17 +34,30 @@ static struct efi_device_path_file_path bootefi_image_path[] = {
 	}
 };
 
+static struct efi_device_path_file_path bootefi_device_path[] = {
+	{
+		.dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE,
+		.dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH,
+		.dp.length = sizeof(bootefi_image_path[0]),
+		.str = { 'b','o','o','t','e','f','i' },
+	}, {
+		.dp.type = DEVICE_PATH_TYPE_END,
+		.dp.sub_type = DEVICE_PATH_SUB_TYPE_END,
+		.dp.length = sizeof(bootefi_image_path[0]),
+	}
+};
+
 static efi_status_t bootefi_open_dp(void *handle, efi_guid_t *protocol,
 			void **protocol_interface, void *agent_handle,
 			void *controller_handle, uint32_t attributes)
 {
-	*protocol_interface = bootefi_image_path;
+	*protocol_interface = 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_image_path,
+	.device_handle = bootefi_device_path,
 	.file_path = bootefi_image_path,
 };
 
@@ -63,7 +76,7 @@ static struct efi_object loaded_image_info_obj = {
 		{
 			/*
 			 * When asking for the device path interface, return
-			 * bootefi_image_path
+			 * bootefi_device_path
 			 */
 			.guid = &efi_guid_device_path,
 			.open = &bootefi_open_dp,
@@ -73,11 +86,11 @@ 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_image_path,
+	.handle = bootefi_device_path,
 	.protocols = {
 		{
 			/* When asking for the device path interface, return
-			 * bootefi_image_path */
+			 * bootefi_device_path */
 			.guid = &efi_guid_device_path,
 			.open = &bootefi_open_dp,
 		}
@@ -192,7 +205,7 @@ U_BOOT_CMD(
 	bootefi_help_text
 );
 
-void efi_set_bootdev(const char *dev, const char *devnr)
+void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
 {
 	__maybe_unused struct blk_desc *desc;
 	char devname[16] = { 0 }; /* dp->str is u16[16] long */
@@ -217,7 +230,12 @@ void efi_set_bootdev(const char *dev, const char *devnr)
 	if (colon)
 		*colon = '\0';
 
-	/* Patch the bootefi_image_path to the target device */
+	/* Patch bootefi_device_path to the target device */
+	memset(bootefi_device_path[0].str, 0, sizeof(bootefi_device_path[0].str));
+	ascii2unicode(bootefi_device_path[0].str, devname);
+
+	/* Patch bootefi_image_path to the target file path */
 	memset(bootefi_image_path[0].str, 0, sizeof(bootefi_image_path[0].str));
+	snprintf(devname, sizeof(devname), "%s", path);
 	ascii2unicode(bootefi_image_path[0].str, devname);
 }
diff --git a/cmd/fs.c b/cmd/fs.c
index be8f289..abfe5be 100644
--- a/cmd/fs.c
+++ b/cmd/fs.c
@@ -27,7 +27,8 @@ U_BOOT_CMD(
 static int do_load_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
 				char * const argv[])
 {
-	efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "");
+	efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
+			(argc > 4) ? argv[4] : "");
 	return do_load(cmdtp, flag, argc, argv, FS_TYPE_ANY);
 }
 
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 9f61fc4..88b8149 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -112,7 +112,7 @@ efi_status_t efi_exit_func(efi_status_t ret);
 /* Call this to relocate the runtime section to an address space */
 void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map);
 /* Call this to set the current device name */
-void efi_set_bootdev(const char *dev, const char *devnr);
+void efi_set_bootdev(const char *dev, const char *devnr, const char *path);
 
 /* Generic EFI memory allocator, call this to get memory */
 void *efi_alloc(uint64_t len, int memory_type);
@@ -155,6 +155,7 @@ static inline void ascii2unicode(u16 *unicode, char *ascii)
 
 /* No loader configured, stub out EFI_ENTRY */
 static inline void efi_restore_gd(void) { }
-static inline void efi_set_bootdev(const char *dev, const char *devnr) { }
+static inline void efi_set_bootdev(const char *dev, const char *devnr,
+				   const char *path) { }
 
 #endif
-- 
1.8.5.6

[U-Boot] [PATCH 7/8] efi_loader: Increase path string to 32 characters

[Alexander Graf]

Whenever we want to tell our payload about a path, we limit ourselves
to a reasonable amount of characters. So far we only passed in device
names - exceeding 16 chars was unlikely there.

However by now we also pass real file path information, so let's increase
the limit to 32 characters. That way common paths like "boot/efi/bootaa64.efi"
fit just fine.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 cmd/bootefi.c             | 2 +-
 include/efi_api.h         | 2 +-
 lib/efi_loader/efi_disk.c | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index adcf645..f502996 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -208,7 +208,7 @@ U_BOOT_CMD(
 void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
 {
 	__maybe_unused struct blk_desc *desc;
-	char devname[16] = { 0 }; /* dp->str is u16[16] long */
+	char devname[32] = { 0 }; /* dp->str is u16[32] long */
 	char *colon;
 
 	/* Assemble the condensed device name we use in efi_disk.c */
diff --git a/include/efi_api.h b/include/efi_api.h
index 6960448..51d7586 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -259,7 +259,7 @@ struct efi_device_path {
 
 struct efi_device_path_file_path {
 	struct efi_device_path dp;
-	u16 str[16];
+	u16 str[32];
 };
 
 #define BLOCK_IO_GUID \
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index b3d56a8..28e5b7f 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -194,7 +194,7 @@ static int efi_disk_create_eltorito(struct blk_desc *desc,
 {
 	int disks = 0;
 #ifdef CONFIG_ISO_PARTITION
-	char devname[16] = { 0 }; /* dp->str is u16[16] long */
+	char devname[32] = { 0 }; /* dp->str is u16[32] long */
 	disk_partition_t info;
 	int part = 1;
 
@@ -231,7 +231,7 @@ int efi_disk_register(void)
 		printf("Scanning disks on %s...\n", cur_drvr->name);
 		for (i = 0; i < 4; i++) {
 			struct blk_desc *desc;
-			char devname[16] = { 0 }; /* dp->str is u16[16] long */
+			char devname[32] = { 0 }; /* dp->str is u16[32] long */
 
 			desc = blk_get_dev(cur_drvr->name, i);
 			if (!desc)
-- 
1.8.5.6

[U-Boot] [PATCH 8/8] distro: Enable iso partition code

[Alexander Graf]

Now that we can properly boot EFI payloads from iso el torito
images, let's enable support for isos by default in the distro
header.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 include/config_distro_defaults.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/config_distro_defaults.h b/include/config_distro_defaults.h
index 2ba7cf4..076be4d 100644
--- a/include/config_distro_defaults.h
+++ b/include/config_distro_defaults.h
@@ -62,6 +62,7 @@
 #define CONFIG_MENU
 #define CONFIG_DOS_PARTITION
 #define CONFIG_EFI_PARTITION
+#define CONFIG_ISO_PARTITION
 #define CONFIG_SUPPORT_RAW_INITRD
 #define CONFIG_SYS_HUSH_PARSER
 
-- 
1.8.5.6

[U-Boot] [PATCH 0/8] efi_loader: Support loading from El Torito isos

[Andreas Färber]

Am 11.04.2016 um 16:16 schrieb Alexander Graf:
> Alexander Graf (8):
>   iso: Make little endian and 64bit safe
>   iso: Start with partition 1
>   iso: Allow 512 byte sector size
>   efi_loader: Split drive add into function
>   efi_loader: Add el torito support
>   efi_loader: Pass file path to payload
>   efi_loader: Increase path string to 32 characters
>   distro: Enable iso partition code

Tested-by: Andreas F?rber <afaerber@suse.de>

Doesn't break the regular EFI boot path for me.

Regards,
Andreas

-- 
SUSE Linux GmbH, Maxfeldstr. 5, 90409 N?rnberg, Germany
GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)

[U-Boot] [U-Boot,1/8] iso: Make little endian and 64bit safe

[Tom Rini]

On Mon, Apr 11, 2016 at 04:16:14PM +0200, Alexander Graf wrote:

> The iso partition table implementation has a few endian and 64bit
> problems. Clean it up a bit to become endian and bitness safe.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160421/0791120e/attachment.sig>

[U-Boot] [U-Boot,2/8] iso: Start with partition 1

[Tom Rini]

On Mon, Apr 11, 2016 at 04:16:15PM +0200, Alexander Graf wrote:

> The generic partition code treats partition 0 as "whole disk". So
> we should start with partition 1 as the first partition in the iso
> partition table.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160421/cb2c9b09/attachment.sig>

[U-Boot] [U-Boot,3/8] iso: Allow 512 byte sector size

[Tom Rini]

On Mon, Apr 11, 2016 at 04:16:16PM +0200, Alexander Graf wrote:

> Real CD-ROMs are pretty obsolete these days. Usually people still keep
> iso files around, but just put them on USB sticks or SD cards and expect
> them to "just work".
> 
> To support this use case with El Torito images, add support for 512 byte
> sector size to the iso parsing code.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160421/0a611202/attachment.sig>

[U-Boot] [U-Boot, 4/8] efi_loader: Split drive add into function

[Tom Rini]

On Mon, Apr 11, 2016 at 04:16:17PM +0200, Alexander Graf wrote:

> The snippet of code to add a drive to our drive list needs to
> get called from 2 places in the future. Split it into a separate
> function.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160421/97073c1f/attachment.sig>

[U-Boot] [U-Boot,5/8] efi_loader: Add el torito support

[Tom Rini]

On Mon, Apr 11, 2016 at 04:16:18PM +0200, Alexander Graf wrote:

> When loading an el torito image, uEFI exposes said image as a raw
> block device to the payload.
> 
> Let's do the same by creating new block devices with added offsets for
> the respective el torito partitions.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160421/bd83996d/attachment.sig>

[U-Boot] [U-Boot,6/8] efi_loader: Pass file path to payload

[Tom Rini]

On Mon, Apr 11, 2016 at 04:16:19PM +0200, Alexander Graf wrote:

> The payload gets information on where it got loaded from. This includes
> the device as well as file path.
> 
> So far we've treated both as the same thing and always gave it the device
> name. However, in some situations grub2 actually wants to find its loading
> path to find its configuration file.
> 
> So let's split the two semantically separte bits into separate structs and
> pass the loaded file name into our payload when we load it using "load".
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160421/4e32243d/attachment.sig>

[U-Boot] [U-Boot, 7/8] efi_loader: Increase path string to 32 characters

[Tom Rini]

On Mon, Apr 11, 2016 at 04:16:20PM +0200, Alexander Graf wrote:

> Whenever we want to tell our payload about a path, we limit ourselves
> to a reasonable amount of characters. So far we only passed in device
> names - exceeding 16 chars was unlikely there.
> 
> However by now we also pass real file path information, so let's increase
> the limit to 32 characters. That way common paths like "boot/efi/bootaa64.efi"
> fit just fine.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160421/9ebe2167/attachment.sig>

[U-Boot] [U-Boot,8/8] distro: Enable iso partition code

[Tom Rini]

On Mon, Apr 11, 2016 at 04:16:21PM +0200, Alexander Graf wrote:

> Now that we can properly boot EFI payloads from iso el torito
> images, let's enable support for isos by default in the distro
> header.
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160421/0c346fea/attachment.sig>

[U-Boot] [U-Boot,6/8] efi_loader: Pass file path to payload

[Heinrich Schuchardt]

On 04/11/2016 04:16 PM, Alexander Graf wrote:
> The payload gets information on where it got loaded from. This includes
> the device as well as file path.
> 
> So far we've treated both as the same thing and always gave it the device
> name. However, in some situations grub2 actually wants to find its loading
> path to find its configuration file.
> 
> So let's split the two semantically separte bits into separate structs and
> pass the loaded file name into our payload when we load it using "load".
> 
> Signed-off-by: Alexander Graf <agraf@suse.de>
> ---
>  cmd/bootefi.c        | 32 +++++++++++++++++++++++++-------
>  cmd/fs.c             |  3 ++-
>  include/efi_loader.h |  5 +++--
>  3 files changed, 30 insertions(+), 10 deletions(-)
> 
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 0d09aa1..adcf645 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -34,17 +34,30 @@ static struct efi_device_path_file_path bootefi_image_path[] = {
>  	}
>  };
>  
> +static struct efi_device_path_file_path bootefi_device_path[] = {
> +	{
> +		.dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE,
> +		.dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH,
> +		.dp.length = sizeof(bootefi_image_path[0]),
> +		.str = { 'b','o','o','t','e','f','i' },
> +	}, {
> +		.dp.type = DEVICE_PATH_TYPE_END,
> +		.dp.sub_type = DEVICE_PATH_SUB_TYPE_END,
> +		.dp.length = sizeof(bootefi_image_path[0]),
> +	}
> +};
> +
>  static efi_status_t bootefi_open_dp(void *handle, efi_guid_t *protocol,
>  			void **protocol_interface, void *agent_handle,
>  			void *controller_handle, uint32_t attributes)
>  {
> -	*protocol_interface = bootefi_image_path;
> +	*protocol_interface = 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_image_path,
> +	.device_handle = bootefi_device_path,
>  	.file_path = bootefi_image_path,
>  };
>  
> @@ -63,7 +76,7 @@ static struct efi_object loaded_image_info_obj = {
>  		{
>  			/*
>  			 * When asking for the device path interface, return
> -			 * bootefi_image_path
> +			 * bootefi_device_path
>  			 */
>  			.guid = &efi_guid_device_path,
>  			.open = &bootefi_open_dp,
> @@ -73,11 +86,11 @@ 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_image_path,
> +	.handle = bootefi_device_path,
>  	.protocols = {
>  		{
>  			/* When asking for the device path interface, return
> -			 * bootefi_image_path */
> +			 * bootefi_device_path */
>  			.guid = &efi_guid_device_path,
>  			.open = &bootefi_open_dp,
>  		}
> @@ -192,7 +205,7 @@ U_BOOT_CMD(
>  	bootefi_help_text
>  );
>  
> -void efi_set_bootdev(const char *dev, const char *devnr)
> +void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
>  {
>  	__maybe_unused struct blk_desc *desc;
>  	char devname[16] = { 0 }; /* dp->str is u16[16] long */
> @@ -217,7 +230,12 @@ void efi_set_bootdev(const char *dev, const char *devnr)
>  	if (colon)
>  		*colon = '\0';
>  
> -	/* Patch the bootefi_image_path to the target device */
> +	/* Patch bootefi_device_path to the target device */
> +	memset(bootefi_device_path[0].str, 0, sizeof(bootefi_device_path[0].str));
> +	ascii2unicode(bootefi_device_path[0].str, devname);
> +
> +	/* Patch bootefi_image_path to the target file path */
>  	memset(bootefi_image_path[0].str, 0, sizeof(bootefi_image_path[0].str));
> +	snprintf(devname, sizeof(devname), "%s", path);
>  	ascii2unicode(bootefi_image_path[0].str, devname);
>  }
> diff --git a/cmd/fs.c b/cmd/fs.c
> index be8f289..abfe5be 100644
> --- a/cmd/fs.c
> +++ b/cmd/fs.c
> @@ -27,7 +27,8 @@ U_BOOT_CMD(
>  static int do_load_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
>  				char * const argv[])
>  {
> -	efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "");
> +	efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
> +			(argc > 4) ? argv[4] : "");

Hi Alex,

efi_set_bootdev is used here to set both the device from which a file
was loaded and the filename used as image_path.

Why should we assume that the last file loaded is the EFI executable and
not the device tree (or any other file)?

Shouldn't we at least check that this file is an EFI executable or driver?

Best regards

Heinrich

>  	return do_load(cmdtp, flag, argc, argv, FS_TYPE_ANY);
>  }
>  
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 9f61fc4..88b8149 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -112,7 +112,7 @@ efi_status_t efi_exit_func(efi_status_t ret);
>  /* Call this to relocate the runtime section to an address space */
>  void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map);
>  /* Call this to set the current device name */
> -void efi_set_bootdev(const char *dev, const char *devnr);
> +void efi_set_bootdev(const char *dev, const char *devnr, const char *path);
>  
>  /* Generic EFI memory allocator, call this to get memory */
>  void *efi_alloc(uint64_t len, int memory_type);
> @@ -155,6 +155,7 @@ static inline void ascii2unicode(u16 *unicode, char *ascii)
>  
>  /* No loader configured, stub out EFI_ENTRY */
>  static inline void efi_restore_gd(void) { }
> -static inline void efi_set_bootdev(const char *dev, const char *devnr) { }
> +static inline void efi_set_bootdev(const char *dev, const char *devnr,
> +				   const char *path) { }
>  
>  #endif
> 

[U-Boot] [U-Boot,6/8] efi_loader: Pass file path to payload

[Alexander Graf]

On 08/13/2018 09:21 PM, Heinrich Schuchardt wrote:
> On 04/11/2016 04:16 PM, Alexander Graf wrote:
>> The payload gets information on where it got loaded from. This includes
>> the device as well as file path.
>>
>> So far we've treated both as the same thing and always gave it the device
>> name. However, in some situations grub2 actually wants to find its loading
>> path to find its configuration file.
>>
>> So let's split the two semantically separte bits into separate structs and
>> pass the loaded file name into our payload when we load it using "load".
>>
>> Signed-off-by: Alexander Graf <agraf@suse.de>
>> ---
>>   cmd/bootefi.c        | 32 +++++++++++++++++++++++++-------
>>   cmd/fs.c             |  3 ++-
>>   include/efi_loader.h |  5 +++--
>>   3 files changed, 30 insertions(+), 10 deletions(-)
>>
>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
>> index 0d09aa1..adcf645 100644
>> --- a/cmd/bootefi.c
>> +++ b/cmd/bootefi.c
>> @@ -34,17 +34,30 @@ static struct efi_device_path_file_path bootefi_image_path[] = {
>>   	}
>>   };
>>   
>> +static struct efi_device_path_file_path bootefi_device_path[] = {
>> +	{
>> +		.dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE,
>> +		.dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH,
>> +		.dp.length = sizeof(bootefi_image_path[0]),
>> +		.str = { 'b','o','o','t','e','f','i' },
>> +	}, {
>> +		.dp.type = DEVICE_PATH_TYPE_END,
>> +		.dp.sub_type = DEVICE_PATH_SUB_TYPE_END,
>> +		.dp.length = sizeof(bootefi_image_path[0]),
>> +	}
>> +};
>> +
>>   static efi_status_t bootefi_open_dp(void *handle, efi_guid_t *protocol,
>>   			void **protocol_interface, void *agent_handle,
>>   			void *controller_handle, uint32_t attributes)
>>   {
>> -	*protocol_interface = bootefi_image_path;
>> +	*protocol_interface = 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_image_path,
>> +	.device_handle = bootefi_device_path,
>>   	.file_path = bootefi_image_path,
>>   };
>>   
>> @@ -63,7 +76,7 @@ static struct efi_object loaded_image_info_obj = {
>>   		{
>>   			/*
>>   			 * When asking for the device path interface, return
>> -			 * bootefi_image_path
>> +			 * bootefi_device_path
>>   			 */
>>   			.guid = &efi_guid_device_path,
>>   			.open = &bootefi_open_dp,
>> @@ -73,11 +86,11 @@ 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_image_path,
>> +	.handle = bootefi_device_path,
>>   	.protocols = {
>>   		{
>>   			/* When asking for the device path interface, return
>> -			 * bootefi_image_path */
>> +			 * bootefi_device_path */
>>   			.guid = &efi_guid_device_path,
>>   			.open = &bootefi_open_dp,
>>   		}
>> @@ -192,7 +205,7 @@ U_BOOT_CMD(
>>   	bootefi_help_text
>>   );
>>   
>> -void efi_set_bootdev(const char *dev, const char *devnr)
>> +void efi_set_bootdev(const char *dev, const char *devnr, const char *path)
>>   {
>>   	__maybe_unused struct blk_desc *desc;
>>   	char devname[16] = { 0 }; /* dp->str is u16[16] long */
>> @@ -217,7 +230,12 @@ void efi_set_bootdev(const char *dev, const char *devnr)
>>   	if (colon)
>>   		*colon = '\0';
>>   
>> -	/* Patch the bootefi_image_path to the target device */
>> +	/* Patch bootefi_device_path to the target device */
>> +	memset(bootefi_device_path[0].str, 0, sizeof(bootefi_device_path[0].str));
>> +	ascii2unicode(bootefi_device_path[0].str, devname);
>> +
>> +	/* Patch bootefi_image_path to the target file path */
>>   	memset(bootefi_image_path[0].str, 0, sizeof(bootefi_image_path[0].str));
>> +	snprintf(devname, sizeof(devname), "%s", path);
>>   	ascii2unicode(bootefi_image_path[0].str, devname);
>>   }
>> diff --git a/cmd/fs.c b/cmd/fs.c
>> index be8f289..abfe5be 100644
>> --- a/cmd/fs.c
>> +++ b/cmd/fs.c
>> @@ -27,7 +27,8 @@ U_BOOT_CMD(
>>   static int do_load_wrapper(cmd_tbl_t *cmdtp, int flag, int argc,
>>   				char * const argv[])
>>   {
>> -	efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "");
>> +	efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
>> +			(argc > 4) ? argv[4] : "");
> Hi Alex,
>
> efi_set_bootdev is used here to set both the device from which a file
> was loaded and the filename used as image_path.
>
> Why should we assume that the last file loaded is the EFI executable and
> not the device tree (or any other file)?

The image path logic goes hand in hand with the distro boot logic which 
tries to always load the efi binary last. If you found a case where it 
doesn't, let's fix that up please.

> Shouldn't we at least check that this file is an EFI executable or driver?

Well, that would only be a marginal improvement, no? And in fact I'm not 
sure I'd be very happy to clutter the load code with such logic.

The "real" fix would be to create a LoadedImage object for every "load" 
call and then just search for the one that starts at the bootefi load 
address. But I'm also not sure people who don't want to run any efi 
payloads want to pay the memory penalty that incurs.


Alex