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

[Heinrich Schuchardt]

On 16.01.24 13:36, 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 at ventanamicro.com>

Thank you for respinning this series.

For future submissions it would be preferable to have the changes
between the versions also in the individual patches.

> ---
>   common/spl/Kconfig      |  8 +++++++
>   common/spl/spl_blk_fs.c | 51 ++++++++++++++++++++++++++++++-----------
>   common/spl/spl_fat.c    | 34 +++++++++++++++++++++++----
>   3 files changed, 75 insertions(+), 18 deletions(-)
>
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index cf7ffc9b112..48e4e43196a 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -1292,6 +1292,14 @@ 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"
> +	default y if BOOT_DEFAULTS
> +	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 04eac6f306b..a2e8c2ce910 100644
> --- a/common/spl/spl_blk_fs.c
> +++ b/common/spl/spl_blk_fs.c
> @@ -10,12 +10,15 @@
>   #include <spl_load.h>
>   #include <image.h>
>   #include <fs.h>
> +#include <part.h>
>   #include <asm/cache.h>
>   #include <asm/io.h>
>
>   struct blk_dev {
>   	const char *ifname;
>   	const char *filename;
> +	int devnum;
> +	int partnum;
>   	char dev_part_str[8];
>   };
>
> @@ -44,6 +47,29 @@ 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);

Using log functions allows to provide more information if
CONFIG_SPL_LOG=y otherwise they fall back to printf() and debug().

> +	debug("Loading file %s from %s %s\n", filename, dev->ifname,
> +	      dev->dev_part_str);

log_debug(

> +	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",

SPL binary size is limited on many systems. There is already a message
telling that we are in SPL. I would suggest to abbreviate the message.

log_err("Can't access %s %s\n", dev->ifname, dev->dev_part_str);

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

log_err("File not found %s\n", filename);

Moving to log functions and adjusting the messages can be done in a
follow up patch. I don't want to stop this series.

Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>


> +	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)
> @@ -53,7 +79,7 @@ int spl_blk_load_image(struct spl_image_info *spl_image,
>   	loff_t filesize;
>   	struct blk_dev dev;
>   	struct spl_load_info load;
> -	int ret;
> +	int ret, part;
>
>   	blk_desc = blk_get_devnum_by_uclass_id(uclass_id, devnum);
>   	if (!blk_desc) {
> @@ -65,21 +91,18 @@ int spl_blk_load_image(struct spl_image_info *spl_image,
>
>   	dev.filename = filename;
>   	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;
> +	dev.partnum = partnum;
> +	if (IS_ENABLED(CONFIG_SPL_ESP_BOOT)) {
> +		part = part_get_esp(blk_desc);
> +		if (part)
> +			dev.partnum = part;
> +		else
> +			return -ENODEV;
>   	}
> -
> -	ret = fs_size(filename, &filesize);
> -	if (ret) {
> -		printf("spl: unable to get file size: %s. Err - %d\n",
> -		       filename, ret);
> +	ret = spl_blk_file_size(&dev, filename, &filesize);
> +	if (ret)
>   		return ret;
> -	}
>
>   	load.read = spl_fit_read;
>   	if (IS_ENABLED(CONFIG_SPL_FS_FAT_DMA_ALIGN))
> diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
> index a52f9e178e6..8c426a3f3e7 100644
> --- a/common/spl/spl_fat.c
> +++ b/common/spl/spl_fat.c
> @@ -60,10 +60,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;
> @@ -103,6 +103,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,