[PATCH v1 0/2] SPL EBBR - EFI System Partition support

[PATCH v1 0/2] SPL EBBR - EFI System Partition support

[Mayuresh Chitale]

This series adds support to locate an EFI System Partition on a disk and
boot the next stage from such a parition if found. The next stage image
is expected to be under the FIRMWARE directory as described in the EBBR
specification [1]. Also update the spl_blk_fs and spl_fat drivers to
prefer booting from ESP and fall back to the configured parition in
case of a failure.

These patches are built on top of the following series from Sean Anderson:
https://lists.denx.de/pipermail/u-boot/2023-August/525665.html

[1] : https://github.com/ARM-software/ebbr/blob/854ba0e68590102667b84a5ba4e0b076a3f5f2cb/source/chapter4-firmware-media.rst

The patches are also present in the branch below:
https://github.com/mdchitale/u-boot/tree/mchitale_spl_ebbr_v1

Mayuresh Chitale (2):
  part: Add a function to find ESP partition
  spl: Add support for booting from ESP

 common/spl/Kconfig      |  7 +++++
 common/spl/spl_blk_fs.c | 61 +++++++++++++++++++++++++++++------------
 common/spl/spl_fat.c    | 34 ++++++++++++++++++++---
 disk/part.c             | 16 +++++++++++
 include/part.h          | 10 +++++++
 5 files changed, 107 insertions(+), 21 deletions(-)

-- 
2.34.1

[PATCH v1 1/2] part: Add a function to find ESP partition

[Mayuresh Chitale]

If a disk has an EFI system partition (ESP) then it can be used to
locate the boot files. Add a function to find the ESP.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 disk/part.c    | 16 ++++++++++++++++
 include/part.h | 10 ++++++++++
 2 files changed, 26 insertions(+)

diff --git a/disk/part.c b/disk/part.c
index 72241b7b23..2be0866671 100644
--- a/disk/part.c
+++ b/disk/part.c
@@ -841,3 +841,19 @@ int part_get_bootable(struct blk_desc *desc)
 
 	return 0;
 }
+
+int part_get_esp(struct blk_desc *desc)
+{
+	struct disk_partition info;
+	int p;
+
+	for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
+		int ret;
+
+		ret = part_get_info(desc, p, &info);
+		if (!ret && (info.bootable & PART_EFI_SYSTEM_PARTITION))
+			return p;
+	}
+
+	return 0;
+}
diff --git a/include/part.h b/include/part.h
index db34bc6bb7..036f9fd762 100644
--- a/include/part.h
+++ b/include/part.h
@@ -689,6 +689,13 @@ int part_get_type_by_name(const char *name);
  * @return first bootable partition, or 0 if there is none
  */
 int part_get_bootable(struct blk_desc *desc);
+/**
+ * part_get_esp() - Find the EFI system partition
+ *
+ * @desc: Block-device descriptor
+ * @Return the EFI system partition, or 0 if there is none
+ */
+int part_get_esp(struct blk_desc *desc);
 
 #else
 static inline int part_driver_get_count(void)
@@ -700,6 +707,9 @@ static inline struct part_driver *part_driver_get_first(void)
 static inline bool part_get_bootable(struct blk_desc *desc)
 { return false; }
 
+static int part_get_esp(struct blk_desc *desc)
+{ return 0; }
+
 #endif /* CONFIG_PARTITIONS */
 
 #endif /* _PART_H */
-- 
2.34.1

[PATCH v1 2/2] spl: Add support for booting from ESP

[Mayuresh Chitale]

Some platforms as described by EBBR specification may store images in
the FIRMWARE directory of the UEFI system partition(ESP). Add support
to boot from the EFI system partition if it is enabled for a platform.

Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
---
 common/spl/Kconfig      |  7 +++++
 common/spl/spl_blk_fs.c | 61 +++++++++++++++++++++++++++++------------
 common/spl/spl_fat.c    | 34 ++++++++++++++++++++---
 3 files changed, 81 insertions(+), 21 deletions(-)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 76bde18515..92984b19da 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -1241,6 +1241,13 @@ config SPL_SATA_RAW_U_BOOT_SECTOR
 	  Sector on the SATA disk to load U-Boot from, when the SATA disk is being
 	  used in raw mode. Units: SATA disk sectors (1 sector = 512 bytes).
 
+config SPL_ESP_BOOT
+	bool "Load next stage boot image from the UEFI system partition"
+	select SPL_PARTITION_TYPE_GUID
+	help
+	  When enabled, first try to boot from the UEFI system partition as
+	  described in the Ch.4 of the EBBR specification.
+
 config SPL_NVME
 	bool "NVM Express device support"
 	depends on BLK
diff --git a/common/spl/spl_blk_fs.c b/common/spl/spl_blk_fs.c
index 5268daaaff..78d1478426 100644
--- a/common/spl/spl_blk_fs.c
+++ b/common/spl/spl_blk_fs.c
@@ -9,9 +9,12 @@
 #include <spl.h>
 #include <image.h>
 #include <fs.h>
+#include <part.h>
 
 struct blk_dev {
 	const char *ifname;
+	int devnum;
+	int partnum;
 	char dev_part_str[8];
 };
 
@@ -39,16 +42,38 @@ static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
 	return actlen;
 }
 
+static int spl_blk_file_size(struct blk_dev *dev, const char *filename,
+			     loff_t *filesize)
+{
+	int ret;
+
+	snprintf(dev->dev_part_str, sizeof(dev->dev_part_str) - 1, "%x:%x",
+		 dev->devnum, dev->partnum);
+	debug("Loading file %s from %s %s\n", filename, dev->ifname,
+	      dev->dev_part_str);
+	ret = fs_set_blk_dev(dev->ifname, dev->dev_part_str, FS_TYPE_ANY);
+	if (ret) {
+		printf("spl: unable to set blk_dev %s %s. Err - %d\n",
+		       dev->ifname, dev->dev_part_str, ret);
+		return ret;
+	}
+
+	ret = fs_size(filename, filesize);
+	if (ret)
+		printf("spl: unable to get size, file: %s. Err - %d\n",
+		       filename, ret);
+	return ret;
+}
+
 int spl_blk_load_image(struct spl_image_info *spl_image,
 		       struct spl_boot_device *bootdev,
 		       enum uclass_id uclass_id, int devnum, int partnum)
 {
 	const char *filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME;
-	struct legacy_img_hdr *header;
 	struct blk_desc *blk_desc;
 	loff_t filesize;
 	struct blk_dev dev;
-	int ret;
+	int ret, part;
 	struct spl_load_info load = {
 		.read = spl_fit_read,
 		.bl_len = 1,
@@ -63,24 +88,26 @@ int spl_blk_load_image(struct spl_image_info *spl_image,
 	}
 
 	blk_show_device(uclass_id, devnum);
-	header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));
-
 	dev.ifname = blk_get_uclass_name(uclass_id);
-	snprintf(dev.dev_part_str, sizeof(dev.dev_part_str) - 1, "%x:%x",
-		 devnum, partnum);
-	ret = fs_set_blk_dev(dev.ifname, dev.dev_part_str, FS_TYPE_ANY);
-	if (ret) {
-		printf("spl: unable to set blk_dev %s %s. Err - %d\n",
-		       dev.ifname, dev.dev_part_str, ret);
-		return ret;
+	dev.devnum = devnum;
+	/*
+	 * First try to boot from EFI System partition. In case of failure,
+	 * fall back to the configured partition.
+	 */
+	if (IS_ENABLED(CONFIG_SPL_ESP_BOOT)) {
+		part = part_get_esp(blk_desc);
+		if (part) {
+			dev.partnum = part;
+			ret = spl_blk_file_size(&dev, filename, &filesize);
+			if (!ret)
+				goto out;
+		}
 	}
 
-	ret = fs_size(filename, &filesize);
-	if (ret) {
-		printf("spl: unable to get file size: %s. Err - %d\n",
-		       filename, ret);
+	dev.partnum = partnum;
+	ret = spl_blk_file_size(&dev, filename, &filesize);
+	if (ret)
 		return ret;
-	}
-
+out:
 	return spl_load(spl_image, bootdev, &load, filesize, 0);
 }
diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
index 6530bcd5a7..63091f2bd6 100644
--- a/common/spl/spl_fat.c
+++ b/common/spl/spl_fat.c
@@ -54,10 +54,10 @@ static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
 	return actread;
 }
 
-int spl_load_image_fat(struct spl_image_info *spl_image,
-		       struct spl_boot_device *bootdev,
-		       struct blk_desc *block_dev, int partition,
-		       const char *filename)
+int spl_load_image_fat_one(struct spl_image_info *spl_image,
+			   struct spl_boot_device *bootdev,
+			   struct blk_desc *block_dev, int partition,
+			   const char *filename)
 {
 	int err;
 	loff_t size;
@@ -96,6 +96,32 @@ end:
 	return err;
 }
 
+int spl_load_image_fat(struct spl_image_info *spl_image,
+		       struct spl_boot_device *bootdev,
+		       struct blk_desc *block_dev, int partition,
+		       const char *filename)
+{
+	int err, part;
+
+	/*
+	 * First try to boot from EFI System partition. In case of failure,
+	 * fall back to the configured partition.
+	 */
+	if (IS_ENABLED(CONFIG_SPL_ESP_BOOT)) {
+		part = part_get_esp(block_dev);
+		if (part) {
+			err = spl_load_image_fat_one(spl_image, bootdev,
+						     block_dev, part,
+						     filename);
+			if (!err)
+				return err;
+		}
+	}
+
+	return spl_load_image_fat_one(spl_image, bootdev, block_dev,
+				      partition, filename);
+}
+
 #if CONFIG_IS_ENABLED(OS_BOOT)
 int spl_load_image_fat_os(struct spl_image_info *spl_image,
 			  struct spl_boot_device *bootdev,
-- 
2.34.1

Re: [PATCH v1 0/2] SPL EBBR - EFI System Partition support

[Heinrich Schuchardt]

On 14.09.23 12:08, Mayuresh Chitale wrote:
> This series adds support to locate an EFI System Partition on a disk and
> boot the next stage from such a parition if found. The next stage image
> is expected to be under the FIRMWARE directory as described in the EBBR
> specification [1]. Also update the spl_blk_fs and spl_fat drivers to
> prefer booting from ESP and fall back to the configured parition in
> case of a failure.
>
> These patches are built on top of the following series from Sean Anderson:
> https://lists.denx.de/pipermail/u-boot/2023-August/525665.html
>
> [1] : https://github.com/ARM-software/ebbr/blob/854ba0e68590102667b84a5ba4e0b076a3f5f2cb/source/chapter4-firmware-media.rst

Hello Sean,

your series [1] is in status "changes requested". Will a new version
come up soon, or can this change go first?

Best regards

Heinrich


>
> The patches are also present in the branch below:
> https://github.com/mdchitale/u-boot/tree/mchitale_spl_ebbr_v1
>
> Mayuresh Chitale (2):
>    part: Add a function to find ESP partition
>    spl: Add support for booting from ESP
>
>   common/spl/Kconfig      |  7 +++++
>   common/spl/spl_blk_fs.c | 61 +++++++++++++++++++++++++++++------------
>   common/spl/spl_fat.c    | 34 ++++++++++++++++++++---
>   disk/part.c             | 16 +++++++++++
>   include/part.h          | 10 +++++++
>   5 files changed, 107 insertions(+), 21 deletions(-)
>

Re: [PATCH v1 0/2] SPL EBBR - EFI System Partition support

[Sean Anderson]

Hi Heinrich,

On 9/14/23 10:15, Heinrich Schuchardt wrote:
> On 14.09.23 12:08, Mayuresh Chitale wrote:
>> This series adds support to locate an EFI System Partition on a disk and
>> boot the next stage from such a parition if found. The next stage image
>> is expected to be under the FIRMWARE directory as described in the EBBR
>> specification [1]. Also update the spl_blk_fs and spl_fat drivers to
>> prefer booting from ESP and fall back to the configured parition in
>> case of a failure.
>>
>> These patches are built on top of the following series from Sean Anderson:
>> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2flists.denx.de%2fpipermail%2fu%2dboot%2f2023%2dAugust%2f525665.html&umid=9846b220-30e5-47d9-980b-6b2a067d6959&auth=d807158c60b7d2502abde8a2fc01f40662980862-8c7a372e72c3ce6a78488f17bac32c55586848b7
>>
>> [1] : https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fgithub.com%2fARM%2dsoftware%2febbr%2fblob%2f854ba0e68590102667b84a5ba4e0b076a3f5f2cb%2fsource%2fchapter4%2dfirmware%2dmedia.rst&umid=9846b220-30e5-47d9-980b-6b2a067d6959&auth=d807158c60b7d2502abde8a2fc01f40662980862-c4109227e8dc78dcfe731b6e4cfe9dfef856e354
> 
> Hello Sean,
> 
> your series [1] is in status "changes requested". Will a new version
> come up soon, or can this change go first?

I had been working to incorporate your suggestion regarding bl_len.
However, I have not had the time recently to finish that. I also need to
have another look at the size changes once that is implemented. I had
been hoping to use [1] to get some general size reductions on most
platforms, but I am going to have to revisit that as well. I would like
to work on this, but at the moment it's not high priority for me.
Changes like this should likely go in first, since I suspect it will
take another revision or two to finish up that series.

--Sean

[1] https://lore.kernel.org/u-boot/20230810221138.GY3630934@bill-the-cat/

> 
> 
>>
>> The patches are also present in the branch below:
>> https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fgithub.com%2fmdchitale%2fu%2dboot%2ftree%2fmchitale%5fspl%5febbr%5fv1&umid=9846b220-30e5-47d9-980b-6b2a067d6959&auth=d807158c60b7d2502abde8a2fc01f40662980862-6f91c84d8d5661347bae86e4cb72a96341f7048d
>>
>> Mayuresh Chitale (2):
>>    part: Add a function to find ESP partition
>>    spl: Add support for booting from ESP
>>
>>   common/spl/Kconfig      |  7 +++++
>>   common/spl/spl_blk_fs.c | 61 +++++++++++++++++++++++++++++------------
>>   common/spl/spl_fat.c    | 34 ++++++++++++++++++++---
>>   disk/part.c             | 16 +++++++++++
>>   include/part.h          | 10 +++++++
>>   5 files changed, 107 insertions(+), 21 deletions(-)
>>
> 

Re: [PATCH v1 2/2] spl: Add support for booting from ESP

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1886 bytes --]

On Thu, Sep 14, 2023 at 03:38:21PM +0530, Mayuresh Chitale wrote:

> Some platforms as described by EBBR specification may store images in
> the FIRMWARE directory of the UEFI system partition(ESP). Add support
> to boot from the EFI system partition if it is enabled for a platform.
> 
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
[snip]
> +config SPL_ESP_BOOT
> +	bool "Load next stage boot image from the UEFI system partition"
> +	select SPL_PARTITION_TYPE_GUID
> +	help
> +	  When enabled, first try to boot from the UEFI system partition as
> +	  described in the Ch.4 of the EBBR specification.

We need to select this by perhaps BOOT_DEFAULTS or BOOTMETH_DISTRO,
whatever is supposed to signify that yes, this is going to be a
SystemReady IR (or higher) compliant build.

[snip]
>  int spl_blk_load_image(struct spl_image_info *spl_image,
>  		       struct spl_boot_device *bootdev,
>  		       enum uclass_id uclass_id, int devnum, int partnum)
>  {
>  	const char *filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME;
> -	struct legacy_img_hdr *header;
[snip]
> @@ -63,24 +88,26 @@ int spl_blk_load_image(struct spl_image_info *spl_image,
>  	}
>  
>  	blk_show_device(uclass_id, devnum);
> -	header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));
> -
[snip]

Is this an artifact of Sean's patch series where "header" is no longer
actually used?  As-is this won't compile on top of next (which is where
given Sean's feedback, the series needs to be rebased upon anyhow).

> +	/*
> +	 * First try to boot from EFI System partition. In case of failure,
> +	 * fall back to the configured partition.
> +	 */

I don't know that this is the right behavior.  If we're configured to
boot from partition X, we boot from partition X.  If we're configured to
find the ESP and use that, we find and use that.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v1 1/2] part: Add a function to find ESP partition

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 1378 bytes --]

On Thu, Sep 14, 2023 at 03:38:20PM +0530, Mayuresh Chitale wrote:
> If a disk has an EFI system partition (ESP) then it can be used to
> locate the boot files. Add a function to find the ESP.
> 
> Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  disk/part.c    | 16 ++++++++++++++++
>  include/part.h | 10 ++++++++++
>  2 files changed, 26 insertions(+)
> 
> diff --git a/disk/part.c b/disk/part.c
> index 72241b7b23..2be0866671 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -841,3 +841,19 @@ int part_get_bootable(struct blk_desc *desc)
>  
>  	return 0;
>  }
> +
> +int part_get_esp(struct blk_desc *desc)
> +{
> +	struct disk_partition info;
> +	int p;
> +
> +	for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
> +		int ret;
> +
> +		ret = part_get_info(desc, p, &info);
> +		if (!ret && (info.bootable & PART_EFI_SYSTEM_PARTITION))
> +			return p;
> +	}
> +
> +	return 0;
> +}
> diff --git a/include/part.h b/include/part.h
> index db34bc6bb7..036f9fd762 100644
> --- a/include/part.h
> +++ b/include/part.h
> @@ -689,6 +689,13 @@ int part_get_type_by_name(const char *name);
>   * @return first bootable partition, or 0 if there is none
>   */
>  int part_get_bootable(struct blk_desc *desc);
> +/**

Missing blank line?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v1 2/2] spl: Add support for booting from ESP

[mchitale]

On Thu, 2023-09-14 at 12:29 -0400, Tom Rini wrote:
> On Thu, Sep 14, 2023 at 03:38:21PM +0530, Mayuresh Chitale wrote:
> 
> > Some platforms as described by EBBR specification may store images
> > in
> > the FIRMWARE directory of the UEFI system partition(ESP). Add
> > support
> > to boot from the EFI system partition if it is enabled for a
> > platform.
> > 
> > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> [snip]
> > +config SPL_ESP_BOOT
> > +	bool "Load next stage boot image from the UEFI system
> > partition"
> > +	select SPL_PARTITION_TYPE_GUID
> > +	help
> > +	  When enabled, first try to boot from the UEFI system
> > partition as
> > +	  described in the Ch.4 of the EBBR specification.
> 
> We need to select this by perhaps BOOT_DEFAULTS or BOOTMETH_DISTRO,
> whatever is supposed to signify that yes, this is going to be a
> SystemReady IR (or higher) compliant build.
I am not sure about the SystemReady compliance but I think the config
option can be enabled for those platforms which require it.
> [snip]
> >  int spl_blk_load_image(struct spl_image_info *spl_image,
> >  		       struct spl_boot_device *bootdev,
> >  		       enum uclass_id uclass_id, int devnum, int
> > partnum)
> >  {
> >  	const char *filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME;
> > -	struct legacy_img_hdr *header;
> [snip]
> > @@ -63,24 +88,26 @@ int spl_blk_load_image(struct spl_image_info
> > *spl_image,
> >  	}
> >  
> >  	blk_show_device(uclass_id, devnum);
> > -	header = spl_get_load_buffer(-sizeof(*header),
> > sizeof(*header));
> > -
> [snip]
> 
> Is this an artifact of Sean's patch series where "header" is no
> longer
> actually used?  As-is this won't compile on top of next (which is
> where
> given Sean's feedback, the series needs to be rebased upon anyhow).
Yes, I will rebase on the latest next without Sean's series and resend
the patch.
> > +	/*
> > +	 * First try to boot from EFI System partition. In case of
> > failure,
> > +	 * fall back to the configured partition.
> > +	 */
> 
> I don't know that this is the right behavior.  If we're configured to
> boot from partition X, we boot from partition X.  If we're configured
> to
> find the ESP and use that, we find and use that.

This is to maintain compatibility and make more attempts to boot. So
the same SPL binary can load images from devices with or without the
ESP.

Re: [PATCH v1 1/2] part: Add a function to find ESP partition

[mchitale]

On Thu, 2023-09-14 at 12:29 -0400, Tom Rini wrote:
> On Thu, Sep 14, 2023 at 03:38:20PM +0530, Mayuresh Chitale wrote:
> > If a disk has an EFI system partition (ESP) then it can be used to
> > locate the boot files. Add a function to find the ESP.
> > 
> > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com
> > >
> > ---
> >  disk/part.c    | 16 ++++++++++++++++
> >  include/part.h | 10 ++++++++++
> >  2 files changed, 26 insertions(+)
> > 
> > diff --git a/disk/part.c b/disk/part.c
> > index 72241b7b23..2be0866671 100644
> > --- a/disk/part.c
> > +++ b/disk/part.c
> > @@ -841,3 +841,19 @@ int part_get_bootable(struct blk_desc *desc)
> >  
> >  	return 0;
> >  }
> > +
> > +int part_get_esp(struct blk_desc *desc)
> > +{
> > +	struct disk_partition info;
> > +	int p;
> > +
> > +	for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
> > +		int ret;
> > +
> > +		ret = part_get_info(desc, p, &info);
> > +		if (!ret && (info.bootable &
> > PART_EFI_SYSTEM_PARTITION))
> > +			return p;
> > +	}
> > +
> > +	return 0;
> > +}
> > diff --git a/include/part.h b/include/part.h
> > index db34bc6bb7..036f9fd762 100644
> > --- a/include/part.h
> > +++ b/include/part.h
> > @@ -689,6 +689,13 @@ int part_get_type_by_name(const char *name);
> >   * @return first bootable partition, or 0 if there is none
> >   */
> >  int part_get_bootable(struct blk_desc *desc);
> > +/**
> 
> Missing blank line?
> 
Ok, I will add it.

Re: [PATCH v1 2/2] spl: Add support for booting from ESP

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2924 bytes --]

On Thu, Sep 21, 2023 at 05:03:37PM +0530, mchitale@ventanamicro.com wrote:
> On Thu, 2023-09-14 at 12:29 -0400, Tom Rini wrote:
> > On Thu, Sep 14, 2023 at 03:38:21PM +0530, Mayuresh Chitale wrote:
> > 
> > > Some platforms as described by EBBR specification may store images
> > > in
> > > the FIRMWARE directory of the UEFI system partition(ESP). Add
> > > support
> > > to boot from the EFI system partition if it is enabled for a
> > > platform.
> > > 
> > > Signed-off-by: Mayuresh Chitale <mchitale@ventanamicro.com>
> > [snip]
> > > +config SPL_ESP_BOOT
> > > +	bool "Load next stage boot image from the UEFI system
> > > partition"
> > > +	select SPL_PARTITION_TYPE_GUID
> > > +	help
> > > +	  When enabled, first try to boot from the UEFI system
> > > partition as
> > > +	  described in the Ch.4 of the EBBR specification.
> > 
> > We need to select this by perhaps BOOT_DEFAULTS or BOOTMETH_DISTRO,
> > whatever is supposed to signify that yes, this is going to be a
> > SystemReady IR (or higher) compliant build.
> I am not sure about the SystemReady compliance but I think the config
> option can be enabled for those platforms which require it.

It's "nearly everyone" that needs the option, which is why I'm saying it
needs to be part of the high level options that a board selects to say
they want to be compliant.

> > [snip]
> > >  int spl_blk_load_image(struct spl_image_info *spl_image,
> > >  		       struct spl_boot_device *bootdev,
> > >  		       enum uclass_id uclass_id, int devnum, int
> > > partnum)
> > >  {
> > >  	const char *filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME;
> > > -	struct legacy_img_hdr *header;
> > [snip]
> > > @@ -63,24 +88,26 @@ int spl_blk_load_image(struct spl_image_info
> > > *spl_image,
> > >  	}
> > >  
> > >  	blk_show_device(uclass_id, devnum);
> > > -	header = spl_get_load_buffer(-sizeof(*header),
> > > sizeof(*header));
> > > -
> > [snip]
> > 
> > Is this an artifact of Sean's patch series where "header" is no
> > longer
> > actually used?  As-is this won't compile on top of next (which is
> > where
> > given Sean's feedback, the series needs to be rebased upon anyhow).
> Yes, I will rebase on the latest next without Sean's series and resend
> the patch.

OK.

> > > +	/*
> > > +	 * First try to boot from EFI System partition. In case of
> > > failure,
> > > +	 * fall back to the configured partition.
> > > +	 */
> > 
> > I don't know that this is the right behavior.  If we're configured to
> > boot from partition X, we boot from partition X.  If we're configured
> > to
> > find the ESP and use that, we find and use that.
> 
> This is to maintain compatibility and make more attempts to boot. So
> the same SPL binary can load images from devices with or without the
> ESP.

Yes, I don't think that's what we want.  It should be an either/or not a
a then b.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]