Hi Raymond,
On Wed, Dec 27, 2023 at 4:21 PM Raymond Mao <raymond....@linaro.org> wrote:
>
> Hi Simon,
>
> On Tue, 26 Dec 2023 at 04:48, Simon Glass <s...@chromium.org> wrote:
>>
>> Hi Raymond,
>>
>> On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao <raymond....@linaro.org> wrote:
>> >
>> > Add platform custom function to get bloblist from boot arguments.
>> > Check whether boot arguments aligns with the register conventions
>> > defined in FW Handoff spec v0.9.
>> > Add bloblist related options into qemu default config.
>> >
>> > Signed-off-by: Raymond Mao <raymond....@linaro.org>
>> > ---
>> > Changes in v2
>> > - Remove low level code for copying boot arguments.
>> > - Refactor board_fdt_blob_setup() and remove direct access of gd->bloblist.
>> > Changes in v3
>> > - Optimize board_bloblist_from_boot_arg().
>> >
>> > board/emulation/qemu-arm/qemu-arm.c | 30 +++++++++++++++++++++++++++++
>> > configs/qemu_arm64_defconfig | 3 +++
>> > 2 files changed, 33 insertions(+)
>> >
>> > diff --git a/board/emulation/qemu-arm/qemu-arm.c
>> > b/board/emulation/qemu-arm/qemu-arm.c
>> > index 942f1fff57..e225011bf0 100644
>> > --- a/board/emulation/qemu-arm/qemu-arm.c
>> > +++ b/board/emulation/qemu-arm/qemu-arm.c
>> > @@ -4,6 +4,7 @@
>> > */
>> >
>> > #include <common.h>
>> > +#include <bloblist.h>
>> > #include <cpu_func.h>
>> > #include <dm.h>
>> > #include <efi.h>
>> > @@ -102,6 +103,15 @@ static struct mm_region qemu_arm64_mem_map[] = {
>> > struct mm_region *mem_map = qemu_arm64_mem_map;
>> > #endif
>> >
>> > +/*
>> > + * Boot parameters saved from start.S
>> > + * saved_args[0]: FDT base address
>> > + * saved_args[1]: Bloblist signature
>> > + * saved_args[2]: must be 0
>> > + * saved_args[3]: Bloblist base address
>> > + */
>> > +extern unsigned long saved_args[];
>> > +
>> > int board_init(void)
>> > {
>> > return 0;
>> > @@ -144,6 +154,26 @@ void *board_fdt_blob_setup(int *err)
>> > return (void *)CFG_SYS_SDRAM_BASE;
>> > }
>> >
>> > +int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)
>> > +{
>> > + int ret = -ENOENT;
>> > +
>> > + if (!IS_ENABLED(CONFIG_OF_BOARD) || !IS_ENABLED(CONFIG_BLOBLIST))
>> > + return -ENOENT;
>> > +
>> > + ret = bloblist_check(saved_args[3], size);
>> > + if (ret)
>> > + return ret;
>>
>> What about saved_args[1] ?
>>
> The magic and version of register conventions cannot be checked now since
> there is
> a bug in the spec. PSB:
> ```
> X1[23:0]: set to the TL signature (4a0f_b10b)
> X1[31:24]: version of the register convention used. Set to 1 for the AArch64
> convention specified in this document.
> ```
> Signature (4a0f_b10b) takes all [31:0] and no space for version of the
> register convention.
> This needs to be updated on TF-A and OP-TEE as well after the spec is updated.
> For now I will just skip the checking.
>
>>
>> > +
>> > + /* Check the register conventions */
>> > + ret = bloblist_check_reg_conv(saved_args[0], saved_args[2]);
>> > + if (!ret)
>> > + /* Relocate the bloblist to the fixed address */
>> > + ret = bloblist_reloc((void *)addr, CONFIG_BLOBLIST_SIZE);
>> > +
>> > + return ret;
>> > +}
>> > +
>>
>> This should be a generic function, e.g. in arch/arm
>>
> I didn't find a place under arch/arm that is good for connect with the saved
> boot
> args. Do you have any suggestions?
How about arch/arm/lib/bloblist.c ?
>
> [...]
Regards,
Simon
