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>

diff --git a/common/bloblist.c b/common/bloblist.c
index c86ea029c8d77c3a93c5dea4a409c1dd0fc053be..173f28d8ec9ec9339bc7e537ab3b4ed2ff1f538e 100644 (file)
--- 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 028b0a9383c519d7b54a86be479d79500314c700..68535237641a5affdd589c228aeacd006cf5acae 100644 (file)
--- 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[])
 {