[PATCH v2] vbe: Allow probing the VBE bootmeth to fail in OS fixup

[PATCH v2] vbe: Allow probing the VBE bootmeth to fail in OS fixup

[Simon Glass]

This device is created when there are no bootmeths defined in the device
tree. But it cannot be probed without a device tree node.

For now, ignore a probe failure.

Signed-off-by: Simon Glass <sjg@chromium.org>
Reported-by: Karsten Merker <merker@debian.org>
Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Fixes: a56f663f0707 ("vbe: Add info about the VBE device to the fwupd node")
---

Changes in v2:
- With 'with' typo
- Change to a debug message and add a comment

 boot/vbe_simple_os.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/boot/vbe_simple_os.c b/boot/vbe_simple_os.c
index b2041a95a30..8c641ec07e2 100644
--- a/boot/vbe_simple_os.c
+++ b/boot/vbe_simple_os.c
@@ -72,6 +72,18 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct event *event)
 		chosen = oftree_path(tree, "/chosen");
 		if (!ofnode_valid(chosen))
 			continue;
+
+		ret = device_probe(dev);
+		if (ret) {
+			/*
+			 * This should become an error when VBE is updated to
+			 * only bind this device when a node exists
+			 */
+			log_debug("VBE device '%s' failed to probe (err=%d)",
+				  dev->name, ret);
+			return 0;
+		}
+
 		ret = ofnode_add_subnode(chosen, "fwupd", &node);
 		if (ret && ret != -EEXIST)
 			return log_msg_ret("fwu", ret);
@@ -80,10 +92,6 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct event *event)
 		if (ret && ret != -EEXIST)
 			return log_msg_ret("dev", ret);
 
-		ret = device_probe(dev);
-		if (ret)
-			return log_msg_ret("probe", ret);
-
 		/* Copy over the vbe properties for fwupd */
 		log_debug("Fixing up: %s\n", dev->name);
 		ret = ofnode_copy_props(dev_ofnode(dev), subnode);
-- 
2.39.0.314.g84b9a713c41-goog

Re: [PATCH v2] vbe: Allow probing the VBE bootmeth to fail in OS fixup

[Vagrant Cascadian]

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

On 2023-01-12, Simon Glass wrote:
> This device is created when there are no bootmeths defined in the device
> tree. But it cannot be probed without a device tree node.
>
> For now, ignore a probe failure.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reported-by: Karsten Merker <merker@debian.org>
> Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Fixes: a56f663f0707 ("vbe: Add info about the VBE device to the fwupd node")

I was able to reproduce the issue using the qemu-riscv64 instructions
Karsten provided, and applying the patch fixes it, thanks!

Tested-by: Vagrant Cascadian <vagrant@debian.org>

live well,
  vagrant

> ---
>
> Changes in v2:
> - With 'with' typo
> - Change to a debug message and add a comment
>
>  boot/vbe_simple_os.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/boot/vbe_simple_os.c b/boot/vbe_simple_os.c
> index b2041a95a30..8c641ec07e2 100644
> --- a/boot/vbe_simple_os.c
> +++ b/boot/vbe_simple_os.c
> @@ -72,6 +72,18 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct event *event)
>  		chosen = oftree_path(tree, "/chosen");
>  		if (!ofnode_valid(chosen))
>  			continue;
> +
> +		ret = device_probe(dev);
> +		if (ret) {
> +			/*
> +			 * This should become an error when VBE is updated to
> +			 * only bind this device when a node exists
> +			 */
> +			log_debug("VBE device '%s' failed to probe (err=%d)",
> +				  dev->name, ret);
> +			return 0;
> +		}
> +
>  		ret = ofnode_add_subnode(chosen, "fwupd", &node);
>  		if (ret && ret != -EEXIST)
>  			return log_msg_ret("fwu", ret);
> @@ -80,10 +92,6 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct event *event)
>  		if (ret && ret != -EEXIST)
>  			return log_msg_ret("dev", ret);
>  
> -		ret = device_probe(dev);
> -		if (ret)
> -			return log_msg_ret("probe", ret);
> -
>  		/* Copy over the vbe properties for fwupd */
>  		log_debug("Fixing up: %s\n", dev->name);
>  		ret = ofnode_copy_props(dev_ofnode(dev), subnode);

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

Re: [PATCH v2] vbe: Allow probing the VBE bootmeth to fail in OS fixup

[Karsten Merker]

On Thu, Jan 12, 2023 at 04:48:54PM -0700 Simon Glass wrote:

> This device is created when there are no bootmeths defined in the device
> tree. But it cannot be probed without a device tree node.
> 
> For now, ignore a probe failure.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reported-by: Karsten Merker <merker@debian.org>
> Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Fixes: a56f663f0707 ("vbe: Add info about the VBE device to the fwupd node")

Tested-by: Karsten Merker <merker@debian.org>

Hello,

applying this patch fixes the boot failure on the riscv64
qemu virt platform:

U-Boot 2023.01-00001-g5c392f7ced (Jan 15 2023 - 21:34:26 +0100)

CPU:   rv64imafdch_zicsr_zifencei_zihintpause_zba_zbb_zbc_zbs_sstc
Model: riscv-virtio,qemu
DRAM:  8 GiB
Core:  31 devices, 15 uclasses, devicetree: board
Flash: 32 MiB
Loading Environment from nowhere... OK
In:    serial@10000000
Out:   serial@10000000
Err:   serial@10000000
Net:   eth0: virtio-net#2
Working FDT set to ff7344b0
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: 1:	Debian GNU/Linux bookworm/sid 6.1.0-1-riscv64
Retrieving file: /boot/initrd.img-6.1.0-1-riscv64
Retrieving file: /boot/vmlinux-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 ...

[    0.000000] Linux version 6.1.0-1-riscv64 (debian-kernel@lists.debian.org) (gcc-12 (Debian 12.2.0-11) 12.2.0, GNU ld (GNU Binutils for Debian) 2.39.90.20221231) #1 SMP Debian 6.1.4-1 (2023-01-07)
[    0.000000] random: crng init done
[    0.000000] OF: fdt: Ignoring memory range 0x80000000 - 0x80200000
[    0.000000] Machine model: riscv-virtio,qemu
[...]

Regards and many thanks,
Karsten
-- 
Hiermit widerspreche ich ausdrücklich der Nutzung sowie der Weitergabe
meiner personenbezogenen Daten für Zwecke der Werbung sowie der Markt-
oder Meinungsforschung.

Re: [PATCH v2] vbe: Allow probing the VBE bootmeth to fail in OS fixup

[Tom Rini]

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

On Thu, Jan 12, 2023 at 04:48:54PM -0700, Simon Glass wrote:

> This device is created when there are no bootmeths defined in the device
> tree. But it cannot be probed without a device tree node.
> 
> For now, ignore a probe failure.
> 
> Signed-off-by: Simon Glass <sjg@chromium.org>
> Reported-by: Karsten Merker <merker@debian.org>
> Suggested-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Fixes: a56f663f0707 ("vbe: Add info about the VBE device to the fwupd node")
> Tested-by: Vagrant Cascadian <vagrant@debian.org>
> Tested-by: Karsten Merker <merker@debian.org>

Applied to u-boot/master, thanks!

-- 
Tom

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