[PATCH v3 00/10] microvm: add usb support

[PATCH v3 00/10] microvm: add usb support

[Gerd Hoffmann]

This depends on the pending microvm pull request.

v2:
 - add some xhci cleanups.
 - move aml builder to sysbus-xhci.
 - add acpi testcase for usb=on (and while being at it rtc=on too).
v3:
 - fix Kconfig deps for XHCI

Gerd Hoffmann (10):
  acpi: add aml builder stubs
  usb/xhci: add include/hw/usb/xhci.h header file
  usb/xhci: add xhci_sysbus_build_aml() helper
  usb/xhci: fixup xhci kconfig deps
  microvm: add usb support
  tests/acpi: allow updates for expected data files
  tests/acpi: add empty tests/data/acpi/microvm/DSDT.{usb,rtc} files
  tests/acpi: add microvm usb test
  tests/acpi: add microvm rtc test
  tests/acpi: update expected data files

 hw/usb/hcd-xhci-sysbus.h         |   1 -
 hw/usb/hcd-xhci.h                |  22 ++++++--------------
 include/hw/i386/microvm.h        |   5 ++++-
 include/hw/usb/xhci.h            |  21 +++++++++++++++++++
 hw/acpi/aml-build-stub.c         |  14 +++++++++++++
 hw/i386/acpi-microvm.c           |   9 ++++++++
 hw/i386/microvm.c                |  13 ++++++++++++
 hw/usb/hcd-xhci-nec.c            |   4 ++--
 hw/usb/hcd-xhci-pci.c            |   4 ++--
 hw/usb/hcd-xhci-sysbus.c         |  19 +++++++++++++++--
 hw/usb/hcd-xhci.c                |  34 +++++++++++++++----------------
 tests/qtest/bios-tables-test.c   |  24 ++++++++++++++++++++++
 hw/i386/Kconfig                  |   1 +
 hw/usb/Kconfig                   |   6 ++----
 tests/data/acpi/microvm/DSDT.rtc | Bin 0 -> 404 bytes
 tests/data/acpi/microvm/DSDT.usb | Bin 0 -> 414 bytes
 16 files changed, 131 insertions(+), 46 deletions(-)
 create mode 100644 include/hw/usb/xhci.h
 create mode 100644 tests/data/acpi/microvm/DSDT.rtc
 create mode 100644 tests/data/acpi/microvm/DSDT.usb

-- 
2.27.0

[PATCH v3 01/10] acpi: add aml builder stubs

[Gerd Hoffmann]

Add stubs for aml_interrupt and aml_memory32_fixed,
these will be needed by followup patches,

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/acpi/aml-build-stub.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/hw/acpi/aml-build-stub.c b/hw/acpi/aml-build-stub.c
index 58b2e162277f..8d8ad1a31497 100644
--- a/hw/acpi/aml-build-stub.c
+++ b/hw/acpi/aml-build-stub.c
@@ -57,6 +57,20 @@ Aml *aml_irq_no_flags(uint8_t irq)
     return NULL;
 }
 
+Aml *aml_interrupt(AmlConsumerAndProducer con_and_pro,
+                   AmlLevelAndEdge level_and_edge,
+                   AmlActiveHighAndLow high_and_low, AmlShared shared,
+                   uint32_t *irq_list, uint8_t irq_count)
+{
+    return NULL;
+}
+
+Aml *aml_memory32_fixed(uint32_t addr, uint32_t size,
+                        AmlReadAndWrite read_and_write)
+{
+    return NULL;
+}
+
 Aml *aml_int(const uint64_t val)
 {
     return NULL;
-- 
2.27.0

[PATCH v3 02/10] usb/xhci: add include/hw/usb/xhci.h header file

[Gerd Hoffmann]

Move a bunch of defines which might be needed outside core xhci
code to that place.  Add XHCI_ prefixes to avoid name clashes.
No functional change.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/hcd-xhci-sysbus.h |  1 -
 hw/usb/hcd-xhci.h        | 22 ++++++----------------
 include/hw/usb/xhci.h    | 19 +++++++++++++++++++
 hw/usb/hcd-xhci-nec.c    |  4 ++--
 hw/usb/hcd-xhci-pci.c    |  4 ++--
 hw/usb/hcd-xhci-sysbus.c |  4 ++--
 hw/usb/hcd-xhci.c        | 34 ++++++++++++++++------------------
 7 files changed, 47 insertions(+), 41 deletions(-)
 create mode 100644 include/hw/usb/xhci.h

diff --git a/hw/usb/hcd-xhci-sysbus.h b/hw/usb/hcd-xhci-sysbus.h
index a308753ceb9b..fdfcbbee3bf4 100644
--- a/hw/usb/hcd-xhci-sysbus.h
+++ b/hw/usb/hcd-xhci-sysbus.h
@@ -15,7 +15,6 @@
 #include "hcd-xhci.h"
 #include "hw/sysbus.h"
 
-#define TYPE_XHCI_SYSBUS "sysbus-xhci"
 #define XHCI_SYSBUS(obj) \
     OBJECT_CHECK(XHCISysbusState, (obj), TYPE_XHCI_SYSBUS)
 
diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
index f859a17e73ee..ccf50ae28bed 100644
--- a/hw/usb/hcd-xhci.h
+++ b/hw/usb/hcd-xhci.h
@@ -24,23 +24,13 @@
 #include "qom/object.h"
 
 #include "hw/usb.h"
+#include "hw/usb/xhci.h"
 #include "sysemu/dma.h"
 
-#define TYPE_XHCI "base-xhci"
-#define TYPE_NEC_XHCI "nec-usb-xhci"
-#define TYPE_QEMU_XHCI "qemu-xhci"
-
 OBJECT_DECLARE_SIMPLE_TYPE(XHCIState, XHCI)
 
-#define MAXPORTS_2 15
-#define MAXPORTS_3 15
-
-#define MAXPORTS (MAXPORTS_2 + MAXPORTS_3)
-#define MAXSLOTS 64
-#define MAXINTRS 16
-
 /* Very pessimistic, let's hope it's enough for all cases */
-#define EV_QUEUE (((3 * 24) + 16) * MAXSLOTS)
+#define EV_QUEUE (((3 * 24) + 16) * XHCI_MAXSLOTS)
 
 typedef struct XHCIStreamContext XHCIStreamContext;
 typedef struct XHCIEPContext XHCIEPContext;
@@ -217,15 +207,15 @@ typedef struct XHCIState {
     uint32_t dcbaap_high;
     uint32_t config;
 
-    USBPort  uports[MAX_CONST(MAXPORTS_2, MAXPORTS_3)];
-    XHCIPort ports[MAXPORTS];
-    XHCISlot slots[MAXSLOTS];
+    USBPort  uports[MAX_CONST(XHCI_MAXPORTS_2, XHCI_MAXPORTS_3)];
+    XHCIPort ports[XHCI_MAXPORTS];
+    XHCISlot slots[XHCI_MAXSLOTS];
     uint32_t numports;
 
     /* Runtime Registers */
     int64_t mfindex_start;
     QEMUTimer *mfwrap_timer;
-    XHCIInterrupter intr[MAXINTRS];
+    XHCIInterrupter intr[XHCI_MAXINTRS];
 
     XHCIRing cmd_ring;
 
diff --git a/include/hw/usb/xhci.h b/include/hw/usb/xhci.h
new file mode 100644
index 000000000000..dc0c29930dcc
--- /dev/null
+++ b/include/hw/usb/xhci.h
@@ -0,0 +1,19 @@
+#ifndef HW_USB_XHCI_H
+#define HW_USB_XHCI_H
+
+#define TYPE_XHCI "base-xhci"
+#define TYPE_NEC_XHCI "nec-usb-xhci"
+#define TYPE_QEMU_XHCI "qemu-xhci"
+#define TYPE_XHCI_SYSBUS "sysbus-xhci"
+
+#define XHCI_MAXPORTS_2 15
+#define XHCI_MAXPORTS_3 15
+
+#define XHCI_MAXPORTS (XHCI_MAXPORTS_2 + XHCI_MAXPORTS_3)
+#define XHCI_MAXSLOTS 64
+#define XHCI_MAXINTRS 16
+
+/* must be power of 2 */
+#define XHCI_LEN_REGS 0x4000
+
+#endif
diff --git a/hw/usb/hcd-xhci-nec.c b/hw/usb/hcd-xhci-nec.c
index 2efa6fa0f8af..5707b2cabd16 100644
--- a/hw/usb/hcd-xhci-nec.c
+++ b/hw/usb/hcd-xhci-nec.c
@@ -34,8 +34,8 @@ static Property nec_xhci_properties[] = {
                     xhci.flags, XHCI_FLAG_SS_FIRST, true),
     DEFINE_PROP_BIT("force-pcie-endcap", XHCIPciState, xhci.flags,
                     XHCI_FLAG_FORCE_PCIE_ENDCAP, false),
-    DEFINE_PROP_UINT32("intrs", XHCIPciState, xhci.numintrs, MAXINTRS),
-    DEFINE_PROP_UINT32("slots", XHCIPciState, xhci.numslots, MAXSLOTS),
+    DEFINE_PROP_UINT32("intrs", XHCIPciState, xhci.numintrs, XHCI_MAXINTRS),
+    DEFINE_PROP_UINT32("slots", XHCIPciState, xhci.numslots, XHCI_MAXSLOTS),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
index a6d746e1da38..b78fcd2bb2d5 100644
--- a/hw/usb/hcd-xhci-pci.c
+++ b/hw/usb/hcd-xhci-pci.c
@@ -240,8 +240,8 @@ static void qemu_xhci_instance_init(Object *obj)
 
     s->msi      = ON_OFF_AUTO_OFF;
     s->msix     = ON_OFF_AUTO_AUTO;
-    xhci->numintrs = MAXINTRS;
-    xhci->numslots = MAXSLOTS;
+    xhci->numintrs = XHCI_MAXINTRS;
+    xhci->numslots = XHCI_MAXSLOTS;
     xhci_set_flag(xhci, XHCI_FLAG_SS_FIRST);
 }
 
diff --git a/hw/usb/hcd-xhci-sysbus.c b/hw/usb/hcd-xhci-sysbus.c
index 852ca5103b4d..570618467356 100644
--- a/hw/usb/hcd-xhci-sysbus.c
+++ b/hw/usb/hcd-xhci-sysbus.c
@@ -69,8 +69,8 @@ static void xhci_sysbus_instance_init(Object *obj)
 }
 
 static Property xhci_sysbus_props[] = {
-    DEFINE_PROP_UINT32("intrs", XHCISysbusState, xhci.numintrs, MAXINTRS),
-    DEFINE_PROP_UINT32("slots", XHCISysbusState, xhci.numslots, MAXSLOTS),
+    DEFINE_PROP_UINT32("intrs", XHCISysbusState, xhci.numintrs, XHCI_MAXINTRS),
+    DEFINE_PROP_UINT32("slots", XHCISysbusState, xhci.numslots, XHCI_MAXSLOTS),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 5e8bed9ef90c..79ce5c4be6c4 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -46,15 +46,13 @@
 #define TRANSFER_LIMIT  256
 
 #define LEN_CAP         0x40
-#define LEN_OPER        (0x400 + 0x10 * MAXPORTS)
-#define LEN_RUNTIME     ((MAXINTRS + 1) * 0x20)
-#define LEN_DOORBELL    ((MAXSLOTS + 1) * 0x20)
+#define LEN_OPER        (0x400 + 0x10 * XHCI_MAXPORTS)
+#define LEN_RUNTIME     ((XHCI_MAXINTRS + 1) * 0x20)
+#define LEN_DOORBELL    ((XHCI_MAXSLOTS + 1) * 0x20)
 
 #define OFF_OPER        LEN_CAP
 #define OFF_RUNTIME     0x1000
 #define OFF_DOORBELL    0x2000
-/* must be power of 2 */
-#define LEN_REGS        0x4000
 
 #if (OFF_OPER + LEN_OPER) > OFF_RUNTIME
 #error Increase OFF_RUNTIME
@@ -62,8 +60,8 @@
 #if (OFF_RUNTIME + LEN_RUNTIME) > OFF_DOORBELL
 #error Increase OFF_DOORBELL
 #endif
-#if (OFF_DOORBELL + LEN_DOORBELL) > LEN_REGS
-# error Increase LEN_REGS
+#if (OFF_DOORBELL + LEN_DOORBELL) > XHCI_LEN_REGS
+# error Increase XHCI_LEN_REGS
 #endif
 
 /* bit definitions */
@@ -3276,11 +3274,11 @@ static void usb_xhci_init(XHCIState *xhci)
 
     xhci->usbsts = USBSTS_HCH;
 
-    if (xhci->numports_2 > MAXPORTS_2) {
-        xhci->numports_2 = MAXPORTS_2;
+    if (xhci->numports_2 > XHCI_MAXPORTS_2) {
+        xhci->numports_2 = XHCI_MAXPORTS_2;
     }
-    if (xhci->numports_3 > MAXPORTS_3) {
-        xhci->numports_3 = MAXPORTS_3;
+    if (xhci->numports_3 > XHCI_MAXPORTS_3) {
+        xhci->numports_3 = XHCI_MAXPORTS_3;
     }
     usbports = MAX(xhci->numports_2, xhci->numports_3);
     xhci->numports = xhci->numports_2 + xhci->numports_3;
@@ -3302,7 +3300,7 @@ static void usb_xhci_init(XHCIState *xhci)
                 USB_SPEED_MASK_LOW  |
                 USB_SPEED_MASK_FULL |
                 USB_SPEED_MASK_HIGH;
-            assert(i < MAXPORTS);
+            assert(i < XHCI_MAXPORTS);
             snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
             speedmask |= port->speedmask;
         }
@@ -3316,7 +3314,7 @@ static void usb_xhci_init(XHCIState *xhci)
             }
             port->uport = &xhci->uports[i];
             port->speedmask = USB_SPEED_MASK_SUPER;
-            assert(i < MAXPORTS);
+            assert(i < XHCI_MAXPORTS);
             snprintf(port->name, sizeof(port->name), "usb3 port #%d", i+1);
             speedmask |= port->speedmask;
         }
@@ -3331,8 +3329,8 @@ static void usb_xhci_realize(DeviceState *dev, Error **errp)
 
     XHCIState *xhci = XHCI(dev);
 
-    if (xhci->numintrs > MAXINTRS) {
-        xhci->numintrs = MAXINTRS;
+    if (xhci->numintrs > XHCI_MAXINTRS) {
+        xhci->numintrs = XHCI_MAXINTRS;
     }
     while (xhci->numintrs & (xhci->numintrs - 1)) {   /* ! power of 2 */
         xhci->numintrs++;
@@ -3340,8 +3338,8 @@ static void usb_xhci_realize(DeviceState *dev, Error **errp)
     if (xhci->numintrs < 1) {
         xhci->numintrs = 1;
     }
-    if (xhci->numslots > MAXSLOTS) {
-        xhci->numslots = MAXSLOTS;
+    if (xhci->numslots > XHCI_MAXSLOTS) {
+        xhci->numslots = XHCI_MAXSLOTS;
     }
     if (xhci->numslots < 1) {
         xhci->numslots = 1;
@@ -3355,7 +3353,7 @@ static void usb_xhci_realize(DeviceState *dev, Error **errp)
     usb_xhci_init(xhci);
     xhci->mfwrap_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, xhci_mfwrap_timer, xhci);
 
-    memory_region_init(&xhci->mem, OBJECT(dev), "xhci", LEN_REGS);
+    memory_region_init(&xhci->mem, OBJECT(dev), "xhci", XHCI_LEN_REGS);
     memory_region_init_io(&xhci->mem_cap, OBJECT(dev), &xhci_cap_ops, xhci,
                           "capabilities", LEN_CAP);
     memory_region_init_io(&xhci->mem_oper, OBJECT(dev), &xhci_oper_ops, xhci,
-- 
2.27.0

[PATCH v3 03/10] usb/xhci: add xhci_sysbus_build_aml() helper

[Gerd Hoffmann]

The helper generates an acpi dsdt device entry
for the xhci sysbus device.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/hw/usb/xhci.h    |  2 ++
 hw/usb/hcd-xhci-sysbus.c | 15 +++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/include/hw/usb/xhci.h b/include/hw/usb/xhci.h
index dc0c29930dcc..5c90e1373e55 100644
--- a/include/hw/usb/xhci.h
+++ b/include/hw/usb/xhci.h
@@ -16,4 +16,6 @@
 /* must be power of 2 */
 #define XHCI_LEN_REGS 0x4000
 
+void xhci_sysbus_build_aml(Aml *scope, uint32_t mmio, unsigned int irq);
+
 #endif
diff --git a/hw/usb/hcd-xhci-sysbus.c b/hw/usb/hcd-xhci-sysbus.c
index 570618467356..29185d2261fb 100644
--- a/hw/usb/hcd-xhci-sysbus.c
+++ b/hw/usb/hcd-xhci-sysbus.c
@@ -13,6 +13,7 @@
 #include "trace.h"
 #include "qapi/error.h"
 #include "hcd-xhci-sysbus.h"
+#include "hw/acpi/aml-build.h"
 #include "hw/irq.h"
 
 static void xhci_sysbus_intr_raise(XHCIState *xhci, int n, bool level)
@@ -68,6 +69,20 @@ static void xhci_sysbus_instance_init(Object *obj)
     s->xhci.intr_raise = xhci_sysbus_intr_raise;
 }
 
+void xhci_sysbus_build_aml(Aml *scope, uint32_t mmio, unsigned int irq)
+{
+    Aml *dev = aml_device("XHCI");
+    Aml *crs = aml_resource_template();
+
+    aml_append(crs, aml_memory32_fixed(mmio, XHCI_LEN_REGS, AML_READ_WRITE));
+    aml_append(crs, aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
+                                  AML_EXCLUSIVE, &irq, 1));
+
+    aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0D10")));
+    aml_append(dev, aml_name_decl("_CRS", crs));
+    aml_append(scope, dev);
+}
+
 static Property xhci_sysbus_props[] = {
     DEFINE_PROP_UINT32("intrs", XHCISysbusState, xhci.numintrs, XHCI_MAXINTRS),
     DEFINE_PROP_UINT32("slots", XHCISysbusState, xhci.numslots, XHCI_MAXSLOTS),
-- 
2.27.0

[PATCH v3 04/10] usb/xhci: fixup xhci kconfig deps

[Gerd Hoffmann]

USB_XHCI does not depend on PCI any more.
USB_XHCI_SYSBUS must select USB_XHCI not USB.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/usb/Kconfig | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index 4dd2ba9630cb..a674ce4c542e 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -32,8 +32,6 @@ config USB_EHCI_SYSBUS
 
 config USB_XHCI
     bool
-    default y if PCI_DEVICES
-    depends on PCI
     select USB
 
 config USB_XHCI_PCI
@@ -50,8 +48,8 @@ config USB_XHCI_NEC
 
 config USB_XHCI_SYSBUS
     bool
-    default y if USB_XHCI
-    select USB
+    default y
+    select USB_XHCI
 
 config USB_MUSB
     bool
-- 
2.27.0

[PATCH v3 05/10] microvm: add usb support

[Gerd Hoffmann]

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

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 +++++++++++++
 hw/i386/Kconfig           |  1 +
 4 files changed, 27 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 68a7f424acf9..6672cfa254ee 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;
diff --git a/hw/i386/Kconfig b/hw/i386/Kconfig
index 32aa15533bd8..eea059ffef54 100644
--- a/hw/i386/Kconfig
+++ b/hw/i386/Kconfig
@@ -105,6 +105,7 @@ config MICROVM
     select VIRTIO_MMIO
     select ACPI_HW_REDUCED
     select PCI_EXPRESS_GENERIC_BRIDGE
+    select USB_XHCI_SYSBUS
 
 config X86_IOMMU
     bool
-- 
2.27.0

[PATCH v3 06/10] tests/acpi: allow updates for expected data files

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index dfb8523c8bf4..afd6b04a475a 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1 +1,3 @@
 /* List of comma-separated changed AML files to ignore */
+"tests/data/acpi/microvm/DSDT.usb",
+"tests/data/acpi/microvm/DSDT.rtc",
-- 
2.27.0

[PATCH v3 07/10] tests/acpi: add empty tests/data/acpi/microvm/DSDT.{usb, rtc} files

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 tests/data/acpi/microvm/DSDT.rtc | 0
 tests/data/acpi/microvm/DSDT.usb | 0
 2 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tests/data/acpi/microvm/DSDT.rtc
 create mode 100644 tests/data/acpi/microvm/DSDT.usb

diff --git a/tests/data/acpi/microvm/DSDT.rtc b/tests/data/acpi/microvm/DSDT.rtc
new file mode 100644
index 000000000000..e69de29bb2d1
diff --git a/tests/data/acpi/microvm/DSDT.usb b/tests/data/acpi/microvm/DSDT.usb
new file mode 100644
index 000000000000..e69de29bb2d1
-- 
2.27.0

[PATCH v3 08/10] tests/acpi: add microvm usb test

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 tests/qtest/bios-tables-test.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 564762449233..17e11800274d 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1110,6 +1110,17 @@ static void test_acpi_microvm_tcg(void)
     free_test_data(&data);
 }
 
+static void test_acpi_microvm_usb_tcg(void)
+{
+    test_data data;
+
+    test_acpi_microvm_prepare(&data);
+    data.variant = ".usb";
+    test_acpi_one(" -machine microvm,acpi=on,usb=on,rtc=off",
+                  &data);
+    free_test_data(&data);
+}
+
 static void test_acpi_microvm_pcie_tcg(void)
 {
     test_data data;
@@ -1246,6 +1257,7 @@ int main(int argc, char *argv[])
         qtest_add_func("acpi/piix4/acpihmat", test_acpi_piix4_tcg_acpi_hmat);
         qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
         qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
+        qtest_add_func("acpi/microvm/usb", test_acpi_microvm_usb_tcg);
         if (strcmp(arch, "x86_64") == 0) {
             qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
         }
-- 
2.27.0

[PATCH v3 09/10] tests/acpi: add microvm rtc test

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 tests/qtest/bios-tables-test.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 17e11800274d..3830a40d1041 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -1121,6 +1121,17 @@ static void test_acpi_microvm_usb_tcg(void)
     free_test_data(&data);
 }
 
+static void test_acpi_microvm_rtc_tcg(void)
+{
+    test_data data;
+
+    test_acpi_microvm_prepare(&data);
+    data.variant = ".rtc";
+    test_acpi_one(" -machine microvm,acpi=on,rtc=on",
+                  &data);
+    free_test_data(&data);
+}
+
 static void test_acpi_microvm_pcie_tcg(void)
 {
     test_data data;
@@ -1258,6 +1269,7 @@ int main(int argc, char *argv[])
         qtest_add_func("acpi/q35/acpihmat", test_acpi_q35_tcg_acpi_hmat);
         qtest_add_func("acpi/microvm", test_acpi_microvm_tcg);
         qtest_add_func("acpi/microvm/usb", test_acpi_microvm_usb_tcg);
+        qtest_add_func("acpi/microvm/rtc", test_acpi_microvm_rtc_tcg);
         if (strcmp(arch, "x86_64") == 0) {
             qtest_add_func("acpi/microvm/pcie", test_acpi_microvm_pcie_tcg);
         }
-- 
2.27.0

[PATCH v3 10/10] tests/acpi: update expected data files

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 tests/qtest/bios-tables-test-allowed-diff.h |   2 --
 tests/data/acpi/microvm/DSDT.rtc            | Bin 0 -> 404 bytes
 tests/data/acpi/microvm/DSDT.usb            | Bin 0 -> 414 bytes
 3 files changed, 2 deletions(-)

diff --git a/tests/qtest/bios-tables-test-allowed-diff.h b/tests/qtest/bios-tables-test-allowed-diff.h
index afd6b04a475a..dfb8523c8bf4 100644
--- a/tests/qtest/bios-tables-test-allowed-diff.h
+++ b/tests/qtest/bios-tables-test-allowed-diff.h
@@ -1,3 +1 @@
 /* List of comma-separated changed AML files to ignore */
-"tests/data/acpi/microvm/DSDT.usb",
-"tests/data/acpi/microvm/DSDT.rtc",
diff --git a/tests/data/acpi/microvm/DSDT.rtc b/tests/data/acpi/microvm/DSDT.rtc
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..5960d6929a2f88d76aaa324b88b80908ff0698dc 100644
GIT binary patch
literal 404
zcmXv~%TB^T6ur|@C{v1~(GX2c2#H%WV2JB>I+P{_YNvonO@>ckYuq3U64&|xe1f0i
z=ddE)p}Lv-I_KVVCQ?!HMF1=w&cs-C!{<yy*uxf>+(b6L(kj&D_I|V!BUg`;$bXWP
zIp=%;u2w}rJy(ldo%@u=ggMM1tyd_w12LQQPxw~w;}BQRlL%i!II9SZ`Y}1!2OHcT
z1H-w?3!(ox9<DcgPp~ztfw(;&2A0-i8C5tx5Lv=`{{g{xz{kNKtxdg;I@qL%CXlp|
zxs0I&us_mT`R<ugNF~$m<*k;BqPaH3YR^93R@4G&?XqPa(oMO&Hc4eXn`T^aQQblz
zVQrD~$7+^N^BI21uo}_^iWdPIHWqGZXV(M*?e8O40H}+Q&Pf+zFGx^BebJ~<2V1|a
Cp;Z9@

literal 0
HcmV?d00001

diff --git a/tests/data/acpi/microvm/DSDT.usb b/tests/data/acpi/microvm/DSDT.usb
index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..14423381ea235ed42f6f0f7d98e793c271a4e7c1 100644
GIT binary patch
literal 414
zcmY+A%}T>S6ov1kO`A@eI*M2ngyI9}v{KxfOr}YoKQZYaM3_E;bSG|V76o^Ld+8(i
zC_YYC#XB8cxS6^4-1#`?PLKtYPXb`A{IQ6#w!cV3fEr4mYkjlmOl7{xi|hVe^c@w&
zf%_nbQ_gu89F<KxT6dI4vtyUikT9Fsr11=S(G}xi=kUJaecQxQ(>TCW14jm6)D6k5
zv@3yg#XvKIN+<(mOgxdqoOf=~1W6YMSZSXot-n>EL~580p|wn92n~Sko|g01x^9Kc
z<e+_espM=@d(zEv%RXL~)B<X4l6mUWbzYq7!k8tagbOZ=7Y6amvy|VN*<h58aoe65
z(kiA99&VzuT+r601_YX4d$0g77a=W^tGHQ2BL4dTm;DFeeGm7)06hSVJ)%R_Qzz9j
HwNd&31|3%c

literal 0
HcmV?d00001

-- 
2.27.0

RE: [PATCH v3 02/10] usb/xhci: add include/hw/usb/xhci.h header file

[Sai Pavan Boddu]

Hi,
> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Tuesday, October 20, 2020 1:19 PM
> To: qemu-devel@nongnu.org
> Cc: Sergio Lopez <slp@redhat.com>; Sai Pavan Boddu <saipava@xilinx.com>;
> Igor Mammedov <imammedo@redhat.com>; Michael S. Tsirkin
> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Paolo Bonzini <pbonzini@redhat.com>; Thomas Huth <thuth@redhat.com>;
> Richard Henderson <rth@twiddle.net>; Gerd Hoffmann <kraxel@redhat.com>;
> Eduardo Habkost <ehabkost@redhat.com>; Laurent Vivier
> <lvivier@redhat.com>
> Subject: [PATCH v3 02/10] usb/xhci: add include/hw/usb/xhci.h header file
> 
> Move a bunch of defines which might be needed outside core xhci code to that
> place.  Add XHCI_ prefixes to avoid name clashes.
> No functional change.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
[Sai Pavan Boddu] Looks good.

Reviewed-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>

Regards,
Sai Pavan


> ---
>  hw/usb/hcd-xhci-sysbus.h |  1 -
>  hw/usb/hcd-xhci.h        | 22 ++++++----------------
>  include/hw/usb/xhci.h    | 19 +++++++++++++++++++
>  hw/usb/hcd-xhci-nec.c    |  4 ++--
>  hw/usb/hcd-xhci-pci.c    |  4 ++--
>  hw/usb/hcd-xhci-sysbus.c |  4 ++--
>  hw/usb/hcd-xhci.c        | 34 ++++++++++++++++------------------
>  7 files changed, 47 insertions(+), 41 deletions(-)  create mode 100644
> include/hw/usb/xhci.h
> 
> diff --git a/hw/usb/hcd-xhci-sysbus.h b/hw/usb/hcd-xhci-sysbus.h index
> a308753ceb9b..fdfcbbee3bf4 100644
> --- a/hw/usb/hcd-xhci-sysbus.h
> +++ b/hw/usb/hcd-xhci-sysbus.h
> @@ -15,7 +15,6 @@
>  #include "hcd-xhci.h"
>  #include "hw/sysbus.h"
> 
> -#define TYPE_XHCI_SYSBUS "sysbus-xhci"
>  #define XHCI_SYSBUS(obj) \
>      OBJECT_CHECK(XHCISysbusState, (obj), TYPE_XHCI_SYSBUS)
> 
> diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h index
> f859a17e73ee..ccf50ae28bed 100644
> --- a/hw/usb/hcd-xhci.h
> +++ b/hw/usb/hcd-xhci.h
> @@ -24,23 +24,13 @@
>  #include "qom/object.h"
> 
>  #include "hw/usb.h"
> +#include "hw/usb/xhci.h"
>  #include "sysemu/dma.h"
> 
> -#define TYPE_XHCI "base-xhci"
> -#define TYPE_NEC_XHCI "nec-usb-xhci"
> -#define TYPE_QEMU_XHCI "qemu-xhci"
> -
>  OBJECT_DECLARE_SIMPLE_TYPE(XHCIState, XHCI)
> 
> -#define MAXPORTS_2 15
> -#define MAXPORTS_3 15
> -
> -#define MAXPORTS (MAXPORTS_2 + MAXPORTS_3) -#define MAXSLOTS 64 -
> #define MAXINTRS 16
> -
>  /* Very pessimistic, let's hope it's enough for all cases */ -#define EV_QUEUE
> (((3 * 24) + 16) * MAXSLOTS)
> +#define EV_QUEUE (((3 * 24) + 16) * XHCI_MAXSLOTS)
> 
>  typedef struct XHCIStreamContext XHCIStreamContext;  typedef struct
> XHCIEPContext XHCIEPContext; @@ -217,15 +207,15 @@ typedef struct
> XHCIState {
>      uint32_t dcbaap_high;
>      uint32_t config;
> 
> -    USBPort  uports[MAX_CONST(MAXPORTS_2, MAXPORTS_3)];
> -    XHCIPort ports[MAXPORTS];
> -    XHCISlot slots[MAXSLOTS];
> +    USBPort  uports[MAX_CONST(XHCI_MAXPORTS_2, XHCI_MAXPORTS_3)];
> +    XHCIPort ports[XHCI_MAXPORTS];
> +    XHCISlot slots[XHCI_MAXSLOTS];
>      uint32_t numports;
> 
>      /* Runtime Registers */
>      int64_t mfindex_start;
>      QEMUTimer *mfwrap_timer;
> -    XHCIInterrupter intr[MAXINTRS];
> +    XHCIInterrupter intr[XHCI_MAXINTRS];
> 
>      XHCIRing cmd_ring;
> 
> diff --git a/include/hw/usb/xhci.h b/include/hw/usb/xhci.h new file mode
> 100644 index 000000000000..dc0c29930dcc
> --- /dev/null
> +++ b/include/hw/usb/xhci.h
> @@ -0,0 +1,19 @@
> +#ifndef HW_USB_XHCI_H
> +#define HW_USB_XHCI_H
> +
> +#define TYPE_XHCI "base-xhci"
> +#define TYPE_NEC_XHCI "nec-usb-xhci"
> +#define TYPE_QEMU_XHCI "qemu-xhci"
> +#define TYPE_XHCI_SYSBUS "sysbus-xhci"
> +
> +#define XHCI_MAXPORTS_2 15
> +#define XHCI_MAXPORTS_3 15
> +
> +#define XHCI_MAXPORTS (XHCI_MAXPORTS_2 + XHCI_MAXPORTS_3) #define
> +XHCI_MAXSLOTS 64 #define XHCI_MAXINTRS 16
> +
> +/* must be power of 2 */
> +#define XHCI_LEN_REGS 0x4000
> +
> +#endif
> diff --git a/hw/usb/hcd-xhci-nec.c b/hw/usb/hcd-xhci-nec.c index
> 2efa6fa0f8af..5707b2cabd16 100644
> --- a/hw/usb/hcd-xhci-nec.c
> +++ b/hw/usb/hcd-xhci-nec.c
> @@ -34,8 +34,8 @@ static Property nec_xhci_properties[] = {
>                      xhci.flags, XHCI_FLAG_SS_FIRST, true),
>      DEFINE_PROP_BIT("force-pcie-endcap", XHCIPciState, xhci.flags,
>                      XHCI_FLAG_FORCE_PCIE_ENDCAP, false),
> -    DEFINE_PROP_UINT32("intrs", XHCIPciState, xhci.numintrs, MAXINTRS),
> -    DEFINE_PROP_UINT32("slots", XHCIPciState, xhci.numslots, MAXSLOTS),
> +    DEFINE_PROP_UINT32("intrs", XHCIPciState, xhci.numintrs,
> XHCI_MAXINTRS),
> +    DEFINE_PROP_UINT32("slots", XHCIPciState, xhci.numslots,
> + XHCI_MAXSLOTS),
>      DEFINE_PROP_END_OF_LIST(),
>  };
> 
> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c index
> a6d746e1da38..b78fcd2bb2d5 100644
> --- a/hw/usb/hcd-xhci-pci.c
> +++ b/hw/usb/hcd-xhci-pci.c
> @@ -240,8 +240,8 @@ static void qemu_xhci_instance_init(Object *obj)
> 
>      s->msi      = ON_OFF_AUTO_OFF;
>      s->msix     = ON_OFF_AUTO_AUTO;
> -    xhci->numintrs = MAXINTRS;
> -    xhci->numslots = MAXSLOTS;
> +    xhci->numintrs = XHCI_MAXINTRS;
> +    xhci->numslots = XHCI_MAXSLOTS;
>      xhci_set_flag(xhci, XHCI_FLAG_SS_FIRST);  }
> 
> diff --git a/hw/usb/hcd-xhci-sysbus.c b/hw/usb/hcd-xhci-sysbus.c index
> 852ca5103b4d..570618467356 100644
> --- a/hw/usb/hcd-xhci-sysbus.c
> +++ b/hw/usb/hcd-xhci-sysbus.c
> @@ -69,8 +69,8 @@ static void xhci_sysbus_instance_init(Object *obj)  }
> 
>  static Property xhci_sysbus_props[] = {
> -    DEFINE_PROP_UINT32("intrs", XHCISysbusState, xhci.numintrs, MAXINTRS),
> -    DEFINE_PROP_UINT32("slots", XHCISysbusState, xhci.numslots, MAXSLOTS),
> +    DEFINE_PROP_UINT32("intrs", XHCISysbusState, xhci.numintrs,
> XHCI_MAXINTRS),
> +    DEFINE_PROP_UINT32("slots", XHCISysbusState, xhci.numslots,
> + XHCI_MAXSLOTS),
>      DEFINE_PROP_END_OF_LIST(),
>  };
> 
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index
> 5e8bed9ef90c..79ce5c4be6c4 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -46,15 +46,13 @@
>  #define TRANSFER_LIMIT  256
> 
>  #define LEN_CAP         0x40
> -#define LEN_OPER        (0x400 + 0x10 * MAXPORTS)
> -#define LEN_RUNTIME     ((MAXINTRS + 1) * 0x20)
> -#define LEN_DOORBELL    ((MAXSLOTS + 1) * 0x20)
> +#define LEN_OPER        (0x400 + 0x10 * XHCI_MAXPORTS)
> +#define LEN_RUNTIME     ((XHCI_MAXINTRS + 1) * 0x20)
> +#define LEN_DOORBELL    ((XHCI_MAXSLOTS + 1) * 0x20)
> 
>  #define OFF_OPER        LEN_CAP
>  #define OFF_RUNTIME     0x1000
>  #define OFF_DOORBELL    0x2000
> -/* must be power of 2 */
> -#define LEN_REGS        0x4000
> 
>  #if (OFF_OPER + LEN_OPER) > OFF_RUNTIME  #error Increase OFF_RUNTIME
> @@ -62,8 +60,8 @@  #if (OFF_RUNTIME + LEN_RUNTIME) > OFF_DOORBELL
> #error Increase OFF_DOORBELL  #endif -#if (OFF_DOORBELL + LEN_DOORBELL)
> > LEN_REGS -# error Increase LEN_REGS
> +#if (OFF_DOORBELL + LEN_DOORBELL) > XHCI_LEN_REGS # error Increase
> +XHCI_LEN_REGS
>  #endif
> 
>  /* bit definitions */
> @@ -3276,11 +3274,11 @@ static void usb_xhci_init(XHCIState *xhci)
> 
>      xhci->usbsts = USBSTS_HCH;
> 
> -    if (xhci->numports_2 > MAXPORTS_2) {
> -        xhci->numports_2 = MAXPORTS_2;
> +    if (xhci->numports_2 > XHCI_MAXPORTS_2) {
> +        xhci->numports_2 = XHCI_MAXPORTS_2;
>      }
> -    if (xhci->numports_3 > MAXPORTS_3) {
> -        xhci->numports_3 = MAXPORTS_3;
> +    if (xhci->numports_3 > XHCI_MAXPORTS_3) {
> +        xhci->numports_3 = XHCI_MAXPORTS_3;
>      }
>      usbports = MAX(xhci->numports_2, xhci->numports_3);
>      xhci->numports = xhci->numports_2 + xhci->numports_3; @@ -3302,7
> +3300,7 @@ static void usb_xhci_init(XHCIState *xhci)
>                  USB_SPEED_MASK_LOW  |
>                  USB_SPEED_MASK_FULL |
>                  USB_SPEED_MASK_HIGH;
> -            assert(i < MAXPORTS);
> +            assert(i < XHCI_MAXPORTS);
>              snprintf(port->name, sizeof(port->name), "usb2 port #%d", i+1);
>              speedmask |= port->speedmask;
>          }
> @@ -3316,7 +3314,7 @@ static void usb_xhci_init(XHCIState *xhci)
>              }
>              port->uport = &xhci->uports[i];
>              port->speedmask = USB_SPEED_MASK_SUPER;
> -            assert(i < MAXPORTS);
> +            assert(i < XHCI_MAXPORTS);
>              snprintf(port->name, sizeof(port->name), "usb3 port #%d", i+1);
>              speedmask |= port->speedmask;
>          }
> @@ -3331,8 +3329,8 @@ static void usb_xhci_realize(DeviceState *dev, Error
> **errp)
> 
>      XHCIState *xhci = XHCI(dev);
> 
> -    if (xhci->numintrs > MAXINTRS) {
> -        xhci->numintrs = MAXINTRS;
> +    if (xhci->numintrs > XHCI_MAXINTRS) {
> +        xhci->numintrs = XHCI_MAXINTRS;
>      }
>      while (xhci->numintrs & (xhci->numintrs - 1)) {   /* ! power of 2 */
>          xhci->numintrs++;
> @@ -3340,8 +3338,8 @@ static void usb_xhci_realize(DeviceState *dev, Error
> **errp)
>      if (xhci->numintrs < 1) {
>          xhci->numintrs = 1;
>      }
> -    if (xhci->numslots > MAXSLOTS) {
> -        xhci->numslots = MAXSLOTS;
> +    if (xhci->numslots > XHCI_MAXSLOTS) {
> +        xhci->numslots = XHCI_MAXSLOTS;
>      }
>      if (xhci->numslots < 1) {
>          xhci->numslots = 1;
> @@ -3355,7 +3353,7 @@ static void usb_xhci_realize(DeviceState *dev, Error
> **errp)
>      usb_xhci_init(xhci);
>      xhci->mfwrap_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL,
> xhci_mfwrap_timer, xhci);
> 
> -    memory_region_init(&xhci->mem, OBJECT(dev), "xhci", LEN_REGS);
> +    memory_region_init(&xhci->mem, OBJECT(dev), "xhci", XHCI_LEN_REGS);
>      memory_region_init_io(&xhci->mem_cap, OBJECT(dev), &xhci_cap_ops,
> xhci,
>                            "capabilities", LEN_CAP);
>      memory_region_init_io(&xhci->mem_oper, OBJECT(dev), &xhci_oper_ops,
> xhci,
> --
> 2.27.0

RE: [PATCH v3 04/10] usb/xhci: fixup xhci kconfig deps

[Sai Pavan Boddu]

Hi,

> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Tuesday, October 20, 2020 1:19 PM
> To: qemu-devel@nongnu.org
> Cc: Sergio Lopez <slp@redhat.com>; Sai Pavan Boddu <saipava@xilinx.com>;
> Igor Mammedov <imammedo@redhat.com>; Michael S. Tsirkin
> <mst@redhat.com>; Marcel Apfelbaum <marcel.apfelbaum@gmail.com>;
> Paolo Bonzini <pbonzini@redhat.com>; Thomas Huth <thuth@redhat.com>;
> Richard Henderson <rth@twiddle.net>; Gerd Hoffmann <kraxel@redhat.com>;
> Eduardo Habkost <ehabkost@redhat.com>; Laurent Vivier
> <lvivier@redhat.com>
> Subject: [PATCH v3 04/10] usb/xhci: fixup xhci kconfig deps
> 
> USB_XHCI does not depend on PCI any more.
> USB_XHCI_SYSBUS must select USB_XHCI not USB.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
[Sai Pavan Boddu] 
Looks clean now. I forgot to take care of this previously, this was one of our requirements initially.

Reviewed-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>

Regards,
Sai Pavan

> ---
>  hw/usb/Kconfig | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig index
> 4dd2ba9630cb..a674ce4c542e 100644
> --- a/hw/usb/Kconfig
> +++ b/hw/usb/Kconfig
> @@ -32,8 +32,6 @@ config USB_EHCI_SYSBUS
> 
>  config USB_XHCI
>      bool
> -    default y if PCI_DEVICES
> -    depends on PCI
>      select USB
> 
>  config USB_XHCI_PCI
> @@ -50,8 +48,8 @@ config USB_XHCI_NEC
> 
>  config USB_XHCI_SYSBUS
>      bool
> -    default y if USB_XHCI
> -    select USB
> +    default y
> +    select USB_XHCI
> 
>  config USB_MUSB
>      bool
> --
> 2.27.0

Re: [PATCH v3 04/10] usb/xhci: fixup xhci kconfig deps

[Bruce Rogers]

On Tue, 2020-10-20 at 09:48 +0200, Gerd Hoffmann wrote:
> USB_XHCI does not depend on PCI any more.
> USB_XHCI_SYSBUS must select USB_XHCI not USB.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  hw/usb/Kconfig | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
> index 4dd2ba9630cb..a674ce4c542e 100644
> --- a/hw/usb/Kconfig
> +++ b/hw/usb/Kconfig
> @@ -32,8 +32,6 @@ config USB_EHCI_SYSBUS
>  
>  config USB_XHCI
>      bool
> -    default y if PCI_DEVICES
> -    depends on PCI
>      select USB
>  
>  config USB_XHCI_PCI
> @@ -50,8 +48,8 @@ config USB_XHCI_NEC
>  
>  config USB_XHCI_SYSBUS
>      bool
> -    default y if USB_XHCI
> -    select USB
> +    default y
> +    select USB_XHCI
>  
>  config USB_MUSB
>      bool

I was reviewing what device changes are happening between v5.1.0 and
v5.2.0 for the QEMU arch's we support and noticed for s390x system
emulation that usb devices have appeared in the info qdm list in the
monitor.

I bisected the change to this patch, now commit 7114f6eac333d99b, which
does a 'default y' without any conditionals. I'm fairly sure that was
not the intent, though I do know what the proper conditionals should
be.

Can you take a look at it?

Thanks,

Bruce

Re: [PATCH v3 04/10] usb/xhci: fixup xhci kconfig deps

[Gerd Hoffmann]

> >  config USB_XHCI
> > -    default y if PCI_DEVICES

> >  config USB_XHCI_SYSBUS
> > +    default y

> I was reviewing what device changes are happening between v5.1.0 and
> v5.2.0 for the QEMU arch's we support and noticed for s390x system
> emulation that usb devices have appeared in the info qdm list in the
> monitor.
> 
> I bisected the change to this patch, now commit 7114f6eac333d99b, which
> does a 'default y' without any conditionals. I'm fairly sure that was
> not the intent, though I do know what the proper conditionals should
> be.

I'm open to suggestions.  Depending on PCI_DEVICES doesn't work any more
because USB_XHCI_SYSBUS doesn't need PCI ...

take care,
  Gerd

Re: [PATCH v3 04/10] usb/xhci: fixup xhci kconfig deps

[Paolo Bonzini]

On 20/11/20 01:27, Bruce Rogers wrote:
>>   config USB_XHCI_SYSBUS
>>       bool
>> -    default y if USB_XHCI
>> -    select USB
>> +    default y
>> +    select USB_XHCI
>>   
>>   config USB_MUSB
>>       bool
> I was reviewing what device changes are happening between v5.1.0 and
> v5.2.0 for the QEMU arch's we support and noticed for s390x system
> emulation that usb devices have appeared in the info qdm list in the
> monitor.
> 
> I bisected the change to this patch, now commit 7114f6eac333d99b, which
> does a 'default y' without any conditionals. I'm fairly sure that was
> not the intent, though I do know what the proper conditionals should
> be.
> 
> Can you take a look at it?

Generally, SYSBUS devices should never be "default y" because they're 
not user creatable.  Also kconfig.rst says

   Devices are usually ``default y`` if and only if they have at least
   one ``depends on``; the default could be conditional on a device
   group.

In this case, the right thing to do is to remove "default y" here; 
microvm already has a "select USB_XHCI_SYSBUS" so it would still work. 
That said, I would also change

     select PCI_EXPRESS_GENERIC_BRIDGE
     select USB_XHCI_SYSBUS

for microvm to "imply".  Again, according to kconfig.rst

   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.

Thanks,

Paolo