diff mbox series

[v4,1/7] usb/hcd-xhci: Make dma read/writes hooks pci free

Message ID 1598642380-27817-2-git-send-email-sai.pavan.boddu@xilinx.com (mailing list archive)
State New, archived
Headers show
Series Make hcd-xhci independent of pci hooks | expand

Commit Message

Sai Pavan Boddu Aug. 28, 2020, 7:19 p.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé Sept. 1, 2020, 4:34 p.m. UTC | #1
On 8/28/20 9:19 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 67a18fe..2590f7a 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 946af51..edbd926 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"

AddressSpace is forward-declared in "qemu/typedefs.h",
so no need to include it here (yes in the sources including
hcd-xhci.h).

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

> +
>  #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;
>
Sai Pavan Boddu Sept. 8, 2020, 9:58 a.m. UTC | #2
Hi Philippe,

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> Sent: Tuesday, September 1, 2020 10:05 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>; Peter Maydell
> <peter.maydell@linaro.org>; Markus Armbruster <armbru@redhat.com>;
> 'Marc-André Lureau' <marcandre.lureau@redhat.com>; Paolo Bonzini
> <pbonzini@redhat.com>; Gerd Hoffmann <kraxel@redhat.com>; Edgar
> Iglesias <edgari@xilinx.com>; Francisco Eduardo Iglesias
> <figlesia@xilinx.com>
> Cc: qemu-devel@nongnu.org; Alistair Francis <alistair.francis@wdc.com>;
> Eduardo Habkost <ehabkost@redhat.com>; Ying Fang
> <fangying1@huawei.com>; Vikram Garhwal <fnuv@xilinx.com>; Paul
> Zimmerman <pauldzim@gmail.com>
> Subject: Re: [PATCH v4 1/7] usb/hcd-xhci: Make dma read/writes hooks pci
> free
> 
> On 8/28/20 9:19 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
> > 67a18fe..2590f7a 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
> > 946af51..edbd926 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"
> 
> AddressSpace is forward-declared in "qemu/typedefs.h", so no need to
> include it here (yes in the sources including hcd-xhci.h).
[Sai Pavan Boddu] Yes you are right!, but without this " dma_addr_t " is undefined. 
At this point of the patch, hcd-xhci.h is compiled along with pci.h which would provide 'dma_addr_t', but when we strip the pci wrapper around hcd-xhci we would miss it. Let me know, if its good to add later in the patch series when the split happens.

Regards,
Sai Pavan
> 
> With that fixed:
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> > +
> >  #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;
> >
Philippe Mathieu-Daudé Sept. 8, 2020, 10:30 a.m. UTC | #3
>> On 8/28/20 9:19 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(-)
>>>
[...]
>>> --- 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"
>>
>> AddressSpace is forward-declared in "qemu/typedefs.h", so no need to
>> include it here (yes in the sources including hcd-xhci.h).
> [Sai Pavan Boddu] Yes you are right!, but without this " dma_addr_t " is undefined. 
> At this point of the patch, hcd-xhci.h is compiled along with pci.h which would provide 'dma_addr_t', but when we strip the pci wrapper around hcd-xhci we would miss it. Let me know, if its good to add later in the patch series when the split happens.

OK :( I'd prefer to only include "sysemu/dma.h" in hw/usb/hcd-xhci.c.

Cc'ing David who added dma_addr_t in commit e5332e6334f
("iommu: Introduce IOMMU emulation infrastructure"), it
might be simpler to move its declaration to "qemu/typedefs.h"
too.

So no change needed for this patch.

> 
> Regards,
> Sai Pavan
>>
>> With that fixed:
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>>> +
>>>  #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;
>>>
>
Philippe Mathieu-Daudé Sept. 8, 2020, 10:34 a.m. UTC | #4
On 9/8/20 12:30 PM, Philippe Mathieu-Daudé wrote:
>>> On 8/28/20 9:19 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(-)
>>>>
> [...]
>>>> --- 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"
>>>
>>> AddressSpace is forward-declared in "qemu/typedefs.h", so no need to
>>> include it here (yes in the sources including hcd-xhci.h).
>> [Sai Pavan Boddu] Yes you are right!, but without this " dma_addr_t " is undefined. 
>> At this point of the patch, hcd-xhci.h is compiled along with pci.h which would provide 'dma_addr_t', but when we strip the pci wrapper around hcd-xhci we would miss it. Let me know, if its good to add later in the patch series when the split happens.
> 
> OK :( I'd prefer to only include "sysemu/dma.h" in hw/usb/hcd-xhci.c.
> 
> Cc'ing David who added dma_addr_t in commit e5332e6334f
> ("iommu: Introduce IOMMU emulation infrastructure"), it
> might be simpler to move its declaration to "qemu/typedefs.h"
> too.

Btw this type should be poisoned for user-mode.

> 
> So no change needed for this patch.
> 
>>
>> Regards,
>> Sai Pavan
>>>
>>> With that fixed:
>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>
>>>> +
>>>>  #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;
>>>>
>>
>
Sai Pavan Boddu Sept. 8, 2020, 11:35 a.m. UTC | #5
Hi Philippe,

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> Sent: Tuesday, September 8, 2020 4:05 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>; Peter Maydell
> <peter.maydell@linaro.org>; Markus Armbruster <armbru@redhat.com>;
> 'Marc-André Lureau' <marcandre.lureau@redhat.com>; Paolo Bonzini
> <pbonzini@redhat.com>; Gerd Hoffmann <kraxel@redhat.com>; Edgar
> Iglesias <edgari@xilinx.com>; Francisco Eduardo Iglesias
> <figlesia@xilinx.com>; David Gibson <david@gibson.dropbear.id.au>
> Cc: qemu-devel@nongnu.org; Alistair Francis <alistair.francis@wdc.com>;
> Eduardo Habkost <ehabkost@redhat.com>; Ying Fang
> <fangying1@huawei.com>; Vikram Garhwal <fnuv@xilinx.com>; Paul
> Zimmerman <pauldzim@gmail.com>
> Subject: Re: [PATCH v4 1/7] usb/hcd-xhci: Make dma read/writes hooks pci
> free
> 
> On 9/8/20 12:30 PM, Philippe Mathieu-Daudé wrote:
> >>> On 8/28/20 9:19 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(-)
> >>>>
> > [...]
> >>>> --- 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"
> >>>
> >>> AddressSpace is forward-declared in "qemu/typedefs.h", so no need to
> >>> include it here (yes in the sources including hcd-xhci.h).
> >> [Sai Pavan Boddu] Yes you are right!, but without this " dma_addr_t " is
> undefined.
> >> At this point of the patch, hcd-xhci.h is compiled along with pci.h which
> would provide 'dma_addr_t', but when we strip the pci wrapper around hcd-
> xhci we would miss it. Let me know, if its good to add later in the patch series
> when the split happens.
> >
> > OK :( I'd prefer to only include "sysemu/dma.h" in hw/usb/hcd-xhci.c.
> >
> > Cc'ing David who added dma_addr_t in commit e5332e6334f
> > ("iommu: Introduce IOMMU emulation infrastructure"), it might be
> > simpler to move its declaration to "qemu/typedefs.h"
> > too.
> 
> Btw this type should be poisoned for user-mode.
[Sai Pavan Boddu] Thanks, I would try to define them in typedefs.h. 
BTW I'm not clear about poisoning then in user-mode. Do you mean hide these defines for user-mode ?

Regards
Sai Pavan
> 
> >
> > So no change needed for this patch.
> >
> >>
> >> Regards,
> >> Sai Pavan
> >>>
> >>> With that fixed:
> >>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>>
> >>>> +
> >>>>  #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;
> >>>>
> >>
> >
Philippe Mathieu-Daudé Sept. 8, 2020, 12:26 p.m. UTC | #6
On 9/8/20 1:35 PM, Sai Pavan Boddu wrote:
> Hi Philippe,
> 
>> -----Original Message-----
>> From: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Sent: Tuesday, September 8, 2020 4:05 PM
>> To: Sai Pavan Boddu <saipava@xilinx.com>; Peter Maydell
>> <peter.maydell@linaro.org>; Markus Armbruster <armbru@redhat.com>;
>> 'Marc-André Lureau' <marcandre.lureau@redhat.com>; Paolo Bonzini
>> <pbonzini@redhat.com>; Gerd Hoffmann <kraxel@redhat.com>; Edgar
>> Iglesias <edgari@xilinx.com>; Francisco Eduardo Iglesias
>> <figlesia@xilinx.com>; David Gibson <david@gibson.dropbear.id.au>
>> Cc: qemu-devel@nongnu.org; Alistair Francis <alistair.francis@wdc.com>;
>> Eduardo Habkost <ehabkost@redhat.com>; Ying Fang
>> <fangying1@huawei.com>; Vikram Garhwal <fnuv@xilinx.com>; Paul
>> Zimmerman <pauldzim@gmail.com>
>> Subject: Re: [PATCH v4 1/7] usb/hcd-xhci: Make dma read/writes hooks pci
>> free
>>
>> On 9/8/20 12:30 PM, Philippe Mathieu-Daudé wrote:
>>>>> On 8/28/20 9:19 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(-)
>>>>>>
>>> [...]
>>>>>> --- 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"
>>>>>
>>>>> AddressSpace is forward-declared in "qemu/typedefs.h", so no need to
>>>>> include it here (yes in the sources including hcd-xhci.h).
>>>> [Sai Pavan Boddu] Yes you are right!, but without this " dma_addr_t " is
>> undefined.
>>>> At this point of the patch, hcd-xhci.h is compiled along with pci.h which
>> would provide 'dma_addr_t', but when we strip the pci wrapper around hcd-
>> xhci we would miss it. Let me know, if its good to add later in the patch series
>> when the split happens.
>>>
>>> OK :( I'd prefer to only include "sysemu/dma.h" in hw/usb/hcd-xhci.c.
>>>
>>> Cc'ing David who added dma_addr_t in commit e5332e6334f
>>> ("iommu: Introduce IOMMU emulation infrastructure"), it might be
>>> simpler to move its declaration to "qemu/typedefs.h"
>>> too.
>>
>> Btw this type should be poisoned for user-mode.
> [Sai Pavan Boddu] Thanks, I would try to define them in typedefs.h. 
> BTW I'm not clear about poisoning then in user-mode. Do you mean hide these defines for user-mode ?

This doesn't "hide" but poison them: if you try to use them the
compiler (cpp actually) will raise an error.

This was not a direct comment for you, more something to discuss
out of the scope of your series ;) No need to try to declare it
poisoned now.

> 
> Regards
> Sai Pavan
>>
>>>
>>> So no change needed for this patch.
>>>
>>>>
>>>> Regards,
>>>> Sai Pavan
>>>>>
>>>>> With that fixed:
>>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>>>>
>>>>>> +
>>>>>>  #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;
>>>>>>
>>>>
>>>
>
Sai Pavan Boddu Sept. 9, 2020, 10:50 a.m. UTC | #7
Hi Philippe,

> -----Original Message-----
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> Sent: Tuesday, September 8, 2020 5:57 PM
> To: Sai Pavan Boddu <saipava@xilinx.com>; Peter Maydell
> <peter.maydell@linaro.org>; Markus Armbruster <armbru@redhat.com>;
> 'Marc-André Lureau' <marcandre.lureau@redhat.com>; Paolo Bonzini
> <pbonzini@redhat.com>; Gerd Hoffmann <kraxel@redhat.com>; Edgar
> Iglesias <edgari@xilinx.com>; Francisco Eduardo Iglesias
> <figlesia@xilinx.com>; David Gibson <david@gibson.dropbear.id.au>
> Cc: qemu-devel@nongnu.org; Alistair Francis <alistair.francis@wdc.com>;
> Eduardo Habkost <ehabkost@redhat.com>; Ying Fang
> <fangying1@huawei.com>; Vikram Garhwal <fnuv@xilinx.com>; Paul
> Zimmerman <pauldzim@gmail.com>
> Subject: Re: [PATCH v4 1/7] usb/hcd-xhci: Make dma read/writes hooks pci
> free
> 
> On 9/8/20 1:35 PM, Sai Pavan Boddu wrote:
> > Hi Philippe,
> >
> >> -----Original Message-----
> >> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> >> Sent: Tuesday, September 8, 2020 4:05 PM
> >> To: Sai Pavan Boddu <saipava@xilinx.com>; Peter Maydell
> >> <peter.maydell@linaro.org>; Markus Armbruster <armbru@redhat.com>;
> >> 'Marc-André Lureau' <marcandre.lureau@redhat.com>; Paolo Bonzini
> >> <pbonzini@redhat.com>; Gerd Hoffmann <kraxel@redhat.com>; Edgar
> >> Iglesias <edgari@xilinx.com>; Francisco Eduardo Iglesias
> >> <figlesia@xilinx.com>; David Gibson <david@gibson.dropbear.id.au>
> >> Cc: qemu-devel@nongnu.org; Alistair Francis
> >> <alistair.francis@wdc.com>; Eduardo Habkost <ehabkost@redhat.com>;
> >> Ying Fang <fangying1@huawei.com>; Vikram Garhwal <fnuv@xilinx.com>;
> >> Paul Zimmerman <pauldzim@gmail.com>
> >> Subject: Re: [PATCH v4 1/7] usb/hcd-xhci: Make dma read/writes hooks
> >> pci free
> >>
> >> On 9/8/20 12:30 PM, Philippe Mathieu-Daudé wrote:
> >>>>> On 8/28/20 9:19 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(-)
> >>>>>>
> >>> [...]
> >>>>>> --- 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"
> >>>>>
> >>>>> AddressSpace is forward-declared in "qemu/typedefs.h", so no need
> >>>>> to include it here (yes in the sources including hcd-xhci.h).
> >>>> [Sai Pavan Boddu] Yes you are right!, but without this " dma_addr_t
> >>>> " is
> >> undefined.
> >>>> At this point of the patch, hcd-xhci.h is compiled along with pci.h
> >>>> which
> >> would provide 'dma_addr_t', but when we strip the pci wrapper around
> >> hcd- xhci we would miss it. Let me know, if its good to add later in
> >> the patch series when the split happens.
> >>>
> >>> OK :( I'd prefer to only include "sysemu/dma.h" in hw/usb/hcd-xhci.c.
> >>>
> >>> Cc'ing David who added dma_addr_t in commit e5332e6334f
> >>> ("iommu: Introduce IOMMU emulation infrastructure"), it might be
> >>> simpler to move its declaration to "qemu/typedefs.h"
> >>> too.
> >>
> >> Btw this type should be poisoned for user-mode.
> > [Sai Pavan Boddu] Thanks, I would try to define them in typedefs.h.
> > BTW I'm not clear about poisoning then in user-mode. Do you mean hide
> these defines for user-mode ?
> 
> This doesn't "hide" but poison them: if you try to use them the compiler (cpp
> actually) will raise an error.
> 
> This was not a direct comment for you, more something to discuss out of the
> scope of your series ;) No need to try to declare it poisoned now.
[Sai Pavan Boddu] One more thing, which came up during implementation is dma_memory_read/ dma_memory_write functions which used in this patch would require that header.

Regards,
Sai Pavan
> 
> >
> > Regards
> > Sai Pavan
> >>
> >>>
> >>> So no change needed for this patch.
> >>>
> >>>>
> >>>> Regards,
> >>>> Sai Pavan
> >>>>>
> >>>>> With that fixed:
> >>>>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>>>>
> >>>>>> +
> >>>>>>  #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;
> >>>>>>
> >>>>
> >>>
> >
David Gibson Sept. 10, 2020, 11:41 p.m. UTC | #8
On Tue, Sep 08, 2020 at 12:30:17PM +0200, Philippe Mathieu-Daudé wrote:
> >> On 8/28/20 9:19 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(-)
> >>>
> [...]
> >>> --- 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"
> >>
> >> AddressSpace is forward-declared in "qemu/typedefs.h", so no need to
> >> include it here (yes in the sources including hcd-xhci.h).
> > [Sai Pavan Boddu] Yes you are right!, but without this " dma_addr_t " is undefined. 
> > At this point of the patch, hcd-xhci.h is compiled along with pci.h which would provide 'dma_addr_t', but when we strip the pci wrapper around hcd-xhci we would miss it. Let me know, if its good to add later in the patch series when the split happens.
> 
> OK :( I'd prefer to only include "sysemu/dma.h" in hw/usb/hcd-xhci.c.
> 
> Cc'ing David who added dma_addr_t in commit e5332e6334f
> ("iommu: Introduce IOMMU emulation infrastructure"), it
> might be simpler to move its declaration to "qemu/typedefs.h"
> too.

Moving it to qemu/typedefs.h seems fine to me fwiw.

> 
> So no change needed for this patch.
> 
> > 
> > Regards,
> > Sai Pavan
> >>
> >> With that fixed:
> >> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>
> >>> +
> >>>  #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;
> >>>
> > 
>
David Gibson Sept. 10, 2020, 11:42 p.m. UTC | #9
On Tue, Sep 08, 2020 at 12:34:59PM +0200, Philippe Mathieu-Daudé wrote:
> On 9/8/20 12:30 PM, Philippe Mathieu-Daudé wrote:
> >>> On 8/28/20 9:19 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(-)
> >>>>
> > [...]
> >>>> --- 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"
> >>>
> >>> AddressSpace is forward-declared in "qemu/typedefs.h", so no need to
> >>> include it here (yes in the sources including hcd-xhci.h).
> >> [Sai Pavan Boddu] Yes you are right!, but without this " dma_addr_t " is undefined. 
> >> At this point of the patch, hcd-xhci.h is compiled along with pci.h which would provide 'dma_addr_t', but when we strip the pci wrapper around hcd-xhci we would miss it. Let me know, if its good to add later in the patch series when the split happens.
> > 
> > OK :( I'd prefer to only include "sysemu/dma.h" in hw/usb/hcd-xhci.c.
> > 
> > Cc'ing David who added dma_addr_t in commit e5332e6334f
> > ("iommu: Introduce IOMMU emulation infrastructure"), it
> > might be simpler to move its declaration to "qemu/typedefs.h"
> > too.
> 
> Btw this type should be poisoned for user-mode.

Yes, yes it should.


> 
> > 
> > So no change needed for this patch.
> > 
> >>
> >> Regards,
> >> Sai Pavan
> >>>
> >>> With that fixed:
> >>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> >>>
> >>>> +
> >>>>  #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;
> >>>>
> >>
> > 
>
diff mbox series

Patch

diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 67a18fe..2590f7a 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 946af51..edbd926 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;