[PATCH v2 00/16] pxe: Allow extlinux booting without CMDLINE enabled

[Tom Rini]

On Sun, Apr 14, 2024 at 06:58:03PM +0200, Jonas Karlman wrote:
> Hi Tom and Simon,
> 
> On 2024-04-11 03:45, Tom Rini wrote:
> > On Thu, 14 Dec 2023 21:18:58 -0700, Simon Glass wrote:
> > 
> >> This series is the culmanation of the current line of refactoring
> >> series. It adjusts pxe to call the booting functionality directly
> >> rather than going through the command-line interface.
> >>
> >> With this is is possible to boot using the extlinux bootmeth without
> >> the command line enabled.
> >>
> >> [...]
> > 
> > Applied to u-boot/master, thanks!
> > 
> 
> This series is causing boot issues using extlinux in bootm_run_states():
> 
>   ERROR: booting os 'Invalid OS' (0) is not supported
> 
> Following extlinux.conf was used:
> 
>   label linux
>   	kernel /Image.gz
>   	initrd /initramfs.cpio.gz
> 
> Before this series booting works, bootm_run_states() is first called
> with states=0x1 (BOOTM_STATE_START):
> 
>   Scanning bootdev 'mmc at fe2b0000.bootdev':
>     1  extlinux     ready   mmc          1  mmc at fe2b0000.bootdev.part /extlinux/extlinux.conf
>   ** Booting bootflow 'mmc at fe2b0000.bootdev.part_1' with extlinux
>   1:      linux
>   Retrieving file: /Image.gz
>   Retrieving file: /initramfs.cpio.gz
>   bootm_run_states(): images->state: 0, states: 1
>   bootm_run_states(): images->os.os: 0
>   bootm_run_states(): images->os.arch: 0
>   bootm_run_states(): boot_fn: 0000000000000000, need_boot_fn: 0
>      Uncompressing Kernel Image to 0
>   ## Flattened Device Tree blob at edef8410
>      Booting using the fdt blob at 0xedef8410
>   Working FDT set to edef8410
>   bootm_run_states(): images->state: 1, states: 1710
>      Loading Ramdisk to ecdfd000, end eceb274d ... OK
>   bootm_run_states(): images->os.os: 5
>   bootm_run_states(): images->os.arch: 16
>   boot_fn: 00000000eff2b83c, need_boot_fn: 0
>      Loading Device Tree to 00000000ecde8000, end 00000000ecdfc97f ... OK
>   Working FDT set to ecde8000
> 
> After this series booting fails, bootm_run_states() is first called
> with states=0x1710.
> 
>   Scanning bootdev 'mmc at fe2b0000.bootdev':
>     1  extlinux     ready   mmc          1  mmc at fe2b0000.bootdev.part /extlinux/extlinux.conf
>   ** Booting bootflow 'mmc at fe2b0000.bootdev.part_1' with extlinux
>   1:      linux
>   Retrieving file: /Image.gz
>   Retrieving file: /initramfs.cpio.gz
>   bootm_run_states(): images->state: 0, states: 1710
>   bootm_run_states(): images->os.os: 0
>   bootm_run_states(): images->os.arch: 0
>   bootm_run_states(): boot_fn: 0000000000000000, need_boot_fn: 0
>   ERROR: booting os 'Invalid OS' (0) is not supported
>   Boot failed (err=-14)
> 
> Looks like booti_start() -> bootm_run_states(bmi, BOOTM_STATE_START) is
> no longer called due to changes in this series.

I think the problem is with:
commit 6d803ec9cc757bda0c48f2fd419cb6878eab92c4
Author: Simon Glass <sjg at chromium.org>
Date:   Thu Dec 14 21:19:12 2023 -0700

    pxe: Allow booting without CMDLINE for bootm methods
    
    Use bootm_run() and booti_run() to boot rather than the command line.
    This allows extlinux to be used without CMDLINE being enabled.
    
    Collect any error but do not return it, to match the existing code.
    
    Signed-off-by: Simon Glass <sjg at chromium.org>

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
[snip]
@@ -641,23 +644,18 @@ static int label_run_boot(struct pxe_context *ctx, struct pxe_label *label,
 	if (!fdt_addr && genimg_get_format(buf) != IMAGE_FORMAT_FIT)
 		fdt_addr = env_get("fdtcontroladdr");
 
-	if (fdt_addr) {
-		if (!bootm_argv[2])
-			bootm_argv[2] = "-";
-		bootm_argc = 4;
-	}
-	bootm_argv[3] = (char *)fdt_addr;
+	bmi.conf_fdt = fdt_addr;
 
 	/* Try bootm for legacy and FIT format image */
 	if (genimg_get_format(buf) != IMAGE_FORMAT_INVALID &&
-            IS_ENABLED(CONFIG_CMD_BOOTM))
-		do_bootm(ctx->cmdtp, 0, bootm_argc, bootm_argv);
+	    IS_ENABLED(CONFIG_BOOTM))
+		ret = bootm_run(&bmi);
 	/* Try booting an AArch64 Linux kernel image */
-	else if (IS_ENABLED(CONFIG_CMD_BOOTI))
-		do_booti(ctx->cmdtp, 0, bootm_argc, bootm_argv);
+	else if (IS_ENABLED(CONFIG_BOOTM))
+		ret = booti_run(&bmi);
 	/* Try booting a Image */
-	else if (IS_ENABLED(CONFIG_CMD_BOOTZ))
-		do_bootz(ctx->cmdtp, 0, bootm_argc, bootm_argv);
+	else if (IS_ENABLED(CONFIG_BOOTM))
+		ret = bootz_run(&bmi);
 	/* Try booting an x86_64 Linux kernel image */
 	else if (IS_ENABLED(CONFIG_CMD_ZBOOT))
 		do_zboot_parent(ctx->cmdtp, 0, zboot_argc, zboot_argv, NULL);

And this doesn't seem equivalent really. The logic used to be checking
if we had bootm/booti/bootz and now it's always checking for
CONFIG_BOOTM. Jonas, can you please share the defconfig you used here as
well? But I think for now reverting the series is the best path forward,
unfortunately.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240418/386592c2/attachment.sig>