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

Tom Rini
2022-05-18T15:53:07Z

[-- Attachment #1: Type: text/plain, Size: 5577 bytes --]

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@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

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.

--
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]