On 18/05/2022 19:15, Neil Armstrong wrote:
On 18/05/2022 17:52, Tom Rini wrote:
On Wed, May 18, 2022 at 10:40:12AM +0200, Neil Armstrong wrote:
Hi,
On 14/10/2021 10:40, Peter Hoyes wrote:
From: Peter Hoyes <peter.ho...@arm.com>
If using OF_CONTROL, fdtcontroladdr is set to the fdt used to
configure
U-Boot. When using PXE, if no fdt is defined in the menu file, and
there is no fdt at fdt_addr, add fall back on fdtcontroladdr too.
We are developing board support for the Armv8r64 FVP using
config_distro_bootcmd. We are also using OF_BOARD and would like the
PXE boot option to default to the fdt provided by
board_fdt_blob_setup.
Signed-off-by: Peter Hoyes <peter.ho...@arm.com>
---
cmd/pxe_utils.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/cmd/pxe_utils.c b/cmd/pxe_utils.c
index 067c24e5ff..8f8e69ca97 100644
--- a/cmd/pxe_utils.c
+++ b/cmd/pxe_utils.c
@@ -556,7 +556,10 @@ static int label_boot(struct cmd_tbl *cmdtp,
struct pxe_label *label)
* Scenario 2: If there is an fdt_addr specified, pass it
along to
* bootm, and adjust argc appropriately.
*
- * Scenario 3: fdt blob is not available.
+ * Scenario 3: If there is an fdtcontroladdr specified, pass
it along to
+ * bootm, and adjust argc appropriately.
+ *
+ * Scenario 4: fdt blob is not available.
*/
bootm_argv[3] = env_get("fdt_addr_r");
@@ -646,6 +649,9 @@ static int label_boot(struct cmd_tbl *cmdtp,
struct pxe_label *label)
if (!bootm_argv[3])
bootm_argv[3] = env_get("fdt_addr");
+ if (!bootm_argv[3])
+ bootm_argv[3] = env_get("fdtcontroladdr");
+
if (bootm_argv[3]) {
if (!bootm_argv[2])
bootm_argv[2] = "-";
This change makes a regression when using a FIT image as kernel
image within an extlinux.conf
Before this change, when a DT wasn't specified, the DT in the FIT
image was selected,
not the u-boot DT is selected.
While it should work because the U-Boot DT should work in Linux,
it's not always the case.
Before:
Found /extlinux/extlinux.conf
Retrieving file: /extlinux/extlinux.conf
1: Yocto
Retrieving file: /fitImage
append: root=PARTUUID=fd26d57f-02 rootwait console=ttyAML0,115200
## Loading kernel from FIT Image at 08080000 ...
Using 'conf-amlogic_meson-gxl-s905x-libretech-cc.dtb' configuration
Trying 'kernel-1' kernel subimage
Description: Linux kernel
Type: Kernel Image
Compression: gzip compressed
Data Start: 0x08080120
Data Size: 9926956 Bytes = 9.5 MiB
Architecture: AArch64
OS: Linux
Load Address: 0x01080000
Entry Point: 0x01080000
Hash algo: sha256
Hash value:
5181a76e4e7a728e24cd8569f8e48c543ac259bf4d66591a3dc5e166d709429e
Verifying Hash Integrity ... sha256+ OK
## Loading fdt from FIT Image at 08080000 ...
Using 'conf-amlogic_meson-gxl-s905x-libretech-cc.dtb' configuration
Trying 'fdt-amlogic_meson-gxl-s905x-libretech-cc.dtb' fdt subimage
Description: Flattened Device Tree blob
Type: Flat Device Tree
Compression: uncompressed
Data Start: 0x089f7b78
Data Size: 29092 Bytes = 28.4 KiB
Architecture: AArch64
Hash algo: sha256
Hash value:
72e5e4fcbb4aa59042377720e5636132ba790d85b6c3f6442446acc63f48cf67
Verifying Hash Integrity ... sha256+ OK
Booting using the fdt blob at 0x89f7b78
Uncompressing Kernel Image
Loading Device Tree to 000000007bf2f000, end 000000007bf391a3
... OK
After:
Found /extlinux/extlinux.conf
Retrieving file: /extlinux/extlinux.conf
1: Yocto
Retrieving file: /fitImage
append: root=PARTUUID=fd26d57f-02 rootwait console=ttyAML0,115200
## Loading kernel from FIT Image at 08080000 ...
Using 'conf-amlogic_meson-gxl-s905x-libretech-cc.dtb' configuration
Trying 'kernel-1' kernel subimage
Description: Linux kernel
Type: Kernel Image
Compression: gzip compressed
Data Start: 0x08080120
Data Size: 9926956 Bytes = 9.5 MiB
Architecture: AArch64
OS: Linux
Load Address: 0x01080000
Entry Point: 0x01080000
Hash algo: sha256
Hash value:
5181a76e4e7a728e24cd8569f8e48c543ac259bf4d66591a3dc5e166d709429e
Verifying Hash Integrity ... sha256+ OK
## Flattened Device Tree blob at 7bf40210
Booting using the fdt blob at 0x7bf40210
Uncompressing Kernel Image
Loading Device Tree to 000000007bf2c000, end 000000007bf38ba7
... OK
Note this is the "best in town" boot method in Yocto/OE when using
WIC, FIT Image and
generating extlinux.conf from the WIC "bootimg-partition" plugin.
So this change will regress all platforms using this scheme.
In case we only want the FIT image and not DTB, the fdtdir is
ommited at:
https://github.com/openembedded/openembedded-core/blob/9015dec93233c7d45fd0c9885ff5d4ec23ad377d/scripts/lib/wic/plugins/source/bootimg-partition.py#L150
Well this is certainly tricky. Do we be "boring" and say that the
built-in device tree should be used, or "boring" and say that the distro
provided device tree should be used. I assume that in the OE case we're
not always going to be booting U-Boot that was built, so updating the
logic there to have the correct dtb isn't right either.
I'm not against this logic, it should definitely be aligned on EFI and
permit
using built-in DT, but the logic breaks fitImage loading via extlinux,
because
now we have no way to say "use the fitImage DT".
This change should have been elsewhere, like in the non-FIT bootm path
when
no DT was supplied and DT is mandatory for the platform.
Or by passing a keyword like fdt-built-in or ftdfile=builtin.
I agree the correct logic should be:
* Use the extlinux.conf fdt file, if defined
* Use a FIT fdt, if available
* Use fdtcontroladdr, as a last resort
, so FIT images was an oversight.
==><=======================================================
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 0c24becae3..22040c2340 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -682,7 +682,7 @@ static int label_boot(struct pxe_context *ctx,
struct pxe_label *label)
fdtfile = fdtfilefree;
}
- if (fdtfile) {
+ if (strcmp(fdtfile, "builtin") && fdtfile) {
int err = get_relfile_envaddr(ctx, fdtfile,
"fdt_addr_r", NULL);
@@ -724,7 +724,7 @@ static int label_boot(struct pxe_context *ctx,
struct pxe_label *label)
if (!bootm_argv[3])
bootm_argv[3] = env_get("fdt_addr");
- if (!bootm_argv[3])
+ if (!strcmp(fdtfile, "builtin"))
bootm_argv[3] = env_get("fdtcontroladdr");
if (bootm_argv[3]) {
==><=======================================================
Or by checking the image format before using fdtcontroladdr like:
==><=======================================================
diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index 0c24becae3..f4f6369de6 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -724,7 +724,7 @@ static int label_boot(struct pxe_context *ctx,
struct pxe_label *label)
if (!bootm_argv[3])
bootm_argv[3] = env_get("fdt_addr");
- if (!bootm_argv[3])
+ if (genimg_get_format(buf) != IMAGE_FORMAT_FIT && !bootm_argv[3])
bootm_argv[3] = env_get("fdtcontroladdr");
if (bootm_argv[3]) {
==><=======================================================
Neil
Your second diff above seems like the least invasive way of achieving
this behavior.
I did wonder whether it would make sense to add a global
'fdtcontroladdr' fallback for bootm (i.e. in boot_get_fdt in
image-fdt.c), so that:
bootm $kernel_addr_r
falls back to $fdtcontroladdr (if valid) as a last resort. Then we could
remove the branch in pxe_utils, and the FIT logic would happen in the
right order. It looks like bootefi already does something similar, but I
guess modifying the bootm behavior would affect a lot of existing boards.
Peter
