[PATCH v2 0/3] Make hcd-xhci independent of pci hooks

[PATCH v2 0/3] Make hcd-xhci independent of pci hooks

[Sai Pavan Boddu]

Hi

This patch series attempts to make 'hcd-xhci' an independent model so
it can be used by both pci and system-bus interface.

Sorry for the late followup, below are the changes for V2

V2:
    Make XHCIState non-qom
    Use container_of functions for retriving pci device instance
    Initialize the AddressSpace pointer in PATCH 1/3 itself

Sai Pavan Boddu (3):
  usb/hcd-xhci: Make dma read/writes hooks pci free
  usb/hcd-xhci: Move qemu-xhci device to hcd-xhci-pci.c
  usb/hcd-xhci: Split pci wrapper for xhci base model

 hw/usb/Kconfig        |   6 ++
 hw/usb/Makefile.objs  |   1 +
 hw/usb/hcd-xhci-nec.c |  14 +--
 hw/usb/hcd-xhci-pci.c | 246 +++++++++++++++++++++++++++++++++++++++++++
 hw/usb/hcd-xhci-pci.h |  47 +++++++++
 hw/usb/hcd-xhci.c     | 287 ++++++++------------------------------------------
 hw/usb/hcd-xhci.h     |  30 +++---
 7 files changed, 369 insertions(+), 262 deletions(-)
 create mode 100644 hw/usb/hcd-xhci-pci.c
 create mode 100644 hw/usb/hcd-xhci-pci.h

-- 
2.7.4

[PATCH v2 1/3] usb/hcd-xhci: Make dma read/writes hooks pci free

[Sai Pavan Boddu]

This patch starts making the hcd-xhci.c pci free, as part of this
restructuring dma read/writes are handled without passing pci object.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/usb/hcd-xhci.c | 24 +++++++++++-------------
 hw/usb/hcd-xhci.h |  3 +++
 2 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index b330e36..fa6ce98 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -495,7 +495,7 @@ static inline void xhci_dma_read_u32s(XHCIState *xhci, dma_addr_t addr,
 
     assert((len % sizeof(uint32_t)) == 0);
 
-    pci_dma_read(PCI_DEVICE(xhci), addr, buf, len);
+    dma_memory_read(xhci->as, addr, buf, len);
 
     for (i = 0; i < (len / sizeof(uint32_t)); i++) {
         buf[i] = le32_to_cpu(buf[i]);
@@ -515,7 +515,7 @@ static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr,
     for (i = 0; i < n; i++) {
         tmp[i] = cpu_to_le32(buf[i]);
     }
-    pci_dma_write(PCI_DEVICE(xhci), addr, tmp, len);
+    dma_memory_write(xhci->as, addr, tmp, len);
 }
 
 static XHCIPort *xhci_lookup_port(XHCIState *xhci, struct USBPort *uport)
@@ -644,7 +644,6 @@ static void xhci_die(XHCIState *xhci)
 
 static void xhci_write_event(XHCIState *xhci, XHCIEvent *event, int v)
 {
-    PCIDevice *pci_dev = PCI_DEVICE(xhci);
     XHCIInterrupter *intr = &xhci->intr[v];
     XHCITRB ev_trb;
     dma_addr_t addr;
@@ -663,7 +662,7 @@ static void xhci_write_event(XHCIState *xhci, XHCIEvent *event, int v)
                                ev_trb.status, ev_trb.control);
 
     addr = intr->er_start + TRB_SIZE*intr->er_ep_idx;
-    pci_dma_write(pci_dev, addr, &ev_trb, TRB_SIZE);
+    dma_memory_write(xhci->as, addr, &ev_trb, TRB_SIZE);
 
     intr->er_ep_idx++;
     if (intr->er_ep_idx >= intr->er_size) {
@@ -720,12 +719,11 @@ static void xhci_ring_init(XHCIState *xhci, XHCIRing *ring,
 static TRBType xhci_ring_fetch(XHCIState *xhci, XHCIRing *ring, XHCITRB *trb,
                                dma_addr_t *addr)
 {
-    PCIDevice *pci_dev = PCI_DEVICE(xhci);
     uint32_t link_cnt = 0;
 
     while (1) {
         TRBType type;
-        pci_dma_read(pci_dev, ring->dequeue, trb, TRB_SIZE);
+        dma_memory_read(xhci->as, ring->dequeue, trb, TRB_SIZE);
         trb->addr = ring->dequeue;
         trb->ccs = ring->ccs;
         le64_to_cpus(&trb->parameter);
@@ -762,7 +760,6 @@ static TRBType xhci_ring_fetch(XHCIState *xhci, XHCIRing *ring, XHCITRB *trb,
 
 static int xhci_ring_chain_length(XHCIState *xhci, const XHCIRing *ring)
 {
-    PCIDevice *pci_dev = PCI_DEVICE(xhci);
     XHCITRB trb;
     int length = 0;
     dma_addr_t dequeue = ring->dequeue;
@@ -773,7 +770,7 @@ static int xhci_ring_chain_length(XHCIState *xhci, const XHCIRing *ring)
 
     while (1) {
         TRBType type;
-        pci_dma_read(pci_dev, dequeue, &trb, TRB_SIZE);
+        dma_memory_read(xhci->as, dequeue, &trb, TRB_SIZE);
         le64_to_cpus(&trb.parameter);
         le32_to_cpus(&trb.status);
         le32_to_cpus(&trb.control);
@@ -828,7 +825,7 @@ static void xhci_er_reset(XHCIState *xhci, int v)
         xhci_die(xhci);
         return;
     }
-    pci_dma_read(PCI_DEVICE(xhci), erstba, &seg, sizeof(seg));
+    dma_memory_read(xhci->as, erstba, &seg, sizeof(seg));
     le32_to_cpus(&seg.addr_low);
     le32_to_cpus(&seg.addr_high);
     le32_to_cpus(&seg.size);
@@ -1440,7 +1437,7 @@ static int xhci_xfer_create_sgl(XHCITransfer *xfer, int in_xfer)
     int i;
 
     xfer->int_req = false;
-    pci_dma_sglist_init(&xfer->sgl, PCI_DEVICE(xhci), xfer->trb_count);
+    qemu_sglist_init(&xfer->sgl, DEVICE(xhci), xfer->trb_count, xhci->as);
     for (i = 0; i < xfer->trb_count; i++) {
         XHCITRB *trb = &xfer->trbs[i];
         dma_addr_t addr;
@@ -2101,7 +2098,7 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid,
     assert(slotid >= 1 && slotid <= xhci->numslots);
 
     dcbaap = xhci_addr64(xhci->dcbaap_low, xhci->dcbaap_high);
-    poctx = ldq_le_pci_dma(PCI_DEVICE(xhci), dcbaap + 8 * slotid);
+    poctx = ldq_le_dma(xhci->as, dcbaap + 8 * slotid);
     ictx = xhci_mask64(pictx);
     octx = xhci_mask64(poctx);
 
@@ -2439,7 +2436,7 @@ static TRBCCode xhci_get_port_bandwidth(XHCIState *xhci, uint64_t pctx)
     /* TODO: actually implement real values here */
     bw_ctx[0] = 0;
     memset(&bw_ctx[1], 80, xhci->numports); /* 80% */
-    pci_dma_write(PCI_DEVICE(xhci), ctx, bw_ctx, sizeof(bw_ctx));
+    dma_memory_write(xhci->as, ctx, bw_ctx, sizeof(bw_ctx));
 
     return CC_SUCCESS;
 }
@@ -3431,6 +3428,7 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
     }
 
     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);
@@ -3531,7 +3529,7 @@ static int usb_xhci_post_load(void *opaque, int version_id)
             continue;
         }
         slot->ctx =
-            xhci_mask64(ldq_le_pci_dma(pci_dev, dcbaap + 8 * slotid));
+            xhci_mask64(ldq_le_dma(xhci->as, dcbaap + 8 * slotid));
         xhci_dma_read_u32s(xhci, slot->ctx, slot_ctx, sizeof(slot_ctx));
         slot->uport = xhci_lookup_uport(xhci, slot_ctx);
         if (!slot->uport) {
diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
index 2fad4df..18bed7e 100644
--- a/hw/usb/hcd-xhci.h
+++ b/hw/usb/hcd-xhci.h
@@ -22,6 +22,8 @@
 #ifndef HW_USB_HCD_XHCI_H
 #define HW_USB_HCD_XHCI_H
 
+#include "sysemu/dma.h"
+
 #define TYPE_XHCI "base-xhci"
 #define TYPE_NEC_XHCI "nec-usb-xhci"
 #define TYPE_QEMU_XHCI "qemu-xhci"
@@ -189,6 +191,7 @@ struct XHCIState {
 
     USBBus bus;
     MemoryRegion mem;
+    AddressSpace *as;
     MemoryRegion mem_cap;
     MemoryRegion mem_oper;
     MemoryRegion mem_runtime;
-- 
2.7.4

[PATCH v2 2/3] usb/hcd-xhci: Move qemu-xhci device to hcd-xhci-pci.c

[Sai Pavan Boddu]

Move pci specific devices to new file. This set the environment to move all
pci specific hooks in hcd-xhci.c to hcd-xhci-pci.c.

Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
---
 hw/usb/hcd-xhci-pci.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++
 hw/usb/hcd-xhci.c     | 32 ++------------------------
 hw/usb/hcd-xhci.h     |  2 ++
 3 files changed, 68 insertions(+), 30 deletions(-)
 create mode 100644 hw/usb/hcd-xhci-pci.c

diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
new file mode 100644
index 0000000..26af683
--- /dev/null
+++ b/hw/usb/hcd-xhci-pci.c
@@ -0,0 +1,64 @@
+/*
+ * USB xHCI controller with PCI system bus 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-03-01; Author: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
+ * Moved the pci specific content for hcd-xhci.c to hcd-xhci-pci.c
+ *
+ * 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/>.
+ */
+#include "qemu/osdep.h"
+#include "hw/pci/pci.h"
+#include "hw/qdev-properties.h"
+#include "hw/pci/msi.h"
+#include "hw/pci/msix.h"
+#include "hcd-xhci.h"
+#include "trace.h"
+#include "qapi/error.h"
+
+static void qemu_xhci_class_init(ObjectClass *klass, void *data)
+{
+    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+    k->vendor_id    = PCI_VENDOR_ID_REDHAT;
+    k->device_id    = PCI_DEVICE_ID_REDHAT_XHCI;
+    k->revision     = 0x01;
+}
+
+static void qemu_xhci_instance_init(Object *obj)
+{
+    XHCIState *xhci = XHCI(obj);
+
+    xhci->msi      = ON_OFF_AUTO_OFF;
+    xhci->msix     = ON_OFF_AUTO_AUTO;
+    xhci->numintrs = MAXINTRS;
+    xhci->numslots = MAXSLOTS;
+    xhci_set_flag(xhci, XHCI_FLAG_SS_FIRST);
+}
+
+static const TypeInfo qemu_xhci_info = {
+    .name          = TYPE_QEMU_XHCI,
+    .parent        = TYPE_XHCI,
+    .class_init    = qemu_xhci_class_init,
+    .instance_init = qemu_xhci_instance_init,
+};
+
+static void xhci_register_types(void)
+{
+    type_register_static(&qemu_xhci_info);
+}
+
+type_init(xhci_register_types)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index fa6ce98..d340518 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -429,12 +429,12 @@ static const char *ep_state_name(uint32_t state)
                        ARRAY_SIZE(ep_state_names));
 }
 
-static bool xhci_get_flag(XHCIState *xhci, enum xhci_flags bit)
+bool xhci_get_flag(XHCIState *xhci, enum xhci_flags bit)
 {
     return xhci->flags & (1 << bit);
 }
 
-static void xhci_set_flag(XHCIState *xhci, enum xhci_flags bit)
+void xhci_set_flag(XHCIState *xhci, enum xhci_flags bit)
 {
     xhci->flags |= (1 << bit);
 }
@@ -3727,37 +3727,9 @@ static const TypeInfo xhci_info = {
     },
 };
 
-static void qemu_xhci_class_init(ObjectClass *klass, void *data)
-{
-    PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
-    k->vendor_id    = PCI_VENDOR_ID_REDHAT;
-    k->device_id    = PCI_DEVICE_ID_REDHAT_XHCI;
-    k->revision     = 0x01;
-}
-
-static void qemu_xhci_instance_init(Object *obj)
-{
-    XHCIState *xhci = XHCI(obj);
-
-    xhci->msi      = ON_OFF_AUTO_OFF;
-    xhci->msix     = ON_OFF_AUTO_AUTO;
-    xhci->numintrs = MAXINTRS;
-    xhci->numslots = MAXSLOTS;
-    xhci_set_flag(xhci, XHCI_FLAG_SS_FIRST);
-}
-
-static const TypeInfo qemu_xhci_info = {
-    .name          = TYPE_QEMU_XHCI,
-    .parent        = TYPE_XHCI,
-    .class_init    = qemu_xhci_class_init,
-    .instance_init = qemu_xhci_instance_init,
-};
-
 static void xhci_register_types(void)
 {
     type_register_static(&xhci_info);
-    type_register_static(&qemu_xhci_info);
 }
 
 type_init(xhci_register_types)
diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
index 18bed7e..ca03481 100644
--- a/hw/usb/hcd-xhci.h
+++ b/hw/usb/hcd-xhci.h
@@ -232,4 +232,6 @@ struct XHCIState {
     bool nec_quirks;
 };
 
+bool xhci_get_flag(XHCIState *xhci, enum xhci_flags bit);
+void xhci_set_flag(XHCIState *xhci, enum xhci_flags bit);
 #endif
-- 
2.7.4

[PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci base model

[Sai Pavan Boddu]

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

Re: [PATCH v2 1/3] usb/hcd-xhci: Make dma read/writes hooks pci free

[Philippe Mathieu-Daudé]

On 6/24/20 4:16 PM, Sai Pavan Boddu wrote:
> This patch starts making the hcd-xhci.c pci free, as part of this
> restructuring dma read/writes are handled without passing pci object.
> 
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> ---
>  hw/usb/hcd-xhci.c | 24 +++++++++++-------------
>  hw/usb/hcd-xhci.h |  3 +++
>  2 files changed, 14 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
> index b330e36..fa6ce98 100644
> --- a/hw/usb/hcd-xhci.c
> +++ b/hw/usb/hcd-xhci.c
> @@ -495,7 +495,7 @@ static inline void xhci_dma_read_u32s(XHCIState *xhci, dma_addr_t addr,
>  
>      assert((len % sizeof(uint32_t)) == 0);
>  
> -    pci_dma_read(PCI_DEVICE(xhci), addr, buf, len);
> +    dma_memory_read(xhci->as, addr, buf, len);
>  
>      for (i = 0; i < (len / sizeof(uint32_t)); i++) {
>          buf[i] = le32_to_cpu(buf[i]);
> @@ -515,7 +515,7 @@ static inline void xhci_dma_write_u32s(XHCIState *xhci, dma_addr_t addr,
>      for (i = 0; i < n; i++) {
>          tmp[i] = cpu_to_le32(buf[i]);
>      }
> -    pci_dma_write(PCI_DEVICE(xhci), addr, tmp, len);
> +    dma_memory_write(xhci->as, addr, tmp, len);
>  }
>  
>  static XHCIPort *xhci_lookup_port(XHCIState *xhci, struct USBPort *uport)
> @@ -644,7 +644,6 @@ static void xhci_die(XHCIState *xhci)
>  
>  static void xhci_write_event(XHCIState *xhci, XHCIEvent *event, int v)
>  {
> -    PCIDevice *pci_dev = PCI_DEVICE(xhci);
>      XHCIInterrupter *intr = &xhci->intr[v];
>      XHCITRB ev_trb;
>      dma_addr_t addr;
> @@ -663,7 +662,7 @@ static void xhci_write_event(XHCIState *xhci, XHCIEvent *event, int v)
>                                 ev_trb.status, ev_trb.control);
>  
>      addr = intr->er_start + TRB_SIZE*intr->er_ep_idx;
> -    pci_dma_write(pci_dev, addr, &ev_trb, TRB_SIZE);
> +    dma_memory_write(xhci->as, addr, &ev_trb, TRB_SIZE);
>  
>      intr->er_ep_idx++;
>      if (intr->er_ep_idx >= intr->er_size) {
> @@ -720,12 +719,11 @@ static void xhci_ring_init(XHCIState *xhci, XHCIRing *ring,
>  static TRBType xhci_ring_fetch(XHCIState *xhci, XHCIRing *ring, XHCITRB *trb,
>                                 dma_addr_t *addr)
>  {
> -    PCIDevice *pci_dev = PCI_DEVICE(xhci);
>      uint32_t link_cnt = 0;
>  
>      while (1) {
>          TRBType type;
> -        pci_dma_read(pci_dev, ring->dequeue, trb, TRB_SIZE);
> +        dma_memory_read(xhci->as, ring->dequeue, trb, TRB_SIZE);
>          trb->addr = ring->dequeue;
>          trb->ccs = ring->ccs;
>          le64_to_cpus(&trb->parameter);
> @@ -762,7 +760,6 @@ static TRBType xhci_ring_fetch(XHCIState *xhci, XHCIRing *ring, XHCITRB *trb,
>  
>  static int xhci_ring_chain_length(XHCIState *xhci, const XHCIRing *ring)
>  {
> -    PCIDevice *pci_dev = PCI_DEVICE(xhci);
>      XHCITRB trb;
>      int length = 0;
>      dma_addr_t dequeue = ring->dequeue;
> @@ -773,7 +770,7 @@ static int xhci_ring_chain_length(XHCIState *xhci, const XHCIRing *ring)
>  
>      while (1) {
>          TRBType type;
> -        pci_dma_read(pci_dev, dequeue, &trb, TRB_SIZE);
> +        dma_memory_read(xhci->as, dequeue, &trb, TRB_SIZE);
>          le64_to_cpus(&trb.parameter);
>          le32_to_cpus(&trb.status);
>          le32_to_cpus(&trb.control);
> @@ -828,7 +825,7 @@ static void xhci_er_reset(XHCIState *xhci, int v)
>          xhci_die(xhci);
>          return;
>      }
> -    pci_dma_read(PCI_DEVICE(xhci), erstba, &seg, sizeof(seg));
> +    dma_memory_read(xhci->as, erstba, &seg, sizeof(seg));
>      le32_to_cpus(&seg.addr_low);
>      le32_to_cpus(&seg.addr_high);
>      le32_to_cpus(&seg.size);
> @@ -1440,7 +1437,7 @@ static int xhci_xfer_create_sgl(XHCITransfer *xfer, int in_xfer)
>      int i;
>  
>      xfer->int_req = false;
> -    pci_dma_sglist_init(&xfer->sgl, PCI_DEVICE(xhci), xfer->trb_count);
> +    qemu_sglist_init(&xfer->sgl, DEVICE(xhci), xfer->trb_count, xhci->as);
>      for (i = 0; i < xfer->trb_count; i++) {
>          XHCITRB *trb = &xfer->trbs[i];
>          dma_addr_t addr;
> @@ -2101,7 +2098,7 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid,
>      assert(slotid >= 1 && slotid <= xhci->numslots);
>  
>      dcbaap = xhci_addr64(xhci->dcbaap_low, xhci->dcbaap_high);
> -    poctx = ldq_le_pci_dma(PCI_DEVICE(xhci), dcbaap + 8 * slotid);
> +    poctx = ldq_le_dma(xhci->as, dcbaap + 8 * slotid);
>      ictx = xhci_mask64(pictx);
>      octx = xhci_mask64(poctx);
>  
> @@ -2439,7 +2436,7 @@ static TRBCCode xhci_get_port_bandwidth(XHCIState *xhci, uint64_t pctx)
>      /* TODO: actually implement real values here */
>      bw_ctx[0] = 0;
>      memset(&bw_ctx[1], 80, xhci->numports); /* 80% */
> -    pci_dma_write(PCI_DEVICE(xhci), ctx, bw_ctx, sizeof(bw_ctx));
> +    dma_memory_write(xhci->as, ctx, bw_ctx, sizeof(bw_ctx));
>  
>      return CC_SUCCESS;
>  }
> @@ -3431,6 +3428,7 @@ static void usb_xhci_realize(struct PCIDevice *dev, Error **errp)
>      }
>  
>      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);
> @@ -3531,7 +3529,7 @@ static int usb_xhci_post_load(void *opaque, int version_id)
>              continue;
>          }
>          slot->ctx =
> -            xhci_mask64(ldq_le_pci_dma(pci_dev, dcbaap + 8 * slotid));
> +            xhci_mask64(ldq_le_dma(xhci->as, dcbaap + 8 * slotid));
>          xhci_dma_read_u32s(xhci, slot->ctx, slot_ctx, sizeof(slot_ctx));
>          slot->uport = xhci_lookup_uport(xhci, slot_ctx);
>          if (!slot->uport) {
> diff --git a/hw/usb/hcd-xhci.h b/hw/usb/hcd-xhci.h
> index 2fad4df..18bed7e 100644
> --- a/hw/usb/hcd-xhci.h
> +++ b/hw/usb/hcd-xhci.h
> @@ -22,6 +22,8 @@
>  #ifndef HW_USB_HCD_XHCI_H
>  #define HW_USB_HCD_XHCI_H
>  
> +#include "sysemu/dma.h"

Not needed because forward-declared in "qemu/typedefs.h".

Moving the inclusion to hw/usb/hcd-xhci.c:

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> +
>  #define TYPE_XHCI "base-xhci"
>  #define TYPE_NEC_XHCI "nec-usb-xhci"
>  #define TYPE_QEMU_XHCI "qemu-xhci"
> @@ -189,6 +191,7 @@ struct XHCIState {
>  
>      USBBus bus;
>      MemoryRegion mem;
> +    AddressSpace *as;
>      MemoryRegion mem_cap;
>      MemoryRegion mem_oper;
>      MemoryRegion mem_runtime;
> 

Re: [PATCH v2 2/3] usb/hcd-xhci: Move qemu-xhci device to hcd-xhci-pci.c

[Markus Armbruster]

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

> Move pci specific devices to new file. This set the environment to move all
> pci specific hooks in hcd-xhci.c to hcd-xhci-pci.c.
>
> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> ---
>  hw/usb/hcd-xhci-pci.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  hw/usb/hcd-xhci.c     | 32 ++------------------------
>  hw/usb/hcd-xhci.h     |  2 ++
>  3 files changed, 68 insertions(+), 30 deletions(-)
>  create mode 100644 hw/usb/hcd-xhci-pci.c
>
> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
> new file mode 100644
> index 0000000..26af683
> --- /dev/null
> +++ b/hw/usb/hcd-xhci-pci.c
> @@ -0,0 +1,64 @@
> +/*
> + * USB xHCI controller with PCI system bus emulation

Scratch "system".

> + *
> + * Copyright (c) 2011 Securiforest
> + * Date: 2011-05-11 ;  Author: Hector Martin <hector@marcansoft.com>

Let's use the opportunity to drop the "Date: " part, because we don't
have it anywhere else.

> + * Based on usb-ohci.c, emulates Renesas NEC USB 3.0
> + * Date: 2020-03-01; Author: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>

And no new "Date: " parts, please.

> + * Moved the pci specific content for hcd-xhci.c to hcd-xhci-pci.c
> + *
> + * 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/>.
> + */
[...]

Re: [PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci base model

[Markus Armbruster]

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?

Re: [PATCH v2 2/3] usb/hcd-xhci: Move qemu-xhci device to hcd-xhci-pci.c

[Philippe Mathieu-Daudé]

On 6/25/20 10:06 AM, Markus Armbruster wrote:
> Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:
> 
>> Move pci specific devices to new file. This set the environment to move all
>> pci specific hooks in hcd-xhci.c to hcd-xhci-pci.c.
>>
>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>> ---
>>  hw/usb/hcd-xhci-pci.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++
>>  hw/usb/hcd-xhci.c     | 32 ++------------------------
>>  hw/usb/hcd-xhci.h     |  2 ++
>>  3 files changed, 68 insertions(+), 30 deletions(-)
>>  create mode 100644 hw/usb/hcd-xhci-pci.c
>>
>> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c
>> new file mode 100644
>> index 0000000..26af683
>> --- /dev/null
>> +++ b/hw/usb/hcd-xhci-pci.c
>> @@ -0,0 +1,64 @@
>> +/*
>> + * USB xHCI controller with PCI system bus emulation
> 
> Scratch "system".
> 
>> + *
>> + * Copyright (c) 2011 Securiforest
>> + * Date: 2011-05-11 ;  Author: Hector Martin <hector@marcansoft.com>
> 
> Let's use the opportunity to drop the "Date: " part, because we don't
> have it anywhere else.

Good opportunity to suggest the SPDX tags again :P

/*
 * SPDX-FileCopyrightText: 2011 Securiforest
 * SPDX-FileContributor: Hector Martin <hector@marcansoft.com>
 * SPDX-FileCopyrightText: 2020 Xilinx Inc.
 * SPDX-FileContributor: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
 * SPDX-License-Identifier: GPL-2.0-or-later
 */

https://spdx.org/rdf/ontology/spdx-2-0-rc/dataproperties/fileContributor___-1635717172.html

> 
>> + * Based on usb-ohci.c, emulates Renesas NEC USB 3.0
>> + * Date: 2020-03-01; Author: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> 
> And no new "Date: " parts, please.
> 
>> + * Moved the pci specific content for hcd-xhci.c to hcd-xhci-pci.c
>> + *
>> + * 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/>.
>> + */
> [...]
> 

RE: [PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci base model

[Sai Pavan Boddu]

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

Re: [PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci base model

[Markus Armbruster]

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.

RE: [PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci base model

[Sai Pavan Boddu]

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.

Re: [PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci base model

[Markus Armbruster]

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.

Re: [PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci base model

[Gerd Hoffmann]

  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

RE: [PATCH v2 3/3] usb/hcd-xhci: Split pci wrapper for xhci base model

[Sai Pavan Boddu]

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

RE: [PATCH v2 2/3] usb/hcd-xhci: Move qemu-xhci device to hcd-xhci-pci.c

[Sai Pavan Boddu]

HI Philippe,

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> Sent: Thursday, June 25, 2020 1:48 PM
> To: Markus Armbruster <armbru@redhat.com>; Sai Pavan Boddu
> <saipava@xilinx.com>; Thomas Huth <thuth@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>; Peter Maydell
> <peter.maydell@linaro.org>; 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>
> Subject: Re: [PATCH v2 2/3] usb/hcd-xhci: Move qemu-xhci device to hcd-
> xhci-pci.c
> 
> On 6/25/20 10:06 AM, Markus Armbruster wrote:
> > Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:
> >
> >> Move pci specific devices to new file. This set the environment to
> >> move all pci specific hooks in hcd-xhci.c to hcd-xhci-pci.c.
> >>
> >> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> >> ---
> >>  hw/usb/hcd-xhci-pci.c | 64
> +++++++++++++++++++++++++++++++++++++++++++++++++++
> >>  hw/usb/hcd-xhci.c     | 32 ++------------------------
> >>  hw/usb/hcd-xhci.h     |  2 ++
> >>  3 files changed, 68 insertions(+), 30 deletions(-)  create mode
> >> 100644 hw/usb/hcd-xhci-pci.c
> >>
> >> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c new file
> >> mode 100644 index 0000000..26af683
> >> --- /dev/null
> >> +++ b/hw/usb/hcd-xhci-pci.c
> >> @@ -0,0 +1,64 @@
> >> +/*
> >> + * USB xHCI controller with PCI system bus emulation
> >
> > Scratch "system".
> >
> >> + *
> >> + * Copyright (c) 2011 Securiforest
> >> + * Date: 2011-05-11 ;  Author: Hector Martin <hector@marcansoft.com>
> >
> > Let's use the opportunity to drop the "Date: " part, because we don't
> > have it anywhere else.
> 
> Good opportunity to suggest the SPDX tags again :P
> 
> /*
>  * SPDX-FileCopyrightText: 2011 Securiforest
>  * SPDX-FileContributor: Hector Martin <hector@marcansoft.com>
>  * SPDX-FileCopyrightText: 2020 Xilinx Inc.
>  * SPDX-FileContributor: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>  * SPDX-License-Identifier: GPL-2.0-or-later  */	
[Sai Pavan Boddu] I would include this in V4, Forgot them in V3.

Thanks,
Sai Pavan
> 
> https://spdx.org/rdf/ontology/spdx-2-0-
> rc/dataproperties/fileContributor___-1635717172.html
> 
> >
> >> + * Based on usb-ohci.c, emulates Renesas NEC USB 3.0
> >> + * Date: 2020-03-01; Author: Sai Pavan Boddu
> >> + <sai.pavan.boddu@xilinx.com>
> >
> > And no new "Date: " parts, please.
> >
> >> + * Moved the pci specific content for hcd-xhci.c to hcd-xhci-pci.c
> >> + *
> >> + * 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/>.
> >> + */
> > [...]
> >

Re: [PATCH v2 2/3] usb/hcd-xhci: Move qemu-xhci device to hcd-xhci-pci.c

[Thomas Huth]

On 20/07/2020 10.00, Sai Pavan Boddu wrote:
> HI Philippe,
> 
>> -----Original Message-----
>> From: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Sent: Thursday, June 25, 2020 1:48 PM
>> To: Markus Armbruster <armbru@redhat.com>; Sai Pavan Boddu
>> <saipava@xilinx.com>; Thomas Huth <thuth@redhat.com>
>> Cc: Gerd Hoffmann <kraxel@redhat.com>; Peter Maydell
>> <peter.maydell@linaro.org>; 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>
>> Subject: Re: [PATCH v2 2/3] usb/hcd-xhci: Move qemu-xhci device to hcd-
>> xhci-pci.c
>>
>> On 6/25/20 10:06 AM, Markus Armbruster wrote:
>>> Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:
>>>
>>>> Move pci specific devices to new file. This set the environment to
>>>> move all pci specific hooks in hcd-xhci.c to hcd-xhci-pci.c.
>>>>
>>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>>> ---
>>>>  hw/usb/hcd-xhci-pci.c | 64
>> +++++++++++++++++++++++++++++++++++++++++++++++++++
>>>>  hw/usb/hcd-xhci.c     | 32 ++------------------------
>>>>  hw/usb/hcd-xhci.h     |  2 ++
>>>>  3 files changed, 68 insertions(+), 30 deletions(-)  create mode
>>>> 100644 hw/usb/hcd-xhci-pci.c
>>>>
>>>> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c new file
>>>> mode 100644 index 0000000..26af683
>>>> --- /dev/null
>>>> +++ b/hw/usb/hcd-xhci-pci.c
>>>> @@ -0,0 +1,64 @@
>>>> +/*
>>>> + * USB xHCI controller with PCI system bus emulation
>>>
>>> Scratch "system".
>>>
>>>> + *
>>>> + * Copyright (c) 2011 Securiforest
>>>> + * Date: 2011-05-11 ;  Author: Hector Martin <hector@marcansoft.com>
>>>
>>> Let's use the opportunity to drop the "Date: " part, because we don't
>>> have it anywhere else.
>>
>> Good opportunity to suggest the SPDX tags again :P
>>
>> /*
>>  * SPDX-FileCopyrightText: 2011 Securiforest
>>  * SPDX-FileContributor: Hector Martin <hector@marcansoft.com>
>>  * SPDX-FileCopyrightText: 2020 Xilinx Inc.
>>  * SPDX-FileContributor: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
>>  * SPDX-License-Identifier: GPL-2.0-or-later  */	
> [Sai Pavan Boddu] I would include this in V4, Forgot them in V3.
> 
> Thanks,
> Sai Pavan
>>
>> https://spdx.org/rdf/ontology/spdx-2-0-
>> rc/dataproperties/fileContributor___-1635717172.html
>>
>>>
>>>> + * Based on usb-ohci.c, emulates Renesas NEC USB 3.0
>>>> + * Date: 2020-03-01; Author: Sai Pavan Boddu
>>>> + <sai.pavan.boddu@xilinx.com>
>>>
>>> And no new "Date: " parts, please.
>>>
>>>> + * Moved the pci specific content for hcd-xhci.c to hcd-xhci-pci.c
>>>> + *
>>>> + * 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.

And while you're at it: There was never a "version 2" of the Lesser GPL.
In version 2.0, it was still called "Library" GPL. So it is quite likely
that version 2.1 is meant here instead.

 Thomas

RE: [PATCH v2 2/3] usb/hcd-xhci: Move qemu-xhci device to hcd-xhci-pci.c

[Sai Pavan Boddu]

Hi Thomas,

> -----Original Message-----
> From: Thomas Huth <thuth@redhat.com>
> Sent: Monday, July 20, 2020 1:37 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>; Philippe Mathieu-Daudé
> <philmd@redhat.com>; Markus Armbruster <armbru@redhat.com>
> Cc: Gerd Hoffmann <kraxel@redhat.com>; Peter Maydell
> <peter.maydell@linaro.org>; 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>
> Subject: Re: [PATCH v2 2/3] usb/hcd-xhci: Move qemu-xhci device to hcd-
> xhci-pci.c
> 
> On 20/07/2020 10.00, Sai Pavan Boddu wrote:
> > HI Philippe,
> >
> >> -----Original Message-----
> >> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> Sent: Thursday, June 25, 2020 1:48 PM
> >> To: Markus Armbruster <armbru@redhat.com>; Sai Pavan Boddu
> >> <saipava@xilinx.com>; Thomas Huth <thuth@redhat.com>
> >> Cc: Gerd Hoffmann <kraxel@redhat.com>; Peter Maydell
> >> <peter.maydell@linaro.org>; 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>
> >> Subject: Re: [PATCH v2 2/3] usb/hcd-xhci: Move qemu-xhci device to
> >> hcd- xhci-pci.c
> >>
> >> On 6/25/20 10:06 AM, Markus Armbruster wrote:
> >>> Sai Pavan Boddu <sai.pavan.boddu@xilinx.com> writes:
> >>>
> >>>> Move pci specific devices to new file. This set the environment to
> >>>> move all pci specific hooks in hcd-xhci.c to hcd-xhci-pci.c.
> >>>>
> >>>> Signed-off-by: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> >>>> ---
> >>>>  hw/usb/hcd-xhci-pci.c | 64
> >> +++++++++++++++++++++++++++++++++++++++++++++++++++
> >>>>  hw/usb/hcd-xhci.c     | 32 ++------------------------
> >>>>  hw/usb/hcd-xhci.h     |  2 ++
> >>>>  3 files changed, 68 insertions(+), 30 deletions(-)  create mode
> >>>> 100644 hw/usb/hcd-xhci-pci.c
> >>>>
> >>>> diff --git a/hw/usb/hcd-xhci-pci.c b/hw/usb/hcd-xhci-pci.c new file
> >>>> mode 100644 index 0000000..26af683
> >>>> --- /dev/null
> >>>> +++ b/hw/usb/hcd-xhci-pci.c
> >>>> @@ -0,0 +1,64 @@
> >>>> +/*
> >>>> + * USB xHCI controller with PCI system bus emulation
> >>>
> >>> Scratch "system".
> >>>
> >>>> + *
> >>>> + * Copyright (c) 2011 Securiforest
> >>>> + * Date: 2011-05-11 ;  Author: Hector Martin
> >>>> + <hector@marcansoft.com>
> >>>
> >>> Let's use the opportunity to drop the "Date: " part, because we
> >>> don't have it anywhere else.
> >>
> >> Good opportunity to suggest the SPDX tags again :P
> >>
> >> /*
> >>  * SPDX-FileCopyrightText: 2011 Securiforest
> >>  * SPDX-FileContributor: Hector Martin <hector@marcansoft.com>
> >>  * SPDX-FileCopyrightText: 2020 Xilinx Inc.
> >>  * SPDX-FileContributor: Sai Pavan Boddu <sai.pavan.boddu@xilinx.com>
> >>  * SPDX-License-Identifier: GPL-2.0-or-later  */
> > [Sai Pavan Boddu] I would include this in V4, Forgot them in V3.
> >
> > Thanks,
> > Sai Pavan
> >>
> >> https://spdx.org/rdf/ontology/spdx-2-0-
> >> rc/dataproperties/fileContributor___-1635717172.html
> >>
> >>>
> >>>> + * Based on usb-ohci.c, emulates Renesas NEC USB 3.0
> >>>> + * Date: 2020-03-01; Author: Sai Pavan Boddu
> >>>> + <sai.pavan.boddu@xilinx.com>
> >>>
> >>> And no new "Date: " parts, please.
> >>>
> >>>> + * Moved the pci specific content for hcd-xhci.c to hcd-xhci-pci.c
> >>>> + *
> >>>> + * 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.
> 
> And while you're at it: There was never a "version 2" of the Lesser GPL.
> In version 2.0, it was still called "Library" GPL. So it is quite likely that version
> 2.1 is meant here instead.
[Sai Pavan Boddu] I have less knowledge here. But indeed I don’t find LGPL 2.0 https://www.gnu.org/licenses/licenses.html#LicenseURLs

BTW, I still see our repository use combination of GPL and LGPL. Is there any general rule to follow at high level.

Regards,
Sai Pavan
> 
>  Thomas

Re: [PATCH v2 2/3] usb/hcd-xhci: Move qemu-xhci device to hcd-xhci-pci.c

[Thomas Huth]

 Hi,

On 22/07/2020 09.49, Sai Pavan Boddu wrote:
[...]
>>>>>> + * 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.
>>
>> And while you're at it: There was never a "version 2" of the Lesser GPL.
>> In version 2.0, it was still called "Library" GPL. So it is quite likely that version
>> 2.1 is meant here instead.
> [Sai Pavan Boddu] I have less knowledge here. But indeed I don’t find LGPL 2.0 https://www.gnu.org/licenses/licenses.html#LicenseURLs

You can find version 2.0 here, for example:

 https://www.gnu.org/licenses/old-licenses/old-licenses.html#LGPL

... but as I said, v2.0 is called "Library" GPL instead of "Lesser" GPL.

> BTW, I still see our repository use combination of GPL and LGPL. Is there any general rule to follow at high level.

As long as the license is a standard license that is compatible with the
GPLv2 or any later version, you should be fine. See the LICENSE file in
the top directory of the sources for details.

As a general rule, I'd say either use "GPLv2 or later" (see the file
COPYING in the main directory) or "LGPLv2.1 or later" (see COPYING.LIB
in the main directory) for new code, unless you contribute to the tcg/
folder where MIT or BSD is preferred instead.

 Thomas

Re: [PATCH v2 2/3] usb/hcd-xhci: Move qemu-xhci device to hcd-xhci-pci.c

[Markus Armbruster]

Thomas Huth <thuth@redhat.com> writes:

>  Hi,
>
> On 22/07/2020 09.49, Sai Pavan Boddu wrote:
> [...]
>>>>>>> + * 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.
>>>
>>> And while you're at it: There was never a "version 2" of the Lesser GPL.
>>> In version 2.0, it was still called "Library" GPL. So it is quite likely that version
>>> 2.1 is meant here instead.
>> [Sai Pavan Boddu] I have less knowledge here. But indeed I don’t find LGPL 2.0 https://www.gnu.org/licenses/licenses.html#LicenseURLs
>
> You can find version 2.0 here, for example:
>
>  https://www.gnu.org/licenses/old-licenses/old-licenses.html#LGPL
>
> ... but as I said, v2.0 is called "Library" GPL instead of "Lesser" GPL.
>
>> BTW, I still see our repository use combination of GPL and LGPL. Is there any general rule to follow at high level.
>
> As long as the license is a standard license that is compatible with the
> GPLv2 or any later version, you should be fine. See the LICENSE file in
> the top directory of the sources for details.
>
> As a general rule, I'd say either use "GPLv2 or later" (see the file
> COPYING in the main directory) or "LGPLv2.1 or later" (see COPYING.LIB
> in the main directory) for new code, unless you contribute to the tcg/
> folder where MIT or BSD is preferred instead.

Please use "GPLv2 or later".

If you believe you have a compelling reason for using a different
license (compatible with GPLv2, of course), then explain yourself in the
commit message.