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.