On 26/07/19 08:47, tony.ngu...@bt.com wrote: > +static bool memory_region_endianness_inverted(MemoryRegion *mr) > { > #ifdef TARGET_WORDS_BIGENDIAN > return mr->ops->endianness == DEVICE_LITTLE_ENDIAN; > @@ -361,23 +361,27 @@ static bool > memory_region_wrong_endianness(MemoryRegion *mr) > #endif > } > > -static void adjust_endianness(MemoryRegion *mr, uint64_t *data, > unsigned size) > +static void adjust_endianness(MemoryRegion *mr, uint64_t *data, MemOp op) > { > - if (memory_region_wrong_endianness(mr)) { > - switch (size) { > - case 1: > + if (memory_region_endianness_inverted(mr)) { > + op ^= MO_BSWAP; > + }
Here it should not matter: the caller of memory_region_dispatch_read should includes one of MO_TE/MO_LE/MO_BE in the op (or nothing for host endianness). Then memory_region_endianness_inverted can be: if (mr->ops->endianness == DEVICE_NATIVE_ENDIAN) return (op & MO_BSWAP) != MO_TE; else if (mr->ops->endianness == DEVICE_BIG_ENDIAN) return (op & MO_BSWAP) != MO_BE; else if (mr->ops->endianness == DEVICE_LITTLE_ENDIAN) return (op & MO_BSWAP) != MO_LE; and adjust_endianness does if (memory_region_endianness_inverted(mr, op)) { switch (op & MO_SIZE) { ... } } I think the changes should be split in two parts. Before this patch, you modify all callers of memory_region_dispatch_* so that they already pass the right endianness op; however, you leave the existing swap in place. So for example in load_helper you'd have in a previous patch + /* FIXME: io_readx ignores MO_BSWAP. */ + op = SIZE_MEMOP(size) | (big_endian ? MO_BE : MO_LE); res = io_readx(env, &env_tlb(env)->d[mmu_idx].iotlb[index], - mmu_idx, addr, retaddr, access_type, SIZE_MEMOP(size)); + mmu_idx, addr, retaddr, access_type, op); return handle_bswap(res, size, big_endian); Then, in this patch, you remove the handle_bswap call as well as the FIXME comment. Paolo