[PATCH 0/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[PATCH 0/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Christoph Willing]

Use of Qemu's -kernel option (thus also -initrd & -append) is currently not
working correctly under UEFI boot. The nominated kernel is loaded and the
initrd is opened successfully but there is no access to the VM filesystem.
Booting without the -kernel option i.e. using the VM's internal kernel & intird
works as expected with UEFI.

This behaviour has been observed in all of the four Linux systems tested for
verification.

The commit at which this behaviour appears  has been identified and the patch
proposed here just reverses it i.e. we now run TryRunningQemuKernel() after
PlatformBdsConnectSequence() instead of before it. When the proposed patch is
applied, all four systems are subsequently able to boot correctly under UEFI.

Christoph Willing (1):
  OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.32.0

[PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Christoph Willing]

Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3504

This change ensures that PlatformBdsConnectSequence()  is run
before TryRunningQemuKernel().

It reverses commit a34a886962561f6d8550b2a1bb193798ca456431 which aimed
to improve UEFI boot time. Unfortunately that commit now results in boot
failure when using qemu's -kernel option, therefore rendering the -initrd
& -append options useless.

Such failures are seen in all releases from edk2-stable201808 to
edk2-stable202105 as well as the master branch.

The last releases which actually work correctly are vUDK2017 and vUDK2018
as they don't contain the a34a88 commit.

This change has been tested with edk2-stable201808, edk2-stable201811,
edk2-stable202105 as well as master (at 21/7/2021). It results in correct
booting using the -kernel option in all the test cases. No measurable
change in boot times was discernible compared with the unaffected vUDK2017
& vUDK2018 releases.

Signed-off-by: Christoph Willing <chris.willing@linux.com>
---
 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
index b0e9742937..f08ace87b3 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
@@ -1517,14 +1517,14 @@ PlatformBootManagerAfterConsole (
   Tcg2PhysicalPresenceLibProcessRequest (NULL);
 
   //
-  // Process QEMU's -kernel command line option
+  // Perform some platform specific connect sequence
   //
-  TryRunningQemuKernel ();
+  PlatformBdsConnectSequence ();
 
   //
-  // Perform some platform specific connect sequence
+  // Process QEMU's -kernel command line option
   //
-  PlatformBdsConnectSequence ();
+  TryRunningQemuKernel ();
 
   EfiBootManagerRefreshAllBootOption ();
 
-- 
2.32.0

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Ard Biesheuvel]

On Wed, 28 Jul 2021 at 04:18, Christoph Willing <chris.willing@linux.com> wrote:
>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3504
>
> This change ensures that PlatformBdsConnectSequence()  is run
> before TryRunningQemuKernel().
>
> It reverses commit a34a886962561f6d8550b2a1bb193798ca456431 which aimed
> to improve UEFI boot time. Unfortunately that commit now results in boot
> failure when using qemu's -kernel option, therefore rendering the -initrd
> & -append options useless.
>
> Such failures are seen in all releases from edk2-stable201808 to
> edk2-stable202105 as well as the master branch.
>

Can we pinpoint the failure condition a bit more? I heavily rely on
-kernel for my local OVMF testing and other uses, and I am pretty sure
it hasn't been broken for 3 years.

So what else are you doing that triggers this failure? Is it related
to boot order, the type of devices, etc?


> The last releases which actually work correctly are vUDK2017 and vUDK2018
> as they don't contain the a34a88 commit.
>
> This change has been tested with edk2-stable201808, edk2-stable201811,
> edk2-stable202105 as well as master (at 21/7/2021). It results in correct
> booting using the -kernel option in all the test cases. No measurable
> change in boot times was discernible compared with the unaffected vUDK2017
> & vUDK2018 releases.
>
> Signed-off-by: Christoph Willing <chris.willing@linux.com>
> ---
>  OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index b0e9742937..f08ace87b3 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -1517,14 +1517,14 @@ PlatformBootManagerAfterConsole (
>    Tcg2PhysicalPresenceLibProcessRequest (NULL);
>
>    //
> -  // Process QEMU's -kernel command line option
> +  // Perform some platform specific connect sequence
>    //
> -  TryRunningQemuKernel ();
> +  PlatformBdsConnectSequence ();
>
>    //
> -  // Perform some platform specific connect sequence
> +  // Process QEMU's -kernel command line option
>    //
> -  PlatformBdsConnectSequence ();
> +  TryRunningQemuKernel ();
>
>    EfiBootManagerRefreshAllBootOption ();
>
> --
> 2.32.0
>
>
>
> ------------
> Groups.io Links: You receive all messages sent to this group.
> View/Reply Online (#78261): https://edk2.groups.io/g/devel/message/78261
> Mute This Topic: https://groups.io/mt/84497355/5717338
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ardb+tianocore@kernel.org]
> ------------
>
>

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Christoph Willing]

With soft feature freeze started, I wonder if this patch could be
reviewed and pushed for edk2-stable202108 tag? I think it has languished
because I didn't initially Cc appropriately - pls add others as necessary.

This patch is a trivial (I think) change which fixes a long standing and
annoying bug for those booting Qemu with UEFI using external kernel &
initrd.

Thanks,
chris

On 28/7/21 12:02 pm, Christoph Willing wrote:
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3504
> 
> This change ensures that PlatformBdsConnectSequence()  is run
> before TryRunningQemuKernel().
> 
> It reverses commit a34a886962561f6d8550b2a1bb193798ca456431 which aimed
> to improve UEFI boot time. Unfortunately that commit now results in boot
> failure when using qemu's -kernel option, therefore rendering the -initrd
> & -append options useless.
> 
> Such failures are seen in all releases from edk2-stable201808 to
> edk2-stable202105 as well as the master branch.
> 
> The last releases which actually work correctly are vUDK2017 and vUDK2018
> as they don't contain the a34a88 commit.
> 
> This change has been tested with edk2-stable201808, edk2-stable201811,
> edk2-stable202105 as well as master (at 21/7/2021). It results in correct
> booting using the -kernel option in all the test cases. No measurable
> change in boot times was discernible compared with the unaffected vUDK2017
> & vUDK2018 releases.
> 
> Signed-off-by: Christoph Willing <chris.willing@linux.com>
> ---
>  OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index b0e9742937..f08ace87b3 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -1517,14 +1517,14 @@ PlatformBootManagerAfterConsole (
>    Tcg2PhysicalPresenceLibProcessRequest (NULL);
>  
>    //
> -  // Process QEMU's -kernel command line option
> +  // Perform some platform specific connect sequence
>    //
> -  TryRunningQemuKernel ();
> +  PlatformBdsConnectSequence ();
>  
>    //
> -  // Perform some platform specific connect sequence
> +  // Process QEMU's -kernel command line option
>    //
> -  PlatformBdsConnectSequence ();
> +  TryRunningQemuKernel ();
>  
>    EfiBootManagerRefreshAllBootOption ();
>  
> 

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[James Bottomley]

On Mon, 2021-08-09 at 22:53 +1000, Christoph Willing wrote:
> With soft feature freeze started, I wonder if this patch could be
> reviewed and pushed for edk2-stable202108 tag? I think it has
> languished because I didn't initially Cc appropriately - pls add
> others as necessary.
> 
> This patch is a trivial (I think) change which fixes a long standing
> and annoying bug for those booting Qemu with UEFI using external
> kernel & initrd.

I'm with Ard on this one: -kernel is working just fine for me and the
team at IBM working on Kata containers.  It sounds like this might be a
problem local to your environment, so we need to debug it to understand
the issue rather than blindly reverse existing commits.

Regards,

James

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Christoph Willing]

On 10/8/21 12:52 am, James Bottomley wrote:
> On Mon, 2021-08-09 at 22:53 +1000, Christoph Willing wrote:
>> With soft feature freeze started, I wonder if this patch could be
>> reviewed and pushed for edk2-stable202108 tag? I think it has
>> languished because I didn't initially Cc appropriately - pls add
>> others as necessary.
>>
>> This patch is a trivial (I think) change which fixes a long standing
>> and annoying bug for those booting Qemu with UEFI using external
>> kernel & initrd.
> 
> I'm with Ard on this one: -kernel is working just fine for me and the
> team at IBM working on Kata containers.  It sounds like this might be a
> problem local to your environment, so we need to debug it to understand
> the issue rather than blindly reverse existing commits.
> 
Thanks for responding James & Ard.

Below is the script I'm using to create, then run, the VM. To verify
that it works normally with UEFI boot, it initially uses the internal
kernel & initrd.

The OVMF_CODE & my_VARS lines contain git hash to identify the build
from which OVMF_CODE.fd & OVMF_VARS.fd were taken; 97fdcg is from a
build of yesterday's git master.

After the OS has been installed, I can run the VM multiple times to
verify that it boots under UEFI OK (I see the TianoCore splash screen)
with internal kernel.


#!/bin/bash

/usr/bin/qemu-kvm \
-name "UEFI Testing" \
-enable-kvm \
-cpu kvm64 \
-smp cores=4 \
-boot once=c \
-m 8192 \
-device intel-hda \
-device hda-duplex \
-vga virtio \
-drive if=pflash,format=raw,file=OVMF_CODE_97fdcb.fd,readonly=on \
-drive if=pflash,format=raw,file=my_VARS_97fdcb.fd \
-drive file=disk.img,format=raw,cache=none,index=0,media=disk \
-cdrom
/storage/iso/slackware/slackware64-15.0/slackware64-15.0-20210807.iso \
-daemonize \
"$@"


To now use external kernel, I add the lines:

-kernel /var/cache/vmbuilder/boot/15.0/x86_64/vmlinuz \
-initrd /var/cache/vmbuilder/boot/15.0/x86_64/initrd \
-append "root=/dev/sda2 rootfstype=ext4 ro vga=0x386" \

to the script just after "-boot once=c" (but I doubt the exact
positioning makes any difference).

In this case, I see the kernel running and initrd unpacked and its
modules loaded but the root partition is unable to be mounted - the disk
is not visible (running 'ls -l /dev/sd*' in recovery shell gives 'ls:
/dev/sd*: No such file or directory').

The last lines of the Qemu screen are:

/boot/initrd-5.13.8.gz: Loading kernel modules from initrd image:
insmod /lib/modules/5.13.8/kernel/fs/jbd2/jbd2.ko
insmod /lib/modules/5.13.8/kernel/fs/mbcache.ko
insmod /lib/modules/5.13.8/kernel/fs/ext4/ext4.ko
mount: mounting /dev/sda2 on /mnt failed: No such file or directory
ERROR: No /sbin/init found on rootdev (or not mounted). Trouble ahead.
       You can try to fix it. Type 'exit' when things are done.

At that point I'm dropped into a recovery shell to try fixing something
but there's nothing that can be done since the disk containing the OS is
not visible.


However if I now change the script's OVMF files to those built from a
patched git master, the VM boots all the way to login prompt.

I'm using qemu-6.0.0 on SLackware64 but I've found exactly the same
behaviour using other OS's (Ubuntu 20.04 with 4.2-3ubuntu6.17 and Clear
Linux with 5.2.0)

I've also tried using OVMF files from Ubuntu hirsute's ovmf package
(2020.11-4) with same bad result. Of course, in this case, I was unable
to use a patched version.

>From the above, I think I've done everything possible to verify the
problem and a possible fix. Is there something fundamentally wrong in
the way I'm going about this?

chris

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Gerd Hoffmann]

  Hi,

> insmod /lib/modules/5.13.8/kernel/fs/jbd2/jbd2.ko
> insmod /lib/modules/5.13.8/kernel/fs/mbcache.ko
> insmod /lib/modules/5.13.8/kernel/fs/ext4/ext4.ko
> mount: mounting /dev/sda2 on /mnt failed: No such file or directory
> ERROR: No /sbin/init found on rootdev (or not mounted). Trouble ahead.
>        You can try to fix it. Type 'exit' when things are done.
> 
> At that point I'm dropped into a recovery shell to try fixing something
> but there's nothing that can be done since the disk containing the OS is
> not visible.

> From the above, I think I've done everything possible to verify the
> problem and a possible fix. Is there something fundamentally wrong in
> the way I'm going about this?

Root cause is not clear.  Grab the kernel log ('dmesg') from a good and
a bad boot and comparing them should give a clue.

take care,
  Gerd

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Christoph Willing]

On 10/8/21 4:01 pm, Gerd Hoffmann wrote:
>   Hi,
> 
>> insmod /lib/modules/5.13.8/kernel/fs/jbd2/jbd2.ko
>> insmod /lib/modules/5.13.8/kernel/fs/mbcache.ko
>> insmod /lib/modules/5.13.8/kernel/fs/ext4/ext4.ko
>> mount: mounting /dev/sda2 on /mnt failed: No such file or directory
>> ERROR: No /sbin/init found on rootdev (or not mounted). Trouble ahead.
>>        You can try to fix it. Type 'exit' when things are done.
>>
>> At that point I'm dropped into a recovery shell to try fixing something
>> but there's nothing that can be done since the disk containing the OS is
>> not visible.
> 
>> From the above, I think I've done everything possible to verify the
>> problem and a possible fix. Is there something fundamentally wrong in
>> the way I'm going about this?
> 
> Root cause is not clear.  Grab the kernel log ('dmesg') from a good and
> a bad boot and comparing them should give a clue.
> 
Thanks for your interest Gerd,

I added "-serial mon:stdio" to the qemu invocation and "console=ttyS0"
to the -append line to capture all boot output for both good and bad
cases and saved the results in files.

Rather than spam the list with those outputs, I've added them as
attachments to the bz report at
https://bugzilla.tianocore.org/show_bug.cgi?id=3504

When I first diff'd them, the slight difference in time stamps meant
every line showed up as different - real differences were hard to see.
Therefore I amended the files to replace the time stamps with "xxxx",
enabling diff to show actual differences more meaningfully.

I also attached there the output of a diff between the bad & good cases.
To be honest I can't see anything striking to explain the difference in
behaviour. To me, the main difference is that in the good case, an QEMU
HARDDISK & QEMU DVD-ROM are discovered and allows the system to boot but
no obvious indication why they are discovered in one case but not the
other. Apart from that, there are a few differences in some memory
locations being used but I have insufficient knowledge to judge whether
those differences are significant.

Is it possible for anyone to have a quick look through those files for
anything that may indicate what is causing the defective booting please?

Thanks,
chris

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[James Bottomley]

On Tue, 2021-08-10 at 10:10 +1000, Christoph Willing wrote:
> On 10/8/21 12:52 am, James Bottomley wrote:
> > On Mon, 2021-08-09 at 22:53 +1000, Christoph Willing wrote:
> > > With soft feature freeze started, I wonder if this patch could be
> > > reviewed and pushed for edk2-stable202108 tag? I think it has
> > > languished because I didn't initially Cc appropriately - pls add
> > > others as necessary.
> > > 
> > > This patch is a trivial (I think) change which fixes a long
> > > standing
> > > and annoying bug for those booting Qemu with UEFI using external
> > > kernel & initrd.
> > 
> > I'm with Ard on this one: -kernel is working just fine for me and
> > the
> > team at IBM working on Kata containers.  It sounds like this might
> > be a
> > problem local to your environment, so we need to debug it to
> > understand
> > the issue rather than blindly reverse existing commits.
> > 
> Thanks for responding James & Ard.
> 
> Below is the script I'm using to create, then run, the VM. To verify
> that it works normally with UEFI boot, it initially uses the internal
> kernel & initrd.
> 
> The OVMF_CODE & my_VARS lines contain git hash to identify the build
> from which OVMF_CODE.fd & OVMF_VARS.fd were taken; 97fdcg is from a
> build of yesterday's git master.
> 
> After the OS has been installed, I can run the VM multiple times to
> verify that it boots under UEFI OK (I see the TianoCore splash
> screen)
> with internal kernel.
> 
> 
> #!/bin/bash
> 
> /usr/bin/qemu-kvm \
> -name "UEFI Testing" \
> -enable-kvm \
> -cpu kvm64 \
> -smp cores=4 \
> -boot once=c \
> -m 8192 \
> -device intel-hda \
> -device hda-duplex \
> -vga virtio \
> -drive if=pflash,format=raw,file=OVMF_CODE_97fdcb.fd,readonly=on \
> -drive if=pflash,format=raw,file=my_VARS_97fdcb.fd \
> -drive file=disk.img,format=raw,cache=none,index=0,media=disk \
> -cdrom
> /storage/iso/slackware/slackware64-15.0/slackware64-15.0-20210807.iso 
> \
> -daemonize \
> "$@"

There's no definition of a disk device in here.

> To now use external kernel, I add the lines:
> 
> -kernel /var/cache/vmbuilder/boot/15.0/x86_64/vmlinuz \
> -initrd /var/cache/vmbuilder/boot/15.0/x86_64/initrd \
> -append "root=/dev/sda2 rootfstype=ext4 ro vga=0x386" \
> 
> to the script just after "-boot once=c" (but I doubt the exact
> positioning makes any difference).
> 
> In this case, I see the kernel running and initrd unpacked and its
> modules loaded but the root partition is unable to be mounted - the
> disk
> is not visible (running 'ls -l /dev/sd*' in recovery shell gives 'ls:
> /dev/sd*: No such file or directory').
> 
> The last lines of the Qemu screen are:
> 
> /boot/initrd-5.13.8.gz: Loading kernel modules from initrd image:
> insmod /lib/modules/5.13.8/kernel/fs/jbd2/jbd2.ko
> insmod /lib/modules/5.13.8/kernel/fs/mbcache.ko
> insmod /lib/modules/5.13.8/kernel/fs/ext4/ext4.ko
> mount: mounting /dev/sda2 on /mnt failed: No such file or directory

Which looks like why this failed.

Where's the vmm supposed to get /dev/sda from?  It sort of seems like
the CD rom boot script thinks it was mounted as a USB device in this
case.

In the working kernel dmesg Gerd requested, what does it mount as root?
sda?  In which case what does the kernel say about where it got sda
from?

James

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Christoph Willing]

On 11/8/21 12:26 am, James Bottomley wrote:
> On Tue, 2021-08-10 at 10:10 +1000, Christoph Willing wrote:
>> On 10/8/21 12:52 am, James Bottomley wrote:
>>> On Mon, 2021-08-09 at 22:53 +1000, Christoph Willing wrote:
>>>> With soft feature freeze started, I wonder if this patch could be
>>>> reviewed and pushed for edk2-stable202108 tag? I think it has
>>>> languished because I didn't initially Cc appropriately - pls add
>>>> others as necessary.
>>>>
>>>> This patch is a trivial (I think) change which fixes a long
>>>> standing
>>>> and annoying bug for those booting Qemu with UEFI using external
>>>> kernel & initrd.
>>>
>>> I'm with Ard on this one: -kernel is working just fine for me and
>>> the
>>> team at IBM working on Kata containers.  It sounds like this might
>>> be a
>>> problem local to your environment, so we need to debug it to
>>> understand
>>> the issue rather than blindly reverse existing commits.
>>>
>> Thanks for responding James & Ard.
>>
>> Below is the script I'm using to create, then run, the VM. To verify
>> that it works normally with UEFI boot, it initially uses the internal
>> kernel & initrd.
>>
>> The OVMF_CODE & my_VARS lines contain git hash to identify the build
>> from which OVMF_CODE.fd & OVMF_VARS.fd were taken; 97fdcg is from a
>> build of yesterday's git master.
>>
>> After the OS has been installed, I can run the VM multiple times to
>> verify that it boots under UEFI OK (I see the TianoCore splash
>> screen)
>> with internal kernel.
>>
>>
>> #!/bin/bash
>>
>> /usr/bin/qemu-kvm \
>> -name "UEFI Testing" \
>> -enable-kvm \
>> -cpu kvm64 \
>> -smp cores=4 \
>> -boot once=c \
>> -m 8192 \
>> -device intel-hda \
>> -device hda-duplex \
>> -vga virtio \
>> -drive if=pflash,format=raw,file=OVMF_CODE_97fdcb.fd,readonly=on \
>> -drive if=pflash,format=raw,file=my_VARS_97fdcb.fd \
>> -drive file=disk.img,format=raw,cache=none,index=0,media=disk \
>> -cdrom
>> /storage/iso/slackware/slackware64-15.0/slackware64-15.0-20210807.iso 
>> \
>> -daemonize \
>> "$@"
> 
> There's no definition of a disk device in here.
> 
-drive file=disk.img,format=raw,cache=none,index=0,media=disk \


>> To now use external kernel, I add the lines:
>>
>> -kernel /var/cache/vmbuilder/boot/15.0/x86_64/vmlinuz \
>> -initrd /var/cache/vmbuilder/boot/15.0/x86_64/initrd \
>> -append "root=/dev/sda2 rootfstype=ext4 ro vga=0x386" \
>>
>> to the script just after "-boot once=c" (but I doubt the exact
>> positioning makes any difference).
>>
>> In this case, I see the kernel running and initrd unpacked and its
>> modules loaded but the root partition is unable to be mounted - the
>> disk
>> is not visible (running 'ls -l /dev/sd*' in recovery shell gives 'ls:
>> /dev/sd*: No such file or directory').
>>
>> The last lines of the Qemu screen are:
>>
>> /boot/initrd-5.13.8.gz: Loading kernel modules from initrd image:
>> insmod /lib/modules/5.13.8/kernel/fs/jbd2/jbd2.ko
>> insmod /lib/modules/5.13.8/kernel/fs/mbcache.ko
>> insmod /lib/modules/5.13.8/kernel/fs/ext4/ext4.ko
>> mount: mounting /dev/sda2 on /mnt failed: No such file or directory
> 
> Which looks like why this failed.
> 
Exactly. I should have mentioned that specifically in the patch comment,
although the bz report at
https://bugzilla.tianocore.org/show_bug.cgi?id=3504 already says:
"The kernel and initrd are loaded but have no access to the VM itself."


> Where's the vmm supposed to get /dev/sda from?  It sort of seems like
> the CD rom boot script thinks it was mounted as a USB device in this
> case.
> 
It should find /dev/sda in the virtual disk, just as it is correctly
found in the case of the patched code.


> In the working kernel dmesg Gerd requested, what does it mount as root?
> sda?  In which case what does the kernel say about where it got sda
> from?
> 
Yes it mounts /dev/sda2 as root.

The boot logs are are now attached at
https://bugzilla.tianocore.org/show_bug.cgi?id=3504 as well a diff
between good and bad boots (patched & unpatched code). There's nothing
obvious (to me) as to why the unpatched code can't find the virtual disk.

My simple minded guess is that PlatformBdsConnectSequence() performs
some preparatory work that enables the kernel to have access to the vm
(therefore to the virtual disk) by the time TryRunningQemuKernel() is
called. At the moment however, TryRunningQemuKernel() is called before
PlatformBdsConnectSequence() so that preparatory work hasn't been done
and the disk can't be found.

chris

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[James Bottomley]

On Wed, 2021-08-11 at 09:04 +1000, Christoph Willing wrote:
> On 11/8/21 12:26 am, James Bottomley wrote:
> [...]
> > In the working kernel dmesg Gerd requested, what does it mount as
> > root? sda?  In which case what does the kernel say about where it
> > got sda from?
> > 
> Yes it mounts /dev/sda2 as root.
> 
> The boot logs are are now attached at
> https://bugzilla.tianocore.org/show_bug.cgi?id=3504 as well a diff
> between good and bad boots (patched & unpatched code). There's
> nothing obvious (to me) as to why the unpatched code can't find the
> virtual disk.
> 
> My simple minded guess is that PlatformBdsConnectSequence() performs
> some preparatory work that enables the kernel to have access to the
> vm (therefore to the virtual disk) by the time TryRunningQemuKernel()
> is called. At the moment however, TryRunningQemuKernel() is called
> before PlatformBdsConnectSequence() so that preparatory work hasn't
> been done and the disk can't be found.

I suspect the problem is that it no longer creates default devices if
you don't specify them.  If I look at my working version of a command
line boot, it's this:

qemu-kvm \
    -drive if=pflash,format=raw,unit=0,file=/home/jejb/git/edk2/Build/OvmfX64/DEBUG_GCC5/FV/OVMF.fd,readonly \
    -debugcon file:debug.log -global isa-debugcon.iobase=0x402 \
    -kernel bzImage \
    -initrd initrd.img \
    -append "console=ttyS0 rd.driver.pre=virtio_scsi,9p,9pnet_virtio root=/dev/sda1" \
    -m 2048 \
    -serial stdio \
    -drive if=none,id=sd00,file=debian.img,format=qcow2 \
    -fsdev local,security_model=passthrough,id=modules,path=/tmp/lib/modules \
    -fsdev local,security_model=passthrough,id=home,path=/home/jejb \
    -device virtio-scsi-pci,id=scsi \
    -device virtio-9p-pci,fsdev=modules,id=sd01,mount_tag=modules \
    -device virtio-9p-pci,fsdev=home,id=sd02,mount_tag=home \
    -device scsi-hd,bus=scsi.0,drive=sd00 \
    -device e1000,netdev=net0 \
    -netdev user,id=net0,hostfwd=tcp::5555-:22 \
    -display vnc=:5 \


#    -chardev socket,id=chrtpm,path=`pwd`/tpmdev/swtpm-sock \
#    -tpmdev emulator,id=tpm0,chardev=chrtpm \
#    -device tpm-tis,tpmdev=tpm0 \

You can ignore all the TPM stuff and my 9p pass throughs for home
directory and /tmp modules directory (to make it easy to boot any
kernel I build).  The relevant lines for you are:

    -drive if=none,id=sd00,file=debian.img,format=qcow2 \
    -device virtio-scsi-pci,id=scsi \
    -device scsi-hd,bus=scsi.0,drive=sd00 \

Which is how I connect /dev/sda to the qcow2 debian image I use to
boot.  You have the drive line but no device lines giving the driver
... if you add device lines can you then get it to boot?

James

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Christoph Willing]

On 11/8/21 9:24 am, James Bottomley wrote:
> On Wed, 2021-08-11 at 09:04 +1000, Christoph Willing wrote:
>> On 11/8/21 12:26 am, James Bottomley wrote:
>> [...]
>>> In the working kernel dmesg Gerd requested, what does it mount as
>>> root? sda?  In which case what does the kernel say about where it
>>> got sda from?
>>>
>> Yes it mounts /dev/sda2 as root.
>>
>> The boot logs are are now attached at
>> https://bugzilla.tianocore.org/show_bug.cgi?id=3504 as well a diff
>> between good and bad boots (patched & unpatched code). There's
>> nothing obvious (to me) as to why the unpatched code can't find the
>> virtual disk.
>>
>> My simple minded guess is that PlatformBdsConnectSequence() performs
>> some preparatory work that enables the kernel to have access to the
>> vm (therefore to the virtual disk) by the time TryRunningQemuKernel()
>> is called. At the moment however, TryRunningQemuKernel() is called
>> before PlatformBdsConnectSequence() so that preparatory work hasn't
>> been done and the disk can't be found.
> 
> I suspect the problem is that it no longer creates default devices if
> you don't specify them.  If I look at my working version of a command
> line boot, it's this:
> 
> qemu-kvm \
>     -drive if=pflash,format=raw,unit=0,file=/home/jejb/git/edk2/Build/OvmfX64/DEBUG_GCC5/FV/OVMF.fd,readonly \
>     -debugcon file:debug.log -global isa-debugcon.iobase=0x402 \
>     -kernel bzImage \
>     -initrd initrd.img \
>     -append "console=ttyS0 rd.driver.pre=virtio_scsi,9p,9pnet_virtio root=/dev/sda1" \
>     -m 2048 \
>     -serial stdio \
>     -drive if=none,id=sd00,file=debian.img,format=qcow2 \
>     -fsdev local,security_model=passthrough,id=modules,path=/tmp/lib/modules \
>     -fsdev local,security_model=passthrough,id=home,path=/home/jejb \
>     -device virtio-scsi-pci,id=scsi \
>     -device virtio-9p-pci,fsdev=modules,id=sd01,mount_tag=modules \
>     -device virtio-9p-pci,fsdev=home,id=sd02,mount_tag=home \
>     -device scsi-hd,bus=scsi.0,drive=sd00 \
>     -device e1000,netdev=net0 \
>     -netdev user,id=net0,hostfwd=tcp::5555-:22 \
>     -display vnc=:5 \
> 
> 
> #    -chardev socket,id=chrtpm,path=`pwd`/tpmdev/swtpm-sock \
> #    -tpmdev emulator,id=tpm0,chardev=chrtpm \
> #    -device tpm-tis,tpmdev=tpm0 \
> 
> You can ignore all the TPM stuff and my 9p pass throughs for home
> directory and /tmp modules directory (to make it easy to boot any
> kernel I build).  The relevant lines for you are:
> 
>     -drive if=none,id=sd00,file=debian.img,format=qcow2 \
>     -device virtio-scsi-pci,id=scsi \
>     -device scsi-hd,bus=scsi.0,drive=sd00 \
> 
> Which is how I connect /dev/sda to the qcow2 debian image I use to
> boot.  You have the drive line but no device lines giving the driver
> ... if you add device lines can you then get it to boot?
> 
Thanks for the suggestion James,

I amended my script to have:

-drive if=none,id=sd00,file=disk.img,format=raw \
-device virtio-scsi-pci,id=scsi \
-device scsi-hd,bus=scsi.0,drive=sd00 \

but no joy unfortunately.

I also stripped out some other qemu options like -enable-kvm,-cpu,-cores
etc., in case they caused some weird impacts during boot but that also
had no effect.

chris

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Gerd Hoffmann]

  Hi,

> I amended my script to have:
> 
> -drive if=none,id=sd00,file=disk.img,format=raw \
> -device virtio-scsi-pci,id=scsi \
> -device scsi-hd,bus=scsi.0,drive=sd00 \

That switches storage from ide to virtio-scsi.

Which is a good idea from the performance point of view.  Will obviously
also workaround ide problems.  But might fail now due to virtio-scsi
driver not being part of your initrd.

take care,
  Gerd

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Christoph Willing]

On 11/8/21 4:12 pm, Gerd Hoffmann wrote:
>   Hi,
> 
>> I amended my script to have:
>>
>> -drive if=none,id=sd00,file=disk.img,format=raw \
>> -device virtio-scsi-pci,id=scsi \
>> -device scsi-hd,bus=scsi.0,drive=sd00 \
> 
> That switches storage from ide to virtio-scsi.
> 
> Which is a good idea from the performance point of view.  Will obviously
> also workaround ide problems.  But might fail now due to virtio-scsi
> driver not being part of your initrd.
> 

Thank you James & Gerd,

Your suggestions & comments led me to reconsider the contents of the
initrd and the problem is now fixed.

I had been using an initrd that worked perfectly in a non-UEFI boot and
half worked with UEFI boot (the working half being when code was
patched). However when I generated a completely new initrd after
installing a new system in UEFI mode, I could see many virtio modules
being added which were not part of the old initrd I had been using.
Booting with the new initrd works perfectly in  all boot modes using
OVMF files created without any code patching.

Is there a particular procedure to withdraw my patch request, or does it
die a quiet death by being ignored now?

Thanks again,
chris

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Ard Biesheuvel]

On Wed, 11 Aug 2021 at 11:55, Christoph Willing
<475.chris.willing@gmail.com> wrote:
>
> On 11/8/21 4:12 pm, Gerd Hoffmann wrote:
> >   Hi,
> >
> >> I amended my script to have:
> >>
> >> -drive if=none,id=sd00,file=disk.img,format=raw \
> >> -device virtio-scsi-pci,id=scsi \
> >> -device scsi-hd,bus=scsi.0,drive=sd00 \
> >
> > That switches storage from ide to virtio-scsi.
> >
> > Which is a good idea from the performance point of view.  Will obviously
> > also workaround ide problems.  But might fail now due to virtio-scsi
> > driver not being part of your initrd.
> >
>
> Thank you James & Gerd,
>
> Your suggestions & comments led me to reconsider the contents of the
> initrd and the problem is now fixed.
>
> I had been using an initrd that worked perfectly in a non-UEFI boot and
> half worked with UEFI boot (the working half being when code was
> patched). However when I generated a completely new initrd after
> installing a new system in UEFI mode, I could see many virtio modules
> being added which were not part of the old initrd I had been using.
> Booting with the new initrd works perfectly in  all boot modes using
> OVMF files created without any code patching.
>
> Is there a particular procedure to withdraw my patch request, or does it
> die a quiet death by being ignored now?
>

Glad to hear that the mystery is solved now.

As for the patch, we will just disregard it - no need for any other
action on your part.

-- 
Ard.

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Christoph Willing]

On 11/8/21 8:30 pm, Ard Biesheuvel wrote:
> On Wed, 11 Aug 2021 at 11:55, Christoph Willing
> <475.chris.willing@gmail.com> wrote:
>>
>> On 11/8/21 4:12 pm, Gerd Hoffmann wrote:
>>>   Hi,
>>>
>>>> I amended my script to have:
>>>>
>>>> -drive if=none,id=sd00,file=disk.img,format=raw \
>>>> -device virtio-scsi-pci,id=scsi \
>>>> -device scsi-hd,bus=scsi.0,drive=sd00 \
>>>
>>> That switches storage from ide to virtio-scsi.
>>>
>>> Which is a good idea from the performance point of view.  Will obviously
>>> also workaround ide problems.  But might fail now due to virtio-scsi
>>> driver not being part of your initrd.
>>>
>>
>> Thank you James & Gerd,
>>
>> Your suggestions & comments led me to reconsider the contents of the
>> initrd and the problem is now fixed.
>>
>> I had been using an initrd that worked perfectly in a non-UEFI boot and
>> half worked with UEFI boot (the working half being when code was
>> patched). However when I generated a completely new initrd after
>> installing a new system in UEFI mode, I could see many virtio modules
>> being added which were not part of the old initrd I had been using.
>> Booting with the new initrd works perfectly in  all boot modes using
>> OVMF files created without any code patching.
>>
>> Is there a particular procedure to withdraw my patch request, or does it
>> die a quiet death by being ignored now?
>>
> 
> Glad to hear that the mystery is solved now.
> 
> As for the patch, we will just disregard it - no need for any other
> action on your part.
> 

Hi all,

I just wanted to add some clarification about why the problem is fixed.

It not really just about adding virtio* modules to the initrd. That was
only needed because I changed the drive to use a virtio-scsi-pci device
as James had suggested.

While that all worked fine, probably with better performance (as Gerd
mentioned) it was still unsatisfying that I couldn't make the plain ide
work. It turns out that the ide interface can work without changes to
the initrd but invocation requires separate -drive and -device options,
somewhat like James' scsi version, namely:

-device ahci,id=ahci \
-device ide-hd,drive=ide0,bus=ahci.0 \
-drive
if=none,id=ide0,file=disk.img,format=raw,cache=none,index=0,media=disk \

instead of my original shortcut version:

-drive file=disk.img,format=raw,cache=none,index=0,media=disk \


I'll probably stick with the better performing scsi version from now on
although it's still interesting that the shortcut version works without
UEFI but fails with UEFI.

chris

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Gerd Hoffmann]

  Hi,

> While that all worked fine, probably with better performance (as Gerd
> mentioned) it was still unsatisfying that I couldn't make the plain ide
> work. It turns out that the ide interface can work without changes to
> the initrd but invocation requires separate -drive and -device options,
> somewhat like James' scsi version, namely:
> 
> -device ahci,id=ahci \
> -device ide-hd,drive=ide0,bus=ahci.0 \
> -drive
> if=none,id=ide0,file=disk.img,format=raw,cache=none,index=0,media=disk \

That is again something different, namely sata ...

> instead of my original shortcut version:
> 
> -drive file=disk.img,format=raw,cache=none,index=0,media=disk \

... while this is the older pata (with "-machine pc").

To add even more confusion: When using the more modern q35 chipset
("-machine q35") qemu will also use the more modern sata when you
use the shortcut version to attach your disk.

So, in general it is a good idea to explicitly add the storage
controller and devices you want as suggested by James.  Results
in less surprises ;)

> I'll probably stick with the better performing scsi version from now on
> although it's still interesting that the shortcut version works without
> UEFI but fails with UEFI.

I've seen guest driver failures due to missing firmware initialization
before.  Specifically firmware not setting the busmaster bit in pci
config space leading to guest driver's dma transfers failing (which of
course is a guest driver bug).

I can imagine ovmf taking shortcuts for direct kernel boot could have
that kind of effect too, and of course it could vary depending on the
kind of storage controller used.

take care,
  Gerd

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Christoph Willing]

On 12/8/21 11:36 pm, Gerd Hoffmann wrote:
>   Hi,
> 
>> While that all worked fine, probably with better performance (as Gerd
>> mentioned) it was still unsatisfying that I couldn't make the plain ide
>> work. It turns out that the ide interface can work without changes to
>> the initrd but invocation requires separate -drive and -device options,
>> somewhat like James' scsi version, namely:
>>
>> -device ahci,id=ahci \
>> -device ide-hd,drive=ide0,bus=ahci.0 \
>> -drive
>> if=none,id=ide0,file=disk.img,format=raw,cache=none,index=0,media=disk \
> 
> That is again something different, namely sata ...
> 
>> instead of my original shortcut version:
>>
>> -drive file=disk.img,format=raw,cache=none,index=0,media=disk \
> 
> ... while this is the older pata (with "-machine pc").
> 
> To add even more confusion: When using the more modern q35 chipset
> ("-machine q35") qemu will also use the more modern sata when you
> use the shortcut version to attach your disk.
> 
> So, in general it is a good idea to explicitly add the storage
> controller and devices you want as suggested by James.  Results
> in less surprises ;)
> 
Thanks Gerd,

That is the missing piece of the puzzle. I added "-machine q35" and
reverted to the shortcut -drive option and boot was successful with
UEFI. Although I will use the longer form in future, it was still
annoying that the shortcut version failed. The pata/sata explanation is
quite satisfying and I can finally let this issue go :)

chris

>> I'll probably stick with the better performing scsi version from now on
>> although it's still interesting that the shortcut version works without
>> UEFI but fails with UEFI.
> 
> I've seen guest driver failures due to missing firmware initialization
> before.  Specifically firmware not setting the busmaster bit in pci
> config space leading to guest driver's dma transfers failing (which of
> course is a guest driver bug).
> 
> I can imagine ovmf taking shortcuts for direct kernel boot could have
> that kind of effect too, and of course it could vary depending on the
> kind of storage controller used.
> 
> take care,
>   Gerd
> 
> 
> 
> 
> 

Re: [edk2-devel] [PATCH 1/1] OvmfPkg PlatformBootManagerLib: Move TryRunningQemuKernel()

[Gerd Hoffmann]

  Hi,

> > -drive file=disk.img,format=raw,cache=none,index=0,media=disk \
> > -cdrom
> > /storage/iso/slackware/slackware64-15.0/slackware64-15.0-20210807.iso 
> 
> There's no definition of a disk device in here.

First line is disk, second is cdrom.  Both are attached to the piix4 ide
controller.

Linux finds the controller just fine in both cases (see bugzilla for
the logs):

	ata1: PATA max MWDMA2 cmd 0x1f0 ctl 0x3f6 bmdma 0xc040 irq 14
	ata2: PATA max MWDMA2 cmd 0x170 ctl 0x376 bmdma 0xc048 irq 15

But only in one case it finds disk and cdrom:

	ata1.00: ATA-7: QEMU HARDDISK, 2.5+, max UDMA/100
	ata1.00: 41943040 sectors, multi 16: LBA48 
	ata2.00: ATAPI: QEMU DVD-ROM, 2.5+, max UDMA/100
	[ ... ]

Not obvious why.  There isn't an error message in the non-working case.
Does ovmf talk to the ide controller in case the order of the two calls
is changed?  Maybe that does some initialization which the linux driver
depends on.

take care,
  Gerd