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

[Paolo Bonzini <pbonzini@redhat.com>]

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