bloblist: Tidy up the data alignment

The intention which bloblists is that each blob's data is aligned in
memory. At present it is only the headers that are aligned.

Update the code to correct this and add a little more documentation.

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

2 files changed, 63 insertions, 7 deletions

diff --git a/common/bloblist.c b/common/bloblist.c
index c86ea029c8..173f28d8ec 100644
--- a/common/bloblist.c
+++ b/common/bloblist.c
@@ -11,6 +11,20 @@
 #include <spl.h>
 #include <u-boot/crc.h>
 
+/*
+ * A bloblist is a single contiguous chunk of memory with a header
+ * (struct bloblist_hdr) and a number of blobs in it.
+ *
+ * Each blob starts on a BLOBLIST_ALIGN boundary relative to the start of the
+ * bloblist and consists of a struct bloblist_rec, some padding to the required
+ * alignment for the blog and then the actual data. The padding ensures that the
+ * 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;
 
 static const char *const tag_name[] = {
@@ -73,9 +87,14 @@ static int bloblist_addrec(uint tag, int size, struct bloblist_rec **recp)
 {
 	struct bloblist_hdr *hdr = gd->bloblist;
 	struct bloblist_rec *rec;
-	int new_alloced;
+	int data_start, new_alloced;
+
+	/* Figure out where the new data will start */
+	data_start = hdr->alloced + sizeof(*rec);
+	data_start = ALIGN(data_start, BLOBLIST_ALIGN);
 
-	new_alloced = hdr->alloced + sizeof(*rec) + ALIGN(size, BLOBLIST_ALIGN);
+	/* Calculate the new allocated total */
+	new_alloced = data_start + ALIGN(size, BLOBLIST_ALIGN);
 	if (new_alloced >= hdr->size) {
 		log(LOGC_BLOBLIST, LOGL_ERR,
 		    "Failed to allocate %x bytes size=%x, need size=%x\n",
@@ -83,15 +102,16 @@ static int bloblist_addrec(uint tag, int size, struct bloblist_rec **recp)
 		return log_msg_ret("bloblist add", -ENOSPC);
 	}
 	rec = (void *)hdr + hdr->alloced;
-	hdr->alloced = new_alloced;
 
 	rec->tag = tag;
-	rec->hdr_size = sizeof(*rec);
+	rec->hdr_size = data_start - hdr->alloced;
 	rec->size = size;
 	rec->spare = 0;
 
 	/* Zero the record data */
-	memset(rec + 1, '\0', rec->size);
+	memset((void *)rec + rec->hdr_size, '\0', rec->size);
+
+	hdr->alloced = new_alloced;
 	*recp = rec;
 
 	return 0;
@@ -139,7 +159,7 @@ void *bloblist_add(uint tag, int size)
 	if (bloblist_addrec(tag, size, &rec))
 		return NULL;
 
-	return rec + 1;
+	return (void *)rec + rec->hdr_size;
 }
 
 int bloblist_ensure_size(uint tag, int size, void **blobp)
diff --git a/test/bloblist.c b/test/bloblist.c
index 028b0a9383..6853523764 100644
--- a/test/bloblist.c
+++ b/test/bloblist.c
@@ -29,6 +29,8 @@ enum {
 
 	TEST_ADDR		= CONFIG_BLOBLIST_ADDR,
 	TEST_BLOBLIST_SIZE	= 0x100,
+
+	ERASE_BYTE		= '\xff',
 };
 
 static struct bloblist_hdr *clear_bloblist(void)
@@ -41,7 +43,7 @@ static struct bloblist_hdr *clear_bloblist(void)
 	 * to 0xff for testing purposes.
 	 */
 	hdr = map_sysmem(CONFIG_BLOBLIST_ADDR, TEST_BLOBLIST_SIZE);
-	memset(hdr, '\xff', TEST_BLOBLIST_SIZE);
+	memset(hdr, ERASE_BYTE, TEST_BLOBLIST_SIZE);
 	memset(hdr, '\0', sizeof(*hdr));
 
 	return hdr;
@@ -292,6 +294,40 @@ static int bloblist_test_cmd_list(struct unit_test_state *uts)
 }
 BLOBLIST_TEST(bloblist_test_cmd_list, 0);
 
+/* Test alignment of bloblist blobs */
+static int bloblist_test_align(struct unit_test_state *uts)
+{
+	struct bloblist_hdr *hdr;
+	int i;
+
+	/* At the start there should be no records */
+	hdr = clear_bloblist();
+	ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
+	ut_assertnull(bloblist_find(TEST_TAG, TEST_BLOBLIST_SIZE));
+
+	/* Check the alignment */
+	for (i = 0; i < 3; i++) {
+		int size = i * 3;
+		ulong addr;
+		char *data;
+		int j;
+
+		data = bloblist_add(i, size);
+		ut_assertnonnull(data);
+		addr = map_to_sysmem(data);
+		ut_asserteq(0, addr & (BLOBLIST_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++)
+			ut_asserteq(ERASE_BYTE, data[j]);
+	}
+
+	return 0;
+}
+BLOBLIST_TEST(bloblist_test_align, 0);
+
 int do_ut_bloblist(struct cmd_tbl *cmdtp, int flag, int argc,
 		   char *const argv[])
 {