This patch sets the base to use xhci as sysbus model, for which pci
specific hooks are moved to hcd-xhci-pci.c. As a part of this requirment
msi/msix interrupts handling is moved under XHCIPCIState, and XHCIState
is  non qom object, make use of 'container_of' calls to retrive
XHCIPciState. Made required changes for qemu-xhci-nec.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/usb/Kconfig        |   6 ++
 hw/usb/Makefile.objs  |   1 +
 hw/usb/hcd-xhci-nec.c |  14 +--
 hw/usb/hcd-xhci-pci.c | 192 +++++++++++++++++++++++++++++++++++++++--
 hw/usb/hcd-xhci-pci.h |  47 ++++++++++
 hw/usb/hcd-xhci.c     | 235 ++++++++------------------------------------------
 hw/usb/hcd-xhci.h     |  23 +++--
 7 files changed, 293 insertions(+), 225 deletions(-)
 create mode 100644 hw/usb/hcd-xhci-pci.h

diff --git a/hw/usb/Kconfig b/hw/usb/Kconfig
index d4d8c37..d9965c1 100644
--- a/hw/usb/Kconfig
+++ b/hw/usb/Kconfig
@@ -36,6 +36,12 @@ config USB_XHCI
     depends on PCI
     select USB
 
+config USB_XHCI_PCI
+    bool
+    default y if PCI_DEVICES
+    depends on PCI
+    select USB_XHCI
+
 config USB_XHCI_NEC
     bool
     default y if PCI_DEVICES
diff --git a/hw/usb/Makefile.objs b/hw/usb/Makefile.objs
index fa5c3fa..98b1899 100644
--- a/hw/usb/Makefile.objs
+++ b/hw/usb/Makefile.objs
@@ -11,6 +11,7 @@ common-obj-$(CONFIG_USB_EHCI_PCI) += hcd-ehci-pci.o
 common-obj-$(CONFIG_USB_EHCI_SYSBUS) += hcd-ehci-sysbus.o
 common-obj-$(CONFIG_USB_XHCI) += hcd-xhci.o
 common-obj-$(CONFIG_USB_XHCI_NEC) += hcd-xhci-nec.o
+common-obj-$(CONFIG_USB_XHCI_PCI) += hcd-xhci-pci.o
 common-obj-$(CONFIG_USB_MUSB) += hcd-musb.o
 common-obj-$(CONFIG_USB_DWC2) += hcd-dwc2.o
 
diff --git a/hw/usb/hcd-xhci-nec.c b/hw/usb/hcd-xhci-nec.c
index e6a5a22..1756d97 100644
--- a/hw/usb/hcd-xhci-nec.c
+++ b/hw/usb/hcd-xhci-nec.c
@@ -25,17 +25,17 @@
 #include "hw/pci/pci.h"
 #include "hw/qdev-properties.h"
 
-#include "hcd-xhci.h"
+#include "hcd-xhci-pci.h"
 
 static Property nec_xhci_properties[] = {
-    DEFINE_PROP_ON_OFF_AUTO("msi", XHCIState, msi, ON_OFF_AUTO_AUTO),
-    DEFINE_PROP_ON_OFF_AUTO("msix", XHCIState, msix, ON_OFF_AUTO_AUTO),
+    DEFINE_PROP_ON_OFF_AUTO("msi", XHCIPciState, msi, ON_OFF_AUTO_AUTO),
+    DEFINE_PROP_ON_OFF_AUTO("msix", XHCIPciState, msix, ON_OFF_AUTO_AUTO),
     DEFINE_PROP_BIT("superspeed-ports-first",
                     XHCIState, flags, XHCI_FLAG_SS_FIRST, true),
-    DEFINE_PROP_BIT("force-pcie-endcap", XHCIState, flags,
+    DEFINE_PROP_BIT("force-pcie-endcap", XHCIPciState, xhci.flags,
                     XHCI_FLAG_FORCE_PCIE_ENDCAP, false),
-    DEFINE_PROP_UINT32("intrs", XHCIState, numintrs, MAXINTRS),
-    DEFINE_PROP_UINT32("slots", XHCIState, numslots, MAXSLOTS),
+    DEFINE_PROP_UINT32("intrs", XHCIPciState, xhci.numintrs, MAXINTRS),
+    DEFINE_PROP_UINT32("slots", XHCIPciState, xhci.numslots, MAXSLOTS),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -52,7 +52,7 @@ static void nec_xhci_class_init(ObjectClass *klass, void *data)
 
 static const TypeInfo nec_xhci_info = {
     .name          = TYPE_NEC_XHCI,
-    .parent        = TYPE_XHCI,
+    .parent        = TYPE_XHCI_PCI,
     .class_init    = nec_xhci_class_init,
 };
 
diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
index 26af683..005870b 100644
--- a/hw/usb/hcd-xhci-pci.c
+++ b/hw/usb/hcd-xhci-pci.c
@@ -23,12 +23,192 @@
 #include "qemu/osdep.h"
 #include "hw/pci/pci.h"
 #include "hw/qdev-properties.h"
+#include "migration/vmstate.h"
 #include "hw/pci/msi.h"
 #include "hw/pci/msix.h"
-#include "hcd-xhci.h"
+#include "hcd-xhci-pci.h"
 #include "trace.h"
 #include "qapi/error.h"
 
+#define OFF_MSIX_TABLE  0x3000
+#define OFF_MSIX_PBA    0x3800
+
+static void xhci_pci_intr_update(XHCIState *xhci, int n, bool enable)
+{
+    XHCIPciState *s = container_of(xhci, XHCIPciState, xhci);
+    PCIDevice *pci_dev = PCI_DEVICE(s);
+
+    if (!msix_enabled(pci_dev)) {
+        return;
+    }
+    if (enable == !!s->msix_used[n]) {
+        return;
+    }
+    if (enable) {
+        trace_usb_xhci_irq_msix_use(n);
+        msix_vector_use(pci_dev, n);
+        s->msix_used[n] = 1;
+    } else {
+        trace_usb_xhci_irq_msix_unuse(n);
+        msix_vector_unuse(pci_dev, n);
+        s->msix_used[n] = 0;
+    }
+}
+
+static void xhci_pci_intr_raise(XHCIState *xhci, int n, bool level)
+{
+    XHCIPciState *s = container_of(xhci, XHCIPciState, xhci);
+    PCIDevice *pci_dev = PCI_DEVICE(s);
+
+    if (n == 0 &&
+        !(msix_enabled(pci_dev) ||
+         msi_enabled(pci_dev))) {
+        pci_set_irq(pci_dev, level);
+    }
+    if (msix_enabled(pci_dev)) {
+        msix_notify(pci_dev, n);
+        return;
+    }
+
+    if (msi_enabled(pci_dev)) {
+        msi_notify(pci_dev, n);
+        return;
+    }
+}
+
+static void xhci_pci_reset(DeviceState *dev)
+{
+    XHCIPciState *s = XHCI_PCI(dev);
+
+    xhci_reset(&s->xhci);
+}
+
+static void usb_xhci_pci_realize(struct PCIDevice *dev, Error **errp)
+{
+    int ret;
+    Error *err = NULL;
+    XHCIPciState *s = XHCI_PCI(dev);
+
+    dev->config[PCI_CLASS_PROG] = 0x30;    /* xHCI */
+    dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin 1 */
+    dev->config[PCI_CACHE_LINE_SIZE] = 0x10;
+    dev->config[0x60] = 0x30; /* release number */
+
+    s->xhci.hostOpaque = DEVICE(s);
+    if (strcmp(object_get_typename(OBJECT(dev)), TYPE_NEC_XHCI) == 0) {
+        s->xhci.nec_quirks = true;
+    }
+    s->xhci.intr_update = xhci_pci_intr_update;
+    s->xhci.intr_raise = xhci_pci_intr_raise;
+    usb_xhci_realize(&s->xhci, DEVICE(dev), errp);
+
+    if (s->msi != ON_OFF_AUTO_OFF) {
+        ret = msi_init(dev, 0x70, s->xhci.numintrs, true, false, &err);
+        /*
+         * Any error other than -ENOTSUP(board's MSI support is broken)
+         * is a programming error
+         */
+        assert(!ret || ret == -ENOTSUP);
+        if (ret && s->msi == ON_OFF_AUTO_ON) {
+            /* Can't satisfy user's explicit msi=on request, fail */
+            error_append_hint(&err, "You have to use msi=auto (default) or "
+                    "msi=off with this machine type.\n");
+            error_propagate(errp, err);
+            return;
+        }
+        assert(!err || s->msi == ON_OFF_AUTO_AUTO);
+        /* With msi=auto, we fall back to MSI off silently */
+        error_free(err);
+    }
+    pci_register_bar(dev, 0,
+                     PCI_BASE_ADDRESS_SPACE_MEMORY |
+                     PCI_BASE_ADDRESS_MEM_TYPE_64,
+                     &s->xhci.mem);
+
+    if (pci_bus_is_express(pci_get_bus(dev)) ||
+        xhci_get_flag(&s->xhci, XHCI_FLAG_FORCE_PCIE_ENDCAP)) {
+        ret = pcie_endpoint_cap_init(dev, 0xa0);
+        assert(ret > 0);
+    }
+
+    if (s->msix != ON_OFF_AUTO_OFF) {
+        /* TODO check for errors, and should fail when msix=on */
+        msix_init(dev, s->xhci.numintrs,
+                  &s->xhci.mem, 0, OFF_MSIX_TABLE,
+                  &s->xhci.mem, 0, OFF_MSIX_PBA,
+                  0x90, NULL);
+    }
+    s->xhci.as = pci_get_address_space(dev);
+}
+
+static void usb_xhci_pci_exit(PCIDevice *dev)
+{
+    XHCIPciState *s = XHCI_PCI(dev);
+    /* destroy msix memory region */
+    if (dev->msix_table && dev->msix_pba
+        && dev->msix_entry_used) {
+        msix_uninit(dev, &s->xhci.mem, &s->xhci.mem);
+    }
+    usb_xhci_unrealize(&s->xhci, DEVICE(dev), NULL);
+}
+
+static Property xhci_pci_properties[] = {
+    DEFINE_PROP_BIT("streams", XHCIPciState, xhci.flags,
+                    XHCI_FLAG_ENABLE_STREAMS, true),
+    DEFINE_PROP_UINT32("p2", XHCIPciState, xhci.numports_2, 4),
+    DEFINE_PROP_UINT32("p3", XHCIPciState, xhci.numports_3, 4),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static const VMStateDescription vmstate_xhci_pci = {
+    .name = "xhci-pci",
+    .version_id = 1,
+    .fields = (VMStateField[]) {
+        VMSTATE_PCI_DEVICE(parent_obj, XHCIPciState),
+        VMSTATE_MSIX(parent_obj, XHCIPciState),
+        VMSTATE_UINT8_ARRAY(msix_used, XHCIPciState, MAXINTRS),
+        VMSTATE_STRUCT(xhci, XHCIPciState, 2, vmstate_xhci, XHCIState),
+        VMSTATE_END_OF_LIST()
+    }
+};
+
+static void xhci_instance_init(Object *obj)
+{
+    /*
+     * QEMU_PCI_CAP_EXPRESS initialization does not depend on QEMU command
+     * line, therefore, no need to wait to realize like other devices
+     */
+    PCI_DEVICE(obj)->cap_present |= QEMU_PCI_CAP_EXPRESS;
+}
+
+static void xhci_class_init(ObjectClass *klass, void *data)
+{
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    device_class_set_props(dc, xhci_pci_properties);
+    dc->reset   = xhci_pci_reset;
+    dc->vmsd    = &vmstate_xhci_pci;
+    set_bit(DEVICE_CATEGORY_USB, dc->categories);
+    k->realize      = usb_xhci_pci_realize;
+    k->exit         = usb_xhci_pci_exit;
+    k->class_id     = PCI_CLASS_SERIAL_USB;
+}
+
+static const TypeInfo xhci_pci_info = {
+    .name          = TYPE_XHCI_PCI,
+    .parent        = TYPE_PCI_DEVICE,
+    .instance_size = sizeof(XHCIPciState),
+    .class_init    = xhci_class_init,
+    .instance_init = xhci_instance_init,
+    .abstract      = true,
+    .interfaces = (InterfaceInfo[]) {
+        { INTERFACE_PCIE_DEVICE },
+        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
+        { }
+    },
+};
+
 static void qemu_xhci_class_init(ObjectClass *klass, void *data)
 {
     PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
@@ -40,10 +220,11 @@ static void qemu_xhci_class_init(ObjectClass *klass, void *data)
 
 static void qemu_xhci_instance_init(Object *obj)
 {
-    XHCIState *xhci = XHCI(obj);
+    XHCIPciState *s = XHCI_PCI(obj);
+    XHCIState *xhci = &s->xhci;
 
-    xhci->msi      = ON_OFF_AUTO_OFF;
-    xhci->msix     = ON_OFF_AUTO_AUTO;
+    s->msi      = ON_OFF_AUTO_OFF;
+    s->msix     = ON_OFF_AUTO_AUTO;
     xhci->numintrs = MAXINTRS;
     xhci->numslots = MAXSLOTS;
     xhci_set_flag(xhci, XHCI_FLAG_SS_FIRST);
@@ -51,13 +232,14 @@ static void qemu_xhci_instance_init(Object *obj)
 
 static const TypeInfo qemu_xhci_info = {
     .name          = TYPE_QEMU_XHCI,
-    .parent        = TYPE_XHCI,
+    .parent        = TYPE_XHCI_PCI,
     .class_init    = qemu_xhci_class_init,
     .instance_init = qemu_xhci_instance_init,
 };
 
 static void xhci_register_types(void)
 {
+    type_register_static(&xhci_pci_info);
     type_register_static(&qemu_xhci_info);
 }
 
diff --git a/hw/usb/hcd-xhci-pci.h b/hw/usb/hcd-xhci-pci.h
new file mode 100644
index 0000000..6e01054
--- /dev/null
+++ b/hw/usb/hcd-xhci-pci.h
@@ -0,0 +1,47 @@
+/*
+ * USB xHCI controller emulation
+ *
+ * Copyright (c) 2011 Securiforest
+ * Date: 2011-05-11 ;  Author: Hector Martin <hector@marcansoft.com>
+ * Based on usb-ohci.c, emulates Renesas NEC USB 3.0
+ * Date: 2020-01-1; Author: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
+ * PCI hooks are moved from XHCIState to XHCIPciState
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef HW_USB_HCD_XHCI_PCI_H
+#define HW_USB_HCD_XHCI_PCI_H
+
+#include "hw/usb.h"
+#include "hcd-xhci.h"
+
+#define TYPE_XHCI_PCI "pci-xhci"
+#define XHCI_PCI(obj) \
+    OBJECT_CHECK(XHCIPciState, (obj), TYPE_XHCI_PCI)
+
+
+typedef struct XHCIPciState {
+    /*< private >*/
+    PCIDevice parent_obj;
+    /*< public >*/
+    XHCIState xhci;
+    OnOffAuto msi;
+    OnOffAuto msix;
+    uint8_t msix_used[MAXINTRS];
+    bool enable_streams;
+    uint32_t numports_2, numports_3;
+} XHCIPciState;
+
+#endif
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index d340518..2e460d5 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -25,10 +25,7 @@
 #include "qemu/queue.h"
 #include "hw/usb.h"
 #include "migration/vmstate.h"
-#include "hw/pci/pci.h"
 #include "hw/qdev-properties.h"
-#include "hw/pci/msi.h"
-#include "hw/pci/msix.h"
 #include "trace.h"
 #include "qapi/error.h"
 
@@ -57,8 +54,6 @@
 #define OFF_OPER        LEN_CAP
 #define OFF_RUNTIME     0x1000
 #define OFF_DOORBELL    0x2000
-#define OFF_MSIX_TABLE  0x3000
-#define OFF_MSIX_PBA    0x3800
 /* must be power of 2 */
 #define LEN_REGS        0x4000
 
@@ -548,54 +543,28 @@ static XHCIPort *xhci_lookup_port(XHCIState *xhci, struct USBPort *uport)
     return &xhci->ports[index];
 }
 
-static void xhci_intx_update(XHCIState *xhci)
+static void xhci_intr_update(XHCIState *xhci, int v)
 {
-    PCIDevice *pci_dev = PCI_DEVICE(xhci);
     int level = 0;
 
-    if (msix_enabled(pci_dev) ||
-        msi_enabled(pci_dev)) {
-        return;
-    }
-
-    if (xhci->intr[0].iman & IMAN_IP &&
-        xhci->intr[0].iman & IMAN_IE &&
-        xhci->usbcmd & USBCMD_INTE) {
-        level = 1;
-    }
-
-    trace_usb_xhci_irq_intx(level);
-    pci_set_irq(pci_dev, level);
-}
-
-static void xhci_msix_update(XHCIState *xhci, int v)
-{
-    PCIDevice *pci_dev = PCI_DEVICE(xhci);
-    bool enabled;
-
-    if (!msix_enabled(pci_dev)) {
-        return;
-    }
-
-    enabled = xhci->intr[v].iman & IMAN_IE;
-    if (enabled == xhci->intr[v].msix_used) {
-        return;
+    if (v == 0) {
+        if (xhci->intr[0].iman & IMAN_IP &&
+            xhci->intr[0].iman & IMAN_IE &&
+            xhci->usbcmd & USBCMD_INTE) {
+            level = 1;
+        }
+        if (xhci->intr_raise) {
+            xhci->intr_raise(xhci, 0, level);
+        }
     }
-
-    if (enabled) {
-        trace_usb_xhci_irq_msix_use(v);
-        msix_vector_use(pci_dev, v);
-        xhci->intr[v].msix_used = true;
-    } else {
-        trace_usb_xhci_irq_msix_unuse(v);
-        msix_vector_unuse(pci_dev, v);
-        xhci->intr[v].msix_used = false;
+    if (xhci->intr_update) {
+        xhci->intr_update(xhci, v,
+                     xhci->intr[v].iman & IMAN_IE);
     }
 }
 
 static void xhci_intr_raise(XHCIState *xhci, int v)
 {
-    PCIDevice *pci_dev = PCI_DEVICE(xhci);
     bool pending = (xhci->intr[v].erdp_low & ERDP_EHB);
 
     xhci->intr[v].erdp_low |= ERDP_EHB;
@@ -612,22 +581,8 @@ static void xhci_intr_raise(XHCIState *xhci, int v)
     if (!(xhci->usbcmd & USBCMD_INTE)) {
         return;
     }
-
-    if (msix_enabled(pci_dev)) {
-        trace_usb_xhci_irq_msix(v);
-        msix_notify(pci_dev, v);
-        return;
-    }
-
-    if (msi_enabled(pci_dev)) {
-        trace_usb_xhci_irq_msi(v);
-        msi_notify(pci_dev, v);
-        return;
-    }
-
-    if (v == 0) {
-        trace_usb_xhci_irq_intx(1);
-        pci_irq_assert(pci_dev);
+    if (xhci->intr_raise) {
+        xhci->intr_raise(xhci, v, true);
     }
 }
 
@@ -1437,7 +1392,7 @@ static int xhci_xfer_create_sgl(XHCITransfer *xfer, int in_xfer)
     int i;
 
     xfer->int_req = false;
-    qemu_sglist_init(&xfer->sgl, DEVICE(xhci), xfer->trb_count, xhci->as);
+    qemu_sglist_init(&xfer->sgl, xhci->hostOpaque, xfer->trb_count, xhci->as);
     for (i = 0; i < xfer->trb_count; i++) {
         XHCITRB *trb = &xfer->trbs[i];
         dma_addr_t addr;
@@ -2680,9 +2635,8 @@ static void xhci_port_reset(XHCIPort *port, bool warm_reset)
     xhci_port_notify(port, PORTSC_PRC);
 }
 
-static void xhci_reset(DeviceState *dev)
+void xhci_reset(XHCIState *xhci)
 {
-    XHCIState *xhci = XHCI(dev);
     int i;
 
     trace_usb_xhci_reset();
@@ -2715,7 +2669,6 @@ static void xhci_reset(DeviceState *dev)
         xhci->intr[i].erstba_high = 0;
         xhci->intr[i].erdp_low = 0;
         xhci->intr[i].erdp_high = 0;
-        xhci->intr[i].msix_used = 0;
 
         xhci->intr[i].er_ep_idx = 0;
         xhci->intr[i].er_pcs = 1;
@@ -2938,7 +2891,6 @@ static void xhci_oper_write(void *ptr, hwaddr reg,
                             uint64_t val, unsigned size)
 {
     XHCIState *xhci = ptr;
-    DeviceState *d = DEVICE(ptr);
 
     trace_usb_xhci_oper_write(reg, val);
 
@@ -2960,15 +2912,15 @@ static void xhci_oper_write(void *ptr, hwaddr reg,
         xhci->usbcmd = val & 0xc0f;
         xhci_mfwrap_update(xhci);
         if (val & USBCMD_HCRST) {
-            xhci_reset(d);
+            xhci_reset(xhci);
         }
-        xhci_intx_update(xhci);
+        xhci_intr_update(xhci, 0);
         break;
 
     case 0x04: /* USBSTS */
         /* these bits are write-1-to-clear */
         xhci->usbsts &= ~(val & (USBSTS_HSE|USBSTS_EINT|USBSTS_PCD|USBSTS_SRE));
-        xhci_intx_update(xhci);
+        xhci_intr_update(xhci, 0);
         break;
 
     case 0x14: /* DNCTRL */
@@ -3071,10 +3023,7 @@ static void xhci_runtime_write(void *ptr, hwaddr reg,
         }
         intr->iman &= ~IMAN_IE;
         intr->iman |= val & IMAN_IE;
-        if (v == 0) {
-            xhci_intx_update(xhci);
-        }
-        xhci_msix_update(xhci, v);
+        xhci_intr_update(xhci, v);
         break;
     case 0x04: /* IMOD */
         intr->imod = val;
@@ -3319,7 +3268,6 @@ static USBBusOps xhci_bus_ops = {
 
 static void usb_xhci_init(XHCIState *xhci)
 {
-    DeviceState *dev = DEVICE(xhci);
     XHCIPort *port;
     unsigned int i, usbports, speedmask;
 
@@ -3334,7 +3282,7 @@ static void usb_xhci_init(XHCIState *xhci)
     usbports = MAX(xhci->numports_2, xhci->numports_3);
     xhci->numports = xhci->numports_2 + xhci->numports_3;
 
-    usb_bus_new(&xhci->bus, sizeof(xhci->bus), &xhci_bus_ops, dev);
+    usb_bus_new(&xhci->bus, sizeof(xhci->bus), &xhci_bus_ops, xhci->hostOpaque);
 
     for (i = 0; i < usbports; i++) {
         speedmask = 0;
@@ -3374,21 +3322,10 @@ static void usb_xhci_init(XHCIState *xhci)
     }
 }
 
-static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
+void usb_xhci_realize(XHCIState *xhci, DeviceState *dev, Error **errp)
 {
-    int i, ret;
-    Error *err = NULL;
-
-    XHCIState *xhci = XHCI(dev);
-
-    dev->config[PCI_CLASS_PROG] = 0x30;    /* xHCI */
-    dev->config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin 1 */
-    dev->config[PCI_CACHE_LINE_SIZE] = 0x10;
-    dev->config[0x60] = 0x30; /* release number */
+    int i;
 
-    if (strcmp(object_get_typename(OBJECT(dev)), TYPE_NEC_XHCI) == 0) {
-        xhci->nec_quirks = true;
-    }
     if (xhci->numintrs > MAXINTRS) {
         xhci->numintrs = MAXINTRS;
     }
@@ -3410,36 +3347,18 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
         xhci->max_pstreams_mask = 0;
     }
 
-    if (xhci->msi != ON_OFF_AUTO_OFF) {
-        ret = msi_init(dev, 0x70, xhci->numintrs, true, false, &err);
-        /* Any error other than -ENOTSUP(board's MSI support is broken)
-         * is a programming error */
-        assert(!ret || ret == -ENOTSUP);
-        if (ret && xhci->msi == ON_OFF_AUTO_ON) {
-            /* Can't satisfy user's explicit msi=on request, fail */
-            error_append_hint(&err, "You have to use msi=auto (default) or "
-                    "msi=off with this machine type.\n");
-            error_propagate(errp, err);
-            return;
-        }
-        assert(!err || xhci->msi == ON_OFF_AUTO_AUTO);
-        /* With msi=auto, we fall back to MSI off silently */
-        error_free(err);
-    }
-
     usb_xhci_init(xhci);
-    xhci->as = pci_get_address_space(dev);
     xhci->mfwrap_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, xhci_mfwrap_timer, xhci);
 
-    memory_region_init(&xhci->mem, OBJECT(xhci), "xhci", LEN_REGS);
-    memory_region_init_io(&xhci->mem_cap, OBJECT(xhci), &xhci_cap_ops, xhci,
+    memory_region_init(&xhci->mem, OBJECT(dev), "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(xhci), &xhci_oper_ops, xhci,
+    memory_region_init_io(&xhci->mem_oper, OBJECT(dev), &xhci_oper_ops, xhci,
                           "operational", 0x400);
-    memory_region_init_io(&xhci->mem_runtime, OBJECT(xhci), &xhci_runtime_ops, xhci,
-                          "runtime", LEN_RUNTIME);
-    memory_region_init_io(&xhci->mem_doorbell, OBJECT(xhci), &xhci_doorbell_ops, xhci,
-                          "doorbell", LEN_DOORBELL);
+    memory_region_init_io(&xhci->mem_runtime, OBJECT(dev), &xhci_runtime_ops,
+                          xhci, "runtime", LEN_RUNTIME);
+    memory_region_init_io(&xhci->mem_doorbell, OBJECT(dev), &xhci_doorbell_ops,
+                          xhci, "doorbell", LEN_DOORBELL);
 
     memory_region_add_subregion(&xhci->mem, 0,            &xhci->mem_cap);
     memory_region_add_subregion(&xhci->mem, OFF_OPER,     &xhci->mem_oper);
@@ -3450,34 +3369,15 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
         XHCIPort *port = &xhci->ports[i];
         uint32_t offset = OFF_OPER + 0x400 + 0x10 * i;
         port->xhci = xhci;
-        memory_region_init_io(&port->mem, OBJECT(xhci), &xhci_port_ops, port,
+        memory_region_init_io(&port->mem, OBJECT(dev), &xhci_port_ops, port,
                               port->name, 0x10);
         memory_region_add_subregion(&xhci->mem, offset, &port->mem);
     }
-
-    pci_register_bar(dev, 0,
-                     PCI_BASE_ADDRESS_SPACE_MEMORY|PCI_BASE_ADDRESS_MEM_TYPE_64,
-                     &xhci->mem);
-
-    if (pci_bus_is_express(pci_get_bus(dev)) ||
-        xhci_get_flag(xhci, XHCI_FLAG_FORCE_PCIE_ENDCAP)) {
-        ret = pcie_endpoint_cap_init(dev, 0xa0);
-        assert(ret > 0);
-    }
-
-    if (xhci->msix != ON_OFF_AUTO_OFF) {
-        /* TODO check for errors, and should fail when msix=on */
-        msix_init(dev, xhci->numintrs,
-                  &xhci->mem, 0, OFF_MSIX_TABLE,
-                  &xhci->mem, 0, OFF_MSIX_PBA,
-                  0x90, NULL);
-    }
 }
 
-static void usb_xhci_exit(PCIDevice *dev)
+void usb_xhci_unrealize(XHCIState *xhci, DeviceState *dev, Error **errp)
 {
     int i;
-    XHCIState *xhci = XHCI(dev);
 
     trace_usb_xhci_exit();
 
@@ -3501,19 +3401,12 @@ static void usb_xhci_exit(PCIDevice *dev)
         memory_region_del_subregion(&xhci->mem, &port->mem);
     }
 
-    /* destroy msix memory region */
-    if (dev->msix_table && dev->msix_pba
-        && dev->msix_entry_used) {
-        msix_uninit(dev, &xhci->mem, &xhci->mem);
-    }
-
     usb_bus_release(&xhci->bus);
 }
 
 static int usb_xhci_post_load(void *opaque, int version_id)
 {
     XHCIState *xhci = opaque;
-    PCIDevice *pci_dev = PCI_DEVICE(xhci);
     XHCISlot *slot;
     XHCIEPContext *epctx;
     dma_addr_t dcbaap, pctx;
@@ -3559,11 +3452,7 @@ static int usb_xhci_post_load(void *opaque, int version_id)
     }
 
     for (intr = 0; intr < xhci->numintrs; intr++) {
-        if (xhci->intr[intr].msix_used) {
-            msix_vector_use(pci_dev, intr);
-        } else {
-            msix_vector_unuse(pci_dev, intr);
-        }
+        xhci_intr_update(xhci, intr);
     }
 
     return 0;
@@ -3632,7 +3521,6 @@ static const VMStateDescription vmstate_xhci_intr = {
         VMSTATE_UINT32(erdp_high,     XHCIInterrupter),
 
         /* state */
-        VMSTATE_BOOL(msix_used,       XHCIInterrupter),
         VMSTATE_BOOL(er_pcs,          XHCIInterrupter),
         VMSTATE_UINT64(er_start,      XHCIInterrupter),
         VMSTATE_UINT32(er_size,       XHCIInterrupter),
@@ -3650,14 +3538,11 @@ static const VMStateDescription vmstate_xhci_intr = {
     }
 };
 
-static const VMStateDescription vmstate_xhci = {
+const VMStateDescription vmstate_xhci = {
     .name = "xhci",
     .version_id = 1,
     .post_load = usb_xhci_post_load,
     .fields = (VMStateField[]) {
-        VMSTATE_PCI_DEVICE(parent_obj, XHCIState),
-        VMSTATE_MSIX(parent_obj, XHCIState),
-
         VMSTATE_STRUCT_VARRAY_UINT32(ports, XHCIState, numports, 1,
                                      vmstate_xhci_port, XHCIPort),
         VMSTATE_STRUCT_VARRAY_UINT32(slots, XHCIState, numslots, 1,
@@ -3683,53 +3568,3 @@ static const VMStateDescription vmstate_xhci = {
         VMSTATE_END_OF_LIST()
     }
 };
-
-static Property xhci_properties[] = {
-    DEFINE_PROP_BIT("streams", XHCIState, flags,
-                    XHCI_FLAG_ENABLE_STREAMS, true),
-    DEFINE_PROP_UINT32("p2",    XHCIState, numports_2, 4),
-    DEFINE_PROP_UINT32("p3",    XHCIState, numports_3, 4),
-    DEFINE_PROP_END_OF_LIST(),
-};
-
-static void xhci_instance_init(Object *obj)
-{
-    /* QEMU_PCI_CAP_EXPRESS initialization does not depend on QEMU command
-     * line, therefore, no need to wait to realize like other devices */
-    PCI_DEVICE(obj)->cap_present |= QEMU_PCI_CAP_EXPRESS;
-}
-
-static void xhci_class_init(ObjectClass *klass, void *data)
-{
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-    DeviceClass *dc = DEVICE_CLASS(klass);
-
-    dc->vmsd    = &vmstate_xhci;
-    device_class_set_props(dc, xhci_properties);
-    dc->reset   = xhci_reset;
-    set_bit(DEVICE_CATEGORY_USB, dc->categories);
-    k->realize      = usb_xhci_realize;
-    k->exit         = usb_xhci_exit;
-    k->class_id     = PCI_CLASS_SERIAL_USB;
-}
-
-static const TypeInfo xhci_info = {
-    .name          = TYPE_XHCI,
-    .parent        = TYPE_PCI_DEVICE,
-    .instance_size = sizeof(XHCIState),
-    .class_init    = xhci_class_init,
-    .instance_init = xhci_instance_init,
-    .abstract      = true,
-    .interfaces = (InterfaceInfo[]) {
-        { INTERFACE_PCIE_DEVICE },
-        { INTERFACE_CONVENTIONAL_PCI_DEVICE },
-        { }
-    },
-};
-
-static void xhci_register_types(void)
-{
-    type_register_static(&xhci_info);
-}
-
-type_init(xhci_register_types)
diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
index ca03481..33ed327 100644
--- a/hw/usb/hcd-xhci.h
+++ b/hw/usb/hcd-xhci.h
@@ -24,13 +24,9 @@
 
 #include "sysemu/dma.h"
 
-#define TYPE_XHCI "base-xhci"
 #define TYPE_NEC_XHCI "nec-usb-xhci"
 #define TYPE_QEMU_XHCI "qemu-xhci"
 
-#define XHCI(obj) \
-    OBJECT_CHECK(XHCIState, (obj), TYPE_XHCI)
-
 #define MAXPORTS_2 15
 #define MAXPORTS_3 15
 
@@ -170,7 +166,7 @@ typedef struct XHCIInterrupter {
     uint32_t erdp_low;
     uint32_t erdp_high;
 
-    bool msix_used, er_pcs;
+    bool er_pcs;
 
     dma_addr_t er_start;
     uint32_t er_size;
@@ -184,11 +180,7 @@ typedef struct XHCIInterrupter {
 
 } XHCIInterrupter;
 
-struct XHCIState {
-    /*< private >*/
-    PCIDevice parent_obj;
-    /*< public >*/
-
+typedef struct XHCIState {
     USBBus bus;
     MemoryRegion mem;
     AddressSpace *as;
@@ -204,8 +196,9 @@ struct XHCIState {
     uint32_t numslots;
     uint32_t flags;
     uint32_t max_pstreams_mask;
-    OnOffAuto msi;
-    OnOffAuto msix;
+    void (*intr_update)(XHCIState *s, int n, bool enable);
+    void (*intr_raise)(XHCIState *s, int n, bool level);
+    DeviceState *hostOpaque;
 
     /* Operational Registers */
     uint32_t usbcmd;
@@ -230,8 +223,12 @@ struct XHCIState {
     XHCIRing cmd_ring;
 
     bool nec_quirks;
-};
+} XHCIState;
 
+extern const VMStateDescription vmstate_xhci;
+void xhci_reset(XHCIState *xhci);
+void usb_xhci_realize(XHCIState *xhci, DeviceState *dev, Error **errp);
+void usb_xhci_unrealize(XHCIState *xhci, DeviceState *dev, Error **errp);
 bool xhci_get_flag(XHCIState *xhci, enum xhci_flags bit);
 void xhci_set_flag(XHCIState *xhci, enum xhci_flags bit);
 #endif
-- 
2.7.4

Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:

> This patch sets the base to use xhci as sysbus model, for which pci
> specific hooks are moved to hcd-xhci-pci.c. As a part of this requirment
> msi/msix interrupts handling is moved under XHCIPCIState, and XHCIState
> is  non qom object, make use of 'container_of' calls to retrive
> XHCIPciState. Made required changes for qemu-xhci-nec.
>
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>

I can't see a "sysbus model".  What I can see is

         TYPE_DEVICE
              |
       TYPE_PCI_DEVICE
              |
        TYPE_XHCI_PCI (renamed from TYPE_XHCI)
          /       \
TYPE_QEMU_XHCI TYPE_NEC_XHCI

All but the two leaves are abstract.

Do you intend to add a "sysbus model" in a future patch?

Hi Markus,

> -----Original Message-----
> From: Markus Armbruster <armbru@redhat.com>
> Sent: Thursday, June 25, 2020 1:42 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>; Peter Maydell
> <peter.maydell@linaro.org>; Thomas Huth <thuth@redhat.com>; Eduardo
> Habkost <ehabkost@redhat.com>; qemu-devel@nongnu.org; Alistair Francis
> <alistair.francis@wdc.com>; 'Marc-André Lureau'
> <marcandre.lureau@redhat.com>; Ying Fang <fangying1@huawei.com>;
> Paolo Bonzini <pbonzini@redhat.com>; 'Philippe Mathieu-Daudé'
> <philmd@redhat.com>
> Subject: Re: [PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci base
> model
> 
> Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:
> 
> > This patch sets the base to use xhci as sysbus model, for which pci
> > specific hooks are moved to hcd-xhci-pci.c. As a part of this
> > requirment msi/msix interrupts handling is moved under XHCIPCIState,
> > and XHCIState is  non qom object, make use of 'container_of' calls to
> > retrive XHCIPciState. Made required changes for qemu-xhci-nec.
> >
> > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> 
> I can't see a "sysbus model".  What I can see is
> 
>          TYPE_DEVICE
>               |
>        TYPE_PCI_DEVICE
>               |
>         TYPE_XHCI_PCI (renamed from TYPE_XHCI)
>           /       \
> TYPE_QEMU_XHCI TYPE_NEC_XHCI
> 
> All but the two leaves are abstract.
> 
> Do you intend to add a "sysbus model" in a future patch?
[Sai Pavan Boddu]  Yes. I would be sending it along with that a device which would be using it. (i.e for zynqmp soc )
Let me know, if its good to include hcd-xhci-sysbus.c here ?

Regards,
Sai Pavan

Sai Pavan Boddu <saipava@xilinx.com> writes:

> Hi Markus,
>
>> -----Original Message-----
>> From: Markus Armbruster <armbru@redhat.com>
>> Sent: Thursday, June 25, 2020 1:42 PM
>> To: Sai Pavan Boddu <saipava@xilinx.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>; Peter Maydell
>> <peter.maydell@linaro.org>; Thomas Huth <thuth@redhat.com>; Eduardo
>> Habkost <ehabkost@redhat.com>; qemu-devel@nongnu.org; Alistair Francis
>> <alistair.francis@wdc.com>; 'Marc-André Lureau'
>> <marcandre.lureau@redhat.com>; Ying Fang <fangying1@huawei.com>;
>> Paolo Bonzini <pbonzini@redhat.com>; 'Philippe Mathieu-Daudé'
>> <philmd@redhat.com>
>> Subject: Re: [PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci base
>> model
>> 
>> Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:
>> 
>> > This patch sets the base to use xhci as sysbus model, for which pci
>> > specific hooks are moved to hcd-xhci-pci.c. As a part of this
>> > requirment msi/msix interrupts handling is moved under XHCIPCIState,
>> > and XHCIState is  non qom object, make use of 'container_of' calls to
>> > retrive XHCIPciState. Made required changes for qemu-xhci-nec.
>> >
>> > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>> 
>> I can't see a "sysbus model".  What I can see is
>> 
>>          TYPE_DEVICE
>>               |
>>        TYPE_PCI_DEVICE
>>               |
>>         TYPE_XHCI_PCI (renamed from TYPE_XHCI)
>>           /       \
>> TYPE_QEMU_XHCI TYPE_NEC_XHCI
>> 
>> All but the two leaves are abstract.
>> 
>> Do you intend to add a "sysbus model" in a future patch?
> [Sai Pavan Boddu]  Yes. I would be sending it along with that a device which would be using it. (i.e for zynqmp soc )
> Let me know, if its good to include hcd-xhci-sysbus.c here ?

I'm not sure this series is worthwhile this future patch.  Up to the
maintainer.

Here's a clean way to provide different bus connectors (say PCI and
sysbus) for the same core device:

Make the core device a TYPE_DEVICE.

For each desired bus, have a bus-specific device that contains a core
device.  Use object_initialize_child() for the component.

Example: core device TYPE_SERIAL, PCI device TYPE_PCI_SERIAL, ISA device
TYPE_ISA_SERIAL, sysbus devices TYPE_SERIAL_IO. TYPE_SERIAL_MM.

HI Markus,

> -----Original Message-----
> From: Markus Armbruster <armbru@redhat.com>
> Sent: Friday, June 26, 2020 11:42 AM
> To: Sai Pavan Boddu <saipava@xilinx.com>
> Cc: Peter Maydell <peter.maydell@linaro.org>; Thomas Huth
> <thuth@redhat.com>; Eduardo Habkost <ehabkost@redhat.com>; qemu-
> devel@nongnu.org; Alistair Francis <alistair.francis@wdc.com>; Gerd
> Hoffmann <kraxel@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>;
> Ying Fang <fangying1@huawei.com>; 'Marc-André Lureau'
> <marcandre.lureau@redhat.com>; 'Philippe Mathieu-Daudé'
> <philmd@redhat.com>
> Subject: Re: [PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci base
> model
> 
> Sai Pavan Boddu <saipava@xilinx.com> writes:
> 
> > Hi Markus,
> >
> >> -----Original Message-----
> >> From: Markus Armbruster <armbru@redhat.com>
> >> Sent: Thursday, June 25, 2020 1:42 PM
> >> To: Sai Pavan Boddu <saipava@xilinx.com>
> >> Cc: Gerd Hoffmann <kraxel@redhat.com>; Peter Maydell
> >> <peter.maydell@linaro.org>; Thomas Huth <thuth@redhat.com>;
> Eduardo
> >> Habkost <ehabkost@redhat.com>; qemu-devel@nongnu.org; Alistair
> >> Francis <alistair.francis@wdc.com>; 'Marc-André Lureau'
> >> <marcandre.lureau@redhat.com>; Ying Fang <fangying1@huawei.com>;
> >> Paolo Bonzini <pbonzini@redhat.com>; 'Philippe Mathieu-Daudé'
> >> <philmd@redhat.com>
> >> Subject: Re: [PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci
> >> base model
> >>
> >> Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:
> >>
> >> > This patch sets the base to use xhci as sysbus model, for which pci
> >> > specific hooks are moved to hcd-xhci-pci.c. As a part of this
> >> > requirment msi/msix interrupts handling is moved under
> >> > XHCIPCIState, and XHCIState is  non qom object, make use of
> >> > 'container_of' calls to retrive XHCIPciState. Made required changes for
> qemu-xhci-nec.
> >> >
> >> > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> >>
> >> I can't see a "sysbus model".  What I can see is
> >>
> >>          TYPE_DEVICE
> >>               |
> >>        TYPE_PCI_DEVICE
> >>               |
> >>         TYPE_XHCI_PCI (renamed from TYPE_XHCI)
> >>           /       \
> >> TYPE_QEMU_XHCI TYPE_NEC_XHCI
> >>
> >> All but the two leaves are abstract.
> >>
> >> Do you intend to add a "sysbus model" in a future patch?
> > [Sai Pavan Boddu]  Yes. I would be sending it along with that a device
> > which would be using it. (i.e for zynqmp soc ) Let me know, if its good to
> include hcd-xhci-sysbus.c here ?
> 
> I'm not sure this series is worthwhile this future patch.  Up to the maintainer.
> 
> Here's a clean way to provide different bus connectors (say PCI and
> sysbus) for the same core device:
> 
> Make the core device a TYPE_DEVICE.
> 
> For each desired bus, have a bus-specific device that contains a core device.
> Use object_initialize_child() for the component.
[Sai Pavan Boddu] This was my V1 implementation.
Changed it to non-qom structure after some feedback from @Gred. Felt like XHCIState will not be used standalone.

Thanks,
sai Pavan
> 
> Example: core device TYPE_SERIAL, PCI device TYPE_PCI_SERIAL, ISA device
> TYPE_ISA_SERIAL, sysbus devices TYPE_SERIAL_IO. TYPE_SERIAL_MM.

Sai Pavan Boddu <saipava@xilinx.com> writes:

> HI Markus,
>
>> -----Original Message-----
>> From: Markus Armbruster <armbru@redhat.com>
>> Sent: Friday, June 26, 2020 11:42 AM
>> To: Sai Pavan Boddu <saipava@xilinx.com>
>> Cc: Peter Maydell <peter.maydell@linaro.org>; Thomas Huth
>> <thuth@redhat.com>; Eduardo Habkost <ehabkost@redhat.com>; qemu-
>> devel@nongnu.org; Alistair Francis <alistair.francis@wdc.com>; Gerd
>> Hoffmann <kraxel@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>;
>> Ying Fang <fangying1@huawei.com>; 'Marc-André Lureau'
>> <marcandre.lureau@redhat.com>; 'Philippe Mathieu-Daudé'
>> <philmd@redhat.com>
>> Subject: Re: [PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci base
>> model
>> 
>> Sai Pavan Boddu <saipava@xilinx.com> writes:
>> 
>> > Hi Markus,
>> >
>> >> -----Original Message-----
>> >> From: Markus Armbruster <armbru@redhat.com>
>> >> Sent: Thursday, June 25, 2020 1:42 PM
>> >> To: Sai Pavan Boddu <saipava@xilinx.com>
>> >> Cc: Gerd Hoffmann <kraxel@redhat.com>; Peter Maydell
>> >> <peter.maydell@linaro.org>; Thomas Huth <thuth@redhat.com>;
>> Eduardo
>> >> Habkost <ehabkost@redhat.com>; qemu-devel@nongnu.org; Alistair
>> >> Francis <alistair.francis@wdc.com>; 'Marc-André Lureau'
>> >> <marcandre.lureau@redhat.com>; Ying Fang <fangying1@huawei.com>;
>> >> Paolo Bonzini <pbonzini@redhat.com>; 'Philippe Mathieu-Daudé'
>> >> <philmd@redhat.com>
>> >> Subject: Re: [PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci
>> >> base model
>> >>
>> >> Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:
>> >>
>> >> > This patch sets the base to use xhci as sysbus model, for which pci
>> >> > specific hooks are moved to hcd-xhci-pci.c. As a part of this
>> >> > requirment msi/msix interrupts handling is moved under
>> >> > XHCIPCIState, and XHCIState is  non qom object, make use of
>> >> > 'container_of' calls to retrive XHCIPciState. Made required changes for
>> qemu-xhci-nec.
>> >> >
>> >> > Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>> >>
>> >> I can't see a "sysbus model".  What I can see is
>> >>
>> >>          TYPE_DEVICE
>> >>               |
>> >>        TYPE_PCI_DEVICE
>> >>               |
>> >>         TYPE_XHCI_PCI (renamed from TYPE_XHCI)
>> >>           /       \
>> >> TYPE_QEMU_XHCI TYPE_NEC_XHCI
>> >>
>> >> All but the two leaves are abstract.
>> >>
>> >> Do you intend to add a "sysbus model" in a future patch?
>> > [Sai Pavan Boddu]  Yes. I would be sending it along with that a device
>> > which would be using it. (i.e for zynqmp soc ) Let me know, if its good to
>> include hcd-xhci-sysbus.c here ?
>> 
>> I'm not sure this series is worthwhile this future patch.  Up to the maintainer.
>> 
>> Here's a clean way to provide different bus connectors (say PCI and
>> sysbus) for the same core device:
>> 
>> Make the core device a TYPE_DEVICE.
>> 
>> For each desired bus, have a bus-specific device that contains a core device.
>> Use object_initialize_child() for the component.
> [Sai Pavan Boddu] This was my V1 implementation.
> Changed it to non-qom structure after some feedback from @Gred. Felt like XHCIState will not be used standalone.

I'll gladly defer to Gerd's judgement here.

  Hi,

> > >> Do you intend to add a "sysbus model" in a future patch?
> > > [Sai Pavan Boddu]  Yes. I would be sending it along with that a device
> > > which would be using it. (i.e for zynqmp soc ) Let me know, if its good to
> > include hcd-xhci-sysbus.c here ?

I think this would be useful, to see how the code separation plays out
on the sysbus side.

> > Here's a clean way to provide different bus connectors (say PCI and
> > sysbus) for the same core device:
> > 
> > Make the core device a TYPE_DEVICE.
> > 
> > For each desired bus, have a bus-specific device that contains a core device.
> > Use object_initialize_child() for the component.

> This was my V1 implementation.
> Changed it to non-qom structure after some feedback from @Gred. Felt like XHCIState will not be used standalone.

> > Example: core device TYPE_SERIAL, PCI device TYPE_PCI_SERIAL, ISA device
> > TYPE_ISA_SERIAL, sysbus devices TYPE_SERIAL_IO. TYPE_SERIAL_MM.

/me goes check out the serial code ...

For reference: commit which transforms serial into the structure
above is this:

    commit 7781b88ee458ff933459503ade0b0a6ddaad08de
    Author: Marc-André Lureau <marcandre.lureau@redhat.com>
    Date:   Mon Oct 21 23:32:12 2019 +0200

        serial: initial qom-ification

Note that this patch doesn't change structs, so ISASerialState still
looks this way:

struct ISASerialState {
    ISADevice parent_obj;
    [ ... ]
    SerialState state;
};

So you can likewise keep your current "struct XHCIPciState" struct
layout and still turn XHCIState into a device object.  Which is nice to
have as it better models the hardware (xhci core behind pci connector).

take care,
  Gerd

Hi Gred,

> -----Original Message-----
> From: Gerd Hoffmann <kraxel@redhat.com>
> Sent: Tuesday, June 30, 2020 1:09 AM
> To: Sai Pavan Boddu <saipava@xilinx.com>
> Cc: Markus Armbruster <armbru@redhat.com>; Peter Maydell
> <peter.maydell@linaro.org>; Thomas Huth <thuth@redhat.com>; Eduardo
> Habkost <ehabkost@redhat.com>; qemu-devel@nongnu.org; Alistair Francis
> <alistair.francis@wdc.com>; Paolo Bonzini <pbonzini@redhat.com>; Ying
> Fang <fangying1@huawei.com>; 'Marc-André Lureau'
> <marcandre.lureau@redhat.com>; 'Philippe Mathieu-Daudé'
> <philmd@redhat.com>
> Subject: Re: [PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci base
> model
> 
>   Hi,
> 
> > > >> Do you intend to add a "sysbus model" in a future patch?
> > > > [Sai Pavan Boddu]  Yes. I would be sending it along with that a
> > > > device which would be using it. (i.e for zynqmp soc ) Let me know,
> > > > if its good to
> > > include hcd-xhci-sysbus.c here ?
> 
> I think this would be useful, to see how the code separation plays out on the
> sysbus side.
> 
> > > Here's a clean way to provide different bus connectors (say PCI and
> > > sysbus) for the same core device:
> > >
> > > Make the core device a TYPE_DEVICE.
> > >
> > > For each desired bus, have a bus-specific device that contains a core
> device.
> > > Use object_initialize_child() for the component.
> 
> > This was my V1 implementation.
> > Changed it to non-qom structure after some feedback from @Gred. Felt
> like XHCIState will not be used standalone.
> 
> > > Example: core device TYPE_SERIAL, PCI device TYPE_PCI_SERIAL, ISA
> > > device TYPE_ISA_SERIAL, sysbus devices TYPE_SERIAL_IO.
> TYPE_SERIAL_MM.
> 
> /me goes check out the serial code ...
> 
> For reference: commit which transforms serial into the structure above is
> this:
> 
>     commit 7781b88ee458ff933459503ade0b0a6ddaad08de
>     Author: Marc-André Lureau <marcandre.lureau@redhat.com>
>     Date:   Mon Oct 21 23:32:12 2019 +0200
> 
>         serial: initial qom-ification
> 
> Note that this patch doesn't change structs, so ISASerialState still looks this
> way:
> 
> struct ISASerialState {
>     ISADevice parent_obj;
>     [ ... ]
>     SerialState state;
> };
> 
> So you can likewise keep your current "struct XHCIPciState" struct layout and
> still turn XHCIState into a device object.  Which is nice to have as it better
> models the hardware (xhci core behind pci connector).
[Sai Pavan Boddu] Ok, as marcus pointed. We might be able to use it just as, child of a pci/sysbus based controller. Rather than using it standalone. I agree it looks nice that way.

Thanks,
Sai Pavan
> 
> take care,
>   Gerd