* [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3)
@ 2011-10-31 6:06 David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 01/14] Define DMA address and direction types David Gibson
` (13 more replies)
0 siblings, 14 replies; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
A while back, Eduard - Gabriel Munteanu send a series of patches
implementing support for emulating the AMD IOMMU in conjunction with
qemu emulated PCI devices. A revised patch series added support for
the Intel IOMMU, and I also send a revised version of this series
which added support for the hypervisor mediated IOMMU on the pseries
machine.
Richard Henderson also weighed in on the discussion, and there's still
a cretain amount to be thrashed out in terms of exactly how to set up
an IOMMU / DMA translation subsystem.
However, really only 2 or 3 patches in any of these series have
contained anything interesting. The rest of the series has been
converting existing PCI emulated devices to use the new DMA interface
which worked through the IOMMU translation, whatever it was. While we
keep working out what we want for the guts of the IOMMU support, these
device conversion patches keep bitrotting against updates to the
various device implementations themselves.
Really, regardless of whether we're actually implementing IOMMU
translation, it makes sense that qemu code should distinguish between
when it is really operating in CPU physical addresses and when it is
operating in bus or DMA addresses which might have some kind of
translation into physical addresses.
This series, therefore, begins the conversion of existing PCI device
emulation code to use new (stub) pci dma access functions. These are,
for now, just defined to be untranslated cpu physical memory accesses,
as before, but has three advantages:
* It becomes obvious where the code is working with dma addresses,
so it's easier to grep for what might be affected by an IOMMU or
other bus address translation.
* The new stubs take the PCIDevice *, from which any of the various
suggested IOMMU interfaces should be able to locate the correct
IOMMU translation context.
* The new pci_dma_{read,write}() functions have a return value.
When we do have IOMMU support, translation failures could lead to
these functions failing, so we want a way to report it.
This series converts all the easy cases. It doesn't yet handle
devices which have both PCI and non-PCI variants, such as AHCI, OHCI
and ne2k. Unlike the earlier version of this series, functions using
scatter gather _are_ covered, though.
Changes since v2:
* Moved the stub functions from pci.c to pci.h, making them inlines
* Changed the stX/ldX_pci_dma() functions call stX/ldX_phys()
directly, rather than using pci_dma_rw(). This means that they
should access the host memory atomically, previously this was an
unstated and dangerous semantic change.
* Cleaned up and split out definition of dma_addr_t, DMADirection and
their use in the scatter/gather code.
* Added a DMA_ADDR_FMT define for using dma_addr_t values in printf()
and used this in some of the driver updates.
^ permalink raw reply [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 01/14] Define DMA address and direction types
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
@ 2011-10-31 6:06 ` David Gibson
2011-11-01 22:21 ` Anthony Liguori
2011-10-31 6:06 ` [Qemu-devel] [PATCH 02/14] Use dma_addr_t type for scatter/gather code David Gibson
` (12 subsequent siblings)
13 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
As a preliminary to adding more extensive DMA and IOMMU infrastructure
support into qemu, this patch defines a dma_addr_t for storing DMA bus
addresses and a DMADirection enum which describes whether a DMA is
from an external device to main memory or from main memory to an
external device.
For now dma_addr_t is just defined to be target_phys_addr_t, but in
future, we can change this to support machines where we have bus
addresses which don't necessarily have the same format as CPU physical
addresses.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
dma.h | 9 +++++++++
1 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/dma.h b/dma.h
index 2bdc236..56e163a 100644
--- a/dma.h
+++ b/dma.h
@@ -18,6 +18,15 @@
typedef struct ScatterGatherEntry ScatterGatherEntry;
#if defined(TARGET_PHYS_ADDR_BITS)
+typedef target_phys_addr_t dma_addr_t;
+
+#define DMA_ADDR_FMT TARGET_FMT_plx
+
+typedef enum {
+ DMA_DIRECTION_TO_DEVICE = 0,
+ DMA_DIRECTION_FROM_DEVICE = 1,
+} DMADirection;
+
struct ScatterGatherEntry {
target_phys_addr_t base;
target_phys_addr_t len;
--
1.7.7
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 02/14] Use dma_addr_t type for scatter/gather code
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 01/14] Define DMA address and direction types David Gibson
@ 2011-10-31 6:06 ` David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 03/14] Add stub functions for PCI device models to do PCI DMA David Gibson
` (11 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
This patch uses the newly created dma_addr_t type throughout the
scatter/gather handling code in dma-helpers.c whenever we need to
represent a dma bus address. This makes a better distinction as to
what is a bus address and what is a cpu physical address. Since we
don't support IOMMUs yet, they can't be very different for now, but
that will change in future, and this preliminary helps clarify what's
going on.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
dma-helpers.c | 5 ++---
dma.h | 9 ++++-----
2 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/dma-helpers.c b/dma-helpers.c
index 86d2d0a..bdcd38c 100644
--- a/dma-helpers.c
+++ b/dma-helpers.c
@@ -18,8 +18,7 @@ void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint)
qsg->size = 0;
}
-void qemu_sglist_add(QEMUSGList *qsg, target_phys_addr_t base,
- target_phys_addr_t len)
+void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, dma_addr_t len)
{
if (qsg->nsg == qsg->nalloc) {
qsg->nalloc = 2 * qsg->nalloc + 1;
@@ -45,7 +44,7 @@ typedef struct {
bool to_dev;
bool in_cancel;
int sg_cur_index;
- target_phys_addr_t sg_cur_byte;
+ dma_addr_t sg_cur_byte;
QEMUIOVector iov;
QEMUBH *bh;
DMAIOFunc *io_func;
diff --git a/dma.h b/dma.h
index 56e163a..a13209d 100644
--- a/dma.h
+++ b/dma.h
@@ -28,20 +28,19 @@ typedef enum {
} DMADirection;
struct ScatterGatherEntry {
- target_phys_addr_t base;
- target_phys_addr_t len;
+ dma_addr_t base;
+ dma_addr_t len;
};
struct QEMUSGList {
ScatterGatherEntry *sg;
int nsg;
int nalloc;
- target_phys_addr_t size;
+ dma_addr_t size;
};
void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint);
-void qemu_sglist_add(QEMUSGList *qsg, target_phys_addr_t base,
- target_phys_addr_t len);
+void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, dma_addr_t len);
void qemu_sglist_destroy(QEMUSGList *qsg);
#endif
--
1.7.7
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 03/14] Add stub functions for PCI device models to do PCI DMA
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 01/14] Define DMA address and direction types David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 02/14] Use dma_addr_t type for scatter/gather code David Gibson
@ 2011-10-31 6:06 ` David Gibson
2011-11-02 7:17 ` Michael S. Tsirkin
2011-10-31 6:06 ` [Qemu-devel] [PATCH 04/14] rtl8139: Use PCI DMA stub functions David Gibson
` (10 subsequent siblings)
13 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
This patch adds functions to pci.[ch] to perform PCI DMA operations.
At present, these are just stubs which perform directly cpu physical
memory accesses. Stubs are included which are analogous to
cpu_physical_memory_{read,write}(), the stX_phys() and ldX_phys()
functions and cpu_physical_memory_{map,unmap}().
In addition, a wrapper around qemu_sglist_init() is provided, which
also takes a PCIDevice *. It's assumed that _init() is the only
sglist function which will need wrapping, the idea being that once we
have IOMMU support whatever IOMMU context handle the wrapper derives
from the PCI device will be stored within the sglist structure for
later use.
Using these stubs, however, distinguishes PCI device DMA transactions from
other accesses to physical memory, which will allow PCI IOMMU support to
be added in one place, rather than updating every PCI driver at that time.
That is, it allows us to update individual PCI drivers to support an IOMMU
without having yet determined the details of how the IOMMU emulation will
operate. This will let us remove the most bitrot-sensitive part of an
IOMMU patch in advance.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/pci.h | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 67 insertions(+), 0 deletions(-)
diff --git a/hw/pci.h b/hw/pci.h
index 86a81c8..c449a90 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -6,6 +6,7 @@
#include "qdev.h"
#include "memory.h"
+#include "dma.h"
/* PCI includes legacy ISA access. */
#include "isa.h"
@@ -487,4 +488,70 @@ static inline uint32_t pci_config_size(const PCIDevice *d)
return pci_is_express(d) ? PCIE_CONFIG_SPACE_SIZE : PCI_CONFIG_SPACE_SIZE;
}
+/* DMA access functions */
+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);
+ return 0;
+}
+
+static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr,
+ void *buf, dma_addr_t len)
+{
+ return pci_dma_rw(dev, addr, buf, len, DMA_DIRECTION_TO_DEVICE);
+}
+
+static inline int pci_dma_write(PCIDevice *dev, dma_addr_t addr,
+ const void *buf, dma_addr_t len)
+{
+ return pci_dma_rw(dev, addr, (void *) buf, len, DMA_DIRECTION_FROM_DEVICE);
+}
+
+#define PCI_DMA_DEFINE_LDST(_l, _s, _bits) \
+ static inline uint##_bits##_t ld##_l##_pci_dma(PCIDevice *dev, \
+ dma_addr_t addr) \
+ { \
+ return ld##_l##_phys(addr); \
+ } \
+ static inline void st##_s##_pci_dma(PCIDevice *dev, \
+ dma_addr_t addr, uint##_bits##_t val) \
+ { \
+ st##_s##_phys(addr, val); \
+ }
+
+PCI_DMA_DEFINE_LDST(ub, b, 8);
+PCI_DMA_DEFINE_LDST(uw_le, w_le, 16)
+PCI_DMA_DEFINE_LDST(l_le, l_le, 32);
+PCI_DMA_DEFINE_LDST(q_le, q_le, 64);
+PCI_DMA_DEFINE_LDST(uw_be, w_be, 16)
+PCI_DMA_DEFINE_LDST(l_be, l_be, 32);
+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,
+ 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;
+ 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);
+}
+
+static inline void pci_dma_sglist_init(QEMUSGList *qsg, PCIDevice *dev,
+ int alloc_hint)
+{
+ qemu_sglist_init(qsg, alloc_hint);
+}
+
#endif
--
1.7.7
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 04/14] rtl8139: Use PCI DMA stub functions
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
` (2 preceding siblings ...)
2011-10-31 6:06 ` [Qemu-devel] [PATCH 03/14] Add stub functions for PCI device models to do PCI DMA David Gibson
@ 2011-10-31 6:06 ` David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 05/14] eepro100: " David Gibson
` (9 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
From: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
This updates the rtl8139 device emulation to use the explicit PCI DMA
functions, instead of directly calling physical memory access functions.
Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/rtl8139.c | 106 +++++++++++++++++++++++++++++----------------------------
1 files changed, 54 insertions(+), 52 deletions(-)
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 3753950..4c37993 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -53,6 +53,7 @@
#include "hw.h"
#include "pci.h"
+#include "dma.h"
#include "qemu-timer.h"
#include "net.h"
#include "loader.h"
@@ -427,9 +428,6 @@ typedef struct RTL8139TallyCounters
/* Clears all tally counters */
static void RTL8139TallyCounters_clear(RTL8139TallyCounters* counters);
-/* Writes tally counters to specified physical memory address */
-static void RTL8139TallyCounters_physical_memory_write(target_phys_addr_t tc_addr, RTL8139TallyCounters* counters);
-
typedef struct RTL8139State {
PCIDevice dev;
uint8_t phys[8]; /* mac address */
@@ -512,6 +510,9 @@ typedef struct RTL8139State {
int rtl8139_mmio_io_addr_dummy;
} RTL8139State;
+/* Writes tally counters to memory via DMA */
+static void RTL8139TallyCounters_dma_write(RTL8139State *s, dma_addr_t tc_addr);
+
static void rtl8139_set_next_tctr_time(RTL8139State *s, int64_t current_time);
static void prom9346_decode_command(EEprom9346 *eeprom, uint8_t command)
@@ -773,15 +774,15 @@ static void rtl8139_write_buffer(RTL8139State *s, const void *buf, int size)
if (size > wrapped)
{
- cpu_physical_memory_write( s->RxBuf + s->RxBufAddr,
- buf, size-wrapped );
+ pci_dma_write(&s->dev, s->RxBuf + s->RxBufAddr,
+ buf, size-wrapped);
}
/* reset buffer pointer */
s->RxBufAddr = 0;
- cpu_physical_memory_write( s->RxBuf + s->RxBufAddr,
- buf + (size-wrapped), wrapped );
+ pci_dma_write(&s->dev, s->RxBuf + s->RxBufAddr,
+ buf + (size-wrapped), wrapped);
s->RxBufAddr = wrapped;
@@ -790,13 +791,13 @@ static void rtl8139_write_buffer(RTL8139State *s, const void *buf, int size)
}
/* non-wrapping path or overwrapping enabled */
- cpu_physical_memory_write( s->RxBuf + s->RxBufAddr, buf, size );
+ pci_dma_write(&s->dev, s->RxBuf + s->RxBufAddr, buf, size);
s->RxBufAddr += size;
}
#define MIN_BUF_SIZE 60
-static inline target_phys_addr_t rtl8139_addr64(uint32_t low, uint32_t high)
+static inline dma_addr_t rtl8139_addr64(uint32_t low, uint32_t high)
{
#if TARGET_PHYS_ADDR_BITS > 32
return low | ((target_phys_addr_t)high << 32);
@@ -979,24 +980,24 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
/* w3 high 32bit of Rx buffer ptr */
int descriptor = s->currCPlusRxDesc;
- target_phys_addr_t cplus_rx_ring_desc;
+ dma_addr_t cplus_rx_ring_desc;
cplus_rx_ring_desc = rtl8139_addr64(s->RxRingAddrLO, s->RxRingAddrHI);
cplus_rx_ring_desc += 16 * descriptor;
DPRINTF("+++ C+ mode reading RX descriptor %d from host memory at "
- "%08x %08x = "TARGET_FMT_plx"\n", descriptor, s->RxRingAddrHI,
+ "%08x %08x = "DMA_ADDR_FMT"\n", descriptor, s->RxRingAddrHI,
s->RxRingAddrLO, cplus_rx_ring_desc);
uint32_t val, rxdw0,rxdw1,rxbufLO,rxbufHI;
- cpu_physical_memory_read(cplus_rx_ring_desc, (uint8_t *)&val, 4);
+ pci_dma_read(&s->dev, cplus_rx_ring_desc, (uint8_t *)&val, 4);
rxdw0 = le32_to_cpu(val);
- cpu_physical_memory_read(cplus_rx_ring_desc+4, (uint8_t *)&val, 4);
+ pci_dma_read(&s->dev, cplus_rx_ring_desc+4, (uint8_t *)&val, 4);
rxdw1 = le32_to_cpu(val);
- cpu_physical_memory_read(cplus_rx_ring_desc+8, (uint8_t *)&val, 4);
+ pci_dma_read(&s->dev, cplus_rx_ring_desc+8, (uint8_t *)&val, 4);
rxbufLO = le32_to_cpu(val);
- cpu_physical_memory_read(cplus_rx_ring_desc+12, (uint8_t *)&val, 4);
+ pci_dma_read(&s->dev, cplus_rx_ring_desc+12, (uint8_t *)&val, 4);
rxbufHI = le32_to_cpu(val);
DPRINTF("+++ C+ mode RX descriptor %d %08x %08x %08x %08x\n",
@@ -1060,16 +1061,16 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
return size_;
}
- target_phys_addr_t rx_addr = rtl8139_addr64(rxbufLO, rxbufHI);
+ dma_addr_t rx_addr = rtl8139_addr64(rxbufLO, rxbufHI);
/* receive/copy to target memory */
if (dot1q_buf) {
- cpu_physical_memory_write(rx_addr, buf, 2 * ETHER_ADDR_LEN);
- cpu_physical_memory_write(rx_addr + 2 * ETHER_ADDR_LEN,
- buf + 2 * ETHER_ADDR_LEN + VLAN_HLEN,
- size - 2 * ETHER_ADDR_LEN);
+ pci_dma_write(&s->dev, rx_addr, buf, 2 * ETHER_ADDR_LEN);
+ pci_dma_write(&s->dev, rx_addr + 2 * ETHER_ADDR_LEN,
+ buf + 2 * ETHER_ADDR_LEN + VLAN_HLEN,
+ size - 2 * ETHER_ADDR_LEN);
} else {
- cpu_physical_memory_write(rx_addr, buf, size);
+ pci_dma_write(&s->dev, rx_addr, buf, size);
}
if (s->CpCmd & CPlusRxChkSum)
@@ -1079,7 +1080,7 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
/* write checksum */
val = cpu_to_le32(crc32(0, buf, size_));
- cpu_physical_memory_write( rx_addr+size, (uint8_t *)&val, 4);
+ pci_dma_write(&s->dev, rx_addr+size, (uint8_t *)&val, 4);
/* first segment of received packet flag */
#define CP_RX_STATUS_FS (1<<29)
@@ -1125,9 +1126,9 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
/* update ring data */
val = cpu_to_le32(rxdw0);
- cpu_physical_memory_write(cplus_rx_ring_desc, (uint8_t *)&val, 4);
+ pci_dma_write(&s->dev, cplus_rx_ring_desc, (uint8_t *)&val, 4);
val = cpu_to_le32(rxdw1);
- cpu_physical_memory_write(cplus_rx_ring_desc+4, (uint8_t *)&val, 4);
+ pci_dma_write(&s->dev, cplus_rx_ring_desc+4, (uint8_t *)&val, 4);
/* update tally counter */
++s->tally_counters.RxOk;
@@ -1307,50 +1308,51 @@ static void RTL8139TallyCounters_clear(RTL8139TallyCounters* counters)
counters->TxUndrn = 0;
}
-static void RTL8139TallyCounters_physical_memory_write(target_phys_addr_t tc_addr, RTL8139TallyCounters* tally_counters)
+static void RTL8139TallyCounters_dma_write(RTL8139State *s, dma_addr_t tc_addr)
{
+ RTL8139TallyCounters *tally_counters = &s->tally_counters;
uint16_t val16;
uint32_t val32;
uint64_t val64;
val64 = cpu_to_le64(tally_counters->TxOk);
- cpu_physical_memory_write(tc_addr + 0, (uint8_t *)&val64, 8);
+ pci_dma_write(&s->dev, tc_addr + 0, (uint8_t *)&val64, 8);
val64 = cpu_to_le64(tally_counters->RxOk);
- cpu_physical_memory_write(tc_addr + 8, (uint8_t *)&val64, 8);
+ pci_dma_write(&s->dev, tc_addr + 8, (uint8_t *)&val64, 8);
val64 = cpu_to_le64(tally_counters->TxERR);
- cpu_physical_memory_write(tc_addr + 16, (uint8_t *)&val64, 8);
+ pci_dma_write(&s->dev, tc_addr + 16, (uint8_t *)&val64, 8);
val32 = cpu_to_le32(tally_counters->RxERR);
- cpu_physical_memory_write(tc_addr + 24, (uint8_t *)&val32, 4);
+ pci_dma_write(&s->dev, tc_addr + 24, (uint8_t *)&val32, 4);
val16 = cpu_to_le16(tally_counters->MissPkt);
- cpu_physical_memory_write(tc_addr + 28, (uint8_t *)&val16, 2);
+ pci_dma_write(&s->dev, tc_addr + 28, (uint8_t *)&val16, 2);
val16 = cpu_to_le16(tally_counters->FAE);
- cpu_physical_memory_write(tc_addr + 30, (uint8_t *)&val16, 2);
+ pci_dma_write(&s->dev, tc_addr + 30, (uint8_t *)&val16, 2);
val32 = cpu_to_le32(tally_counters->Tx1Col);
- cpu_physical_memory_write(tc_addr + 32, (uint8_t *)&val32, 4);
+ pci_dma_write(&s->dev, tc_addr + 32, (uint8_t *)&val32, 4);
val32 = cpu_to_le32(tally_counters->TxMCol);
- cpu_physical_memory_write(tc_addr + 36, (uint8_t *)&val32, 4);
+ pci_dma_write(&s->dev, tc_addr + 36, (uint8_t *)&val32, 4);
val64 = cpu_to_le64(tally_counters->RxOkPhy);
- cpu_physical_memory_write(tc_addr + 40, (uint8_t *)&val64, 8);
+ pci_dma_write(&s->dev, tc_addr + 40, (uint8_t *)&val64, 8);
val64 = cpu_to_le64(tally_counters->RxOkBrd);
- cpu_physical_memory_write(tc_addr + 48, (uint8_t *)&val64, 8);
+ pci_dma_write(&s->dev, tc_addr + 48, (uint8_t *)&val64, 8);
val32 = cpu_to_le32(tally_counters->RxOkMul);
- cpu_physical_memory_write(tc_addr + 56, (uint8_t *)&val32, 4);
+ pci_dma_write(&s->dev, tc_addr + 56, (uint8_t *)&val32, 4);
val16 = cpu_to_le16(tally_counters->TxAbt);
- cpu_physical_memory_write(tc_addr + 60, (uint8_t *)&val16, 2);
+ pci_dma_write(&s->dev, tc_addr + 60, (uint8_t *)&val16, 2);
val16 = cpu_to_le16(tally_counters->TxUndrn);
- cpu_physical_memory_write(tc_addr + 62, (uint8_t *)&val16, 2);
+ pci_dma_write(&s->dev, tc_addr + 62, (uint8_t *)&val16, 2);
}
/* Loads values of tally counters from VM state file */
@@ -1842,7 +1844,7 @@ static int rtl8139_transmit_one(RTL8139State *s, int descriptor)
DPRINTF("+++ transmit reading %d bytes from host memory at 0x%08x\n",
txsize, s->TxAddr[descriptor]);
- cpu_physical_memory_read(s->TxAddr[descriptor], txbuffer, txsize);
+ pci_dma_read(&s->dev, s->TxAddr[descriptor], txbuffer, txsize);
/* Mark descriptor as transferred */
s->TxStatus[descriptor] |= TxHostOwns;
@@ -1963,25 +1965,24 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
int descriptor = s->currCPlusTxDesc;
- target_phys_addr_t cplus_tx_ring_desc =
- rtl8139_addr64(s->TxAddr[0], s->TxAddr[1]);
+ dma_addr_t cplus_tx_ring_desc = rtl8139_addr64(s->TxAddr[0], s->TxAddr[1]);
/* Normal priority ring */
cplus_tx_ring_desc += 16 * descriptor;
DPRINTF("+++ C+ mode reading TX descriptor %d from host memory at "
- "%08x0x%08x = 0x"TARGET_FMT_plx"\n", descriptor, s->TxAddr[1],
+ "%08x0x%08x = 0x"DMA_ADDR_FMT"\n", descriptor, s->TxAddr[1],
s->TxAddr[0], cplus_tx_ring_desc);
uint32_t val, txdw0,txdw1,txbufLO,txbufHI;
- cpu_physical_memory_read(cplus_tx_ring_desc, (uint8_t *)&val, 4);
+ pci_dma_read(&s->dev, cplus_tx_ring_desc, (uint8_t *)&val, 4);
txdw0 = le32_to_cpu(val);
- cpu_physical_memory_read(cplus_tx_ring_desc+4, (uint8_t *)&val, 4);
+ pci_dma_read(&s->dev, cplus_tx_ring_desc+4, (uint8_t *)&val, 4);
txdw1 = le32_to_cpu(val);
- cpu_physical_memory_read(cplus_tx_ring_desc+8, (uint8_t *)&val, 4);
+ pci_dma_read(&s->dev, cplus_tx_ring_desc+8, (uint8_t *)&val, 4);
txbufLO = le32_to_cpu(val);
- cpu_physical_memory_read(cplus_tx_ring_desc+12, (uint8_t *)&val, 4);
+ pci_dma_read(&s->dev, cplus_tx_ring_desc+12, (uint8_t *)&val, 4);
txbufHI = le32_to_cpu(val);
DPRINTF("+++ C+ mode TX descriptor %d %08x %08x %08x %08x\n", descriptor,
@@ -2047,7 +2048,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
}
int txsize = txdw0 & CP_TX_BUFFER_SIZE_MASK;
- target_phys_addr_t tx_addr = rtl8139_addr64(txbufLO, txbufHI);
+ dma_addr_t tx_addr = rtl8139_addr64(txbufLO, txbufHI);
/* make sure we have enough space to assemble the packet */
if (!s->cplus_txbuffer)
@@ -2086,10 +2087,11 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
/* append more data to the packet */
DPRINTF("+++ C+ mode transmit reading %d bytes from host memory at "
- TARGET_FMT_plx" to offset %d\n", txsize, tx_addr,
- s->cplus_txbuffer_offset);
+ DMA_ADDR_FMT" to offset %d\n", txsize, tx_addr,
+ s->cplus_txbuffer_offset);
- cpu_physical_memory_read(tx_addr, s->cplus_txbuffer + s->cplus_txbuffer_offset, txsize);
+ pci_dma_read(&s->dev, tx_addr,
+ s->cplus_txbuffer + s->cplus_txbuffer_offset, txsize);
s->cplus_txbuffer_offset += txsize;
/* seek to next Rx descriptor */
@@ -2116,7 +2118,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
/* update ring data */
val = cpu_to_le32(txdw0);
- cpu_physical_memory_write(cplus_tx_ring_desc, (uint8_t *)&val, 4);
+ pci_dma_write(&s->dev, cplus_tx_ring_desc, (uint8_t *)&val, 4);
/* Now decide if descriptor being processed is holding the last segment of packet */
if (txdw0 & CP_TX_LS)
@@ -2475,7 +2477,7 @@ static void rtl8139_TxStatus_write(RTL8139State *s, uint32_t txRegOffset, uint32
target_phys_addr_t tc_addr = rtl8139_addr64(s->TxStatus[0] & ~0x3f, s->TxStatus[1]);
/* dump tally counters to specified memory location */
- RTL8139TallyCounters_physical_memory_write( tc_addr, &s->tally_counters);
+ RTL8139TallyCounters_dma_write(s, tc_addr);
/* mark dump completed */
s->TxStatus[0] &= ~0x8;
--
1.7.7
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
` (3 preceding siblings ...)
2011-10-31 6:06 ` [Qemu-devel] [PATCH 04/14] rtl8139: Use PCI DMA stub functions David Gibson
@ 2011-10-31 6:06 ` David Gibson
2011-10-31 16:45 ` Stefan Weil
2011-11-02 7:16 ` Michael S. Tsirkin
2011-10-31 6:06 ` [Qemu-devel] [PATCH 06/14] ac97: " David Gibson
` (8 subsequent siblings)
13 siblings, 2 replies; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
From: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
This updates the eepro100 device emulation to use the explicit PCI DMA
functions, instead of directly calling physical memory access functions.
Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
Signed-off-by: David Gibson <dwg@au1.ibm.com>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/eepro100.c | 121 +++++++++++++++++++++++----------------------------------
1 files changed, 49 insertions(+), 72 deletions(-)
diff --git a/hw/eepro100.c b/hw/eepro100.c
index 4e3c52f..7d59e71 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -46,6 +46,7 @@
#include "net.h"
#include "eeprom93xx.h"
#include "sysemu.h"
+#include "dma.h"
/* QEMU sends frames smaller than 60 bytes to ethernet nics.
* Such frames are rejected by real nics and their emulations.
@@ -315,38 +316,6 @@ static const uint16_t eepro100_mdi_mask[] = {
0xffff, 0xffff, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
};
-/* Read a 16 bit little endian value from physical memory. */
-static uint16_t e100_ldw_le_phys(target_phys_addr_t addr)
-{
- /* Load 16 bit (little endian) word from emulated hardware. */
- uint16_t val;
- cpu_physical_memory_read(addr, &val, sizeof(val));
- return le16_to_cpu(val);
-}
-
-/* Read a 32 bit little endian value from physical memory. */
-static uint32_t e100_ldl_le_phys(target_phys_addr_t addr)
-{
- /* Load 32 bit (little endian) word from emulated hardware. */
- uint32_t val;
- cpu_physical_memory_read(addr, &val, sizeof(val));
- return le32_to_cpu(val);
-}
-
-/* Write a 16 bit little endian value to physical memory. */
-static void e100_stw_le_phys(target_phys_addr_t addr, uint16_t val)
-{
- val = cpu_to_le16(val);
- cpu_physical_memory_write(addr, &val, sizeof(val));
-}
-
-/* Write a 32 bit little endian value to physical memory. */
-static void e100_stl_le_phys(target_phys_addr_t addr, uint32_t val)
-{
- val = cpu_to_le32(val);
- cpu_physical_memory_write(addr, &val, sizeof(val));
-}
-
#define POLYNOMIAL 0x04c11db6
/* From FreeBSD */
@@ -744,21 +713,26 @@ static void dump_statistics(EEPRO100State * s)
* values which really matter.
* Number of data should check configuration!!!
*/
- cpu_physical_memory_write(s->statsaddr, &s->statistics, s->stats_size);
- e100_stl_le_phys(s->statsaddr + 0, s->statistics.tx_good_frames);
- e100_stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
- e100_stl_le_phys(s->statsaddr + 48, s->statistics.rx_resource_errors);
- e100_stl_le_phys(s->statsaddr + 60, s->statistics.rx_short_frame_errors);
+ pci_dma_write(&s->dev, s->statsaddr,
+ (uint8_t *) &s->statistics, s->stats_size);
+ stl_le_pci_dma(&s->dev, s->statsaddr + 0,
+ s->statistics.tx_good_frames);
+ stl_le_pci_dma(&s->dev, s->statsaddr + 36,
+ s->statistics.rx_good_frames);
+ stl_le_pci_dma(&s->dev, s->statsaddr + 48,
+ s->statistics.rx_resource_errors);
+ stl_le_pci_dma(&s->dev, s->statsaddr + 60,
+ s->statistics.rx_short_frame_errors);
#if 0
- e100_stw_le_phys(s->statsaddr + 76, s->statistics.xmt_tco_frames);
- e100_stw_le_phys(s->statsaddr + 78, s->statistics.rcv_tco_frames);
+ stw_le_pci_dma(&s->dev, s->statsaddr + 76, s->statistics.xmt_tco_frames);
+ stw_le_pci_dma(&s->dev, s->statsaddr + 78, s->statistics.rcv_tco_frames);
missing("CU dump statistical counters");
#endif
}
static void read_cb(EEPRO100State *s)
{
- cpu_physical_memory_read(s->cb_address, &s->tx, sizeof(s->tx));
+ pci_dma_read(&s->dev, s->cb_address, (uint8_t *) &s->tx, sizeof(s->tx));
s->tx.status = le16_to_cpu(s->tx.status);
s->tx.command = le16_to_cpu(s->tx.command);
s->tx.link = le32_to_cpu(s->tx.link);
@@ -788,18 +762,17 @@ static void tx_command(EEPRO100State *s)
}
assert(tcb_bytes <= sizeof(buf));
while (size < tcb_bytes) {
- uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
- uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
+ uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev, tbd_address);
+ uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev, tbd_address + 4);
#if 0
- uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
+ uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
#endif
tbd_address += 8;
TRACE(RXTX, logout
("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n",
tx_buffer_address, tx_buffer_size));
tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
- cpu_physical_memory_read(tx_buffer_address, &buf[size],
- tx_buffer_size);
+ pci_dma_read(&s->dev, tx_buffer_address, &buf[size], tx_buffer_size);
size += tx_buffer_size;
}
if (tbd_array == 0xffffffff) {
@@ -810,16 +783,19 @@ static void tx_command(EEPRO100State *s)
if (s->has_extended_tcb_support && !(s->configuration[6] & BIT(4))) {
/* Extended Flexible TCB. */
for (; tbd_count < 2; tbd_count++) {
- uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
- uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
- uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
+ uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev,
+ tbd_address);
+ uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev,
+ tbd_address + 4);
+ uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev,
+ tbd_address + 6);
tbd_address += 8;
TRACE(RXTX, logout
("TBD (extended flexible mode): buffer address 0x%08x, size 0x%04x\n",
tx_buffer_address, tx_buffer_size));
tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
- cpu_physical_memory_read(tx_buffer_address, &buf[size],
- tx_buffer_size);
+ pci_dma_read(&s->dev, tx_buffer_address,
+ &buf[size], tx_buffer_size);
size += tx_buffer_size;
if (tx_buffer_el & 1) {
break;
@@ -828,16 +804,16 @@ static void tx_command(EEPRO100State *s)
}
tbd_address = tbd_array;
for (; tbd_count < s->tx.tbd_count; tbd_count++) {
- uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
- uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
- uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
+ uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev, tbd_address);
+ uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev, tbd_address + 4);
+ uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
tbd_address += 8;
TRACE(RXTX, logout
("TBD (flexible mode): buffer address 0x%08x, size 0x%04x\n",
tx_buffer_address, tx_buffer_size));
tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
- cpu_physical_memory_read(tx_buffer_address, &buf[size],
- tx_buffer_size);
+ pci_dma_read(&s->dev, tx_buffer_address,
+ &buf[size], tx_buffer_size);
size += tx_buffer_size;
if (tx_buffer_el & 1) {
break;
@@ -862,7 +838,7 @@ static void set_multicast_list(EEPRO100State *s)
TRACE(OTHER, logout("multicast list, multicast count = %u\n", multicast_count));
for (i = 0; i < multicast_count; i += 6) {
uint8_t multicast_addr[6];
- cpu_physical_memory_read(s->cb_address + 10 + i, multicast_addr, 6);
+ pci_dma_read(&s->dev, s->cb_address + 10 + i, multicast_addr, 6);
TRACE(OTHER, logout("multicast entry %s\n", nic_dump(multicast_addr, 6)));
unsigned mcast_idx = compute_mcast_idx(multicast_addr);
assert(mcast_idx < 64);
@@ -896,12 +872,12 @@ static void action_command(EEPRO100State *s)
/* Do nothing. */
break;
case CmdIASetup:
- cpu_physical_memory_read(s->cb_address + 8, &s->conf.macaddr.a[0], 6);
+ pci_dma_read(&s->dev, s->cb_address + 8, &s->conf.macaddr.a[0], 6);
TRACE(OTHER, logout("macaddr: %s\n", nic_dump(&s->conf.macaddr.a[0], 6)));
break;
case CmdConfigure:
- cpu_physical_memory_read(s->cb_address + 8, &s->configuration[0],
- sizeof(s->configuration));
+ pci_dma_read(&s->dev, s->cb_address + 8,
+ &s->configuration[0], sizeof(s->configuration));
TRACE(OTHER, logout("configuration: %s\n",
nic_dump(&s->configuration[0], 16)));
TRACE(OTHER, logout("configuration: %s\n",
@@ -938,7 +914,8 @@ static void action_command(EEPRO100State *s)
break;
}
/* Write new status. */
- e100_stw_le_phys(s->cb_address, s->tx.status | ok_status | STATUS_C);
+ stw_le_pci_dma(&s->dev, s->cb_address,
+ s->tx.status | ok_status | STATUS_C);
if (bit_i) {
/* CU completed action. */
eepro100_cx_interrupt(s);
@@ -1005,7 +982,7 @@ static void eepro100_cu_command(EEPRO100State * s, uint8_t val)
/* Dump statistical counters. */
TRACE(OTHER, logout("val=0x%02x (dump stats)\n", val));
dump_statistics(s);
- e100_stl_le_phys(s->statsaddr + s->stats_size, 0xa005);
+ stl_le_pci_dma(&s->dev, s->statsaddr + s->stats_size, 0xa005);
break;
case CU_CMD_BASE:
/* Load CU base. */
@@ -1016,7 +993,7 @@ static void eepro100_cu_command(EEPRO100State * s, uint8_t val)
/* Dump and reset statistical counters. */
TRACE(OTHER, logout("val=0x%02x (dump stats and reset)\n", val));
dump_statistics(s);
- e100_stl_le_phys(s->statsaddr + s->stats_size, 0xa007);
+ stl_le_pci_dma(&s->dev, s->statsaddr + s->stats_size, 0xa007);
memset(&s->statistics, 0, sizeof(s->statistics));
break;
case CU_SRESUME:
@@ -1310,10 +1287,10 @@ static void eepro100_write_port(EEPRO100State *s)
case PORT_SELFTEST:
TRACE(OTHER, logout("selftest address=0x%08x\n", address));
eepro100_selftest_t data;
- cpu_physical_memory_read(address, &data, sizeof(data));
+ pci_dma_read(&s->dev, address, (uint8_t *) &data, sizeof(data));
data.st_sign = 0xffffffff;
data.st_result = 0;
- cpu_physical_memory_write(address, &data, sizeof(data));
+ pci_dma_write(&s->dev, address, (uint8_t *) &data, sizeof(data));
break;
case PORT_SELECTIVE_RESET:
TRACE(OTHER, logout("selective reset, selftest address=0x%08x\n", address));
@@ -1729,8 +1706,8 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
}
/* !!! */
eepro100_rx_t rx;
- cpu_physical_memory_read(s->ru_base + s->ru_offset, &rx,
- sizeof(eepro100_rx_t));
+ pci_dma_read(&s->dev, s->ru_base + s->ru_offset,
+ (uint8_t *) &rx, sizeof(eepro100_rx_t));
uint16_t rfd_command = le16_to_cpu(rx.command);
uint16_t rfd_size = le16_to_cpu(rx.size);
@@ -1746,10 +1723,10 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
#endif
TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n",
rfd_command, rx.link, rx.rx_buf_addr, rfd_size));
- e100_stw_le_phys(s->ru_base + s->ru_offset +
- offsetof(eepro100_rx_t, status), rfd_status);
- e100_stw_le_phys(s->ru_base + s->ru_offset +
- offsetof(eepro100_rx_t, count), size);
+ stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
+ offsetof(eepro100_rx_t, status), rfd_status);
+ stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
+ offsetof(eepro100_rx_t, count), size);
/* Early receive interrupt not supported. */
#if 0
eepro100_er_interrupt(s);
@@ -1763,8 +1740,8 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
#if 0
assert(!(s->configuration[17] & BIT(0)));
#endif
- cpu_physical_memory_write(s->ru_base + s->ru_offset +
- sizeof(eepro100_rx_t), buf, size);
+ pci_dma_write(&s->dev, s->ru_base + s->ru_offset +
+ sizeof(eepro100_rx_t), buf, size);
s->statistics.rx_good_frames++;
eepro100_fr_interrupt(s);
s->ru_offset = le32_to_cpu(rx.link);
--
1.7.7
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 06/14] ac97: Use PCI DMA stub functions
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
` (4 preceding siblings ...)
2011-10-31 6:06 ` [Qemu-devel] [PATCH 05/14] eepro100: " David Gibson
@ 2011-10-31 6:06 ` David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 07/14] es1370: " David Gibson
` (7 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
From: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
This updates the ac97 device emulation to use the explicit PCI DMA
functions, instead of directly calling physical memory access functions.
Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ac97.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/hw/ac97.c b/hw/ac97.c
index bc69d4e..6800af4 100644
--- a/hw/ac97.c
+++ b/hw/ac97.c
@@ -18,6 +18,7 @@
#include "audiodev.h"
#include "audio/audio.h"
#include "pci.h"
+#include "dma.h"
enum {
AC97_Reset = 0x00,
@@ -224,7 +225,7 @@ static void fetch_bd (AC97LinkState *s, AC97BusMasterRegs *r)
{
uint8_t b[8];
- cpu_physical_memory_read (r->bdbar + r->civ * 8, b, 8);
+ pci_dma_read (&s->dev, r->bdbar + r->civ * 8, b, 8);
r->bd_valid = 1;
r->bd.addr = le32_to_cpu (*(uint32_t *) &b[0]) & ~3;
r->bd.ctl_len = le32_to_cpu (*(uint32_t *) &b[4]);
@@ -973,7 +974,7 @@ static int write_audio (AC97LinkState *s, AC97BusMasterRegs *r,
while (temp) {
int copied;
to_copy = audio_MIN (temp, sizeof (tmpbuf));
- cpu_physical_memory_read (addr, tmpbuf, to_copy);
+ pci_dma_read (&s->dev, addr, tmpbuf, to_copy);
copied = AUD_write (s->voice_po, tmpbuf, to_copy);
dolog ("write_audio max=%x to_copy=%x copied=%x\n",
max, to_copy, copied);
@@ -1054,7 +1055,7 @@ static int read_audio (AC97LinkState *s, AC97BusMasterRegs *r,
*stop = 1;
break;
}
- cpu_physical_memory_write (addr, tmpbuf, acquired);
+ pci_dma_write (&s->dev, addr, tmpbuf, acquired);
temp -= acquired;
addr += acquired;
nread += acquired;
--
1.7.7
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 07/14] es1370: Use PCI DMA stub functions
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
` (5 preceding siblings ...)
2011-10-31 6:06 ` [Qemu-devel] [PATCH 06/14] ac97: " David Gibson
@ 2011-10-31 6:06 ` David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 08/14] e1000: " David Gibson
` (6 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
From: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
This updates the es1370 device emulation to use the explicit PCI DMA
functions, instead of directly calling physical memory access functions.
Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/es1370.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/hw/es1370.c b/hw/es1370.c
index 2daadde..c5c16b0 100644
--- a/hw/es1370.c
+++ b/hw/es1370.c
@@ -30,6 +30,7 @@
#include "audiodev.h"
#include "audio/audio.h"
#include "pci.h"
+#include "dma.h"
/* Missing stuff:
SCTRL_P[12](END|ST)INC
@@ -802,7 +803,7 @@ static void es1370_transfer_audio (ES1370State *s, struct chan *d, int loop_sel,
if (!acquired)
break;
- cpu_physical_memory_write (addr, tmpbuf, acquired);
+ pci_dma_write (&s->dev, addr, tmpbuf, acquired);
temp -= acquired;
addr += acquired;
@@ -816,7 +817,7 @@ static void es1370_transfer_audio (ES1370State *s, struct chan *d, int loop_sel,
int copied, to_copy;
to_copy = audio_MIN ((size_t) temp, sizeof (tmpbuf));
- cpu_physical_memory_read (addr, tmpbuf, to_copy);
+ pci_dma_read (&s->dev, addr, tmpbuf, to_copy);
copied = AUD_write (voice, tmpbuf, to_copy);
if (!copied)
break;
--
1.7.7
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 08/14] e1000: Use PCI DMA stub functions
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
` (6 preceding siblings ...)
2011-10-31 6:06 ` [Qemu-devel] [PATCH 07/14] es1370: " David Gibson
@ 2011-10-31 6:06 ` David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 09/14] lsi53c895a: " David Gibson
` (5 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
From: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
This updates the e1000 device emulation to use the explicit PCI DMA
functions, instead of directly calling physical memory access functions.
Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/e1000.c | 29 +++++++++++++++--------------
1 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/hw/e1000.c b/hw/e1000.c
index ce8fc8b..986ed9c 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -31,6 +31,7 @@
#include "net/checksum.h"
#include "loader.h"
#include "sysemu.h"
+#include "dma.h"
#include "e1000_hw.h"
@@ -465,7 +466,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
bytes = split_size;
if (tp->size + bytes > msh)
bytes = msh - tp->size;
- cpu_physical_memory_read(addr, tp->data + tp->size, bytes);
+ pci_dma_read(&s->dev, addr, tp->data + tp->size, bytes);
if ((sz = tp->size + bytes) >= hdr && tp->size < hdr)
memmove(tp->header, tp->data, hdr);
tp->size = sz;
@@ -480,7 +481,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
// context descriptor TSE is not set, while data descriptor TSE is set
DBGOUT(TXERR, "TCP segmentaion Error\n");
} else {
- cpu_physical_memory_read(addr, tp->data + tp->size, split_size);
+ pci_dma_read(&s->dev, addr, tp->data + tp->size, split_size);
tp->size += split_size;
}
@@ -496,7 +497,7 @@ process_tx_desc(E1000State *s, struct e1000_tx_desc *dp)
}
static uint32_t
-txdesc_writeback(target_phys_addr_t base, struct e1000_tx_desc *dp)
+txdesc_writeback(E1000State *s, dma_addr_t base, struct e1000_tx_desc *dp)
{
uint32_t txd_upper, txd_lower = le32_to_cpu(dp->lower.data);
@@ -505,8 +506,8 @@ txdesc_writeback(target_phys_addr_t base, struct e1000_tx_desc *dp)
txd_upper = (le32_to_cpu(dp->upper.data) | E1000_TXD_STAT_DD) &
~(E1000_TXD_STAT_EC | E1000_TXD_STAT_LC | E1000_TXD_STAT_TU);
dp->upper.data = cpu_to_le32(txd_upper);
- cpu_physical_memory_write(base + ((char *)&dp->upper - (char *)dp),
- (void *)&dp->upper, sizeof(dp->upper));
+ pci_dma_write(&s->dev, base + ((char *)&dp->upper - (char *)dp),
+ (void *)&dp->upper, sizeof(dp->upper));
return E1000_ICR_TXDW;
}
@@ -521,7 +522,7 @@ static uint64_t tx_desc_base(E1000State *s)
static void
start_xmit(E1000State *s)
{
- target_phys_addr_t base;
+ dma_addr_t base;
struct e1000_tx_desc desc;
uint32_t tdh_start = s->mac_reg[TDH], cause = E1000_ICS_TXQE;
@@ -533,14 +534,14 @@ start_xmit(E1000State *s)
while (s->mac_reg[TDH] != s->mac_reg[TDT]) {
base = tx_desc_base(s) +
sizeof(struct e1000_tx_desc) * s->mac_reg[TDH];
- cpu_physical_memory_read(base, (void *)&desc, sizeof(desc));
+ pci_dma_read(&s->dev, base, (void *)&desc, sizeof(desc));
DBGOUT(TX, "index %d: %p : %x %x\n", s->mac_reg[TDH],
(void *)(intptr_t)desc.buffer_addr, desc.lower.data,
desc.upper.data);
process_tx_desc(s, &desc);
- cause |= txdesc_writeback(base, &desc);
+ cause |= txdesc_writeback(s, base, &desc);
if (++s->mac_reg[TDH] * sizeof(desc) >= s->mac_reg[TDLEN])
s->mac_reg[TDH] = 0;
@@ -668,7 +669,7 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf, size_t size)
{
E1000State *s = DO_UPCAST(NICState, nc, nc)->opaque;
struct e1000_rx_desc desc;
- target_phys_addr_t base;
+ dma_addr_t base;
unsigned int n, rdt;
uint32_t rdh_start;
uint16_t vlan_special = 0;
@@ -713,7 +714,7 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf, size_t size)
desc_size = s->rxbuf_size;
}
base = rx_desc_base(s) + sizeof(desc) * s->mac_reg[RDH];
- cpu_physical_memory_read(base, (void *)&desc, sizeof(desc));
+ pci_dma_read(&s->dev, base, (void *)&desc, sizeof(desc));
desc.special = vlan_special;
desc.status |= (vlan_status | E1000_RXD_STAT_DD);
if (desc.buffer_addr) {
@@ -722,9 +723,9 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf, size_t size)
if (copy_size > s->rxbuf_size) {
copy_size = s->rxbuf_size;
}
- cpu_physical_memory_write(le64_to_cpu(desc.buffer_addr),
- (void *)(buf + desc_offset + vlan_offset),
- copy_size);
+ pci_dma_write(&s->dev, le64_to_cpu(desc.buffer_addr),
+ (void *)(buf + desc_offset + vlan_offset),
+ copy_size);
}
desc_offset += desc_size;
desc.length = cpu_to_le16(desc_size);
@@ -738,7 +739,7 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf, size_t size)
} else { // as per intel docs; skip descriptors with null buf addr
DBGOUT(RX, "Null RX descriptor!!\n");
}
- cpu_physical_memory_write(base, (void *)&desc, sizeof(desc));
+ pci_dma_write(&s->dev, base, (void *)&desc, sizeof(desc));
if (++s->mac_reg[RDH] * sizeof(desc) >= s->mac_reg[RDLEN])
s->mac_reg[RDH] = 0;
--
1.7.7
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 09/14] lsi53c895a: Use PCI DMA stub functions
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
` (7 preceding siblings ...)
2011-10-31 6:06 ` [Qemu-devel] [PATCH 08/14] e1000: " David Gibson
@ 2011-10-31 6:06 ` David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 10/14] pcnet-pci: " David Gibson
` (4 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
From: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
This updates the lsi53c895a device emulation to use the explicit PCI DMA
functions, instead of directly calling physical memory access functions.
Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/lsi53c895a.c | 33 ++++++++++++++++-----------------
1 files changed, 16 insertions(+), 17 deletions(-)
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index e077ec0..65996db 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -15,6 +15,8 @@
#include "hw.h"
#include "pci.h"
#include "scsi.h"
+#include "block_int.h"
+#include "dma.h"
//#define DEBUG_LSI
//#define DEBUG_LSI_REG
@@ -390,10 +392,7 @@ static inline uint32_t read_dword(LSIState *s, uint32_t addr)
{
uint32_t buf;
- /* XXX: an optimization here used to fast-path the read from scripts
- * memory. But that bypasses any iommu.
- */
- cpu_physical_memory_read(addr, (uint8_t *)&buf, 4);
+ pci_dma_read(&s->dev, addr, (uint8_t *)&buf, 4);
return cpu_to_le32(buf);
}
@@ -532,7 +531,7 @@ static void lsi_bad_selection(LSIState *s, uint32_t id)
static void lsi_do_dma(LSIState *s, int out)
{
uint32_t count, id;
- target_phys_addr_t addr;
+ dma_addr_t addr;
SCSIDevice *dev;
assert(s->current);
@@ -562,7 +561,7 @@ static void lsi_do_dma(LSIState *s, int out)
else if (s->sbms)
addr |= ((uint64_t)s->sbms << 32);
- DPRINTF("DMA addr=0x" TARGET_FMT_plx " len=%d\n", addr, count);
+ DPRINTF("DMA addr=0x" DMA_ADDR_FMT " len=%d\n", addr, count);
s->csbc += count;
s->dnad += count;
s->dbc -= count;
@@ -571,9 +570,9 @@ static void lsi_do_dma(LSIState *s, int out)
}
/* ??? Set SFBR to first data byte. */
if (out) {
- cpu_physical_memory_read(addr, s->current->dma_buf, count);
+ pci_dma_read(&s->dev, addr, s->current->dma_buf, count);
} else {
- cpu_physical_memory_write(addr, s->current->dma_buf, count);
+ pci_dma_write(&s->dev, addr, s->current->dma_buf, count);
}
s->current->dma_len -= count;
if (s->current->dma_len == 0) {
@@ -766,7 +765,7 @@ static void lsi_do_command(LSIState *s)
DPRINTF("Send command len=%d\n", s->dbc);
if (s->dbc > 16)
s->dbc = 16;
- cpu_physical_memory_read(s->dnad, buf, s->dbc);
+ pci_dma_read(&s->dev, s->dnad, buf, s->dbc);
s->sfbr = buf[0];
s->command_complete = 0;
@@ -817,7 +816,7 @@ static void lsi_do_status(LSIState *s)
s->dbc = 1;
status = s->status;
s->sfbr = status;
- cpu_physical_memory_write(s->dnad, &status, 1);
+ pci_dma_write(&s->dev, s->dnad, &status, 1);
lsi_set_phase(s, PHASE_MI);
s->msg_action = 1;
lsi_add_msg_byte(s, 0); /* COMMAND COMPLETE */
@@ -831,7 +830,7 @@ static void lsi_do_msgin(LSIState *s)
len = s->msg_len;
if (len > s->dbc)
len = s->dbc;
- cpu_physical_memory_write(s->dnad, s->msg, len);
+ pci_dma_write(&s->dev, s->dnad, s->msg, len);
/* Linux drivers rely on the last byte being in the SIDL. */
s->sidl = s->msg[len - 1];
s->msg_len -= len;
@@ -863,7 +862,7 @@ static void lsi_do_msgin(LSIState *s)
static uint8_t lsi_get_msgbyte(LSIState *s)
{
uint8_t data;
- cpu_physical_memory_read(s->dnad, &data, 1);
+ pci_dma_read(&s->dev, s->dnad, &data, 1);
s->dnad++;
s->dbc--;
return data;
@@ -1015,8 +1014,8 @@ static void lsi_memcpy(LSIState *s, uint32_t dest, uint32_t src, int count)
DPRINTF("memcpy dest 0x%08x src 0x%08x count %d\n", dest, src, count);
while (count) {
n = (count > LSI_BUF_SIZE) ? LSI_BUF_SIZE : count;
- cpu_physical_memory_read(src, buf, n);
- cpu_physical_memory_write(dest, buf, n);
+ pci_dma_read(&s->dev, src, buf, n);
+ pci_dma_write(&s->dev, dest, buf, n);
src += n;
dest += n;
count -= n;
@@ -1084,7 +1083,7 @@ again:
/* 32-bit Table indirect */
offset = sxt24(addr);
- cpu_physical_memory_read(s->dsa + offset, (uint8_t *)buf, 8);
+ pci_dma_read(&s->dev, s->dsa + offset, (uint8_t *)buf, 8);
/* byte count is stored in bits 0:23 only */
s->dbc = cpu_to_le32(buf[0]) & 0xffffff;
s->rbc = s->dbc;
@@ -1443,7 +1442,7 @@ again:
n = (insn & 7);
reg = (insn >> 16) & 0xff;
if (insn & (1 << 24)) {
- cpu_physical_memory_read(addr, data, n);
+ pci_dma_read(&s->dev, addr, data, n);
DPRINTF("Load reg 0x%x size %d addr 0x%08x = %08x\n", reg, n,
addr, *(int *)data);
for (i = 0; i < n; i++) {
@@ -1454,7 +1453,7 @@ again:
for (i = 0; i < n; i++) {
data[i] = lsi_reg_readb(s, reg + i);
}
- cpu_physical_memory_write(addr, data, n);
+ pci_dma_write(&s->dev, addr, data, n);
}
}
}
--
1.7.7
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 10/14] pcnet-pci: Use PCI DMA stub functions
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
` (8 preceding siblings ...)
2011-10-31 6:06 ` [Qemu-devel] [PATCH 09/14] lsi53c895a: " David Gibson
@ 2011-10-31 6:06 ` David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 11/14] intel-hda: " David Gibson
` (3 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
From: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
This updates the pcnet-pci device emulation to use the explicit PCI DMA
functions, instead of directly calling physical memory access functions.
Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/pcnet-pci.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/pcnet-pci.c b/hw/pcnet-pci.c
index fb2a00c..41a6e07 100644
--- a/hw/pcnet-pci.c
+++ b/hw/pcnet-pci.c
@@ -31,6 +31,7 @@
#include "net.h"
#include "loader.h"
#include "qemu-timer.h"
+#include "dma.h"
#include "pcnet.h"
@@ -230,13 +231,13 @@ static const MemoryRegionOps pcnet_mmio_ops = {
static void pci_physical_memory_write(void *dma_opaque, target_phys_addr_t addr,
uint8_t *buf, int len, int do_bswap)
{
- cpu_physical_memory_write(addr, buf, len);
+ pci_dma_write(dma_opaque, addr, buf, len);
}
static void pci_physical_memory_read(void *dma_opaque, target_phys_addr_t addr,
uint8_t *buf, int len, int do_bswap)
{
- cpu_physical_memory_read(addr, buf, len);
+ pci_dma_read(dma_opaque, addr, buf, len);
}
static void pci_pcnet_cleanup(VLANClientState *nc)
@@ -302,6 +303,7 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
s->irq = pci_dev->irq[0];
s->phys_mem_read = pci_physical_memory_read;
s->phys_mem_write = pci_physical_memory_write;
+ s->dma_opaque = pci_dev;
if (!pci_dev->qdev.hotplugged) {
static int loaded = 0;
--
1.7.7
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 11/14] intel-hda: Use PCI DMA stub functions
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
` (9 preceding siblings ...)
2011-10-31 6:06 ` [Qemu-devel] [PATCH 10/14] pcnet-pci: " David Gibson
@ 2011-10-31 6:06 ` David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 12/14] PCI IDE: " David Gibson
` (2 subsequent siblings)
13 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
This updates the intel-hda device emulation to use the explicit PCI DMA
functions, instead of directly calling physical memory access functions.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
hw/intel-hda.c | 14 +++++++-------
1 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/hw/intel-hda.c b/hw/intel-hda.c
index f97775c..888ca96 100644
--- a/hw/intel-hda.c
+++ b/hw/intel-hda.c
@@ -24,6 +24,7 @@
#include "audiodev.h"
#include "intel-hda.h"
#include "intel-hda-defs.h"
+#include "dma.h"
/* --------------------------------------------------------------------- */
/* hda bus */
@@ -328,7 +329,7 @@ static void intel_hda_corb_run(IntelHDAState *d)
rp = (d->corb_rp + 1) & 0xff;
addr = intel_hda_addr(d->corb_lbase, d->corb_ubase);
- verb = ldl_le_phys(addr + 4*rp);
+ verb = ldl_le_pci_dma(&d->pci, addr + 4*rp);
d->corb_rp = rp;
dprint(d, 2, "%s: [rp 0x%x] verb 0x%08x\n", __FUNCTION__, rp, verb);
@@ -360,8 +361,8 @@ static void intel_hda_response(HDACodecDevice *dev, bool solicited, uint32_t res
ex = (solicited ? 0 : (1 << 4)) | dev->cad;
wp = (d->rirb_wp + 1) & 0xff;
addr = intel_hda_addr(d->rirb_lbase, d->rirb_ubase);
- stl_le_phys(addr + 8*wp, response);
- stl_le_phys(addr + 8*wp + 4, ex);
+ stl_le_pci_dma(&d->pci, addr + 8*wp, response);
+ stl_le_pci_dma(&d->pci, addr + 8*wp + 4, ex);
d->rirb_wp = wp;
dprint(d, 2, "%s: [wp 0x%x] response 0x%x, extra 0x%x\n",
@@ -426,8 +427,7 @@ static bool intel_hda_xfer(HDACodecDevice *dev, uint32_t stnr, bool output,
dprint(d, 3, "dma: entry %d, pos %d/%d, copy %d\n",
st->be, st->bp, st->bpl[st->be].len, copy);
- cpu_physical_memory_rw(st->bpl[st->be].addr + st->bp,
- buf, copy, !output);
+ pci_dma_rw(&d->pci, st->bpl[st->be].addr + st->bp, buf, copy, !output);
st->lpib += copy;
st->bp += copy;
buf += copy;
@@ -449,7 +449,7 @@ static bool intel_hda_xfer(HDACodecDevice *dev, uint32_t stnr, bool output,
}
if (d->dp_lbase & 0x01) {
addr = intel_hda_addr(d->dp_lbase & ~0x01, d->dp_ubase);
- stl_le_phys(addr + 8*s, st->lpib);
+ stl_le_pci_dma(&d->pci, addr + 8*s, st->lpib);
}
dprint(d, 3, "dma: --\n");
@@ -471,7 +471,7 @@ static void intel_hda_parse_bdl(IntelHDAState *d, IntelHDAStream *st)
g_free(st->bpl);
st->bpl = g_malloc(sizeof(bpl) * st->bentries);
for (i = 0; i < st->bentries; i++, addr += 16) {
- cpu_physical_memory_read(addr, buf, 16);
+ pci_dma_read(&d->pci, addr, buf, 16);
st->bpl[i].addr = le64_to_cpu(*(uint64_t *)buf);
st->bpl[i].len = le32_to_cpu(*(uint32_t *)(buf + 8));
st->bpl[i].flags = le32_to_cpu(*(uint32_t *)(buf + 12));
--
1.7.7
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 12/14] PCI IDE: Use PCI DMA stub functions
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
` (10 preceding siblings ...)
2011-10-31 6:06 ` [Qemu-devel] [PATCH 11/14] intel-hda: " David Gibson
@ 2011-10-31 6:06 ` David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 13/14] usb-ehci: " David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 14/14] usb-uhci: " David Gibson
13 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
This updates the PCI IDE device emulation to use the explicit PCI DMA
wrapper to initialize its scatter/gathjer structure. This means this
driver should not need further changes when the sglist interface is
extended to support IOMMUs.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/ide/pci.c | 19 ++++++++++---------
1 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/hw/ide/pci.c b/hw/ide/pci.c
index f133c42..49b823d 100644
--- a/hw/ide/pci.c
+++ b/hw/ide/pci.c
@@ -62,7 +62,8 @@ static int bmdma_prepare_buf(IDEDMA *dma, int is_write)
} prd;
int l, len;
- qemu_sglist_init(&s->sg, s->nsector / (BMDMA_PAGE_SIZE / 512) + 1);
+ pci_dma_sglist_init(&s->sg, &bm->pci_dev->dev,
+ s->nsector / (BMDMA_PAGE_SIZE / 512) + 1);
s->io_buffer_size = 0;
for(;;) {
if (bm->cur_prd_len == 0) {
@@ -70,7 +71,7 @@ static int bmdma_prepare_buf(IDEDMA *dma, int is_write)
if (bm->cur_prd_last ||
(bm->cur_addr - bm->addr) >= BMDMA_PAGE_SIZE)
return s->io_buffer_size != 0;
- cpu_physical_memory_read(bm->cur_addr, (uint8_t *)&prd, 8);
+ pci_dma_read(&bm->pci_dev->dev, bm->cur_addr, (uint8_t *)&prd, 8);
bm->cur_addr += 8;
prd.addr = le32_to_cpu(prd.addr);
prd.size = le32_to_cpu(prd.size);
@@ -112,7 +113,7 @@ static int bmdma_rw_buf(IDEDMA *dma, int is_write)
if (bm->cur_prd_last ||
(bm->cur_addr - bm->addr) >= BMDMA_PAGE_SIZE)
return 0;
- cpu_physical_memory_read(bm->cur_addr, (uint8_t *)&prd, 8);
+ pci_dma_read(&bm->pci_dev->dev, bm->cur_addr, (uint8_t *)&prd, 8);
bm->cur_addr += 8;
prd.addr = le32_to_cpu(prd.addr);
prd.size = le32_to_cpu(prd.size);
@@ -127,11 +128,11 @@ static int bmdma_rw_buf(IDEDMA *dma, int is_write)
l = bm->cur_prd_len;
if (l > 0) {
if (is_write) {
- cpu_physical_memory_write(bm->cur_prd_addr,
- s->io_buffer + s->io_buffer_index, l);
+ pci_dma_write(&bm->pci_dev->dev, bm->cur_prd_addr,
+ s->io_buffer + s->io_buffer_index, l);
} else {
- cpu_physical_memory_read(bm->cur_prd_addr,
- s->io_buffer + s->io_buffer_index, l);
+ pci_dma_read(&bm->pci_dev->dev, bm->cur_prd_addr,
+ s->io_buffer + s->io_buffer_index, l);
}
bm->cur_prd_addr += l;
bm->cur_prd_len -= l;
@@ -326,7 +327,7 @@ void bmdma_cmd_writeb(BMDMAState *bm, uint32_t val)
bm->cmd = val & 0x09;
}
-static uint64_t bmdma_addr_read(void *opaque, target_phys_addr_t addr,
+static uint64_t bmdma_addr_read(void *opaque, dma_addr_t addr,
unsigned width)
{
BMDMAState *bm = opaque;
@@ -340,7 +341,7 @@ static uint64_t bmdma_addr_read(void *opaque, target_phys_addr_t addr,
return data;
}
-static void bmdma_addr_write(void *opaque, target_phys_addr_t addr,
+static void bmdma_addr_write(void *opaque, dma_addr_t addr,
uint64_t data, unsigned width)
{
BMDMAState *bm = opaque;
--
1.7.7
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 13/14] usb-ehci: Use PCI DMA stub functions
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
` (11 preceding siblings ...)
2011-10-31 6:06 ` [Qemu-devel] [PATCH 12/14] PCI IDE: " David Gibson
@ 2011-10-31 6:06 ` David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 14/14] usb-uhci: " David Gibson
13 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
This updates the usb-ehci device emulation to use the explicit PCI DMA
wrapper to initialize its scatter/gathjer structure. This means this
driver should not need further changes when the sglist interface is
extended to support IOMMUs.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/usb-ehci.c | 44 +++++++++++++++++++++++++-------------------
1 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c
index bd374c1..cdd5aae 100644
--- a/hw/usb-ehci.c
+++ b/hw/usb-ehci.c
@@ -1101,12 +1101,13 @@ static void ehci_mem_writel(void *ptr, target_phys_addr_t addr, uint32_t val)
// TODO : Put in common header file, duplication from usb-ohci.c
/* Get an array of dwords from main memory */
-static inline int get_dwords(uint32_t addr, uint32_t *buf, int num)
+static inline int get_dwords(EHCIState *ehci, uint32_t addr,
+ uint32_t *buf, int num)
{
int i;
for(i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
- cpu_physical_memory_rw(addr,(uint8_t *)buf, sizeof(*buf), 0);
+ pci_dma_read(&ehci->dev, addr, (uint8_t *)buf, sizeof(*buf));
*buf = le32_to_cpu(*buf);
}
@@ -1114,13 +1115,14 @@ static inline int get_dwords(uint32_t addr, uint32_t *buf, int num)
}
/* Put an array of dwords in to main memory */
-static inline int put_dwords(uint32_t addr, uint32_t *buf, int num)
+static inline int put_dwords(EHCIState *ehci, uint32_t addr,
+ uint32_t *buf, int num)
{
int i;
for(i = 0; i < num; i++, buf++, addr += sizeof(*buf)) {
uint32_t tmp = cpu_to_le32(*buf);
- cpu_physical_memory_rw(addr,(uint8_t *)&tmp, sizeof(tmp), 1);
+ pci_dma_write(&ehci->dev, addr, (uint8_t *)&tmp, sizeof(tmp));
}
return 1;
@@ -1169,7 +1171,8 @@ static int ehci_qh_do_overlay(EHCIQueue *q)
q->qh.bufptr[1] &= ~BUFPTR_CPROGMASK_MASK;
q->qh.bufptr[2] &= ~BUFPTR_FRAMETAG_MASK;
- put_dwords(NLPTR_GET(q->qhaddr), (uint32_t *) &q->qh, sizeof(EHCIqh) >> 2);
+ put_dwords(q->ehci, NLPTR_GET(q->qhaddr), (uint32_t *) &q->qh,
+ sizeof(EHCIqh) >> 2);
return 0;
}
@@ -1177,12 +1180,12 @@ static int ehci_qh_do_overlay(EHCIQueue *q)
static int ehci_init_transfer(EHCIQueue *q)
{
uint32_t cpage, offset, bytes, plen;
- target_phys_addr_t page;
+ dma_addr_t page;
cpage = get_field(q->qh.token, QTD_TOKEN_CPAGE);
bytes = get_field(q->qh.token, QTD_TOKEN_TBYTES);
offset = q->qh.bufptr[0] & ~QTD_BUFPTR_MASK;
- qemu_sglist_init(&q->sgl, 5);
+ pci_dma_sglist_init(&q->sgl, &q->ehci->dev, 5);
while (bytes > 0) {
if (cpage > 4) {
@@ -1428,7 +1431,7 @@ static int ehci_process_itd(EHCIState *ehci,
return USB_RET_PROCERR;
}
- qemu_sglist_init(&ehci->isgl, 2);
+ pci_dma_sglist_init(&ehci->isgl, &ehci->dev, 2);
if (off + len > 4096) {
/* transfer crosses page border */
uint32_t len2 = off + len - 4096;
@@ -1532,7 +1535,8 @@ static int ehci_state_waitlisthead(EHCIState *ehci, int async)
/* Find the head of the list (4.9.1.1) */
for(i = 0; i < MAX_QH; i++) {
- get_dwords(NLPTR_GET(entry), (uint32_t *) &qh, sizeof(EHCIqh) >> 2);
+ get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &qh,
+ sizeof(EHCIqh) >> 2);
ehci_trace_qh(NULL, NLPTR_GET(entry), &qh);
if (qh.epchar & QH_EPCHAR_H) {
@@ -1629,7 +1633,8 @@ static EHCIQueue *ehci_state_fetchqh(EHCIState *ehci, int async)
goto out;
}
- get_dwords(NLPTR_GET(q->qhaddr), (uint32_t *) &q->qh, sizeof(EHCIqh) >> 2);
+ get_dwords(ehci, NLPTR_GET(q->qhaddr),
+ (uint32_t *) &q->qh, sizeof(EHCIqh) >> 2);
ehci_trace_qh(q, NLPTR_GET(q->qhaddr), &q->qh);
if (q->async == EHCI_ASYNC_INFLIGHT) {
@@ -1698,7 +1703,7 @@ static int ehci_state_fetchitd(EHCIState *ehci, int async)
assert(!async);
entry = ehci_get_fetch_addr(ehci, async);
- get_dwords(NLPTR_GET(entry),(uint32_t *) &itd,
+ get_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &itd,
sizeof(EHCIitd) >> 2);
ehci_trace_itd(ehci, entry, &itd);
@@ -1706,8 +1711,8 @@ static int ehci_state_fetchitd(EHCIState *ehci, int async)
return -1;
}
- put_dwords(NLPTR_GET(entry), (uint32_t *) &itd,
- sizeof(EHCIitd) >> 2);
+ put_dwords(ehci, NLPTR_GET(entry), (uint32_t *) &itd,
+ sizeof(EHCIitd) >> 2);
ehci_set_fetch_addr(ehci, async, itd.next);
ehci_set_state(ehci, async, EST_FETCHENTRY);
@@ -1722,7 +1727,7 @@ static int ehci_state_fetchsitd(EHCIState *ehci, int async)
assert(!async);
entry = ehci_get_fetch_addr(ehci, async);
- get_dwords(NLPTR_GET(entry), (uint32_t *)&sitd,
+ get_dwords(ehci, NLPTR_GET(entry), (uint32_t *)&sitd,
sizeof(EHCIsitd) >> 2);
ehci_trace_sitd(ehci, entry, &sitd);
@@ -1784,7 +1789,8 @@ static int ehci_state_fetchqtd(EHCIQueue *q, int async)
{
int again = 0;
- get_dwords(NLPTR_GET(q->qtdaddr),(uint32_t *) &q->qtd, sizeof(EHCIqtd) >> 2);
+ get_dwords(q->ehci, NLPTR_GET(q->qtdaddr), (uint32_t *) &q->qtd,
+ sizeof(EHCIqtd) >> 2);
ehci_trace_qtd(q, NLPTR_GET(q->qtdaddr), &q->qtd);
if (q->qtd.token & QTD_TOKEN_ACTIVE) {
@@ -1827,7 +1833,7 @@ static void ehci_flush_qh(EHCIQueue *q)
uint32_t dwords = sizeof(EHCIqh) >> 2;
uint32_t addr = NLPTR_GET(q->qhaddr);
- put_dwords(addr + 3 * sizeof(uint32_t), qh + 3, dwords - 3);
+ put_dwords(q->ehci, addr + 3 * sizeof(uint32_t), qh + 3, dwords - 3);
}
static int ehci_state_execute(EHCIQueue *q, int async)
@@ -1947,8 +1953,8 @@ static int ehci_state_writeback(EHCIQueue *q, int async)
/* Write back the QTD from the QH area */
ehci_trace_qtd(q, NLPTR_GET(q->qtdaddr), (EHCIqtd*) &q->qh.next_qtd);
- put_dwords(NLPTR_GET(q->qtdaddr),(uint32_t *) &q->qh.next_qtd,
- sizeof(EHCIqtd) >> 2);
+ put_dwords(q->ehci, NLPTR_GET(q->qtdaddr), (uint32_t *) &q->qh.next_qtd,
+ sizeof(EHCIqtd) >> 2);
/*
* EHCI specs say go horizontal here.
@@ -2148,7 +2154,7 @@ static void ehci_advance_periodic_state(EHCIState *ehci)
}
list |= ((ehci->frindex & 0x1ff8) >> 1);
- cpu_physical_memory_rw(list, (uint8_t *) &entry, sizeof entry, 0);
+ pci_dma_read(&ehci->dev, list, (uint8_t *) &entry, sizeof entry);
entry = le32_to_cpu(entry);
DPRINTF("PERIODIC state adv fr=%d. [%08X] -> %08X\n",
--
1.7.7
^ permalink raw reply related [flat|nested] 33+ messages in thread
* [Qemu-devel] [PATCH 14/14] usb-uhci: Use PCI DMA stub functions
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
` (12 preceding siblings ...)
2011-10-31 6:06 ` [Qemu-devel] [PATCH 13/14] usb-ehci: " David Gibson
@ 2011-10-31 6:06 ` David Gibson
13 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2011-10-31 6:06 UTC (permalink / raw)
To: mst, anthony; +Cc: joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth
This updates the usb-uhci device emulation to use the explicit PCI DMA
wrapper to initialize its scatter/gathjer structure. This means this
driver should not need further changes when the sglist interface is
extended to support IOMMUs.
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
hw/usb-uhci.c | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index 171d787..f9e3ea5 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -178,7 +178,7 @@ static UHCIAsync *uhci_async_alloc(UHCIState *s)
async->done = 0;
async->isoc = 0;
usb_packet_init(&async->packet);
- qemu_sglist_init(&async->sgl, 1);
+ pci_dma_sglist_init(&async->sgl, &s->dev, 1);
return async;
}
@@ -876,7 +876,7 @@ static void uhci_async_complete(USBPort *port, USBPacket *packet)
uint32_t link = async->td;
uint32_t int_mask = 0, val;
- cpu_physical_memory_read(link & ~0xf, (uint8_t *) &td, sizeof(td));
+ pci_dma_read(&s->dev, link & ~0xf, (uint8_t *) &td, sizeof(td));
le32_to_cpus(&td.link);
le32_to_cpus(&td.ctrl);
le32_to_cpus(&td.token);
@@ -888,8 +888,8 @@ static void uhci_async_complete(USBPort *port, USBPacket *packet)
/* update the status bits of the TD */
val = cpu_to_le32(td.ctrl);
- cpu_physical_memory_write((link & ~0xf) + 4,
- (const uint8_t *)&val, sizeof(val));
+ pci_dma_write(&s->dev, (link & ~0xf) + 4,
+ (const uint8_t *)&val, sizeof(val));
uhci_async_free(s, async);
} else {
async->done = 1;
@@ -952,7 +952,7 @@ static void uhci_process_frame(UHCIState *s)
DPRINTF("uhci: processing frame %d addr 0x%x\n" , s->frnum, frame_addr);
- cpu_physical_memory_read(frame_addr, (uint8_t *)&link, 4);
+ pci_dma_read(&s->dev, frame_addr, (uint8_t *)&link, 4);
le32_to_cpus(&link);
int_mask = 0;
@@ -976,7 +976,7 @@ static void uhci_process_frame(UHCIState *s)
break;
}
- cpu_physical_memory_read(link & ~0xf, (uint8_t *) &qh, sizeof(qh));
+ pci_dma_read(&s->dev, link & ~0xf, (uint8_t *) &qh, sizeof(qh));
le32_to_cpus(&qh.link);
le32_to_cpus(&qh.el_link);
@@ -996,7 +996,7 @@ static void uhci_process_frame(UHCIState *s)
}
/* TD */
- cpu_physical_memory_read(link & ~0xf, (uint8_t *) &td, sizeof(td));
+ pci_dma_read(&s->dev, link & ~0xf, (uint8_t *) &td, sizeof(td));
le32_to_cpus(&td.link);
le32_to_cpus(&td.ctrl);
le32_to_cpus(&td.token);
@@ -1010,8 +1010,8 @@ static void uhci_process_frame(UHCIState *s)
if (old_td_ctrl != td.ctrl) {
/* update the status bits of the TD */
val = cpu_to_le32(td.ctrl);
- cpu_physical_memory_write((link & ~0xf) + 4,
- (const uint8_t *)&val, sizeof(val));
+ pci_dma_write(&s->dev, (link & ~0xf) + 4,
+ (const uint8_t *)&val, sizeof(val));
}
if (ret < 0) {
@@ -1039,8 +1039,8 @@ static void uhci_process_frame(UHCIState *s)
/* update QH element link */
qh.el_link = link;
val = cpu_to_le32(qh.el_link);
- cpu_physical_memory_write((curr_qh & ~0xf) + 4,
- (const uint8_t *)&val, sizeof(val));
+ pci_dma_write(&s->dev, (curr_qh & ~0xf) + 4,
+ (const uint8_t *)&val, sizeof(val));
if (!depth_first(link)) {
/* done with this QH */
--
1.7.7
^ permalink raw reply related [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-10-31 6:06 ` [Qemu-devel] [PATCH 05/14] eepro100: " David Gibson
@ 2011-10-31 16:45 ` Stefan Weil
2011-11-01 20:24 ` Anthony Liguori
2011-11-02 7:16 ` Michael S. Tsirkin
1 sibling, 1 reply; 33+ messages in thread
From: Stefan Weil @ 2011-10-31 16:45 UTC (permalink / raw)
To: David Gibson
Cc: mst, joerg.roedel, agraf, qemu-devel, avi, eduard.munteanu, rth
Am 31.10.2011 07:06, schrieb David Gibson:
> From: Eduard - Gabriel Munteanu<eduard.munteanu@linux360.ro>
>
> This updates the eepro100 device emulation to use the explicit PCI DMA
> functions, instead of directly calling physical memory access functions.
>
> Signed-off-by: Eduard - Gabriel Munteanu<eduard.munteanu@linux360.ro>
> Signed-off-by: David Gibson<dwg@au1.ibm.com>
> Signed-off-by: Alexey Kardashevskiy<aik@ozlabs.ru>
> ---
> hw/eepro100.c | 121 +++++++++++++++++++++++----------------------------------
> 1 files changed, 49 insertions(+), 72 deletions(-)
>
> diff --git a/hw/eepro100.c b/hw/eepro100.c
> index 4e3c52f..7d59e71 100644
> --- a/hw/eepro100.c
> +++ b/hw/eepro100.c
> @@ -46,6 +46,7 @@
> #include "net.h"
> #include "eeprom93xx.h"
> #include "sysemu.h"
> +#include "dma.h"
>
> /* QEMU sends frames smaller than 60 bytes to ethernet nics.
> * Such frames are rejected by real nics and their emulations.
> @@ -315,38 +316,6 @@ static const uint16_t eepro100_mdi_mask[] = {
> 0xffff, 0xffff, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
> };
>
> -/* Read a 16 bit little endian value from physical memory. */
> -static uint16_t e100_ldw_le_phys(target_phys_addr_t addr)
> -{
> - /* Load 16 bit (little endian) word from emulated hardware. */
> - uint16_t val;
> - cpu_physical_memory_read(addr,&val, sizeof(val));
> - return le16_to_cpu(val);
> -}
> -
> -/* Read a 32 bit little endian value from physical memory. */
> -static uint32_t e100_ldl_le_phys(target_phys_addr_t addr)
> -{
> - /* Load 32 bit (little endian) word from emulated hardware. */
> - uint32_t val;
> - cpu_physical_memory_read(addr,&val, sizeof(val));
> - return le32_to_cpu(val);
> -}
> -
> -/* Write a 16 bit little endian value to physical memory. */
> -static void e100_stw_le_phys(target_phys_addr_t addr, uint16_t val)
> -{
> - val = cpu_to_le16(val);
> - cpu_physical_memory_write(addr,&val, sizeof(val));
> -}
> -
> -/* Write a 32 bit little endian value to physical memory. */
> -static void e100_stl_le_phys(target_phys_addr_t addr, uint32_t val)
> -{
> - val = cpu_to_le32(val);
> - cpu_physical_memory_write(addr,&val, sizeof(val));
> -}
> -
> #define POLYNOMIAL 0x04c11db6
>
> /* From FreeBSD */
> @@ -744,21 +713,26 @@ static void dump_statistics(EEPRO100State * s)
> * values which really matter.
> * Number of data should check configuration!!!
> */
> - cpu_physical_memory_write(s->statsaddr,&s->statistics, s->stats_size);
> - e100_stl_le_phys(s->statsaddr + 0, s->statistics.tx_good_frames);
> - e100_stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
> - e100_stl_le_phys(s->statsaddr + 48, s->statistics.rx_resource_errors);
> - e100_stl_le_phys(s->statsaddr + 60, s->statistics.rx_short_frame_errors);
> + pci_dma_write(&s->dev, s->statsaddr,
> + (uint8_t *)&s->statistics, s->stats_size);
The type cast is not needed and should be removed.
> + stl_le_pci_dma(&s->dev, s->statsaddr + 0,
> + s->statistics.tx_good_frames);
> + stl_le_pci_dma(&s->dev, s->statsaddr + 36,
> + s->statistics.rx_good_frames);
> + stl_le_pci_dma(&s->dev, s->statsaddr + 48,
> + s->statistics.rx_resource_errors);
> + stl_le_pci_dma(&s->dev, s->statsaddr + 60,
> + s->statistics.rx_short_frame_errors);
> #if 0
> - e100_stw_le_phys(s->statsaddr + 76, s->statistics.xmt_tco_frames);
> - e100_stw_le_phys(s->statsaddr + 78, s->statistics.rcv_tco_frames);
> + stw_le_pci_dma(&s->dev, s->statsaddr + 76, s->statistics.xmt_tco_frames);
> + stw_le_pci_dma(&s->dev, s->statsaddr + 78, s->statistics.rcv_tco_frames);
> missing("CU dump statistical counters");
> #endif
> }
>
> static void read_cb(EEPRO100State *s)
> {
> - cpu_physical_memory_read(s->cb_address,&s->tx, sizeof(s->tx));
> + pci_dma_read(&s->dev, s->cb_address, (uint8_t *)&s->tx, sizeof(s->tx));
The type cast is not needed and should be removed.
> s->tx.status = le16_to_cpu(s->tx.status);
> s->tx.command = le16_to_cpu(s->tx.command);
> s->tx.link = le32_to_cpu(s->tx.link);
> @@ -788,18 +762,17 @@ static void tx_command(EEPRO100State *s)
> }
> assert(tcb_bytes<= sizeof(buf));
> while (size< tcb_bytes) {
> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev, tbd_address);
> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev, tbd_address + 4);
> #if 0
> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
> #endif
> tbd_address += 8;
> TRACE(RXTX, logout
> ("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n",
> tx_buffer_address, tx_buffer_size));
> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> - cpu_physical_memory_read(tx_buffer_address,&buf[size],
> - tx_buffer_size);
> + pci_dma_read(&s->dev, tx_buffer_address,&buf[size], tx_buffer_size);
> size += tx_buffer_size;
> }
> if (tbd_array == 0xffffffff) {
> @@ -810,16 +783,19 @@ static void tx_command(EEPRO100State *s)
> if (s->has_extended_tcb_support&& !(s->configuration[6]& BIT(4))) {
> /* Extended Flexible TCB. */
> for (; tbd_count< 2; tbd_count++) {
> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev,
> + tbd_address);
> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev,
> + tbd_address + 4);
> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev,
> + tbd_address + 6);
> tbd_address += 8;
> TRACE(RXTX, logout
> ("TBD (extended flexible mode): buffer address 0x%08x, size 0x%04x\n",
> tx_buffer_address, tx_buffer_size));
> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> - cpu_physical_memory_read(tx_buffer_address,&buf[size],
> - tx_buffer_size);
> + pci_dma_read(&s->dev, tx_buffer_address,
> +&buf[size], tx_buffer_size);
> size += tx_buffer_size;
> if (tx_buffer_el& 1) {
> break;
> @@ -828,16 +804,16 @@ static void tx_command(EEPRO100State *s)
> }
> tbd_address = tbd_array;
> for (; tbd_count< s->tx.tbd_count; tbd_count++) {
> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev, tbd_address);
> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev, tbd_address + 4);
> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
> tbd_address += 8;
> TRACE(RXTX, logout
> ("TBD (flexible mode): buffer address 0x%08x, size 0x%04x\n",
> tx_buffer_address, tx_buffer_size));
> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> - cpu_physical_memory_read(tx_buffer_address,&buf[size],
> - tx_buffer_size);
> + pci_dma_read(&s->dev, tx_buffer_address,
> +&buf[size], tx_buffer_size);
> size += tx_buffer_size;
> if (tx_buffer_el& 1) {
> break;
> @@ -862,7 +838,7 @@ static void set_multicast_list(EEPRO100State *s)
> TRACE(OTHER, logout("multicast list, multicast count = %u\n", multicast_count));
> for (i = 0; i< multicast_count; i += 6) {
> uint8_t multicast_addr[6];
> - cpu_physical_memory_read(s->cb_address + 10 + i, multicast_addr, 6);
> + pci_dma_read(&s->dev, s->cb_address + 10 + i, multicast_addr, 6);
> TRACE(OTHER, logout("multicast entry %s\n", nic_dump(multicast_addr, 6)));
> unsigned mcast_idx = compute_mcast_idx(multicast_addr);
> assert(mcast_idx< 64);
> @@ -896,12 +872,12 @@ static void action_command(EEPRO100State *s)
> /* Do nothing. */
> break;
> case CmdIASetup:
> - cpu_physical_memory_read(s->cb_address + 8,&s->conf.macaddr.a[0], 6);
> + pci_dma_read(&s->dev, s->cb_address + 8,&s->conf.macaddr.a[0], 6);
> TRACE(OTHER, logout("macaddr: %s\n", nic_dump(&s->conf.macaddr.a[0], 6)));
> break;
> case CmdConfigure:
> - cpu_physical_memory_read(s->cb_address + 8,&s->configuration[0],
> - sizeof(s->configuration));
> + pci_dma_read(&s->dev, s->cb_address + 8,
> +&s->configuration[0], sizeof(s->configuration));
> TRACE(OTHER, logout("configuration: %s\n",
> nic_dump(&s->configuration[0], 16)));
> TRACE(OTHER, logout("configuration: %s\n",
> @@ -938,7 +914,8 @@ static void action_command(EEPRO100State *s)
> break;
> }
> /* Write new status. */
> - e100_stw_le_phys(s->cb_address, s->tx.status | ok_status | STATUS_C);
> + stw_le_pci_dma(&s->dev, s->cb_address,
> + s->tx.status | ok_status | STATUS_C);
> if (bit_i) {
> /* CU completed action. */
> eepro100_cx_interrupt(s);
> @@ -1005,7 +982,7 @@ static void eepro100_cu_command(EEPRO100State * s, uint8_t val)
> /* Dump statistical counters. */
> TRACE(OTHER, logout("val=0x%02x (dump stats)\n", val));
> dump_statistics(s);
> - e100_stl_le_phys(s->statsaddr + s->stats_size, 0xa005);
> + stl_le_pci_dma(&s->dev, s->statsaddr + s->stats_size, 0xa005);
> break;
> case CU_CMD_BASE:
> /* Load CU base. */
> @@ -1016,7 +993,7 @@ static void eepro100_cu_command(EEPRO100State * s, uint8_t val)
> /* Dump and reset statistical counters. */
> TRACE(OTHER, logout("val=0x%02x (dump stats and reset)\n", val));
> dump_statistics(s);
> - e100_stl_le_phys(s->statsaddr + s->stats_size, 0xa007);
> + stl_le_pci_dma(&s->dev, s->statsaddr + s->stats_size, 0xa007);
> memset(&s->statistics, 0, sizeof(s->statistics));
> break;
> case CU_SRESUME:
> @@ -1310,10 +1287,10 @@ static void eepro100_write_port(EEPRO100State *s)
> case PORT_SELFTEST:
> TRACE(OTHER, logout("selftest address=0x%08x\n", address));
> eepro100_selftest_t data;
> - cpu_physical_memory_read(address,&data, sizeof(data));
> + pci_dma_read(&s->dev, address, (uint8_t *)&data, sizeof(data));
The type cast is not needed and should be removed.
> data.st_sign = 0xffffffff;
> data.st_result = 0;
> - cpu_physical_memory_write(address,&data, sizeof(data));
> + pci_dma_write(&s->dev, address, (uint8_t *)&data, sizeof(data));
The type cast is not needed and should be removed.
> break;
> case PORT_SELECTIVE_RESET:
> TRACE(OTHER, logout("selective reset, selftest address=0x%08x\n", address));
> @@ -1729,8 +1706,8 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
> }
> /* !!! */
> eepro100_rx_t rx;
> - cpu_physical_memory_read(s->ru_base + s->ru_offset,&rx,
> - sizeof(eepro100_rx_t));
> + pci_dma_read(&s->dev, s->ru_base + s->ru_offset,
> + (uint8_t *)&rx, sizeof(eepro100_rx_t));
The type cast is not needed and should be removed.
> uint16_t rfd_command = le16_to_cpu(rx.command);
> uint16_t rfd_size = le16_to_cpu(rx.size);
>
> @@ -1746,10 +1723,10 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
> #endif
> TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n",
> rfd_command, rx.link, rx.rx_buf_addr, rfd_size));
> - e100_stw_le_phys(s->ru_base + s->ru_offset +
> - offsetof(eepro100_rx_t, status), rfd_status);
> - e100_stw_le_phys(s->ru_base + s->ru_offset +
> - offsetof(eepro100_rx_t, count), size);
> + stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
> + offsetof(eepro100_rx_t, status), rfd_status);
> + stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
> + offsetof(eepro100_rx_t, count), size);
> /* Early receive interrupt not supported. */
> #if 0
> eepro100_er_interrupt(s);
> @@ -1763,8 +1740,8 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
> #if 0
> assert(!(s->configuration[17]& BIT(0)));
> #endif
> - cpu_physical_memory_write(s->ru_base + s->ru_offset +
> - sizeof(eepro100_rx_t), buf, size);
> + pci_dma_write(&s->dev, s->ru_base + s->ru_offset +
> + sizeof(eepro100_rx_t), buf, size);
> s->statistics.rx_good_frames++;
> eepro100_fr_interrupt(s);
> s->ru_offset = le32_to_cpu(rx.link);
Hi,
the patch looks reasonable. I only suggest a formal change:
There are lots of unnecessary type casts in several of your patches.
I marked them here, but they should be removed anywhere.
Kind regards,
Stefan
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-10-31 16:45 ` Stefan Weil
@ 2011-11-01 20:24 ` Anthony Liguori
2011-11-02 7:40 ` Michael S. Tsirkin
2011-11-02 11:01 ` Michael S. Tsirkin
0 siblings, 2 replies; 33+ messages in thread
From: Anthony Liguori @ 2011-11-01 20:24 UTC (permalink / raw)
To: Stefan Weil
Cc: mst, joerg.roedel, qemu-devel, agraf, avi, eduard.munteanu, rth,
David Gibson
On 10/31/2011 11:45 AM, Stefan Weil wrote:
> Am 31.10.2011 07:06, schrieb David Gibson:
>> From: Eduard - Gabriel Munteanu<eduard.munteanu@linux360.ro>
>>
>> This updates the eepro100 device emulation to use the explicit PCI DMA
>> functions, instead of directly calling physical memory access functions.
>>
>> Signed-off-by: Eduard - Gabriel Munteanu<eduard.munteanu@linux360.ro>
>> Signed-off-by: David Gibson<dwg@au1.ibm.com>
>> Signed-off-by: Alexey Kardashevskiy<aik@ozlabs.ru>
>> ---
>> hw/eepro100.c | 121 +++++++++++++++++++++++----------------------------------
>> 1 files changed, 49 insertions(+), 72 deletions(-)
>>
>> diff --git a/hw/eepro100.c b/hw/eepro100.c
>> index 4e3c52f..7d59e71 100644
>> --- a/hw/eepro100.c
>> +++ b/hw/eepro100.c
>> @@ -46,6 +46,7 @@
>> #include "net.h"
>> #include "eeprom93xx.h"
>> #include "sysemu.h"
>> +#include "dma.h"
>>
>> /* QEMU sends frames smaller than 60 bytes to ethernet nics.
>> * Such frames are rejected by real nics and their emulations.
>> @@ -315,38 +316,6 @@ static const uint16_t eepro100_mdi_mask[] = {
>> 0xffff, 0xffff, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
>> };
>>
>> -/* Read a 16 bit little endian value from physical memory. */
>> -static uint16_t e100_ldw_le_phys(target_phys_addr_t addr)
>> -{
>> - /* Load 16 bit (little endian) word from emulated hardware. */
>> - uint16_t val;
>> - cpu_physical_memory_read(addr,&val, sizeof(val));
>> - return le16_to_cpu(val);
>> -}
>> -
>> -/* Read a 32 bit little endian value from physical memory. */
>> -static uint32_t e100_ldl_le_phys(target_phys_addr_t addr)
>> -{
>> - /* Load 32 bit (little endian) word from emulated hardware. */
>> - uint32_t val;
>> - cpu_physical_memory_read(addr,&val, sizeof(val));
>> - return le32_to_cpu(val);
>> -}
>> -
>> -/* Write a 16 bit little endian value to physical memory. */
>> -static void e100_stw_le_phys(target_phys_addr_t addr, uint16_t val)
>> -{
>> - val = cpu_to_le16(val);
>> - cpu_physical_memory_write(addr,&val, sizeof(val));
>> -}
>> -
>> -/* Write a 32 bit little endian value to physical memory. */
>> -static void e100_stl_le_phys(target_phys_addr_t addr, uint32_t val)
>> -{
>> - val = cpu_to_le32(val);
>> - cpu_physical_memory_write(addr,&val, sizeof(val));
>> -}
>> -
>> #define POLYNOMIAL 0x04c11db6
>>
>> /* From FreeBSD */
>> @@ -744,21 +713,26 @@ static void dump_statistics(EEPRO100State * s)
>> * values which really matter.
>> * Number of data should check configuration!!!
>> */
>> - cpu_physical_memory_write(s->statsaddr,&s->statistics, s->stats_size);
>> - e100_stl_le_phys(s->statsaddr + 0, s->statistics.tx_good_frames);
>> - e100_stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
>> - e100_stl_le_phys(s->statsaddr + 48, s->statistics.rx_resource_errors);
>> - e100_stl_le_phys(s->statsaddr + 60, s->statistics.rx_short_frame_errors);
>> + pci_dma_write(&s->dev, s->statsaddr,
>> + (uint8_t *)&s->statistics, s->stats_size);
> The type cast is not needed and should be removed.
>> + stl_le_pci_dma(&s->dev, s->statsaddr + 0,
>> + s->statistics.tx_good_frames);
>> + stl_le_pci_dma(&s->dev, s->statsaddr + 36,
>> + s->statistics.rx_good_frames);
>> + stl_le_pci_dma(&s->dev, s->statsaddr + 48,
>> + s->statistics.rx_resource_errors);
>> + stl_le_pci_dma(&s->dev, s->statsaddr + 60,
>> + s->statistics.rx_short_frame_errors);
>> #if 0
>> - e100_stw_le_phys(s->statsaddr + 76, s->statistics.xmt_tco_frames);
>> - e100_stw_le_phys(s->statsaddr + 78, s->statistics.rcv_tco_frames);
>> + stw_le_pci_dma(&s->dev, s->statsaddr + 76, s->statistics.xmt_tco_frames);
>> + stw_le_pci_dma(&s->dev, s->statsaddr + 78, s->statistics.rcv_tco_frames);
>> missing("CU dump statistical counters");
>> #endif
>> }
>>
>> static void read_cb(EEPRO100State *s)
>> {
>> - cpu_physical_memory_read(s->cb_address,&s->tx, sizeof(s->tx));
>> + pci_dma_read(&s->dev, s->cb_address, (uint8_t *)&s->tx, sizeof(s->tx));
> The type cast is not needed and should be removed.
>> s->tx.status = le16_to_cpu(s->tx.status);
>> s->tx.command = le16_to_cpu(s->tx.command);
>> s->tx.link = le32_to_cpu(s->tx.link);
>> @@ -788,18 +762,17 @@ static void tx_command(EEPRO100State *s)
>> }
>> assert(tcb_bytes<= sizeof(buf));
>> while (size< tcb_bytes) {
>> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
>> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
>> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev, tbd_address);
>> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev, tbd_address + 4);
>> #if 0
>> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
>> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
>> #endif
>> tbd_address += 8;
>> TRACE(RXTX, logout
>> ("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n",
>> tx_buffer_address, tx_buffer_size));
>> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
>> - cpu_physical_memory_read(tx_buffer_address,&buf[size],
>> - tx_buffer_size);
>> + pci_dma_read(&s->dev, tx_buffer_address,&buf[size], tx_buffer_size);
>> size += tx_buffer_size;
>> }
>> if (tbd_array == 0xffffffff) {
>> @@ -810,16 +783,19 @@ static void tx_command(EEPRO100State *s)
>> if (s->has_extended_tcb_support&& !(s->configuration[6]& BIT(4))) {
>> /* Extended Flexible TCB. */
>> for (; tbd_count< 2; tbd_count++) {
>> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
>> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
>> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
>> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev,
>> + tbd_address);
>> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev,
>> + tbd_address + 4);
>> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev,
>> + tbd_address + 6);
>> tbd_address += 8;
>> TRACE(RXTX, logout
>> ("TBD (extended flexible mode): buffer address 0x%08x, size 0x%04x\n",
>> tx_buffer_address, tx_buffer_size));
>> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
>> - cpu_physical_memory_read(tx_buffer_address,&buf[size],
>> - tx_buffer_size);
>> + pci_dma_read(&s->dev, tx_buffer_address,
>> +&buf[size], tx_buffer_size);
>> size += tx_buffer_size;
>> if (tx_buffer_el& 1) {
>> break;
>> @@ -828,16 +804,16 @@ static void tx_command(EEPRO100State *s)
>> }
>> tbd_address = tbd_array;
>> for (; tbd_count< s->tx.tbd_count; tbd_count++) {
>> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
>> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
>> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
>> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev, tbd_address);
>> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev, tbd_address + 4);
>> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
>> tbd_address += 8;
>> TRACE(RXTX, logout
>> ("TBD (flexible mode): buffer address 0x%08x, size 0x%04x\n",
>> tx_buffer_address, tx_buffer_size));
>> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
>> - cpu_physical_memory_read(tx_buffer_address,&buf[size],
>> - tx_buffer_size);
>> + pci_dma_read(&s->dev, tx_buffer_address,
>> +&buf[size], tx_buffer_size);
>> size += tx_buffer_size;
>> if (tx_buffer_el& 1) {
>> break;
>> @@ -862,7 +838,7 @@ static void set_multicast_list(EEPRO100State *s)
>> TRACE(OTHER, logout("multicast list, multicast count = %u\n", multicast_count));
>> for (i = 0; i< multicast_count; i += 6) {
>> uint8_t multicast_addr[6];
>> - cpu_physical_memory_read(s->cb_address + 10 + i, multicast_addr, 6);
>> + pci_dma_read(&s->dev, s->cb_address + 10 + i, multicast_addr, 6);
>> TRACE(OTHER, logout("multicast entry %s\n", nic_dump(multicast_addr, 6)));
>> unsigned mcast_idx = compute_mcast_idx(multicast_addr);
>> assert(mcast_idx< 64);
>> @@ -896,12 +872,12 @@ static void action_command(EEPRO100State *s)
>> /* Do nothing. */
>> break;
>> case CmdIASetup:
>> - cpu_physical_memory_read(s->cb_address + 8,&s->conf.macaddr.a[0], 6);
>> + pci_dma_read(&s->dev, s->cb_address + 8,&s->conf.macaddr.a[0], 6);
>> TRACE(OTHER, logout("macaddr: %s\n", nic_dump(&s->conf.macaddr.a[0], 6)));
>> break;
>> case CmdConfigure:
>> - cpu_physical_memory_read(s->cb_address + 8,&s->configuration[0],
>> - sizeof(s->configuration));
>> + pci_dma_read(&s->dev, s->cb_address + 8,
>> +&s->configuration[0], sizeof(s->configuration));
>> TRACE(OTHER, logout("configuration: %s\n",
>> nic_dump(&s->configuration[0], 16)));
>> TRACE(OTHER, logout("configuration: %s\n",
>> @@ -938,7 +914,8 @@ static void action_command(EEPRO100State *s)
>> break;
>> }
>> /* Write new status. */
>> - e100_stw_le_phys(s->cb_address, s->tx.status | ok_status | STATUS_C);
>> + stw_le_pci_dma(&s->dev, s->cb_address,
>> + s->tx.status | ok_status | STATUS_C);
>> if (bit_i) {
>> /* CU completed action. */
>> eepro100_cx_interrupt(s);
>> @@ -1005,7 +982,7 @@ static void eepro100_cu_command(EEPRO100State * s,
>> uint8_t val)
>> /* Dump statistical counters. */
>> TRACE(OTHER, logout("val=0x%02x (dump stats)\n", val));
>> dump_statistics(s);
>> - e100_stl_le_phys(s->statsaddr + s->stats_size, 0xa005);
>> + stl_le_pci_dma(&s->dev, s->statsaddr + s->stats_size, 0xa005);
>> break;
>> case CU_CMD_BASE:
>> /* Load CU base. */
>> @@ -1016,7 +993,7 @@ static void eepro100_cu_command(EEPRO100State * s,
>> uint8_t val)
>> /* Dump and reset statistical counters. */
>> TRACE(OTHER, logout("val=0x%02x (dump stats and reset)\n", val));
>> dump_statistics(s);
>> - e100_stl_le_phys(s->statsaddr + s->stats_size, 0xa007);
>> + stl_le_pci_dma(&s->dev, s->statsaddr + s->stats_size, 0xa007);
>> memset(&s->statistics, 0, sizeof(s->statistics));
>> break;
>> case CU_SRESUME:
>> @@ -1310,10 +1287,10 @@ static void eepro100_write_port(EEPRO100State *s)
>> case PORT_SELFTEST:
>> TRACE(OTHER, logout("selftest address=0x%08x\n", address));
>> eepro100_selftest_t data;
>> - cpu_physical_memory_read(address,&data, sizeof(data));
>> + pci_dma_read(&s->dev, address, (uint8_t *)&data, sizeof(data));
> The type cast is not needed and should be removed.
>> data.st_sign = 0xffffffff;
>> data.st_result = 0;
>> - cpu_physical_memory_write(address,&data, sizeof(data));
>> + pci_dma_write(&s->dev, address, (uint8_t *)&data, sizeof(data));
> The type cast is not needed and should be removed.
>> break;
>> case PORT_SELECTIVE_RESET:
>> TRACE(OTHER, logout("selective reset, selftest address=0x%08x\n", address));
>> @@ -1729,8 +1706,8 @@ static ssize_t nic_receive(VLANClientState *nc, const
>> uint8_t * buf, size_t size
>> }
>> /* !!! */
>> eepro100_rx_t rx;
>> - cpu_physical_memory_read(s->ru_base + s->ru_offset,&rx,
>> - sizeof(eepro100_rx_t));
>> + pci_dma_read(&s->dev, s->ru_base + s->ru_offset,
>> + (uint8_t *)&rx, sizeof(eepro100_rx_t));
>
> The type cast is not needed and should be removed.
>
>
>> uint16_t rfd_command = le16_to_cpu(rx.command);
>> uint16_t rfd_size = le16_to_cpu(rx.size);
>>
>> @@ -1746,10 +1723,10 @@ static ssize_t nic_receive(VLANClientState *nc, const
>> uint8_t * buf, size_t size
>> #endif
>> TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n",
>> rfd_command, rx.link, rx.rx_buf_addr, rfd_size));
>> - e100_stw_le_phys(s->ru_base + s->ru_offset +
>> - offsetof(eepro100_rx_t, status), rfd_status);
>> - e100_stw_le_phys(s->ru_base + s->ru_offset +
>> - offsetof(eepro100_rx_t, count), size);
>> + stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
>> + offsetof(eepro100_rx_t, status), rfd_status);
>> + stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
>> + offsetof(eepro100_rx_t, count), size);
>> /* Early receive interrupt not supported. */
>> #if 0
>> eepro100_er_interrupt(s);
>> @@ -1763,8 +1740,8 @@ static ssize_t nic_receive(VLANClientState *nc, const
>> uint8_t * buf, size_t size
>> #if 0
>> assert(!(s->configuration[17]& BIT(0)));
>> #endif
>> - cpu_physical_memory_write(s->ru_base + s->ru_offset +
>> - sizeof(eepro100_rx_t), buf, size);
>> + pci_dma_write(&s->dev, s->ru_base + s->ru_offset +
>> + sizeof(eepro100_rx_t), buf, size);
>> s->statistics.rx_good_frames++;
>> eepro100_fr_interrupt(s);
>> s->ru_offset = le32_to_cpu(rx.link);
>
>
> Hi,
>
> the patch looks reasonable. I only suggest a formal change:
>
> There are lots of unnecessary type casts in several of your patches.
> I marked them here, but they should be removed anywhere.
Agreed. However, I'm going to apply this series as I'd like to get it in for
the freeze. But David, please follow up with a patch to remove the unnecessary
type casts.
Regards,
Anthony Liguori
>
> Kind regards,
> Stefan
>
>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 01/14] Define DMA address and direction types
2011-10-31 6:06 ` [Qemu-devel] [PATCH 01/14] Define DMA address and direction types David Gibson
@ 2011-11-01 22:21 ` Anthony Liguori
0 siblings, 0 replies; 33+ messages in thread
From: Anthony Liguori @ 2011-11-01 22:21 UTC (permalink / raw)
To: David Gibson
Cc: mst, joerg.roedel, agraf, qemu-devel, avi, eduard.munteanu, rth
On 10/31/2011 01:06 AM, David Gibson wrote:
> As a preliminary to adding more extensive DMA and IOMMU infrastructure
> support into qemu, this patch defines a dma_addr_t for storing DMA bus
> addresses and a DMADirection enum which describes whether a DMA is
> from an external device to main memory or from main memory to an
> external device.
>
> For now dma_addr_t is just defined to be target_phys_addr_t, but in
> future, we can change this to support machines where we have bus
> addresses which don't necessarily have the same format as CPU physical
> addresses.
>
> Signed-off-by: David Gibson<david@gibson.dropbear.id.au>
Applied all. Thanks.
Regards,
Anthony Liguori
> ---
> dma.h | 9 +++++++++
> 1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/dma.h b/dma.h
> index 2bdc236..56e163a 100644
> --- a/dma.h
> +++ b/dma.h
> @@ -18,6 +18,15 @@
> typedef struct ScatterGatherEntry ScatterGatherEntry;
>
> #if defined(TARGET_PHYS_ADDR_BITS)
> +typedef target_phys_addr_t dma_addr_t;
> +
> +#define DMA_ADDR_FMT TARGET_FMT_plx
> +
> +typedef enum {
> + DMA_DIRECTION_TO_DEVICE = 0,
> + DMA_DIRECTION_FROM_DEVICE = 1,
> +} DMADirection;
> +
> struct ScatterGatherEntry {
> target_phys_addr_t base;
> target_phys_addr_t len;
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-10-31 6:06 ` [Qemu-devel] [PATCH 05/14] eepro100: " David Gibson
2011-10-31 16:45 ` Stefan Weil
@ 2011-11-02 7:16 ` Michael S. Tsirkin
2011-11-03 5:16 ` David Gibson
1 sibling, 1 reply; 33+ messages in thread
From: Michael S. Tsirkin @ 2011-11-02 7:16 UTC (permalink / raw)
To: David Gibson; +Cc: joerg.roedel, agraf, qemu-devel, avi, eduard.munteanu, rth
On Mon, Oct 31, 2011 at 05:06:49PM +1100, David Gibson wrote:
> From: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
>
> This updates the eepro100 device emulation to use the explicit PCI DMA
> functions, instead of directly calling physical memory access functions.
>
> Signed-off-by: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
> Signed-off-by: David Gibson <dwg@au1.ibm.com>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/eepro100.c | 121 +++++++++++++++++++++++----------------------------------
> 1 files changed, 49 insertions(+), 72 deletions(-)
>
> diff --git a/hw/eepro100.c b/hw/eepro100.c
> index 4e3c52f..7d59e71 100644
> --- a/hw/eepro100.c
> +++ b/hw/eepro100.c
> @@ -46,6 +46,7 @@
> #include "net.h"
> #include "eeprom93xx.h"
> #include "sysemu.h"
> +#include "dma.h"
>
> /* QEMU sends frames smaller than 60 bytes to ethernet nics.
> * Such frames are rejected by real nics and their emulations.
> @@ -315,38 +316,6 @@ static const uint16_t eepro100_mdi_mask[] = {
> 0xffff, 0xffff, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
> };
>
> -/* Read a 16 bit little endian value from physical memory. */
> -static uint16_t e100_ldw_le_phys(target_phys_addr_t addr)
> -{
> - /* Load 16 bit (little endian) word from emulated hardware. */
> - uint16_t val;
> - cpu_physical_memory_read(addr, &val, sizeof(val));
> - return le16_to_cpu(val);
> -}
> -
> -/* Read a 32 bit little endian value from physical memory. */
> -static uint32_t e100_ldl_le_phys(target_phys_addr_t addr)
> -{
> - /* Load 32 bit (little endian) word from emulated hardware. */
> - uint32_t val;
> - cpu_physical_memory_read(addr, &val, sizeof(val));
> - return le32_to_cpu(val);
> -}
> -
> -/* Write a 16 bit little endian value to physical memory. */
> -static void e100_stw_le_phys(target_phys_addr_t addr, uint16_t val)
> -{
> - val = cpu_to_le16(val);
> - cpu_physical_memory_write(addr, &val, sizeof(val));
> -}
> -
> -/* Write a 32 bit little endian value to physical memory. */
> -static void e100_stl_le_phys(target_phys_addr_t addr, uint32_t val)
> -{
> - val = cpu_to_le32(val);
> - cpu_physical_memory_write(addr, &val, sizeof(val));
> -}
> -
> #define POLYNOMIAL 0x04c11db6
>
> /* From FreeBSD */
> @@ -744,21 +713,26 @@ static void dump_statistics(EEPRO100State * s)
> * values which really matter.
> * Number of data should check configuration!!!
> */
> - cpu_physical_memory_write(s->statsaddr, &s->statistics, s->stats_size);
> - e100_stl_le_phys(s->statsaddr + 0, s->statistics.tx_good_frames);
> - e100_stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
> - e100_stl_le_phys(s->statsaddr + 48, s->statistics.rx_resource_errors);
> - e100_stl_le_phys(s->statsaddr + 60, s->statistics.rx_short_frame_errors);
> + pci_dma_write(&s->dev, s->statsaddr,
> + (uint8_t *) &s->statistics, s->stats_size);
> + stl_le_pci_dma(&s->dev, s->statsaddr + 0,
> + s->statistics.tx_good_frames);
> + stl_le_pci_dma(&s->dev, s->statsaddr + 36,
> + s->statistics.rx_good_frames);
> + stl_le_pci_dma(&s->dev, s->statsaddr + 48,
> + s->statistics.rx_resource_errors);
> + stl_le_pci_dma(&s->dev, s->statsaddr + 60,
> + s->statistics.rx_short_frame_errors);
This might introduce a bug: stlXX APIs assume aligned addresses,
an address in statsaddr is user-controlled so I'm not sure
it's always aligned.
Why isn't the patch simply replacing cpu_physical_memory_read
with pci_XXX ? Any cleanups should be done separately.
> #if 0
> - e100_stw_le_phys(s->statsaddr + 76, s->statistics.xmt_tco_frames);
> - e100_stw_le_phys(s->statsaddr + 78, s->statistics.rcv_tco_frames);
> + stw_le_pci_dma(&s->dev, s->statsaddr + 76, s->statistics.xmt_tco_frames);
> + stw_le_pci_dma(&s->dev, s->statsaddr + 78, s->statistics.rcv_tco_frames);
> missing("CU dump statistical counters");
> #endif
> }
>
> static void read_cb(EEPRO100State *s)
> {
> - cpu_physical_memory_read(s->cb_address, &s->tx, sizeof(s->tx));
> + pci_dma_read(&s->dev, s->cb_address, (uint8_t *) &s->tx, sizeof(s->tx));
> s->tx.status = le16_to_cpu(s->tx.status);
> s->tx.command = le16_to_cpu(s->tx.command);
> s->tx.link = le32_to_cpu(s->tx.link);
> @@ -788,18 +762,17 @@ static void tx_command(EEPRO100State *s)
> }
> assert(tcb_bytes <= sizeof(buf));
> while (size < tcb_bytes) {
> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev, tbd_address);
> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev, tbd_address + 4);
> #if 0
> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
> #endif
> tbd_address += 8;
> TRACE(RXTX, logout
> ("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n",
> tx_buffer_address, tx_buffer_size));
> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> - cpu_physical_memory_read(tx_buffer_address, &buf[size],
> - tx_buffer_size);
> + pci_dma_read(&s->dev, tx_buffer_address, &buf[size], tx_buffer_size);
> size += tx_buffer_size;
> }
> if (tbd_array == 0xffffffff) {
> @@ -810,16 +783,19 @@ static void tx_command(EEPRO100State *s)
> if (s->has_extended_tcb_support && !(s->configuration[6] & BIT(4))) {
> /* Extended Flexible TCB. */
> for (; tbd_count < 2; tbd_count++) {
> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev,
> + tbd_address);
> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev,
> + tbd_address + 4);
> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev,
> + tbd_address + 6);
> tbd_address += 8;
> TRACE(RXTX, logout
> ("TBD (extended flexible mode): buffer address 0x%08x, size 0x%04x\n",
> tx_buffer_address, tx_buffer_size));
> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> - cpu_physical_memory_read(tx_buffer_address, &buf[size],
> - tx_buffer_size);
> + pci_dma_read(&s->dev, tx_buffer_address,
> + &buf[size], tx_buffer_size);
> size += tx_buffer_size;
> if (tx_buffer_el & 1) {
> break;
> @@ -828,16 +804,16 @@ static void tx_command(EEPRO100State *s)
> }
> tbd_address = tbd_array;
> for (; tbd_count < s->tx.tbd_count; tbd_count++) {
> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev, tbd_address);
> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev, tbd_address + 4);
> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
> tbd_address += 8;
> TRACE(RXTX, logout
> ("TBD (flexible mode): buffer address 0x%08x, size 0x%04x\n",
> tx_buffer_address, tx_buffer_size));
> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> - cpu_physical_memory_read(tx_buffer_address, &buf[size],
> - tx_buffer_size);
> + pci_dma_read(&s->dev, tx_buffer_address,
> + &buf[size], tx_buffer_size);
> size += tx_buffer_size;
> if (tx_buffer_el & 1) {
> break;
> @@ -862,7 +838,7 @@ static void set_multicast_list(EEPRO100State *s)
> TRACE(OTHER, logout("multicast list, multicast count = %u\n", multicast_count));
> for (i = 0; i < multicast_count; i += 6) {
> uint8_t multicast_addr[6];
> - cpu_physical_memory_read(s->cb_address + 10 + i, multicast_addr, 6);
> + pci_dma_read(&s->dev, s->cb_address + 10 + i, multicast_addr, 6);
> TRACE(OTHER, logout("multicast entry %s\n", nic_dump(multicast_addr, 6)));
> unsigned mcast_idx = compute_mcast_idx(multicast_addr);
> assert(mcast_idx < 64);
> @@ -896,12 +872,12 @@ static void action_command(EEPRO100State *s)
> /* Do nothing. */
> break;
> case CmdIASetup:
> - cpu_physical_memory_read(s->cb_address + 8, &s->conf.macaddr.a[0], 6);
> + pci_dma_read(&s->dev, s->cb_address + 8, &s->conf.macaddr.a[0], 6);
> TRACE(OTHER, logout("macaddr: %s\n", nic_dump(&s->conf.macaddr.a[0], 6)));
> break;
> case CmdConfigure:
> - cpu_physical_memory_read(s->cb_address + 8, &s->configuration[0],
> - sizeof(s->configuration));
> + pci_dma_read(&s->dev, s->cb_address + 8,
> + &s->configuration[0], sizeof(s->configuration));
> TRACE(OTHER, logout("configuration: %s\n",
> nic_dump(&s->configuration[0], 16)));
> TRACE(OTHER, logout("configuration: %s\n",
> @@ -938,7 +914,8 @@ static void action_command(EEPRO100State *s)
> break;
> }
> /* Write new status. */
> - e100_stw_le_phys(s->cb_address, s->tx.status | ok_status | STATUS_C);
> + stw_le_pci_dma(&s->dev, s->cb_address,
> + s->tx.status | ok_status | STATUS_C);
> if (bit_i) {
> /* CU completed action. */
> eepro100_cx_interrupt(s);
> @@ -1005,7 +982,7 @@ static void eepro100_cu_command(EEPRO100State * s, uint8_t val)
> /* Dump statistical counters. */
> TRACE(OTHER, logout("val=0x%02x (dump stats)\n", val));
> dump_statistics(s);
> - e100_stl_le_phys(s->statsaddr + s->stats_size, 0xa005);
> + stl_le_pci_dma(&s->dev, s->statsaddr + s->stats_size, 0xa005);
> break;
> case CU_CMD_BASE:
> /* Load CU base. */
> @@ -1016,7 +993,7 @@ static void eepro100_cu_command(EEPRO100State * s, uint8_t val)
> /* Dump and reset statistical counters. */
> TRACE(OTHER, logout("val=0x%02x (dump stats and reset)\n", val));
> dump_statistics(s);
> - e100_stl_le_phys(s->statsaddr + s->stats_size, 0xa007);
> + stl_le_pci_dma(&s->dev, s->statsaddr + s->stats_size, 0xa007);
> memset(&s->statistics, 0, sizeof(s->statistics));
> break;
> case CU_SRESUME:
> @@ -1310,10 +1287,10 @@ static void eepro100_write_port(EEPRO100State *s)
> case PORT_SELFTEST:
> TRACE(OTHER, logout("selftest address=0x%08x\n", address));
> eepro100_selftest_t data;
> - cpu_physical_memory_read(address, &data, sizeof(data));
> + pci_dma_read(&s->dev, address, (uint8_t *) &data, sizeof(data));
> data.st_sign = 0xffffffff;
> data.st_result = 0;
> - cpu_physical_memory_write(address, &data, sizeof(data));
> + pci_dma_write(&s->dev, address, (uint8_t *) &data, sizeof(data));
> break;
> case PORT_SELECTIVE_RESET:
> TRACE(OTHER, logout("selective reset, selftest address=0x%08x\n", address));
> @@ -1729,8 +1706,8 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
> }
> /* !!! */
> eepro100_rx_t rx;
> - cpu_physical_memory_read(s->ru_base + s->ru_offset, &rx,
> - sizeof(eepro100_rx_t));
> + pci_dma_read(&s->dev, s->ru_base + s->ru_offset,
> + (uint8_t *) &rx, sizeof(eepro100_rx_t));
> uint16_t rfd_command = le16_to_cpu(rx.command);
> uint16_t rfd_size = le16_to_cpu(rx.size);
>
> @@ -1746,10 +1723,10 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
> #endif
> TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n",
> rfd_command, rx.link, rx.rx_buf_addr, rfd_size));
> - e100_stw_le_phys(s->ru_base + s->ru_offset +
> - offsetof(eepro100_rx_t, status), rfd_status);
> - e100_stw_le_phys(s->ru_base + s->ru_offset +
> - offsetof(eepro100_rx_t, count), size);
> + stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
> + offsetof(eepro100_rx_t, status), rfd_status);
> + stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
> + offsetof(eepro100_rx_t, count), size);
> /* Early receive interrupt not supported. */
> #if 0
> eepro100_er_interrupt(s);
> @@ -1763,8 +1740,8 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
> #if 0
> assert(!(s->configuration[17] & BIT(0)));
> #endif
> - cpu_physical_memory_write(s->ru_base + s->ru_offset +
> - sizeof(eepro100_rx_t), buf, size);
> + pci_dma_write(&s->dev, s->ru_base + s->ru_offset +
> + sizeof(eepro100_rx_t), buf, size);
> s->statistics.rx_good_frames++;
> eepro100_fr_interrupt(s);
> s->ru_offset = le32_to_cpu(rx.link);
> --
> 1.7.7
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 03/14] Add stub functions for PCI device models to do PCI DMA
2011-10-31 6:06 ` [Qemu-devel] [PATCH 03/14] Add stub functions for PCI device models to do PCI DMA David Gibson
@ 2011-11-02 7:17 ` Michael S. Tsirkin
2011-11-02 11:47 ` David Gibson
0 siblings, 1 reply; 33+ messages in thread
From: Michael S. Tsirkin @ 2011-11-02 7:17 UTC (permalink / raw)
To: David Gibson; +Cc: joerg.roedel, agraf, qemu-devel, avi, eduard.munteanu, rth
On Mon, Oct 31, 2011 at 05:06:47PM +1100, David Gibson wrote:
> This patch adds functions to pci.[ch] to perform PCI DMA operations.
> At present, these are just stubs which perform directly cpu physical
> memory accesses. Stubs are included which are analogous to
> cpu_physical_memory_{read,write}(), the stX_phys() and ldX_phys()
> functions and cpu_physical_memory_{map,unmap}().
>
> In addition, a wrapper around qemu_sglist_init() is provided, which
> also takes a PCIDevice *. It's assumed that _init() is the only
> sglist function which will need wrapping, the idea being that once we
> have IOMMU support whatever IOMMU context handle the wrapper derives
> from the PCI device will be stored within the sglist structure for
> later use.
>
> Using these stubs, however, distinguishes PCI device DMA transactions from
> other accesses to physical memory, which will allow PCI IOMMU support to
> be added in one place, rather than updating every PCI driver at that time.
>
> That is, it allows us to update individual PCI drivers to support an IOMMU
> without having yet determined the details of how the IOMMU emulation will
> operate. This will let us remove the most bitrot-sensitive part of an
> IOMMU patch in advance.
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
> hw/pci.h | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 67 insertions(+), 0 deletions(-)
>
> diff --git a/hw/pci.h b/hw/pci.h
> index 86a81c8..c449a90 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -6,6 +6,7 @@
>
> #include "qdev.h"
> #include "memory.h"
> +#include "dma.h"
>
> /* PCI includes legacy ISA access. */
> #include "isa.h"
> @@ -487,4 +488,70 @@ static inline uint32_t pci_config_size(const PCIDevice *d)
> return pci_is_express(d) ? PCIE_CONFIG_SPACE_SIZE : PCI_CONFIG_SPACE_SIZE;
> }
>
> +/* DMA access functions */
> +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);
> + return 0;
> +}
> +
> +static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr,
> + void *buf, dma_addr_t len)
> +{
> + return pci_dma_rw(dev, addr, buf, len, DMA_DIRECTION_TO_DEVICE);
> +}
> +
> +static inline int pci_dma_write(PCIDevice *dev, dma_addr_t addr,
> + const void *buf, dma_addr_t len)
> +{
> + return pci_dma_rw(dev, addr, (void *) buf, len, DMA_DIRECTION_FROM_DEVICE);
> +}
> +
> +#define PCI_DMA_DEFINE_LDST(_l, _s, _bits) \
> + static inline uint##_bits##_t ld##_l##_pci_dma(PCIDevice *dev, \
> + dma_addr_t addr) \
> + { \
> + return ld##_l##_phys(addr); \
> + } \
> + static inline void st##_s##_pci_dma(PCIDevice *dev, \
> + dma_addr_t addr, uint##_bits##_t val) \
> + { \
> + st##_s##_phys(addr, val); \
> + }
> +
I'd much rather name APIs pci_dma_XXX.
why use a suffix and not a prefix?
> +PCI_DMA_DEFINE_LDST(ub, b, 8);
> +PCI_DMA_DEFINE_LDST(uw_le, w_le, 16)
> +PCI_DMA_DEFINE_LDST(l_le, l_le, 32);
> +PCI_DMA_DEFINE_LDST(q_le, q_le, 64);
> +PCI_DMA_DEFINE_LDST(uw_be, w_be, 16)
> +PCI_DMA_DEFINE_LDST(l_be, l_be, 32);
> +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,
> + 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;
> + 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);
> +}
> +
> +static inline void pci_dma_sglist_init(QEMUSGList *qsg, PCIDevice *dev,
> + int alloc_hint)
> +{
> + qemu_sglist_init(qsg, alloc_hint);
> +}
> +
> #endif
> --
> 1.7.7
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-11-01 20:24 ` Anthony Liguori
@ 2011-11-02 7:40 ` Michael S. Tsirkin
2011-11-02 12:46 ` Anthony Liguori
2011-11-02 17:56 ` Alexander Graf
2011-11-02 11:01 ` Michael S. Tsirkin
1 sibling, 2 replies; 33+ messages in thread
From: Michael S. Tsirkin @ 2011-11-02 7:40 UTC (permalink / raw)
To: Anthony Liguori
Cc: Stefan Weil, eduard.munteanu, qemu-devel, agraf, avi,
joerg.roedel, rth, David Gibson
On Tue, Nov 01, 2011 at 03:24:28PM -0500, Anthony Liguori wrote:
> On 10/31/2011 11:45 AM, Stefan Weil wrote:
> >Am 31.10.2011 07:06, schrieb David Gibson:
> >>From: Eduard - Gabriel Munteanu<eduard.munteanu@linux360.ro>
> >>
> >>This updates the eepro100 device emulation to use the explicit PCI DMA
> >>functions, instead of directly calling physical memory access functions.
> >>
> >>Signed-off-by: Eduard - Gabriel Munteanu<eduard.munteanu@linux360.ro>
> >>Signed-off-by: David Gibson<dwg@au1.ibm.com>
> >>Signed-off-by: Alexey Kardashevskiy<aik@ozlabs.ru>
> >>---
> >>hw/eepro100.c | 121 +++++++++++++++++++++++----------------------------------
> >>1 files changed, 49 insertions(+), 72 deletions(-)
> >>
> >>diff --git a/hw/eepro100.c b/hw/eepro100.c
> >>index 4e3c52f..7d59e71 100644
> >>--- a/hw/eepro100.c
> >>+++ b/hw/eepro100.c
> >>@@ -46,6 +46,7 @@
> >>#include "net.h"
> >>#include "eeprom93xx.h"
> >>#include "sysemu.h"
> >>+#include "dma.h"
> >>
> >>/* QEMU sends frames smaller than 60 bytes to ethernet nics.
> >>* Such frames are rejected by real nics and their emulations.
> >>@@ -315,38 +316,6 @@ static const uint16_t eepro100_mdi_mask[] = {
> >>0xffff, 0xffff, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
> >>};
> >>
> >>-/* Read a 16 bit little endian value from physical memory. */
> >>-static uint16_t e100_ldw_le_phys(target_phys_addr_t addr)
> >>-{
> >>- /* Load 16 bit (little endian) word from emulated hardware. */
> >>- uint16_t val;
> >>- cpu_physical_memory_read(addr,&val, sizeof(val));
> >>- return le16_to_cpu(val);
> >>-}
> >>-
> >>-/* Read a 32 bit little endian value from physical memory. */
> >>-static uint32_t e100_ldl_le_phys(target_phys_addr_t addr)
> >>-{
> >>- /* Load 32 bit (little endian) word from emulated hardware. */
> >>- uint32_t val;
> >>- cpu_physical_memory_read(addr,&val, sizeof(val));
> >>- return le32_to_cpu(val);
> >>-}
> >>-
> >>-/* Write a 16 bit little endian value to physical memory. */
> >>-static void e100_stw_le_phys(target_phys_addr_t addr, uint16_t val)
> >>-{
> >>- val = cpu_to_le16(val);
> >>- cpu_physical_memory_write(addr,&val, sizeof(val));
> >>-}
> >>-
> >>-/* Write a 32 bit little endian value to physical memory. */
> >>-static void e100_stl_le_phys(target_phys_addr_t addr, uint32_t val)
> >>-{
> >>- val = cpu_to_le32(val);
> >>- cpu_physical_memory_write(addr,&val, sizeof(val));
> >>-}
> >>-
> >>#define POLYNOMIAL 0x04c11db6
> >>
> >>/* From FreeBSD */
> >>@@ -744,21 +713,26 @@ static void dump_statistics(EEPRO100State * s)
> >>* values which really matter.
> >>* Number of data should check configuration!!!
> >>*/
> >>- cpu_physical_memory_write(s->statsaddr,&s->statistics, s->stats_size);
> >>- e100_stl_le_phys(s->statsaddr + 0, s->statistics.tx_good_frames);
> >>- e100_stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
> >>- e100_stl_le_phys(s->statsaddr + 48, s->statistics.rx_resource_errors);
> >>- e100_stl_le_phys(s->statsaddr + 60, s->statistics.rx_short_frame_errors);
> >>+ pci_dma_write(&s->dev, s->statsaddr,
> >>+ (uint8_t *)&s->statistics, s->stats_size);
> >The type cast is not needed and should be removed.
> >>+ stl_le_pci_dma(&s->dev, s->statsaddr + 0,
> >>+ s->statistics.tx_good_frames);
> >>+ stl_le_pci_dma(&s->dev, s->statsaddr + 36,
> >>+ s->statistics.rx_good_frames);
> >>+ stl_le_pci_dma(&s->dev, s->statsaddr + 48,
> >>+ s->statistics.rx_resource_errors);
> >>+ stl_le_pci_dma(&s->dev, s->statsaddr + 60,
> >>+ s->statistics.rx_short_frame_errors);
> >>#if 0
> >>- e100_stw_le_phys(s->statsaddr + 76, s->statistics.xmt_tco_frames);
> >>- e100_stw_le_phys(s->statsaddr + 78, s->statistics.rcv_tco_frames);
> >>+ stw_le_pci_dma(&s->dev, s->statsaddr + 76, s->statistics.xmt_tco_frames);
> >>+ stw_le_pci_dma(&s->dev, s->statsaddr + 78, s->statistics.rcv_tco_frames);
> >>missing("CU dump statistical counters");
> >>#endif
> >>}
> >>
> >>static void read_cb(EEPRO100State *s)
> >>{
> >>- cpu_physical_memory_read(s->cb_address,&s->tx, sizeof(s->tx));
> >>+ pci_dma_read(&s->dev, s->cb_address, (uint8_t *)&s->tx, sizeof(s->tx));
> >The type cast is not needed and should be removed.
> >>s->tx.status = le16_to_cpu(s->tx.status);
> >>s->tx.command = le16_to_cpu(s->tx.command);
> >>s->tx.link = le32_to_cpu(s->tx.link);
> >>@@ -788,18 +762,17 @@ static void tx_command(EEPRO100State *s)
> >>}
> >>assert(tcb_bytes<= sizeof(buf));
> >>while (size< tcb_bytes) {
> >>- uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
> >>- uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
> >>+ uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev, tbd_address);
> >>+ uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev, tbd_address + 4);
> >>#if 0
> >>- uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
> >>+ uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
> >>#endif
> >>tbd_address += 8;
> >>TRACE(RXTX, logout
> >>("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n",
> >>tx_buffer_address, tx_buffer_size));
> >>tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> >>- cpu_physical_memory_read(tx_buffer_address,&buf[size],
> >>- tx_buffer_size);
> >>+ pci_dma_read(&s->dev, tx_buffer_address,&buf[size], tx_buffer_size);
> >>size += tx_buffer_size;
> >>}
> >>if (tbd_array == 0xffffffff) {
> >>@@ -810,16 +783,19 @@ static void tx_command(EEPRO100State *s)
> >>if (s->has_extended_tcb_support&& !(s->configuration[6]& BIT(4))) {
> >>/* Extended Flexible TCB. */
> >>for (; tbd_count< 2; tbd_count++) {
> >>- uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
> >>- uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
> >>- uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
> >>+ uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev,
> >>+ tbd_address);
> >>+ uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev,
> >>+ tbd_address + 4);
> >>+ uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev,
> >>+ tbd_address + 6);
> >>tbd_address += 8;
> >>TRACE(RXTX, logout
> >>("TBD (extended flexible mode): buffer address 0x%08x, size 0x%04x\n",
> >>tx_buffer_address, tx_buffer_size));
> >>tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> >>- cpu_physical_memory_read(tx_buffer_address,&buf[size],
> >>- tx_buffer_size);
> >>+ pci_dma_read(&s->dev, tx_buffer_address,
> >>+&buf[size], tx_buffer_size);
> >>size += tx_buffer_size;
> >>if (tx_buffer_el& 1) {
> >>break;
> >>@@ -828,16 +804,16 @@ static void tx_command(EEPRO100State *s)
> >>}
> >>tbd_address = tbd_array;
> >>for (; tbd_count< s->tx.tbd_count; tbd_count++) {
> >>- uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
> >>- uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
> >>- uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
> >>+ uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev, tbd_address);
> >>+ uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev, tbd_address + 4);
> >>+ uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
> >>tbd_address += 8;
> >>TRACE(RXTX, logout
> >>("TBD (flexible mode): buffer address 0x%08x, size 0x%04x\n",
> >>tx_buffer_address, tx_buffer_size));
> >>tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
> >>- cpu_physical_memory_read(tx_buffer_address,&buf[size],
> >>- tx_buffer_size);
> >>+ pci_dma_read(&s->dev, tx_buffer_address,
> >>+&buf[size], tx_buffer_size);
> >>size += tx_buffer_size;
> >>if (tx_buffer_el& 1) {
> >>break;
> >>@@ -862,7 +838,7 @@ static void set_multicast_list(EEPRO100State *s)
> >>TRACE(OTHER, logout("multicast list, multicast count = %u\n", multicast_count));
> >>for (i = 0; i< multicast_count; i += 6) {
> >>uint8_t multicast_addr[6];
> >>- cpu_physical_memory_read(s->cb_address + 10 + i, multicast_addr, 6);
> >>+ pci_dma_read(&s->dev, s->cb_address + 10 + i, multicast_addr, 6);
> >>TRACE(OTHER, logout("multicast entry %s\n", nic_dump(multicast_addr, 6)));
> >>unsigned mcast_idx = compute_mcast_idx(multicast_addr);
> >>assert(mcast_idx< 64);
> >>@@ -896,12 +872,12 @@ static void action_command(EEPRO100State *s)
> >>/* Do nothing. */
> >>break;
> >>case CmdIASetup:
> >>- cpu_physical_memory_read(s->cb_address + 8,&s->conf.macaddr.a[0], 6);
> >>+ pci_dma_read(&s->dev, s->cb_address + 8,&s->conf.macaddr.a[0], 6);
> >>TRACE(OTHER, logout("macaddr: %s\n", nic_dump(&s->conf.macaddr.a[0], 6)));
> >>break;
> >>case CmdConfigure:
> >>- cpu_physical_memory_read(s->cb_address + 8,&s->configuration[0],
> >>- sizeof(s->configuration));
> >>+ pci_dma_read(&s->dev, s->cb_address + 8,
> >>+&s->configuration[0], sizeof(s->configuration));
> >>TRACE(OTHER, logout("configuration: %s\n",
> >>nic_dump(&s->configuration[0], 16)));
> >>TRACE(OTHER, logout("configuration: %s\n",
> >>@@ -938,7 +914,8 @@ static void action_command(EEPRO100State *s)
> >>break;
> >>}
> >>/* Write new status. */
> >>- e100_stw_le_phys(s->cb_address, s->tx.status | ok_status | STATUS_C);
> >>+ stw_le_pci_dma(&s->dev, s->cb_address,
> >>+ s->tx.status | ok_status | STATUS_C);
> >>if (bit_i) {
> >>/* CU completed action. */
> >>eepro100_cx_interrupt(s);
> >>@@ -1005,7 +982,7 @@ static void eepro100_cu_command(EEPRO100State * s,
> >>uint8_t val)
> >>/* Dump statistical counters. */
> >>TRACE(OTHER, logout("val=0x%02x (dump stats)\n", val));
> >>dump_statistics(s);
> >>- e100_stl_le_phys(s->statsaddr + s->stats_size, 0xa005);
> >>+ stl_le_pci_dma(&s->dev, s->statsaddr + s->stats_size, 0xa005);
> >>break;
> >>case CU_CMD_BASE:
> >>/* Load CU base. */
> >>@@ -1016,7 +993,7 @@ static void eepro100_cu_command(EEPRO100State * s,
> >>uint8_t val)
> >>/* Dump and reset statistical counters. */
> >>TRACE(OTHER, logout("val=0x%02x (dump stats and reset)\n", val));
> >>dump_statistics(s);
> >>- e100_stl_le_phys(s->statsaddr + s->stats_size, 0xa007);
> >>+ stl_le_pci_dma(&s->dev, s->statsaddr + s->stats_size, 0xa007);
> >>memset(&s->statistics, 0, sizeof(s->statistics));
> >>break;
> >>case CU_SRESUME:
> >>@@ -1310,10 +1287,10 @@ static void eepro100_write_port(EEPRO100State *s)
> >>case PORT_SELFTEST:
> >>TRACE(OTHER, logout("selftest address=0x%08x\n", address));
> >>eepro100_selftest_t data;
> >>- cpu_physical_memory_read(address,&data, sizeof(data));
> >>+ pci_dma_read(&s->dev, address, (uint8_t *)&data, sizeof(data));
> >The type cast is not needed and should be removed.
> >>data.st_sign = 0xffffffff;
> >>data.st_result = 0;
> >>- cpu_physical_memory_write(address,&data, sizeof(data));
> >>+ pci_dma_write(&s->dev, address, (uint8_t *)&data, sizeof(data));
> >The type cast is not needed and should be removed.
> >>break;
> >>case PORT_SELECTIVE_RESET:
> >>TRACE(OTHER, logout("selective reset, selftest address=0x%08x\n", address));
> >>@@ -1729,8 +1706,8 @@ static ssize_t nic_receive(VLANClientState *nc, const
> >>uint8_t * buf, size_t size
> >>}
> >>/* !!! */
> >>eepro100_rx_t rx;
> >>- cpu_physical_memory_read(s->ru_base + s->ru_offset,&rx,
> >>- sizeof(eepro100_rx_t));
> >>+ pci_dma_read(&s->dev, s->ru_base + s->ru_offset,
> >>+ (uint8_t *)&rx, sizeof(eepro100_rx_t));
> >
> >The type cast is not needed and should be removed.
> >
> >
> >>uint16_t rfd_command = le16_to_cpu(rx.command);
> >>uint16_t rfd_size = le16_to_cpu(rx.size);
> >>
> >>@@ -1746,10 +1723,10 @@ static ssize_t nic_receive(VLANClientState *nc, const
> >>uint8_t * buf, size_t size
> >>#endif
> >>TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n",
> >>rfd_command, rx.link, rx.rx_buf_addr, rfd_size));
> >>- e100_stw_le_phys(s->ru_base + s->ru_offset +
> >>- offsetof(eepro100_rx_t, status), rfd_status);
> >>- e100_stw_le_phys(s->ru_base + s->ru_offset +
> >>- offsetof(eepro100_rx_t, count), size);
> >>+ stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
> >>+ offsetof(eepro100_rx_t, status), rfd_status);
> >>+ stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
> >>+ offsetof(eepro100_rx_t, count), size);
> >>/* Early receive interrupt not supported. */
> >>#if 0
> >>eepro100_er_interrupt(s);
> >>@@ -1763,8 +1740,8 @@ static ssize_t nic_receive(VLANClientState *nc, const
> >>uint8_t * buf, size_t size
> >>#if 0
> >>assert(!(s->configuration[17]& BIT(0)));
> >>#endif
> >>- cpu_physical_memory_write(s->ru_base + s->ru_offset +
> >>- sizeof(eepro100_rx_t), buf, size);
> >>+ pci_dma_write(&s->dev, s->ru_base + s->ru_offset +
> >>+ sizeof(eepro100_rx_t), buf, size);
> >>s->statistics.rx_good_frames++;
> >>eepro100_fr_interrupt(s);
> >>s->ru_offset = le32_to_cpu(rx.link);
> >
> >
> >Hi,
> >
> >the patch looks reasonable. I only suggest a formal change:
> >
> >There are lots of unnecessary type casts in several of your patches.
> >I marked them here, but they should be removed anywhere.
>
> Agreed. However, I'm going to apply this series as I'd like to get
> it in for the freeze.
What does it get us, applying it before freeze?
It's an api change without new functionality.
Seems better on -next branch.
> But David, please follow up with a patch to
> remove the unnecessary type casts.
>
> Regards,
>
> Anthony Liguori
>
> >
> >Kind regards,
> >Stefan
> >
> >
> >
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-11-01 20:24 ` Anthony Liguori
2011-11-02 7:40 ` Michael S. Tsirkin
@ 2011-11-02 11:01 ` Michael S. Tsirkin
1 sibling, 0 replies; 33+ messages in thread
From: Michael S. Tsirkin @ 2011-11-02 11:01 UTC (permalink / raw)
To: Anthony Liguori
Cc: Stefan Weil, eduard.munteanu, qemu-devel, agraf, avi,
joerg.roedel, rth, David Gibson
On Tue, Nov 01, 2011 at 03:24:28PM -0500, Anthony Liguori wrote:
> >Hi,
> >
> >the patch looks reasonable. I only suggest a formal change:
> >
> >There are lots of unnecessary type casts in several of your patches.
> >I marked them here, but they should be removed anywhere.
>
> Agreed. However, I'm going to apply this series as I'd like to get
> it in for the freeze. But David, please follow up with a patch to
> remove the unnecessary type casts.
>
> Regards,
>
> Anthony Liguori
Further, eepro100 now needs to be fixed to use pci_dma_write/read
instead of stXXX/ldXXX, as address passed in apparently can be
unaligned.
--
MST
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 03/14] Add stub functions for PCI device models to do PCI DMA
2011-11-02 7:17 ` Michael S. Tsirkin
@ 2011-11-02 11:47 ` David Gibson
0 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2011-11-02 11:47 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: joerg.roedel, agraf, qemu-devel, avi, eduard.munteanu, rth
On Wed, Nov 02, 2011 at 09:17:37AM +0200, Michael S. Tsirkin wrote:
> On Mon, Oct 31, 2011 at 05:06:47PM +1100, David Gibson wrote:
> > This patch adds functions to pci.[ch] to perform PCI DMA operations.
> > At present, these are just stubs which perform directly cpu physical
> > memory accesses. Stubs are included which are analogous to
> > cpu_physical_memory_{read,write}(), the stX_phys() and ldX_phys()
> > functions and cpu_physical_memory_{map,unmap}().
> >
> > In addition, a wrapper around qemu_sglist_init() is provided, which
> > also takes a PCIDevice *. It's assumed that _init() is the only
> > sglist function which will need wrapping, the idea being that once we
> > have IOMMU support whatever IOMMU context handle the wrapper derives
> > from the PCI device will be stored within the sglist structure for
> > later use.
> >
> > Using these stubs, however, distinguishes PCI device DMA transactions from
> > other accesses to physical memory, which will allow PCI IOMMU support to
> > be added in one place, rather than updating every PCI driver at that time.
> >
> > That is, it allows us to update individual PCI drivers to support an IOMMU
> > without having yet determined the details of how the IOMMU emulation will
> > operate. This will let us remove the most bitrot-sensitive part of an
> > IOMMU patch in advance.
> >
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> > ---
> > hw/pci.h | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 files changed, 67 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/pci.h b/hw/pci.h
> > index 86a81c8..c449a90 100644
> > --- a/hw/pci.h
> > +++ b/hw/pci.h
> > @@ -6,6 +6,7 @@
> >
> > #include "qdev.h"
> > #include "memory.h"
> > +#include "dma.h"
> >
> > /* PCI includes legacy ISA access. */
> > #include "isa.h"
> > @@ -487,4 +488,70 @@ static inline uint32_t pci_config_size(const PCIDevice *d)
> > return pci_is_express(d) ? PCIE_CONFIG_SPACE_SIZE : PCI_CONFIG_SPACE_SIZE;
> > }
> >
> > +/* DMA access functions */
> > +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);
> > + return 0;
> > +}
> > +
> > +static inline int pci_dma_read(PCIDevice *dev, dma_addr_t addr,
> > + void *buf, dma_addr_t len)
> > +{
> > + return pci_dma_rw(dev, addr, buf, len, DMA_DIRECTION_TO_DEVICE);
> > +}
> > +
> > +static inline int pci_dma_write(PCIDevice *dev, dma_addr_t addr,
> > + const void *buf, dma_addr_t len)
> > +{
> > + return pci_dma_rw(dev, addr, (void *) buf, len, DMA_DIRECTION_FROM_DEVICE);
> > +}
> > +
> > +#define PCI_DMA_DEFINE_LDST(_l, _s, _bits) \
> > + static inline uint##_bits##_t ld##_l##_pci_dma(PCIDevice *dev, \
> > + dma_addr_t addr) \
> > + { \
> > + return ld##_l##_phys(addr); \
> > + } \
> > + static inline void st##_s##_pci_dma(PCIDevice *dev, \
> > + dma_addr_t addr, uint##_bits##_t val) \
> > + { \
> > + st##_s##_phys(addr, val); \
> > + }
> > +
>
> I'd much rather name APIs pci_dma_XXX.
> why use a suffix and not a prefix?
Because the analogous stX_phys/ldX_phys functions do.
--
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] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-11-02 7:40 ` Michael S. Tsirkin
@ 2011-11-02 12:46 ` Anthony Liguori
2011-11-02 17:56 ` Alexander Graf
1 sibling, 0 replies; 33+ messages in thread
From: Anthony Liguori @ 2011-11-02 12:46 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Stefan Weil, eduard.munteanu, agraf, qemu-devel, avi,
joerg.roedel, David Gibson, rth
On 11/02/2011 02:40 AM, Michael S. Tsirkin wrote:
> On Tue, Nov 01, 2011 at 03:24:28PM -0500, Anthony Liguori wrote:
>> On 10/31/2011 11:45 AM, Stefan Weil wrote:
>>> Am 31.10.2011 07:06, schrieb David Gibson:
>>>> From: Eduard - Gabriel Munteanu<eduard.munteanu@linux360.ro>
>>>>
>>>> This updates the eepro100 device emulation to use the explicit PCI DMA
>>>> functions, instead of directly calling physical memory access functions.
>>>>
>>>> Signed-off-by: Eduard - Gabriel Munteanu<eduard.munteanu@linux360.ro>
>>>> Signed-off-by: David Gibson<dwg@au1.ibm.com>
>>>> Signed-off-by: Alexey Kardashevskiy<aik@ozlabs.ru>
>>>> ---
>>>> hw/eepro100.c | 121 +++++++++++++++++++++++----------------------------------
>>>> 1 files changed, 49 insertions(+), 72 deletions(-)
>>>>
>>>> diff --git a/hw/eepro100.c b/hw/eepro100.c
>>>> index 4e3c52f..7d59e71 100644
>>>> --- a/hw/eepro100.c
>>>> +++ b/hw/eepro100.c
>>>> @@ -46,6 +46,7 @@
>>>> #include "net.h"
>>>> #include "eeprom93xx.h"
>>>> #include "sysemu.h"
>>>> +#include "dma.h"
>>>>
>>>> /* QEMU sends frames smaller than 60 bytes to ethernet nics.
>>>> * Such frames are rejected by real nics and their emulations.
>>>> @@ -315,38 +316,6 @@ static const uint16_t eepro100_mdi_mask[] = {
>>>> 0xffff, 0xffff, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
>>>> };
>>>>
>>>> -/* Read a 16 bit little endian value from physical memory. */
>>>> -static uint16_t e100_ldw_le_phys(target_phys_addr_t addr)
>>>> -{
>>>> - /* Load 16 bit (little endian) word from emulated hardware. */
>>>> - uint16_t val;
>>>> - cpu_physical_memory_read(addr,&val, sizeof(val));
>>>> - return le16_to_cpu(val);
>>>> -}
>>>> -
>>>> -/* Read a 32 bit little endian value from physical memory. */
>>>> -static uint32_t e100_ldl_le_phys(target_phys_addr_t addr)
>>>> -{
>>>> - /* Load 32 bit (little endian) word from emulated hardware. */
>>>> - uint32_t val;
>>>> - cpu_physical_memory_read(addr,&val, sizeof(val));
>>>> - return le32_to_cpu(val);
>>>> -}
>>>> -
>>>> -/* Write a 16 bit little endian value to physical memory. */
>>>> -static void e100_stw_le_phys(target_phys_addr_t addr, uint16_t val)
>>>> -{
>>>> - val = cpu_to_le16(val);
>>>> - cpu_physical_memory_write(addr,&val, sizeof(val));
>>>> -}
>>>> -
>>>> -/* Write a 32 bit little endian value to physical memory. */
>>>> -static void e100_stl_le_phys(target_phys_addr_t addr, uint32_t val)
>>>> -{
>>>> - val = cpu_to_le32(val);
>>>> - cpu_physical_memory_write(addr,&val, sizeof(val));
>>>> -}
>>>> -
>>>> #define POLYNOMIAL 0x04c11db6
>>>>
>>>> /* From FreeBSD */
>>>> @@ -744,21 +713,26 @@ static void dump_statistics(EEPRO100State * s)
>>>> * values which really matter.
>>>> * Number of data should check configuration!!!
>>>> */
>>>> - cpu_physical_memory_write(s->statsaddr,&s->statistics, s->stats_size);
>>>> - e100_stl_le_phys(s->statsaddr + 0, s->statistics.tx_good_frames);
>>>> - e100_stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
>>>> - e100_stl_le_phys(s->statsaddr + 48, s->statistics.rx_resource_errors);
>>>> - e100_stl_le_phys(s->statsaddr + 60, s->statistics.rx_short_frame_errors);
>>>> + pci_dma_write(&s->dev, s->statsaddr,
>>>> + (uint8_t *)&s->statistics, s->stats_size);
>>> The type cast is not needed and should be removed.
>>>> + stl_le_pci_dma(&s->dev, s->statsaddr + 0,
>>>> + s->statistics.tx_good_frames);
>>>> + stl_le_pci_dma(&s->dev, s->statsaddr + 36,
>>>> + s->statistics.rx_good_frames);
>>>> + stl_le_pci_dma(&s->dev, s->statsaddr + 48,
>>>> + s->statistics.rx_resource_errors);
>>>> + stl_le_pci_dma(&s->dev, s->statsaddr + 60,
>>>> + s->statistics.rx_short_frame_errors);
>>>> #if 0
>>>> - e100_stw_le_phys(s->statsaddr + 76, s->statistics.xmt_tco_frames);
>>>> - e100_stw_le_phys(s->statsaddr + 78, s->statistics.rcv_tco_frames);
>>>> + stw_le_pci_dma(&s->dev, s->statsaddr + 76, s->statistics.xmt_tco_frames);
>>>> + stw_le_pci_dma(&s->dev, s->statsaddr + 78, s->statistics.rcv_tco_frames);
>>>> missing("CU dump statistical counters");
>>>> #endif
>>>> }
>>>>
>>>> static void read_cb(EEPRO100State *s)
>>>> {
>>>> - cpu_physical_memory_read(s->cb_address,&s->tx, sizeof(s->tx));
>>>> + pci_dma_read(&s->dev, s->cb_address, (uint8_t *)&s->tx, sizeof(s->tx));
>>> The type cast is not needed and should be removed.
>>>> s->tx.status = le16_to_cpu(s->tx.status);
>>>> s->tx.command = le16_to_cpu(s->tx.command);
>>>> s->tx.link = le32_to_cpu(s->tx.link);
>>>> @@ -788,18 +762,17 @@ static void tx_command(EEPRO100State *s)
>>>> }
>>>> assert(tcb_bytes<= sizeof(buf));
>>>> while (size< tcb_bytes) {
>>>> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
>>>> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
>>>> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev, tbd_address);
>>>> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev, tbd_address + 4);
>>>> #if 0
>>>> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
>>>> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
>>>> #endif
>>>> tbd_address += 8;
>>>> TRACE(RXTX, logout
>>>> ("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n",
>>>> tx_buffer_address, tx_buffer_size));
>>>> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
>>>> - cpu_physical_memory_read(tx_buffer_address,&buf[size],
>>>> - tx_buffer_size);
>>>> + pci_dma_read(&s->dev, tx_buffer_address,&buf[size], tx_buffer_size);
>>>> size += tx_buffer_size;
>>>> }
>>>> if (tbd_array == 0xffffffff) {
>>>> @@ -810,16 +783,19 @@ static void tx_command(EEPRO100State *s)
>>>> if (s->has_extended_tcb_support&& !(s->configuration[6]& BIT(4))) {
>>>> /* Extended Flexible TCB. */
>>>> for (; tbd_count< 2; tbd_count++) {
>>>> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
>>>> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
>>>> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
>>>> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev,
>>>> + tbd_address);
>>>> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev,
>>>> + tbd_address + 4);
>>>> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev,
>>>> + tbd_address + 6);
>>>> tbd_address += 8;
>>>> TRACE(RXTX, logout
>>>> ("TBD (extended flexible mode): buffer address 0x%08x, size 0x%04x\n",
>>>> tx_buffer_address, tx_buffer_size));
>>>> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
>>>> - cpu_physical_memory_read(tx_buffer_address,&buf[size],
>>>> - tx_buffer_size);
>>>> + pci_dma_read(&s->dev, tx_buffer_address,
>>>> +&buf[size], tx_buffer_size);
>>>> size += tx_buffer_size;
>>>> if (tx_buffer_el& 1) {
>>>> break;
>>>> @@ -828,16 +804,16 @@ static void tx_command(EEPRO100State *s)
>>>> }
>>>> tbd_address = tbd_array;
>>>> for (; tbd_count< s->tx.tbd_count; tbd_count++) {
>>>> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
>>>> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
>>>> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
>>>> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev, tbd_address);
>>>> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev, tbd_address + 4);
>>>> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
>>>> tbd_address += 8;
>>>> TRACE(RXTX, logout
>>>> ("TBD (flexible mode): buffer address 0x%08x, size 0x%04x\n",
>>>> tx_buffer_address, tx_buffer_size));
>>>> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
>>>> - cpu_physical_memory_read(tx_buffer_address,&buf[size],
>>>> - tx_buffer_size);
>>>> + pci_dma_read(&s->dev, tx_buffer_address,
>>>> +&buf[size], tx_buffer_size);
>>>> size += tx_buffer_size;
>>>> if (tx_buffer_el& 1) {
>>>> break;
>>>> @@ -862,7 +838,7 @@ static void set_multicast_list(EEPRO100State *s)
>>>> TRACE(OTHER, logout("multicast list, multicast count = %u\n", multicast_count));
>>>> for (i = 0; i< multicast_count; i += 6) {
>>>> uint8_t multicast_addr[6];
>>>> - cpu_physical_memory_read(s->cb_address + 10 + i, multicast_addr, 6);
>>>> + pci_dma_read(&s->dev, s->cb_address + 10 + i, multicast_addr, 6);
>>>> TRACE(OTHER, logout("multicast entry %s\n", nic_dump(multicast_addr, 6)));
>>>> unsigned mcast_idx = compute_mcast_idx(multicast_addr);
>>>> assert(mcast_idx< 64);
>>>> @@ -896,12 +872,12 @@ static void action_command(EEPRO100State *s)
>>>> /* Do nothing. */
>>>> break;
>>>> case CmdIASetup:
>>>> - cpu_physical_memory_read(s->cb_address + 8,&s->conf.macaddr.a[0], 6);
>>>> + pci_dma_read(&s->dev, s->cb_address + 8,&s->conf.macaddr.a[0], 6);
>>>> TRACE(OTHER, logout("macaddr: %s\n", nic_dump(&s->conf.macaddr.a[0], 6)));
>>>> break;
>>>> case CmdConfigure:
>>>> - cpu_physical_memory_read(s->cb_address + 8,&s->configuration[0],
>>>> - sizeof(s->configuration));
>>>> + pci_dma_read(&s->dev, s->cb_address + 8,
>>>> +&s->configuration[0], sizeof(s->configuration));
>>>> TRACE(OTHER, logout("configuration: %s\n",
>>>> nic_dump(&s->configuration[0], 16)));
>>>> TRACE(OTHER, logout("configuration: %s\n",
>>>> @@ -938,7 +914,8 @@ static void action_command(EEPRO100State *s)
>>>> break;
>>>> }
>>>> /* Write new status. */
>>>> - e100_stw_le_phys(s->cb_address, s->tx.status | ok_status | STATUS_C);
>>>> + stw_le_pci_dma(&s->dev, s->cb_address,
>>>> + s->tx.status | ok_status | STATUS_C);
>>>> if (bit_i) {
>>>> /* CU completed action. */
>>>> eepro100_cx_interrupt(s);
>>>> @@ -1005,7 +982,7 @@ static void eepro100_cu_command(EEPRO100State * s,
>>>> uint8_t val)
>>>> /* Dump statistical counters. */
>>>> TRACE(OTHER, logout("val=0x%02x (dump stats)\n", val));
>>>> dump_statistics(s);
>>>> - e100_stl_le_phys(s->statsaddr + s->stats_size, 0xa005);
>>>> + stl_le_pci_dma(&s->dev, s->statsaddr + s->stats_size, 0xa005);
>>>> break;
>>>> case CU_CMD_BASE:
>>>> /* Load CU base. */
>>>> @@ -1016,7 +993,7 @@ static void eepro100_cu_command(EEPRO100State * s,
>>>> uint8_t val)
>>>> /* Dump and reset statistical counters. */
>>>> TRACE(OTHER, logout("val=0x%02x (dump stats and reset)\n", val));
>>>> dump_statistics(s);
>>>> - e100_stl_le_phys(s->statsaddr + s->stats_size, 0xa007);
>>>> + stl_le_pci_dma(&s->dev, s->statsaddr + s->stats_size, 0xa007);
>>>> memset(&s->statistics, 0, sizeof(s->statistics));
>>>> break;
>>>> case CU_SRESUME:
>>>> @@ -1310,10 +1287,10 @@ static void eepro100_write_port(EEPRO100State *s)
>>>> case PORT_SELFTEST:
>>>> TRACE(OTHER, logout("selftest address=0x%08x\n", address));
>>>> eepro100_selftest_t data;
>>>> - cpu_physical_memory_read(address,&data, sizeof(data));
>>>> + pci_dma_read(&s->dev, address, (uint8_t *)&data, sizeof(data));
>>> The type cast is not needed and should be removed.
>>>> data.st_sign = 0xffffffff;
>>>> data.st_result = 0;
>>>> - cpu_physical_memory_write(address,&data, sizeof(data));
>>>> + pci_dma_write(&s->dev, address, (uint8_t *)&data, sizeof(data));
>>> The type cast is not needed and should be removed.
>>>> break;
>>>> case PORT_SELECTIVE_RESET:
>>>> TRACE(OTHER, logout("selective reset, selftest address=0x%08x\n", address));
>>>> @@ -1729,8 +1706,8 @@ static ssize_t nic_receive(VLANClientState *nc, const
>>>> uint8_t * buf, size_t size
>>>> }
>>>> /* !!! */
>>>> eepro100_rx_t rx;
>>>> - cpu_physical_memory_read(s->ru_base + s->ru_offset,&rx,
>>>> - sizeof(eepro100_rx_t));
>>>> + pci_dma_read(&s->dev, s->ru_base + s->ru_offset,
>>>> + (uint8_t *)&rx, sizeof(eepro100_rx_t));
>>>
>>> The type cast is not needed and should be removed.
>>>
>>>
>>>> uint16_t rfd_command = le16_to_cpu(rx.command);
>>>> uint16_t rfd_size = le16_to_cpu(rx.size);
>>>>
>>>> @@ -1746,10 +1723,10 @@ static ssize_t nic_receive(VLANClientState *nc, const
>>>> uint8_t * buf, size_t size
>>>> #endif
>>>> TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n",
>>>> rfd_command, rx.link, rx.rx_buf_addr, rfd_size));
>>>> - e100_stw_le_phys(s->ru_base + s->ru_offset +
>>>> - offsetof(eepro100_rx_t, status), rfd_status);
>>>> - e100_stw_le_phys(s->ru_base + s->ru_offset +
>>>> - offsetof(eepro100_rx_t, count), size);
>>>> + stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
>>>> + offsetof(eepro100_rx_t, status), rfd_status);
>>>> + stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
>>>> + offsetof(eepro100_rx_t, count), size);
>>>> /* Early receive interrupt not supported. */
>>>> #if 0
>>>> eepro100_er_interrupt(s);
>>>> @@ -1763,8 +1740,8 @@ static ssize_t nic_receive(VLANClientState *nc, const
>>>> uint8_t * buf, size_t size
>>>> #if 0
>>>> assert(!(s->configuration[17]& BIT(0)));
>>>> #endif
>>>> - cpu_physical_memory_write(s->ru_base + s->ru_offset +
>>>> - sizeof(eepro100_rx_t), buf, size);
>>>> + pci_dma_write(&s->dev, s->ru_base + s->ru_offset +
>>>> + sizeof(eepro100_rx_t), buf, size);
>>>> s->statistics.rx_good_frames++;
>>>> eepro100_fr_interrupt(s);
>>>> s->ru_offset = le32_to_cpu(rx.link);
>>>
>>>
>>> Hi,
>>>
>>> the patch looks reasonable. I only suggest a formal change:
>>>
>>> There are lots of unnecessary type casts in several of your patches.
>>> I marked them here, but they should be removed anywhere.
>>
>> Agreed. However, I'm going to apply this series as I'd like to get
>> it in for the freeze.
>
> What does it get us, applying it before freeze?
> It's an api change without new functionality.
> Seems better on -next branch.
It was posted before soft freeze, had minimal feedback, and touches a lot of
files (is not pleasant to rebase).
Since it's just an internal change, it seems low risk to me.
We need a DMA API. There's a lot of work moving forward to convert devices to
use a DMA API. I'd rather start with an imperfect API and get the ball rolling
on conversions.
Regards,
Anthony Liguori
>
>> But David, please follow up with a patch to
>> remove the unnecessary type casts.
>>
>> Regards,
>>
>> Anthony Liguori
>>
>>>
>>> Kind regards,
>>> Stefan
>>>
>>>
>>>
>
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-11-02 17:56 ` Alexander Graf
@ 2011-11-02 17:19 ` Anthony Liguori
2011-11-02 17:28 ` Alexander Graf
2011-11-02 19:11 ` Michael S. Tsirkin
1 sibling, 1 reply; 33+ messages in thread
From: Anthony Liguori @ 2011-11-02 17:19 UTC (permalink / raw)
To: Alexander Graf
Cc: joerg.roedel, Michael S. Tsirkin, Stefan Weil, qemu-devel, avi,
eduard.munteanu, David Gibson, rth
On 11/02/2011 12:56 PM, Alexander Graf wrote:
> Michael S. Tsirkin wrote:
>> On Tue, Nov 01, 2011 at 03:24:28PM -0500, Anthony Liguori wrote:
>>
>> What does it get us, applying it before freeze?
>> It's an api change without new functionality.
>> Seems better on -next branch.
>>
>
> It gets us that downstreams can convert to the API for 1.0. It just
> feels a lot more right to change APIs for a 1.0 release than a 1.1 release.
>
> Also, it gives it more exposure and if anyone is inclined to have a
> small patch to add IOMMU support for a specific platform downstream,
> they can do so.
>
> Overall, I think it's a good idea to pull it in for 1.0. It certainly
> makes David's life easier :). Since we also have PCI support in -M
> pseries now we could even declare the non-usage of an IOMMU there as a
> bug and fix it for 1.0.1.
I was in violent agreement up until this last sentence ;-)
Having new bidirectional memory APIs is a really big positive infrastructure
change for us. It's been something we've needed for a long time.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-11-02 17:19 ` Anthony Liguori
@ 2011-11-02 17:28 ` Alexander Graf
0 siblings, 0 replies; 33+ messages in thread
From: Alexander Graf @ 2011-11-02 17:28 UTC (permalink / raw)
To: Anthony Liguori
Cc: joerg.roedel, Michael S. Tsirkin, Stefan Weil, qemu-devel, avi,
eduard.munteanu, David Gibson, rth
Anthony Liguori wrote:
> On 11/02/2011 12:56 PM, Alexander Graf wrote:
>> Michael S. Tsirkin wrote:
>>> On Tue, Nov 01, 2011 at 03:24:28PM -0500, Anthony Liguori wrote:
>>>
>>> What does it get us, applying it before freeze?
>>> It's an api change without new functionality.
>>> Seems better on -next branch.
>>>
>>
>> It gets us that downstreams can convert to the API for 1.0. It just
>> feels a lot more right to change APIs for a 1.0 release than a 1.1
>> release.
>>
>> Also, it gives it more exposure and if anyone is inclined to have a
>> small patch to add IOMMU support for a specific platform downstream,
>> they can do so.
>>
>> Overall, I think it's a good idea to pull it in for 1.0. It certainly
>> makes David's life easier :). Since we also have PCI support in -M
>> pseries now we could even declare the non-usage of an IOMMU there as a
>> bug and fix it for 1.0.1.
>
> I was in violent agreement up until this last sentence ;-)
Heh :). Well, I'm just saying that if the patch ends up being trivial
and really really useful and necessary for users, we at least have the
chance now.
Alex
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-11-02 7:40 ` Michael S. Tsirkin
2011-11-02 12:46 ` Anthony Liguori
@ 2011-11-02 17:56 ` Alexander Graf
2011-11-02 17:19 ` Anthony Liguori
2011-11-02 19:11 ` Michael S. Tsirkin
1 sibling, 2 replies; 33+ messages in thread
From: Alexander Graf @ 2011-11-02 17:56 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Stefan Weil, eduard.munteanu, qemu-devel, avi, joerg.roedel, rth,
David Gibson
Michael S. Tsirkin wrote:
> On Tue, Nov 01, 2011 at 03:24:28PM -0500, Anthony Liguori wrote:
>
>> On 10/31/2011 11:45 AM, Stefan Weil wrote:
>>
>>> Am 31.10.2011 07:06, schrieb David Gibson:
>>>
>>>> From: Eduard - Gabriel Munteanu<eduard.munteanu@linux360.ro>
>>>>
>>>> This updates the eepro100 device emulation to use the explicit PCI DMA
>>>> functions, instead of directly calling physical memory access functions.
>>>>
>>>> Signed-off-by: Eduard - Gabriel Munteanu<eduard.munteanu@linux360.ro>
>>>> Signed-off-by: David Gibson<dwg@au1.ibm.com>
>>>> Signed-off-by: Alexey Kardashevskiy<aik@ozlabs.ru>
>>>> ---
>>>> hw/eepro100.c | 121 +++++++++++++++++++++++----------------------------------
>>>> 1 files changed, 49 insertions(+), 72 deletions(-)
>>>>
>>>> diff --git a/hw/eepro100.c b/hw/eepro100.c
>>>> index 4e3c52f..7d59e71 100644
>>>> --- a/hw/eepro100.c
>>>> +++ b/hw/eepro100.c
>>>> @@ -46,6 +46,7 @@
>>>> #include "net.h"
>>>> #include "eeprom93xx.h"
>>>> #include "sysemu.h"
>>>> +#include "dma.h"
>>>>
>>>> /* QEMU sends frames smaller than 60 bytes to ethernet nics.
>>>> * Such frames are rejected by real nics and their emulations.
>>>> @@ -315,38 +316,6 @@ static const uint16_t eepro100_mdi_mask[] = {
>>>> 0xffff, 0xffff, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000, 0x0000,
>>>> };
>>>>
>>>> -/* Read a 16 bit little endian value from physical memory. */
>>>> -static uint16_t e100_ldw_le_phys(target_phys_addr_t addr)
>>>> -{
>>>> - /* Load 16 bit (little endian) word from emulated hardware. */
>>>> - uint16_t val;
>>>> - cpu_physical_memory_read(addr,&val, sizeof(val));
>>>> - return le16_to_cpu(val);
>>>> -}
>>>> -
>>>> -/* Read a 32 bit little endian value from physical memory. */
>>>> -static uint32_t e100_ldl_le_phys(target_phys_addr_t addr)
>>>> -{
>>>> - /* Load 32 bit (little endian) word from emulated hardware. */
>>>> - uint32_t val;
>>>> - cpu_physical_memory_read(addr,&val, sizeof(val));
>>>> - return le32_to_cpu(val);
>>>> -}
>>>> -
>>>> -/* Write a 16 bit little endian value to physical memory. */
>>>> -static void e100_stw_le_phys(target_phys_addr_t addr, uint16_t val)
>>>> -{
>>>> - val = cpu_to_le16(val);
>>>> - cpu_physical_memory_write(addr,&val, sizeof(val));
>>>> -}
>>>> -
>>>> -/* Write a 32 bit little endian value to physical memory. */
>>>> -static void e100_stl_le_phys(target_phys_addr_t addr, uint32_t val)
>>>> -{
>>>> - val = cpu_to_le32(val);
>>>> - cpu_physical_memory_write(addr,&val, sizeof(val));
>>>> -}
>>>> -
>>>> #define POLYNOMIAL 0x04c11db6
>>>>
>>>> /* From FreeBSD */
>>>> @@ -744,21 +713,26 @@ static void dump_statistics(EEPRO100State * s)
>>>> * values which really matter.
>>>> * Number of data should check configuration!!!
>>>> */
>>>> - cpu_physical_memory_write(s->statsaddr,&s->statistics, s->stats_size);
>>>> - e100_stl_le_phys(s->statsaddr + 0, s->statistics.tx_good_frames);
>>>> - e100_stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
>>>> - e100_stl_le_phys(s->statsaddr + 48, s->statistics.rx_resource_errors);
>>>> - e100_stl_le_phys(s->statsaddr + 60, s->statistics.rx_short_frame_errors);
>>>> + pci_dma_write(&s->dev, s->statsaddr,
>>>> + (uint8_t *)&s->statistics, s->stats_size);
>>>>
>>> The type cast is not needed and should be removed.
>>>
>>>> + stl_le_pci_dma(&s->dev, s->statsaddr + 0,
>>>> + s->statistics.tx_good_frames);
>>>> + stl_le_pci_dma(&s->dev, s->statsaddr + 36,
>>>> + s->statistics.rx_good_frames);
>>>> + stl_le_pci_dma(&s->dev, s->statsaddr + 48,
>>>> + s->statistics.rx_resource_errors);
>>>> + stl_le_pci_dma(&s->dev, s->statsaddr + 60,
>>>> + s->statistics.rx_short_frame_errors);
>>>> #if 0
>>>> - e100_stw_le_phys(s->statsaddr + 76, s->statistics.xmt_tco_frames);
>>>> - e100_stw_le_phys(s->statsaddr + 78, s->statistics.rcv_tco_frames);
>>>> + stw_le_pci_dma(&s->dev, s->statsaddr + 76, s->statistics.xmt_tco_frames);
>>>> + stw_le_pci_dma(&s->dev, s->statsaddr + 78, s->statistics.rcv_tco_frames);
>>>> missing("CU dump statistical counters");
>>>> #endif
>>>> }
>>>>
>>>> static void read_cb(EEPRO100State *s)
>>>> {
>>>> - cpu_physical_memory_read(s->cb_address,&s->tx, sizeof(s->tx));
>>>> + pci_dma_read(&s->dev, s->cb_address, (uint8_t *)&s->tx, sizeof(s->tx));
>>>>
>>> The type cast is not needed and should be removed.
>>>
>>>> s->tx.status = le16_to_cpu(s->tx.status);
>>>> s->tx.command = le16_to_cpu(s->tx.command);
>>>> s->tx.link = le32_to_cpu(s->tx.link);
>>>> @@ -788,18 +762,17 @@ static void tx_command(EEPRO100State *s)
>>>> }
>>>> assert(tcb_bytes<= sizeof(buf));
>>>> while (size< tcb_bytes) {
>>>> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
>>>> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
>>>> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev, tbd_address);
>>>> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev, tbd_address + 4);
>>>> #if 0
>>>> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
>>>> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
>>>> #endif
>>>> tbd_address += 8;
>>>> TRACE(RXTX, logout
>>>> ("TBD (simplified mode): buffer address 0x%08x, size 0x%04x\n",
>>>> tx_buffer_address, tx_buffer_size));
>>>> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
>>>> - cpu_physical_memory_read(tx_buffer_address,&buf[size],
>>>> - tx_buffer_size);
>>>> + pci_dma_read(&s->dev, tx_buffer_address,&buf[size], tx_buffer_size);
>>>> size += tx_buffer_size;
>>>> }
>>>> if (tbd_array == 0xffffffff) {
>>>> @@ -810,16 +783,19 @@ static void tx_command(EEPRO100State *s)
>>>> if (s->has_extended_tcb_support&& !(s->configuration[6]& BIT(4))) {
>>>> /* Extended Flexible TCB. */
>>>> for (; tbd_count< 2; tbd_count++) {
>>>> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
>>>> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
>>>> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
>>>> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev,
>>>> + tbd_address);
>>>> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev,
>>>> + tbd_address + 4);
>>>> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev,
>>>> + tbd_address + 6);
>>>> tbd_address += 8;
>>>> TRACE(RXTX, logout
>>>> ("TBD (extended flexible mode): buffer address 0x%08x, size 0x%04x\n",
>>>> tx_buffer_address, tx_buffer_size));
>>>> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
>>>> - cpu_physical_memory_read(tx_buffer_address,&buf[size],
>>>> - tx_buffer_size);
>>>> + pci_dma_read(&s->dev, tx_buffer_address,
>>>> +&buf[size], tx_buffer_size);
>>>> size += tx_buffer_size;
>>>> if (tx_buffer_el& 1) {
>>>> break;
>>>> @@ -828,16 +804,16 @@ static void tx_command(EEPRO100State *s)
>>>> }
>>>> tbd_address = tbd_array;
>>>> for (; tbd_count< s->tx.tbd_count; tbd_count++) {
>>>> - uint32_t tx_buffer_address = e100_ldl_le_phys(tbd_address);
>>>> - uint16_t tx_buffer_size = e100_ldw_le_phys(tbd_address + 4);
>>>> - uint16_t tx_buffer_el = e100_ldw_le_phys(tbd_address + 6);
>>>> + uint32_t tx_buffer_address = ldl_le_pci_dma(&s->dev, tbd_address);
>>>> + uint16_t tx_buffer_size = lduw_le_pci_dma(&s->dev, tbd_address + 4);
>>>> + uint16_t tx_buffer_el = lduw_le_pci_dma(&s->dev, tbd_address + 6);
>>>> tbd_address += 8;
>>>> TRACE(RXTX, logout
>>>> ("TBD (flexible mode): buffer address 0x%08x, size 0x%04x\n",
>>>> tx_buffer_address, tx_buffer_size));
>>>> tx_buffer_size = MIN(tx_buffer_size, sizeof(buf) - size);
>>>> - cpu_physical_memory_read(tx_buffer_address,&buf[size],
>>>> - tx_buffer_size);
>>>> + pci_dma_read(&s->dev, tx_buffer_address,
>>>> +&buf[size], tx_buffer_size);
>>>> size += tx_buffer_size;
>>>> if (tx_buffer_el& 1) {
>>>> break;
>>>> @@ -862,7 +838,7 @@ static void set_multicast_list(EEPRO100State *s)
>>>> TRACE(OTHER, logout("multicast list, multicast count = %u\n", multicast_count));
>>>> for (i = 0; i< multicast_count; i += 6) {
>>>> uint8_t multicast_addr[6];
>>>> - cpu_physical_memory_read(s->cb_address + 10 + i, multicast_addr, 6);
>>>> + pci_dma_read(&s->dev, s->cb_address + 10 + i, multicast_addr, 6);
>>>> TRACE(OTHER, logout("multicast entry %s\n", nic_dump(multicast_addr, 6)));
>>>> unsigned mcast_idx = compute_mcast_idx(multicast_addr);
>>>> assert(mcast_idx< 64);
>>>> @@ -896,12 +872,12 @@ static void action_command(EEPRO100State *s)
>>>> /* Do nothing. */
>>>> break;
>>>> case CmdIASetup:
>>>> - cpu_physical_memory_read(s->cb_address + 8,&s->conf.macaddr.a[0], 6);
>>>> + pci_dma_read(&s->dev, s->cb_address + 8,&s->conf.macaddr.a[0], 6);
>>>> TRACE(OTHER, logout("macaddr: %s\n", nic_dump(&s->conf.macaddr.a[0], 6)));
>>>> break;
>>>> case CmdConfigure:
>>>> - cpu_physical_memory_read(s->cb_address + 8,&s->configuration[0],
>>>> - sizeof(s->configuration));
>>>> + pci_dma_read(&s->dev, s->cb_address + 8,
>>>> +&s->configuration[0], sizeof(s->configuration));
>>>> TRACE(OTHER, logout("configuration: %s\n",
>>>> nic_dump(&s->configuration[0], 16)));
>>>> TRACE(OTHER, logout("configuration: %s\n",
>>>> @@ -938,7 +914,8 @@ static void action_command(EEPRO100State *s)
>>>> break;
>>>> }
>>>> /* Write new status. */
>>>> - e100_stw_le_phys(s->cb_address, s->tx.status | ok_status | STATUS_C);
>>>> + stw_le_pci_dma(&s->dev, s->cb_address,
>>>> + s->tx.status | ok_status | STATUS_C);
>>>> if (bit_i) {
>>>> /* CU completed action. */
>>>> eepro100_cx_interrupt(s);
>>>> @@ -1005,7 +982,7 @@ static void eepro100_cu_command(EEPRO100State * s,
>>>> uint8_t val)
>>>> /* Dump statistical counters. */
>>>> TRACE(OTHER, logout("val=0x%02x (dump stats)\n", val));
>>>> dump_statistics(s);
>>>> - e100_stl_le_phys(s->statsaddr + s->stats_size, 0xa005);
>>>> + stl_le_pci_dma(&s->dev, s->statsaddr + s->stats_size, 0xa005);
>>>> break;
>>>> case CU_CMD_BASE:
>>>> /* Load CU base. */
>>>> @@ -1016,7 +993,7 @@ static void eepro100_cu_command(EEPRO100State * s,
>>>> uint8_t val)
>>>> /* Dump and reset statistical counters. */
>>>> TRACE(OTHER, logout("val=0x%02x (dump stats and reset)\n", val));
>>>> dump_statistics(s);
>>>> - e100_stl_le_phys(s->statsaddr + s->stats_size, 0xa007);
>>>> + stl_le_pci_dma(&s->dev, s->statsaddr + s->stats_size, 0xa007);
>>>> memset(&s->statistics, 0, sizeof(s->statistics));
>>>> break;
>>>> case CU_SRESUME:
>>>> @@ -1310,10 +1287,10 @@ static void eepro100_write_port(EEPRO100State *s)
>>>> case PORT_SELFTEST:
>>>> TRACE(OTHER, logout("selftest address=0x%08x\n", address));
>>>> eepro100_selftest_t data;
>>>> - cpu_physical_memory_read(address,&data, sizeof(data));
>>>> + pci_dma_read(&s->dev, address, (uint8_t *)&data, sizeof(data));
>>>>
>>> The type cast is not needed and should be removed.
>>>
>>>> data.st_sign = 0xffffffff;
>>>> data.st_result = 0;
>>>> - cpu_physical_memory_write(address,&data, sizeof(data));
>>>> + pci_dma_write(&s->dev, address, (uint8_t *)&data, sizeof(data));
>>>>
>>> The type cast is not needed and should be removed.
>>>
>>>> break;
>>>> case PORT_SELECTIVE_RESET:
>>>> TRACE(OTHER, logout("selective reset, selftest address=0x%08x\n", address));
>>>> @@ -1729,8 +1706,8 @@ static ssize_t nic_receive(VLANClientState *nc, const
>>>> uint8_t * buf, size_t size
>>>> }
>>>> /* !!! */
>>>> eepro100_rx_t rx;
>>>> - cpu_physical_memory_read(s->ru_base + s->ru_offset,&rx,
>>>> - sizeof(eepro100_rx_t));
>>>> + pci_dma_read(&s->dev, s->ru_base + s->ru_offset,
>>>> + (uint8_t *)&rx, sizeof(eepro100_rx_t));
>>>>
>>> The type cast is not needed and should be removed.
>>>
>>>
>>>
>>>> uint16_t rfd_command = le16_to_cpu(rx.command);
>>>> uint16_t rfd_size = le16_to_cpu(rx.size);
>>>>
>>>> @@ -1746,10 +1723,10 @@ static ssize_t nic_receive(VLANClientState *nc, const
>>>> uint8_t * buf, size_t size
>>>> #endif
>>>> TRACE(OTHER, logout("command 0x%04x, link 0x%08x, addr 0x%08x, size %u\n",
>>>> rfd_command, rx.link, rx.rx_buf_addr, rfd_size));
>>>> - e100_stw_le_phys(s->ru_base + s->ru_offset +
>>>> - offsetof(eepro100_rx_t, status), rfd_status);
>>>> - e100_stw_le_phys(s->ru_base + s->ru_offset +
>>>> - offsetof(eepro100_rx_t, count), size);
>>>> + stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
>>>> + offsetof(eepro100_rx_t, status), rfd_status);
>>>> + stw_le_pci_dma(&s->dev, s->ru_base + s->ru_offset +
>>>> + offsetof(eepro100_rx_t, count), size);
>>>> /* Early receive interrupt not supported. */
>>>> #if 0
>>>> eepro100_er_interrupt(s);
>>>> @@ -1763,8 +1740,8 @@ static ssize_t nic_receive(VLANClientState *nc, const
>>>> uint8_t * buf, size_t size
>>>> #if 0
>>>> assert(!(s->configuration[17]& BIT(0)));
>>>> #endif
>>>> - cpu_physical_memory_write(s->ru_base + s->ru_offset +
>>>> - sizeof(eepro100_rx_t), buf, size);
>>>> + pci_dma_write(&s->dev, s->ru_base + s->ru_offset +
>>>> + sizeof(eepro100_rx_t), buf, size);
>>>> s->statistics.rx_good_frames++;
>>>> eepro100_fr_interrupt(s);
>>>> s->ru_offset = le32_to_cpu(rx.link);
>>>>
>>> Hi,
>>>
>>> the patch looks reasonable. I only suggest a formal change:
>>>
>>> There are lots of unnecessary type casts in several of your patches.
>>> I marked them here, but they should be removed anywhere.
>>>
>> Agreed. However, I'm going to apply this series as I'd like to get
>> it in for the freeze.
>>
>
> What does it get us, applying it before freeze?
> It's an api change without new functionality.
> Seems better on -next branch.
>
It gets us that downstreams can convert to the API for 1.0. It just
feels a lot more right to change APIs for a 1.0 release than a 1.1 release.
Also, it gives it more exposure and if anyone is inclined to have a
small patch to add IOMMU support for a specific platform downstream,
they can do so.
Overall, I think it's a good idea to pull it in for 1.0. It certainly
makes David's life easier :). Since we also have PCI support in -M
pseries now we could even declare the non-usage of an IOMMU there as a
bug and fix it for 1.0.1. Without, no PCI device except for virtio works.
Alex
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-11-02 17:56 ` Alexander Graf
2011-11-02 17:19 ` Anthony Liguori
@ 2011-11-02 19:11 ` Michael S. Tsirkin
2011-11-03 0:52 ` David Gibson
1 sibling, 1 reply; 33+ messages in thread
From: Michael S. Tsirkin @ 2011-11-02 19:11 UTC (permalink / raw)
To: Alexander Graf
Cc: Stefan Weil, eduard.munteanu, qemu-devel, avi, joerg.roedel, rth,
David Gibson
On Wed, Nov 02, 2011 at 06:56:55PM +0100, Alexander Graf wrote:
> It gets us that downstreams can convert to the API for 1.0. It just
> feels a lot more right to change APIs for a 1.0 release than a 1.1 release.
Anyway, I hope at least what looks like the alignment bug
in eepro100 introduced by this patch gets fixed before 1.0.
--
MST
^ permalink raw reply [flat|nested] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-11-02 19:11 ` Michael S. Tsirkin
@ 2011-11-03 0:52 ` David Gibson
0 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2011-11-03 0:52 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Stefan Weil, eduard.munteanu, qemu-devel, Alexander Graf, avi,
joerg.roedel, rth
On Wed, Nov 02, 2011 at 09:11:57PM +0200, Michael S. Tsirkin wrote:
> On Wed, Nov 02, 2011 at 06:56:55PM +0100, Alexander Graf wrote:
> > It gets us that downstreams can convert to the API for 1.0. It just
> > feels a lot more right to change APIs for a 1.0 release than a 1.1 release.
>
> Anyway, I hope at least what looks like the alignment bug
> in eepro100 introduced by this patch gets fixed before 1.0.
Yeah, yeah, I'm getting to it. I've been slowed down by illness and
have had higher priority bugs to work on first.
--
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] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-11-02 7:16 ` Michael S. Tsirkin
@ 2011-11-03 5:16 ` David Gibson
2011-11-03 12:25 ` Michael S. Tsirkin
0 siblings, 1 reply; 33+ messages in thread
From: David Gibson @ 2011-11-03 5:16 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: joerg.roedel, agraf, qemu-devel, avi, eduard.munteanu, rth
On Wed, Nov 02, 2011 at 09:16:34AM +0200, Michael S. Tsirkin wrote:
> On Mon, Oct 31, 2011 at 05:06:49PM +1100, David Gibson wrote:
> > From: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
[snip]
> > @@ -744,21 +713,26 @@ static void dump_statistics(EEPRO100State * s)
> > * values which really matter.
> > * Number of data should check configuration!!!
> > */
> > - cpu_physical_memory_write(s->statsaddr, &s->statistics, s->stats_size);
> > - e100_stl_le_phys(s->statsaddr + 0, s->statistics.tx_good_frames);
> > - e100_stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
> > - e100_stl_le_phys(s->statsaddr + 48, s->statistics.rx_resource_errors);
> > - e100_stl_le_phys(s->statsaddr + 60, s->statistics.rx_short_frame_errors);
> > + pci_dma_write(&s->dev, s->statsaddr,
> > + (uint8_t *) &s->statistics, s->stats_size);
> > + stl_le_pci_dma(&s->dev, s->statsaddr + 0,
> > + s->statistics.tx_good_frames);
> > + stl_le_pci_dma(&s->dev, s->statsaddr + 36,
> > + s->statistics.rx_good_frames);
> > + stl_le_pci_dma(&s->dev, s->statsaddr + 48,
> > + s->statistics.rx_resource_errors);
> > + stl_le_pci_dma(&s->dev, s->statsaddr + 60,
> > + s->statistics.rx_short_frame_errors);
>
> This might introduce a bug: stlXX APIs assume aligned addresses,
> an address in statsaddr is user-controlled so I'm not sure
> it's always aligned.
>
> Why isn't the patch simply replacing cpu_physical_memory_read
> with pci_XXX ? Any cleanups should be done separately.
Because it seemed like a good idea at the time. When I first wrote
this, the possibility of unaligned addresses wasn't obvious to me.
So, I'm working on fixing this now. I can take one of two approaches:
- Simply revert this part of the change, reinstate the e100_stl
functions as calling into dma_write().
- Alter the stX_dma() functions to work for unaligned addresses (by
falling back to dma_rw() in that case). This is a little more
involved but might make device writing safer in future.
Anthony, Michael, any preferred direction here?
--
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] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-11-03 5:16 ` David Gibson
@ 2011-11-03 12:25 ` Michael S. Tsirkin
2011-11-04 0:28 ` David Gibson
2011-11-05 8:32 ` Stefan Weil
0 siblings, 2 replies; 33+ messages in thread
From: Michael S. Tsirkin @ 2011-11-03 12:25 UTC (permalink / raw)
To: anthony, agraf, rth, joerg.roedel, eduard.munteanu, avi,
qemu-devel
On Thu, Nov 03, 2011 at 04:16:34PM +1100, David Gibson wrote:
> On Wed, Nov 02, 2011 at 09:16:34AM +0200, Michael S. Tsirkin wrote:
> > On Mon, Oct 31, 2011 at 05:06:49PM +1100, David Gibson wrote:
> > > From: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
> [snip]
> > > @@ -744,21 +713,26 @@ static void dump_statistics(EEPRO100State * s)
> > > * values which really matter.
> > > * Number of data should check configuration!!!
> > > */
> > > - cpu_physical_memory_write(s->statsaddr, &s->statistics, s->stats_size);
> > > - e100_stl_le_phys(s->statsaddr + 0, s->statistics.tx_good_frames);
> > > - e100_stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
> > > - e100_stl_le_phys(s->statsaddr + 48, s->statistics.rx_resource_errors);
> > > - e100_stl_le_phys(s->statsaddr + 60, s->statistics.rx_short_frame_errors);
> > > + pci_dma_write(&s->dev, s->statsaddr,
> > > + (uint8_t *) &s->statistics, s->stats_size);
> > > + stl_le_pci_dma(&s->dev, s->statsaddr + 0,
> > > + s->statistics.tx_good_frames);
> > > + stl_le_pci_dma(&s->dev, s->statsaddr + 36,
> > > + s->statistics.rx_good_frames);
> > > + stl_le_pci_dma(&s->dev, s->statsaddr + 48,
> > > + s->statistics.rx_resource_errors);
> > > + stl_le_pci_dma(&s->dev, s->statsaddr + 60,
> > > + s->statistics.rx_short_frame_errors);
> >
> > This might introduce a bug: stlXX APIs assume aligned addresses,
> > an address in statsaddr is user-controlled so I'm not sure
> > it's always aligned.
> >
> > Why isn't the patch simply replacing cpu_physical_memory_read
> > with pci_XXX ? Any cleanups should be done separately.
>
> Because it seemed like a good idea at the time. When I first wrote
> this, the possibility of unaligned addresses wasn't obvious to me.
> So, I'm working on fixing this now. I can take one of two approaches:
>
> - Simply revert this part of the change, reinstate the e100_stl
> functions as calling into dma_write().
>
> - Alter the stX_dma() functions to work for unaligned addresses (by
> falling back to dma_rw() in that case). This is a little more
> involved but might make device writing safer in future.
Yes but then we lose the atomicity guarantee. So this might
still result in subtle emulation bugs.
> Anthony, Michael, any preferred direction here?
For 1.0 I'd go for option 1 as the simplest.
> --
> 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] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-11-03 12:25 ` Michael S. Tsirkin
@ 2011-11-04 0:28 ` David Gibson
2011-11-05 8:32 ` Stefan Weil
1 sibling, 0 replies; 33+ messages in thread
From: David Gibson @ 2011-11-04 0:28 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: joerg.roedel, agraf, qemu-devel, avi, eduard.munteanu, rth
On Thu, Nov 03, 2011 at 02:25:45PM +0200, Michael S. Tsirkin wrote:
> On Thu, Nov 03, 2011 at 04:16:34PM +1100, David Gibson wrote:
> > On Wed, Nov 02, 2011 at 09:16:34AM +0200, Michael S. Tsirkin wrote:
> > > On Mon, Oct 31, 2011 at 05:06:49PM +1100, David Gibson wrote:
> > > > From: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
> > [snip]
> > > > @@ -744,21 +713,26 @@ static void dump_statistics(EEPRO100State * s)
> > > > * values which really matter.
> > > > * Number of data should check configuration!!!
> > > > */
> > > > - cpu_physical_memory_write(s->statsaddr, &s->statistics, s->stats_size);
> > > > - e100_stl_le_phys(s->statsaddr + 0, s->statistics.tx_good_frames);
> > > > - e100_stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
> > > > - e100_stl_le_phys(s->statsaddr + 48, s->statistics.rx_resource_errors);
> > > > - e100_stl_le_phys(s->statsaddr + 60, s->statistics.rx_short_frame_errors);
> > > > + pci_dma_write(&s->dev, s->statsaddr,
> > > > + (uint8_t *) &s->statistics, s->stats_size);
> > > > + stl_le_pci_dma(&s->dev, s->statsaddr + 0,
> > > > + s->statistics.tx_good_frames);
> > > > + stl_le_pci_dma(&s->dev, s->statsaddr + 36,
> > > > + s->statistics.rx_good_frames);
> > > > + stl_le_pci_dma(&s->dev, s->statsaddr + 48,
> > > > + s->statistics.rx_resource_errors);
> > > > + stl_le_pci_dma(&s->dev, s->statsaddr + 60,
> > > > + s->statistics.rx_short_frame_errors);
> > >
> > > This might introduce a bug: stlXX APIs assume aligned addresses,
> > > an address in statsaddr is user-controlled so I'm not sure
> > > it's always aligned.
> > >
> > > Why isn't the patch simply replacing cpu_physical_memory_read
> > > with pci_XXX ? Any cleanups should be done separately.
> >
> > Because it seemed like a good idea at the time. When I first wrote
> > this, the possibility of unaligned addresses wasn't obvious to me.
> > So, I'm working on fixing this now. I can take one of two approaches:
> >
> > - Simply revert this part of the change, reinstate the e100_stl
> > functions as calling into dma_write().
> >
> > - Alter the stX_dma() functions to work for unaligned addresses (by
> > falling back to dma_rw() in that case). This is a little more
> > involved but might make device writing safer in future.
>
> Yes but then we lose the atomicity guarantee. So this might
> still result in subtle emulation bugs.
Sorry, I should have been clearer - I was planning to fall back to
dma_rw() *only* for unaligned addresses. Aligned addresses would
still have the atomicity guarantee.
> > Anthony, Michael, any preferred direction here?
>
> For 1.0 I'd go for option 1 as the simplest.
--
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] 33+ messages in thread
* Re: [Qemu-devel] [PATCH 05/14] eepro100: Use PCI DMA stub functions
2011-11-03 12:25 ` Michael S. Tsirkin
2011-11-04 0:28 ` David Gibson
@ 2011-11-05 8:32 ` Stefan Weil
1 sibling, 0 replies; 33+ messages in thread
From: Stefan Weil @ 2011-11-05 8:32 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: QEMU Developers, David Gibson
On Wed, Nov 02, 2011 at 09:16:34AM +0200, Michael S. Tsirkin wrote:
> On Mon, Oct 31, 2011 at 05:06:49PM +1100, David Gibson wrote:
>> From: Eduard - Gabriel Munteanu <eduard.munteanu@linux360.ro>
[snip]
>> @@ -744,21 +713,26 @@ static void dump_statistics(EEPRO100State * s)
>> * values which really matter.
>> * Number of data should check configuration!!!
>> */
>> - cpu_physical_memory_write(s->statsaddr, &s->statistics, s->stats_size);
>> - e100_stl_le_phys(s->statsaddr + 0, s->statistics.tx_good_frames);
>> - e100_stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
>> - e100_stl_le_phys(s->statsaddr + 48, s->statistics.rx_resource_errors);
>> - e100_stl_le_phys(s->statsaddr + 60,
>> s->statistics.rx_short_frame_errors);
>> + pci_dma_write(&s->dev, s->statsaddr,
>> + (uint8_t *) &s->statistics, s->stats_size);
>> + stl_le_pci_dma(&s->dev, s->statsaddr + 0,
>> + s->statistics.tx_good_frames);
>> + stl_le_pci_dma(&s->dev, s->statsaddr + 36,
>> + s->statistics.rx_good_frames);
>> + stl_le_pci_dma(&s->dev, s->statsaddr + 48,
>> + s->statistics.rx_resource_errors);
>> + stl_le_pci_dma(&s->dev, s->statsaddr + 60,
>> + s->statistics.rx_short_frame_errors);
>
> This might introduce a bug: stlXX APIs assume aligned addresses,
> an address in statsaddr is user-controlled so I'm not sure
> it's always aligned.
>
> Why isn't the patch simply replacing cpu_physical_memory_read
> with pci_XXX ? Any cleanups should be done separately.
Hello,
I just sent a patch for eepro100.c which enforces aligned addresses
for s->statsaddr, so most of David's changes are now safe.
Please include the patch in QEMU 1.0.
Thanks,
Stefan Weil
^ permalink raw reply [flat|nested] 33+ messages in thread
end of thread, other threads:[~2011-11-05 8:33 UTC | newest]
Thread overview: 33+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-31 6:06 [Qemu-devel] [0/14] Preliminary work for IOMMU emulation support (v3) David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 01/14] Define DMA address and direction types David Gibson
2011-11-01 22:21 ` Anthony Liguori
2011-10-31 6:06 ` [Qemu-devel] [PATCH 02/14] Use dma_addr_t type for scatter/gather code David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 03/14] Add stub functions for PCI device models to do PCI DMA David Gibson
2011-11-02 7:17 ` Michael S. Tsirkin
2011-11-02 11:47 ` David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 04/14] rtl8139: Use PCI DMA stub functions David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 05/14] eepro100: " David Gibson
2011-10-31 16:45 ` Stefan Weil
2011-11-01 20:24 ` Anthony Liguori
2011-11-02 7:40 ` Michael S. Tsirkin
2011-11-02 12:46 ` Anthony Liguori
2011-11-02 17:56 ` Alexander Graf
2011-11-02 17:19 ` Anthony Liguori
2011-11-02 17:28 ` Alexander Graf
2011-11-02 19:11 ` Michael S. Tsirkin
2011-11-03 0:52 ` David Gibson
2011-11-02 11:01 ` Michael S. Tsirkin
2011-11-02 7:16 ` Michael S. Tsirkin
2011-11-03 5:16 ` David Gibson
2011-11-03 12:25 ` Michael S. Tsirkin
2011-11-04 0:28 ` David Gibson
2011-11-05 8:32 ` Stefan Weil
2011-10-31 6:06 ` [Qemu-devel] [PATCH 06/14] ac97: " David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 07/14] es1370: " David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 08/14] e1000: " David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 09/14] lsi53c895a: " David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 10/14] pcnet-pci: " David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 11/14] intel-hda: " David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 12/14] PCI IDE: " David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 13/14] usb-ehci: " David Gibson
2011-10-31 6:06 ` [Qemu-devel] [PATCH 14/14] usb-uhci: " David Gibson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).