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.
