Re: [BUG] issues with new bootflow, uefi and virtio

["Mathew McBride" <matt@traverse.com.au>]

Hi Vincent,

On Thu, Apr 6, 2023, at 1:04 AM, Vincent Stehlé wrote:
> Hi,
> 
> I am hitting an issue with the new bootflow when booting with UEFI from a
> virtio device on Qemu Arm.
> 
> It seems the device number computation in efiload_read_file() does not work
> in the general virtio case, where it will pick the virtio device number
> instead of the block device index. On Qemu arm virt machine, many virtio
> mmio devices are provisioned in the memory map and no assumption can be
> made on the number of the actual virtio device in use in the general case.
> 
> This is an extract of the output of `dm tree' on this platform, focused on
> the virtio device from which I would like to boot:
> 
>   virtio    31  [ + ]   virtio-mmio     |-- virtio_mmio@a003e00
>   blk        0  [ + ]   virtio-blk      |   |-- virtio-blk#31
>   partition  0  [ + ]   blk_partition   |   |   |-- virtio-blk#31:1
>   partition  1  [ + ]   blk_partition   |   |   `-- virtio-blk#31:2
>   bootdev    2  [ + ]   virtio_bootdev  |   `-- virtio-blk#31.bootdev
> 
> In this extract the actual virtio device number is 31, as will be picked by
> efiload_read_file(), but the desired block device index is zero, as would
> be used with e.g. `ls virtio 0'.

I came across the exact same issue a few days ago. Below is a patch which I believe fixes the problem, by using the devnum of blk uclass (virtio 0) instead of the sequence number of the parent udevice (e.g virtio-blk#35).

Separately, the devnum was previously being parsed as a hex which meant "virtio_blk#35" was actually being parsed as "virtio_blk#23". That confused me for a while.

If the patch looks good I can re-post it directly to the ML. I'm not 100% sure that I got it right.

In case the email mangles the patch, you can grab a diff here as well: https://gitlab.com/traversetech/ls1088firmware/u-boot/-/commit/5ed3315b4a297f143fb84f44117b5b31e5617af5

- Matt

------------

From 5ed3315b4a297f143fb84f44117b5b31e5617af5 Mon Sep 17 00:00:00 2001
From: Mathew McBride <matt@traverse.com.au>
Date: Wed, 5 Apr 2023 02:44:48 +0000
Subject: [PATCH] bootstd: use blk uclass device numbers to set efi bootdev

When loading a file from a block device, efiload_read_file
was using the seq_num of the device (e.g "35" of virtio_blk#35)
instead of the block device id (e.g what you get from running
the corresponding device scan command, like "virtio 0")

This cause EFI booting from these devices to fail as an
invalid device number is passed to blk_get_device_part_str:

Scanning bootdev 'virtio-blk#35.bootdev':
distro_efi_read_bootflow_file start (efi,fname=<NULL>)
distro_efi_read_bootflow_file start (efi,fname=<NULL>)
setting bootdev virtio, 35, efi/boot/bootaa64.efi, 00000000beef9a40, 170800
efi_dp_from_name calling blk_get_device_part_str
dev=virtio devnr=35 path=efi/boot/bootaa64.efi
blk_get_device_part_str (virtio,35)
blk_get_device_by_str (virtio, 35)
** Bad device specification virtio 35 **
Using default device tree: dtb/qemu-arm.dtb
No device tree available
0  efi          ready   virtio       1  virtio-blk#35.bootdev.par efi/boot/bootaa64.efi
** Booting bootflow 'virtio-blk#35.bootdev.part_1' with efi
blk_get_device_part_str (virtio,0:1)
blk_get_device_by_str (virtio, 0)
No UEFI binary known at beef9a40 (image buf=00000000beef9a40,addr=0000000000000000)
Boot failed (err=-22)

Signed-off-by: Mathew McBride <matt@traverse.com.au>
---
 boot/bootmeth_efi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index 6a97ac02ff..bc106aa736 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -117,7 +117,7 @@ static int efiload_read_file(struct blk_desc *desc, struct bootflow *bflow)
 	 * this can go away.
 	 */
 	media_dev = dev_get_parent(bflow->dev);
-	snprintf(devnum_str, sizeof(devnum_str), "%x", dev_seq(media_dev));
+	snprintf(devnum_str, sizeof(devnum_str), "%d", desc->devnum);
 
 	strlcpy(dirname, bflow->fname, sizeof(dirname));
 	last_slash = strrchr(dirname, '/');
-- 
2.30.1