Re: [PATCH v2 4/9] microvm: add usb support

[Igor Mammedov <imammedo@redhat.com>]

On Thu,  1 Oct 2020 10:28:29 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:

> Wire up "usb=on" machine option, when enabled add
> a sysbus xhci controller with 8 ports.

first pci, and now usb - not so micro anymore.

why not just reuse pci to add usb controller?
what/how much do we win avoiding PCI?

> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/hw/i386/microvm.h |  5 ++++-
>  hw/i386/acpi-microvm.c    |  9 +++++++++
>  hw/i386/microvm.c         | 13 +++++++++++++
>  3 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
> index 91b064575d55..0fc216007777 100644
> --- a/include/hw/i386/microvm.h
> +++ b/include/hw/i386/microvm.h
> @@ -41,7 +41,7 @@
>   *   7     |  parallel  |
>   *   8     |  rtc       | rtc (rtc=on)
>   *   9     |  acpi      | acpi (ged)
> - *  10     |  pci lnk   |
> + *  10     |  pci lnk   | xhci (usb=on)
>   *  11     |  pci lnk   |
>   *  12     |  ps2       | pcie
>   *  13     |  fpu       | pcie
> @@ -60,6 +60,9 @@
>  #define GED_MMIO_BASE_REGS    (GED_MMIO_BASE + 0x200)
>  #define GED_MMIO_IRQ          9
>  
> +#define MICROVM_XHCI_BASE     0xfe900000
> +#define MICROVM_XHCI_IRQ      10
> +
>  #define PCIE_MMIO_BASE        0xc0000000
>  #define PCIE_MMIO_SIZE        0x20000000
>  #define PCIE_ECAM_BASE        0xe0000000
> diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
> index f16f2311955c..7e8a6894ba26 100644
> --- a/hw/i386/acpi-microvm.c
> +++ b/hw/i386/acpi-microvm.c
> @@ -35,6 +35,7 @@
>  #include "hw/i386/microvm.h"
>  #include "hw/pci/pci.h"
>  #include "hw/pci/pcie_host.h"
> +#include "hw/usb/xhci.h"
>  #include "hw/virtio/virtio-mmio.h"
>  
>  #include "acpi-common.h"
> @@ -89,6 +90,13 @@ static void acpi_dsdt_add_virtio(Aml *scope,
>      }
>  }
>  
> +static void acpi_dsdt_add_xhci(Aml *scope, MicrovmMachineState *mms)
> +{
> +    if (machine_usb(MACHINE(mms))) {
> +        xhci_sysbus_build_aml(scope, MICROVM_XHCI_BASE, MICROVM_XHCI_IRQ);
> +    }
> +}
> +
>  static void acpi_dsdt_add_pci(Aml *scope, MicrovmMachineState *mms)
>  {
>      if (mms->pcie != ON_OFF_AUTO_ON) {
> @@ -123,6 +131,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
>                    GED_MMIO_IRQ, AML_SYSTEM_MEMORY, GED_MMIO_BASE);
>      acpi_dsdt_add_power_button(sb_scope);
>      acpi_dsdt_add_virtio(sb_scope, mms);
> +    acpi_dsdt_add_xhci(sb_scope, mms);
>      acpi_dsdt_add_pci(sb_scope, mms);
>      aml_append(dsdt, sb_scope);
>  
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index 17e3f2f15265..0f61fc61ca44 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -47,6 +47,7 @@
>  #include "hw/acpi/acpi.h"
>  #include "hw/acpi/generic_event_device.h"
>  #include "hw/pci-host/gpex.h"
> +#include "hw/usb/xhci.h"
>  
>  #include "cpu.h"
>  #include "elf.h"
> @@ -197,6 +198,18 @@ static void microvm_devices_init(MicrovmMachineState *mms)
>          x86ms->acpi_dev = HOTPLUG_HANDLER(dev);
>      }
>  
> +    if (x86_machine_is_acpi_enabled(x86ms) && machine_usb(MACHINE(mms))) {
> +        DeviceState *dev = qdev_new(TYPE_XHCI_SYSBUS);
> +        qdev_prop_set_uint32(dev, "intrs", 1);
> +        qdev_prop_set_uint32(dev, "slots", XHCI_MAXSLOTS);
> +        qdev_prop_set_uint32(dev, "p2", 8);
> +        qdev_prop_set_uint32(dev, "p3", 8);
> +        sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
> +        sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, MICROVM_XHCI_BASE);
> +        sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
> +                           x86ms->gsi[MICROVM_XHCI_IRQ]);
> +    }
> +
>      if (x86_machine_is_acpi_enabled(x86ms) && mms->pcie == ON_OFF_AUTO_ON) {
>          /* use topmost 25% of the address space available */
>          hwaddr phys_size = (hwaddr)1 << X86_CPU(first_cpu)->phys_bits;