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

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(-)

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;

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!

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) = {

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!

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; }

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!

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

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!

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

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!

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

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!

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

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

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)

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!

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

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!

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
participants (4)
-
Alexander Graf
-
Andreas Färber
-
Heinrich Schuchardt
-
Tom Rini