[PATCH 00/20] hw/net/xilinx_ethlite: Map RAM buffers as RAM and remove tswap() calls

[PATCH 00/20] hw/net/xilinx_ethlite: Map RAM buffers as RAM and remove tswap() calls

[Philippe Mathieu-Daudé]

This is the result of a long discussion with Edgar (started few
years ago!) and Paolo:
https://lore.kernel.org/qemu-devel/34f6fe2f-06e0-4e2a-a361-2d662f6814b5@redhat.com/
After clarification from Richard on MMIO/RAM accesses, I figured
strengthening the model regions would make things obvious,
eventually allowing to remove the tswap() calls for good.

This costly series mostly plays around with MemoryRegions.

The model has a mix of RAM/MMIO in its address range. Currently
they are implemented as a MMIO array of u32. Since the core
memory layer swaps accesses for MMIO, the device implementation
has to swap them back.
In order to avoid that, we'll map the RAM regions as RAM MRs.
First we move each MMIO register to new MMIO regions (RX and TX).
Then what is left are the RAM buffers; we convert them to RAM MRs,
removing the need for tswap() at all.

Once reviewed, I'll respin my "hw/microblaze: Allow running
cross-endian vCPUs" series based on this.

Please review,

Phil.

Philippe Mathieu-Daudé (20):
  hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit
  hw/net/xilinx_ethlite: Convert some debug logs to trace events
  hw/net/xilinx_ethlite: Remove unuseful debug logs
  hw/net/xilinx_ethlite: Update QOM style
  hw/net/xilinx_ethlite: Correct maximum RX buffer size
  hw/net/xilinx_ethlite: Map MDIO registers (as unimplemented)
  hw/net/xilinx_ethlite: Rename rxbuf -> port_index
  hw/net/xilinx_ethlite: Add addr_to_port_index() helper
  hw/net/xilinx_ethlite: Introduce txbuf_ptr() helper
  hw/net/xilinx_ethlite: Introduce rxbuf_ptr() helper
  hw/net/xilinx_ethlite: Access RX_CTRL register for each port
  hw/net/xilinx_ethlite: Access TX_GIE register for each port
  hw/net/xilinx_ethlite: Access TX_LEN register for each port
  hw/net/xilinx_ethlite: Access TX_CTRL register for each port
  hw/net/xilinx_ethlite: Map RX_CTRL as MMIO
  hw/net/xilinx_ethlite: Map TX_LEN as MMIO
  hw/net/xilinx_ethlite: Map TX_GIE as MMIO
  hw/net/xilinx_ethlite: Map TX_CTRL as MMIO
  hw/net/xilinx_ethlite: Map the RAM buffer as RAM memory region
  hw/net/xilinx_ethlite: Rename 'mmio' MR as 'container'

 hw/char/xilinx_uartlite.c |   4 +
 hw/intc/xilinx_intc.c     |   4 +
 hw/net/xilinx_ethlite.c   | 357 ++++++++++++++++++++++++--------------
 hw/timer/xilinx_timer.c   |   4 +
 hw/net/trace-events       |   4 +
 5 files changed, 246 insertions(+), 127 deletions(-)

-- 
2.45.2

[PATCH 01/20] hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit

[Philippe Mathieu-Daudé]

All these MemoryRegionOps read() and write() handlers are
implemented expecting 32-bit accesses. Clarify that setting
.impl.min/max_access_size fields.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Anton Johansson <anjo@rev.ng>
---
 hw/char/xilinx_uartlite.c | 4 ++++
 hw/intc/xilinx_intc.c     | 4 ++++
 hw/net/xilinx_ethlite.c   | 4 ++++
 hw/timer/xilinx_timer.c   | 4 ++++
 4 files changed, 16 insertions(+)

diff --git a/hw/char/xilinx_uartlite.c b/hw/char/xilinx_uartlite.c
index f325084f8b..3022b3d8ef 100644
--- a/hw/char/xilinx_uartlite.c
+++ b/hw/char/xilinx_uartlite.c
@@ -170,6 +170,10 @@ static const MemoryRegionOps uart_ops = {
     .read = uart_read,
     .write = uart_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
     .valid = {
         .min_access_size = 1,
         .max_access_size = 4
diff --git a/hw/intc/xilinx_intc.c b/hw/intc/xilinx_intc.c
index 6e5012e66e..8fb6b4f1a5 100644
--- a/hw/intc/xilinx_intc.c
+++ b/hw/intc/xilinx_intc.c
@@ -144,6 +144,10 @@ static const MemoryRegionOps pic_ops = {
     .read = pic_read,
     .write = pic_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4
diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index bd81290808..e84b4cdd35 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -170,6 +170,10 @@ static const MemoryRegionOps eth_ops = {
     .read = eth_read,
     .write = eth_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4
diff --git a/hw/timer/xilinx_timer.c b/hw/timer/xilinx_timer.c
index 32a9df69e0..383fc8b3c8 100644
--- a/hw/timer/xilinx_timer.c
+++ b/hw/timer/xilinx_timer.c
@@ -193,6 +193,10 @@ static const MemoryRegionOps timer_ops = {
     .read = timer_read,
     .write = timer_write,
     .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 4,
+        .max_access_size = 4,
+    },
     .valid = {
         .min_access_size = 4,
         .max_access_size = 4
-- 
2.45.2

[PATCH 02/20] hw/net/xilinx_ethlite: Convert some debug logs to trace events

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 5 +++--
 hw/net/trace-events     | 4 ++++
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index e84b4cdd35..bb330a233f 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -30,6 +30,7 @@
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
 #include "net/net.h"
+#include "trace.h"
 
 #define D(x)
 #define R_TX_BUF0     0
@@ -198,13 +199,13 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
         return size;
 
     if (s->regs[rxbase + R_RX_CTRL0] & CTRL_S) {
-        D(qemu_log("ethlite lost packet %x\n", s->regs[R_RX_CTRL0]));
+        trace_ethlite_pkt_lost(s->regs[R_RX_CTRL0]);
         return -1;
     }
 
     D(qemu_log("%s %zd rxbase=%x\n", __func__, size, rxbase));
     if (size > (R_MAX - R_RX_BUF0 - rxbase) * 4) {
-        D(qemu_log("ethlite packet is too big, size=%x\n", size));
+        trace_ethlite_pkt_size_too_big(size);
         return -1;
     }
     memcpy(&s->regs[rxbase + R_RX_BUF0], buf, size);
diff --git a/hw/net/trace-events b/hw/net/trace-events
index d0f1d8c0fb..2b36cd967e 100644
--- a/hw/net/trace-events
+++ b/hw/net/trace-events
@@ -511,3 +511,7 @@ xen_netdev_connect(int dev, unsigned int tx, unsigned int rx, int port) "vif%u t
 xen_netdev_frontend_changed(const char *dev, int state) "vif%s state %d"
 xen_netdev_tx(int dev, int ref, int off, int len, unsigned int flags, const char *c, const char *d, const char *m, const char *e) "vif%u ref %u off %u len %u flags 0x%x%s%s%s%s"
 xen_netdev_rx(int dev, int idx, int status, int flags) "vif%u idx %d status %d flags 0x%x"
+
+# xilinx_ethlite.c
+ethlite_pkt_lost(uint32_t rx_ctrl) "rx_ctrl:0x%" PRIx32
+ethlite_pkt_size_too_big(uint64_t size) "size:0x%" PRIx64
-- 
2.45.2

[PATCH 03/20] hw/net/xilinx_ethlite: Remove unuseful debug logs

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index bb330a233f..2b52597f03 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -32,7 +32,6 @@
 #include "net/net.h"
 #include "trace.h"
 
-#define D(x)
 #define R_TX_BUF0     0
 #define R_TX_LEN0     (0x07f4 / 4)
 #define R_TX_GIE0     (0x07f8 / 4)
@@ -100,7 +99,6 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
         case R_RX_CTRL1:
         case R_RX_CTRL0:
             r = s->regs[addr];
-            D(qemu_log("%s " HWADDR_FMT_plx "=%x\n", __func__, addr * 4, r));
             break;
 
         default:
@@ -126,13 +124,10 @@ eth_write(void *opaque, hwaddr addr,
             if (addr == R_TX_CTRL1)
                 base = 0x800 / 4;
 
-            D(qemu_log("%s addr=" HWADDR_FMT_plx " val=%x\n",
-                       __func__, addr * 4, value));
             if ((value & (CTRL_P | CTRL_S)) == CTRL_S) {
                 qemu_send_packet(qemu_get_queue(s->nic),
                                  (void *) &s->regs[base],
                                  s->regs[base + R_TX_LEN0]);
-                D(qemu_log("eth_tx %d\n", s->regs[base + R_TX_LEN0]));
                 if (s->regs[base + R_TX_CTRL0] & CTRL_I)
                     eth_pulse_irq(s);
             } else if ((value & (CTRL_P | CTRL_S)) == (CTRL_P | CTRL_S)) {
@@ -156,8 +151,6 @@ eth_write(void *opaque, hwaddr addr,
         case R_TX_LEN0:
         case R_TX_LEN1:
         case R_TX_GIE0:
-            D(qemu_log("%s addr=" HWADDR_FMT_plx " val=%x\n",
-                       __func__, addr * 4, value));
             s->regs[addr] = value;
             break;
 
@@ -203,7 +196,6 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
         return -1;
     }
 
-    D(qemu_log("%s %zd rxbase=%x\n", __func__, size, rxbase));
     if (size > (R_MAX - R_RX_BUF0 - rxbase) * 4) {
         trace_ethlite_pkt_size_too_big(size);
         return -1;
-- 
2.45.2

[PATCH 04/20] hw/net/xilinx_ethlite: Update QOM style

[Philippe Mathieu-Daudé]

Use XlnxXpsEthLite typedef, OBJECT_DECLARE_SIMPLE_TYPE macro;
convert type_init() to DEFINE_TYPES().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 48 +++++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 26 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 2b52597f03..0f59811c78 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -53,10 +53,9 @@
 #define CTRL_S     0x1
 
 #define TYPE_XILINX_ETHLITE "xlnx.xps-ethernetlite"
-DECLARE_INSTANCE_CHECKER(struct xlx_ethlite, XILINX_ETHLITE,
-                         TYPE_XILINX_ETHLITE)
+OBJECT_DECLARE_SIMPLE_TYPE(XlnxXpsEthLite, XILINX_ETHLITE)
 
-struct xlx_ethlite
+struct XlnxXpsEthLite
 {
     SysBusDevice parent_obj;
 
@@ -73,7 +72,7 @@ struct xlx_ethlite
     uint32_t regs[R_MAX];
 };
 
-static inline void eth_pulse_irq(struct xlx_ethlite *s)
+static inline void eth_pulse_irq(XlnxXpsEthLite *s)
 {
     /* Only the first gie reg is active.  */
     if (s->regs[R_TX_GIE0] & GIE_GIE) {
@@ -84,7 +83,7 @@ static inline void eth_pulse_irq(struct xlx_ethlite *s)
 static uint64_t
 eth_read(void *opaque, hwaddr addr, unsigned int size)
 {
-    struct xlx_ethlite *s = opaque;
+    XlnxXpsEthLite *s = opaque;
     uint32_t r = 0;
 
     addr >>= 2;
@@ -112,7 +111,7 @@ static void
 eth_write(void *opaque, hwaddr addr,
           uint64_t val64, unsigned int size)
 {
-    struct xlx_ethlite *s = opaque;
+    XlnxXpsEthLite *s = opaque;
     unsigned int base = 0;
     uint32_t value = val64;
 
@@ -176,7 +175,7 @@ static const MemoryRegionOps eth_ops = {
 
 static bool eth_can_rx(NetClientState *nc)
 {
-    struct xlx_ethlite *s = qemu_get_nic_opaque(nc);
+    XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
     unsigned int rxbase = s->rxbuf * (0x800 / 4);
 
     return !(s->regs[rxbase + R_RX_CTRL0] & CTRL_S);
@@ -184,7 +183,7 @@ static bool eth_can_rx(NetClientState *nc)
 
 static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
 {
-    struct xlx_ethlite *s = qemu_get_nic_opaque(nc);
+    XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
     unsigned int rxbase = s->rxbuf * (0x800 / 4);
 
     /* DA filter.  */
@@ -214,7 +213,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
 
 static void xilinx_ethlite_reset(DeviceState *dev)
 {
-    struct xlx_ethlite *s = XILINX_ETHLITE(dev);
+    XlnxXpsEthLite *s = XILINX_ETHLITE(dev);
 
     s->rxbuf = 0;
 }
@@ -228,7 +227,7 @@ static NetClientInfo net_xilinx_ethlite_info = {
 
 static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
 {
-    struct xlx_ethlite *s = XILINX_ETHLITE(dev);
+    XlnxXpsEthLite *s = XILINX_ETHLITE(dev);
 
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
     s->nic = qemu_new_nic(&net_xilinx_ethlite_info, &s->conf,
@@ -239,7 +238,7 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
 
 static void xilinx_ethlite_init(Object *obj)
 {
-    struct xlx_ethlite *s = XILINX_ETHLITE(obj);
+    XlnxXpsEthLite *s = XILINX_ETHLITE(obj);
 
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
 
@@ -249,9 +248,9 @@ static void xilinx_ethlite_init(Object *obj)
 }
 
 static Property xilinx_ethlite_properties[] = {
-    DEFINE_PROP_UINT32("tx-ping-pong", struct xlx_ethlite, c_tx_pingpong, 1),
-    DEFINE_PROP_UINT32("rx-ping-pong", struct xlx_ethlite, c_rx_pingpong, 1),
-    DEFINE_NIC_PROPERTIES(struct xlx_ethlite, conf),
+    DEFINE_PROP_UINT32("tx-ping-pong", XlnxXpsEthLite, c_tx_pingpong, 1),
+    DEFINE_PROP_UINT32("rx-ping-pong", XlnxXpsEthLite, c_rx_pingpong, 1),
+    DEFINE_NIC_PROPERTIES(XlnxXpsEthLite, conf),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -264,17 +263,14 @@ static void xilinx_ethlite_class_init(ObjectClass *klass, void *data)
     device_class_set_props(dc, xilinx_ethlite_properties);
 }
 
-static const TypeInfo xilinx_ethlite_info = {
-    .name          = TYPE_XILINX_ETHLITE,
-    .parent        = TYPE_SYS_BUS_DEVICE,
-    .instance_size = sizeof(struct xlx_ethlite),
-    .instance_init = xilinx_ethlite_init,
-    .class_init    = xilinx_ethlite_class_init,
+static const TypeInfo xilinx_ethlite_types[] = {
+    {
+        .name          = TYPE_XILINX_ETHLITE,
+        .parent        = TYPE_SYS_BUS_DEVICE,
+        .instance_size = sizeof(XlnxXpsEthLite),
+        .instance_init = xilinx_ethlite_init,
+        .class_init    = xilinx_ethlite_class_init,
+    },
 };
 
-static void xilinx_ethlite_register_types(void)
-{
-    type_register_static(&xilinx_ethlite_info);
-}
-
-type_init(xilinx_ethlite_register_types)
+DEFINE_TYPES(xilinx_ethlite_types)
-- 
2.45.2

[PATCH 05/20] hw/net/xilinx_ethlite: Correct maximum RX buffer size

[Philippe Mathieu-Daudé]

The current max RX bufsize is set to 0x800. This is
invalid, since it contains the MMIO registers region.
Add the correct definition and use it.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 0f59811c78..e6f6179fce 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -46,6 +46,8 @@
 #define R_RX_CTRL1    (0x1ffc / 4)
 #define R_MAX         (0x2000 / 4)
 
+#define RX_BUFSZ_MAX  0x07e0
+
 #define GIE_GIE    0x80000000
 
 #define CTRL_I     0x8
@@ -195,7 +197,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
         return -1;
     }
 
-    if (size > (R_MAX - R_RX_BUF0 - rxbase) * 4) {
+    if (size > RX_BUFSZ_MAX) {
         trace_ethlite_pkt_size_too_big(size);
         return -1;
     }
-- 
2.45.2

[PATCH 06/20] hw/net/xilinx_ethlite: Map MDIO registers (as unimplemented)

[Philippe Mathieu-Daudé]

Rather than handling the MDIO registers as RAM, map them
as unimplemented I/O within the device MR.

The memory flat view becomes:

  (qemu) info mtree -f
  FlatView #0
   Root memory region: system
    0000000081000000-00000000810007e3 (prio 0, i/o): xlnx.xps-ethernetlite
    00000000810007e4-00000000810007f3 (prio 0, i/o): ethlite.mdio
    00000000810007f4-0000000081001fff (prio 0, i/o): xlnx.xps-ethernetlite @00000000000007f4

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index e6f6179fce..76b1e7d826 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -25,13 +25,17 @@
 #include "qemu/osdep.h"
 #include "qemu/module.h"
 #include "qom/object.h"
+#include "qapi/error.h"
 #include "exec/tswap.h"
 #include "hw/sysbus.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
+#include "hw/misc/unimp.h"
 #include "net/net.h"
 #include "trace.h"
 
+#define A_MDIO_BASE   0x07e4
+
 #define R_TX_BUF0     0
 #define R_TX_LEN0     (0x07f4 / 4)
 #define R_TX_GIE0     (0x07f8 / 4)
@@ -71,6 +75,7 @@ struct XlnxXpsEthLite
     unsigned int txbuf;
     unsigned int rxbuf;
 
+    UnimplementedDeviceState mdio;
     uint32_t regs[R_MAX];
 };
 
@@ -231,6 +236,14 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
 {
     XlnxXpsEthLite *s = XILINX_ETHLITE(dev);
 
+    object_initialize_child(OBJECT(dev), "ethlite.mdio", &s->mdio,
+                           TYPE_UNIMPLEMENTED_DEVICE);
+    qdev_prop_set_string(DEVICE(&s->mdio), "name", "ethlite.mdio");
+    qdev_prop_set_uint64(DEVICE(&s->mdio), "size", 4 * 4);
+    sysbus_realize(SYS_BUS_DEVICE(&s->mdio), &error_fatal);
+    memory_region_add_subregion(&s->mmio, A_MDIO_BASE,
+                            sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mdio), 0));
+
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
     s->nic = qemu_new_nic(&net_xilinx_ethlite_info, &s->conf,
                           object_get_typename(OBJECT(dev)), dev->id,
-- 
2.45.2

[PATCH 07/20] hw/net/xilinx_ethlite: Rename rxbuf -> port_index

[Philippe Mathieu-Daudé]

'rxbuf' is the index of the port used. Rename it as 'port_index'.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 76b1e7d826..20919b4f54 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -72,8 +72,7 @@ struct XlnxXpsEthLite
 
     uint32_t c_tx_pingpong;
     uint32_t c_rx_pingpong;
-    unsigned int txbuf;
-    unsigned int rxbuf;
+    unsigned int port_index;
 
     UnimplementedDeviceState mdio;
     uint32_t regs[R_MAX];
@@ -183,7 +182,7 @@ static const MemoryRegionOps eth_ops = {
 static bool eth_can_rx(NetClientState *nc)
 {
     XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
-    unsigned int rxbase = s->rxbuf * (0x800 / 4);
+    unsigned int rxbase = s->port_index * (0x800 / 4);
 
     return !(s->regs[rxbase + R_RX_CTRL0] & CTRL_S);
 }
@@ -191,7 +190,7 @@ static bool eth_can_rx(NetClientState *nc)
 static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
 {
     XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
-    unsigned int rxbase = s->rxbuf * (0x800 / 4);
+    unsigned int rxbase = s->port_index * (0x800 / 4);
 
     /* DA filter.  */
     if (!(buf[0] & 0x80) && memcmp(&s->conf.macaddr.a[0], buf, 6))
@@ -214,7 +213,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
     }
 
     /* If c_rx_pingpong was set flip buffers.  */
-    s->rxbuf ^= s->c_rx_pingpong;
+    s->port_index ^= s->c_rx_pingpong;
     return size;
 }
 
@@ -222,7 +221,7 @@ static void xilinx_ethlite_reset(DeviceState *dev)
 {
     XlnxXpsEthLite *s = XILINX_ETHLITE(dev);
 
-    s->rxbuf = 0;
+    s->port_index = 0;
 }
 
 static NetClientInfo net_xilinx_ethlite_info = {
-- 
2.45.2

[PATCH 08/20] hw/net/xilinx_ethlite: Add addr_to_port_index() helper

[Philippe Mathieu-Daudé]

For a particular physical address within the EthLite MMIO range,
addr_to_port_index() returns which port is accessed.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 20919b4f54..fe91891310 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -24,6 +24,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/module.h"
+#include "qemu/bitops.h"
 #include "qom/object.h"
 #include "qapi/error.h"
 #include "exec/tswap.h"
@@ -86,6 +87,12 @@ static inline void eth_pulse_irq(XlnxXpsEthLite *s)
     }
 }
 
+__attribute__((unused))
+static unsigned addr_to_port_index(hwaddr addr)
+{
+    return extract64(addr, 11, 1);
+}
+
 static uint64_t
 eth_read(void *opaque, hwaddr addr, unsigned int size)
 {
@@ -190,7 +197,8 @@ static bool eth_can_rx(NetClientState *nc)
 static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
 {
     XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
-    unsigned int rxbase = s->port_index * (0x800 / 4);
+    unsigned int port_index = s->port_index;
+    unsigned int rxbase = port_index * (0x800 / 4);
 
     /* DA filter.  */
     if (!(buf[0] & 0x80) && memcmp(&s->conf.macaddr.a[0], buf, 6))
-- 
2.45.2

[PATCH 09/20] hw/net/xilinx_ethlite: Introduce txbuf_ptr() helper

[Philippe Mathieu-Daudé]

txbuf_ptr() points to the beginning of a (RAM) TX buffer
within the device state.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index fe91891310..d4882f43f7 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -87,12 +87,18 @@ static inline void eth_pulse_irq(XlnxXpsEthLite *s)
     }
 }
 
-__attribute__((unused))
 static unsigned addr_to_port_index(hwaddr addr)
 {
     return extract64(addr, 11, 1);
 }
 
+static void *txbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
+{
+    unsigned int rxbase = port_index * (0x800 / 4);
+
+    return &s->regs[rxbase + R_TX_BUF0];
+}
+
 static uint64_t
 eth_read(void *opaque, hwaddr addr, unsigned int size)
 {
@@ -125,6 +131,7 @@ eth_write(void *opaque, hwaddr addr,
           uint64_t val64, unsigned int size)
 {
     XlnxXpsEthLite *s = opaque;
+    unsigned int port_index = addr_to_port_index(addr);
     unsigned int base = 0;
     uint32_t value = val64;
 
@@ -138,12 +145,12 @@ eth_write(void *opaque, hwaddr addr,
 
             if ((value & (CTRL_P | CTRL_S)) == CTRL_S) {
                 qemu_send_packet(qemu_get_queue(s->nic),
-                                 (void *) &s->regs[base],
+                                 txbuf_ptr(s, port_index),
                                  s->regs[base + R_TX_LEN0]);
                 if (s->regs[base + R_TX_CTRL0] & CTRL_I)
                     eth_pulse_irq(s);
             } else if ((value & (CTRL_P | CTRL_S)) == (CTRL_P | CTRL_S)) {
-                memcpy(&s->conf.macaddr.a[0], &s->regs[base], 6);
+                memcpy(&s->conf.macaddr.a[0], txbuf_ptr(s, port_index), 6);
                 if (s->regs[base + R_TX_CTRL0] & CTRL_I)
                     eth_pulse_irq(s);
             }
-- 
2.45.2

[PATCH 10/20] hw/net/xilinx_ethlite: Introduce rxbuf_ptr() helper

[Philippe Mathieu-Daudé]

rxbuf_ptr() points to the beginning of a (RAM) RX buffer
within the device state.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index d4882f43f7..fdbf25fd91 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -99,6 +99,13 @@ static void *txbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
     return &s->regs[rxbase + R_TX_BUF0];
 }
 
+static void *rxbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
+{
+    unsigned int rxbase = port_index * (0x800 / 4);
+
+    return &s->regs[rxbase + R_RX_BUF0];
+}
+
 static uint64_t
 eth_read(void *opaque, hwaddr addr, unsigned int size)
 {
@@ -220,7 +227,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
         trace_ethlite_pkt_size_too_big(size);
         return -1;
     }
-    memcpy(&s->regs[rxbase + R_RX_BUF0], buf, size);
+    memcpy(rxbuf_ptr(s, port_index), buf, size);
 
     s->regs[rxbase + R_RX_CTRL0] |= CTRL_S;
     if (s->regs[R_RX_CTRL0] & CTRL_I) {
-- 
2.45.2

[PATCH 11/20] hw/net/xilinx_ethlite: Access RX_CTRL register for each port

[Philippe Mathieu-Daudé]

Rather than accessing the registers within the mixed RAM/MMIO
region as indexed register, declare a per-port RX_CTRL. This
will help to map the RAM as RAM (keeping MMIO as MMIO) in few
commits.

Previous s->regs[R_RX_CTRL0] and s->regs[R_RX_CTRL1] are now
unused. Not a concern, this array will soon disappear.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 31 +++++++++++++++++++++----------
 1 file changed, 21 insertions(+), 10 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index fdbf25fd91..605451a522 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -59,6 +59,13 @@
 #define CTRL_P     0x2
 #define CTRL_S     0x1
 
+typedef struct XlnxXpsEthLitePort
+{
+    struct {
+        uint32_t rx_ctrl;
+    } reg;
+} XlnxXpsEthLitePort;
+
 #define TYPE_XILINX_ETHLITE "xlnx.xps-ethernetlite"
 OBJECT_DECLARE_SIMPLE_TYPE(XlnxXpsEthLite, XILINX_ETHLITE)
 
@@ -76,6 +83,7 @@ struct XlnxXpsEthLite
     unsigned int port_index;
 
     UnimplementedDeviceState mdio;
+    XlnxXpsEthLitePort port[2];
     uint32_t regs[R_MAX];
 };
 
@@ -110,6 +118,7 @@ static uint64_t
 eth_read(void *opaque, hwaddr addr, unsigned int size)
 {
     XlnxXpsEthLite *s = opaque;
+    unsigned port_index = addr_to_port_index(addr);
     uint32_t r = 0;
 
     addr >>= 2;
@@ -121,11 +130,13 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
         case R_TX_LEN1:
         case R_TX_CTRL1:
         case R_TX_CTRL0:
-        case R_RX_CTRL1:
-        case R_RX_CTRL0:
             r = s->regs[addr];
             break;
 
+        case R_RX_CTRL1:
+        case R_RX_CTRL0:
+            r = s->port[port_index].reg.rx_ctrl;
+
         default:
             r = tswap32(s->regs[addr]);
             break;
@@ -173,7 +184,9 @@ eth_write(void *opaque, hwaddr addr,
             if (!(value & CTRL_S)) {
                 qemu_flush_queued_packets(qemu_get_queue(s->nic));
             }
-            /* fall through */
+            s->port[port_index].reg.rx_ctrl = value;
+            break;
+
         case R_TX_LEN0:
         case R_TX_LEN1:
         case R_TX_GIE0:
@@ -203,23 +216,21 @@ static const MemoryRegionOps eth_ops = {
 static bool eth_can_rx(NetClientState *nc)
 {
     XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
-    unsigned int rxbase = s->port_index * (0x800 / 4);
 
-    return !(s->regs[rxbase + R_RX_CTRL0] & CTRL_S);
+    return !(s->port[s->port_index].reg.rx_ctrl & CTRL_S);
 }
 
 static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
 {
     XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
     unsigned int port_index = s->port_index;
-    unsigned int rxbase = port_index * (0x800 / 4);
 
     /* DA filter.  */
     if (!(buf[0] & 0x80) && memcmp(&s->conf.macaddr.a[0], buf, 6))
         return size;
 
-    if (s->regs[rxbase + R_RX_CTRL0] & CTRL_S) {
-        trace_ethlite_pkt_lost(s->regs[R_RX_CTRL0]);
+    if (s->port[port_index].reg.rx_ctrl & CTRL_S) {
+        trace_ethlite_pkt_lost(s->port[port_index].reg.rx_ctrl);
         return -1;
     }
 
@@ -229,8 +240,8 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
     }
     memcpy(rxbuf_ptr(s, port_index), buf, size);
 
-    s->regs[rxbase + R_RX_CTRL0] |= CTRL_S;
-    if (s->regs[R_RX_CTRL0] & CTRL_I) {
+    s->port[port_index].reg.rx_ctrl |= CTRL_S;
+    if (s->port[port_index].reg.rx_ctrl & CTRL_I) {
         eth_pulse_irq(s);
     }
 
-- 
2.45.2

[PATCH 12/20] hw/net/xilinx_ethlite: Access TX_GIE register for each port

[Philippe Mathieu-Daudé]

Rather than accessing the registers within the mixed RAM/MMIO
region as indexed register, declare a per-port TX_GIE. This
will help to map the RAM as RAM (keeping MMIO as MMIO) in few
commits.

Previous s->regs[R_TX_GIE0] and s->regs[R_TX_GIE1] are now
unused. Not a concern, this array will soon disappear.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 605451a522..4cb4781e70 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -62,6 +62,8 @@
 typedef struct XlnxXpsEthLitePort
 {
     struct {
+        uint32_t tx_gie;
+
         uint32_t rx_ctrl;
     } reg;
 } XlnxXpsEthLitePort;
@@ -90,7 +92,7 @@ struct XlnxXpsEthLite
 static inline void eth_pulse_irq(XlnxXpsEthLite *s)
 {
     /* Only the first gie reg is active.  */
-    if (s->regs[R_TX_GIE0] & GIE_GIE) {
+    if (s->port[0].reg.tx_gie & GIE_GIE) {
         qemu_irq_pulse(s->irq);
     }
 }
@@ -126,6 +128,9 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
     switch (addr)
     {
         case R_TX_GIE0:
+            r = s->port[port_index].reg.tx_gie;
+            break;
+
         case R_TX_LEN0:
         case R_TX_LEN1:
         case R_TX_CTRL1:
@@ -189,10 +194,13 @@ eth_write(void *opaque, hwaddr addr,
 
         case R_TX_LEN0:
         case R_TX_LEN1:
-        case R_TX_GIE0:
             s->regs[addr] = value;
             break;
 
+        case R_TX_GIE0:
+            s->port[port_index].reg.tx_gie = value;
+            break;
+
         default:
             s->regs[addr] = tswap32(value);
             break;
-- 
2.45.2

[PATCH 13/20] hw/net/xilinx_ethlite: Access TX_LEN register for each port

[Philippe Mathieu-Daudé]

Rather than accessing the registers within the mixed RAM/MMIO
region as indexed register, declare a per-port TX_LEN. This
will help to map the RAM as RAM (keeping MMIO as MMIO) in few
commits.

Previous s->regs[R_TX_LEN0] and s->regs[R_TX_LEN1] are now
unused. Not a concern, this array will soon disappear.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 4cb4781e70..1a3b295b4b 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -62,6 +62,7 @@
 typedef struct XlnxXpsEthLitePort
 {
     struct {
+        uint32_t tx_len;
         uint32_t tx_gie;
 
         uint32_t rx_ctrl;
@@ -133,6 +134,9 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
 
         case R_TX_LEN0:
         case R_TX_LEN1:
+            r = s->port[port_index].reg.tx_len;
+            break;
+
         case R_TX_CTRL1:
         case R_TX_CTRL0:
             r = s->regs[addr];
@@ -169,7 +173,7 @@ eth_write(void *opaque, hwaddr addr,
             if ((value & (CTRL_P | CTRL_S)) == CTRL_S) {
                 qemu_send_packet(qemu_get_queue(s->nic),
                                  txbuf_ptr(s, port_index),
-                                 s->regs[base + R_TX_LEN0]);
+                                 s->port[port_index].reg.tx_len);
                 if (s->regs[base + R_TX_CTRL0] & CTRL_I)
                     eth_pulse_irq(s);
             } else if ((value & (CTRL_P | CTRL_S)) == (CTRL_P | CTRL_S)) {
@@ -194,7 +198,7 @@ eth_write(void *opaque, hwaddr addr,
 
         case R_TX_LEN0:
         case R_TX_LEN1:
-            s->regs[addr] = value;
+            s->port[port_index].reg.tx_len = value;
             break;
 
         case R_TX_GIE0:
-- 
2.45.2

[PATCH 14/20] hw/net/xilinx_ethlite: Access TX_CTRL register for each port

[Philippe Mathieu-Daudé]

Rather than accessing the registers within the mixed RAM/MMIO
region as indexed register, declare a per-port TX_CTRL. This
will help to map the RAM as RAM (keeping MMIO as MMIO) in few
commits.

Previous s->regs[R_TX_CTRL0] and s->regs[R_TX_CTRL1] are now
unused. Not a concern, this array will soon disappear.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 1a3b295b4b..4d86851f38 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -64,6 +64,7 @@ typedef struct XlnxXpsEthLitePort
     struct {
         uint32_t tx_len;
         uint32_t tx_gie;
+        uint32_t tx_ctrl;
 
         uint32_t rx_ctrl;
     } reg;
@@ -139,7 +140,7 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
 
         case R_TX_CTRL1:
         case R_TX_CTRL0:
-            r = s->regs[addr];
+            r = s->port[port_index].reg.tx_ctrl;
             break;
 
         case R_RX_CTRL1:
@@ -159,7 +160,6 @@ eth_write(void *opaque, hwaddr addr,
 {
     XlnxXpsEthLite *s = opaque;
     unsigned int port_index = addr_to_port_index(addr);
-    unsigned int base = 0;
     uint32_t value = val64;
 
     addr >>= 2;
@@ -167,24 +167,23 @@ eth_write(void *opaque, hwaddr addr,
     {
         case R_TX_CTRL0:
         case R_TX_CTRL1:
-            if (addr == R_TX_CTRL1)
-                base = 0x800 / 4;
-
             if ((value & (CTRL_P | CTRL_S)) == CTRL_S) {
                 qemu_send_packet(qemu_get_queue(s->nic),
                                  txbuf_ptr(s, port_index),
                                  s->port[port_index].reg.tx_len);
-                if (s->regs[base + R_TX_CTRL0] & CTRL_I)
+                if (s->port[port_index].reg.tx_ctrl & CTRL_I) {
                     eth_pulse_irq(s);
+                }
             } else if ((value & (CTRL_P | CTRL_S)) == (CTRL_P | CTRL_S)) {
                 memcpy(&s->conf.macaddr.a[0], txbuf_ptr(s, port_index), 6);
-                if (s->regs[base + R_TX_CTRL0] & CTRL_I)
+                if (s->port[port_index].reg.tx_ctrl & CTRL_I) {
                     eth_pulse_irq(s);
+                }
             }
 
             /* We are fast and get ready pretty much immediately so
                we actually never flip the S nor P bits to one.  */
-            s->regs[addr] = value & ~(CTRL_P | CTRL_S);
+            s->port[port_index].reg.tx_ctrl = value & ~(CTRL_P | CTRL_S);
             break;
 
         /* Keep these native.  */
-- 
2.45.2

[PATCH 15/20] hw/net/xilinx_ethlite: Map RX_CTRL as MMIO

[Philippe Mathieu-Daudé]

Declare RX registers as MMIO region, split it out
of the current mixed RAM/MMIO region.
The memory flat view becomes:

  FlatView #0
   Root memory region: system
    0000000081000000-00000000810007e3 (prio 0, i/o): xlnx.xps-ethernetlite
    00000000810007e4-00000000810007f3 (prio 0, i/o): ethlite.mdio
    00000000810007f4-00000000810017fb (prio 0, i/o): xlnx.xps-ethernetlite @00000000000007f4
    00000000810017fc-00000000810017ff (prio 0, i/o): ethlite.rx[0]io
    0000000081001800-0000000081001ffb (prio 0, i/o): xlnx.xps-ethernetlite @0000000000001800
    0000000081001ffc-0000000081001fff (prio 0, i/o): ethlite.rx[1]io

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 79 +++++++++++++++++++++++++++++++++--------
 1 file changed, 65 insertions(+), 14 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 4d86851f38..161fd97f06 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -46,13 +46,18 @@
 #define R_TX_CTRL1    (0x0ffc / 4)
 
 #define R_RX_BUF0     (0x1000 / 4)
-#define R_RX_CTRL0    (0x17fc / 4)
+#define A_RX_BASE0    0x17fc
 #define R_RX_BUF1     (0x1800 / 4)
-#define R_RX_CTRL1    (0x1ffc / 4)
+#define A_RX_BASE1    0x1ffc
 #define R_MAX         (0x2000 / 4)
 
 #define RX_BUFSZ_MAX  0x07e0
 
+enum {
+    RX_CTRL = 0,
+    RX_MAX
+};
+
 #define GIE_GIE    0x80000000
 
 #define CTRL_I     0x8
@@ -61,6 +66,8 @@
 
 typedef struct XlnxXpsEthLitePort
 {
+    MemoryRegion rxio;
+
     struct {
         uint32_t tx_len;
         uint32_t tx_gie;
@@ -118,6 +125,53 @@ static void *rxbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
     return &s->regs[rxbase + R_RX_BUF0];
 }
 
+static uint64_t port_rx_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    XlnxXpsEthLite *s = opaque;
+    unsigned port_index = addr_to_port_index(addr);
+    uint32_t r = 0;
+
+    switch (addr >> 2) {
+        case RX_CTRL:
+            r = s->port[port_index].reg.rx_ctrl;
+            break;
+        default:
+            g_assert_not_reached();
+    }
+
+    return r;
+}
+
+static void port_rx_write(void *opaque, hwaddr addr, uint64_t value,
+                          unsigned int size)
+{
+    XlnxXpsEthLite *s = opaque;
+
+    switch (addr >> 2) {
+        case RX_CTRL:
+            if (!(value & CTRL_S)) {
+                qemu_flush_queued_packets(qemu_get_queue(s->nic));
+            }
+            break;
+        default:
+            g_assert_not_reached();
+    }
+}
+
+static const MemoryRegionOps eth_portrx_ops = {
+        .read = port_rx_read,
+        .write = port_rx_write,
+        .endianness = DEVICE_NATIVE_ENDIAN,
+        .impl = {
+            .min_access_size = 4,
+            .max_access_size = 4,
+        },
+        .valid = {
+            .min_access_size = 4,
+            .max_access_size = 4,
+        },
+};
+
 static uint64_t
 eth_read(void *opaque, hwaddr addr, unsigned int size)
 {
@@ -143,10 +197,6 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
             r = s->port[port_index].reg.tx_ctrl;
             break;
 
-        case R_RX_CTRL1:
-        case R_RX_CTRL0:
-            r = s->port[port_index].reg.rx_ctrl;
-
         default:
             r = tswap32(s->regs[addr]);
             break;
@@ -187,14 +237,6 @@ eth_write(void *opaque, hwaddr addr,
             break;
 
         /* Keep these native.  */
-        case R_RX_CTRL0:
-        case R_RX_CTRL1:
-            if (!(value & CTRL_S)) {
-                qemu_flush_queued_packets(qemu_get_queue(s->nic));
-            }
-            s->port[port_index].reg.rx_ctrl = value;
-            break;
-
         case R_TX_LEN0:
         case R_TX_LEN1:
             s->port[port_index].reg.tx_len = value;
@@ -287,6 +329,15 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(&s->mmio, A_MDIO_BASE,
                             sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mdio), 0));
 
+    for (unsigned i = 0; i < 2; i++) {
+        memory_region_init_io(&s->port[i].rxio, OBJECT(dev),
+                              &eth_portrx_ops, s,
+                              i ? "ethlite.rx[1]io" : "ethlite.rx[0]io",
+                              4 * RX_MAX);
+        memory_region_add_subregion(&s->mmio, i ? A_RX_BASE1 : A_RX_BASE0,
+                                    &s->port[i].rxio);
+    }
+
     qemu_macaddr_default_if_unset(&s->conf.macaddr);
     s->nic = qemu_new_nic(&net_xilinx_ethlite_info, &s->conf,
                           object_get_typename(OBJECT(dev)), dev->id,
-- 
2.45.2

[PATCH 16/20] hw/net/xilinx_ethlite: Map TX_LEN as MMIO

[Philippe Mathieu-Daudé]

Declare TX registers as MMIO region, split it out
of the current mixed RAM/MMIO region.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 71 ++++++++++++++++++++++++++++++++++-------
 1 file changed, 59 insertions(+), 12 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 161fd97f06..159b2b0c64 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -38,11 +38,11 @@
 #define A_MDIO_BASE   0x07e4
 
 #define R_TX_BUF0     0
-#define R_TX_LEN0     (0x07f4 / 4)
+#define A_TX_BASE0    0x07f4
 #define R_TX_GIE0     (0x07f8 / 4)
 #define R_TX_CTRL0    (0x07fc / 4)
 #define R_TX_BUF1     (0x0800 / 4)
-#define R_TX_LEN1     (0x0ff4 / 4)
+#define A_TX_BASE1    0x0ff4
 #define R_TX_CTRL1    (0x0ffc / 4)
 
 #define R_RX_BUF0     (0x1000 / 4)
@@ -53,6 +53,11 @@
 
 #define RX_BUFSZ_MAX  0x07e0
 
+enum {
+    TX_LEN =  0,
+    TX_MAX
+};
+
 enum {
     RX_CTRL = 0,
     RX_MAX
@@ -125,6 +130,51 @@ static void *rxbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
     return &s->regs[rxbase + R_RX_BUF0];
 }
 
+static uint64_t port_tx_read(void *opaque, hwaddr addr, unsigned int size)
+{
+    XlnxXpsEthLite *s = opaque;
+    unsigned port_index = addr_to_port_index(addr);
+    uint32_t r = 0;
+
+    switch (addr >> 2) {
+        case TX_LEN:
+            r = s->port[port_index].reg.tx_len;
+            break;
+        default:
+            g_assert_not_reached();
+    }
+
+    return r;
+}
+
+static void port_tx_write(void *opaque, hwaddr addr, uint64_t value,
+                          unsigned int size)
+{
+    XlnxXpsEthLite *s = opaque;
+
+    switch (addr >> 2) {
+        case TX_LEN:
+            s->port[port_index].reg.tx_len = value;
+            break;
+        default:
+            g_assert_not_reached();
+    }
+}
+
+static const MemoryRegionOps eth_porttx_ops = {
+        .read = port_tx_read,
+        .write = port_tx_write,
+        .endianness = DEVICE_NATIVE_ENDIAN,
+        .impl = {
+            .min_access_size = 4,
+            .max_access_size = 4,
+        },
+        .valid = {
+            .min_access_size = 4,
+            .max_access_size = 4,
+        },
+};
+
 static uint64_t port_rx_read(void *opaque, hwaddr addr, unsigned int size)
 {
     XlnxXpsEthLite *s = opaque;
@@ -187,11 +237,6 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
             r = s->port[port_index].reg.tx_gie;
             break;
 
-        case R_TX_LEN0:
-        case R_TX_LEN1:
-            r = s->port[port_index].reg.tx_len;
-            break;
-
         case R_TX_CTRL1:
         case R_TX_CTRL0:
             r = s->port[port_index].reg.tx_ctrl;
@@ -237,11 +282,6 @@ eth_write(void *opaque, hwaddr addr,
             break;
 
         /* Keep these native.  */
-        case R_TX_LEN0:
-        case R_TX_LEN1:
-            s->port[port_index].reg.tx_len = value;
-            break;
-
         case R_TX_GIE0:
             s->port[port_index].reg.tx_gie = value;
             break;
@@ -330,6 +370,13 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
                             sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mdio), 0));
 
     for (unsigned i = 0; i < 2; i++) {
+        memory_region_init_io(&s->port[i].txio, OBJECT(dev),
+                              &eth_porttx_ops, s,
+                              i ? "ethlite.tx[1]io" : "ethlite.tx[0]io",
+                              4 * TX_MAX);
+        memory_region_add_subregion(&s->mmio, i ? A_TX_BASE1 : A_TX_BASE0,
+                                    &s->port[i].txio);
+
         memory_region_init_io(&s->port[i].rxio, OBJECT(dev),
                               &eth_portrx_ops, s,
                               i ? "ethlite.rx[1]io" : "ethlite.rx[0]io",
-- 
2.45.2

[PATCH 17/20] hw/net/xilinx_ethlite: Map TX_GIE as MMIO

[Philippe Mathieu-Daudé]

Add TX_GIE to the TX registers MMIO region.

Before TX_GIE1 was accessed as RAM, with no effect.
Now it is accessed as MMIO, also without any effect.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 159b2b0c64..f7a5b1620a 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -39,7 +39,6 @@
 
 #define R_TX_BUF0     0
 #define A_TX_BASE0    0x07f4
-#define R_TX_GIE0     (0x07f8 / 4)
 #define R_TX_CTRL0    (0x07fc / 4)
 #define R_TX_BUF1     (0x0800 / 4)
 #define A_TX_BASE1    0x0ff4
@@ -55,6 +54,7 @@
 
 enum {
     TX_LEN =  0,
+    TX_GIE =  1,
     TX_MAX
 };
 
@@ -140,6 +140,9 @@ static uint64_t port_tx_read(void *opaque, hwaddr addr, unsigned int size)
         case TX_LEN:
             r = s->port[port_index].reg.tx_len;
             break;
+        case TX_GIE:
+            r = s->port[port_index].reg.tx_gie;
+            break;
         default:
             g_assert_not_reached();
     }
@@ -156,6 +159,9 @@ static void port_tx_write(void *opaque, hwaddr addr, uint64_t value,
         case TX_LEN:
             s->port[port_index].reg.tx_len = value;
             break;
+        case TX_GIE:
+            s->port[port_index].reg.tx_gie = value;
+            break;
         default:
             g_assert_not_reached();
     }
@@ -233,10 +239,6 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
 
     switch (addr)
     {
-        case R_TX_GIE0:
-            r = s->port[port_index].reg.tx_gie;
-            break;
-
         case R_TX_CTRL1:
         case R_TX_CTRL0:
             r = s->port[port_index].reg.tx_ctrl;
@@ -281,11 +283,6 @@ eth_write(void *opaque, hwaddr addr,
             s->port[port_index].reg.tx_ctrl = value & ~(CTRL_P | CTRL_S);
             break;
 
-        /* Keep these native.  */
-        case R_TX_GIE0:
-            s->port[port_index].reg.tx_gie = value;
-            break;
-
         default:
             s->regs[addr] = tswap32(value);
             break;
-- 
2.45.2

[PATCH 18/20] hw/net/xilinx_ethlite: Map TX_CTRL as MMIO

[Philippe Mathieu-Daudé]

Add TX_CTRL to the TX registers MMIO region.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 56 +++++++++++++++++++----------------------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index f7a5b1620a..f681b91769 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -39,10 +39,8 @@
 
 #define R_TX_BUF0     0
 #define A_TX_BASE0    0x07f4
-#define R_TX_CTRL0    (0x07fc / 4)
 #define R_TX_BUF1     (0x0800 / 4)
 #define A_TX_BASE1    0x0ff4
-#define R_TX_CTRL1    (0x0ffc / 4)
 
 #define R_RX_BUF0     (0x1000 / 4)
 #define A_RX_BASE0    0x17fc
@@ -55,6 +53,7 @@
 enum {
     TX_LEN =  0,
     TX_GIE =  1,
+    TX_CTRL = 2,
     TX_MAX
 };
 
@@ -71,6 +70,7 @@ enum {
 
 typedef struct XlnxXpsEthLitePort
 {
+    MemoryRegion txio;
     MemoryRegion rxio;
 
     struct {
@@ -143,6 +143,9 @@ static uint64_t port_tx_read(void *opaque, hwaddr addr, unsigned int size)
         case TX_GIE:
             r = s->port[port_index].reg.tx_gie;
             break;
+        case TX_CTRL:
+            r = s->port[port_index].reg.tx_ctrl;
+            break;
         default:
             g_assert_not_reached();
     }
@@ -154,6 +157,7 @@ static void port_tx_write(void *opaque, hwaddr addr, uint64_t value,
                           unsigned int size)
 {
     XlnxXpsEthLite *s = opaque;
+    unsigned port_index = addr_to_port_index(addr);
 
     switch (addr >> 2) {
         case TX_LEN:
@@ -162,6 +166,26 @@ static void port_tx_write(void *opaque, hwaddr addr, uint64_t value,
         case TX_GIE:
             s->port[port_index].reg.tx_gie = value;
             break;
+        case TX_CTRL:
+            if ((value & (CTRL_P | CTRL_S)) == CTRL_S) {
+                qemu_send_packet(qemu_get_queue(s->nic),
+                                 txbuf_ptr(s, port_index),
+                                 s->port[port_index].reg.tx_len);
+                if (s->port[port_index].reg.tx_ctrl & CTRL_I) {
+                    eth_pulse_irq(s);
+                }
+            } else if ((value & (CTRL_P | CTRL_S)) == (CTRL_P | CTRL_S)) {
+                memcpy(&s->conf.macaddr.a[0], txbuf_ptr(s, port_index), 6);
+                if (s->port[port_index].reg.tx_ctrl & CTRL_I) {
+                    eth_pulse_irq(s);
+                }
+            }
+            /*
+             * We are fast and get ready pretty much immediately
+             * so we actually never flip the S nor P bits to one.
+             */
+            s->port[port_index].reg.tx_ctrl = value & ~(CTRL_P | CTRL_S);
+            break;
         default:
             g_assert_not_reached();
     }
@@ -232,18 +256,12 @@ static uint64_t
 eth_read(void *opaque, hwaddr addr, unsigned int size)
 {
     XlnxXpsEthLite *s = opaque;
-    unsigned port_index = addr_to_port_index(addr);
     uint32_t r = 0;
 
     addr >>= 2;
 
     switch (addr)
     {
-        case R_TX_CTRL1:
-        case R_TX_CTRL0:
-            r = s->port[port_index].reg.tx_ctrl;
-            break;
-
         default:
             r = tswap32(s->regs[addr]);
             break;
@@ -256,33 +274,11 @@ eth_write(void *opaque, hwaddr addr,
           uint64_t val64, unsigned int size)
 {
     XlnxXpsEthLite *s = opaque;
-    unsigned int port_index = addr_to_port_index(addr);
     uint32_t value = val64;
 
     addr >>= 2;
     switch (addr) 
     {
-        case R_TX_CTRL0:
-        case R_TX_CTRL1:
-            if ((value & (CTRL_P | CTRL_S)) == CTRL_S) {
-                qemu_send_packet(qemu_get_queue(s->nic),
-                                 txbuf_ptr(s, port_index),
-                                 s->port[port_index].reg.tx_len);
-                if (s->port[port_index].reg.tx_ctrl & CTRL_I) {
-                    eth_pulse_irq(s);
-                }
-            } else if ((value & (CTRL_P | CTRL_S)) == (CTRL_P | CTRL_S)) {
-                memcpy(&s->conf.macaddr.a[0], txbuf_ptr(s, port_index), 6);
-                if (s->port[port_index].reg.tx_ctrl & CTRL_I) {
-                    eth_pulse_irq(s);
-                }
-            }
-
-            /* We are fast and get ready pretty much immediately so
-               we actually never flip the S nor P bits to one.  */
-            s->port[port_index].reg.tx_ctrl = value & ~(CTRL_P | CTRL_S);
-            break;
-
         default:
             s->regs[addr] = tswap32(value);
             break;
-- 
2.45.2

[PATCH 19/20] hw/net/xilinx_ethlite: Map the RAM buffer as RAM memory region

[Philippe Mathieu-Daudé]

Rather than using I/O registers for RAM buffer, having to
swap endianness back and forth (because the core memory layer
automatically swaps endiannes for us), declare the buffers
as RAM regions. Remove the now unused s->regs[] array.

The memory flat view becomes:

  FlatView #0
   Root memory region: system
    0000000081000000-00000000810007f3 (prio 0, ram): ethlite.tx[0]buf
    00000000810007f4-00000000810007ff (prio 0, i/o): ethlite.tx[0]io
    0000000081000800-0000000081000ff3 (prio 0, ram): ethlite.tx[1]buf
    0000000081000ff4-0000000081000fff (prio 0, i/o): ethlite.tx[1]io
    0000000081001000-00000000810017f3 (prio 0, ram): ethlite.rx[0]buf
    00000000810017fc-00000000810017ff (prio 0, i/o): ethlite.rx[0]io
    0000000081001800-0000000081001ff3 (prio 0, ram): ethlite.rx[1]buf
    0000000081001ffc-0000000081001fff (prio 0, i/o): ethlite.rx[1]io

Mention the device datasheet in the file header.

Reported-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 79 +++++++++++------------------------------
 1 file changed, 20 insertions(+), 59 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index f681b91769..da453465ca 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -2,6 +2,10 @@
  * QEMU model of the Xilinx Ethernet Lite MAC.
  *
  * Copyright (c) 2009 Edgar E. Iglesias.
+ * Copyright (c) 2024 Linaro, Ltd
+ *
+ * DS580: https://docs.amd.com/v/u/en-US/xps_ethernetlite
+ * LogiCORE IP XPS Ethernet Lite Media Access Controller
  *
  * Permission is hereby granted, free of charge, to any person obtaining a copy
  * of this software and associated documentation files (the "Software"), to deal
@@ -27,7 +31,6 @@
 #include "qemu/bitops.h"
 #include "qom/object.h"
 #include "qapi/error.h"
-#include "exec/tswap.h"
 #include "hw/sysbus.h"
 #include "hw/irq.h"
 #include "hw/qdev-properties.h"
@@ -46,7 +49,6 @@
 #define A_RX_BASE0    0x17fc
 #define R_RX_BUF1     (0x1800 / 4)
 #define A_RX_BASE1    0x1ffc
-#define R_MAX         (0x2000 / 4)
 
 #define RX_BUFSZ_MAX  0x07e0
 
@@ -72,6 +74,8 @@ typedef struct XlnxXpsEthLitePort
 {
     MemoryRegion txio;
     MemoryRegion rxio;
+    MemoryRegion txbuf;
+    MemoryRegion rxbuf;
 
     struct {
         uint32_t tx_len;
@@ -100,7 +104,6 @@ struct XlnxXpsEthLite
 
     UnimplementedDeviceState mdio;
     XlnxXpsEthLitePort port[2];
-    uint32_t regs[R_MAX];
 };
 
 static inline void eth_pulse_irq(XlnxXpsEthLite *s)
@@ -118,16 +121,12 @@ static unsigned addr_to_port_index(hwaddr addr)
 
 static void *txbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
 {
-    unsigned int rxbase = port_index * (0x800 / 4);
-
-    return &s->regs[rxbase + R_TX_BUF0];
+    return memory_region_get_ram_ptr(&s->port[port_index].txbuf);
 }
 
 static void *rxbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
 {
-    unsigned int rxbase = port_index * (0x800 / 4);
-
-    return &s->regs[rxbase + R_RX_BUF0];
+    return memory_region_get_ram_ptr(&s->port[port_index].rxbuf);
 }
 
 static uint64_t port_tx_read(void *opaque, hwaddr addr, unsigned int size)
@@ -252,53 +251,6 @@ static const MemoryRegionOps eth_portrx_ops = {
         },
 };
 
-static uint64_t
-eth_read(void *opaque, hwaddr addr, unsigned int size)
-{
-    XlnxXpsEthLite *s = opaque;
-    uint32_t r = 0;
-
-    addr >>= 2;
-
-    switch (addr)
-    {
-        default:
-            r = tswap32(s->regs[addr]);
-            break;
-    }
-    return r;
-}
-
-static void
-eth_write(void *opaque, hwaddr addr,
-          uint64_t val64, unsigned int size)
-{
-    XlnxXpsEthLite *s = opaque;
-    uint32_t value = val64;
-
-    addr >>= 2;
-    switch (addr) 
-    {
-        default:
-            s->regs[addr] = tswap32(value);
-            break;
-    }
-}
-
-static const MemoryRegionOps eth_ops = {
-    .read = eth_read,
-    .write = eth_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
-    .impl = {
-        .min_access_size = 4,
-        .max_access_size = 4,
-    },
-    .valid = {
-        .min_access_size = 4,
-        .max_access_size = 4
-    }
-};
-
 static bool eth_can_rx(NetClientState *nc)
 {
     XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
@@ -354,6 +306,9 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
 {
     XlnxXpsEthLite *s = XILINX_ETHLITE(dev);
 
+    memory_region_init(&s->mmio, OBJECT(dev),
+                       "xlnx.xps-ethernetlite", 0x2000);
+
     object_initialize_child(OBJECT(dev), "ethlite.mdio", &s->mdio,
                            TYPE_UNIMPLEMENTED_DEVICE);
     qdev_prop_set_string(DEVICE(&s->mdio), "name", "ethlite.mdio");
@@ -363,6 +318,10 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
                             sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mdio), 0));
 
     for (unsigned i = 0; i < 2; i++) {
+        memory_region_init_ram(&s->port[i].txbuf, OBJECT(dev),
+                               i ? "ethlite.tx[1]buf" : "ethlite.tx[0]buf",
+                               0x07f4, &error_abort);
+        memory_region_add_subregion(&s->mmio, 0x0800 * i, &s->port[i].txbuf);
         memory_region_init_io(&s->port[i].txio, OBJECT(dev),
                               &eth_porttx_ops, s,
                               i ? "ethlite.tx[1]io" : "ethlite.tx[0]io",
@@ -370,6 +329,11 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
         memory_region_add_subregion(&s->mmio, i ? A_TX_BASE1 : A_TX_BASE0,
                                     &s->port[i].txio);
 
+        memory_region_init_ram(&s->port[i].rxbuf, OBJECT(dev),
+                               i ? "ethlite.rx[1]buf" : "ethlite.rx[0]buf",
+                               0x07f4, &error_abort);
+        memory_region_add_subregion(&s->mmio, 0x1000 + 0x0800 * i,
+                                    &s->port[i].rxbuf);
         memory_region_init_io(&s->port[i].rxio, OBJECT(dev),
                               &eth_portrx_ops, s,
                               i ? "ethlite.rx[1]io" : "ethlite.rx[0]io",
@@ -390,9 +354,6 @@ static void xilinx_ethlite_init(Object *obj)
     XlnxXpsEthLite *s = XILINX_ETHLITE(obj);
 
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
-
-    memory_region_init_io(&s->mmio, obj, &eth_ops, s,
-                          "xlnx.xps-ethernetlite", R_MAX * 4);
     sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
 }
 
-- 
2.45.2

[PATCH 20/20] hw/net/xilinx_ethlite: Rename 'mmio' MR as 'container'

[Philippe Mathieu-Daudé]

Having all its address range mapped by subregions,
s->mmio MemoryRegion effectively became a container.
Rename it as 'container' for clarity.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 hw/net/xilinx_ethlite.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index da453465ca..c65001cf46 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -93,7 +93,7 @@ struct XlnxXpsEthLite
 {
     SysBusDevice parent_obj;
 
-    MemoryRegion mmio;
+    MemoryRegion container;
     qemu_irq irq;
     NICState *nic;
     NICConf conf;
@@ -306,7 +306,7 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
 {
     XlnxXpsEthLite *s = XILINX_ETHLITE(dev);
 
-    memory_region_init(&s->mmio, OBJECT(dev),
+    memory_region_init(&s->container, OBJECT(dev),
                        "xlnx.xps-ethernetlite", 0x2000);
 
     object_initialize_child(OBJECT(dev), "ethlite.mdio", &s->mdio,
@@ -314,31 +314,31 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
     qdev_prop_set_string(DEVICE(&s->mdio), "name", "ethlite.mdio");
     qdev_prop_set_uint64(DEVICE(&s->mdio), "size", 4 * 4);
     sysbus_realize(SYS_BUS_DEVICE(&s->mdio), &error_fatal);
-    memory_region_add_subregion(&s->mmio, A_MDIO_BASE,
+    memory_region_add_subregion(&s->container, A_MDIO_BASE,
                             sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mdio), 0));
 
     for (unsigned i = 0; i < 2; i++) {
         memory_region_init_ram(&s->port[i].txbuf, OBJECT(dev),
                                i ? "ethlite.tx[1]buf" : "ethlite.tx[0]buf",
                                0x07f4, &error_abort);
-        memory_region_add_subregion(&s->mmio, 0x0800 * i, &s->port[i].txbuf);
+        memory_region_add_subregion(&s->container, 0x0800 * i, &s->port[i].txbuf);
         memory_region_init_io(&s->port[i].txio, OBJECT(dev),
                               &eth_porttx_ops, s,
                               i ? "ethlite.tx[1]io" : "ethlite.tx[0]io",
                               4 * TX_MAX);
-        memory_region_add_subregion(&s->mmio, i ? A_TX_BASE1 : A_TX_BASE0,
+        memory_region_add_subregion(&s->container, i ? A_TX_BASE1 : A_TX_BASE0,
                                     &s->port[i].txio);
 
         memory_region_init_ram(&s->port[i].rxbuf, OBJECT(dev),
                                i ? "ethlite.rx[1]buf" : "ethlite.rx[0]buf",
                                0x07f4, &error_abort);
-        memory_region_add_subregion(&s->mmio, 0x1000 + 0x0800 * i,
+        memory_region_add_subregion(&s->container, 0x1000 + 0x0800 * i,
                                     &s->port[i].rxbuf);
         memory_region_init_io(&s->port[i].rxio, OBJECT(dev),
                               &eth_portrx_ops, s,
                               i ? "ethlite.rx[1]io" : "ethlite.rx[0]io",
                               4 * RX_MAX);
-        memory_region_add_subregion(&s->mmio, i ? A_RX_BASE1 : A_RX_BASE0,
+        memory_region_add_subregion(&s->container, i ? A_RX_BASE1 : A_RX_BASE0,
                                     &s->port[i].rxio);
     }
 
@@ -354,7 +354,7 @@ static void xilinx_ethlite_init(Object *obj)
     XlnxXpsEthLite *s = XILINX_ETHLITE(obj);
 
     sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
-    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
+    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->container);
 }
 
 static Property xilinx_ethlite_properties[] = {
-- 
2.45.2

Re: [PATCH 02/20] hw/net/xilinx_ethlite: Convert some debug logs to trace events

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:26PM +0100, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>



> ---
>  hw/net/xilinx_ethlite.c | 5 +++--
>  hw/net/trace-events     | 4 ++++
>  2 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index e84b4cdd35..bb330a233f 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -30,6 +30,7 @@
>  #include "hw/irq.h"
>  #include "hw/qdev-properties.h"
>  #include "net/net.h"
> +#include "trace.h"
>  
>  #define D(x)
>  #define R_TX_BUF0     0
> @@ -198,13 +199,13 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>          return size;
>  
>      if (s->regs[rxbase + R_RX_CTRL0] & CTRL_S) {
> -        D(qemu_log("ethlite lost packet %x\n", s->regs[R_RX_CTRL0]));
> +        trace_ethlite_pkt_lost(s->regs[R_RX_CTRL0]);
>          return -1;
>      }
>  
>      D(qemu_log("%s %zd rxbase=%x\n", __func__, size, rxbase));
>      if (size > (R_MAX - R_RX_BUF0 - rxbase) * 4) {
> -        D(qemu_log("ethlite packet is too big, size=%x\n", size));
> +        trace_ethlite_pkt_size_too_big(size);
>          return -1;
>      }
>      memcpy(&s->regs[rxbase + R_RX_BUF0], buf, size);
> diff --git a/hw/net/trace-events b/hw/net/trace-events
> index d0f1d8c0fb..2b36cd967e 100644
> --- a/hw/net/trace-events
> +++ b/hw/net/trace-events
> @@ -511,3 +511,7 @@ xen_netdev_connect(int dev, unsigned int tx, unsigned int rx, int port) "vif%u t
>  xen_netdev_frontend_changed(const char *dev, int state) "vif%s state %d"
>  xen_netdev_tx(int dev, int ref, int off, int len, unsigned int flags, const char *c, const char *d, const char *m, const char *e) "vif%u ref %u off %u len %u flags 0x%x%s%s%s%s"
>  xen_netdev_rx(int dev, int idx, int status, int flags) "vif%u idx %d status %d flags 0x%x"
> +
> +# xilinx_ethlite.c
> +ethlite_pkt_lost(uint32_t rx_ctrl) "rx_ctrl:0x%" PRIx32
> +ethlite_pkt_size_too_big(uint64_t size) "size:0x%" PRIx64
> -- 
> 2.45.2
> 

Re: [PATCH 03/20] hw/net/xilinx_ethlite: Remove unuseful debug logs

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:27PM +0100, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> ---
>  hw/net/xilinx_ethlite.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index bb330a233f..2b52597f03 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -32,7 +32,6 @@
>  #include "net/net.h"
>  #include "trace.h"
>  
> -#define D(x)
>  #define R_TX_BUF0     0
>  #define R_TX_LEN0     (0x07f4 / 4)
>  #define R_TX_GIE0     (0x07f8 / 4)
> @@ -100,7 +99,6 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
>          case R_RX_CTRL1:
>          case R_RX_CTRL0:
>              r = s->regs[addr];
> -            D(qemu_log("%s " HWADDR_FMT_plx "=%x\n", __func__, addr * 4, r));
>              break;
>  
>          default:
> @@ -126,13 +124,10 @@ eth_write(void *opaque, hwaddr addr,
>              if (addr == R_TX_CTRL1)
>                  base = 0x800 / 4;
>  
> -            D(qemu_log("%s addr=" HWADDR_FMT_plx " val=%x\n",
> -                       __func__, addr * 4, value));
>              if ((value & (CTRL_P | CTRL_S)) == CTRL_S) {
>                  qemu_send_packet(qemu_get_queue(s->nic),
>                                   (void *) &s->regs[base],
>                                   s->regs[base + R_TX_LEN0]);
> -                D(qemu_log("eth_tx %d\n", s->regs[base + R_TX_LEN0]));
>                  if (s->regs[base + R_TX_CTRL0] & CTRL_I)
>                      eth_pulse_irq(s);
>              } else if ((value & (CTRL_P | CTRL_S)) == (CTRL_P | CTRL_S)) {
> @@ -156,8 +151,6 @@ eth_write(void *opaque, hwaddr addr,
>          case R_TX_LEN0:
>          case R_TX_LEN1:
>          case R_TX_GIE0:
> -            D(qemu_log("%s addr=" HWADDR_FMT_plx " val=%x\n",
> -                       __func__, addr * 4, value));
>              s->regs[addr] = value;
>              break;
>  
> @@ -203,7 +196,6 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>          return -1;
>      }
>  
> -    D(qemu_log("%s %zd rxbase=%x\n", __func__, size, rxbase));
>      if (size > (R_MAX - R_RX_BUF0 - rxbase) * 4) {
>          trace_ethlite_pkt_size_too_big(size);
>          return -1;
> -- 
> 2.45.2
> 

Re: [PATCH 04/20] hw/net/xilinx_ethlite: Update QOM style

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:28PM +0100, Philippe Mathieu-Daudé wrote:
> Use XlnxXpsEthLite typedef, OBJECT_DECLARE_SIMPLE_TYPE macro;
> convert type_init() to DEFINE_TYPES().
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> ---
>  hw/net/xilinx_ethlite.c | 48 +++++++++++++++++++----------------------
>  1 file changed, 22 insertions(+), 26 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index 2b52597f03..0f59811c78 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -53,10 +53,9 @@
>  #define CTRL_S     0x1
>  
>  #define TYPE_XILINX_ETHLITE "xlnx.xps-ethernetlite"
> -DECLARE_INSTANCE_CHECKER(struct xlx_ethlite, XILINX_ETHLITE,
> -                         TYPE_XILINX_ETHLITE)
> +OBJECT_DECLARE_SIMPLE_TYPE(XlnxXpsEthLite, XILINX_ETHLITE)
>  
> -struct xlx_ethlite
> +struct XlnxXpsEthLite
>  {
>      SysBusDevice parent_obj;
>  
> @@ -73,7 +72,7 @@ struct xlx_ethlite
>      uint32_t regs[R_MAX];
>  };
>  
> -static inline void eth_pulse_irq(struct xlx_ethlite *s)
> +static inline void eth_pulse_irq(XlnxXpsEthLite *s)
>  {
>      /* Only the first gie reg is active.  */
>      if (s->regs[R_TX_GIE0] & GIE_GIE) {
> @@ -84,7 +83,7 @@ static inline void eth_pulse_irq(struct xlx_ethlite *s)
>  static uint64_t
>  eth_read(void *opaque, hwaddr addr, unsigned int size)
>  {
> -    struct xlx_ethlite *s = opaque;
> +    XlnxXpsEthLite *s = opaque;
>      uint32_t r = 0;
>  
>      addr >>= 2;
> @@ -112,7 +111,7 @@ static void
>  eth_write(void *opaque, hwaddr addr,
>            uint64_t val64, unsigned int size)
>  {
> -    struct xlx_ethlite *s = opaque;
> +    XlnxXpsEthLite *s = opaque;
>      unsigned int base = 0;
>      uint32_t value = val64;
>  
> @@ -176,7 +175,7 @@ static const MemoryRegionOps eth_ops = {
>  
>  static bool eth_can_rx(NetClientState *nc)
>  {
> -    struct xlx_ethlite *s = qemu_get_nic_opaque(nc);
> +    XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
>      unsigned int rxbase = s->rxbuf * (0x800 / 4);
>  
>      return !(s->regs[rxbase + R_RX_CTRL0] & CTRL_S);
> @@ -184,7 +183,7 @@ static bool eth_can_rx(NetClientState *nc)
>  
>  static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>  {
> -    struct xlx_ethlite *s = qemu_get_nic_opaque(nc);
> +    XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
>      unsigned int rxbase = s->rxbuf * (0x800 / 4);
>  
>      /* DA filter.  */
> @@ -214,7 +213,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>  
>  static void xilinx_ethlite_reset(DeviceState *dev)
>  {
> -    struct xlx_ethlite *s = XILINX_ETHLITE(dev);
> +    XlnxXpsEthLite *s = XILINX_ETHLITE(dev);
>  
>      s->rxbuf = 0;
>  }
> @@ -228,7 +227,7 @@ static NetClientInfo net_xilinx_ethlite_info = {
>  
>  static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
>  {
> -    struct xlx_ethlite *s = XILINX_ETHLITE(dev);
> +    XlnxXpsEthLite *s = XILINX_ETHLITE(dev);
>  
>      qemu_macaddr_default_if_unset(&s->conf.macaddr);
>      s->nic = qemu_new_nic(&net_xilinx_ethlite_info, &s->conf,
> @@ -239,7 +238,7 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
>  
>  static void xilinx_ethlite_init(Object *obj)
>  {
> -    struct xlx_ethlite *s = XILINX_ETHLITE(obj);
> +    XlnxXpsEthLite *s = XILINX_ETHLITE(obj);
>  
>      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
>  
> @@ -249,9 +248,9 @@ static void xilinx_ethlite_init(Object *obj)
>  }
>  
>  static Property xilinx_ethlite_properties[] = {
> -    DEFINE_PROP_UINT32("tx-ping-pong", struct xlx_ethlite, c_tx_pingpong, 1),
> -    DEFINE_PROP_UINT32("rx-ping-pong", struct xlx_ethlite, c_rx_pingpong, 1),
> -    DEFINE_NIC_PROPERTIES(struct xlx_ethlite, conf),
> +    DEFINE_PROP_UINT32("tx-ping-pong", XlnxXpsEthLite, c_tx_pingpong, 1),
> +    DEFINE_PROP_UINT32("rx-ping-pong", XlnxXpsEthLite, c_rx_pingpong, 1),
> +    DEFINE_NIC_PROPERTIES(XlnxXpsEthLite, conf),
>      DEFINE_PROP_END_OF_LIST(),
>  };
>  
> @@ -264,17 +263,14 @@ static void xilinx_ethlite_class_init(ObjectClass *klass, void *data)
>      device_class_set_props(dc, xilinx_ethlite_properties);
>  }
>  
> -static const TypeInfo xilinx_ethlite_info = {
> -    .name          = TYPE_XILINX_ETHLITE,
> -    .parent        = TYPE_SYS_BUS_DEVICE,
> -    .instance_size = sizeof(struct xlx_ethlite),
> -    .instance_init = xilinx_ethlite_init,
> -    .class_init    = xilinx_ethlite_class_init,
> +static const TypeInfo xilinx_ethlite_types[] = {
> +    {
> +        .name          = TYPE_XILINX_ETHLITE,
> +        .parent        = TYPE_SYS_BUS_DEVICE,
> +        .instance_size = sizeof(XlnxXpsEthLite),
> +        .instance_init = xilinx_ethlite_init,
> +        .class_init    = xilinx_ethlite_class_init,
> +    },
>  };
>  
> -static void xilinx_ethlite_register_types(void)
> -{
> -    type_register_static(&xilinx_ethlite_info);
> -}
> -
> -type_init(xilinx_ethlite_register_types)
> +DEFINE_TYPES(xilinx_ethlite_types)
> -- 
> 2.45.2
> 

Re: [PATCH 05/20] hw/net/xilinx_ethlite: Correct maximum RX buffer size

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:29PM +0100, Philippe Mathieu-Daudé wrote:
> The current max RX bufsize is set to 0x800. This is
> invalid, since it contains the MMIO registers region.
> Add the correct definition and use it.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> ---
>  hw/net/xilinx_ethlite.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index 0f59811c78..e6f6179fce 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -46,6 +46,8 @@
>  #define R_RX_CTRL1    (0x1ffc / 4)
>  #define R_MAX         (0x2000 / 4)
>  
> +#define RX_BUFSZ_MAX  0x07e0
> +
>  #define GIE_GIE    0x80000000
>  
>  #define CTRL_I     0x8
> @@ -195,7 +197,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>          return -1;
>      }
>  
> -    if (size > (R_MAX - R_RX_BUF0 - rxbase) * 4) {
> +    if (size > RX_BUFSZ_MAX) {
>          trace_ethlite_pkt_size_too_big(size);
>          return -1;
>      }
> -- 
> 2.45.2
> 

Re: [PATCH 06/20] hw/net/xilinx_ethlite: Map MDIO registers (as unimplemented)

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:30PM +0100, Philippe Mathieu-Daudé wrote:
> Rather than handling the MDIO registers as RAM, map them
> as unimplemented I/O within the device MR.
> 
> The memory flat view becomes:
> 
>   (qemu) info mtree -f
>   FlatView #0
>    Root memory region: system
>     0000000081000000-00000000810007e3 (prio 0, i/o): xlnx.xps-ethernetlite
>     00000000810007e4-00000000810007f3 (prio 0, i/o): ethlite.mdio
>     00000000810007f4-0000000081001fff (prio 0, i/o): xlnx.xps-ethernetlite @00000000000007f4
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>



> ---
>  hw/net/xilinx_ethlite.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index e6f6179fce..76b1e7d826 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -25,13 +25,17 @@
>  #include "qemu/osdep.h"
>  #include "qemu/module.h"
>  #include "qom/object.h"
> +#include "qapi/error.h"
>  #include "exec/tswap.h"
>  #include "hw/sysbus.h"
>  #include "hw/irq.h"
>  #include "hw/qdev-properties.h"
> +#include "hw/misc/unimp.h"
>  #include "net/net.h"
>  #include "trace.h"
>  
> +#define A_MDIO_BASE   0x07e4
> +
>  #define R_TX_BUF0     0
>  #define R_TX_LEN0     (0x07f4 / 4)
>  #define R_TX_GIE0     (0x07f8 / 4)
> @@ -71,6 +75,7 @@ struct XlnxXpsEthLite
>      unsigned int txbuf;
>      unsigned int rxbuf;
>  
> +    UnimplementedDeviceState mdio;
>      uint32_t regs[R_MAX];
>  };
>  
> @@ -231,6 +236,14 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
>  {
>      XlnxXpsEthLite *s = XILINX_ETHLITE(dev);
>  
> +    object_initialize_child(OBJECT(dev), "ethlite.mdio", &s->mdio,
> +                           TYPE_UNIMPLEMENTED_DEVICE);
> +    qdev_prop_set_string(DEVICE(&s->mdio), "name", "ethlite.mdio");
> +    qdev_prop_set_uint64(DEVICE(&s->mdio), "size", 4 * 4);
> +    sysbus_realize(SYS_BUS_DEVICE(&s->mdio), &error_fatal);
> +    memory_region_add_subregion(&s->mmio, A_MDIO_BASE,
> +                            sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mdio), 0));
> +
>      qemu_macaddr_default_if_unset(&s->conf.macaddr);
>      s->nic = qemu_new_nic(&net_xilinx_ethlite_info, &s->conf,
>                            object_get_typename(OBJECT(dev)), dev->id,
> -- 
> 2.45.2
> 

Re: [PATCH 07/20] hw/net/xilinx_ethlite: Rename rxbuf -> port_index

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:31PM +0100, Philippe Mathieu-Daudé wrote:
> 'rxbuf' is the index of the port used. Rename it as 'port_index'.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/net/xilinx_ethlite.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index 76b1e7d826..20919b4f54 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -72,8 +72,7 @@ struct XlnxXpsEthLite
>  
>      uint32_t c_tx_pingpong;
>      uint32_t c_rx_pingpong;
> -    unsigned int txbuf;
> -    unsigned int rxbuf;
> +    unsigned int port_index;


May want to add a comment that you're refering to the dual port RAM
index and not some other port...

Either way:
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>





>  
>      UnimplementedDeviceState mdio;
>      uint32_t regs[R_MAX];
> @@ -183,7 +182,7 @@ static const MemoryRegionOps eth_ops = {
>  static bool eth_can_rx(NetClientState *nc)
>  {
>      XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
> -    unsigned int rxbase = s->rxbuf * (0x800 / 4);
> +    unsigned int rxbase = s->port_index * (0x800 / 4);
>  
>      return !(s->regs[rxbase + R_RX_CTRL0] & CTRL_S);
>  }
> @@ -191,7 +190,7 @@ static bool eth_can_rx(NetClientState *nc)
>  static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>  {
>      XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
> -    unsigned int rxbase = s->rxbuf * (0x800 / 4);
> +    unsigned int rxbase = s->port_index * (0x800 / 4);
>  
>      /* DA filter.  */
>      if (!(buf[0] & 0x80) && memcmp(&s->conf.macaddr.a[0], buf, 6))
> @@ -214,7 +213,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>      }
>  
>      /* If c_rx_pingpong was set flip buffers.  */
> -    s->rxbuf ^= s->c_rx_pingpong;
> +    s->port_index ^= s->c_rx_pingpong;
>      return size;
>  }
>  
> @@ -222,7 +221,7 @@ static void xilinx_ethlite_reset(DeviceState *dev)
>  {
>      XlnxXpsEthLite *s = XILINX_ETHLITE(dev);
>  
> -    s->rxbuf = 0;
> +    s->port_index = 0;
>  }
>  
>  static NetClientInfo net_xilinx_ethlite_info = {
> -- 
> 2.45.2
> 

Re: [PATCH 08/20] hw/net/xilinx_ethlite: Add addr_to_port_index() helper

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:32PM +0100, Philippe Mathieu-Daudé wrote:
> For a particular physical address within the EthLite MMIO range,
> addr_to_port_index() returns which port is accessed.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/net/xilinx_ethlite.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index 20919b4f54..fe91891310 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -24,6 +24,7 @@
>  
>  #include "qemu/osdep.h"
>  #include "qemu/module.h"
> +#include "qemu/bitops.h"
>  #include "qom/object.h"
>  #include "qapi/error.h"
>  #include "exec/tswap.h"
> @@ -86,6 +87,12 @@ static inline void eth_pulse_irq(XlnxXpsEthLite *s)
>      }
>  }
>  
> +__attribute__((unused))
> +static unsigned addr_to_port_index(hwaddr addr)
> +{
> +    return extract64(addr, 11, 1);
> +}
> +

Shouldn't you add addr_to_port_index in the following patch and avoid
the temporary unused attribute?


>  static uint64_t
>  eth_read(void *opaque, hwaddr addr, unsigned int size)
>  {
> @@ -190,7 +197,8 @@ static bool eth_can_rx(NetClientState *nc)
>  static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>  {
>      XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
> -    unsigned int rxbase = s->port_index * (0x800 / 4);
> +    unsigned int port_index = s->port_index;
> +    unsigned int rxbase = port_index * (0x800 / 4);


Hmm, AFAICT s->port_index is an unsigned int, what is the purpose of this change?



>  
>      /* DA filter.  */
>      if (!(buf[0] & 0x80) && memcmp(&s->conf.macaddr.a[0], buf, 6))
> -- 
> 2.45.2
> 

Re: [PATCH 09/20] hw/net/xilinx_ethlite: Introduce txbuf_ptr() helper

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:33PM +0100, Philippe Mathieu-Daudé wrote:
> txbuf_ptr() points to the beginning of a (RAM) TX buffer
> within the device state.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>



> ---
>  hw/net/xilinx_ethlite.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index fe91891310..d4882f43f7 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -87,12 +87,18 @@ static inline void eth_pulse_irq(XlnxXpsEthLite *s)
>      }
>  }
>  
> -__attribute__((unused))
>  static unsigned addr_to_port_index(hwaddr addr)
>  {
>      return extract64(addr, 11, 1);
>  }
>  
> +static void *txbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
> +{
> +    unsigned int rxbase = port_index * (0x800 / 4);
> +
> +    return &s->regs[rxbase + R_TX_BUF0];
> +}
> +
>  static uint64_t
>  eth_read(void *opaque, hwaddr addr, unsigned int size)
>  {
> @@ -125,6 +131,7 @@ eth_write(void *opaque, hwaddr addr,
>            uint64_t val64, unsigned int size)
>  {
>      XlnxXpsEthLite *s = opaque;
> +    unsigned int port_index = addr_to_port_index(addr);
>      unsigned int base = 0;
>      uint32_t value = val64;
>  
> @@ -138,12 +145,12 @@ eth_write(void *opaque, hwaddr addr,
>  
>              if ((value & (CTRL_P | CTRL_S)) == CTRL_S) {
>                  qemu_send_packet(qemu_get_queue(s->nic),
> -                                 (void *) &s->regs[base],
> +                                 txbuf_ptr(s, port_index),
>                                   s->regs[base + R_TX_LEN0]);
>                  if (s->regs[base + R_TX_CTRL0] & CTRL_I)
>                      eth_pulse_irq(s);
>              } else if ((value & (CTRL_P | CTRL_S)) == (CTRL_P | CTRL_S)) {
> -                memcpy(&s->conf.macaddr.a[0], &s->regs[base], 6);
> +                memcpy(&s->conf.macaddr.a[0], txbuf_ptr(s, port_index), 6);
>                  if (s->regs[base + R_TX_CTRL0] & CTRL_I)
>                      eth_pulse_irq(s);
>              }
> -- 
> 2.45.2
> 

Re: [PATCH 10/20] hw/net/xilinx_ethlite: Introduce rxbuf_ptr() helper

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:34PM +0100, Philippe Mathieu-Daudé wrote:
> rxbuf_ptr() points to the beginning of a (RAM) RX buffer
> within the device state.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> ---
>  hw/net/xilinx_ethlite.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index d4882f43f7..fdbf25fd91 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -99,6 +99,13 @@ static void *txbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
>      return &s->regs[rxbase + R_TX_BUF0];
>  }
>  
> +static void *rxbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
> +{
> +    unsigned int rxbase = port_index * (0x800 / 4);
> +
> +    return &s->regs[rxbase + R_RX_BUF0];
> +}
> +
>  static uint64_t
>  eth_read(void *opaque, hwaddr addr, unsigned int size)
>  {
> @@ -220,7 +227,7 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>          trace_ethlite_pkt_size_too_big(size);
>          return -1;
>      }
> -    memcpy(&s->regs[rxbase + R_RX_BUF0], buf, size);
> +    memcpy(rxbuf_ptr(s, port_index), buf, size);
>  
>      s->regs[rxbase + R_RX_CTRL0] |= CTRL_S;
>      if (s->regs[R_RX_CTRL0] & CTRL_I) {
> -- 
> 2.45.2
> 

Re: [PATCH 11/20] hw/net/xilinx_ethlite: Access RX_CTRL register for each port

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:35PM +0100, Philippe Mathieu-Daudé wrote:
> Rather than accessing the registers within the mixed RAM/MMIO
> region as indexed register, declare a per-port RX_CTRL. This
> will help to map the RAM as RAM (keeping MMIO as MMIO) in few
> commits.
> 
> Previous s->regs[R_RX_CTRL0] and s->regs[R_RX_CTRL1] are now
> unused. Not a concern, this array will soon disappear.


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/net/xilinx_ethlite.c | 31 +++++++++++++++++++++----------
>  1 file changed, 21 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index fdbf25fd91..605451a522 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -59,6 +59,13 @@
>  #define CTRL_P     0x2
>  #define CTRL_S     0x1
>  
> +typedef struct XlnxXpsEthLitePort
> +{
> +    struct {
> +        uint32_t rx_ctrl;
> +    } reg;
> +} XlnxXpsEthLitePort;
> +
>  #define TYPE_XILINX_ETHLITE "xlnx.xps-ethernetlite"
>  OBJECT_DECLARE_SIMPLE_TYPE(XlnxXpsEthLite, XILINX_ETHLITE)
>  
> @@ -76,6 +83,7 @@ struct XlnxXpsEthLite
>      unsigned int port_index;
>  
>      UnimplementedDeviceState mdio;
> +    XlnxXpsEthLitePort port[2];
>      uint32_t regs[R_MAX];
>  };
>  
> @@ -110,6 +118,7 @@ static uint64_t
>  eth_read(void *opaque, hwaddr addr, unsigned int size)
>  {
>      XlnxXpsEthLite *s = opaque;
> +    unsigned port_index = addr_to_port_index(addr);
>      uint32_t r = 0;
>  
>      addr >>= 2;
> @@ -121,11 +130,13 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
>          case R_TX_LEN1:
>          case R_TX_CTRL1:
>          case R_TX_CTRL0:
> -        case R_RX_CTRL1:
> -        case R_RX_CTRL0:
>              r = s->regs[addr];
>              break;
>  
> +        case R_RX_CTRL1:
> +        case R_RX_CTRL0:
> +            r = s->port[port_index].reg.rx_ctrl;
> +
>          default:
>              r = tswap32(s->regs[addr]);
>              break;
> @@ -173,7 +184,9 @@ eth_write(void *opaque, hwaddr addr,
>              if (!(value & CTRL_S)) {
>                  qemu_flush_queued_packets(qemu_get_queue(s->nic));
>              }
> -            /* fall through */
> +            s->port[port_index].reg.rx_ctrl = value;
> +            break;
> +
>          case R_TX_LEN0:
>          case R_TX_LEN1:
>          case R_TX_GIE0:
> @@ -203,23 +216,21 @@ static const MemoryRegionOps eth_ops = {
>  static bool eth_can_rx(NetClientState *nc)
>  {
>      XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
> -    unsigned int rxbase = s->port_index * (0x800 / 4);
>  
> -    return !(s->regs[rxbase + R_RX_CTRL0] & CTRL_S);
> +    return !(s->port[s->port_index].reg.rx_ctrl & CTRL_S);
>  }
>  
>  static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>  {
>      XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
>      unsigned int port_index = s->port_index;
> -    unsigned int rxbase = port_index * (0x800 / 4);
>  
>      /* DA filter.  */
>      if (!(buf[0] & 0x80) && memcmp(&s->conf.macaddr.a[0], buf, 6))
>          return size;
>  
> -    if (s->regs[rxbase + R_RX_CTRL0] & CTRL_S) {
> -        trace_ethlite_pkt_lost(s->regs[R_RX_CTRL0]);
> +    if (s->port[port_index].reg.rx_ctrl & CTRL_S) {
> +        trace_ethlite_pkt_lost(s->port[port_index].reg.rx_ctrl);
>          return -1;
>      }
>  
> @@ -229,8 +240,8 @@ static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>      }
>      memcpy(rxbuf_ptr(s, port_index), buf, size);
>  
> -    s->regs[rxbase + R_RX_CTRL0] |= CTRL_S;
> -    if (s->regs[R_RX_CTRL0] & CTRL_I) {
> +    s->port[port_index].reg.rx_ctrl |= CTRL_S;
> +    if (s->port[port_index].reg.rx_ctrl & CTRL_I) {
>          eth_pulse_irq(s);
>      }
>  
> -- 
> 2.45.2
> 

Re: [PATCH 12/20] hw/net/xilinx_ethlite: Access TX_GIE register for each port

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:36PM +0100, Philippe Mathieu-Daudé wrote:
> Rather than accessing the registers within the mixed RAM/MMIO
> region as indexed register, declare a per-port TX_GIE. This
> will help to map the RAM as RAM (keeping MMIO as MMIO) in few
> commits.
> 
> Previous s->regs[R_TX_GIE0] and s->regs[R_TX_GIE1] are now
> unused. Not a concern, this array will soon disappear.


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/net/xilinx_ethlite.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index 605451a522..4cb4781e70 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -62,6 +62,8 @@
>  typedef struct XlnxXpsEthLitePort
>  {
>      struct {
> +        uint32_t tx_gie;
> +
>          uint32_t rx_ctrl;
>      } reg;
>  } XlnxXpsEthLitePort;
> @@ -90,7 +92,7 @@ struct XlnxXpsEthLite
>  static inline void eth_pulse_irq(XlnxXpsEthLite *s)
>  {
>      /* Only the first gie reg is active.  */
> -    if (s->regs[R_TX_GIE0] & GIE_GIE) {
> +    if (s->port[0].reg.tx_gie & GIE_GIE) {
>          qemu_irq_pulse(s->irq);
>      }
>  }
> @@ -126,6 +128,9 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
>      switch (addr)
>      {
>          case R_TX_GIE0:
> +            r = s->port[port_index].reg.tx_gie;
> +            break;
> +
>          case R_TX_LEN0:
>          case R_TX_LEN1:
>          case R_TX_CTRL1:
> @@ -189,10 +194,13 @@ eth_write(void *opaque, hwaddr addr,
>  
>          case R_TX_LEN0:
>          case R_TX_LEN1:
> -        case R_TX_GIE0:
>              s->regs[addr] = value;
>              break;
>  
> +        case R_TX_GIE0:
> +            s->port[port_index].reg.tx_gie = value;
> +            break;
> +
>          default:
>              s->regs[addr] = tswap32(value);
>              break;
> -- 
> 2.45.2
> 

Re: [PATCH 13/20] hw/net/xilinx_ethlite: Access TX_LEN register for each port

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:37PM +0100, Philippe Mathieu-Daudé wrote:
> Rather than accessing the registers within the mixed RAM/MMIO
> region as indexed register, declare a per-port TX_LEN. This
> will help to map the RAM as RAM (keeping MMIO as MMIO) in few
> commits.
> 
> Previous s->regs[R_TX_LEN0] and s->regs[R_TX_LEN1] are now
> unused. Not a concern, this array will soon disappear.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/net/xilinx_ethlite.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index 4cb4781e70..1a3b295b4b 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -62,6 +62,7 @@
>  typedef struct XlnxXpsEthLitePort
>  {
>      struct {
> +        uint32_t tx_len;
>          uint32_t tx_gie;
>  
>          uint32_t rx_ctrl;
> @@ -133,6 +134,9 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
>  
>          case R_TX_LEN0:
>          case R_TX_LEN1:
> +            r = s->port[port_index].reg.tx_len;
> +            break;
> +
>          case R_TX_CTRL1:
>          case R_TX_CTRL0:
>              r = s->regs[addr];
> @@ -169,7 +173,7 @@ eth_write(void *opaque, hwaddr addr,
>              if ((value & (CTRL_P | CTRL_S)) == CTRL_S) {
>                  qemu_send_packet(qemu_get_queue(s->nic),
>                                   txbuf_ptr(s, port_index),
> -                                 s->regs[base + R_TX_LEN0]);
> +                                 s->port[port_index].reg.tx_len);
>                  if (s->regs[base + R_TX_CTRL0] & CTRL_I)
>                      eth_pulse_irq(s);
>              } else if ((value & (CTRL_P | CTRL_S)) == (CTRL_P | CTRL_S)) {
> @@ -194,7 +198,7 @@ eth_write(void *opaque, hwaddr addr,
>  
>          case R_TX_LEN0:
>          case R_TX_LEN1:
> -            s->regs[addr] = value;
> +            s->port[port_index].reg.tx_len = value;
>              break;
>  
>          case R_TX_GIE0:
> -- 
> 2.45.2
> 

Re: [PATCH 14/20] hw/net/xilinx_ethlite: Access TX_CTRL register for each port

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:38PM +0100, Philippe Mathieu-Daudé wrote:
> Rather than accessing the registers within the mixed RAM/MMIO
> region as indexed register, declare a per-port TX_CTRL. This
> will help to map the RAM as RAM (keeping MMIO as MMIO) in few
> commits.
> 
> Previous s->regs[R_TX_CTRL0] and s->regs[R_TX_CTRL1] are now
> unused. Not a concern, this array will soon disappear.


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/net/xilinx_ethlite.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index 1a3b295b4b..4d86851f38 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -64,6 +64,7 @@ typedef struct XlnxXpsEthLitePort
>      struct {
>          uint32_t tx_len;
>          uint32_t tx_gie;
> +        uint32_t tx_ctrl;
>  
>          uint32_t rx_ctrl;
>      } reg;
> @@ -139,7 +140,7 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
>  
>          case R_TX_CTRL1:
>          case R_TX_CTRL0:
> -            r = s->regs[addr];
> +            r = s->port[port_index].reg.tx_ctrl;
>              break;
>  
>          case R_RX_CTRL1:
> @@ -159,7 +160,6 @@ eth_write(void *opaque, hwaddr addr,
>  {
>      XlnxXpsEthLite *s = opaque;
>      unsigned int port_index = addr_to_port_index(addr);
> -    unsigned int base = 0;
>      uint32_t value = val64;
>  
>      addr >>= 2;
> @@ -167,24 +167,23 @@ eth_write(void *opaque, hwaddr addr,
>      {
>          case R_TX_CTRL0:
>          case R_TX_CTRL1:
> -            if (addr == R_TX_CTRL1)
> -                base = 0x800 / 4;
> -
>              if ((value & (CTRL_P | CTRL_S)) == CTRL_S) {
>                  qemu_send_packet(qemu_get_queue(s->nic),
>                                   txbuf_ptr(s, port_index),
>                                   s->port[port_index].reg.tx_len);
> -                if (s->regs[base + R_TX_CTRL0] & CTRL_I)
> +                if (s->port[port_index].reg.tx_ctrl & CTRL_I) {
>                      eth_pulse_irq(s);
> +                }
>              } else if ((value & (CTRL_P | CTRL_S)) == (CTRL_P | CTRL_S)) {
>                  memcpy(&s->conf.macaddr.a[0], txbuf_ptr(s, port_index), 6);
> -                if (s->regs[base + R_TX_CTRL0] & CTRL_I)
> +                if (s->port[port_index].reg.tx_ctrl & CTRL_I) {
>                      eth_pulse_irq(s);
> +                }
>              }
>  
>              /* We are fast and get ready pretty much immediately so
>                 we actually never flip the S nor P bits to one.  */
> -            s->regs[addr] = value & ~(CTRL_P | CTRL_S);
> +            s->port[port_index].reg.tx_ctrl = value & ~(CTRL_P | CTRL_S);
>              break;
>  
>          /* Keep these native.  */
> -- 
> 2.45.2
> 

Re: [PATCH 15/20] hw/net/xilinx_ethlite: Map RX_CTRL as MMIO

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:39PM +0100, Philippe Mathieu-Daudé wrote:
> Declare RX registers as MMIO region, split it out
> of the current mixed RAM/MMIO region.
> The memory flat view becomes:
> 
>   FlatView #0
>    Root memory region: system
>     0000000081000000-00000000810007e3 (prio 0, i/o): xlnx.xps-ethernetlite
>     00000000810007e4-00000000810007f3 (prio 0, i/o): ethlite.mdio
>     00000000810007f4-00000000810017fb (prio 0, i/o): xlnx.xps-ethernetlite @00000000000007f4
>     00000000810017fc-00000000810017ff (prio 0, i/o): ethlite.rx[0]io
>     0000000081001800-0000000081001ffb (prio 0, i/o): xlnx.xps-ethernetlite @0000000000001800
>     0000000081001ffc-0000000081001fff (prio 0, i/o): ethlite.rx[1]io
>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/net/xilinx_ethlite.c | 79 +++++++++++++++++++++++++++++++++--------
>  1 file changed, 65 insertions(+), 14 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index 4d86851f38..161fd97f06 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -46,13 +46,18 @@
>  #define R_TX_CTRL1    (0x0ffc / 4)
>  
>  #define R_RX_BUF0     (0x1000 / 4)
> -#define R_RX_CTRL0    (0x17fc / 4)
> +#define A_RX_BASE0    0x17fc
>  #define R_RX_BUF1     (0x1800 / 4)
> -#define R_RX_CTRL1    (0x1ffc / 4)
> +#define A_RX_BASE1    0x1ffc
>  #define R_MAX         (0x2000 / 4)
>  
>  #define RX_BUFSZ_MAX  0x07e0
>  
> +enum {
> +    RX_CTRL = 0,
> +    RX_MAX
> +};
> +
>  #define GIE_GIE    0x80000000
>  
>  #define CTRL_I     0x8
> @@ -61,6 +66,8 @@
>  
>  typedef struct XlnxXpsEthLitePort
>  {
> +    MemoryRegion rxio;
> +
>      struct {
>          uint32_t tx_len;
>          uint32_t tx_gie;
> @@ -118,6 +125,53 @@ static void *rxbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
>      return &s->regs[rxbase + R_RX_BUF0];
>  }
>  
> +static uint64_t port_rx_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    XlnxXpsEthLite *s = opaque;
> +    unsigned port_index = addr_to_port_index(addr);
> +    uint32_t r = 0;
> +
> +    switch (addr >> 2) {
> +        case RX_CTRL:
> +            r = s->port[port_index].reg.rx_ctrl;
> +            break;
> +        default:
> +            g_assert_not_reached();
> +    }
> +
> +    return r;
> +}
> +
> +static void port_rx_write(void *opaque, hwaddr addr, uint64_t value,
> +                          unsigned int size)
> +{
> +    XlnxXpsEthLite *s = opaque;
> +
> +    switch (addr >> 2) {
> +        case RX_CTRL:
> +            if (!(value & CTRL_S)) {
> +                qemu_flush_queued_packets(qemu_get_queue(s->nic));
> +            }
> +            break;
> +        default:
> +            g_assert_not_reached();
> +    }
> +}
> +
> +static const MemoryRegionOps eth_portrx_ops = {
> +        .read = port_rx_read,
> +        .write = port_rx_write,
> +        .endianness = DEVICE_NATIVE_ENDIAN,
> +        .impl = {
> +            .min_access_size = 4,
> +            .max_access_size = 4,
> +        },
> +        .valid = {
> +            .min_access_size = 4,
> +            .max_access_size = 4,
> +        },
> +};
> +
>  static uint64_t
>  eth_read(void *opaque, hwaddr addr, unsigned int size)
>  {
> @@ -143,10 +197,6 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
>              r = s->port[port_index].reg.tx_ctrl;
>              break;
>  
> -        case R_RX_CTRL1:
> -        case R_RX_CTRL0:
> -            r = s->port[port_index].reg.rx_ctrl;
> -
>          default:
>              r = tswap32(s->regs[addr]);
>              break;
> @@ -187,14 +237,6 @@ eth_write(void *opaque, hwaddr addr,
>              break;
>  
>          /* Keep these native.  */
> -        case R_RX_CTRL0:
> -        case R_RX_CTRL1:
> -            if (!(value & CTRL_S)) {
> -                qemu_flush_queued_packets(qemu_get_queue(s->nic));
> -            }
> -            s->port[port_index].reg.rx_ctrl = value;
> -            break;
> -
>          case R_TX_LEN0:
>          case R_TX_LEN1:
>              s->port[port_index].reg.tx_len = value;
> @@ -287,6 +329,15 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
>      memory_region_add_subregion(&s->mmio, A_MDIO_BASE,
>                              sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mdio), 0));
>  
> +    for (unsigned i = 0; i < 2; i++) {
> +        memory_region_init_io(&s->port[i].rxio, OBJECT(dev),
> +                              &eth_portrx_ops, s,
> +                              i ? "ethlite.rx[1]io" : "ethlite.rx[0]io",
> +                              4 * RX_MAX);
> +        memory_region_add_subregion(&s->mmio, i ? A_RX_BASE1 : A_RX_BASE0,
> +                                    &s->port[i].rxio);
> +    }
> +
>      qemu_macaddr_default_if_unset(&s->conf.macaddr);
>      s->nic = qemu_new_nic(&net_xilinx_ethlite_info, &s->conf,
>                            object_get_typename(OBJECT(dev)), dev->id,
> -- 
> 2.45.2
> 

Re: [PATCH 16/20] hw/net/xilinx_ethlite: Map TX_LEN as MMIO

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:40PM +0100, Philippe Mathieu-Daudé wrote:
> Declare TX registers as MMIO region, split it out
> of the current mixed RAM/MMIO region.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> ---
>  hw/net/xilinx_ethlite.c | 71 ++++++++++++++++++++++++++++++++++-------
>  1 file changed, 59 insertions(+), 12 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index 161fd97f06..159b2b0c64 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -38,11 +38,11 @@
>  #define A_MDIO_BASE   0x07e4
>  
>  #define R_TX_BUF0     0
> -#define R_TX_LEN0     (0x07f4 / 4)
> +#define A_TX_BASE0    0x07f4
>  #define R_TX_GIE0     (0x07f8 / 4)
>  #define R_TX_CTRL0    (0x07fc / 4)
>  #define R_TX_BUF1     (0x0800 / 4)
> -#define R_TX_LEN1     (0x0ff4 / 4)
> +#define A_TX_BASE1    0x0ff4
>  #define R_TX_CTRL1    (0x0ffc / 4)
>  
>  #define R_RX_BUF0     (0x1000 / 4)
> @@ -53,6 +53,11 @@
>  
>  #define RX_BUFSZ_MAX  0x07e0
>  
> +enum {
> +    TX_LEN =  0,
> +    TX_MAX
> +};
> +
>  enum {
>      RX_CTRL = 0,
>      RX_MAX
> @@ -125,6 +130,51 @@ static void *rxbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
>      return &s->regs[rxbase + R_RX_BUF0];
>  }
>  
> +static uint64_t port_tx_read(void *opaque, hwaddr addr, unsigned int size)
> +{
> +    XlnxXpsEthLite *s = opaque;
> +    unsigned port_index = addr_to_port_index(addr);
> +    uint32_t r = 0;
> +
> +    switch (addr >> 2) {
> +        case TX_LEN:
> +            r = s->port[port_index].reg.tx_len;
> +            break;
> +        default:
> +            g_assert_not_reached();
> +    }
> +
> +    return r;
> +}
> +
> +static void port_tx_write(void *opaque, hwaddr addr, uint64_t value,
> +                          unsigned int size)
> +{
> +    XlnxXpsEthLite *s = opaque;
> +
> +    switch (addr >> 2) {
> +        case TX_LEN:
> +            s->port[port_index].reg.tx_len = value;
> +            break;
> +        default:
> +            g_assert_not_reached();
> +    }
> +}
> +
> +static const MemoryRegionOps eth_porttx_ops = {
> +        .read = port_tx_read,
> +        .write = port_tx_write,
> +        .endianness = DEVICE_NATIVE_ENDIAN,
> +        .impl = {
> +            .min_access_size = 4,
> +            .max_access_size = 4,
> +        },
> +        .valid = {
> +            .min_access_size = 4,
> +            .max_access_size = 4,
> +        },
> +};
> +
>  static uint64_t port_rx_read(void *opaque, hwaddr addr, unsigned int size)
>  {
>      XlnxXpsEthLite *s = opaque;
> @@ -187,11 +237,6 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
>              r = s->port[port_index].reg.tx_gie;
>              break;
>  
> -        case R_TX_LEN0:
> -        case R_TX_LEN1:
> -            r = s->port[port_index].reg.tx_len;
> -            break;
> -
>          case R_TX_CTRL1:
>          case R_TX_CTRL0:
>              r = s->port[port_index].reg.tx_ctrl;
> @@ -237,11 +282,6 @@ eth_write(void *opaque, hwaddr addr,
>              break;
>  
>          /* Keep these native.  */
> -        case R_TX_LEN0:
> -        case R_TX_LEN1:
> -            s->port[port_index].reg.tx_len = value;
> -            break;
> -
>          case R_TX_GIE0:
>              s->port[port_index].reg.tx_gie = value;
>              break;
> @@ -330,6 +370,13 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
>                              sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mdio), 0));
>  
>      for (unsigned i = 0; i < 2; i++) {
> +        memory_region_init_io(&s->port[i].txio, OBJECT(dev),
> +                              &eth_porttx_ops, s,
> +                              i ? "ethlite.tx[1]io" : "ethlite.tx[0]io",
> +                              4 * TX_MAX);
> +        memory_region_add_subregion(&s->mmio, i ? A_TX_BASE1 : A_TX_BASE0,
> +                                    &s->port[i].txio);
> +
>          memory_region_init_io(&s->port[i].rxio, OBJECT(dev),
>                                &eth_portrx_ops, s,
>                                i ? "ethlite.rx[1]io" : "ethlite.rx[0]io",
> -- 
> 2.45.2
> 

Re: [PATCH 17/20] hw/net/xilinx_ethlite: Map TX_GIE as MMIO

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:41PM +0100, Philippe Mathieu-Daudé wrote:
> Add TX_GIE to the TX registers MMIO region.
> 
> Before TX_GIE1 was accessed as RAM, with no effect.
> Now it is accessed as MMIO, also without any effect.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>


Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> ---
>  hw/net/xilinx_ethlite.c | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index 159b2b0c64..f7a5b1620a 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -39,7 +39,6 @@
>  
>  #define R_TX_BUF0     0
>  #define A_TX_BASE0    0x07f4
> -#define R_TX_GIE0     (0x07f8 / 4)
>  #define R_TX_CTRL0    (0x07fc / 4)
>  #define R_TX_BUF1     (0x0800 / 4)
>  #define A_TX_BASE1    0x0ff4
> @@ -55,6 +54,7 @@
>  
>  enum {
>      TX_LEN =  0,
> +    TX_GIE =  1,
>      TX_MAX
>  };
>  
> @@ -140,6 +140,9 @@ static uint64_t port_tx_read(void *opaque, hwaddr addr, unsigned int size)
>          case TX_LEN:
>              r = s->port[port_index].reg.tx_len;
>              break;
> +        case TX_GIE:
> +            r = s->port[port_index].reg.tx_gie;
> +            break;
>          default:
>              g_assert_not_reached();
>      }
> @@ -156,6 +159,9 @@ static void port_tx_write(void *opaque, hwaddr addr, uint64_t value,
>          case TX_LEN:
>              s->port[port_index].reg.tx_len = value;
>              break;
> +        case TX_GIE:
> +            s->port[port_index].reg.tx_gie = value;
> +            break;
>          default:
>              g_assert_not_reached();
>      }
> @@ -233,10 +239,6 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
>  
>      switch (addr)
>      {
> -        case R_TX_GIE0:
> -            r = s->port[port_index].reg.tx_gie;
> -            break;
> -
>          case R_TX_CTRL1:
>          case R_TX_CTRL0:
>              r = s->port[port_index].reg.tx_ctrl;
> @@ -281,11 +283,6 @@ eth_write(void *opaque, hwaddr addr,
>              s->port[port_index].reg.tx_ctrl = value & ~(CTRL_P | CTRL_S);
>              break;
>  
> -        /* Keep these native.  */
> -        case R_TX_GIE0:
> -            s->port[port_index].reg.tx_gie = value;
> -            break;
> -
>          default:
>              s->regs[addr] = tswap32(value);
>              break;
> -- 
> 2.45.2
> 

Re: [PATCH 18/20] hw/net/xilinx_ethlite: Map TX_CTRL as MMIO

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:42PM +0100, Philippe Mathieu-Daudé wrote:
> Add TX_CTRL to the TX registers MMIO region.
>

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>



> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/net/xilinx_ethlite.c | 56 +++++++++++++++++++----------------------
>  1 file changed, 26 insertions(+), 30 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index f7a5b1620a..f681b91769 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -39,10 +39,8 @@
>  
>  #define R_TX_BUF0     0
>  #define A_TX_BASE0    0x07f4
> -#define R_TX_CTRL0    (0x07fc / 4)
>  #define R_TX_BUF1     (0x0800 / 4)
>  #define A_TX_BASE1    0x0ff4
> -#define R_TX_CTRL1    (0x0ffc / 4)
>  
>  #define R_RX_BUF0     (0x1000 / 4)
>  #define A_RX_BASE0    0x17fc
> @@ -55,6 +53,7 @@
>  enum {
>      TX_LEN =  0,
>      TX_GIE =  1,
> +    TX_CTRL = 2,
>      TX_MAX
>  };
>  
> @@ -71,6 +70,7 @@ enum {
>  
>  typedef struct XlnxXpsEthLitePort
>  {
> +    MemoryRegion txio;
>      MemoryRegion rxio;
>  
>      struct {
> @@ -143,6 +143,9 @@ static uint64_t port_tx_read(void *opaque, hwaddr addr, unsigned int size)
>          case TX_GIE:
>              r = s->port[port_index].reg.tx_gie;
>              break;
> +        case TX_CTRL:
> +            r = s->port[port_index].reg.tx_ctrl;
> +            break;
>          default:
>              g_assert_not_reached();
>      }
> @@ -154,6 +157,7 @@ static void port_tx_write(void *opaque, hwaddr addr, uint64_t value,
>                            unsigned int size)
>  {
>      XlnxXpsEthLite *s = opaque;
> +    unsigned port_index = addr_to_port_index(addr);
>  
>      switch (addr >> 2) {
>          case TX_LEN:
> @@ -162,6 +166,26 @@ static void port_tx_write(void *opaque, hwaddr addr, uint64_t value,
>          case TX_GIE:
>              s->port[port_index].reg.tx_gie = value;
>              break;
> +        case TX_CTRL:
> +            if ((value & (CTRL_P | CTRL_S)) == CTRL_S) {
> +                qemu_send_packet(qemu_get_queue(s->nic),
> +                                 txbuf_ptr(s, port_index),
> +                                 s->port[port_index].reg.tx_len);
> +                if (s->port[port_index].reg.tx_ctrl & CTRL_I) {
> +                    eth_pulse_irq(s);
> +                }
> +            } else if ((value & (CTRL_P | CTRL_S)) == (CTRL_P | CTRL_S)) {
> +                memcpy(&s->conf.macaddr.a[0], txbuf_ptr(s, port_index), 6);
> +                if (s->port[port_index].reg.tx_ctrl & CTRL_I) {
> +                    eth_pulse_irq(s);
> +                }
> +            }
> +            /*
> +             * We are fast and get ready pretty much immediately
> +             * so we actually never flip the S nor P bits to one.
> +             */
> +            s->port[port_index].reg.tx_ctrl = value & ~(CTRL_P | CTRL_S);
> +            break;
>          default:
>              g_assert_not_reached();
>      }
> @@ -232,18 +256,12 @@ static uint64_t
>  eth_read(void *opaque, hwaddr addr, unsigned int size)
>  {
>      XlnxXpsEthLite *s = opaque;
> -    unsigned port_index = addr_to_port_index(addr);
>      uint32_t r = 0;
>  
>      addr >>= 2;
>  
>      switch (addr)
>      {
> -        case R_TX_CTRL1:
> -        case R_TX_CTRL0:
> -            r = s->port[port_index].reg.tx_ctrl;
> -            break;
> -
>          default:
>              r = tswap32(s->regs[addr]);
>              break;
> @@ -256,33 +274,11 @@ eth_write(void *opaque, hwaddr addr,
>            uint64_t val64, unsigned int size)
>  {
>      XlnxXpsEthLite *s = opaque;
> -    unsigned int port_index = addr_to_port_index(addr);
>      uint32_t value = val64;
>  
>      addr >>= 2;
>      switch (addr) 
>      {
> -        case R_TX_CTRL0:
> -        case R_TX_CTRL1:
> -            if ((value & (CTRL_P | CTRL_S)) == CTRL_S) {
> -                qemu_send_packet(qemu_get_queue(s->nic),
> -                                 txbuf_ptr(s, port_index),
> -                                 s->port[port_index].reg.tx_len);
> -                if (s->port[port_index].reg.tx_ctrl & CTRL_I) {
> -                    eth_pulse_irq(s);
> -                }
> -            } else if ((value & (CTRL_P | CTRL_S)) == (CTRL_P | CTRL_S)) {
> -                memcpy(&s->conf.macaddr.a[0], txbuf_ptr(s, port_index), 6);
> -                if (s->port[port_index].reg.tx_ctrl & CTRL_I) {
> -                    eth_pulse_irq(s);
> -                }
> -            }
> -
> -            /* We are fast and get ready pretty much immediately so
> -               we actually never flip the S nor P bits to one.  */
> -            s->port[port_index].reg.tx_ctrl = value & ~(CTRL_P | CTRL_S);
> -            break;
> -
>          default:
>              s->regs[addr] = tswap32(value);
>              break;
> -- 
> 2.45.2
> 

Re: [PATCH 19/20] hw/net/xilinx_ethlite: Map the RAM buffer as RAM memory region

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:43PM +0100, Philippe Mathieu-Daudé wrote:
> Rather than using I/O registers for RAM buffer, having to
> swap endianness back and forth (because the core memory layer
> automatically swaps endiannes for us), declare the buffers
> as RAM regions. Remove the now unused s->regs[] array.
> 
> The memory flat view becomes:
> 
>   FlatView #0
>    Root memory region: system
>     0000000081000000-00000000810007f3 (prio 0, ram): ethlite.tx[0]buf
>     00000000810007f4-00000000810007ff (prio 0, i/o): ethlite.tx[0]io
>     0000000081000800-0000000081000ff3 (prio 0, ram): ethlite.tx[1]buf
>     0000000081000ff4-0000000081000fff (prio 0, i/o): ethlite.tx[1]io
>     0000000081001000-00000000810017f3 (prio 0, ram): ethlite.rx[0]buf
>     00000000810017fc-00000000810017ff (prio 0, i/o): ethlite.rx[0]io
>     0000000081001800-0000000081001ff3 (prio 0, ram): ethlite.rx[1]buf
>     0000000081001ffc-0000000081001fff (prio 0, i/o): ethlite.rx[1]io
> 
> Mention the device datasheet in the file header.

Nice!

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/net/xilinx_ethlite.c | 79 +++++++++++------------------------------
>  1 file changed, 20 insertions(+), 59 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index f681b91769..da453465ca 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -2,6 +2,10 @@
>   * QEMU model of the Xilinx Ethernet Lite MAC.
>   *
>   * Copyright (c) 2009 Edgar E. Iglesias.
> + * Copyright (c) 2024 Linaro, Ltd
> + *
> + * DS580: https://docs.amd.com/v/u/en-US/xps_ethernetlite
> + * LogiCORE IP XPS Ethernet Lite Media Access Controller
>   *
>   * Permission is hereby granted, free of charge, to any person obtaining a copy
>   * of this software and associated documentation files (the "Software"), to deal
> @@ -27,7 +31,6 @@
>  #include "qemu/bitops.h"
>  #include "qom/object.h"
>  #include "qapi/error.h"
> -#include "exec/tswap.h"
>  #include "hw/sysbus.h"
>  #include "hw/irq.h"
>  #include "hw/qdev-properties.h"
> @@ -46,7 +49,6 @@
>  #define A_RX_BASE0    0x17fc
>  #define R_RX_BUF1     (0x1800 / 4)
>  #define A_RX_BASE1    0x1ffc
> -#define R_MAX         (0x2000 / 4)
>  
>  #define RX_BUFSZ_MAX  0x07e0
>  
> @@ -72,6 +74,8 @@ typedef struct XlnxXpsEthLitePort
>  {
>      MemoryRegion txio;
>      MemoryRegion rxio;
> +    MemoryRegion txbuf;
> +    MemoryRegion rxbuf;
>  
>      struct {
>          uint32_t tx_len;
> @@ -100,7 +104,6 @@ struct XlnxXpsEthLite
>  
>      UnimplementedDeviceState mdio;
>      XlnxXpsEthLitePort port[2];
> -    uint32_t regs[R_MAX];
>  };
>  
>  static inline void eth_pulse_irq(XlnxXpsEthLite *s)
> @@ -118,16 +121,12 @@ static unsigned addr_to_port_index(hwaddr addr)
>  
>  static void *txbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
>  {
> -    unsigned int rxbase = port_index * (0x800 / 4);
> -
> -    return &s->regs[rxbase + R_TX_BUF0];
> +    return memory_region_get_ram_ptr(&s->port[port_index].txbuf);
>  }
>  
>  static void *rxbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
>  {
> -    unsigned int rxbase = port_index * (0x800 / 4);
> -
> -    return &s->regs[rxbase + R_RX_BUF0];
> +    return memory_region_get_ram_ptr(&s->port[port_index].rxbuf);
>  }
>  
>  static uint64_t port_tx_read(void *opaque, hwaddr addr, unsigned int size)
> @@ -252,53 +251,6 @@ static const MemoryRegionOps eth_portrx_ops = {
>          },
>  };
>  
> -static uint64_t
> -eth_read(void *opaque, hwaddr addr, unsigned int size)
> -{
> -    XlnxXpsEthLite *s = opaque;
> -    uint32_t r = 0;
> -
> -    addr >>= 2;
> -
> -    switch (addr)
> -    {
> -        default:
> -            r = tswap32(s->regs[addr]);
> -            break;
> -    }
> -    return r;
> -}
> -
> -static void
> -eth_write(void *opaque, hwaddr addr,
> -          uint64_t val64, unsigned int size)
> -{
> -    XlnxXpsEthLite *s = opaque;
> -    uint32_t value = val64;
> -
> -    addr >>= 2;
> -    switch (addr) 
> -    {
> -        default:
> -            s->regs[addr] = tswap32(value);
> -            break;
> -    }
> -}
> -
> -static const MemoryRegionOps eth_ops = {
> -    .read = eth_read,
> -    .write = eth_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> -    .impl = {
> -        .min_access_size = 4,
> -        .max_access_size = 4,
> -    },
> -    .valid = {
> -        .min_access_size = 4,
> -        .max_access_size = 4
> -    }
> -};
> -
>  static bool eth_can_rx(NetClientState *nc)
>  {
>      XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
> @@ -354,6 +306,9 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
>  {
>      XlnxXpsEthLite *s = XILINX_ETHLITE(dev);
>  
> +    memory_region_init(&s->mmio, OBJECT(dev),
> +                       "xlnx.xps-ethernetlite", 0x2000);
> +
>      object_initialize_child(OBJECT(dev), "ethlite.mdio", &s->mdio,
>                             TYPE_UNIMPLEMENTED_DEVICE);
>      qdev_prop_set_string(DEVICE(&s->mdio), "name", "ethlite.mdio");
> @@ -363,6 +318,10 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
>                              sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mdio), 0));
>  
>      for (unsigned i = 0; i < 2; i++) {
> +        memory_region_init_ram(&s->port[i].txbuf, OBJECT(dev),
> +                               i ? "ethlite.tx[1]buf" : "ethlite.tx[0]buf",
> +                               0x07f4, &error_abort);
> +        memory_region_add_subregion(&s->mmio, 0x0800 * i, &s->port[i].txbuf);
>          memory_region_init_io(&s->port[i].txio, OBJECT(dev),
>                                &eth_porttx_ops, s,
>                                i ? "ethlite.tx[1]io" : "ethlite.tx[0]io",
> @@ -370,6 +329,11 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
>          memory_region_add_subregion(&s->mmio, i ? A_TX_BASE1 : A_TX_BASE0,
>                                      &s->port[i].txio);
>  
> +        memory_region_init_ram(&s->port[i].rxbuf, OBJECT(dev),
> +                               i ? "ethlite.rx[1]buf" : "ethlite.rx[0]buf",
> +                               0x07f4, &error_abort);
> +        memory_region_add_subregion(&s->mmio, 0x1000 + 0x0800 * i,
> +                                    &s->port[i].rxbuf);
>          memory_region_init_io(&s->port[i].rxio, OBJECT(dev),
>                                &eth_portrx_ops, s,
>                                i ? "ethlite.rx[1]io" : "ethlite.rx[0]io",
> @@ -390,9 +354,6 @@ static void xilinx_ethlite_init(Object *obj)
>      XlnxXpsEthLite *s = XILINX_ETHLITE(obj);
>  
>      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
> -
> -    memory_region_init_io(&s->mmio, obj, &eth_ops, s,
> -                          "xlnx.xps-ethernetlite", R_MAX * 4);
>      sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
>  }
>  
> -- 
> 2.45.2
> 

Re: [PATCH 20/20] hw/net/xilinx_ethlite: Rename 'mmio' MR as 'container'

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:44PM +0100, Philippe Mathieu-Daudé wrote:
> Having all its address range mapped by subregions,
> s->mmio MemoryRegion effectively became a container.
> Rename it as 'container' for clarity.

Reviewed-by: Edgar E. Iglesias <edgar.iglesias@amd.com>


> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  hw/net/xilinx_ethlite.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index da453465ca..c65001cf46 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -93,7 +93,7 @@ struct XlnxXpsEthLite
>  {
>      SysBusDevice parent_obj;
>  
> -    MemoryRegion mmio;
> +    MemoryRegion container;
>      qemu_irq irq;
>      NICState *nic;
>      NICConf conf;
> @@ -306,7 +306,7 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
>  {
>      XlnxXpsEthLite *s = XILINX_ETHLITE(dev);
>  
> -    memory_region_init(&s->mmio, OBJECT(dev),
> +    memory_region_init(&s->container, OBJECT(dev),
>                         "xlnx.xps-ethernetlite", 0x2000);
>  
>      object_initialize_child(OBJECT(dev), "ethlite.mdio", &s->mdio,
> @@ -314,31 +314,31 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
>      qdev_prop_set_string(DEVICE(&s->mdio), "name", "ethlite.mdio");
>      qdev_prop_set_uint64(DEVICE(&s->mdio), "size", 4 * 4);
>      sysbus_realize(SYS_BUS_DEVICE(&s->mdio), &error_fatal);
> -    memory_region_add_subregion(&s->mmio, A_MDIO_BASE,
> +    memory_region_add_subregion(&s->container, A_MDIO_BASE,
>                              sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mdio), 0));
>  
>      for (unsigned i = 0; i < 2; i++) {
>          memory_region_init_ram(&s->port[i].txbuf, OBJECT(dev),
>                                 i ? "ethlite.tx[1]buf" : "ethlite.tx[0]buf",
>                                 0x07f4, &error_abort);
> -        memory_region_add_subregion(&s->mmio, 0x0800 * i, &s->port[i].txbuf);
> +        memory_region_add_subregion(&s->container, 0x0800 * i, &s->port[i].txbuf);
>          memory_region_init_io(&s->port[i].txio, OBJECT(dev),
>                                &eth_porttx_ops, s,
>                                i ? "ethlite.tx[1]io" : "ethlite.tx[0]io",
>                                4 * TX_MAX);
> -        memory_region_add_subregion(&s->mmio, i ? A_TX_BASE1 : A_TX_BASE0,
> +        memory_region_add_subregion(&s->container, i ? A_TX_BASE1 : A_TX_BASE0,
>                                      &s->port[i].txio);
>  
>          memory_region_init_ram(&s->port[i].rxbuf, OBJECT(dev),
>                                 i ? "ethlite.rx[1]buf" : "ethlite.rx[0]buf",
>                                 0x07f4, &error_abort);
> -        memory_region_add_subregion(&s->mmio, 0x1000 + 0x0800 * i,
> +        memory_region_add_subregion(&s->container, 0x1000 + 0x0800 * i,
>                                      &s->port[i].rxbuf);
>          memory_region_init_io(&s->port[i].rxio, OBJECT(dev),
>                                &eth_portrx_ops, s,
>                                i ? "ethlite.rx[1]io" : "ethlite.rx[0]io",
>                                4 * RX_MAX);
> -        memory_region_add_subregion(&s->mmio, i ? A_RX_BASE1 : A_RX_BASE0,
> +        memory_region_add_subregion(&s->container, i ? A_RX_BASE1 : A_RX_BASE0,
>                                      &s->port[i].rxio);
>      }
>  
> @@ -354,7 +354,7 @@ static void xilinx_ethlite_init(Object *obj)
>      XlnxXpsEthLite *s = XILINX_ETHLITE(obj);
>  
>      sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
> -    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
> +    sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->container);
>  }
>  
>  static Property xilinx_ethlite_properties[] = {
> -- 
> 2.45.2
> 

Re: [PATCH 00/20] hw/net/xilinx_ethlite: Map RAM buffers as RAM and remove tswap() calls

[Edgar E. Iglesias]

On Tue, Nov 12, 2024 at 07:10:24PM +0100, Philippe Mathieu-Daudé wrote:
> This is the result of a long discussion with Edgar (started few
> years ago!) and Paolo:
> https://lore.kernel.org/qemu-devel/34f6fe2f-06e0-4e2a-a361-2d662f6814b5@redhat.com/
> After clarification from Richard on MMIO/RAM accesses, I figured
> strengthening the model regions would make things obvious,
> eventually allowing to remove the tswap() calls for good.
> 
> This costly series mostly plays around with MemoryRegions.
> 
> The model has a mix of RAM/MMIO in its address range. Currently
> they are implemented as a MMIO array of u32. Since the core
> memory layer swaps accesses for MMIO, the device implementation
> has to swap them back.
> In order to avoid that, we'll map the RAM regions as RAM MRs.
> First we move each MMIO register to new MMIO regions (RX and TX).
> Then what is left are the RAM buffers; we convert them to RAM MRs,
> removing the need for tswap() at all.
> 
> Once reviewed, I'll respin my "hw/microblaze: Allow running
> cross-endian vCPUs" series based on this.


Thanks Phil,

This looks good to me. Have you tested this with the Images I provied
a while back or some other way?

Cheers,
Edgar




> 
> Please review,
> 
> Phil.
> 
> Philippe Mathieu-Daudé (20):
>   hw/microblaze: Restrict MemoryRegionOps are implemented as 32-bit
>   hw/net/xilinx_ethlite: Convert some debug logs to trace events
>   hw/net/xilinx_ethlite: Remove unuseful debug logs
>   hw/net/xilinx_ethlite: Update QOM style
>   hw/net/xilinx_ethlite: Correct maximum RX buffer size
>   hw/net/xilinx_ethlite: Map MDIO registers (as unimplemented)
>   hw/net/xilinx_ethlite: Rename rxbuf -> port_index
>   hw/net/xilinx_ethlite: Add addr_to_port_index() helper
>   hw/net/xilinx_ethlite: Introduce txbuf_ptr() helper
>   hw/net/xilinx_ethlite: Introduce rxbuf_ptr() helper
>   hw/net/xilinx_ethlite: Access RX_CTRL register for each port
>   hw/net/xilinx_ethlite: Access TX_GIE register for each port
>   hw/net/xilinx_ethlite: Access TX_LEN register for each port
>   hw/net/xilinx_ethlite: Access TX_CTRL register for each port
>   hw/net/xilinx_ethlite: Map RX_CTRL as MMIO
>   hw/net/xilinx_ethlite: Map TX_LEN as MMIO
>   hw/net/xilinx_ethlite: Map TX_GIE as MMIO
>   hw/net/xilinx_ethlite: Map TX_CTRL as MMIO
>   hw/net/xilinx_ethlite: Map the RAM buffer as RAM memory region
>   hw/net/xilinx_ethlite: Rename 'mmio' MR as 'container'
> 
>  hw/char/xilinx_uartlite.c |   4 +
>  hw/intc/xilinx_intc.c     |   4 +
>  hw/net/xilinx_ethlite.c   | 357 ++++++++++++++++++++++++--------------
>  hw/timer/xilinx_timer.c   |   4 +
>  hw/net/trace-events       |   4 +
>  5 files changed, 246 insertions(+), 127 deletions(-)
> 
> -- 
> 2.45.2
> 

Re: [PATCH 19/20] hw/net/xilinx_ethlite: Map the RAM buffer as RAM memory region

[Paolo Bonzini]

On 11/12/24 19:10, Philippe Mathieu-Daudé wrote:
> Rather than using I/O registers for RAM buffer, having to
> swap endianness back and forth (because the core memory layer
> automatically swaps endiannes for us), declare the buffers
> as RAM regions. Remove the now unused s->regs[] array.
> 
> The memory flat view becomes:
> 
>    FlatView #0
>     Root memory region: system
>      0000000081000000-00000000810007f3 (prio 0, ram): ethlite.tx[0]buf
>      00000000810007f4-00000000810007ff (prio 0, i/o): ethlite.tx[0]io
>      0000000081000800-0000000081000ff3 (prio 0, ram): ethlite.tx[1]buf
>      0000000081000ff4-0000000081000fff (prio 0, i/o): ethlite.tx[1]io
>      0000000081001000-00000000810017f3 (prio 0, ram): ethlite.rx[0]buf
>      00000000810017fc-00000000810017ff (prio 0, i/o): ethlite.rx[0]io
>      0000000081001800-0000000081001ff3 (prio 0, ram): ethlite.rx[1]buf
>      0000000081001ffc-0000000081001fff (prio 0, i/o): ethlite.rx[1]io

The receive buffers should end at 7fb and ffb; no need to repost of course.

Paolo

> 
> Mention the device datasheet in the file header.
> 
> Reported-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   hw/net/xilinx_ethlite.c | 79 +++++++++++------------------------------
>   1 file changed, 20 insertions(+), 59 deletions(-)
> 
> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
> index f681b91769..da453465ca 100644
> --- a/hw/net/xilinx_ethlite.c
> +++ b/hw/net/xilinx_ethlite.c
> @@ -2,6 +2,10 @@
>    * QEMU model of the Xilinx Ethernet Lite MAC.
>    *
>    * Copyright (c) 2009 Edgar E. Iglesias.
> + * Copyright (c) 2024 Linaro, Ltd
> + *
> + * DS580: https://docs.amd.com/v/u/en-US/xps_ethernetlite
> + * LogiCORE IP XPS Ethernet Lite Media Access Controller
>    *
>    * Permission is hereby granted, free of charge, to any person obtaining a copy
>    * of this software and associated documentation files (the "Software"), to deal
> @@ -27,7 +31,6 @@
>   #include "qemu/bitops.h"
>   #include "qom/object.h"
>   #include "qapi/error.h"
> -#include "exec/tswap.h"
>   #include "hw/sysbus.h"
>   #include "hw/irq.h"
>   #include "hw/qdev-properties.h"
> @@ -46,7 +49,6 @@
>   #define A_RX_BASE0    0x17fc
>   #define R_RX_BUF1     (0x1800 / 4)
>   #define A_RX_BASE1    0x1ffc
> -#define R_MAX         (0x2000 / 4)
>   
>   #define RX_BUFSZ_MAX  0x07e0
>   
> @@ -72,6 +74,8 @@ typedef struct XlnxXpsEthLitePort
>   {
>       MemoryRegion txio;
>       MemoryRegion rxio;
> +    MemoryRegion txbuf;
> +    MemoryRegion rxbuf;
>   
>       struct {
>           uint32_t tx_len;
> @@ -100,7 +104,6 @@ struct XlnxXpsEthLite
>   
>       UnimplementedDeviceState mdio;
>       XlnxXpsEthLitePort port[2];
> -    uint32_t regs[R_MAX];
>   };
>   
>   static inline void eth_pulse_irq(XlnxXpsEthLite *s)
> @@ -118,16 +121,12 @@ static unsigned addr_to_port_index(hwaddr addr)
>   
>   static void *txbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
>   {
> -    unsigned int rxbase = port_index * (0x800 / 4);
> -
> -    return &s->regs[rxbase + R_TX_BUF0];
> +    return memory_region_get_ram_ptr(&s->port[port_index].txbuf);
>   }
>   
>   static void *rxbuf_ptr(XlnxXpsEthLite *s, unsigned port_index)
>   {
> -    unsigned int rxbase = port_index * (0x800 / 4);
> -
> -    return &s->regs[rxbase + R_RX_BUF0];
> +    return memory_region_get_ram_ptr(&s->port[port_index].rxbuf);
>   }
>   
>   static uint64_t port_tx_read(void *opaque, hwaddr addr, unsigned int size)
> @@ -252,53 +251,6 @@ static const MemoryRegionOps eth_portrx_ops = {
>           },
>   };
>   
> -static uint64_t
> -eth_read(void *opaque, hwaddr addr, unsigned int size)
> -{
> -    XlnxXpsEthLite *s = opaque;
> -    uint32_t r = 0;
> -
> -    addr >>= 2;
> -
> -    switch (addr)
> -    {
> -        default:
> -            r = tswap32(s->regs[addr]);
> -            break;
> -    }
> -    return r;
> -}
> -
> -static void
> -eth_write(void *opaque, hwaddr addr,
> -          uint64_t val64, unsigned int size)
> -{
> -    XlnxXpsEthLite *s = opaque;
> -    uint32_t value = val64;
> -
> -    addr >>= 2;
> -    switch (addr)
> -    {
> -        default:
> -            s->regs[addr] = tswap32(value);
> -            break;
> -    }
> -}
> -
> -static const MemoryRegionOps eth_ops = {
> -    .read = eth_read,
> -    .write = eth_write,
> -    .endianness = DEVICE_NATIVE_ENDIAN,
> -    .impl = {
> -        .min_access_size = 4,
> -        .max_access_size = 4,
> -    },
> -    .valid = {
> -        .min_access_size = 4,
> -        .max_access_size = 4
> -    }
> -};
> -
>   static bool eth_can_rx(NetClientState *nc)
>   {
>       XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
> @@ -354,6 +306,9 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
>   {
>       XlnxXpsEthLite *s = XILINX_ETHLITE(dev);
>   
> +    memory_region_init(&s->mmio, OBJECT(dev),
> +                       "xlnx.xps-ethernetlite", 0x2000);
> +
>       object_initialize_child(OBJECT(dev), "ethlite.mdio", &s->mdio,
>                              TYPE_UNIMPLEMENTED_DEVICE);
>       qdev_prop_set_string(DEVICE(&s->mdio), "name", "ethlite.mdio");
> @@ -363,6 +318,10 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
>                               sysbus_mmio_get_region(SYS_BUS_DEVICE(&s->mdio), 0));
>   
>       for (unsigned i = 0; i < 2; i++) {
> +        memory_region_init_ram(&s->port[i].txbuf, OBJECT(dev),
> +                               i ? "ethlite.tx[1]buf" : "ethlite.tx[0]buf",
> +                               0x07f4, &error_abort);
> +        memory_region_add_subregion(&s->mmio, 0x0800 * i, &s->port[i].txbuf);
>           memory_region_init_io(&s->port[i].txio, OBJECT(dev),
>                                 &eth_porttx_ops, s,
>                                 i ? "ethlite.tx[1]io" : "ethlite.tx[0]io",
> @@ -370,6 +329,11 @@ static void xilinx_ethlite_realize(DeviceState *dev, Error **errp)
>           memory_region_add_subregion(&s->mmio, i ? A_TX_BASE1 : A_TX_BASE0,
>                                       &s->port[i].txio);
>   
> +        memory_region_init_ram(&s->port[i].rxbuf, OBJECT(dev),
> +                               i ? "ethlite.rx[1]buf" : "ethlite.rx[0]buf",
> +                               0x07f4, &error_abort);
> +        memory_region_add_subregion(&s->mmio, 0x1000 + 0x0800 * i,
> +                                    &s->port[i].rxbuf);
>           memory_region_init_io(&s->port[i].rxio, OBJECT(dev),
>                                 &eth_portrx_ops, s,
>                                 i ? "ethlite.rx[1]io" : "ethlite.rx[0]io",
> @@ -390,9 +354,6 @@ static void xilinx_ethlite_init(Object *obj)
>       XlnxXpsEthLite *s = XILINX_ETHLITE(obj);
>   
>       sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
> -
> -    memory_region_init_io(&s->mmio, obj, &eth_ops, s,
> -                          "xlnx.xps-ethernetlite", R_MAX * 4);
>       sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
>   }
>   

Re: [PATCH 19/20] hw/net/xilinx_ethlite: Map the RAM buffer as RAM memory region

[Philippe Mathieu-Daudé]

On 13/11/24 18:21, Paolo Bonzini wrote:
> On 11/12/24 19:10, Philippe Mathieu-Daudé wrote:
>> Rather than using I/O registers for RAM buffer, having to
>> swap endianness back and forth (because the core memory layer
>> automatically swaps endiannes for us), declare the buffers
>> as RAM regions. Remove the now unused s->regs[] array.
>>
>> The memory flat view becomes:
>>
>>    FlatView #0
>>     Root memory region: system
>>      0000000081000000-00000000810007f3 (prio 0, ram): ethlite.tx[0]buf
>>      00000000810007f4-00000000810007ff (prio 0, i/o): ethlite.tx[0]io
>>      0000000081000800-0000000081000ff3 (prio 0, ram): ethlite.tx[1]buf
>>      0000000081000ff4-0000000081000fff (prio 0, i/o): ethlite.tx[1]io
>>      0000000081001000-00000000810017f3 (prio 0, ram): ethlite.rx[0]buf
>>      00000000810017fc-00000000810017ff (prio 0, i/o): ethlite.rx[0]io
>>      0000000081001800-0000000081001ff3 (prio 0, ram): ethlite.rx[1]buf
>>      0000000081001ffc-0000000081001fff (prio 0, i/o): ethlite.rx[1]io
> 
> The receive buffers should end at 7fb and ffb; no need to repost of course.

Nice catch. Actually, looking at the datasheet p. 20, Table 11 "XPS
Ethernet Lite MAC Memory Map" we have

0x0000 - 0x07E0  TxPingBuf
0x07E4 - 0x07F0  MDIO if C_INCLUDE_MDIO else Reserved
0x07F4 - 0x07FC  TxPingIO
0x0800 - 0x0FE0  TxPongBuf
0x0FE4 - 0x0FF0  Reserved
0x0FF4 - 0x0FFC  TxPongIO
0x1000 - 0x17E0  RxPingBuf
0x17E4 - 0x17F8  Reserved
0x17FC - 0x17FC  RxPingIO
0x1800 - 0x1FE0  RxPongBuf
0x1FE4 - 0x1FF8  Reserved
0x1FFC - 0x1FFC  RxPongIO

I'll update appropriately.

Thanks,

Phil.

Re: [PATCH 00/20] hw/net/xilinx_ethlite: Map RAM buffers as RAM and remove tswap() calls

[Philippe Mathieu-Daudé]

On 13/11/24 15:36, Edgar E. Iglesias wrote:
> On Tue, Nov 12, 2024 at 07:10:24PM +0100, Philippe Mathieu-Daudé wrote:
>> This is the result of a long discussion with Edgar (started few
>> years ago!) and Paolo:
>> https://lore.kernel.org/qemu-devel/34f6fe2f-06e0-4e2a-a361-2d662f6814b5@redhat.com/
>> After clarification from Richard on MMIO/RAM accesses, I figured
>> strengthening the model regions would make things obvious,
>> eventually allowing to remove the tswap() calls for good.
>>
>> This costly series mostly plays around with MemoryRegions.
>>
>> The model has a mix of RAM/MMIO in its address range. Currently
>> they are implemented as a MMIO array of u32. Since the core
>> memory layer swaps accesses for MMIO, the device implementation
>> has to swap them back.
>> In order to avoid that, we'll map the RAM regions as RAM MRs.
>> First we move each MMIO register to new MMIO regions (RX and TX).
>> Then what is left are the RAM buffers; we convert them to RAM MRs,
>> removing the need for tswap() at all.
>>
>> Once reviewed, I'll respin my "hw/microblaze: Allow running
>> cross-endian vCPUs" series based on this.
> 
> 
> Thanks Phil,
> 
> This looks good to me. Have you tested this with the Images I provied
> a while back or some other way?

I'm running the same functional tests run on CI:

$ make check-functional-microblaze{,el}
[1/7] Generating qemu-version.h with a custom command (wrapped by meson 
to capture output)
[1/7] Generating qemu-version.h with a custom command (wrapped by meson 
to capture output)
/Users/philmd/qemu/build/pyvenv/bin/meson test  --no-rebuild -t 1 
--setup thorough   --print-errorlogs  --suite func-microblazeel  --suite 
func-microblazeel-thorough
/Users/philmd/qemu/build/pyvenv/bin/meson test  --no-rebuild -t 1 
--setup thorough   --print-errorlogs  --suite func-microblaze  --suite 
func-microblaze-thorough
1/4 qemu:func-quick+func-microblazeel / 
func-microblazeel-empty_cpu_model                                     OK 
              0.18s   1 subtests passed
1/4 qemu:func-quick+func-microblaze / func-microblaze-empty_cpu_model 
                                OK              0.18s   1 subtests passed
2/4 qemu:func-quick+func-microblaze / func-microblaze-version 
                                OK              0.18s   1 subtests passed
2/4 qemu:func-quick+func-microblazeel / func-microblazeel-version 
                                      OK              0.18s   1 subtests 
passed
3/4 qemu:func-quick+func-microblaze / func-microblaze-info_usernet 
                                OK              0.28s   1 subtests passed
3/4 qemu:func-quick+func-microblazeel / func-microblazeel-info_usernet 
                                      OK              0.28s   1 subtests 
passed
4/4 qemu:func-thorough+func-microblaze-thorough+thorough / 
func-microblaze-microblaze_s3adsp1800        OK              0.57s   1 
subtests passed

Ok:                 4
Expected Fail:      0
Fail:               0
Unexpected Pass:    0
Skipped:            0
Timeout:            0

Full log written to /Users/philmd/qemu/build/meson-logs/testlog-thorough.txt
4/4 qemu:func-thorough+func-microblazeel-thorough+thorough / 
func-microblazeel-microblazeel_s3adsp1800        OK              1.50s 
1 subtests passed

Ok:                 4
Expected Fail:      0
Fail:               0
Unexpected Pass:    0
Skipped:            0
Timeout:            0

Full log written to /Users/philmd/qemu/build/meson-logs/testlog-thorough.txt
$

Re: [PATCH 08/20] hw/net/xilinx_ethlite: Add addr_to_port_index() helper

[Philippe Mathieu-Daudé]

On 13/11/24 15:23, Edgar E. Iglesias wrote:
> On Tue, Nov 12, 2024 at 07:10:32PM +0100, Philippe Mathieu-Daudé wrote:
>> For a particular physical address within the EthLite MMIO range,
>> addr_to_port_index() returns which port is accessed.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   hw/net/xilinx_ethlite.c | 10 +++++++++-
>>   1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
>> index 20919b4f54..fe91891310 100644
>> --- a/hw/net/xilinx_ethlite.c
>> +++ b/hw/net/xilinx_ethlite.c
>> @@ -24,6 +24,7 @@
>>   
>>   #include "qemu/osdep.h"
>>   #include "qemu/module.h"
>> +#include "qemu/bitops.h"
>>   #include "qom/object.h"
>>   #include "qapi/error.h"
>>   #include "exec/tswap.h"
>> @@ -86,6 +87,12 @@ static inline void eth_pulse_irq(XlnxXpsEthLite *s)
>>       }
>>   }
>>   
>> +__attribute__((unused))
>> +static unsigned addr_to_port_index(hwaddr addr)
>> +{
>> +    return extract64(addr, 11, 1);
>> +}
>> +
> 
> Shouldn't you add addr_to_port_index in the following patch and avoid
> the temporary unused attribute?

OK.

> 
>>   static uint64_t
>>   eth_read(void *opaque, hwaddr addr, unsigned int size)
>>   {
>> @@ -190,7 +197,8 @@ static bool eth_can_rx(NetClientState *nc)
>>   static ssize_t eth_rx(NetClientState *nc, const uint8_t *buf, size_t size)
>>   {
>>       XlnxXpsEthLite *s = qemu_get_nic_opaque(nc);
>> -    unsigned int rxbase = s->port_index * (0x800 / 4);
>> +    unsigned int port_index = s->port_index;
>> +    unsigned int rxbase = port_index * (0x800 / 4);
> 
> 
> Hmm, AFAICT s->port_index is an unsigned int, what is the purpose of this change?

Likely a rebase mistake, this belongs to the next patch indeed.