[PATCH v3 0/9] Handoff bloblist from previous boot stage
This patch set depends on another series: "[PATCH v3 00/14] Support Firmware Handoff spec via bloblist".
This patch set implements Qemu-Arm platform custom functions to retrieve the bloblist (aka. Transfer List) from previous loader via boot arguments when CONFIG_OF_BOARD option is enabled and all boot arguments are compliant to the register conventions defined in the Firmware Handoff spec v0.9.
Qemu-Arm platform custom function will load the FDT from the bloblist if it exists. Otherwise it fallbacks to get the FDT from the specified memory address.
If a platform vendor wish to have different behaviors for loading bloblist or FDT from the previous boot stage, it is required to implement the custom functions board_bloblist_from_boot_arg() and board_fdt_blob_setup().
Raymond Mao (9): bloblist: add API to check the register conventions bloblist: check bloblist with specified buffer size bloblist: refactor of bloblist_reloc() arm: armv7: save boot arguments arm: armv8: save boot arguments qemu-arm: Get bloblist from boot arguments bloblist: Load the bloblist from the previous loader fdt: update the document and Kconfig description qemu-arm: get FDT from bloblist
arch/arm/cpu/armv7/start.S | 13 ++++++ arch/arm/cpu/armv8/start.S | 14 ++++++ board/emulation/qemu-arm/qemu-arm.c | 42 ++++++++++++++++- common/bloblist.c | 72 ++++++++++++++++++++--------- common/board_f.c | 8 +--- configs/qemu_arm64_defconfig | 3 ++ doc/develop/devicetree/control.rst | 6 +-- dts/Kconfig | 7 ++- include/bloblist.h | 36 +++++++++++++-- test/bloblist.c | 8 ++-- 10 files changed, 166 insertions(+), 43 deletions(-)
Add bloblist_check_reg_conv() to check whether the bloblist is compliant to the register conventions defined in Firmware Handoff specification. This API can be used for all Arm platforms.
Signed-off-by: Raymond Mao raymond.mao@linaro.org --- Changes in v2 - Refactor of bloblist_check_reg_conv(). Changes in v3 - bloblist_check_reg_conv() returns -ENOENT if OF_BOARD is disabled.
common/bloblist.c | 13 +++++++++++++ include/bloblist.h | 12 ++++++++++++ 2 files changed, 25 insertions(+)
diff --git a/common/bloblist.c b/common/bloblist.c index 625e480f6b..ba17dd851a 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -542,3 +542,16 @@ int bloblist_maybe_init(void)
return 0; } + +int bloblist_check_reg_conv(ulong rfdt, ulong rzero) +{ + if (!IS_ENABLED(CONFIG_OF_BOARD)) + return -ENOENT; + + if (rzero || rfdt != (ulong)bloblist_find(BLOBLISTT_CONTROL_FDT, 0)) { + gd->bloblist = NULL; /* Reset the gd bloblist pointer */ + return -EIO; + } + + return 0; +} diff --git a/include/bloblist.h b/include/bloblist.h index 84fc943819..bd32e38a06 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -461,4 +461,16 @@ static inline int bloblist_maybe_init(void) } #endif /* BLOBLIST */
+/** + * bloblist_check_reg_conv() - Check whether the bloblist is compliant to + * the register conventions according to the + * Firmware Handoff spec. + * + * @rfdt: Register that holds the FDT base address. + * @rzero: Register that must be zero. + * Return: 0 if OK, -EIO if the bloblist is not compliant to the register + * conventions, -ENOENT if OF_BOARD is disabled. + */ +int bloblist_check_reg_conv(ulong rfdt, ulong rzero); + #endif /* __BLOBLIST_H */
Hi Raymond,
On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao raymond.mao@linaro.org wrote:
Add bloblist_check_reg_conv() to check whether the bloblist is compliant to the register conventions defined in Firmware Handoff specification. This API can be used for all Arm platforms.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- Refactor of bloblist_check_reg_conv().
Changes in v3
- bloblist_check_reg_conv() returns -ENOENT if OF_BOARD is disabled.
common/bloblist.c | 13 +++++++++++++ include/bloblist.h | 12 ++++++++++++ 2 files changed, 25 insertions(+)
diff --git a/common/bloblist.c b/common/bloblist.c index 625e480f6b..ba17dd851a 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -542,3 +542,16 @@ int bloblist_maybe_init(void)
return 0;}
+int bloblist_check_reg_conv(ulong rfdt, ulong rzero) +{
if (!IS_ENABLED(CONFIG_OF_BOARD))return -ENOENT;if (rzero || rfdt != (ulong)bloblist_find(BLOBLISTT_CONTROL_FDT, 0)) {gd->bloblist = NULL; /* Reset the gd bloblist pointer */return -EIO;}
Where does the magic 4a0fb10b value get checked?
return 0;+} diff --git a/include/bloblist.h b/include/bloblist.h index 84fc943819..bd32e38a06 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -461,4 +461,16 @@ static inline int bloblist_maybe_init(void) } #endif /* BLOBLIST */
+/**
- bloblist_check_reg_conv() - Check whether the bloblist is compliant to
the register conventions according to the
Firmware Handoff spec.
- @rfdt: Register that holds the FDT base address.
- @rzero: Register that must be zero.
- Return: 0 if OK, -EIO if the bloblist is not compliant to the register
conventions, -ENOENT if OF_BOARD is disabled.- */
+int bloblist_check_reg_conv(ulong rfdt, ulong rzero);
#endif /* __BLOBLIST_H */
2.25.1
Regards, Simon
Hi Simon,
On Tue, 26 Dec 2023 at 04:48, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao raymond.mao@linaro.org wrote:
Add bloblist_check_reg_conv() to check whether the bloblist is compliant to the register conventions defined in Firmware Handoff specification. This API can be used for all Arm platforms.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- Refactor of bloblist_check_reg_conv().
Changes in v3
- bloblist_check_reg_conv() returns -ENOENT if OF_BOARD is disabled.
common/bloblist.c | 13 +++++++++++++ include/bloblist.h | 12 ++++++++++++ 2 files changed, 25 insertions(+)
diff --git a/common/bloblist.c b/common/bloblist.c index 625e480f6b..ba17dd851a 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -542,3 +542,16 @@ int bloblist_maybe_init(void)
return 0;}
+int bloblist_check_reg_conv(ulong rfdt, ulong rzero) +{
if (!IS_ENABLED(CONFIG_OF_BOARD))return -ENOENT;if (rzero || rfdt != (ulong)bloblist_find(BLOBLISTT_CONTROL_FDT,0)) {
gd->bloblist = NULL; /* Reset the gd bloblist pointer */return -EIO;}Where does the magic 4a0fb10b value get checked?
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.
[...]
Regards, Raymond
Hi Raymond,
On Wed, Dec 27, 2023 at 4:06 PM Raymond Mao raymond.mao@linaro.org wrote:
Hi Simon,
On Tue, 26 Dec 2023 at 04:48, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao raymond.mao@linaro.org wrote:
Add bloblist_check_reg_conv() to check whether the bloblist is compliant to the register conventions defined in Firmware Handoff specification. This API can be used for all Arm platforms.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- Refactor of bloblist_check_reg_conv().
Changes in v3
- bloblist_check_reg_conv() returns -ENOENT if OF_BOARD is disabled.
common/bloblist.c | 13 +++++++++++++ include/bloblist.h | 12 ++++++++++++ 2 files changed, 25 insertions(+)
diff --git a/common/bloblist.c b/common/bloblist.c index 625e480f6b..ba17dd851a 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -542,3 +542,16 @@ int bloblist_maybe_init(void)
return 0;}
+int bloblist_check_reg_conv(ulong rfdt, ulong rzero) +{
if (!IS_ENABLED(CONFIG_OF_BOARD))return -ENOENT;if (rzero || rfdt != (ulong)bloblist_find(BLOBLISTT_CONTROL_FDT, 0)) {gd->bloblist = NULL; /* Reset the gd bloblist pointer */return -EIO;}Where does the magic 4a0fb10b value get checked?
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)
That should say 0f_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.
We should fix the spec instead.
Regards, Simon
Hi Simon,
On Thu, 28 Dec 2023 at 08:38, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Wed, Dec 27, 2023 at 4:06 PM Raymond Mao raymond.mao@linaro.org wrote:
Hi Simon,
On Tue, 26 Dec 2023 at 04:48, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao raymond.mao@linaro.org
wrote:
Add bloblist_check_reg_conv() to check whether the bloblist is
compliant
to the register conventions defined in Firmware Handoff specification. This API can be used for all Arm platforms.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- Refactor of bloblist_check_reg_conv().
Changes in v3
- bloblist_check_reg_conv() returns -ENOENT if OF_BOARD is disabled.
common/bloblist.c | 13 +++++++++++++ include/bloblist.h | 12 ++++++++++++ 2 files changed, 25 insertions(+)
diff --git a/common/bloblist.c b/common/bloblist.c index 625e480f6b..ba17dd851a 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -542,3 +542,16 @@ int bloblist_maybe_init(void)
return 0;}
+int bloblist_check_reg_conv(ulong rfdt, ulong rzero) +{
if (!IS_ENABLED(CONFIG_OF_BOARD))return -ENOENT;if (rzero || rfdt !=(ulong)bloblist_find(BLOBLISTT_CONTROL_FDT, 0)) {
gd->bloblist = NULL; /* Reset the gd bloblistpointer */
return -EIO;}Where does the magic 4a0fb10b value get checked?
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)That should say 0f_b10b
However, currently we have switched to `4a0f_b10b` in both TF-A and OP-TEE.
I don't want to break the dataflow that is currently working. I prefer to make patches to TF-A, OP-TEE and U-Boot at the same time when spec v1.0 is ready.
[...]
Thanks and regards, Raymond
Hi Raymond,
On Thu, Dec 28, 2023 at 3:13 PM Raymond Mao raymond.mao@linaro.org wrote:
Hi Simon,
On Thu, 28 Dec 2023 at 08:38, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Wed, Dec 27, 2023 at 4:06 PM Raymond Mao raymond.mao@linaro.org wrote:
Hi Simon,
On Tue, 26 Dec 2023 at 04:48, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao raymond.mao@linaro.org wrote:
Add bloblist_check_reg_conv() to check whether the bloblist is compliant to the register conventions defined in Firmware Handoff specification. This API can be used for all Arm platforms.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- Refactor of bloblist_check_reg_conv().
Changes in v3
- bloblist_check_reg_conv() returns -ENOENT if OF_BOARD is disabled.
common/bloblist.c | 13 +++++++++++++ include/bloblist.h | 12 ++++++++++++ 2 files changed, 25 insertions(+)
diff --git a/common/bloblist.c b/common/bloblist.c index 625e480f6b..ba17dd851a 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -542,3 +542,16 @@ int bloblist_maybe_init(void)
return 0;}
+int bloblist_check_reg_conv(ulong rfdt, ulong rzero) +{
if (!IS_ENABLED(CONFIG_OF_BOARD))return -ENOENT;if (rzero || rfdt != (ulong)bloblist_find(BLOBLISTT_CONTROL_FDT, 0)) {gd->bloblist = NULL; /* Reset the gd bloblist pointer */return -EIO;}Where does the magic 4a0fb10b value get checked?
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)That should say 0f_b10b
However, currently we have switched to `4a0f_b10b` in both TF-A and OP-TEE. I don't want to break the dataflow that is currently working. I prefer to make patches to TF-A, OP-TEE and U-Boot at the same time when spec v1.0 is ready.
Yes, sure, but we should fix them too!
[...]
Thanks and regards, Raymond
Regards, Simon
Instead of expecting the bloblist total size to be the same as the pre-allocated buffer size, practically we are more interested in whether the pre-allocated buffer size is bigger than the bloblist total size.
Signed-off-by: Raymond Mao raymond.mao@linaro.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- Changes in v2 - New patch file created for v2.
common/bloblist.c | 2 +- test/bloblist.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/bloblist.c b/common/bloblist.c index ba17dd851a..db3fbb20cf 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -384,7 +384,7 @@ int bloblist_check(ulong addr, uint size) return log_msg_ret("Bad magic", -ENOENT); if (hdr->version != BLOBLIST_VERSION) return log_msg_ret("Bad version", -EPROTONOSUPPORT); - if (!hdr->total_size || (size && hdr->total_size != size)) + if (!hdr->total_size || (size && hdr->total_size > size)) return log_msg_ret("Bad total size", -EFBIG); if (hdr->used_size > hdr->total_size) return log_msg_ret("Bad used size", -ENOENT); diff --git a/test/bloblist.c b/test/bloblist.c index 17d9dd03d0..7dab9addf8 100644 --- a/test/bloblist.c +++ b/test/bloblist.c @@ -207,7 +207,7 @@ static int bloblist_test_checksum(struct unit_test_state *uts) hdr->flags++;
hdr->total_size--; - ut_asserteq(-EFBIG, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); + ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); hdr->total_size++;
hdr->spare++;
Hi Raymond,
On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao raymond.mao@linaro.org wrote:
Instead of expecting the bloblist total size to be the same as the pre-allocated buffer size, practically we are more interested in whether the pre-allocated buffer size is bigger than the bloblist total size.
Signed-off-by: Raymond Mao raymond.mao@linaro.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Changes in v2
- New patch file created for v2.
common/bloblist.c | 2 +- test/bloblist.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/bloblist.c b/common/bloblist.c index ba17dd851a..db3fbb20cf 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -384,7 +384,7 @@ int bloblist_check(ulong addr, uint size)
Can you please also update the function comment for bloblist_check() ?
return log_msg_ret("Bad magic", -ENOENT); if (hdr->version != BLOBLIST_VERSION) return log_msg_ret("Bad version", -EPROTONOSUPPORT);
if (!hdr->total_size || (size && hdr->total_size != size))
if (!hdr->total_size || (size && hdr->total_size > size)) return log_msg_ret("Bad total size", -EFBIG); if (hdr->used_size > hdr->total_size) return log_msg_ret("Bad used size", -ENOENT);diff --git a/test/bloblist.c b/test/bloblist.c index 17d9dd03d0..7dab9addf8 100644 --- a/test/bloblist.c +++ b/test/bloblist.c @@ -207,7 +207,7 @@ static int bloblist_test_checksum(struct unit_test_state *uts) hdr->flags++;
hdr->total_size--;
ut_asserteq(-EFBIG, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE)); hdr->total_size++; hdr->spare++;-- 2.25.1
Regards, Simon
Hi Simon,
On Tue, 26 Dec 2023 at 04:47, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao raymond.mao@linaro.org wrote:
Instead of expecting the bloblist total size to be the same as the pre-allocated buffer size, practically we are more interested in whether the pre-allocated buffer size is bigger than the bloblist total size.
Signed-off-by: Raymond Mao raymond.mao@linaro.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
Changes in v2
- New patch file created for v2.
common/bloblist.c | 2 +- test/bloblist.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/common/bloblist.c b/common/bloblist.c index ba17dd851a..db3fbb20cf 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -384,7 +384,7 @@ int bloblist_check(ulong addr, uint size)
Can you please also update the function comment for bloblist_check() ?
Yes. I will update it.
[...]
Regards, Raymond
The current bloblist pointer and size can be retrieved from global data, so we don't need to pass them from the function arguments. This change also help to remove all external access of gd->bloblist outside of bloblist module.
Signed-off-by: Raymond Mao raymond.mao@linaro.org --- Changes in v2 - New patch file created for v2. Changes in v3 - Check the space size before copying the bloblist. - Add return code of bloblist_reloc().
common/bloblist.c | 10 ++++++++-- common/board_f.c | 8 ++------ include/bloblist.h | 8 ++++---- test/bloblist.c | 6 ++---- 4 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/common/bloblist.c b/common/bloblist.c index db3fbb20cf..5ad1db137a 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -472,13 +472,19 @@ void bloblist_show_list(void) } }
-void bloblist_reloc(void *to, uint to_size, void *from, uint from_size) +int bloblist_reloc(void *to, uint to_size) { struct bloblist_hdr *hdr;
- memcpy(to, from, from_size); + if (to_size < gd->bloblist->total_size) + return -ENOSPC; + + memcpy(to, gd->bloblist, gd->bloblist->total_size); hdr = to; hdr->total_size = to_size; + gd->bloblist = to; + + return 0; }
int bloblist_init(void) diff --git a/common/board_f.c b/common/board_f.c index d4d7d01f8f..00b0430889 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -676,13 +676,9 @@ static int reloc_bloblist(void) return 0; } if (gd->new_bloblist) { - int size = CONFIG_BLOBLIST_SIZE; - debug("Copying bloblist from %p to %p, size %x\n", - gd->bloblist, gd->new_bloblist, size); - bloblist_reloc(gd->new_bloblist, CONFIG_BLOBLIST_SIZE_RELOC, - gd->bloblist, size); - gd->bloblist = gd->new_bloblist; + gd->bloblist, gd->new_bloblist, gd->bloblist->total_size); + bloblist_reloc(gd->new_bloblist, CONFIG_BLOBLIST_SIZE_RELOC); } #endif
diff --git a/include/bloblist.h b/include/bloblist.h index bd32e38a06..c1dab11f78 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -418,11 +418,11 @@ const char *bloblist_tag_name(enum bloblist_tag_t tag); * bloblist_reloc() - Relocate the bloblist and optionally resize it * * @to: Pointer to new bloblist location (must not overlap old location) - * @to_size: New size for bloblist (must be larger than from_size) - * @from: Pointer to bloblist to relocate - * @from_size: Size of bloblist to relocate + * @to_size: New size for bloblist + * Return: 0 if OK, -ENOSPC if the new size is small than the bloblist total + * size. */ -void bloblist_reloc(void *to, uint to_size, void *from, uint from_size); +int bloblist_reloc(void *to, uint to_size);
/** * bloblist_init() - Init the bloblist system with a single bloblist diff --git a/test/bloblist.c b/test/bloblist.c index 7dab9addf8..1c60bbac36 100644 --- a/test/bloblist.c +++ b/test/bloblist.c @@ -376,13 +376,12 @@ static int bloblist_test_reloc(struct unit_test_state *uts) { const uint large_size = TEST_BLOBLIST_SIZE; const uint small_size = 0x20; - void *old_ptr, *new_ptr; + void *new_ptr; void *blob1, *blob2; ulong new_addr; ulong new_size;
ut_assertok(bloblist_new(TEST_ADDR, TEST_BLOBLIST_SIZE, 0, 0)); - old_ptr = map_sysmem(TEST_ADDR, TEST_BLOBLIST_SIZE);
/* Add one blob and then one that won't fit */ blob1 = bloblist_add(TEST_TAG, small_size, 0); @@ -394,8 +393,7 @@ static int bloblist_test_reloc(struct unit_test_state *uts) new_addr = TEST_ADDR + TEST_BLOBLIST_SIZE; new_size = TEST_BLOBLIST_SIZE + 0x100; new_ptr = map_sysmem(new_addr, TEST_BLOBLIST_SIZE); - bloblist_reloc(new_ptr, new_size, old_ptr, TEST_BLOBLIST_SIZE); - gd->bloblist = new_ptr; + ut_assertok(bloblist_reloc(new_ptr, new_size));
/* Check the old blob is there and that we can now add the bigger one */ ut_assertnonnull(bloblist_find(TEST_TAG, small_size));
On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao raymond.mao@linaro.org wrote:
The current bloblist pointer and size can be retrieved from global data, so we don't need to pass them from the function arguments. This change also help to remove all external access of gd->bloblist outside of bloblist module.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- New patch file created for v2.
Changes in v3
- Check the space size before copying the bloblist.
- Add return code of bloblist_reloc().
common/bloblist.c | 10 ++++++++-- common/board_f.c | 8 ++------ include/bloblist.h | 8 ++++---- test/bloblist.c | 6 ++---- 4 files changed, 16 insertions(+), 16 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Hi Raymond
On Fri, 22 Dec 2023 at 23:31, Raymond Mao raymond.mao@linaro.org wrote:
The current bloblist pointer and size can be retrieved from global data, so we don't need to pass them from the function arguments. This change also help to remove all external access of gd->bloblist outside of bloblist module.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- New patch file created for v2.
Changes in v3
- Check the space size before copying the bloblist.
- Add return code of bloblist_reloc().
common/bloblist.c | 10 ++++++++-- common/board_f.c | 8 ++------ include/bloblist.h | 8 ++++---- test/bloblist.c | 6 ++---- 4 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/common/bloblist.c b/common/bloblist.c index db3fbb20cf..5ad1db137a 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -472,13 +472,19 @@ void bloblist_show_list(void) } }
-void bloblist_reloc(void *to, uint to_size, void *from, uint from_size) +int bloblist_reloc(void *to, uint to_size) { struct bloblist_hdr *hdr;
memcpy(to, from, from_size);
if (to_size < gd->bloblist->total_size)return -ENOSPC;memcpy(to, gd->bloblist, gd->bloblist->total_size); hdr = to; hdr->total_size = to_size;gd->bloblist = to;return 0;}
int bloblist_init(void) diff --git a/common/board_f.c b/common/board_f.c index d4d7d01f8f..00b0430889 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -676,13 +676,9 @@ static int reloc_bloblist(void) return 0; } if (gd->new_bloblist) {
int size = CONFIG_BLOBLIST_SIZE;debug("Copying bloblist from %p to %p, size %x\n",gd->bloblist, gd->new_bloblist, size);bloblist_reloc(gd->new_bloblist, CONFIG_BLOBLIST_SIZE_RELOC,gd->bloblist, size);gd->bloblist = gd->new_bloblist;
gd->bloblist, gd->new_bloblist, gd->bloblist->total_size);bloblist_reloc(gd->new_bloblist, CONFIG_BLOBLIST_SIZE_RELOC);
Why aren't we checking bloblist_reloc() for an error?
}#endif
[...]
Thanks /Ilias
Hi Ilias,
On Wed, 27 Dec 2023 at 03:33, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Raymond
On Fri, 22 Dec 2023 at 23:31, Raymond Mao raymond.mao@linaro.org wrote:
The current bloblist pointer and size can be retrieved from global data, so we don't need to pass them from the function arguments. This change also help to remove all external access of gd->bloblist outside of bloblist module.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- New patch file created for v2.
Changes in v3
- Check the space size before copying the bloblist.
- Add return code of bloblist_reloc().
common/bloblist.c | 10 ++++++++-- common/board_f.c | 8 ++------ include/bloblist.h | 8 ++++---- test/bloblist.c | 6 ++---- 4 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/common/bloblist.c b/common/bloblist.c index db3fbb20cf..5ad1db137a 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -472,13 +472,19 @@ void bloblist_show_list(void) } }
-void bloblist_reloc(void *to, uint to_size, void *from, uint from_size) +int bloblist_reloc(void *to, uint to_size) { struct bloblist_hdr *hdr;
memcpy(to, from, from_size);
if (to_size < gd->bloblist->total_size)return -ENOSPC;memcpy(to, gd->bloblist, gd->bloblist->total_size); hdr = to; hdr->total_size = to_size;gd->bloblist = to;return 0;}
int bloblist_init(void) diff --git a/common/board_f.c b/common/board_f.c index d4d7d01f8f..00b0430889 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -676,13 +676,9 @@ static int reloc_bloblist(void) return 0; } if (gd->new_bloblist) {
int size = CONFIG_BLOBLIST_SIZE;debug("Copying bloblist from %p to %p, size %x\n",gd->bloblist, gd->new_bloblist, size);bloblist_reloc(gd->new_bloblist,CONFIG_BLOBLIST_SIZE_RELOC,
gd->bloblist, size);gd->bloblist = gd->new_bloblist;
gd->bloblist, gd->new_bloblist,gd->bloblist->total_size);
bloblist_reloc(gd->new_bloblist,CONFIG_BLOBLIST_SIZE_RELOC);
Why aren't we checking bloblist_reloc() for an error?
Yes. This should be checked as well here. Will update in v4.
[...]
Regards, Raymond
Save boot arguments r[0-3] into an array for handover of bloblist from previous boot stage.
Signed-off-by: Raymond Mao raymond.mao@linaro.org --- Changes in v2 - New patch file created for v2. Changes in v3 - Swap value of r0 with r2.
arch/arm/cpu/armv7/start.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 69e281b086..2ca63ca32c 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -152,9 +152,22 @@ ENDPROC(c_runtime_cpu_setup) * *************************************************************************/ WEAK(save_boot_params) +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST)) + ldr r12, =saved_args + /* Intentionally swapping r0 with r2 */ + stm r12, {r2, r1, r0, r3} +#endif b save_boot_params_ret @ back to my caller ENDPROC(save_boot_params)
+.section .data +.global saved_args +saved_args: + .rept 4 + .word 0 + .endr +END(saved_args) + #ifdef CONFIG_ARMV7_LPAE WEAK(switch_to_hypervisor) b switch_to_hypervisor_ret
Hi Raymond,
On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao raymond.mao@linaro.org wrote:
Save boot arguments r[0-3] into an array for handover of bloblist from previous boot stage.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- New patch file created for v2.
Changes in v3
- Swap value of r0 with r2.
arch/arm/cpu/armv7/start.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 69e281b086..2ca63ca32c 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -152,9 +152,22 @@ ENDPROC(c_runtime_cpu_setup)
*************************************************************************/ WEAK(save_boot_params) +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST))
ldr r12, =saved_args/* Intentionally swapping r0 with r2 */stm r12, {r2, r1, r0, r3}
I thought the instruction was a bitmap, so what does the 'swapping' actually change?
+#endif b save_boot_params_ret @ back to my caller ENDPROC(save_boot_params)
+.section .data +.global saved_args +saved_args:
.rept 4.word 0.endr+END(saved_args)
#ifdef CONFIG_ARMV7_LPAE WEAK(switch_to_hypervisor) b switch_to_hypervisor_ret -- 2.25.1
Regards, Simon
Hi Simon,
On Tue, 26 Dec 2023 at 04:48, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao raymond.mao@linaro.org wrote:
Save boot arguments r[0-3] into an array for handover of bloblist from previous boot stage.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- New patch file created for v2.
Changes in v3
- Swap value of r0 with r2.
arch/arm/cpu/armv7/start.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 69e281b086..2ca63ca32c 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -152,9 +152,22 @@ ENDPROC(c_runtime_cpu_setup)
*************************************************************************/
WEAK(save_boot_params) +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST))
ldr r12, =saved_args/* Intentionally swapping r0 with r2 */stm r12, {r2, r1, r0, r3}I thought the instruction was a bitmap, so what does the 'swapping' actually change?
The swapping is due to different register conventions between aarch32 and
aarch64. Actually swapping here is to avoid the complexity of swapping this in C code. I will update the comments here.
[...]
Regards, Raymond
Hi Raymond,
On Wed, Dec 27, 2023 at 3:52 PM Raymond Mao raymond.mao@linaro.org wrote:
Hi Simon,
On Tue, 26 Dec 2023 at 04:48, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao raymond.mao@linaro.org wrote:
Save boot arguments r[0-3] into an array for handover of bloblist from previous boot stage.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- New patch file created for v2.
Changes in v3
- Swap value of r0 with r2.
arch/arm/cpu/armv7/start.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 69e281b086..2ca63ca32c 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -152,9 +152,22 @@ ENDPROC(c_runtime_cpu_setup)
*************************************************************************/ WEAK(save_boot_params) +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST))
ldr r12, =saved_args/* Intentionally swapping r0 with r2 */stm r12, {r2, r1, r0, r3}I thought the instruction was a bitmap, so what does the 'swapping' actually change?
The swapping is due to different register conventions between aarch32 and aarch64. Actually swapping here is to avoid the complexity of swapping this in C code. I will update the comments here.
[...]
But how does this work? stm does not support changing the ordering...what am I missing?
Regards, Simon
Hi Simon,
On Thu, 28 Dec 2023 at 08:37, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Wed, Dec 27, 2023 at 3:52 PM Raymond Mao raymond.mao@linaro.org wrote:
Hi Simon,
On Tue, 26 Dec 2023 at 04:48, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao raymond.mao@linaro.org
wrote:
Save boot arguments r[0-3] into an array for handover of bloblist from previous boot stage.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- New patch file created for v2.
Changes in v3
- Swap value of r0 with r2.
arch/arm/cpu/armv7/start.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 69e281b086..2ca63ca32c 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -152,9 +152,22 @@ ENDPROC(c_runtime_cpu_setup)
*************************************************************************/
WEAK(save_boot_params) +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST))
ldr r12, =saved_args/* Intentionally swapping r0 with r2 */stm r12, {r2, r1, r0, r3}I thought the instruction was a bitmap, so what does the 'swapping' actually change?
The swapping is due to different register conventions between aarch32
and aarch64.
Actually swapping here is to avoid the complexity of swapping this in C
code.
I will update the comments here.
[...]
But how does this work? stm does not support changing the ordering...what am I missing?
Ah, my bad. You are right. I have to use `str` to store one by one in this case.
[...]
Thanks and regards, Raymond
Hi Raymond,
On Fri, 22 Dec 2023 at 23:31, Raymond Mao raymond.mao@linaro.org wrote:
Save boot arguments r[0-3] into an array for handover of bloblist from previous boot stage.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- New patch file created for v2.
Changes in v3
- Swap value of r0 with r2.
arch/arm/cpu/armv7/start.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 69e281b086..2ca63ca32c 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -152,9 +152,22 @@ ENDPROC(c_runtime_cpu_setup)
*************************************************************************/ WEAK(save_boot_params) +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST))
ldr r12, =saved_args/* Intentionally swapping r0 with r2 */
That isn't very helpful. It's obvious what the code does here. Add a comment on *why* (to keep the c function simpler etc)
Thanks /Ilias
stm r12, {r2, r1, r0, r3}+#endif b save_boot_params_ret @ back to my caller ENDPROC(save_boot_params)
+.section .data +.global saved_args +saved_args:
.rept 4.word 0.endr+END(saved_args)
#ifdef CONFIG_ARMV7_LPAE WEAK(switch_to_hypervisor) b switch_to_hypervisor_ret -- 2.25.1
Hi Ilias and Simon,
On Wed, 27 Dec 2023 at 03:30, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
Hi Raymond,
On Fri, 22 Dec 2023 at 23:31, Raymond Mao raymond.mao@linaro.org wrote:
Save boot arguments r[0-3] into an array for handover of bloblist from previous boot stage.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- New patch file created for v2.
Changes in v3
- Swap value of r0 with r2.
arch/arm/cpu/armv7/start.S | 13 +++++++++++++ 1 file changed, 13 insertions(+)
diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 69e281b086..2ca63ca32c 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -152,9 +152,22 @@ ENDPROC(c_runtime_cpu_setup)
*************************************************************************/
WEAK(save_boot_params) +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST))
ldr r12, =saved_args/* Intentionally swapping r0 with r2 */That isn't very helpful. It's obvious what the code does here. Add a comment on *why* (to keep the c function simpler etc)
Yes. I will update the comment.
[...]
Regards, Raymond
Save boot arguments x[0-3] into an array for handover of bloblist from previous boot stage.
Signed-off-by: Raymond Mao raymond.mao@linaro.org --- Changes in v2 - New patch file created for v2.
arch/arm/cpu/armv8/start.S | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index 6cc1d26e5e..8e704f590e 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -370,5 +370,19 @@ ENTRY(c_runtime_cpu_setup) ENDPROC(c_runtime_cpu_setup)
WEAK(save_boot_params) +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST)) + adr x9, saved_args + stp x0, x1, [x9] + /* Increment the address by 16 bytes for the next pair of values */ + stp x2, x3, [x9, #16] +#endif b save_boot_params_ret /* back to my caller */ ENDPROC(save_boot_params) + +.section .data +.global saved_args +saved_args: + .rept 4 + .xword 0 + .endr +END(saved_args)
On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao raymond.mao@linaro.org wrote:
Save boot arguments x[0-3] into an array for handover of bloblist from previous boot stage.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- New patch file created for v2.
arch/arm/cpu/armv8/start.S | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index 6cc1d26e5e..8e704f590e 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -370,5 +370,19 @@ ENTRY(c_runtime_cpu_setup) ENDPROC(c_runtime_cpu_setup)
WEAK(save_boot_params) +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST))
adr x9, saved_argsstp x0, x1, [x9]/* Increment the address by 16 bytes for the next pair of values */
That seems obvious, but I suppose a comment here is fine.
stp x2, x3, [x9, #16]+#endif b save_boot_params_ret /* back to my caller */ ENDPROC(save_boot_params)
+.section .data +.global saved_args +saved_args:
.rept 4.xword 0.endr+END(saved_args)
2.25.1
Regards, Simon
On Fri, 22 Dec 2023 at 23:32, Raymond Mao raymond.mao@linaro.org wrote:
Save boot arguments x[0-3] into an array for handover of bloblist from previous boot stage.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- New patch file created for v2.
arch/arm/cpu/armv8/start.S | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S index 6cc1d26e5e..8e704f590e 100644 --- a/arch/arm/cpu/armv8/start.S +++ b/arch/arm/cpu/armv8/start.S @@ -370,5 +370,19 @@ ENTRY(c_runtime_cpu_setup) ENDPROC(c_runtime_cpu_setup)
WEAK(save_boot_params) +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST))
adr x9, saved_argsstp x0, x1, [x9]/* Increment the address by 16 bytes for the next pair of values */stp x2, x3, [x9, #16]+#endif b save_boot_params_ret /* back to my caller */ ENDPROC(save_boot_params)
+.section .data +.global saved_args +saved_args:
.rept 4.xword 0.endr+END(saved_args)
2.25.1
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
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.mao@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; + + /* 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; +} + void enable_caches(void) { icache_enable(); diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig index c010c25a92..418f48001c 100644 --- a/configs/qemu_arm64_defconfig +++ b/configs/qemu_arm64_defconfig @@ -69,3 +69,6 @@ CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_PCI=y CONFIG_SEMIHOSTING=y CONFIG_TPM=y +CONFIG_BLOBLIST=y +CONFIG_BLOBLIST_ADDR=0x40004000 +CONFIG_BLOBLIST_SIZE=0x4000
Hi Raymond,
On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao raymond.mao@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.mao@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] ?
/* 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
void enable_caches(void) { icache_enable(); diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig index c010c25a92..418f48001c 100644 --- a/configs/qemu_arm64_defconfig +++ b/configs/qemu_arm64_defconfig @@ -69,3 +69,6 @@ CONFIG_USB_EHCI_HCD=y CONFIG_USB_EHCI_PCI=y CONFIG_SEMIHOSTING=y CONFIG_TPM=y +CONFIG_BLOBLIST=y +CONFIG_BLOBLIST_ADDR=0x40004000
+CONFIG_BLOBLIST_SIZE=0x4000
2.25.1
Regards, Simon
Hi Simon,
On Tue, 26 Dec 2023 at 04:48, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao raymond.mao@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.mao@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?
[...]
Regards, Raymond
Hi Raymond,
On Wed, Dec 27, 2023 at 4:21 PM Raymond Mao raymond.mao@linaro.org wrote:
Hi Simon,
On Tue, 26 Dec 2023 at 04:48, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao raymond.mao@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.mao@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
During bloblist initialization, when CONFIG_OF_BOARD is defined, invoke the platform custom function to load the bloblist via boot arguments from the previous loader. If the bloblist exists, copy it into the fixed bloblist memory region.
Signed-off-by: Raymond Mao raymond.mao@linaro.org --- common/bloblist.c | 47 ++++++++++++++++++++++++++++------------------ include/bloblist.h | 16 ++++++++++++++++ 2 files changed, 45 insertions(+), 18 deletions(-)
diff --git a/common/bloblist.c b/common/bloblist.c index 5ad1db137a..e66afcde64 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -492,31 +492,38 @@ int bloblist_init(void) bool fixed = IS_ENABLED(CONFIG_BLOBLIST_FIXED); int ret = -ENOENT; ulong addr, size; - bool expected; - - /** - * We don't expect to find an existing bloblist in the first phase of - * U-Boot that runs. Also we have no way to receive the address of an - * allocated bloblist from a previous stage, so it must be at a fixed + /* + * If U-Boot is not in the first phase, an existing bloblist must be + * at a fixed address. + */ + bool from_addr = fixed && !u_boot_first_phase(); + /* + * If U-Boot is in the first phase that a board specific routine should + * install the bloblist passed from previous loader to this fixed * address. */ - expected = fixed && !u_boot_first_phase(); + bool from_board = fixed && IS_ENABLED(CONFIG_OF_BOARD) && + u_boot_first_phase(); + if (spl_prev_phase() == PHASE_TPL && !IS_ENABLED(CONFIG_TPL_BLOBLIST)) - expected = false; + from_addr = false; if (fixed) addr = IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED, CONFIG_BLOBLIST_ADDR); size = CONFIG_BLOBLIST_SIZE; - if (expected) { + + if (from_board) + ret = board_bloblist_from_boot_arg(addr, size); + else if (from_addr) ret = bloblist_check(addr, size); - if (ret) { - log_warning("Expected bloblist at %lx not found (err=%d)\n", - addr, ret); - } else { - /* Get the real size, if it is not what we expected */ - size = gd->bloblist->total_size; - } - } + + if (ret) + log_warning("Bloblist at %lx not found (err=%d)\n", + addr, ret); + else + /* Get the real size */ + size = gd->bloblist->total_size; + if (ret) { if (CONFIG_IS_ENABLED(BLOBLIST_ALLOC)) { void *ptr = memalign(BLOBLIST_ALIGN, size); @@ -525,7 +532,8 @@ int bloblist_init(void) return log_msg_ret("alloc", -ENOMEM); addr = map_to_sysmem(ptr); } else if (!fixed) { - return log_msg_ret("!fixed", ret); + return log_msg_ret("BLOBLIST_FIXED is not enabled", + ret); } log_debug("Creating new bloblist size %lx at %lx\n", size, addr); @@ -538,6 +546,9 @@ int bloblist_init(void) return log_msg_ret("ini", ret); gd->flags |= GD_FLG_BLOBLIST_READY;
+ bloblist_show_stats(); + bloblist_show_list(); + return 0; }
diff --git a/include/bloblist.h b/include/bloblist.h index c1dab11f78..2f4246576e 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -445,6 +445,22 @@ int bloblist_reloc(void *to, uint to_size); */ int bloblist_init(void);
+#if (IS_ENABLED(CONFIG_ARCH_QEMU) && IS_ENABLED(CONFIG_ARM)) +/* Board custom function for qemu-arm */ +int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size); +#else +/* + * A board need to implement this custom function if it needs to retrieve + * bloblist from a previous loader + */ +static inline +int board_bloblist_from_boot_arg(unsigned long __always_unused addr, + unsigned long __always_unused size) +{ + return -1; +} +#endif + #if CONFIG_IS_ENABLED(BLOBLIST) /** * bloblist_maybe_init() - Init the bloblist system if not already done
Hi Raymond,
On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao raymond.mao@linaro.org wrote:
During bloblist initialization, when CONFIG_OF_BOARD is defined, invoke the platform custom function to load the bloblist via boot arguments from the previous loader. If the bloblist exists, copy it into the fixed bloblist memory region.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
common/bloblist.c | 47 ++++++++++++++++++++++++++++------------------ include/bloblist.h | 16 ++++++++++++++++ 2 files changed, 45 insertions(+), 18 deletions(-)
We should not be using OF_BOARD for this...particularly as I understood our intent was to make it go away.
For now I would like to have OF_BLOBLIST, with OF_BOARD being the fallback for when we need board-specific things. See [1]. It is also very important that U-Boot reports that the DT came from a bloblist.
Perhaps I should send just that patch so that Ilias and I can figure it out.
diff --git a/common/bloblist.c b/common/bloblist.c index 5ad1db137a..e66afcde64 100644 --- a/common/bloblist.c +++ b/common/bloblist.c @@ -492,31 +492,38 @@ int bloblist_init(void) bool fixed = IS_ENABLED(CONFIG_BLOBLIST_FIXED); int ret = -ENOENT; ulong addr, size;
bool expected;/*** We don't expect to find an existing bloblist in the first phase of* U-Boot that runs. Also we have no way to receive the address of an* allocated bloblist from a previous stage, so it must be at a fixed
/** If U-Boot is not in the first phase, an existing bloblist must be* at a fixed address.
BTW there is now BLOBLIST_ALLOC
*/bool from_addr = fixed && !u_boot_first_phase();/** If U-Boot is in the first phase that a board specific routine should* install the bloblist passed from previous loader to this fixed * address. */
expected = fixed && !u_boot_first_phase();
bool from_board = fixed && IS_ENABLED(CONFIG_OF_BOARD) &&u_boot_first_phase();if (spl_prev_phase() == PHASE_TPL && !IS_ENABLED(CONFIG_TPL_BLOBLIST))
expected = false;
from_addr = false; if (fixed) addr = IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED, CONFIG_BLOBLIST_ADDR); size = CONFIG_BLOBLIST_SIZE;
if (expected) {
if (from_board)ret = board_bloblist_from_boot_arg(addr, size);else if (from_addr) ret = bloblist_check(addr, size);
if (ret) {log_warning("Expected bloblist at %lx not found (err=%d)\n",addr, ret);} else {/* Get the real size, if it is not what we expected */size = gd->bloblist->total_size;}}
if (ret)log_warning("Bloblist at %lx not found (err=%d)\n",addr, ret);else/* Get the real size */size = gd->bloblist->total_size;if (ret) { if (CONFIG_IS_ENABLED(BLOBLIST_ALLOC)) { void *ptr = memalign(BLOBLIST_ALIGN, size);@@ -525,7 +532,8 @@ int bloblist_init(void) return log_msg_ret("alloc", -ENOMEM); addr = map_to_sysmem(ptr); } else if (!fixed) {
return log_msg_ret("!fixed", ret);
return log_msg_ret("BLOBLIST_FIXED is not enabled",ret); } log_debug("Creating new bloblist size %lx at %lx\n", size, addr);@@ -538,6 +546,9 @@ int bloblist_init(void) return log_msg_ret("ini", ret); gd->flags |= GD_FLG_BLOBLIST_READY;
bloblist_show_stats();bloblist_show_list();
Can you put those two lines behind a BLOBLIST_DEBUG option, perhaps?
return 0;}
diff --git a/include/bloblist.h b/include/bloblist.h index c1dab11f78..2f4246576e 100644 --- a/include/bloblist.h +++ b/include/bloblist.h @@ -445,6 +445,22 @@ int bloblist_reloc(void *to, uint to_size); */ int bloblist_init(void);
+#if (IS_ENABLED(CONFIG_ARCH_QEMU) && IS_ENABLED(CONFIG_ARM)) +/* Board custom function for qemu-arm */ +int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size); +#else +/*
- A board need to implement this custom function if it needs to retrieve
- bloblist from a previous loader
- */
+static inline +int board_bloblist_from_boot_arg(unsigned long __always_unused addr,
unsigned long __always_unused size)+{
return -1;+} +#endif
#if CONFIG_IS_ENABLED(BLOBLIST) /**
- bloblist_maybe_init() - Init the bloblist system if not already done
-- 2.25.1
Regards, Simon
[1] https://patchwork.ozlabs.org/project/uboot/patch/20230924192536.1812799-37-s...
Update the document and Kconfig to describe the behavior of board specific custom functions when CONFIG_OF_BOARD is defined.
Signed-off-by: Raymond Mao raymond.mao@linaro.org --- doc/develop/devicetree/control.rst | 6 +++--- dts/Kconfig | 7 +++++-- 2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/doc/develop/devicetree/control.rst b/doc/develop/devicetree/control.rst index cbb65c9b17..e061f4e812 100644 --- a/doc/develop/devicetree/control.rst +++ b/doc/develop/devicetree/control.rst @@ -104,9 +104,9 @@ in u-boot.bin so you can still just flash u-boot.bin onto your board. If you are using CONFIG_SPL_FRAMEWORK, then u-boot.img will be built to include the device tree binary.
-If CONFIG_OF_BOARD is defined, a board-specific routine will provide the -devicetree at runtime, for example if an earlier bootloader stage creates -it and passes it to U-Boot. +If CONFIG_OF_BOARD is defined, board-specific routines will provide the +bloblist and devicetree at runtime, for example if an earlier bootloader stage +creates it and passes it to U-Boot.
If CONFIG_SANDBOX is defined, then it will be read from a file on startup. Use the -d flag to U-Boot to specify the file to read, -D for the diff --git a/dts/Kconfig b/dts/Kconfig index 00c0aeff89..12d61dc748 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -110,8 +110,11 @@ config OF_BOARD default y if SANDBOX || OF_HAS_PRIOR_STAGE help If this option is enabled, the device tree is provided at runtime by - a custom function called board_fdt_blob_setup(). The board must - implement this function if it wishes to provide special behaviour. + a custom function called board_fdt_blob_setup(). + If this option is enabled, bloblist is provided at runtime by a + custom function called board_bloblist_from_boot_arg(). + The board must implement these functions if it wishes to provide + special behaviour.
With this option, the device tree build by U-Boot may be overridden or ignored. See OF_HAS_PRIOR_STAGE.
Hi Raymond,
On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao raymond.mao@linaro.org wrote:
Update the document and Kconfig to describe the behavior of board specific custom functions when CONFIG_OF_BOARD is defined.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
doc/develop/devicetree/control.rst | 6 +++--- dts/Kconfig | 7 +++++-- 2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/doc/develop/devicetree/control.rst b/doc/develop/devicetree/control.rst index cbb65c9b17..e061f4e812 100644 --- a/doc/develop/devicetree/control.rst +++ b/doc/develop/devicetree/control.rst @@ -104,9 +104,9 @@ in u-boot.bin so you can still just flash u-boot.bin onto your board. If you are using CONFIG_SPL_FRAMEWORK, then u-boot.img will be built to include the device tree binary.
-If CONFIG_OF_BOARD is defined, a board-specific routine will provide the -devicetree at runtime, for example if an earlier bootloader stage creates -it and passes it to U-Boot. +If CONFIG_OF_BOARD is defined, board-specific routines will provide the +bloblist and devicetree at runtime, for example if an earlier bootloader stage +creates it and passes it to U-Boot.
If CONFIG_SANDBOX is defined, then it will be read from a file on startup. Use the -d flag to U-Boot to specify the file to read, -D for the diff --git a/dts/Kconfig b/dts/Kconfig index 00c0aeff89..12d61dc748 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -110,8 +110,11 @@ config OF_BOARD default y if SANDBOX || OF_HAS_PRIOR_STAGE help If this option is enabled, the device tree is provided at runtime by
a custom function called board_fdt_blob_setup(). The board mustimplement this function if it wishes to provide special behaviour.
a custom function called board_fdt_blob_setup().If this option is enabled, bloblist is provided at runtime by acustom function called board_bloblist_from_boot_arg().The board must implement these functions if it wishes to providespecial behaviour.
Again, this spec is not board-specific, so it should work on any ARM board, without board-specific code, just by enabling the feature.
With this option, the device tree build by U-Boot may be overridden or ignored. See OF_HAS_PRIOR_STAGE.-- 2.25.1
Regards, SImon
Hi Simon,
On Tue, 26 Dec 2023 at 04:48, Simon Glass sjg@chromium.org wrote:
Hi Raymond,
On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao raymond.mao@linaro.org wrote:
Update the document and Kconfig to describe the behavior of board specific custom functions when CONFIG_OF_BOARD is defined.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
doc/develop/devicetree/control.rst | 6 +++--- dts/Kconfig | 7 +++++-- 2 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/doc/develop/devicetree/control.rst
b/doc/develop/devicetree/control.rst
index cbb65c9b17..e061f4e812 100644 --- a/doc/develop/devicetree/control.rst +++ b/doc/develop/devicetree/control.rst @@ -104,9 +104,9 @@ in u-boot.bin so you can still just flash u-boot.bin
onto your board. If you are
using CONFIG_SPL_FRAMEWORK, then u-boot.img will be built to include
the device
tree binary.
-If CONFIG_OF_BOARD is defined, a board-specific routine will provide the -devicetree at runtime, for example if an earlier bootloader stage
creates
-it and passes it to U-Boot. +If CONFIG_OF_BOARD is defined, board-specific routines will provide the +bloblist and devicetree at runtime, for example if an earlier
bootloader stage
+creates it and passes it to U-Boot.
If CONFIG_SANDBOX is defined, then it will be read from a file on startup. Use the -d flag to U-Boot to specify the file to read, -D for
the
diff --git a/dts/Kconfig b/dts/Kconfig index 00c0aeff89..12d61dc748 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -110,8 +110,11 @@ config OF_BOARD default y if SANDBOX || OF_HAS_PRIOR_STAGE help If this option is enabled, the device tree is provided at
runtime by
a custom function called board_fdt_blob_setup(). The board mustimplement this function if it wishes to provide specialbehaviour.
a custom function called board_fdt_blob_setup().If this option is enabled, bloblist is provided at runtime by acustom function called board_bloblist_from_boot_arg().The board must implement these functions if it wishes toprovide
special behaviour.Again, this spec is not board-specific, so it should work on any ARM board, without board-specific code, just by enabling the feature.
dts/Kconfig is for all platforms. And from the view of a non-ARM platform,
this is still platform-specific.
[...]
Regards, Raymond
Get devicetree from a bloblist if it exists. If not, fallback to get FDT from the specified memory address.
Signed-off-by: Raymond Mao raymond.mao@linaro.org --- Changes in v2 - Refactor of board_fdt_blob_setup().
board/emulation/qemu-arm/qemu-arm.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c index e225011bf0..d326668caf 100644 --- a/board/emulation/qemu-arm/qemu-arm.c +++ b/board/emulation/qemu-arm/qemu-arm.c @@ -149,9 +149,17 @@ int dram_init_banksize(void)
void *board_fdt_blob_setup(int *err) { + void *fdt = NULL; *err = 0; - /* QEMU loads a generated DTB for us at the start of RAM. */ - return (void *)CFG_SYS_SDRAM_BASE; + + /* Check if a DTB exists in bloblist */ + if (IS_ENABLED(CONFIG_BLOBLIST) && !bloblist_maybe_init()) + fdt = bloblist_find(BLOBLISTT_CONTROL_FDT, 0); + if (!fdt) + /* QEMU loads a generated DTB for us at the start of RAM. */ + return (void *)CFG_SYS_SDRAM_BASE; + + return fdt; }
int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)
Hi,
On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao raymond.mao@linaro.org wrote:
Get devicetree from a bloblist if it exists. If not, fallback to get FDT from the specified memory address.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- Refactor of board_fdt_blob_setup().
board/emulation/qemu-arm/qemu-arm.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
I believe this patch will not be needed, since we have fdtdec_setup() to do this.
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c index e225011bf0..d326668caf 100644 --- a/board/emulation/qemu-arm/qemu-arm.c +++ b/board/emulation/qemu-arm/qemu-arm.c @@ -149,9 +149,17 @@ int dram_init_banksize(void)
void *board_fdt_blob_setup(int *err) {
void *fdt = NULL; *err = 0;
/* QEMU loads a generated DTB for us at the start of RAM. */return (void *)CFG_SYS_SDRAM_BASE;
/* Check if a DTB exists in bloblist */if (IS_ENABLED(CONFIG_BLOBLIST) && !bloblist_maybe_init())fdt = bloblist_find(BLOBLISTT_CONTROL_FDT, 0);if (!fdt)/* QEMU loads a generated DTB for us at the start of RAM. */return (void *)CFG_SYS_SDRAM_BASE;return fdt;}
int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)
2.25.1
Regards, Simon
On Tue, 26 Dec 2023 at 11:48, Simon Glass sjg@chromium.org wrote:
Hi,
On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao raymond.mao@linaro.org wrote:
Get devicetree from a bloblist if it exists. If not, fallback to get FDT from the specified memory address.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- Refactor of board_fdt_blob_setup().
board/emulation/qemu-arm/qemu-arm.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
I believe this patch will not be needed, since we have fdtdec_setup() to do this.
Yep, it's not. The board-specific portion should be limited to what happens if the bloblist is *not* detected by fdtdec_setup() (assuming the user wants to allow a fallback)
diff --git a/board/emulation/qemu-arm/qemu-arm.c b/board/emulation/qemu-arm/qemu-arm.c index e225011bf0..d326668caf 100644 --- a/board/emulation/qemu-arm/qemu-arm.c +++ b/board/emulation/qemu-arm/qemu-arm.c @@ -149,9 +149,17 @@ int dram_init_banksize(void)
void *board_fdt_blob_setup(int *err) {
void *fdt = NULL; *err = 0;
/* QEMU loads a generated DTB for us at the start of RAM. */return (void *)CFG_SYS_SDRAM_BASE;
/* Check if a DTB exists in bloblist */if (IS_ENABLED(CONFIG_BLOBLIST) && !bloblist_maybe_init())fdt = bloblist_find(BLOBLISTT_CONTROL_FDT, 0);if (!fdt)/* QEMU loads a generated DTB for us at the start of RAM. */return (void *)CFG_SYS_SDRAM_BASE;return fdt;}
int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)
2.25.1
Regards, Simon
Hi Ilias and Simon,
On Wed, 27 Dec 2023 at 04:32, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
On Tue, 26 Dec 2023 at 11:48, Simon Glass sjg@chromium.org wrote:
Hi,
On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao raymond.mao@linaro.org
wrote:
Get devicetree from a bloblist if it exists. If not, fallback to get FDT from the specified memory address.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- Refactor of board_fdt_blob_setup().
board/emulation/qemu-arm/qemu-arm.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
I believe this patch will not be needed, since we have fdtdec_setup() to do this.
Yep, it's not. The board-specific portion should be limited to what happens if the bloblist is *not* detected by fdtdec_setup() (assuming the user wants to allow a fallback)
But I think whether we allow a fallback or not is really something that
should be considered by a board vendor other than a common logic in fdtdec_setup(). And a board vendor can also decide whether to allow a FDT that comes from bloblist by this custom function.
[...]
Regards, Raymond
On Wed, 27 Dec 2023 at 17:43, Raymond Mao raymond.mao@linaro.org wrote:
Hi Ilias and Simon,
On Wed, 27 Dec 2023 at 04:32, Ilias Apalodimas ilias.apalodimas@linaro.org wrote:
On Tue, 26 Dec 2023 at 11:48, Simon Glass sjg@chromium.org wrote:
Hi,
On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao raymond.mao@linaro.org wrote:
Get devicetree from a bloblist if it exists. If not, fallback to get FDT from the specified memory address.
Signed-off-by: Raymond Mao raymond.mao@linaro.org
Changes in v2
- Refactor of board_fdt_blob_setup().
board/emulation/qemu-arm/qemu-arm.c | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-)
I believe this patch will not be needed, since we have fdtdec_setup() to do this.
Yep, it's not. The board-specific portion should be limited to what happens if the bloblist is *not* detected by fdtdec_setup() (assuming the user wants to allow a fallback)
But I think whether we allow a fallback or not is really something that should be considered by a board vendor other than a common logic in fdtdec_setup().
It's not. The bloblist is coming from the standard register calling convention. Doesn't it make more sense to have the code - Scan for a bloblist if BLOBLIST is enabled - Fallback to board-specific methods if that register does not contain a bloblist, or contains one without a DT entry
And a board vendor can also decide whether to allow a FDT that comes from bloblist by this custom function.
I am not sure I understand this one. That decision should be in u-boot proper, the vendor should only be allowed to provide his special DT if we dont find it in a transfer list
Thanks /Ilias
[...]
Regards, Raymond
participants (3)
-
Ilias Apalodimas -
Raymond Mao -
Simon Glass