* [Qemu-devel] [PATCH v2 0/6] Add support for Cadence GEM priority queues
@ 2016-07-26 0:12 Alistair Francis
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 1/6] cadence_gem: QOMify Cadence GEM Alistair Francis
` (5 more replies)
0 siblings, 6 replies; 15+ messages in thread
From: Alistair Francis @ 2016-07-26 0:12 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.
V2:
- Add screening support
- Fixup some broken logic
- Expand the commit messages
Alistair Francis (6):
cadence_gem: QOMify Cadence GEM
cadence_gem: Add the num-priority-queues property
cadence_gem: Add support for screening
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 | 549 +++++++++++++++++++++++++++++++------------
include/hw/net/cadence_gem.h | 17 +-
3 files changed, 416 insertions(+), 152 deletions(-)
--
2.7.4
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2 1/6] cadence_gem: QOMify Cadence GEM
2016-07-26 0:12 [Qemu-devel] [PATCH v2 0/6] Add support for Cadence GEM priority queues Alistair Francis
@ 2016-07-26 0:12 ` Alistair Francis
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 2/6] cadence_gem: Add the num-priority-queues property Alistair Francis
` (4 subsequent siblings)
5 siblings, 0 replies; 15+ messages in thread
From: Alistair Francis @ 2016-07-26 0:12 UTC (permalink / raw)
To: qemu-devel, qemu-arm; +Cc: alistair.francis, crosthwaitepeter, peter.maydell
The sysbus_init_irq() call will eventually depend on a property so it needs to
be in the realize function.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
V2:
- Update commit message
hw/net/cadence_gem.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index db1b301..7adc2a8 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1214,24 +1214,30 @@ 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);
+ sysbus_init_irq(SYS_BUS_DEVICE(dev), &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");
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);
- return 0;
+ sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->iomem);
}
static const VMStateDescription vmstate_cadence_gem = {
@@ -1257,9 +1263,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 +1274,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] 15+ messages in thread
* [Qemu-devel] [PATCH v2 2/6] cadence_gem: Add the num-priority-queues property
2016-07-26 0:12 [Qemu-devel] [PATCH v2 0/6] Add support for Cadence GEM priority queues Alistair Francis
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 1/6] cadence_gem: QOMify Cadence GEM Alistair Francis
@ 2016-07-26 0:12 ` Alistair Francis
2016-07-26 11:47 ` Peter Maydell
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 3/6] cadence_gem: Add support for screening Alistair Francis
` (3 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Alistair Francis @ 2016-07-26 0:12 UTC (permalink / raw)
To: qemu-devel, qemu-arm; +Cc: alistair.francis, crosthwaitepeter, peter.maydell
The Cadence GEM hardware supports N number priority queues, this patch is a
step towards that by adding the property to set the queues. At the moment
behaviour doesn't change as we only use queue 0.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
V2:
- Fixup commit message
- Add the property in this patch
- Increase vmstate version
hw/net/cadence_gem.c | 82 ++++++++++++++++++++++++--------------------
include/hw/net/cadence_gem.h | 13 ++++---
2 files changed, 53 insertions(+), 42 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 7adc2a8..deae122 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);
@@ -1243,13 +1245,15 @@ static void gem_init(Object *obj)
static const VMStateDescription vmstate_cadence_gem = {
.name = "cadence_gem",
.version_id = 2,
- .minimum_version_id = 2,
+ .minimum_version_id = 3,
.fields = (VMStateField[]) {
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(),
}
@@ -1257,6 +1261,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 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] 15+ messages in thread
* [Qemu-devel] [PATCH v2 3/6] cadence_gem: Add support for screening
2016-07-26 0:12 [Qemu-devel] [PATCH v2 0/6] Add support for Cadence GEM priority queues Alistair Francis
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 1/6] cadence_gem: QOMify Cadence GEM Alistair Francis
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 2/6] cadence_gem: Add the num-priority-queues property Alistair Francis
@ 2016-07-26 0:12 ` Alistair Francis
2016-07-26 12:01 ` Peter Maydell
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 4/6] cadence_gem: Add queue support Alistair Francis
` (2 subsequent siblings)
5 siblings, 1 reply; 15+ messages in thread
From: Alistair Francis @ 2016-07-26 0:12 UTC (permalink / raw)
To: qemu-devel, qemu-arm; +Cc: alistair.francis, crosthwaitepeter, peter.maydell
The Cadence GEM hardware allows incoming data to be 'screened' based on some
register values. Add support for these screens.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
---
V2:
- Initial commit
hw/net/cadence_gem.c | 151 +++++++++++++++++++++++++++++++++++++++++++
include/hw/net/cadence_gem.h | 2 +
2 files changed, 153 insertions(+)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index deae122..d38bc1e 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -26,6 +26,7 @@
#include <zlib.h> /* For crc32 */
#include "hw/net/cadence_gem.h"
+#include "qemu/log.h"
#include "net/checksum.h"
#ifdef CADENCE_GEM_ERR_DEBUG
@@ -141,6 +142,37 @@
#define GEM_DESCONF6 (0x00000294/4)
#define GEM_DESCONF7 (0x00000298/4)
+#define GEM_SCREENING_TYPE1_REGISTER_0 (0x00000500 / 4)
+
+#define GEM_ST1R_UDP_PORT_MATCH_ENABLE (1 << 29)
+#define GEM_ST1R_DSTC_ENABLE (1 << 28)
+#define GEM_ST1R_UDP_PORT_MATCH_SHIFT (12)
+#define GEM_ST1R_UDP_PORT_MATCH_WIDTH (27 - GEM_ST1R_UDP_PORT_MATCH_SHIFT + 1)
+#define GEM_ST1R_DSTC_MATCH_SHIFT (4)
+#define GEM_ST1R_DSTC_MATCH_WIDTH (11 - GEM_ST1R_DSTC_MATCH_SHIFT + 1)
+#define GEM_ST1R_QUEUE_SHIFT (0)
+#define GEM_ST1R_QUEUE_WIDTH (3 - GEM_ST1R_QUEUE_SHIFT + 1)
+
+#define GEM_SCREENING_TYPE2_REGISTER_0 (0x00000540 / 4)
+
+#define GEM_ST2R_COMPARE_A_ENABLE (1 << 18)
+#define GEM_ST2R_COMPARE_A_SHIFT (13)
+#define GEM_ST2R_COMPARE_WIDTH (17 - GEM_ST2R_COMPARE_A_SHIFT + 1)
+#define GEM_ST2R_ETHERTYPE_ENABLE (1 << 12)
+#define GEM_ST2R_ETHERTYPE_INDEX_SHIFT (9)
+#define GEM_ST2R_ETHERTYPE_INDEX_WIDTH (11 - GEM_ST2R_ETHERTYPE_INDEX_SHIFT \
+ + 1)
+#define GEM_ST2R_QUEUE_SHIFT (0)
+#define GEM_ST2R_QUEUE_WIDTH (3 - GEM_ST2R_QUEUE_SHIFT + 1)
+
+#define GEM_SCREENING_TYPE2_ETHERTYPE_REG_0 (0x000006e0 / 4)
+#define GEM_TYPE2_COMPARE_0_WORD_0 (0x00000700 / 4)
+
+#define GEM_T2CW1_COMPARE_OFFSET_SHIFT (7)
+#define GEM_T2CW1_COMPARE_OFFSET_WIDTH (8 - GEM_T2CW1_COMPARE_OFFSET_SHIFT + 1)
+#define GEM_T2CW1_OFFSET_VALUE_SHIFT (0)
+#define GEM_T2CW1_OFFSET_VALUE_WIDTH (6 - GEM_T2CW1_OFFSET_VALUE_SHIFT + 1)
+
/*****************************************/
#define GEM_NWCTRL_TXSTART 0x00000200 /* Transmit Enable */
#define GEM_NWCTRL_TXENA 0x00000008 /* Transmit Enable */
@@ -601,6 +633,121 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
return GEM_RX_REJECT;
}
+/* Figure out which queue the recieved data should be sent to */
+static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr)
+{
+ uint32_t reg;
+ bool matched, mismatched;
+ int i, j;
+
+ for (i = 0; i < s->num_type1_screeners; i++) {
+ reg = s->regs[GEM_SCREENING_TYPE1_REGISTER_0 + i];
+ matched = false;
+ mismatched = false;
+
+ /* Screening is based on UDP Port */
+ if (reg & GEM_ST1R_UDP_PORT_MATCH_ENABLE) {
+ uint16_t udp_port = rxbuf_ptr[14 + 22] << 8 | rxbuf_ptr[14 + 23];
+ if (udp_port == extract32(reg, GEM_ST1R_UDP_PORT_MATCH_SHIFT,
+ GEM_ST1R_UDP_PORT_MATCH_WIDTH)) {
+ matched = true;
+ } else {
+ mismatched = true;
+ }
+ }
+
+ /* Screening is based on DS/TC */
+ if (reg & GEM_ST1R_DSTC_ENABLE) {
+ uint16_t dscp = rxbuf_ptr[14 + 1];
+ if (dscp == extract32(reg, GEM_ST1R_DSTC_MATCH_SHIFT,
+ GEM_ST1R_DSTC_MATCH_WIDTH)) {
+ matched = true;
+ } else {
+ mismatched = true;
+ }
+ }
+
+ if (matched && !mismatched) {
+ return extract32(reg, GEM_ST1R_QUEUE_SHIFT, GEM_ST1R_QUEUE_WIDTH);
+ }
+ }
+
+ for (i = 0; i < s->num_type2_screeners; i++) {
+ reg = s->regs[GEM_SCREENING_TYPE2_REGISTER_0 + i];
+ matched = false;
+ mismatched = false;
+
+ if (reg & GEM_ST2R_ETHERTYPE_ENABLE) {
+ uint16_t type = rxbuf_ptr[12] << 8 | rxbuf_ptr[13];
+ int et_idx = extract32(reg, GEM_ST2R_ETHERTYPE_INDEX_SHIFT,
+ GEM_ST2R_ETHERTYPE_INDEX_WIDTH);
+
+ if (et_idx > s->num_type2_screeners) {
+ qemu_log_mask(LOG_GUEST_ERROR, "Out of range ethertype "
+ "register index: %d\n", et_idx);
+ }
+ if (type == s->regs[GEM_SCREENING_TYPE2_ETHERTYPE_REG_0 +
+ et_idx]) {
+ matched = true;
+ } else {
+ mismatched = true;
+ }
+ }
+
+ /* Compare A, B, C */
+ for (j = 0; j < 3; j++) {
+ uint32_t cr0, cr1, mask;
+ uint16_t rx_cmp;
+ int offset;
+ int cr_idx = extract32(reg, GEM_ST2R_COMPARE_A_SHIFT + j * 6,
+ GEM_ST2R_COMPARE_WIDTH);
+
+ if (!(reg & (GEM_ST2R_COMPARE_A_ENABLE << (j * 6)))) {
+ continue;
+ }
+ if (cr_idx > s->num_type2_screeners) {
+ qemu_log_mask(LOG_GUEST_ERROR, "Out of range compare "
+ "register index: %d\n", cr_idx);
+ }
+
+ cr0 = s->regs[GEM_TYPE2_COMPARE_0_WORD_0 + cr_idx * 2];
+ cr1 = s->regs[GEM_TYPE2_COMPARE_0_WORD_0 + cr_idx * 2 + 1];
+ offset = extract32(cr1, GEM_T2CW1_OFFSET_VALUE_SHIFT,
+ GEM_T2CW1_OFFSET_VALUE_WIDTH);
+
+ switch (extract32(cr1, GEM_T2CW1_COMPARE_OFFSET_SHIFT,
+ GEM_T2CW1_COMPARE_OFFSET_WIDTH)) {
+ case (3): /* Skip UDP header */
+ qemu_log_mask(LOG_UNIMP, "TCP compare offsets"
+ "unimplemented - assuming UDP\n");
+ offset += 8;
+ /* Fallthrough */
+ case (2): /* skip the IP header */
+ offset += 20;
+ /* Fallthrough */
+ case (1): /* Count from after the ethertype */
+ offset += 14;
+ }
+
+ rx_cmp = rxbuf_ptr[offset] << 8 | rxbuf_ptr[offset];
+ mask = extract32(cr0, 0, 16);
+
+ if ((rx_cmp & mask) == (extract32(cr0, 16, 16) & mask)) {
+ matched = true;
+ } else {
+ mismatched = true;
+ }
+ }
+
+ if (matched && !mismatched) {
+ return extract32(reg, GEM_ST2R_QUEUE_SHIFT, GEM_ST2R_QUEUE_WIDTH);
+ }
+ }
+
+ /* We made it here, assume it's queue 0 */
+ return 0;
+}
+
static void gem_get_rx_desc(CadenceGEMState *s)
{
DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[0]);
@@ -1263,6 +1410,10 @@ static Property gem_properties[] = {
DEFINE_NIC_PROPERTIES(CadenceGEMState, conf),
DEFINE_PROP_UINT8("num-priority-queues", CadenceGEMState,
num_priority_queues, 1),
+ DEFINE_PROP_UINT8("num-type1-screeners", CadenceGEMState,
+ num_type1_screeners, 4),
+ DEFINE_PROP_UINT8("num-type2-screeners", CadenceGEMState,
+ num_type2_screeners, 4),
DEFINE_PROP_END_OF_LIST(),
};
diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
index 77e0582..f2f78c0 100644
--- a/include/hw/net/cadence_gem.h
+++ b/include/hw/net/cadence_gem.h
@@ -46,6 +46,8 @@ typedef struct CadenceGEMState {
/* Static properties */
uint8_t num_priority_queues;
+ uint8_t num_type1_screeners;
+ uint8_t num_type2_screeners;
/* GEM registers backing store */
uint32_t regs[CADENCE_GEM_MAXREG];
--
2.7.4
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2 4/6] cadence_gem: Add queue support
2016-07-26 0:12 [Qemu-devel] [PATCH v2 0/6] Add support for Cadence GEM priority queues Alistair Francis
` (2 preceding siblings ...)
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 3/6] cadence_gem: Add support for screening Alistair Francis
@ 2016-07-26 0:12 ` Alistair Francis
2016-07-26 12:06 ` Peter Maydell
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 5/6] cadence_gem: Correct indentation Alistair Francis
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 6/6] xlnx-zynqmp: Set the number of priority queues Alistair Francis
5 siblings, 1 reply; 15+ messages in thread
From: Alistair Francis @ 2016-07-26 0:12 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.
V2:
- Use the new screening function
- Update interrupt generation
- Increase vmstate to 3.0
hw/net/cadence_gem.c | 180 ++++++++++++++++++++++++++++++++-----------
include/hw/net/cadence_gem.h | 2 +-
2 files changed, 135 insertions(+), 47 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index d38bc1e..28c2ddb 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -142,6 +142,30 @@
#define GEM_DESCONF6 (0x00000294/4)
#define GEM_DESCONF7 (0x00000298/4)
+#define GEM_INT_Q1_STATUS (0x00000400 / 4)
+#define GEM_INT_Q1_MASK (0x00000640 / 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_SCREENING_TYPE1_REGISTER_0 (0x00000500 / 4)
#define GEM_ST1R_UDP_PORT_MATCH_ENABLE (1 << 29)
@@ -316,9 +340,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));
@@ -448,6 +472,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);
@@ -460,18 +485,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;
}
@@ -482,9 +509,20 @@ 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]);
+ int i;
+
+ if (!s->num_priority_queues && s->regs[GEM_ISR]) {
+ /* No priority queues, just trigger the interrupt */
+ DB_PRINT("asserting int.\n", i);
qemu_set_irq(s->irq[0], 1);
+ return;
+ }
+
+ for (i = 0; i < s->num_priority_queues; ++i) {
+ if (s->regs[GEM_INT_Q1_STATUS + i]) {
+ DB_PRINT("asserting int. (q=%d)\n", i);
+ qemu_set_irq(s->irq[i], 1);
+ }
}
}
@@ -748,17 +786,17 @@ static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr)
return 0;
}
-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) {
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 */
@@ -779,6 +817,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);
@@ -857,6 +896,9 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
DB_PRINT("config bufsize: %d packet size: %ld\n", rxbufsize, size);
+ /* Find which queue we are targetting */
+ q = get_queue_from_screen(s, rxbuf_ptr);
+
while (bytes_to_copy) {
/* Do nothing if receive is not enabled. */
if (!gem_can_receive(nc)) {
@@ -865,10 +907,10 @@ 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[0]));
+ rx_desc_get_buffer(s->rx_desc[q]));
/* Copy packet data to emulated DMA buffer */
- cpu_physical_memory_write(rx_desc_get_buffer(s->rx_desc[0]) +
+ cpu_physical_memory_write(rx_desc_get_buffer(s->rx_desc[q]) +
rxbuf_offset,
rxbuf_ptr, MIN(bytes_to_copy, rxbufsize));
rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
@@ -876,47 +918,48 @@ 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. */
- cpu_physical_memory_write(s->rx_desc_addr[0],
- (uint8_t *)s->rx_desc[0],
- sizeof(s->rx_desc[0]));
+ cpu_physical_memory_write(s->rx_desc_addr[q],
+ (uint8_t *)s->rx_desc[q],
+ sizeof(s->rx_desc[q]));
/* 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 */
@@ -988,6 +1031,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)) {
@@ -1003,8 +1047,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,
@@ -1016,7 +1061,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.
@@ -1051,22 +1096,28 @@ 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]);
+ /* Update queue interrupt status */
+ if (s->num_priority_queues) {
+ s->regs[GEM_INT_Q1_STATUS + q] |=
+ GEM_INT_TXCMPL & ~(s->regs[GEM_INT_Q1_MASK + q]);
+ }
+
/* Handle interrupt consequences */
gem_update_int_status(s);
@@ -1108,6 +1159,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)
@@ -1214,7 +1266,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
{
CadenceGEMState *s;
uint32_t retval;
-
+ int i;
s = (CadenceGEMState *)opaque;
offset >>= 2;
@@ -1224,8 +1276,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) {
@@ -1250,6 +1304,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;
}
@@ -1262,6 +1317,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;
@@ -1281,14 +1337,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));
@@ -1301,9 +1361,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;
@@ -1311,10 +1377,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:
@@ -1351,8 +1433,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 = {
@@ -1366,8 +1451,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);
@@ -1391,8 +1479,8 @@ static void gem_init(Object *obj)
static const VMStateDescription vmstate_cadence_gem = {
.name = "cadence_gem",
- .version_id = 2,
- .minimum_version_id = 3,
+ .version_id = 3,
+ .minimum_version_id = 0,
.fields = (VMStateField[]) {
VMSTATE_UINT32_ARRAY(regs, CadenceGEMState, CADENCE_GEM_MAXREG),
VMSTATE_UINT16_ARRAY(phy_regs, CadenceGEMState, 32),
diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
index f2f78c0..47a1661 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] 15+ messages in thread
* [Qemu-devel] [PATCH v2 5/6] cadence_gem: Correct indentation
2016-07-26 0:12 [Qemu-devel] [PATCH v2 0/6] Add support for Cadence GEM priority queues Alistair Francis
` (3 preceding siblings ...)
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 4/6] cadence_gem: Add queue support Alistair Francis
@ 2016-07-26 0:12 ` Alistair Francis
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 6/6] xlnx-zynqmp: Set the number of priority queues Alistair Francis
5 siblings, 0 replies; 15+ messages in thread
From: Alistair Francis @ 2016-07-26 0:12 UTC (permalink / raw)
To: qemu-devel, qemu-arm; +Cc: alistair.francis, crosthwaitepeter, peter.maydell
Fix up the indentation inside the for loop that was introduced in the previous
patch. This commit is almost empty if viewed using 'git show -w', except for a
few changes that were required to avoid the 80 charecter line limit.
Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
V2:
- Conform to 80 chareceter line limit
- Improve commit message
- Re-apply based on previous patches
hw/net/cadence_gem.c | 196 ++++++++++++++++++++++++++-------------------------
1 file changed, 100 insertions(+), 96 deletions(-)
diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
index 28c2ddb..452d072 100644
--- a/hw/net/cadence_gem.c
+++ b/hw/net/cadence_gem.c
@@ -1048,117 +1048,121 @@ 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]);
- /* Update queue interrupt status */
- if (s->num_priority_queues) {
- s->regs[GEM_INT_Q1_STATUS + q] |=
- GEM_INT_TXCMPL & ~(s->regs[GEM_INT_Q1_MASK + q]);
+ 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;
}
- /* 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);
+ /* 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]);
+
+ /* Update queue interrupt status */
+ if (s->num_priority_queues) {
+ s->regs[GEM_INT_Q1_STATUS + q] |=
+ GEM_INT_TXCMPL & ~(s->regs[GEM_INT_Q1_MASK + q]);
+ }
+
+ /* 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;
}
- /* 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);
+ /* 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] 15+ messages in thread
* [Qemu-devel] [PATCH v2 6/6] xlnx-zynqmp: Set the number of priority queues
2016-07-26 0:12 [Qemu-devel] [PATCH v2 0/6] Add support for Cadence GEM priority queues Alistair Francis
` (4 preceding siblings ...)
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 5/6] cadence_gem: Correct indentation Alistair Francis
@ 2016-07-26 0:12 ` Alistair Francis
5 siblings, 0 replies; 15+ messages in thread
From: Alistair Francis @ 2016-07-26 0:12 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>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
---
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] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/6] cadence_gem: Add the num-priority-queues property
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 2/6] cadence_gem: Add the num-priority-queues property Alistair Francis
@ 2016-07-26 11:47 ` Peter Maydell
2016-07-26 16:10 ` Alistair Francis
0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2016-07-26 11:47 UTC (permalink / raw)
To: Alistair Francis; +Cc: QEMU Developers, qemu-arm, Peter Crosthwaite
On 26 July 2016 at 01:12, Alistair Francis <alistair.francis@xilinx.com> wrote:
> The Cadence GEM hardware supports N number priority queues, this patch is a
> step towards that by adding the property to set the queues. At the moment
> behaviour doesn't change as we only use queue 0.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
> @@ -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);
At some point realize() needs to error-check the num-priority-queues
property (ie check it isn't >8). Do you do that later in the series?
(if so fine, if not, this patch is as good a place as any to put it.)
> @@ -1243,13 +1245,15 @@ static void gem_init(Object *obj)
> static const VMStateDescription vmstate_cadence_gem = {
> .name = "cadence_gem",
> .version_id = 2,
> - .minimum_version_id = 2,
> + .minimum_version_id = 3,
You need to bump .version_id too.
Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/6] cadence_gem: Add support for screening
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 3/6] cadence_gem: Add support for screening Alistair Francis
@ 2016-07-26 12:01 ` Peter Maydell
2016-07-26 16:42 ` Alistair Francis
0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2016-07-26 12:01 UTC (permalink / raw)
To: Alistair Francis; +Cc: QEMU Developers, qemu-arm, Peter Crosthwaite
On 26 July 2016 at 01:12, Alistair Francis <alistair.francis@xilinx.com> wrote:
> The Cadence GEM hardware allows incoming data to be 'screened' based on some
> register values. Add support for these screens.
>
> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
> ---
> V2:
> - Initial commit
>
> hw/net/cadence_gem.c | 151 +++++++++++++++++++++++++++++++++++++++++++
> include/hw/net/cadence_gem.h | 2 +
> 2 files changed, 153 insertions(+)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index deae122..d38bc1e 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -26,6 +26,7 @@
> #include <zlib.h> /* For crc32 */
>
> #include "hw/net/cadence_gem.h"
> +#include "qemu/log.h"
> #include "net/checksum.h"
>
> #ifdef CADENCE_GEM_ERR_DEBUG
> @@ -141,6 +142,37 @@
> #define GEM_DESCONF6 (0x00000294/4)
> #define GEM_DESCONF7 (0x00000298/4)
>
> +#define GEM_SCREENING_TYPE1_REGISTER_0 (0x00000500 / 4)
> +
> +#define GEM_ST1R_UDP_PORT_MATCH_ENABLE (1 << 29)
> +#define GEM_ST1R_DSTC_ENABLE (1 << 28)
> +#define GEM_ST1R_UDP_PORT_MATCH_SHIFT (12)
> +#define GEM_ST1R_UDP_PORT_MATCH_WIDTH (27 - GEM_ST1R_UDP_PORT_MATCH_SHIFT + 1)
> +#define GEM_ST1R_DSTC_MATCH_SHIFT (4)
> +#define GEM_ST1R_DSTC_MATCH_WIDTH (11 - GEM_ST1R_DSTC_MATCH_SHIFT + 1)
> +#define GEM_ST1R_QUEUE_SHIFT (0)
> +#define GEM_ST1R_QUEUE_WIDTH (3 - GEM_ST1R_QUEUE_SHIFT + 1)
> +
> +#define GEM_SCREENING_TYPE2_REGISTER_0 (0x00000540 / 4)
> +
> +#define GEM_ST2R_COMPARE_A_ENABLE (1 << 18)
> +#define GEM_ST2R_COMPARE_A_SHIFT (13)
> +#define GEM_ST2R_COMPARE_WIDTH (17 - GEM_ST2R_COMPARE_A_SHIFT + 1)
> +#define GEM_ST2R_ETHERTYPE_ENABLE (1 << 12)
> +#define GEM_ST2R_ETHERTYPE_INDEX_SHIFT (9)
> +#define GEM_ST2R_ETHERTYPE_INDEX_WIDTH (11 - GEM_ST2R_ETHERTYPE_INDEX_SHIFT \
> + + 1)
> +#define GEM_ST2R_QUEUE_SHIFT (0)
> +#define GEM_ST2R_QUEUE_WIDTH (3 - GEM_ST2R_QUEUE_SHIFT + 1)
> +
> +#define GEM_SCREENING_TYPE2_ETHERTYPE_REG_0 (0x000006e0 / 4)
> +#define GEM_TYPE2_COMPARE_0_WORD_0 (0x00000700 / 4)
> +
> +#define GEM_T2CW1_COMPARE_OFFSET_SHIFT (7)
> +#define GEM_T2CW1_COMPARE_OFFSET_WIDTH (8 - GEM_T2CW1_COMPARE_OFFSET_SHIFT + 1)
> +#define GEM_T2CW1_OFFSET_VALUE_SHIFT (0)
> +#define GEM_T2CW1_OFFSET_VALUE_WIDTH (6 - GEM_T2CW1_OFFSET_VALUE_SHIFT + 1)
> +
> /*****************************************/
> #define GEM_NWCTRL_TXSTART 0x00000200 /* Transmit Enable */
> #define GEM_NWCTRL_TXENA 0x00000008 /* Transmit Enable */
> @@ -601,6 +633,121 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
> return GEM_RX_REJECT;
> }
>
> +/* Figure out which queue the recieved data should be sent to */
"received"
> +static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr)
Nothing seems to call this -- this probably results in a complaint
about an unused function if you build at this point in the series
(possibly only with optimisation on).
Do we need to also pass in the length of the rxbuf to avoid
reading beyond the end of short packets?
> +{
> + uint32_t reg;
> + bool matched, mismatched;
> + int i, j;
> +
> + for (i = 0; i < s->num_type1_screeners; i++) {
> + reg = s->regs[GEM_SCREENING_TYPE1_REGISTER_0 + i];
> + matched = false;
> + mismatched = false;
> +
> + /* Screening is based on UDP Port */
> + if (reg & GEM_ST1R_UDP_PORT_MATCH_ENABLE) {
> + uint16_t udp_port = rxbuf_ptr[14 + 22] << 8 | rxbuf_ptr[14 + 23];
> + if (udp_port == extract32(reg, GEM_ST1R_UDP_PORT_MATCH_SHIFT,
> + GEM_ST1R_UDP_PORT_MATCH_WIDTH)) {
> + matched = true;
> + } else {
> + mismatched = true;
> + }
> + }
> +
> + /* Screening is based on DS/TC */
> + if (reg & GEM_ST1R_DSTC_ENABLE) {
> + uint16_t dscp = rxbuf_ptr[14 + 1];
Why uint16_t if we're only reading one byte?
> + if (dscp == extract32(reg, GEM_ST1R_DSTC_MATCH_SHIFT,
> + GEM_ST1R_DSTC_MATCH_WIDTH)) {
> + matched = true;
> + } else {
> + mismatched = true;
> + }
> + }
> +
> + if (matched && !mismatched) {
> + return extract32(reg, GEM_ST1R_QUEUE_SHIFT, GEM_ST1R_QUEUE_WIDTH);
> + }
> + }
> +
> + for (i = 0; i < s->num_type2_screeners; i++) {
> + reg = s->regs[GEM_SCREENING_TYPE2_REGISTER_0 + i];
> + matched = false;
> + mismatched = false;
> +
> + if (reg & GEM_ST2R_ETHERTYPE_ENABLE) {
> + uint16_t type = rxbuf_ptr[12] << 8 | rxbuf_ptr[13];
> + int et_idx = extract32(reg, GEM_ST2R_ETHERTYPE_INDEX_SHIFT,
> + GEM_ST2R_ETHERTYPE_INDEX_WIDTH);
> +
> + if (et_idx > s->num_type2_screeners) {
> + qemu_log_mask(LOG_GUEST_ERROR, "Out of range ethertype "
> + "register index: %d\n", et_idx);
> + }
> + if (type == s->regs[GEM_SCREENING_TYPE2_ETHERTYPE_REG_0 +
> + et_idx]) {
> + matched = true;
> + } else {
> + mismatched = true;
> + }
> + }
> +
> + /* Compare A, B, C */
> + for (j = 0; j < 3; j++) {
> + uint32_t cr0, cr1, mask;
> + uint16_t rx_cmp;
> + int offset;
> + int cr_idx = extract32(reg, GEM_ST2R_COMPARE_A_SHIFT + j * 6,
> + GEM_ST2R_COMPARE_WIDTH);
> +
> + if (!(reg & (GEM_ST2R_COMPARE_A_ENABLE << (j * 6)))) {
> + continue;
> + }
> + if (cr_idx > s->num_type2_screeners) {
> + qemu_log_mask(LOG_GUEST_ERROR, "Out of range compare "
> + "register index: %d\n", cr_idx);
> + }
> +
> + cr0 = s->regs[GEM_TYPE2_COMPARE_0_WORD_0 + cr_idx * 2];
> + cr1 = s->regs[GEM_TYPE2_COMPARE_0_WORD_0 + cr_idx * 2 + 1];
> + offset = extract32(cr1, GEM_T2CW1_OFFSET_VALUE_SHIFT,
> + GEM_T2CW1_OFFSET_VALUE_WIDTH);
> +
> + switch (extract32(cr1, GEM_T2CW1_COMPARE_OFFSET_SHIFT,
> + GEM_T2CW1_COMPARE_OFFSET_WIDTH)) {
You pulled this out into 'offset' so you can just switch (offset).
> + case (3): /* Skip UDP header */
You don't need brackets around the constants in case labels.
> + qemu_log_mask(LOG_UNIMP, "TCP compare offsets"
> + "unimplemented - assuming UDP\n");
> + offset += 8;
> + /* Fallthrough */
> + case (2): /* skip the IP header */
> + offset += 20;
> + /* Fallthrough */
> + case (1): /* Count from after the ethertype */
> + offset += 14;
'break' here would be a good idea.
What should happen for case 0? Guest error?
> + }
> +
> + rx_cmp = rxbuf_ptr[offset] << 8 | rxbuf_ptr[offset];
> + mask = extract32(cr0, 0, 16);
> +
> + if ((rx_cmp & mask) == (extract32(cr0, 16, 16) & mask)) {
> + matched = true;
> + } else {
> + mismatched = true;
> + }
> + }
> +
> + if (matched && !mismatched) {
> + return extract32(reg, GEM_ST2R_QUEUE_SHIFT, GEM_ST2R_QUEUE_WIDTH);
> + }
> + }
> +
> + /* We made it here, assume it's queue 0 */
> + return 0;
> +}
> +
> static void gem_get_rx_desc(CadenceGEMState *s)
> {
> DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[0]);
> @@ -1263,6 +1410,10 @@ static Property gem_properties[] = {
> DEFINE_NIC_PROPERTIES(CadenceGEMState, conf),
> DEFINE_PROP_UINT8("num-priority-queues", CadenceGEMState,
> num_priority_queues, 1),
> + DEFINE_PROP_UINT8("num-type1-screeners", CadenceGEMState,
> + num_type1_screeners, 4),
> + DEFINE_PROP_UINT8("num-type2-screeners", CadenceGEMState,
> + num_type2_screeners, 4),
realize() should sanity check the properties aren't set too large.
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
> index 77e0582..f2f78c0 100644
> --- a/include/hw/net/cadence_gem.h
> +++ b/include/hw/net/cadence_gem.h
> @@ -46,6 +46,8 @@ typedef struct CadenceGEMState {
>
> /* Static properties */
> uint8_t num_priority_queues;
> + uint8_t num_type1_screeners;
> + uint8_t num_type2_screeners;
>
> /* GEM registers backing store */
> uint32_t regs[CADENCE_GEM_MAXREG];
> --
> 2.7.4
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/6] cadence_gem: Add queue support
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 4/6] cadence_gem: Add queue support Alistair Francis
@ 2016-07-26 12:06 ` Peter Maydell
2016-07-26 16:22 ` Alistair Francis
0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2016-07-26 12:06 UTC (permalink / raw)
To: Alistair Francis; +Cc: QEMU Developers, qemu-arm, Peter Crosthwaite
On 26 July 2016 at 01:12, 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.
>
> V2:
> - Use the new screening function
> - Update interrupt generation
> - Increase vmstate to 3.0
>
> hw/net/cadence_gem.c | 180 ++++++++++++++++++++++++++++++++-----------
> include/hw/net/cadence_gem.h | 2 +-
> 2 files changed, 135 insertions(+), 47 deletions(-)
>
> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
> index d38bc1e..28c2ddb 100644
> --- a/hw/net/cadence_gem.c
> +++ b/hw/net/cadence_gem.c
> @@ -142,6 +142,30 @@
> #define GEM_DESCONF6 (0x00000294/4)
> #define GEM_DESCONF7 (0x00000298/4)
>
> +#define GEM_INT_Q1_STATUS (0x00000400 / 4)
> +#define GEM_INT_Q1_MASK (0x00000640 / 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_SCREENING_TYPE1_REGISTER_0 (0x00000500 / 4)
>
> #define GEM_ST1R_UDP_PORT_MATCH_ENABLE (1 << 29)
> @@ -316,9 +340,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));
> @@ -448,6 +472,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);
>
> @@ -460,18 +485,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;
> }
> @@ -482,9 +509,20 @@ 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]);
> + int i;
> +
> + if (!s->num_priority_queues && s->regs[GEM_ISR]) {
Other parts of the code assume that num_priority_queues can't
be zero (ie that the smallest case is "one priority queue").
Either they're wrong or this is.
> + /* No priority queues, just trigger the interrupt */
> + DB_PRINT("asserting int.\n", i);
> qemu_set_irq(s->irq[0], 1);
> + return;
> + }
> +
> + for (i = 0; i < s->num_priority_queues; ++i) {
> + if (s->regs[GEM_INT_Q1_STATUS + i]) {
> + DB_PRINT("asserting int. (q=%d)\n", i);
> + qemu_set_irq(s->irq[i], 1);
> + }
> }
> }
>
> @@ -748,17 +786,17 @@ static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr)
> return 0;
> }
>
> -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) {
> 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 */
> @@ -779,6 +817,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);
>
> @@ -857,6 +896,9 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>
> DB_PRINT("config bufsize: %d packet size: %ld\n", rxbufsize, size);
>
> + /* Find which queue we are targetting */
> + q = get_queue_from_screen(s, rxbuf_ptr);
> +
> while (bytes_to_copy) {
> /* Do nothing if receive is not enabled. */
> if (!gem_can_receive(nc)) {
> @@ -865,10 +907,10 @@ 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[0]));
> + rx_desc_get_buffer(s->rx_desc[q]));
>
> /* Copy packet data to emulated DMA buffer */
> - cpu_physical_memory_write(rx_desc_get_buffer(s->rx_desc[0]) +
> + cpu_physical_memory_write(rx_desc_get_buffer(s->rx_desc[q]) +
> rxbuf_offset,
> rxbuf_ptr, MIN(bytes_to_copy, rxbufsize));
> rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
> @@ -876,47 +918,48 @@ 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. */
> - cpu_physical_memory_write(s->rx_desc_addr[0],
> - (uint8_t *)s->rx_desc[0],
> - sizeof(s->rx_desc[0]));
> + cpu_physical_memory_write(s->rx_desc_addr[q],
> + (uint8_t *)s->rx_desc[q],
> + sizeof(s->rx_desc[q]));
>
> /* 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 */
> @@ -988,6 +1031,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 earlier?
>
> /* Do nothing if transmit is not enabled. */
> if (!(s->regs[GEM_NWCTRL] & GEM_NWCTRL_TXENA)) {
> @@ -1003,8 +1047,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];
(This is an example of code which assumes num_priority_queues is at least 1.)
>
> DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
> cpu_physical_memory_read(packet_desc_addr,
> @@ -1016,7 +1061,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.
> @@ -1051,22 +1096,28 @@ 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]);
>
> + /* Update queue interrupt status */
> + if (s->num_priority_queues) {
> + s->regs[GEM_INT_Q1_STATUS + q] |=
> + GEM_INT_TXCMPL & ~(s->regs[GEM_INT_Q1_MASK + q]);
> + }
> +
> /* Handle interrupt consequences */
> gem_update_int_status(s);
>
> @@ -1108,6 +1159,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)
> @@ -1214,7 +1266,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
> {
> CadenceGEMState *s;
> uint32_t retval;
> -
> + int i;
> s = (CadenceGEMState *)opaque;
>
> offset >>= 2;
> @@ -1224,8 +1276,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) {
> @@ -1250,6 +1304,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;
> }
>
> @@ -1262,6 +1317,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;
> @@ -1281,14 +1337,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));
> @@ -1301,9 +1361,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;
> @@ -1311,10 +1377,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:
> @@ -1351,8 +1433,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 = {
> @@ -1366,8 +1451,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);
>
> @@ -1391,8 +1479,8 @@ static void gem_init(Object *obj)
>
> static const VMStateDescription vmstate_cadence_gem = {
> .name = "cadence_gem",
> - .version_id = 2,
> - .minimum_version_id = 3,
> + .version_id = 3,
> + .minimum_version_id = 0,
Something odd has happened here -- the minimum_version_id is wrong.
> .fields = (VMStateField[]) {
> VMSTATE_UINT32_ARRAY(regs, CadenceGEMState, CADENCE_GEM_MAXREG),
> VMSTATE_UINT16_ARRAY(phy_regs, CadenceGEMState, 32),
> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
> index f2f78c0..47a1661 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
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/6] cadence_gem: Add the num-priority-queues property
2016-07-26 11:47 ` Peter Maydell
@ 2016-07-26 16:10 ` Alistair Francis
0 siblings, 0 replies; 15+ messages in thread
From: Alistair Francis @ 2016-07-26 16:10 UTC (permalink / raw)
To: Peter Maydell
Cc: Alistair Francis, Peter Crosthwaite, qemu-arm, QEMU Developers
On Tue, Jul 26, 2016 at 4:47 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 26 July 2016 at 01:12, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> The Cadence GEM hardware supports N number priority queues, this patch is a
>> step towards that by adding the property to set the queues. At the moment
>> behaviour doesn't change as we only use queue 0.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>
>> @@ -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);
>
> At some point realize() needs to error-check the num-priority-queues
> property (ie check it isn't >8). Do you do that later in the series?
> (if so fine, if not, this patch is as good a place as any to put it.)
I'll add it in here.
>
>> @@ -1243,13 +1245,15 @@ static void gem_init(Object *obj)
>> static const VMStateDescription vmstate_cadence_gem = {
>> .name = "cadence_gem",
>> .version_id = 2,
>> - .minimum_version_id = 2,
>> + .minimum_version_id = 3,
>
> You need to bump .version_id too.
Yep, I misread it and thought it was a a.b version format. I'll fix up
the other one as well.
>
> Otherwise
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Thanks!
Alistair
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/6] cadence_gem: Add queue support
2016-07-26 12:06 ` Peter Maydell
@ 2016-07-26 16:22 ` Alistair Francis
0 siblings, 0 replies; 15+ messages in thread
From: Alistair Francis @ 2016-07-26 16:22 UTC (permalink / raw)
To: Peter Maydell
Cc: Alistair Francis, Peter Crosthwaite, qemu-arm, QEMU Developers
On Tue, Jul 26, 2016 at 5:06 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 26 July 2016 at 01:12, 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.
>>
>> V2:
>> - Use the new screening function
>> - Update interrupt generation
>> - Increase vmstate to 3.0
>>
>> hw/net/cadence_gem.c | 180 ++++++++++++++++++++++++++++++++-----------
>> include/hw/net/cadence_gem.h | 2 +-
>> 2 files changed, 135 insertions(+), 47 deletions(-)
>>
>> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
>> index d38bc1e..28c2ddb 100644
>> --- a/hw/net/cadence_gem.c
>> +++ b/hw/net/cadence_gem.c
>> @@ -142,6 +142,30 @@
>> #define GEM_DESCONF6 (0x00000294/4)
>> #define GEM_DESCONF7 (0x00000298/4)
>>
>> +#define GEM_INT_Q1_STATUS (0x00000400 / 4)
>> +#define GEM_INT_Q1_MASK (0x00000640 / 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_SCREENING_TYPE1_REGISTER_0 (0x00000500 / 4)
>>
>> #define GEM_ST1R_UDP_PORT_MATCH_ENABLE (1 << 29)
>> @@ -316,9 +340,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));
>> @@ -448,6 +472,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);
>>
>> @@ -460,18 +485,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;
>> }
>> @@ -482,9 +509,20 @@ 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]);
>> + int i;
>> +
>> + if (!s->num_priority_queues && s->regs[GEM_ISR]) {
>
> Other parts of the code assume that num_priority_queues can't
> be zero (ie that the smallest case is "one priority queue").
> Either they're wrong or this is.
This is, I have updated it and fixed everything else.
Thanks,
Alistair
>
>> + /* No priority queues, just trigger the interrupt */
>> + DB_PRINT("asserting int.\n", i);
>> qemu_set_irq(s->irq[0], 1);
>> + return;
>> + }
>> +
>> + for (i = 0; i < s->num_priority_queues; ++i) {
>> + if (s->regs[GEM_INT_Q1_STATUS + i]) {
>> + DB_PRINT("asserting int. (q=%d)\n", i);
>> + qemu_set_irq(s->irq[i], 1);
>> + }
>> }
>> }
>>
>> @@ -748,17 +786,17 @@ static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr)
>> return 0;
>> }
>>
>> -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) {
>> 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 */
>> @@ -779,6 +817,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);
>>
>> @@ -857,6 +896,9 @@ static ssize_t gem_receive(NetClientState *nc, const uint8_t *buf, size_t size)
>>
>> DB_PRINT("config bufsize: %d packet size: %ld\n", rxbufsize, size);
>>
>> + /* Find which queue we are targetting */
>> + q = get_queue_from_screen(s, rxbuf_ptr);
>> +
>> while (bytes_to_copy) {
>> /* Do nothing if receive is not enabled. */
>> if (!gem_can_receive(nc)) {
>> @@ -865,10 +907,10 @@ 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[0]));
>> + rx_desc_get_buffer(s->rx_desc[q]));
>>
>> /* Copy packet data to emulated DMA buffer */
>> - cpu_physical_memory_write(rx_desc_get_buffer(s->rx_desc[0]) +
>> + cpu_physical_memory_write(rx_desc_get_buffer(s->rx_desc[q]) +
>> rxbuf_offset,
>> rxbuf_ptr, MIN(bytes_to_copy, rxbufsize));
>> rxbuf_ptr += MIN(bytes_to_copy, rxbufsize);
>> @@ -876,47 +918,48 @@ 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. */
>> - cpu_physical_memory_write(s->rx_desc_addr[0],
>> - (uint8_t *)s->rx_desc[0],
>> - sizeof(s->rx_desc[0]));
>> + cpu_physical_memory_write(s->rx_desc_addr[q],
>> + (uint8_t *)s->rx_desc[q],
>> + sizeof(s->rx_desc[q]));
>>
>> /* 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 */
>> @@ -988,6 +1031,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 earlier?
>
>>
>> /* Do nothing if transmit is not enabled. */
>> if (!(s->regs[GEM_NWCTRL] & GEM_NWCTRL_TXENA)) {
>> @@ -1003,8 +1047,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];
>
> (This is an example of code which assumes num_priority_queues is at least 1.)
>
>>
>> DB_PRINT("read descriptor 0x%" HWADDR_PRIx "\n", packet_desc_addr);
>> cpu_physical_memory_read(packet_desc_addr,
>> @@ -1016,7 +1061,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.
>> @@ -1051,22 +1096,28 @@ 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]);
>>
>> + /* Update queue interrupt status */
>> + if (s->num_priority_queues) {
>> + s->regs[GEM_INT_Q1_STATUS + q] |=
>> + GEM_INT_TXCMPL & ~(s->regs[GEM_INT_Q1_MASK + q]);
>> + }
>> +
>> /* Handle interrupt consequences */
>> gem_update_int_status(s);
>>
>> @@ -1108,6 +1159,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)
>> @@ -1214,7 +1266,7 @@ static uint64_t gem_read(void *opaque, hwaddr offset, unsigned size)
>> {
>> CadenceGEMState *s;
>> uint32_t retval;
>> -
>> + int i;
>> s = (CadenceGEMState *)opaque;
>>
>> offset >>= 2;
>> @@ -1224,8 +1276,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) {
>> @@ -1250,6 +1304,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;
>> }
>>
>> @@ -1262,6 +1317,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;
>> @@ -1281,14 +1337,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));
>> @@ -1301,9 +1361,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;
>> @@ -1311,10 +1377,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:
>> @@ -1351,8 +1433,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 = {
>> @@ -1366,8 +1451,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);
>>
>> @@ -1391,8 +1479,8 @@ static void gem_init(Object *obj)
>>
>> static const VMStateDescription vmstate_cadence_gem = {
>> .name = "cadence_gem",
>> - .version_id = 2,
>> - .minimum_version_id = 3,
>> + .version_id = 3,
>> + .minimum_version_id = 0,
>
> Something odd has happened here -- the minimum_version_id is wrong.
>
>> .fields = (VMStateField[]) {
>> VMSTATE_UINT32_ARRAY(regs, CadenceGEMState, CADENCE_GEM_MAXREG),
>> VMSTATE_UINT16_ARRAY(phy_regs, CadenceGEMState, 32),
>> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
>> index f2f78c0..47a1661 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
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/6] cadence_gem: Add support for screening
2016-07-26 12:01 ` Peter Maydell
@ 2016-07-26 16:42 ` Alistair Francis
2016-07-26 16:55 ` Peter Maydell
0 siblings, 1 reply; 15+ messages in thread
From: Alistair Francis @ 2016-07-26 16:42 UTC (permalink / raw)
To: Peter Maydell
Cc: Alistair Francis, Peter Crosthwaite, qemu-arm, QEMU Developers
On Tue, Jul 26, 2016 at 5:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 26 July 2016 at 01:12, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> The Cadence GEM hardware allows incoming data to be 'screened' based on some
>> register values. Add support for these screens.
>>
>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>> ---
>> V2:
>> - Initial commit
>>
>> hw/net/cadence_gem.c | 151 +++++++++++++++++++++++++++++++++++++++++++
>> include/hw/net/cadence_gem.h | 2 +
>> 2 files changed, 153 insertions(+)
>>
>> diff --git a/hw/net/cadence_gem.c b/hw/net/cadence_gem.c
>> index deae122..d38bc1e 100644
>> --- a/hw/net/cadence_gem.c
>> +++ b/hw/net/cadence_gem.c
>> @@ -26,6 +26,7 @@
>> #include <zlib.h> /* For crc32 */
>>
>> #include "hw/net/cadence_gem.h"
>> +#include "qemu/log.h"
>> #include "net/checksum.h"
>>
>> #ifdef CADENCE_GEM_ERR_DEBUG
>> @@ -141,6 +142,37 @@
>> #define GEM_DESCONF6 (0x00000294/4)
>> #define GEM_DESCONF7 (0x00000298/4)
>>
>> +#define GEM_SCREENING_TYPE1_REGISTER_0 (0x00000500 / 4)
>> +
>> +#define GEM_ST1R_UDP_PORT_MATCH_ENABLE (1 << 29)
>> +#define GEM_ST1R_DSTC_ENABLE (1 << 28)
>> +#define GEM_ST1R_UDP_PORT_MATCH_SHIFT (12)
>> +#define GEM_ST1R_UDP_PORT_MATCH_WIDTH (27 - GEM_ST1R_UDP_PORT_MATCH_SHIFT + 1)
>> +#define GEM_ST1R_DSTC_MATCH_SHIFT (4)
>> +#define GEM_ST1R_DSTC_MATCH_WIDTH (11 - GEM_ST1R_DSTC_MATCH_SHIFT + 1)
>> +#define GEM_ST1R_QUEUE_SHIFT (0)
>> +#define GEM_ST1R_QUEUE_WIDTH (3 - GEM_ST1R_QUEUE_SHIFT + 1)
>> +
>> +#define GEM_SCREENING_TYPE2_REGISTER_0 (0x00000540 / 4)
>> +
>> +#define GEM_ST2R_COMPARE_A_ENABLE (1 << 18)
>> +#define GEM_ST2R_COMPARE_A_SHIFT (13)
>> +#define GEM_ST2R_COMPARE_WIDTH (17 - GEM_ST2R_COMPARE_A_SHIFT + 1)
>> +#define GEM_ST2R_ETHERTYPE_ENABLE (1 << 12)
>> +#define GEM_ST2R_ETHERTYPE_INDEX_SHIFT (9)
>> +#define GEM_ST2R_ETHERTYPE_INDEX_WIDTH (11 - GEM_ST2R_ETHERTYPE_INDEX_SHIFT \
>> + + 1)
>> +#define GEM_ST2R_QUEUE_SHIFT (0)
>> +#define GEM_ST2R_QUEUE_WIDTH (3 - GEM_ST2R_QUEUE_SHIFT + 1)
>> +
>> +#define GEM_SCREENING_TYPE2_ETHERTYPE_REG_0 (0x000006e0 / 4)
>> +#define GEM_TYPE2_COMPARE_0_WORD_0 (0x00000700 / 4)
>> +
>> +#define GEM_T2CW1_COMPARE_OFFSET_SHIFT (7)
>> +#define GEM_T2CW1_COMPARE_OFFSET_WIDTH (8 - GEM_T2CW1_COMPARE_OFFSET_SHIFT + 1)
>> +#define GEM_T2CW1_OFFSET_VALUE_SHIFT (0)
>> +#define GEM_T2CW1_OFFSET_VALUE_WIDTH (6 - GEM_T2CW1_OFFSET_VALUE_SHIFT + 1)
>> +
>> /*****************************************/
>> #define GEM_NWCTRL_TXSTART 0x00000200 /* Transmit Enable */
>> #define GEM_NWCTRL_TXENA 0x00000008 /* Transmit Enable */
>> @@ -601,6 +633,121 @@ static int gem_mac_address_filter(CadenceGEMState *s, const uint8_t *packet)
>> return GEM_RX_REJECT;
>> }
>>
>> +/* Figure out which queue the recieved data should be sent to */
>
> "received"
Fixed.
>
>> +static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr)
>
> Nothing seems to call this -- this probably results in a complaint
> about an unused function if you build at this point in the series
> (possibly only with optimisation on).
There is a warning about this. I wasn't sure what the position on
warnings in between patch a series was. I can squash this into the
next patch, I was just worried that the patch was getting a little
big.
What do you think?
>
> Do we need to also pass in the length of the rxbuf to avoid
> reading beyond the end of short packets?
That is probably a good idea.
>
>> +{
>> + uint32_t reg;
>> + bool matched, mismatched;
>> + int i, j;
>> +
>> + for (i = 0; i < s->num_type1_screeners; i++) {
>> + reg = s->regs[GEM_SCREENING_TYPE1_REGISTER_0 + i];
>> + matched = false;
>> + mismatched = false;
>> +
>> + /* Screening is based on UDP Port */
>> + if (reg & GEM_ST1R_UDP_PORT_MATCH_ENABLE) {
>> + uint16_t udp_port = rxbuf_ptr[14 + 22] << 8 | rxbuf_ptr[14 + 23];
>> + if (udp_port == extract32(reg, GEM_ST1R_UDP_PORT_MATCH_SHIFT,
>> + GEM_ST1R_UDP_PORT_MATCH_WIDTH)) {
>> + matched = true;
>> + } else {
>> + mismatched = true;
>> + }
>> + }
>> +
>> + /* Screening is based on DS/TC */
>> + if (reg & GEM_ST1R_DSTC_ENABLE) {
>> + uint16_t dscp = rxbuf_ptr[14 + 1];
>
> Why uint16_t if we're only reading one byte?
Good point, fixed.
>
>> + if (dscp == extract32(reg, GEM_ST1R_DSTC_MATCH_SHIFT,
>> + GEM_ST1R_DSTC_MATCH_WIDTH)) {
>> + matched = true;
>> + } else {
>> + mismatched = true;
>> + }
>> + }
>> +
>> + if (matched && !mismatched) {
>> + return extract32(reg, GEM_ST1R_QUEUE_SHIFT, GEM_ST1R_QUEUE_WIDTH);
>> + }
>> + }
>> +
>> + for (i = 0; i < s->num_type2_screeners; i++) {
>> + reg = s->regs[GEM_SCREENING_TYPE2_REGISTER_0 + i];
>> + matched = false;
>> + mismatched = false;
>> +
>> + if (reg & GEM_ST2R_ETHERTYPE_ENABLE) {
>> + uint16_t type = rxbuf_ptr[12] << 8 | rxbuf_ptr[13];
>> + int et_idx = extract32(reg, GEM_ST2R_ETHERTYPE_INDEX_SHIFT,
>> + GEM_ST2R_ETHERTYPE_INDEX_WIDTH);
>> +
>> + if (et_idx > s->num_type2_screeners) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "Out of range ethertype "
>> + "register index: %d\n", et_idx);
>> + }
>> + if (type == s->regs[GEM_SCREENING_TYPE2_ETHERTYPE_REG_0 +
>> + et_idx]) {
>> + matched = true;
>> + } else {
>> + mismatched = true;
>> + }
>> + }
>> +
>> + /* Compare A, B, C */
>> + for (j = 0; j < 3; j++) {
>> + uint32_t cr0, cr1, mask;
>> + uint16_t rx_cmp;
>> + int offset;
>> + int cr_idx = extract32(reg, GEM_ST2R_COMPARE_A_SHIFT + j * 6,
>> + GEM_ST2R_COMPARE_WIDTH);
>> +
>> + if (!(reg & (GEM_ST2R_COMPARE_A_ENABLE << (j * 6)))) {
>> + continue;
>> + }
>> + if (cr_idx > s->num_type2_screeners) {
>> + qemu_log_mask(LOG_GUEST_ERROR, "Out of range compare "
>> + "register index: %d\n", cr_idx);
>> + }
>> +
>> + cr0 = s->regs[GEM_TYPE2_COMPARE_0_WORD_0 + cr_idx * 2];
>> + cr1 = s->regs[GEM_TYPE2_COMPARE_0_WORD_0 + cr_idx * 2 + 1];
>> + offset = extract32(cr1, GEM_T2CW1_OFFSET_VALUE_SHIFT,
>> + GEM_T2CW1_OFFSET_VALUE_WIDTH);
>> +
>> + switch (extract32(cr1, GEM_T2CW1_COMPARE_OFFSET_SHIFT,
>> + GEM_T2CW1_COMPARE_OFFSET_WIDTH)) {
>
> You pulled this out into 'offset' so you can just switch (offset).
They are actually different.
>
>> + case (3): /* Skip UDP header */
>
> You don't need brackets around the constants in case labels.
Fixed
>
>> + qemu_log_mask(LOG_UNIMP, "TCP compare offsets"
>> + "unimplemented - assuming UDP\n");
>> + offset += 8;
>> + /* Fallthrough */
>> + case (2): /* skip the IP header */
>> + offset += 20;
>> + /* Fallthrough */
>> + case (1): /* Count from after the ethertype */
>> + offset += 14;
>
> 'break' here would be a good idea.
>
> What should happen for case 0? Guest error?
No change to offset. I added a case for it.
>
>> + }
>> +
>> + rx_cmp = rxbuf_ptr[offset] << 8 | rxbuf_ptr[offset];
>> + mask = extract32(cr0, 0, 16);
>> +
>> + if ((rx_cmp & mask) == (extract32(cr0, 16, 16) & mask)) {
>> + matched = true;
>> + } else {
>> + mismatched = true;
>> + }
>> + }
>> +
>> + if (matched && !mismatched) {
>> + return extract32(reg, GEM_ST2R_QUEUE_SHIFT, GEM_ST2R_QUEUE_WIDTH);
>> + }
>> + }
>> +
>> + /* We made it here, assume it's queue 0 */
>> + return 0;
>> +}
>> +
>> static void gem_get_rx_desc(CadenceGEMState *s)
>> {
>> DB_PRINT("read descriptor 0x%x\n", (unsigned)s->rx_desc_addr[0]);
>> @@ -1263,6 +1410,10 @@ static Property gem_properties[] = {
>> DEFINE_NIC_PROPERTIES(CadenceGEMState, conf),
>> DEFINE_PROP_UINT8("num-priority-queues", CadenceGEMState,
>> num_priority_queues, 1),
>> + DEFINE_PROP_UINT8("num-type1-screeners", CadenceGEMState,
>> + num_type1_screeners, 4),
>> + DEFINE_PROP_UINT8("num-type2-screeners", CadenceGEMState,
>> + num_type2_screeners, 4),
>
> realize() should sanity check the properties aren't set too large.
Ok, adding it.
Thanks,
Alistair
>
>> DEFINE_PROP_END_OF_LIST(),
>> };
>>
>> diff --git a/include/hw/net/cadence_gem.h b/include/hw/net/cadence_gem.h
>> index 77e0582..f2f78c0 100644
>> --- a/include/hw/net/cadence_gem.h
>> +++ b/include/hw/net/cadence_gem.h
>> @@ -46,6 +46,8 @@ typedef struct CadenceGEMState {
>>
>> /* Static properties */
>> uint8_t num_priority_queues;
>> + uint8_t num_type1_screeners;
>> + uint8_t num_type2_screeners;
>>
>> /* GEM registers backing store */
>> uint32_t regs[CADENCE_GEM_MAXREG];
>> --
>> 2.7.4
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/6] cadence_gem: Add support for screening
2016-07-26 16:42 ` Alistair Francis
@ 2016-07-26 16:55 ` Peter Maydell
2016-07-26 17:33 ` Alistair Francis
0 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2016-07-26 16:55 UTC (permalink / raw)
To: Alistair Francis; +Cc: Peter Crosthwaite, qemu-arm, QEMU Developers
On 26 July 2016 at 17:42, Alistair Francis <alistair.francis@xilinx.com> wrote:
> On Tue, Jul 26, 2016 at 5:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 26 July 2016 at 01:12, Alistair Francis <alistair.francis@xilinx.com> wrote:
>>> The Cadence GEM hardware allows incoming data to be 'screened' based on some
>>> register values. Add support for these screens.
>>>
>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>>> +static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr)
>>
>> Nothing seems to call this -- this probably results in a complaint
>> about an unused function if you build at this point in the series
>> (possibly only with optimisation on).
>
> There is a warning about this. I wasn't sure what the position on
> warnings in between patch a series was. I can squash this into the
> next patch, I was just worried that the patch was getting a little
> big.
>
> What do you think?
Warnings are compile failures by default, so they break bisection.
That makes them worth avoiding.
Here's a rearrangement that I think should work, though it's
a little tedious:
(1) change patch 2/6 so that instead of using hardcoded [0] for
the array dereferences it uses [q], with an "int q = 0;" at the
top of the relevant functions (gem_transmit and gem_receive).
(This will also reduce churn in patch 4 since we just go from
foo to foo[q] rather than foo to foo[0] to foo[q].)
(2) Then this patch can switch the 'q = 0' to the
+ /* Find which queue we are targetting */
+ q = get_queue_from_screen(s, rxbuf_ptr);
that's currently in patch 4. (It'll always return 0 at this point,
since the registers can't be written by the guest yet.)
>>> + offset = extract32(cr1, GEM_T2CW1_OFFSET_VALUE_SHIFT,
>>> + GEM_T2CW1_OFFSET_VALUE_WIDTH);
>>> +
>>> + switch (extract32(cr1, GEM_T2CW1_COMPARE_OFFSET_SHIFT,
>>> + GEM_T2CW1_COMPARE_OFFSET_WIDTH)) {
>>
>> You pulled this out into 'offset' so you can just switch (offset).
>
> They are actually different.
Oops, so they are...
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/6] cadence_gem: Add support for screening
2016-07-26 16:55 ` Peter Maydell
@ 2016-07-26 17:33 ` Alistair Francis
0 siblings, 0 replies; 15+ messages in thread
From: Alistair Francis @ 2016-07-26 17:33 UTC (permalink / raw)
To: Peter Maydell
Cc: Alistair Francis, Peter Crosthwaite, qemu-arm, QEMU Developers
On Tue, Jul 26, 2016 at 9:55 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 26 July 2016 at 17:42, Alistair Francis <alistair.francis@xilinx.com> wrote:
>> On Tue, Jul 26, 2016 at 5:01 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> On 26 July 2016 at 01:12, Alistair Francis <alistair.francis@xilinx.com> wrote:
>>>> The Cadence GEM hardware allows incoming data to be 'screened' based on some
>>>> register values. Add support for these screens.
>>>>
>>>> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com>
>
>>>> +static int get_queue_from_screen(CadenceGEMState *s, uint8_t *rxbuf_ptr)
>>>
>>> Nothing seems to call this -- this probably results in a complaint
>>> about an unused function if you build at this point in the series
>>> (possibly only with optimisation on).
>>
>> There is a warning about this. I wasn't sure what the position on
>> warnings in between patch a series was. I can squash this into the
>> next patch, I was just worried that the patch was getting a little
>> big.
>>
>> What do you think?
>
> Warnings are compile failures by default, so they break bisection.
> That makes them worth avoiding.
>
> Here's a rearrangement that I think should work, though it's
> a little tedious:
>
> (1) change patch 2/6 so that instead of using hardcoded [0] for
> the array dereferences it uses [q], with an "int q = 0;" at the
> top of the relevant functions (gem_transmit and gem_receive).
> (This will also reduce churn in patch 4 since we just go from
> foo to foo[q] rather than foo to foo[0] to foo[q].)
>
> (2) Then this patch can switch the 'q = 0' to the
> + /* Find which queue we are targetting */
> + q = get_queue_from_screen(s, rxbuf_ptr);
>
> that's currently in patch 4. (It'll always return 0 at this point,
> since the registers can't be written by the guest yet.)
I was hoping to avoid that, but it actually wasn't too bad. Sending
the next version now.
Thanks,
Alistair
>
>>>> + offset = extract32(cr1, GEM_T2CW1_OFFSET_VALUE_SHIFT,
>>>> + GEM_T2CW1_OFFSET_VALUE_WIDTH);
>>>> +
>>>> + switch (extract32(cr1, GEM_T2CW1_COMPARE_OFFSET_SHIFT,
>>>> + GEM_T2CW1_COMPARE_OFFSET_WIDTH)) {
>>>
>>> You pulled this out into 'offset' so you can just switch (offset).
>>
>> They are actually different.
>
> Oops, so they are...
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2016-07-26 17:33 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-26 0:12 [Qemu-devel] [PATCH v2 0/6] Add support for Cadence GEM priority queues Alistair Francis
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 1/6] cadence_gem: QOMify Cadence GEM Alistair Francis
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 2/6] cadence_gem: Add the num-priority-queues property Alistair Francis
2016-07-26 11:47 ` Peter Maydell
2016-07-26 16:10 ` Alistair Francis
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 3/6] cadence_gem: Add support for screening Alistair Francis
2016-07-26 12:01 ` Peter Maydell
2016-07-26 16:42 ` Alistair Francis
2016-07-26 16:55 ` Peter Maydell
2016-07-26 17:33 ` Alistair Francis
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 4/6] cadence_gem: Add queue support Alistair Francis
2016-07-26 12:06 ` Peter Maydell
2016-07-26 16:22 ` Alistair Francis
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 5/6] cadence_gem: Correct indentation Alistair Francis
2016-07-26 0:12 ` [Qemu-devel] [PATCH v2 6/6] xlnx-zynqmp: Set the number of priority queues 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.