Move the TryRunningQemuKernel() call back to its original place. This
improves the UEFI boot time for VMs that have "-kernel", many disks or
NICs, and no "bootindex" properties. A well-known example is
guestfish/libguestfs.

For more info on the TryRunningQemuKernel() location, see the following
commits: 23d04b58e27b, a78c4836ea0b, 158990b941e4.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Xiang Zheng <xiang.zheng@linaro.org>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
index 36e0eed2384a..5d5e51d8c870 100644
--- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
+++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
@@ -667,31 +667,31 @@ PlatformBootManagerAfterConsole (
 
   //
   // Show the splash screen.
   //
   BootLogoEnableLogo ();
 
+  //
+  // Process QEMU's -kernel command line option. The kernel booted this way
+  // will receive ACPI tables: in PlatformBootManagerBeforeConsole(), we
+  // connected any and all PCI root bridges, and then signaled the ACPI
+  // platform driver.
+  //
+  TryRunningQemuKernel ();
+
   //
   // Connect the purported boot devices.
   //
   Status = ConnectDevicesFromQemu ();
   if (RETURN_ERROR (Status)) {
     //
     // Connect the rest of the devices.
     //
     EfiBootManagerConnectAll ();
   }
 
-  //
-  // Process QEMU's -kernel command line option. Note that the kernel booted
-  // this way should receive ACPI tables, which is why we connect all devices
-  // first (see above) -- PCI enumeration blocks ACPI table installation, if
-  // there is a PCI host.
-  //
-  TryRunningQemuKernel ();
-
   //
   // Enumerate all possible boot options, then filter and reorder them based on
   // the QEMU configuration.
   //
   EfiBootManagerRefreshAllBootOption ();
 
-- 
2.14.1.3.gb7cf6e02401b


_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 15 March 2018 at 19:02, Laszlo Ersek <lersek@redhat.com> wrote:
> Move the TryRunningQemuKernel() call back to its original place.

When was it moved and why?

> This
> improves the UEFI boot time for VMs that have "-kernel", many disks or
> NICs, and no "bootindex" properties. A well-known example is
> guestfish/libguestfs.
>
> For more info on the TryRunningQemuKernel() location, see the following
> commits: 23d04b58e27b, a78c4836ea0b, 158990b941e4.
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Xiang Zheng <xiang.zheng@linaro.org>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> index 36e0eed2384a..5d5e51d8c870 100644
> --- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -667,31 +667,31 @@ PlatformBootManagerAfterConsole (
>
>    //
>    // Show the splash screen.
>    //
>    BootLogoEnableLogo ();
>
> +  //
> +  // Process QEMU's -kernel command line option. The kernel booted this way
> +  // will receive ACPI tables: in PlatformBootManagerBeforeConsole(), we
> +  // connected any and all PCI root bridges, and then signaled the ACPI
> +  // platform driver.
> +  //
> +  TryRunningQemuKernel ();
> +
>    //
>    // Connect the purported boot devices.
>    //
>    Status = ConnectDevicesFromQemu ();
>    if (RETURN_ERROR (Status)) {
>      //
>      // Connect the rest of the devices.
>      //
>      EfiBootManagerConnectAll ();
>    }
>
> -  //
> -  // Process QEMU's -kernel command line option. Note that the kernel booted
> -  // this way should receive ACPI tables, which is why we connect all devices
> -  // first (see above) -- PCI enumeration blocks ACPI table installation, if
> -  // there is a PCI host.
> -  //
> -  TryRunningQemuKernel ();
> -
>    //
>    // Enumerate all possible boot options, then filter and reorder them based on
>    // the QEMU configuration.
>    //
>    EfiBootManagerRefreshAllBootOption ();
>
> --
> 2.14.1.3.gb7cf6e02401b
>
>
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 03/16/18 10:58, Ard Biesheuvel wrote:
> On 15 March 2018 at 19:02, Laszlo Ersek <lersek@redhat.com> wrote:
>> Move the TryRunningQemuKernel() call back to its original place.
> 
> When was it moved and why?

See, I'm at a loss about the detail you expect in commit messages. :) I
had spent 10 minutes on wording this commit message. Near the end of my
struggle, I had exactly one sentence on each of those three commits that
I listed at the bottom (23d04b58e27b, a78c4836ea0b, 158990b941e4). Those
three sentences were going to give the answer to your question. I still
worried you might file them under "personal journey" or "stream of
consciousness", and I cut them; I only left the three commit hashes at
the end as pointers.

So, the executive summary is: "TryRunningQemuKernel() was moved in
a78c4836ea0b, because the guest kernel depended on ACPI tables, which
depended on our ACPI platform driver, which at the time depended on
PciBusDxe itself reporting 'enumeration complete', which at the time
depended on our BdsLibConnectAll() call".

The somewhat expanded answer is, from scratch:

(1) in commit 23d04b58e27b, we introduced TryRunningQemuKernel() in the
    right spot (for boot performance), between
    PlatformBdsConnectConsole() and BdsLibConnectAll();

(2) in commit a78c4836ea0b, we moved TryRunningQemuKernel() after
    BdsLibConnectAll(): the direct-booted kernel needed ACPI tables
    (reflecting PCI resources correctly), and at that time, we only
    connected the root bridges to PciBusDxe as part of
    BdsLibConnectAll() (which signaled the ACPI platform driver via
    "gEfiPciEnumerationCompleteProtocolGuid");

(3) in commit 60dc18a17c516 and (more importantly) 158990b941e4,
    connecting the root bridges and cueing the ACPI platform driver were
    finally separated from BdsLibConnectAll(), but we didn't realize we
    could move TryRunningQemuKernel() back above BdsLibConnectAll().

So, after 158990b941e4, we can do it now.

Thanks,
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 16 March 2018 at 13:40, Laszlo Ersek <lersek@redhat.com> wrote:
> On 03/16/18 10:58, Ard Biesheuvel wrote:
>> On 15 March 2018 at 19:02, Laszlo Ersek <lersek@redhat.com> wrote:
>>> Move the TryRunningQemuKernel() call back to its original place.
>>
>> When was it moved and why?
>
> See, I'm at a loss about the detail you expect in commit messages. :)

I see. But in this case, I think it is rather obvious that when you
move something back to where it was moved away from earlier, you have
to justify why the original reason for moving it no longer applies, or
is overruled by something more important.

> I
> had spent 10 minutes on wording this commit message. Near the end of my
> struggle, I had exactly one sentence on each of those three commits that
> I listed at the bottom (23d04b58e27b, a78c4836ea0b, 158990b941e4). Those
> three sentences were going to give the answer to your question. I still
> worried you might file them under "personal journey" or "stream of
> consciousness", and I cut them; I only left the three commit hashes at
> the end as pointers.
>

I looked at those commit logs but it wasn't obvious to me.

> So, the executive summary is: "TryRunningQemuKernel() was moved in
> a78c4836ea0b, because the guest kernel depended on ACPI tables, which
> depended on our ACPI platform driver, which at the time depended on
> PciBusDxe itself reporting 'enumeration complete', which at the time
> depended on our BdsLibConnectAll() call".
>
> The somewhat expanded answer is, from scratch:
>
> (1) in commit 23d04b58e27b, we introduced TryRunningQemuKernel() in the
>     right spot (for boot performance), between
>     PlatformBdsConnectConsole() and BdsLibConnectAll();
>
> (2) in commit a78c4836ea0b, we moved TryRunningQemuKernel() after
>     BdsLibConnectAll(): the direct-booted kernel needed ACPI tables
>     (reflecting PCI resources correctly), and at that time, we only
>     connected the root bridges to PciBusDxe as part of
>     BdsLibConnectAll() (which signaled the ACPI platform driver via
>     "gEfiPciEnumerationCompleteProtocolGuid");
>
> (3) in commit 60dc18a17c516 and (more importantly) 158990b941e4,
>     connecting the root bridges and cueing the ACPI platform driver were
>     finally separated from BdsLibConnectAll(), but we didn't realize we
>     could move TryRunningQemuKernel() back above BdsLibConnectAll().
>
> So, after 158990b941e4, we can do it now.
>

Thanks for clearing that up.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

On 03/16/18 14:45, Ard Biesheuvel wrote:
> On 16 March 2018 at 13:40, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 03/16/18 10:58, Ard Biesheuvel wrote:
>>> On 15 March 2018 at 19:02, Laszlo Ersek <lersek@redhat.com> wrote:
>>>> Move the TryRunningQemuKernel() call back to its original place.
>>>
>>> When was it moved and why?
>>
>> See, I'm at a loss about the detail you expect in commit messages. :)
> 
> I see. But in this case, I think it is rather obvious that when you
> move something back to where it was moved away from earlier, you have
> to justify why the original reason for moving it no longer applies, or
> is overruled by something more important.
> 
>> I
>> had spent 10 minutes on wording this commit message. Near the end of my
>> struggle, I had exactly one sentence on each of those three commits that
>> I listed at the bottom (23d04b58e27b, a78c4836ea0b, 158990b941e4). Those
>> three sentences were going to give the answer to your question. I still
>> worried you might file them under "personal journey" or "stream of
>> consciousness", and I cut them; I only left the three commit hashes at
>> the end as pointers.
>>
> 
> I looked at those commit logs but it wasn't obvious to me.
> 
>> So, the executive summary is: "TryRunningQemuKernel() was moved in
>> a78c4836ea0b, because the guest kernel depended on ACPI tables, which
>> depended on our ACPI platform driver, which at the time depended on
>> PciBusDxe itself reporting 'enumeration complete', which at the time
>> depended on our BdsLibConnectAll() call".
>>
>> The somewhat expanded answer is, from scratch:
>>
>> (1) in commit 23d04b58e27b, we introduced TryRunningQemuKernel() in the
>>     right spot (for boot performance), between
>>     PlatformBdsConnectConsole() and BdsLibConnectAll();
>>
>> (2) in commit a78c4836ea0b, we moved TryRunningQemuKernel() after
>>     BdsLibConnectAll(): the direct-booted kernel needed ACPI tables
>>     (reflecting PCI resources correctly), and at that time, we only
>>     connected the root bridges to PciBusDxe as part of
>>     BdsLibConnectAll() (which signaled the ACPI platform driver via
>>     "gEfiPciEnumerationCompleteProtocolGuid");
>>
>> (3) in commit 60dc18a17c516 and (more importantly) 158990b941e4,
>>     connecting the root bridges and cueing the ACPI platform driver were
>>     finally separated from BdsLibConnectAll(), but we didn't realize we
>>     could move TryRunningQemuKernel() back above BdsLibConnectAll().
>>
>> So, after 158990b941e4, we can do it now.
>>
> 
> Thanks for clearing that up.

Are you OK with the patch with this information available?

Should I include some (or all) of the above in the commit message?

Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel