[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-riscv] [Qemu-devel] [PATCH v5 11/15] memory: Single byte swap
From: |
Richard Henderson |
Subject: |
Re: [Qemu-riscv] [Qemu-devel] [PATCH v5 11/15] memory: Single byte swap along the I/O path |
Date: |
Fri, 26 Jul 2019 07:45:10 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
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~
- Re: [Qemu-riscv] [Qemu-devel] [PATCH v5 09/15] cputlb: Access MemoryRegion with MemOp, (continued)
- [Qemu-riscv] [Qemu-devel] [PATCH v5 10/15] memory: Access MemoryRegion with MemOp semantics, tony.nguyen, 2019/07/26
- [Qemu-riscv] [Qemu-devel] [PATCH v5 11/15] memory: Single byte swap along the I/O path, tony.nguyen, 2019/07/26
- [Qemu-riscv] [Qemu-devel] [PATCH v5 12/15] cpu: TLB_FLAGS_MASK bit to force memory slow path, tony.nguyen, 2019/07/26
- [Qemu-riscv] [Qemu-devel] [PATCH v5 13/15] cputlb: Byte swap memory transaction attribute, tony.nguyen, 2019/07/26
- [Qemu-riscv] [Qemu-devel] [PATCH v5 14/15] target/sparc: Add TLB entry with attributes, tony.nguyen, 2019/07/26
- [Qemu-riscv] [Qemu-devel] [PATCH v5 15/15] target/sparc: sun4u Invert Endian TTE bit, tony.nguyen, 2019/07/26