* [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces
@ 2010-02-09 22:01 Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 01/15] pci: add new bus functions Anthony Liguori
` (16 more replies)
0 siblings, 17 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Alex Graf
This is a work in progress that I wanted to share giving some of the discussions
around rwhandlers. The idea is to make PCI devices have a common set of
functions to interact with the CPU that is driven entirely through the PCI bus.
I've tested the network card conversions, but have not yet tested the other
bits.
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 01/15] pci: add new bus functions
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-10 8:09 ` Isaku Yamahata
2010-02-10 8:57 ` [Qemu-devel] " Michael S. Tsirkin
2010-02-09 22:01 ` [Qemu-devel] [PATCH 02/15] rtl8139: convert to new PCI interfaces Anthony Liguori
` (15 subsequent siblings)
16 siblings, 2 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
- mapping and managing io regions
- reading and writing physical memory
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/pci.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
hw/pci.h | 18 ++++++-
2 files changed, 181 insertions(+), 9 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 9ad63dd..5460f27 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -664,6 +664,9 @@ static void pci_unregister_io_regions(PCIDevice *pci_dev)
r->addr),
r->filtered_size,
IO_MEM_UNASSIGNED);
+ if (!r->map_func && r->read && r->write) {
+ cpu_unregister_io_memory(r->io_memory_addr);
+ }
}
}
}
@@ -687,16 +690,15 @@ static int pci_unregister_device(DeviceState *dev)
return 0;
}
-void pci_register_bar(PCIDevice *pci_dev, int region_num,
- pcibus_t size, int type,
- PCIMapIORegionFunc *map_func)
+static PCIIORegion *do_pci_register_bar(PCIDevice *pci_dev, int region_num,
+ pcibus_t size, int type)
{
PCIIORegion *r;
uint32_t addr;
pcibus_t wmask;
if ((unsigned int)region_num >= PCI_NUM_REGIONS)
- return;
+ return NULL;
if (size & (size-1)) {
fprintf(stderr, "ERROR: PCI region size must be pow2 "
@@ -709,7 +711,6 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
r->size = size;
r->filtered_size = size;
r->type = type;
- r->map_func = map_func;
wmask = ~(size - 1);
addr = pci_bar(pci_dev, region_num);
@@ -726,6 +727,100 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
pci_set_long(pci_dev->wmask + addr, wmask & 0xffffffff);
pci_set_long(pci_dev->cmask + addr, 0xffffffff);
}
+
+ return r;
+}
+
+void pci_register_bar(PCIDevice *pci_dev, int region_num,
+ pcibus_t size, int type,
+ PCIMapIORegionFunc *map_func)
+{
+ PCIIORegion *r;
+ r = do_pci_register_bar(pci_dev, region_num, size, type);
+ if (r) {
+ r->map_func = map_func;
+ }
+}
+
+void pci_memory_read(PCIDevice *pci_dev, pcibus_t addr, void *buf, int len)
+{
+ cpu_physical_memory_read(addr, buf, len);
+}
+
+void pci_memory_write(PCIDevice *pci_dev, pcibus_t addr,
+ const void *buf, int len)
+{
+ cpu_physical_memory_write(addr, buf, len);
+}
+
+static void pci_io_region_writeb(void *opaque, target_phys_addr_t addr,
+ uint32_t value)
+{
+ PCIIORegion *r = opaque;
+ r->write(r->dev, addr, 1, value);
+}
+
+static void pci_io_region_writew(void *opaque, target_phys_addr_t addr,
+ uint32_t value)
+{
+ PCIIORegion *r = opaque;
+ r->write(r->dev, addr, 2, value);
+}
+
+static void pci_io_region_writel(void *opaque, target_phys_addr_t addr,
+ uint32_t value)
+{
+ PCIIORegion *r = opaque;
+ r->write(r->dev, addr, 4, value);
+}
+
+static uint32_t pci_io_region_readb(void *opaque, target_phys_addr_t addr)
+{
+ PCIIORegion *r = opaque;
+ return r->read(r->dev, addr, 1);
+}
+
+static uint32_t pci_io_region_readw(void *opaque, target_phys_addr_t addr)
+{
+ PCIIORegion *r = opaque;
+ return r->read(r->dev, addr, 2);
+}
+
+static uint32_t pci_io_region_readl(void *opaque, target_phys_addr_t addr)
+{
+ PCIIORegion *r = opaque;
+ return r->read(r->dev, addr, 4);
+}
+
+static CPUReadMemoryFunc * const pci_io_region_readfn[3] = {
+ pci_io_region_readb,
+ pci_io_region_readw,
+ pci_io_region_readl,
+};
+
+static CPUWriteMemoryFunc * const pci_io_region_writefn[3] = {
+ pci_io_region_writeb,
+ pci_io_region_writew,
+ pci_io_region_writel,
+};
+
+void pci_register_io_region(PCIDevice *d, int region_num,
+ pcibus_t size, int type,
+ PCIIOReadFunc *readcb, PCIIOWriteFunc *writecb)
+{
+ PCIIORegion *r;
+ r = do_pci_register_bar(d, region_num, size, type);
+ if (r) {
+ r->map_func = NULL;
+ r->dev = d;
+ r->read = readcb;
+ r->write = writecb;
+ if (r->type != PCI_BASE_ADDRESS_SPACE_IO && r->read && r->write) {
+ r->io_memory_addr = cpu_register_io_memory(pci_io_region_readfn,
+ pci_io_region_writefn,
+ r);
+ }
+ }
}
static uint32_t pci_config_get_io_base(PCIDevice *d,
@@ -897,6 +992,45 @@ static pcibus_t pci_bar_address(PCIDevice *d,
return new_addr;
}
+static void pci_io_region_ioport_writeb(void *opaque, uint32_t addr,
+ uint32_t value)
+{
+ PCIIORegion *r = opaque;
+ r->write(r->dev, (addr - r->addr), 1, value);
+}
+
+static void pci_io_region_ioport_writew(void *opaque, uint32_t addr,
+ uint32_t value)
+{
+ PCIIORegion *r = opaque;
+ r->write(r->dev, (addr - r->addr), 2, value);
+}
+
+static void pci_io_region_ioport_writel(void *opaque, uint32_t addr,
+ uint32_t value)
+{
+ PCIIORegion *r = opaque;
+ r->write(r->dev, (addr - r->addr), 4, value);
+}
+
+static uint32_t pci_io_region_ioport_readb(void *opaque, uint32_t addr)
+{
+ PCIIORegion *r = opaque;
+ return r->read(r->dev, (addr - r->addr), 1);
+}
+
+static uint32_t pci_io_region_ioport_readw(void *opaque, uint32_t addr)
+{
+ PCIIORegion *r = opaque;
+ return r->read(r->dev, (addr - r->addr), 2);
+}
+
+static uint32_t pci_io_region_ioport_readl(void *opaque, uint32_t addr)
+{
+ PCIIORegion *r = opaque;
+ return r->read(r->dev, (addr - r->addr), 4);
+}
+
static void pci_update_mappings(PCIDevice *d)
{
PCIIORegion *r;
@@ -952,10 +1086,32 @@ static void pci_update_mappings(PCIDevice *d)
* addr & (size - 1) != 0.
*/
if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
- r->map_func(d, i, r->addr, r->filtered_size, r->type);
+ if (r->map_func) {
+ r->map_func(d, i, r->addr, r->filtered_size, r->type);
+ } else {
+ register_ioport_write(r->addr, r->filtered_size, 1,
+ pci_io_region_ioport_writeb, r);
+ register_ioport_write(r->addr, r->filtered_size, 2,
+ pci_io_region_ioport_writew, r);
+ register_ioport_write(r->addr, r->filtered_size, 4,
+ pci_io_region_ioport_writel, r);
+ register_ioport_read(r->addr, r->filtered_size, 1,
+ pci_io_region_ioport_readb, r);
+ register_ioport_read(r->addr, r->filtered_size, 2,
+ pci_io_region_ioport_readw, r);
+ register_ioport_read(r->addr, r->filtered_size, 4,
+ pci_io_region_ioport_readl, r);
+ }
} else {
- r->map_func(d, i, pci_to_cpu_addr(d->bus, r->addr),
- r->filtered_size, r->type);
+ if (r->map_func) {
+ r->map_func(d, i, pci_to_cpu_addr(d->bus, r->addr),
+ r->filtered_size, r->type);
+ } else if (r->read && r->write) {
+ cpu_register_physical_memory(pci_to_cpu_addr(d->bus,
+ r->addr),
+ r->filtered_size,
+ r->io_memory_addr);
+ }
}
}
}
diff --git a/hw/pci.h b/hw/pci.h
index 8b511d2..3edf28f 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -81,13 +81,21 @@ typedef void PCIMapIORegionFunc(PCIDevice *pci_dev, int region_num,
pcibus_t addr, pcibus_t size, int type);
typedef int PCIUnregisterFunc(PCIDevice *pci_dev);
+typedef uint32_t (PCIIOReadFunc)(PCIDevice *pci_dev, pcibus_t addr, int size);
+typedef void (PCIIOWriteFunc)(PCIDevice *pci_dev, pcibus_t addr, int size,
+ uint32_t value);
+
typedef struct PCIIORegion {
+ PCIDevice *dev;
pcibus_t addr; /* current PCI mapping address. -1 means not mapped */
#define PCI_BAR_UNMAPPED (~(pcibus_t)0)
pcibus_t size;
pcibus_t filtered_size;
uint8_t type;
- PCIMapIORegionFunc *map_func;
+ PCIMapIORegionFunc *map_func; /* legacy mapping function */
+ PCIIOReadFunc *read;
+ PCIIOWriteFunc *write;
+ int io_memory_addr;
} PCIIORegion;
#define PCI_ROM_SLOT 6
@@ -190,6 +198,14 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
pcibus_t size, int type,
PCIMapIORegionFunc *map_func);
+void pci_register_io_region(PCIDevice *d, int region_num,
+ pcibus_t size, int type,
+ PCIIOReadFunc *readcb, PCIIOWriteFunc *writecb);
+
+void pci_memory_read(PCIDevice *pci_dev, pcibus_t addr, void *buf, int len);
+void pci_memory_write(PCIDevice *pci_dev, pcibus_t addr,
+ const void *buf, int len);
+
int pci_add_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
--
1.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 02/15] rtl8139: convert to new PCI interfaces
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 01/15] pci: add new bus functions Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 03/15] lsi53c895a: convert to new pci interfaces Anthony Liguori
` (14 subsequent siblings)
16 siblings, 0 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
- we drop the special target specific logic for 64-bit addresses, need to check
this out
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/rtl8139.c | 314 ++++++++++++++++++++++++----------------------------------
1 files changed, 128 insertions(+), 186 deletions(-)
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index f04dd54..95e7aeb 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -414,9 +414,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 */
@@ -494,6 +491,9 @@ typedef struct RTL8139State {
} RTL8139State;
+/* Writes tally counters to specified physical memory address */
+static void RTL8139TallyCounters_physical_memory_write(RTL8139State *s, pcibus_t tc_addr, RTL8139TallyCounters* counters);
+
static void prom9346_decode_command(EEprom9346 *eeprom, uint8_t command)
{
DEBUG_PRINT(("RTL8139: eeprom command 0x%02x\n", command));
@@ -753,15 +753,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_memory_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_memory_write( &s->dev, s->RxBuf + s->RxBufAddr,
+ buf + (size-wrapped), wrapped );
s->RxBufAddr = wrapped;
@@ -770,19 +770,15 @@ 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_memory_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 pcibus_t rtl8139_addr64(uint32_t low, uint32_t high)
{
-#if TARGET_PHYS_ADDR_BITS > 32
- return low | ((target_phys_addr_t)high << 32);
-#else
- return low;
-#endif
+ return low | ((pcibus_t)high << 32);
}
static int rtl8139_can_receive(VLANClientState *nc)
@@ -954,7 +950,7 @@ 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;
+ pcibus_t cplus_rx_ring_desc;
cplus_rx_ring_desc = rtl8139_addr64(s->RxRingAddrLO, s->RxRingAddrHI);
cplus_rx_ring_desc += 16 * descriptor;
@@ -964,13 +960,13 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
uint32_t val, rxdw0,rxdw1,rxbufLO,rxbufHI;
- cpu_physical_memory_read(cplus_rx_ring_desc, (uint8_t *)&val, 4);
+ pci_memory_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_memory_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_memory_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_memory_read(&s->dev, cplus_rx_ring_desc+12, (uint8_t *)&val, 4);
rxbufHI = le32_to_cpu(val);
DEBUG_PRINT(("RTL8139: +++ C+ mode RX descriptor %d %08x %08x %08x %08x\n",
@@ -1012,10 +1008,10 @@ 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);
+ pcibus_t rx_addr = rtl8139_addr64(rxbufLO, rxbufHI);
/* receive/copy to target memory */
- cpu_physical_memory_write( rx_addr, buf, size );
+ pci_memory_write(&s->dev, rx_addr, buf, size );
if (s->CpCmd & CPlusRxChkSum)
{
@@ -1028,7 +1024,7 @@ static ssize_t rtl8139_do_receive(VLANClientState *nc, const uint8_t *buf, size_
#else
val = 0;
#endif
- cpu_physical_memory_write( rx_addr+size, (uint8_t *)&val, 4);
+ pci_memory_write(&s->dev, rx_addr+size, (uint8_t *)&val, 4);
/* first segment of received packet flag */
#define CP_RX_STATUS_FS (1<<29)
@@ -1077,9 +1073,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_memory_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_memory_write(&s->dev, cplus_rx_ring_desc+4, (uint8_t *)&val, 4);
/* update tally counter */
++s->tally_counters.RxOk;
@@ -1275,50 +1271,50 @@ 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_physical_memory_write(RTL8139State *s, pcibus_t tc_addr, RTL8139TallyCounters* 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_memory_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_memory_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_memory_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_memory_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_memory_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_memory_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_memory_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_memory_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_memory_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_memory_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_memory_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_memory_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_memory_write(&s->dev, tc_addr + 62, (uint8_t *)&val16, 2);
}
/* Loads values of tally counters from VM state file */
@@ -1776,7 +1772,7 @@ static int rtl8139_transmit_one(RTL8139State *s, int descriptor)
DEBUG_PRINT(("RTL8139: +++ 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_memory_read(&s->dev, s->TxAddr[descriptor], txbuffer, txsize);
/* Mark descriptor as transferred */
s->TxStatus[descriptor] |= TxHostOwns;
@@ -1896,7 +1892,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
int descriptor = s->currCPlusTxDesc;
- target_phys_addr_t cplus_tx_ring_desc =
+ pcibus_t cplus_tx_ring_desc =
rtl8139_addr64(s->TxAddr[0], s->TxAddr[1]);
/* Normal priority ring */
@@ -1907,14 +1903,14 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
uint32_t val, txdw0,txdw1,txbufLO,txbufHI;
- cpu_physical_memory_read(cplus_tx_ring_desc, (uint8_t *)&val, 4);
+ pci_memory_read(&s->dev, cplus_tx_ring_desc, (uint8_t *)&val, 4);
txdw0 = le32_to_cpu(val);
/* TODO: implement VLAN tagging support, VLAN tag data is read to txdw1 */
- cpu_physical_memory_read(cplus_tx_ring_desc+4, (uint8_t *)&val, 4);
+ pci_memory_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_memory_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_memory_read(&s->dev, cplus_tx_ring_desc+12, (uint8_t *)&val, 4);
txbufHI = le32_to_cpu(val);
DEBUG_PRINT(("RTL8139: +++ C+ mode TX descriptor %d %08x %08x %08x %08x\n",
@@ -1983,7 +1979,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);
+ pcibus_t tx_addr = rtl8139_addr64(txbufLO, txbufHI);
/* make sure we have enough space to assemble the packet */
if (!s->cplus_txbuffer)
@@ -2021,7 +2017,7 @@ static int rtl8139_cplus_transmit_one(RTL8139State *s)
DEBUG_PRINT(("RTL8139: +++ C+ mode transmit reading %d bytes from host memory at %016" PRIx64 " to offset %d\n",
txsize, (uint64_t)tx_addr, s->cplus_txbuffer_offset));
- cpu_physical_memory_read(tx_addr, s->cplus_txbuffer + s->cplus_txbuffer_offset, txsize);
+ pci_memory_read(&s->dev, tx_addr, s->cplus_txbuffer + s->cplus_txbuffer_offset, txsize);
s->cplus_txbuffer_offset += txsize;
/* seek to next Rx descriptor */
@@ -2048,10 +2044,10 @@ 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_memory_write(&s->dev, cplus_tx_ring_desc, (uint8_t *)&val, 4);
/* TODO: implement VLAN tagging support, VLAN tag data is read to txdw1 */
// val = cpu_to_le32(txdw1);
-// cpu_physical_memory_write(cplus_tx_ring_desc+4, &val, 4);
+// pci_memory_write(&s->dev, cplus_tx_ring_desc+4, &val, 4);
/* Now decide if descriptor being processed is holding the last segment of packet */
if (txdw0 & CP_TX_LS)
@@ -2374,10 +2370,10 @@ static void rtl8139_TxStatus_write(RTL8139State *s, uint32_t txRegOffset, uint32
if (descriptor == 0 && (val & 0x8))
{
- target_phys_addr_t tc_addr = rtl8139_addr64(s->TxStatus[0] & ~0x3f, s->TxStatus[1]);
+ pcibus_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_physical_memory_write( s, tc_addr, &s->tally_counters);
/* mark dump completed */
s->TxStatus[0] &= ~0x8;
@@ -2596,10 +2592,8 @@ static uint32_t rtl8139_MultiIntr_read(RTL8139State *s)
return ret;
}
-static void rtl8139_io_writeb(void *opaque, uint8_t addr, uint32_t val)
+static void rtl8139_io_writeb(RTL8139State *s, uint8_t addr, uint32_t val)
{
- RTL8139State *s = opaque;
-
addr &= 0xff;
switch (addr)
@@ -2680,10 +2674,8 @@ static void rtl8139_io_writeb(void *opaque, uint8_t addr, uint32_t val)
}
}
-static void rtl8139_io_writew(void *opaque, uint8_t addr, uint32_t val)
+static void rtl8139_io_writew(RTL8139State *s, uint8_t addr, uint32_t val)
{
- RTL8139State *s = opaque;
-
addr &= 0xfe;
switch (addr)
@@ -2733,16 +2725,14 @@ static void rtl8139_io_writew(void *opaque, uint8_t addr, uint32_t val)
default:
DEBUG_PRINT(("RTL8139: ioport write(w) addr=0x%x val=0x%04x via write(b)\n", addr, val));
- rtl8139_io_writeb(opaque, addr, val & 0xff);
- rtl8139_io_writeb(opaque, addr + 1, (val >> 8) & 0xff);
+ rtl8139_io_writeb(s, addr, val & 0xff);
+ rtl8139_io_writeb(s, addr + 1, (val >> 8) & 0xff);
break;
}
}
-static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
+static void rtl8139_io_writel(RTL8139State *s, uint8_t addr, uint32_t val)
{
- RTL8139State *s = opaque;
-
addr &= 0xfc;
switch (addr)
@@ -2795,17 +2785,16 @@ static void rtl8139_io_writel(void *opaque, uint8_t addr, uint32_t val)
default:
DEBUG_PRINT(("RTL8139: ioport write(l) addr=0x%x val=0x%08x via write(b)\n", addr, val));
- rtl8139_io_writeb(opaque, addr, val & 0xff);
- rtl8139_io_writeb(opaque, addr + 1, (val >> 8) & 0xff);
- rtl8139_io_writeb(opaque, addr + 2, (val >> 16) & 0xff);
- rtl8139_io_writeb(opaque, addr + 3, (val >> 24) & 0xff);
+ rtl8139_io_writeb(s, addr, val & 0xff);
+ rtl8139_io_writeb(s, addr + 1, (val >> 8) & 0xff);
+ rtl8139_io_writeb(s, addr + 2, (val >> 16) & 0xff);
+ rtl8139_io_writeb(s, addr + 3, (val >> 24) & 0xff);
break;
}
}
-static uint32_t rtl8139_io_readb(void *opaque, uint8_t addr)
+static uint32_t rtl8139_io_readb(RTL8139State *s, uint8_t addr)
{
- RTL8139State *s = opaque;
int ret;
addr &= 0xff;
@@ -2877,9 +2866,8 @@ static uint32_t rtl8139_io_readb(void *opaque, uint8_t addr)
return ret;
}
-static uint32_t rtl8139_io_readw(void *opaque, uint8_t addr)
+static uint32_t rtl8139_io_readw(RTL8139State *s, uint8_t addr)
{
- RTL8139State *s = opaque;
uint32_t ret;
addr &= 0xfe; /* mask lower bit */
@@ -2944,8 +2932,8 @@ static uint32_t rtl8139_io_readw(void *opaque, uint8_t addr)
default:
DEBUG_PRINT(("RTL8139: ioport read(w) addr=0x%x via read(b)\n", addr));
- ret = rtl8139_io_readb(opaque, addr);
- ret |= rtl8139_io_readb(opaque, addr + 1) << 8;
+ ret = rtl8139_io_readb(s, addr);
+ ret |= rtl8139_io_readb(s, addr + 1) << 8;
DEBUG_PRINT(("RTL8139: ioport read(w) addr=0x%x val=0x%04x\n", addr, ret));
break;
@@ -2954,9 +2942,8 @@ static uint32_t rtl8139_io_readw(void *opaque, uint8_t addr)
return ret;
}
-static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
+static uint32_t rtl8139_io_readl(RTL8139State *s, uint8_t addr)
{
- RTL8139State *s = opaque;
uint32_t ret;
addr &= 0xfc; /* also mask low 2 bits */
@@ -3012,10 +2999,10 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
default:
DEBUG_PRINT(("RTL8139: ioport read(l) addr=0x%x via read(b)\n", addr));
- ret = rtl8139_io_readb(opaque, addr);
- ret |= rtl8139_io_readb(opaque, addr + 1) << 8;
- ret |= rtl8139_io_readb(opaque, addr + 2) << 16;
- ret |= rtl8139_io_readb(opaque, addr + 3) << 24;
+ ret = rtl8139_io_readb(s, addr);
+ ret |= rtl8139_io_readb(s, addr + 1) << 8;
+ ret |= rtl8139_io_readb(s, addr + 2) << 16;
+ ret |= rtl8139_io_readb(s, addr + 3) << 24;
DEBUG_PRINT(("RTL8139: read(l) addr=0x%x val=%08x\n", addr, ret));
break;
@@ -3026,82 +3013,6 @@ static uint32_t rtl8139_io_readl(void *opaque, uint8_t addr)
/* */
-static void rtl8139_ioport_writeb(void *opaque, uint32_t addr, uint32_t val)
-{
- rtl8139_io_writeb(opaque, addr & 0xFF, val);
-}
-
-static void rtl8139_ioport_writew(void *opaque, uint32_t addr, uint32_t val)
-{
- rtl8139_io_writew(opaque, addr & 0xFF, val);
-}
-
-static void rtl8139_ioport_writel(void *opaque, uint32_t addr, uint32_t val)
-{
- rtl8139_io_writel(opaque, addr & 0xFF, val);
-}
-
-static uint32_t rtl8139_ioport_readb(void *opaque, uint32_t addr)
-{
- return rtl8139_io_readb(opaque, addr & 0xFF);
-}
-
-static uint32_t rtl8139_ioport_readw(void *opaque, uint32_t addr)
-{
- return rtl8139_io_readw(opaque, addr & 0xFF);
-}
-
-static uint32_t rtl8139_ioport_readl(void *opaque, uint32_t addr)
-{
- return rtl8139_io_readl(opaque, addr & 0xFF);
-}
-
-/* */
-
-static void rtl8139_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
- rtl8139_io_writeb(opaque, addr & 0xFF, val);
-}
-
-static void rtl8139_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
-#ifdef TARGET_WORDS_BIGENDIAN
- val = bswap16(val);
-#endif
- rtl8139_io_writew(opaque, addr & 0xFF, val);
-}
-
-static void rtl8139_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
-#ifdef TARGET_WORDS_BIGENDIAN
- val = bswap32(val);
-#endif
- rtl8139_io_writel(opaque, addr & 0xFF, val);
-}
-
-static uint32_t rtl8139_mmio_readb(void *opaque, target_phys_addr_t addr)
-{
- return rtl8139_io_readb(opaque, addr & 0xFF);
-}
-
-static uint32_t rtl8139_mmio_readw(void *opaque, target_phys_addr_t addr)
-{
- uint32_t val = rtl8139_io_readw(opaque, addr & 0xFF);
-#ifdef TARGET_WORDS_BIGENDIAN
- val = bswap16(val);
-#endif
- return val;
-}
-
-static uint32_t rtl8139_mmio_readl(void *opaque, target_phys_addr_t addr)
-{
- uint32_t val = rtl8139_io_readl(opaque, addr & 0xFF);
-#ifdef TARGET_WORDS_BIGENDIAN
- val = bswap32(val);
-#endif
- return val;
-}
-
static int rtl8139_post_load(void *opaque, int version_id)
{
RTL8139State* s = opaque;
@@ -3196,40 +3107,77 @@ static const VMStateDescription vmstate_rtl8139 = {
/***********************************************************/
/* PCI RTL8139 definitions */
-static void rtl8139_mmio_map(PCIDevice *pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
+static uint32_t rtl8139_mmio_read(PCIDevice *dev, pcibus_t addr, int size)
{
- RTL8139State *s = DO_UPCAST(RTL8139State, dev, pci_dev);
+ RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev);
+ uint32_t value;
- cpu_register_physical_memory(addr + 0, 0x100, s->rtl8139_mmio_io_addr);
+ if (size == 1) {
+ value = rtl8139_io_readb(s, addr);
+ } else if (size == 2) {
+ value = rtl8139_io_readw(s, addr);
+#ifdef TARGET_WORDS_BIGENDIAN
+ value = bswap16(value);
+#endif
+ } else {
+ value = rtl8139_io_readl(s, addr & 0xFF);
+#ifdef TARGET_WORDS_BIGENDIAN
+ value = bswap32(value);
+#endif
+ }
+
+ return value;
}
-static void rtl8139_ioport_map(PCIDevice *pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
+static void rtl8139_mmio_write(PCIDevice *dev, pcibus_t addr, int size,
+ uint32_t value)
{
- RTL8139State *s = DO_UPCAST(RTL8139State, dev, pci_dev);
+ RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev);
- register_ioport_write(addr, 0x100, 1, rtl8139_ioport_writeb, s);
- register_ioport_read( addr, 0x100, 1, rtl8139_ioport_readb, s);
+ if (size == 1) {
+ rtl8139_io_writeb(s, addr, value);
+ } else if (size == 2) {
+#ifdef TARGET_WORDS_BIGENDIAN
+ value = bswap16(value);
+#endif
+ rtl8139_io_writew(s, addr, value);
+ } else if (size == 4) {
+#ifdef TARGET_WORDS_BIGENDIAN
+ value = bswap32(value);
+#endif
+ rtl8139_io_writel(s, addr, value);
+ }
+}
+
+static uint32_t rtl8139_io_read(PCIDevice *dev, pcibus_t addr, int size)
+{
+ RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev);
+ uint32_t value;
- register_ioport_write(addr, 0x100, 2, rtl8139_ioport_writew, s);
- register_ioport_read( addr, 0x100, 2, rtl8139_ioport_readw, s);
+ if (size == 1) {
+ value = rtl8139_io_readb(s, addr);
+ } else if (size == 2) {
+ value = rtl8139_io_readw(s, addr);
+ } else {
+ value = rtl8139_io_readl(s, addr);
+ }
- register_ioport_write(addr, 0x100, 4, rtl8139_ioport_writel, s);
- register_ioport_read( addr, 0x100, 4, rtl8139_ioport_readl, s);
+ return value;
}
-static CPUReadMemoryFunc * const rtl8139_mmio_read[3] = {
- rtl8139_mmio_readb,
- rtl8139_mmio_readw,
- rtl8139_mmio_readl,
-};
+static void rtl8139_io_write(PCIDevice *dev, pcibus_t addr, int size,
+ uint32_t value)
+{
+ RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev);
-static CPUWriteMemoryFunc * const rtl8139_mmio_write[3] = {
- rtl8139_mmio_writeb,
- rtl8139_mmio_writew,
- rtl8139_mmio_writel,
-};
+ if (size == 1) {
+ rtl8139_io_writeb(s, addr, value);
+ } else if (size == 2) {
+ rtl8139_io_writew(s, addr, value);
+ } else {
+ rtl8139_io_writel(s, addr, value);
+ }
+}
static inline int64_t rtl8139_get_next_tctr_time(RTL8139State *s, int64_t current_time)
{
@@ -3296,7 +3244,6 @@ static int pci_rtl8139_uninit(PCIDevice *dev)
{
RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev);
- cpu_unregister_io_memory(s->rtl8139_mmio_io_addr);
if (s->cplus_txbuffer) {
qemu_free(s->cplus_txbuffer);
s->cplus_txbuffer = NULL;
@@ -3336,15 +3283,10 @@ static int pci_rtl8139_init(PCIDevice *dev)
* list bit in status register, and offset 0xdc seems unused. */
pci_conf[PCI_CAPABILITY_LIST] = 0xdc;
- /* I/O handler for memory-mapped I/O */
- s->rtl8139_mmio_io_addr =
- cpu_register_io_memory(rtl8139_mmio_read, rtl8139_mmio_write, s);
-
- pci_register_bar(&s->dev, 0, 0x100,
- PCI_BASE_ADDRESS_SPACE_IO, rtl8139_ioport_map);
-
- pci_register_bar(&s->dev, 1, 0x100,
- PCI_BASE_ADDRESS_SPACE_MEMORY, rtl8139_mmio_map);
+ pci_register_io_region(&s->dev, 0, 0x100, PCI_BASE_ADDRESS_SPACE_IO,
+ rtl8139_io_read, rtl8139_io_write);
+ pci_register_io_region(&s->dev, 1, 0x100, PCI_BASE_ADDRESS_SPACE_MEMORY,
+ rtl8139_mmio_read, rtl8139_mmio_write);
qemu_macaddr_default_if_unset(&s->conf.macaddr);
--
1.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 03/15] lsi53c895a: convert to new pci interfaces
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 01/15] pci: add new bus functions Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 02/15] rtl8139: convert to new PCI interfaces Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 04/15] e1000: convert to new pci interface Anthony Liguori
` (13 subsequent siblings)
16 siblings, 0 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
- we drop special case of reading from script memory
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/lsi53c895a.c | 318 +++++++++++++++----------------------------------------
1 files changed, 87 insertions(+), 231 deletions(-)
diff --git a/hw/lsi53c895a.c b/hw/lsi53c895a.c
index 0daea40..6112694 100644
--- a/hw/lsi53c895a.c
+++ b/hw/lsi53c895a.c
@@ -187,7 +187,6 @@ typedef struct {
PCIDevice dev;
int mmio_io_addr;
int ram_io_addr;
- uint32_t script_ram_base;
int carry; /* ??? Should this be an a visible register somewhere? */
int sense;
@@ -376,12 +375,7 @@ static void lsi_reselect(LSIState *s, lsi_request *p);
static inline uint32_t read_dword(LSIState *s, uint32_t addr)
{
uint32_t buf;
-
- /* Optimize reading from SCRIPTS RAM. */
- if ((addr & 0xffffe000) == s->script_ram_base) {
- return s->script_ram[(addr & 0x1fff) >> 2];
- }
- cpu_physical_memory_read(addr, (uint8_t *)&buf, 4);
+ pci_memory_read(&s->dev, addr, (uint8_t *)&buf, 4);
return cpu_to_le32(buf);
}
@@ -507,7 +501,7 @@ static void lsi_resume_script(LSIState *s)
static void lsi_do_dma(LSIState *s, int out)
{
uint32_t count;
- target_phys_addr_t addr;
+ pcibus_t addr;
assert(s->current);
if (!s->current->dma_len) {
@@ -529,7 +523,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" FMT_pcibus " len=%d\n", addr, count);
s->csbc += count;
s->dnad += count;
s->dbc -= count;
@@ -541,9 +535,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_memory_read(&s->dev, addr, s->current->dma_buf, count);
} else {
- cpu_physical_memory_write(addr, s->current->dma_buf, count);
+ pci_memory_write(&s->dev, addr, s->current->dma_buf, count);
}
s->current->dma_len -= count;
if (s->current->dma_len == 0) {
@@ -706,7 +700,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_memory_read(&s->dev, s->dnad, buf, s->dbc);
s->sfbr = buf[0];
s->command_complete = 0;
@@ -750,7 +744,7 @@ static void lsi_do_status(LSIState *s)
s->dbc = 1;
sense = s->sense;
s->sfbr = sense;
- cpu_physical_memory_write(s->dnad, &sense, 1);
+ pci_memory_write(&s->dev, s->dnad, &sense, 1);
lsi_set_phase(s, PHASE_MI);
s->msg_action = 1;
lsi_add_msg_byte(s, 0); /* COMMAND COMPLETE */
@@ -770,7 +764,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_memory_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;
@@ -802,7 +796,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_memory_read(&s->dev, s->dnad, &data, 1);
s->dnad++;
s->dbc--;
return data;
@@ -889,8 +883,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_memory_read(&s->dev, src, buf, n);
+ pci_memory_write(&s->dev, dest, buf, n);
src += n;
dest += n;
count -= n;
@@ -958,7 +952,7 @@ again:
/* 32-bit Table indirect */
offset = sxt24(addr);
- cpu_physical_memory_read(s->dsa + offset, (uint8_t *)buf, 8);
+ pci_memory_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;
@@ -1320,7 +1314,7 @@ again:
n = (insn & 7);
reg = (insn >> 16) & 0xff;
if (insn & (1 << 24)) {
- cpu_physical_memory_read(addr, data, n);
+ pci_memory_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++) {
@@ -1331,7 +1325,7 @@ again:
for (i = 0; i < n; i++) {
data[i] = lsi_reg_readb(s, reg + i);
}
- cpu_physical_memory_write(addr, data, n);
+ pci_memory_write(&s->dev, addr, data, n);
}
}
}
@@ -1760,240 +1754,119 @@ static void lsi_reg_writeb(LSIState *s, int offset, uint8_t val)
#undef CASE_SET_REG32
}
-static void lsi_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
- LSIState *s = opaque;
-
- lsi_reg_writeb(s, addr & 0xff, val);
-}
-
-static void lsi_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
- LSIState *s = opaque;
-
- addr &= 0xff;
- lsi_reg_writeb(s, addr, val & 0xff);
- lsi_reg_writeb(s, addr + 1, (val >> 8) & 0xff);
-}
-
-static void lsi_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
+static void lsi_mmio_write(PCIDevice *dev,
+ pcibus_t addr, int size, uint32_t val)
{
- LSIState *s = opaque;
+ LSIState *s = DO_UPCAST(LSIState, dev, dev);
addr &= 0xff;
lsi_reg_writeb(s, addr, val & 0xff);
- lsi_reg_writeb(s, addr + 1, (val >> 8) & 0xff);
- lsi_reg_writeb(s, addr + 2, (val >> 16) & 0xff);
- lsi_reg_writeb(s, addr + 3, (val >> 24) & 0xff);
+ if (size > 1) {
+ lsi_reg_writeb(s, addr + 1, (val >> 8) & 0xff);
+ }
+ if (size > 2) {
+ lsi_reg_writeb(s, addr + 2, (val >> 16) & 0xff);
+ lsi_reg_writeb(s, addr + 3, (val >> 24) & 0xff);
+ }
}
-static uint32_t lsi_mmio_readb(void *opaque, target_phys_addr_t addr)
+static uint32_t lsi_mmio_read(PCIDevice *dev, pcibus_t addr, int size)
{
- LSIState *s = opaque;
-
- return lsi_reg_readb(s, addr & 0xff);
-}
-
-static uint32_t lsi_mmio_readw(void *opaque, target_phys_addr_t addr)
-{
- LSIState *s = opaque;
+ LSIState *s = DO_UPCAST(LSIState, dev, dev);
uint32_t val;
addr &= 0xff;
val = lsi_reg_readb(s, addr);
- val |= lsi_reg_readb(s, addr + 1) << 8;
- return val;
-}
-
-static uint32_t lsi_mmio_readl(void *opaque, target_phys_addr_t addr)
-{
- LSIState *s = opaque;
- uint32_t val;
- addr &= 0xff;
- val = lsi_reg_readb(s, addr);
- val |= lsi_reg_readb(s, addr + 1) << 8;
- val |= lsi_reg_readb(s, addr + 2) << 16;
- val |= lsi_reg_readb(s, addr + 3) << 24;
+ if (size > 1) {
+ val |= lsi_reg_readb(s, addr + 1) << 8;
+ }
+ if (size > 2) {
+ val |= lsi_reg_readb(s, addr + 2) << 16;
+ val |= lsi_reg_readb(s, addr + 3) << 24;
+ }
return val;
}
-static CPUReadMemoryFunc * const lsi_mmio_readfn[3] = {
- lsi_mmio_readb,
- lsi_mmio_readw,
- lsi_mmio_readl,
-};
-
-static CPUWriteMemoryFunc * const lsi_mmio_writefn[3] = {
- lsi_mmio_writeb,
- lsi_mmio_writew,
- lsi_mmio_writel,
-};
-
-static void lsi_ram_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
+static void lsi_ram_write(PCIDevice *dev, pcibus_t addr, int size, uint32_t val)
{
- LSIState *s = opaque;
+ LSIState *s = DO_UPCAST(LSIState, dev, dev);
uint32_t newval;
- int shift;
addr &= 0x1fff;
newval = s->script_ram[addr >> 2];
- shift = (addr & 3) * 8;
- newval &= ~(0xff << shift);
- newval |= val << shift;
- s->script_ram[addr >> 2] = newval;
-}
-
-static void lsi_ram_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
- LSIState *s = opaque;
- uint32_t newval;
- addr &= 0x1fff;
- newval = s->script_ram[addr >> 2];
- if (addr & 2) {
- newval = (newval & 0xffff) | (val << 16);
+ if (size == 1) {
+ int shift;
+ shift = (addr & 3) * 8;
+ newval &= ~(0xff << shift);
+ newval |= val << shift;
+ } else if (size == 2) {
+ if (addr & 2) {
+ newval = (newval & 0xffff) | (val << 16);
+ } else {
+ newval = (newval & 0xffff0000) | val;
+ }
} else {
- newval = (newval & 0xffff0000) | val;
+ newval = val;
}
- s->script_ram[addr >> 2] = newval;
-}
-
-
-static void lsi_ram_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
- LSIState *s = opaque;
- addr &= 0x1fff;
- s->script_ram[addr >> 2] = val;
-}
-
-static uint32_t lsi_ram_readb(void *opaque, target_phys_addr_t addr)
-{
- LSIState *s = opaque;
- uint32_t val;
-
- addr &= 0x1fff;
- val = s->script_ram[addr >> 2];
- val >>= (addr & 3) * 8;
- return val & 0xff;
+ s->script_ram[addr >> 2] = newval;
}
-static uint32_t lsi_ram_readw(void *opaque, target_phys_addr_t addr)
+static uint32_t lsi_ram_read(PCIDevice *dev, pcibus_t addr, int size)
{
- LSIState *s = opaque;
+ LSIState *s = DO_UPCAST(LSIState, dev, dev);
uint32_t val;
addr &= 0x1fff;
val = s->script_ram[addr >> 2];
- if (addr & 2)
- val >>= 16;
- return le16_to_cpu(val);
-}
-
-static uint32_t lsi_ram_readl(void *opaque, target_phys_addr_t addr)
-{
- LSIState *s = opaque;
- addr &= 0x1fff;
- return le32_to_cpu(s->script_ram[addr >> 2]);
-}
-
-static CPUReadMemoryFunc * const lsi_ram_readfn[3] = {
- lsi_ram_readb,
- lsi_ram_readw,
- lsi_ram_readl,
-};
-
-static CPUWriteMemoryFunc * const lsi_ram_writefn[3] = {
- lsi_ram_writeb,
- lsi_ram_writew,
- lsi_ram_writel,
-};
-
-static uint32_t lsi_io_readb(void *opaque, uint32_t addr)
-{
- LSIState *s = opaque;
- return lsi_reg_readb(s, addr & 0xff);
-}
+ if (size == 1) {
+ val >>= (addr & 3) * 8;
+ val &= 0xff;
+ } else if (size == 2) {
+ if (addr & 2) {
+ val >>= 16;
+ }
+ val = le16_to_cpu(val);
+ } else {
+ val = le32_to_cpu(val);
+ }
-static uint32_t lsi_io_readw(void *opaque, uint32_t addr)
-{
- LSIState *s = opaque;
- uint32_t val;
- addr &= 0xff;
- val = lsi_reg_readb(s, addr);
- val |= lsi_reg_readb(s, addr + 1) << 8;
return val;
}
-static uint32_t lsi_io_readl(void *opaque, uint32_t addr)
+static uint32_t lsi_io_read(PCIDevice *dev, pcibus_t addr, int size)
{
- LSIState *s = opaque;
+ LSIState *s = DO_UPCAST(LSIState, dev, dev);
uint32_t val;
+
addr &= 0xff;
val = lsi_reg_readb(s, addr);
- val |= lsi_reg_readb(s, addr + 1) << 8;
- val |= lsi_reg_readb(s, addr + 2) << 16;
- val |= lsi_reg_readb(s, addr + 3) << 24;
- return val;
-}
+ if (size > 1) {
+ val |= lsi_reg_readb(s, addr + 1) << 8;
+ }
+ if (size > 2) {
+ val |= lsi_reg_readb(s, addr + 2) << 16;
+ val |= lsi_reg_readb(s, addr + 3) << 24;
+ }
-static void lsi_io_writeb(void *opaque, uint32_t addr, uint32_t val)
-{
- LSIState *s = opaque;
- lsi_reg_writeb(s, addr & 0xff, val);
+ return val;
}
-static void lsi_io_writew(void *opaque, uint32_t addr, uint32_t val)
+static void lsi_io_write(PCIDevice *dev, pcibus_t addr, int size, uint32_t val)
{
- LSIState *s = opaque;
- addr &= 0xff;
- lsi_reg_writeb(s, addr, val & 0xff);
- lsi_reg_writeb(s, addr + 1, (val >> 8) & 0xff);
-}
+ LSIState *s = DO_UPCAST(LSIState, dev, dev);
-static void lsi_io_writel(void *opaque, uint32_t addr, uint32_t val)
-{
- LSIState *s = opaque;
addr &= 0xff;
lsi_reg_writeb(s, addr, val & 0xff);
- lsi_reg_writeb(s, addr + 1, (val >> 8) & 0xff);
- lsi_reg_writeb(s, addr + 2, (val >> 16) & 0xff);
- lsi_reg_writeb(s, addr + 3, (val >> 24) & 0xff);
-}
-
-static void lsi_io_mapfunc(PCIDevice *pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
-{
- LSIState *s = DO_UPCAST(LSIState, dev, pci_dev);
-
- DPRINTF("Mapping IO at %08"FMT_PCIBUS"\n", addr);
-
- register_ioport_write(addr, 256, 1, lsi_io_writeb, s);
- register_ioport_read(addr, 256, 1, lsi_io_readb, s);
- register_ioport_write(addr, 256, 2, lsi_io_writew, s);
- register_ioport_read(addr, 256, 2, lsi_io_readw, s);
- register_ioport_write(addr, 256, 4, lsi_io_writel, s);
- register_ioport_read(addr, 256, 4, lsi_io_readl, s);
-}
-
-static void lsi_ram_mapfunc(PCIDevice *pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
-{
- LSIState *s = DO_UPCAST(LSIState, dev, pci_dev);
-
- DPRINTF("Mapping ram at %08"FMT_PCIBUS"\n", addr);
- s->script_ram_base = addr;
- cpu_register_physical_memory(addr + 0, 0x2000, s->ram_io_addr);
-}
-
-static void lsi_mmio_mapfunc(PCIDevice *pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
-{
- LSIState *s = DO_UPCAST(LSIState, dev, pci_dev);
-
- DPRINTF("Mapping registers at %08"FMT_PCIBUS"\n", addr);
- cpu_register_physical_memory(addr + 0, 0x400, s->mmio_io_addr);
+ if (size > 1) {
+ lsi_reg_writeb(s, addr + 1, (val >> 8) & 0xff);
+ }
+ if (size > 20) {
+ lsi_reg_writeb(s, addr + 2, (val >> 16) & 0xff);
+ lsi_reg_writeb(s, addr + 3, (val >> 24) & 0xff);
+ }
}
static void lsi_pre_save(void *opaque)
@@ -2090,16 +1963,6 @@ static const VMStateDescription vmstate_lsi_scsi = {
}
};
-static int lsi_scsi_uninit(PCIDevice *d)
-{
- LSIState *s = DO_UPCAST(LSIState, dev, d);
-
- cpu_unregister_io_memory(s->mmio_io_addr);
- cpu_unregister_io_memory(s->ram_io_addr);
-
- return 0;
-}
-
static int lsi_scsi_init(PCIDevice *dev)
{
LSIState *s = DO_UPCAST(LSIState, dev, dev);
@@ -2122,18 +1985,12 @@ static int lsi_scsi_init(PCIDevice *dev)
/* Interrupt pin 1 */
pci_conf[PCI_INTERRUPT_PIN] = 0x01;
- s->mmio_io_addr = cpu_register_io_memory(lsi_mmio_readfn,
- lsi_mmio_writefn, s);
- s->ram_io_addr = cpu_register_io_memory(lsi_ram_readfn,
- lsi_ram_writefn, s);
-
- /* TODO: use dev and get rid of cast below */
- pci_register_bar((struct PCIDevice *)s, 0, 256,
- PCI_BASE_ADDRESS_SPACE_IO, lsi_io_mapfunc);
- pci_register_bar((struct PCIDevice *)s, 1, 0x400,
- PCI_BASE_ADDRESS_SPACE_MEMORY, lsi_mmio_mapfunc);
- pci_register_bar((struct PCIDevice *)s, 2, 0x2000,
- PCI_BASE_ADDRESS_SPACE_MEMORY, lsi_ram_mapfunc);
+ pci_register_io_region(dev, 0, 256, PCI_BASE_ADDRESS_SPACE_IO,
+ lsi_io_read, lsi_io_write);
+ pci_register_io_region(dev, 1, 0x400, PCI_BASE_ADDRESS_SPACE_MEMORY,
+ lsi_mmio_read, lsi_mmio_write);
+ pci_register_io_region(dev, 2, 0x2000, PCI_BASE_ADDRESS_SPACE_MEMORY,
+ lsi_ram_read, lsi_ram_write);
QTAILQ_INIT(&s->queue);
lsi_soft_reset(s);
@@ -2151,7 +2008,6 @@ static PCIDeviceInfo lsi_info = {
.qdev.size = sizeof(LSIState),
.qdev.vmsd = &vmstate_lsi_scsi,
.init = lsi_scsi_init,
- .exit = lsi_scsi_uninit,
};
static void lsi53c895a_register_devices(void)
--
1.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 04/15] e1000: convert to new pci interface
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
` (2 preceding siblings ...)
2010-02-09 22:01 ` [Qemu-devel] [PATCH 03/15] lsi53c895a: convert to new pci interfaces Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 05/15] wdt_i6300esb: fix io type leakage Anthony Liguori
` (12 subsequent siblings)
16 siblings, 0 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
- eliminated coalesced mmio support, need new api
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/e1000.c | 164 ++++++++++++++++++++++--------------------------------------
1 files changed, 60 insertions(+), 104 deletions(-)
diff --git a/hw/e1000.c b/hw/e1000.c
index fd3059a..823088c 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -148,12 +148,18 @@ static const char phy_regcap[0x20] = {
[PHY_ID2] = PHY_R, [M88E1000_PHY_SPEC_STATUS] = PHY_R
};
-static void
-ioport_map(PCIDevice *pci_dev, int region_num, pcibus_t addr,
- pcibus_t size, int type)
+static uint32_t e1000_io_read(PCIDevice *dev, pcibus_t addr, int size)
+{
+ DBGOUT(IO, "e1000_io_read addr=0x%04"FMT_PCIBUS
+ " size=%d\n", addr, size);
+ return 0xFFFFFFFF;
+}
+
+static void e1000_io_write(PCIDevice *dev, pcibus_t addr, int size,
+ uint32_t value)
{
- DBGOUT(IO, "e1000_ioport_map addr=0x%04"FMT_PCIBUS
- " size=0x%08"FMT_PCIBUS"\n", addr, size);
+ DBGOUT(IO, "e1000_io_write addr=0x%04"FMT_PCIBUS
+ " size=%d value=%u\n", addr, size, value);
}
static void
@@ -456,7 +462,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_memory_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;
@@ -471,7 +477,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_memory_read(&s->dev, addr, tp->data + tp->size, split_size);
tp->size += split_size;
}
@@ -487,7 +493,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, pcibus_t base, struct e1000_tx_desc *dp)
{
uint32_t txd_upper, txd_lower = le32_to_cpu(dp->lower.data);
@@ -496,7 +502,7 @@ 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),
+ pci_memory_write(&s->dev, base + ((char *)&dp->upper - (char *)dp),
(void *)&dp->upper, sizeof(dp->upper));
return E1000_ICR_TXDW;
}
@@ -504,7 +510,7 @@ txdesc_writeback(target_phys_addr_t base, struct e1000_tx_desc *dp)
static void
start_xmit(E1000State *s)
{
- target_phys_addr_t base;
+ pcibus_t base;
struct e1000_tx_desc desc;
uint32_t tdh_start = s->mac_reg[TDH], cause = E1000_ICS_TXQE;
@@ -516,14 +522,14 @@ start_xmit(E1000State *s)
while (s->mac_reg[TDH] != s->mac_reg[TDT]) {
base = ((uint64_t)s->mac_reg[TDBAH] << 32) + s->mac_reg[TDBAL] +
sizeof(struct e1000_tx_desc) * s->mac_reg[TDH];
- cpu_physical_memory_read(base, (void *)&desc, sizeof(desc));
+ pci_memory_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;
@@ -622,7 +628,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;
+ pcibus_t base;
unsigned int n, rdt;
uint32_t rdh_start;
uint16_t vlan_special = 0;
@@ -657,17 +663,17 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf, size_t size)
}
base = ((uint64_t)s->mac_reg[RDBAH] << 32) + s->mac_reg[RDBAL] +
sizeof(desc) * s->mac_reg[RDH];
- cpu_physical_memory_read(base, (void *)&desc, sizeof(desc));
+ pci_memory_read(&s->dev, base, (void *)&desc, sizeof(desc));
desc.special = vlan_special;
desc.status |= (vlan_status | E1000_RXD_STAT_DD);
if (desc.buffer_addr) {
- cpu_physical_memory_write(le64_to_cpu(desc.buffer_addr),
+ pci_memory_write(&s->dev, le64_to_cpu(desc.buffer_addr),
(void *)(buf + vlan_offset), size);
desc.length = cpu_to_le16(size);
desc.status |= E1000_RXD_STAT_EOP|E1000_RXD_STAT_IXSM;
} 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_memory_write(&s->dev, base, (void *)&desc, sizeof(desc));
if (++s->mac_reg[RDH] * sizeof(desc) >= s->mac_reg[RDLEN])
s->mac_reg[RDH] = 0;
@@ -822,72 +828,56 @@ static void (*macreg_writeops[])(E1000State *, int, uint32_t) = {
[MTA ... MTA+127] = &mac_writereg,
[VFTA ... VFTA+127] = &mac_writereg,
};
+
enum { NWRITEOPS = ARRAY_SIZE(macreg_writeops) };
-static void
-e1000_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
+static uint32_t size_mask(int size)
{
- E1000State *s = opaque;
+ return (1ULL << (size * 8)) - 1;
+}
+
+static int addr_shift(pcibus_t addr)
+{
+ return (8 * (addr & 3));
+}
+
+static void e1000_mmio_write(PCIDevice *dev, pcibus_t addr, int size,
+ uint32_t value)
+{
+ E1000State *d = DO_UPCAST(E1000State, dev, dev);
unsigned int index = (addr & 0x1ffff) >> 2;
#ifdef TARGET_WORDS_BIGENDIAN
- val = bswap32(val);
+ value = bswap32(value);
#endif
+ value = (value & size_mask(size)) << addr_shift(addr);
+
if (index < NWRITEOPS && macreg_writeops[index])
- macreg_writeops[index](s, index, val);
+ macreg_writeops[index](d, index, value);
else if (index < NREADOPS && macreg_readops[index])
- DBGOUT(MMIO, "e1000_mmio_writel RO %x: 0x%04x\n", index<<2, val);
+ DBGOUT(MMIO, "e1000_mmio_writel RO %x: 0x%04x\n", index<<2, value);
else
DBGOUT(UNKNOWN, "MMIO unknown write addr=0x%08x,val=0x%08x\n",
- index<<2, val);
+ index<<2, value);
}
-static void
-e1000_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
+static uint32_t e1000_mmio_read(PCIDevice *dev, pcibus_t addr, int size)
{
- // emulate hw without byte enables: no RMW
- e1000_mmio_writel(opaque, addr & ~3,
- (val & 0xffff) << (8*(addr & 3)));
-}
-
-static void
-e1000_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
- // emulate hw without byte enables: no RMW
- e1000_mmio_writel(opaque, addr & ~3,
- (val & 0xff) << (8*(addr & 3)));
-}
+ E1000State *d = DO_UPCAST(E1000State, dev, dev);
+ unsigned int index = (addr & 0x1fffc) >> 2;
+ uint32_t value = 0;
-static uint32_t
-e1000_mmio_readl(void *opaque, target_phys_addr_t addr)
-{
- E1000State *s = opaque;
- unsigned int index = (addr & 0x1ffff) >> 2;
+ if (index < NREADOPS && macreg_readops[index]) {
+ value = macreg_readops[index](d, index);
+ } else {
+ DBGOUT(UNKNOWN, "MMIO unknown read addr=0x%08x\n", index<<2);
+ }
- if (index < NREADOPS && macreg_readops[index])
- {
- uint32_t val = macreg_readops[index](s, index);
+ value = (value >> addr_shift(addr)) & size_mask(size);
#ifdef TARGET_WORDS_BIGENDIAN
- val = bswap32(val);
+ value = bswap32(value);
#endif
- return val;
- }
- DBGOUT(UNKNOWN, "MMIO unknown read addr=0x%08x\n", index<<2);
- return 0;
-}
-
-static uint32_t
-e1000_mmio_readb(void *opaque, target_phys_addr_t addr)
-{
- return ((e1000_mmio_readl(opaque, addr & ~3)) >>
- (8 * (addr & 3))) & 0xff;
-}
-
-static uint32_t
-e1000_mmio_readw(void *opaque, target_phys_addr_t addr)
-{
- return ((e1000_mmio_readl(opaque, addr & ~3)) >>
- (8 * (addr & 3))) & 0xffff;
+ return value;
}
static bool is_version_1(void *opaque, int version_id)
@@ -1009,38 +999,6 @@ static const uint32_t mac_reg_init[] = {
/* PCI interface */
-static CPUWriteMemoryFunc * const e1000_mmio_write[] = {
- e1000_mmio_writeb, e1000_mmio_writew, e1000_mmio_writel
-};
-
-static CPUReadMemoryFunc * const e1000_mmio_read[] = {
- e1000_mmio_readb, e1000_mmio_readw, e1000_mmio_readl
-};
-
-static void
-e1000_mmio_map(PCIDevice *pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
-{
- E1000State *d = DO_UPCAST(E1000State, dev, pci_dev);
- int i;
- const uint32_t excluded_regs[] = {
- E1000_MDIC, E1000_ICR, E1000_ICS, E1000_IMS,
- E1000_IMC, E1000_TCTL, E1000_TDT, PNPMMIO_SIZE
- };
-
-
- DBGOUT(MMIO, "e1000_mmio_map addr=0x%08"FMT_PCIBUS" 0x%08"FMT_PCIBUS"\n",
- addr, size);
-
- cpu_register_physical_memory(addr, PNPMMIO_SIZE, d->mmio_index);
- qemu_register_coalesced_mmio(addr, excluded_regs[0]);
-
- for (i = 0; excluded_regs[i] != PNPMMIO_SIZE; i++)
- qemu_register_coalesced_mmio(addr + excluded_regs[i] + 4,
- excluded_regs[i + 1] -
- excluded_regs[i] - 4);
-}
-
static void
e1000_cleanup(VLANClientState *nc)
{
@@ -1054,7 +1012,6 @@ pci_e1000_uninit(PCIDevice *dev)
{
E1000State *d = DO_UPCAST(E1000State, dev, dev);
- cpu_unregister_io_memory(d->mmio_index);
qemu_del_vlan_client(&d->nic->nc);
return 0;
}
@@ -1102,14 +1059,13 @@ static int pci_e1000_init(PCIDevice *pci_dev)
/* TODO: RST# value should be 0 if programmable, PCI spec 6.2.4 */
pci_conf[PCI_INTERRUPT_PIN] = 1; // interrupt pin 0
- d->mmio_index = cpu_register_io_memory(e1000_mmio_read,
- e1000_mmio_write, d);
-
- pci_register_bar((PCIDevice *)d, 0, PNPMMIO_SIZE,
- PCI_BASE_ADDRESS_SPACE_MEMORY, e1000_mmio_map);
+ pci_register_io_region(pci_dev, 0, PNPMMIO_SIZE,
+ PCI_BASE_ADDRESS_SPACE_MEMORY,
+ e1000_mmio_read, e1000_mmio_write);
- pci_register_bar((PCIDevice *)d, 1, IOPORT_SIZE,
- PCI_BASE_ADDRESS_SPACE_IO, ioport_map);
+ pci_register_io_region(pci_dev, 1, IOPORT_SIZE,
+ PCI_BASE_ADDRESS_SPACE_IO,
+ e1000_io_read, e1000_io_write);
memmove(d->eeprom_data, e1000_eeprom_template,
sizeof e1000_eeprom_template);
--
1.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 05/15] wdt_i6300esb: fix io type leakage
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
` (3 preceding siblings ...)
2010-02-09 22:01 ` [Qemu-devel] [PATCH 04/15] e1000: convert to new pci interface Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 06/15] wdt_i6300esb: convert to new pci inteface Anthony Liguori
` (11 subsequent siblings)
16 siblings, 0 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
The watchdog device registers an io type in the pci map_func callback. This
callback is invoked whenever the OS needs to reposition the IO region in memory.
While we automatically unmap previous mappings, we don't unregister the io type
(since the PCI layer does not know about this).
The current code will leak io types and eventually exhaust them. You can
reproduce it by repeatedly rebooting a guest for about 30 times.
The fix is to register the io type once at init.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/wdt_i6300esb.c | 30 +++++++++++++++++-------------
1 files changed, 17 insertions(+), 13 deletions(-)
diff --git a/hw/wdt_i6300esb.c b/hw/wdt_i6300esb.c
index be0e89e..6c08c32 100644
--- a/hw/wdt_i6300esb.c
+++ b/hw/wdt_i6300esb.c
@@ -98,6 +98,8 @@ struct I6300State {
int previous_reboot_flag; /* If the watchdog caused the previous
* reboot, this flag will be set.
*/
+
+ int io_mem;
};
typedef struct I6300State I6300State;
@@ -342,27 +344,27 @@ static void i6300esb_mem_writel(void *vp, target_phys_addr_t addr, uint32_t val)
}
}
+static CPUReadMemoryFunc * const mem_read[3] = {
+ i6300esb_mem_readb,
+ i6300esb_mem_readw,
+ i6300esb_mem_readl,
+};
+
+static CPUWriteMemoryFunc * const mem_write[3] = {
+ i6300esb_mem_writeb,
+ i6300esb_mem_writew,
+ i6300esb_mem_writel,
+};
+
static void i6300esb_map(PCIDevice *dev, int region_num,
pcibus_t addr, pcibus_t size, int type)
{
- static CPUReadMemoryFunc * const mem_read[3] = {
- i6300esb_mem_readb,
- i6300esb_mem_readw,
- i6300esb_mem_readl,
- };
- static CPUWriteMemoryFunc * const mem_write[3] = {
- i6300esb_mem_writeb,
- i6300esb_mem_writew,
- i6300esb_mem_writel,
- };
I6300State *d = DO_UPCAST(I6300State, dev, dev);
- int io_mem;
i6300esb_debug("addr = %"FMT_PCIBUS", size = %"FMT_PCIBUS", type = %d\n",
addr, size, type);
- io_mem = cpu_register_io_memory(mem_read, mem_write, d);
- cpu_register_physical_memory (addr, 0x10, io_mem);
+ cpu_register_physical_memory (addr, 0x10, d->io_mem);
/* qemu_register_coalesced_mmio (addr, 0x10); ? */
}
@@ -406,6 +408,8 @@ static int i6300esb_init(PCIDevice *dev)
d->stage = 1;
d->unlock_state = 0;
d->previous_reboot_flag = 0;
+ d->io_mem = cpu_register_io_memory(mem_read, mem_write, d);
+
pci_conf = d->dev.config;
pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
--
1.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 06/15] wdt_i6300esb: convert to new pci inteface
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
` (4 preceding siblings ...)
2010-02-09 22:01 ` [Qemu-devel] [PATCH 05/15] wdt_i6300esb: fix io type leakage Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API Anthony Liguori
` (10 subsequent siblings)
16 siblings, 0 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
- kept same semantics for writes, probably can be simplified
- fixes an iotype leak for hot unplug
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/wdt_i6300esb.c | 122 ++++++++++++-----------------------------------------
1 files changed, 27 insertions(+), 95 deletions(-)
diff --git a/hw/wdt_i6300esb.c b/hw/wdt_i6300esb.c
index 6c08c32..6de5866 100644
--- a/hw/wdt_i6300esb.c
+++ b/hw/wdt_i6300esb.c
@@ -98,8 +98,6 @@ struct I6300State {
int previous_reboot_flag; /* If the watchdog caused the previous
* reboot, this flag will be set.
*/
-
- int io_mem;
};
typedef struct I6300State I6300State;
@@ -245,21 +243,14 @@ static uint32_t i6300esb_config_read(PCIDevice *dev, uint32_t addr, int len)
}
}
-static uint32_t i6300esb_mem_readb(void *vp, target_phys_addr_t addr)
-{
- i6300esb_debug ("addr = %x\n", (int) addr);
-
- return 0;
-}
-
-static uint32_t i6300esb_mem_readw(void *vp, target_phys_addr_t addr)
+static uint32_t i6300esb_mem_read(PCIDevice *dev, pcibus_t addr, int size)
{
+ I6300State *d = DO_UPCAST(I6300State, dev, dev);
uint32_t data = 0;
- I6300State *d = vp;
i6300esb_debug("addr = %x\n", (int) addr);
- if (addr == 0xc) {
+ if (size == 2 && addr == 0xc) {
/* The previous reboot flag is really bit 9, but there is
* a bug in the Linux driver where it thinks it's bit 12.
* Set both.
@@ -270,28 +261,10 @@ static uint32_t i6300esb_mem_readw(void *vp, target_phys_addr_t addr)
return data;
}
-static uint32_t i6300esb_mem_readl(void *vp, target_phys_addr_t addr)
-{
- i6300esb_debug("addr = %x\n", (int) addr);
-
- return 0;
-}
-
-static void i6300esb_mem_writeb(void *vp, target_phys_addr_t addr, uint32_t val)
+static void i6300esb_mem_write(PCIDevice *dev, pcibus_t addr, int size,
+ uint32_t val)
{
- I6300State *d = vp;
-
- i6300esb_debug("addr = %x, val = %x\n", (int) addr, val);
-
- if (addr == 0xc && val == 0x80)
- d->unlock_state = 1;
- else if (addr == 0xc && val == 0x86 && d->unlock_state == 1)
- d->unlock_state = 2;
-}
-
-static void i6300esb_mem_writew(void *vp, target_phys_addr_t addr, uint32_t val)
-{
- I6300State *d = vp;
+ I6300State *d = DO_UPCAST(I6300State, dev, dev);
i6300esb_debug("addr = %x, val = %x\n", (int) addr, val);
@@ -301,20 +274,27 @@ static void i6300esb_mem_writew(void *vp, target_phys_addr_t addr, uint32_t val)
d->unlock_state = 2;
else {
if (d->unlock_state == 2) {
- if (addr == 0xc) {
- if ((val & 0x100) != 0)
- /* This is the "ping" from the userspace watchdog in
- * the guest ...
+ if (size == 2) {
+ if (addr == 0xc) {
+ if ((val & 0x100) != 0)
+ /* This is the "ping" from the userspace watchdog in
+ * the guest ...
+ */
+ i6300esb_restart_timer(d, 1);
+
+ /* Setting bit 9 resets the previous reboot flag.
+ * There's a bug in the Linux driver where it sets
+ * bit 12 instead.
*/
- i6300esb_restart_timer(d, 1);
-
- /* Setting bit 9 resets the previous reboot flag.
- * There's a bug in the Linux driver where it sets
- * bit 12 instead.
- */
- if ((val & 0x200) != 0 || (val & 0x1000) != 0) {
- d->previous_reboot_flag = 0;
+ if ((val & 0x200) != 0 || (val & 0x1000) != 0) {
+ d->previous_reboot_flag = 0;
+ }
}
+ } else if (size == 4) {
+ if (addr == 0)
+ d->timer1_preload = val & 0xfffff;
+ else if (addr == 4)
+ d->timer2_preload = val & 0xfffff;
}
d->unlock_state = 0;
@@ -322,52 +302,6 @@ static void i6300esb_mem_writew(void *vp, target_phys_addr_t addr, uint32_t val)
}
}
-static void i6300esb_mem_writel(void *vp, target_phys_addr_t addr, uint32_t val)
-{
- I6300State *d = vp;
-
- i6300esb_debug ("addr = %x, val = %x\n", (int) addr, val);
-
- if (addr == 0xc && val == 0x80)
- d->unlock_state = 1;
- else if (addr == 0xc && val == 0x86 && d->unlock_state == 1)
- d->unlock_state = 2;
- else {
- if (d->unlock_state == 2) {
- if (addr == 0)
- d->timer1_preload = val & 0xfffff;
- else if (addr == 4)
- d->timer2_preload = val & 0xfffff;
-
- d->unlock_state = 0;
- }
- }
-}
-
-static CPUReadMemoryFunc * const mem_read[3] = {
- i6300esb_mem_readb,
- i6300esb_mem_readw,
- i6300esb_mem_readl,
-};
-
-static CPUWriteMemoryFunc * const mem_write[3] = {
- i6300esb_mem_writeb,
- i6300esb_mem_writew,
- i6300esb_mem_writel,
-};
-
-static void i6300esb_map(PCIDevice *dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
-{
- I6300State *d = DO_UPCAST(I6300State, dev, dev);
-
- i6300esb_debug("addr = %"FMT_PCIBUS", size = %"FMT_PCIBUS", type = %d\n",
- addr, size, type);
-
- cpu_register_physical_memory (addr, 0x10, d->io_mem);
- /* qemu_register_coalesced_mmio (addr, 0x10); ? */
-}
-
static const VMStateDescription vmstate_i6300esb = {
.name = "i6300esb_wdt",
.version_id = sizeof(I6300State),
@@ -408,8 +342,6 @@ static int i6300esb_init(PCIDevice *dev)
d->stage = 1;
d->unlock_state = 0;
d->previous_reboot_flag = 0;
- d->io_mem = cpu_register_io_memory(mem_read, mem_write, d);
-
pci_conf = d->dev.config;
pci_config_set_vendor_id(pci_conf, PCI_VENDOR_ID_INTEL);
@@ -417,8 +349,8 @@ static int i6300esb_init(PCIDevice *dev)
pci_config_set_class(pci_conf, PCI_CLASS_SYSTEM_OTHER);
pci_conf[PCI_HEADER_TYPE] = 0x00;
- pci_register_bar(&d->dev, 0, 0x10,
- PCI_BASE_ADDRESS_SPACE_MEMORY, i6300esb_map);
+ pci_register_io_region(&d->dev, 0, 0x10, PCI_BASE_ADDRESS_SPACE_MEMORY,
+ i6300esb_mem_read, i6300esb_mem_write);
return 0;
}
--
1.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
` (5 preceding siblings ...)
2010-02-09 22:01 ` [Qemu-devel] [PATCH 06/15] wdt_i6300esb: convert to new pci inteface Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-09 22:45 ` malc
2010-02-09 22:01 ` [Qemu-devel] [PATCH 08/15] es1370: convert to new pci interface Anthony Liguori
` (9 subsequent siblings)
16 siblings, 1 reply; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
- fixed bug with size of registered ioport regions
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/ac97.c | 146 +++++++++++++++++++++++++------------------------------------
1 files changed, 60 insertions(+), 86 deletions(-)
diff --git a/hw/ac97.c b/hw/ac97.c
index 4319bc8..9fdf591 100644
--- a/hw/ac97.c
+++ b/hw/ac97.c
@@ -223,7 +223,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_memory_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]);
@@ -569,50 +569,35 @@ static void mixer_reset (AC97LinkState *s)
/**
* Native audio mixer
- * I/O Reads
*/
-static uint32_t nam_readb (void *opaque, uint32_t addr)
-{
- AC97LinkState *s = opaque;
- dolog ("U nam readb %#x\n", addr);
- s->cas = 0;
- return ~0U;
-}
-static uint32_t nam_readw (void *opaque, uint32_t addr)
+static uint32_t nam_read (PCIDevice *dev, pcibus_t addr, int size)
{
- AC97LinkState *s = opaque;
- uint32_t val = ~0U;
- uint32_t index = addr - s->base[0];
- s->cas = 0;
- val = mixer_load (s, index);
- return val;
-}
+ AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, dev);
+ uint32_t value;
+
+ if (size == 2) {
+ value = mixer_load (s, addr);
+ } else {
+ dolog ("U nam read[%d] %#" FMT_PCIBUS "\n", size, addr);
+ s->cas = 0;
+ value = ~0U;
+ }
-static uint32_t nam_readl (void *opaque, uint32_t addr)
-{
- AC97LinkState *s = opaque;
- dolog ("U nam readl %#x\n", addr);
- s->cas = 0;
- return ~0U;
+ return value;
}
-/**
- * Native audio mixer
- * I/O Writes
- */
-static void nam_writeb (void *opaque, uint32_t addr, uint32_t val)
+static void nam_write (PCIDevice *dev, pcibus_t addr, int size, uint32_t val)
{
- AC97LinkState *s = opaque;
- dolog ("U nam writeb %#x <- %#x\n", addr, val);
- s->cas = 0;
-}
+ AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, dev);
+ uint32_t index = addr;
-static void nam_writew (void *opaque, uint32_t addr, uint32_t val)
-{
- AC97LinkState *s = opaque;
- uint32_t index = addr - s->base[0];
s->cas = 0;
+ if (size != 2) {
+ dolog ("U nam write[%d] %#" FMT_PCIBUS " <- %#x\n", addr, val);
+ return;
+ }
+
switch (index) {
case AC97_Reset:
mixer_reset (s);
@@ -699,22 +684,13 @@ static void nam_writew (void *opaque, uint32_t addr, uint32_t val)
}
}
-static void nam_writel (void *opaque, uint32_t addr, uint32_t val)
-{
- AC97LinkState *s = opaque;
- dolog ("U nam writel %#x <- %#x\n", addr, val);
- s->cas = 0;
-}
-
/**
* Native audio bus master
* I/O Reads
*/
-static uint32_t nabm_readb (void *opaque, uint32_t addr)
+static uint32_t nabm_readb (AC97LinkState *s, uint32_t index)
{
- AC97LinkState *s = opaque;
AC97BusMasterRegs *r = NULL;
- uint32_t index = addr - s->base[1];
uint32_t val = ~0U;
switch (index) {
@@ -765,11 +741,9 @@ static uint32_t nabm_readb (void *opaque, uint32_t addr)
return val;
}
-static uint32_t nabm_readw (void *opaque, uint32_t addr)
+static uint32_t nabm_readw (AC97LinkState *s, uint32_t index)
{
- AC97LinkState *s = opaque;
AC97BusMasterRegs *r = NULL;
- uint32_t index = addr - s->base[1];
uint32_t val = ~0U;
switch (index) {
@@ -794,11 +768,9 @@ static uint32_t nabm_readw (void *opaque, uint32_t addr)
return val;
}
-static uint32_t nabm_readl (void *opaque, uint32_t addr)
+static uint32_t nabm_readl (AC97LinkState *s, uint32_t index)
{
- AC97LinkState *s = opaque;
AC97BusMasterRegs *r = NULL;
- uint32_t index = addr - s->base[1];
uint32_t val = ~0U;
switch (index) {
@@ -840,15 +812,29 @@ static uint32_t nabm_readl (void *opaque, uint32_t addr)
return val;
}
+static uint32_t nabm_read (PCIDevice *dev, pcibus_t addr, int size)
+{
+ AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, dev);
+ uint32_t value;
+
+ if (size == 1) {
+ value = nabm_readb (s, addr);
+ } else if (size == 2) {
+ value = nabm_readw (s, addr);
+ } else {
+ value = nabm_readl (s, addr);
+ }
+
+ return value;
+}
+
/**
* Native audio bus master
* I/O Writes
*/
-static void nabm_writeb (void *opaque, uint32_t addr, uint32_t val)
+static void nabm_writeb (AC97LinkState *s, uint32_t index, uint32_t val)
{
- AC97LinkState *s = opaque;
AC97BusMasterRegs *r = NULL;
- uint32_t index = addr - s->base[1];
switch (index) {
case PI_LVI:
case PO_LVI:
@@ -954,6 +940,19 @@ static void nabm_writel (void *opaque, uint32_t addr, uint32_t val)
}
}
+static void nabm_write (PCIDevice *dev, pcibus_t addr, int size, uint32_t value)
+{
+ AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, dev);
+
+ if (size == 1) {
+ nabm_writeb (s, addr, value);
+ } else if (size == 2) {
+ nabm_writew (s, addr, value);
+ } else {
+ nabm_writel (s, addr, value);
+ }
+}
+
static int write_audio (AC97LinkState *s, AC97BusMasterRegs *r,
int max, int *stop)
{
@@ -972,7 +971,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_memory_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);
@@ -1056,7 +1055,7 @@ static int read_audio (AC97LinkState *s, AC97BusMasterRegs *r,
*stop = 1;
break;
}
- cpu_physical_memory_write (addr, tmpbuf, acquired);
+ pci_memory_write (&s->dev, addr, tmpbuf, acquired);
temp -= acquired;
addr += acquired;
nread += acquired;
@@ -1234,32 +1233,6 @@ static const VMStateDescription vmstate_ac97 = {
}
};
-static void ac97_map (PCIDevice *pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
-{
- AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, pci_dev);
- PCIDevice *d = &s->dev;
-
- if (!region_num) {
- s->base[0] = addr;
- register_ioport_read (addr, 256 * 1, 1, nam_readb, d);
- register_ioport_read (addr, 256 * 2, 2, nam_readw, d);
- register_ioport_read (addr, 256 * 4, 4, nam_readl, d);
- register_ioport_write (addr, 256 * 1, 1, nam_writeb, d);
- register_ioport_write (addr, 256 * 2, 2, nam_writew, d);
- register_ioport_write (addr, 256 * 4, 4, nam_writel, d);
- }
- else {
- s->base[1] = addr;
- register_ioport_read (addr, 64 * 1, 1, nabm_readb, d);
- register_ioport_read (addr, 64 * 2, 2, nabm_readw, d);
- register_ioport_read (addr, 64 * 4, 4, nabm_readl, d);
- register_ioport_write (addr, 64 * 1, 1, nabm_writeb, d);
- register_ioport_write (addr, 64 * 2, 2, nabm_writew, d);
- register_ioport_write (addr, 64 * 4, 4, nabm_writel, d);
- }
-}
-
static void ac97_on_reset (void *opaque)
{
AC97LinkState *s = opaque;
@@ -1321,9 +1294,10 @@ static int ac97_initfn (PCIDevice *dev)
/* TODO: RST# value should be 0. */
c[PCI_INTERRUPT_PIN] = 0x01; /* intr_pn interrupt pin ro */
- pci_register_bar (&s->dev, 0, 256 * 4, PCI_BASE_ADDRESS_SPACE_IO,
- ac97_map);
- pci_register_bar (&s->dev, 1, 64 * 4, PCI_BASE_ADDRESS_SPACE_IO, ac97_map);
+ pci_register_io_region (&s->dev, 0, 256 * 4, PCI_BASE_ADDRESS_SPACE_IO,
+ nam_read, nam_write);
+ pci_register_io_region (&s->dev, 1, 64 * 4, PCI_BASE_ADDRESS_SPACE_IO,
+ nabm_read, nabm_write);
qemu_register_reset (ac97_on_reset, s);
AUD_register_card ("ac97", &s->card);
ac97_on_reset (s);
--
1.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 08/15] es1370: convert to new pci interface
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
` (6 preceding siblings ...)
2010-02-09 22:01 ` [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 09/15] eepro100: " Anthony Liguori
` (8 subsequent siblings)
16 siblings, 0 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/es1370.c | 64 ++++++++++++++++++++++++++++++++--------------------------
1 files changed, 35 insertions(+), 29 deletions(-)
diff --git a/hw/es1370.c b/hw/es1370.c
index 40cb48c..fdf63f1 100644
--- a/hw/es1370.c
+++ b/hw/es1370.c
@@ -157,9 +157,9 @@ static const unsigned dac1_samplerate[] = { 5512, 11025, 22050, 44100 };
#define ADC_CHANNEL 2
#define IO_READ_PROTO(n) \
-static uint32_t n (void *opaque, uint32_t addr)
+static uint32_t n (ES1370State *s, uint32_t addr)
#define IO_WRITE_PROTO(n) \
-static void n (void *opaque, uint32_t addr, uint32_t val)
+static void n (ES1370State *s, uint32_t addr, uint32_t val)
static void es1370_dac1_callback (void *opaque, int free);
static void es1370_dac2_callback (void *opaque, int free);
@@ -474,7 +474,6 @@ static inline uint32_t es1370_fixup (ES1370State *s, uint32_t addr)
IO_WRITE_PROTO (es1370_writeb)
{
- ES1370State *s = opaque;
uint32_t shift, mask;
addr = es1370_fixup (s, addr);
@@ -512,7 +511,6 @@ IO_WRITE_PROTO (es1370_writeb)
IO_WRITE_PROTO (es1370_writew)
{
- ES1370State *s = opaque;
addr = es1370_fixup (s, addr);
uint32_t shift, mask;
struct chan *d = &s->chan[0];
@@ -549,7 +547,6 @@ IO_WRITE_PROTO (es1370_writew)
IO_WRITE_PROTO (es1370_writel)
{
- ES1370State *s = opaque;
struct chan *d = &s->chan[0];
addr = es1370_fixup (s, addr);
@@ -613,9 +610,22 @@ IO_WRITE_PROTO (es1370_writel)
}
}
+static void es1370_write (PCIDevice *dev, pcibus_t addr, int size,
+ uint32_t value)
+{
+ ES1370State *s = DO_UPCAST (ES1370State, dev, dev);
+
+ if (size == 1) {
+ es1370_writeb (s, addr, value);
+ } else if (size == 2) {
+ es1370_writew (s, addr, value);
+ } else {
+ es1370_writel (s, addr, value);
+ }
+}
+
IO_READ_PROTO (es1370_readb)
{
- ES1370State *s = opaque;
uint32_t val;
addr = es1370_fixup (s, addr);
@@ -650,7 +660,6 @@ IO_READ_PROTO (es1370_readb)
IO_READ_PROTO (es1370_readw)
{
- ES1370State *s = opaque;
struct chan *d = &s->chan[0];
uint32_t val;
@@ -692,7 +701,6 @@ IO_READ_PROTO (es1370_readw)
IO_READ_PROTO (es1370_readl)
{
- ES1370State *s = opaque;
uint32_t val;
struct chan *d = &s->chan[0];
@@ -775,6 +783,21 @@ IO_READ_PROTO (es1370_readl)
return val;
}
+static uint32_t es1370_read (PCIDevice *dev, pcibus_t addr, int size)
+{
+ ES1370State *s = DO_UPCAST(ES1370State, dev, dev);
+ uint32_t val;
+
+ if (size == 1) {
+ val = es1370_readb (s, addr);
+ } else if (size == 2) {
+ val = es1370_readw (s, addr);
+ } else {
+ val = es1370_readl (s, addr);
+ }
+
+ return val;
+}
static void es1370_transfer_audio (ES1370State *s, struct chan *d, int loop_sel,
int max, int *irq)
@@ -802,7 +825,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_memory_write (&s->dev, addr, tmpbuf, acquired);
temp -= acquired;
addr += acquired;
@@ -816,7 +839,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_memory_read (&s->dev, addr, tmpbuf, to_copy);
copied = AUD_write (voice, tmpbuf, to_copy);
if (!copied)
break;
@@ -906,24 +929,6 @@ static void es1370_adc_callback (void *opaque, int avail)
es1370_run_channel (s, ADC_CHANNEL, avail);
}
-static void es1370_map (PCIDevice *pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
-{
- ES1370State *s = DO_UPCAST (ES1370State, dev, pci_dev);
-
- (void) region_num;
- (void) size;
- (void) type;
-
- register_ioport_write (addr, 0x40 * 4, 1, es1370_writeb, s);
- register_ioport_write (addr, 0x40 * 2, 2, es1370_writew, s);
- register_ioport_write (addr, 0x40, 4, es1370_writel, s);
-
- register_ioport_read (addr, 0x40 * 4, 1, es1370_readb, s);
- register_ioport_read (addr, 0x40 * 2, 2, es1370_readw, s);
- register_ioport_read (addr, 0x40, 4, es1370_readl, s);
-}
-
static const VMStateDescription vmstate_es1370_channel = {
.name = "es1370_channel",
.version_id = 2,
@@ -1023,7 +1028,8 @@ static int es1370_initfn (PCIDevice *dev)
c[PCI_MIN_GNT] = 0x0c;
c[PCI_MAX_LAT] = 0x80;
- pci_register_bar (&s->dev, 0, 256, PCI_BASE_ADDRESS_SPACE_IO, es1370_map);
+ pci_register_io_region (&s->dev, 0, 256, PCI_BASE_ADDRESS_SPACE_IO,
+ es1370_read, es1370_write);
qemu_register_reset (es1370_on_reset, s);
AUD_register_card ("es1370", &s->card);
--
1.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 09/15] eepro100: convert to new pci interface
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
` (7 preceding siblings ...)
2010-02-09 22:01 ` [Qemu-devel] [PATCH 08/15] es1370: convert to new pci interface Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-10 6:32 ` Stefan Weil
2010-02-09 22:01 ` [Qemu-devel] [PATCH 10/15] virtio-pci: " Anthony Liguori
` (7 subsequent siblings)
16 siblings, 1 reply; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
- Removed some dead defines for TARGET_I386 so that we could build once
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/eepro100.c | 238 ++++++++++++++-------------------------------------------
1 files changed, 57 insertions(+), 181 deletions(-)
diff --git a/hw/eepro100.c b/hw/eepro100.c
index b33dbb8..16230c9 100644
--- a/hw/eepro100.c
+++ b/hw/eepro100.c
@@ -33,10 +33,6 @@
* Open Source Software Developer Manual
*/
-#if defined(TARGET_I386)
-# warning "PXE boot still not working!"
-#endif
-
#include <stddef.h> /* offsetof */
#include <stdbool.h>
#include "hw.h"
@@ -193,13 +189,10 @@ typedef enum {
typedef struct {
PCIDevice dev;
uint8_t mult[8]; /* multicast mask array */
- int mmio_index;
NICState *nic;
NICConf conf;
uint8_t scb_stat; /* SCB stat/ack byte */
uint8_t int_stat; /* PCI interrupt status */
- /* region must not be saved by nic_save. */
- uint32_t region[3]; /* PCI region addresses */
uint16_t mdimem[32];
eeprom_t *eeprom;
uint32_t device; /* device variant */
@@ -257,10 +250,10 @@ static const uint16_t eepro100_mdi_mask[] = {
};
/* XXX: optimize */
-static void stl_le_phys(target_phys_addr_t addr, uint32_t val)
+static void stl_le_phys(EEPRO100State *s, pcibus_t addr, uint32_t val)
{
val = cpu_to_le32(val);
- cpu_physical_memory_write(addr, (const uint8_t *)&val, sizeof(val));
+ pci_memory_write(&s->dev, addr, &val, sizeof(val));
}
#define POLYNOMIAL 0x04c11db6
@@ -434,10 +427,6 @@ static void pci_reset(EEPRO100State * s)
PCI_CONFIG_8(PCI_LATENCY_TIMER, 0x20); // latency timer = 32 clocks
/* PCI Header Type */
/* BIST (built-in self test) */
-#if defined(TARGET_I386)
-// !!! workaround for buggy bios
-//~ #define PCI_BASE_ADDRESS_MEM_PREFETCH 0
-#endif
#if 0
/* PCI Base Address Registers */
/* CSR Memory Mapped Base Address */
@@ -756,12 +745,12 @@ static void dump_statistics(EEPRO100State * s)
* values which really matter.
* Number of data should check configuration!!!
*/
- cpu_physical_memory_write(s->statsaddr,
- (uint8_t *) & s->statistics, s->stats_size);
- stl_le_phys(s->statsaddr + 0, s->statistics.tx_good_frames);
- stl_le_phys(s->statsaddr + 36, s->statistics.rx_good_frames);
- stl_le_phys(s->statsaddr + 48, s->statistics.rx_resource_errors);
- stl_le_phys(s->statsaddr + 60, s->statistics.rx_short_frame_errors);
+ pci_memory_write(&s->dev, s->statsaddr,
+ &s->statistics, s->stats_size);
+ stl_le_phys(s, s->statsaddr + 0, s->statistics.tx_good_frames);
+ stl_le_phys(s, s->statsaddr + 36, s->statistics.rx_good_frames);
+ stl_le_phys(s, s->statsaddr + 48, s->statistics.rx_resource_errors);
+ stl_le_phys(s, s->statsaddr + 60, s->statistics.rx_short_frame_errors);
//~ stw_le_phys(s->statsaddr + 76, s->statistics.xmt_tco_frames);
//~ stw_le_phys(s->statsaddr + 78, s->statistics.rcv_tco_frames);
//~ missing("CU dump statistical counters");
@@ -797,8 +786,8 @@ static void tx_command(EEPRO100State *s)
("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_memory_read(&s->dev, tx_buffer_address, &buf[size],
+ tx_buffer_size);
size += tx_buffer_size;
}
if (tbd_array == 0xffffffff) {
@@ -817,8 +806,8 @@ static void tx_command(EEPRO100State *s)
("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_memory_read(&s->dev, tx_buffer_address, &buf[size],
+ tx_buffer_size);
size += tx_buffer_size;
if (tx_buffer_el & 1) {
break;
@@ -835,8 +824,8 @@ static void tx_command(EEPRO100State *s)
("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_memory_read(&s->dev, tx_buffer_address, &buf[size],
+ tx_buffer_size);
size += tx_buffer_size;
if (tx_buffer_el & 1) {
break;
@@ -859,7 +848,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_memory_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);
@@ -871,7 +860,7 @@ static void action_command(EEPRO100State *s)
{
for (;;) {
s->cb_address = s->cu_base + s->cu_offset;
- cpu_physical_memory_read(s->cb_address, (uint8_t *)&s->tx, sizeof(s->tx));
+ pci_memory_read(&s->dev, s->cb_address, &s->tx, sizeof(s->tx));
uint16_t status = le16_to_cpu(s->tx.status);
uint16_t command = le16_to_cpu(s->tx.command);
logout
@@ -890,12 +879,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_memory_read(&s->dev, s->cb_address + 8, &s->conf.macaddr.a[0], 6);
TRACE(OTHER, logout("macaddr: %s\n", nic_dump(&s->macaddr[0], 6)));
break;
case CmdConfigure:
- cpu_physical_memory_read(s->cb_address + 8, &s->configuration[0],
- sizeof(s->configuration));
+ pci_memory_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)));
break;
case CmdMulticastList:
@@ -985,7 +974,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);
- stl_le_phys(s->statsaddr + s->stats_size, 0xa005);
+ stl_le_phys(s, s->statsaddr + s->stats_size, 0xa005);
break;
case CU_CMD_BASE:
/* Load CU base. */
@@ -996,7 +985,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);
- stl_le_phys(s->statsaddr + s->stats_size, 0xa007);
+ stl_le_phys(s, s->statsaddr + s->stats_size, 0xa007);
memset(&s->statistics, 0, sizeof(s->statistics));
break;
case CU_SRESUME:
@@ -1282,10 +1271,10 @@ static void eepro100_write_port(EEPRO100State * s, uint32_t val)
case PORT_SELFTEST:
TRACE(OTHER, logout("selftest address=0x%08x\n", address));
eepro100_selftest_t data;
- cpu_physical_memory_read(address, (uint8_t *) & data, sizeof(data));
+ pci_memory_read(&s->dev, address, & data, sizeof(data));
data.st_sign = 0xffffffff;
data.st_result = 0;
- cpu_physical_memory_write(address, (uint8_t *) & data, sizeof(data));
+ pci_memory_write(&s->dev, address, & data, sizeof(data));
break;
case PORT_SELECTIVE_RESET:
TRACE(OTHER, logout("selective reset, selftest address=0x%08x\n", address));
@@ -1491,147 +1480,37 @@ static void eepro100_write4(EEPRO100State * s, uint32_t addr, uint32_t val)
}
}
-/*****************************************************************************
- *
- * Port mapped I/O.
- *
- ****************************************************************************/
-
-static uint32_t ioport_read1(void *opaque, uint32_t addr)
-{
- EEPRO100State *s = opaque;
- //~ logout("addr=%s\n", regname(addr));
- return eepro100_read1(s, addr - s->region[1]);
-}
-
-static uint32_t ioport_read2(void *opaque, uint32_t addr)
-{
- EEPRO100State *s = opaque;
- return eepro100_read2(s, addr - s->region[1]);
-}
-
-static uint32_t ioport_read4(void *opaque, uint32_t addr)
-{
- EEPRO100State *s = opaque;
- return eepro100_read4(s, addr - s->region[1]);
-}
-
-static void ioport_write1(void *opaque, uint32_t addr, uint32_t val)
-{
- EEPRO100State *s = opaque;
- //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
- eepro100_write1(s, addr - s->region[1], val);
-}
-
-static void ioport_write2(void *opaque, uint32_t addr, uint32_t val)
-{
- EEPRO100State *s = opaque;
- eepro100_write2(s, addr - s->region[1], val);
-}
-
-static void ioport_write4(void *opaque, uint32_t addr, uint32_t val)
-{
- EEPRO100State *s = opaque;
- eepro100_write4(s, addr - s->region[1], val);
-}
-
/***********************************************************/
/* PCI EEPRO100 definitions */
-static void pci_map(PCIDevice * pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
+static void pci_io_write(PCIDevice *dev, pcibus_t addr, int size,
+ uint32_t value)
{
- EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
-
- TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", "
- "size=0x%08"FMT_PCIBUS", type=%d\n",
- region_num, addr, size, type));
+ EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, dev);
- assert(region_num == 1);
- register_ioport_write(addr, size, 1, ioport_write1, s);
- register_ioport_read(addr, size, 1, ioport_read1, s);
- register_ioport_write(addr, size, 2, ioport_write2, s);
- register_ioport_read(addr, size, 2, ioport_read2, s);
- register_ioport_write(addr, size, 4, ioport_write4, s);
- register_ioport_read(addr, size, 4, ioport_read4, s);
-
- s->region[region_num] = addr;
-}
-
-/*****************************************************************************
- *
- * Memory mapped I/O.
- *
- ****************************************************************************/
-
-static void pci_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
- EEPRO100State *s = opaque;
- //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
- eepro100_write1(s, addr, val);
-}
-
-static void pci_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
- EEPRO100State *s = opaque;
- //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
- eepro100_write2(s, addr, val);
-}
-
-static void pci_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
-{
- EEPRO100State *s = opaque;
- //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
- eepro100_write4(s, addr, val);
-}
-
-static uint32_t pci_mmio_readb(void *opaque, target_phys_addr_t addr)
-{
- EEPRO100State *s = opaque;
- //~ logout("addr=%s\n", regname(addr));
- return eepro100_read1(s, addr);
-}
-
-static uint32_t pci_mmio_readw(void *opaque, target_phys_addr_t addr)
-{
- EEPRO100State *s = opaque;
- //~ logout("addr=%s\n", regname(addr));
- return eepro100_read2(s, addr);
-}
-
-static uint32_t pci_mmio_readl(void *opaque, target_phys_addr_t addr)
-{
- EEPRO100State *s = opaque;
- //~ logout("addr=%s\n", regname(addr));
- return eepro100_read4(s, addr);
+ if (size == 1) {
+ eepro100_write1(s, addr, value);
+ } else if (size == 2) {
+ eepro100_write2(s, addr, value);
+ } else {
+ eepro100_write4(s, addr, value);
+ }
}
-static CPUWriteMemoryFunc * const pci_mmio_write[] = {
- pci_mmio_writeb,
- pci_mmio_writew,
- pci_mmio_writel
-};
-
-static CPUReadMemoryFunc * const pci_mmio_read[] = {
- pci_mmio_readb,
- pci_mmio_readw,
- pci_mmio_readl
-};
-
-static void pci_mmio_map(PCIDevice * pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
+static uint32_t pci_io_read(PCIDevice *dev, pcibus_t addr, int size)
{
- EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
+ EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, dev);
+ uint32_t value;
- TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", "
- "size=0x%08"FMT_PCIBUS", type=%d\n",
- region_num, addr, size, type));
-
- if (region_num == 0) {
- /* Map control / status registers. */
- cpu_register_physical_memory(addr, size, s->mmio_index);
- s->region[region_num] = addr;
+ if (size == 1) {
+ value = eepro100_read1(s, addr);
+ } else if (size == 2) {
+ value = eepro100_read2(s, addr);
+ } else {
+ value = eepro100_read4(s, addr);
}
+
+ return value;
}
static int nic_can_receive(VLANClientState *nc)
@@ -1722,8 +1601,8 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
//~ !!!
//~ $3 = {status = 0x0, command = 0xc000, link = 0x2d220, rx_buf_addr = 0x207dc, count = 0x0, size = 0x5f8, packet = {0x0 <repeats 1518 times>}}
eepro100_rx_t rx;
- cpu_physical_memory_read(s->ru_base + s->ru_offset, (uint8_t *) & rx,
- offsetof(eepro100_rx_t, packet));
+ pci_memory_read(&s->dev, s->ru_base + s->ru_offset, & rx,
+ offsetof(eepro100_rx_t, packet));
uint16_t rfd_command = le16_to_cpu(rx.command);
uint16_t rfd_size = le16_to_cpu(rx.size);
@@ -1749,8 +1628,8 @@ static ssize_t nic_receive(VLANClientState *nc, const uint8_t * buf, size_t size
}
/* TODO: check stripping enable bit. */
//~ assert(!(s->configuration[17] & 1));
- cpu_physical_memory_write(s->ru_base + s->ru_offset +
- offsetof(eepro100_rx_t, packet), buf, size);
+ pci_memory_write(&s->dev, s->ru_base + s->ru_offset +
+ offsetof(eepro100_rx_t, packet), buf, size);
s->statistics.rx_good_frames++;
eepro100_fr_interrupt(s);
s->ru_offset = le32_to_cpu(rx.link);
@@ -1833,7 +1712,6 @@ static int pci_nic_uninit(PCIDevice *pci_dev)
{
EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
- cpu_unregister_io_memory(s->mmio_index);
vmstate_unregister(s->vmstate, s);
eeprom93xx_free(s->eeprom);
qemu_del_vlan_client(&s->nic->nc);
@@ -1862,21 +1740,19 @@ static int nic_init(PCIDevice *pci_dev, uint32_t device)
* i82559 and later support 64 or 256 word EEPROM. */
s->eeprom = eeprom93xx_new(EEPROM_SIZE);
- /* Handler for memory-mapped I/O */
- s->mmio_index =
- cpu_register_io_memory(pci_mmio_read, pci_mmio_write, s);
-
- pci_register_bar(&s->dev, 0, PCI_MEM_SIZE,
+ pci_register_io_region(&s->dev, 0, PCI_MEM_SIZE,
PCI_BASE_ADDRESS_SPACE_MEMORY |
- PCI_BASE_ADDRESS_MEM_PREFETCH, pci_mmio_map);
- pci_register_bar(&s->dev, 1, PCI_IO_SIZE, PCI_BASE_ADDRESS_SPACE_IO,
- pci_map);
- pci_register_bar(&s->dev, 2, PCI_FLASH_SIZE, PCI_BASE_ADDRESS_SPACE_MEMORY,
- pci_mmio_map);
+ PCI_BASE_ADDRESS_MEM_PREFETCH,
+ pci_io_read, pci_io_write);
+ pci_register_io_region(&s->dev, 1, PCI_IO_SIZE,
+ PCI_BASE_ADDRESS_SPACE_IO,
+ pci_io_read, pci_io_write);
+ pci_register_io_region(&s->dev, 2, PCI_FLASH_SIZE,
+ PCI_BASE_ADDRESS_SPACE_MEMORY,
+ NULL, NULL);
qemu_macaddr_default_if_unset(&s->conf.macaddr);
logout("macaddr: %s\n", nic_dump(&s->macaddr[0], 6));
- assert(s->region[1] == 0);
nic_reset(s);
--
1.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 10/15] virtio-pci: convert to new pci interface
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
` (8 preceding siblings ...)
2010-02-09 22:01 ` [Qemu-devel] [PATCH 09/15] eepro100: " Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 11/15] pci: add pci_register_msix_region Anthony Liguori
` (6 subsequent siblings)
16 siblings, 0 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
- in each config_readN function, we already do get_config so drop it in map
- virtio still has knowledge of target_phys_addr_t, syborg makes it hard to
remove
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/virtio-pci.c | 110 ++++++++++++++-----------------------------------------
1 files changed, 28 insertions(+), 82 deletions(-)
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index 709d13e..a2ba294 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -169,9 +169,9 @@ static void virtio_pci_reset(DeviceState *d)
msix_reset(&proxy->pci_dev);
}
-static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
+static void virtio_ioport_write(VirtIOPCIProxy *proxy, uint32_t addr,
+ uint32_t val)
{
- VirtIOPCIProxy *proxy = opaque;
VirtIODevice *vdev = proxy->vdev;
target_phys_addr_t pa;
@@ -189,6 +189,8 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
vdev->guest_features = val;
break;
case VIRTIO_PCI_QUEUE_PFN:
+ /* FIXME need to rethink how we handle this. We should use PCI memory
+ * access functions for everything here. Syborg complicates that. */
pa = (target_phys_addr_t)val << VIRTIO_PCI_QUEUE_ADDR_SHIFT;
if (pa == 0) {
virtio_reset(proxy->vdev);
@@ -277,96 +279,40 @@ static uint32_t virtio_ioport_read(VirtIOPCIProxy *proxy, uint32_t addr)
return ret;
}
-static uint32_t virtio_pci_config_readb(void *opaque, uint32_t addr)
+static uint32_t virtio_pci_config_read(PCIDevice *dev, pcibus_t addr, int size)
{
- VirtIOPCIProxy *proxy = opaque;
+ VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, dev);
uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
- addr -= proxy->addr;
- if (addr < config)
- return virtio_ioport_read(proxy, addr);
- addr -= config;
- return virtio_config_readb(proxy->vdev, addr);
-}
+ uint32_t value;
-static uint32_t virtio_pci_config_readw(void *opaque, uint32_t addr)
-{
- VirtIOPCIProxy *proxy = opaque;
- uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
- addr -= proxy->addr;
- if (addr < config)
- return virtio_ioport_read(proxy, addr);
- addr -= config;
- return virtio_config_readw(proxy->vdev, addr);
-}
-
-static uint32_t virtio_pci_config_readl(void *opaque, uint32_t addr)
-{
- VirtIOPCIProxy *proxy = opaque;
- uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
- addr -= proxy->addr;
- if (addr < config)
- return virtio_ioport_read(proxy, addr);
- addr -= config;
- return virtio_config_readl(proxy->vdev, addr);
-}
-
-static void virtio_pci_config_writeb(void *opaque, uint32_t addr, uint32_t val)
-{
- VirtIOPCIProxy *proxy = opaque;
- uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
- addr -= proxy->addr;
if (addr < config) {
- virtio_ioport_write(proxy, addr, val);
- return;
+ value = virtio_ioport_read(proxy, addr);
+ } else if (size == 1) {
+ value = virtio_config_readb(proxy->vdev, addr - config);
+ } else if (size == 2) {
+ value = virtio_config_readw(proxy->vdev, addr - config);
+ } else {
+ value = virtio_config_readl(proxy->vdev, addr - config);
}
- addr -= config;
- virtio_config_writeb(proxy->vdev, addr, val);
-}
-static void virtio_pci_config_writew(void *opaque, uint32_t addr, uint32_t val)
-{
- VirtIOPCIProxy *proxy = opaque;
- uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
- addr -= proxy->addr;
- if (addr < config) {
- virtio_ioport_write(proxy, addr, val);
- return;
- }
- addr -= config;
- virtio_config_writew(proxy->vdev, addr, val);
+ return value;
}
-static void virtio_pci_config_writel(void *opaque, uint32_t addr, uint32_t val)
+static void virtio_pci_config_write(PCIDevice *dev, pcibus_t addr, int size,
+ uint32_t value)
{
- VirtIOPCIProxy *proxy = opaque;
+ VirtIOPCIProxy *proxy = DO_UPCAST(VirtIOPCIProxy, pci_dev, dev);
uint32_t config = VIRTIO_PCI_CONFIG(&proxy->pci_dev);
- addr -= proxy->addr;
+
if (addr < config) {
- virtio_ioport_write(proxy, addr, val);
- return;
+ virtio_ioport_write(proxy, addr, value);
+ } else if (size == 1) {
+ virtio_config_writeb(proxy->vdev, addr - config, value);
+ } else if (size == 2) {
+ virtio_config_writew(proxy->vdev, addr - config, value);
+ } else {
+ virtio_config_writel(proxy->vdev, addr - config, value);
}
- addr -= config;
- virtio_config_writel(proxy->vdev, addr, val);
-}
-
-static void virtio_map(PCIDevice *pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
-{
- VirtIOPCIProxy *proxy = container_of(pci_dev, VirtIOPCIProxy, pci_dev);
- VirtIODevice *vdev = proxy->vdev;
- unsigned config_len = VIRTIO_PCI_REGION_SIZE(pci_dev) + vdev->config_len;
-
- proxy->addr = addr;
-
- register_ioport_write(addr, config_len, 1, virtio_pci_config_writeb, proxy);
- register_ioport_write(addr, config_len, 2, virtio_pci_config_writew, proxy);
- register_ioport_write(addr, config_len, 4, virtio_pci_config_writel, proxy);
- register_ioport_read(addr, config_len, 1, virtio_pci_config_readb, proxy);
- register_ioport_read(addr, config_len, 2, virtio_pci_config_readw, proxy);
- register_ioport_read(addr, config_len, 4, virtio_pci_config_readl, proxy);
-
- if (vdev->config_len)
- vdev->get_config(vdev, vdev->config);
}
static void virtio_write_config(PCIDevice *pci_dev, uint32_t address,
@@ -439,8 +385,8 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
if (size & (size-1))
size = 1 << qemu_fls(size);
- pci_register_bar(&proxy->pci_dev, 0, size, PCI_BASE_ADDRESS_SPACE_IO,
- virtio_map);
+ pci_register_io_region(&proxy->pci_dev, 0, size, PCI_BASE_ADDRESS_SPACE_IO,
+ virtio_pci_config_read, virtio_pci_config_write);
virtio_bind_device(vdev, &virtio_pci_bindings, proxy);
proxy->host_features |= 0x1 << VIRTIO_F_NOTIFY_ON_EMPTY;
--
1.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 11/15] pci: add pci_register_msix_region
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
` (9 preceding siblings ...)
2010-02-09 22:01 ` [Qemu-devel] [PATCH 10/15] virtio-pci: " Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 12/15] ne2000: convert to new pci interface Anthony Liguori
` (5 subsequent siblings)
16 siblings, 0 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
And make virtio use it.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/pci.c | 8 ++++++++
hw/pci.h | 2 ++
hw/virtio-pci.c | 5 +----
3 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/hw/pci.c b/hw/pci.c
index 5460f27..50ae917 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -28,6 +28,7 @@
#include "sysemu.h"
#include "loader.h"
#include "qemu-objects.h"
+#include "msix.h"
//#define DEBUG_PCI
#ifdef DEBUG_PCI
@@ -823,6 +824,13 @@ void pci_register_io_region(PCIDevice *d, int region_num,
}
}
+void pci_register_msix_region(PCIDevice *d, int region_num)
+{
+ pci_register_bar(d, region_num, msix_bar_size(d),
+ PCI_BASE_ADDRESS_SPACE_MEMORY,
+ msix_mmio_map);
+}
+
static uint32_t pci_config_get_io_base(PCIDevice *d,
uint32_t base, uint32_t base_upper16)
{
diff --git a/hw/pci.h b/hw/pci.h
index 3edf28f..9645fce 100644
--- a/hw/pci.h
+++ b/hw/pci.h
@@ -202,6 +202,8 @@ void pci_register_io_region(PCIDevice *d, int region_num,
pcibus_t size, int type,
PCIIOReadFunc *readcb, PCIIOWriteFunc *writecb);
+void pci_register_msix_region(PCIDevice *d, int region_num);
+
void pci_memory_read(PCIDevice *pci_dev, pcibus_t addr, void *buf, int len);
void pci_memory_write(PCIDevice *pci_dev, pcibus_t addr,
const void *buf, int len);
diff --git a/hw/virtio-pci.c b/hw/virtio-pci.c
index a2ba294..81c4d37 100644
--- a/hw/virtio-pci.c
+++ b/hw/virtio-pci.c
@@ -372,10 +372,7 @@ static void virtio_init_pci(VirtIOPCIProxy *proxy, VirtIODevice *vdev,
config[0x3d] = 1;
if (vdev->nvectors && !msix_init(&proxy->pci_dev, vdev->nvectors, 1, 0)) {
- pci_register_bar(&proxy->pci_dev, 1,
- msix_bar_size(&proxy->pci_dev),
- PCI_BASE_ADDRESS_SPACE_MEMORY,
- msix_mmio_map);
+ pci_register_msix_region(&proxy->pci_dev, 1);
} else
vdev->nvectors = 0;
--
1.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 12/15] ne2000: convert to new pci interface
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
` (10 preceding siblings ...)
2010-02-09 22:01 ` [Qemu-devel] [PATCH 11/15] pci: add pci_register_msix_region Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 13/15] pcnet: " Anthony Liguori
` (4 subsequent siblings)
16 siblings, 0 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/ne2000.c | 50 +++++++++++++++++++++++++++++++++++---------------
1 files changed, 35 insertions(+), 15 deletions(-)
diff --git a/hw/ne2000.c b/hw/ne2000.c
index 78fe14f..6952680 100644
--- a/hw/ne2000.c
+++ b/hw/ne2000.c
@@ -678,24 +678,44 @@ static const VMStateDescription vmstate_pci_ne2000 = {
/***********************************************************/
/* PCI NE2000 definitions */
-static void ne2000_map(PCIDevice *pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
+static uint32_t ne2000_read(PCIDevice *dev, pcibus_t addr, int size)
{
- PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, pci_dev);
+ PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, dev);
NE2000State *s = &d->ne2000;
+ uint32_t value;
- register_ioport_write(addr, 16, 1, ne2000_ioport_write, s);
- register_ioport_read(addr, 16, 1, ne2000_ioport_read, s);
+ if (addr < 0x10) {
+ value = ne2000_ioport_read(s, addr);
+ } else if (addr < 0x1f) {
+ if (size == 4) {
+ value = ne2000_asic_ioport_readl(s, addr);
+ } else {
+ value = ne2000_asic_ioport_read(s, addr);
+ }
+ } else {
+ value = ne2000_reset_ioport_read(s, addr);
+ }
- register_ioport_write(addr + 0x10, 1, 1, ne2000_asic_ioport_write, s);
- register_ioport_read(addr + 0x10, 1, 1, ne2000_asic_ioport_read, s);
- register_ioport_write(addr + 0x10, 2, 2, ne2000_asic_ioport_write, s);
- register_ioport_read(addr + 0x10, 2, 2, ne2000_asic_ioport_read, s);
- register_ioport_write(addr + 0x10, 4, 4, ne2000_asic_ioport_writel, s);
- register_ioport_read(addr + 0x10, 4, 4, ne2000_asic_ioport_readl, s);
+ return value;
+}
- register_ioport_write(addr + 0x1f, 1, 1, ne2000_reset_ioport_write, s);
- register_ioport_read(addr + 0x1f, 1, 1, ne2000_reset_ioport_read, s);
+static void ne2000_write(PCIDevice *dev, pcibus_t addr, int size,
+ uint32_t value)
+{
+ PCINE2000State *d = DO_UPCAST(PCINE2000State, dev, dev);
+ NE2000State *s = &d->ne2000;
+
+ if (addr < 0x10) {
+ ne2000_ioport_write(s, addr, value);
+ } else if (addr < 0x1f) {
+ if (size == 4) {
+ ne2000_asic_ioport_writel(s, addr, value);
+ } else {
+ ne2000_asic_ioport_write(s, addr, value);
+ }
+ } else {
+ ne2000_reset_ioport_write(s, addr, value);
+ }
}
static void ne2000_cleanup(VLANClientState *nc)
@@ -727,8 +747,8 @@ static int pci_ne2000_init(PCIDevice *pci_dev)
/* TODO: RST# value should be 0. PCI spec 6.2.4 */
pci_conf[PCI_INTERRUPT_PIN] = 1; // interrupt pin 0
- pci_register_bar(&d->dev, 0, 0x100,
- PCI_BASE_ADDRESS_SPACE_IO, ne2000_map);
+ pci_register_io_region(&d->dev, 0, 0x100, PCI_BASE_ADDRESS_SPACE_IO,
+ ne2000_read, ne2000_write);
s = &d->ne2000;
s->irq = d->dev.irq[0];
--
1.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 13/15] pcnet: convert to new pci interface
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
` (11 preceding siblings ...)
2010-02-09 22:01 ` [Qemu-devel] [PATCH 12/15] ne2000: convert to new pci interface Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 14/15] usb-uhci: " Anthony Liguori
` (3 subsequent siblings)
16 siblings, 0 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
- this is not a clean conversion because of lance. we need a more robust way
to deal with a single chip being used for multiple devices
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/pcnet.c | 103 +++++++++++++++++++++++++++++++++++------------------------
1 files changed, 61 insertions(+), 42 deletions(-)
diff --git a/hw/pcnet.c b/hw/pcnet.c
index 44b5b31..36ca420 100644
--- a/hw/pcnet.c
+++ b/hw/pcnet.c
@@ -1726,23 +1726,38 @@ static uint32_t pcnet_ioport_readl(void *opaque, uint32_t addr)
return val;
}
-static void pcnet_ioport_map(PCIDevice *pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
+static uint32_t pcnet_ioport_read(PCIDevice *pci_dev, pcibus_t addr, int size)
{
PCNetState *d = &DO_UPCAST(PCIPCNetState, pci_dev, pci_dev)->state;
+ uint32_t value = ~0U;
-#ifdef PCNET_DEBUG_IO
- printf("pcnet_ioport_map addr=0x%04"FMT_PCIBUS" size=0x%04"FMT_PCIBUS"\n",
- addr, size);
-#endif
+ if (addr < 0x10) {
+ if (size == 1) {
+ value = pcnet_aprom_readb(d, addr);
+ }
+ } else if (size == 2) {
+ value = pcnet_ioport_readw(d, addr);
+ } else if (size == 4) {
+ value = pcnet_ioport_readl(d, addr);
+ }
- register_ioport_write(addr, 16, 1, pcnet_aprom_writeb, d);
- register_ioport_read(addr, 16, 1, pcnet_aprom_readb, d);
+ return value;
+}
- register_ioport_write(addr + 0x10, 0x10, 2, pcnet_ioport_writew, d);
- register_ioport_read(addr + 0x10, 0x10, 2, pcnet_ioport_readw, d);
- register_ioport_write(addr + 0x10, 0x10, 4, pcnet_ioport_writel, d);
- register_ioport_read(addr + 0x10, 0x10, 4, pcnet_ioport_readl, d);
+static void pcnet_ioport_write(PCIDevice *pci_dev, pcibus_t addr, int size,
+ uint32_t value)
+{
+ PCNetState *d = &DO_UPCAST(PCIPCNetState, pci_dev, pci_dev)->state;
+
+ if (addr < 0x10) {
+ if (size == 1) {
+ pcnet_aprom_writeb(d, addr, value);
+ }
+ } else if (size == 2) {
+ pcnet_ioport_writew(d, addr, value);
+ } else if (size == 4) {
+ pcnet_ioport_writel(d, addr, value);
+ }
}
static void pcnet_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
@@ -1903,41 +1918,48 @@ int pcnet_common_init(DeviceState *dev, PCNetState *s, NetClientInfo *info)
/* PCI interface */
-static CPUWriteMemoryFunc * const pcnet_mmio_write[] = {
- &pcnet_mmio_writeb,
- &pcnet_mmio_writew,
- &pcnet_mmio_writel
-};
+static void pcnet_mmio_write(PCIDevice *dev, pcibus_t addr, int size,
+ uint32_t value)
+{
+ PCNetState *d = &DO_UPCAST(PCIPCNetState, pci_dev, dev)->state;
-static CPUReadMemoryFunc * const pcnet_mmio_read[] = {
- &pcnet_mmio_readb,
- &pcnet_mmio_readw,
- &pcnet_mmio_readl
-};
+ if (size == 1) {
+ pcnet_mmio_writeb(d, addr, value);
+ } else if (size == 2) {
+ pcnet_mmio_writew(d, addr, value);
+ } else {
+ pcnet_mmio_writel(d, addr, value);
+ }
+}
-static void pcnet_mmio_map(PCIDevice *pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
+static uint32_t pcnet_mmio_read(PCIDevice *dev, pcibus_t addr, int size)
{
- PCIPCNetState *d = DO_UPCAST(PCIPCNetState, pci_dev, pci_dev);
+ PCNetState *d = &DO_UPCAST(PCIPCNetState, pci_dev, dev)->state;
+ uint32_t value;
-#ifdef PCNET_DEBUG_IO
- printf("pcnet_mmio_map addr=0x%08"FMT_PCIBUS" 0x%08"FMT_PCIBUS"\n",
- addr, size);
-#endif
+ if (size == 1) {
+ value = pcnet_mmio_readb(d, addr);
+ } else if (size == 2) {
+ value = pcnet_mmio_readw(d, addr);
+ } else {
+ value = pcnet_mmio_readl(d, addr);
+ }
- cpu_register_physical_memory(addr, PCNET_PNPMMIO_SIZE, d->state.mmio_index);
+ return value;
}
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);
+ PCIDevice *dev = dma_opaque;
+ pci_memory_write(dev, 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);
+ PCIDevice *dev = dma_opaque;
+ pci_memory_read(dev, addr, buf, len);
}
static void pci_pcnet_cleanup(VLANClientState *nc)
@@ -1951,7 +1973,6 @@ static int pci_pcnet_uninit(PCIDevice *dev)
{
PCIPCNetState *d = DO_UPCAST(PCIPCNetState, pci_dev, dev);
- cpu_unregister_io_memory(d->state.mmio_index);
qemu_del_timer(d->state.poll_timer);
qemu_free_timer(d->state.poll_timer);
qemu_del_vlan_client(&d->state.nic->nc);
@@ -2002,20 +2023,18 @@ static int pci_pcnet_init(PCIDevice *pci_dev)
pci_conf[PCI_MIN_GNT] = 0x06;
pci_conf[PCI_MAX_LAT] = 0xff;
- /* Handler for memory-mapped I/O */
- s->mmio_index =
- cpu_register_io_memory(pcnet_mmio_read, pcnet_mmio_write, &d->state);
-
- /* TODO: use pci_dev, avoid cast below. */
- pci_register_bar((PCIDevice *)d, 0, PCNET_IOPORT_SIZE,
- PCI_BASE_ADDRESS_SPACE_IO, pcnet_ioport_map);
+ pci_register_io_region(pci_dev, 0, PCNET_IOPORT_SIZE,
+ PCI_BASE_ADDRESS_SPACE_IO,
+ pcnet_ioport_read, pcnet_ioport_write);
- pci_register_bar((PCIDevice *)d, 1, PCNET_PNPMMIO_SIZE,
- PCI_BASE_ADDRESS_SPACE_MEMORY, pcnet_mmio_map);
+ pci_register_io_region(pci_dev, 1, PCNET_PNPMMIO_SIZE,
+ PCI_BASE_ADDRESS_SPACE_MEMORY,
+ pcnet_mmio_read, pcnet_mmio_write);
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.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 14/15] usb-uhci: convert to new pci interface
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
` (12 preceding siblings ...)
2010-02-09 22:01 ` [Qemu-devel] [PATCH 13/15] pcnet: " Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 15/15] pci: byte swap as PCI interface layer Anthony Liguori
` (2 subsequent siblings)
16 siblings, 0 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/usb-uhci.c | 41 +++++++++++++++++++++++++++++------------
1 files changed, 29 insertions(+), 12 deletions(-)
diff --git a/hw/usb-uhci.c b/hw/usb-uhci.c
index 434070e..0d049d8 100644
--- a/hw/usb-uhci.c
+++ b/hw/usb-uhci.c
@@ -1047,17 +1047,34 @@ static void uhci_frame_timer(void *opaque)
qemu_mod_timer(s->frame_timer, expire_time);
}
-static void uhci_map(PCIDevice *pci_dev, int region_num,
- pcibus_t addr, pcibus_t size, int type)
+static uint32_t uhci_ioport_read(PCIDevice *dev, pcibus_t addr, int size)
{
- UHCIState *s = (UHCIState *)pci_dev;
-
- register_ioport_write(addr, 32, 2, uhci_ioport_writew, s);
- register_ioport_read(addr, 32, 2, uhci_ioport_readw, s);
- register_ioport_write(addr, 32, 4, uhci_ioport_writel, s);
- register_ioport_read(addr, 32, 4, uhci_ioport_readl, s);
- register_ioport_write(addr, 32, 1, uhci_ioport_writeb, s);
- register_ioport_read(addr, 32, 1, uhci_ioport_readb, s);
+ UHCIState *s = DO_UPCAST(UHCIState, dev, dev);
+ uint32_t value;
+
+ if (size == 1) {
+ value = uhci_ioport_readb(s, addr);
+ } else if (size == 2) {
+ value = uhci_ioport_readw(s, addr);
+ } else {
+ value = uhci_ioport_readl(s, addr);
+ }
+
+ return value;
+}
+
+static void uhci_ioport_write(PCIDevice *dev, pcibus_t addr, int size,
+ uint32_t value)
+{
+ UHCIState *s = DO_UPCAST(UHCIState, dev, dev);
+
+ if (size == 1) {
+ uhci_ioport_writeb(s, addr, value);
+ } else if (size == 2) {
+ uhci_ioport_writew(s, addr, value);
+ } else {
+ uhci_ioport_writel(s, addr, value);
+ }
}
static int usb_uhci_common_initfn(UHCIState *s)
@@ -1084,8 +1101,8 @@ static int usb_uhci_common_initfn(UHCIState *s)
/* Use region 4 for consistency with real hardware. BSD guests seem
to rely on this. */
- pci_register_bar(&s->dev, 4, 0x20,
- PCI_BASE_ADDRESS_SPACE_IO, uhci_map);
+ pci_register_io_region(&s->dev, 4, 0x20, PCI_BASE_ADDRESS_SPACE_IO,
+ uhci_ioport_read, uhci_ioport_write);
return 0;
}
--
1.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* [Qemu-devel] [PATCH 15/15] pci: byte swap as PCI interface layer
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
` (13 preceding siblings ...)
2010-02-09 22:01 ` [Qemu-devel] [PATCH 14/15] usb-uhci: " Anthony Liguori
@ 2010-02-09 22:01 ` Anthony Liguori
2010-02-10 9:31 ` [Qemu-devel] Re: [PATCH 0/15][RFC] New PCI interfaces Michael S. Tsirkin
2010-02-10 18:34 ` [Qemu-devel] " Blue Swirl
16 siblings, 0 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Michael Tsirkin, Anthony Liguori, Alex Graf
PCI devices have a fixed byte order that is independent of the host byte order.
For the most part, PCI devices are little endian. Because we have allowed PCI
devices in the past to directly register memory functions with the CPU, these
devices have been subject to automagic swapping which means that they end up
returning target endianness instead of the fixed endianness that the device
supports.
This patch fixes this by allow devices to return native words in their interface
and allowing the PCI bus to do appropriate swapping before the data makes it
to the CPU.
Right now, most devices do this wrong so this should, in theory, fix a lot of
PCI devices on big endian targets.
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
---
hw/e1000.c | 9 +--------
hw/pci.c | 18 ++++++++++++++++--
hw/rtl8139.c | 44 +-------------------------------------------
3 files changed, 18 insertions(+), 53 deletions(-)
diff --git a/hw/e1000.c b/hw/e1000.c
index 823088c..0738e38 100644
--- a/hw/e1000.c
+++ b/hw/e1000.c
@@ -847,9 +847,6 @@ static void e1000_mmio_write(PCIDevice *dev, pcibus_t addr, int size,
E1000State *d = DO_UPCAST(E1000State, dev, dev);
unsigned int index = (addr & 0x1ffff) >> 2;
-#ifdef TARGET_WORDS_BIGENDIAN
- value = bswap32(value);
-#endif
value = (value & size_mask(size)) << addr_shift(addr);
if (index < NWRITEOPS && macreg_writeops[index])
@@ -873,11 +870,7 @@ static uint32_t e1000_mmio_read(PCIDevice *dev, pcibus_t addr, int size)
DBGOUT(UNKNOWN, "MMIO unknown read addr=0x%08x\n", index<<2);
}
- value = (value >> addr_shift(addr)) & size_mask(size);
-#ifdef TARGET_WORDS_BIGENDIAN
- value = bswap32(value);
-#endif
- return value;
+ return (value >> addr_shift(addr)) & size_mask(size);
}
static bool is_version_1(void *opaque, int version_id)
diff --git a/hw/pci.c b/hw/pci.c
index 50ae917..f1816f9 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -765,6 +765,9 @@ static void pci_io_region_writew(void *opaque, target_phys_addr_t addr,
uint32_t value)
{
PCIIORegion *r = opaque;
+#ifdef TARGET_WORDS_BIGENDIAN
+ value = bswap16(value);
+#endif
r->write(r->dev, addr, 2, value);
}
@@ -772,6 +775,9 @@ static void pci_io_region_writel(void *opaque, target_phys_addr_t addr,
uint32_t value)
{
PCIIORegion *r = opaque;
+#ifdef TARGET_WORDS_BIGENDIAN
+ value = bswap32(value);
+#endif
r->write(r->dev, addr, 4, value);
}
@@ -784,13 +790,21 @@ static uint32_t pci_io_region_readb(void *opaque, target_phys_addr_t addr)
static uint32_t pci_io_region_readw(void *opaque, target_phys_addr_t addr)
{
PCIIORegion *r = opaque;
- return r->read(r->dev, addr, 2);
+ uint32_t value = r->read(r->dev, addr, 2);
+#ifdef TARGET_WORDS_BIGENDIAN
+ value = bswap16(value);
+#endif
+ return value;
}
static uint32_t pci_io_region_readl(void *opaque, target_phys_addr_t addr)
{
PCIIORegion *r = opaque;
- return r->read(r->dev, addr, 4);
+ uint32_t value = r->read(r->dev, addr, 4);
+#ifdef TARGET_WORDS_BIGENDIAN
+ value = bswap32(value);
+#endif
+ return value;
}
static CPUReadMemoryFunc * const pci_io_region_readfn[3] = {
diff --git a/hw/rtl8139.c b/hw/rtl8139.c
index 95e7aeb..a812c5f 100644
--- a/hw/rtl8139.c
+++ b/hw/rtl8139.c
@@ -3107,48 +3107,6 @@ static const VMStateDescription vmstate_rtl8139 = {
/***********************************************************/
/* PCI RTL8139 definitions */
-static uint32_t rtl8139_mmio_read(PCIDevice *dev, pcibus_t addr, int size)
-{
- RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev);
- uint32_t value;
-
- if (size == 1) {
- value = rtl8139_io_readb(s, addr);
- } else if (size == 2) {
- value = rtl8139_io_readw(s, addr);
-#ifdef TARGET_WORDS_BIGENDIAN
- value = bswap16(value);
-#endif
- } else {
- value = rtl8139_io_readl(s, addr & 0xFF);
-#ifdef TARGET_WORDS_BIGENDIAN
- value = bswap32(value);
-#endif
- }
-
- return value;
-}
-
-static void rtl8139_mmio_write(PCIDevice *dev, pcibus_t addr, int size,
- uint32_t value)
-{
- RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev);
-
- if (size == 1) {
- rtl8139_io_writeb(s, addr, value);
- } else if (size == 2) {
-#ifdef TARGET_WORDS_BIGENDIAN
- value = bswap16(value);
-#endif
- rtl8139_io_writew(s, addr, value);
- } else if (size == 4) {
-#ifdef TARGET_WORDS_BIGENDIAN
- value = bswap32(value);
-#endif
- rtl8139_io_writel(s, addr, value);
- }
-}
-
static uint32_t rtl8139_io_read(PCIDevice *dev, pcibus_t addr, int size)
{
RTL8139State *s = DO_UPCAST(RTL8139State, dev, dev);
@@ -3286,7 +3244,7 @@ static int pci_rtl8139_init(PCIDevice *dev)
pci_register_io_region(&s->dev, 0, 0x100, PCI_BASE_ADDRESS_SPACE_IO,
rtl8139_io_read, rtl8139_io_write);
pci_register_io_region(&s->dev, 1, 0x100, PCI_BASE_ADDRESS_SPACE_MEMORY,
- rtl8139_mmio_read, rtl8139_mmio_write);
+ rtl8139_io_read, rtl8139_io_write);
qemu_macaddr_default_if_unset(&s->conf.macaddr);
--
1.6.5.2
^ permalink raw reply related [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API
2010-02-09 22:01 ` [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API Anthony Liguori
@ 2010-02-09 22:45 ` malc
2010-02-09 22:52 ` Anthony Liguori
0 siblings, 1 reply; 38+ messages in thread
From: malc @ 2010-02-09 22:45 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Michael Tsirkin, qemu-devel, Alex Graf
On Tue, 9 Feb 2010, Anthony Liguori wrote:
> - fixed bug with size of registered ioport regions
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
> hw/ac97.c | 146 +++++++++++++++++++++++++------------------------------------
> 1 files changed, 60 insertions(+), 86 deletions(-)
>
> diff --git a/hw/ac97.c b/hw/ac97.c
> index 4319bc8..9fdf591 100644
> --- a/hw/ac97.c
> +++ b/hw/ac97.c
> @@ -223,7 +223,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_memory_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]);
> @@ -569,50 +569,35 @@ static void mixer_reset (AC97LinkState *s)
>
> /**
> * Native audio mixer
> - * I/O Reads
> */
> -static uint32_t nam_readb (void *opaque, uint32_t addr)
> -{
> - AC97LinkState *s = opaque;
> - dolog ("U nam readb %#x\n", addr);
> - s->cas = 0;
> - return ~0U;
> -}
>
> -static uint32_t nam_readw (void *opaque, uint32_t addr)
> +static uint32_t nam_read (PCIDevice *dev, pcibus_t addr, int size)
> {
> - AC97LinkState *s = opaque;
> - uint32_t val = ~0U;
> - uint32_t index = addr - s->base[0];
> - s->cas = 0;
> - val = mixer_load (s, index);
> - return val;
> -}
> + AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, dev);
> + uint32_t value;
> +
> + if (size == 2) {
> + value = mixer_load (s, addr);
> + } else {
> + dolog ("U nam read[%d] %#" FMT_PCIBUS "\n", size, addr);
> + s->cas = 0;
> + value = ~0U;
> + }
>
Yeah right, and then PCI SIG adds qword accessors and all hell breaks
loose, this interface sucks..
[..snip..]
--
mailto:av1474@comtv.ru
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API
2010-02-09 22:45 ` malc
@ 2010-02-09 22:52 ` Anthony Liguori
2010-02-09 23:06 ` malc
0 siblings, 1 reply; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 22:52 UTC (permalink / raw)
To: malc; +Cc: Michael Tsirkin, qemu-devel, Alex Graf
On 02/09/2010 04:45 PM, malc wrote:
> On Tue, 9 Feb 2010, Anthony Liguori wrote:
>
>
>> - fixed bug with size of registered ioport regions
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> ---
>> hw/ac97.c | 146 +++++++++++++++++++++++++------------------------------------
>> 1 files changed, 60 insertions(+), 86 deletions(-)
>>
>> diff --git a/hw/ac97.c b/hw/ac97.c
>> index 4319bc8..9fdf591 100644
>> --- a/hw/ac97.c
>> +++ b/hw/ac97.c
>> @@ -223,7 +223,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_memory_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]);
>> @@ -569,50 +569,35 @@ static void mixer_reset (AC97LinkState *s)
>>
>> /**
>> * Native audio mixer
>> - * I/O Reads
>> */
>> -static uint32_t nam_readb (void *opaque, uint32_t addr)
>> -{
>> - AC97LinkState *s = opaque;
>> - dolog ("U nam readb %#x\n", addr);
>> - s->cas = 0;
>> - return ~0U;
>> -}
>>
>> -static uint32_t nam_readw (void *opaque, uint32_t addr)
>> +static uint32_t nam_read (PCIDevice *dev, pcibus_t addr, int size)
>> {
>> - AC97LinkState *s = opaque;
>> - uint32_t val = ~0U;
>> - uint32_t index = addr - s->base[0];
>> - s->cas = 0;
>> - val = mixer_load (s, index);
>> - return val;
>> -}
>> + AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, dev);
>> + uint32_t value;
>> +
>> + if (size == 2) {
>> + value = mixer_load (s, addr);
>> + } else {
>> + dolog ("U nam read[%d] %#" FMT_PCIBUS "\n", size, addr);
>> + s->cas = 0;
>> + value = ~0U;
>> + }
>>
>>
> Yeah right, and then PCI SIG adds qword accessors and all hell breaks
> loose, this interface sucks..
>
We already have this problem with the current interface.
The options to address would be to always return/accept a uint64_t or to
deal with a void *. The later interface has non-obvious byte order
semantics.
I avoided the former to avoid complaints about possibly introducing a
performance regression with passing larger data types. I don't believe
that it would be a problem though.
Regards,
Anthony Liguori
> [..snip..]
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API
2010-02-09 22:52 ` Anthony Liguori
@ 2010-02-09 23:06 ` malc
2010-02-09 23:10 ` Anthony Liguori
0 siblings, 1 reply; 38+ messages in thread
From: malc @ 2010-02-09 23:06 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Michael Tsirkin, qemu-devel, Alex Graf
On Tue, 9 Feb 2010, Anthony Liguori wrote:
> On 02/09/2010 04:45 PM, malc wrote:
> > On Tue, 9 Feb 2010, Anthony Liguori wrote:
> >
> >
> > > - fixed bug with size of registered ioport regions
> > >
> > > Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
> > > ---
> > > hw/ac97.c | 146
> > > +++++++++++++++++++++++++------------------------------------
> > > 1 files changed, 60 insertions(+), 86 deletions(-)
> > >
> > > diff --git a/hw/ac97.c b/hw/ac97.c
> > > index 4319bc8..9fdf591 100644
> > > --- a/hw/ac97.c
> > > +++ b/hw/ac97.c
> > > @@ -223,7 +223,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_memory_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]);
> > > @@ -569,50 +569,35 @@ static void mixer_reset (AC97LinkState *s)
> > >
> > > /**
> > > * Native audio mixer
> > > - * I/O Reads
> > > */
> > > -static uint32_t nam_readb (void *opaque, uint32_t addr)
> > > -{
> > > - AC97LinkState *s = opaque;
> > > - dolog ("U nam readb %#x\n", addr);
> > > - s->cas = 0;
> > > - return ~0U;
> > > -}
> > >
> > > -static uint32_t nam_readw (void *opaque, uint32_t addr)
> > > +static uint32_t nam_read (PCIDevice *dev, pcibus_t addr, int size)
> > > {
> > > - AC97LinkState *s = opaque;
> > > - uint32_t val = ~0U;
> > > - uint32_t index = addr - s->base[0];
> > > - s->cas = 0;
> > > - val = mixer_load (s, index);
> > > - return val;
> > > -}
> > > + AC97LinkState *s = DO_UPCAST (AC97LinkState, dev, dev);
> > > + uint32_t value;
> > > +
> > > + if (size == 2) {
> > > + value = mixer_load (s, addr);
> > > + } else {
> > > + dolog ("U nam read[%d] %#" FMT_PCIBUS "\n", size, addr);
> > > + s->cas = 0;
> > > + value = ~0U;
> > > + }
> > >
> > >
> > Yeah right, and then PCI SIG adds qword accessors and all hell breaks
> > loose, this interface sucks..
> >
>
> We already have this problem with the current interface.
Uh, i've meant the registration of one function to rule them all, instead
of how it's done currently - separate accessors for b/w/l/whatever.
>
> The options to address would be to always return/accept a uint64_t or to deal
> with a void *. The later interface has non-obvious byte order semantics.
>
> I avoided the former to avoid complaints about possibly introducing a
> performance regression with passing larger data types. I don't believe that
> it would be a problem though.
>
> Regards,
>
> Anthony Liguori
>
> > [..snip..]
> >
> >
--
mailto:av1474@comtv.ru
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API
2010-02-09 23:06 ` malc
@ 2010-02-09 23:10 ` Anthony Liguori
2010-02-09 23:36 ` malc
0 siblings, 1 reply; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 23:10 UTC (permalink / raw)
To: malc; +Cc: Michael Tsirkin, qemu-devel, Alex Graf
On 02/09/2010 05:06 PM, malc wrote:
>
>> We already have this problem with the current interface.
>>
> Uh, i've meant the registration of one function to rule them all, instead
> of how it's done currently - separate accessors for b/w/l/whatever.
>
How does that make any difference? Both the ioport and memory
registrations interface take the same function pointer regardless of
access size.
If you wanted to introduce a quad-word specific accessor, you would need
to introduce a different registration mechanism or you would have to
change the signature for all of the functions.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API
2010-02-09 23:10 ` Anthony Liguori
@ 2010-02-09 23:36 ` malc
2010-02-09 23:52 ` Anthony Liguori
0 siblings, 1 reply; 38+ messages in thread
From: malc @ 2010-02-09 23:36 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Michael Tsirkin, qemu-devel, Alex Graf
On Tue, 9 Feb 2010, Anthony Liguori wrote:
> On 02/09/2010 05:06 PM, malc wrote:
> >
> > > We already have this problem with the current interface.
> > >
> > Uh, i've meant the registration of one function to rule them all, instead
> > of how it's done currently - separate accessors for b/w/l/whatever.
> >
>
> How does that make any difference? Both the ioport and memory registrations
> interface take the same function pointer regardless of access size.
>
> If you wanted to introduce a quad-word specific accessor, you would need to
> introduce a different registration mechanism or you would have to change the
> signature for all of the functions.
Let's see:
Currently we have this
readb(...):
dostuff
return stuff
readw(...):
dostuff
return stuff
You are replacing it with
read(size...):
if (size == 1): do1
elif (size == 2): do2
else: # and here your code assumes that everything is handy dandy
# and size is 4
do4
The interface being implicit rather than explicit about the sizes
makes this possible, so i'm against it. The code was written the
way it was written for a purpose.
--
mailto:av1474@comtv.ru
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API
2010-02-09 23:36 ` malc
@ 2010-02-09 23:52 ` Anthony Liguori
2010-02-10 0:24 ` malc
0 siblings, 1 reply; 38+ messages in thread
From: Anthony Liguori @ 2010-02-09 23:52 UTC (permalink / raw)
To: malc; +Cc: Alex Graf, qemu-devel, Michael Tsirkin
On 02/09/2010 05:36 PM, malc wrote:
> Let's see:
>
> Currently we have this
>
>
> readb(...):
> dostuff
> return stuff
>
> readw(...):
> dostuff
> return stuff
>
And this is completely wrong.
For the most part, device models don't consistently handle access to
registers via their non native sizes. Some devices return the lower
part of a dword register when accessed with a word accessor. Some
devices only return the register when there's a dword access.
What's worse, is that if a device only registers a byte accessor,
because of the default accessors, a word access returns two registers
but a dword access returns ~0U. Some device models actually rely on
this behaviour.
> You are replacing it with
>
> read(size...):
> if (size == 1): do1
> elif (size == 2): do2
> else: # and here your code assumes that everything is handy dandy
> # and size is 4
> do4
>
This is ugly because it's a programmatic conversion. Once we have this
API, we can switch to:
read(addr, size...):
switch(addr):
case REG0:
return s->reg0;
case REG1:
return s->reg1;
Along with having a table like:
pci_regs = { {REG0, DWORD}, {REG1, BYTE}, {REG2, BYTE}, {REG3, WORD} }
That allows the PCI layer to invoke the callback and do the right
operations to transparently handle the access size without the device
model having to deal with it. The reason I've left size in the callback
at all is that I suspect there will always be a device that behaves
weirdly and needs to know about the accessor size. But for the most
part, I think this model will make things much more consistent and
eliminate a huge class of emulation bugs.
We can even take it further, and do something like:
pci_regs = {
PCI_REG_DWORD(REG0, DeviceState, reg0),
PCI_REG_BYTE(REG1, DeviceState, reg1),
...
}
In which case, we only need to handle the case in switch() if we need to
implement a side effect of the register operation.
But none of this is possible if we completely rely on every device to
implement non-dword access in it's own broken way.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API
2010-02-09 23:52 ` Anthony Liguori
@ 2010-02-10 0:24 ` malc
2010-02-11 14:20 ` Anthony Liguori
0 siblings, 1 reply; 38+ messages in thread
From: malc @ 2010-02-10 0:24 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Alex Graf, qemu-devel, Michael Tsirkin
On Tue, 9 Feb 2010, Anthony Liguori wrote:
> On 02/09/2010 05:36 PM, malc wrote:
> > Let's see:
> >
> > Currently we have this
> >
> >
> > readb(...):
> > dostuff
> > return stuff
> >
> > readw(...):
> > dostuff
> > return stuff
> >
>
> And this is completely wrong.
It's completely right, until some future C standard implements proper
algebraic data types.
>
> For the most part, device models don't consistently handle access to registers
> via their non native sizes. Some devices return the lower part of a dword
> register when accessed with a word accessor. Some devices only return the
> register when there's a dword access.
That's up to device to register an accessor and return what's appropriate.
>
> What's worse, is that if a device only registers a byte accessor, because of
> the default accessors, a word access returns two registers but a dword access
> returns ~0U. Some device models actually rely on this behaviour.
>
> > You are replacing it with
> >
> > read(size...):
> > if (size == 1): do1
> > elif (size == 2): do2
> > else: # and here your code assumes that everything is handy dandy
> > # and size is 4
> > do4
> >
>
> This is ugly because it's a programmatic conversion. Once we have this API,
> we can switch to:
>
> read(addr, size...):
> switch(addr):
> case REG0:
> return s->reg0;
> case REG1:
> return s->reg1;
This is exactly equivalent to your original if, due to the C language
being what it is.
> Along with having a table like:
>
> pci_regs = { {REG0, DWORD}, {REG1, BYTE}, {REG2, BYTE}, {REG3, WORD} }
>
> That allows the PCI layer to invoke the callback and do the right operations
> to transparently handle the access size without the device model having to
> deal with it. The reason I've left size in the callback at all is that I
> suspect there will always be a device that behaves weirdly and needs to know
> about the accessor size. But for the most part, I think this model will make
> things much more consistent and eliminate a huge class of emulation bugs.
Eh? Care to name one emulation bug?
>
> We can even take it further, and do something like:
>
> pci_regs = {
> PCI_REG_DWORD(REG0, DeviceState, reg0),
> PCI_REG_BYTE(REG1, DeviceState, reg1),
> ...
> }
>
> In which case, we only need to handle the case in switch() if we need to
> implement a side effect of the register operation.
>
> But none of this is possible if we completely rely on every device to
> implement non-dword access in it's own broken way.
>
Lovely, we are heading full speed into fanatsy land, where PCI BUS itself
accesses device specific stuff.
--
mailto:av1474@comtv.ru
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 09/15] eepro100: convert to new pci interface
2010-02-09 22:01 ` [Qemu-devel] [PATCH 09/15] eepro100: " Anthony Liguori
@ 2010-02-10 6:32 ` Stefan Weil
2010-02-10 9:49 ` [Qemu-devel] " Michael S. Tsirkin
2010-02-10 14:13 ` [Qemu-devel] " Anthony Liguori
0 siblings, 2 replies; 38+ messages in thread
From: Stefan Weil @ 2010-02-10 6:32 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Michael Tsirkin, qemu-devel
See my inline comments.
Anthony Liguori schrieb:
> - Removed some dead defines for TARGET_I386 so that we could build once
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
> hw/eepro100.c | 238 ++++++++++++++-------------------------------------------
> 1 files changed, 57 insertions(+), 181 deletions(-)
>
> diff --git a/hw/eepro100.c b/hw/eepro100.c
> index b33dbb8..16230c9 100644
> --- a/hw/eepro100.c
> +++ b/hw/eepro100.c
> @@ -33,10 +33,6 @@
> * Open Source Software Developer Manual
> */
>
> -#if defined(TARGET_I386)
> -# warning "PXE boot still not working!"
> -#endif
> -
>
You did not fix PXE boot here, did you?
So the warning or a comment should stay there.
> #include <stddef.h> /* offsetof */
> #include <stdbool.h>
> #include "hw.h"
>
...
> -static void ioport_write2(void *opaque, uint32_t addr, uint32_t val)
> -{
> - EEPRO100State *s = opaque;
> - eepro100_write2(s, addr - s->region[1], val);
> -}
> -
> -static void ioport_write4(void *opaque, uint32_t addr, uint32_t val)
> -{
> - EEPRO100State *s = opaque;
> - eepro100_write4(s, addr - s->region[1], val);
> -}
> -
> /***********************************************************/
> /* PCI EEPRO100 definitions */
>
> -static void pci_map(PCIDevice * pci_dev, int region_num,
> - pcibus_t addr, pcibus_t size, int type)
> +static void pci_io_write(PCIDevice *dev, pcibus_t addr, int size,
> + uint32_t value)
> {
> - EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
> -
> - TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", "
> - "size=0x%08"FMT_PCIBUS", type=%d\n",
> - region_num, addr, size, type));
> + EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, dev);
>
Please don't change the name of the PCIDevice pointer argument
from pci_dev to dev.
This dev, dev in DO_UPCAST is ugly and misleading.
>
> - assert(region_num == 1);
> - register_ioport_write(addr, size, 1, ioport_write1, s);
> - register_ioport_read(addr, size, 1, ioport_read1, s);
> - register_ioport_write(addr, size, 2, ioport_write2, s);
> - register_ioport_read(addr, size, 2, ioport_read2, s);
> - register_ioport_write(addr, size, 4, ioport_write4, s);
> - register_ioport_read(addr, size, 4, ioport_read4, s);
> -
> - s->region[region_num] = addr;
> -}
> -
> -/*****************************************************************************
> - *
> - * Memory mapped I/O.
> - *
> - ****************************************************************************/
> -
> -static void pci_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
> -{
> - EEPRO100State *s = opaque;
> - //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
> - eepro100_write1(s, addr, val);
> -}
> -
> -static void pci_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
> -{
> - EEPRO100State *s = opaque;
> - //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
> - eepro100_write2(s, addr, val);
> -}
> -
> -static void pci_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
> -{
> - EEPRO100State *s = opaque;
> - //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
> - eepro100_write4(s, addr, val);
> -}
> -
> -static uint32_t pci_mmio_readb(void *opaque, target_phys_addr_t addr)
> -{
> - EEPRO100State *s = opaque;
> - //~ logout("addr=%s\n", regname(addr));
> - return eepro100_read1(s, addr);
> -}
> -
> -static uint32_t pci_mmio_readw(void *opaque, target_phys_addr_t addr)
> -{
> - EEPRO100State *s = opaque;
> - //~ logout("addr=%s\n", regname(addr));
> - return eepro100_read2(s, addr);
> -}
> -
> -static uint32_t pci_mmio_readl(void *opaque, target_phys_addr_t addr)
> -{
> - EEPRO100State *s = opaque;
> - //~ logout("addr=%s\n", regname(addr));
> - return eepro100_read4(s, addr);
> + if (size == 1) {
> + eepro100_write1(s, addr, value);
> + } else if (size == 2) {
> + eepro100_write2(s, addr, value);
> + } else {
> + eepro100_write4(s, addr, value);
> + }
> }
>
> -static CPUWriteMemoryFunc * const pci_mmio_write[] = {
> - pci_mmio_writeb,
> - pci_mmio_writew,
> - pci_mmio_writel
> -};
> -
> -static CPUReadMemoryFunc * const pci_mmio_read[] = {
> - pci_mmio_readb,
> - pci_mmio_readw,
> - pci_mmio_readl
> -};
> -
> -static void pci_mmio_map(PCIDevice * pci_dev, int region_num,
> - pcibus_t addr, pcibus_t size, int type)
> +static uint32_t pci_io_read(PCIDevice *dev, pcibus_t addr, int size)
> {
>
Don't change pci_dev -> dev. See above.
...
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 01/15] pci: add new bus functions
2010-02-09 22:01 ` [Qemu-devel] [PATCH 01/15] pci: add new bus functions Anthony Liguori
@ 2010-02-10 8:09 ` Isaku Yamahata
2010-02-10 8:57 ` [Qemu-devel] " Michael S. Tsirkin
1 sibling, 0 replies; 38+ messages in thread
From: Isaku Yamahata @ 2010-02-10 8:09 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Michael Tsirkin, qemu-devel, Alex Graf
On Tue, Feb 09, 2010 at 04:01:25PM -0600, Anthony Liguori wrote:
> - mapping and managing io regions
> - reading and writing physical memory
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> ---
> hw/pci.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> hw/pci.h | 18 ++++++-
> 2 files changed, 181 insertions(+), 9 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 9ad63dd..5460f27 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -664,6 +664,9 @@ static void pci_unregister_io_regions(PCIDevice *pci_dev)
> r->addr),
> r->filtered_size,
> IO_MEM_UNASSIGNED);
> + if (!r->map_func && r->read && r->write) {
> + cpu_unregister_io_memory(r->io_memory_addr);
> + }
> }
> }
> }
> @@ -687,16 +690,15 @@ static int pci_unregister_device(DeviceState *dev)
> return 0;
> }
>
> -void pci_register_bar(PCIDevice *pci_dev, int region_num,
> - pcibus_t size, int type,
> - PCIMapIORegionFunc *map_func)
> +static PCIIORegion *do_pci_register_bar(PCIDevice *pci_dev, int region_num,
> + pcibus_t size, int type)
> {
> PCIIORegion *r;
> uint32_t addr;
> pcibus_t wmask;
>
> if ((unsigned int)region_num >= PCI_NUM_REGIONS)
> - return;
> + return NULL;
>
> if (size & (size-1)) {
> fprintf(stderr, "ERROR: PCI region size must be pow2 "
> @@ -709,7 +711,6 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> r->size = size;
> r->filtered_size = size;
> r->type = type;
> - r->map_func = map_func;
>
> wmask = ~(size - 1);
> addr = pci_bar(pci_dev, region_num);
> @@ -726,6 +727,100 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> pci_set_long(pci_dev->wmask + addr, wmask & 0xffffffff);
> pci_set_long(pci_dev->cmask + addr, 0xffffffff);
> }
> +
> + return r;
> +}
> +
> +void pci_register_bar(PCIDevice *pci_dev, int region_num,
> + pcibus_t size, int type,
> + PCIMapIORegionFunc *map_func)
> +{
> + PCIIORegion *r;
> + r = do_pci_register_bar(pci_dev, region_num, size, type);
> + if (r) {
> + r->map_func = map_func;
> + }
> +}
> +
> +void pci_memory_read(PCIDevice *pci_dev, pcibus_t addr, void *buf, int len)
> +{
> + cpu_physical_memory_read(addr, buf, len);
> +}
> +
> +void pci_memory_write(PCIDevice *pci_dev, pcibus_t addr,
> + const void *buf, int len)
> +{
> + cpu_physical_memory_write(addr, buf, len);
> +}
> +
Isn't something like cpu_to_pci_addr() needed in theory?
> +static void pci_io_region_writeb(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, addr, 1, value);
> +}
> +
> +static void pci_io_region_writew(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, addr, 2, value);
> +}
> +
> +static void pci_io_region_writel(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, addr, 4, value);
> +}
> +
> +static uint32_t pci_io_region_readb(void *opaque, target_phys_addr_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, addr, 1);
> +}
> +
> +static uint32_t pci_io_region_readw(void *opaque, target_phys_addr_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, addr, 2);
> +}
> +
> +static uint32_t pci_io_region_readl(void *opaque, target_phys_addr_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, addr, 4);
> +}
They pass addr, not offset from base address.
pci_io_region_ioport_ {read, write}[bwl]() pass offset.
It's inconsistent. offset should be passed.
Isn't the inverse of pci_to_cpu_addr() necessary in theory?
> +
> +static CPUReadMemoryFunc * const pci_io_region_readfn[3] = {
> + pci_io_region_readb,
> + pci_io_region_readw,
> + pci_io_region_readl,
> +};
> +
> +static CPUWriteMemoryFunc * const pci_io_region_writefn[3] = {
> + pci_io_region_writeb,
> + pci_io_region_writew,
> + pci_io_region_writel,
> +};
> +
> +void pci_register_io_region(PCIDevice *d, int region_num,
> + pcibus_t size, int type,
> + PCIIOReadFunc *readcb, PCIIOWriteFunc *writecb)
> +{
> + PCIIORegion *r;
> + r = do_pci_register_bar(d, region_num, size, type);
> + if (r) {
> + r->map_func = NULL;
> + r->dev = d;
> + r->read = readcb;
> + r->write = writecb;
> + if (r->type != PCI_BASE_ADDRESS_SPACE_IO && r->read && r->write) {
> + r->io_memory_addr = cpu_register_io_memory(pci_io_region_readfn,
> + pci_io_region_writefn,
> + r);
> + }
> + }
> }
>
> static uint32_t pci_config_get_io_base(PCIDevice *d,
> @@ -897,6 +992,45 @@ static pcibus_t pci_bar_address(PCIDevice *d,
> return new_addr;
> }
>
> +static void pci_io_region_ioport_writeb(void *opaque, uint32_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, (addr - r->addr), 1, value);
> +}
> +
> +static void pci_io_region_ioport_writew(void *opaque, uint32_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, (addr - r->addr), 2, value);
> +}
> +
> +static void pci_io_region_ioport_writel(void *opaque, uint32_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, (addr - r->addr), 4, value);
> +}
> +
> +static uint32_t pci_io_region_ioport_readb(void *opaque, uint32_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, (addr - r->addr), 1);
> +}
> +
> +static uint32_t pci_io_region_ioport_readw(void *opaque, uint32_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, (addr - r->addr), 2);
> +}
> +
> +static uint32_t pci_io_region_ioport_readl(void *opaque, uint32_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, (addr - r->addr), 4);
> +}
> +
addr - (r->addr & ~(r->size - 1))
r->addr isn't always base address because of bridge filtering.
> static void pci_update_mappings(PCIDevice *d)
> {
> PCIIORegion *r;
> @@ -952,10 +1086,32 @@ static void pci_update_mappings(PCIDevice *d)
> * addr & (size - 1) != 0.
> */
> if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
> - r->map_func(d, i, r->addr, r->filtered_size, r->type);
> + if (r->map_func) {
> + r->map_func(d, i, r->addr, r->filtered_size, r->type);
> + } else {
> + register_ioport_write(r->addr, r->filtered_size, 1,
> + pci_io_region_ioport_writeb, r);
> + register_ioport_write(r->addr, r->filtered_size, 2,
> + pci_io_region_ioport_writew, r);
> + register_ioport_write(r->addr, r->filtered_size, 4,
> + pci_io_region_ioport_writel, r);
> + register_ioport_read(r->addr, r->filtered_size, 1,
> + pci_io_region_ioport_readb, r);
> + register_ioport_read(r->addr, r->filtered_size, 2,
> + pci_io_region_ioport_readw, r);
> + register_ioport_read(r->addr, r->filtered_size, 4,
> + pci_io_region_ioport_readl, r);
> + }
> } else {
> - r->map_func(d, i, pci_to_cpu_addr(d->bus, r->addr),
> - r->filtered_size, r->type);
> + if (r->map_func) {
> + r->map_func(d, i, pci_to_cpu_addr(d->bus, r->addr),
> + r->filtered_size, r->type);
> + } else if (r->read && r->write) {
> + cpu_register_physical_memory(pci_to_cpu_addr(d->bus,
> + r->addr),
> + r->filtered_size,
> + r->io_memory_addr);
> + }
> }
> }
> }
> diff --git a/hw/pci.h b/hw/pci.h
> index 8b511d2..3edf28f 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -81,13 +81,21 @@ typedef void PCIMapIORegionFunc(PCIDevice *pci_dev, int region_num,
> pcibus_t addr, pcibus_t size, int type);
> typedef int PCIUnregisterFunc(PCIDevice *pci_dev);
>
> +typedef uint32_t (PCIIOReadFunc)(PCIDevice *pci_dev, pcibus_t addr, int size);
> +typedef void (PCIIOWriteFunc)(PCIDevice *pci_dev, pcibus_t addr, int size,
> + uint32_t value);
> +
> typedef struct PCIIORegion {
> + PCIDevice *dev;
> pcibus_t addr; /* current PCI mapping address. -1 means not mapped */
> #define PCI_BAR_UNMAPPED (~(pcibus_t)0)
> pcibus_t size;
> pcibus_t filtered_size;
> uint8_t type;
> - PCIMapIORegionFunc *map_func;
> + PCIMapIORegionFunc *map_func; /* legacy mapping function */
> + PCIIOReadFunc *read;
> + PCIIOWriteFunc *write;
> + int io_memory_addr;
> } PCIIORegion;
>
> #define PCI_ROM_SLOT 6
> @@ -190,6 +198,14 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> pcibus_t size, int type,
> PCIMapIORegionFunc *map_func);
>
> +void pci_register_io_region(PCIDevice *d, int region_num,
> + pcibus_t size, int type,
> + PCIIOReadFunc *readcb, PCIIOWriteFunc *writecb);
> +
> +void pci_memory_read(PCIDevice *pci_dev, pcibus_t addr, void *buf, int len);
> +void pci_memory_write(PCIDevice *pci_dev, pcibus_t addr,
> + const void *buf, int len);
> +
> int pci_add_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
>
> void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
> --
> 1.6.5.2
>
>
>
--
yamahata
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] Re: [PATCH 01/15] pci: add new bus functions
2010-02-09 22:01 ` [Qemu-devel] [PATCH 01/15] pci: add new bus functions Anthony Liguori
2010-02-10 8:09 ` Isaku Yamahata
@ 2010-02-10 8:57 ` Michael S. Tsirkin
1 sibling, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2010-02-10 8:57 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Michael Tsirkin, qemu-devel, Alex Graf
On Tue, Feb 09, 2010 at 04:01:25PM -0600, Anthony Liguori wrote:
> - mapping and managing io regions
> - reading and writing physical memory
>
> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
One thing I'm concerned with here is that this adds
another level of indirection on datapath operations.
Note that rwhandler is only used for config cycles,
which aren't normally datapath.
Can the patch be split to separate mapping and read/write parts?
I think mapping is the harder part, and also where we have actual
emulation bugs now. It would thus probably be uncontroversial.
Also, maybe it's time to bite the bullet and change memory/io callbacks
in exec.c (that rwhandler currently layers above): pass in length
and use some generic type like addr_t for address?
This would let us get rid of both rwhandler and most of this patch.
> ---
> hw/pci.c | 172 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
> hw/pci.h | 18 ++++++-
> 2 files changed, 181 insertions(+), 9 deletions(-)
>
> diff --git a/hw/pci.c b/hw/pci.c
> index 9ad63dd..5460f27 100644
> --- a/hw/pci.c
> +++ b/hw/pci.c
> @@ -664,6 +664,9 @@ static void pci_unregister_io_regions(PCIDevice *pci_dev)
> r->addr),
> r->filtered_size,
> IO_MEM_UNASSIGNED);
> + if (!r->map_func && r->read && r->write) {
> + cpu_unregister_io_memory(r->io_memory_addr);
> + }
> }
> }
> }
> @@ -687,16 +690,15 @@ static int pci_unregister_device(DeviceState *dev)
> return 0;
> }
>
> -void pci_register_bar(PCIDevice *pci_dev, int region_num,
> - pcibus_t size, int type,
> - PCIMapIORegionFunc *map_func)
> +static PCIIORegion *do_pci_register_bar(PCIDevice *pci_dev, int region_num,
> + pcibus_t size, int type)
> {
> PCIIORegion *r;
> uint32_t addr;
> pcibus_t wmask;
>
> if ((unsigned int)region_num >= PCI_NUM_REGIONS)
> - return;
> + return NULL;
>
> if (size & (size-1)) {
> fprintf(stderr, "ERROR: PCI region size must be pow2 "
> @@ -709,7 +711,6 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> r->size = size;
> r->filtered_size = size;
> r->type = type;
> - r->map_func = map_func;
>
> wmask = ~(size - 1);
> addr = pci_bar(pci_dev, region_num);
> @@ -726,6 +727,100 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> pci_set_long(pci_dev->wmask + addr, wmask & 0xffffffff);
> pci_set_long(pci_dev->cmask + addr, 0xffffffff);
> }
> +
> + return r;
> +}
> +
> +void pci_register_bar(PCIDevice *pci_dev, int region_num,
> + pcibus_t size, int type,
> + PCIMapIORegionFunc *map_func)
> +{
> + PCIIORegion *r;
> + r = do_pci_register_bar(pci_dev, region_num, size, type);
> + if (r) {
> + r->map_func = map_func;
> + }
> +}
> +
> +void pci_memory_read(PCIDevice *pci_dev, pcibus_t addr, void *buf, int len)
> +{
> + cpu_physical_memory_read(addr, buf, len);
> +}
> +
> +void pci_memory_write(PCIDevice *pci_dev, pcibus_t addr,
> + const void *buf, int len)
> +{
> + cpu_physical_memory_write(addr, buf, len);
> +}
> +
> +static void pci_io_region_writeb(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, addr, 1, value);
> +}
> +
> +static void pci_io_region_writew(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, addr, 2, value);
> +}
> +
> +static void pci_io_region_writel(void *opaque, target_phys_addr_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, addr, 4, value);
> +}
> +
> +static uint32_t pci_io_region_readb(void *opaque, target_phys_addr_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, addr, 1);
> +}
> +
> +static uint32_t pci_io_region_readw(void *opaque, target_phys_addr_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, addr, 2);
> +}
> +
> +static uint32_t pci_io_region_readl(void *opaque, target_phys_addr_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, addr, 4);
> +}
> +
> +static CPUReadMemoryFunc * const pci_io_region_readfn[3] = {
> + pci_io_region_readb,
> + pci_io_region_readw,
> + pci_io_region_readl,
> +};
> +
> +static CPUWriteMemoryFunc * const pci_io_region_writefn[3] = {
> + pci_io_region_writeb,
> + pci_io_region_writew,
> + pci_io_region_writel,
> +};
> +
> +void pci_register_io_region(PCIDevice *d, int region_num,
> + pcibus_t size, int type,
> + PCIIOReadFunc *readcb, PCIIOWriteFunc *writecb)
> +{
> + PCIIORegion *r;
> + r = do_pci_register_bar(d, region_num, size, type);
> + if (r) {
> + r->map_func = NULL;
> + r->dev = d;
> + r->read = readcb;
> + r->write = writecb;
> + if (r->type != PCI_BASE_ADDRESS_SPACE_IO && r->read && r->write) {
> + r->io_memory_addr = cpu_register_io_memory(pci_io_region_readfn,
> + pci_io_region_writefn,
> + r);
> + }
> + }
> }
>
> static uint32_t pci_config_get_io_base(PCIDevice *d,
> @@ -897,6 +992,45 @@ static pcibus_t pci_bar_address(PCIDevice *d,
> return new_addr;
> }
>
> +static void pci_io_region_ioport_writeb(void *opaque, uint32_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, (addr - r->addr), 1, value);
> +}
> +
> +static void pci_io_region_ioport_writew(void *opaque, uint32_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, (addr - r->addr), 2, value);
> +}
> +
> +static void pci_io_region_ioport_writel(void *opaque, uint32_t addr,
> + uint32_t value)
> +{
> + PCIIORegion *r = opaque;
> + r->write(r->dev, (addr - r->addr), 4, value);
> +}
> +
> +static uint32_t pci_io_region_ioport_readb(void *opaque, uint32_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, (addr - r->addr), 1);
> +}
> +
> +static uint32_t pci_io_region_ioport_readw(void *opaque, uint32_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, (addr - r->addr), 2);
> +}
> +
> +static uint32_t pci_io_region_ioport_readl(void *opaque, uint32_t addr)
> +{
> + PCIIORegion *r = opaque;
> + return r->read(r->dev, (addr - r->addr), 4);
> +}
> +
> static void pci_update_mappings(PCIDevice *d)
> {
> PCIIORegion *r;
> @@ -952,10 +1086,32 @@ static void pci_update_mappings(PCIDevice *d)
> * addr & (size - 1) != 0.
> */
> if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
> - r->map_func(d, i, r->addr, r->filtered_size, r->type);
> + if (r->map_func) {
> + r->map_func(d, i, r->addr, r->filtered_size, r->type);
> + } else {
> + register_ioport_write(r->addr, r->filtered_size, 1,
> + pci_io_region_ioport_writeb, r);
> + register_ioport_write(r->addr, r->filtered_size, 2,
> + pci_io_region_ioport_writew, r);
> + register_ioport_write(r->addr, r->filtered_size, 4,
> + pci_io_region_ioport_writel, r);
> + register_ioport_read(r->addr, r->filtered_size, 1,
> + pci_io_region_ioport_readb, r);
> + register_ioport_read(r->addr, r->filtered_size, 2,
> + pci_io_region_ioport_readw, r);
> + register_ioport_read(r->addr, r->filtered_size, 4,
> + pci_io_region_ioport_readl, r);
> + }
> } else {
> - r->map_func(d, i, pci_to_cpu_addr(d->bus, r->addr),
> - r->filtered_size, r->type);
> + if (r->map_func) {
> + r->map_func(d, i, pci_to_cpu_addr(d->bus, r->addr),
> + r->filtered_size, r->type);
> + } else if (r->read && r->write) {
> + cpu_register_physical_memory(pci_to_cpu_addr(d->bus,
> + r->addr),
> + r->filtered_size,
> + r->io_memory_addr);
> + }
> }
> }
> }
> diff --git a/hw/pci.h b/hw/pci.h
> index 8b511d2..3edf28f 100644
> --- a/hw/pci.h
> +++ b/hw/pci.h
> @@ -81,13 +81,21 @@ typedef void PCIMapIORegionFunc(PCIDevice *pci_dev, int region_num,
> pcibus_t addr, pcibus_t size, int type);
> typedef int PCIUnregisterFunc(PCIDevice *pci_dev);
>
> +typedef uint32_t (PCIIOReadFunc)(PCIDevice *pci_dev, pcibus_t addr, int size);
> +typedef void (PCIIOWriteFunc)(PCIDevice *pci_dev, pcibus_t addr, int size,
> + uint32_t value);
> +
> typedef struct PCIIORegion {
> + PCIDevice *dev;
> pcibus_t addr; /* current PCI mapping address. -1 means not mapped */
> #define PCI_BAR_UNMAPPED (~(pcibus_t)0)
> pcibus_t size;
> pcibus_t filtered_size;
> uint8_t type;
> - PCIMapIORegionFunc *map_func;
> + PCIMapIORegionFunc *map_func; /* legacy mapping function */
> + PCIIOReadFunc *read;
> + PCIIOWriteFunc *write;
> + int io_memory_addr;
> } PCIIORegion;
>
> #define PCI_ROM_SLOT 6
> @@ -190,6 +198,14 @@ void pci_register_bar(PCIDevice *pci_dev, int region_num,
> pcibus_t size, int type,
> PCIMapIORegionFunc *map_func);
>
> +void pci_register_io_region(PCIDevice *d, int region_num,
> + pcibus_t size, int type,
> + PCIIOReadFunc *readcb, PCIIOWriteFunc *writecb);
> +
> +void pci_memory_read(PCIDevice *pci_dev, pcibus_t addr, void *buf, int len);
> +void pci_memory_write(PCIDevice *pci_dev, pcibus_t addr,
> + const void *buf, int len);
> +
> int pci_add_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
>
> void pci_del_capability(PCIDevice *pci_dev, uint8_t cap_id, uint8_t cap_size);
> --
> 1.6.5.2
>
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] Re: [PATCH 0/15][RFC] New PCI interfaces
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
` (14 preceding siblings ...)
2010-02-09 22:01 ` [Qemu-devel] [PATCH 15/15] pci: byte swap as PCI interface layer Anthony Liguori
@ 2010-02-10 9:31 ` Michael S. Tsirkin
2010-02-10 18:34 ` [Qemu-devel] " Blue Swirl
16 siblings, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2010-02-10 9:31 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Michael Tsirkin, qemu-devel, Alex Graf
On Tue, Feb 09, 2010 at 04:01:24PM -0600, Anthony Liguori wrote:
> This is a work in progress that I wanted to share giving some of the discussions
> around rwhandlers. The idea is to make PCI devices have a common set of
> functions to interact with the CPU that is driven entirely through the PCI bus.
>
> I've tested the network card conversions, but have not yet tested the other
> bits.
It would definitely be an improvement when all mapping would be done in
pci core. We could thus finally fix bridge filtering.
The hardest part with this change would be legacy vga class.
I am not so sure about adding an extra level of indirection
for all memory/io transactions. What we do now is perform
setup during bus scan, afterwards cpu calls device directly,
which looks nicer on the surface. Maybe we just need to
make callbacks more generic.
--
MST
^ permalink raw reply [flat|nested] 38+ messages in thread
* [Qemu-devel] Re: [PATCH 09/15] eepro100: convert to new pci interface
2010-02-10 6:32 ` Stefan Weil
@ 2010-02-10 9:49 ` Michael S. Tsirkin
2010-02-10 10:43 ` Markus Armbruster
2010-02-10 14:13 ` [Qemu-devel] " Anthony Liguori
1 sibling, 1 reply; 38+ messages in thread
From: Michael S. Tsirkin @ 2010-02-10 9:49 UTC (permalink / raw)
To: Stefan Weil; +Cc: Anthony Liguori, qemu-devel, Michael Tsirkin
On Wed, Feb 10, 2010 at 07:32:10AM +0100, Stefan Weil wrote:
> See my inline comments.
>
>
> Anthony Liguori schrieb:
> > - Removed some dead defines for TARGET_I386 so that we could build once
> >
> > Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
> > ---
> > hw/eepro100.c | 238 ++++++++++++++-------------------------------------------
> > 1 files changed, 57 insertions(+), 181 deletions(-)
> >
> > diff --git a/hw/eepro100.c b/hw/eepro100.c
> > index b33dbb8..16230c9 100644
> > --- a/hw/eepro100.c
> > +++ b/hw/eepro100.c
> > @@ -33,10 +33,6 @@
> > * Open Source Software Developer Manual
> > */
> >
> > -#if defined(TARGET_I386)
> > -# warning "PXE boot still not working!"
> > -#endif
> > -
> >
>
> You did not fix PXE boot here, did you?
> So the warning or a comment should stay there.
>
> > #include <stddef.h> /* offsetof */
> > #include <stdbool.h>
> > #include "hw.h"
> >
> ...
>
>
> > -static void ioport_write2(void *opaque, uint32_t addr, uint32_t val)
> > -{
> > - EEPRO100State *s = opaque;
> > - eepro100_write2(s, addr - s->region[1], val);
> > -}
> > -
> > -static void ioport_write4(void *opaque, uint32_t addr, uint32_t val)
> > -{
> > - EEPRO100State *s = opaque;
> > - eepro100_write4(s, addr - s->region[1], val);
> > -}
> > -
> > /***********************************************************/
> > /* PCI EEPRO100 definitions */
> >
> > -static void pci_map(PCIDevice * pci_dev, int region_num,
> > - pcibus_t addr, pcibus_t size, int type)
> > +static void pci_io_write(PCIDevice *dev, pcibus_t addr, int size,
> > + uint32_t value)
> > {
> > - EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
> > -
> > - TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", "
> > - "size=0x%08"FMT_PCIBUS", type=%d\n",
> > - region_num, addr, size, type));
> > + EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, dev);
> >
>
> Please don't change the name of the PCIDevice pointer argument
> from pci_dev to dev.
>
> This dev, dev in DO_UPCAST is ugly and misleading.
It's a matter of taste: I actually think it's pretty: I never remember
which argument of DO_UPCAST is which, and if both are dev it doesn't
matter.
>
> >
> > - assert(region_num == 1);
> > - register_ioport_write(addr, size, 1, ioport_write1, s);
> > - register_ioport_read(addr, size, 1, ioport_read1, s);
> > - register_ioport_write(addr, size, 2, ioport_write2, s);
> > - register_ioport_read(addr, size, 2, ioport_read2, s);
> > - register_ioport_write(addr, size, 4, ioport_write4, s);
> > - register_ioport_read(addr, size, 4, ioport_read4, s);
> > -
> > - s->region[region_num] = addr;
> > -}
> > -
> > -/*****************************************************************************
> > - *
> > - * Memory mapped I/O.
> > - *
> > - ****************************************************************************/
> > -
> > -static void pci_mmio_writeb(void *opaque, target_phys_addr_t addr, uint32_t val)
> > -{
> > - EEPRO100State *s = opaque;
> > - //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
> > - eepro100_write1(s, addr, val);
> > -}
> > -
> > -static void pci_mmio_writew(void *opaque, target_phys_addr_t addr, uint32_t val)
> > -{
> > - EEPRO100State *s = opaque;
> > - //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
> > - eepro100_write2(s, addr, val);
> > -}
> > -
> > -static void pci_mmio_writel(void *opaque, target_phys_addr_t addr, uint32_t val)
> > -{
> > - EEPRO100State *s = opaque;
> > - //~ logout("addr=%s val=0x%02x\n", regname(addr), val);
> > - eepro100_write4(s, addr, val);
> > -}
> > -
> > -static uint32_t pci_mmio_readb(void *opaque, target_phys_addr_t addr)
> > -{
> > - EEPRO100State *s = opaque;
> > - //~ logout("addr=%s\n", regname(addr));
> > - return eepro100_read1(s, addr);
> > -}
> > -
> > -static uint32_t pci_mmio_readw(void *opaque, target_phys_addr_t addr)
> > -{
> > - EEPRO100State *s = opaque;
> > - //~ logout("addr=%s\n", regname(addr));
> > - return eepro100_read2(s, addr);
> > -}
> > -
> > -static uint32_t pci_mmio_readl(void *opaque, target_phys_addr_t addr)
> > -{
> > - EEPRO100State *s = opaque;
> > - //~ logout("addr=%s\n", regname(addr));
> > - return eepro100_read4(s, addr);
> > + if (size == 1) {
> > + eepro100_write1(s, addr, value);
> > + } else if (size == 2) {
> > + eepro100_write2(s, addr, value);
> > + } else {
> > + eepro100_write4(s, addr, value);
> > + }
> > }
> >
> > -static CPUWriteMemoryFunc * const pci_mmio_write[] = {
> > - pci_mmio_writeb,
> > - pci_mmio_writew,
> > - pci_mmio_writel
> > -};
> > -
> > -static CPUReadMemoryFunc * const pci_mmio_read[] = {
> > - pci_mmio_readb,
> > - pci_mmio_readw,
> > - pci_mmio_readl
> > -};
> > -
> > -static void pci_mmio_map(PCIDevice * pci_dev, int region_num,
> > - pcibus_t addr, pcibus_t size, int type)
> > +static uint32_t pci_io_read(PCIDevice *dev, pcibus_t addr, int size)
> > {
> >
>
> Don't change pci_dev -> dev. See above.
>
> ...
>
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 09/15] eepro100: convert to new pci interface
2010-02-10 9:49 ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-02-10 10:43 ` Markus Armbruster
2010-02-10 10:48 ` Michael S. Tsirkin
0 siblings, 1 reply; 38+ messages in thread
From: Markus Armbruster @ 2010-02-10 10:43 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: Anthony Liguori, qemu-devel
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Wed, Feb 10, 2010 at 07:32:10AM +0100, Stefan Weil wrote:
>> See my inline comments.
>>
>>
>> Anthony Liguori schrieb:
[...]
>> > -static void pci_map(PCIDevice * pci_dev, int region_num,
>> > - pcibus_t addr, pcibus_t size, int type)
>> > +static void pci_io_write(PCIDevice *dev, pcibus_t addr, int size,
>> > + uint32_t value)
>> > {
>> > - EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
>> > -
>> > - TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", "
>> > - "size=0x%08"FMT_PCIBUS", type=%d\n",
>> > - region_num, addr, size, type));
>> > + EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, dev);
>> >
>>
>> Please don't change the name of the PCIDevice pointer argument
>> from pci_dev to dev.
>>
>> This dev, dev in DO_UPCAST is ugly and misleading.
>
> It's a matter of taste: I actually think it's pretty: I never remember
> which argument of DO_UPCAST is which, and if both are dev it doesn't
> matter.
I also have trouble remembering how to use DO_UPCAST(), but playing
games with identifiers is a poor fix for that. I find the use of the
same name "dev" for two different things in the same expression rather
confusing. Could we improve DO_UPCAST() instead?
For what it's worth, I have no trouble with container_of(). DO_UPCAST()
takes the same arguments in a different order, which I find irritating.
[...]
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] Re: [PATCH 09/15] eepro100: convert to new pci interface
2010-02-10 10:43 ` Markus Armbruster
@ 2010-02-10 10:48 ` Michael S. Tsirkin
0 siblings, 0 replies; 38+ messages in thread
From: Michael S. Tsirkin @ 2010-02-10 10:48 UTC (permalink / raw)
To: Markus Armbruster; +Cc: Anthony Liguori, qemu-devel
On Wed, Feb 10, 2010 at 11:43:29AM +0100, Markus Armbruster wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
>
> > On Wed, Feb 10, 2010 at 07:32:10AM +0100, Stefan Weil wrote:
> >> See my inline comments.
> >>
> >>
> >> Anthony Liguori schrieb:
> [...]
> >> > -static void pci_map(PCIDevice * pci_dev, int region_num,
> >> > - pcibus_t addr, pcibus_t size, int type)
> >> > +static void pci_io_write(PCIDevice *dev, pcibus_t addr, int size,
> >> > + uint32_t value)
> >> > {
> >> > - EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
> >> > -
> >> > - TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", "
> >> > - "size=0x%08"FMT_PCIBUS", type=%d\n",
> >> > - region_num, addr, size, type));
> >> > + EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, dev);
> >> >
> >>
> >> Please don't change the name of the PCIDevice pointer argument
> >> from pci_dev to dev.
> >>
> >> This dev, dev in DO_UPCAST is ugly and misleading.
> >
> > It's a matter of taste: I actually think it's pretty: I never remember
> > which argument of DO_UPCAST is which, and if both are dev it doesn't
> > matter.
>
> I also have trouble remembering how to use DO_UPCAST(), but playing
> games with identifiers is a poor fix for that. I find the use of the
> same name "dev" for two different things in the same expression rather
> confusing. Could we improve DO_UPCAST() instead?
>
> For what it's worth, I have no trouble with container_of(). DO_UPCAST()
> takes the same arguments in a different order, which I find irritating.
>
> [...]
IMO it would be ideal to remove offset assumptions where possible
and then we can use container_of.
--
MST
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 09/15] eepro100: convert to new pci interface
2010-02-10 6:32 ` Stefan Weil
2010-02-10 9:49 ` [Qemu-devel] " Michael S. Tsirkin
@ 2010-02-10 14:13 ` Anthony Liguori
1 sibling, 0 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-10 14:13 UTC (permalink / raw)
To: Stefan Weil; +Cc: Michael Tsirkin, Anthony Liguori, qemu-devel
On 02/10/2010 12:32 AM, Stefan Weil wrote:
> See my inline comments.
>
>
> Anthony Liguori schrieb:
>
>> - Removed some dead defines for TARGET_I386 so that we could build once
>>
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> ---
>> hw/eepro100.c | 238 ++++++++++++++-------------------------------------------
>> 1 files changed, 57 insertions(+), 181 deletions(-)
>>
>> diff --git a/hw/eepro100.c b/hw/eepro100.c
>> index b33dbb8..16230c9 100644
>> --- a/hw/eepro100.c
>> +++ b/hw/eepro100.c
>> @@ -33,10 +33,6 @@
>> * Open Source Software Developer Manual
>> */
>>
>> -#if defined(TARGET_I386)
>> -# warning "PXE boot still not working!"
>> -#endif
>> -
>>
>>
> You did not fix PXE boot here, did you?
> So the warning or a comment should stay there.
>
A comment is fine, but the TARGET_I386 makes this file unnecessarily
dependent on TARGET. With this change, we only need to build eepro100.o
once.
>> /***********************************************************/
>> /* PCI EEPRO100 definitions */
>>
>> -static void pci_map(PCIDevice * pci_dev, int region_num,
>> - pcibus_t addr, pcibus_t size, int type)
>> +static void pci_io_write(PCIDevice *dev, pcibus_t addr, int size,
>> + uint32_t value)
>> {
>> - EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, pci_dev);
>> -
>> - TRACE(OTHER, logout("region %d, addr=0x%08"FMT_PCIBUS", "
>> - "size=0x%08"FMT_PCIBUS", type=%d\n",
>> - region_num, addr, size, type));
>> + EEPRO100State *s = DO_UPCAST(EEPRO100State, dev, dev);
>>
>>
> Please don't change the name of the PCIDevice pointer argument
> from pci_dev to dev.
>
> This dev, dev in DO_UPCAST is ugly and misleading.
>
It's very common and I changed it for consistency. I honestly don't
care though either way.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
` (15 preceding siblings ...)
2010-02-10 9:31 ` [Qemu-devel] Re: [PATCH 0/15][RFC] New PCI interfaces Michael S. Tsirkin
@ 2010-02-10 18:34 ` Blue Swirl
2010-02-10 19:29 ` Anthony Liguori
16 siblings, 1 reply; 38+ messages in thread
From: Blue Swirl @ 2010-02-10 18:34 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Michael Tsirkin, qemu-devel, Alex Graf
On Wed, Feb 10, 2010 at 12:01 AM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> This is a work in progress that I wanted to share giving some of the discussions
> around rwhandlers. The idea is to make PCI devices have a common set of
> functions to interact with the CPU that is driven entirely through the PCI bus.
>
> I've tested the network card conversions, but have not yet tested the other
> bits.
By itself, the patches look OK. But given that the conclusion of the
generic DMA discussion was to aim for mapping approach (in order to
prepare for zero copy DMA), I wonder how the patches fit to the larger
picture. Perhaps instead of pci specific functions, a more generic
memory object with read and write methods could be introduced, which
would be more useful in the longer term?
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces
2010-02-10 18:34 ` [Qemu-devel] " Blue Swirl
@ 2010-02-10 19:29 ` Anthony Liguori
2010-02-10 20:41 ` Richard Henderson
2010-03-01 2:51 ` Paul Brook
0 siblings, 2 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-10 19:29 UTC (permalink / raw)
To: Blue Swirl; +Cc: Michael Tsirkin, qemu-devel, Alex Graf
On 02/10/2010 12:34 PM, Blue Swirl wrote:
> On Wed, Feb 10, 2010 at 12:01 AM, Anthony Liguori<aliguori@us.ibm.com> wrote:
>
>> This is a work in progress that I wanted to share giving some of the discussions
>> around rwhandlers. The idea is to make PCI devices have a common set of
>> functions to interact with the CPU that is driven entirely through the PCI bus.
>>
>> I've tested the network card conversions, but have not yet tested the other
>> bits.
>>
> By itself, the patches look OK. But given that the conclusion of the
> generic DMA discussion was to aim for mapping approach (in order to
> prepare for zero copy DMA), I wonder how the patches fit to the larger
> picture. Perhaps instead of pci specific functions, a more generic
> memory object with read and write methods could be introduced, which
> would be more useful in the longer term?
>
This is a good point and it's something that needs to be addressed for
the virtio conversion.
I was thinking:
void *pci_memory_map(PCIDevice *dev, pcibus_t addr, pcibus_t *plen, int
is_write);
void pci_memory_unmap(PCIDevice *dev, void *buf, pcibus_t *plen, int
is_write, pcibus_t access_len);
Not too surprising.
Since virtio devices can live on two busses (sysbus with Syborg or PCI),
we need to introduce a set of virtio specific functions.
void virtio_memory_read(VirtIODevice *dev, uint64_t addr, void *buf, int len);
void virtio_memory_write(VirtIODevice *dev, uint64_t addr, const void *buf, int len);
void *virtio_memory_map(VirtIODevice *dev, uint64_t addr, uint64_t *plen, int is_write);
void virtio_memory_unmap(VirtIODevice *dev, uint64_t addr, uint64_t *plen, int is_write, uint64_t access_len);
Inside the VirtIODevice, there would be corresponding function pointers, and depending on whether it was a PCI device or a Syborg device, it would call pci_memory_map or cpu_physical_memory_map.
If we introduced a consistent address type, it would be possible to make generic memory access functions that used a state variable to mask this from the user.
I personally prefer the explicit interfaces though. It makes it easy to look at a PCI device and see that it only uses PCI interfaces. I also think that some bus concepts will be difficult to abstract in a generic way.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces
2010-02-10 19:29 ` Anthony Liguori
@ 2010-02-10 20:41 ` Richard Henderson
2010-02-10 21:13 ` Anthony Liguori
2010-03-01 2:51 ` Paul Brook
1 sibling, 1 reply; 38+ messages in thread
From: Richard Henderson @ 2010-02-10 20:41 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Blue Swirl, Michael Tsirkin, qemu-devel, Alex Graf
On 02/10/2010 11:29 AM, Anthony Liguori wrote:
> void *pci_memory_map(PCIDevice *dev, pcibus_t addr, pcibus_t *plen, int
> is_write);
>
> void pci_memory_unmap(PCIDevice *dev, void *buf, pcibus_t *plen, int
> is_write, pcibus_t access_len);
Are these functions intended to be controllable by the root bus object?
It would be awfully nice if we would design in a hook that allowed
iommu mapping to be done properly.
r~
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces
2010-02-10 20:41 ` Richard Henderson
@ 2010-02-10 21:13 ` Anthony Liguori
2010-02-12 17:40 ` Blue Swirl
0 siblings, 1 reply; 38+ messages in thread
From: Anthony Liguori @ 2010-02-10 21:13 UTC (permalink / raw)
To: Richard Henderson; +Cc: Blue Swirl, Michael Tsirkin, qemu-devel, Alex Graf
On 02/10/2010 02:41 PM, Richard Henderson wrote:
> On 02/10/2010 11:29 AM, Anthony Liguori wrote:
>> void *pci_memory_map(PCIDevice *dev, pcibus_t addr, pcibus_t *plen, int
>> is_write);
>>
>> void pci_memory_unmap(PCIDevice *dev, void *buf, pcibus_t *plen, int
>> is_write, pcibus_t access_len);
>
> Are these functions intended to be controllable by the root bus
> object? It would be awfully nice if we would design in a hook that
> allowed iommu mapping to be done properly.
Yes, that's the point. For something like virtio, you would have a call
chain like:
virtio_memory_map -> pci_memory_map -> sysbus_memory_map[1] ->
cpu_memory_map.
Each layer has the ability to do things like implement iommu mapping.
[1] I think it might make sense to have a sysbus layer but I'm not 100%
sure yet.
Regards,
Anthony Liguori
>
> r~
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API
2010-02-10 0:24 ` malc
@ 2010-02-11 14:20 ` Anthony Liguori
0 siblings, 0 replies; 38+ messages in thread
From: Anthony Liguori @ 2010-02-11 14:20 UTC (permalink / raw)
To: malc; +Cc: Alex Graf, qemu-devel, Michael Tsirkin
On 02/09/2010 06:24 PM, malc wrote:
> On Tue, 9 Feb 2010, Anthony Liguori wrote:
>
>
>> On 02/09/2010 05:36 PM, malc wrote:
>>
>>> Let's see:
>>>
>>> Currently we have this
>>>
>>>
>>> readb(...):
>>> dostuff
>>> return stuff
>>>
>>> readw(...):
>>> dostuff
>>> return stuff
>>>
>>>
>> And this is completely wrong.
>>
> It's completely right, until some future C standard implements proper
> algebraic data types.
>
I've been thinking about what to do here and I think what I've come up
with is to change the callbacks to take be a separate type. So instead
of just:
void (PCIOWriteFunc)(PCIDevice *, pcibus_t addr, int size, uint32_t value);
uint32_t (PCIOReadFunc)(PCIDevice *, pcibus_t addr, int size);
We would have:
typedef struct PCIIOHandler
{
void (*write)(PCIIOHandler *, pcibus_t addr, int size, uint32_t);
uint32_t (*read)(PCIIOHandler *, pcibus_t addr, int size);
} PCIIOHandler;
And then we would have:
pci_register_io_region(PCIDevice *, ... PCIIOHandler *handler);
We can then introduce a mechanism to dispatch to individual functions
based on size. That way, instead of pushing an if (size == 1) else if
(size == 2) check to each device, we let the device decide how it wants
to be invoked.
>> For the most part, device models don't consistently handle access to registers
>> via their non native sizes. Some devices return the lower part of a dword
>> register when accessed with a word accessor. Some devices only return the
>> register when there's a dword access.
>>
> That's up to device to register an accessor and return what's appropriate.
>
I think the point though is to make it possible for common mechanisms to
be formalized into common code. For instance, a common set of rules
will be, all registers can be accessed in byte, word, or dword size but
most be accessed at natural size boundaries. We should make it simple
for a device to essentially choose that pattern and not have to worry
about explicitly coding it themselves.
>> We can even take it further, and do something like:
>>
>> pci_regs = {
>> PCI_REG_DWORD(REG0, DeviceState, reg0),
>> PCI_REG_BYTE(REG1, DeviceState, reg1),
>> ...
>> }
>>
>> In which case, we only need to handle the case in switch() if we need to
>> implement a side effect of the register operation.
>>
>> But none of this is possible if we completely rely on every device to
>> implement non-dword access in it's own broken way.
>>
>>
> Lovely, we are heading full speed into fanatsy land, where PCI BUS itself
> accesses device specific stuff.
>
I never said the PCI BUS is the one that actually got this info. It
could just be common code that implements this logic.
But there are good reasons to normalize device emulation at this level.
For instance, it allows us to implement consistent tracing for device
models using symbolic names for registers as opposed to only being able
to trace IO operations at addresses.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces
2010-02-10 21:13 ` Anthony Liguori
@ 2010-02-12 17:40 ` Blue Swirl
0 siblings, 0 replies; 38+ messages in thread
From: Blue Swirl @ 2010-02-12 17:40 UTC (permalink / raw)
To: Anthony Liguori; +Cc: Michael Tsirkin, Alex Graf, qemu-devel, Richard Henderson
On Wed, Feb 10, 2010 at 11:13 PM, Anthony Liguori <anthony@codemonkey.ws> wrote:
> On 02/10/2010 02:41 PM, Richard Henderson wrote:
>>
>> On 02/10/2010 11:29 AM, Anthony Liguori wrote:
>>>
>>> void *pci_memory_map(PCIDevice *dev, pcibus_t addr, pcibus_t *plen, int
>>> is_write);
>>>
>>> void pci_memory_unmap(PCIDevice *dev, void *buf, pcibus_t *plen, int
>>> is_write, pcibus_t access_len);
>>
>> Are these functions intended to be controllable by the root bus object?
>> It would be awfully nice if we would design in a hook that allowed iommu
>> mapping to be done properly.
>
> Yes, that's the point. For something like virtio, you would have a call
> chain like:
>
> virtio_memory_map -> pci_memory_map -> sysbus_memory_map[1] ->
> cpu_memory_map.
>
> Each layer has the ability to do things like implement iommu mapping.
>
> [1] I think it might make sense to have a sysbus layer but I'm not 100% sure
> yet.
In Sun4c, CPU MMU handles also IO, so there we could benefit from
sysbus layer. Sun4c is very low priority though.
For emulation of caches, there would need to be another layer between
CPU and memory, which would not be shared with DMA.
^ permalink raw reply [flat|nested] 38+ messages in thread
* Re: [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces
2010-02-10 19:29 ` Anthony Liguori
2010-02-10 20:41 ` Richard Henderson
@ 2010-03-01 2:51 ` Paul Brook
1 sibling, 0 replies; 38+ messages in thread
From: Paul Brook @ 2010-03-01 2:51 UTC (permalink / raw)
To: qemu-devel; +Cc: Blue Swirl, Michael Tsirkin, Alex Graf
> Since virtio devices can live on two busses (sysbus with Syborg or PCI),
> we need to introduce a set of virtio specific functions.
>...
> Inside the VirtIODevice, there would be corresponding function pointers,
> and depending on whether it was a PCI device or a Syborg device, it would
> call pci_memory_map or cpu_physical_memory_map.
You mean VirtIOBindings not VirtIODevice, right? Virtio devices should not be
accessing guest memory directly, they should be using a virtqueue.
Paul
^ permalink raw reply [flat|nested] 38+ messages in thread
end of thread, other threads:[~2010-03-01 2:51 UTC | newest]
Thread overview: 38+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-02-09 22:01 [Qemu-devel] [PATCH 0/15][RFC] New PCI interfaces Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 01/15] pci: add new bus functions Anthony Liguori
2010-02-10 8:09 ` Isaku Yamahata
2010-02-10 8:57 ` [Qemu-devel] " Michael S. Tsirkin
2010-02-09 22:01 ` [Qemu-devel] [PATCH 02/15] rtl8139: convert to new PCI interfaces Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 03/15] lsi53c895a: convert to new pci interfaces Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 04/15] e1000: convert to new pci interface Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 05/15] wdt_i6300esb: fix io type leakage Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 06/15] wdt_i6300esb: convert to new pci inteface Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 07/15] ac97: convert to new PCI API Anthony Liguori
2010-02-09 22:45 ` malc
2010-02-09 22:52 ` Anthony Liguori
2010-02-09 23:06 ` malc
2010-02-09 23:10 ` Anthony Liguori
2010-02-09 23:36 ` malc
2010-02-09 23:52 ` Anthony Liguori
2010-02-10 0:24 ` malc
2010-02-11 14:20 ` Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 08/15] es1370: convert to new pci interface Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 09/15] eepro100: " Anthony Liguori
2010-02-10 6:32 ` Stefan Weil
2010-02-10 9:49 ` [Qemu-devel] " Michael S. Tsirkin
2010-02-10 10:43 ` Markus Armbruster
2010-02-10 10:48 ` Michael S. Tsirkin
2010-02-10 14:13 ` [Qemu-devel] " Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 10/15] virtio-pci: " Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 11/15] pci: add pci_register_msix_region Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 12/15] ne2000: convert to new pci interface Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 13/15] pcnet: " Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 14/15] usb-uhci: " Anthony Liguori
2010-02-09 22:01 ` [Qemu-devel] [PATCH 15/15] pci: byte swap as PCI interface layer Anthony Liguori
2010-02-10 9:31 ` [Qemu-devel] Re: [PATCH 0/15][RFC] New PCI interfaces Michael S. Tsirkin
2010-02-10 18:34 ` [Qemu-devel] " Blue Swirl
2010-02-10 19:29 ` Anthony Liguori
2010-02-10 20:41 ` Richard Henderson
2010-02-10 21:13 ` Anthony Liguori
2010-02-12 17:40 ` Blue Swirl
2010-03-01 2:51 ` Paul Brook
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).