* [Qemu-devel] [0/8] RFC: A second batch of preliminaries towards guest visible IOMMUS
@ 2012-02-24 3:27 David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 1/8] Use DMADirection type for dma_bdrv_io David Gibson
` (7 more replies)
0 siblings, 8 replies; 18+ messages in thread
From: David Gibson @ 2012-02-24 3:27 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
Before qemu 1.0 there was some discussion, and several patch series
proposed for adding support for guest visible IOMMU emulation to qemu.
No agreement was reached on how to do this, but a patch series did go
in preparation - a conversion of most existing PCI devices to use
helper functions for their DMA which could be converted to do IOMMU
translation in one place.
This patch series is a further set of preliminaries, generalizing
these wrapper functions to support non-PCI functions, and using this
to convert several more drivers.
It also introduces the concept of a DMAContext, describing a DMA
address space. With this series the only one supported is NULL,
meaning "no translation". However, this provides a place that we can
put the necessary callback information to support IOMMU translation.
If this series doesn't attract too many screams, I'll fold in feedback
and repost for inclusion.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 1/8] Use DMADirection type for dma_bdrv_io
2012-02-24 3:27 [Qemu-devel] [0/8] RFC: A second batch of preliminaries towards guest visible IOMMUS David Gibson
@ 2012-02-24 3:27 ` David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 2/8] Better support for dma_addr_t variables David Gibson
` (6 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2012-02-24 3:27 UTC (permalink / raw)
To: anthony; +Cc: Kevin Wolf, qemu-devel, David Gibson
Currently dma_bdrv_io() takes a 'to_dev' boolean parameter to
determine the direction of DMA it is emulating. We already have a
DMADirection enum designed specifically to encode DMA directions.
This patch uses it for dma_bdrv_io() as well.
Cc: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
dma-helpers.c | 20 ++++++++++++--------
dma.h | 2 +-
hw/ide/core.c | 3 ++-
hw/ide/macio.c | 3 ++-
4 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/dma-helpers.c b/dma-helpers.c
index f08cdb5..57eb22c 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -42,7 +42,7 @@ typedef struct {
BlockDriverAIOCB *acb;
QEMUSGList *sg;
uint64_t sector_num;
- bool to_dev;
+ DMADirection dir;
bool in_cancel;
int sg_cur_index;
dma_addr_t sg_cur_byte;
@@ -76,7 +76,8 @@ static void dma_bdrv_unmap(DMAAIOCB *dbs)
for (i = 0; i < dbs->iov.niov; ++i) {
cpu_physical_memory_unmap(dbs->iov.iov[i].iov_base,
- dbs->iov.iov[i].iov_len, !dbs->to_dev,
+ dbs->iov.iov[i].iov_len,
+ dbs->dir != DMA_DIRECTION_TO_DEVICE,
dbs->iov.iov[i].iov_len);
}
qemu_iovec_reset(&dbs->iov);
@@ -123,7 +124,8 @@ static void dma_bdrv_cb(void *opaque, int ret)
while (dbs->sg_cur_index < dbs->sg->nsg) {
cur_addr = dbs->sg->sg[dbs->sg_cur_index].base + dbs->sg_cur_byte;
cur_len = dbs->sg->sg[dbs->sg_cur_index].len - dbs->sg_cur_byte;
- mem = cpu_physical_memory_map(cur_addr, &cur_len, !dbs->to_dev);
+ mem = cpu_physical_memory_map(cur_addr, &cur_len,
+ dbs->dir != DMA_DIRECTION_TO_DEVICE);
if (!mem)
break;
qemu_iovec_add(&dbs->iov, mem, cur_len);
@@ -170,11 +172,11 @@ static AIOPool dma_aio_pool = {
BlockDriverAIOCB *dma_bdrv_io(
BlockDriverState *bs, QEMUSGList *sg, uint64_t sector_num,
DMAIOFunc *io_func, BlockDriverCompletionFunc *cb,
- void *opaque, bool to_dev)
+ void *opaque, DMADirection dir)
{
DMAAIOCB *dbs = qemu_aio_get(&dma_aio_pool, bs, cb, opaque);
- trace_dma_bdrv_io(dbs, bs, sector_num, to_dev);
+ trace_dma_bdrv_io(dbs, bs, sector_num, (dir == DMA_DIRECTION_TO_DEVICE));
dbs->acb = NULL;
dbs->bs = bs;
@@ -182,7 +184,7 @@ BlockDriverAIOCB *dma_bdrv_io(
dbs->sector_num = sector_num;
dbs->sg_cur_index = 0;
dbs->sg_cur_byte = 0;
- dbs->to_dev = to_dev;
+ dbs->dir = dir;
dbs->io_func = io_func;
dbs->bh = NULL;
qemu_iovec_init(&dbs->iov, sg->nsg);
@@ -195,12 +197,14 @@ BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs,
QEMUSGList *sg, uint64_t sector,
void (*cb)(void *opaque, int ret), void *opaque)
{
- return dma_bdrv_io(bs, sg, sector, bdrv_aio_readv, cb, opaque, false);
+ return dma_bdrv_io(bs, sg, sector, bdrv_aio_readv, cb, opaque,
+ DMA_DIRECTION_FROM_DEVICE);
}
BlockDriverAIOCB *dma_bdrv_write(BlockDriverState *bs,
QEMUSGList *sg, uint64_t sector,
void (*cb)(void *opaque, int ret), void *opaque)
{
- return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque, true);
+ return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque,
+ DMA_DIRECTION_TO_DEVICE);
}
diff --git a/dma.h b/dma.h
index a13209d..0847b42 100644
--- a/dma.h
+++ b/dma.h
@@ -51,7 +51,7 @@ typedef BlockDriverAIOCB *DMAIOFunc(BlockDriverState *bs, int64_t sector_num,
BlockDriverAIOCB *dma_bdrv_io(BlockDriverState *bs,
QEMUSGList *sg, uint64_t sector_num,
DMAIOFunc *io_func, BlockDriverCompletionFunc *cb,
- void *opaque, bool to_dev);
+ void *opaque, DMADirection dir);
BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs,
QEMUSGList *sg, uint64_t sector,
BlockDriverCompletionFunc *cb, void *opaque);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 56b219b..e173117 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -604,7 +604,8 @@ void ide_dma_cb(void *opaque, int ret)
break;
case IDE_DMA_TRIM:
s->bus->dma->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
- ide_issue_trim, ide_dma_cb, s, true);
+ ide_issue_trim, ide_dma_cb, s,
+ DMA_DIRECTION_TO_DEVICE);
break;
}
return;
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index abbc41b..edcf885 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -149,7 +149,8 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
break;
case IDE_DMA_TRIM:
m->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
- ide_issue_trim, pmac_ide_transfer_cb, s, true);
+ ide_issue_trim, pmac_ide_transfer_cb, s,
+ DMA_DIRECTION_TO_DEVICE);
break;
}
return;
--
1.7.9
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 2/8] Better support for dma_addr_t variables
2012-02-24 3:27 [Qemu-devel] [0/8] RFC: A second batch of preliminaries towards guest visible IOMMUS David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 1/8] Use DMADirection type for dma_bdrv_io David Gibson
@ 2012-02-24 3:27 ` David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 3/8] usb-xhci: Use PCI DMA helper functions David Gibson
` (5 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2012-02-24 3:27 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel, David Gibson
A while back, we introduced the dma_addr_t type, which is supposed to
be used for bus visible memory addresses. At present, this is an
alias for target_phys_addr_t, but this will change when we eventually
add support for guest visible IOMMUs.
There are some instances of target_phys_addr_t in the code now which
should really be dma_addr_t, but can't be trivially converted due to
missing features which this patch corrects.
* We add DMA_ADDR_BITS analagous to TARGET_PHYS_ADDR_BITS. This is
important where we need to make a compile-time (#if) based on the
size of dma_addr_t.
* We add a new helper macro to create device properties which take a
dma_addr_t. This lets us correctly convert code which cuurrently
has DEFINE_PROP_TADDR() for variables which should be dma_addr_t
instead of target_phys_addr_t.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
Makefile.objs | 2 +-
dma.h | 1 +
hw/qdev-dma.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/qdev-dma.h | 7 +++++
4 files changed, 85 insertions(+), 1 deletions(-)
create mode 100644 hw/qdev-dma.c
create mode 100644 hw/qdev-dma.h
diff --git a/Makefile.objs b/Makefile.objs
index 67ee3df..1d643bb 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -287,7 +287,7 @@ hw-obj-$(CONFIG_LSI_SCSI_PCI) += lsi53c895a.o
hw-obj-$(CONFIG_ESP) += esp.o
hw-obj-y += dma-helpers.o sysbus.o isa-bus.o
-hw-obj-y += qdev-addr.o
+hw-obj-y += qdev-addr.o qdev-dma.o
# VGA
hw-obj-$(CONFIG_VGA_PCI) += vga-pci.o
diff --git a/dma.h b/dma.h
index 0847b42..79be131 100644
--- a/dma.h
+++ b/dma.h
@@ -20,6 +20,7 @@ typedef struct ScatterGatherEntry ScatterGatherEntry;
#if defined(TARGET_PHYS_ADDR_BITS)
typedef target_phys_addr_t dma_addr_t;
+#define DMA_ADDR_BITS TARGET_PHYS_ADDR_BITS
#define DMA_ADDR_FMT TARGET_FMT_plx
typedef enum {
diff --git a/hw/qdev-dma.c b/hw/qdev-dma.c
new file mode 100644
index 0000000..63c9768
--- /dev/null
+++ b/hw/qdev-dma.c
@@ -0,0 +1,76 @@
+#include "qdev.h"
+#include "qdev-dma.h"
+#include "dma.h"
+
+/* --- target physical address --- */
+
+static int parse_dmaaddr(DeviceState *dev, Property *prop, const char *str)
+{
+ dma_addr_t *ptr = qdev_get_prop_ptr(dev, prop);
+
+ *ptr = strtoull(str, NULL, 16);
+ return 0;
+}
+
+static int print_dmaaddr(DeviceState *dev, Property *prop, char *dest, size_t len)
+{
+ dma_addr_t *ptr = qdev_get_prop_ptr(dev, prop);
+ return snprintf(dest, len, "0x" DMA_ADDR_FMT, *ptr);
+}
+
+static void get_dmaaddr(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ DeviceState *dev = DEVICE(obj);
+ Property *prop = opaque;
+ dma_addr_t *ptr = qdev_get_prop_ptr(dev, prop);
+ int64_t value;
+
+ value = *ptr;
+ visit_type_int(v, &value, name, errp);
+}
+
+static void set_dmaaddr(Object *obj, Visitor *v, void *opaque,
+ const char *name, Error **errp)
+{
+ DeviceState *dev = DEVICE(obj);
+ Property *prop = opaque;
+ dma_addr_t *ptr = qdev_get_prop_ptr(dev, prop);
+ Error *local_err = NULL;
+ int64_t value;
+
+ if (dev->state != DEV_STATE_CREATED) {
+ error_set(errp, QERR_PERMISSION_DENIED);
+ return;
+ }
+
+ visit_type_int(v, &value, name, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
+ }
+ if ((uint64_t)value <= (uint64_t) ~(dma_addr_t)0) {
+ *ptr = value;
+ } else {
+ error_set(errp, QERR_PROPERTY_VALUE_OUT_OF_RANGE,
+ dev->id?:"", name, value, (uint64_t) 0,
+ (uint64_t) ~(dma_addr_t)0);
+ }
+}
+
+
+PropertyInfo qdev_prop_dmaaddr = {
+ .name = "dmaaddr",
+ .parse = parse_dmaaddr,
+ .print = print_dmaaddr,
+ .get = get_dmaaddr,
+ .set = set_dmaaddr,
+};
+
+void qdev_prop_set_dmaaddr(DeviceState *dev, const char *name, dma_addr_t value)
+{
+ Error *errp = NULL;
+ object_property_set_int(OBJECT(dev), value, name, &errp);
+ assert(!errp);
+
+}
diff --git a/hw/qdev-dma.h b/hw/qdev-dma.h
new file mode 100644
index 0000000..1390a8d
--- /dev/null
+++ b/hw/qdev-dma.h
@@ -0,0 +1,7 @@
+#include "dma.h"
+
+#define DEFINE_PROP_DMAADDR(_n, _s, _f, _d) \
+ DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_dmaaddr, dma_addr_t)
+
+extern PropertyInfo qdev_prop_dmaaddr;
+void qdev_prop_set_dmaaddr(DeviceState *dev, const char *name, dma_addr_t value);
--
1.7.9
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 3/8] usb-xhci: Use PCI DMA helper functions
2012-02-24 3:27 [Qemu-devel] [0/8] RFC: A second batch of preliminaries towards guest visible IOMMUS David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 1/8] Use DMADirection type for dma_bdrv_io David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 2/8] Better support for dma_addr_t variables David Gibson
@ 2012-02-24 3:27 ` David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 4/8] Add universal " David Gibson
` (4 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2012-02-24 3:27 UTC (permalink / raw)
To: anthony; +Cc: Gerd Hoffman, qemu-devel, David Gibson
Shortly before 1.0, we added helper functions / wrappers for doing PCI DMA
from individual devices. This makes what's going on clearer and means that
when we add IOMMU support somewhere in the future, only the general PCI
code will have to change, not every device that uses PCI DMA.
However, usb-xhci is not using these wrappers, despite being a PCI only
device. This patch remedies the situation, using the pci dma functions
instead of direct calls to cpu_physical_memory_{read,write}(). Likewise
address parameters for DMA are changed to dma_addr_t instead of
target_phys_addr_t.
Cc: Gerd Hoffman <kraxel@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/usb-xhci.c | 175 +++++++++++++++++++++++++++++----------------------------
1 files changed, 89 insertions(+), 86 deletions(-)
diff --git a/hw/usb-xhci.c b/hw/usb-xhci.c
index 008b0b5..bef457b 100644
--- a/hw/usb-xhci.c
+++ b/hw/usb-xhci.c
@@ -22,7 +22,6 @@
#include "qemu-timer.h"
#include "usb.h"
#include "pci.h"
-#include "qdev-addr.h"
#include "msi.h"
//#define DEBUG_XHCI
@@ -140,7 +139,7 @@ typedef struct XHCITRB {
uint64_t parameter;
uint32_t status;
uint32_t control;
- target_phys_addr_t addr;
+ dma_addr_t addr;
bool ccs;
} XHCITRB;
@@ -291,8 +290,8 @@ typedef enum EPType {
} EPType;
typedef struct XHCIRing {
- target_phys_addr_t base;
- target_phys_addr_t dequeue;
+ dma_addr_t base;
+ dma_addr_t dequeue;
bool ccs;
} XHCIRing;
@@ -345,7 +344,7 @@ typedef struct XHCIEPContext {
unsigned int next_bg;
XHCITransfer bg_transfers[BG_XFERS];
EPType type;
- target_phys_addr_t pctx;
+ dma_addr_t pctx;
unsigned int max_psize;
bool has_bg;
uint32_t state;
@@ -353,7 +352,7 @@ typedef struct XHCIEPContext {
typedef struct XHCISlot {
bool enabled;
- target_phys_addr_t ctx;
+ dma_addr_t ctx;
unsigned int port;
unsigned int devaddr;
XHCIEPContext * eps[31];
@@ -402,7 +401,7 @@ struct XHCIState {
uint32_t erdp_low;
uint32_t erdp_high;
- target_phys_addr_t er_start;
+ dma_addr_t er_start;
uint32_t er_size;
bool er_pcs;
unsigned int er_ep_idx;
@@ -479,18 +478,18 @@ static const char *trb_name(XHCITRB *trb)
static void xhci_kick_ep(XHCIState *xhci, unsigned int slotid,
unsigned int epid);
-static inline target_phys_addr_t xhci_addr64(uint32_t low, uint32_t high)
+static inline dma_addr_t xhci_addr64(uint32_t low, uint32_t high)
{
-#if TARGET_PHYS_ADDR_BITS > 32
- return low | ((target_phys_addr_t)high << 32);
+#if DMA_ADDR_BITS > 32
+ return low | ((dma_addr_t)high << 32);
#else
return low;
#endif
}
-static inline target_phys_addr_t xhci_mask64(uint64_t addr)
+static inline dma_addr_t xhci_mask64(uint64_t addr)
{
-#if TARGET_PHYS_ADDR_BITS > 32
+#if DMA_ADDR_BITS > 32
return addr;
#else
return addr & 0xffffffff;
@@ -532,7 +531,7 @@ static void xhci_die(XHCIState *xhci)
static void xhci_write_event(XHCIState *xhci, XHCIEvent *event)
{
XHCITRB ev_trb;
- target_phys_addr_t addr;
+ dma_addr_t addr;
ev_trb.parameter = cpu_to_le64(event->ptr);
ev_trb.status = cpu_to_le32(event->length | (event->ccode << 24));
@@ -548,7 +547,7 @@ static void xhci_write_event(XHCIState *xhci, XHCIEvent *event)
trb_name(&ev_trb));
addr = xhci->er_start + TRB_SIZE*xhci->er_ep_idx;
- cpu_physical_memory_write(addr, (uint8_t *) &ev_trb, TRB_SIZE);
+ pci_dma_write(&xhci->pci_dev, addr, (uint8_t *) &ev_trb, TRB_SIZE);
xhci->er_ep_idx++;
if (xhci->er_ep_idx >= xhci->er_size) {
@@ -559,7 +558,7 @@ static void xhci_write_event(XHCIState *xhci, XHCIEvent *event)
static void xhci_events_update(XHCIState *xhci)
{
- target_phys_addr_t erdp;
+ dma_addr_t erdp;
unsigned int dp_idx;
bool do_irq = 0;
@@ -570,8 +569,8 @@ static void xhci_events_update(XHCIState *xhci)
erdp = xhci_addr64(xhci->erdp_low, xhci->erdp_high);
if (erdp < xhci->er_start ||
erdp >= (xhci->er_start + TRB_SIZE*xhci->er_size)) {
- fprintf(stderr, "xhci: ERDP out of bounds: "TARGET_FMT_plx"\n", erdp);
- fprintf(stderr, "xhci: ER at "TARGET_FMT_plx" len %d\n",
+ fprintf(stderr, "xhci: ERDP out of bounds: "DMA_ADDR_FMT"\n", erdp);
+ fprintf(stderr, "xhci: ER at "DMA_ADDR_FMT" len %d\n",
xhci->er_start, xhci->er_size);
xhci_die(xhci);
return;
@@ -630,7 +629,7 @@ static void xhci_events_update(XHCIState *xhci)
static void xhci_event(XHCIState *xhci, XHCIEvent *event)
{
- target_phys_addr_t erdp;
+ dma_addr_t erdp;
unsigned int dp_idx;
if (xhci->er_full) {
@@ -649,8 +648,8 @@ static void xhci_event(XHCIState *xhci, XHCIEvent *event)
erdp = xhci_addr64(xhci->erdp_low, xhci->erdp_high);
if (erdp < xhci->er_start ||
erdp >= (xhci->er_start + TRB_SIZE*xhci->er_size)) {
- fprintf(stderr, "xhci: ERDP out of bounds: "TARGET_FMT_plx"\n", erdp);
- fprintf(stderr, "xhci: ER at "TARGET_FMT_plx" len %d\n",
+ fprintf(stderr, "xhci: ERDP out of bounds: "DMA_ADDR_FMT"\n", erdp);
+ fprintf(stderr, "xhci: ER at "DMA_ADDR_FMT" len %d\n",
xhci->er_start, xhci->er_size);
xhci_die(xhci);
return;
@@ -686,7 +685,7 @@ static void xhci_event(XHCIState *xhci, XHCIEvent *event)
}
static void xhci_ring_init(XHCIState *xhci, XHCIRing *ring,
- target_phys_addr_t base)
+ dma_addr_t base)
{
ring->base = base;
ring->dequeue = base;
@@ -694,18 +693,18 @@ static void xhci_ring_init(XHCIState *xhci, XHCIRing *ring,
}
static TRBType xhci_ring_fetch(XHCIState *xhci, XHCIRing *ring, XHCITRB *trb,
- target_phys_addr_t *addr)
+ dma_addr_t *addr)
{
while (1) {
TRBType type;
- cpu_physical_memory_read(ring->dequeue, (uint8_t *) trb, TRB_SIZE);
+ pci_dma_read(&xhci->pci_dev, ring->dequeue, (uint8_t *) trb, TRB_SIZE);
trb->addr = ring->dequeue;
trb->ccs = ring->ccs;
le64_to_cpus(&trb->parameter);
le32_to_cpus(&trb->status);
le32_to_cpus(&trb->control);
- DPRINTF("xhci: TRB fetched [" TARGET_FMT_plx "]: "
+ DPRINTF("xhci: TRB fetched [" DMA_ADDR_FMT "]: "
"%016" PRIx64 " %08x %08x %s\n",
ring->dequeue, trb->parameter, trb->status, trb->control,
trb_name(trb));
@@ -735,19 +734,19 @@ static int xhci_ring_chain_length(XHCIState *xhci, const XHCIRing *ring)
{
XHCITRB trb;
int length = 0;
- target_phys_addr_t dequeue = ring->dequeue;
+ dma_addr_t dequeue = ring->dequeue;
bool ccs = ring->ccs;
/* hack to bundle together the two/three TDs that make a setup transfer */
bool control_td_set = 0;
while (1) {
TRBType type;
- cpu_physical_memory_read(dequeue, (uint8_t *) &trb, TRB_SIZE);
+ pci_dma_read(&xhci->pci_dev, dequeue, (uint8_t *) &trb, TRB_SIZE);
le64_to_cpus(&trb.parameter);
le32_to_cpus(&trb.status);
le32_to_cpus(&trb.control);
- DPRINTF("xhci: TRB peeked [" TARGET_FMT_plx "]: "
+ DPRINTF("xhci: TRB peeked [" DMA_ADDR_FMT "]: "
"%016" PRIx64 " %08x %08x\n",
dequeue, trb.parameter, trb.status, trb.control);
@@ -790,8 +789,8 @@ static void xhci_er_reset(XHCIState *xhci)
xhci_die(xhci);
return;
}
- target_phys_addr_t erstba = xhci_addr64(xhci->erstba_low, xhci->erstba_high);
- cpu_physical_memory_read(erstba, (uint8_t *) &seg, sizeof(seg));
+ dma_addr_t erstba = xhci_addr64(xhci->erstba_low, xhci->erstba_high);
+ pci_dma_read(&xhci->pci_dev, erstba, (uint8_t *) &seg, sizeof(seg));
le32_to_cpus(&seg.addr_low);
le32_to_cpus(&seg.addr_high);
le32_to_cpus(&seg.size);
@@ -807,7 +806,7 @@ static void xhci_er_reset(XHCIState *xhci)
xhci->er_pcs = 1;
xhci->er_full = 0;
- DPRINTF("xhci: event ring:" TARGET_FMT_plx " [%d]\n",
+ DPRINTF("xhci: event ring:" DMA_ADDR_FMT " [%d]\n",
xhci->er_start, xhci->er_size);
}
@@ -833,24 +832,24 @@ static void xhci_set_ep_state(XHCIState *xhci, XHCIEPContext *epctx,
return;
}
- cpu_physical_memory_read(epctx->pctx, (uint8_t *) ctx, sizeof(ctx));
+ pci_dma_read(&xhci->pci_dev, epctx->pctx, (uint8_t *) ctx, sizeof(ctx));
ctx[0] &= ~EP_STATE_MASK;
ctx[0] |= state;
ctx[2] = epctx->ring.dequeue | epctx->ring.ccs;
ctx[3] = (epctx->ring.dequeue >> 16) >> 16;
- DPRINTF("xhci: set epctx: " TARGET_FMT_plx " state=%d dequeue=%08x%08x\n",
+ DPRINTF("xhci: set epctx: " DMA_ADDR_FMT " state=%d dequeue=%08x%08x\n",
epctx->pctx, state, ctx[3], ctx[2]);
- cpu_physical_memory_write(epctx->pctx, (uint8_t *) ctx, sizeof(ctx));
+ pci_dma_write(&xhci->pci_dev, epctx->pctx, (uint8_t *) ctx, sizeof(ctx));
epctx->state = state;
}
static TRBCCode xhci_enable_ep(XHCIState *xhci, unsigned int slotid,
- unsigned int epid, target_phys_addr_t pctx,
+ unsigned int epid, dma_addr_t pctx,
uint32_t *ctx)
{
XHCISlot *slot;
XHCIEPContext *epctx;
- target_phys_addr_t dequeue;
+ dma_addr_t dequeue;
int i;
assert(slotid >= 1 && slotid <= MAXSLOTS);
@@ -1087,7 +1086,7 @@ static TRBCCode xhci_set_ep_dequeue(XHCIState *xhci, unsigned int slotid,
{
XHCISlot *slot;
XHCIEPContext *epctx;
- target_phys_addr_t dequeue;
+ dma_addr_t dequeue;
assert(slotid >= 1 && slotid <= MAXSLOTS);
@@ -1142,7 +1141,7 @@ static int xhci_xfer_data(XHCITransfer *xfer, uint8_t *data,
for (i = 0; i < xfer->trb_count; i++) {
XHCITRB *trb = &xfer->trbs[i];
- target_phys_addr_t addr;
+ dma_addr_t addr;
unsigned int chunk = 0;
switch (TRB_TYPE(*trb)) {
@@ -1173,11 +1172,11 @@ static int xhci_xfer_data(XHCITransfer *xfer, uint8_t *data,
memcpy(data, &idata, chunk);
} else {
DPRINTF("xhci_xfer_data: r/w(%d) %d bytes at "
- TARGET_FMT_plx "\n", in_xfer, chunk, addr);
+ DMA_ADDR_FMT "\n", in_xfer, chunk, addr);
if (in_xfer) {
- cpu_physical_memory_write(addr, data, chunk);
+ pci_dma_write(&xhci->pci_dev, addr, data, chunk);
} else {
- cpu_physical_memory_read(addr, data, chunk);
+ pci_dma_read(&xhci->pci_dev, addr, data, chunk);
}
#ifdef DEBUG_DATA
unsigned int count = chunk;
@@ -1240,7 +1239,7 @@ static void xhci_stall_ep(XHCITransfer *xfer)
epctx->ring.ccs = xfer->trbs[0].ccs;
xhci_set_ep_state(xhci, epctx, EP_HALTED);
DPRINTF("xhci: stalled slot %d ep %d\n", xfer->slotid, xfer->epid);
- DPRINTF("xhci: will continue at "TARGET_FMT_plx"\n", epctx->ring.dequeue);
+ DPRINTF("xhci: will continue at "DMA_ADDR_FMT"\n", epctx->ring.dequeue);
}
static int xhci_submit(XHCIState *xhci, XHCITransfer *xfer,
@@ -1811,7 +1810,7 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid,
{
XHCISlot *slot;
USBDevice *dev;
- target_phys_addr_t ictx, octx, dcbaap;
+ dma_addr_t ictx, octx, dcbaap;
uint64_t poctx;
uint32_t ictl_ctx[2];
uint32_t slot_ctx[4];
@@ -1824,15 +1823,15 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid,
DPRINTF("xhci_address_slot(%d)\n", slotid);
dcbaap = xhci_addr64(xhci->dcbaap_low, xhci->dcbaap_high);
- cpu_physical_memory_read(dcbaap + 8*slotid,
- (uint8_t *) &poctx, sizeof(poctx));
+ pci_dma_read(&xhci->pci_dev, dcbaap + 8*slotid,
+ (uint8_t *) &poctx, sizeof(poctx));
ictx = xhci_mask64(pictx);
octx = xhci_mask64(le64_to_cpu(poctx));
- DPRINTF("xhci: input context at "TARGET_FMT_plx"\n", ictx);
- DPRINTF("xhci: output context at "TARGET_FMT_plx"\n", octx);
+ DPRINTF("xhci: input context at "DMA_ADDR_FMT"\n", ictx);
+ DPRINTF("xhci: output context at "DMA_ADDR_FMT"\n", octx);
- cpu_physical_memory_read(ictx, (uint8_t *) ictl_ctx, sizeof(ictl_ctx));
+ pci_dma_read(&xhci->pci_dev, ictx, (uint8_t *) ictl_ctx, sizeof(ictl_ctx));
if (ictl_ctx[0] != 0x0 || ictl_ctx[1] != 0x3) {
fprintf(stderr, "xhci: invalid input context control %08x %08x\n",
@@ -1840,8 +1839,9 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid,
return CC_TRB_ERROR;
}
- cpu_physical_memory_read(ictx+32, (uint8_t *) slot_ctx, sizeof(slot_ctx));
- cpu_physical_memory_read(ictx+64, (uint8_t *) ep0_ctx, sizeof(ep0_ctx));
+ pci_dma_read(&xhci->pci_dev, ictx+32, (uint8_t *) slot_ctx,
+ sizeof(slot_ctx));
+ pci_dma_read(&xhci->pci_dev, ictx+64, (uint8_t *) ep0_ctx, sizeof(ep0_ctx));
DPRINTF("xhci: input slot context: %08x %08x %08x %08x\n",
slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]);
@@ -1890,8 +1890,9 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid,
DPRINTF("xhci: output ep0 context: %08x %08x %08x %08x %08x\n",
ep0_ctx[0], ep0_ctx[1], ep0_ctx[2], ep0_ctx[3], ep0_ctx[4]);
- cpu_physical_memory_write(octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
- cpu_physical_memory_write(octx+32, (uint8_t *) ep0_ctx, sizeof(ep0_ctx));
+ pci_dma_write(&xhci->pci_dev, octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
+ pci_dma_write(&xhci->pci_dev, octx+32, (uint8_t *) ep0_ctx,
+ sizeof(ep0_ctx));
return res;
}
@@ -1900,7 +1901,7 @@ static TRBCCode xhci_address_slot(XHCIState *xhci, unsigned int slotid,
static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid,
uint64_t pictx, bool dc)
{
- target_phys_addr_t ictx, octx;
+ dma_addr_t ictx, octx;
uint32_t ictl_ctx[2];
uint32_t slot_ctx[4];
uint32_t islot_ctx[4];
@@ -1914,8 +1915,8 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid,
ictx = xhci_mask64(pictx);
octx = xhci->slots[slotid-1].ctx;
- DPRINTF("xhci: input context at "TARGET_FMT_plx"\n", ictx);
- DPRINTF("xhci: output context at "TARGET_FMT_plx"\n", octx);
+ DPRINTF("xhci: input context at "DMA_ADDR_FMT"\n", ictx);
+ DPRINTF("xhci: output context at "DMA_ADDR_FMT"\n", octx);
if (dc) {
for (i = 2; i <= 31; i++) {
@@ -1924,17 +1925,19 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid,
}
}
- cpu_physical_memory_read(octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
+ pci_dma_read(&xhci->pci_dev, octx, (uint8_t *) slot_ctx,
+ sizeof(slot_ctx));
slot_ctx[3] &= ~(SLOT_STATE_MASK << SLOT_STATE_SHIFT);
slot_ctx[3] |= SLOT_ADDRESSED << SLOT_STATE_SHIFT;
DPRINTF("xhci: output slot context: %08x %08x %08x %08x\n",
slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]);
- cpu_physical_memory_write(octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
+ pci_dma_write(&xhci->pci_dev, octx, (uint8_t *) slot_ctx,
+ sizeof(slot_ctx));
return CC_SUCCESS;
}
- cpu_physical_memory_read(ictx, (uint8_t *) ictl_ctx, sizeof(ictl_ctx));
+ pci_dma_read(&xhci->pci_dev, ictx, (uint8_t *) ictl_ctx, sizeof(ictl_ctx));
if ((ictl_ctx[0] & 0x3) != 0x0 || (ictl_ctx[1] & 0x3) != 0x1) {
fprintf(stderr, "xhci: invalid input context control %08x %08x\n",
@@ -1942,8 +1945,8 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid,
return CC_TRB_ERROR;
}
- cpu_physical_memory_read(ictx+32, (uint8_t *) islot_ctx, sizeof(islot_ctx));
- cpu_physical_memory_read(octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
+ pci_dma_read(&xhci->pci_dev, ictx+32, (uint8_t *) islot_ctx, sizeof(islot_ctx));
+ pci_dma_read(&xhci->pci_dev, octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
if (SLOT_STATE(slot_ctx[3]) < SLOT_ADDRESSED) {
fprintf(stderr, "xhci: invalid slot state %08x\n", slot_ctx[3]);
@@ -1955,7 +1958,7 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid,
xhci_disable_ep(xhci, slotid, i);
}
if (ictl_ctx[1] & (1<<i)) {
- cpu_physical_memory_read(ictx+32+(32*i),
+ pci_dma_read(&xhci->pci_dev, ictx+32+(32*i),
(uint8_t *) ep_ctx, sizeof(ep_ctx));
DPRINTF("xhci: input ep%d.%d context: %08x %08x %08x %08x %08x\n",
i/2, i%2, ep_ctx[0], ep_ctx[1], ep_ctx[2],
@@ -1968,7 +1971,7 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid,
DPRINTF("xhci: output ep%d.%d context: %08x %08x %08x %08x %08x\n",
i/2, i%2, ep_ctx[0], ep_ctx[1], ep_ctx[2],
ep_ctx[3], ep_ctx[4]);
- cpu_physical_memory_write(octx+(32*i),
+ pci_dma_write(&xhci->pci_dev, octx+(32*i),
(uint8_t *) ep_ctx, sizeof(ep_ctx));
}
}
@@ -1981,7 +1984,7 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid,
DPRINTF("xhci: output slot context: %08x %08x %08x %08x\n",
slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]);
- cpu_physical_memory_write(octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
+ pci_dma_write(&xhci->pci_dev, octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
return CC_SUCCESS;
}
@@ -1990,7 +1993,7 @@ static TRBCCode xhci_configure_slot(XHCIState *xhci, unsigned int slotid,
static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid,
uint64_t pictx)
{
- target_phys_addr_t ictx, octx;
+ dma_addr_t ictx, octx;
uint32_t ictl_ctx[2];
uint32_t iep0_ctx[5];
uint32_t ep0_ctx[5];
@@ -2003,10 +2006,10 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid,
ictx = xhci_mask64(pictx);
octx = xhci->slots[slotid-1].ctx;
- DPRINTF("xhci: input context at "TARGET_FMT_plx"\n", ictx);
- DPRINTF("xhci: output context at "TARGET_FMT_plx"\n", octx);
+ DPRINTF("xhci: input context at "DMA_ADDR_FMT"\n", ictx);
+ DPRINTF("xhci: output context at "DMA_ADDR_FMT"\n", octx);
- cpu_physical_memory_read(ictx, (uint8_t *) ictl_ctx, sizeof(ictl_ctx));
+ pci_dma_read(&xhci->pci_dev, ictx, (uint8_t *) ictl_ctx, sizeof(ictl_ctx));
if (ictl_ctx[0] != 0x0 || ictl_ctx[1] & ~0x3) {
fprintf(stderr, "xhci: invalid input context control %08x %08x\n",
@@ -2015,13 +2018,13 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid,
}
if (ictl_ctx[1] & 0x1) {
- cpu_physical_memory_read(ictx+32,
+ pci_dma_read(&xhci->pci_dev, ictx+32,
(uint8_t *) islot_ctx, sizeof(islot_ctx));
DPRINTF("xhci: input slot context: %08x %08x %08x %08x\n",
islot_ctx[0], islot_ctx[1], islot_ctx[2], islot_ctx[3]);
- cpu_physical_memory_read(octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
+ pci_dma_read(&xhci->pci_dev, octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
slot_ctx[1] &= ~0xFFFF; /* max exit latency */
slot_ctx[1] |= islot_ctx[1] & 0xFFFF;
@@ -2031,18 +2034,18 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid,
DPRINTF("xhci: output slot context: %08x %08x %08x %08x\n",
slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]);
- cpu_physical_memory_write(octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
+ pci_dma_write(&xhci->pci_dev, octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
}
if (ictl_ctx[1] & 0x2) {
- cpu_physical_memory_read(ictx+64,
+ pci_dma_read(&xhci->pci_dev, ictx+64,
(uint8_t *) iep0_ctx, sizeof(iep0_ctx));
DPRINTF("xhci: input ep0 context: %08x %08x %08x %08x %08x\n",
iep0_ctx[0], iep0_ctx[1], iep0_ctx[2],
iep0_ctx[3], iep0_ctx[4]);
- cpu_physical_memory_read(octx+32, (uint8_t *) ep0_ctx, sizeof(ep0_ctx));
+ pci_dma_read(&xhci->pci_dev, octx+32, (uint8_t *) ep0_ctx, sizeof(ep0_ctx));
ep0_ctx[1] &= ~0xFFFF0000; /* max packet size*/
ep0_ctx[1] |= iep0_ctx[1] & 0xFFFF0000;
@@ -2050,7 +2053,7 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid,
DPRINTF("xhci: output ep0 context: %08x %08x %08x %08x %08x\n",
ep0_ctx[0], ep0_ctx[1], ep0_ctx[2], ep0_ctx[3], ep0_ctx[4]);
- cpu_physical_memory_write(octx+32,
+ pci_dma_write(&xhci->pci_dev, octx+32,
(uint8_t *) ep0_ctx, sizeof(ep0_ctx));
}
@@ -2060,7 +2063,7 @@ static TRBCCode xhci_evaluate_slot(XHCIState *xhci, unsigned int slotid,
static TRBCCode xhci_reset_slot(XHCIState *xhci, unsigned int slotid)
{
uint32_t slot_ctx[4];
- target_phys_addr_t octx;
+ dma_addr_t octx;
int i;
assert(slotid >= 1 && slotid <= MAXSLOTS);
@@ -2068,7 +2071,7 @@ static TRBCCode xhci_reset_slot(XHCIState *xhci, unsigned int slotid)
octx = xhci->slots[slotid-1].ctx;
- DPRINTF("xhci: output context at "TARGET_FMT_plx"\n", octx);
+ DPRINTF("xhci: output context at "DMA_ADDR_FMT"\n", octx);
for (i = 2; i <= 31; i++) {
if (xhci->slots[slotid-1].eps[i-1]) {
@@ -2076,12 +2079,12 @@ static TRBCCode xhci_reset_slot(XHCIState *xhci, unsigned int slotid)
}
}
- cpu_physical_memory_read(octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
+ pci_dma_read(&xhci->pci_dev, octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
slot_ctx[3] &= ~(SLOT_STATE_MASK << SLOT_STATE_SHIFT);
slot_ctx[3] |= SLOT_DEFAULT << SLOT_STATE_SHIFT;
DPRINTF("xhci: output slot context: %08x %08x %08x %08x\n",
slot_ctx[0], slot_ctx[1], slot_ctx[2], slot_ctx[3]);
- cpu_physical_memory_write(octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
+ pci_dma_write(&xhci->pci_dev, octx, (uint8_t *) slot_ctx, sizeof(slot_ctx));
return CC_SUCCESS;
}
@@ -2104,19 +2107,19 @@ static unsigned int xhci_get_slot(XHCIState *xhci, XHCIEvent *event, XHCITRB *tr
static TRBCCode xhci_get_port_bandwidth(XHCIState *xhci, uint64_t pctx)
{
- target_phys_addr_t ctx;
+ dma_addr_t ctx;
uint8_t bw_ctx[MAXPORTS+1];
DPRINTF("xhci_get_port_bandwidth()\n");
ctx = xhci_mask64(pctx);
- DPRINTF("xhci: bandwidth context at "TARGET_FMT_plx"\n", ctx);
+ DPRINTF("xhci: bandwidth context at "DMA_ADDR_FMT"\n", ctx);
/* TODO: actually implement real values here */
bw_ctx[0] = 0;
memset(&bw_ctx[1], 80, MAXPORTS); /* 80% */
- cpu_physical_memory_write(ctx, bw_ctx, sizeof(bw_ctx));
+ pci_dma_write(&xhci->pci_dev, ctx, bw_ctx, sizeof(bw_ctx));
return CC_SUCCESS;
}
@@ -2137,13 +2140,13 @@ static uint32_t xhci_nec_challenge(uint32_t hi, uint32_t lo)
return ~val;
}
-static void xhci_via_challenge(uint64_t addr)
+static void xhci_via_challenge(XHCIState *xhci, uint64_t addr)
{
uint32_t buf[8];
uint32_t obuf[8];
- target_phys_addr_t paddr = xhci_mask64(addr);
+ dma_addr_t paddr = xhci_mask64(addr);
- cpu_physical_memory_read(paddr, (uint8_t *) &buf, 32);
+ pci_dma_read(&xhci->pci_dev, paddr, (uint8_t *) &buf, 32);
memcpy(obuf, buf, sizeof(obuf));
@@ -2159,7 +2162,7 @@ static void xhci_via_challenge(uint64_t addr)
obuf[7] = obuf[2] ^ obuf[3] ^ 0x65866593;
}
- cpu_physical_memory_write(paddr, (uint8_t *) &obuf, 32);
+ pci_dma_write(&xhci->pci_dev, paddr, (uint8_t *) &obuf, 32);
}
static void xhci_process_commands(XHCIState *xhci)
@@ -2167,7 +2170,7 @@ static void xhci_process_commands(XHCIState *xhci)
XHCITRB trb;
TRBType type;
XHCIEvent event = {ER_COMMAND_COMPLETE, CC_SUCCESS};
- target_phys_addr_t addr;
+ dma_addr_t addr;
unsigned int i, slotid = 0;
DPRINTF("xhci_process_commands()\n");
@@ -2256,7 +2259,7 @@ static void xhci_process_commands(XHCIState *xhci)
event.ccode = xhci_get_port_bandwidth(xhci, trb.parameter);
break;
case CR_VENDOR_VIA_CHALLENGE_RESPONSE:
- xhci_via_challenge(trb.parameter);
+ xhci_via_challenge(xhci, trb.parameter);
break;
case CR_VENDOR_NEC_FIRMWARE_REVISION:
event.type = 48; /* NEC reply */
@@ -2546,7 +2549,7 @@ static void xhci_oper_write(XHCIState *xhci, uint32_t reg, uint32_t val)
xhci_event(xhci, &event);
DPRINTF("xhci: command ring stopped (CRCR=%08x)\n", xhci->crcr_low);
} else {
- target_phys_addr_t base = xhci_addr64(xhci->crcr_low & ~0x3f, val);
+ dma_addr_t base = xhci_addr64(xhci->crcr_low & ~0x3f, val);
xhci_ring_init(xhci, &xhci->cmd_ring, base);
}
xhci->crcr_low &= ~(CRCR_CA | CRCR_CS);
--
1.7.9
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 4/8] Add universal DMA helper functions
2012-02-24 3:27 [Qemu-devel] [0/8] RFC: A second batch of preliminaries towards guest visible IOMMUS David Gibson
` (2 preceding siblings ...)
2012-02-24 3:27 ` [Qemu-devel] [PATCH 3/8] usb-xhci: Use PCI DMA helper functions David Gibson
@ 2012-02-24 3:27 ` David Gibson
2012-02-26 10:04 ` Michael S. Tsirkin
2012-02-26 21:24 ` Eduard - Gabriel Munteanu
2012-02-24 3:27 ` [Qemu-devel] [PATCH 5/8] usb-ohci: Use " David Gibson
` (3 subsequent siblings)
7 siblings, 2 replies; 18+ messages in thread
From: David Gibson @ 2012-02-24 3:27 UTC (permalink / raw)
To: anthony
Cc: Michael S. Tsirkin, Joerg Rodel, qemu-devel,
Eduard - Gabriel Munteanu, David Gibson, Richard Henderson
Not that long ago, every device implementation using DMA directly
accessed guest memory using cpu_physical_memory_*(). This meant that
adding support for a guest visible IOMMU would require changing every
one of these devices to go through IOMMU translation.
Shortly before qemu 1.0, I made a start on fixing this by providing
helper functions for PCI DMA. These are currently just stubs which
call the direct access functions, but mean that an IOMMU can be
implemented in one place, rather than for every PCI device.
Clearly, this doesn't help for non PCI devices, which could also be
IOMMU translated on some platforms. It is also problematic for the
devices which have both PCI and non-PCI version (e.g. OHCI, AHCI) - we
cannot use the the pci_dma_*() functions, because they assume the
presence of a PCIDevice, but we don't want to have to check between
pci_dma_*() and cpu_physical_memory_*() every time we do a DMA in the
device code.
This patch makes the first step on addressing both these problems, by
introducing new (stub) dma helper functions which can be used for any
DMA capable device.
These dma functions take a DMAContext *, a new (currently empty)
variable describing the DMA address space in which the operation is to
take place. NULL indicates untranslated DMA directly into guest
physical address space. The intention is that in future non-NULL
values will given information about any necessary IOMMU translation.
DMA using devices must obtain a DMAContext (or, potentially, contexts)
from their bus or platform. For now this patch just converts the PCI
wrappers to be implemented in terms of the universal wrappers,
converting other drivers can take place over time.
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Joerg Rodel <Joerg.Rodel@amd.com>
Cc: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
dma.h | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
hw/pci.h | 21 ++++++++------
2 files changed, 99 insertions(+), 9 deletions(-)
diff --git a/dma.h b/dma.h
index 79be131..d7428df 100644
--- a/dma.h
+++ b/dma.h
@@ -28,6 +28,93 @@ typedef enum {
DMA_DIRECTION_FROM_DEVICE = 1,
} DMADirection;
+typedef struct DMAContext {
+} DMAContext;
+
+typedef void DMAInvalidateMapFunc(void *);
+
+static inline int dma_memory_rw(DMAContext *dma, dma_addr_t addr,
+ void *buf, dma_addr_t len, DMADirection dir)
+{
+ cpu_physical_memory_rw(addr, buf, (target_phys_addr_t)len,
+ dir == DMA_DIRECTION_FROM_DEVICE);
+ return 0;
+}
+
+static inline int dma_memory_read(DMAContext *dma, dma_addr_t addr,
+ void *buf, dma_addr_t len)
+{
+ return dma_memory_rw(dma, addr, buf, len, DMA_DIRECTION_TO_DEVICE);
+}
+
+static inline int dma_memory_write(DMAContext *dma, dma_addr_t addr,
+ const void *buf, dma_addr_t len)
+{
+ return dma_memory_rw(dma, addr, (void *)buf, len,
+ DMA_DIRECTION_FROM_DEVICE);
+}
+
+static inline void *dma_memory_map(DMAContext *dma,
+ DMAInvalidateMapFunc *cb, void *opaque,
+ dma_addr_t addr, dma_addr_t *len,
+ DMADirection dir)
+{
+ target_phys_addr_t xlen = *len;
+ void *p;
+
+ p = cpu_physical_memory_map(addr, &xlen,
+ dir == DMA_DIRECTION_FROM_DEVICE);
+ *len = xlen;
+ return p;
+}
+
+static inline void dma_memory_unmap(DMAContext *dma,
+ void *buffer, dma_addr_t len,
+ DMADirection dir, dma_addr_t access_len)
+{
+ return cpu_physical_memory_unmap(buffer, (target_phys_addr_t)len,
+ dir == DMA_DIRECTION_FROM_DEVICE,
+ access_len);
+}
+
+#define DEFINE_LDST_DMA(_lname, _sname, _bits, _end) \
+ static inline uint##_bits##_t ld##_lname##_##_end##_dma(DMAContext *dma, \
+ dma_addr_t addr) \
+ { \
+ uint##_bits##_t val; \
+ dma_memory_read(dma, addr, &val, (_bits) / 8); \
+ return _end##_bits##_to_cpu(val); \
+ } \
+ static inline void st##_sname##_##_end##_dma(DMAContext *dma, \
+ dma_addr_t addr, \
+ uint##_bits##_t val) \
+ { \
+ val = cpu_to_##_end##_bits(val); \
+ dma_memory_write(dma, addr, &val, (_bits) / 8); \
+ }
+
+static inline uint8_t ldub_dma(DMAContext *dma, dma_addr_t addr)
+{
+ uint8_t val;
+
+ dma_memory_read(dma, addr, &val, 1);
+ return val;
+}
+
+static inline void stb_dma(DMAContext *dma, dma_addr_t addr, uint8_t val)
+{
+ dma_memory_write(dma, addr, &val, 1);
+}
+
+DEFINE_LDST_DMA(uw, w, 16, le);
+DEFINE_LDST_DMA(l, l, 32, le);
+DEFINE_LDST_DMA(q, q, 64, le);
+DEFINE_LDST_DMA(uw, w, 16, be);
+DEFINE_LDST_DMA(l, l, 32, be);
+DEFINE_LDST_DMA(q, q, 64, be);
+
+#undef DEFINE_LDST_DMA
+
struct ScatterGatherEntry {
dma_addr_t base;
dma_addr_t len;
diff --git a/hw/pci.h b/hw/pci.h
index ee53f26..64734a1 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -544,10 +544,15 @@ static inline uint32_t pci_config_size(const PCIDevice *d)
}
/* DMA access functions */
+static inline DMAContext *pci_dma_context(PCIDevice *dev)
+{
+ return NULL;
+}
+
static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr,
void *buf, dma_addr_t len, DMADirection dir)
{
- cpu_physical_memory_rw(addr, buf, len, dir == DMA_DIRECTION_FROM_DEVICE);
+ dma_memory_rw(pci_dma_context(dev), addr, buf, len, dir);
return 0;
}
@@ -567,12 +572,12 @@ static inline int pci_dma_write(PCIDevice *dev, dma_addr_t addr,
static inline uint##_bits##_t ld##_l##_pci_dma(PCIDevice *dev, \
dma_addr_t addr) \
{ \
- return ld##_l##_phys(addr); \
+ return ld##_l##_dma(pci_dma_context(dev), addr); \
} \
static inline void st##_s##_pci_dma(PCIDevice *dev, \
- dma_addr_t addr, uint##_bits##_t val) \
+ dma_addr_t addr, uint##_bits##_t val) \
{ \
- st##_s##_phys(addr, val); \
+ st##_s##_dma(pci_dma_context(dev), addr, val); \
}
PCI_DMA_DEFINE_LDST(ub, b, 8);
@@ -586,21 +591,19 @@ PCI_DMA_DEFINE_LDST(q_be, q_be, 64);
#undef PCI_DMA_DEFINE_LDST
static inline void *pci_dma_map(PCIDevice *dev, dma_addr_t addr,
+ DMAInvalidateMapFunc *cb, void *opaque,
dma_addr_t *plen, DMADirection dir)
{
- target_phys_addr_t len = *plen;
void *buf;
- buf = cpu_physical_memory_map(addr, &len, dir == DMA_DIRECTION_FROM_DEVICE);
- *plen = len;
+ buf = dma_memory_map(pci_dma_context(dev), cb, opaque, addr, plen, dir);
return buf;
}
static inline void pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len,
DMADirection dir, dma_addr_t access_len)
{
- cpu_physical_memory_unmap(buffer, len, dir == DMA_DIRECTION_FROM_DEVICE,
- access_len);
+ dma_memory_unmap(pci_dma_context(dev), buffer, len, dir, access_len);
}
static inline void pci_dma_sglist_init(QEMUSGList *qsg, PCIDevice *dev,
--
1.7.9
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 5/8] usb-ohci: Use universal DMA helper functions
2012-02-24 3:27 [Qemu-devel] [0/8] RFC: A second batch of preliminaries towards guest visible IOMMUS David Gibson
` (3 preceding siblings ...)
2012-02-24 3:27 ` [Qemu-devel] [PATCH 4/8] Add universal " David Gibson
@ 2012-02-24 3:27 ` David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 6/8] Make sglists and dma_bdrv helpers use new universal DMA herlpers David Gibson
` (2 subsequent siblings)
7 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2012-02-24 3:27 UTC (permalink / raw)
To: anthony; +Cc: Michael S. Tsirkin, Gerd Hoffmann, qemu-devel, David Gibson
The OHCI device emulation can provide both PCI and SysBus OHCI
implementations. Because of this, it was not previously converted to
use the PCI DMA helper functions.
This patch converts it to use the new universal DMA helper functions.
In the PCI case, it obtains its DMAContext from pci_dma_context(), in
the SysBus case, it uses NULL - i.e. assumes for now that there will
be no IOMMU translation for a SysBus OHCI.
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/usb-ohci.c | 93 +++++++++++++++++++++++++++++++-------------------------
1 files changed, 51 insertions(+), 42 deletions(-)
diff --git a/hw/usb-ohci.c b/hw/usb-ohci.c
index 5f88bc3..efc3bd2 100644
--- a/hw/usb-ohci.c
+++ b/hw/usb-ohci.c
@@ -32,7 +32,7 @@
#include "pci.h"
#include "usb-ohci.h"
#include "sysbus.h"
-#include "qdev-addr.h"
+#include "qdev-dma.h"
//#define DEBUG_OHCI
/* Dump packet contents. */
@@ -63,6 +63,7 @@ typedef struct {
USBBus bus;
qemu_irq irq;
MemoryRegion mem;
+ DMAContext *dma;
int num_ports;
const char *name;
@@ -105,7 +106,7 @@ typedef struct {
uint32_t htest;
/* SM501 local memory offset */
- target_phys_addr_t localmem_base;
+ dma_addr_t localmem_base;
/* Active packets. */
uint32_t old_ctl;
@@ -484,14 +485,14 @@ static void ohci_reset(void *opaque)
/* Get an array of dwords from main memory */
static inline int get_dwords(OHCIState *ohci,
- uint32_t addr, uint32_t *buf, int num)
+ dma_addr_t addr, uint32_t *buf, int num)
{
int i;
addr += ohci->localmem_base;
for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
- cpu_physical_memory_read(addr, buf, sizeof(*buf));
+ dma_memory_read(ohci->dma, addr, buf, sizeof(*buf));
*buf = le32_to_cpu(*buf);
}
@@ -500,7 +501,7 @@ static inline int get_dwords(OHCIState *ohci,
/* Put an array of dwords in to main memory */
static inline int put_dwords(OHCIState *ohci,
- uint32_t addr, uint32_t *buf, int num)
+ dma_addr_t addr, uint32_t *buf, int num)
{
int i;
@@ -508,7 +509,7 @@ static inline int put_dwords(OHCIState *ohci,
for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
uint32_t tmp = cpu_to_le32(*buf);
- cpu_physical_memory_write(addr, &tmp, sizeof(tmp));
+ dma_memory_write(ohci->dma, addr, &tmp, sizeof(tmp));
}
return 1;
@@ -516,14 +517,14 @@ static inline int put_dwords(OHCIState *ohci,
/* Get an array of words from main memory */
static inline int get_words(OHCIState *ohci,
- uint32_t addr, uint16_t *buf, int num)
+ dma_addr_t addr, uint16_t *buf, int num)
{
int i;
addr += ohci->localmem_base;
for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
- cpu_physical_memory_read(addr, buf, sizeof(*buf));
+ dma_memory_read(ohci->dma, addr, buf, sizeof(*buf));
*buf = le16_to_cpu(*buf);
}
@@ -532,7 +533,7 @@ static inline int get_words(OHCIState *ohci,
/* Put an array of words in to main memory */
static inline int put_words(OHCIState *ohci,
- uint32_t addr, uint16_t *buf, int num)
+ dma_addr_t addr, uint16_t *buf, int num)
{
int i;
@@ -540,40 +541,40 @@ static inline int put_words(OHCIState *ohci,
for (i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
uint16_t tmp = cpu_to_le16(*buf);
- cpu_physical_memory_write(addr, &tmp, sizeof(tmp));
+ dma_memory_write(ohci->dma, addr, &tmp, sizeof(tmp));
}
return 1;
}
static inline int ohci_read_ed(OHCIState *ohci,
- uint32_t addr, struct ohci_ed *ed)
+ dma_addr_t addr, struct ohci_ed *ed)
{
return get_dwords(ohci, addr, (uint32_t *)ed, sizeof(*ed) >> 2);
}
static inline int ohci_read_td(OHCIState *ohci,
- uint32_t addr, struct ohci_td *td)
+ dma_addr_t addr, struct ohci_td *td)
{
return get_dwords(ohci, addr, (uint32_t *)td, sizeof(*td) >> 2);
}
static inline int ohci_read_iso_td(OHCIState *ohci,
- uint32_t addr, struct ohci_iso_td *td)
+ dma_addr_t addr, struct ohci_iso_td *td)
{
return (get_dwords(ohci, addr, (uint32_t *)td, 4) &&
get_words(ohci, addr + 16, td->offset, 8));
}
static inline int ohci_read_hcca(OHCIState *ohci,
- uint32_t addr, struct ohci_hcca *hcca)
+ dma_addr_t addr, struct ohci_hcca *hcca)
{
- cpu_physical_memory_read(addr + ohci->localmem_base, hcca, sizeof(*hcca));
+ dma_memory_read(ohci->dma, addr + ohci->localmem_base, hcca, sizeof(*hcca));
return 1;
}
static inline int ohci_put_ed(OHCIState *ohci,
- uint32_t addr, struct ohci_ed *ed)
+ dma_addr_t addr, struct ohci_ed *ed)
{
/* ed->tail is under control of the HCD, so we need to split
* the write back into two parts
@@ -584,64 +585,63 @@ static inline int ohci_put_ed(OHCIState *ohci,
}
static inline int ohci_put_td(OHCIState *ohci,
- uint32_t addr, struct ohci_td *td)
+ dma_addr_t addr, struct ohci_td *td)
{
return put_dwords(ohci, addr, (uint32_t *)td, sizeof(*td) >> 2);
}
static inline int ohci_put_iso_td(OHCIState *ohci,
- uint32_t addr, struct ohci_iso_td *td)
+ dma_addr_t addr, struct ohci_iso_td *td)
{
return (put_dwords(ohci, addr, (uint32_t *)td, 4) &&
put_words(ohci, addr + 16, td->offset, 8));
}
static inline int ohci_put_hcca(OHCIState *ohci,
- uint32_t addr, struct ohci_hcca *hcca)
+ dma_addr_t addr, struct ohci_hcca *hcca)
{
- cpu_physical_memory_write(addr + ohci->localmem_base + HCCA_WRITEBACK_OFFSET,
- (char *)hcca + HCCA_WRITEBACK_OFFSET,
- HCCA_WRITEBACK_SIZE);
+ dma_memory_write(ohci->dma,
+ addr + ohci->localmem_base + HCCA_WRITEBACK_OFFSET,
+ (char *)hcca + HCCA_WRITEBACK_OFFSET,
+ HCCA_WRITEBACK_SIZE);
return 1;
}
/* Read/Write the contents of a TD from/to main memory. */
static void ohci_copy_td(OHCIState *ohci, struct ohci_td *td,
- uint8_t *buf, int len, int write)
+ uint8_t *buf, int len, DMADirection dir)
{
- uint32_t ptr;
- uint32_t n;
+ dma_addr_t ptr, n;
ptr = td->cbp;
n = 0x1000 - (ptr & 0xfff);
if (n > len)
n = len;
- cpu_physical_memory_rw(ptr + ohci->localmem_base, buf, n, write);
+ dma_memory_rw(ohci->dma, ptr + ohci->localmem_base, buf, n, dir);
if (n == len)
return;
ptr = td->be & ~0xfffu;
buf += n;
- cpu_physical_memory_rw(ptr + ohci->localmem_base, buf, len - n, write);
+ dma_memory_rw(ohci->dma, ptr + ohci->localmem_base, buf, len - n, dir);
}
/* Read/Write the contents of an ISO TD from/to main memory. */
static void ohci_copy_iso_td(OHCIState *ohci,
uint32_t start_addr, uint32_t end_addr,
- uint8_t *buf, int len, int write)
+ uint8_t *buf, int len, DMADirection dir)
{
- uint32_t ptr;
- uint32_t n;
+ dma_addr_t ptr, n;
ptr = start_addr;
n = 0x1000 - (ptr & 0xfff);
if (n > len)
n = len;
- cpu_physical_memory_rw(ptr + ohci->localmem_base, buf, n, write);
+ dma_memory_rw(ohci->dma, ptr + ohci->localmem_base, buf, n, dir);
if (n == len)
return;
ptr = end_addr & ~0xfffu;
buf += n;
- cpu_physical_memory_rw(ptr + ohci->localmem_base, buf, len - n, write);
+ dma_memory_rw(ohci->dma, ptr + ohci->localmem_base, buf, len - n, dir);
}
static void ohci_process_lists(OHCIState *ohci, int completion);
@@ -804,7 +804,8 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
}
if (len && dir != OHCI_TD_DIR_IN) {
- ohci_copy_iso_td(ohci, start_addr, end_addr, ohci->usb_buf, len, 0);
+ ohci_copy_iso_td(ohci, start_addr, end_addr, ohci->usb_buf, len,
+ DMA_DIRECTION_TO_DEVICE);
}
if (completion) {
@@ -828,7 +829,8 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
/* Writeback */
if (dir == OHCI_TD_DIR_IN && ret >= 0 && ret <= len) {
/* IN transfer succeeded */
- ohci_copy_iso_td(ohci, start_addr, end_addr, ohci->usb_buf, ret, 1);
+ ohci_copy_iso_td(ohci, start_addr, end_addr, ohci->usb_buf, ret,
+ DMA_DIRECTION_FROM_DEVICE);
OHCI_SET_BM(iso_td.offset[relative_frame_number], TD_PSW_CC,
OHCI_CC_NOERROR);
OHCI_SET_BM(iso_td.offset[relative_frame_number], TD_PSW_SIZE, ret);
@@ -971,7 +973,8 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
pktlen = len;
}
if (!completion) {
- ohci_copy_td(ohci, &td, ohci->usb_buf, pktlen, 0);
+ ohci_copy_td(ohci, &td, ohci->usb_buf, pktlen,
+ DMA_DIRECTION_TO_DEVICE);
}
}
}
@@ -1021,7 +1024,8 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
}
if (ret >= 0) {
if (dir == OHCI_TD_DIR_IN) {
- ohci_copy_td(ohci, &td, ohci->usb_buf, ret, 1);
+ ohci_copy_td(ohci, &td, ohci->usb_buf, ret,
+ DMA_DIRECTION_FROM_DEVICE);
#ifdef DEBUG_PACKET
DPRINTF(" data:");
for (i = 0; i < ret; i++)
@@ -1747,11 +1751,14 @@ static USBBusOps ohci_bus_ops = {
};
static int usb_ohci_init(OHCIState *ohci, DeviceState *dev,
- int num_ports, uint32_t localmem_base,
- char *masterbus, uint32_t firstport)
+ int num_ports, dma_addr_t localmem_base,
+ char *masterbus, uint32_t firstport,
+ DMAContext *dma)
{
int i;
+ ohci->dma = dma;
+
if (usb_frame_time == 0) {
#ifdef OHCI_TIME_WARP
usb_frame_time = get_ticks_per_sec();
@@ -1816,7 +1823,8 @@ static int usb_ohci_initfn_pci(struct PCIDevice *dev)
ohci->pci_dev.config[PCI_INTERRUPT_PIN] = 0x01; /* interrupt pin A */
if (usb_ohci_init(&ohci->state, &dev->qdev, ohci->num_ports, 0,
- ohci->masterbus, ohci->firstport) != 0) {
+ ohci->masterbus, ohci->firstport,
+ pci_dma_context(dev)) != 0) {
return -1;
}
ohci->state.irq = ohci->pci_dev.irq[0];
@@ -1835,7 +1843,7 @@ typedef struct {
SysBusDevice busdev;
OHCIState ohci;
uint32_t num_ports;
- target_phys_addr_t dma_offset;
+ dma_addr_t dma_offset;
} OHCISysBusState;
static int ohci_init_pxa(SysBusDevice *dev)
@@ -1843,7 +1851,8 @@ static int ohci_init_pxa(SysBusDevice *dev)
OHCISysBusState *s = FROM_SYSBUS(OHCISysBusState, dev);
/* Cannot fail as we pass NULL for masterbus */
- usb_ohci_init(&s->ohci, &dev->qdev, s->num_ports, s->dma_offset, NULL, 0);
+ usb_ohci_init(&s->ohci, &dev->qdev, s->num_ports, s->dma_offset, NULL, 0,
+ NULL);
sysbus_init_irq(dev, &s->ohci.irq);
sysbus_init_mmio(dev, &s->ohci.mem);
@@ -1879,7 +1888,7 @@ static TypeInfo ohci_pci_info = {
static Property ohci_sysbus_properties[] = {
DEFINE_PROP_UINT32("num-ports", OHCISysBusState, num_ports, 3),
- DEFINE_PROP_TADDR("dma-offset", OHCISysBusState, dma_offset, 3),
+ DEFINE_PROP_DMAADDR("dma-offset", OHCISysBusState, dma_offset, 3),
DEFINE_PROP_END_OF_LIST(),
};
--
1.7.9
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 6/8] Make sglists and dma_bdrv helpers use new universal DMA herlpers
2012-02-24 3:27 [Qemu-devel] [0/8] RFC: A second batch of preliminaries towards guest visible IOMMUS David Gibson
` (4 preceding siblings ...)
2012-02-24 3:27 ` [Qemu-devel] [PATCH 5/8] usb-ohci: Use " David Gibson
@ 2012-02-24 3:27 ` David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 7/8] ide/ahci: Use universal DMA helper functions David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 8/8] Make dma_addr_t 64 bit always David Gibson
7 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2012-02-24 3:27 UTC (permalink / raw)
To: anthony; +Cc: Kevin Wolf, Michael S. Tsirkin, qemu-devel, David Gibson
dma-helpers.c contains a number of helper functions for doing
scatter/gather DMA, and various block device related DMA. Currently,
these directly access guest memory using cpu_physical_memory_*(),
assuming no IOMMU translation.
This patch updates this code to use the new universal DMA helper
functions. qemu_sglist_init() now takes a DMAContext * to describe
the DMA address space in which the scatter/gather will take place.
We minimally update the callers qemu_sglist_init() to pass NULL
(i.e. no translation, same as current behaviour). Some of those
callers should pass something else in some cases to allow proper IOMMU
translation in future, but that will be fixed in later patches.
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
dma-helpers.c | 26 ++++++++++++++++++--------
dma.h | 3 ++-
hw/ide/ahci.c | 3 ++-
hw/ide/macio.c | 4 ++--
hw/pci.h | 2 +-
5 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/dma-helpers.c b/dma-helpers.c
index 57eb22c..c1759cd 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -11,12 +11,13 @@
#include "block_int.h"
#include "trace.h"
-void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint)
+void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint, DMAContext *dma)
{
qsg->sg = g_malloc(alloc_hint * sizeof(ScatterGatherEntry));
qsg->nsg = 0;
qsg->nalloc = alloc_hint;
qsg->size = 0;
+ qsg->dma = dma;
}
void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, dma_addr_t len)
@@ -75,10 +76,9 @@ static void dma_bdrv_unmap(DMAAIOCB *dbs)
int i;
for (i = 0; i < dbs->iov.niov; ++i) {
- cpu_physical_memory_unmap(dbs->iov.iov[i].iov_base,
- dbs->iov.iov[i].iov_len,
- dbs->dir != DMA_DIRECTION_TO_DEVICE,
- dbs->iov.iov[i].iov_len);
+ dma_memory_unmap(dbs->sg->dma, dbs->iov.iov[i].iov_base,
+ dbs->iov.iov[i].iov_len, dbs->dir,
+ dbs->iov.iov[i].iov_len);
}
qemu_iovec_reset(&dbs->iov);
}
@@ -104,10 +104,20 @@ static void dma_complete(DMAAIOCB *dbs, int ret)
}
}
+static void dma_bdrv_cancel(void *opaque)
+{
+ DMAAIOCB *dbs = opaque;
+
+ bdrv_aio_cancel(dbs->acb);
+ dma_bdrv_unmap(dbs);
+ qemu_iovec_destroy(&dbs->iov);
+ qemu_aio_release(dbs);
+}
+
static void dma_bdrv_cb(void *opaque, int ret)
{
DMAAIOCB *dbs = (DMAAIOCB *)opaque;
- target_phys_addr_t cur_addr, cur_len;
+ dma_addr_t cur_addr, cur_len;
void *mem;
trace_dma_bdrv_cb(dbs, ret);
@@ -124,8 +134,8 @@ static void dma_bdrv_cb(void *opaque, int ret)
while (dbs->sg_cur_index < dbs->sg->nsg) {
cur_addr = dbs->sg->sg[dbs->sg_cur_index].base + dbs->sg_cur_byte;
cur_len = dbs->sg->sg[dbs->sg_cur_index].len - dbs->sg_cur_byte;
- mem = cpu_physical_memory_map(cur_addr, &cur_len,
- dbs->dir != DMA_DIRECTION_TO_DEVICE);
+ mem = dma_memory_map(dbs->sg->dma, dma_bdrv_cancel, dbs,
+ cur_addr, &cur_len, dbs->dir);
if (!mem)
break;
qemu_iovec_add(&dbs->iov, mem, cur_len);
diff --git a/dma.h b/dma.h
index d7428df..b8cfd1d 100644
--- a/dma.h
+++ b/dma.h
@@ -125,9 +125,10 @@ struct QEMUSGList {
int nsg;
int nalloc;
dma_addr_t size;
+ DMAContext *dma;
};
-void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint);
+void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint, DMAContext *dma);
void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, dma_addr_t len);
void qemu_sglist_destroy(QEMUSGList *qsg);
#endif
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index b515f41..aa05a48 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -716,7 +716,8 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist)
if (sglist_alloc_hint > 0) {
AHCI_SG *tbl = (AHCI_SG *)prdt;
- qemu_sglist_init(sglist, sglist_alloc_hint);
+ /* FIXME: pass the correct DMAContext */
+ qemu_sglist_init(sglist, sglist_alloc_hint, NULL);
for (i = 0; i < sglist_alloc_hint; i++) {
/* flags_size is zero-based */
qemu_sglist_add(sglist, le64_to_cpu(tbl[i].addr),
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index edcf885..568a299 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -76,7 +76,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret)
s->io_buffer_size = io->len;
- qemu_sglist_init(&s->sg, io->len / MACIO_PAGE_SIZE + 1);
+ qemu_sglist_init(&s->sg, io->len / MACIO_PAGE_SIZE + 1, NULL);
qemu_sglist_add(&s->sg, io->addr, io->len);
io->addr += io->len;
io->len = 0;
@@ -133,7 +133,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
s->io_buffer_index = 0;
s->io_buffer_size = io->len;
- qemu_sglist_init(&s->sg, io->len / MACIO_PAGE_SIZE + 1);
+ qemu_sglist_init(&s->sg, io->len / MACIO_PAGE_SIZE + 1, NULL);
qemu_sglist_add(&s->sg, io->addr, io->len);
io->addr += io->len;
io->len = 0;
diff --git a/hw/pci.h b/hw/pci.h
index 64734a1..9d3f314 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -609,7 +609,7 @@ static inline void pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len,
static inline void pci_dma_sglist_init(QEMUSGList *qsg, PCIDevice *dev,
int alloc_hint)
{
- qemu_sglist_init(qsg, alloc_hint);
+ qemu_sglist_init(qsg, alloc_hint, pci_dma_context(dev));
}
extern const VMStateDescription vmstate_pci_device;
--
1.7.9
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 7/8] ide/ahci: Use universal DMA helper functions
2012-02-24 3:27 [Qemu-devel] [0/8] RFC: A second batch of preliminaries towards guest visible IOMMUS David Gibson
` (5 preceding siblings ...)
2012-02-24 3:27 ` [Qemu-devel] [PATCH 6/8] Make sglists and dma_bdrv helpers use new universal DMA herlpers David Gibson
@ 2012-02-24 3:27 ` David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 8/8] Make dma_addr_t 64 bit always David Gibson
7 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2012-02-24 3:27 UTC (permalink / raw)
To: anthony; +Cc: Kevin Wolf, Michael S. Tsirkin, qemu-devel, David Gibson
The AHCI device can provide both PCI and SysBus AHCI device
emulations. For this reason, it wasn't previously converted to use
the pci_dma_*() helper functions. Now that we have universal DMA
helper functions, this converts AHCI to use them.
The DMAContext is obtained from pci_dma_context() in the PCI case and
set to NULL in the SysBus case (i.e. we assume for now that a SysBus
AHCI has no IOMMU translation).
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ide/ahci.c | 7 ++++---
hw/ide/ahci.h | 3 ++-
hw/ide/ich.c | 2 +-
3 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index aa05a48..624da69 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -717,7 +717,7 @@ static int ahci_populate_sglist(AHCIDevice *ad, QEMUSGList *sglist)
AHCI_SG *tbl = (AHCI_SG *)prdt;
/* FIXME: pass the correct DMAContext */
- qemu_sglist_init(sglist, sglist_alloc_hint, NULL);
+ qemu_sglist_init(sglist, sglist_alloc_hint, ad->hba->dma);
for (i = 0; i < sglist_alloc_hint; i++) {
/* flags_size is zero-based */
qemu_sglist_add(sglist, le64_to_cpu(tbl[i].addr),
@@ -1171,11 +1171,12 @@ static const IDEDMAOps ahci_dma_ops = {
.reset = ahci_dma_reset,
};
-void ahci_init(AHCIState *s, DeviceState *qdev, int ports)
+void ahci_init(AHCIState *s, DeviceState *qdev, DMAContext *dma, int ports)
{
qemu_irq *irqs;
int i;
+ s->dma = dma;
s->ports = ports;
s->dev = g_malloc0(sizeof(AHCIDevice) * ports);
ahci_reg_init(s);
@@ -1238,7 +1239,7 @@ static const VMStateDescription vmstate_sysbus_ahci = {
static int sysbus_ahci_init(SysBusDevice *dev)
{
SysbusAHCIState *s = FROM_SYSBUS(SysbusAHCIState, dev);
- ahci_init(&s->ahci, &dev->qdev, s->num_ports);
+ ahci_init(&s->ahci, &dev->qdev, NULL, s->num_ports);
sysbus_init_mmio(dev, &s->ahci.mem);
sysbus_init_irq(dev, &s->ahci.irq);
diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index b223d2c..af8c6ef 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -299,6 +299,7 @@ typedef struct AHCIState {
uint32_t idp_index; /* Current IDP index */
int ports;
qemu_irq irq;
+ DMAContext *dma;
} AHCIState;
typedef struct AHCIPCIState {
@@ -329,7 +330,7 @@ typedef struct NCQFrame {
uint8_t reserved10;
} QEMU_PACKED NCQFrame;
-void ahci_init(AHCIState *s, DeviceState *qdev, int ports);
+void ahci_init(AHCIState *s, DeviceState *qdev, DMAContext *dma, int ports);
void ahci_uninit(AHCIState *s);
void ahci_reset(void *opaque);
diff --git a/hw/ide/ich.c b/hw/ide/ich.c
index 560ae37..5354e13 100644
--- a/hw/ide/ich.c
+++ b/hw/ide/ich.c
@@ -91,7 +91,7 @@ static int pci_ich9_ahci_init(PCIDevice *dev)
uint8_t *sata_cap;
d = DO_UPCAST(struct AHCIPCIState, card, dev);
- ahci_init(&d->ahci, &dev->qdev, 6);
+ ahci_init(&d->ahci, &dev->qdev, pci_dma_context(dev), 6);
pci_config_set_prog_interface(d->card.config, AHCI_PROGMODE_MAJOR_REV_1);
--
1.7.9
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH 8/8] Make dma_addr_t 64 bit always
2012-02-24 3:27 [Qemu-devel] [0/8] RFC: A second batch of preliminaries towards guest visible IOMMUS David Gibson
` (6 preceding siblings ...)
2012-02-24 3:27 ` [Qemu-devel] [PATCH 7/8] ide/ahci: Use universal DMA helper functions David Gibson
@ 2012-02-24 3:27 ` David Gibson
2012-02-24 4:57 ` David Gibson
7 siblings, 1 reply; 18+ messages in thread
From: David Gibson @ 2012-02-24 3:27 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel, David Gibson
---
dma.h | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/dma.h b/dma.h
index b8cfd1d..ee540f4 100644
--- a/dma.h
+++ b/dma.h
@@ -18,10 +18,13 @@
typedef struct ScatterGatherEntry ScatterGatherEntry;
#if defined(TARGET_PHYS_ADDR_BITS)
-typedef target_phys_addr_t dma_addr_t;
+/* Bus addresses can be different size from CPU physical addresses,
+ * and indeed they can be different on different busses. So make
+ * these always 64-bit which should handle every usual case */
+typedef uint64_t dma_addr_t;
-#define DMA_ADDR_BITS TARGET_PHYS_ADDR_BITS
-#define DMA_ADDR_FMT TARGET_FMT_plx
+#define DMA_ADDR_BITS 64
+#define DMA_ADDR_FMT "%" PRIx64
typedef enum {
DMA_DIRECTION_TO_DEVICE = 0,
--
1.7.9
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 8/8] Make dma_addr_t 64 bit always
2012-02-24 3:27 ` [Qemu-devel] [PATCH 8/8] Make dma_addr_t 64 bit always David Gibson
@ 2012-02-24 4:57 ` David Gibson
2012-02-26 21:46 ` Blue Swirl
0 siblings, 1 reply; 18+ messages in thread
From: David Gibson @ 2012-02-24 4:57 UTC (permalink / raw)
To: anthony; +Cc: qemu-devel
On Fri, Feb 24, 2012 at 02:27:43PM +1100, David Gibson wrote:
Oops, ignore this one folks. As is probably obvious, this was a
testing patch not meant to go into the main series.
> ---
> dma.h | 9 ++++++---
> 1 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/dma.h b/dma.h
> index b8cfd1d..ee540f4 100644
> --- a/dma.h
> +++ b/dma.h
> @@ -18,10 +18,13 @@
> typedef struct ScatterGatherEntry ScatterGatherEntry;
>
> #if defined(TARGET_PHYS_ADDR_BITS)
> -typedef target_phys_addr_t dma_addr_t;
> +/* Bus addresses can be different size from CPU physical addresses,
> + * and indeed they can be different on different busses. So make
> + * these always 64-bit which should handle every usual case */
> +typedef uint64_t dma_addr_t;
>
> -#define DMA_ADDR_BITS TARGET_PHYS_ADDR_BITS
> -#define DMA_ADDR_FMT TARGET_FMT_plx
> +#define DMA_ADDR_BITS 64
> +#define DMA_ADDR_FMT "%" PRIx64
>
> typedef enum {
> DMA_DIRECTION_TO_DEVICE = 0,
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] Add universal DMA helper functions
2012-02-24 3:27 ` [Qemu-devel] [PATCH 4/8] Add universal " David Gibson
@ 2012-02-26 10:04 ` Michael S. Tsirkin
2012-02-26 21:09 ` Eduard - Gabriel Munteanu
2012-02-27 0:22 ` David Gibson
2012-02-26 21:24 ` Eduard - Gabriel Munteanu
1 sibling, 2 replies; 18+ messages in thread
From: Michael S. Tsirkin @ 2012-02-26 10:04 UTC (permalink / raw)
To: David Gibson
Cc: Joerg Rodel, Richard Henderson, qemu-devel, anthony,
Eduard - Gabriel Munteanu
On Fri, Feb 24, 2012 at 02:27:39PM +1100, David Gibson wrote:
> Not that long ago, every device implementation using DMA directly
> accessed guest memory using cpu_physical_memory_*(). This meant that
> adding support for a guest visible IOMMU would require changing every
> one of these devices to go through IOMMU translation.
>
> Shortly before qemu 1.0, I made a start on fixing this by providing
> helper functions for PCI DMA. These are currently just stubs which
> call the direct access functions, but mean that an IOMMU can be
> implemented in one place, rather than for every PCI device.
>
> Clearly, this doesn't help for non PCI devices, which could also be
> IOMMU translated on some platforms. It is also problematic for the
> devices which have both PCI and non-PCI version (e.g. OHCI, AHCI) - we
> cannot use the the pci_dma_*() functions, because they assume the
> presence of a PCIDevice, but we don't want to have to check between
> pci_dma_*() and cpu_physical_memory_*() every time we do a DMA in the
> device code.
>
> This patch makes the first step on addressing both these problems, by
> introducing new (stub) dma helper functions which can be used for any
> DMA capable device.
>
> These dma functions take a DMAContext *, a new (currently empty)
> variable describing the DMA address space in which the operation is to
> take place. NULL indicates untranslated DMA directly into guest
> physical address space. The intention is that in future non-NULL
> values will given information about any necessary IOMMU translation.
>
> DMA using devices must obtain a DMAContext (or, potentially, contexts)
> from their bus or platform. For now this patch just converts the PCI
> wrappers to be implemented in terms of the universal wrappers,
> converting other drivers can take place over time.
>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Joerg Rodel <Joerg.Rodel@amd.com>
> Cc: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
> Cc: Richard Henderson <rth@twiddle.net>
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
I'm a bit confused with all the stubbing going on.
Is this the final form of the pci_* functions or just
a stub? If the final form, we probably should just
open-code them - they don't buy us much.
If not, let's add a comment?
> ---
> dma.h | 87 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> hw/pci.h | 21 ++++++++------
> 2 files changed, 99 insertions(+), 9 deletions(-)
>
> diff --git a/dma.h b/dma.h
> index 79be131..d7428df 100644
> --- a/dma.h
> +++ b/dma.h
> @@ -28,6 +28,93 @@ typedef enum {
> DMA_DIRECTION_FROM_DEVICE = 1,
> } DMADirection;
>
> +typedef struct DMAContext {
why do we need the empty struct? Someone
will allocate an instance of it?
If not,
typedef struct DMAContext DMAContext;
in qemu-common.h would be enough.
> +
> +typedef void DMAInvalidateMapFunc(void *);
> +
> +static inline int dma_memory_rw(DMAContext *dma, dma_addr_t addr,
> + void *buf, dma_addr_t len, DMADirection dir)
> +{
> + cpu_physical_memory_rw(addr, buf, (target_phys_addr_t)len,
> + dir == DMA_DIRECTION_FROM_DEVICE);
> + return 0;
> +}
> +
> +static inline int dma_memory_read(DMAContext *dma, dma_addr_t addr,
> + void *buf, dma_addr_t len)
> +{
> + return dma_memory_rw(dma, addr, buf, len, DMA_DIRECTION_TO_DEVICE);
> +}
> +
> +static inline int dma_memory_write(DMAContext *dma, dma_addr_t addr,
> + const void *buf, dma_addr_t len)
> +{
> + return dma_memory_rw(dma, addr, (void *)buf, len,
> + DMA_DIRECTION_FROM_DEVICE);
> +}
> +
> +static inline void *dma_memory_map(DMAContext *dma,
> + DMAInvalidateMapFunc *cb, void *opaque,
> + dma_addr_t addr, dma_addr_t *len,
> + DMADirection dir)
> +{
> + target_phys_addr_t xlen = *len;
> + void *p;
> +
> + p = cpu_physical_memory_map(addr, &xlen,
> + dir == DMA_DIRECTION_FROM_DEVICE);
> + *len = xlen;
> + return p;
> +}
> +
> +static inline void dma_memory_unmap(DMAContext *dma,
> + void *buffer, dma_addr_t len,
> + DMADirection dir, dma_addr_t access_len)
> +{
> + return cpu_physical_memory_unmap(buffer, (target_phys_addr_t)len,
> + dir == DMA_DIRECTION_FROM_DEVICE,
> + access_len);
> +}
> +
> +#define DEFINE_LDST_DMA(_lname, _sname, _bits, _end) \
> + static inline uint##_bits##_t ld##_lname##_##_end##_dma(DMAContext *dma, \
> + dma_addr_t addr) \
> + { \
> + uint##_bits##_t val; \
> + dma_memory_read(dma, addr, &val, (_bits) / 8); \
> + return _end##_bits##_to_cpu(val); \
> + } \
> + static inline void st##_sname##_##_end##_dma(DMAContext *dma, \
> + dma_addr_t addr, \
> + uint##_bits##_t val) \
> + { \
> + val = cpu_to_##_end##_bits(val); \
> + dma_memory_write(dma, addr, &val, (_bits) / 8); \
> + }
> +
> +static inline uint8_t ldub_dma(DMAContext *dma, dma_addr_t addr)
> +{
> + uint8_t val;
> +
> + dma_memory_read(dma, addr, &val, 1);
> + return val;
> +}
> +
> +static inline void stb_dma(DMAContext *dma, dma_addr_t addr, uint8_t val)
> +{
> + dma_memory_write(dma, addr, &val, 1);
> +}
> +
> +DEFINE_LDST_DMA(uw, w, 16, le);
> +DEFINE_LDST_DMA(l, l, 32, le);
> +DEFINE_LDST_DMA(q, q, 64, le);
> +DEFINE_LDST_DMA(uw, w, 16, be);
> +DEFINE_LDST_DMA(l, l, 32, be);
> +DEFINE_LDST_DMA(q, q, 64, be);
> +
> +#undef DEFINE_LDST_DMA
> +
> struct ScatterGatherEntry {
> dma_addr_t base;
> dma_addr_t len;
> diff --git a/hw/pci.h b/hw/pci.h
> index ee53f26..64734a1 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -544,10 +544,15 @@ static inline uint32_t pci_config_size(const PCIDevice *d)
> }
>
> /* DMA access functions */
> +static inline DMAContext *pci_dma_context(PCIDevice *dev)
> +{
> + return NULL;
This is a stub, right? Pls add a comment.
> +}
> +
> static inline int pci_dma_rw(PCIDevice *dev, dma_addr_t addr,
> void *buf, dma_addr_t len, DMADirection dir)
> {
> - cpu_physical_memory_rw(addr, buf, len, dir == DMA_DIRECTION_FROM_DEVICE);
> + dma_memory_rw(pci_dma_context(dev), addr, buf, len, dir);
> return 0;
> }
>
> @@ -567,12 +572,12 @@ static inline int pci_dma_write(PCIDevice *dev, dma_addr_t addr,
> static inline uint##_bits##_t ld##_l##_pci_dma(PCIDevice *dev, \
> dma_addr_t addr) \
> { \
> - return ld##_l##_phys(addr); \
> + return ld##_l##_dma(pci_dma_context(dev), addr); \
> } \
> static inline void st##_s##_pci_dma(PCIDevice *dev, \
> - dma_addr_t addr, uint##_bits##_t val) \
> + dma_addr_t addr, uint##_bits##_t val) \
> { \
> - st##_s##_phys(addr, val); \
> + st##_s##_dma(pci_dma_context(dev), addr, val); \
> }
>
> PCI_DMA_DEFINE_LDST(ub, b, 8);
> @@ -586,21 +591,19 @@ PCI_DMA_DEFINE_LDST(q_be, q_be, 64);
> #undef PCI_DMA_DEFINE_LDST
>
> static inline void *pci_dma_map(PCIDevice *dev, dma_addr_t addr,
> + DMAInvalidateMapFunc *cb, void *opaque,
> dma_addr_t *plen, DMADirection dir)
> {
> - target_phys_addr_t len = *plen;
> void *buf;
>
> - buf = cpu_physical_memory_map(addr, &len, dir == DMA_DIRECTION_FROM_DEVICE);
> - *plen = len;
> + buf = dma_memory_map(pci_dma_context(dev), cb, opaque, addr, plen, dir);
> return buf;
> }
>
> static inline void pci_dma_unmap(PCIDevice *dev, void *buffer, dma_addr_t len,
> DMADirection dir, dma_addr_t access_len)
> {
> - cpu_physical_memory_unmap(buffer, len, dir == DMA_DIRECTION_FROM_DEVICE,
> - access_len);
> + dma_memory_unmap(pci_dma_context(dev), buffer, len, dir, access_len);
> }
>
> static inline void pci_dma_sglist_init(QEMUSGList *qsg, PCIDevice *dev,
> --
> 1.7.9
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] Add universal DMA helper functions
2012-02-26 10:04 ` Michael S. Tsirkin
@ 2012-02-26 21:09 ` Eduard - Gabriel Munteanu
2012-02-27 0:22 ` David Gibson
1 sibling, 0 replies; 18+ messages in thread
From: Eduard - Gabriel Munteanu @ 2012-02-26 21:09 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Joerg Rodel, Richard Henderson, qemu-devel, anthony, David Gibson
On Sun, Feb 26, 2012 at 12:04:49PM +0200, Michael S. Tsirkin wrote:
> On Fri, Feb 24, 2012 at 02:27:39PM +1100, David Gibson wrote:
> > Not that long ago, every device implementation using DMA directly
> > accessed guest memory using cpu_physical_memory_*(). This meant that
> > adding support for a guest visible IOMMU would require changing every
> > one of these devices to go through IOMMU translation.
> >
> > Shortly before qemu 1.0, I made a start on fixing this by providing
> > helper functions for PCI DMA. These are currently just stubs which
> > call the direct access functions, but mean that an IOMMU can be
> > implemented in one place, rather than for every PCI device.
> >
> > Clearly, this doesn't help for non PCI devices, which could also be
> > IOMMU translated on some platforms. It is also problematic for the
> > devices which have both PCI and non-PCI version (e.g. OHCI, AHCI) - we
> > cannot use the the pci_dma_*() functions, because they assume the
> > presence of a PCIDevice, but we don't want to have to check between
> > pci_dma_*() and cpu_physical_memory_*() every time we do a DMA in the
> > device code.
> >
> > This patch makes the first step on addressing both these problems, by
> > introducing new (stub) dma helper functions which can be used for any
> > DMA capable device.
> >
> > These dma functions take a DMAContext *, a new (currently empty)
> > variable describing the DMA address space in which the operation is to
> > take place. NULL indicates untranslated DMA directly into guest
> > physical address space. The intention is that in future non-NULL
> > values will given information about any necessary IOMMU translation.
> >
> > DMA using devices must obtain a DMAContext (or, potentially, contexts)
> > from their bus or platform. For now this patch just converts the PCI
> > wrappers to be implemented in terms of the universal wrappers,
> > converting other drivers can take place over time.
> >
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Joerg Rodel <Joerg.Rodel@amd.com>
> > Cc: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
> > Cc: Richard Henderson <rth@twiddle.net>
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>
> I'm a bit confused with all the stubbing going on.
> Is this the final form of the pci_* functions or just
> a stub? If the final form, we probably should just
> open-code them - they don't buy us much.
> If not, let's add a comment?
>
It is a stub. The final form needs to deal with address translation,
permissions checking and invalidating memory maps.
[snip]
Eduard
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] Add universal DMA helper functions
2012-02-24 3:27 ` [Qemu-devel] [PATCH 4/8] Add universal " David Gibson
2012-02-26 10:04 ` Michael S. Tsirkin
@ 2012-02-26 21:24 ` Eduard - Gabriel Munteanu
1 sibling, 0 replies; 18+ messages in thread
From: Eduard - Gabriel Munteanu @ 2012-02-26 21:24 UTC (permalink / raw)
To: David Gibson
Cc: Joerg Rodel, Richard Henderson, qemu-devel, anthony, Michael S. Tsirkin
On Fri, Feb 24, 2012 at 02:27:39PM +1100, David Gibson wrote:
> Not that long ago, every device implementation using DMA directly
> accessed guest memory using cpu_physical_memory_*(). This meant that
> adding support for a guest visible IOMMU would require changing every
> one of these devices to go through IOMMU translation.
>
> Shortly before qemu 1.0, I made a start on fixing this by providing
> helper functions for PCI DMA. These are currently just stubs which
> call the direct access functions, but mean that an IOMMU can be
> implemented in one place, rather than for every PCI device.
>
> Clearly, this doesn't help for non PCI devices, which could also be
> IOMMU translated on some platforms. It is also problematic for the
> devices which have both PCI and non-PCI version (e.g. OHCI, AHCI) - we
> cannot use the the pci_dma_*() functions, because they assume the
> presence of a PCIDevice, but we don't want to have to check between
> pci_dma_*() and cpu_physical_memory_*() every time we do a DMA in the
> device code.
>
> This patch makes the first step on addressing both these problems, by
> introducing new (stub) dma helper functions which can be used for any
> DMA capable device.
>
> These dma functions take a DMAContext *, a new (currently empty)
> variable describing the DMA address space in which the operation is to
> take place. NULL indicates untranslated DMA directly into guest
> physical address space. The intention is that in future non-NULL
> values will given information about any necessary IOMMU translation.
>
> DMA using devices must obtain a DMAContext (or, potentially, contexts)
> from their bus or platform. For now this patch just converts the PCI
> wrappers to be implemented in terms of the universal wrappers,
> converting other drivers can take place over time.
>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Joerg Rodel <Joerg.Rodel@amd.com>
> Cc: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
> Cc: Richard Henderson <rth@twiddle.net>
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Hi,
Thanks for pushing this forward. I got caught up in other stuff, perhaps
I'll be able to beat you to submitting the actual implementation soon. :-)
[snip]
Eduard
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 8/8] Make dma_addr_t 64 bit always
2012-02-24 4:57 ` David Gibson
@ 2012-02-26 21:46 ` Blue Swirl
2012-02-27 12:58 ` David Gibson
0 siblings, 1 reply; 18+ messages in thread
From: Blue Swirl @ 2012-02-26 21:46 UTC (permalink / raw)
To: anthony, qemu-devel
On Fri, Feb 24, 2012 at 04:57, David Gibson <david@gibson.dropbear.id.au> wrote:
> On Fri, Feb 24, 2012 at 02:27:43PM +1100, David Gibson wrote:
>
> Oops, ignore this one folks. As is probably obvious, this was a
> testing patch not meant to go into the main series.
Actually I'm not sure what would be the correct way to calculate the
size. For example, on Sparc32 the virtual address space and CPU
registers are 32 bits, physical address space 36 bits, but device
virtual memory address space (DVMA, used by devices to talk to IOMMU)
is only 32 bits.
>> ---
>> dma.h | 9 ++++++---
>> 1 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/dma.h b/dma.h
>> index b8cfd1d..ee540f4 100644
>> --- a/dma.h
>> +++ b/dma.h
>> @@ -18,10 +18,13 @@
>> typedef struct ScatterGatherEntry ScatterGatherEntry;
>>
>> #if defined(TARGET_PHYS_ADDR_BITS)
>> -typedef target_phys_addr_t dma_addr_t;
>> +/* Bus addresses can be different size from CPU physical addresses,
>> + * and indeed they can be different on different busses. So make
>> + * these always 64-bit which should handle every usual case */
>> +typedef uint64_t dma_addr_t;
>>
>> -#define DMA_ADDR_BITS TARGET_PHYS_ADDR_BITS
>> -#define DMA_ADDR_FMT TARGET_FMT_plx
>> +#define DMA_ADDR_BITS 64
>> +#define DMA_ADDR_FMT "%" PRIx64
>>
>> typedef enum {
>> DMA_DIRECTION_TO_DEVICE = 0,
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] Add universal DMA helper functions
2012-02-26 10:04 ` Michael S. Tsirkin
2012-02-26 21:09 ` Eduard - Gabriel Munteanu
@ 2012-02-27 0:22 ` David Gibson
2012-02-27 10:33 ` Michael S. Tsirkin
1 sibling, 1 reply; 18+ messages in thread
From: David Gibson @ 2012-02-27 0:22 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Joerg Rodel, Richard Henderson, qemu-devel, anthony,
Eduard - Gabriel Munteanu
On Sun, Feb 26, 2012 at 12:04:49PM +0200, Michael S. Tsirkin wrote:
> On Fri, Feb 24, 2012 at 02:27:39PM +1100, David Gibson wrote:
> > Not that long ago, every device implementation using DMA directly
> > accessed guest memory using cpu_physical_memory_*(). This meant that
> > adding support for a guest visible IOMMU would require changing every
> > one of these devices to go through IOMMU translation.
> >
> > Shortly before qemu 1.0, I made a start on fixing this by providing
> > helper functions for PCI DMA. These are currently just stubs which
> > call the direct access functions, but mean that an IOMMU can be
> > implemented in one place, rather than for every PCI device.
> >
> > Clearly, this doesn't help for non PCI devices, which could also be
> > IOMMU translated on some platforms. It is also problematic for the
> > devices which have both PCI and non-PCI version (e.g. OHCI, AHCI) - we
> > cannot use the the pci_dma_*() functions, because they assume the
> > presence of a PCIDevice, but we don't want to have to check between
> > pci_dma_*() and cpu_physical_memory_*() every time we do a DMA in the
> > device code.
> >
> > This patch makes the first step on addressing both these problems, by
> > introducing new (stub) dma helper functions which can be used for any
> > DMA capable device.
> >
> > These dma functions take a DMAContext *, a new (currently empty)
> > variable describing the DMA address space in which the operation is to
> > take place. NULL indicates untranslated DMA directly into guest
> > physical address space. The intention is that in future non-NULL
> > values will given information about any necessary IOMMU translation.
> >
> > DMA using devices must obtain a DMAContext (or, potentially, contexts)
> > from their bus or platform. For now this patch just converts the PCI
> > wrappers to be implemented in terms of the universal wrappers,
> > converting other drivers can take place over time.
> >
> > Cc: Michael S. Tsirkin <mst@redhat.com>
> > Cc: Joerg Rodel <Joerg.Rodel@amd.com>
> > Cc: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
> > Cc: Richard Henderson <rth@twiddle.net>
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>
> I'm a bit confused with all the stubbing going on.
> Is this the final form of the pci_* functions or just
> a stub? If the final form, we probably should just
> open-code them - they don't buy us much.
> If not, let's add a comment?
Well.. it's the intended final form of pci_dma_*() - which do become
trivial wrappers, yes. It's _not_ the intended final form of dma_*(),
which need to grow code to do actual IOMMU translation. I'll add a
comment about this in the next round.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] Add universal DMA helper functions
2012-02-27 0:22 ` David Gibson
@ 2012-02-27 10:33 ` Michael S. Tsirkin
2012-02-27 11:55 ` Eduard - Gabriel Munteanu
0 siblings, 1 reply; 18+ messages in thread
From: Michael S. Tsirkin @ 2012-02-27 10:33 UTC (permalink / raw)
To: anthony, qemu-devel, Joerg Rodel, Eduard - Gabriel Munteanu,
Richard Henderson
On Mon, Feb 27, 2012 at 11:22:43AM +1100, David Gibson wrote:
> On Sun, Feb 26, 2012 at 12:04:49PM +0200, Michael S. Tsirkin wrote:
> > On Fri, Feb 24, 2012 at 02:27:39PM +1100, David Gibson wrote:
> > > Not that long ago, every device implementation using DMA directly
> > > accessed guest memory using cpu_physical_memory_*(). This meant that
> > > adding support for a guest visible IOMMU would require changing every
> > > one of these devices to go through IOMMU translation.
> > >
> > > Shortly before qemu 1.0, I made a start on fixing this by providing
> > > helper functions for PCI DMA. These are currently just stubs which
> > > call the direct access functions, but mean that an IOMMU can be
> > > implemented in one place, rather than for every PCI device.
> > >
> > > Clearly, this doesn't help for non PCI devices, which could also be
> > > IOMMU translated on some platforms. It is also problematic for the
> > > devices which have both PCI and non-PCI version (e.g. OHCI, AHCI) - we
> > > cannot use the the pci_dma_*() functions, because they assume the
> > > presence of a PCIDevice, but we don't want to have to check between
> > > pci_dma_*() and cpu_physical_memory_*() every time we do a DMA in the
> > > device code.
> > >
> > > This patch makes the first step on addressing both these problems, by
> > > introducing new (stub) dma helper functions which can be used for any
> > > DMA capable device.
> > >
> > > These dma functions take a DMAContext *, a new (currently empty)
> > > variable describing the DMA address space in which the operation is to
> > > take place. NULL indicates untranslated DMA directly into guest
> > > physical address space. The intention is that in future non-NULL
> > > values will given information about any necessary IOMMU translation.
> > >
> > > DMA using devices must obtain a DMAContext (or, potentially, contexts)
> > > from their bus or platform. For now this patch just converts the PCI
> > > wrappers to be implemented in terms of the universal wrappers,
> > > converting other drivers can take place over time.
> > >
> > > Cc: Michael S. Tsirkin <mst@redhat.com>
> > > Cc: Joerg Rodel <Joerg.Rodel@amd.com>
> > > Cc: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
> > > Cc: Richard Henderson <rth@twiddle.net>
> > >
> > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> >
> > I'm a bit confused with all the stubbing going on.
> > Is this the final form of the pci_* functions or just
> > a stub? If the final form, we probably should just
> > open-code them - they don't buy us much.
> > If not, let's add a comment?
>
> Well.. it's the intended final form of pci_dma_*() - which do become
> trivial wrappers, yes.
I'd say let's drop them then (in a follow-up patch). The topic is
confusing enough without having to wade through layers of wrappers :)
> It's _not_ the intended final form of dma_*(),
> which need to grow code to do actual IOMMU translation. I'll add a
> comment about this in the next round.
>
> --
> David Gibson | I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 4/8] Add universal DMA helper functions
2012-02-27 10:33 ` Michael S. Tsirkin
@ 2012-02-27 11:55 ` Eduard - Gabriel Munteanu
0 siblings, 0 replies; 18+ messages in thread
From: Eduard - Gabriel Munteanu @ 2012-02-27 11:55 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Joerg Rodel, qemu-devel, anthony, Richard Henderson
On Mon, Feb 27, 2012 at 12:33:49PM +0200, Michael S. Tsirkin wrote:
> On Mon, Feb 27, 2012 at 11:22:43AM +1100, David Gibson wrote:
> > On Sun, Feb 26, 2012 at 12:04:49PM +0200, Michael S. Tsirkin wrote:
> > > On Fri, Feb 24, 2012 at 02:27:39PM +1100, David Gibson wrote:
[snip]
> > >
> > > I'm a bit confused with all the stubbing going on.
> > > Is this the final form of the pci_* functions or just
> > > a stub? If the final form, we probably should just
> > > open-code them - they don't buy us much.
> > > If not, let's add a comment?
> >
> > Well.. it's the intended final form of pci_dma_*() - which do become
> > trivial wrappers, yes.
>
> I'd say let's drop them then (in a follow-up patch). The topic is
> confusing enough without having to wade through layers of wrappers :)
>
Drop them how? Using dma_* stuff directly? That might work, but I
remember others suggesting we should use a specialized PCI wrapper.
Perhaps it makes sense if some other bus, or PCI itself at some point,
needs to do something special.
> > It's _not_ the intended final form of dma_*(),
> > which need to grow code to do actual IOMMU translation. I'll add a
> > comment about this in the next round.
> >
> > --
> > David Gibson | I'll have my music baroque, and my code
> > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
> > | _way_ _around_!
> > http://www.ozlabs.org/~dgibson
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH 8/8] Make dma_addr_t 64 bit always
2012-02-26 21:46 ` Blue Swirl
@ 2012-02-27 12:58 ` David Gibson
0 siblings, 0 replies; 18+ messages in thread
From: David Gibson @ 2012-02-27 12:58 UTC (permalink / raw)
To: Blue Swirl; +Cc: qemu-devel, anthony
On Sun, Feb 26, 2012 at 09:46:52PM +0000, Blue Swirl wrote:
> On Fri, Feb 24, 2012 at 04:57, David Gibson <david@gibson.dropbear.id.au> wrote:
> > On Fri, Feb 24, 2012 at 02:27:43PM +1100, David Gibson wrote:
> >
> > Oops, ignore this one folks. As is probably obvious, this was a
> > testing patch not meant to go into the main series.
>
> Actually I'm not sure what would be the correct way to calculate the
> size. For example, on Sparc32 the virtual address space and CPU
> registers are 32 bits, physical address space 36 bits, but device
> virtual memory address space (DVMA, used by devices to talk to IOMMU)
> is only 32 bits.
Well, yes, in fact I think we probably do need to use 64-bit addresses
always (or at least whenever we have any kind of IOMMU support
available), because it's almost impossible to be certain that a
platform could never have a 64-bit bus of some kind plugged in.
But that's a matter for another day.
--
David Gibson | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2012-02-27 13:31 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-02-24 3:27 [Qemu-devel] [0/8] RFC: A second batch of preliminaries towards guest visible IOMMUS David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 1/8] Use DMADirection type for dma_bdrv_io David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 2/8] Better support for dma_addr_t variables David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 3/8] usb-xhci: Use PCI DMA helper functions David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 4/8] Add universal " David Gibson
2012-02-26 10:04 ` Michael S. Tsirkin
2012-02-26 21:09 ` Eduard - Gabriel Munteanu
2012-02-27 0:22 ` David Gibson
2012-02-27 10:33 ` Michael S. Tsirkin
2012-02-27 11:55 ` Eduard - Gabriel Munteanu
2012-02-26 21:24 ` Eduard - Gabriel Munteanu
2012-02-24 3:27 ` [Qemu-devel] [PATCH 5/8] usb-ohci: Use " David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 6/8] Make sglists and dma_bdrv helpers use new universal DMA herlpers David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 7/8] ide/ahci: Use universal DMA helper functions David Gibson
2012-02-24 3:27 ` [Qemu-devel] [PATCH 8/8] Make dma_addr_t 64 bit always David Gibson
2012-02-24 4:57 ` David Gibson
2012-02-26 21:46 ` Blue Swirl
2012-02-27 12:58 ` David Gibson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.