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

[Kevin O'Connor]

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 at 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