Re: [PATCH] cmd: pxe_utils: Check fdtcontroladdr in label_boot

[Neil Armstrong <narmstrong@baylibre.com>]

Hi,

On 14/10/2021 10:40, Peter Hoyes wrote:
> From: Peter Hoyes <Peter.Hoyes@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@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

Neil