On 6/25/19 7:09 PM, Laurent Vivier wrote: > Le 25/06/2019 à 17:57, Philippe Mathieu-Daudé a écrit : >> On 6/24/19 10:07 PM, Laurent Vivier wrote: >>> Hi, >>> >>> Jason, Can I have an Acked-by from you (as network devices maintainer)? >> >> Hmm something seems odd here indeed... >> >> What a stable model! This file has no logical modification since its >> introduction, a65f56eeba "Implement sonic netcard (MIPS Jazz)" >> >> Here we had: >> >> static void dp8393x_writeb(void *opaque, hwaddr addr, uint32_t val) >> { >> uint16_t old_val = dp8393x_readw(opaque, addr & ~0x1); >> >> switch (addr & 3) { >> case 0: >> val = val | (old_val & 0xff00); >> break; >> case 1: >> val = (val << 8) | (old_val & 0x00ff); >> break; >> } >> dp8393x_writew(opaque, addr & ~0x1, val); >> } >> >> So we had 16-bit endian shifting there. >> >> And few lines below: >> >> /* XXX: Check byte ordering */ >> ... >> /* Calculate the ethernet checksum */ >> #ifdef SONIC_CALCULATE_RXCRC >> checksum = cpu_to_le32(crc32(0, buf, rx_len)); >> #else >> checksum = 0; >> #endif >> >> After various housekeeping, we get: >> >> 84689cbb97 "net/dp8393x: do not use old_mmio accesses" >> >> The MIPS Jazz is known to run in both endianess, but I haven't checked >> if at that time both were available. >> >> Have you tried this patch? >> >> -- >8 -- >> diff --git a/hw/net/dp8393x.c b/hw/net/dp8393x.c >> index bdb0b3b2c2..646e11206f 100644 >> @@ -651,7 +651,7 @@ static const MemoryRegionOps dp8393x_ops = { >> .write = dp8393x_write, >> .impl.min_access_size = 2, >> .impl.max_access_size = 2, >> - .endianness = DEVICE_NATIVE_ENDIAN, >> + .endianness = DEVICE_LITTLE_ENDIAN, >> }; >> --- >> >> (but then mips64-softmmu Jazz would have networking broken). >> > > I doesn't help, the endianness is a MemoryRegion property (see > memory_region_wrong_endianness()) so it is used when the CPU writes to > the device MMIO, not when the device accesses the other memory. > In this case, it reads from system_memory. Perhaps we can create the > address_space with a system_memory in big endian mode?
Ah I missed that... What about not using address_space_rw(data) but directly use address_space_lduw_le() and address_space_stw_le() instead?