Re: [RFC PATCH-for-8.0 3/3] hw/net/xilinx_ethlite: Replace tswap32() by be32_to_cpu()

[Edgar E. Iglesias]

On Tue, Dec 13, 2022 at 03:22:54PM +0000, Peter Maydell wrote:
> On Tue, 13 Dec 2022 at 14:23, Edgar E. Iglesias
> <edgar.iglesias@gmail.com> wrote:
> >
> > On Tue, Dec 13, 2022 at 02:18:42PM +0000, Peter Maydell wrote:
> > > On Tue, 13 Dec 2022 at 14:14, Edgar E. Iglesias
> > > <edgar.iglesias@gmail.com> wrote:
> > > >
> > > > On Tue, Dec 13, 2022 at 01:53:15PM +0000, Peter Maydell wrote:
> > > > > On Tue, 13 Dec 2022 at 12:52, Philippe Mathieu-Daudé 
> > > > > <philmd@linaro.org> wrote:
> > > > > >
> > > > > > This partly revert commit d48751ed4f ("xilinx-ethlite:
> > > > > > Simplify byteswapping to/from brams") which states the
> > > > > > packet data is stored in big-endian.
> 
> > > > > This is a change of behaviour for this device in the
> > > > > qemu-system-microblazeel petalogix-s3adsp1800 board, because
> > > > > previously on that system the bytes of the rx buffer would
> > > > > appear in the registers in little-endian order and now they
> > > > > will appear in big-endian order.
> > > > >
> > > > > Edgar, do you know what the real hardware does here ?
> > >
> > > > Yeah, I think these tx/rx buffers (the default case with tswap32)
> > > > should be modelled as plain RAM's (they are just RAM's on real HW).
> > > > Because we're modeling as MMIO regs, I think we get into endianness
> > > > trouble when the ethernet output logic treats the content as a blob
> > > > (thus the need for byteswaps). Does that make sense?
> > >
> > > As a concrete question: if I do a 32-bit load from the buffer
> > > register into a CPU register, do I get a different value
> > > on the BE microblaze hardware vs LE microblaze ?
> >
> > Yes, I beleive so.
> >
> > If the CPU stores the value and reads it back, you get the same. But
> > the representation on the RAM's is different between LE/BE.
> > But if the Ethernet logic writes Ethernet packet data into the buffer,
> > LE and BE MicroBlazes will read differient values from the buffers.
> > These buffer "registers" are just RAM's I beleive.
> 
> Thanks. That suggests that the current code for this device
> is correct, and we would be breaking it on the LE platform
> if we applied this patch.
> 
> I don't suppose you have a guest image for the boards which
> uses ethernet ?

Yes, I do, both little and big endian with ethlite working. Do you have a 
suggestion where to upload?

Best regards,
Edgar