All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/7] spl: Use common function for loading/parsing images
@ 2022-04-01 19:03 Sean Anderson
  2022-04-01 19:03 ` [RFC PATCH 1/7] spl: Add generic spl_load function Sean Anderson
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Sean Anderson @ 2022-04-01 19:03 UTC (permalink / raw)
  To: Simon Glass
  Cc: Marek Behún, u-boot, Stefan Roese, Marek Vasut,
	Pali Rohár, Sean Anderson

This series adds support for loading all image types (Legacy, FIT (with
and without LOAD_FIT_FULL), and i.MX) to the MMC, SPI, NOR, NET, FAT,
and EXT load methods. It does this by introducing a helper function
which handles the minutiae of invoking the proper parsing function, and
reading the rest of the image.

Hopefully, this will make it easier for load methods to support all
image types that U-Boot supports, without having undocumented
unsupported image types. I applied this to several loaders which were
invoking spl_load_simple_fit and/or spl_parse_image_header, but I did
not use it with others (e.g. DFU/RAM) which had complications in the
mix.

Here's some bloat-o-meter for j7200_evm_a72_defconfig with ext4 support
enabled:

add/remove: 1/0 grow/shrink: 2/4 up/down: 224/-232 (-8)
Function                                     old     new   delta
spl_load                                       -     176    +176
spl_fit_read                                  60     104     +44
spl_load_image_ext                           364     368      +4
spl_nor_load_image                           120     108     -12
spl_spi_load_image                           280     232     -48
spl_load_image_fat                           320     264     -56
spl_mmc_load                                 712     596    -116
Total: Before=264476, After=264468, chg -0.00%

ext4 support is +48 bytes, because the original image support was so
bare-bones (just legacy/raw images). For most boards with a few load
methods (where one of them isn't ext4), this series should be no bloat
or a net negative. However, in the worst case this series will add
150-180 bytes.

I haven't tested most of this, so this series is marked RFC. I had a
nand conversion in here as well, but dropped due to difficulties
outlines in [1].

[1] https://lore.kernel.org/u-boot/c2ea7097-9e85-b9ec-e404-bd46eb83dd5b@seco.com/


Sean Anderson (7):
  spl: Add generic spl_load function
  spl: Convert ext to use spl_load
  spl: Convert fat to spl_load
  spl: Convert mmc to spl_load
  spl: Convert net to spl_load
  spl: Convert nor to spl_load
  spl: Convert spi to spl_load

 common/spl/spl.c     | 61 ++++++++++++++++++++++++++++++++++++
 common/spl/spl_ext.c | 24 ++++++++++-----
 common/spl/spl_fat.c | 36 ++++------------------
 common/spl/spl_mmc.c | 73 ++++----------------------------------------
 common/spl/spl_net.c | 24 +++------------
 common/spl/spl_nor.c | 35 ++++-----------------
 common/spl/spl_spi.c | 48 +++++------------------------
 include/spl.h        | 30 +++++++++++++++++-
 8 files changed, 137 insertions(+), 194 deletions(-)

-- 
2.35.1.1320.gc452695387.dirty


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [RFC PATCH 1/7] spl: Add generic spl_load function
  2022-04-01 19:03 [RFC PATCH 0/7] spl: Use common function for loading/parsing images Sean Anderson
@ 2022-04-01 19:03 ` Sean Anderson
  2022-04-06  5:30   ` Stefan Roese
  2022-04-01 19:04 ` [RFC PATCH 2/7] spl: Convert ext to use spl_load Sean Anderson
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Sean Anderson @ 2022-04-01 19:03 UTC (permalink / raw)
  To: Simon Glass
  Cc: Marek Behún, u-boot, Stefan Roese, Marek Vasut,
	Pali Rohár, Sean Anderson

Implementers of SPL_LOAD_IMAGE_METHOD have to correctly determine what
type of image is being loaded and then call the appropriate image load
function correctly. This is tricky, because some image load functions
expect the whole image to already be loaded (CONFIG_SPL_LOAD_FIT_FULL),
some will load the image automatically using spl_load_info.read()
(CONFIG_SPL_LOAD_FIT/CONFIG_SPL_LOAD_IMX_CONTAINER), and some just parse
the header and expect the caller to do the actual loading afterwards
(legacy/raw images). Load methods often only support a subset of the
above methods, meaning that not all image types can be used with all
load methods. Further, the code to invoke these functions is
duplicated between different load functions.

To address this problem, this commit introduces a "spl_load" function.
It aims to handle image detection and correct invocation of each of the
parse/load functions. spl_simple_read is a wrapper around
spl_load_info.read with get_aligned_image* functions inlined for size
purposes. Additionally, we assume that bl_len is a power of 2 so we can
do bitshifts instead of divisions (which is smaller and faster).

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 common/spl/spl.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++
 include/spl.h    | 30 +++++++++++++++++++++++-
 2 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index b452d4feeb..f26df7ac3f 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -398,6 +398,67 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
 	return 0;
 }
 
+static int spl_simple_read(struct spl_load_info *info, void *buf, size_t size,
+			   size_t offset)
+{
+	int ret;
+	size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : bl_len;
+	size_t bl_mask = bl_len - 1;
+	size_t bl_shift = ffs(bl_mask);
+	size_t overhead = offset & bl_mask;
+
+	buf -= overhead;
+	size = (size + overhead + bl_mask) >> bl_shift;
+	offset = offset >> bl_shift;
+
+	ret = info->read(info, offset, size, buf);
+	return ret == size ? 0 : -EIO;
+}
+
+int spl_load(struct spl_image_info *spl_image,
+	     const struct spl_boot_device *bootdev, struct spl_load_info *info,
+	     struct image_header *header, size_t size, size_t sector)
+{
+	int ret;
+	size_t offset = sector * info->bl_len;
+
+	if (image_get_magic(header) == FDT_MAGIC) {
+		if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL)) {
+			void *buf;
+
+			/*
+			 * In order to support verifying images in the FIT, we
+			 * need to load the whole FIT into memory. Try and
+			 * guess how much we need to load by using the total
+			 * size. This will fail for FITs with external data,
+			 * but there's not much we can do about that.
+			 */
+			if (!size)
+				size = roundup(fdt_totalsize(header), 4);
+			buf = spl_get_load_buffer(0, size);
+			ret = spl_simple_read(info, buf, size, offset);
+			if (ret)
+				return ret;
+
+			return spl_parse_image_header(spl_image, bootdev, buf);
+		}
+
+		if (IS_ENABLED(CONFIG_SPL_LOAD_FIT))
+			return spl_load_simple_fit(spl_image, info, sector,
+						   header);
+	}
+
+	if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER))
+		return spl_load_imx_container(spl_image, info, sector);
+
+	ret = spl_parse_image_header(spl_image, bootdev, header);
+	if (ret)
+		return ret;
+
+	return spl_simple_read(info, (void *)spl_image->load_addr,
+			       spl_image->size, offset + spl_image->offset);
+}
+
 __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
 {
 	typedef void __noreturn (*image_entry_noargs_t)(void);
diff --git a/include/spl.h b/include/spl.h
index 8ceb3c0f09..6606f4e5f6 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -236,7 +236,7 @@ struct spl_image_info {
  *
  * @dev: Pointer to the device, e.g. struct mmc *
  * @priv: Private data for the device
- * @bl_len: Block length for reading in bytes
+ * @bl_len: Block length for reading in bytes; must be a power of 2
  * @filename: Name of the fit image file.
  * @read: Function to call to read from the device
  */
@@ -608,6 +608,34 @@ int spl_load_image_ext_os(struct spl_image_info *spl_image,
 			  struct spl_boot_device *bootdev,
 			  struct blk_desc *block_dev, int partition);
 
+/**
+ * spl_load() - Parse a header and load the image
+ * @spl_image: Image data which will be filled in by this function
+ * @bootdev: The device to load from
+ * @info: Describes how to load additional information from @bootdev. At the
+ *        minimum, read() and bl_len must be populated.
+ * @header: The image header. This should already have been loaded. It may be
+ *          clobbered by the load process (if e.g. the load address overlaps).
+ * @size: The size of the image, if it is known in advance. Some boot devices
+ *        (such as filesystems) know how big an image is before parsing the
+ *        header. If this information is unknown, then the size will be
+ *        determined from the header.
+ * @sectors: The offset from the start if @bootdev, in units of @info->bl_len.
+ *           This should have the offset @header was loaded from. It will be
+ *           added to any offsets passed to @info->read().
+ *
+ * This function determines the image type (FIT, legacy, i.MX, raw, etc), calls
+ * the appropriate parsing function, determines the load address, and the loads
+ * the image from storage. It is designed to replace ad-hoc image loading which
+ * may not support all image types (especially when config options are
+ * involved).
+ *
+ * Return: 0 on success, or a negative error on failure
+ */
+int spl_load(struct spl_image_info *spl_image,
+	     const struct spl_boot_device *bootdev, struct spl_load_info *info,
+	     struct image_header *header, size_t size, size_t sector);
+
 /**
  * spl_early_init() - Set up device tree and driver model in SPL if enabled
  *
-- 
2.35.1.1320.gc452695387.dirty


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 2/7] spl: Convert ext to use spl_load
  2022-04-01 19:03 [RFC PATCH 0/7] spl: Use common function for loading/parsing images Sean Anderson
  2022-04-01 19:03 ` [RFC PATCH 1/7] spl: Add generic spl_load function Sean Anderson
@ 2022-04-01 19:04 ` Sean Anderson
  2022-04-06  5:32   ` Stefan Roese
  2022-04-01 19:04 ` [RFC PATCH 3/7] spl: Convert fat to spl_load Sean Anderson
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Sean Anderson @ 2022-04-01 19:04 UTC (permalink / raw)
  To: Simon Glass
  Cc: Marek Behún, u-boot, Stefan Roese, Marek Vasut,
	Pali Rohár, Sean Anderson

This converts the ext load method to use spl_load. As a consequence, it
also adds support for FIT and IMX images.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 common/spl/spl_ext.c | 24 +++++++++++++++++-------
 1 file changed, 17 insertions(+), 7 deletions(-)

diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c
index ebd914c492..1384842776 100644
--- a/common/spl/spl_ext.c
+++ b/common/spl/spl_ext.c
@@ -9,6 +9,18 @@
 #include <errno.h>
 #include <image.h>
 
+static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
+			  ulong size, void *buf)
+{
+	int ret;
+	loff_t actlen;
+
+	ret = ext4fs_read(buf, file_offset, size, &actlen);
+	if (ret)
+		return ret;
+	return actlen;
+}
+
 int spl_load_image_ext(struct spl_image_info *spl_image,
 		       struct spl_boot_device *bootdev,
 		       struct blk_desc *block_dev, int partition,
@@ -18,6 +30,10 @@ int spl_load_image_ext(struct spl_image_info *spl_image,
 	struct image_header *header;
 	loff_t filelen, actlen;
 	struct disk_partition part_info = {};
+	struct spl_load_info load = {
+		.read = spl_fit_read,
+		.bl_len = 1,
+	};
 
 	header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));
 
@@ -47,13 +63,7 @@ int spl_load_image_ext(struct spl_image_info *spl_image,
 		goto end;
 	}
 
-	err = spl_parse_image_header(spl_image, bootdev, header);
-	if (err < 0) {
-		puts("spl: ext: failed to parse image header\n");
-		goto end;
-	}
-
-	err = ext4fs_read((char *)spl_image->load_addr, 0, filelen, &actlen);
+	err = spl_load(spl_image, bootdev, &load, header, filelen, 0);
 
 end:
 #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
-- 
2.35.1.1320.gc452695387.dirty


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 3/7] spl: Convert fat to spl_load
  2022-04-01 19:03 [RFC PATCH 0/7] spl: Use common function for loading/parsing images Sean Anderson
  2022-04-01 19:03 ` [RFC PATCH 1/7] spl: Add generic spl_load function Sean Anderson
  2022-04-01 19:04 ` [RFC PATCH 2/7] spl: Convert ext to use spl_load Sean Anderson
@ 2022-04-01 19:04 ` Sean Anderson
  2022-04-06  5:32   ` Stefan Roese
  2022-04-01 19:04 ` [RFC PATCH 4/7] spl: Convert mmc " Sean Anderson
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Sean Anderson @ 2022-04-01 19:04 UTC (permalink / raw)
  To: Simon Glass
  Cc: Marek Behún, u-boot, Stefan Roese, Marek Vasut,
	Pali Rohár, Sean Anderson

This converts the fat loader to use spl_load.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 common/spl/spl_fat.c | 36 ++++++------------------------------
 1 file changed, 6 insertions(+), 30 deletions(-)

diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
index 5b270541fc..c092eb3481 100644
--- a/common/spl/spl_fat.c
+++ b/common/spl/spl_fat.c
@@ -61,6 +61,11 @@ int spl_load_image_fat(struct spl_image_info *spl_image,
 {
 	int err;
 	struct image_header *header;
+	struct spl_load_info load = {
+		.read = spl_fit_read,
+		.bl_len = 1,
+		.filename = filename,
+	};
 
 	err = spl_register_fat_device(block_dev, partition);
 	if (err)
@@ -72,36 +77,7 @@ int spl_load_image_fat(struct spl_image_info *spl_image,
 	if (err <= 0)
 		goto end;
 
-	if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL) &&
-	    image_get_magic(header) == FDT_MAGIC) {
-		err = file_fat_read(filename, (void *)CONFIG_SYS_LOAD_ADDR, 0);
-		if (err <= 0)
-			goto end;
-		err = spl_parse_image_header(spl_image, bootdev,
-				(struct image_header *)CONFIG_SYS_LOAD_ADDR);
-		if (err == -EAGAIN)
-			return err;
-		if (err == 0)
-			err = 1;
-	} else if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
-	    image_get_magic(header) == FDT_MAGIC) {
-		struct spl_load_info load;
-
-		debug("Found FIT\n");
-		load.read = spl_fit_read;
-		load.bl_len = 1;
-		load.filename = (void *)filename;
-		load.priv = NULL;
-
-		return spl_load_simple_fit(spl_image, &load, 0, header);
-	} else {
-		err = spl_parse_image_header(spl_image, bootdev, header);
-		if (err)
-			goto end;
-
-		err = file_fat_read(filename,
-				    (u8 *)(uintptr_t)spl_image->load_addr, 0);
-	}
+	err = spl_load(spl_image, bootdev, &load, header, err, 0);
 
 end:
 #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
-- 
2.35.1.1320.gc452695387.dirty


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 4/7] spl: Convert mmc to spl_load
  2022-04-01 19:03 [RFC PATCH 0/7] spl: Use common function for loading/parsing images Sean Anderson
                   ` (2 preceding siblings ...)
  2022-04-01 19:04 ` [RFC PATCH 3/7] spl: Convert fat to spl_load Sean Anderson
@ 2022-04-01 19:04 ` Sean Anderson
  2022-04-06  5:32   ` Stefan Roese
  2022-04-01 19:04 ` [RFC PATCH 5/7] spl: Convert net " Sean Anderson
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Sean Anderson @ 2022-04-01 19:04 UTC (permalink / raw)
  To: Simon Glass
  Cc: Marek Behún, u-boot, Stefan Roese, Marek Vasut,
	Pali Rohár, Sean Anderson

This converts the mmc loader to spl_load. Legacy images are handled by
spl_load (via spl_parse_image_header), so mmc_load_legacy can be
omitted.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 common/spl/spl_mmc.c | 73 ++++----------------------------------------
 1 file changed, 6 insertions(+), 67 deletions(-)

diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index 1c41d24ff4..113566166f 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -17,48 +17,6 @@
 #include <mmc.h>
 #include <image.h>
 
-static int mmc_load_legacy(struct spl_image_info *spl_image,
-			   struct spl_boot_device *bootdev,
-			   struct mmc *mmc,
-			   ulong sector, struct image_header *header)
-{
-	u32 image_offset_sectors;
-	u32 image_size_sectors;
-	unsigned long count;
-	u32 image_offset;
-	int ret;
-
-	ret = spl_parse_image_header(spl_image, bootdev, header);
-	if (ret)
-		return ret;
-
-	/* convert offset to sectors - round down */
-	image_offset_sectors = spl_image->offset / mmc->read_bl_len;
-	/* calculate remaining offset */
-	image_offset = spl_image->offset % mmc->read_bl_len;
-
-	/* convert size to sectors - round up */
-	image_size_sectors = (spl_image->size + mmc->read_bl_len - 1) /
-			     mmc->read_bl_len;
-
-	/* Read the header too to avoid extra memcpy */
-	count = blk_dread(mmc_get_blk_desc(mmc),
-			  sector + image_offset_sectors,
-			  image_size_sectors,
-			  (void *)(ulong)spl_image->load_addr);
-	debug("read %x sectors to %lx\n", image_size_sectors,
-	      spl_image->load_addr);
-	if (count != image_size_sectors)
-		return -EIO;
-
-	if (image_offset)
-		memmove((void *)(ulong)spl_image->load_addr,
-			(void *)(ulong)spl_image->load_addr + image_offset,
-			spl_image->size);
-
-	return 0;
-}
-
 static ulong h_spl_load_read(struct spl_load_info *load, ulong sector,
 			     ulong count, void *buf)
 {
@@ -86,6 +44,11 @@ int mmc_load_image_raw_sector(struct spl_image_info *spl_image,
 	struct image_header *header;
 	struct blk_desc *bd = mmc_get_blk_desc(mmc);
 	int ret = 0;
+	struct spl_load_info load = {
+		.dev = mmc,
+		.bl_len = mmc->read_bl_len,
+		.read = h_spl_load_read,
+	};
 
 	header = spl_get_load_buffer(-sizeof(*header), bd->blksz);
 
@@ -97,31 +60,7 @@ int mmc_load_image_raw_sector(struct spl_image_info *spl_image,
 		goto end;
 	}
 
-	if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
-	    image_get_magic(header) == FDT_MAGIC) {
-		struct spl_load_info load;
-
-		debug("Found FIT\n");
-		load.dev = mmc;
-		load.priv = NULL;
-		load.filename = NULL;
-		load.bl_len = mmc->read_bl_len;
-		load.read = h_spl_load_read;
-		ret = spl_load_simple_fit(spl_image, &load, sector, header);
-	} else if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER)) {
-		struct spl_load_info load;
-
-		load.dev = mmc;
-		load.priv = NULL;
-		load.filename = NULL;
-		load.bl_len = mmc->read_bl_len;
-		load.read = h_spl_load_read;
-
-		ret = spl_load_imx_container(spl_image, &load, sector);
-	} else {
-		ret = mmc_load_legacy(spl_image, bootdev, mmc, sector, header);
-	}
-
+	ret = spl_load(spl_image, bootdev, &load, header, 0, sector);
 end:
 	if (ret) {
 #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
-- 
2.35.1.1320.gc452695387.dirty


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 5/7] spl: Convert net to spl_load
  2022-04-01 19:03 [RFC PATCH 0/7] spl: Use common function for loading/parsing images Sean Anderson
                   ` (3 preceding siblings ...)
  2022-04-01 19:04 ` [RFC PATCH 4/7] spl: Convert mmc " Sean Anderson
@ 2022-04-01 19:04 ` Sean Anderson
  2022-04-06  5:32   ` Stefan Roese
  2022-04-01 19:04 ` [RFC PATCH 6/7] spl: Convert nor " Sean Anderson
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Sean Anderson @ 2022-04-01 19:04 UTC (permalink / raw)
  To: Simon Glass
  Cc: Marek Behún, u-boot, Stefan Roese, Marek Vasut,
	Pali Rohár, Sean Anderson

This converts the net load method to use spl_load. As a result, it also
adds support for LOAD_FIT_FULL and IMX images.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 common/spl/spl_net.c | 24 +++++-------------------
 1 file changed, 5 insertions(+), 19 deletions(-)

diff --git a/common/spl/spl_net.c b/common/spl/spl_net.c
index a853e6aead..3b4374add6 100644
--- a/common/spl/spl_net.c
+++ b/common/spl/spl_net.c
@@ -29,6 +29,10 @@ static int spl_net_load_image(struct spl_image_info *spl_image,
 			      struct spl_boot_device *bootdev)
 {
 	struct image_header *header = (struct image_header *)image_load_addr;
+	struct spl_load_info load = {
+		.bl_len = 1,
+		.read = spl_net_load_read,
+	};
 	int rv;
 
 	env_init();
@@ -47,25 +51,7 @@ static int spl_net_load_image(struct spl_image_info *spl_image,
 		return rv;
 	}
 
-	if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
-	    image_get_magic(header) == FDT_MAGIC) {
-		struct spl_load_info load;
-
-		debug("Found FIT\n");
-		load.bl_len = 1;
-		load.read = spl_net_load_read;
-		rv = spl_load_simple_fit(spl_image, &load, 0, header);
-	} else {
-		debug("Legacy image\n");
-
-		rv = spl_parse_image_header(spl_image, bootdev, header);
-		if (rv)
-			return rv;
-
-		memcpy((void *)spl_image->load_addr, header, spl_image->size);
-	}
-
-	return rv;
+	return spl_load(spl_image, bootdev, &load, header, 0, 0);
 }
 #endif
 
-- 
2.35.1.1320.gc452695387.dirty


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 6/7] spl: Convert nor to spl_load
  2022-04-01 19:03 [RFC PATCH 0/7] spl: Use common function for loading/parsing images Sean Anderson
                   ` (4 preceding siblings ...)
  2022-04-01 19:04 ` [RFC PATCH 5/7] spl: Convert net " Sean Anderson
@ 2022-04-01 19:04 ` Sean Anderson
  2022-04-06  5:33   ` Stefan Roese
  2022-04-01 19:04 ` [RFC PATCH 7/7] spl: Convert spi " Sean Anderson
  2022-04-06  5:26 ` [RFC PATCH 0/7] spl: Use common function for loading/parsing images Stefan Roese
  7 siblings, 1 reply; 18+ messages in thread
From: Sean Anderson @ 2022-04-01 19:04 UTC (permalink / raw)
  To: Simon Glass
  Cc: Marek Behún, u-boot, Stefan Roese, Marek Vasut,
	Pali Rohár, Sean Anderson

This converts the nor load method to use spl_load. As a result it also
adds support LOAD_FIT_FULL.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 common/spl/spl_nor.c | 35 ++++++-----------------------------
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/common/spl/spl_nor.c b/common/spl/spl_nor.c
index 0f4fff8493..90ece77af1 100644
--- a/common/spl/spl_nor.c
+++ b/common/spl/spl_nor.c
@@ -26,8 +26,11 @@ unsigned long __weak spl_nor_get_uboot_base(void)
 static int spl_nor_load_image(struct spl_image_info *spl_image,
 			      struct spl_boot_device *bootdev)
 {
-	__maybe_unused const struct image_header *header;
-	__maybe_unused struct spl_load_info load;
+	struct image_header *header = (void *)spl_nor_get_uboot_base();
+	struct spl_load_info load = {
+		.bl_len = 1,
+		.read = spl_nor_load_read,
+	};
 
 	/*
 	 * Loading of the payload to SDRAM is done with skipping of
@@ -91,32 +94,6 @@ static int spl_nor_load_image(struct spl_image_info *spl_image,
 	 * Load real U-Boot from its location in NOR flash to its
 	 * defined location in SDRAM
 	 */
-#ifdef CONFIG_SPL_LOAD_FIT
-	header = (const struct image_header *)spl_nor_get_uboot_base();
-	if (image_get_magic(header) == FDT_MAGIC) {
-		debug("Found FIT format U-Boot\n");
-		load.bl_len = 1;
-		load.read = spl_nor_load_read;
-		return spl_load_simple_fit(spl_image, &load,
-					   spl_nor_get_uboot_base(),
-					   (void *)header);
-	}
-#endif
-	if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER)) {
-		load.bl_len = 1;
-		load.read = spl_nor_load_read;
-		return spl_load_imx_container(spl_image, &load,
-					      spl_nor_get_uboot_base());
-	}
-
-	/* Legacy image handling */
-	if (IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_SUPPORT)) {
-		load.bl_len = 1;
-		load.read = spl_nor_load_read;
-		return spl_load_legacy_img(spl_image, bootdev, &load,
-					   spl_nor_get_uboot_base());
-	}
-
-	return 0;
+	return spl_load(spl_image, bootdev, &load, header, 0, 0);
 }
 SPL_LOAD_IMAGE_METHOD("NOR", 0, BOOT_DEVICE_NOR, spl_nor_load_image);
-- 
2.35.1.1320.gc452695387.dirty


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [RFC PATCH 7/7] spl: Convert spi to spl_load
  2022-04-01 19:03 [RFC PATCH 0/7] spl: Use common function for loading/parsing images Sean Anderson
                   ` (5 preceding siblings ...)
  2022-04-01 19:04 ` [RFC PATCH 6/7] spl: Convert nor " Sean Anderson
@ 2022-04-01 19:04 ` Sean Anderson
  2022-04-06  5:33   ` Stefan Roese
  2022-04-06  5:26 ` [RFC PATCH 0/7] spl: Use common function for loading/parsing images Stefan Roese
  7 siblings, 1 reply; 18+ messages in thread
From: Sean Anderson @ 2022-04-01 19:04 UTC (permalink / raw)
  To: Simon Glass
  Cc: Marek Behún, u-boot, Stefan Roese, Marek Vasut,
	Pali Rohár, Sean Anderson

This converts the spi load method to use spl_load. As a consequence, it
also adds support for LOAD_FIT_FULL.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 common/spl/spl_spi.c | 48 +++++++-------------------------------------
 1 file changed, 7 insertions(+), 41 deletions(-)

diff --git a/common/spl/spl_spi.c b/common/spl/spl_spi.c
index cf3f7ef4c0..037db1a19f 100644
--- a/common/spl/spl_spi.c
+++ b/common/spl/spl_spi.c
@@ -83,6 +83,10 @@ static int spl_spi_load_image(struct spl_image_info *spl_image,
 	unsigned int payload_offs;
 	struct spi_flash *flash;
 	struct image_header *header;
+	struct spl_load_info load = {
+		.bl_len = 1,
+		.read = spl_spi_fit_read,
+	};
 
 	/*
 	 * Load U-Boot image from SPI flash into RAM
@@ -99,6 +103,7 @@ static int spl_spi_load_image(struct spl_image_info *spl_image,
 		return -ENODEV;
 	}
 
+	load.dev = flash;
 	payload_offs = spl_spi_get_uboot_offs(flash);
 
 	header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));
@@ -121,47 +126,8 @@ static int spl_spi_load_image(struct spl_image_info *spl_image,
 			return err;
 		}
 
-		if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL) &&
-		    image_get_magic(header) == FDT_MAGIC) {
-			err = spi_flash_read(flash, payload_offs,
-					     roundup(fdt_totalsize(header), 4),
-					     (void *)CONFIG_SYS_LOAD_ADDR);
-			if (err)
-				return err;
-			err = spl_parse_image_header(spl_image, bootdev,
-					(struct image_header *)CONFIG_SYS_LOAD_ADDR);
-		} else if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
-			   image_get_magic(header) == FDT_MAGIC) {
-			struct spl_load_info load;
-
-			debug("Found FIT\n");
-			load.dev = flash;
-			load.priv = NULL;
-			load.filename = NULL;
-			load.bl_len = 1;
-			load.read = spl_spi_fit_read;
-			err = spl_load_simple_fit(spl_image, &load,
-						  payload_offs,
-						  header);
-		} else if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER)) {
-			struct spl_load_info load;
-
-			load.dev = flash;
-			load.priv = NULL;
-			load.filename = NULL;
-			load.bl_len = 1;
-			load.read = spl_spi_fit_read;
-
-			err = spl_load_imx_container(spl_image, &load,
-						     payload_offs);
-		} else {
-			err = spl_parse_image_header(spl_image, bootdev, header);
-			if (err)
-				return err;
-			err = spi_flash_read(flash, payload_offs + spl_image->offset,
-					     spl_image->size,
-					     (void *)spl_image->load_addr);
-		}
+		err = spl_load(spl_image, bootdev, &load, header, 0,
+			       payload_offs);
 	}
 
 	return err;
-- 
2.35.1.1320.gc452695387.dirty


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 0/7] spl: Use common function for loading/parsing images
  2022-04-01 19:03 [RFC PATCH 0/7] spl: Use common function for loading/parsing images Sean Anderson
                   ` (6 preceding siblings ...)
  2022-04-01 19:04 ` [RFC PATCH 7/7] spl: Convert spi " Sean Anderson
@ 2022-04-06  5:26 ` Stefan Roese
  7 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2022-04-06  5:26 UTC (permalink / raw)
  To: Sean Anderson, Simon Glass
  Cc: Marek Behún, u-boot, Marek Vasut, Pali Rohár

On 4/1/22 21:03, Sean Anderson wrote:
> This series adds support for loading all image types (Legacy, FIT (with
> and without LOAD_FIT_FULL), and i.MX) to the MMC, SPI, NOR, NET, FAT,
> and EXT load methods. It does this by introducing a helper function
> which handles the minutiae of invoking the proper parsing function, and
> reading the rest of the image.
> 
> Hopefully, this will make it easier for load methods to support all
> image types that U-Boot supports, without having undocumented
> unsupported image types. I applied this to several loaders which were
> invoking spl_load_simple_fit and/or spl_parse_image_header, but I did
> not use it with others (e.g. DFU/RAM) which had complications in the
> mix.
> 
> Here's some bloat-o-meter for j7200_evm_a72_defconfig with ext4 support
> enabled:
> 
> add/remove: 1/0 grow/shrink: 2/4 up/down: 224/-232 (-8)
> Function                                     old     new   delta
> spl_load                                       -     176    +176
> spl_fit_read                                  60     104     +44
> spl_load_image_ext                           364     368      +4
> spl_nor_load_image                           120     108     -12
> spl_spi_load_image                           280     232     -48
> spl_load_image_fat                           320     264     -56
> spl_mmc_load                                 712     596    -116
> Total: Before=264476, After=264468, chg -0.00%
> 
> ext4 support is +48 bytes, because the original image support was so
> bare-bones (just legacy/raw images). For most boards with a few load
> methods (where one of them isn't ext4), this series should be no bloat
> or a net negative. However, in the worst case this series will add
> 150-180 bytes.
> 
> I haven't tested most of this, so this series is marked RFC. I had a
> nand conversion in here as well, but dropped due to difficulties
> outlines in [1].
> 
> [1] https://lore.kernel.org/u-boot/c2ea7097-9e85-b9ec-e404-bd46eb83dd5b@seco.com/
> 
> 
> Sean Anderson (7):
>    spl: Add generic spl_load function
>    spl: Convert ext to use spl_load
>    spl: Convert fat to spl_load
>    spl: Convert mmc to spl_load
>    spl: Convert net to spl_load
>    spl: Convert nor to spl_load
>    spl: Convert spi to spl_load
> 
>   common/spl/spl.c     | 61 ++++++++++++++++++++++++++++++++++++
>   common/spl/spl_ext.c | 24 ++++++++++-----
>   common/spl/spl_fat.c | 36 ++++------------------
>   common/spl/spl_mmc.c | 73 ++++----------------------------------------
>   common/spl/spl_net.c | 24 +++------------
>   common/spl/spl_nor.c | 35 ++++-----------------
>   common/spl/spl_spi.c | 48 +++++------------------------
>   include/spl.h        | 30 +++++++++++++++++-
>   8 files changed, 137 insertions(+), 194 deletions(-)
> 

I like the idea of consolidating this SPL code a lot. Thanks for working
on this.

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/7] spl: Add generic spl_load function
  2022-04-01 19:03 ` [RFC PATCH 1/7] spl: Add generic spl_load function Sean Anderson
@ 2022-04-06  5:30   ` Stefan Roese
  2022-04-07 15:10     ` Sean Anderson
  0 siblings, 1 reply; 18+ messages in thread
From: Stefan Roese @ 2022-04-06  5:30 UTC (permalink / raw)
  To: Sean Anderson, Simon Glass
  Cc: Marek Behún, u-boot, Marek Vasut, Pali Rohár

On 4/1/22 21:03, Sean Anderson wrote:
> Implementers of SPL_LOAD_IMAGE_METHOD have to correctly determine what
> type of image is being loaded and then call the appropriate image load
> function correctly. This is tricky, because some image load functions
> expect the whole image to already be loaded (CONFIG_SPL_LOAD_FIT_FULL),
> some will load the image automatically using spl_load_info.read()
> (CONFIG_SPL_LOAD_FIT/CONFIG_SPL_LOAD_IMX_CONTAINER), and some just parse
> the header and expect the caller to do the actual loading afterwards
> (legacy/raw images). Load methods often only support a subset of the
> above methods, meaning that not all image types can be used with all
> load methods. Further, the code to invoke these functions is
> duplicated between different load functions.
> 
> To address this problem, this commit introduces a "spl_load" function.
> It aims to handle image detection and correct invocation of each of the
> parse/load functions. spl_simple_read is a wrapper around
> spl_load_info.read with get_aligned_image* functions inlined for size
> purposes. Additionally, we assume that bl_len is a power of 2 so we can
> do bitshifts instead of divisions (which is smaller and faster).
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
> 
>   common/spl/spl.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++
>   include/spl.h    | 30 +++++++++++++++++++++++-
>   2 files changed, 90 insertions(+), 1 deletion(-)
> 
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index b452d4feeb..f26df7ac3f 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -398,6 +398,67 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
>   	return 0;
>   }
>   
> +static int spl_simple_read(struct spl_load_info *info, void *buf, size_t size,
> +			   size_t offset)
> +{
> +	int ret;
> +	size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : bl_len;
> +	size_t bl_mask = bl_len - 1;
> +	size_t bl_shift = ffs(bl_mask);
> +	size_t overhead = offset & bl_mask;
> +

Nitpicking comment:

It's preferred in general to use the reverse XMAS tree ordering of the
declared variables. So I would expect at least to have "int ret" as
last statement above. If you address this, then please in the complete
series.

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> +	buf -= overhead;
> +	size = (size + overhead + bl_mask) >> bl_shift;
> +	offset = offset >> bl_shift;
> +
> +	ret = info->read(info, offset, size, buf);
> +	return ret == size ? 0 : -EIO;
> +}
> +
> +int spl_load(struct spl_image_info *spl_image,
> +	     const struct spl_boot_device *bootdev, struct spl_load_info *info,
> +	     struct image_header *header, size_t size, size_t sector)
> +{
> +	int ret;
> +	size_t offset = sector * info->bl_len;
> +
> +	if (image_get_magic(header) == FDT_MAGIC) {
> +		if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL)) {
> +			void *buf;
> +
> +			/*
> +			 * In order to support verifying images in the FIT, we
> +			 * need to load the whole FIT into memory. Try and
> +			 * guess how much we need to load by using the total
> +			 * size. This will fail for FITs with external data,
> +			 * but there's not much we can do about that.
> +			 */
> +			if (!size)
> +				size = roundup(fdt_totalsize(header), 4);
> +			buf = spl_get_load_buffer(0, size);
> +			ret = spl_simple_read(info, buf, size, offset);
> +			if (ret)
> +				return ret;
> +
> +			return spl_parse_image_header(spl_image, bootdev, buf);
> +		}
> +
> +		if (IS_ENABLED(CONFIG_SPL_LOAD_FIT))
> +			return spl_load_simple_fit(spl_image, info, sector,
> +						   header);
> +	}
> +
> +	if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER))
> +		return spl_load_imx_container(spl_image, info, sector);
> +
> +	ret = spl_parse_image_header(spl_image, bootdev, header);
> +	if (ret)
> +		return ret;
> +
> +	return spl_simple_read(info, (void *)spl_image->load_addr,
> +			       spl_image->size, offset + spl_image->offset);
> +}
> +
>   __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
>   {
>   	typedef void __noreturn (*image_entry_noargs_t)(void);
> diff --git a/include/spl.h b/include/spl.h
> index 8ceb3c0f09..6606f4e5f6 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -236,7 +236,7 @@ struct spl_image_info {
>    *
>    * @dev: Pointer to the device, e.g. struct mmc *
>    * @priv: Private data for the device
> - * @bl_len: Block length for reading in bytes
> + * @bl_len: Block length for reading in bytes; must be a power of 2
>    * @filename: Name of the fit image file.
>    * @read: Function to call to read from the device
>    */
> @@ -608,6 +608,34 @@ int spl_load_image_ext_os(struct spl_image_info *spl_image,
>   			  struct spl_boot_device *bootdev,
>   			  struct blk_desc *block_dev, int partition);
>   
> +/**
> + * spl_load() - Parse a header and load the image
> + * @spl_image: Image data which will be filled in by this function
> + * @bootdev: The device to load from
> + * @info: Describes how to load additional information from @bootdev. At the
> + *        minimum, read() and bl_len must be populated.
> + * @header: The image header. This should already have been loaded. It may be
> + *          clobbered by the load process (if e.g. the load address overlaps).
> + * @size: The size of the image, if it is known in advance. Some boot devices
> + *        (such as filesystems) know how big an image is before parsing the
> + *        header. If this information is unknown, then the size will be
> + *        determined from the header.
> + * @sectors: The offset from the start if @bootdev, in units of @info->bl_len.
> + *           This should have the offset @header was loaded from. It will be
> + *           added to any offsets passed to @info->read().
> + *
> + * This function determines the image type (FIT, legacy, i.MX, raw, etc), calls
> + * the appropriate parsing function, determines the load address, and the loads
> + * the image from storage. It is designed to replace ad-hoc image loading which
> + * may not support all image types (especially when config options are
> + * involved).
> + *
> + * Return: 0 on success, or a negative error on failure
> + */
> +int spl_load(struct spl_image_info *spl_image,
> +	     const struct spl_boot_device *bootdev, struct spl_load_info *info,
> +	     struct image_header *header, size_t size, size_t sector);
> +
>   /**
>    * spl_early_init() - Set up device tree and driver model in SPL if enabled
>    *

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 2/7] spl: Convert ext to use spl_load
  2022-04-01 19:04 ` [RFC PATCH 2/7] spl: Convert ext to use spl_load Sean Anderson
@ 2022-04-06  5:32   ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2022-04-06  5:32 UTC (permalink / raw)
  To: Sean Anderson, Simon Glass
  Cc: Marek Behún, u-boot, Marek Vasut, Pali Rohár

On 4/1/22 21:04, Sean Anderson wrote:
> This converts the ext load method to use spl_load. As a consequence, it
> also adds support for FIT and IMX images.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
> 
>   common/spl/spl_ext.c | 24 +++++++++++++++++-------
>   1 file changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c
> index ebd914c492..1384842776 100644
> --- a/common/spl/spl_ext.c
> +++ b/common/spl/spl_ext.c
> @@ -9,6 +9,18 @@
>   #include <errno.h>
>   #include <image.h>
>   
> +static ulong spl_fit_read(struct spl_load_info *load, ulong file_offset,
> +			  ulong size, void *buf)
> +{
> +	int ret;
> +	loff_t actlen;
> +
> +	ret = ext4fs_read(buf, file_offset, size, &actlen);
> +	if (ret)
> +		return ret;
> +	return actlen;
> +}
> +
>   int spl_load_image_ext(struct spl_image_info *spl_image,
>   		       struct spl_boot_device *bootdev,
>   		       struct blk_desc *block_dev, int partition,
> @@ -18,6 +30,10 @@ int spl_load_image_ext(struct spl_image_info *spl_image,
>   	struct image_header *header;
>   	loff_t filelen, actlen;
>   	struct disk_partition part_info = {};
> +	struct spl_load_info load = {
> +		.read = spl_fit_read,
> +		.bl_len = 1,
> +	};
>   
>   	header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));
>   
> @@ -47,13 +63,7 @@ int spl_load_image_ext(struct spl_image_info *spl_image,
>   		goto end;
>   	}
>   
> -	err = spl_parse_image_header(spl_image, bootdev, header);
> -	if (err < 0) {
> -		puts("spl: ext: failed to parse image header\n");
> -		goto end;
> -	}
> -
> -	err = ext4fs_read((char *)spl_image->load_addr, 0, filelen, &actlen);
> +	err = spl_load(spl_image, bootdev, &load, header, filelen, 0);
>   
>   end:
>   #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 3/7] spl: Convert fat to spl_load
  2022-04-01 19:04 ` [RFC PATCH 3/7] spl: Convert fat to spl_load Sean Anderson
@ 2022-04-06  5:32   ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2022-04-06  5:32 UTC (permalink / raw)
  To: Sean Anderson, Simon Glass
  Cc: Marek Behún, u-boot, Marek Vasut, Pali Rohár

On 4/1/22 21:04, Sean Anderson wrote:
> This converts the fat loader to use spl_load.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
> 
>   common/spl/spl_fat.c | 36 ++++++------------------------------
>   1 file changed, 6 insertions(+), 30 deletions(-)
> 
> diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
> index 5b270541fc..c092eb3481 100644
> --- a/common/spl/spl_fat.c
> +++ b/common/spl/spl_fat.c
> @@ -61,6 +61,11 @@ int spl_load_image_fat(struct spl_image_info *spl_image,
>   {
>   	int err;
>   	struct image_header *header;
> +	struct spl_load_info load = {
> +		.read = spl_fit_read,
> +		.bl_len = 1,
> +		.filename = filename,
> +	};
>   
>   	err = spl_register_fat_device(block_dev, partition);
>   	if (err)
> @@ -72,36 +77,7 @@ int spl_load_image_fat(struct spl_image_info *spl_image,
>   	if (err <= 0)
>   		goto end;
>   
> -	if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL) &&
> -	    image_get_magic(header) == FDT_MAGIC) {
> -		err = file_fat_read(filename, (void *)CONFIG_SYS_LOAD_ADDR, 0);
> -		if (err <= 0)
> -			goto end;
> -		err = spl_parse_image_header(spl_image, bootdev,
> -				(struct image_header *)CONFIG_SYS_LOAD_ADDR);
> -		if (err == -EAGAIN)
> -			return err;
> -		if (err == 0)
> -			err = 1;
> -	} else if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
> -	    image_get_magic(header) == FDT_MAGIC) {
> -		struct spl_load_info load;
> -
> -		debug("Found FIT\n");
> -		load.read = spl_fit_read;
> -		load.bl_len = 1;
> -		load.filename = (void *)filename;
> -		load.priv = NULL;
> -
> -		return spl_load_simple_fit(spl_image, &load, 0, header);
> -	} else {
> -		err = spl_parse_image_header(spl_image, bootdev, header);
> -		if (err)
> -			goto end;
> -
> -		err = file_fat_read(filename,
> -				    (u8 *)(uintptr_t)spl_image->load_addr, 0);
> -	}
> +	err = spl_load(spl_image, bootdev, &load, header, err, 0);
>   
>   end:
>   #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 4/7] spl: Convert mmc to spl_load
  2022-04-01 19:04 ` [RFC PATCH 4/7] spl: Convert mmc " Sean Anderson
@ 2022-04-06  5:32   ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2022-04-06  5:32 UTC (permalink / raw)
  To: Sean Anderson, Simon Glass
  Cc: Marek Behún, u-boot, Marek Vasut, Pali Rohár

On 4/1/22 21:04, Sean Anderson wrote:
> This converts the mmc loader to spl_load. Legacy images are handled by
> spl_load (via spl_parse_image_header), so mmc_load_legacy can be
> omitted.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
> 
>   common/spl/spl_mmc.c | 73 ++++----------------------------------------
>   1 file changed, 6 insertions(+), 67 deletions(-)
> 
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index 1c41d24ff4..113566166f 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -17,48 +17,6 @@
>   #include <mmc.h>
>   #include <image.h>
>   
> -static int mmc_load_legacy(struct spl_image_info *spl_image,
> -			   struct spl_boot_device *bootdev,
> -			   struct mmc *mmc,
> -			   ulong sector, struct image_header *header)
> -{
> -	u32 image_offset_sectors;
> -	u32 image_size_sectors;
> -	unsigned long count;
> -	u32 image_offset;
> -	int ret;
> -
> -	ret = spl_parse_image_header(spl_image, bootdev, header);
> -	if (ret)
> -		return ret;
> -
> -	/* convert offset to sectors - round down */
> -	image_offset_sectors = spl_image->offset / mmc->read_bl_len;
> -	/* calculate remaining offset */
> -	image_offset = spl_image->offset % mmc->read_bl_len;
> -
> -	/* convert size to sectors - round up */
> -	image_size_sectors = (spl_image->size + mmc->read_bl_len - 1) /
> -			     mmc->read_bl_len;
> -
> -	/* Read the header too to avoid extra memcpy */
> -	count = blk_dread(mmc_get_blk_desc(mmc),
> -			  sector + image_offset_sectors,
> -			  image_size_sectors,
> -			  (void *)(ulong)spl_image->load_addr);
> -	debug("read %x sectors to %lx\n", image_size_sectors,
> -	      spl_image->load_addr);
> -	if (count != image_size_sectors)
> -		return -EIO;
> -
> -	if (image_offset)
> -		memmove((void *)(ulong)spl_image->load_addr,
> -			(void *)(ulong)spl_image->load_addr + image_offset,
> -			spl_image->size);
> -
> -	return 0;
> -}
> -
>   static ulong h_spl_load_read(struct spl_load_info *load, ulong sector,
>   			     ulong count, void *buf)
>   {
> @@ -86,6 +44,11 @@ int mmc_load_image_raw_sector(struct spl_image_info *spl_image,
>   	struct image_header *header;
>   	struct blk_desc *bd = mmc_get_blk_desc(mmc);
>   	int ret = 0;
> +	struct spl_load_info load = {
> +		.dev = mmc,
> +		.bl_len = mmc->read_bl_len,
> +		.read = h_spl_load_read,
> +	};
>   
>   	header = spl_get_load_buffer(-sizeof(*header), bd->blksz);
>   
> @@ -97,31 +60,7 @@ int mmc_load_image_raw_sector(struct spl_image_info *spl_image,
>   		goto end;
>   	}
>   
> -	if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
> -	    image_get_magic(header) == FDT_MAGIC) {
> -		struct spl_load_info load;
> -
> -		debug("Found FIT\n");
> -		load.dev = mmc;
> -		load.priv = NULL;
> -		load.filename = NULL;
> -		load.bl_len = mmc->read_bl_len;
> -		load.read = h_spl_load_read;
> -		ret = spl_load_simple_fit(spl_image, &load, sector, header);
> -	} else if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER)) {
> -		struct spl_load_info load;
> -
> -		load.dev = mmc;
> -		load.priv = NULL;
> -		load.filename = NULL;
> -		load.bl_len = mmc->read_bl_len;
> -		load.read = h_spl_load_read;
> -
> -		ret = spl_load_imx_container(spl_image, &load, sector);
> -	} else {
> -		ret = mmc_load_legacy(spl_image, bootdev, mmc, sector, header);
> -	}
> -
> +	ret = spl_load(spl_image, bootdev, &load, header, 0, sector);
>   end:
>   	if (ret) {
>   #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 5/7] spl: Convert net to spl_load
  2022-04-01 19:04 ` [RFC PATCH 5/7] spl: Convert net " Sean Anderson
@ 2022-04-06  5:32   ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2022-04-06  5:32 UTC (permalink / raw)
  To: Sean Anderson, Simon Glass
  Cc: Marek Behún, u-boot, Marek Vasut, Pali Rohár

On 4/1/22 21:04, Sean Anderson wrote:
> This converts the net load method to use spl_load. As a result, it also
> adds support for LOAD_FIT_FULL and IMX images.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
> 
>   common/spl/spl_net.c | 24 +++++-------------------
>   1 file changed, 5 insertions(+), 19 deletions(-)
> 
> diff --git a/common/spl/spl_net.c b/common/spl/spl_net.c
> index a853e6aead..3b4374add6 100644
> --- a/common/spl/spl_net.c
> +++ b/common/spl/spl_net.c
> @@ -29,6 +29,10 @@ static int spl_net_load_image(struct spl_image_info *spl_image,
>   			      struct spl_boot_device *bootdev)
>   {
>   	struct image_header *header = (struct image_header *)image_load_addr;
> +	struct spl_load_info load = {
> +		.bl_len = 1,
> +		.read = spl_net_load_read,
> +	};
>   	int rv;
>   
>   	env_init();
> @@ -47,25 +51,7 @@ static int spl_net_load_image(struct spl_image_info *spl_image,
>   		return rv;
>   	}
>   
> -	if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
> -	    image_get_magic(header) == FDT_MAGIC) {
> -		struct spl_load_info load;
> -
> -		debug("Found FIT\n");
> -		load.bl_len = 1;
> -		load.read = spl_net_load_read;
> -		rv = spl_load_simple_fit(spl_image, &load, 0, header);
> -	} else {
> -		debug("Legacy image\n");
> -
> -		rv = spl_parse_image_header(spl_image, bootdev, header);
> -		if (rv)
> -			return rv;
> -
> -		memcpy((void *)spl_image->load_addr, header, spl_image->size);
> -	}
> -
> -	return rv;
> +	return spl_load(spl_image, bootdev, &load, header, 0, 0);
>   }
>   #endif
>   

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 6/7] spl: Convert nor to spl_load
  2022-04-01 19:04 ` [RFC PATCH 6/7] spl: Convert nor " Sean Anderson
@ 2022-04-06  5:33   ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2022-04-06  5:33 UTC (permalink / raw)
  To: Sean Anderson, Simon Glass
  Cc: Marek Behún, u-boot, Marek Vasut, Pali Rohár

On 4/1/22 21:04, Sean Anderson wrote:
> This converts the nor load method to use spl_load. As a result it also
> adds support LOAD_FIT_FULL.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
> 
>   common/spl/spl_nor.c | 35 ++++++-----------------------------
>   1 file changed, 6 insertions(+), 29 deletions(-)
> 
> diff --git a/common/spl/spl_nor.c b/common/spl/spl_nor.c
> index 0f4fff8493..90ece77af1 100644
> --- a/common/spl/spl_nor.c
> +++ b/common/spl/spl_nor.c
> @@ -26,8 +26,11 @@ unsigned long __weak spl_nor_get_uboot_base(void)
>   static int spl_nor_load_image(struct spl_image_info *spl_image,
>   			      struct spl_boot_device *bootdev)
>   {
> -	__maybe_unused const struct image_header *header;
> -	__maybe_unused struct spl_load_info load;
> +	struct image_header *header = (void *)spl_nor_get_uboot_base();
> +	struct spl_load_info load = {
> +		.bl_len = 1,
> +		.read = spl_nor_load_read,
> +	};
>   
>   	/*
>   	 * Loading of the payload to SDRAM is done with skipping of
> @@ -91,32 +94,6 @@ static int spl_nor_load_image(struct spl_image_info *spl_image,
>   	 * Load real U-Boot from its location in NOR flash to its
>   	 * defined location in SDRAM
>   	 */
> -#ifdef CONFIG_SPL_LOAD_FIT
> -	header = (const struct image_header *)spl_nor_get_uboot_base();
> -	if (image_get_magic(header) == FDT_MAGIC) {
> -		debug("Found FIT format U-Boot\n");
> -		load.bl_len = 1;
> -		load.read = spl_nor_load_read;
> -		return spl_load_simple_fit(spl_image, &load,
> -					   spl_nor_get_uboot_base(),
> -					   (void *)header);
> -	}
> -#endif
> -	if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER)) {
> -		load.bl_len = 1;
> -		load.read = spl_nor_load_read;
> -		return spl_load_imx_container(spl_image, &load,
> -					      spl_nor_get_uboot_base());
> -	}
> -
> -	/* Legacy image handling */
> -	if (IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_SUPPORT)) {
> -		load.bl_len = 1;
> -		load.read = spl_nor_load_read;
> -		return spl_load_legacy_img(spl_image, bootdev, &load,
> -					   spl_nor_get_uboot_base());
> -	}
> -
> -	return 0;
> +	return spl_load(spl_image, bootdev, &load, header, 0, 0);
>   }
>   SPL_LOAD_IMAGE_METHOD("NOR", 0, BOOT_DEVICE_NOR, spl_nor_load_image);

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 7/7] spl: Convert spi to spl_load
  2022-04-01 19:04 ` [RFC PATCH 7/7] spl: Convert spi " Sean Anderson
@ 2022-04-06  5:33   ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2022-04-06  5:33 UTC (permalink / raw)
  To: Sean Anderson, Simon Glass
  Cc: Marek Behún, u-boot, Marek Vasut, Pali Rohár

On 4/1/22 21:04, Sean Anderson wrote:
> This converts the spi load method to use spl_load. As a consequence, it
> also adds support for LOAD_FIT_FULL.
> 
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> ---
> 
>   common/spl/spl_spi.c | 48 +++++++-------------------------------------
>   1 file changed, 7 insertions(+), 41 deletions(-)
> 
> diff --git a/common/spl/spl_spi.c b/common/spl/spl_spi.c
> index cf3f7ef4c0..037db1a19f 100644
> --- a/common/spl/spl_spi.c
> +++ b/common/spl/spl_spi.c
> @@ -83,6 +83,10 @@ static int spl_spi_load_image(struct spl_image_info *spl_image,
>   	unsigned int payload_offs;
>   	struct spi_flash *flash;
>   	struct image_header *header;
> +	struct spl_load_info load = {
> +		.bl_len = 1,
> +		.read = spl_spi_fit_read,
> +	};
>   
>   	/*
>   	 * Load U-Boot image from SPI flash into RAM
> @@ -99,6 +103,7 @@ static int spl_spi_load_image(struct spl_image_info *spl_image,
>   		return -ENODEV;
>   	}
>   
> +	load.dev = flash;
>   	payload_offs = spl_spi_get_uboot_offs(flash);
>   
>   	header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));
> @@ -121,47 +126,8 @@ static int spl_spi_load_image(struct spl_image_info *spl_image,
>   			return err;
>   		}
>   
> -		if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL) &&
> -		    image_get_magic(header) == FDT_MAGIC) {
> -			err = spi_flash_read(flash, payload_offs,
> -					     roundup(fdt_totalsize(header), 4),
> -					     (void *)CONFIG_SYS_LOAD_ADDR);
> -			if (err)
> -				return err;
> -			err = spl_parse_image_header(spl_image, bootdev,
> -					(struct image_header *)CONFIG_SYS_LOAD_ADDR);
> -		} else if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
> -			   image_get_magic(header) == FDT_MAGIC) {
> -			struct spl_load_info load;
> -
> -			debug("Found FIT\n");
> -			load.dev = flash;
> -			load.priv = NULL;
> -			load.filename = NULL;
> -			load.bl_len = 1;
> -			load.read = spl_spi_fit_read;
> -			err = spl_load_simple_fit(spl_image, &load,
> -						  payload_offs,
> -						  header);
> -		} else if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER)) {
> -			struct spl_load_info load;
> -
> -			load.dev = flash;
> -			load.priv = NULL;
> -			load.filename = NULL;
> -			load.bl_len = 1;
> -			load.read = spl_spi_fit_read;
> -
> -			err = spl_load_imx_container(spl_image, &load,
> -						     payload_offs);
> -		} else {
> -			err = spl_parse_image_header(spl_image, bootdev, header);
> -			if (err)
> -				return err;
> -			err = spi_flash_read(flash, payload_offs + spl_image->offset,
> -					     spl_image->size,
> -					     (void *)spl_image->load_addr);
> -		}
> +		err = spl_load(spl_image, bootdev, &load, header, 0,
> +			       payload_offs);
>   	}
>   
>   	return err;

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/7] spl: Add generic spl_load function
  2022-04-06  5:30   ` Stefan Roese
@ 2022-04-07 15:10     ` Sean Anderson
  2022-04-08  4:43       ` Stefan Roese
  0 siblings, 1 reply; 18+ messages in thread
From: Sean Anderson @ 2022-04-07 15:10 UTC (permalink / raw)
  To: Stefan Roese, Simon Glass
  Cc: Marek Behún, u-boot, Marek Vasut, Pali Rohár



On 4/6/22 1:30 AM, Stefan Roese wrote:
> On 4/1/22 21:03, Sean Anderson wrote:
>> Implementers of SPL_LOAD_IMAGE_METHOD have to correctly determine what
>> type of image is being loaded and then call the appropriate image load
>> function correctly. This is tricky, because some image load functions
>> expect the whole image to already be loaded (CONFIG_SPL_LOAD_FIT_FULL),
>> some will load the image automatically using spl_load_info.read()
>> (CONFIG_SPL_LOAD_FIT/CONFIG_SPL_LOAD_IMX_CONTAINER), and some just parse
>> the header and expect the caller to do the actual loading afterwards
>> (legacy/raw images). Load methods often only support a subset of the
>> above methods, meaning that not all image types can be used with all
>> load methods. Further, the code to invoke these functions is
>> duplicated between different load functions.
>>
>> To address this problem, this commit introduces a "spl_load" function.
>> It aims to handle image detection and correct invocation of each of the
>> parse/load functions. spl_simple_read is a wrapper around
>> spl_load_info.read with get_aligned_image* functions inlined for size
>> purposes. Additionally, we assume that bl_len is a power of 2 so we can
>> do bitshifts instead of divisions (which is smaller and faster).
>>
>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>> ---
>>
>>   common/spl/spl.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/spl.h    | 30 +++++++++++++++++++++++-
>>   2 files changed, 90 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>> index b452d4feeb..f26df7ac3f 100644
>> --- a/common/spl/spl.c
>> +++ b/common/spl/spl.c
>> @@ -398,6 +398,67 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
>>       return 0;
>>   }
>>   +static int spl_simple_read(struct spl_load_info *info, void *buf, size_t size,
>> +               size_t offset)
>> +{
>> +    int ret;
>> +    size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : bl_len;
>> +    size_t bl_mask = bl_len - 1;
>> +    size_t bl_shift = ffs(bl_mask);
>> +    size_t overhead = offset & bl_mask;
>> +
> 
> Nitpicking comment:
> 
> It's preferred in general to use the reverse XMAS tree ordering of the
> declared variables. So I would expect at least to have "int ret" as
> last statement above. If you address this, then please in the complete
> series.

I thought only Linux's net subsystem had this requirement. I can reorder
things for you if you'd like. However, some of these variables have
dependencies so they cannot all be reordered :)

--Sean

> Reviewed-by: Stefan Roese <sr@denx.de>
> 
> Thanks,
> Stefan
> 
>> +    buf -= overhead;
>> +    size = (size + overhead + bl_mask) >> bl_shift;
>> +    offset = offset >> bl_shift;
>> +
>> +    ret = info->read(info, offset, size, buf);
>> +    return ret == size ? 0 : -EIO;
>> +}
>> +
>> +int spl_load(struct spl_image_info *spl_image,
>> +         const struct spl_boot_device *bootdev, struct spl_load_info *info,
>> +         struct image_header *header, size_t size, size_t sector)
>> +{
>> +    int ret;
>> +    size_t offset = sector * info->bl_len;
>> +
>> +    if (image_get_magic(header) == FDT_MAGIC) {
>> +        if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL)) {
>> +            void *buf;
>> +
>> +            /*
>> +             * In order to support verifying images in the FIT, we
>> +             * need to load the whole FIT into memory. Try and
>> +             * guess how much we need to load by using the total
>> +             * size. This will fail for FITs with external data,
>> +             * but there's not much we can do about that.
>> +             */
>> +            if (!size)
>> +                size = roundup(fdt_totalsize(header), 4);
>> +            buf = spl_get_load_buffer(0, size);
>> +            ret = spl_simple_read(info, buf, size, offset);
>> +            if (ret)
>> +                return ret;
>> +
>> +            return spl_parse_image_header(spl_image, bootdev, buf);
>> +        }
>> +
>> +        if (IS_ENABLED(CONFIG_SPL_LOAD_FIT))
>> +            return spl_load_simple_fit(spl_image, info, sector,
>> +                           header);
>> +    }
>> +
>> +    if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER))
>> +        return spl_load_imx_container(spl_image, info, sector);
>> +
>> +    ret = spl_parse_image_header(spl_image, bootdev, header);
>> +    if (ret)
>> +        return ret;
>> +
>> +    return spl_simple_read(info, (void *)spl_image->load_addr,
>> +                   spl_image->size, offset + spl_image->offset);
>> +}
>> +
>>   __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
>>   {
>>       typedef void __noreturn (*image_entry_noargs_t)(void);
>> diff --git a/include/spl.h b/include/spl.h
>> index 8ceb3c0f09..6606f4e5f6 100644
>> --- a/include/spl.h
>> +++ b/include/spl.h
>> @@ -236,7 +236,7 @@ struct spl_image_info {
>>    *
>>    * @dev: Pointer to the device, e.g. struct mmc *
>>    * @priv: Private data for the device
>> - * @bl_len: Block length for reading in bytes
>> + * @bl_len: Block length for reading in bytes; must be a power of 2
>>    * @filename: Name of the fit image file.
>>    * @read: Function to call to read from the device
>>    */
>> @@ -608,6 +608,34 @@ int spl_load_image_ext_os(struct spl_image_info *spl_image,
>>                 struct spl_boot_device *bootdev,
>>                 struct blk_desc *block_dev, int partition);
>>   +/**
>> + * spl_load() - Parse a header and load the image
>> + * @spl_image: Image data which will be filled in by this function
>> + * @bootdev: The device to load from
>> + * @info: Describes how to load additional information from @bootdev. At the
>> + *        minimum, read() and bl_len must be populated.
>> + * @header: The image header. This should already have been loaded. It may be
>> + *          clobbered by the load process (if e.g. the load address overlaps).
>> + * @size: The size of the image, if it is known in advance. Some boot devices
>> + *        (such as filesystems) know how big an image is before parsing the
>> + *        header. If this information is unknown, then the size will be
>> + *        determined from the header.
>> + * @sectors: The offset from the start if @bootdev, in units of @info->bl_len.
>> + *           This should have the offset @header was loaded from. It will be
>> + *           added to any offsets passed to @info->read().
>> + *
>> + * This function determines the image type (FIT, legacy, i.MX, raw, etc), calls
>> + * the appropriate parsing function, determines the load address, and the loads
>> + * the image from storage. It is designed to replace ad-hoc image loading which
>> + * may not support all image types (especially when config options are
>> + * involved).
>> + *
>> + * Return: 0 on success, or a negative error on failure
>> + */
>> +int spl_load(struct spl_image_info *spl_image,
>> +         const struct spl_boot_device *bootdev, struct spl_load_info *info,
>> +         struct image_header *header, size_t size, size_t sector);
>> +
>>   /**
>>    * spl_early_init() - Set up device tree and driver model in SPL if enabled
>>    *
> 
> Viele Grüße,
> Stefan Roese
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [RFC PATCH 1/7] spl: Add generic spl_load function
  2022-04-07 15:10     ` Sean Anderson
@ 2022-04-08  4:43       ` Stefan Roese
  0 siblings, 0 replies; 18+ messages in thread
From: Stefan Roese @ 2022-04-08  4:43 UTC (permalink / raw)
  To: Sean Anderson, Simon Glass
  Cc: Marek Behún, u-boot, Marek Vasut, Pali Rohár

On 4/7/22 17:10, Sean Anderson wrote:
> 
> 
> On 4/6/22 1:30 AM, Stefan Roese wrote:
>> On 4/1/22 21:03, Sean Anderson wrote:
>>> Implementers of SPL_LOAD_IMAGE_METHOD have to correctly determine what
>>> type of image is being loaded and then call the appropriate image load
>>> function correctly. This is tricky, because some image load functions
>>> expect the whole image to already be loaded (CONFIG_SPL_LOAD_FIT_FULL),
>>> some will load the image automatically using spl_load_info.read()
>>> (CONFIG_SPL_LOAD_FIT/CONFIG_SPL_LOAD_IMX_CONTAINER), and some just parse
>>> the header and expect the caller to do the actual loading afterwards
>>> (legacy/raw images). Load methods often only support a subset of the
>>> above methods, meaning that not all image types can be used with all
>>> load methods. Further, the code to invoke these functions is
>>> duplicated between different load functions.
>>>
>>> To address this problem, this commit introduces a "spl_load" function.
>>> It aims to handle image detection and correct invocation of each of the
>>> parse/load functions. spl_simple_read is a wrapper around
>>> spl_load_info.read with get_aligned_image* functions inlined for size
>>> purposes. Additionally, we assume that bl_len is a power of 2 so we can
>>> do bitshifts instead of divisions (which is smaller and faster).
>>>
>>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>>> ---
>>>
>>>    common/spl/spl.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>    include/spl.h    | 30 +++++++++++++++++++++++-
>>>    2 files changed, 90 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>>> index b452d4feeb..f26df7ac3f 100644
>>> --- a/common/spl/spl.c
>>> +++ b/common/spl/spl.c
>>> @@ -398,6 +398,67 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
>>>        return 0;
>>>    }
>>>    +static int spl_simple_read(struct spl_load_info *info, void *buf, size_t size,
>>> +               size_t offset)
>>> +{
>>> +    int ret;
>>> +    size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : bl_len;
>>> +    size_t bl_mask = bl_len - 1;
>>> +    size_t bl_shift = ffs(bl_mask);
>>> +    size_t overhead = offset & bl_mask;
>>> +
>>
>> Nitpicking comment:
>>
>> It's preferred in general to use the reverse XMAS tree ordering of the
>> declared variables. So I would expect at least to have "int ret" as
>> last statement above. If you address this, then please in the complete
>> series.
> 
> I thought only Linux's net subsystem had this requirement. I can reorder
> things for you if you'd like. However, some of these variables have
> dependencies so they cannot all be reordered :)

Thanks. Perhaps it's only my personal preference. I find it more
structured this way. Still I think that I remember having seen this
request of reverse XMAS tree ordering here on the U-Boot list as well.

Nevertheless this is no blocker whatsoever. Please make such changes
only if a v2 of the patches is required.

Thanks,
Stefan

> --Sean
> 
>> Reviewed-by: Stefan Roese <sr@denx.de>
>>
>> Thanks,
>> Stefan
>>
>>> +    buf -= overhead;
>>> +    size = (size + overhead + bl_mask) >> bl_shift;
>>> +    offset = offset >> bl_shift;
>>> +
>>> +    ret = info->read(info, offset, size, buf);
>>> +    return ret == size ? 0 : -EIO;
>>> +}
>>> +
>>> +int spl_load(struct spl_image_info *spl_image,
>>> +         const struct spl_boot_device *bootdev, struct spl_load_info *info,
>>> +         struct image_header *header, size_t size, size_t sector)
>>> +{
>>> +    int ret;
>>> +    size_t offset = sector * info->bl_len;
>>> +
>>> +    if (image_get_magic(header) == FDT_MAGIC) {
>>> +        if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL)) {
>>> +            void *buf;
>>> +
>>> +            /*
>>> +             * In order to support verifying images in the FIT, we
>>> +             * need to load the whole FIT into memory. Try and
>>> +             * guess how much we need to load by using the total
>>> +             * size. This will fail for FITs with external data,
>>> +             * but there's not much we can do about that.
>>> +             */
>>> +            if (!size)
>>> +                size = roundup(fdt_totalsize(header), 4);
>>> +            buf = spl_get_load_buffer(0, size);
>>> +            ret = spl_simple_read(info, buf, size, offset);
>>> +            if (ret)
>>> +                return ret;
>>> +
>>> +            return spl_parse_image_header(spl_image, bootdev, buf);
>>> +        }
>>> +
>>> +        if (IS_ENABLED(CONFIG_SPL_LOAD_FIT))
>>> +            return spl_load_simple_fit(spl_image, info, sector,
>>> +                           header);
>>> +    }
>>> +
>>> +    if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER))
>>> +        return spl_load_imx_container(spl_image, info, sector);
>>> +
>>> +    ret = spl_parse_image_header(spl_image, bootdev, header);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    return spl_simple_read(info, (void *)spl_image->load_addr,
>>> +                   spl_image->size, offset + spl_image->offset);
>>> +}
>>> +
>>>    __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
>>>    {
>>>        typedef void __noreturn (*image_entry_noargs_t)(void);
>>> diff --git a/include/spl.h b/include/spl.h
>>> index 8ceb3c0f09..6606f4e5f6 100644
>>> --- a/include/spl.h
>>> +++ b/include/spl.h
>>> @@ -236,7 +236,7 @@ struct spl_image_info {
>>>     *
>>>     * @dev: Pointer to the device, e.g. struct mmc *
>>>     * @priv: Private data for the device
>>> - * @bl_len: Block length for reading in bytes
>>> + * @bl_len: Block length for reading in bytes; must be a power of 2
>>>     * @filename: Name of the fit image file.
>>>     * @read: Function to call to read from the device
>>>     */
>>> @@ -608,6 +608,34 @@ int spl_load_image_ext_os(struct spl_image_info *spl_image,
>>>                  struct spl_boot_device *bootdev,
>>>                  struct blk_desc *block_dev, int partition);
>>>    +/**
>>> + * spl_load() - Parse a header and load the image
>>> + * @spl_image: Image data which will be filled in by this function
>>> + * @bootdev: The device to load from
>>> + * @info: Describes how to load additional information from @bootdev. At the
>>> + *        minimum, read() and bl_len must be populated.
>>> + * @header: The image header. This should already have been loaded. It may be
>>> + *          clobbered by the load process (if e.g. the load address overlaps).
>>> + * @size: The size of the image, if it is known in advance. Some boot devices
>>> + *        (such as filesystems) know how big an image is before parsing the
>>> + *        header. If this information is unknown, then the size will be
>>> + *        determined from the header.
>>> + * @sectors: The offset from the start if @bootdev, in units of @info->bl_len.
>>> + *           This should have the offset @header was loaded from. It will be
>>> + *           added to any offsets passed to @info->read().
>>> + *
>>> + * This function determines the image type (FIT, legacy, i.MX, raw, etc), calls
>>> + * the appropriate parsing function, determines the load address, and the loads
>>> + * the image from storage. It is designed to replace ad-hoc image loading which
>>> + * may not support all image types (especially when config options are
>>> + * involved).
>>> + *
>>> + * Return: 0 on success, or a negative error on failure
>>> + */
>>> +int spl_load(struct spl_image_info *spl_image,
>>> +         const struct spl_boot_device *bootdev, struct spl_load_info *info,
>>> +         struct image_header *header, size_t size, size_t sector);
>>> +
>>>    /**
>>>     * spl_early_init() - Set up device tree and driver model in SPL if enabled
>>>     *
>>
>> Viele Grüße,
>> Stefan Roese
>>

Viele Grüße,
Stefan Roese

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2022-04-08  4:43 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-01 19:03 [RFC PATCH 0/7] spl: Use common function for loading/parsing images Sean Anderson
2022-04-01 19:03 ` [RFC PATCH 1/7] spl: Add generic spl_load function Sean Anderson
2022-04-06  5:30   ` Stefan Roese
2022-04-07 15:10     ` Sean Anderson
2022-04-08  4:43       ` Stefan Roese
2022-04-01 19:04 ` [RFC PATCH 2/7] spl: Convert ext to use spl_load Sean Anderson
2022-04-06  5:32   ` Stefan Roese
2022-04-01 19:04 ` [RFC PATCH 3/7] spl: Convert fat to spl_load Sean Anderson
2022-04-06  5:32   ` Stefan Roese
2022-04-01 19:04 ` [RFC PATCH 4/7] spl: Convert mmc " Sean Anderson
2022-04-06  5:32   ` Stefan Roese
2022-04-01 19:04 ` [RFC PATCH 5/7] spl: Convert net " Sean Anderson
2022-04-06  5:32   ` Stefan Roese
2022-04-01 19:04 ` [RFC PATCH 6/7] spl: Convert nor " Sean Anderson
2022-04-06  5:33   ` Stefan Roese
2022-04-01 19:04 ` [RFC PATCH 7/7] spl: Convert spi " Sean Anderson
2022-04-06  5:33   ` Stefan Roese
2022-04-06  5:26 ` [RFC PATCH 0/7] spl: Use common function for loading/parsing images Stefan Roese

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.