Re: [PATCH 12/19] hw/net/xilinx_ethlite: Only expect big-endian accesses

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/5/24 13:04, Philippe Mathieu-Daudé wrote:
The Xilinx 'ethlite' device was added in commit b43848a100
("xilinx: Add ethlite emulation"), being only built back
then for a big-endian MicroBlaze target (see commit 72b675caac
"microblaze: Hook into the build-system").

I/O endianness access was then clarified in commit d48751ed4f
("xilinx-ethlite: Simplify byteswapping to/from brams"). Here
the 'fix' was to use tswap32(). Since the machine was built as
big-endian target, tswap32() use means the fix was for a little
endian host. While the datasheet (reference added in file header)
is not precise about it, we interpret such change as the device
expects accesses in big-endian order. Besides, this is what other
Xilinx/MicroBlaze devices use (see the 3 previous commits).

Correct the MemoryRegionOps endianness. Add a 'access-little-endian'
property, so if the bus access expect little-endian order we swap
the values. Replace the tswap32() calls accordingly.

Set the property on the single machine using this device.

Signed-off-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
---
  hw/microblaze/petalogix_s3adsp1800_mmu.c |  1 +
  hw/net/xilinx_ethlite.c                  | 20 ++++++++++++++++----
  2 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/hw/microblaze/petalogix_s3adsp1800_mmu.c b/hw/microblaze/petalogix_s3adsp1800_mmu.c
index 8110be83715..8407dbee12a 100644
--- a/hw/microblaze/petalogix_s3adsp1800_mmu.c
+++ b/hw/microblaze/petalogix_s3adsp1800_mmu.c
@@ -123,6 +123,7 @@ petalogix_s3adsp1800_init(MachineState *machine)
      qemu_configure_nic_device(dev, true, NULL);
      qdev_prop_set_uint32(dev, "tx-ping-pong", 0);
      qdev_prop_set_uint32(dev, "rx-ping-pong", 0);
+    qdev_prop_set_bit(dev, "access-little-endian", !TARGET_BIG_ENDIAN);
      sysbus_realize_and_unref(SYS_BUS_DEVICE(dev), &error_fatal);
      sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, ETHLITE_BASEADDR);
      sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0, irq[ETHLITE_IRQ]);
diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index ede7c172748..44ef11ebf89 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -3,6 +3,9 @@
   *
   * Copyright (c) 2009 Edgar E. Iglesias.
   *
+ * 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
   * in the Software without restriction, including without limitation the rights
@@ -25,7 +28,6 @@
  #include "qemu/osdep.h"
  #include "qemu/module.h"
  #include "qom/object.h"
-#include "exec/tswap.h"
  #include "hw/sysbus.h"
  #include "hw/irq.h"
  #include "hw/qdev-properties.h"
@@ -65,6 +67,7 @@ struct xlx_ethlite
      NICState *nic;
      NICConf conf;
+ bool access_little_endian;
      uint32_t c_tx_pingpong;
      uint32_t c_rx_pingpong;
      unsigned int txbuf;
@@ -103,9 +106,12 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
              break;
default:
-            r = tswap32(s->regs[addr]);
+            r = s->regs[addr];
              break;
      }
+    if (s->access_little_endian) {
+        bswap32s(&r);
+    }
      return r;
  }
@@ -117,6 +123,10 @@ eth_write(void *opaque, hwaddr addr,
      unsigned int base = 0;
      uint32_t value = val64;
+ if (s->access_little_endian) {
+        bswap32s(&value);
+    }
+
      addr >>= 2;
      switch (addr)
      {
@@ -161,7 +171,7 @@ eth_write(void *opaque, hwaddr addr,
              break;
default:
-            s->regs[addr] = tswap32(value);
+            s->regs[addr] = value;
              break;
      }
  }
@@ -169,7 +179,7 @@ eth_write(void *opaque, hwaddr addr,
  static const MemoryRegionOps eth_ops = {
      .read = eth_read,
      .write = eth_write,
-    .endianness = DEVICE_NATIVE_ENDIAN,
+    .endianness = DEVICE_BIG_ENDIAN,
      .impl = {
          .min_access_size = 4,
          .max_access_size = 4,
@@ -256,6 +266,8 @@ static void xilinx_ethlite_init(Object *obj)
  }
static Property xilinx_ethlite_properties[] = {
+    DEFINE_PROP_BOOL("access-little-endian", struct xlx_ethlite,
+                     access_little_endian, false),

I'm not a fan of performing any swapping within device code.
The memory subsystem should do all of it.

A better solution is two tables, eth_ops_{be,le}, which differ only in the setting of .endianness. Handle the property by registering the correct MemoryRegionOps during init.


r~




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux