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

Hi Ilias,

On Thu, Dec 28, 2023 at 7:37 AM 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

Yes that would be good, but since the spec only recently changed the names, how about a 'rename' patch on top, if that is easier?

I was hoping that this series could be applied as is, or close to it. It looks like a good base to me.

Regards, Simon