On 7/26/19 2:39 AM, Paolo Bonzini wrote: > 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;
Possibly outside the scope of this patch set, I'd like to replace enum device_endian with MemOp, with exactly the above enumerator replacement. In the meantime, perhaps a conversion function static MemOp devendian_memop(enum device_endian d) { switch (d) { case DEVICE_NATIVE_ENDIAN: return MO_TE; case DEVICE_BIG_ENDIAN: return MO_BE; case DEVICE_LITTLE_ENDIAN: return MO_LE; default: g_assert_not_reached(); } } With that, this would simplify to return (op & MO_BSWAP) != devendian_memop(mr->ops->endianness); > 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. Agreed. r~