[PATCH v4 1/3] bloblist: add api to get blob with size
Raymond Mao
raymond.mao at linaro.org
Mon Jan 13 19:23:07 CET 2025
Hi Heinrich,
On Mon, 13 Jan 2025 at 12:50, Heinrich Schuchardt <xypron.glpk at gmx.de>
wrote:
> On 13.01.25 15:33, Raymond Mao wrote:
> > Hi Heinrich,
> >
> > On Fri, 10 Jan 2025 at 19:04, Heinrich Schuchardt <xypron.glpk at gmx.de
> > <mailto:xypron.glpk at gmx.de>> wrote:
> >
> > Am 10. Januar 2025 22:56:33 MEZ schrieb Raymond Mao
> > <raymond.mao at linaro.org <mailto:raymond.mao at linaro.org>>:
> > >bloblist_find function only returns the pointer of blob data,
> > >which is fine for those self-describing data like FDT.
> > >But as a common scenario, an interface is needed to retrieve both
> > >the pointer and the size of the blob data.
> > >
> > >Add a few ut test cases for the new api.
> > >
> > >Signed-off-by: Raymond Mao <raymond.mao at linaro.org
> > <mailto:raymond.mao at linaro.org>>
> > >Reviewed-by: Simon Glass <sjg at chromium.org <mailto:
> sjg at chromium.org>>
> > >Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org
> > <mailto:ilias.apalodimas at linaro.org>>
> > >---
> > >Changes in v2
> > >- Rename function argument.
> > >- Add ut tests.
> > >Changes in v3
> > >- None.
> > >Changes in v4
> > >- None.
> > >
> > > common/bloblist.c | 17 +++++++++++++++--
> > > include/bloblist.h | 18 ++++++++++++++++++
> > > test/common/bloblist.c | 4 ++++
> > > 3 files changed, 37 insertions(+), 2 deletions(-)
> > >
> > >diff --git a/common/bloblist.c b/common/bloblist.c
> > >index 110bb9dc44..ab48a3cdb9 100644
> > >--- a/common/bloblist.c
> > >+++ b/common/bloblist.c
> > >@@ -222,14 +222,27 @@ static int bloblist_ensurerec(uint tag,
> > struct bloblist_rec **recp, int size,
> > > }
> > >
> > > void *bloblist_find(uint tag, int size)
> > >+{
> > >+ void *blob = NULL;
> > >+ int blob_size;
> > >+
> > >+ blob = bloblist_get_blob(tag, &blob_size);
> > >+
> > >+ if (size && size != blob_size)
> > >+ return NULL;
> > >+
> > >+ return blob;
> > >+}
> > >+
> > >+void *bloblist_get_blob(uint tag, int *sizep)
> > > {
> > > struct bloblist_rec *rec;
> > >
> > > rec = bloblist_findrec(tag);
> > > if (!rec)
> > > return NULL;
> > >- if (size && size != rec->size)
> > >- return NULL;
> > >+
> > >+ *sizep = rec->size;
> > >
> > > return (void *)rec + rec_hdr_size(rec);
> > > }
> > >diff --git a/include/bloblist.h b/include/bloblist.h
> > >index f999391f74..52ba0ddcf8 100644
> > >--- a/include/bloblist.h
> > >+++ b/include/bloblist.h
> > >@@ -250,6 +250,24 @@ static inline void
> > *bloblist_check_magic(ulong addr)
> > > return ptr;
> > > }
> > >
> > >+#if CONFIG_IS_ENABLED(BLOBLIST)
> >
> > Why should we use an #ifdef here?
> > Why would a caller invoke the function if the configuration is
> disabled?
> >
> > Adding #ifdef here is to have the inline function so that we don't need
> > to add the macro to all of its callers.
>
> There is just one. See patch 3, where you already check the CONFIG
> albeit in the wrong place.
>
>
Currently it is only one for TPM eventlog, but according to the Firmware
Handoff spec we will use Transfer List (aka bloblist in U-Boot) to handover
different information (blobs) from previous boot stage, so that we will
need to call bloblist_get_blob in multiple subsystems in the near future.
In patch #3, checking BLOBLIST is a temporary solution since we don't have
a good idea at the moment on a required config to enable/disable "handoff
from previous boot stage using bloblist" - which is separate from BLOBLIST
itself.
As discussed with Tom, I use BLOBLIST as a temporary solution but this will
be replaced in the future.
See my reply in patch #3 for more information.
Regards,
Raymond
> >
> > >+/**
> > >+ * bloblist_get_blob() - Find a blob and get the size of it
> > >+ *
> > >+ * Searches the bloblist and returns the blob with the matching
> tag
> > >+ *
> > >+ * @tag: Tag to search for (enum bloblist_tag_t)
> > >+ * @sizep: Size of the blob found
> > >+ * Return: pointer to bloblist if found, or NULL if not found
> > >+ */
> > >+void *bloblist_get_blob(uint tag, int *sizep);
> >
> > If tag is expected to be a value from the enum, we should not use
> > uint here.
> >
> > This is just following the original convention of existing bloblist
> > functions which are all using uint for the tag.
>
> @Simon: Why aren't we using enum?
>
> Best regards
>
> Heinrich
>
> >
> > Regards,
> > Raymond
> >
> >
> > >+#else
> > >+static inline void *bloblist_get_blob(uint tag, int *sizep)
> > >+{
> > >+ return NULL;
> > >+}
> > >+#endif
> > >+
> > > /**
> > > * bloblist_find() - Find a blob
> > > *
> > >diff --git a/test/common/bloblist.c b/test/common/bloblist.c
> > >index 4bca62110a..324127596f 100644
> > >--- a/test/common/bloblist.c
> > >+++ b/test/common/bloblist.c
> > >@@ -98,10 +98,12 @@ static int bloblist_test_blob(struct
> > unit_test_state *uts)
> > > struct bloblist_hdr *hdr;
> > > struct bloblist_rec *rec, *rec2;
> > > char *data;
> > >+ int size = 0;
> > >
> > > /* At the start there should be no records */
> > > hdr = clear_bloblist();
> > > ut_assertnull(bloblist_find(TEST_TAG, TEST_BLOBLIST_SIZE));
> > >+ ut_assertnull(bloblist_get_blob(TEST_TAG, &size));
> > > ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0,
> 0));
> > > ut_asserteq(sizeof(struct bloblist_hdr),
> bloblist_get_size());
> > > ut_asserteq(TEST_BLOBLIST_SIZE, bloblist_get_total_size());
> > >@@ -114,6 +116,8 @@ static int bloblist_test_blob(struct
> > unit_test_state *uts)
> > > ut_asserteq_addr(rec + 1, data);
> > > data = bloblist_find(TEST_TAG, TEST_SIZE);
> > > ut_asserteq_addr(rec + 1, data);
> > >+ ut_asserteq_addr(bloblist_get_blob(TEST_TAG, &size), data);
> > >+ ut_asserteq(size, TEST_SIZE);
> > >
> > > /* Check the data is zeroed */
> > > ut_assertok(check_zero(data, TEST_SIZE));
> >
>
>
More information about the U-Boot
mailing list