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@linaro.org>
---
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
On Tue, Nov 05, 2024 at 02:04:24PM +0100, 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.
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@linaro.org>
> ---
> 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
>
On 11/5/24 14: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.
I don't understand. This machine type is little-endian only and
expecting inverted accesses, isn't it? Therefore, all that you need is
> - .endianness = DEVICE_NATIVE_ENDIAN,
> + .endianness = DEVICE_BIG_ENDIAN,
And removing the tswap altogether. The big-endian petalogix board will
start getting "correct" values (not swapped anymore). That's a feature,
not a bug.
Paolo
On 5/11/24 14:18, Paolo Bonzini wrote:
> On 11/5/24 14: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.
>
> I don't understand. This machine type is little-endian only and
> expecting inverted accesses, isn't it? Therefore, all that you need is
>
>> - .endianness = DEVICE_NATIVE_ENDIAN,
>> + .endianness = DEVICE_BIG_ENDIAN,
>
> And removing the tswap altogether. The big-endian petalogix board will
> start getting "correct" values (not swapped anymore). That's a feature,
> not a bug.
The feature is memory.c swapping MemoryRegionOps depending on the
*qemu-system binary* target endianness.
We assumed most guest vCPUs run with the same endianness of the binary.
Now we want to swap wrt the vCPU, not the binary. So indeed this patch
effectively undo the memory.c swapping (feature).
I suppose the better way is to modify memory.c, possibly passing MemOp
all over. For HW accel where vCPU endianness is forced to host one,
this would become a no-op. Lot of rework in perspective.
On 11/6/24 00:29, Philippe Mathieu-Daudé wrote: > We assumed most guest vCPUs run with the same endianness of the binary. > > Now we want to swap wrt the vCPU, not the binary. So indeed this patch > effectively undo the memory.c swapping (feature). > > I suppose the better way is to modify memory.c, possibly passing MemOp > all over. For HW accel where vCPU endianness is forced to host one, > this would become a no-op. Lot of rework in perspective. It should be much easier than that. First of all, this is when memory.c swaps according to DEVICE_*_ENDIAN: guest \ host little-endian big-endian little-endian BIG LITTLE, NATIVE big-endian BIG, NATIVE LITTLE tswap swaps in the two cases marked "NATIVE" (same as DEVICE_NATIVE_ENDIAN). ldl_le_p swaps in the two cases marked "LITTLE" (same as DEVICE_LITTLE_ENDIAN). ldl_be_p swaps in the two cases marked "BIG" (same as DEVICE_BIG_ENDIAN). First of all, current code does different things for RAM vs. the other registers. After your patch it's the same, which seems fishy. Anyway let's focus on RAM for now. Current code (unconditional tswap + DEVICE_NATIVE_ENDIAN) always performs an even number of swaps: guest \ host little-endian big-endian little-endian none tswap+memory.c big-endian tswap+memory.c none That's what Edgar says - it's just RAM. So with your patch the behavior becomes: guest \ host little-endian big-endian little-endian bswap+memory.c bswap big-endian memory.c none Behavior changes in the cross-endianness case. LE-on-LE remains the same. It seems to break BE hosts since petalogix is a qemu-system-microblazeel board. If this reasoning is correct, together with DEVICE_BIG_ENDIAN you need cpu_to_be32 instead of tswap: guest \ host little-endian big-endian little-endian cpu_to_be32+memory.c none big-endian cpu_to_be32+memory.c none However, things are different for the R_RX* and R_TX* cases. Before: guest \ host little-endian big-endian little-endian none memory.c big-endian memory.c none Your patch here keeps the same evenness of swaps, even if who swaps changes: guest \ host little-endian big-endian little-endian bswap+memory.c bswap big-endian memory.c none Is this just a change in migration format for the RAM ara? Then I guess your patch works, though I'd prefer Richard's suggestion of flipping the endianness in the MemoryRegionOps. However, since you said the board is LE-only, maybe the following also works and seems simpler? 1) use DEVICE_LITTLE_ENDIAN (i.e. always the same perspective as qemu-microblazeel) 2) use cpu_to_le32 for RAM and nothing for the other registers But again, maybe I'm completely wrong. Paolo
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@linaro.org>
> ---
> 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~
On 5/11/24 13:30, Richard Henderson wrote:
> 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@linaro.org>
>> ---
>> 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.
Squashing this on top to have the two tables, but we still need to
swap back the effect of memory.c swapping for the binary, so I don't
think this is what you want:
-- >8 --
diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c
index 44ef11ebf89..d58757eb919 100644
--- a/hw/net/xilinx_ethlite.c
+++ b/hw/net/xilinx_ethlite.c
@@ -67,6 +67,7 @@ struct xlx_ethlite
NICState *nic;
NICConf conf;
+ bool model_little_endian;
bool access_little_endian;
uint32_t c_tx_pingpong;
uint32_t c_rx_pingpong;
@@ -109,7 +110,7 @@ eth_read(void *opaque, hwaddr addr, unsigned int size)
r = s->regs[addr];
break;
}
- if (s->access_little_endian) {
+ if (s->access_little_endian ^ s->model_little_endian) {
bswap32s(&r);
}
return r;
@@ -123,7 +124,7 @@ eth_write(void *opaque, hwaddr addr,
unsigned int base = 0;
uint32_t value = val64;
- if (s->access_little_endian) {
+ if (s->access_little_endian ^ s->model_little_endian) {
bswap32s(&value);
}
@@ -176,7 +177,7 @@ eth_write(void *opaque, hwaddr addr,
}
}
-static const MemoryRegionOps eth_ops = {
+static const MemoryRegionOps eth_ops_be = {
.read = eth_read,
.write = eth_write,
.endianness = DEVICE_BIG_ENDIAN,
@@ -190,6 +191,20 @@ static const MemoryRegionOps eth_ops = {
},
};
+static const MemoryRegionOps eth_ops_le = {
+ .read = eth_read,
+ .write = eth_write,
+ .endianness = DEVICE_LITTLE_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)
{
struct xlx_ethlite *s = qemu_get_nic_opaque(nc);
@@ -247,6 +262,11 @@ static void xilinx_ethlite_realize(DeviceState
*dev, Error **errp)
{
struct xlx_ethlite *s = XILINX_ETHLITE(dev);
+ memory_region_init_io(&s->mmio, OBJECT(dev),
+ s->model_little_endian ? ð_ops_le :
ð_ops_be, s,
+ "xlnx.xps-ethernetlite", R_MAX * 4);
+ sysbus_init_mmio(SYS_BUS_DEVICE(dev), &s->mmio);
+
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,
@@ -259,15 +279,13 @@ static void xilinx_ethlite_init(Object *obj)
struct xlx_ethlite *s = XILINX_ETHLITE(obj);
sysbus_init_irq(SYS_BUS_DEVICE(obj), &s->irq);
-
- memory_region_init_io(&s->mmio, obj, ð_ops, s,
- "xlnx.xps-ethernetlite", R_MAX * 4);
- sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->mmio);
}
static Property xilinx_ethlite_properties[] = {
DEFINE_PROP_BOOL("access-little-endian", struct xlx_ethlite,
access_little_endian, false),
+ DEFINE_PROP_BOOL("model-little-endian", struct xlx_ethlite,
+ model_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),
---
© 2016 - 2024 Red Hat, Inc.