[PATCH 00/14] bloblist: Align to firmware handoff

[PATCH 00/14] bloblist: Align to firmware handoff

[Simon Glass]

In moving from v0.8 to v0.9 the Firmware Handoff specification made some
changes, including:

   - Use a packed format for headers to reduce space
   - Add an explicit alignment field in the header
   - Renumber all the tags and reduce their size to 24 bits
   - Drop use of the blob header to specify alignment, in favour of a
     'void' blob type

This series attempts to align to that specification, including updating
the API and tests. It is likely that refinements will be made as other
projects implement the spec too.

As before the code is dual-licensed, to permit use in projects with a
permissive license.


Simon Glass (14):
  bloblist: Update the tag numbering
  bloblist: Adjust API to align in powers of 2
  bloblist: Change the magic value
  bloblist: Set version to 1
  bloblist: Access record hdr_size and tag via a function
  bloblist: Drop the flags value
  bloblist: Drop the spare values
  bloblist: Change the checksum algorithm
  bloblist: Checksum the entire bloblist
  bloblist: Handle alignment with a void entry
  bloblist: Reduce blob-header size
  bloblist: Reduce bloblist header size
  bloblist: Add alignment to bloblist_new()
  bloblist: Update documentation and header comment

 arch/x86/lib/tables.c    |   3 +-
 common/bloblist.c        | 102 ++++++++++++++++++++-------------
 doc/develop/bloblist.rst |   4 +-
 include/bloblist.h       | 121 +++++++++++++++++++--------------------
 test/bloblist.c          |  53 +++++++++--------
 5 files changed, 152 insertions(+), 131 deletions(-)

-- 
2.41.0.487.g6d72f3e995-goog

[PATCH 01/14] bloblist: Update the tag numbering

[Simon Glass]

Align this with the new v0.9 spec. It only has a single area for all
non-standard tags.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/bloblist.c  |  2 +-
 include/bloblist.h | 37 +++++++++++++------------------------
 test/bloblist.c    |  2 +-
 3 files changed, 15 insertions(+), 26 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 2144b10e1d0..ca3e6efa800 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -36,7 +36,7 @@ static struct tag_name {
 	enum bloblist_tag_t tag;
 	const char *name;
 } tag_name[] = {
-	{ BLOBLISTT_NONE, "(none)" },
+	{ BLOBLISTT_VOID, "(void)" },
 
 	/* BLOBLISTT_AREA_FIRMWARE_TOP */
 
diff --git a/include/bloblist.h b/include/bloblist.h
index 7ea72c6bd46..bad5fbbb889 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -81,7 +81,7 @@ enum {
 
 /* Supported tags - add new ones to tag_name in bloblist.c */
 enum bloblist_tag_t {
-	BLOBLISTT_NONE = 0,
+	BLOBLISTT_VOID = 0,
 
 	/*
 	 * Standard area to allocate blobs used across firmware components, for
@@ -105,37 +105,26 @@ enum bloblist_tag_t {
 	BLOBLISTT_VBOOT_CTX = 0x106,	/* Chromium OS verified boot context */
 
 	/*
+	 * Tags from here are on reserved for private use within a single
+	 * firmware binary (i.e. a single executable or phase of a project).
+	 * These tags can be passed between binaries within a local
+	 * implementation, but cannot be used in upstream code. Allocate a
+	 * tag in one of the areas above if you want that.
+	 *
 	 * Project-specific tags are permitted here. Projects can be open source
 	 * or not, but the format of the data must be fuily documented in an
 	 * open source project, including all fields, bits, etc. Naming should
 	 * be: BLOBLISTT_<project>_<purpose_here>
-	 */
-	BLOBLISTT_PROJECT_AREA = 0x8000,
-	BLOBLISTT_U_BOOT_SPL_HANDOFF = 0x8000, /* Hand-off info from SPL */
-	BLOBLISTT_VBE		= 0x8001,	/* VBE per-phase state */
-	BLOBLISTT_U_BOOT_VIDEO = 0x8002, /* Video information from SPL */
-
-	/*
-	 * Vendor-specific tags are permitted here. Projects can be open source
+	 *
+	 * Vendor-specific tags are also permitted. Projects can be open source
 	 * or not, but the format of the data must be fuily documented in an
 	 * open source project, including all fields, bits, etc. Naming should
 	 * be BLOBLISTT_<vendor>_<purpose_here>
 	 */
-	BLOBLISTT_VENDOR_AREA = 0xc000,
-
-	/* Tags after this are not allocated for now */
-	BLOBLISTT_EXPANSION = 0x10000,
-
-	/*
-	 * Tags from here are on reserved for private use within a single
-	 * firmware binary (i.e. a single executable or phase of a project).
-	 * These tags can be passed between binaries within a local
-	 * implementation, but cannot be used in upstream code. Allocate a
-	 * tag in one of the areas above if you want that.
-	 *
-	 * This area may move in future.
-	 */
-	BLOBLISTT_PRIVATE_AREA = 0xffff0000,
+	BLOBLISTT_PRIVATE_AREA		= 0xfff000,
+	BLOBLISTT_U_BOOT_SPL_HANDOFF	= 0xfff000, /* Hand-off info from SPL */
+	BLOBLISTT_VBE			= 0xfff001, /* VBE per-phase state */
+	BLOBLISTT_U_BOOT_VIDEO		= 0xfff002, /* Video info from SPL */
 };
 
 /**
diff --git a/test/bloblist.c b/test/bloblist.c
index 720be7e244f..df9a99d7bd2 100644
--- a/test/bloblist.c
+++ b/test/bloblist.c
@@ -291,7 +291,7 @@ static int bloblist_test_cmd_list(struct unit_test_state *uts)
 	console_record_reset();
 	run_command("bloblist list", 0);
 	ut_assert_nextline("Address       Size   Tag Name");
-	ut_assert_nextline("%08lx  %8x  8000 SPL hand-off",
+	ut_assert_nextline("%08lx  %8x  fff000 SPL hand-off",
 			   (ulong)map_to_sysmem(data), TEST_SIZE);
 	ut_assert_nextline("%08lx  %8x   106 Chrome OS vboot context",
 			   (ulong)map_to_sysmem(data2), TEST_SIZE2);
-- 
2.41.0.487.g6d72f3e995-goog

[PATCH 02/14] bloblist: Adjust API to align in powers of 2

[Simon Glass]

The updated bloblist structure stores the alignment as a power-of-two
value in its structures. Adjust the API to use this, to avoid needing to
calling ilog2().

Drop a stale comment while we are here.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/x86/lib/tables.c |  3 ++-
 common/bloblist.c     | 24 +++++++++++-------------
 include/bloblist.h    | 12 +++++++-----
 test/bloblist.c       |  4 ++--
 4 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
index 67bc0a72aeb..8c437738075 100644
--- a/arch/x86/lib/tables.c
+++ b/arch/x86/lib/tables.c
@@ -16,6 +16,7 @@
 #include <asm/mpspec.h>
 #include <asm/tables.h>
 #include <asm/coreboot_tables.h>
+#include <linux/log2.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -101,7 +102,7 @@ int write_tables(void)
 			if (!gd->arch.table_end)
 				gd->arch.table_end = rom_addr;
 			rom_addr = (ulong)bloblist_add(table->tag, size,
-						       table->align);
+						       ilog2(table->align));
 			if (!rom_addr)
 				return log_msg_ret("bloblist", -ENOBUFS);
 
diff --git a/common/bloblist.c b/common/bloblist.c
index ca3e6efa800..b9332c03ca7 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -26,8 +26,6 @@
  * start address of the data in each blob is aligned as required. Note that
  * each blob's *data* is aligned to BLOBLIST_ALIGN regardless of the alignment
  * of the bloblist itself or the blob header.
- *
- * So far, only BLOBLIST_ALIGN alignment is supported.
  */
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -117,24 +115,24 @@ static struct bloblist_rec *bloblist_findrec(uint tag)
 	return NULL;
 }
 
-static int bloblist_addrec(uint tag, int size, int align,
+static int bloblist_addrec(uint tag, int size, int align_log2,
 			   struct bloblist_rec **recp)
 {
 	struct bloblist_hdr *hdr = gd->bloblist;
 	struct bloblist_rec *rec;
 	int data_start, new_alloced;
 
-	if (!align)
-		align = BLOBLIST_ALIGN;
+	if (!align_log2)
+		align_log2 = BLOBLIST_ALIGN_LOG2;
 
 	/* Figure out where the new data will start */
 	data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*rec);
 
 	/* Align the address and then calculate the offset from ->alloced */
-	data_start = ALIGN(data_start, align) - map_to_sysmem(hdr);
+	data_start = ALIGN(data_start, 1U << align_log2) - map_to_sysmem(hdr);
 
 	/* Calculate the new allocated total */
-	new_alloced = data_start + ALIGN(size, align);
+	new_alloced = data_start + ALIGN(size, 1U << align_log2);
 
 	if (new_alloced > hdr->size) {
 		log_err("Failed to allocate %x bytes size=%x, need size=%x\n",
@@ -158,7 +156,7 @@ static int bloblist_addrec(uint tag, int size, int align,
 }
 
 static int bloblist_ensurerec(uint tag, struct bloblist_rec **recp, int size,
-			      int align)
+			      int align_log2)
 {
 	struct bloblist_rec *rec;
 
@@ -171,7 +169,7 @@ static int bloblist_ensurerec(uint tag, struct bloblist_rec **recp, int size,
 	} else {
 		int ret;
 
-		ret = bloblist_addrec(tag, size, align, &rec);
+		ret = bloblist_addrec(tag, size, align_log2, &rec);
 		if (ret)
 			return ret;
 	}
@@ -193,22 +191,22 @@ void *bloblist_find(uint tag, int size)
 	return (void *)rec + rec->hdr_size;
 }
 
-void *bloblist_add(uint tag, int size, int align)
+void *bloblist_add(uint tag, int size, int align_log2)
 {
 	struct bloblist_rec *rec;
 
-	if (bloblist_addrec(tag, size, align, &rec))
+	if (bloblist_addrec(tag, size, align_log2, &rec))
 		return NULL;
 
 	return (void *)rec + rec->hdr_size;
 }
 
-int bloblist_ensure_size(uint tag, int size, int align, void **blobp)
+int bloblist_ensure_size(uint tag, int size, int align_log2, void **blobp)
 {
 	struct bloblist_rec *rec;
 	int ret;
 
-	ret = bloblist_ensurerec(tag, &rec, size, align);
+	ret = bloblist_ensurerec(tag, &rec, size, align_log2);
 	if (ret)
 		return ret;
 	*blobp = (void *)rec + rec->hdr_size;
diff --git a/include/bloblist.h b/include/bloblist.h
index bad5fbbb889..e6ce98334d3 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -76,7 +76,9 @@
 enum {
 	BLOBLIST_VERSION	= 0,
 	BLOBLIST_MAGIC		= 0xb00757a3,
-	BLOBLIST_ALIGN		= 16,
+
+	BLOBLIST_ALIGN_LOG2	= 4,
+	BLOBLIST_ALIGN		= 1 << BLOBLIST_ALIGN_LOG2,
 };
 
 /* Supported tags - add new ones to tag_name in bloblist.c */
@@ -238,11 +240,11 @@ void *bloblist_find(uint tag, int size);
  *
  * @tag:	Tag to add (enum bloblist_tag_t)
  * @size:	Size of the blob
- * @align:	Alignment of the blob (in bytes), 0 for default
+ * @align_log2:	Alignment of the blob (in bytes log2), 0 for default
  * Return: pointer to the newly added block, or NULL if there is not enough
  * space for the blob
  */
-void *bloblist_add(uint tag, int size, int align);
+void *bloblist_add(uint tag, int size, int align_log2);
 
 /**
  * bloblist_ensure_size() - Find or add a blob
@@ -252,11 +254,11 @@ void *bloblist_add(uint tag, int size, int align);
  * @tag:	Tag to add (enum bloblist_tag_t)
  * @size:	Size of the blob
  * @blobp:	Returns a pointer to blob on success
- * @align:	Alignment of the blob (in bytes), 0 for default
+ * @align_log2:	Alignment of the blob (in bytes log2), 0 for default
  * Return: 0 if OK, -ENOSPC if it is missing and could not be added due to lack
  * of space, or -ESPIPE it exists but has the wrong size
  */
-int bloblist_ensure_size(uint tag, int size, int align, void **blobp);
+int bloblist_ensure_size(uint tag, int size, int align_log2, void **blobp);
 
 /**
  * bloblist_ensure() - Find or add a blob
diff --git a/test/bloblist.c b/test/bloblist.c
index df9a99d7bd2..3d988fe05ae 100644
--- a/test/bloblist.c
+++ b/test/bloblist.c
@@ -336,7 +336,7 @@ static int bloblist_test_align(struct unit_test_state *uts)
 
 	/* Check larger alignment */
 	for (i = 0; i < 3; i++) {
-		int align = 32 << i;
+		int align = 5 - i;
 
 		data = bloblist_add(3 + i, i * 4, align);
 		ut_assertnonnull(data);
@@ -351,7 +351,7 @@ static int bloblist_test_align(struct unit_test_state *uts)
 	ut_assertok(bloblist_new(TEST_ADDR + BLOBLIST_ALIGN, TEST_BLOBLIST_SIZE,
 				 0));
 
-	data = bloblist_add(1, 5, BLOBLIST_ALIGN * 2);
+	data = bloblist_add(1, 5, BLOBLIST_ALIGN_LOG2 + 1);
 	ut_assertnonnull(data);
 	addr = map_to_sysmem(data);
 	ut_asserteq(0, addr & (BLOBLIST_ALIGN * 2 - 1));
-- 
2.41.0.487.g6d72f3e995-goog

[PATCH 03/14] bloblist: Change the magic value

[Simon Glass]

This uses a new value with spec v0.9 so change it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/bloblist.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/bloblist.h b/include/bloblist.h
index e6ce98334d3..6ef3e9466e8 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -75,7 +75,7 @@
 
 enum {
 	BLOBLIST_VERSION	= 0,
-	BLOBLIST_MAGIC		= 0xb00757a3,
+	BLOBLIST_MAGIC		= 0x6ed0ff,
 
 	BLOBLIST_ALIGN_LOG2	= 4,
 	BLOBLIST_ALIGN		= 1 << BLOBLIST_ALIGN_LOG2,
-- 
2.41.0.487.g6d72f3e995-goog

[PATCH 04/14] bloblist: Set version to 1

[Simon Glass]

The new bloblist for v0.9 has version 1 so update this value.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/bloblist.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/bloblist.h b/include/bloblist.h
index 6ef3e9466e8..74932dcae6f 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -74,7 +74,7 @@
 #include <mapmem.h>
 
 enum {
-	BLOBLIST_VERSION	= 0,
+	BLOBLIST_VERSION	= 1,
 	BLOBLIST_MAGIC		= 0x6ed0ff,
 
 	BLOBLIST_ALIGN_LOG2	= 4,
-- 
2.41.0.487.g6d72f3e995-goog

[PATCH 05/14] bloblist: Access record hdr_size and tag via a function

[Simon Glass]

These values currently use a simple field. With spec v0.9 they have moved
to a packed format. Convert most accesses to use functions, so this change
can be accomodated.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/bloblist.c | 38 +++++++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 13 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index b9332c03ca7..0def7fc9b2f 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -73,13 +73,23 @@ static struct bloblist_rec *bloblist_first_blob(struct bloblist_hdr *hdr)
 	return (struct bloblist_rec *)((void *)hdr + hdr->hdr_size);
 }
 
+static inline uint rec_hdr_size(struct bloblist_rec *rec)
+{
+	return rec->hdr_size;
+}
+
+static inline uint rec_tag(struct bloblist_rec *rec)
+{
+	return rec->tag;
+}
+
 static ulong bloblist_blob_end_ofs(struct bloblist_hdr *hdr,
 				   struct bloblist_rec *rec)
 {
 	ulong offset;
 
 	offset = (void *)rec - (void *)hdr;
-	offset += rec->hdr_size + ALIGN(rec->size, BLOBLIST_ALIGN);
+	offset += rec_hdr_size(rec) + ALIGN(rec->size, BLOBLIST_ALIGN);
 
 	return offset;
 }
@@ -108,7 +118,7 @@ static struct bloblist_rec *bloblist_findrec(uint tag)
 		return NULL;
 
 	foreach_rec(rec, hdr) {
-		if (rec->tag == tag)
+		if (rec_tag(rec) == tag)
 			return rec;
 	}
 
@@ -147,7 +157,7 @@ static int bloblist_addrec(uint tag, int size, int align_log2,
 	rec->spare = 0;
 
 	/* Zero the record data */
-	memset((void *)rec + rec->hdr_size, '\0', rec->size);
+	memset((void *)rec + rec_hdr_size(rec), '\0', rec->size);
 
 	hdr->alloced = new_alloced;
 	*recp = rec;
@@ -188,7 +198,7 @@ void *bloblist_find(uint tag, int size)
 	if (size && size != rec->size)
 		return NULL;
 
-	return (void *)rec + rec->hdr_size;
+	return (void *)rec + rec_hdr_size(rec);
 }
 
 void *bloblist_add(uint tag, int size, int align_log2)
@@ -198,7 +208,7 @@ void *bloblist_add(uint tag, int size, int align_log2)
 	if (bloblist_addrec(tag, size, align_log2, &rec))
 		return NULL;
 
-	return (void *)rec + rec->hdr_size;
+	return (void *)rec + rec_hdr_size(rec);
 }
 
 int bloblist_ensure_size(uint tag, int size, int align_log2, void **blobp)
@@ -209,7 +219,7 @@ int bloblist_ensure_size(uint tag, int size, int align_log2, void **blobp)
 	ret = bloblist_ensurerec(tag, &rec, size, align_log2);
 	if (ret)
 		return ret;
-	*blobp = (void *)rec + rec->hdr_size;
+	*blobp = (void *)rec + rec_hdr_size(rec);
 
 	return 0;
 }
@@ -221,7 +231,7 @@ void *bloblist_ensure(uint tag, int size)
 	if (bloblist_ensurerec(tag, &rec, size, 0))
 		return NULL;
 
-	return (void *)rec + rec->hdr_size;
+	return (void *)rec + rec_hdr_size(rec);
 }
 
 int bloblist_ensure_size_ret(uint tag, int *sizep, void **blobp)
@@ -234,7 +244,7 @@ int bloblist_ensure_size_ret(uint tag, int *sizep, void **blobp)
 		*sizep = rec->size;
 	else if (ret)
 		return ret;
-	*blobp = (void *)rec + rec->hdr_size;
+	*blobp = (void *)rec + rec_hdr_size(rec);
 
 	return 0;
 }
@@ -270,7 +280,7 @@ static int bloblist_resize_rec(struct bloblist_hdr *hdr,
 
 	/* Zero the new part of the blob */
 	if (expand_by > 0) {
-		memset((void *)rec + rec->hdr_size + rec->size, '\0',
+		memset((void *)rec + rec_hdr_size(rec) + rec->size, '\0',
 		       new_size - rec->size);
 	}
 
@@ -304,8 +314,9 @@ static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr)
 	chksum = crc32(0, (unsigned char *)hdr,
 		       offsetof(struct bloblist_hdr, chksum));
 	foreach_rec(rec, hdr) {
-		chksum = crc32(chksum, (void *)rec, rec->hdr_size);
-		chksum = crc32(chksum, (void *)rec + rec->hdr_size, rec->size);
+		chksum = crc32(chksum, (void *)rec, rec_hdr_size(rec));
+		chksum = crc32(chksum, (void *)rec + rec_hdr_size(rec),
+			       rec->size);
 	}
 
 	return chksum;
@@ -413,8 +424,9 @@ void bloblist_show_list(void)
 	for (rec = bloblist_first_blob(hdr); rec;
 	     rec = bloblist_next_blob(hdr, rec)) {
 		printf("%08lx  %8x  %4x %s\n",
-		       (ulong)map_to_sysmem((void *)rec + rec->hdr_size),
-		       rec->size, rec->tag, bloblist_tag_name(rec->tag));
+		       (ulong)map_to_sysmem((void *)rec + rec_hdr_size(rec)),
+		       rec->size, rec_tag(rec),
+		       bloblist_tag_name(rec_tag(rec)));
 	}
 }
 
-- 
2.41.0.487.g6d72f3e995-goog

[PATCH 06/14] bloblist: Drop the flags value

[Simon Glass]

There is no flags value in spec v0.9 so drop it.

For now it is still present in the header, with an underscore, so that
tests continue to pass.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/bloblist.c  |  5 ++---
 include/bloblist.h |  6 ++----
 test/bloblist.c    | 45 +++++++++++++++++++--------------------------
 3 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 0def7fc9b2f..73f93687015 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -322,7 +322,7 @@ static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr)
 	return chksum;
 }
 
-int bloblist_new(ulong addr, uint size, uint flags)
+int bloblist_new(ulong addr, uint size)
 {
 	struct bloblist_hdr *hdr;
 
@@ -334,7 +334,6 @@ int bloblist_new(ulong addr, uint size, uint flags)
 	memset(hdr, '\0', sizeof(*hdr));
 	hdr->version = BLOBLIST_VERSION;
 	hdr->hdr_size = sizeof(*hdr);
-	hdr->flags = flags;
 	hdr->magic = BLOBLIST_MAGIC;
 	hdr->size = size;
 	hdr->alloced = hdr->hdr_size;
@@ -481,7 +480,7 @@ int bloblist_init(void)
 		}
 		log_debug("Creating new bloblist size %lx at %lx\n", size,
 			  addr);
-		ret = bloblist_new(addr, size, 0);
+		ret = bloblist_new(addr, size);
 	} else {
 		log_debug("Found existing bloblist size %lx at %lx\n", size,
 			  addr);
diff --git a/include/bloblist.h b/include/bloblist.h
index 74932dcae6f..0b1aaf3253e 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -150,7 +150,6 @@ enum bloblist_tag_t {
  * @version: BLOBLIST_VERSION
  * @hdr_size: Size of this header, normally sizeof(struct bloblist_hdr). The
  *	first bloblist_rec starts at this offset from the start of the header
- * @flags: Space for BLOBLISTF... flags (none yet)
  * @size: Total size of the bloblist (non-zero if valid) including this header.
  *	The bloblist extends for this many bytes from the start of this header.
  *	When adding new records, the bloblist can grow up to this size.
@@ -168,7 +167,7 @@ struct bloblist_hdr {
 	u32 magic;
 	u32 version;
 	u32 hdr_size;
-	u32 flags;
+	u32 _flags;
 
 	u32 size;
 	u32 alloced;
@@ -303,11 +302,10 @@ int bloblist_resize(uint tag, int new_size);
  *
  * @addr: Address of bloblist
  * @size: Initial size for bloblist
- * @flags: Flags to use for bloblist
  * Return: 0 if OK, -EFAULT if addr is not aligned correctly, -ENOSPC is the
  * area is not large enough
  */
-int bloblist_new(ulong addr, uint size, uint flags);
+int bloblist_new(ulong addr, uint size);
 
 /**
  * bloblist_check() - Check if a bloblist exists
diff --git a/test/bloblist.c b/test/bloblist.c
index 3d988fe05ae..11bd362a0d5 100644
--- a/test/bloblist.c
+++ b/test/bloblist.c
@@ -72,15 +72,15 @@ static int bloblist_test_init(struct unit_test_state *uts)
 	hdr = clear_bloblist();
 	ut_asserteq(-ENOENT, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	ut_asserteq_ptr(NULL, bloblist_check_magic(TEST_ADDR));
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	ut_asserteq_ptr(hdr, bloblist_check_magic(TEST_ADDR));
 	hdr->version++;
 	ut_asserteq(-EPROTONOSUPPORT, bloblist_check(TEST_ADDR,
 						     TEST_BLOBLIST_SIZE));
 
-	ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0x10, 0));
-	ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE, 0));
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0x10));
+	ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 
 	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	ut_assertok(bloblist_finish());
@@ -90,9 +90,6 @@ static int bloblist_test_init(struct unit_test_state *uts)
 	ut_asserteq_ptr(NULL, bloblist_check_magic(TEST_ADDR));
 	hdr->magic--;
 
-	hdr->flags++;
-	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
-
 	return 1;
 }
 BLOBLIST_TEST(bloblist_test_init, 0);
@@ -106,7 +103,7 @@ static int bloblist_test_blob(struct unit_test_state *uts)
 	/* At the start there should be no records */
 	hdr = clear_bloblist();
 	ut_assertnull(bloblist_find(TEST_TAG, TEST_BLOBLIST_SIZE));
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	ut_asserteq(TEST_BLOBLIST_SIZE, bloblist_get_size());
 	ut_asserteq(TEST_ADDR, bloblist_get_base());
 	ut_asserteq(map_to_sysmem(hdr), TEST_ADDR);
@@ -144,7 +141,7 @@ static int bloblist_test_blob_ensure(struct unit_test_state *uts)
 
 	/* At the start there should be no records */
 	clear_bloblist();
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 
 	/* Test with an empty bloblist */
 	size = TEST_SIZE;
@@ -176,7 +173,7 @@ static int bloblist_test_bad_blob(struct unit_test_state *uts)
 	void *data;
 
 	hdr = clear_bloblist();
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	data = hdr + 1;
 	data += sizeof(struct bloblist_rec);
 	ut_asserteq_addr(data, bloblist_ensure(TEST_TAG, TEST_SIZE));
@@ -192,7 +189,7 @@ static int bloblist_test_checksum(struct unit_test_state *uts)
 	char *data, *data2;
 
 	hdr = clear_bloblist();
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	ut_assertok(bloblist_finish());
 	ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
 
@@ -201,10 +198,6 @@ static int bloblist_test_checksum(struct unit_test_state *uts)
 	 * change the size or alloced fields, since that will crash the code.
 	 * It has to rely on these being correct.
 	 */
-	hdr->flags--;
-	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
-	hdr->flags++;
-
 	hdr->size--;
 	ut_asserteq(-EFBIG, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	hdr->size++;
@@ -256,7 +249,7 @@ static int bloblist_test_cmd_info(struct unit_test_state *uts)
 	char *data, *data2;
 
 	hdr = clear_bloblist();
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	data = bloblist_ensure(TEST_TAG, TEST_SIZE);
 	data2 = bloblist_ensure(TEST_TAG2, TEST_SIZE2);
 
@@ -282,7 +275,7 @@ static int bloblist_test_cmd_list(struct unit_test_state *uts)
 	char *data, *data2;
 
 	hdr = clear_bloblist();
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	data = bloblist_ensure(TEST_TAG, TEST_SIZE);
 	data2 = bloblist_ensure(TEST_TAG2, TEST_SIZE2);
 
@@ -312,7 +305,7 @@ static int bloblist_test_align(struct unit_test_state *uts)
 
 	/* At the start there should be no records */
 	hdr = clear_bloblist();
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	ut_assertnull(bloblist_find(TEST_TAG, TEST_BLOBLIST_SIZE));
 
 	/* Check the default alignment */
@@ -348,8 +341,8 @@ static int bloblist_test_align(struct unit_test_state *uts)
 	hdr = map_sysmem(TEST_ADDR + BLOBLIST_ALIGN, TEST_BLOBLIST_SIZE);
 	memset(hdr, ERASE_BYTE, TEST_BLOBLIST_SIZE);
 	memset(hdr, '\0', sizeof(*hdr));
-	ut_assertok(bloblist_new(TEST_ADDR + BLOBLIST_ALIGN, TEST_BLOBLIST_SIZE,
-				 0));
+	ut_assertok(bloblist_new(TEST_ADDR + BLOBLIST_ALIGN,
+				 TEST_BLOBLIST_SIZE));
 
 	data = bloblist_add(1, 5, BLOBLIST_ALIGN_LOG2 + 1);
 	ut_assertnonnull(data);
@@ -370,7 +363,7 @@ static int bloblist_test_reloc(struct unit_test_state *uts)
 	ulong new_addr;
 	ulong new_size;
 
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	old_ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE);
 
 	/* Add one blob and then one that won't fit */
@@ -409,7 +402,7 @@ static int bloblist_test_grow(struct unit_test_state *uts)
 	memset(hdr, ERASE_BYTE, TEST_BLOBLIST_SIZE);
 
 	/* Create two blobs */
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	blob1 = bloblist_add(TEST_TAG, small_size, 0);
 	ut_assertnonnull(blob1);
 	ut_assertok(check_zero(blob1, small_size));
@@ -461,7 +454,7 @@ static int bloblist_test_shrink(struct unit_test_state *uts)
 	ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE);
 
 	/* Create two blobs */
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	blob1 = bloblist_add(TEST_TAG, small_size, 0);
 	ut_assertnonnull(blob1);
 	strcpy(blob1, test1_str);
@@ -511,7 +504,7 @@ static int bloblist_test_resize_fail(struct unit_test_state *uts)
 	ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE);
 
 	/* Create two blobs */
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	blob1 = bloblist_add(TEST_TAG, small_size, 0);
 	ut_assertnonnull(blob1);
 
@@ -548,7 +541,7 @@ static int bloblist_test_resize_last(struct unit_test_state *uts)
 	hdr = ptr;
 
 	/* Create two blobs */
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	blob1 = bloblist_add(TEST_TAG, small_size, 0);
 	ut_assertnonnull(blob1);
 
@@ -593,7 +586,7 @@ static int bloblist_test_blob_maxsize(struct unit_test_state *uts)
 
 	/* At the start there should be no records */
 	clear_bloblist();
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 
 	/* Add a blob that takes up all space */
 	size = TEST_BLOBLIST_SIZE - sizeof(struct bloblist_hdr) -
-- 
2.41.0.487.g6d72f3e995-goog

[PATCH 07/14] bloblist: Drop the spare values

[Simon Glass]

There are no spare values in spec v0.9 so drop them.

For now they are still present in the headers, with an underscore, so that
tests continue to pass.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/bloblist.c  | 1 -
 include/bloblist.h | 6 ++----
 test/bloblist.c    | 4 ----
 3 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 73f93687015..96b82fe625c 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -154,7 +154,6 @@ static int bloblist_addrec(uint tag, int size, int align_log2,
 	rec->tag = tag;
 	rec->hdr_size = data_start - hdr->alloced;
 	rec->size = size;
-	rec->spare = 0;
 
 	/* Zero the record data */
 	memset((void *)rec + rec_hdr_size(rec), '\0', rec->size);
diff --git a/include/bloblist.h b/include/bloblist.h
index 0b1aaf3253e..6592ea75403 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -156,7 +156,6 @@ enum bloblist_tag_t {
  * @alloced: Total size allocated so far for this bloblist. This starts out as
  *	sizeof(bloblist_hdr) since we need at least that much space to store a
  *	valid bloblist
- * @spare: Spare space (for future use)
  * @chksum: CRC32 for the entire bloblist allocated area. Since any of the
  *	blobs can be altered after being created, this checksum is only valid
  *	when the bloblist is finalised before jumping to the next stage of boot.
@@ -171,7 +170,7 @@ struct bloblist_hdr {
 
 	u32 size;
 	u32 alloced;
-	u32 spare;
+	u32 _spare;
 	u32 chksum;
 };
 
@@ -188,13 +187,12 @@ struct bloblist_hdr {
  *	record's data starts at this offset from the start of the record
  * @size: Size of record in bytes, excluding the header size. This does not
  *	need to be aligned (e.g. 3 is OK).
- * @spare: Spare space for other things
  */
 struct bloblist_rec {
 	u32 tag;
 	u32 hdr_size;
 	u32 size;
-	u32 spare;
+	u32 _spare;
 };
 
 /**
diff --git a/test/bloblist.c b/test/bloblist.c
index 11bd362a0d5..c2c2c3f3c11 100644
--- a/test/bloblist.c
+++ b/test/bloblist.c
@@ -202,10 +202,6 @@ static int bloblist_test_checksum(struct unit_test_state *uts)
 	ut_asserteq(-EFBIG, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	hdr->size++;
 
-	hdr->spare++;
-	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
-	hdr->spare--;
-
 	hdr->chksum++;
 	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	hdr->chksum--;
-- 
2.41.0.487.g6d72f3e995-goog

[PATCH 08/14] bloblist: Change the checksum algorithm

[Simon Glass]

Use a sinple 8-bit checksum for bloblist, as specified by the spec
version 0.9

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/bloblist.c  | 14 ++++++++------
 include/bloblist.h |  5 ++---
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 96b82fe625c..77892582b90 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -13,6 +13,7 @@
 #include <malloc.h>
 #include <mapmem.h>
 #include <spl.h>
+#include <tables_csum.h>
 #include <asm/global_data.h>
 #include <u-boot/crc.h>
 
@@ -308,14 +309,15 @@ int bloblist_resize(uint tag, int new_size)
 static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr)
 {
 	struct bloblist_rec *rec;
-	u32 chksum;
+	u8 chksum;
 
-	chksum = crc32(0, (unsigned char *)hdr,
-		       offsetof(struct bloblist_hdr, chksum));
+	chksum = table_compute_checksum(hdr, hdr->hdr_size);
+	chksum += hdr->chksum;
 	foreach_rec(rec, hdr) {
-		chksum = crc32(chksum, (void *)rec, rec_hdr_size(rec));
-		chksum = crc32(chksum, (void *)rec + rec_hdr_size(rec),
-			       rec->size);
+		chksum -= table_compute_checksum((void *)rec,
+						 rec_hdr_size(rec));
+		chksum -= table_compute_checksum((void *)rec +
+						 rec_hdr_size(rec), rec->size);
 	}
 
 	return chksum;
diff --git a/include/bloblist.h b/include/bloblist.h
index 6592ea75403..ee3644878b2 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -156,11 +156,10 @@ enum bloblist_tag_t {
  * @alloced: Total size allocated so far for this bloblist. This starts out as
  *	sizeof(bloblist_hdr) since we need at least that much space to store a
  *	valid bloblist
- * @chksum: CRC32 for the entire bloblist allocated area. Since any of the
+ * @chksum: checksum for the entire bloblist allocated area. Since any of the
  *	blobs can be altered after being created, this checksum is only valid
  *	when the bloblist is finalised before jumping to the next stage of boot.
- *	Note that chksum is last to make it easier to exclude it from the
- *	checksum calculation.
+ *	This is the value needed to make all checksummed bytes sum to 0
  */
 struct bloblist_hdr {
 	u32 magic;
-- 
2.41.0.487.g6d72f3e995-goog

[PATCH 09/14] bloblist: Checksum the entire bloblist

[Simon Glass]

Spec v0.9 specifies that the entire bloblist area is checksummed,
including unused portions. Update the code to follow this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/bloblist.c |  9 +--------
 test/bloblist.c   | 10 ++++++++--
 2 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 77892582b90..df5c63dae15 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -308,17 +308,10 @@ int bloblist_resize(uint tag, int new_size)
 
 static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr)
 {
-	struct bloblist_rec *rec;
 	u8 chksum;
 
-	chksum = table_compute_checksum(hdr, hdr->hdr_size);
+	chksum = table_compute_checksum(hdr, hdr->alloced);
 	chksum += hdr->chksum;
-	foreach_rec(rec, hdr) {
-		chksum -= table_compute_checksum((void *)rec,
-						 rec_hdr_size(rec));
-		chksum -= table_compute_checksum((void *)rec +
-						 rec_hdr_size(rec), rec->size);
-	}
 
 	return chksum;
 }
diff --git a/test/bloblist.c b/test/bloblist.c
index c2c2c3f3c11..9039aaae10d 100644
--- a/test/bloblist.c
+++ b/test/bloblist.c
@@ -226,12 +226,18 @@ static int bloblist_test_checksum(struct unit_test_state *uts)
 	*data2 -= 1;
 
 	/*
-	 * Changing data outside the range of valid data should not affect
-	 * the checksum.
+	 * Changing data outside the range of valid data should affect the
+	 * checksum.
 	 */
 	ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	data[TEST_SIZE]++;
+	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	data[TEST_SIZE]--;
+	ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+
 	data2[TEST_SIZE2]++;
+	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	data[TEST_SIZE]--;
 	ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
 
 	return 0;
-- 
2.41.0.487.g6d72f3e995-goog

[PATCH 10/14] bloblist: Handle alignment with a void entry

[Simon Glass]

Rather than setting the alignment using the header size, add an entirely
new entry to cover the gap left by the alignment.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/bloblist.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index df5c63dae15..0ab82d3cdbc 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -131,7 +131,7 @@ static int bloblist_addrec(uint tag, int size, int align_log2,
 {
 	struct bloblist_hdr *hdr = gd->bloblist;
 	struct bloblist_rec *rec;
-	int data_start, new_alloced;
+	int data_start, aligned_start, new_alloced;
 
 	if (!align_log2)
 		align_log2 = BLOBLIST_ALIGN_LOG2;
@@ -140,10 +140,25 @@ static int bloblist_addrec(uint tag, int size, int align_log2,
 	data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*rec);
 
 	/* Align the address and then calculate the offset from ->alloced */
-	data_start = ALIGN(data_start, 1U << align_log2) - map_to_sysmem(hdr);
+	aligned_start = ALIGN(data_start, 1U << align_log2) - data_start;
+
+	/* If we need to create a dummy record, create it */
+	if (aligned_start) {
+		int void_size = aligned_start - sizeof(*rec);
+		struct bloblist_rec *vrec;
+		int ret;
+
+		ret = bloblist_addrec(BLOBLISTT_VOID, void_size, 0, &vrec);
+		if (ret)
+			return log_msg_ret("void", ret);
+
+		/* start the record after that */
+		data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*vrec);
+	}
 
 	/* Calculate the new allocated total */
-	new_alloced = data_start + ALIGN(size, 1U << align_log2);
+	new_alloced = data_start - map_to_sysmem(hdr) +
+		ALIGN(size, 1U << align_log2);
 
 	if (new_alloced > hdr->size) {
 		log_err("Failed to allocate %x bytes size=%x, need size=%x\n",
@@ -153,7 +168,7 @@ static int bloblist_addrec(uint tag, int size, int align_log2,
 	rec = (void *)hdr + hdr->alloced;
 
 	rec->tag = tag;
-	rec->hdr_size = data_start - hdr->alloced;
+	rec->hdr_size = sizeof(struct bloblist_rec);
 	rec->size = size;
 
 	/* Zero the record data */
-- 
2.41.0.487.g6d72f3e995-goog

[PATCH 11/14] bloblist: Reduce blob-header size

[Simon Glass]

The v0.9 spec provides for an 8-byte header for each blob, with fewer
fields. Update the implementation to match this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/bloblist.c  | 17 +++++++++--------
 include/bloblist.h | 33 ++++++++++++++++++++++-----------
 test/bloblist.c    | 16 ++++++++--------
 3 files changed, 39 insertions(+), 27 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 0ab82d3cdbc..605bc1f90fe 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -76,12 +76,14 @@ static struct bloblist_rec *bloblist_first_blob(struct bloblist_hdr *hdr)
 
 static inline uint rec_hdr_size(struct bloblist_rec *rec)
 {
-	return rec->hdr_size;
+	return (rec->tag_and_hdr_size & BLOBLISTR_HDR_SIZE_MASK) >>
+		BLOBLISTR_HDR_SIZE_SHIFT;
 }
 
 static inline uint rec_tag(struct bloblist_rec *rec)
 {
-	return rec->tag;
+	return (rec->tag_and_hdr_size & BLOBLISTR_TAG_MASK) >>
+		BLOBLISTR_TAG_SHIFT;
 }
 
 static ulong bloblist_blob_end_ofs(struct bloblist_hdr *hdr,
@@ -90,7 +92,7 @@ static ulong bloblist_blob_end_ofs(struct bloblist_hdr *hdr,
 	ulong offset;
 
 	offset = (void *)rec - (void *)hdr;
-	offset += rec_hdr_size(rec) + ALIGN(rec->size, BLOBLIST_ALIGN);
+	offset += rec_hdr_size(rec) + ALIGN(rec->size, sizeof(*rec));
 
 	return offset;
 }
@@ -134,7 +136,7 @@ static int bloblist_addrec(uint tag, int size, int align_log2,
 	int data_start, aligned_start, new_alloced;
 
 	if (!align_log2)
-		align_log2 = BLOBLIST_ALIGN_LOG2;
+		align_log2 = BLOBLIST_BLOB_ALIGN_LOG2;
 
 	/* Figure out where the new data will start */
 	data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*rec);
@@ -167,8 +169,7 @@ static int bloblist_addrec(uint tag, int size, int align_log2,
 	}
 	rec = (void *)hdr + hdr->alloced;
 
-	rec->tag = tag;
-	rec->hdr_size = sizeof(struct bloblist_rec);
+	rec->tag_and_hdr_size = tag | sizeof(*rec) << BLOBLISTR_HDR_SIZE_SHIFT;
 	rec->size = size;
 
 	/* Zero the record data */
@@ -272,8 +273,8 @@ static int bloblist_resize_rec(struct bloblist_hdr *hdr,
 	int new_alloced;	/* New value for @hdr->alloced */
 	ulong next_ofs;	/* Offset of the record after @rec */
 
-	expand_by = ALIGN(new_size - rec->size, BLOBLIST_ALIGN);
-	new_alloced = ALIGN(hdr->alloced + expand_by, BLOBLIST_ALIGN);
+	expand_by = ALIGN(new_size - rec->size, BLOBLIST_BLOB_ALIGN);
+	new_alloced = ALIGN(hdr->alloced + expand_by, BLOBLIST_BLOB_ALIGN);
 	if (new_size < 0) {
 		log_debug("Attempt to shrink blob size below 0 (%x)\n",
 			  new_size);
diff --git a/include/bloblist.h b/include/bloblist.h
index ee3644878b2..d1e52cf888f 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -24,11 +24,11 @@
  * which would add to code size. For Thumb-2 the code size needed in SPL is
  * approximately 940 bytes (e.g. for chromebook_bob).
  *
- * 5. Bloblist uses 16-byte alignment internally and is designed to start on a
- * 16-byte boundary. Its headers are multiples of 16 bytes. This makes it easier
- * to deal with data structures which need this level of alignment, such as ACPI
- * tables. For use in SPL and TPL the alignment can be relaxed, since it can be
- * relocated to an aligned address in U-Boot proper.
+ * 5. Bloblist uses 8-byte alignment internally and is designed to start on a
+ * 8-byte boundary. Its headers are 8 bytes long. It is possible to achieve
+ * larger alignment (e.g. 16 bytes) by adding a dummy header, For use in SPL and
+ * TPL the alignment can be relaxed, since it can be relocated to an aligned
+ * address in U-Boot proper.
  *
  * 6. Bloblist is designed to be passed to Linux as reserved memory. While linux
  * doesn't understand the bloblist header, it can be passed the indivdual blobs.
@@ -77,6 +77,9 @@ enum {
 	BLOBLIST_VERSION	= 1,
 	BLOBLIST_MAGIC		= 0x6ed0ff,
 
+	BLOBLIST_BLOB_ALIGN_LOG2 = 3,
+	BLOBLIST_BLOB_ALIGN	 = 1 << BLOBLIST_BLOB_ALIGN_LOG2,
+
 	BLOBLIST_ALIGN_LOG2	= 4,
 	BLOBLIST_ALIGN		= 1 << BLOBLIST_ALIGN_LOG2,
 };
@@ -181,17 +184,25 @@ struct bloblist_hdr {
  *
  * NOTE: Only exported for testing purposes. Do not use this struct.
  *
- * @tag: Tag indicating what the record contains
- * @hdr_size: Size of this header, normally sizeof(struct bloblist_rec). The
- *	record's data starts at this offset from the start of the record
+ * @tag_and_hdr_size: Tag indicating what the record contains (bottom 24 bits), and
+ *	size of this header (top 8 bits), normally sizeof(struct bloblist_rec).
+ *	The record's data starts at this offset from the start of the record
  * @size: Size of record in bytes, excluding the header size. This does not
  *	need to be aligned (e.g. 3 is OK).
  */
 struct bloblist_rec {
-	u32 tag;
-	u32 hdr_size;
+	u32 tag_and_hdr_size;
 	u32 size;
-	u32 _spare;
+};
+
+enum {
+	BLOBLISTR_TAG_SHIFT		= 0,
+	BLOBLISTR_TAG_MASK		= 0xffffffU << BLOBLISTR_TAG_SHIFT,
+	BLOBLISTR_HDR_SIZE_SHIFT	= 24,
+	BLOBLISTR_HDR_SIZE_MASK		= 0xffU << BLOBLISTR_HDR_SIZE_SHIFT,
+
+	BLOBLIST_HDR_SIZE		= 16,
+	BLOBLIST_REC_HDR_SIZE		= 8,
 };
 
 /**
diff --git a/test/bloblist.c b/test/bloblist.c
index 9039aaae10d..c1719c2e384 100644
--- a/test/bloblist.c
+++ b/test/bloblist.c
@@ -261,8 +261,8 @@ static int bloblist_test_cmd_info(struct unit_test_state *uts)
 	run_command("bloblist info", 0);
 	ut_assert_nextline("base:     %lx", (ulong)map_to_sysmem(hdr));
 	ut_assert_nextline("size:     400    1 KiB");
-	ut_assert_nextline("alloced:  70     112 Bytes");
-	ut_assert_nextline("free:     390    912 Bytes");
+	ut_assert_nextline("alloced:  58     88 Bytes");
+	ut_assert_nextline("free:     3a8    936 Bytes");
 	ut_assert_console_end();
 	ut_unsilence_console(uts);
 
@@ -320,12 +320,12 @@ static int bloblist_test_align(struct unit_test_state *uts)
 		data = bloblist_add(i, size, 0);
 		ut_assertnonnull(data);
 		addr = map_to_sysmem(data);
-		ut_asserteq(0, addr & (BLOBLIST_ALIGN - 1));
+		ut_asserteq(0, addr & (BLOBLIST_BLOB_ALIGN - 1));
 
 		/* Only the bytes in the blob data should be zeroed */
 		for (j = 0; j < size; j++)
 			ut_asserteq(0, data[j]);
-		for (; j < BLOBLIST_ALIGN; j++)
+		for (; j < BLOBLIST_BLOB_ALIGN; j++)
 			ut_asserteq(ERASE_BYTE, data[j]);
 	}
 
@@ -340,7 +340,7 @@ static int bloblist_test_align(struct unit_test_state *uts)
 	}
 
 	/* Check alignment with an bloblist starting on a smaller alignment */
-	hdr = map_sysmem(TEST_ADDR + BLOBLIST_ALIGN, TEST_BLOBLIST_SIZE);
+	hdr = map_sysmem(TEST_ADDR + BLOBLIST_BLOB_ALIGN, TEST_BLOBLIST_SIZE);
 	memset(hdr, ERASE_BYTE, TEST_BLOBLIST_SIZE);
 	memset(hdr, '\0', sizeof(*hdr));
 	ut_assertok(bloblist_new(TEST_ADDR + BLOBLIST_ALIGN,
@@ -349,7 +349,7 @@ static int bloblist_test_align(struct unit_test_state *uts)
 	data = bloblist_add(1, 5, BLOBLIST_ALIGN_LOG2 + 1);
 	ut_assertnonnull(data);
 	addr = map_to_sysmem(data);
-	ut_asserteq(0, addr & (BLOBLIST_ALIGN * 2 - 1));
+	ut_asserteq(0, addr & (BLOBLIST_BLOB_ALIGN * 2 - 1));
 
 	return 0;
 }
@@ -437,7 +437,7 @@ static int bloblist_test_grow(struct unit_test_state *uts)
 	hdr = ptr;
 	ut_asserteq(sizeof(struct bloblist_hdr) +
 		    sizeof(struct bloblist_rec) * 2 + small_size * 2 +
-		    BLOBLIST_ALIGN,
+		    BLOBLIST_BLOB_ALIGN,
 		    hdr->alloced);
 
 	return 0;
@@ -572,7 +572,7 @@ static int bloblist_test_resize_last(struct unit_test_state *uts)
 	ut_asserteq((u8)ERASE_BYTE, *((u8 *)hdr + alloced_val + 4));
 
 	/* Check that the new top of the allocated blobs has not been touched */
-	alloced_val += BLOBLIST_ALIGN;
+	alloced_val += BLOBLIST_BLOB_ALIGN;
 	ut_asserteq(alloced_val, hdr->alloced);
 	ut_asserteq((u8)ERASE_BYTE, *((u8 *)hdr + hdr->alloced));
 
-- 
2.41.0.487.g6d72f3e995-goog

[PATCH 12/14] bloblist: Reduce bloblist header size

[Simon Glass]

The v0.9 spec provides for a 16-byte header with fewer fields. Update
the implementation to match this.

This also adds an alignment field.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/bloblist.h | 26 +++++++++++++-------------
 test/bloblist.c    |  6 +++---
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/include/bloblist.h b/include/bloblist.h
index d1e52cf888f..13b619cd019 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -150,30 +150,30 @@ enum bloblist_tag_t {
  * from the last.
  *
  * @magic: BLOBLIST_MAGIC
+ * @chksum: checksum for the entire bloblist allocated area. Since any of the
+ *	blobs can be altered after being created, this checksum is only valid
+ *	when the bloblist is finalised before jumping to the next stage of boot.
+ *	This is the value needed to make all chechksummed bytes sum to 0
  * @version: BLOBLIST_VERSION
  * @hdr_size: Size of this header, normally sizeof(struct bloblist_hdr). The
  *	first bloblist_rec starts at this offset from the start of the header
- * @size: Total size of the bloblist (non-zero if valid) including this header.
- *	The bloblist extends for this many bytes from the start of this header.
- *	When adding new records, the bloblist can grow up to this size.
+ * @align_log2: Power of two of the maximum alignment required by this list
  * @alloced: Total size allocated so far for this bloblist. This starts out as
  *	sizeof(bloblist_hdr) since we need at least that much space to store a
  *	valid bloblist
- * @chksum: checksum for the entire bloblist allocated area. Since any of the
- *	blobs can be altered after being created, this checksum is only valid
- *	when the bloblist is finalised before jumping to the next stage of boot.
- *	This is the value needed to make all checksummed bytes sum to 0
+ * @size: Total size of the bloblist (non-zero if valid) including this header.
+ *	The bloblist extends for this many bytes from the start of this header.
+ *	When adding new records, the bloblist can grow up to this size.
  */
 struct bloblist_hdr {
 	u32 magic;
-	u32 version;
-	u32 hdr_size;
-	u32 _flags;
+	u8 chksum;
+	u8 version;
+	u8 hdr_size;
+	u8 align_log2;
 
-	u32 size;
 	u32 alloced;
-	u32 _spare;
-	u32 chksum;
+	u32 size;
 };
 
 /**
diff --git a/test/bloblist.c b/test/bloblist.c
index c1719c2e384..5801160621a 100644
--- a/test/bloblist.c
+++ b/test/bloblist.c
@@ -78,7 +78,7 @@ static int bloblist_test_init(struct unit_test_state *uts)
 	ut_asserteq(-EPROTONOSUPPORT, bloblist_check(TEST_ADDR,
 						     TEST_BLOBLIST_SIZE));
 
-	ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0x10));
+	ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0xc));
 	ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE));
 	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
 
@@ -261,8 +261,8 @@ static int bloblist_test_cmd_info(struct unit_test_state *uts)
 	run_command("bloblist info", 0);
 	ut_assert_nextline("base:     %lx", (ulong)map_to_sysmem(hdr));
 	ut_assert_nextline("size:     400    1 KiB");
-	ut_assert_nextline("alloced:  58     88 Bytes");
-	ut_assert_nextline("free:     3a8    936 Bytes");
+	ut_assert_nextline("alloced:  48     72 Bytes");
+	ut_assert_nextline("free:     3b8    952 Bytes");
 	ut_assert_console_end();
 	ut_unsilence_console(uts);
 
-- 
2.41.0.487.g6d72f3e995-goog

[PATCH 13/14] bloblist: Add alignment to bloblist_new()

[Simon Glass]

Allow the alignment to be specified when creating a bloblist.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 common/bloblist.c  |  5 +++--
 include/bloblist.h |  3 ++-
 test/bloblist.c    | 40 ++++++++++++++++++++++------------------
 3 files changed, 27 insertions(+), 21 deletions(-)

diff --git a/common/bloblist.c b/common/bloblist.c
index 605bc1f90fe..892ad01a1fd 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -332,7 +332,7 @@ static u32 bloblist_calc_chksum(struct bloblist_hdr *hdr)
 	return chksum;
 }
 
-int bloblist_new(ulong addr, uint size)
+int bloblist_new(ulong addr, uint size, uint align_log2)
 {
 	struct bloblist_hdr *hdr;
 
@@ -347,6 +347,7 @@ int bloblist_new(ulong addr, uint size)
 	hdr->magic = BLOBLIST_MAGIC;
 	hdr->size = size;
 	hdr->alloced = hdr->hdr_size;
+	hdr->align_log2 = align_log2 ?: BLOBLIST_BLOB_ALIGN_LOG2;
 	hdr->chksum = 0;
 	gd->bloblist = hdr;
 
@@ -490,7 +491,7 @@ int bloblist_init(void)
 		}
 		log_debug("Creating new bloblist size %lx at %lx\n", size,
 			  addr);
-		ret = bloblist_new(addr, size);
+		ret = bloblist_new(addr, size, 0);
 	} else {
 		log_debug("Found existing bloblist size %lx at %lx\n", size,
 			  addr);
diff --git a/include/bloblist.h b/include/bloblist.h
index 13b619cd019..533db708c22 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -310,10 +310,11 @@ int bloblist_resize(uint tag, int new_size);
  *
  * @addr: Address of bloblist
  * @size: Initial size for bloblist
+ * @align_log2: Log base 2 of maximum alignment provided by this bloblist
  * Return: 0 if OK, -EFAULT if addr is not aligned correctly, -ENOSPC is the
  * area is not large enough
  */
-int bloblist_new(ulong addr, uint size);
+int bloblist_new(ulong addr, uint size, uint align_log2);
 
 /**
  * bloblist_check() - Check if a bloblist exists
diff --git a/test/bloblist.c b/test/bloblist.c
index 5801160621a..4d3752831e7 100644
--- a/test/bloblist.c
+++ b/test/bloblist.c
@@ -72,15 +72,15 @@ static int bloblist_test_init(struct unit_test_state *uts)
 	hdr = clear_bloblist();
 	ut_asserteq(-ENOENT, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	ut_asserteq_ptr(NULL, bloblist_check_magic(TEST_ADDR));
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 	ut_asserteq_ptr(hdr, bloblist_check_magic(TEST_ADDR));
 	hdr->version++;
 	ut_asserteq(-EPROTONOSUPPORT, bloblist_check(TEST_ADDR,
 						     TEST_BLOBLIST_SIZE));
 
-	ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0xc));
-	ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE));
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0xc, 0));
+	ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE, 0));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 
 	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	ut_assertok(bloblist_finish());
@@ -103,7 +103,7 @@ static int bloblist_test_blob(struct unit_test_state *uts)
 	/* At the start there should be no records */
 	hdr = clear_bloblist();
 	ut_assertnull(bloblist_find(TEST_TAG, TEST_BLOBLIST_SIZE));
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 	ut_asserteq(TEST_BLOBLIST_SIZE, bloblist_get_size());
 	ut_asserteq(TEST_ADDR, bloblist_get_base());
 	ut_asserteq(map_to_sysmem(hdr), TEST_ADDR);
@@ -141,7 +141,7 @@ static int bloblist_test_blob_ensure(struct unit_test_state *uts)
 
 	/* At the start there should be no records */
 	clear_bloblist();
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 
 	/* Test with an empty bloblist */
 	size = TEST_SIZE;
@@ -173,7 +173,7 @@ static int bloblist_test_bad_blob(struct unit_test_state *uts)
 	void *data;
 
 	hdr = clear_bloblist();
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 	data = hdr + 1;
 	data += sizeof(struct bloblist_rec);
 	ut_asserteq_addr(data, bloblist_ensure(TEST_TAG, TEST_SIZE));
@@ -189,7 +189,7 @@ static int bloblist_test_checksum(struct unit_test_state *uts)
 	char *data, *data2;
 
 	hdr = clear_bloblist();
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 	ut_assertok(bloblist_finish());
 	ut_assertok(bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
 
@@ -206,6 +206,10 @@ static int bloblist_test_checksum(struct unit_test_state *uts)
 	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
 	hdr->chksum--;
 
+	hdr->align_log2++;
+	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	hdr->align_log2--;
+
 	/* Make sure the checksum changes when we add blobs */
 	data = bloblist_add(TEST_TAG, TEST_SIZE, 0);
 	ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
@@ -251,7 +255,7 @@ static int bloblist_test_cmd_info(struct unit_test_state *uts)
 	char *data, *data2;
 
 	hdr = clear_bloblist();
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 	data = bloblist_ensure(TEST_TAG, TEST_SIZE);
 	data2 = bloblist_ensure(TEST_TAG2, TEST_SIZE2);
 
@@ -277,7 +281,7 @@ static int bloblist_test_cmd_list(struct unit_test_state *uts)
 	char *data, *data2;
 
 	hdr = clear_bloblist();
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 	data = bloblist_ensure(TEST_TAG, TEST_SIZE);
 	data2 = bloblist_ensure(TEST_TAG2, TEST_SIZE2);
 
@@ -307,7 +311,7 @@ static int bloblist_test_align(struct unit_test_state *uts)
 
 	/* At the start there should be no records */
 	hdr = clear_bloblist();
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 	ut_assertnull(bloblist_find(TEST_TAG, TEST_BLOBLIST_SIZE));
 
 	/* Check the default alignment */
@@ -344,7 +348,7 @@ static int bloblist_test_align(struct unit_test_state *uts)
 	memset(hdr, ERASE_BYTE, TEST_BLOBLIST_SIZE);
 	memset(hdr, '\0', sizeof(*hdr));
 	ut_assertok(bloblist_new(TEST_ADDR + BLOBLIST_ALIGN,
-				 TEST_BLOBLIST_SIZE));
+				 TEST_BLOBLIST_SIZE, 0));
 
 	data = bloblist_add(1, 5, BLOBLIST_ALIGN_LOG2 + 1);
 	ut_assertnonnull(data);
@@ -365,7 +369,7 @@ static int bloblist_test_reloc(struct unit_test_state *uts)
 	ulong new_addr;
 	ulong new_size;
 
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 	old_ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE);
 
 	/* Add one blob and then one that won't fit */
@@ -404,7 +408,7 @@ static int bloblist_test_grow(struct unit_test_state *uts)
 	memset(hdr, ERASE_BYTE, TEST_BLOBLIST_SIZE);
 
 	/* Create two blobs */
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 	blob1 = bloblist_add(TEST_TAG, small_size, 0);
 	ut_assertnonnull(blob1);
 	ut_assertok(check_zero(blob1, small_size));
@@ -456,7 +460,7 @@ static int bloblist_test_shrink(struct unit_test_state *uts)
 	ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE);
 
 	/* Create two blobs */
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 	blob1 = bloblist_add(TEST_TAG, small_size, 0);
 	ut_assertnonnull(blob1);
 	strcpy(blob1, test1_str);
@@ -506,7 +510,7 @@ static int bloblist_test_resize_fail(struct unit_test_state *uts)
 	ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE);
 
 	/* Create two blobs */
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 	blob1 = bloblist_add(TEST_TAG, small_size, 0);
 	ut_assertnonnull(blob1);
 
@@ -543,7 +547,7 @@ static int bloblist_test_resize_last(struct unit_test_state *uts)
 	hdr = ptr;
 
 	/* Create two blobs */
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 	blob1 = bloblist_add(TEST_TAG, small_size, 0);
 	ut_assertnonnull(blob1);
 
@@ -588,7 +592,7 @@ static int bloblist_test_blob_maxsize(struct unit_test_state *uts)
 
 	/* At the start there should be no records */
 	clear_bloblist();
-	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
 
 	/* Add a blob that takes up all space */
 	size = TEST_BLOBLIST_SIZE - sizeof(struct bloblist_hdr) -
-- 
2.41.0.487.g6d72f3e995-goog

[PATCH 14/14] bloblist: Update documentation and header comment

[Simon Glass]

Align the documentation with the v0.9 spec.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 doc/develop/bloblist.rst | 4 +++-
 include/bloblist.h       | 1 +
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/doc/develop/bloblist.rst b/doc/develop/bloblist.rst
index 81643c7674b..28431039adc 100644
--- a/doc/develop/bloblist.rst
+++ b/doc/develop/bloblist.rst
@@ -14,6 +14,8 @@ structure defined by the code that owns it.
 For the design goals of bloblist, please see the comments at the top of the
 `bloblist.h` header file.
 
+Bloblist is an implementation with the `Firmware Handoff`_ protocol.
+
 Passing state through the boot process
 --------------------------------------
 
@@ -99,7 +101,7 @@ API documentation
 -----------------
 
 .. kernel-doc:: include/bloblist.h
-
+.. _`Firmware Handoff`: https://github.com/FirmwareHandoff/firmware_handoff
 
 Simon Glass
 sjg@chromium.org
diff --git a/include/bloblist.h b/include/bloblist.h
index 533db708c22..faf0664fa43 100644
--- a/include/bloblist.h
+++ b/include/bloblist.h
@@ -66,6 +66,7 @@
  *
  * Copyright 2018 Google, Inc
  * Written by Simon Glass <sjg@chromium.org>
+ * Adjusted July 2023 to match Firmware handoff specification, Release 0.9
  */
 
 #ifndef __BLOBLIST_H
-- 
2.41.0.487.g6d72f3e995-goog

Re: [PATCH 01/14] bloblist: Update the tag numbering

[Julius Werner]

> diff --git a/include/bloblist.h b/include/bloblist.h
> index 7ea72c6bd46..bad5fbbb889 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h

nit: I would suggest also updating the documentation at the top of
this file (point 7) to clarify that new standardized tags must be
allocated in the firmware_handoff repo?

Also, to point out the obvious, there are a bunch of tags in the
standardized range in this file that don't match the firmware_handoff
spec. I guess you could add them since 0x100 is still free. However,
at least the BLOBLISTT_ACPI_TABLES tag would likely(?) be redundant
with the existing XFERLIST_ACPI_AGGR tag.

> +       BLOBLISTT_U_BOOT_SPL_HANDOFF    = 0xfff000, /* Hand-off info from SPL */
> +       BLOBLISTT_VBE                   = 0xfff001, /* VBE per-phase state */
> +       BLOBLISTT_U_BOOT_VIDEO          = 0xfff002, /* Video info from SPL */

FWIW, according to my view of the tag allocation philosophy, I think
these kinds of tags should be allocated as official tags in the
standardized range (e.g. in a cluster of U-Boot-specific tags). I
would really recommend completely avoiding the non-standardized range
for anything other than local experimentation or tags internal to
closed-source code.

Re: [PATCH 10/14] bloblist: Handle alignment with a void entry

[Julius Werner]

>         /* Calculate the new allocated total */
> -       new_alloced = data_start + ALIGN(size, 1U << align_log2);
> +       new_alloced = data_start - map_to_sysmem(hdr) +
> +               ALIGN(size, 1U << align_log2);

I think this is incorrect. There's no requirement that the size of an
entry must also be aligned as strictly as its start offset. So if
someone calls this code as bloblist_addrec(tag, 16, 8, ptr), then it
will try to create a blob at a 256 byte boundary with only 16 bytes of
data size, which is perfectly legal, but this code here will set
new_alloced as if the data size was also 256. That's not correct and
would likely throw off calculations elsewhere later. The alignment to
the start of the next entry is always just 8 bytes, so this line
should use BLOBLIST_BLOB_ALIGN_LOG2 (or sizeof(*rec)) instead of
align_log2.

>         if (new_alloced > hdr->size) {
>                 log_err("Failed to allocate %x bytes size=%x, need size=%x\n",
> @@ -153,7 +168,7 @@ static int bloblist_addrec(uint tag, int size, int align_log2,
>         rec = (void *)hdr + hdr->alloced;
>
>         rec->tag = tag;
> -       rec->hdr_size = data_start - hdr->alloced;
> +       rec->hdr_size = sizeof(struct bloblist_rec);
>         rec->size = size;

You also need to update the TL header alignment field if the requested
alignment here is greater, e.g. something like

if (hdr->alignment < align_log2)
  hdr->alignment = align_log2;

Re: [PATCH 11/14] bloblist: Reduce blob-header size

[Julius Werner]

> -       rec->tag = tag;
> -       rec->hdr_size = sizeof(struct bloblist_rec);
> +       rec->tag_and_hdr_size = tag | sizeof(*rec) << BLOBLISTR_HDR_SIZE_SHIFT;

nit: Might be a good idea to mask the tag or double-check that it fits
the field size here.

> - * 5. Bloblist uses 16-byte alignment internally and is designed to start on a
> - * 16-byte boundary. Its headers are multiples of 16 bytes. This makes it easier
> - * to deal with data structures which need this level of alignment, such as ACPI
> - * tables. For use in SPL and TPL the alignment can be relaxed, since it can be
> - * relocated to an aligned address in U-Boot proper.
> + * 5. Bloblist uses 8-byte alignment internally and is designed to start on a
> + * 8-byte boundary. Its headers are 8 bytes long. It is possible to achieve
> + * larger alignment (e.g. 16 bytes) by adding a dummy header, For use in SPL and

nit: I would call it a "dummy entry", it's not always just a header.

> +       BLOBLIST_BLOB_ALIGN_LOG2 = 3,
> +       BLOBLIST_BLOB_ALIGN      = 1 << BLOBLIST_BLOB_ALIGN_LOG2,
> +
>         BLOBLIST_ALIGN_LOG2     = 4,
>         BLOBLIST_ALIGN          = 1 << BLOBLIST_ALIGN_LOG2,
>  };
> @@ -181,17 +184,25 @@ struct bloblist_hdr {

Note that there's no specific requirement for the TL header to be
aligned to 16 bytes. Even though it is 16 bytes long, 8 bytes
alignment is enough (so you shouldn't really need a BLOBLIST_ALIGN
that's different from BLOBLIST_BLOB_ALIGN).

There's also some text above this in the docstring for this struct
that refers to 16 bytes and should be updated. (I would recommend also
updating the "it can be safely moved around" part to point to the
instructions for relocation in the firmware_handoff spec, since while
it can be relocated, special care must be taken to preserve alignment
restrictions.

Re: [PATCH 13/14] bloblist: Add alignment to bloblist_new()

[Julius Werner]

I'm not sure why you would add an API to allow setting this
explicitly, that's not really how it is supposed to work according to
the spec. New TLs are always supposed to start with an alignment
requirement of 8 (2^3) and then automatically increase that value if a
TE gets added that has a larger requirement. There should be no use
case where you'd want to initially create a TL that already has a
larger number in that field (it doesn't make any difference... in
particular, note that just because the alignment field has a larger
value doesn't mean that the start of the TL must be aligned to that
larger boundary; the field is only used to preserve the offset from
the next alignment boundary during relocation).

Re: [PATCH 14/14] bloblist: Update documentation and header comment

[Julius Werner]

Here are a couple of other differences I have found between the
bloblist code after applying your patches and the TL specification:

* bloblist seems to explicitly disallow having the same tag more than
once in the list (e.g. see documentation of bloblist_add()), whereas
the TL specification explicitly allows that. You're of course free to
impose this restriction on the way U-Boot uses the spec, but you may
eventually run into compatibility issues if you hardcode that
assumption and one day might ingest a TL from a different writer that
doesn't adhere to it.

* The bloblist_resize() function doesn't preserve alignment
restrictions when relocating other TEs. I think the (only?) safe way
to resize a TE in-place would be to align the distance that following
TEs need to be moved up to (1 << hdr->align_log2), and then insert a
void dummy TE to account for the additional added distance if
necessary.

* The comment at the top of bloblist.c should be updated to reflect
how alignment and padding works in the TL spec.

* The checksum algorithm seems incorrect. U-Boot's
table_compute_checksum() subtracts bytes, but the TE spec says they
need to be summed up.

* The bloblist_reloc() function must be updated to follow the TL
relocation mechanism (i.e. take hdr->align_log2 into account to make
sure the new position preserves alignment requirements... I would say
see https://github.com/FirmwareHandoff/firmware_handoff/blob/main/source/transfer_list.rst#relocating-a-tl
for details, but I just noticed that we made a mistake there, so
please check the version from
https://github.com/FirmwareHandoff/firmware_handoff/pull/12 for the
corrected algorithm).

Re: [PATCH 01/14] bloblist: Update the tag numbering

[Ilias Apalodimas]

Hi Julius

On Wed, 26 Jul 2023 at 23:16, Julius Werner <jwerner@chromium.org> wrote:
>
> > diff --git a/include/bloblist.h b/include/bloblist.h
> > index 7ea72c6bd46..bad5fbbb889 100644
> > --- a/include/bloblist.h
> > +++ b/include/bloblist.h
>
> nit: I would suggest also updating the documentation at the top of
> this file (point 7) to clarify that new standardized tags must be
> allocated in the firmware_handoff repo?
>
> Also, to point out the obvious, there are a bunch of tags in the
> standardized range in this file that don't match the firmware_handoff
> spec. I guess you could add them since 0x100 is still free. However,
> at least the BLOBLISTT_ACPI_TABLES tag would likely(?) be redundant
> with the existing XFERLIST_ACPI_AGGR tag.
>
> > +       BLOBLISTT_U_BOOT_SPL_HANDOFF    = 0xfff000, /* Hand-off info from SPL */
> > +       BLOBLISTT_VBE                   = 0xfff001, /* VBE per-phase state */
> > +       BLOBLISTT_U_BOOT_VIDEO          = 0xfff002, /* Video info from SPL */
>
> FWIW, according to my view of the tag allocation philosophy, I think
> these kinds of tags should be allocated as official tags in the
> standardized range (e.g. in a cluster of U-Boot-specific tags). I
> would really recommend completely avoiding the non-standardized range
> for anything other than local experimentation or tags internal to
> closed-source code.

Ok, but if we do that we need to be careful with the standard.  Things
like BLOBLISTT_U_BOOT_VIDEO dont feel U-Boot specific.

Regards
/Ilias

Re: [PATCH 01/14] bloblist: Update the tag numbering

[Julius Werner]

> Ok, but if we do that we need to be careful with the standard.  Things
> like BLOBLISTT_U_BOOT_VIDEO dont feel U-Boot specific.

The idea behind the Transfer List tag allocation policy is low
friction allocation and organically emerging standards. You're not
supposed to need to have big up-front debates about how exactly e.g. a
video info descriptor should look like or whether it can be shared
with other projects. Projects are just supposed to add what they need
in the moment, in whatever format they prefer, and can call it
something specific to that project to begin with (e.g. U_BOOT_VIDEO).

Then, later, if other projects feel that this is a good format that
would be worthwhile to reuse, they can just start using it. Just
because it says U_BOOT or the ID number is close to the number for
other U-Boot descriptors doesn't mean that it's not appropriate to be
used anywhere else as long as the same structure with the same meaning
makes sense there. If we eventually find that a bunch of different
projects are all using this tag and it has become a de facto standard,
we can also change the name to drop the U_BOOT (or different projects
can even use different names for the same thing, that doesn't really
matter as long as the ID and format matches). Or, if U-Boot eventually
finds that this structure doesn't work well for them anymore they can
just allocate a new U_BOOT_VIDEO2 and stop using the old one (or
rename the old one to U_BOOT_VIDEO_DEPRECATED and call a new ID with a
new format U_BOOT_VIDEO).

Basically, the idea here is that upfront "perfect" design by committee
tends to not work well for these things anyway and the friction it
adds would be too much of a barrier to adoption. So it's better to not
even try, just let everyone allocate what they want, and then later
see what tends to work out well in practice and where there are
opportunities for tag sharing between projects.

RE: [PATCH 01/14] bloblist: Update the tag numbering

[Jose Marinho]

Hi,

> -----Original Message-----
> From: Simon Glass <sjg@chromium.org>
> Sent: Tuesday, July 25, 2023 10:36 PM
> To: U-Boot Mailing List <u-boot@lists.denx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>; Tom Rini
> <trini@konsulko.com>; Julius Werner <jwerner@chromium.org>; Dan Handley
> <Dan.Handley@arm.com>; Jose Marinho <Jose.Marinho@arm.com>; Simon
> Glass <sjg@chromium.org>; Bin Meng <bmeng.cn@gmail.com>; Nikhil M Jain <n-
> jain1@ti.com>
> Subject: [PATCH 01/14] bloblist: Update the tag numbering
> 
> Align this with the new v0.9 spec. It only has a single area for all non-standard
> tags.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/bloblist.c  |  2 +-
>  include/bloblist.h | 37 +++++++++++++------------------------
>  test/bloblist.c    |  2 +-
>  3 files changed, 15 insertions(+), 26 deletions(-)
> 
> diff --git a/common/bloblist.c b/common/bloblist.c index
> 2144b10e1d0..ca3e6efa800 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -36,7 +36,7 @@ static struct tag_name {
>  	enum bloblist_tag_t tag;
>  	const char *name;
>  } tag_name[] = {
> -	{ BLOBLISTT_NONE, "(none)" },
> +	{ BLOBLISTT_VOID, "(void)" },
> 
>  	/* BLOBLISTT_AREA_FIRMWARE_TOP */
> 
> diff --git a/include/bloblist.h b/include/bloblist.h index
> 7ea72c6bd46..bad5fbbb889 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -81,7 +81,7 @@ enum {
> 
>  /* Supported tags - add new ones to tag_name in bloblist.c */  enum
> bloblist_tag_t {
> -	BLOBLISTT_NONE = 0,
> +	BLOBLISTT_VOID = 0,
> 
>  	/*
>  	 * Standard area to allocate blobs used across firmware components, for
> @@ -105,37 +105,26 @@ enum bloblist_tag_t {
>  	BLOBLISTT_VBOOT_CTX = 0x106,	/* Chromium OS verified boot
> context */
> 
>  	/*
> +	 * Tags from here are on reserved for private use within a single
> +	 * firmware binary (i.e. a single executable or phase of a project).
> +	 * These tags can be passed between binaries within a local
> +	 * implementation, but cannot be used in upstream code. Allocate a
> +	 * tag in one of the areas above if you want that.
> +	 *
>  	 * Project-specific tags are permitted here. Projects can be open source
>  	 * or not, but the format of the data must be fuily documented in an

nit: minor typo "fuily" -- line is not being changed in this patch.

>  	 * open source project, including all fields, bits, etc. Naming should
>  	 * be: BLOBLISTT_<project>_<purpose_here>
> -	 */
> -	BLOBLISTT_PROJECT_AREA = 0x8000,
> -	BLOBLISTT_U_BOOT_SPL_HANDOFF = 0x8000, /* Hand-off info from SPL
> */
> -	BLOBLISTT_VBE		= 0x8001,	/* VBE per-phase state */
> -	BLOBLISTT_U_BOOT_VIDEO = 0x8002, /* Video information from SPL */
> -
> -	/*
> -	 * Vendor-specific tags are permitted here. Projects can be open source
> +	 *
> +	 * Vendor-specific tags are also permitted. Projects can be open
> +source
nit: is the newline here intentional?

Jose

>  	 * or not, but the format of the data must be fuily documented in an
>  	 * open source project, including all fields, bits, etc. Naming should
>  	 * be BLOBLISTT_<vendor>_<purpose_here>
>  	 */
> -	BLOBLISTT_VENDOR_AREA = 0xc000,
> -
> -	/* Tags after this are not allocated for now */
> -	BLOBLISTT_EXPANSION = 0x10000,
> -
> -	/*
> -	 * Tags from here are on reserved for private use within a single
> -	 * firmware binary (i.e. a single executable or phase of a project).
> -	 * These tags can be passed between binaries within a local
> -	 * implementation, but cannot be used in upstream code. Allocate a
> -	 * tag in one of the areas above if you want that.
> -	 *
> -	 * This area may move in future.
> -	 */
> -	BLOBLISTT_PRIVATE_AREA = 0xffff0000,
> +	BLOBLISTT_PRIVATE_AREA		= 0xfff000,
> +	BLOBLISTT_U_BOOT_SPL_HANDOFF	= 0xfff000, /* Hand-off info
> from SPL */
> +	BLOBLISTT_VBE			= 0xfff001, /* VBE per-phase state */
> +	BLOBLISTT_U_BOOT_VIDEO		= 0xfff002, /* Video info from
> SPL */
>  };
> 
>  /**
> diff --git a/test/bloblist.c b/test/bloblist.c index 720be7e244f..df9a99d7bd2
> 100644
> --- a/test/bloblist.c
> +++ b/test/bloblist.c
> @@ -291,7 +291,7 @@ static int bloblist_test_cmd_list(struct unit_test_state
> *uts)
>  	console_record_reset();
>  	run_command("bloblist list", 0);
>  	ut_assert_nextline("Address       Size   Tag Name");
> -	ut_assert_nextline("%08lx  %8x  8000 SPL hand-off",
> +	ut_assert_nextline("%08lx  %8x  fff000 SPL hand-off",
>  			   (ulong)map_to_sysmem(data), TEST_SIZE);
>  	ut_assert_nextline("%08lx  %8x   106 Chrome OS vboot context",
>  			   (ulong)map_to_sysmem(data2), TEST_SIZE2);
> --
> 2.41.0.487.g6d72f3e995-goog

RE: [PATCH 12/14] bloblist: Reduce bloblist header size

[Jose Marinho]

> -----Original Message-----
> From: Simon Glass <sjg@chromium.org>
> Sent: Tuesday, July 25, 2023 10:36 PM
> To: U-Boot Mailing List <u-boot@lists.denx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>; Tom Rini
> <trini@konsulko.com>; Julius Werner <jwerner@chromium.org>; Dan Handley
> <Dan.Handley@arm.com>; Jose Marinho <Jose.Marinho@arm.com>; Simon
> Glass <sjg@chromium.org>; Bin Meng <bmeng.cn@gmail.com>; Nikhil M Jain <n-
> jain1@ti.com>
> Subject: [PATCH 12/14] bloblist: Reduce bloblist header size
> 
> The v0.9 spec provides for a 16-byte header with fewer fields. Update the
> implementation to match this.
> 
> This also adds an alignment field.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  include/bloblist.h | 26 +++++++++++++-------------
>  test/bloblist.c    |  6 +++---
>  2 files changed, 16 insertions(+), 16 deletions(-)
> 
> diff --git a/include/bloblist.h b/include/bloblist.h index
> d1e52cf888f..13b619cd019 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -150,30 +150,30 @@ enum bloblist_tag_t {
>   * from the last.
>   *
>   * @magic: BLOBLIST_MAGIC
> + * @chksum: checksum for the entire bloblist allocated area. Since any of the
> + *	blobs can be altered after being created, this checksum is only valid
> + *	when the bloblist is finalised before jumping to the next stage of boot.
> + *	This is the value needed to make all chechksummed bytes sum to 0
>   * @version: BLOBLIST_VERSION
>   * @hdr_size: Size of this header, normally sizeof(struct bloblist_hdr). The
>   *	first bloblist_rec starts at this offset from the start of the header
> - * @size: Total size of the bloblist (non-zero if valid) including this header.
> - *	The bloblist extends for this many bytes from the start of this header.
> - *	When adding new records, the bloblist can grow up to this size.
> + * @align_log2: Power of two of the maximum alignment required by this
> + list
>   * @alloced: Total size allocated so far for this bloblist. This starts out as
>   *	sizeof(bloblist_hdr) since we need at least that much space to store a
>   *	valid bloblist
> - * @chksum: checksum for the entire bloblist allocated area. Since any of the
> - *	blobs can be altered after being created, this checksum is only valid
> - *	when the bloblist is finalised before jumping to the next stage of boot.
> - *	This is the value needed to make all checksummed bytes sum to 0
> + * @size: Total size of the bloblist (non-zero if valid) including this header.
> + *	The bloblist extends for this many bytes from the start of this header.
> + *	When adding new records, the bloblist can grow up to this size.
>   */
>  struct bloblist_hdr {
>  	u32 magic;
> -	u32 version;
> -	u32 hdr_size;
> -	u32 _flags;
> +	u8 chksum;
> +	u8 version;
> +	u8 hdr_size;
> +	u8 align_log2;
> 
> -	u32 size;
>  	u32 alloced;
> -	u32 _spare;
> -	u32 chksum;
> +	u32 size;
>  };

Being (overly) cautious, does it make sense for this struct to be __packed?

> 
>  /**
> diff --git a/test/bloblist.c b/test/bloblist.c index c1719c2e384..5801160621a
> 100644
> --- a/test/bloblist.c
> +++ b/test/bloblist.c
> @@ -78,7 +78,7 @@ static int bloblist_test_init(struct unit_test_state *uts)
>  	ut_asserteq(-EPROTONOSUPPORT, bloblist_check(TEST_ADDR,
>  						     TEST_BLOBLIST_SIZE));
> 
> -	ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0x10));
> +	ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0xc));
>  	ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE));
>  	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE));
> 
> @@ -261,8 +261,8 @@ static int bloblist_test_cmd_info(struct unit_test_state
> *uts)
>  	run_command("bloblist info", 0);
>  	ut_assert_nextline("base:     %lx", (ulong)map_to_sysmem(hdr));
>  	ut_assert_nextline("size:     400    1 KiB");
> -	ut_assert_nextline("alloced:  58     88 Bytes");
> -	ut_assert_nextline("free:     3a8    936 Bytes");
> +	ut_assert_nextline("alloced:  48     72 Bytes");
> +	ut_assert_nextline("free:     3b8    952 Bytes");
>  	ut_assert_console_end();
>  	ut_unsilence_console(uts);
> 
> --
> 2.41.0.487.g6d72f3e995-goog

RE: [PATCH 02/14] bloblist: Adjust API to align in powers of 2

[Jose Marinho]

> -----Original Message-----
> From: Simon Glass <sjg@chromium.org>
> Sent: Tuesday, July 25, 2023 10:36 PM
> To: U-Boot Mailing List <u-boot@lists.denx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>; Tom Rini
> <trini@konsulko.com>; Julius Werner <jwerner@chromium.org>; Dan Handley
> <Dan.Handley@arm.com>; Jose Marinho <Jose.Marinho@arm.com>; Simon
> Glass <sjg@chromium.org>; Bin Meng <bmeng.cn@gmail.com>; Nikhil M Jain <n-
> jain1@ti.com>
> Subject: [PATCH 02/14] bloblist: Adjust API to align in powers of 2
> 
> The updated bloblist structure stores the alignment as a power-of-two value in its
> structures. Adjust the API to use this, to avoid needing to calling ilog2().
> 
> Drop a stale comment while we are here.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  arch/x86/lib/tables.c |  3 ++-
>  common/bloblist.c     | 24 +++++++++++-------------
>  include/bloblist.h    | 12 +++++++-----
>  test/bloblist.c       |  4 ++--
>  4 files changed, 22 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c index
> 67bc0a72aeb..8c437738075 100644
> --- a/arch/x86/lib/tables.c
> +++ b/arch/x86/lib/tables.c
> @@ -16,6 +16,7 @@
>  #include <asm/mpspec.h>
>  #include <asm/tables.h>
>  #include <asm/coreboot_tables.h>
> +#include <linux/log2.h>
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> @@ -101,7 +102,7 @@ int write_tables(void)
>  			if (!gd->arch.table_end)
>  				gd->arch.table_end = rom_addr;
>  			rom_addr = (ulong)bloblist_add(table->tag, size,
> -						       table->align);
> +						       ilog2(table->align));
>  			if (!rom_addr)
>  				return log_msg_ret("bloblist", -ENOBUFS);
> 
> diff --git a/common/bloblist.c b/common/bloblist.c index
> ca3e6efa800..b9332c03ca7 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -26,8 +26,6 @@
>   * start address of the data in each blob is aligned as required. Note that
>   * each blob's *data* is aligned to BLOBLIST_ALIGN regardless of the alignment
>   * of the bloblist itself or the blob header.
> - *
> - * So far, only BLOBLIST_ALIGN alignment is supported.
>   */
> 
>  DECLARE_GLOBAL_DATA_PTR;
> @@ -117,24 +115,24 @@ static struct bloblist_rec *bloblist_findrec(uint tag)
>  	return NULL;
>  }
> 
> -static int bloblist_addrec(uint tag, int size, int align,
> +static int bloblist_addrec(uint tag, int size, int align_log2,
>  			   struct bloblist_rec **recp)
>  {
>  	struct bloblist_hdr *hdr = gd->bloblist;
>  	struct bloblist_rec *rec;
>  	int data_start, new_alloced;
> 
> -	if (!align)
> -		align = BLOBLIST_ALIGN;
> +	if (!align_log2)
> +		align_log2 = BLOBLIST_ALIGN_LOG2;
> 
>  	/* Figure out where the new data will start */
>  	data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*rec);
> 
>  	/* Align the address and then calculate the offset from ->alloced */
> -	data_start = ALIGN(data_start, align) - map_to_sysmem(hdr);
> +	data_start = ALIGN(data_start, 1U << align_log2) - map_to_sysmem(hdr);

nit: Does it make sense to compute "1U << align_log2" once at the top of the function?


> 
>  	/* Calculate the new allocated total */
> -	new_alloced = data_start + ALIGN(size, align);
> +	new_alloced = data_start + ALIGN(size, 1U << align_log2);
> 
>  	if (new_alloced > hdr->size) {
>  		log_err("Failed to allocate %x bytes size=%x, need size=%x\n",
> @@ -158,7 +156,7 @@ static int bloblist_addrec(uint tag, int size, int align,  }
> 
>  static int bloblist_ensurerec(uint tag, struct bloblist_rec **recp, int size,
> -			      int align)
> +			      int align_log2)
>  {
>  	struct bloblist_rec *rec;
> 
> @@ -171,7 +169,7 @@ static int bloblist_ensurerec(uint tag, struct bloblist_rec
> **recp, int size,
>  	} else {
>  		int ret;
> 
> -		ret = bloblist_addrec(tag, size, align, &rec);
> +		ret = bloblist_addrec(tag, size, align_log2, &rec);
>  		if (ret)
>  			return ret;
>  	}
> @@ -193,22 +191,22 @@ void *bloblist_find(uint tag, int size)
>  	return (void *)rec + rec->hdr_size;
>  }
> 
> -void *bloblist_add(uint tag, int size, int align)
> +void *bloblist_add(uint tag, int size, int align_log2)
>  {
>  	struct bloblist_rec *rec;
> 
> -	if (bloblist_addrec(tag, size, align, &rec))
> +	if (bloblist_addrec(tag, size, align_log2, &rec))
>  		return NULL;
> 
>  	return (void *)rec + rec->hdr_size;
>  }
> 
> -int bloblist_ensure_size(uint tag, int size, int align, void **blobp)
> +int bloblist_ensure_size(uint tag, int size, int align_log2, void
> +**blobp)
>  {
>  	struct bloblist_rec *rec;
>  	int ret;
> 
> -	ret = bloblist_ensurerec(tag, &rec, size, align);
> +	ret = bloblist_ensurerec(tag, &rec, size, align_log2);
>  	if (ret)
>  		return ret;
>  	*blobp = (void *)rec + rec->hdr_size;
> diff --git a/include/bloblist.h b/include/bloblist.h index
> bad5fbbb889..e6ce98334d3 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -76,7 +76,9 @@
>  enum {
>  	BLOBLIST_VERSION	= 0,
>  	BLOBLIST_MAGIC		= 0xb00757a3,
> -	BLOBLIST_ALIGN		= 16,
> +
> +	BLOBLIST_ALIGN_LOG2	= 4,
> +	BLOBLIST_ALIGN		= 1 << BLOBLIST_ALIGN_LOG2,
>  };
> 
>  /* Supported tags - add new ones to tag_name in bloblist.c */ @@ -238,11
> +240,11 @@ void *bloblist_find(uint tag, int size);
>   *
>   * @tag:	Tag to add (enum bloblist_tag_t)
>   * @size:	Size of the blob
> - * @align:	Alignment of the blob (in bytes), 0 for default
> + * @align_log2:	Alignment of the blob (in bytes log2), 0 for default
>   * Return: pointer to the newly added block, or NULL if there is not enough
>   * space for the blob
>   */
> -void *bloblist_add(uint tag, int size, int align);
> +void *bloblist_add(uint tag, int size, int align_log2);
> 
>  /**
>   * bloblist_ensure_size() - Find or add a blob @@ -252,11 +254,11 @@ void
> *bloblist_add(uint tag, int size, int align);
>   * @tag:	Tag to add (enum bloblist_tag_t)
>   * @size:	Size of the blob
>   * @blobp:	Returns a pointer to blob on success
> - * @align:	Alignment of the blob (in bytes), 0 for default
> + * @align_log2:	Alignment of the blob (in bytes log2), 0 for default
>   * Return: 0 if OK, -ENOSPC if it is missing and could not be added due to lack
>   * of space, or -ESPIPE it exists but has the wrong size
>   */
> -int bloblist_ensure_size(uint tag, int size, int align, void **blobp);
> +int bloblist_ensure_size(uint tag, int size, int align_log2, void
> +**blobp);
> 
>  /**
>   * bloblist_ensure() - Find or add a blob diff --git a/test/bloblist.c b/test/bloblist.c
> index df9a99d7bd2..3d988fe05ae 100644
> --- a/test/bloblist.c
> +++ b/test/bloblist.c
> @@ -336,7 +336,7 @@ static int bloblist_test_align(struct unit_test_state *uts)
> 
>  	/* Check larger alignment */
>  	for (i = 0; i < 3; i++) {
> -		int align = 32 << i;
> +		int align = 5 - i;
> 
>  		data = bloblist_add(3 + i, i * 4, align);
>  		ut_assertnonnull(data);
> @@ -351,7 +351,7 @@ static int bloblist_test_align(struct unit_test_state *uts)
>  	ut_assertok(bloblist_new(TEST_ADDR + BLOBLIST_ALIGN,
> TEST_BLOBLIST_SIZE,
>  				 0));
> 
> -	data = bloblist_add(1, 5, BLOBLIST_ALIGN * 2);
> +	data = bloblist_add(1, 5, BLOBLIST_ALIGN_LOG2 + 1);
>  	ut_assertnonnull(data);
>  	addr = map_to_sysmem(data);
>  	ut_asserteq(0, addr & (BLOBLIST_ALIGN * 2 - 1));
> --
> 2.41.0.487.g6d72f3e995-goog

Re: [PATCH 12/14] bloblist: Reduce bloblist header size

[Simon Glass]

Hi Jose,

On Wed, 2 Aug 2023 at 04:15, Jose Marinho <Jose.Marinho@arm.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Simon Glass <sjg@chromium.org>
> > Sent: Tuesday, July 25, 2023 10:36 PM
> > To: U-Boot Mailing List <u-boot@lists.denx.de>
> > Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>; Tom Rini
> > <trini@konsulko.com>; Julius Werner <jwerner@chromium.org>; Dan Handley
> > <Dan.Handley@arm.com>; Jose Marinho <Jose.Marinho@arm.com>; Simon
> > Glass <sjg@chromium.org>; Bin Meng <bmeng.cn@gmail.com>; Nikhil M Jain <n-
> > jain1@ti.com>
> > Subject: [PATCH 12/14] bloblist: Reduce bloblist header size
> >
> > The v0.9 spec provides for a 16-byte header with fewer fields. Update the
> > implementation to match this.
> >
> > This also adds an alignment field.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  include/bloblist.h | 26 +++++++++++++-------------
> >  test/bloblist.c    |  6 +++---
> >  2 files changed, 16 insertions(+), 16 deletions(-)
> >
> > diff --git a/include/bloblist.h b/include/bloblist.h index
> > d1e52cf888f..13b619cd019 100644
> > --- a/include/bloblist.h
> > +++ b/include/bloblist.h
> > @@ -150,30 +150,30 @@ enum bloblist_tag_t {
> >   * from the last.
> >   *
> >   * @magic: BLOBLIST_MAGIC
> > + * @chksum: checksum for the entire bloblist allocated area. Since any of the
> > + *   blobs can be altered after being created, this checksum is only valid
> > + *   when the bloblist is finalised before jumping to the next stage of boot.
> > + *   This is the value needed to make all chechksummed bytes sum to 0
> >   * @version: BLOBLIST_VERSION
> >   * @hdr_size: Size of this header, normally sizeof(struct bloblist_hdr). The
> >   *   first bloblist_rec starts at this offset from the start of the header
> > - * @size: Total size of the bloblist (non-zero if valid) including this header.
> > - *   The bloblist extends for this many bytes from the start of this header.
> > - *   When adding new records, the bloblist can grow up to this size.
> > + * @align_log2: Power of two of the maximum alignment required by this
> > + list
> >   * @alloced: Total size allocated so far for this bloblist. This starts out as
> >   *   sizeof(bloblist_hdr) since we need at least that much space to store a
> >   *   valid bloblist
> > - * @chksum: checksum for the entire bloblist allocated area. Since any of the
> > - *   blobs can be altered after being created, this checksum is only valid
> > - *   when the bloblist is finalised before jumping to the next stage of boot.
> > - *   This is the value needed to make all checksummed bytes sum to 0
> > + * @size: Total size of the bloblist (non-zero if valid) including this header.
> > + *   The bloblist extends for this many bytes from the start of this header.
> > + *   When adding new records, the bloblist can grow up to this size.
> >   */
> >  struct bloblist_hdr {
> >       u32 magic;
> > -     u32 version;
> > -     u32 hdr_size;
> > -     u32 _flags;
> > +     u8 chksum;
> > +     u8 version;
> > +     u8 hdr_size;
> > +     u8 align_log2;
> >
> > -     u32 size;
> >       u32 alloced;
> > -     u32 _spare;
> > -     u32 chksum;
> > +     u32 size;
> >  };
>
> Being (overly) cautious, does it make sense for this struct to be __packed?

I prefer not, since we require it to be aligned and don't want the
compiler adding unnecessary access code.

Regards,
Simon

Re: [PATCH 00/14] bloblist: Align to firmware handoff

[Michal Simek]

Hi Simon,

út 25. 7. 2023 v 23:36 odesílatel Simon Glass <sjg@chromium.org> napsal:
>
> In moving from v0.8 to v0.9 the Firmware Handoff specification made some
> changes, including:
>
>    - Use a packed format for headers to reduce space
>    - Add an explicit alignment field in the header
>    - Renumber all the tags and reduce their size to 24 bits
>    - Drop use of the blob header to specify alignment, in favour of a
>      'void' blob type
>
> This series attempts to align to that specification, including updating
> the API and tests. It is likely that refinements will be made as other
> projects implement the spec too.
>
> As before the code is dual-licensed, to permit use in projects with a
> permissive license.
>
>
> Simon Glass (14):
>   bloblist: Update the tag numbering
>   bloblist: Adjust API to align in powers of 2
>   bloblist: Change the magic value
>   bloblist: Set version to 1
>   bloblist: Access record hdr_size and tag via a function
>   bloblist: Drop the flags value
>   bloblist: Drop the spare values
>   bloblist: Change the checksum algorithm
>   bloblist: Checksum the entire bloblist
>   bloblist: Handle alignment with a void entry
>   bloblist: Reduce blob-header size
>   bloblist: Reduce bloblist header size
>   bloblist: Add alignment to bloblist_new()
>   bloblist: Update documentation and header comment
>
>  arch/x86/lib/tables.c    |   3 +-
>  common/bloblist.c        | 102 ++++++++++++++++++++-------------
>  doc/develop/bloblist.rst |   4 +-
>  include/bloblist.h       | 121 +++++++++++++++++++--------------------
>  test/bloblist.c          |  53 +++++++++--------
>  5 files changed, 152 insertions(+), 131 deletions(-)
>
> --
> 2.41.0.487.g6d72f3e995-goog
>

Would it be also possible to align names in the bloblist_hdr structure?
magic->signature
align_log2->alignment
alloced->size
size->max_size

The same is for bloblist_rec.

I don't know the history but spec is about transfer list and transfer entry.
Do you plan to rename it to avoid confusion?

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs

Re: [PATCH 00/14] bloblist: Align to firmware handoff

[Simon Glass]

Hi Michal,

On Wed, 6 Sept 2023 at 12:22, Michal Simek <monstr@monstr.eu> wrote:
>
> Hi Simon,
>
> út 25. 7. 2023 v 23:36 odesílatel Simon Glass <sjg@chromium.org> napsal:
> >
> > In moving from v0.8 to v0.9 the Firmware Handoff specification made some
> > changes, including:
> >
> >    - Use a packed format for headers to reduce space
> >    - Add an explicit alignment field in the header
> >    - Renumber all the tags and reduce their size to 24 bits
> >    - Drop use of the blob header to specify alignment, in favour of a
> >      'void' blob type
> >
> > This series attempts to align to that specification, including updating
> > the API and tests. It is likely that refinements will be made as other
> > projects implement the spec too.
> >
> > As before the code is dual-licensed, to permit use in projects with a
> > permissive license.
> >
> >
> > Simon Glass (14):
> >   bloblist: Update the tag numbering
> >   bloblist: Adjust API to align in powers of 2
> >   bloblist: Change the magic value
> >   bloblist: Set version to 1
> >   bloblist: Access record hdr_size and tag via a function
> >   bloblist: Drop the flags value
> >   bloblist: Drop the spare values
> >   bloblist: Change the checksum algorithm
> >   bloblist: Checksum the entire bloblist
> >   bloblist: Handle alignment with a void entry
> >   bloblist: Reduce blob-header size
> >   bloblist: Reduce bloblist header size
> >   bloblist: Add alignment to bloblist_new()
> >   bloblist: Update documentation and header comment
> >
> >  arch/x86/lib/tables.c    |   3 +-
> >  common/bloblist.c        | 102 ++++++++++++++++++++-------------
> >  doc/develop/bloblist.rst |   4 +-
> >  include/bloblist.h       | 121 +++++++++++++++++++--------------------
> >  test/bloblist.c          |  53 +++++++++--------
> >  5 files changed, 152 insertions(+), 131 deletions(-)
> >
> > --
> > 2.41.0.487.g6d72f3e995-goog
> >
>
> Would it be also possible to align names in the bloblist_hdr structure?
> magic->signature
> align_log2->alignment
> alloced->size
> size->max_size
>
> The same is for bloblist_rec.

OK. I am not sure I like the size/max_size thing so I filed an issue for that.

>
> I don't know the history but spec is about transfer list and transfer entry.
> Do you plan to rename it to avoid confusion?

I don't really like transfer_list_xxx as an API. It is too
long-winded. We used that name since it is unique and descriptive as
to its purpose. But I think that 'bloblist' is a better name in the
code base. Perhaps we could use xferlist instead of bloblist?

Regards,
SImon


>
> Thanks,
> Michal
>
> --
> Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
> w: www.monstr.eu p: +42-0-721842854
> Maintainer of Linux kernel - Xilinx Microblaze
> Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
> U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs


REgards,
Simon

Re: [PATCH 00/14] bloblist: Align to firmware handoff

[Michal Simek]

On 10/28/23 07:35, Simon Glass wrote:
> Hi Michal,
> 
> On Wed, 6 Sept 2023 at 12:22, Michal Simek <monstr@monstr.eu> wrote:
>>
>> Hi Simon,
>>
>> út 25. 7. 2023 v 23:36 odesílatel Simon Glass <sjg@chromium.org> napsal:
>>>
>>> In moving from v0.8 to v0.9 the Firmware Handoff specification made some
>>> changes, including:
>>>
>>>     - Use a packed format for headers to reduce space
>>>     - Add an explicit alignment field in the header
>>>     - Renumber all the tags and reduce their size to 24 bits
>>>     - Drop use of the blob header to specify alignment, in favour of a
>>>       'void' blob type
>>>
>>> This series attempts to align to that specification, including updating
>>> the API and tests. It is likely that refinements will be made as other
>>> projects implement the spec too.
>>>
>>> As before the code is dual-licensed, to permit use in projects with a
>>> permissive license.
>>>
>>>
>>> Simon Glass (14):
>>>    bloblist: Update the tag numbering
>>>    bloblist: Adjust API to align in powers of 2
>>>    bloblist: Change the magic value
>>>    bloblist: Set version to 1
>>>    bloblist: Access record hdr_size and tag via a function
>>>    bloblist: Drop the flags value
>>>    bloblist: Drop the spare values
>>>    bloblist: Change the checksum algorithm
>>>    bloblist: Checksum the entire bloblist
>>>    bloblist: Handle alignment with a void entry
>>>    bloblist: Reduce blob-header size
>>>    bloblist: Reduce bloblist header size
>>>    bloblist: Add alignment to bloblist_new()
>>>    bloblist: Update documentation and header comment
>>>
>>>   arch/x86/lib/tables.c    |   3 +-
>>>   common/bloblist.c        | 102 ++++++++++++++++++++-------------
>>>   doc/develop/bloblist.rst |   4 +-
>>>   include/bloblist.h       | 121 +++++++++++++++++++--------------------
>>>   test/bloblist.c          |  53 +++++++++--------
>>>   5 files changed, 152 insertions(+), 131 deletions(-)
>>>
>>> --
>>> 2.41.0.487.g6d72f3e995-goog
>>>
>>
>> Would it be also possible to align names in the bloblist_hdr structure?
>> magic->signature
>> align_log2->alignment
>> alloced->size
>> size->max_size
>>
>> The same is for bloblist_rec.
> 
> OK. I am not sure I like the size/max_size thing so I filed an issue for that.

Fine for me. At the end they should match with spec. Update in spec is fine.

>>
>> I don't know the history but spec is about transfer list and transfer entry.
>> Do you plan to rename it to avoid confusion?
> 
> I don't really like transfer_list_xxx as an API. It is too
> long-winded. We used that name since it is unique and descriptive as
> to its purpose. But I think that 'bloblist' is a better name in the
> code base. Perhaps we could use xferlist instead of bloblist?

xferlist sounds good to me.

M