Re: U-Boot: extlinux.conf boot menu support appears to be broken by commit 739e8361f3fe78038251216df6096a32bc2d5839

[Manuel Traut <manuel.traut@mt.com>]

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

Hi Karsten,

thanks for your report.

Can you test with the attached patch?
I do not have access to a hardware today, but I will test it tomorrow.

Regards
Manuel

Am 16.01.2023 um 19:50 hat Karsten Merker geschrieben:
> Hello,
> 
> it looks like the following commit has broken the extlinux.conf
> boot menu support in U-Boot:
> 
> -----8<----------8<----------8<----------8<----------8<----------8<-----
> commit 739e8361f3fe78038251216df6096a32bc2d5839
> Author: Manuel Traut <manuel.traut@mt.com>
> Date:   Fri Nov 18 09:00:27 2022 +0100
> 
>     distro/pxeboot: Handle prompt variable
>     
>     Regarding the documentation found here:
>     https://github.com/u-boot/u-boot/blob/master/common/menu.c#L347
>     
>     If both timeout and prompt is set to 0 the default entry shall
>     be booted immediately. However the current behaviour is that
>     the prompt is shown (tested with distroboot) until the user
>     selects an entry (no timeout).
>     
>     This change implements a behaviour as documented. It was tested
>     with distroboot.
>     
>     Signed-off-by: Manuel Traut <manuel.traut@mt.com>
>     Reviewed-by: Simon Glass <sjg@chromium.org>
>     Reviewed-by: Simon Glass <sjg@chromium.org>
> -----8<----------8<----------8<----------8<----------8<----------8<-----
> 
> Since the commit above, a system with the following
> /boot/extlinux/extlinux.conf (which sets timeout to 50)
> immediately boots the first entry in the config without
> displaying a boot menu.  According to the description, that
> should only happen if both prompt and timeout are set to zero in
> the config, but it also happens with timeout set to a non-zero
> value as in the config below:
> 
> -----8<----------8<----------8<----------8<----------8<----------8<-----
> ## /boot/extlinux/extlinux.conf
> ##
> ## IMPORTANT WARNING
> ##
> ## The configuration of this file is generated automatically.
> ## Do not edit this file manually, use: u-boot-update
> 
> default l0
> menu title U-Boot menu
> prompt 0
> timeout 50
> 
> 
> label l0
> 	menu label Debian GNU/Linux bookworm/sid 6.1.0-1-riscv64
> 	linux /boot/vmlinux-6.1.0-1-riscv64
> 	initrd /boot/initrd.img-6.1.0-1-riscv64
> 	
> 	
> 	append root=/dev/vda1 rw noquiet
> 
> label l0r
> 	menu label Debian GNU/Linux bookworm/sid 6.1.0-1-riscv64 (rescue target)
> 	linux /boot/vmlinux-6.1.0-1-riscv64
> 	initrd /boot/initrd.img-6.1.0-1-riscv64
> 	
> 	append root=/dev/vda1 rw noquiet single
> 	
> 
> label l1
> 	menu label Debian GNU/Linux bookworm/sid 6.0.0-6-riscv64
> 	linux /boot/vmlinux-6.0.0-6-riscv64
> 	initrd /boot/initrd.img-6.0.0-6-riscv64
> 	
> 	
> 	append root=/dev/vda1 rw noquiet
> 
> label l1r
> 	menu label Debian GNU/Linux bookworm/sid 6.0.0-6-riscv64 (rescue target)
> 	linux /boot/vmlinux-6.0.0-6-riscv64
> 	initrd /boot/initrd.img-6.0.0-6-riscv64
> 	
> 	append root=/dev/vda1 rw noquiet single
> 	
> 
> label l2
> 	menu label Debian GNU/Linux bookworm/sid 6.0.0-5-riscv64
> 	linux /boot/vmlinux-6.0.0-5-riscv64
> 	initrd /boot/initrd.img-6.0.0-5-riscv64
> 	
> 	
> 	append root=/dev/vda1 rw noquiet
> 
> label l2r
> 	menu label Debian GNU/Linux bookworm/sid 6.0.0-5-riscv64 (rescue target)
> 	linux /boot/vmlinux-6.0.0-5-riscv64
> 	initrd /boot/initrd.img-6.0.0-5-riscv64
> 	
> 	append root=/dev/vda1 rw noquiet single
> -----8<----------8<----------8<----------8<----------8<----------8<-----
> 
> Up to commit 51c5c28af59c19407d6602336ac7374dd1432c49 the boot process
> with the extlinux.conf above looks as follows:
> 
> -----8<----------8<----------8<----------8<----------8<----------8<-----
> Hit any key to stop autoboot:  0 
> 
> Device 0: QEMU VirtIO Block Device
>             Type: Hard Disk
>             Capacity: 102400.0 MB = 100.0 GB (209715200 x 512)
> ... is now current device
> Scanning virtio 0:1...
> Found /boot/extlinux/extlinux.conf
> Retrieving file: /boot/extlinux/extlinux.conf
> U-Boot menu
> 1:	Debian GNU/Linux bookworm/sid 6.1.0-1-riscv64
> 2:	Debian GNU/Linux bookworm/sid 6.1.0-1-riscv64 (rescue target)
> 3:	Debian GNU/Linux bookworm/sid 6.0.0-6-riscv64
> 4:	Debian GNU/Linux bookworm/sid 6.0.0-6-riscv64 (rescue target)
> 5:	Debian GNU/Linux bookworm/sid 6.0.0-5-riscv64
> 6:	Debian GNU/Linux bookworm/sid 6.0.0-5-riscv64 (rescue target)
> Enter choice:    
> -----8<----------8<----------8<----------8<----------8<----------8<-----
> 
> The user can then either select a boot menu entry or - if there is
> no user input - the system boots the first entry after the timeout
> set in extlinux.conf is over.
> 
> Starting with commit 739e8361f3fe78038251216df6096a32bc2d5839,
> the boot process looks as follows, i.e. there isn't a boot menu
> anymore but the system immediately boots the first entry:
> 
> -----8<----------8<----------8<----------8<----------8<----------8<-----
> Device 0: QEMU VirtIO Block Device
>             Type: Hard Disk
>             Capacity: 102400.0 MB = 100.0 GB (209715200 x 512)
> ... is now current device
> Scanning virtio 0:1...
> Found /boot/extlinux/extlinux.conf
> Retrieving file: /boot/extlinux/extlinux.conf
> 1:	Debian GNU/Linux bookworm/sid 6.1.0-1-riscv64
> Retrieving file: /boot/vmlinux-6.1.0-1-riscv64
> Retrieving file: /boot/initrd.img-6.1.0-1-riscv64
> append: root=/dev/vda1 rw noquiet
> Moving Image from 0x84000000 to 0x80200000, end=815e5000
> ## Flattened Device Tree blob at ff7344b0
>    Booting using the fdt blob at 0xff7344b0
> Working FDT set to ff7344b0
>    Using Device Tree in place at 00000000ff7344b0, end 00000000ff738dea
> Working FDT set to ff7344b0
> 
> Starting kernel ...
> [...]
> -----8<----------8<----------8<----------8<----------8<----------8<-----
> 
> Regards,
> Karsten
> -- 
> Hiermit widerspreche ich ausdrücklich der Nutzung sowie der Weitergabe
> meiner personenbezogenen Daten für Zwecke der Werbung sowie der Markt-
> oder Meinungsforschung.

-- 
Manuel Traut

[-- Attachment #2: 0001-menu-Ignore-prompt-variable-if-timeout-is-0.patch --]
[-- Type: text/x-diff, Size: 1130 bytes --]

From 1dd799c08384b21e7f441a8b4731a0e02d5e28d9 Mon Sep 17 00:00:00 2001
From: Manuel Traut <manuel.traut@mt.com>
Date: Tue, 17 Jan 2023 09:30:27 +0100
Subject: [PATCH] menu: Ignore prompt variable if timeout is != 0

Since 739e8361f3fe78038251216df6096a32bc2d5839, a system with the following
/boot/extlinux/extlinux.conf (which sets timeout to 50) immediately boots the
first entry in the config without displaying a boot menu.  According to the
description, that should only happen if both prompt and timeout are set to zero
in the config, but it also happens with timeout set to a non-zero value.

Reported-by: Karsten Merker <merker@debian.org>
Signed-off-by: Manuel Traut <manuel.traut@mt.com>
---
 common/menu.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/common/menu.c b/common/menu.c
index 8fe00965c0..8eabcccc87 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -277,6 +277,9 @@ int menu_get_choice(struct menu *m, void **choice)
 	if (!m->item_cnt)
 		return -ENOENT;
 
+	if (m->timeout)
+		return menu_interactive_choice(m, choice);
+
 	if (!m->prompt)
 		return menu_default_choice(m, choice);
 
-- 
2.39.0