Re: [PATCH 12/19] hw/net/xilinx_ethlite: Only expect big-endian accesses
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