| Message ID | 20230405150458.890460-1-vincent.stehle@arm.com |
|---|---|
| State | Not Applicable |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [BUG] issues with new bootflow, uefi and virtio | expand |
Hi Vincent, On Thu, 6 Apr 2023 at 03:05, Vincent Stehlé <vincent.stehle@arm.com> 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'. This is strange. Can you try 'dm uclass' to see the sequence number for the virtio device? I would expect it to be 0, but I might not fully understand virtio. > > This can be reproduced for example with Buildroot > qemu_aarch64_ebbr_defconfig or qemu_arm_ebbr_defconfig and updating the > U-Boot version to v2023.04. This was working properly with U-Boot versions > up to v2023.01. > > This seems to be very specific to virtio, as the numbers align much more > nicely for e.g. USB mass storage or NVMe. > > To help debugging, the following patch forces the device number to zero in > the case of virtio, which allows to boot again with UEFI and virtio on Qemu > Arm. Hopefully this should give a hint of what is going on. > > I tried to create a fix by looking for the first child device of > UCLASS_BLK, and use its devnum instead in the case of virtio. Sadly, I am > not able to confirm if this is a proper fix as I am hitting other boot > issues in the case of multiple boot devices of the same class it seems... Please also see this: https://patchwork.ozlabs.org/project/uboot/patch/20230402140231.v7.3.Ifa423a8f295b3c11e50821222b0db1e869d0c051@changeid/ (or the whole series) > > Vincent. > > [1]: https://buildroot.org > > --- > boot/bootmeth_efi.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c > index 6a97ac02ff5..e5b0d8614ff 100644 > --- a/boot/bootmeth_efi.c > +++ b/boot/bootmeth_efi.c > @@ -117,7 +117,9 @@ 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), "%x", > + device_get_uclass_id(media_dev) == UCLASS_VIRTIO ? 0 : > + dev_seq(media_dev)); > > strlcpy(dirname, bflow->fname, sizeof(dirname)); > last_slash = strrchr(dirname, '/'); > -- > 2.39.2 > Regards, Simon
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, '/');
On Thu, Apr 06, 2023 at 10:25:15AM +1000, Mathew McBride wrote: .. > 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). Hi Mathew, Thank you for this patch; it works for me and is much simpler that what I had in mind. - Your patch repairs efi with virtio, for Qemu arm and aarch64. - It does not break efi on USB and NVMe (on Qemu). Best regards, Vincent.
On Thu, Apr 06, 2023 at 06:38:16AM +1200, Simon Glass wrote: (virtio device number 31 vs. blk index 0) > This is strange. Can you try 'dm uclass' to see the sequence number > for the virtio device? I would expect it to be 0, but I might not > fully understand virtio. Hi Simon, Thank you for looking into this. Here is the `dm uclass' extract corresponding to virtio on this Qemu system: uclass 126: virtio 0 * virtio_mmio@a000000 @ 7ee977d0, seq 0 1 * virtio_mmio@a000200 @ 7ee97870, seq 1 2 * virtio_mmio@a000400 @ 7ee97910, seq 2 3 * virtio_mmio@a000600 @ 7ee979b0, seq 3 4 * virtio_mmio@a000800 @ 7ee97a50, seq 4 5 * virtio_mmio@a000a00 @ 7ee97af0, seq 5 6 * virtio_mmio@a000c00 @ 7ee97b90, seq 6 7 * virtio_mmio@a000e00 @ 7ee97c30, seq 7 8 * virtio_mmio@a001000 @ 7ee97cd0, seq 8 9 * virtio_mmio@a001200 @ 7ee97d70, seq 9 10 * virtio_mmio@a001400 @ 7ee97e10, seq 10 11 * virtio_mmio@a001600 @ 7ee97eb0, seq 11 12 * virtio_mmio@a001800 @ 7ee97f50, seq 12 13 * virtio_mmio@a001a00 @ 7ee97ff0, seq 13 14 * virtio_mmio@a001c00 @ 7ee98090, seq 14 15 * virtio_mmio@a001e00 @ 7ee98130, seq 15 16 * virtio_mmio@a002000 @ 7ee981d0, seq 16 17 * virtio_mmio@a002200 @ 7ee98270, seq 17 18 * virtio_mmio@a002400 @ 7ee98310, seq 18 19 * virtio_mmio@a002600 @ 7ee983b0, seq 19 20 * virtio_mmio@a002800 @ 7ee98450, seq 20 21 * virtio_mmio@a002a00 @ 7ee984f0, seq 21 22 * virtio_mmio@a002c00 @ 7ee98590, seq 22 23 * virtio_mmio@a002e00 @ 7ee98630, seq 23 24 * virtio_mmio@a003000 @ 7ee986d0, seq 24 25 * virtio_mmio@a003200 @ 7ee98770, seq 25 26 * virtio_mmio@a003400 @ 7ee98810, seq 26 27 * virtio_mmio@a003600 @ 7ee988b0, seq 27 28 * virtio_mmio@a003800 @ 7ee98950, seq 28 29 * virtio_mmio@a003a00 @ 7ee989f0, seq 29 30 * virtio_mmio@a003c00 @ 7ee98a90, seq 30 31 * virtio_mmio@a003e00 @ 7ee98b30, seq 31 (issue with multiple virtio devices) > Please also see this: > > https://patchwork.ozlabs.org/project/uboot/patch/20230402140231.v7.3.Ifa423a8f295b3c11e50821222b0db1e869d0c051@changeid/ > > (or the whole series) Thank you for this patch! When combined with the patch from Mathew[1], it does indeed repair the case of efi boot with two virtio disks, specifically when the first virtio disk is the one we want to boot from. FWIW, the system will not boot when I invert the two virtio disks. Best regards, Vincent. [1]: https://lists.denx.de/pipermail/u-boot/2023-April/514527.html
Hi Mathew, On Thu, 6 Apr 2023 at 12:25, Mathew McBride <matt@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 <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); 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
Hi Vincent, On Thu, 6 Apr 2023 at 22:05, Vincent Stehlé <vincent.stehle@arm.com> wrote: > > On Thu, Apr 06, 2023 at 06:38:16AM +1200, Simon Glass wrote: > (virtio device number 31 vs. blk index 0) > > This is strange. Can you try 'dm uclass' to see the sequence number > > for the virtio device? I would expect it to be 0, but I might not > > fully understand virtio. > > Hi Simon, > > Thank you for looking into this. Here is the `dm uclass' extract corresponding > to virtio on this Qemu system: > > uclass 126: virtio > 0 * virtio_mmio@a000000 @ 7ee977d0, seq 0 > 1 * virtio_mmio@a000200 @ 7ee97870, seq 1 > 2 * virtio_mmio@a000400 @ 7ee97910, seq 2 > 3 * virtio_mmio@a000600 @ 7ee979b0, seq 3 > 4 * virtio_mmio@a000800 @ 7ee97a50, seq 4 > 5 * virtio_mmio@a000a00 @ 7ee97af0, seq 5 > 6 * virtio_mmio@a000c00 @ 7ee97b90, seq 6 > 7 * virtio_mmio@a000e00 @ 7ee97c30, seq 7 > 8 * virtio_mmio@a001000 @ 7ee97cd0, seq 8 > 9 * virtio_mmio@a001200 @ 7ee97d70, seq 9 > 10 * virtio_mmio@a001400 @ 7ee97e10, seq 10 > 11 * virtio_mmio@a001600 @ 7ee97eb0, seq 11 > 12 * virtio_mmio@a001800 @ 7ee97f50, seq 12 > 13 * virtio_mmio@a001a00 @ 7ee97ff0, seq 13 > 14 * virtio_mmio@a001c00 @ 7ee98090, seq 14 > 15 * virtio_mmio@a001e00 @ 7ee98130, seq 15 > 16 * virtio_mmio@a002000 @ 7ee981d0, seq 16 > 17 * virtio_mmio@a002200 @ 7ee98270, seq 17 > 18 * virtio_mmio@a002400 @ 7ee98310, seq 18 > 19 * virtio_mmio@a002600 @ 7ee983b0, seq 19 > 20 * virtio_mmio@a002800 @ 7ee98450, seq 20 > 21 * virtio_mmio@a002a00 @ 7ee984f0, seq 21 > 22 * virtio_mmio@a002c00 @ 7ee98590, seq 22 > 23 * virtio_mmio@a002e00 @ 7ee98630, seq 23 > 24 * virtio_mmio@a003000 @ 7ee986d0, seq 24 > 25 * virtio_mmio@a003200 @ 7ee98770, seq 25 > 26 * virtio_mmio@a003400 @ 7ee98810, seq 26 > 27 * virtio_mmio@a003600 @ 7ee988b0, seq 27 > 28 * virtio_mmio@a003800 @ 7ee98950, seq 28 > 29 * virtio_mmio@a003a00 @ 7ee989f0, seq 29 > 30 * virtio_mmio@a003c00 @ 7ee98a90, seq 30 > 31 * virtio_mmio@a003e00 @ 7ee98b30, seq 31 > > (issue with multiple virtio devices) OK, thanks for that. I had no idea there would be lots of them. > > Please also see this: > > > > https://patchwork.ozlabs.org/project/uboot/patch/20230402140231.v7.3.Ifa423a8f295b3c11e50821222b0db1e869d0c051@changeid/ > > > > (or the whole series) > > Thank you for this patch! > > When combined with the patch from Mathew[1], it does indeed repair the case of > efi boot with two virtio disks, specifically when the first virtio disk is the > one we want to boot from. > FWIW, the system will not boot when I invert the two virtio disks. Is this because it only uses the first virtio device? You could check your boot_targets variable. With standard boot you can use 'virtio' instead of 'vritio0' and it will find any virtio devices. > > Best regards, > Vincent. > > [1]: https://lists.denx.de/pipermail/u-boot/2023-April/514527.html Regards, Simon
On Fri, Apr 07, 2023 at 05:31:06PM +1200, Simon Glass wrote: .. > > When combined with the patch from Mathew[1], it does indeed repair the case of > > efi boot with two virtio disks, specifically when the first virtio disk is the > > one we want to boot from. > > FWIW, the system will not boot when I invert the two virtio disks. > > Is this because it only uses the first virtio device? You could check > your boot_targets variable. With standard boot you can use 'virtio' > instead of 'vritio0' and it will find any virtio devices. Hi Simon, Thank you for the tips; I did not know that you could use a generic `virtio' or a more specific `virtio0' specification in boot_targets. By default the boot_targets variable does indeed contain the generic `virtio' in my case. Quick tests matrix: disk image virtio (#num) blk index boot_targets (#30) 0 (#31) 1 ------------ ------- ------- virtio ok FAIL virtio0 ok (fail) virtio1 (fail) ok This is with both patches, on Qemu. The fails between () are expected. I find it interesting that specifying `virtio1' does work when the bootable image is on the second virtio disk, while auto-detection with `virtio' does not seem to: virtio1 ~~~~~~~ => setenv boot_targets virtio1 => boot Scanning for bootflows in all bootdevs Seq Method State Uclass Part Name Filename --- ------ ----- ------ ---- ---- -------- Scanning global bootmeth 'efi_mgr': Scanning bootdev 'virtio-blk#31.bootdev': 0 efi ready virtio 1 virtio-blk#31.bootdev.par efi/boot/bootaa64.efi ** Booting bootflow 'virtio-blk#31.bootdev.part_1' with efi Using prior-stage device tree Missing TPMv2 device for EFI_TCG_PROTOCOL Booting /efi\boot\bootaa64.efi virtio ~~~~~~ => setenv boot_targets virtio => boot Scanning for bootflows in all bootdevs Seq Method State Uclass Part Name Filename --- ------ ----- ------ ---- ---- -------- Scanning global bootmeth 'efi_mgr': Scanning bootdev 'virtio-blk#30.bootdev': No more bootdevs --- ------ ----- ------ ---- ---- -------- (0 bootflows, 0 valid) The messages seem to indicate that virtio #31 / 1 was not even considered when specifying `virtio'. (Note that I have edited the logs a bit to avoid wrapping lines.) Best regards, Vincent. > > > > > Best regards, > > Vincent. > > > > [1]: https://lists.denx.de/pipermail/u-boot/2023-April/514527.html > > Regards, > Simon
Hi Vincent, On Tue, 11 Apr 2023 at 06:00, Vincent Stehlé <vincent.stehle@arm.com> wrote: > > On Fri, Apr 07, 2023 at 05:31:06PM +1200, Simon Glass wrote: > .. > > > When combined with the patch from Mathew[1], it does indeed repair the case of > > > efi boot with two virtio disks, specifically when the first virtio disk is the > > > one we want to boot from. > > > FWIW, the system will not boot when I invert the two virtio disks. > > > > Is this because it only uses the first virtio device? You could check > > your boot_targets variable. With standard boot you can use 'virtio' > > instead of 'vritio0' and it will find any virtio devices. > > Hi Simon, > > Thank you for the tips; I did not know that you could use a generic `virtio' or > a more specific `virtio0' specification in boot_targets. > By default the boot_targets variable does indeed contain the generic `virtio' in > my case. > > Quick tests matrix: > > disk image virtio > (#num) blk index > boot_targets (#30) 0 (#31) 1 > ------------ ------- ------- > virtio ok FAIL > virtio0 ok (fail) > virtio1 (fail) ok > > This is with both patches, on Qemu. > The fails between () are expected. > > I find it interesting that specifying `virtio1' does work when the bootable > image is on the second virtio disk, while auto-detection with `virtio' does not > seem to: > > virtio1 > ~~~~~~~ > > => setenv boot_targets virtio1 > => boot > Scanning for bootflows in all bootdevs > Seq Method State Uclass Part Name Filename > --- ------ ----- ------ ---- ---- -------- > Scanning global bootmeth 'efi_mgr': > Scanning bootdev 'virtio-blk#31.bootdev': > 0 efi ready virtio 1 virtio-blk#31.bootdev.par efi/boot/bootaa64.efi > ** Booting bootflow 'virtio-blk#31.bootdev.part_1' with efi > Using prior-stage device tree > Missing TPMv2 device for EFI_TCG_PROTOCOL > Booting /efi\boot\bootaa64.efi > > virtio > ~~~~~~ > > => setenv boot_targets virtio > => boot > Scanning for bootflows in all bootdevs > Seq Method State Uclass Part Name Filename > --- ------ ----- ------ ---- ---- -------- > Scanning global bootmeth 'efi_mgr': > Scanning bootdev 'virtio-blk#30.bootdev': > No more bootdevs > --- ------ ----- ------ ---- ---- -------- > (0 bootflows, 0 valid) > > The messages seem to indicate that virtio #31 / 1 was not even considered when > specifying `virtio'. > (Note that I have edited the logs a bit to avoid wrapping lines.) Hmm I noticed the 'bootflow scan virtio' problem as well but haven't got back to it. Thanks for the detailed bug report. I will take a look soon. Regards, Simon
Hi Vincent, On Mon, 24 Apr 2023 at 13:44, Simon Glass <sjg@chromium.org> wrote: > > Hi Vincent, > > On Tue, 11 Apr 2023 at 06:00, Vincent Stehlé <vincent.stehle@arm.com> wrote: > > > > On Fri, Apr 07, 2023 at 05:31:06PM +1200, Simon Glass wrote: > > .. > > > > When combined with the patch from Mathew[1], it does indeed repair the case of > > > > efi boot with two virtio disks, specifically when the first virtio disk is the > > > > one we want to boot from. > > > > FWIW, the system will not boot when I invert the two virtio disks. > > > > > > Is this because it only uses the first virtio device? You could check > > > your boot_targets variable. With standard boot you can use 'virtio' > > > instead of 'vritio0' and it will find any virtio devices. > > > > Hi Simon, > > > > Thank you for the tips; I did not know that you could use a generic `virtio' or > > a more specific `virtio0' specification in boot_targets. > > By default the boot_targets variable does indeed contain the generic `virtio' in > > my case. > > > > Quick tests matrix: > > > > disk image virtio > > (#num) blk index > > boot_targets (#30) 0 (#31) 1 > > ------------ ------- ------- > > virtio ok FAIL > > virtio0 ok (fail) > > virtio1 (fail) ok > > > > This is with both patches, on Qemu. > > The fails between () are expected. > > > > I find it interesting that specifying `virtio1' does work when the bootable > > image is on the second virtio disk, while auto-detection with `virtio' does not > > seem to: > > > > virtio1 > > ~~~~~~~ > > > > => setenv boot_targets virtio1 > > => boot > > Scanning for bootflows in all bootdevs > > Seq Method State Uclass Part Name Filename > > --- ------ ----- ------ ---- ---- -------- > > Scanning global bootmeth 'efi_mgr': > > Scanning bootdev 'virtio-blk#31.bootdev': > > 0 efi ready virtio 1 virtio-blk#31.bootdev.par efi/boot/bootaa64.efi > > ** Booting bootflow 'virtio-blk#31.bootdev.part_1' with efi > > Using prior-stage device tree > > Missing TPMv2 device for EFI_TCG_PROTOCOL > > Booting /efi\boot\bootaa64.efi > > > > virtio > > ~~~~~~ > > > > => setenv boot_targets virtio > > => boot > > Scanning for bootflows in all bootdevs > > Seq Method State Uclass Part Name Filename > > --- ------ ----- ------ ---- ---- -------- > > Scanning global bootmeth 'efi_mgr': > > Scanning bootdev 'virtio-blk#30.bootdev': > > No more bootdevs > > --- ------ ----- ------ ---- ---- -------- > > (0 bootflows, 0 valid) > > > > The messages seem to indicate that virtio #31 / 1 was not even considered when > > specifying `virtio'. > > (Note that I have edited the logs a bit to avoid wrapping lines.) > > Hmm I noticed the 'bootflow scan virtio' problem as well but haven't > got back to it. Thanks for the detailed bug report. > > I will take a look soon. I believe the problem is actually that it is running out of memory. I sent a patch to allow this to be reported properly (with bootflow scan -lae). The .efi files are quite large so loading them when scanning is probably not a good idea. I will have a rethink of this and see if I can implement lazy loading. It doesn't matter for the simple case where we boot the first thing we find, but it would make 'bootflow scan' more reliable when memory is short and the files are large. It would also fit better into U-Boot's 'lazy init' philosophy. Regards, Simon
Hi Vincent, On Tue, 11 Apr 2023 at 06:00, Vincent Stehlé <vincent.stehle@arm.com> wrote: > > On Fri, Apr 07, 2023 at 05:31:06PM +1200, Simon Glass wrote: > .. > > > When combined with the patch from Mathew[1], it does indeed repair the case of > > > efi boot with two virtio disks, specifically when the first virtio disk is the > > > one we want to boot from. > > > FWIW, the system will not boot when I invert the two virtio disks. > > > > Is this because it only uses the first virtio device? You could check > > your boot_targets variable. With standard boot you can use 'virtio' > > instead of 'vritio0' and it will find any virtio devices. > > Hi Simon, > > Thank you for the tips; I did not know that you could use a generic `virtio' or > a more specific `virtio0' specification in boot_targets. > By default the boot_targets variable does indeed contain the generic `virtio' in > my case. > > Quick tests matrix: > > disk image virtio > (#num) blk index > boot_targets (#30) 0 (#31) 1 > ------------ ------- ------- > virtio ok FAIL > virtio0 ok (fail) > virtio1 (fail) ok > > This is with both patches, on Qemu. > The fails between () are expected. > > I find it interesting that specifying `virtio1' does work when the bootable > image is on the second virtio disk, while auto-detection with `virtio' does not > seem to: > > virtio1 > ~~~~~~~ > > => setenv boot_targets virtio1 > => boot > Scanning for bootflows in all bootdevs > Seq Method State Uclass Part Name Filename > --- ------ ----- ------ ---- ---- -------- > Scanning global bootmeth 'efi_mgr': > Scanning bootdev 'virtio-blk#31.bootdev': > 0 efi ready virtio 1 virtio-blk#31.bootdev.par efi/boot/bootaa64.efi > ** Booting bootflow 'virtio-blk#31.bootdev.part_1' with efi > Using prior-stage device tree > Missing TPMv2 device for EFI_TCG_PROTOCOL > Booting /efi\boot\bootaa64.efi > > virtio > ~~~~~~ > > => setenv boot_targets virtio > => boot > Scanning for bootflows in all bootdevs > Seq Method State Uclass Part Name Filename > --- ------ ----- ------ ---- ---- -------- > Scanning global bootmeth 'efi_mgr': > Scanning bootdev 'virtio-blk#30.bootdev': > No more bootdevs > --- ------ ----- ------ ---- ---- -------- > (0 bootflows, 0 valid) > > The messages seem to indicate that virtio #31 / 1 was not even considered when > specifying `virtio'. > (Note that I have edited the logs a bit to avoid wrapping lines.) > Apart from the out-of-memory problem that has been fixed, there is another problem that it only looks at the first of the virtio devices when boot_targets="virtio", as you have found. I believe I have a fix for this so will send a patch. Thank you for testing this. Regards, Simon > Best regards, > Vincent. > > > > > > > > > Best regards, > > > Vincent. > > > > > > [1]: https://lists.denx.de/pipermail/u-boot/2023-April/514527.html > > > > Regards, > > Simon
diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c index 6a97ac02ff5..e5b0d8614ff 100644 --- a/boot/bootmeth_efi.c +++ b/boot/bootmeth_efi.c @@ -117,7 +117,9 @@ 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), "%x", + device_get_uclass_id(media_dev) == UCLASS_VIRTIO ? 0 : + dev_seq(media_dev)); strlcpy(dirname, bflow->fname, sizeof(dirname)); last_slash = strrchr(dirname, '/');