Re: [PATCH v3 11/14] bloblist: Adjust the bloblist header
On Wed, 27 Dec 2023 at 17:09, Raymond Mao wrote:
>
> Hi Ilias,
>
> On Wed, 27 Dec 2023 at 09:58, Ilias Apalodimas
> wrote:
>>
>> On Wed, 27 Dec 2023 at 16:50, Raymond Mao wrote:
>> >
>> > Hi Ilias,
>> >
>> > On Wed, 27 Dec 2023 at 05:17, Ilias Apalodimas
>> > wrote:
>> >>
>> >>
>> >> [...]
>> >>
>> >> > - * @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;
>> >> > - u32 flags;
>> >> > -
>> >> > - u32 size;
>> >> > + u8 chksum;
>> >> > + u8 version;
>> >> > + u8 hdr_size;
>> >> > + u8 align_log2;
>> >> > u32 alloced;
>> >> > + u32 size;
>> >> > + u32 flags;
>> >> > u32 spare;
>> >> > - u32 chksum;
>> >> > };
>> >> >
>> >>
>> >> Aren't fields still missing from the current version?
>> >> e.g max_size and reserved?
>> >>
>> > They are all in. Please see:
>> > [PATCH v3 14/14] bloblist: Align bloblist used_size and total_size to spec
>>
>> Ok. But why aren't we squashing this to #14 then?
>>
> I can squash other changes with #14 except the ones for `align_log2`.
> And `align_log2` with patch #12 due to the dependency.
I think that's better.
Thanks
/Ilias
>
> Regards,
> Raymond
>
>>
>> Thanks
>> /Ilias
>> >
>> > [...]
>> >
>> > Regards,
>> > Raymond
Re: [PATCH v3 11/14] bloblist: Adjust the bloblist header
Hi Ilias,
On Wed, 27 Dec 2023 at 09:58, Ilias Apalodimas
wrote:
> On Wed, 27 Dec 2023 at 16:50, Raymond Mao wrote:
> >
> > Hi Ilias,
> >
> > On Wed, 27 Dec 2023 at 05:17, Ilias Apalodimas <
> ilias.apalodi...@linaro.org> wrote:
> >>
> >>
> >> [...]
> >>
> >> > - * @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;
> >> > - u32 flags;
> >> > -
> >> > - u32 size;
> >> > + u8 chksum;
> >> > + u8 version;
> >> > + u8 hdr_size;
> >> > + u8 align_log2;
> >> > u32 alloced;
> >> > + u32 size;
> >> > + u32 flags;
> >> > u32 spare;
> >> > - u32 chksum;
> >> > };
> >> >
> >>
> >> Aren't fields still missing from the current version?
> >> e.g max_size and reserved?
> >>
> > They are all in. Please see:
> > [PATCH v3 14/14] bloblist: Align bloblist used_size and total_size to
> spec
>
> Ok. But why aren't we squashing this to #14 then?
>
> I can squash other changes with #14 except the ones for `align_log2`.
And `align_log2` with patch #12 due to the dependency.
Regards,
Raymond
> Thanks
> /Ilias
> >
> > [...]
> >
> > Regards,
> > Raymond
>
Re: [PATCH v3 11/14] bloblist: Adjust the bloblist header
On Wed, 27 Dec 2023 at 16:50, Raymond Mao wrote:
>
> Hi Ilias,
>
> On Wed, 27 Dec 2023 at 05:17, Ilias Apalodimas
> wrote:
>>
>>
>> [...]
>>
>> > - * @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;
>> > - u32 flags;
>> > -
>> > - u32 size;
>> > + u8 chksum;
>> > + u8 version;
>> > + u8 hdr_size;
>> > + u8 align_log2;
>> > u32 alloced;
>> > + u32 size;
>> > + u32 flags;
>> > u32 spare;
>> > - u32 chksum;
>> > };
>> >
>>
>> Aren't fields still missing from the current version?
>> e.g max_size and reserved?
>>
> They are all in. Please see:
> [PATCH v3 14/14] bloblist: Align bloblist used_size and total_size to spec
Ok. But why aren't we squashing this to #14 then?
Thanks
/Ilias
>
> [...]
>
> Regards,
> Raymond
Re: [PATCH v3 11/14] bloblist: Adjust the bloblist header
Hi Ilias,
On Wed, 27 Dec 2023 at 05:17, Ilias Apalodimas
wrote:
>
> [...]
>
> > - * @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;
> > - u32 flags;
> > -
> > - u32 size;
> > + u8 chksum;
> > + u8 version;
> > + u8 hdr_size;
> > + u8 align_log2;
> > u32 alloced;
> > + u32 size;
> > + u32 flags;
> > u32 spare;
> > - u32 chksum;
> > };
> >
>
> Aren't fields still missing from the current version?
> e.g max_size and reserved?
>
> They are all in. Please see:
[PATCH v3 14/14] bloblist: Align bloblist used_size and total_size to spec
[...]
Regards,
Raymond
Re: [PATCH v3 11/14] bloblist: Adjust the bloblist header
[...]
> - * @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;
> - u32 flags;
> -
> - u32 size;
> + u8 chksum;
> + u8 version;
> + u8 hdr_size;
> + u8 align_log2;
> u32 alloced;
> + u32 size;
> + u32 flags;
> u32 spare;
> - u32 chksum;
> };
>
Aren't fields still missing from the current version?
e.g max_size and reserved?
> /**
> diff --git a/test/bloblist.c b/test/bloblist.c
> index e6070041d3..a083259d0c 100644
> --- a/test/bloblist.c
> +++ b/test/bloblist.c
> @@ -78,7 +78,7 @@ static int bloblist_test_init(struct unit_test_state *uts)
> ut_asserteq(-EPROTONOSUPPORT, bloblist_check(TEST_ADDR,
>TEST_BLOBLIST_SIZE));
>
> - ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0x10, 0));
> + ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0xc, 0));
> ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE, 0));
> ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
>
> @@ -272,8 +272,8 @@ static int bloblist_test_cmd_info(struct unit_test_state
> *uts)
> run_command("bloblist info", 0);
> ut_assert_nextline("base: %lx", (ulong)map_to_sysmem(hdr));
> ut_assert_nextline("size: 4001 KiB");
> - ut_assert_nextline("alloced: 58 88 Bytes");
> - ut_assert_nextline("free: 3a8936 Bytes");
> + ut_assert_nextline("alloced: 50 80 Bytes");
> + ut_assert_nextline("free: 3b0944 Bytes");
> ut_assert_console_end();
> ut_unsilence_console(uts);
>
> --
> 2.25.1
>
[PATCH v3 11/14] bloblist: Adjust the bloblist header
From: Simon Glass
The v0.9 spec provides for a 24-byte header. Update
the implementation to match this.
This also adds an alignment field.
Signed-off-by: Simon Glass
Co-developed-by: Raymond Mao
Signed-off-by: Raymond Mao
---
Changes in v3
- Update the bloblist header to align to FW handoff spec up to commit 3592349.
- Update the related testcases.
include/bloblist.h | 29 +++--
test/bloblist.c| 6 +++---
2 files changed, 18 insertions(+), 17 deletions(-)
diff --git a/include/bloblist.h b/include/bloblist.h
index 7024d7bf9e..5dba630adb 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.
+ * @align_log2: Power of two of the maximum alignment required by this list
* @alloced: Total 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
+ * @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.
+ * @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;
- u32 flags;
-
- u32 size;
+ u8 chksum;
+ u8 version;
+ u8 hdr_size;
+ u8 align_log2;
u32 alloced;
+ u32 size;
+ u32 flags;
u32 spare;
- u32 chksum;
};
/**
diff --git a/test/bloblist.c b/test/bloblist.c
index e6070041d3..a083259d0c 100644
--- a/test/bloblist.c
+++ b/test/bloblist.c
@@ -78,7 +78,7 @@ static int bloblist_test_init(struct unit_test_state *uts)
ut_asserteq(-EPROTONOSUPPORT, bloblist_check(TEST_ADDR,
TEST_BLOBLIST_SIZE));
- ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0x10, 0));
+ ut_asserteq(-ENOSPC, bloblist_new(TEST_ADDR, 0xc, 0));
ut_asserteq(-EFAULT, bloblist_new(1, TEST_BLOBLIST_SIZE, 0));
ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0));
@@ -272,8 +272,8 @@ static int bloblist_test_cmd_info(struct unit_test_state
*uts)
run_command("bloblist info", 0);
ut_assert_nextline("base: %lx", (ulong)map_to_sysmem(hdr));
ut_assert_nextline("size: 4001 KiB");
- ut_assert_nextline("alloced: 58 88 Bytes");
- ut_assert_nextline("free: 3a8936 Bytes");
+ ut_assert_nextline("alloced: 50 80 Bytes");
+ ut_assert_nextline("free: 3b0944 Bytes");
ut_assert_console_end();
ut_unsilence_console(uts);
--
2.25.1
