Re: [PATCH v9 14/15] hw/i386: Introduce the microvm machine type

[Philippe Mathieu-Daudé]

Hi Paolo,

On 10/16/19 11:29 AM, Paolo Bonzini wrote:
> On 16/10/19 11:26, Philippe Mathieu-Daudé wrote:
> > On 10/16/19 9:46 AM, Paolo Bonzini wrote:
> > > > diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
> > > > index c5c9d4900e..d399dcba52 100644
> > > > --- a/hw/i386/Kconfig
> > > > +++ b/hw/i386/Kconfig
> > > > @@ -92,6 +92,10 @@ config Q35
> > > >        select SMBIOS
> > > >        select FW_CFG_DMA
> > > >    +config MICROVM
> > > > +    bool
> > >
> > > Missing:
> > >
> > >       select ISA_BUS
> > >       select APIC
> > >       select IOAPIC
> > >       select I8259
> > >       select MC146818RTC
> >
> > maybe 'select SERIAL_ISA' too?
>
> Right, but only 'imply'.

Per the documentation, you are correct:

  Boards specify their constituent devices using ``imply`` and ``select``
  directives.  A device should be listed under ``select`` if the board
  cannot be started at all without it.  It should be listed under
  ``imply`` if (depending on the QEMU command line) the board may or
  may not be started without it.  Boards also default to false; they are
  enabled by the ``default-configs/*.mak`` for the target they apply to.

But then the build fails configured with --without-default-devices:

  LINK    x86_64-softmmu/qemu-system-x86_64
/usr/bin/ld: hw/i386/microvm.o: in function `microvm_devices_init':
hw/i386/microvm.c:157: undefined reference to `serial_hds_isa_init'
collect2: error: ld returned 1 exit status
make[1]: *** [Makefile:206: qemu-system-x86_64] Error 1
make: *** [Makefile:483: x86_64-softmmu/all] Error 2

We have:

static void microvm_devices_init(MicrovmMachineState *mms)
{
    ...
    if (mms->isa_serial) {
        serial_hds_isa_init(isa_bus, 0, 1);
    }
    ...
}

With this diff the link succeed:

-- >8 --
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -95,7 +95,7 @@ config Q35

 config MICROVM
     bool
-    imply SERIAL_ISA
+    select SERIAL_ISA
     select ISA_BUS
     select APIC
     select IOAPIC
---

I was going to send this as a patch but suddently remembered this 
thread. I'm not sure what you want so I prefer ask first :)

We have CONFIG_SERIAL_ISA declared in "config-devices.h" so we could 
also use:

-- >8 --
--- a/hw/i386/microvm.c
+++ b/hw/i386/microvm.c
@@ -153,9 +153,11 @@ static void 
microvm_devices_init(MicrovmMachineState *mms)
         microvm_set_rtc(mms, rtc_state);
     }

+#ifdef CONFIG_SERIAL_ISA
     if (mms->isa_serial) {
         serial_hds_isa_init(isa_bus, 0, 1);
     }
+#endif

     if (bios_name == NULL) {
         bios_name = MICROVM_BIOS_FILENAME;
---

The binary links too, and the difference with the other hunk is now the 
device is not listed in 'qemu-system-x86_64 -M microvm -device help'.
However I guess understand we prefer to avoid that kind of ifdef'ry.

Which way to you prefer?

Thanks,

Phil.