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

[Raymond Mao]

Hi Ilias,

On Thu, 28 Dec 2023 at 02:37, Ilias Apalodimas <ilias.apalodimas at 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