Hi Ilias,
On Thu, 28 Dec 2023 at 06:32, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Raymond,
On Wed, 27 Dec 2023 at 23:08, Raymond Mao raymond.mao@linaro.org wrote:
From: Simon Glass sjg@chromium.org
Rather than setting the alignment using the header size, add an entirely new entry to cover the gap left by the alignment.
Signed-off-by: Simon Glass sjg@chromium.org Co-developed-by: Raymond Mao raymond.mao@linaro.org Signed-off-by: Raymond Mao raymond.mao@linaro.org Reviewed-by: Simon Glass sjg@chromium.org
common/bloblist.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/common/bloblist.c b/common/bloblist.c index 705d9c6ae9..73dbbc01c0 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -142,7 +142,7 @@ static int bloblist_addrec(uint tag, int size, int
align_log2,
{ struct bloblist_hdr *hdr = gd->bloblist; struct bloblist_rec *rec;
int data_start, new_alloced;
int data_start, aligned_start, new_alloced; if (!align_log2) align_log2 = BLOBLIST_ALIGN_LOG2;@@ -151,10 +151,25 @@ static int bloblist_addrec(uint tag, int size, int
align_log2,
data_start = map_to_sysmem(hdr) + hdr->alloced + sizeof(*rec); /* Align the address and then calculate the offset from->alloced */
data_start = ALIGN(data_start, 1U << align_log2) -map_to_sysmem(hdr);
aligned_start = ALIGN(data_start, 1U << align_log2) - data_start;/* If we need to create a dummy record, create it */if (aligned_start) {int void_size = aligned_start - sizeof(*rec);struct bloblist_rec *vrec;int ret;ret = bloblist_addrec(BLOBLISTT_VOID, void_size, 0,&vrec);
I just noticed the 'BLOBLISTT'. Is that on purpose? or a typo?
if (ret)return log_msg_ret("void", ret);/* start the record after that */data_start = map_to_sysmem(hdr) + hdr->alloced +sizeof(*vrec);
} /* Calculate the new allocated total */
new_alloced = data_start + ALIGN(size, 1U << align_log2);
new_alloced = data_start - map_to_sysmem(hdr) +ALIGN(size, 1U << align_log2);So, wouldn't it make more sense to add the dummy record and align the whole thing *after* we've added the entry? Doing it like this might leave the last entry on an unaligned boundary, no?
The spec just cares about the TE *data* starts at an aligned address but
not the TE header. Not sure if I fully understand your question but each TE *data* will start at an aligned address after calling `bloblist_addrec()`. And each TE is allowed to have its own alignment value, so we have to do the padding when a TE is being added but cannot predict the alignment value for the next TE - that means we cannot do the padding after each TE is added.
[...]
Thanks and regards, Raymond