[PATCH] cmd: pxe_utils: Check fdtcontroladdr in label_boot
Neil Armstrong
narmstrong at baylibre.com
Wed May 18 20:15:23 CEST 2022
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.Hoyes at 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.Hoyes at 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.
==><=======================================================
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
More information about the U-Boot
mailing list