[SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

Stefano Garzarella posted 1 patch 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/seabios tags/patchew/20181126113912.45478-1-sgarzare@redhat.com
src/bootsplash.c  |  3 +++
src/fw/paravirt.c | 10 ++++++++++
src/fw/paravirt.h |  4 ++++
src/optionroms.c  |  3 ++-
src/post.c        |  3 +++
5 files changed, 22 insertions(+), 1 deletion(-)

[SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

Posted by Stefano Garzarella 2 weeks ago
Speed up the boot phase when qemu uses "linuxboot" optionrom
(qemu -kernel) and the boot-menu is not required.
Under these conditions we can skip the setup of devices and VGA,
because they will be initialized (if they are required) during
the Linux boot phase.

Following the time measured between SeaBIOS entry point and
"linuxboot" entry point:

* Before this patch
  qemu -kernel  | qemu -vga none -kernel
  --------------+-----------------------
  53.5 msec     | 23.34 msec

* After this patch
  qemu -kernel  | qemu -vga none -kernel
  --------------+-----------------------
  12.82 msec    | 10.89 msec

Note: For the measuring, we used the default configuration disabling
debug messages (CONFIG_DEBUG_LEVEL=0) and applying Stephen's patch:
"tpm: Check for TPM related ACPI tables before attempting hw"

Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 src/bootsplash.c  |  3 +++
 src/fw/paravirt.c | 10 ++++++++++
 src/fw/paravirt.h |  4 ++++
 src/optionroms.c  |  3 ++-
 src/post.c        |  3 +++
 5 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/src/bootsplash.c b/src/bootsplash.c
index 165c98d..0eda7f2 100644
--- a/src/bootsplash.c
+++ b/src/bootsplash.c
@@ -8,6 +8,7 @@
 #include "bregs.h" // struct bregs
 #include "config.h" // CONFIG_*
 #include "farptr.h" // FLATPTR_TO_SEG
+#include "fw/paravirt.h" // runningOnQEMUFastBoot
 #include "malloc.h" // free
 #include "output.h" // dprintf
 #include "romfile.h" // romfile_loadfile
@@ -39,6 +40,8 @@ call16_int10(struct bregs *br)
 void
 enable_vga_console(void)
 {
+    if(runningOnQEMUFastBoot())
+        return;
     dprintf(1, "Turning on vga text mode console\n");
     struct bregs br;
 
diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
index 0770c47..9e6e618 100644
--- a/src/fw/paravirt.c
+++ b/src/fw/paravirt.c
@@ -621,4 +621,14 @@ void qemu_cfg_init(void)
     if (nogfx && !romfile_find("etc/sercon-port")
         && !romfile_find("vgaroms/sgabios.bin"))
         const_romfile_add_int("etc/sercon-port", PORT_SERIAL1);
+
+    /*
+     * Enable QEMU fast boot if there is "linuxboot" optionrom and
+     * the boot menu is not required.
+     */
+    if ((romfile_find("genroms/linuxboot_dma.bin")
+        || romfile_find("genroms/linuxboot.bin"))
+        && !romfile_loadint("etc/show-boot-menu", 1)) {
+        PlatformRunningOn |= PF_QEMU_FB;
+    }
 }
diff --git a/src/fw/paravirt.h b/src/fw/paravirt.h
index a14d83e..3713bc2 100644
--- a/src/fw/paravirt.h
+++ b/src/fw/paravirt.h
@@ -9,6 +9,7 @@
 #define PF_QEMU     (1<<0)
 #define PF_XEN      (1<<1)
 #define PF_KVM      (1<<2)
+#define PF_QEMU_FB  (1<<3)
 
 typedef struct QemuCfgDmaAccess {
     u32 control;
@@ -24,6 +25,9 @@ static inline int runningOnQEMU(void) {
     return CONFIG_QEMU || (
         CONFIG_QEMU_HARDWARE && GET_GLOBAL(PlatformRunningOn) & PF_QEMU);
 }
+static inline int runningOnQEMUFastBoot(void) {
+    return runningOnQEMU() && GET_GLOBAL(PlatformRunningOn) & PF_QEMU_FB;
+}
 static inline int runningOnXen(void) {
     return CONFIG_XEN && GET_GLOBAL(PlatformRunningOn) & PF_XEN;
 }
diff --git a/src/optionroms.c b/src/optionroms.c
index fc992f6..c312e91 100644
--- a/src/optionroms.c
+++ b/src/optionroms.c
@@ -8,6 +8,7 @@
 #include "bregs.h" // struct bregs
 #include "config.h" // CONFIG_*
 #include "farptr.h" // FLATPTR_TO_SEG
+#include "fw/paravirt.h" // runningOnQEMUFastBoot
 #include "hw/pci.h" // pci_config_readl
 #include "hw/pcidevice.h" // foreachpci
 #include "hw/pci_ids.h" // PCI_CLASS_DISPLAY_VGA
@@ -428,7 +429,7 @@ vgarom_setup(void)
 {
     int have_vga = 0;
 
-    if (! CONFIG_OPTIONROMS)
+    if (!CONFIG_OPTIONROMS || runningOnQEMUFastBoot())
         return;
 
     dprintf(1, "Scan for VGA option rom\n");
diff --git a/src/post.c b/src/post.c
index f93106a..d1c2a54 100644
--- a/src/post.c
+++ b/src/post.c
@@ -126,6 +126,9 @@ interface_init(void)
 void
 device_hardware_setup(void)
 {
+    if(runningOnQEMUFastBoot())
+        return;
+
     usb_setup();
     ps2port_setup();
     block_setup();
-- 
2.19.1


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

Posted by Kevin O'Connor 2 weeks ago
On Mon, Nov 26, 2018 at 12:39:12PM +0100, Stefano Garzarella wrote:
> Speed up the boot phase when qemu uses "linuxboot" optionrom
> (qemu -kernel) and the boot-menu is not required.
> Under these conditions we can skip the setup of devices and VGA,
> because they will be initialized (if they are required) during
> the Linux boot phase.
> 
> Following the time measured between SeaBIOS entry point and
> "linuxboot" entry point:
> 
> * Before this patch
>   qemu -kernel  | qemu -vga none -kernel
>   --------------+-----------------------
>   53.5 msec     | 23.34 msec
> 
> * After this patch
>   qemu -kernel  | qemu -vga none -kernel
>   --------------+-----------------------
>   12.82 msec    | 10.89 msec
> 
> Note: For the measuring, we used the default configuration disabling
> debug messages (CONFIG_DEBUG_LEVEL=0) and applying Stephen's patch:
> "tpm: Check for TPM related ACPI tables before attempting hw"
> 
> Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> ---
>  src/bootsplash.c  |  3 +++
>  src/fw/paravirt.c | 10 ++++++++++
>  src/fw/paravirt.h |  4 ++++
>  src/optionroms.c  |  3 ++-
>  src/post.c        |  3 +++
>  5 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/src/bootsplash.c b/src/bootsplash.c
> index 165c98d..0eda7f2 100644
> --- a/src/bootsplash.c
> +++ b/src/bootsplash.c
> @@ -8,6 +8,7 @@
>  #include "bregs.h" // struct bregs
>  #include "config.h" // CONFIG_*
>  #include "farptr.h" // FLATPTR_TO_SEG
> +#include "fw/paravirt.h" // runningOnQEMUFastBoot
>  #include "malloc.h" // free
>  #include "output.h" // dprintf
>  #include "romfile.h" // romfile_loadfile
> @@ -39,6 +40,8 @@ call16_int10(struct bregs *br)
>  void
>  enable_vga_console(void)
>  {
> +    if(runningOnQEMUFastBoot())
> +        return;
>      dprintf(1, "Turning on vga text mode console\n");
>      struct bregs br;
>  
> diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> index 0770c47..9e6e618 100644
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -621,4 +621,14 @@ void qemu_cfg_init(void)
>      if (nogfx && !romfile_find("etc/sercon-port")
>          && !romfile_find("vgaroms/sgabios.bin"))
>          const_romfile_add_int("etc/sercon-port", PORT_SERIAL1);
> +
> +    /*
> +     * Enable QEMU fast boot if there is "linuxboot" optionrom and
> +     * the boot menu is not required.
> +     */
> +    if ((romfile_find("genroms/linuxboot_dma.bin")
> +        || romfile_find("genroms/linuxboot.bin"))
> +        && !romfile_loadint("etc/show-boot-menu", 1)) {
> +        PlatformRunningOn |= PF_QEMU_FB;
> +    }

I don't think we should hardcode special meanings to the names of
bootable files.  If QEMU wants SeaBIOS to not perform some type of
hardware init, then I think QEMU should explicitly request that from
SeaBIOS (eg, a "etc/dont-run-hardware-init").

Thanks,
-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

Posted by Stefano Garzarella 1 week ago
On Wed, Nov 28, 2018 at 3:12 AM Kevin O'Connor <kevin@koconnor.net> wrote:
>
> On Mon, Nov 26, 2018 at 12:39:12PM +0100, Stefano Garzarella wrote:
> > Speed up the boot phase when qemu uses "linuxboot" optionrom
> > (qemu -kernel) and the boot-menu is not required.
> > Under these conditions we can skip the setup of devices and VGA,
> > because they will be initialized (if they are required) during
> > the Linux boot phase.
> >
> > Following the time measured between SeaBIOS entry point and
> > "linuxboot" entry point:
> >
> > * Before this patch
> >   qemu -kernel  | qemu -vga none -kernel
> >   --------------+-----------------------
> >   53.5 msec     | 23.34 msec
> >
> > * After this patch
> >   qemu -kernel  | qemu -vga none -kernel
> >   --------------+-----------------------
> >   12.82 msec    | 10.89 msec
> >
> > Note: For the measuring, we used the default configuration disabling
> > debug messages (CONFIG_DEBUG_LEVEL=0) and applying Stephen's patch:
> > "tpm: Check for TPM related ACPI tables before attempting hw"
> >
> > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > ---
> >  src/bootsplash.c  |  3 +++
> >  src/fw/paravirt.c | 10 ++++++++++
> >  src/fw/paravirt.h |  4 ++++
> >  src/optionroms.c  |  3 ++-
> >  src/post.c        |  3 +++
> >  5 files changed, 22 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/bootsplash.c b/src/bootsplash.c
> > index 165c98d..0eda7f2 100644
> > --- a/src/bootsplash.c
> > +++ b/src/bootsplash.c
> > @@ -8,6 +8,7 @@
> >  #include "bregs.h" // struct bregs
> >  #include "config.h" // CONFIG_*
> >  #include "farptr.h" // FLATPTR_TO_SEG
> > +#include "fw/paravirt.h" // runningOnQEMUFastBoot
> >  #include "malloc.h" // free
> >  #include "output.h" // dprintf
> >  #include "romfile.h" // romfile_loadfile
> > @@ -39,6 +40,8 @@ call16_int10(struct bregs *br)
> >  void
> >  enable_vga_console(void)
> >  {
> > +    if(runningOnQEMUFastBoot())
> > +        return;
> >      dprintf(1, "Turning on vga text mode console\n");
> >      struct bregs br;
> >
> > diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> > index 0770c47..9e6e618 100644
> > --- a/src/fw/paravirt.c
> > +++ b/src/fw/paravirt.c
> > @@ -621,4 +621,14 @@ void qemu_cfg_init(void)
> >      if (nogfx && !romfile_find("etc/sercon-port")
> >          && !romfile_find("vgaroms/sgabios.bin"))
> >          const_romfile_add_int("etc/sercon-port", PORT_SERIAL1);
> > +
> > +    /*
> > +     * Enable QEMU fast boot if there is "linuxboot" optionrom and
> > +     * the boot menu is not required.
> > +     */
> > +    if ((romfile_find("genroms/linuxboot_dma.bin")
> > +        || romfile_find("genroms/linuxboot.bin"))
> > +        && !romfile_loadint("etc/show-boot-menu", 1)) {
> > +        PlatformRunningOn |= PF_QEMU_FB;
> > +    }
>
> I don't think we should hardcode special meanings to the names of
> bootable files.  If QEMU wants SeaBIOS to not perform some type of
> hardware init, then I think QEMU should explicitly request that from
> SeaBIOS (eg, a "etc/dont-run-hardware-init").

I agree, it is cleaner. That was one of my doubt but, with an explicit
request, it works only with a new version of QEMU.
Do you think is acceptable?

Thanks,
Stefano

>
> Thanks,
> -Kevin



-- 
Stefano Garzarella
Red Hat

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

Posted by Kevin O'Connor 1 week ago
On Wed, Nov 28, 2018 at 12:30:42PM +0100, Stefano Garzarella wrote:
> On Wed, Nov 28, 2018 at 3:12 AM Kevin O'Connor <kevin@koconnor.net> wrote:
> > On Mon, Nov 26, 2018 at 12:39:12PM +0100, Stefano Garzarella wrote:
> > > Speed up the boot phase when qemu uses "linuxboot" optionrom
> > > (qemu -kernel) and the boot-menu is not required.
> > > Under these conditions we can skip the setup of devices and VGA,
> > > because they will be initialized (if they are required) during
> > > the Linux boot phase.
> > >
> > > Following the time measured between SeaBIOS entry point and
> > > "linuxboot" entry point:
> > >
> > > * Before this patch
> > >   qemu -kernel  | qemu -vga none -kernel
> > >   --------------+-----------------------
> > >   53.5 msec     | 23.34 msec
> > >
> > > * After this patch
> > >   qemu -kernel  | qemu -vga none -kernel
> > >   --------------+-----------------------
> > >   12.82 msec    | 10.89 msec
> > >
> > > Note: For the measuring, we used the default configuration disabling
> > > debug messages (CONFIG_DEBUG_LEVEL=0) and applying Stephen's patch:
> > > "tpm: Check for TPM related ACPI tables before attempting hw"
> > >
> > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > ---
> > >  src/bootsplash.c  |  3 +++
> > >  src/fw/paravirt.c | 10 ++++++++++
> > >  src/fw/paravirt.h |  4 ++++
> > >  src/optionroms.c  |  3 ++-
> > >  src/post.c        |  3 +++
> > >  5 files changed, 22 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/src/bootsplash.c b/src/bootsplash.c
> > > index 165c98d..0eda7f2 100644
> > > --- a/src/bootsplash.c
> > > +++ b/src/bootsplash.c
> > > @@ -8,6 +8,7 @@
> > >  #include "bregs.h" // struct bregs
> > >  #include "config.h" // CONFIG_*
> > >  #include "farptr.h" // FLATPTR_TO_SEG
> > > +#include "fw/paravirt.h" // runningOnQEMUFastBoot
> > >  #include "malloc.h" // free
> > >  #include "output.h" // dprintf
> > >  #include "romfile.h" // romfile_loadfile
> > > @@ -39,6 +40,8 @@ call16_int10(struct bregs *br)
> > >  void
> > >  enable_vga_console(void)
> > >  {
> > > +    if(runningOnQEMUFastBoot())
> > > +        return;
> > >      dprintf(1, "Turning on vga text mode console\n");
> > >      struct bregs br;
> > >
> > > diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> > > index 0770c47..9e6e618 100644
> > > --- a/src/fw/paravirt.c
> > > +++ b/src/fw/paravirt.c
> > > @@ -621,4 +621,14 @@ void qemu_cfg_init(void)
> > >      if (nogfx && !romfile_find("etc/sercon-port")
> > >          && !romfile_find("vgaroms/sgabios.bin"))
> > >          const_romfile_add_int("etc/sercon-port", PORT_SERIAL1);
> > > +
> > > +    /*
> > > +     * Enable QEMU fast boot if there is "linuxboot" optionrom and
> > > +     * the boot menu is not required.
> > > +     */
> > > +    if ((romfile_find("genroms/linuxboot_dma.bin")
> > > +        || romfile_find("genroms/linuxboot.bin"))
> > > +        && !romfile_loadint("etc/show-boot-menu", 1)) {
> > > +        PlatformRunningOn |= PF_QEMU_FB;
> > > +    }
> >
> > I don't think we should hardcode special meanings to the names of
> > bootable files.  If QEMU wants SeaBIOS to not perform some type of
> > hardware init, then I think QEMU should explicitly request that from
> > SeaBIOS (eg, a "etc/dont-run-hardware-init").
> 
> I agree, it is cleaner. That was one of my doubt but, with an explicit
> request, it works only with a new version of QEMU.
> Do you think is acceptable?

It would be good to understand what the source of the delay is.  It
might be possible to avoid the delay without adding a new config
setting.  For example, the patch above will prevent the vga rom from
being run, but it should be possible to accomplish the same with
"-device VGA,romfile=".  The other change this patch makes is to skip
device_hardware_setup() - is it known which items in that function add
to the delay?

Also, what is the target boot time that is considered acceptable?

Thanks,
-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

Posted by Stefano Garzarella 1 week ago
On Wed, Nov 28, 2018 at 3:38 PM Kevin O'Connor <kevin@koconnor.net> wrote:
>
> On Wed, Nov 28, 2018 at 12:30:42PM +0100, Stefano Garzarella wrote:
> > On Wed, Nov 28, 2018 at 3:12 AM Kevin O'Connor <kevin@koconnor.net> wrote:
> > > On Mon, Nov 26, 2018 at 12:39:12PM +0100, Stefano Garzarella wrote:
> > > > Speed up the boot phase when qemu uses "linuxboot" optionrom
> > > > (qemu -kernel) and the boot-menu is not required.
> > > > Under these conditions we can skip the setup of devices and VGA,
> > > > because they will be initialized (if they are required) during
> > > > the Linux boot phase.
> > > >
> > > > Following the time measured between SeaBIOS entry point and
> > > > "linuxboot" entry point:
> > > >
> > > > * Before this patch
> > > >   qemu -kernel  | qemu -vga none -kernel
> > > >   --------------+-----------------------
> > > >   53.5 msec     | 23.34 msec
> > > >
> > > > * After this patch
> > > >   qemu -kernel  | qemu -vga none -kernel
> > > >   --------------+-----------------------
> > > >   12.82 msec    | 10.89 msec
> > > >
> > > > Note: For the measuring, we used the default configuration disabling
> > > > debug messages (CONFIG_DEBUG_LEVEL=0) and applying Stephen's patch:
> > > > "tpm: Check for TPM related ACPI tables before attempting hw"
> > > >
> > > > Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
> > > > ---
> > > >  src/bootsplash.c  |  3 +++
> > > >  src/fw/paravirt.c | 10 ++++++++++
> > > >  src/fw/paravirt.h |  4 ++++
> > > >  src/optionroms.c  |  3 ++-
> > > >  src/post.c        |  3 +++
> > > >  5 files changed, 22 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/src/bootsplash.c b/src/bootsplash.c
> > > > index 165c98d..0eda7f2 100644
> > > > --- a/src/bootsplash.c
> > > > +++ b/src/bootsplash.c
> > > > @@ -8,6 +8,7 @@
> > > >  #include "bregs.h" // struct bregs
> > > >  #include "config.h" // CONFIG_*
> > > >  #include "farptr.h" // FLATPTR_TO_SEG
> > > > +#include "fw/paravirt.h" // runningOnQEMUFastBoot
> > > >  #include "malloc.h" // free
> > > >  #include "output.h" // dprintf
> > > >  #include "romfile.h" // romfile_loadfile
> > > > @@ -39,6 +40,8 @@ call16_int10(struct bregs *br)
> > > >  void
> > > >  enable_vga_console(void)
> > > >  {
> > > > +    if(runningOnQEMUFastBoot())
> > > > +        return;
> > > >      dprintf(1, "Turning on vga text mode console\n");
> > > >      struct bregs br;
> > > >
> > > > diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
> > > > index 0770c47..9e6e618 100644
> > > > --- a/src/fw/paravirt.c
> > > > +++ b/src/fw/paravirt.c
> > > > @@ -621,4 +621,14 @@ void qemu_cfg_init(void)
> > > >      if (nogfx && !romfile_find("etc/sercon-port")
> > > >          && !romfile_find("vgaroms/sgabios.bin"))
> > > >          const_romfile_add_int("etc/sercon-port", PORT_SERIAL1);
> > > > +
> > > > +    /*
> > > > +     * Enable QEMU fast boot if there is "linuxboot" optionrom and
> > > > +     * the boot menu is not required.
> > > > +     */
> > > > +    if ((romfile_find("genroms/linuxboot_dma.bin")
> > > > +        || romfile_find("genroms/linuxboot.bin"))
> > > > +        && !romfile_loadint("etc/show-boot-menu", 1)) {
> > > > +        PlatformRunningOn |= PF_QEMU_FB;
> > > > +    }
> > >
> > > I don't think we should hardcode special meanings to the names of
> > > bootable files.  If QEMU wants SeaBIOS to not perform some type of
> > > hardware init, then I think QEMU should explicitly request that from
> > > SeaBIOS (eg, a "etc/dont-run-hardware-init").
> >
> > I agree, it is cleaner. That was one of my doubt but, with an explicit
> > request, it works only with a new version of QEMU.
> > Do you think is acceptable?
>
> It would be good to understand what the source of the delay is.  It
> might be possible to avoid the delay without adding a new config
> setting.  For example, the patch above will prevent the vga rom from
> being run, but it should be possible to accomplish the same with
> "-device VGA,romfile=".  The other change this patch makes is to skip
> device_hardware_setup() - is it known which items in that function add
> to the delay?

Hi Kevin,
thanks for your advice.

I tried to understand what are the sources of the delay, and I found
that most of the time is spent because of QEMU default devices (eg
e1000 ROM PCI, mouse, keyboard, etc)
At this point, (without the patch above) I disabled the default QEMU
devices (-nodefaults) and I reached 12.5 ms of SeaBIOS boot time
(compared to 22 ms, where 4 ms is taken by the loading of e1000 ROM).

Other 2 ms is taken by enable_vga_console(), where simply comment out
the printf("SeaBIOS (version %s)\n", VERSION) I reached 10.7 ms of
boot time.
Do you think is acceptable to use dprintf(1, ...) instead of printf()?
Or maybe check if we found the VGA.

Note: All these tests are done with CONFIG_DEBUG_LEVEL=0, using
CONFIG_DEBUG_LEVEL=1 the boot time grows up to 24 ms, maybe we should
put CONFIG_DEBUG_LEVEL=0 in the SeaBIOS configuration used in QEMU.

>
> Also, what is the target boot time that is considered acceptable?

I'm using qboot as a reference (6/7 ms). It is very minimal, so 10 ms
I think is considered acceptable for SeaBIOS

Thanks,
Stefano

>
> Thanks,
> -Kevin--
Stefano Garzarella
Red Hat

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

Posted by Kevin O'Connor 1 week ago
On Thu, Nov 29, 2018 at 03:16:39PM +0100, Stefano Garzarella wrote:
> On Wed, Nov 28, 2018 at 3:38 PM Kevin O'Connor <kevin@koconnor.net> wrote:
> > It would be good to understand what the source of the delay is.  It
> > might be possible to avoid the delay without adding a new config
> > setting.  For example, the patch above will prevent the vga rom from
> > being run, but it should be possible to accomplish the same with
> > "-device VGA,romfile=".  The other change this patch makes is to skip
> > device_hardware_setup() - is it known which items in that function add
> > to the delay?
> 
> Hi Kevin,
> thanks for your advice.
> 
> I tried to understand what are the sources of the delay, and I found
> that most of the time is spent because of QEMU default devices (eg
> e1000 ROM PCI, mouse, keyboard, etc)
> At this point, (without the patch above) I disabled the default QEMU
> devices (-nodefaults) and I reached 12.5 ms of SeaBIOS boot time
> (compared to 22 ms, where 4 ms is taken by the loading of e1000 ROM).
> 
> Other 2 ms is taken by enable_vga_console(), where simply comment out
> the printf("SeaBIOS (version %s)\n", VERSION) I reached 10.7 ms of
> boot time.
> Do you think is acceptable to use dprintf(1, ...) instead of printf()?
> Or maybe check if we found the VGA.

Interesting.  I tracked down this printf delay - it's due to the
save/restore of cpu state when thunking to 16bit mode.  (For every
character displayed on the screen the code enters 16bit mode to invoke
the vgabios and it saves/restores the cr0, gdt, fs, gs, a20, nmi
states during that process.)  It's trivial to eliminate the calls when
there is no vgabios though (see patch below).

> Note: All these tests are done with CONFIG_DEBUG_LEVEL=0, using
> CONFIG_DEBUG_LEVEL=1 the boot time grows up to 24 ms, maybe we should
> put CONFIG_DEBUG_LEVEL=0 in the SeaBIOS configuration used in QEMU.

I think the main seabios binary should have CONFIG_DEBUG_LEVEL=1 as it
helps with debug reports.  I suppose an additional binary could be
made for those looking for the fastest possible speed.  (The sole cost
of the debugging is the additional hardware accesses that results from
those debug messages.)

> > Also, what is the target boot time that is considered acceptable?
> 
> I'm using qboot as a reference (6/7 ms). It is very minimal, so 10 ms
> I think is considered acceptable for SeaBIOS

Thanks,
-Kevin


--- a/src/output.c
+++ b/src/output.c
@@ -74,6 +74,9 @@ static struct putcinfo debuginfo = { debug_putc };
 static void
 screenc(char c)
 {
+    if (!MODESEGMENT && GET_IVT(0x10).segoff == FUNC16(entry_10).segoff)
+        // No need to thunk to 16bit mode if vgabios is not present
+        return;
     struct bregs br;
     memset(&br, 0, sizeof(br));
     br.flags = F_IF;

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

Posted by Gerd Hoffmann 1 week ago
  Hi,

> > Note: All these tests are done with CONFIG_DEBUG_LEVEL=0, using
> > CONFIG_DEBUG_LEVEL=1 the boot time grows up to 24 ms, maybe we should
> > put CONFIG_DEBUG_LEVEL=0 in the SeaBIOS configuration used in QEMU.
> 
> I think the main seabios binary should have CONFIG_DEBUG_LEVEL=1 as it
> helps with debug reports.  I suppose an additional binary could be
> made for those looking for the fastest possible speed.  (The sole cost
> of the debugging is the additional hardware accesses that results from
> those debug messages.)

The qemu debugcon (CONFIG_DEBUG_IO) is detecable at runtime, it returns
0xe9 on port reads.  So we should be able to skip that too.  IIRC it
isn't *that* straightforward as seabios is initially mapped read/only so
a simple probe-on-first-putchar, then cache the result in a variable
doesn't work.  We could probe after make_bios_writable though which
should still avoid printing most of the messages.

cheers,
  Gerd


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

Posted by Stefano Garzarella 1 week ago
On Fri, Nov 30, 2018 at 2:06 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
>   Hi,
>
> > > Note: All these tests are done with CONFIG_DEBUG_LEVEL=0, using
> > > CONFIG_DEBUG_LEVEL=1 the boot time grows up to 24 ms, maybe we should
> > > put CONFIG_DEBUG_LEVEL=0 in the SeaBIOS configuration used in QEMU.
> >
> > I think the main seabios binary should have CONFIG_DEBUG_LEVEL=1 as it
> > helps with debug reports.  I suppose an additional binary could be
> > made for those looking for the fastest possible speed.  (The sole cost
> > of the debugging is the additional hardware accesses that results from
> > those debug messages.)
>
> The qemu debugcon (CONFIG_DEBUG_IO) is detecable at runtime, it returns
> 0xe9 on port reads.  So we should be able to skip that too.  IIRC it
> isn't *that* straightforward as seabios is initially mapped read/only so
> a simple probe-on-first-putchar, then cache the result in a variable
> doesn't work.  We could probe after make_bios_writable though which
> should still avoid printing most of the messages.

Great! I just tried the patch below and it works as you said. If I
don't have debugcon in QEMU the outs are avoided and the speed is not
bad (11.4 ms).
If I have debugcon (iobase=0x402), I can see the output.

Do you think can be an acceptable trade-off?

Thanks.
Stefano

diff --git a/src/fw/paravirt.c b/src/fw/paravirt.c
index 0770c47..31c080e 100644
--- a/src/fw/paravirt.c
+++ b/src/fw/paravirt.c
@@ -121,6 +121,10 @@ qemu_preinit(void)
         kvm_detect();
     }

+    // Detect qemu debugcon
+    if (inb(GET_GLOBAL(DebugOutputPort)) != QEMU_DEBUGCON_READBACK)
+        DebugOutputPort = 0;
+
     // On emulators, get memory size from nvram.
     u32 rs = ((rtc_read(CMOS_MEM_EXTMEM2_LOW) << 16)
               | (rtc_read(CMOS_MEM_EXTMEM2_HIGH) << 24));
diff --git a/src/fw/paravirt.h b/src/fw/paravirt.h
index a14d83e..f7e1d4c 100644
--- a/src/fw/paravirt.h
+++ b/src/fw/paravirt.h
@@ -49,6 +49,9 @@ static inline int runningOnKVM(void) {
 // QEMU_CFG_DMA ID bit
 #define QEMU_CFG_VERSION_DMA    2

+// QEMU debugcon read value
+#define QEMU_DEBUGCON_READBACK  0xe9
+
 int qemu_cfg_enabled(void);
 int qemu_cfg_dma_enabled(void);
 void qemu_preinit(void);
diff --git a/src/hw/serialio.c b/src/hw/serialio.c
index 319a85c..4e0f2ac 100644
--- a/src/hw/serialio.c
+++ b/src/hw/serialio.c
@@ -107,7 +107,7 @@ u16 DebugOutputPort VARFSEG = 0x402;
 void
 qemu_debug_putc(char c)
 {
-    if (CONFIG_DEBUG_IO && runningOnQEMU())
+    if (CONFIG_DEBUG_IO && runningOnQEMU() && GET_GLOBAL(DebugOutputPort))
         // Send character to debug port.
         outb(c, GET_GLOBAL(DebugOutputPort));
 }


>
> cheers,
>   Gerd
>


-- 
Stefano Garzarella
Red Hat

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

Posted by Kevin O'Connor 1 week ago
On Fri, Nov 30, 2018 at 03:27:21PM +0100, Stefano Garzarella wrote:
> On Fri, Nov 30, 2018 at 2:06 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> > > > Note: All these tests are done with CONFIG_DEBUG_LEVEL=0, using
> > > > CONFIG_DEBUG_LEVEL=1 the boot time grows up to 24 ms, maybe we should
> > > > put CONFIG_DEBUG_LEVEL=0 in the SeaBIOS configuration used in QEMU.
> > >
> > > I think the main seabios binary should have CONFIG_DEBUG_LEVEL=1 as it
> > > helps with debug reports.  I suppose an additional binary could be
> > > made for those looking for the fastest possible speed.  (The sole cost
> > > of the debugging is the additional hardware accesses that results from
> > > those debug messages.)
> >
> > The qemu debugcon (CONFIG_DEBUG_IO) is detecable at runtime, it returns
> > 0xe9 on port reads.  So we should be able to skip that too.  IIRC it
> > isn't *that* straightforward as seabios is initially mapped read/only so
> > a simple probe-on-first-putchar, then cache the result in a variable
> > doesn't work.  We could probe after make_bios_writable though which
> > should still avoid printing most of the messages.
> 
> Great! I just tried the patch below and it works as you said. If I
> don't have debugcon in QEMU the outs are avoided and the speed is not
> bad (11.4 ms).
> If I have debugcon (iobase=0x402), I can see the output.
> 
> Do you think can be an acceptable trade-off?

Makes sense to me.

> index 0770c47..31c080e 100644
> --- a/src/fw/paravirt.c
> +++ b/src/fw/paravirt.c
> @@ -121,6 +121,10 @@ qemu_preinit(void)
>          kvm_detect();
>      }
> 
> +    // Detect qemu debugcon
> +    if (inb(GET_GLOBAL(DebugOutputPort)) != QEMU_DEBUGCON_READBACK)
> +        DebugOutputPort = 0;

As a minor quibble, this needs to be "if (CONFIG_DEBUG_IO && ...)" to
ensure it gets compiled out if debug io is not in use.  Also, I think
it would be preferable to introduce a serial_debug_postram_preinit()
method in src/hw/serialio.c that does this check.

> --- a/src/hw/serialio.c
> +++ b/src/hw/serialio.c
> @@ -107,7 +107,7 @@ u16 DebugOutputPort VARFSEG = 0x402;
>  void
>  qemu_debug_putc(char c)
>  {
> -    if (CONFIG_DEBUG_IO && runningOnQEMU())
> +    if (CONFIG_DEBUG_IO && runningOnQEMU() && GET_GLOBAL(DebugOutputPort))
>          // Send character to debug port.
>          outb(c, GET_GLOBAL(DebugOutputPort));
>  }

As a minor nit, it would be preferable to invoke
GET_GLOBAL(DebugOutputPort) only once in the success case.

-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

Posted by Stefano Garzarella 6 days ago
On Fri, Nov 30, 2018 at 8:36 PM Kevin O'Connor <kevin@koconnor.net> wrote:
>
> On Fri, Nov 30, 2018 at 03:27:21PM +0100, Stefano Garzarella wrote:
> > On Fri, Nov 30, 2018 at 2:06 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
> > > > > Note: All these tests are done with CONFIG_DEBUG_LEVEL=0, using
> > > > > CONFIG_DEBUG_LEVEL=1 the boot time grows up to 24 ms, maybe we should
> > > > > put CONFIG_DEBUG_LEVEL=0 in the SeaBIOS configuration used in QEMU.
> > > >
> > > > I think the main seabios binary should have CONFIG_DEBUG_LEVEL=1 as it
> > > > helps with debug reports.  I suppose an additional binary could be
> > > > made for those looking for the fastest possible speed.  (The sole cost
> > > > of the debugging is the additional hardware accesses that results from
> > > > those debug messages.)
> > >
> > > The qemu debugcon (CONFIG_DEBUG_IO) is detecable at runtime, it returns
> > > 0xe9 on port reads.  So we should be able to skip that too.  IIRC it
> > > isn't *that* straightforward as seabios is initially mapped read/only so
> > > a simple probe-on-first-putchar, then cache the result in a variable
> > > doesn't work.  We could probe after make_bios_writable though which
> > > should still avoid printing most of the messages.
> >
> > Great! I just tried the patch below and it works as you said. If I
> > don't have debugcon in QEMU the outs are avoided and the speed is not
> > bad (11.4 ms).
> > If I have debugcon (iobase=0x402), I can see the output.
> >
> > Do you think can be an acceptable trade-off?
>
> Makes sense to me.
>
> > index 0770c47..31c080e 100644
> > --- a/src/fw/paravirt.c
> > +++ b/src/fw/paravirt.c
> > @@ -121,6 +121,10 @@ qemu_preinit(void)
> >          kvm_detect();
> >      }
> >
> > +    // Detect qemu debugcon
> > +    if (inb(GET_GLOBAL(DebugOutputPort)) != QEMU_DEBUGCON_READBACK)
> > +        DebugOutputPort = 0;
>
> As a minor quibble, this needs to be "if (CONFIG_DEBUG_IO && ...)" to
> ensure it gets compiled out if debug io is not in use.  Also, I think
> it would be preferable to introduce a serial_debug_postram_preinit()
> method in src/hw/serialio.c that does this check.
>
> > --- a/src/hw/serialio.c
> > +++ b/src/hw/serialio.c
> > @@ -107,7 +107,7 @@ u16 DebugOutputPort VARFSEG = 0x402;
> >  void
> >  qemu_debug_putc(char c)
> >  {
> > -    if (CONFIG_DEBUG_IO && runningOnQEMU())
> > +    if (CONFIG_DEBUG_IO && runningOnQEMU() && GET_GLOBAL(DebugOutputPort))
> >          // Send character to debug port.
> >          outb(c, GET_GLOBAL(DebugOutputPort));
> >  }
>
> As a minor nit, it would be preferable to invoke
> GET_GLOBAL(DebugOutputPort) only once in the success case.
>
> -Kevin

Hi Kevin,
thank you very much, I sent a patch following your suggestions.

Cheers,
Stefano


-- 
Stefano Garzarella
Red Hat

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

Posted by Stefano Garzarella 1 week ago
On Thu, Nov 29, 2018 at 5:55 PM Kevin O'Connor <kevin@koconnor.net> wrote:
>
> On Thu, Nov 29, 2018 at 03:16:39PM +0100, Stefano Garzarella wrote:
> > On Wed, Nov 28, 2018 at 3:38 PM Kevin O'Connor <kevin@koconnor.net> wrote:
> > > It would be good to understand what the source of the delay is.  It
> > > might be possible to avoid the delay without adding a new config
> > > setting.  For example, the patch above will prevent the vga rom from
> > > being run, but it should be possible to accomplish the same with
> > > "-device VGA,romfile=".  The other change this patch makes is to skip
> > > device_hardware_setup() - is it known which items in that function add
> > > to the delay?
> >
> > Hi Kevin,
> > thanks for your advice.
> >
> > I tried to understand what are the sources of the delay, and I found
> > that most of the time is spent because of QEMU default devices (eg
> > e1000 ROM PCI, mouse, keyboard, etc)
> > At this point, (without the patch above) I disabled the default QEMU
> > devices (-nodefaults) and I reached 12.5 ms of SeaBIOS boot time
> > (compared to 22 ms, where 4 ms is taken by the loading of e1000 ROM).
> >
> > Other 2 ms is taken by enable_vga_console(), where simply comment out
> > the printf("SeaBIOS (version %s)\n", VERSION) I reached 10.7 ms of
> > boot time.
> > Do you think is acceptable to use dprintf(1, ...) instead of printf()?
> > Or maybe check if we found the VGA.
>
> Interesting.  I tracked down this printf delay - it's due to the
> save/restore of cpu state when thunking to 16bit mode.  (For every
> character displayed on the screen the code enters 16bit mode to invoke
> the vgabios and it saves/restores the cr0, gdt, fs, gs, a20, nmi
> states during that process.)  It's trivial to eliminate the calls when
> there is no vgabios though (see patch below).

Thanks, the patch works, but unfortunately, when I use qemu
-nographic, the  /etc/sercon-port is set to PORT_SERIAL1 (in
src/fw/paravirt.c:623), bypassing the patch.
Maybe in QEMU is better to set /etc/sercom-port to 0 when there is no
serial port, or when we want a fast boot.

>
> > Note: All these tests are done with CONFIG_DEBUG_LEVEL=0, using
> > CONFIG_DEBUG_LEVEL=1 the boot time grows up to 24 ms, maybe we should
> > put CONFIG_DEBUG_LEVEL=0 in the SeaBIOS configuration used in QEMU.
>
> I think the main seabios binary should have CONFIG_DEBUG_LEVEL=1 as it
> helps with debug reports.  I suppose an additional binary could be
> made for those looking for the fastest possible speed.  (The sole cost
> of the debugging is the additional hardware accesses that results from
> those debug messages.)

Okay, I'll discuss it in the QEMU mailing list.

>
> > > Also, what is the target boot time that is considered acceptable?
> >
> > I'm using qboot as a reference (6/7 ms). It is very minimal, so 10 ms
> > I think is considered acceptable for SeaBIOS
>
> Thanks,
> -Kevin
>
>
> --- a/src/output.c
> +++ b/src/output.c
> @@ -74,6 +74,9 @@ static struct putcinfo debuginfo = { debug_putc };
>  static void
>  screenc(char c)
>  {
> +    if (!MODESEGMENT && GET_IVT(0x10).segoff == FUNC16(entry_10).segoff)
> +        // No need to thunk to 16bit mode if vgabios is not present
> +        return;
>      struct bregs br;
>      memset(&br, 0, sizeof(br));
>      br.flags = F_IF;

Do you plan to commit this patch?

Thanks,
Stefano


--
Stefano Garzarella
Red Hat

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

Posted by Kevin O'Connor 6 days ago
On Fri, Nov 30, 2018 at 11:23:30AM +0100, Stefano Garzarella wrote:
> On Thu, Nov 29, 2018 at 5:55 PM Kevin O'Connor <kevin@koconnor.net> wrote:
> > Interesting.  I tracked down this printf delay - it's due to the
> > save/restore of cpu state when thunking to 16bit mode.  (For every
> > character displayed on the screen the code enters 16bit mode to invoke
> > the vgabios and it saves/restores the cr0, gdt, fs, gs, a20, nmi
> > states during that process.)  It's trivial to eliminate the calls when
> > there is no vgabios though (see patch below).
> 
> Thanks, the patch works, but unfortunately, when I use qemu
> -nographic, the  /etc/sercon-port is set to PORT_SERIAL1 (in
> src/fw/paravirt.c:623), bypassing the patch.
> Maybe in QEMU is better to set /etc/sercom-port to 0 when there is no
> serial port, or when we want a fast boot.

You should be able to use "-device VGA,romfile=" instead.

> > --- a/src/output.c
> > +++ b/src/output.c
> > @@ -74,6 +74,9 @@ static struct putcinfo debuginfo = { debug_putc };
> >  static void
> >  screenc(char c)
> >  {
> > +    if (!MODESEGMENT && GET_IVT(0x10).segoff == FUNC16(entry_10).segoff)
> > +        // No need to thunk to 16bit mode if vgabios is not present
> > +        return;
> >      struct bregs br;
> >      memset(&br, 0, sizeof(br));
> >      br.flags = F_IF;
> 
> Do you plan to commit this patch?

Only if it's useful - does sercon make it not worthwhile?

-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios

Re: [SeaBIOS] [PATCH v2] qemu: fast boot when linuxboot optionrom is used

Posted by Stefano Garzarella 1 day ago
Hi Kevin,

On Wed, Dec 5, 2018 at 6:00 PM Kevin O'Connor <kevin@koconnor.net> wrote:
>
> On Fri, Nov 30, 2018 at 11:23:30AM +0100, Stefano Garzarella wrote:
> > On Thu, Nov 29, 2018 at 5:55 PM Kevin O'Connor <kevin@koconnor.net> wrote:
> > > Interesting.  I tracked down this printf delay - it's due to the
> > > save/restore of cpu state when thunking to 16bit mode.  (For every
> > > character displayed on the screen the code enters 16bit mode to invoke
> > > the vgabios and it saves/restores the cr0, gdt, fs, gs, a20, nmi
> > > states during that process.)  It's trivial to eliminate the calls when
> > > there is no vgabios though (see patch below).
> >
> > Thanks, the patch works, but unfortunately, when I use qemu
> > -nographic, the  /etc/sercon-port is set to PORT_SERIAL1 (in
> > src/fw/paravirt.c:623), bypassing the patch.
> > Maybe in QEMU is better to set /etc/sercom-port to 0 when there is no
> > serial port, or when we want a fast boot.
>
> You should be able to use "-device VGA,romfile=" instead.

My issue was releated to "-nographic" option. In this case if I
disable the VGA or if I use "-device VGA,romfile=", at the end of the
qemu_cfg_init() there are these lines that enable in any case the
sercom:

    // serial console
    u16 nogfx = 0;
    qemu_cfg_read_entry(&nogfx, QEMU_CFG_NOGRAPHIC, sizeof(nogfx));
    if (nogfx && !romfile_find("etc/sercon-port")
        && !romfile_find("vgaroms/sgabios.bin"))
        const_romfile_add_int("etc/sercon-port", PORT_SERIAL1);
}


>
> > > --- a/src/output.c
> > > +++ b/src/output.c
> > > @@ -74,6 +74,9 @@ static struct putcinfo debuginfo = { debug_putc };
> > >  static void
> > >  screenc(char c)
> > >  {
> > > +    if (!MODESEGMENT && GET_IVT(0x10).segoff == FUNC16(entry_10).segoff)
> > > +        // No need to thunk to 16bit mode if vgabios is not present
> > > +        return;
> > >      struct bregs br;
> > >      memset(&br, 0, sizeof(br));
> > >      br.flags = F_IF;
> >
> > Do you plan to commit this patch?
>
> Only if it's useful - does sercon make it not worthwhile?

Yes, I think it is useful. If I don't use "-nographic" it works as expected.

Thanks,
Stefano
>
> -Kevin



--
Stefano Garzarella
Red Hat

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios