* [Qemu-devel] [PATCH v1 0/5] Add support for Cadence GEM priority queues
@ 2016-07-11 23:20 Alistair Francis
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 1/5] cadence_gem: QOMify Cadence GEM Alistair Francis
` (5 more replies)
0 siblings, 6 replies; 19+ messages in thread
From: Alistair Francis @ 2016-07-11 23:20 UTC (permalink / raw)
To: qemu-devel, qemu-arm; +Cc: alistair.francis, crosthwaitepeter, peter.maydell
This patch series adds initial priority queue support to the Cadence GEM
device. This is based on original work by Peter C, that has been ported
to the latest version of QEMU.
There is more GEM work that I'd like to upstream after this, but I
figured this a good place to start. I have done limited testing on the
Xilinx machine, including running some of our GEM regressions, although
they don't cover everything. I can't really think of too many other test
cases that I can run at the moment.
Alistair Francis (5):
cadence_gem: QOMify Cadence GEM
cadence_gem: Arrayify
cadence_gem: Add queue support
cadence_gem: Correct indentation
xlnx-zynqmp: Set the number of priority queues
hw/arm/xlnx-zynqmp.c | 2 +
hw/net/cadence_gem.c | 376 ++++++++++++++++++++++++++-----------------
include/hw/net/cadence_gem.h | 15 +-
3 files changed, 243 insertions(+), 150 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v1 1/5] cadence_gem: QOMify Cadence GEM
2016-07-11 23:20 [Qemu-devel] [PATCH v1 0/5] Add support for Cadence GEM priority queues Alistair Francis
@ 2016-07-11 23:20 ` Alistair Francis
2016-07-25 15:20 ` Peter Maydell
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 2/5] cadence_gem: Arrayify Alistair Francis
` (4 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Alistair Francis @ 2016-07-11 23:20 UTC (permalink / raw)
To: qemu-devel, qemu-arm; +Cc: alistair.francis, crosthwaitepeter, peter.maydell
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
hw/net/cadence_gem.c | 27 ++++++++++++++++-----------
1 file changed, 16 insertions(+), 11 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 8a4be1e..9d64644 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1214,24 +1214,29 @@ static NetClientInfo net_gem_info = {
.link_status_changed = gem_set_link,
};
-static int gem_init(SysBusDevice *sbd)
+static void gem_realize(DeviceState *dev, Error **errp)
{
- DeviceState *dev = DEVICE(sbd);
CadenceGEMState *s = CADENCE_GEM(dev);
- DB_PRINT("\n");
+ sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
- gem_init_register_masks(s);
- memory_region_init_io(&s->iomem, OBJECT(s), &gem_ops, s,
- "enet", sizeof(s->regs));
- sysbus_init_mmio(sbd, &s->iomem);
- sysbus_init_irq(sbd, &s->irq);
qemu_macaddr_default_if_unset(&s->conf.macaddr);
s->nic = qemu_new_nic(&net_gem_info, &s->conf,
object_get_typename(OBJECT(dev)), dev->id, s);
+}
+
+static void gem_init(Object *obj)
+{
+ CadenceGEMState *s = CADENCE_GEM(obj);
+ DeviceState *dev = DEVICE(obj);
+
+ DB_PRINT("\n");
- return 0;
+ gem_init_register_masks(s);
+ memory_region_init_io(&s->iomem, OBJECT(s), &gem_ops, s,
+ "enet", sizeof(s->regs));
+ sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
}
static const VMStateDescription vmstate_cadence_gem = {
@@ -1257,9 +1262,8 @@ static Property gem_properties[] = {
static void gem_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
- SysBusDeviceClass *sdc = SYS_BUS_DEVICE_CLASS(klass);
- sdc->init = gem_init;
+ dc->realize = gem_realize;
dc->props = gem_properties;
dc->vmsd = &vmstate_cadence_gem;
dc->reset = gem_reset;
@@ -1269,6 +1273,7 @@ static const TypeInfo gem_info = {
.name = TYPE_CADENCE_GEM,
.parent = TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(CadenceGEMState),
+ .instance_init = gem_init,
.class_init = gem_class_init,
};
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v1 2/5] cadence_gem: Arrayify
2016-07-11 23:20 [Qemu-devel] [PATCH v1 0/5] Add support for Cadence GEM priority queues Alistair Francis
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 1/5] cadence_gem: QOMify Cadence GEM Alistair Francis
@ 2016-07-11 23:20 ` Alistair Francis
2016-07-25 15:22 ` Peter Maydell
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 3/5] cadence_gem: Add queue support Alistair Francis
` (3 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Alistair Francis @ 2016-07-11 23:20 UTC (permalink / raw)
To: qemu-devel, qemu-arm; +Cc: alistair.francis, crosthwaitepeter, peter.maydell
Arrayify the Cadence GEM device to prepare for adding priority queue support.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
This patch has no real effect, I used it to help debug some issues so I
figured I would leave it in.
hw/net/cadence_gem.c | 78 +++++++++++++++++++++++---------------------
include/hw/net/cadence_gem.h | 13 +++++---
2 files changed, 50 insertions(+), 41 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 9d64644..2dabad5 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -428,18 +428,18 @@ static int gem_can_receive(NetClientState *nc)
return 0;
}
- if (rx_desc_get_ownership(s->rx_desc) == 1) {
+ if (rx_desc_get_ownership(s->rx_desc[0]) == 1) {
if (s->can_rx_state != 2) {
s->can_rx_state = 2;
DB_PRINT("can't receive - busy buffer descriptor 0x%x\n",
- s->rx_desc_addr);
+ s->rx_desc_addr[0]);
}
return 0;
}
if (s->can_rx_state != 0) {
s->can_rx_state = 0;
- DB_PRINT("can receive 0x%x\n", s->rx_desc_addr);
+ DB_PRINT("can receive 0x%x\n", s->rx_desc_addr[0]);
}
return 1;
}
@@ -452,7 +452,7 @@ static void gem_update_int_status(CadenceGEMState *s)
{
if (s->regs[GEM_ISR]) {
DB_PRINT("asserting int. (0x%08x)\n", s->regs[GEM_ISR]);
- qemu_set_irq(s->irq, 1);
+ qemu_set_irq(s->irq[0], 1);
}
}
@@ -603,15 +603,15 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
static void gem_get_rx_desc(CadenceGEMState *s)
{
- DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr);
+ DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[0]);
/* read current descriptor */
- cpu_physical_memory_read(s->rx_desc_addr,
- (uint8_t *)s->rx_desc, sizeof(s->rx_desc));
+ cpu_physical_memory_read(s->rx_desc_addr[0],
+ (uint8_t *)s->rx_desc[0], sizeof(s->rx_desc[0]));
/* Descriptor owned by software ? */
- if (rx_desc_get_ownership(s->rx_desc) == 1) {
+ if (rx_desc_get_ownership(s->rx_desc[0]) == 1) {
DB_PRINT("descriptor 0x%x owned by sw.\n",
- (unsigned)s->rx_desc_addr);
+ (unsigned)s->rx_desc_addr[0]);
s->regs[GEM_RXSTATUS] |= GEM_RXSTATUS_NOBUF;
s->regs[GEM_ISR] |= GEM_INT_RXUSED & ~(s->regs[GEM_IMR]);
/* Handle interrupt consequences */
@@ -718,54 +718,56 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
}
DB_PRINT("copy %d bytes to 0x%x\n", MIN(bytes_to_copy, rxbufsize),
- rx_desc_get_buffer(s->rx_desc));
+ rx_desc_get_buffer(s->rx_desc[0]));
/* Copy packet data to emulated DMA buffer */
- cpu_physical_memory_write(rx_desc_get_buffer(s->rx_desc) + rxbuf_offset,
+ cpu_physical_memory_write(rx_desc_get_buffer(s->rx_desc[0]) +
+ rxbuf_offset,
rxbuf_ptr, MIN(bytes_to_copy, rxbufsize));
rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
bytes_to_copy -= MIN(bytes_to_copy, rxbufsize);
/* Update the descriptor. */
if (first_desc) {
- rx_desc_set_sof(s->rx_desc);
+ rx_desc_set_sof(s->rx_desc[0]);
first_desc = false;
}
if (bytes_to_copy == 0) {
- rx_desc_set_eof(s->rx_desc);
- rx_desc_set_length(s->rx_desc, size);
+ rx_desc_set_eof(s->rx_desc[0]);
+ rx_desc_set_length(s->rx_desc[0], size);
}
- rx_desc_set_ownership(s->rx_desc);
+ rx_desc_set_ownership(s->rx_desc[0]);
switch (maf) {
case GEM_RX_PROMISCUOUS_ACCEPT:
break;
case GEM_RX_BROADCAST_ACCEPT:
- rx_desc_set_broadcast(s->rx_desc);
+ rx_desc_set_broadcast(s->rx_desc[0]);
break;
case GEM_RX_UNICAST_HASH_ACCEPT:
- rx_desc_set_unicast_hash(s->rx_desc);
+ rx_desc_set_unicast_hash(s->rx_desc[0]);
break;
case GEM_RX_MULTICAST_HASH_ACCEPT:
- rx_desc_set_multicast_hash(s->rx_desc);
+ rx_desc_set_multicast_hash(s->rx_desc[0]);
break;
case GEM_RX_REJECT:
abort();
default: /* SAR */
- rx_desc_set_sar(s->rx_desc, maf);
+ rx_desc_set_sar(s->rx_desc[0], maf);
}
/* Descriptor write-back. */
- cpu_physical_memory_write(s->rx_desc_addr,
- (uint8_t *)s->rx_desc, sizeof(s->rx_desc));
+ cpu_physical_memory_write(s->rx_desc_addr[0],
+ (uint8_t *)s->rx_desc[0],
+ sizeof(s->rx_desc[0]));
/* Next descriptor */
- if (rx_desc_get_wrap(s->rx_desc)) {
+ if (rx_desc_get_wrap(s->rx_desc[0])) {
DB_PRINT("wrapping RX descriptor list\n");
- s->rx_desc_addr = s->regs[GEM_RXQBASE];
+ s->rx_desc_addr[0] = s->regs[GEM_RXQBASE];
} else {
DB_PRINT("incrementing RX descriptor list\n");
- s->rx_desc_addr += 8;
+ s->rx_desc_addr[0] += 8;
}
gem_get_rx_desc(s);
}
@@ -855,7 +857,7 @@ static void gem_transmit(CadenceGEMState *s)
total_bytes = 0;
/* read current descriptor */
- packet_desc_addr = s->tx_desc_addr;
+ packet_desc_addr = s->tx_desc_addr[0];
DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
cpu_physical_memory_read(packet_desc_addr,
@@ -902,18 +904,18 @@ static void gem_transmit(CadenceGEMState *s)
/* Modify the 1st descriptor of this packet to be owned by
* the processor.
*/
- cpu_physical_memory_read(s->tx_desc_addr, (uint8_t *)desc_first,
+ cpu_physical_memory_read(s->tx_desc_addr[0], (uint8_t *)desc_first,
sizeof(desc_first));
tx_desc_set_used(desc_first);
- cpu_physical_memory_write(s->tx_desc_addr, (uint8_t *)desc_first,
+ cpu_physical_memory_write(s->tx_desc_addr[0], (uint8_t *)desc_first,
sizeof(desc_first));
/* Advance the hardware current descriptor past this packet */
if (tx_desc_get_wrap(desc)) {
- s->tx_desc_addr = s->regs[GEM_TXQBASE];
+ s->tx_desc_addr[0] = s->regs[GEM_TXQBASE];
} else {
- s->tx_desc_addr = packet_desc_addr + 8;
+ s->tx_desc_addr[0] = packet_desc_addr + 8;
}
- DB_PRINT("TX descriptor next: 0x%08x\n", s->tx_desc_addr);
+ DB_PRINT("TX descriptor next: 0x%08x\n", s->tx_desc_addr[0]);
s->regs[GEM_TXSTATUS] |= GEM_TXSTATUS_TXCMPL;
s->regs[GEM_ISR] |= GEM_INT_TXCMPL & ~(s->regs[GEM_IMR]);
@@ -1076,7 +1078,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
switch (offset) {
case GEM_ISR:
DB_PRINT("lowering irq on ISR read\n");
- qemu_set_irq(s->irq, 0);
+ qemu_set_irq(s->irq[0], 0);
break;
case GEM_PHYMNTNC:
if (retval & GEM_PHYMNTNC_OP_R) {
@@ -1139,7 +1141,7 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
}
if (!(val & GEM_NWCTRL_TXENA)) {
/* Reset to start of Q when transmit disabled. */
- s->tx_desc_addr = s->regs[GEM_TXQBASE];
+ s->tx_desc_addr[0] = s->regs[GEM_TXQBASE];
}
if (gem_can_receive(qemu_get_queue(s->nic))) {
qemu_flush_queued_packets(qemu_get_queue(s->nic));
@@ -1150,10 +1152,10 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
gem_update_int_status(s);
break;
case GEM_RXQBASE:
- s->rx_desc_addr = val;
+ s->rx_desc_addr[0] = val;
break;
case GEM_TXQBASE:
- s->tx_desc_addr = val;
+ s->tx_desc_addr[0] = val;
break;
case GEM_RXSTATUS:
gem_update_int_status(s);
@@ -1218,7 +1220,7 @@ static void gem_realize(DeviceState *dev, Error **errp)
{
CadenceGEMState *s = CADENCE_GEM(dev);
- sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
+ sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[0]);
qemu_macaddr_default_if_unset(&s->conf.macaddr);
@@ -1247,8 +1249,10 @@ static const VMStateDescription vmstate_cadence_gem = {
VMSTATE_UINT32_ARRAY(regs, CadenceGEMState, CADENCE_GEM_MAXREG),
VMSTATE_UINT16_ARRAY(phy_regs, CadenceGEMState, 32),
VMSTATE_UINT8(phy_loop, CadenceGEMState),
- VMSTATE_UINT32(rx_desc_addr, CadenceGEMState),
- VMSTATE_UINT32(tx_desc_addr, CadenceGEMState),
+ VMSTATE_UINT32_ARRAY(rx_desc_addr, CadenceGEMState,
+ MAX_PRIORITY_QUEUES),
+ VMSTATE_UINT32_ARRAY(tx_desc_addr, CadenceGEMState,
+ MAX_PRIORITY_QUEUES),
VMSTATE_BOOL_ARRAY(sar_active, CadenceGEMState, 4),
VMSTATE_END_OF_LIST(),
}
diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
index f2e08e3..77e0582 100644
--- a/include/hw/net/cadence_gem.h
+++ b/include/hw/net/cadence_gem.h
@@ -32,6 +32,8 @@
#define CADENCE_GEM_MAXREG (0x00000640/4) /* Last valid GEM address */
+#define MAX_PRIORITY_QUEUES 8
+
typedef struct CadenceGEMState {
/*< private >*/
SysBusDevice parent_obj;
@@ -40,7 +42,10 @@ typedef struct CadenceGEMState {
MemoryRegion iomem;
NICState *nic;
NICConf conf;
- qemu_irq irq;
+ qemu_irq irq[MAX_PRIORITY_QUEUES];
+
+ /* Static properties */
+ uint8_t num_priority_queues;
/* GEM registers backing store */
uint32_t regs[CADENCE_GEM_MAXREG];
@@ -59,12 +64,12 @@ typedef struct CadenceGEMState {
uint8_t phy_loop; /* Are we in phy loopback? */
/* The current DMA descriptor pointers */
- uint32_t rx_desc_addr;
- uint32_t tx_desc_addr;
+ uint32_t rx_desc_addr[MAX_PRIORITY_QUEUES];
+ uint32_t tx_desc_addr[MAX_PRIORITY_QUEUES];
uint8_t can_rx_state; /* Debug only */
- unsigned rx_desc[2];
+ unsigned rx_desc[MAX_PRIORITY_QUEUES][2];
bool sar_active[4];
} CadenceGEMState;
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v1 3/5] cadence_gem: Add queue support
2016-07-11 23:20 [Qemu-devel] [PATCH v1 0/5] Add support for Cadence GEM priority queues Alistair Francis
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 1/5] cadence_gem: QOMify Cadence GEM Alistair Francis
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 2/5] cadence_gem: Arrayify Alistair Francis
@ 2016-07-11 23:20 ` Alistair Francis
2016-07-25 15:46 ` Peter Maydell
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 4/5] cadence_gem: Correct indentation Alistair Francis
` (2 subsequent siblings)
5 siblings, 1 reply; 19+ messages in thread
From: Alistair Francis @ 2016-07-11 23:20 UTC (permalink / raw)
To: qemu-devel, qemu-arm; +Cc: alistair.francis, crosthwaitepeter, peter.maydell
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
There is a indentation error in this patch in the gem_transmit function.
I have written it like that to make it easier to see the changes. It is
fixed in the next patch.
hw/net/cadence_gem.c | 157 ++++++++++++++++++++++++++++++++-----------
include/hw/net/cadence_gem.h | 2 +-
2 files changed, 118 insertions(+), 41 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 2dabad5..c80e833 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -141,6 +141,29 @@
#define GEM_DESCONF6 (0x00000294/4)
#define GEM_DESCONF7 (0x00000298/4)
+#define GEM_INT_Q1_STATUS (0x00000400 / 4)
+
+#define GEM_TRANSMIT_Q1_PTR (0x00000440 / 4)
+#define GEM_TRANSMIT_Q15_PTR (GEM_TRANSMIT_Q1_PTR + 14)
+
+#define GEM_RECEIVE_Q1_PTR (0x00000480 / 4)
+#define GEM_RECEIVE_Q15_PTR (GEM_RECEIVE_Q1_PTR + 14)
+
+#define GEM_INT_Q1_ENABLE (0x00000600 / 4)
+#define GEM_INT_Q7_ENABLE (GEM_INT_Q1_ENABLE + 6)
+#define GEM_INT_Q8_ENABLE (0x00000660 / 4)
+#define GEM_INT_Q15_ENABLE (GEM_INT_Q8_ENABLE + 7)
+
+#define GEM_INT_Q1_DISABLE (0x00000620 / 4)
+#define GEM_INT_Q7_DISABLE (GEM_INT_Q1_DISABLE + 6)
+#define GEM_INT_Q8_DISABLE (0x00000680 / 4)
+#define GEM_INT_Q15_DISABLE (GEM_INT_Q8_DISABLE + 7)
+
+#define GEM_INT_Q1_MASK (0x00000640 / 4)
+#define GEM_INT_Q7_MASK (GEM_INT_Q1_MASK + 6)
+#define GEM_INT_Q8_MASK (0x000006A0 / 4)
+#define GEM_INT_Q15_MASK (GEM_INT_Q8_MASK + 7)
+
/*****************************************/
#define GEM_NWCTRL_TXSTART 0x00000200 /* Transmit Enable */
#define GEM_NWCTRL_TXENA 0x00000008 /* Transmit Enable */
@@ -284,9 +307,9 @@ static inline unsigned tx_desc_get_length(unsigned *desc)
return desc[1] & DESC_1_LENGTH;
}
-static inline void print_gem_tx_desc(unsigned *desc)
+static inline void print_gem_tx_desc(unsigned *desc, uint8_t queue)
{
- DB_PRINT("TXDESC:\n");
+ DB_PRINT("TXDESC (queue %" PRId8 "):\n", queue);
DB_PRINT("bufaddr: 0x%08x\n", *desc);
DB_PRINT("used_hw: %d\n", tx_desc_get_used(desc));
DB_PRINT("wrap: %d\n", tx_desc_get_wrap(desc));
@@ -416,6 +439,7 @@ static void phy_update_link(CadenceGEMState *s)
static int gem_can_receive(NetClientState *nc)
{
CadenceGEMState *s;
+ int i;
s = qemu_get_nic_opaque(nc);
@@ -428,18 +452,20 @@ static int gem_can_receive(NetClientState *nc)
return 0;
}
- if (rx_desc_get_ownership(s->rx_desc[0]) == 1) {
- if (s->can_rx_state != 2) {
- s->can_rx_state = 2;
- DB_PRINT("can't receive - busy buffer descriptor 0x%x\n",
- s->rx_desc_addr[0]);
+ for (i = 0; i < s->num_priority_queues; i++) {
+ if (rx_desc_get_ownership(s->rx_desc[i]) == 1) {
+ if (s->can_rx_state != 2) {
+ s->can_rx_state = 2;
+ DB_PRINT("can't receive - busy buffer descriptor (q%d) 0x%x\n",
+ i, s->rx_desc_addr[i]);
+ }
+ return 0;
}
- return 0;
}
if (s->can_rx_state != 0) {
s->can_rx_state = 0;
- DB_PRINT("can receive 0x%x\n", s->rx_desc_addr[0]);
+ DB_PRINT("can receive\n");
}
return 1;
}
@@ -450,9 +476,16 @@ static int gem_can_receive(NetClientState *nc)
*/
static void gem_update_int_status(CadenceGEMState *s)
{
- if (s->regs[GEM_ISR]) {
- DB_PRINT("asserting int. (0x%08x)\n", s->regs[GEM_ISR]);
- qemu_set_irq(s->irq[0], 1);
+ int i;
+
+ for (i = 0; i < s->num_priority_queues; ++i) {
+ /* There seems to be no sane way to see which queue triggered the
+ * interrupt.
+ */
+ if (s->regs[GEM_ISR]) {
+ DB_PRINT("asserting int. q=%d)\n", i);
+ qemu_set_irq(s->irq[i], 1);
+ }
}
}
@@ -601,17 +634,18 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
return GEM_RX_REJECT;
}
-static void gem_get_rx_desc(CadenceGEMState *s)
+static void gem_get_rx_desc(CadenceGEMState *s, int q)
{
- DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[0]);
+ DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[q]);
/* read current descriptor */
cpu_physical_memory_read(s->rx_desc_addr[0],
(uint8_t *)s->rx_desc[0], sizeof(s->rx_desc[0]));
/* Descriptor owned by software ? */
- if (rx_desc_get_ownership(s->rx_desc[0]) == 1) {
+ if (rx_desc_get_ownership(s->rx_desc[q]) == 1 &&
+ s->num_priority_queues == 1) {
DB_PRINT("descriptor 0x%x owned by sw.\n",
- (unsigned)s->rx_desc_addr[0]);
+ (unsigned)s->rx_desc_addr[q]);
s->regs[GEM_RXSTATUS] |= GEM_RXSTATUS_NOBUF;
s->regs[GEM_ISR] |= GEM_INT_RXUSED & ~(s->regs[GEM_IMR]);
/* Handle interrupt consequences */
@@ -632,6 +666,7 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
uint8_t *rxbuf_ptr;
bool first_desc = true;
int maf;
+ int q = 0;
s = qemu_get_nic_opaque(nc);
@@ -729,31 +764,31 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
/* Update the descriptor. */
if (first_desc) {
- rx_desc_set_sof(s->rx_desc[0]);
+ rx_desc_set_sof(s->rx_desc[q]);
first_desc = false;
}
if (bytes_to_copy == 0) {
- rx_desc_set_eof(s->rx_desc[0]);
- rx_desc_set_length(s->rx_desc[0], size);
+ rx_desc_set_eof(s->rx_desc[q]);
+ rx_desc_set_length(s->rx_desc[q], size);
}
- rx_desc_set_ownership(s->rx_desc[0]);
+ rx_desc_set_ownership(s->rx_desc[q]);
switch (maf) {
case GEM_RX_PROMISCUOUS_ACCEPT:
break;
case GEM_RX_BROADCAST_ACCEPT:
- rx_desc_set_broadcast(s->rx_desc[0]);
+ rx_desc_set_broadcast(s->rx_desc[q]);
break;
case GEM_RX_UNICAST_HASH_ACCEPT:
- rx_desc_set_unicast_hash(s->rx_desc[0]);
+ rx_desc_set_unicast_hash(s->rx_desc[q]);
break;
case GEM_RX_MULTICAST_HASH_ACCEPT:
- rx_desc_set_multicast_hash(s->rx_desc[0]);
+ rx_desc_set_multicast_hash(s->rx_desc[q]);
break;
case GEM_RX_REJECT:
abort();
default: /* SAR */
- rx_desc_set_sar(s->rx_desc[0], maf);
+ rx_desc_set_sar(s->rx_desc[q], maf);
}
/* Descriptor write-back. */
@@ -762,14 +797,15 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
sizeof(s->rx_desc[0]));
/* Next descriptor */
- if (rx_desc_get_wrap(s->rx_desc[0])) {
+ if (rx_desc_get_wrap(s->rx_desc[q])) {
DB_PRINT("wrapping RX descriptor list\n");
s->rx_desc_addr[0] = s->regs[GEM_RXQBASE];
} else {
DB_PRINT("incrementing RX descriptor list\n");
s->rx_desc_addr[0] += 8;
}
- gem_get_rx_desc(s);
+
+ gem_get_rx_desc(s, q);
}
/* Count it */
@@ -841,6 +877,7 @@ static void gem_transmit(CadenceGEMState *s)
uint8_t tx_packet[2048];
uint8_t *p;
unsigned total_bytes;
+ int8_t q;
/* Do nothing if transmit is not enabled. */
if (!(s->regs[GEM_NWCTRL] & GEM_NWCTRL_TXENA)) {
@@ -856,8 +893,9 @@ static void gem_transmit(CadenceGEMState *s)
p = tx_packet;
total_bytes = 0;
+ for (q = s->num_priority_queues - 1; q >= 0; q--) {
/* read current descriptor */
- packet_desc_addr = s->tx_desc_addr[0];
+ packet_desc_addr = s->tx_desc_addr[q];
DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
cpu_physical_memory_read(packet_desc_addr,
@@ -869,7 +907,7 @@ static void gem_transmit(CadenceGEMState *s)
if (!(s->regs[GEM_NWCTRL] & GEM_NWCTRL_TXENA)) {
return;
}
- print_gem_tx_desc(desc);
+ print_gem_tx_desc(desc, q);
/* The real hardware would eat this (and possibly crash).
* For QEMU let's lend a helping hand.
@@ -904,18 +942,18 @@ static void gem_transmit(CadenceGEMState *s)
/* Modify the 1st descriptor of this packet to be owned by
* the processor.
*/
- cpu_physical_memory_read(s->tx_desc_addr[0], (uint8_t *)desc_first,
+ cpu_physical_memory_read(s->tx_desc_addr[q], (uint8_t *)desc_first,
sizeof(desc_first));
tx_desc_set_used(desc_first);
- cpu_physical_memory_write(s->tx_desc_addr[0], (uint8_t *)desc_first,
+ cpu_physical_memory_write(s->tx_desc_addr[q], (uint8_t *)desc_first,
sizeof(desc_first));
/* Advance the hardware current descriptor past this packet */
if (tx_desc_get_wrap(desc)) {
- s->tx_desc_addr[0] = s->regs[GEM_TXQBASE];
+ s->tx_desc_addr[q] = s->regs[GEM_TXQBASE];
} else {
- s->tx_desc_addr[0] = packet_desc_addr + 8;
+ s->tx_desc_addr[q] = packet_desc_addr + 8;
}
- DB_PRINT("TX descriptor next: 0x%08x\n", s->tx_desc_addr[0]);
+ DB_PRINT("TX descriptor next: 0x%08x\n", s->tx_desc_addr[q]);
s->regs[GEM_TXSTATUS] |= GEM_TXSTATUS_TXCMPL;
s->regs[GEM_ISR] |= GEM_INT_TXCMPL & ~(s->regs[GEM_IMR]);
@@ -961,6 +999,7 @@ static void gem_transmit(CadenceGEMState *s)
s->regs[GEM_ISR] |= GEM_INT_TXUSED & ~(s->regs[GEM_IMR]);
gem_update_int_status(s);
}
+ }
}
static void gem_phy_reset(CadenceGEMState *s)
@@ -1067,7 +1106,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
{
CadenceGEMState *s;
uint32_t retval;
-
+ int i;
s = (CadenceGEMState *)opaque;
offset >>= 2;
@@ -1077,8 +1116,10 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
switch (offset) {
case GEM_ISR:
- DB_PRINT("lowering irq on ISR read\n");
- qemu_set_irq(s->irq[0], 0);
+ DB_PRINT("lowering irqs on ISR read\n");
+ for (i = 0; i < s->num_priority_queues; ++i) {
+ qemu_set_irq(s->irq[i], 0);
+ }
break;
case GEM_PHYMNTNC:
if (retval & GEM_PHYMNTNC_OP_R) {
@@ -1103,6 +1144,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
retval &= ~(s->regs_wo[offset]);
DB_PRINT("0x%08x\n", retval);
+ gem_update_int_status(s);
return retval;
}
@@ -1115,6 +1157,7 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
{
CadenceGEMState *s = (CadenceGEMState *)opaque;
uint32_t readonly;
+ int i;
DB_PRINT("offset: 0x%04x write: 0x%08x ", (unsigned)offset, (unsigned)val);
offset >>= 2;
@@ -1134,14 +1177,18 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
switch (offset) {
case GEM_NWCTRL:
if (val & GEM_NWCTRL_RXENA) {
- gem_get_rx_desc(s);
+ for (i = 0; i < s->num_priority_queues; ++i) {
+ gem_get_rx_desc(s, i);
+ }
}
if (val & GEM_NWCTRL_TXSTART) {
gem_transmit(s);
}
if (!(val & GEM_NWCTRL_TXENA)) {
/* Reset to start of Q when transmit disabled. */
- s->tx_desc_addr[0] = s->regs[GEM_TXQBASE];
+ for (i = 0; i < s->num_priority_queues; i++) {
+ s->tx_desc_addr[i] = s->regs[GEM_TXQBASE];
+ }
}
if (gem_can_receive(qemu_get_queue(s->nic))) {
qemu_flush_queued_packets(qemu_get_queue(s->nic));
@@ -1154,9 +1201,15 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
case GEM_RXQBASE:
s->rx_desc_addr[0] = val;
break;
+ case GEM_RECEIVE_Q1_PTR ... GEM_RECEIVE_Q15_PTR:
+ s->rx_desc_addr[offset - GEM_RECEIVE_Q1_PTR + 1] = val;
+ break;
case GEM_TXQBASE:
s->tx_desc_addr[0] = val;
break;
+ case GEM_TRANSMIT_Q1_PTR ... GEM_TRANSMIT_Q15_PTR:
+ s->tx_desc_addr[offset - GEM_TRANSMIT_Q1_PTR + 1] = val;
+ break;
case GEM_RXSTATUS:
gem_update_int_status(s);
break;
@@ -1164,10 +1217,26 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
s->regs[GEM_IMR] &= ~val;
gem_update_int_status(s);
break;
+ case GEM_INT_Q1_ENABLE ... GEM_INT_Q7_ENABLE:
+ s->regs[GEM_INT_Q1_MASK + offset - GEM_INT_Q1_ENABLE] &= ~val;
+ gem_update_int_status(s);
+ break;
+ case GEM_INT_Q8_ENABLE ... GEM_INT_Q15_ENABLE:
+ s->regs[GEM_INT_Q8_MASK + offset - GEM_INT_Q8_ENABLE] &= ~val;
+ gem_update_int_status(s);
+ break;
case GEM_IDR:
s->regs[GEM_IMR] |= val;
gem_update_int_status(s);
break;
+ case GEM_INT_Q1_DISABLE ... GEM_INT_Q7_DISABLE:
+ s->regs[GEM_INT_Q1_MASK + offset - GEM_INT_Q1_DISABLE] |= val;
+ gem_update_int_status(s);
+ break;
+ case GEM_INT_Q8_DISABLE ... GEM_INT_Q15_DISABLE:
+ s->regs[GEM_INT_Q8_MASK + offset - GEM_INT_Q8_DISABLE] |= val;
+ gem_update_int_status(s);
+ break;
case GEM_SPADDR1LO:
case GEM_SPADDR2LO:
case GEM_SPADDR3LO:
@@ -1204,8 +1273,11 @@ static const MemoryRegionOps gem_ops = {
static void gem_set_link(NetClientState *nc)
{
+ CadenceGEMState *s = qemu_get_nic_opaque(nc);
+
DB_PRINT("\n");
- phy_update_link(qemu_get_nic_opaque(nc));
+ phy_update_link(s);
+ gem_update_int_status(s);
}
static NetClientInfo net_gem_info = {
@@ -1219,8 +1291,11 @@ static NetClientInfo net_gem_info = {
static void gem_realize(DeviceState *dev, Error **errp)
{
CadenceGEMState *s = CADENCE_GEM(dev);
+ int i;
- sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[0]);
+ for (i = 0; i < s->num_priority_queues; ++i) {
+ sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[i]);
+ }
qemu_macaddr_default_if_unset(&s->conf.macaddr);
@@ -1260,6 +1335,8 @@ static const VMStateDescription vmstate_cadence_gem = {
static Property gem_properties[] = {
DEFINE_NIC_PROPERTIES(CadenceGEMState, conf),
+ DEFINE_PROP_UINT8("num-priority-queues", CadenceGEMState,
+ num_priority_queues, 1),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
index 77e0582..60b3ab0 100644
--- a/include/hw/net/cadence_gem.h
+++ b/include/hw/net/cadence_gem.h
@@ -30,7 +30,7 @@
#include "net/net.h"
#include "hw/sysbus.h"
-#define CADENCE_GEM_MAXREG (0x00000640/4) /* Last valid GEM address */
+#define CADENCE_GEM_MAXREG (0x00000800 / 4) /* Last valid GEM address */
#define MAX_PRIORITY_QUEUES 8
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v1 4/5] cadence_gem: Correct indentation
2016-07-11 23:20 [Qemu-devel] [PATCH v1 0/5] Add support for Cadence GEM priority queues Alistair Francis
` (2 preceding siblings ...)
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 3/5] cadence_gem: Add queue support Alistair Francis
@ 2016-07-11 23:20 ` Alistair Francis
2016-07-25 15:48 ` Peter Maydell
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 5/5] xlnx-zynqmp: Set the number of priority queues Alistair Francis
2016-07-12 14:25 ` [Qemu-devel] [PATCH v1 0/5] Add support for Cadence GEM " Peter Maydell
5 siblings, 1 reply; 19+ messages in thread
From: Alistair Francis @ 2016-07-11 23:20 UTC (permalink / raw)
To: qemu-devel, qemu-arm; +Cc: alistair.francis, crosthwaitepeter, peter.maydell
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
hw/net/cadence_gem.c | 182 +++++++++++++++++++++++++--------------------------
1 file changed, 91 insertions(+), 91 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index c80e833..1c09756 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -894,111 +894,111 @@ static void gem_transmit(CadenceGEMState *s)
total_bytes = 0;
for (q = s->num_priority_queues - 1; q >= 0; q--) {
- /* read current descriptor */
- packet_desc_addr = s->tx_desc_addr[q];
-
- DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
- cpu_physical_memory_read(packet_desc_addr,
- (uint8_t *)desc, sizeof(desc));
- /* Handle all descriptors owned by hardware */
- while (tx_desc_get_used(desc) == 0) {
+ /* read current descriptor */
+ packet_desc_addr = s->tx_desc_addr[q];
- /* Do nothing if transmit is not enabled. */
- if (!(s->regs[GEM_NWCTRL] & GEM_NWCTRL_TXENA)) {
- return;
- }
- print_gem_tx_desc(desc, q);
-
- /* The real hardware would eat this (and possibly crash).
- * For QEMU let's lend a helping hand.
- */
- if ((tx_desc_get_buffer(desc) == 0) ||
- (tx_desc_get_length(desc) == 0)) {
- DB_PRINT("Invalid TX descriptor @ 0x%x\n",
- (unsigned)packet_desc_addr);
- break;
- }
-
- if (tx_desc_get_length(desc) > sizeof(tx_packet) - (p - tx_packet)) {
- DB_PRINT("TX descriptor @ 0x%x too large: size 0x%x space 0x%x\n",
- (unsigned)packet_desc_addr,
- (unsigned)tx_desc_get_length(desc),
- sizeof(tx_packet) - (p - tx_packet));
- break;
- }
-
- /* Gather this fragment of the packet from "dma memory" to our contig.
- * buffer.
- */
- cpu_physical_memory_read(tx_desc_get_buffer(desc), p,
- tx_desc_get_length(desc));
- p += tx_desc_get_length(desc);
- total_bytes += tx_desc_get_length(desc);
+ DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
+ cpu_physical_memory_read(packet_desc_addr,
+ (uint8_t *)desc, sizeof(desc));
+ /* Handle all descriptors owned by hardware */
+ while (tx_desc_get_used(desc) == 0) {
- /* Last descriptor for this packet; hand the whole thing off */
- if (tx_desc_get_last(desc)) {
- unsigned desc_first[2];
+ /* Do nothing if transmit is not enabled. */
+ if (!(s->regs[GEM_NWCTRL] & GEM_NWCTRL_TXENA)) {
+ return;
+ }
+ print_gem_tx_desc(desc, q);
- /* Modify the 1st descriptor of this packet to be owned by
- * the processor.
+ /* The real hardware would eat this (and possibly crash).
+ * For QEMU let's lend a helping hand.
*/
- cpu_physical_memory_read(s->tx_desc_addr[q], (uint8_t *)desc_first,
- sizeof(desc_first));
- tx_desc_set_used(desc_first);
- cpu_physical_memory_write(s->tx_desc_addr[q], (uint8_t *)desc_first,
- sizeof(desc_first));
- /* Advance the hardware current descriptor past this packet */
- if (tx_desc_get_wrap(desc)) {
- s->tx_desc_addr[q] = s->regs[GEM_TXQBASE];
- } else {
- s->tx_desc_addr[q] = packet_desc_addr + 8;
+ if ((tx_desc_get_buffer(desc) == 0) ||
+ (tx_desc_get_length(desc) == 0)) {
+ DB_PRINT("Invalid TX descriptor @ 0x%x\n",
+ (unsigned)packet_desc_addr);
+ break;
}
- DB_PRINT("TX descriptor next: 0x%08x\n", s->tx_desc_addr[q]);
-
- s->regs[GEM_TXSTATUS] |= GEM_TXSTATUS_TXCMPL;
- s->regs[GEM_ISR] |= GEM_INT_TXCMPL & ~(s->regs[GEM_IMR]);
- /* Handle interrupt consequences */
- gem_update_int_status(s);
-
- /* Is checksum offload enabled? */
- if (s->regs[GEM_DMACFG] & GEM_DMACFG_TXCSUM_OFFL) {
- net_checksum_calculate(tx_packet, total_bytes);
+ if (tx_desc_get_length(desc) > sizeof(tx_packet) - (p - tx_packet)) {
+ DB_PRINT("TX descriptor @ 0x%x too large: size 0x%x space 0x%x\n",
+ (unsigned)packet_desc_addr,
+ (unsigned)tx_desc_get_length(desc),
+ sizeof(tx_packet) - (p - tx_packet));
+ break;
}
- /* Update MAC statistics */
- gem_transmit_updatestats(s, tx_packet, total_bytes);
+ /* Gather this fragment of the packet from "dma memory" to our contig.
+ * buffer.
+ */
+ cpu_physical_memory_read(tx_desc_get_buffer(desc), p,
+ tx_desc_get_length(desc));
+ p += tx_desc_get_length(desc);
+ total_bytes += tx_desc_get_length(desc);
+
+ /* Last descriptor for this packet; hand the whole thing off */
+ if (tx_desc_get_last(desc)) {
+ unsigned desc_first[2];
+
+ /* Modify the 1st descriptor of this packet to be owned by
+ * the processor.
+ */
+ cpu_physical_memory_read(s->tx_desc_addr[q], (uint8_t *)desc_first,
+ sizeof(desc_first));
+ tx_desc_set_used(desc_first);
+ cpu_physical_memory_write(s->tx_desc_addr[q], (uint8_t *)desc_first,
+ sizeof(desc_first));
+ /* Advance the hardware current descriptor past this packet */
+ if (tx_desc_get_wrap(desc)) {
+ s->tx_desc_addr[q] = s->regs[GEM_TXQBASE];
+ } else {
+ s->tx_desc_addr[q] = packet_desc_addr + 8;
+ }
+ DB_PRINT("TX descriptor next: 0x%08x\n", s->tx_desc_addr[q]);
+
+ s->regs[GEM_TXSTATUS] |= GEM_TXSTATUS_TXCMPL;
+ s->regs[GEM_ISR] |= GEM_INT_TXCMPL & ~(s->regs[GEM_IMR]);
+
+ /* Handle interrupt consequences */
+ gem_update_int_status(s);
+
+ /* Is checksum offload enabled? */
+ if (s->regs[GEM_DMACFG] & GEM_DMACFG_TXCSUM_OFFL) {
+ net_checksum_calculate(tx_packet, total_bytes);
+ }
+
+ /* Update MAC statistics */
+ gem_transmit_updatestats(s, tx_packet, total_bytes);
+
+ /* Send the packet somewhere */
+ if (s->phy_loop || (s->regs[GEM_NWCTRL] & GEM_NWCTRL_LOCALLOOP)) {
+ gem_receive(qemu_get_queue(s->nic), tx_packet, total_bytes);
+ } else {
+ qemu_send_packet(qemu_get_queue(s->nic), tx_packet,
+ total_bytes);
+ }
+
+ /* Prepare for next packet */
+ p = tx_packet;
+ total_bytes = 0;
+ }
- /* Send the packet somewhere */
- if (s->phy_loop || (s->regs[GEM_NWCTRL] & GEM_NWCTRL_LOCALLOOP)) {
- gem_receive(qemu_get_queue(s->nic), tx_packet, total_bytes);
+ /* read next descriptor */
+ if (tx_desc_get_wrap(desc)) {
+ tx_desc_set_last(desc);
+ packet_desc_addr = s->regs[GEM_TXQBASE];
} else {
- qemu_send_packet(qemu_get_queue(s->nic), tx_packet,
- total_bytes);
+ packet_desc_addr += 8;
}
-
- /* Prepare for next packet */
- p = tx_packet;
- total_bytes = 0;
+ DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
+ cpu_physical_memory_read(packet_desc_addr,
+ (uint8_t *)desc, sizeof(desc));
}
- /* read next descriptor */
- if (tx_desc_get_wrap(desc)) {
- tx_desc_set_last(desc);
- packet_desc_addr = s->regs[GEM_TXQBASE];
- } else {
- packet_desc_addr += 8;
+ if (tx_desc_get_used(desc)) {
+ s->regs[GEM_TXSTATUS] |= GEM_TXSTATUS_USED;
+ s->regs[GEM_ISR] |= GEM_INT_TXUSED & ~(s->regs[GEM_IMR]);
+ gem_update_int_status(s);
}
- DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
- cpu_physical_memory_read(packet_desc_addr,
- (uint8_t *)desc, sizeof(desc));
- }
-
- if (tx_desc_get_used(desc)) {
- s->regs[GEM_TXSTATUS] |= GEM_TXSTATUS_USED;
- s->regs[GEM_ISR] |= GEM_INT_TXUSED & ~(s->regs[GEM_IMR]);
- gem_update_int_status(s);
- }
}
}
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* [Qemu-devel] [PATCH v1 5/5] xlnx-zynqmp: Set the number of priority queues
2016-07-11 23:20 [Qemu-devel] [PATCH v1 0/5] Add support for Cadence GEM priority queues Alistair Francis
` (3 preceding siblings ...)
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 4/5] cadence_gem: Correct indentation Alistair Francis
@ 2016-07-11 23:20 ` Alistair Francis
2016-07-25 15:50 ` Peter Maydell
2016-07-12 14:25 ` [Qemu-devel] [PATCH v1 0/5] Add support for Cadence GEM " Peter Maydell
5 siblings, 1 reply; 19+ messages in thread
From: Alistair Francis @ 2016-07-11 23:20 UTC (permalink / raw)
To: qemu-devel, qemu-arm; +Cc: alistair.francis, crosthwaitepeter, peter.maydell
Set the ZynqMP number of priority queues to 2.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
hw/arm/xlnx-zynqmp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
index 23c7199..0d86ba3 100644
--- a/hw/arm/xlnx-zynqmp.c
+++ b/hw/arm/xlnx-zynqmp.c
@@ -332,6 +332,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
qemu_check_nic_model(nd, TYPE_CADENCE_GEM);
qdev_set_nic_properties(DEVICE(&s->gem[i]), nd);
}
+ object_property_set_int(OBJECT(&s->gem[i]), 2, "num-priority-queues",
+ &error_abort);
object_property_set_bool(OBJECT(&s->gem[i]), true, "realized", &err);
if (err) {
error_propagate(errp, err);
--
2.7.4
^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v1 0/5] Add support for Cadence GEM priority queues
2016-07-11 23:20 [Qemu-devel] [PATCH v1 0/5] Add support for Cadence GEM priority queues Alistair Francis
` (4 preceding siblings ...)
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 5/5] xlnx-zynqmp: Set the number of priority queues Alistair Francis
@ 2016-07-12 14:25 ` Peter Maydell
2016-07-12 16:34 ` Alistair Francis
5 siblings, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2016-07-12 14:25 UTC (permalink / raw)
To: Alistair Francis; +Cc: QEMU Developers, qemu-arm, Peter Crosthwaite
On 12 July 2016 at 00:20, Alistair Francis <alistair.francis@xilinx.com> wrote:
> This patch series adds initial priority queue support to the Cadence GEM
> device. This is based on original work by Peter C, that has been ported
> to the latest version of QEMU.
>
> There is more GEM work that I'd like to upstream after this, but I
> figured this a good place to start. I have done limited testing on the
> Xilinx machine, including running some of our GEM regressions, although
> they don't cover everything. I can't really think of too many other test
> cases that I can run at the moment.
Hi; I'll get to a proper review at some point in the future, but
for the moment I just wanted to let you know that I'm classifying
this as "not for 2.7", given where we are in the release process.
thanks
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v1 0/5] Add support for Cadence GEM priority queues
2016-07-12 14:25 ` [Qemu-devel] [PATCH v1 0/5] Add support for Cadence GEM " Peter Maydell
@ 2016-07-12 16:34 ` Alistair Francis
0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2016-07-12 16:34 UTC (permalink / raw)
To: Peter Maydell
Cc: Alistair Francis, Peter Crosthwaite, qemu-arm, QEMU Developers
On Tue, Jul 12, 2016 at 7:25 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 12 July 2016 at 00:20, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> This patch series adds initial priority queue support to the Cadence GEM
>> device. This is based on original work by Peter C, that has been ported
>> to the latest version of QEMU.
>>
>> There is more GEM work that I'd like to upstream after this, but I
>> figured this a good place to start. I have done limited testing on the
>> Xilinx machine, including running some of our GEM regressions, although
>> they don't cover everything. I can't really think of too many other test
>> cases that I can run at the moment.
>
> Hi; I'll get to a proper review at some point in the future, but
> for the moment I just wanted to let you know that I'm classifying
> this as "not for 2.7", given where we are in the release process.
That's fine, I should have sent it earlier.
Thanks,
Alistair
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/5] cadence_gem: QOMify Cadence GEM
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 1/5] cadence_gem: QOMify Cadence GEM Alistair Francis
@ 2016-07-25 15:20 ` Peter Maydell
2016-07-25 16:38 ` Alistair Francis
0 siblings, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2016-07-25 15:20 UTC (permalink / raw)
To: Alistair Francis; +Cc: QEMU Developers, qemu-arm, Peter Crosthwaite
On 12 July 2016 at 00:20, Alistair Francis <alistair.francis@xilinx.com> wrote:
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
> hw/net/cadence_gem.c | 27 ++++++++++++++++-----------
> 1 file changed, 16 insertions(+), 11 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 8a4be1e..9d64644 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -1214,24 +1214,29 @@ static NetClientInfo net_gem_info = {
> .link_status_changed = gem_set_link,
> };
>
> -static int gem_init(SysBusDevice *sbd)
> +static void gem_realize(DeviceState *dev, Error **errp)
> {
> - DeviceState *dev = DEVICE(sbd);
> CadenceGEMState *s = CADENCE_GEM(dev);
>
> - DB_PRINT("\n");
> + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
>
> - gem_init_register_masks(s);
> - memory_region_init_io(&s->iomem, OBJECT(s), &gem_ops, s,
> - "enet", sizeof(s->regs));
> - sysbus_init_mmio(sbd, &s->iomem);
> - sysbus_init_irq(sbd, &s->irq);
> qemu_macaddr_default_if_unset(&s->conf.macaddr);
>
> s->nic = qemu_new_nic(&net_gem_info, &s->conf,
> object_get_typename(OBJECT(dev)), dev->id, s);
> +}
> +
> +static void gem_init(Object *obj)
> +{
> + CadenceGEMState *s = CADENCE_GEM(obj);
> + DeviceState *dev = DEVICE(obj);
> +
> + DB_PRINT("\n");
>
> - return 0;
> + gem_init_register_masks(s);
> + memory_region_init_io(&s->iomem, OBJECT(s), &gem_ops, s,
> + "enet", sizeof(s->regs));
> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
> }
I don't understand the logic behind which things are
in init and which in realize here -- why is
sysbus_init_mmio() in init but sysbus_init_irq() in
realize ?
thanks
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v1 2/5] cadence_gem: Arrayify
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 2/5] cadence_gem: Arrayify Alistair Francis
@ 2016-07-25 15:22 ` Peter Maydell
0 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2016-07-25 15:22 UTC (permalink / raw)
To: Alistair Francis; +Cc: QEMU Developers, qemu-arm, Peter Crosthwaite
On 12 July 2016 at 00:20, Alistair Francis <alistair.francis@xilinx.com> wrote:
> Arrayify the Cadence GEM device to prepare for adding priority queue support.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
> This patch has no real effect, I used it to help debug some issues so I
> figured I would leave it in.
An expanded commit message here would be useful, something along
the lines of the hardware having N priority queues and that
the behaviour doesn't yet change because we only use queue 0,
or whatever. The subject line is also a bit cryptic.
> @@ -1247,8 +1249,10 @@ static const VMStateDescription vmstate_cadence_gem = {
> VMSTATE_UINT32_ARRAY(regs, CadenceGEMState, CADENCE_GEM_MAXREG),
> VMSTATE_UINT16_ARRAY(phy_regs, CadenceGEMState, 32),
> VMSTATE_UINT8(phy_loop, CadenceGEMState),
> - VMSTATE_UINT32(rx_desc_addr, CadenceGEMState),
> - VMSTATE_UINT32(tx_desc_addr, CadenceGEMState),
> + VMSTATE_UINT32_ARRAY(rx_desc_addr, CadenceGEMState,
> + MAX_PRIORITY_QUEUES),
> + VMSTATE_UINT32_ARRAY(tx_desc_addr, CadenceGEMState,
> + MAX_PRIORITY_QUEUES),
> VMSTATE_BOOL_ARRAY(sar_active, CadenceGEMState, 4),
> VMSTATE_END_OF_LIST(),
> }
You need to bump the vmstate version numbers if you change the format.
thanks
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v1 3/5] cadence_gem: Add queue support
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 3/5] cadence_gem: Add queue support Alistair Francis
@ 2016-07-25 15:46 ` Peter Maydell
2016-07-25 23:01 ` Alistair Francis
0 siblings, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2016-07-25 15:46 UTC (permalink / raw)
To: Alistair Francis; +Cc: QEMU Developers, qemu-arm, Peter Crosthwaite
On 12 July 2016 at 00:20, Alistair Francis <alistair.francis@xilinx.com> wrote:
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
> There is a indentation error in this patch in the gem_transmit function.
> I have written it like that to make it easier to see the changes. It is
> fixed in the next patch.
>
> hw/net/cadence_gem.c | 157 ++++++++++++++++++++++++++++++++-----------
> include/hw/net/cadence_gem.h | 2 +-
> 2 files changed, 118 insertions(+), 41 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index 2dabad5..c80e833 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -141,6 +141,29 @@
> #define GEM_DESCONF6 (0x00000294/4)
> #define GEM_DESCONF7 (0x00000298/4)
>
> +#define GEM_INT_Q1_STATUS (0x00000400 / 4)
> +
> +#define GEM_TRANSMIT_Q1_PTR (0x00000440 / 4)
> +#define GEM_TRANSMIT_Q15_PTR (GEM_TRANSMIT_Q1_PTR + 14)
> +
> +#define GEM_RECEIVE_Q1_PTR (0x00000480 / 4)
> +#define GEM_RECEIVE_Q15_PTR (GEM_RECEIVE_Q1_PTR + 14)
> +
> +#define GEM_INT_Q1_ENABLE (0x00000600 / 4)
> +#define GEM_INT_Q7_ENABLE (GEM_INT_Q1_ENABLE + 6)
> +#define GEM_INT_Q8_ENABLE (0x00000660 / 4)
> +#define GEM_INT_Q15_ENABLE (GEM_INT_Q8_ENABLE + 7)
> +
> +#define GEM_INT_Q1_DISABLE (0x00000620 / 4)
> +#define GEM_INT_Q7_DISABLE (GEM_INT_Q1_DISABLE + 6)
> +#define GEM_INT_Q8_DISABLE (0x00000680 / 4)
> +#define GEM_INT_Q15_DISABLE (GEM_INT_Q8_DISABLE + 7)
> +
> +#define GEM_INT_Q1_MASK (0x00000640 / 4)
> +#define GEM_INT_Q7_MASK (GEM_INT_Q1_MASK + 6)
> +#define GEM_INT_Q8_MASK (0x000006A0 / 4)
> +#define GEM_INT_Q15_MASK (GEM_INT_Q8_MASK + 7)
> +
> /*****************************************/
> #define GEM_NWCTRL_TXSTART 0x00000200 /* Transmit Enable */
> #define GEM_NWCTRL_TXENA 0x00000008 /* Transmit Enable */
> @@ -284,9 +307,9 @@ static inline unsigned tx_desc_get_length(unsigned *desc)
> return desc[1] & DESC_1_LENGTH;
> }
>
> -static inline void print_gem_tx_desc(unsigned *desc)
> +static inline void print_gem_tx_desc(unsigned *desc, uint8_t queue)
> {
> - DB_PRINT("TXDESC:\n");
> + DB_PRINT("TXDESC (queue %" PRId8 "):\n", queue);
> DB_PRINT("bufaddr: 0x%08x\n", *desc);
> DB_PRINT("used_hw: %d\n", tx_desc_get_used(desc));
> DB_PRINT("wrap: %d\n", tx_desc_get_wrap(desc));
> @@ -416,6 +439,7 @@ static void phy_update_link(CadenceGEMState *s)
> static int gem_can_receive(NetClientState *nc)
> {
> CadenceGEMState *s;
> + int i;
>
> s = qemu_get_nic_opaque(nc);
>
> @@ -428,18 +452,20 @@ static int gem_can_receive(NetClientState *nc)
> return 0;
> }
>
> - if (rx_desc_get_ownership(s->rx_desc[0]) == 1) {
> - if (s->can_rx_state != 2) {
> - s->can_rx_state = 2;
> - DB_PRINT("can't receive - busy buffer descriptor 0x%x\n",
> - s->rx_desc_addr[0]);
> + for (i = 0; i < s->num_priority_queues; i++) {
> + if (rx_desc_get_ownership(s->rx_desc[i]) == 1) {
> + if (s->can_rx_state != 2) {
> + s->can_rx_state = 2;
> + DB_PRINT("can't receive - busy buffer descriptor (q%d) 0x%x\n",
> + i, s->rx_desc_addr[i]);
> + }
> + return 0;
> }
> - return 0;
> }
>
> if (s->can_rx_state != 0) {
> s->can_rx_state = 0;
> - DB_PRINT("can receive 0x%x\n", s->rx_desc_addr[0]);
> + DB_PRINT("can receive\n");
> }
> return 1;
> }
> @@ -450,9 +476,16 @@ static int gem_can_receive(NetClientState *nc)
> */
> static void gem_update_int_status(CadenceGEMState *s)
> {
> - if (s->regs[GEM_ISR]) {
> - DB_PRINT("asserting int. (0x%08x)\n", s->regs[GEM_ISR]);
> - qemu_set_irq(s->irq[0], 1);
> + int i;
> +
> + for (i = 0; i < s->num_priority_queues; ++i) {
> + /* There seems to be no sane way to see which queue triggered the
> + * interrupt.
> + */
> + if (s->regs[GEM_ISR]) {
> + DB_PRINT("asserting int. q=%d)\n", i);
> + qemu_set_irq(s->irq[i], 1);
I don't understand this. The hardware surely can't trigger
every IRQ line simultaneously and identically ?
> + }
> }
> }
>
> @@ -601,17 +634,18 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
> return GEM_RX_REJECT;
> }
>
> -static void gem_get_rx_desc(CadenceGEMState *s)
> +static void gem_get_rx_desc(CadenceGEMState *s, int q)
> {
> - DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[0]);
> + DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[q]);
> /* read current descriptor */
> cpu_physical_memory_read(s->rx_desc_addr[0],
> (uint8_t *)s->rx_desc[0], sizeof(s->rx_desc[0]));
>
> /* Descriptor owned by software ? */
> - if (rx_desc_get_ownership(s->rx_desc[0]) == 1) {
> + if (rx_desc_get_ownership(s->rx_desc[q]) == 1 &&
> + s->num_priority_queues == 1) {
Why only if num_priority_queues is 1? (This is one of those "looks
a bit odd, is the h/w really like this?" questions; it could be right.)
> DB_PRINT("descriptor 0x%x owned by sw.\n",
> - (unsigned)s->rx_desc_addr[0]);
> + (unsigned)s->rx_desc_addr[q]);
> s->regs[GEM_RXSTATUS] |= GEM_RXSTATUS_NOBUF;
> s->regs[GEM_ISR] |= GEM_INT_RXUSED & ~(s->regs[GEM_IMR]);
> /* Handle interrupt consequences */
> @@ -632,6 +666,7 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
> uint8_t *rxbuf_ptr;
> bool first_desc = true;
> int maf;
> + int q = 0;
Shouldn't we be doing something for each queue, rather than just
having a variable that's always 0?
>
> s = qemu_get_nic_opaque(nc);
>
> @@ -729,31 +764,31 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>
> /* Update the descriptor. */
> if (first_desc) {
> - rx_desc_set_sof(s->rx_desc[0]);
> + rx_desc_set_sof(s->rx_desc[q]);
> first_desc = false;
> }
> if (bytes_to_copy == 0) {
> - rx_desc_set_eof(s->rx_desc[0]);
> - rx_desc_set_length(s->rx_desc[0], size);
> + rx_desc_set_eof(s->rx_desc[q]);
> + rx_desc_set_length(s->rx_desc[q], size);
> }
> - rx_desc_set_ownership(s->rx_desc[0]);
> + rx_desc_set_ownership(s->rx_desc[q]);
>
> switch (maf) {
> case GEM_RX_PROMISCUOUS_ACCEPT:
> break;
> case GEM_RX_BROADCAST_ACCEPT:
> - rx_desc_set_broadcast(s->rx_desc[0]);
> + rx_desc_set_broadcast(s->rx_desc[q]);
> break;
> case GEM_RX_UNICAST_HASH_ACCEPT:
> - rx_desc_set_unicast_hash(s->rx_desc[0]);
> + rx_desc_set_unicast_hash(s->rx_desc[q]);
> break;
> case GEM_RX_MULTICAST_HASH_ACCEPT:
> - rx_desc_set_multicast_hash(s->rx_desc[0]);
> + rx_desc_set_multicast_hash(s->rx_desc[q]);
> break;
> case GEM_RX_REJECT:
> abort();
> default: /* SAR */
> - rx_desc_set_sar(s->rx_desc[0], maf);
> + rx_desc_set_sar(s->rx_desc[q], maf);
> }
>
> /* Descriptor write-back. */
> @@ -762,14 +797,15 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
> sizeof(s->rx_desc[0]));
>
> /* Next descriptor */
> - if (rx_desc_get_wrap(s->rx_desc[0])) {
> + if (rx_desc_get_wrap(s->rx_desc[q])) {
> DB_PRINT("wrapping RX descriptor list\n");
> s->rx_desc_addr[0] = s->regs[GEM_RXQBASE];
> } else {
> DB_PRINT("incrementing RX descriptor list\n");
> s->rx_desc_addr[0] += 8;
> }
> - gem_get_rx_desc(s);
> +
> + gem_get_rx_desc(s, q);
> }
>
> /* Count it */
> @@ -841,6 +877,7 @@ static void gem_transmit(CadenceGEMState *s)
> uint8_t tx_packet[2048];
> uint8_t *p;
> unsigned total_bytes;
> + int8_t q;
Why int8_t here but int everywhere else?
>
> /* Do nothing if transmit is not enabled. */
> if (!(s->regs[GEM_NWCTRL] & GEM_NWCTRL_TXENA)) {
> @@ -856,8 +893,9 @@ static void gem_transmit(CadenceGEMState *s)
> p = tx_packet;
> total_bytes = 0;
>
> + for (q = s->num_priority_queues - 1; q >= 0; q--) {
> /* read current descriptor */
> - packet_desc_addr = s->tx_desc_addr[0];
> + packet_desc_addr = s->tx_desc_addr[q];
>
> DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
> cpu_physical_memory_read(packet_desc_addr,
> @@ -869,7 +907,7 @@ static void gem_transmit(CadenceGEMState *s)
> if (!(s->regs[GEM_NWCTRL] & GEM_NWCTRL_TXENA)) {
> return;
> }
> - print_gem_tx_desc(desc);
> + print_gem_tx_desc(desc, q);
>
> /* The real hardware would eat this (and possibly crash).
> * For QEMU let's lend a helping hand.
> @@ -904,18 +942,18 @@ static void gem_transmit(CadenceGEMState *s)
> /* Modify the 1st descriptor of this packet to be owned by
> * the processor.
> */
> - cpu_physical_memory_read(s->tx_desc_addr[0], (uint8_t *)desc_first,
> + cpu_physical_memory_read(s->tx_desc_addr[q], (uint8_t *)desc_first,
> sizeof(desc_first));
> tx_desc_set_used(desc_first);
> - cpu_physical_memory_write(s->tx_desc_addr[0], (uint8_t *)desc_first,
> + cpu_physical_memory_write(s->tx_desc_addr[q], (uint8_t *)desc_first,
> sizeof(desc_first));
> /* Advance the hardware current descriptor past this packet */
> if (tx_desc_get_wrap(desc)) {
> - s->tx_desc_addr[0] = s->regs[GEM_TXQBASE];
> + s->tx_desc_addr[q] = s->regs[GEM_TXQBASE];
> } else {
> - s->tx_desc_addr[0] = packet_desc_addr + 8;
> + s->tx_desc_addr[q] = packet_desc_addr + 8;
> }
> - DB_PRINT("TX descriptor next: 0x%08x\n", s->tx_desc_addr[0]);
> + DB_PRINT("TX descriptor next: 0x%08x\n", s->tx_desc_addr[q]);
>
> s->regs[GEM_TXSTATUS] |= GEM_TXSTATUS_TXCMPL;
> s->regs[GEM_ISR] |= GEM_INT_TXCMPL & ~(s->regs[GEM_IMR]);
> @@ -961,6 +999,7 @@ static void gem_transmit(CadenceGEMState *s)
> s->regs[GEM_ISR] |= GEM_INT_TXUSED & ~(s->regs[GEM_IMR]);
> gem_update_int_status(s);
> }
> + }
> }
>
> static void gem_phy_reset(CadenceGEMState *s)
> @@ -1067,7 +1106,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
> {
> CadenceGEMState *s;
> uint32_t retval;
> -
> + int i;
> s = (CadenceGEMState *)opaque;
>
> offset >>= 2;
> @@ -1077,8 +1116,10 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
>
> switch (offset) {
> case GEM_ISR:
> - DB_PRINT("lowering irq on ISR read\n");
> - qemu_set_irq(s->irq[0], 0);
> + DB_PRINT("lowering irqs on ISR read\n");
> + for (i = 0; i < s->num_priority_queues; ++i) {
> + qemu_set_irq(s->irq[i], 0);
> + }
> break;
> case GEM_PHYMNTNC:
> if (retval & GEM_PHYMNTNC_OP_R) {
> @@ -1103,6 +1144,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
> retval &= ~(s->regs_wo[offset]);
>
> DB_PRINT("0x%08x\n", retval);
> + gem_update_int_status(s);
> return retval;
> }
>
> @@ -1115,6 +1157,7 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
> {
> CadenceGEMState *s = (CadenceGEMState *)opaque;
> uint32_t readonly;
> + int i;
>
> DB_PRINT("offset: 0x%04x write: 0x%08x ", (unsigned)offset, (unsigned)val);
> offset >>= 2;
> @@ -1134,14 +1177,18 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
> switch (offset) {
> case GEM_NWCTRL:
> if (val & GEM_NWCTRL_RXENA) {
> - gem_get_rx_desc(s);
> + for (i = 0; i < s->num_priority_queues; ++i) {
> + gem_get_rx_desc(s, i);
> + }
> }
> if (val & GEM_NWCTRL_TXSTART) {
> gem_transmit(s);
> }
> if (!(val & GEM_NWCTRL_TXENA)) {
> /* Reset to start of Q when transmit disabled. */
> - s->tx_desc_addr[0] = s->regs[GEM_TXQBASE];
> + for (i = 0; i < s->num_priority_queues; i++) {
> + s->tx_desc_addr[i] = s->regs[GEM_TXQBASE];
> + }
> }
> if (gem_can_receive(qemu_get_queue(s->nic))) {
> qemu_flush_queued_packets(qemu_get_queue(s->nic));
> @@ -1154,9 +1201,15 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
> case GEM_RXQBASE:
> s->rx_desc_addr[0] = val;
> break;
> + case GEM_RECEIVE_Q1_PTR ... GEM_RECEIVE_Q15_PTR:
> + s->rx_desc_addr[offset - GEM_RECEIVE_Q1_PTR + 1] = val;
> + break;
> case GEM_TXQBASE:
> s->tx_desc_addr[0] = val;
> break;
> + case GEM_TRANSMIT_Q1_PTR ... GEM_TRANSMIT_Q15_PTR:
> + s->tx_desc_addr[offset - GEM_TRANSMIT_Q1_PTR + 1] = val;
> + break;
> case GEM_RXSTATUS:
> gem_update_int_status(s);
> break;
> @@ -1164,10 +1217,26 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
> s->regs[GEM_IMR] &= ~val;
> gem_update_int_status(s);
> break;
> + case GEM_INT_Q1_ENABLE ... GEM_INT_Q7_ENABLE:
> + s->regs[GEM_INT_Q1_MASK + offset - GEM_INT_Q1_ENABLE] &= ~val;
> + gem_update_int_status(s);
> + break;
> + case GEM_INT_Q8_ENABLE ... GEM_INT_Q15_ENABLE:
> + s->regs[GEM_INT_Q8_MASK + offset - GEM_INT_Q8_ENABLE] &= ~val;
> + gem_update_int_status(s);
> + break;
> case GEM_IDR:
> s->regs[GEM_IMR] |= val;
> gem_update_int_status(s);
> break;
> + case GEM_INT_Q1_DISABLE ... GEM_INT_Q7_DISABLE:
> + s->regs[GEM_INT_Q1_MASK + offset - GEM_INT_Q1_DISABLE] |= val;
> + gem_update_int_status(s);
> + break;
> + case GEM_INT_Q8_DISABLE ... GEM_INT_Q15_DISABLE:
> + s->regs[GEM_INT_Q8_MASK + offset - GEM_INT_Q8_DISABLE] |= val;
> + gem_update_int_status(s);
> + break;
> case GEM_SPADDR1LO:
> case GEM_SPADDR2LO:
> case GEM_SPADDR3LO:
> @@ -1204,8 +1273,11 @@ static const MemoryRegionOps gem_ops = {
>
> static void gem_set_link(NetClientState *nc)
> {
> + CadenceGEMState *s = qemu_get_nic_opaque(nc);
> +
> DB_PRINT("\n");
> - phy_update_link(qemu_get_nic_opaque(nc));
> + phy_update_link(s);
> + gem_update_int_status(s);
> }
>
> static NetClientInfo net_gem_info = {
> @@ -1219,8 +1291,11 @@ static NetClientInfo net_gem_info = {
> static void gem_realize(DeviceState *dev, Error **errp)
> {
> CadenceGEMState *s = CADENCE_GEM(dev);
> + int i;
>
> - sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[0]);
> + for (i = 0; i < s->num_priority_queues; ++i) {
> + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[i]);
> + }
>
> qemu_macaddr_default_if_unset(&s->conf.macaddr);
>
> @@ -1260,6 +1335,8 @@ static const VMStateDescription vmstate_cadence_gem = {
>
> static Property gem_properties[] = {
> DEFINE_NIC_PROPERTIES(CadenceGEMState, conf),
> + DEFINE_PROP_UINT8("num-priority-queues", CadenceGEMState,
> + num_priority_queues, 1),
This should go in the same patch as adding the field to the
CadenceGEMState struct (don't care whether you move the prop
define or the field addition).
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
> index 77e0582..60b3ab0 100644
> --- a/include/hw/net/cadence_gem.h
> +++ b/include/hw/net/cadence_gem.h
> @@ -30,7 +30,7 @@
> #include "net/net.h"
> #include "hw/sysbus.h"
>
> -#define CADENCE_GEM_MAXREG (0x00000640/4) /* Last valid GEM address */
> +#define CADENCE_GEM_MAXREG (0x00000800 / 4) /* Last valid GEM address */
Changing this define changes the migration format (because it
has an array of this size in it) so you need a version bump.
> #define MAX_PRIORITY_QUEUES 8
>
> --
> 2.7.4
thanks
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v1 4/5] cadence_gem: Correct indentation
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 4/5] cadence_gem: Correct indentation Alistair Francis
@ 2016-07-25 15:48 ` Peter Maydell
2016-07-25 16:34 ` Alistair Francis
0 siblings, 1 reply; 19+ messages in thread
From: Peter Maydell @ 2016-07-25 15:48 UTC (permalink / raw)
To: Alistair Francis; +Cc: QEMU Developers, qemu-arm, Peter Crosthwaite
On 12 July 2016 at 00:20, Alistair Francis <alistair.francis@xilinx.com> wrote:
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
> hw/net/cadence_gem.c | 182 +++++++++++++++++++++++++--------------------------
> 1 file changed, 91 insertions(+), 91 deletions(-)
I assume this patch comes out empty if you use 'git show -w';
would be nice to say so specifically in the commit message if true.
thanks
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v1 5/5] xlnx-zynqmp: Set the number of priority queues
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 5/5] xlnx-zynqmp: Set the number of priority queues Alistair Francis
@ 2016-07-25 15:50 ` Peter Maydell
0 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2016-07-25 15:50 UTC (permalink / raw)
To: Alistair Francis; +Cc: QEMU Developers, qemu-arm, Peter Crosthwaite
On 12 July 2016 at 00:20, Alistair Francis <alistair.francis@xilinx.com> wrote:
> Set the ZynqMP number of priority queues to 2.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
>
> hw/arm/xlnx-zynqmp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/hw/arm/xlnx-zynqmp.c b/hw/arm/xlnx-zynqmp.c
> index 23c7199..0d86ba3 100644
> --- a/hw/arm/xlnx-zynqmp.c
> +++ b/hw/arm/xlnx-zynqmp.c
> @@ -332,6 +332,8 @@ static void xlnx_zynqmp_realize(DeviceState *dev, Error **errp)
> qemu_check_nic_model(nd, TYPE_CADENCE_GEM);
> qdev_set_nic_properties(DEVICE(&s->gem[i]), nd);
> }
> + object_property_set_int(OBJECT(&s->gem[i]), 2, "num-priority-queues",
> + &error_abort);
> object_property_set_bool(OBJECT(&s->gem[i]), true, "realized", &err);
> if (err) {
> error_propagate(errp, err);
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v1 4/5] cadence_gem: Correct indentation
2016-07-25 15:48 ` Peter Maydell
@ 2016-07-25 16:34 ` Alistair Francis
2016-07-25 16:55 ` Peter Maydell
0 siblings, 1 reply; 19+ messages in thread
From: Alistair Francis @ 2016-07-25 16:34 UTC (permalink / raw)
To: Peter Maydell
Cc: Alistair Francis, Peter Crosthwaite, qemu-arm, QEMU Developers
On Mon, Jul 25, 2016 at 8:48 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 12 July 2016 at 00:20, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>>
>> hw/net/cadence_gem.c | 182 +++++++++++++++++++++++++--------------------------
>> 1 file changed, 91 insertions(+), 91 deletions(-)
>
> I assume this patch comes out empty if you use 'git show -w';
> would be nice to say so specifically in the commit message if true.
It is empty, I'll update the commit message.
Thanks,
Alistair
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/5] cadence_gem: QOMify Cadence GEM
2016-07-25 15:20 ` Peter Maydell
@ 2016-07-25 16:38 ` Alistair Francis
2016-07-25 16:42 ` Alistair Francis
0 siblings, 1 reply; 19+ messages in thread
From: Alistair Francis @ 2016-07-25 16:38 UTC (permalink / raw)
To: Peter Maydell
Cc: Alistair Francis, Peter Crosthwaite, qemu-arm, QEMU Developers
On Mon, Jul 25, 2016 at 8:20 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 12 July 2016 at 00:20, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>>
>> hw/net/cadence_gem.c | 27 ++++++++++++++++-----------
>> 1 file changed, 16 insertions(+), 11 deletions(-)
>>
>> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
>> index 8a4be1e..9d64644 100644
>> --- a/hw/net/cadence_gem.c
>> +++ b/hw/net/cadence_gem.c
>> @@ -1214,24 +1214,29 @@ static NetClientInfo net_gem_info = {
>> .link_status_changed = gem_set_link,
>> };
>>
>> -static int gem_init(SysBusDevice *sbd)
>> +static void gem_realize(DeviceState *dev, Error **errp)
>> {
>> - DeviceState *dev = DEVICE(sbd);
>> CadenceGEMState *s = CADENCE_GEM(dev);
>>
>> - DB_PRINT("\n");
>> + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
>>
>> - gem_init_register_masks(s);
>> - memory_region_init_io(&s->iomem, OBJECT(s), &gem_ops, s,
>> - "enet", sizeof(s->regs));
>> - sysbus_init_mmio(sbd, &s->iomem);
>> - sysbus_init_irq(sbd, &s->irq);
>> qemu_macaddr_default_if_unset(&s->conf.macaddr);
>>
>> s->nic = qemu_new_nic(&net_gem_info, &s->conf,
>> object_get_typename(OBJECT(dev)), dev->id, s);
>> +}
>> +
>> +static void gem_init(Object *obj)
>> +{
>> + CadenceGEMState *s = CADENCE_GEM(obj);
>> + DeviceState *dev = DEVICE(obj);
>> +
>> + DB_PRINT("\n");
>>
>> - return 0;
>> + gem_init_register_masks(s);
>> + memory_region_init_io(&s->iomem, OBJECT(s), &gem_ops, s,
>> + "enet", sizeof(s->regs));
>> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
>> }
>
> I don't understand the logic behind which things are
> in init and which in realize here -- why is
> sysbus_init_mmio() in init but sysbus_init_irq() in
> realize ?
That was just a mistake. I have moved all of the *_init_* functions to
the main init function.
Thanks,
Alistair
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/5] cadence_gem: QOMify Cadence GEM
2016-07-25 16:38 ` Alistair Francis
@ 2016-07-25 16:42 ` Alistair Francis
2016-07-25 16:55 ` Peter Maydell
0 siblings, 1 reply; 19+ messages in thread
From: Alistair Francis @ 2016-07-25 16:42 UTC (permalink / raw)
To: Alistair Francis
Cc: Peter Maydell, Peter Crosthwaite, qemu-arm, QEMU Developers
On Mon, Jul 25, 2016 at 9:38 AM, Alistair Francis
<alistair.francis@xilinx.com> wrote:
> On Mon, Jul 25, 2016 at 8:20 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 12 July 2016 at 00:20, Alistair Francis <alistair.francis@xilinx.com> wrote:
>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>> ---
>>>
>>> hw/net/cadence_gem.c | 27 ++++++++++++++++-----------
>>> 1 file changed, 16 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
>>> index 8a4be1e..9d64644 100644
>>> --- a/hw/net/cadence_gem.c
>>> +++ b/hw/net/cadence_gem.c
>>> @@ -1214,24 +1214,29 @@ static NetClientInfo net_gem_info = {
>>> .link_status_changed = gem_set_link,
>>> };
>>>
>>> -static int gem_init(SysBusDevice *sbd)
>>> +static void gem_realize(DeviceState *dev, Error **errp)
>>> {
>>> - DeviceState *dev = DEVICE(sbd);
>>> CadenceGEMState *s = CADENCE_GEM(dev);
>>>
>>> - DB_PRINT("\n");
>>> + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
>>>
>>> - gem_init_register_masks(s);
>>> - memory_region_init_io(&s->iomem, OBJECT(s), &gem_ops, s,
>>> - "enet", sizeof(s->regs));
>>> - sysbus_init_mmio(sbd, &s->iomem);
>>> - sysbus_init_irq(sbd, &s->irq);
>>> qemu_macaddr_default_if_unset(&s->conf.macaddr);
>>>
>>> s->nic = qemu_new_nic(&net_gem_info, &s->conf,
>>> object_get_typename(OBJECT(dev)), dev->id, s);
>>> +}
>>> +
>>> +static void gem_init(Object *obj)
>>> +{
>>> + CadenceGEMState *s = CADENCE_GEM(obj);
>>> + DeviceState *dev = DEVICE(obj);
>>> +
>>> + DB_PRINT("\n");
>>>
>>> - return 0;
>>> + gem_init_register_masks(s);
>>> + memory_region_init_io(&s->iomem, OBJECT(s), &gem_ops, s,
>>> + "enet", sizeof(s->regs));
>>> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
>>> }
>>
>> I don't understand the logic behind which things are
>> in init and which in realize here -- why is
>> sysbus_init_mmio() in init but sysbus_init_irq() in
>> realize ?
>
> That was just a mistake. I have moved all of the *_init_* functions to
> the main init function.
As soon as I sent that mail I remembered why it was like that.
Eventually the sysbus_init_irq() function depends on the number of
queues, which is a property so it can't be set in the init.
Do you think the sysbus_init_mmio() should be moved to the realise as well then?
Thanks,
Alistair
>
> Thanks,
>
> Alistair
>
>>
>> thanks
>> -- PMM
>>
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v1 1/5] cadence_gem: QOMify Cadence GEM
2016-07-25 16:42 ` Alistair Francis
@ 2016-07-25 16:55 ` Peter Maydell
0 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2016-07-25 16:55 UTC (permalink / raw)
To: Alistair Francis; +Cc: Peter Crosthwaite, qemu-arm, QEMU Developers
On 25 July 2016 at 17:42, Alistair Francis <alistair.francis@xilinx.com> wrote:
> On Mon, Jul 25, 2016 at 9:38 AM, Alistair Francis
> <alistair.francis@xilinx.com> wrote:
>> On Mon, Jul 25, 2016 at 8:20 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 12 July 2016 at 00:20, Alistair Francis <alistair.francis@xilinx.com> wrote:
>>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>>> ---
>>>>
>>>> hw/net/cadence_gem.c | 27 ++++++++++++++++-----------
>>>> 1 file changed, 16 insertions(+), 11 deletions(-)
>>>>
>>>> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
>>>> index 8a4be1e..9d64644 100644
>>>> --- a/hw/net/cadence_gem.c
>>>> +++ b/hw/net/cadence_gem.c
>>>> @@ -1214,24 +1214,29 @@ static NetClientInfo net_gem_info = {
>>>> .link_status_changed = gem_set_link,
>>>> };
>>>>
>>>> -static int gem_init(SysBusDevice *sbd)
>>>> +static void gem_realize(DeviceState *dev, Error **errp)
>>>> {
>>>> - DeviceState *dev = DEVICE(sbd);
>>>> CadenceGEMState *s = CADENCE_GEM(dev);
>>>>
>>>> - DB_PRINT("\n");
>>>> + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq);
>>>>
>>>> - gem_init_register_masks(s);
>>>> - memory_region_init_io(&s->iomem, OBJECT(s), &gem_ops, s,
>>>> - "enet", sizeof(s->regs));
>>>> - sysbus_init_mmio(sbd, &s->iomem);
>>>> - sysbus_init_irq(sbd, &s->irq);
>>>> qemu_macaddr_default_if_unset(&s->conf.macaddr);
>>>>
>>>> s->nic = qemu_new_nic(&net_gem_info, &s->conf,
>>>> object_get_typename(OBJECT(dev)), dev->id, s);
>>>> +}
>>>> +
>>>> +static void gem_init(Object *obj)
>>>> +{
>>>> + CadenceGEMState *s = CADENCE_GEM(obj);
>>>> + DeviceState *dev = DEVICE(obj);
>>>> +
>>>> + DB_PRINT("\n");
>>>>
>>>> - return 0;
>>>> + gem_init_register_masks(s);
>>>> + memory_region_init_io(&s->iomem, OBJECT(s), &gem_ops, s,
>>>> + "enet", sizeof(s->regs));
>>>> + sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
>>>> }
>>>
>>> I don't understand the logic behind which things are
>>> in init and which in realize here -- why is
>>> sysbus_init_mmio() in init but sysbus_init_irq() in
>>> realize ?
>>
>> That was just a mistake. I have moved all of the *_init_* functions to
>> the main init function.
>
> As soon as I sent that mail I remembered why it was like that.
>
> Eventually the sysbus_init_irq() function depends on the number of
> queues, which is a property so it can't be set in the init.
>
> Do you think the sysbus_init_mmio() should be moved to the realise as well then?
Oh, I see. This is fine, then, but put a note in the commit message
about why the sysbus_init_irq() is where it is.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v1 4/5] cadence_gem: Correct indentation
2016-07-25 16:34 ` Alistair Francis
@ 2016-07-25 16:55 ` Peter Maydell
0 siblings, 0 replies; 19+ messages in thread
From: Peter Maydell @ 2016-07-25 16:55 UTC (permalink / raw)
To: Alistair Francis; +Cc: Peter Crosthwaite, qemu-arm, QEMU Developers
On 25 July 2016 at 17:34, Alistair Francis <alistair.francis@xilinx.com> wrote:
> On Mon, Jul 25, 2016 at 8:48 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 12 July 2016 at 00:20, Alistair Francis <alistair.francis@xilinx.com> wrote:
>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>> ---
>>>
>>> hw/net/cadence_gem.c | 182 +++++++++++++++++++++++++--------------------------
>>> 1 file changed, 91 insertions(+), 91 deletions(-)
>>
>> I assume this patch comes out empty if you use 'git show -w';
>> would be nice to say so specifically in the commit message if true.
>
> It is empty, I'll update the commit message.
In that case
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH v1 3/5] cadence_gem: Add queue support
2016-07-25 15:46 ` Peter Maydell
@ 2016-07-25 23:01 ` Alistair Francis
0 siblings, 0 replies; 19+ messages in thread
From: Alistair Francis @ 2016-07-25 23:01 UTC (permalink / raw)
To: Peter Maydell
Cc: Alistair Francis, Peter Crosthwaite, qemu-arm, QEMU Developers
On Mon, Jul 25, 2016 at 8:46 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 12 July 2016 at 00:20, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>>
>> There is a indentation error in this patch in the gem_transmit function.
>> I have written it like that to make it easier to see the changes. It is
>> fixed in the next patch.
>>
>> hw/net/cadence_gem.c | 157 ++++++++++++++++++++++++++++++++-----------
>> include/hw/net/cadence_gem.h | 2 +-
>> 2 files changed, 118 insertions(+), 41 deletions(-)
>>
>> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
>> index 2dabad5..c80e833 100644
>> --- a/hw/net/cadence_gem.c
>> +++ b/hw/net/cadence_gem.c
>> @@ -141,6 +141,29 @@
>> #define GEM_DESCONF6 (0x00000294/4)
>> #define GEM_DESCONF7 (0x00000298/4)
>>
>> +#define GEM_INT_Q1_STATUS (0x00000400 / 4)
>> +
>> +#define GEM_TRANSMIT_Q1_PTR (0x00000440 / 4)
>> +#define GEM_TRANSMIT_Q15_PTR (GEM_TRANSMIT_Q1_PTR + 14)
>> +
>> +#define GEM_RECEIVE_Q1_PTR (0x00000480 / 4)
>> +#define GEM_RECEIVE_Q15_PTR (GEM_RECEIVE_Q1_PTR + 14)
>> +
>> +#define GEM_INT_Q1_ENABLE (0x00000600 / 4)
>> +#define GEM_INT_Q7_ENABLE (GEM_INT_Q1_ENABLE + 6)
>> +#define GEM_INT_Q8_ENABLE (0x00000660 / 4)
>> +#define GEM_INT_Q15_ENABLE (GEM_INT_Q8_ENABLE + 7)
>> +
>> +#define GEM_INT_Q1_DISABLE (0x00000620 / 4)
>> +#define GEM_INT_Q7_DISABLE (GEM_INT_Q1_DISABLE + 6)
>> +#define GEM_INT_Q8_DISABLE (0x00000680 / 4)
>> +#define GEM_INT_Q15_DISABLE (GEM_INT_Q8_DISABLE + 7)
>> +
>> +#define GEM_INT_Q1_MASK (0x00000640 / 4)
>> +#define GEM_INT_Q7_MASK (GEM_INT_Q1_MASK + 6)
>> +#define GEM_INT_Q8_MASK (0x000006A0 / 4)
>> +#define GEM_INT_Q15_MASK (GEM_INT_Q8_MASK + 7)
>> +
>> /*****************************************/
>> #define GEM_NWCTRL_TXSTART 0x00000200 /* Transmit Enable */
>> #define GEM_NWCTRL_TXENA 0x00000008 /* Transmit Enable */
>> @@ -284,9 +307,9 @@ static inline unsigned tx_desc_get_length(unsigned *desc)
>> return desc[1] & DESC_1_LENGTH;
>> }
>>
>> -static inline void print_gem_tx_desc(unsigned *desc)
>> +static inline void print_gem_tx_desc(unsigned *desc, uint8_t queue)
>> {
>> - DB_PRINT("TXDESC:\n");
>> + DB_PRINT("TXDESC (queue %" PRId8 "):\n", queue);
>> DB_PRINT("bufaddr: 0x%08x\n", *desc);
>> DB_PRINT("used_hw: %d\n", tx_desc_get_used(desc));
>> DB_PRINT("wrap: %d\n", tx_desc_get_wrap(desc));
>> @@ -416,6 +439,7 @@ static void phy_update_link(CadenceGEMState *s)
>> static int gem_can_receive(NetClientState *nc)
>> {
>> CadenceGEMState *s;
>> + int i;
>>
>> s = qemu_get_nic_opaque(nc);
>>
>> @@ -428,18 +452,20 @@ static int gem_can_receive(NetClientState *nc)
>> return 0;
>> }
>>
>> - if (rx_desc_get_ownership(s->rx_desc[0]) == 1) {
>> - if (s->can_rx_state != 2) {
>> - s->can_rx_state = 2;
>> - DB_PRINT("can't receive - busy buffer descriptor 0x%x\n",
>> - s->rx_desc_addr[0]);
>> + for (i = 0; i < s->num_priority_queues; i++) {
>> + if (rx_desc_get_ownership(s->rx_desc[i]) == 1) {
>> + if (s->can_rx_state != 2) {
>> + s->can_rx_state = 2;
>> + DB_PRINT("can't receive - busy buffer descriptor (q%d) 0x%x\n",
>> + i, s->rx_desc_addr[i]);
>> + }
>> + return 0;
>> }
>> - return 0;
>> }
>>
>> if (s->can_rx_state != 0) {
>> s->can_rx_state = 0;
>> - DB_PRINT("can receive 0x%x\n", s->rx_desc_addr[0]);
>> + DB_PRINT("can receive\n");
>> }
>> return 1;
>> }
>> @@ -450,9 +476,16 @@ static int gem_can_receive(NetClientState *nc)
>> */
>> static void gem_update_int_status(CadenceGEMState *s)
>> {
>> - if (s->regs[GEM_ISR]) {
>> - DB_PRINT("asserting int. (0x%08x)\n", s->regs[GEM_ISR]);
>> - qemu_set_irq(s->irq[0], 1);
>> + int i;
>> +
>> + for (i = 0; i < s->num_priority_queues; ++i) {
>> + /* There seems to be no sane way to see which queue triggered the
>> + * interrupt.
>> + */
>> + if (s->regs[GEM_ISR]) {
>> + DB_PRINT("asserting int. q=%d)\n", i);
>> + qemu_set_irq(s->irq[i], 1);
>
> I don't understand this. The hardware surely can't trigger
> every IRQ line simultaneously and identically ?
Yeah, this is wrong, I have updated it.
>
>> + }
>> }
>> }
>>
>> @@ -601,17 +634,18 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
>> return GEM_RX_REJECT;
>> }
>>
>> -static void gem_get_rx_desc(CadenceGEMState *s)
>> +static void gem_get_rx_desc(CadenceGEMState *s, int q)
>> {
>> - DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[0]);
>> + DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[q]);
>> /* read current descriptor */
>> cpu_physical_memory_read(s->rx_desc_addr[0],
>> (uint8_t *)s->rx_desc[0], sizeof(s->rx_desc[0]));
>>
>> /* Descriptor owned by software ? */
>> - if (rx_desc_get_ownership(s->rx_desc[0]) == 1) {
>> + if (rx_desc_get_ownership(s->rx_desc[q]) == 1 &&
>> + s->num_priority_queues == 1) {
>
> Why only if num_priority_queues is 1? (This is one of those "looks
> a bit odd, is the h/w really like this?" questions; it could be right.)
Another weird thing, fixed.
>
>> DB_PRINT("descriptor 0x%x owned by sw.\n",
>> - (unsigned)s->rx_desc_addr[0]);
>> + (unsigned)s->rx_desc_addr[q]);
>> s->regs[GEM_RXSTATUS] |= GEM_RXSTATUS_NOBUF;
>> s->regs[GEM_ISR] |= GEM_INT_RXUSED & ~(s->regs[GEM_IMR]);
>> /* Handle interrupt consequences */
>> @@ -632,6 +666,7 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>> uint8_t *rxbuf_ptr;
>> bool first_desc = true;
>> int maf;
>> + int q = 0;
>
> Shouldn't we be doing something for each queue, rather than just
> having a variable that's always 0?
Woops, I missed out on the screening logic. Added it in.
>
>>
>> s = qemu_get_nic_opaque(nc);
>>
>> @@ -729,31 +764,31 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>>
>> /* Update the descriptor. */
>> if (first_desc) {
>> - rx_desc_set_sof(s->rx_desc[0]);
>> + rx_desc_set_sof(s->rx_desc[q]);
>> first_desc = false;
>> }
>> if (bytes_to_copy == 0) {
>> - rx_desc_set_eof(s->rx_desc[0]);
>> - rx_desc_set_length(s->rx_desc[0], size);
>> + rx_desc_set_eof(s->rx_desc[q]);
>> + rx_desc_set_length(s->rx_desc[q], size);
>> }
>> - rx_desc_set_ownership(s->rx_desc[0]);
>> + rx_desc_set_ownership(s->rx_desc[q]);
>>
>> switch (maf) {
>> case GEM_RX_PROMISCUOUS_ACCEPT:
>> break;
>> case GEM_RX_BROADCAST_ACCEPT:
>> - rx_desc_set_broadcast(s->rx_desc[0]);
>> + rx_desc_set_broadcast(s->rx_desc[q]);
>> break;
>> case GEM_RX_UNICAST_HASH_ACCEPT:
>> - rx_desc_set_unicast_hash(s->rx_desc[0]);
>> + rx_desc_set_unicast_hash(s->rx_desc[q]);
>> break;
>> case GEM_RX_MULTICAST_HASH_ACCEPT:
>> - rx_desc_set_multicast_hash(s->rx_desc[0]);
>> + rx_desc_set_multicast_hash(s->rx_desc[q]);
>> break;
>> case GEM_RX_REJECT:
>> abort();
>> default: /* SAR */
>> - rx_desc_set_sar(s->rx_desc[0], maf);
>> + rx_desc_set_sar(s->rx_desc[q], maf);
>> }
>>
>> /* Descriptor write-back. */
>> @@ -762,14 +797,15 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>> sizeof(s->rx_desc[0]));
>>
>> /* Next descriptor */
>> - if (rx_desc_get_wrap(s->rx_desc[0])) {
>> + if (rx_desc_get_wrap(s->rx_desc[q])) {
>> DB_PRINT("wrapping RX descriptor list\n");
>> s->rx_desc_addr[0] = s->regs[GEM_RXQBASE];
>> } else {
>> DB_PRINT("incrementing RX descriptor list\n");
>> s->rx_desc_addr[0] += 8;
>> }
>> - gem_get_rx_desc(s);
>> +
>> + gem_get_rx_desc(s, q);
>> }
>>
>> /* Count it */
>> @@ -841,6 +877,7 @@ static void gem_transmit(CadenceGEMState *s)
>> uint8_t tx_packet[2048];
>> uint8_t *p;
>> unsigned total_bytes;
>> + int8_t q;
>
> Why int8_t here but int everywhere else?
No reason, it's an int now.
>
>>
>> /* Do nothing if transmit is not enabled. */
>> if (!(s->regs[GEM_NWCTRL] & GEM_NWCTRL_TXENA)) {
>> @@ -856,8 +893,9 @@ static void gem_transmit(CadenceGEMState *s)
>> p = tx_packet;
>> total_bytes = 0;
>>
>> + for (q = s->num_priority_queues - 1; q >= 0; q--) {
>> /* read current descriptor */
>> - packet_desc_addr = s->tx_desc_addr[0];
>> + packet_desc_addr = s->tx_desc_addr[q];
>>
>> DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
>> cpu_physical_memory_read(packet_desc_addr,
>> @@ -869,7 +907,7 @@ static void gem_transmit(CadenceGEMState *s)
>> if (!(s->regs[GEM_NWCTRL] & GEM_NWCTRL_TXENA)) {
>> return;
>> }
>> - print_gem_tx_desc(desc);
>> + print_gem_tx_desc(desc, q);
>>
>> /* The real hardware would eat this (and possibly crash).
>> * For QEMU let's lend a helping hand.
>> @@ -904,18 +942,18 @@ static void gem_transmit(CadenceGEMState *s)
>> /* Modify the 1st descriptor of this packet to be owned by
>> * the processor.
>> */
>> - cpu_physical_memory_read(s->tx_desc_addr[0], (uint8_t *)desc_first,
>> + cpu_physical_memory_read(s->tx_desc_addr[q], (uint8_t *)desc_first,
>> sizeof(desc_first));
>> tx_desc_set_used(desc_first);
>> - cpu_physical_memory_write(s->tx_desc_addr[0], (uint8_t *)desc_first,
>> + cpu_physical_memory_write(s->tx_desc_addr[q], (uint8_t *)desc_first,
>> sizeof(desc_first));
>> /* Advance the hardware current descriptor past this packet */
>> if (tx_desc_get_wrap(desc)) {
>> - s->tx_desc_addr[0] = s->regs[GEM_TXQBASE];
>> + s->tx_desc_addr[q] = s->regs[GEM_TXQBASE];
>> } else {
>> - s->tx_desc_addr[0] = packet_desc_addr + 8;
>> + s->tx_desc_addr[q] = packet_desc_addr + 8;
>> }
>> - DB_PRINT("TX descriptor next: 0x%08x\n", s->tx_desc_addr[0]);
>> + DB_PRINT("TX descriptor next: 0x%08x\n", s->tx_desc_addr[q]);
>>
>> s->regs[GEM_TXSTATUS] |= GEM_TXSTATUS_TXCMPL;
>> s->regs[GEM_ISR] |= GEM_INT_TXCMPL & ~(s->regs[GEM_IMR]);
>> @@ -961,6 +999,7 @@ static void gem_transmit(CadenceGEMState *s)
>> s->regs[GEM_ISR] |= GEM_INT_TXUSED & ~(s->regs[GEM_IMR]);
>> gem_update_int_status(s);
>> }
>> + }
>> }
>>
>> static void gem_phy_reset(CadenceGEMState *s)
>> @@ -1067,7 +1106,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
>> {
>> CadenceGEMState *s;
>> uint32_t retval;
>> -
>> + int i;
>> s = (CadenceGEMState *)opaque;
>>
>> offset >>= 2;
>> @@ -1077,8 +1116,10 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
>>
>> switch (offset) {
>> case GEM_ISR:
>> - DB_PRINT("lowering irq on ISR read\n");
>> - qemu_set_irq(s->irq[0], 0);
>> + DB_PRINT("lowering irqs on ISR read\n");
>> + for (i = 0; i < s->num_priority_queues; ++i) {
>> + qemu_set_irq(s->irq[i], 0);
>> + }
>> break;
>> case GEM_PHYMNTNC:
>> if (retval & GEM_PHYMNTNC_OP_R) {
>> @@ -1103,6 +1144,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
>> retval &= ~(s->regs_wo[offset]);
>>
>> DB_PRINT("0x%08x\n", retval);
>> + gem_update_int_status(s);
>> return retval;
>> }
>>
>> @@ -1115,6 +1157,7 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
>> {
>> CadenceGEMState *s = (CadenceGEMState *)opaque;
>> uint32_t readonly;
>> + int i;
>>
>> DB_PRINT("offset: 0x%04x write: 0x%08x ", (unsigned)offset, (unsigned)val);
>> offset >>= 2;
>> @@ -1134,14 +1177,18 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
>> switch (offset) {
>> case GEM_NWCTRL:
>> if (val & GEM_NWCTRL_RXENA) {
>> - gem_get_rx_desc(s);
>> + for (i = 0; i < s->num_priority_queues; ++i) {
>> + gem_get_rx_desc(s, i);
>> + }
>> }
>> if (val & GEM_NWCTRL_TXSTART) {
>> gem_transmit(s);
>> }
>> if (!(val & GEM_NWCTRL_TXENA)) {
>> /* Reset to start of Q when transmit disabled. */
>> - s->tx_desc_addr[0] = s->regs[GEM_TXQBASE];
>> + for (i = 0; i < s->num_priority_queues; i++) {
>> + s->tx_desc_addr[i] = s->regs[GEM_TXQBASE];
>> + }
>> }
>> if (gem_can_receive(qemu_get_queue(s->nic))) {
>> qemu_flush_queued_packets(qemu_get_queue(s->nic));
>> @@ -1154,9 +1201,15 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
>> case GEM_RXQBASE:
>> s->rx_desc_addr[0] = val;
>> break;
>> + case GEM_RECEIVE_Q1_PTR ... GEM_RECEIVE_Q15_PTR:
>> + s->rx_desc_addr[offset - GEM_RECEIVE_Q1_PTR + 1] = val;
>> + break;
>> case GEM_TXQBASE:
>> s->tx_desc_addr[0] = val;
>> break;
>> + case GEM_TRANSMIT_Q1_PTR ... GEM_TRANSMIT_Q15_PTR:
>> + s->tx_desc_addr[offset - GEM_TRANSMIT_Q1_PTR + 1] = val;
>> + break;
>> case GEM_RXSTATUS:
>> gem_update_int_status(s);
>> break;
>> @@ -1164,10 +1217,26 @@ static void gem_write(void *opaque, hwaddr offset, uint64_t val,
>> s->regs[GEM_IMR] &= ~val;
>> gem_update_int_status(s);
>> break;
>> + case GEM_INT_Q1_ENABLE ... GEM_INT_Q7_ENABLE:
>> + s->regs[GEM_INT_Q1_MASK + offset - GEM_INT_Q1_ENABLE] &= ~val;
>> + gem_update_int_status(s);
>> + break;
>> + case GEM_INT_Q8_ENABLE ... GEM_INT_Q15_ENABLE:
>> + s->regs[GEM_INT_Q8_MASK + offset - GEM_INT_Q8_ENABLE] &= ~val;
>> + gem_update_int_status(s);
>> + break;
>> case GEM_IDR:
>> s->regs[GEM_IMR] |= val;
>> gem_update_int_status(s);
>> break;
>> + case GEM_INT_Q1_DISABLE ... GEM_INT_Q7_DISABLE:
>> + s->regs[GEM_INT_Q1_MASK + offset - GEM_INT_Q1_DISABLE] |= val;
>> + gem_update_int_status(s);
>> + break;
>> + case GEM_INT_Q8_DISABLE ... GEM_INT_Q15_DISABLE:
>> + s->regs[GEM_INT_Q8_MASK + offset - GEM_INT_Q8_DISABLE] |= val;
>> + gem_update_int_status(s);
>> + break;
>> case GEM_SPADDR1LO:
>> case GEM_SPADDR2LO:
>> case GEM_SPADDR3LO:
>> @@ -1204,8 +1273,11 @@ static const MemoryRegionOps gem_ops = {
>>
>> static void gem_set_link(NetClientState *nc)
>> {
>> + CadenceGEMState *s = qemu_get_nic_opaque(nc);
>> +
>> DB_PRINT("\n");
>> - phy_update_link(qemu_get_nic_opaque(nc));
>> + phy_update_link(s);
>> + gem_update_int_status(s);
>> }
>>
>> static NetClientInfo net_gem_info = {
>> @@ -1219,8 +1291,11 @@ static NetClientInfo net_gem_info = {
>> static void gem_realize(DeviceState *dev, Error **errp)
>> {
>> CadenceGEMState *s = CADENCE_GEM(dev);
>> + int i;
>>
>> - sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[0]);
>> + for (i = 0; i < s->num_priority_queues; ++i) {
>> + sysbus_init_irq(SYS_BUS_DEVICE(dev), &s->irq[i]);
>> + }
>>
>> qemu_macaddr_default_if_unset(&s->conf.macaddr);
>>
>> @@ -1260,6 +1335,8 @@ static const VMStateDescription vmstate_cadence_gem = {
>>
>> static Property gem_properties[] = {
>> DEFINE_NIC_PROPERTIES(CadenceGEMState, conf),
>> + DEFINE_PROP_UINT8("num-priority-queues", CadenceGEMState,
>> + num_priority_queues, 1),
>
> This should go in the same patch as adding the field to the
> CadenceGEMState struct (don't care whether you move the prop
> define or the field addition).
Agreed, I have moved it to an earlier patch.
>
>> DEFINE_PROP_END_OF_LIST(),
>> };
>>
>> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
>> index 77e0582..60b3ab0 100644
>> --- a/include/hw/net/cadence_gem.h
>> +++ b/include/hw/net/cadence_gem.h
>> @@ -30,7 +30,7 @@
>> #include "net/net.h"
>> #include "hw/sysbus.h"
>>
>> -#define CADENCE_GEM_MAXREG (0x00000640/4) /* Last valid GEM address */
>> +#define CADENCE_GEM_MAXREG (0x00000800 / 4) /* Last valid GEM address */
>
> Changing this define changes the migration format (because it
> has an array of this size in it) so you need a version bump.
Done
Thanks,
Alistair
>
>> #define MAX_PRIORITY_QUEUES 8
>>
>> --
>> 2.7.4
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2016-07-25 23:02 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-11 23:20 [Qemu-devel] [PATCH v1 0/5] Add support for Cadence GEM priority queues Alistair Francis
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 1/5] cadence_gem: QOMify Cadence GEM Alistair Francis
2016-07-25 15:20 ` Peter Maydell
2016-07-25 16:38 ` Alistair Francis
2016-07-25 16:42 ` Alistair Francis
2016-07-25 16:55 ` Peter Maydell
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 2/5] cadence_gem: Arrayify Alistair Francis
2016-07-25 15:22 ` Peter Maydell
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 3/5] cadence_gem: Add queue support Alistair Francis
2016-07-25 15:46 ` Peter Maydell
2016-07-25 23:01 ` Alistair Francis
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 4/5] cadence_gem: Correct indentation Alistair Francis
2016-07-25 15:48 ` Peter Maydell
2016-07-25 16:34 ` Alistair Francis
2016-07-25 16:55 ` Peter Maydell
2016-07-11 23:20 ` [Qemu-devel] [PATCH v1 5/5] xlnx-zynqmp: Set the number of priority queues Alistair Francis
2016-07-25 15:50 ` Peter Maydell
2016-07-12 14:25 ` [Qemu-devel] [PATCH v1 0/5] Add support for Cadence GEM " Peter Maydell
2016-07-12 16:34 ` Alistair Francis
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.