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

On Tue, Nov 05, 2024 at 02:04:24PM +0100, Philippe Mathieu-Daudé wrote: I think you're partially correct but not fully. This buffer area is really a RAM and has no endianess. Problem is back then I don't think I was a ware of a way to map RAM memory sub regions so we hacked in byteswaps to swap from host (which usually was little endian) to big endian. This is because register accesses from CPU to device model are kept in host endianess. I think the right way to solve this issue is to map a RAM memory region to represent the BRAM. Cheers, Edgar > > Signed-off-by: Philippe Mathieu-Daudé <philmd(a)linaro.org&gt; > --- > 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), > 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), > -- > 2.45.2 >