[PATCH for-5.2] microvm: fix kconfig for non-default devices

[PATCH for-5.2] microvm: fix kconfig for non-default devices

[Paolo Bonzini]

PCIe and USB support can be disabled on the command line, and therefore
should not be included if QEMU is configured --without-default-devices.

While at it, also remove the "default y" for USB_XHCI_SYSBUS because
sysbus devices are not user creatable; boards that use them will
specify them manually with "imply" or "select" clauses.

Reported-by: Bruce Rogers <brogers@suse.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 hw/i386/Kconfig | 4 ++--
 hw/usb/Kconfig  | 1 -
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index eea059ffef..16296dda84 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -96,6 +96,8 @@ config Q35
 
 config MICROVM
     bool
+    imply PCI_EXPRESS_GENERIC_BRIDGE
+    imply USB_XHCI_SYSBUS
     select SERIAL_ISA # for serial_hds_isa_init()
     select ISA_BUS
     select APIC
@@ -104,8 +106,6 @@ config MICROVM
     select MC146818RTC
     select VIRTIO_MMIO
     select ACPI_HW_REDUCED
-    select PCI_EXPRESS_GENERIC_BRIDGE
-    select USB_XHCI_SYSBUS
 
 config X86_IOMMU
     bool
diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 3b07d9cf68..7fbae18bc8 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -47,7 +47,6 @@ config USB_XHCI_NEC
 
 config USB_XHCI_SYSBUS
     bool
-    default y
     select USB_XHCI
 
 config USB_MUSB
-- 
2.28.0

Re: [PATCH for-5.2] microvm: fix kconfig for non-default devices

[Gerd Hoffmann]

On Fri, Nov 20, 2020 at 09:55:24AM +0100, Paolo Bonzini wrote:
> PCIe and USB support can be disabled on the command line, and therefore
> should not be included if QEMU is configured --without-default-devices.

I don't think this will work.  I expect linking will fail due to
xhci_sysbus_build_aml and acpi_dsdt_add_gpex being not available.

take care,
  Gerd

Re: [PATCH for-5.2] microvm: fix kconfig for non-default devices

[Philippe Mathieu-Daudé]

On 11/20/20 9:55 AM, Paolo Bonzini wrote:
> PCIe and USB support can be disabled on the command line, and therefore
> should not be included if QEMU is configured --without-default-devices.
> 
> While at it, also remove the "default y" for USB_XHCI_SYSBUS because
> sysbus devices are not user creatable; boards that use them will
> specify them manually with "imply" or "select" clauses.
> 
> Reported-by: Bruce Rogers <brogers@suse.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  hw/i386/Kconfig | 4 ++--
>  hw/usb/Kconfig  | 1 -
>  2 files changed, 2 insertions(+), 3 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Re: [PATCH for-5.2] microvm: fix kconfig for non-default devices

[Paolo Bonzini]

On 20/11/20 10:38, Gerd Hoffmann wrote:
> On Fri, Nov 20, 2020 at 09:55:24AM +0100, Paolo Bonzini wrote:
>> PCIe and USB support can be disabled on the command line, and therefore
>> should not be included if QEMU is configured --without-default-devices.
> 
> I don't think this will work.  I expect linking will fail due to
> xhci_sysbus_build_aml and acpi_dsdt_add_gpex being not available.

Hmm, you're right.  On the other hand that also means that you're 
missing a depends on ACPI for USB_XHCI_SYSBUS.  What about moving those 
two directly to hw/acpi/aml-build.c?

Paolo

Re: [PATCH for-5.2] microvm: fix kconfig for non-default devices

[Gerd Hoffmann]

On Fri, Nov 20, 2020 at 01:08:40PM +0100, Paolo Bonzini wrote:
> On 20/11/20 10:38, Gerd Hoffmann wrote:
> > On Fri, Nov 20, 2020 at 09:55:24AM +0100, Paolo Bonzini wrote:
> > > PCIe and USB support can be disabled on the command line, and therefore
> > > should not be included if QEMU is configured --without-default-devices.
> > 
> > I don't think this will work.  I expect linking will fail due to
> > xhci_sysbus_build_aml and acpi_dsdt_add_gpex being not available.
> 
> Hmm, you're right.  On the other hand that also means that you're missing a
> depends on ACPI for USB_XHCI_SYSBUS.

No, we have hw/acpi/aml-build-stub.c ...

> What about moving those two directly
> to hw/acpi/aml-build.c?

Well, I like the aml generator being grouped with the device code and
not at a completely different place.

We could try plumb this through a sysbus class function, so the direct
symbol reference goes away.  Simliar to ISADeviceClass->build_aml().
Maybe also add build_dt() while being at it.

Problem is that with the way sysbus resource management is designed it
isn't that easy to do.  The device just provides the resources (memory
regions, irqs), the machine maps them.  So the device has no clue where
memory is mapped and how the irq is wired.  Also sysbus devices can be
alot more complex than isa devices which basically have ioports and irq
and nothing else.

take care,
  Gerd