Hi Mathew, On Thu, 6 Apr 2023 at 12:25, Mathew McBride <m...@traverse.com.au> wrote: > > 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 <m...@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 <m...@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);
Yes this looks right to me. I am actually changing the same line in a current series, so I can pick up this patch and include it in the series. > > strlcpy(dirname, bflow->fname, sizeof(dirname)); > last_slash = strrchr(dirname, '/'); > -- > 2.30.1 > > > Regards, Simon