Re: [PATCH v4 10/12] bloblist: Adjust the bloblist header

Hi Ilias,

On Thu, 28 Dec 2023 at 02:37, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:

Hi Raymond,

[...]

...

void bloblist_show_list(void) @@ -463,7 +477,7 @@ void bloblist_reloc(void *to, uint to_size, void

*from, uint from_size)

...

    memcpy(to, from, from_size);
    hdr = to;

•   hdr->size = to_size;
  

•   hdr->total_size = to_size;
  

}

int bloblist_init(void) @@ -493,7 +507,7 @@ int bloblist_init(void) addr, ret); } else { /* Get the real size, if it is not what we

expected */

...

•                   size = gd->bloblist->size;
  

•                   size = gd->bloblist->total_size;
            }
    }
    if (ret) {
  

diff --git a/include/bloblist.h b/include/bloblist.h index 7024d7bf9e..4ec4b3d449 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -166,32 +166,33 @@ 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 finalized before jumping to the next stage
    

of boot.

...

• • This is the value needed to make all checksummed 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

...

• • @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.
    

• • @alloced: Total size allocated so far for this bloblist. This starts

out as

...

• • @align_log2: Power of two of the maximum alignment required by this

list

...

• • @used_size: 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
  

• • @total_size: The number of total bytes that the bloblist can occupy.

• • Any blob producer must check if there is sufficient space before
    

adding

...

• • a record to the bloblist.
    

• • @flags: Space for BLOBLISTF... flags (none yet)
  • @spare: Spare space (for future use)

• • @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
    

  */

struct bloblist_hdr { u32 magic;

•   u32 version;
  

•   u32 hdr_size;
  

•   u8 chksum;
  

•   u8 version;
  

•   u8 hdr_size;
  

•   u8 align_log2;
  

•   u32 used_size;
  

•   u32 total_size;
    u32 flags;
  

•   u32 size;
  

•   u32 alloced;
    u32 spare;
  

•   u32 chksum;
  

};

The patch generally looks ok, but while we are renaming things, can't we just copy what the spec says instead of slightly changing the names?

With this applied we end up with struct bloblist_hdr { u32 magic; u8 chksum; u8 version; u8 hdr_size; u8 align_log2; u32 used_size; u32 total_size; u32 flags; u32 spare; };

Can you at least rename the ones you touch here properly?

• Drop the patch that renames spare to _spare
• used_size -> size
• total_size -> max_size (which btw mean different things)
• spare -> reserved

I was been told by Simon for the naming changes of `used_size` and

`total_size` at: https://github.com/FirmwareHandoff/firmware_handoff/pull/25 I will prefer to keep this minor fix until spec v1.0 is released since the spec is still continuously being updated without upgrading the revision number. When a v1.0 spec is released I will update the TF-A, OP-TEE and U-Boot together.

[...]

Thanks and regards, Raymond