On Wed, 10 Jul 2024 at 17:46, Raymond Mao <raymond....@linaro.org> wrote:
>
> Hi Levi,
>
> On Wed, 10 Jul 2024 at 09:53, Levi Yun <yeoreum....@arm.com> wrote:
>>
>> According to recently firmware handsoff spec [1]'s "Register usage at handoff
>> boundary", Transfer List's signature value was changed from 0x40_b10b
>> (3 bytes) to 4a0f_b10b (4 bytes).
>>
>> As updating of TL's signature, register value of x1/r1 should be:
>>
>> In aarch32's r1 value should be
>> R1[23:0]: set to the TL signature (4a0f_b10b -> masked range value:
>> 0f_b10b)
>> R1[31:24]: version of the register convention == 1
>>
>> and
>>
>> In aarch64's x1 value should be
>> X1[31:0]: set to the TL signature (4a0f_b10b)
>> X1[39:32]: version of the register convention == 1
>> X1[63:40]: MBZ
>> (See the [2] and [3]).
>>
>> This patch fix problems:
>> 1. breaking X1 value with updated specification in aarch64
>> - change of length of signature field.
>>
>> 2. previous error value set in R1 in arm32.
>> - length of signature should be 24, but it uses 32bit signature.
>>
>> This patch is a breaking change. It works only TF-A is updated.
>>
>> Link: https://github.com/FirmwareHandoff/firmware_handoff [1]
>> Link: https://github.com/FirmwareHandoff/firmware_handoff/issues/32 [2]
>> Link:
>> https://github.com/FirmwareHandoff/firmware_handoff/commit/5aa7aa1d3a1db75213e458d392b751f0707de027
>> [3]
>> Signed-off-by: Levi Yun <yeoreum....@arm.com>
>> ---
>> v4:
>> - Remove FIXME tags on xfterlist.h
>> - Modify commit message
>>
>> v3:
>> - Restore arch/arm/lib/xferlist.c
>>
>> v2:
>> - Modify commit message
>> - Remove wrong swaping check in xferlist_from_boot_arg()
>>
>> common/bloblist.c | 11 ++++++++++-
>> include/bloblist.h | 10 ++++------
>> 2 files changed, 14 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass <s...@chromium.org>
>>
>> diff --git a/common/bloblist.c b/common/bloblist.c
>> index 11d6422b69..2008ab4d25 100644
>> --- a/common/bloblist.c
>> +++ b/common/bloblist.c
>> @@ -576,7 +576,16 @@ int bloblist_maybe_init(void)
>>
>> int bloblist_check_reg_conv(ulong rfdt, ulong rzero, ulong rsig)
>> {
>> - if (rzero || rsig != (BLOBLIST_MAGIC | BLOBLIST_REGCONV_VER) ||
>> + ulong version = BLOBLIST_REGCONV_VER;
>> + ulong sigval;
>> +
>> + sigval = (IS_ENABLED(CONFIG_64BIT)) ?
>> + ((BLOBLIST_MAGIC & ((1UL <<
>> BLOBLIST_REGCONV_SHIFT_64) - 1)) |
>> + ((version & BLOBLIST_REGCONV_MASK) <<
>> BLOBLIST_REGCONV_SHIFT_64)) :
>> + ((BLOBLIST_MAGIC & ((1UL <<
>> BLOBLIST_REGCONV_SHIFT_32) - 1)) |
>> + ((version & BLOBLIST_REGCONV_MASK) <<
>> BLOBLIST_REGCONV_SHIFT_32));
>> +
>> + if (rzero || rsig != sigval ||
>> rfdt != (ulong)bloblist_find(BLOBLISTT_CONTROL_FDT, 0)) {
I'd argue that this is unnecessary, since the point of passing fdt in
a register is to avoid having to search the bloblist for it.
>> gd->bloblist = NULL; /* Reset the gd bloblist pointer */
>> return -EIO;
>> diff --git a/include/bloblist.h b/include/bloblist.h
>> index 7fbdd622bc..b0706b5637 100644
>> --- a/include/bloblist.h
>> +++ b/include/bloblist.h
>> @@ -78,12 +78,10 @@ enum {
>> BLOBLIST_VERSION = 1,
>> BLOBLIST_MAGIC = 0x4a0fb10b,
>>
>> - /*
>> - * FIXME:
>> - * Register convention version should be placed into a higher byte
>> - * https://github.com/FirmwareHandoff/firmware_handoff/issues/32
>> - */
>> - BLOBLIST_REGCONV_VER = 1 << 24,
>> + BLOBLIST_REGCONV_SHIFT_64 = 32,
>> + BLOBLIST_REGCONV_SHIFT_32 = 24,
>> + BLOBLIST_REGCONV_MASK = 0xff,
>> + BLOBLIST_REGCONV_VER = 1,
>>
>> BLOBLIST_BLOB_ALIGN_LOG2 = 3,
>> BLOBLIST_BLOB_ALIGN = 1 << BLOBLIST_BLOB_ALIGN_LOG2,
>> --
>> LEVI:{C3F47F37-75D8-414A-A8BA-3980EC8A46D7}
>>
> Feel free to add tag:
> Reviewed-by: Raymond Mao <raymond....@linaro.org>
>
> Regards,
> Raymond
>
Regards,
Simon
