[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] [PATCH v2 11/20] hw/virtio: Access MemoryRegion with MemOp
From: |
tony.nguyen |
Subject: |
[Qemu-devel] [PATCH v2 11/20] hw/virtio: Access MemoryRegion with MemOp |
Date: |
Mon, 22 Jul 2019 15:48:00 +0000 |
On 17/07/19 08:06, Paolo Bonzini wrote:
> My main concern is that MO_BE/MO_LE/MO_TE do not really apply to the
> memory.c paths. MO_BSWAP is never passed into the MemOp, even if target
> endianness != host endianness.
>
> Therefore, you could return MO_TE | MO_{8,16,32,64} from this function,
> and change memory_region_endianness_inverted to test
> HOST_WORDS_BIGENDIAN instead of TARGET_WORDS_BIGENDIAN. Then the two
> MO_BSWAPs (one from MO_TE, one from adjust_endianness because
> memory_region_endianness_inverted returns true) cancel out if the
> memory region's endianness is the same as the host's but different
> from the target's.
>
> Some care is needed in virtio_address_space_write and zpci_write_bar. I
> think the latter is okay, while the former could do something like this:
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index ce928f2429..61885f020c 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -541,16 +541,16 @@ void virtio_address_space_write(VirtIOPCIProxy *proxy,
> hwaddr addr,
> val = pci_get_byte(buf);
> break;
> case 2:
> - val = cpu_to_le16(pci_get_word(buf));
> + val = pci_get_word(buf);
> break;
> case 4:
> - val = cpu_to_le32(pci_get_long(buf));
> + val = pci_get_long(buf);
> break;
> default:
> /* As length is under guest control, handle illegal values. */
> return;
> }
> - memory_region_dispatch_write(mr, addr, val, len, MEMTXATTRS_UNSPECIFIED);
> + memory_region_dispatch_write(mr, addr, val, size_memop(len) & ~MO_BSWAP,
> MEMTXATTRS_UNSPECIFIED);
> }
>
> static void
Sorry Paolo, I noted the need to take care in virtio_address_space_write and
zpci_write_bar but did not understand.
> Some care is needed in virtio_address_space_write and zpci_write_bar.
Is this advice for my v1 implementation, or in the case of the
MO_TE | MO_{8,16,32,64} idead suggested in the paragraph before?
Signed-off-by: Tony Nguyen <address@hidden>
---
hw/virtio/virtio-pci.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ce928f2..265f066 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -17,6 +17,7 @@
#include "qemu/osdep.h"
+#include "exec/memop.h"
#include "standard-headers/linux/virtio_pci.h"
#include "hw/virtio/virtio.h"
#include "hw/pci/pci.h"
@@ -550,7 +551,8 @@ void virtio_address_space_write(VirtIOPCIProxy *proxy,
hwaddr addr,
/* As length is under guest control, handle illegal values. */
return;
}
- memory_region_dispatch_write(mr, addr, val, len, MEMTXATTRS_UNSPECIFIED);
+ memory_region_dispatch_write(mr, addr, val, SIZE_MEMOP(len),
+ MEMTXATTRS_UNSPECIFIED);
}
static void
@@ -573,7 +575,8 @@ virtio_address_space_read(VirtIOPCIProxy *proxy, hwaddr
addr,
/* Make sure caller aligned buf properly */
assert(!(((uintptr_t)buf) & (len - 1)));
- memory_region_dispatch_read(mr, addr, &val, len, MEMTXATTRS_UNSPECIFIED);
+ memory_region_dispatch_read(mr, addr, &val, SIZE_MEMOP(len),
+ MEMTXATTRS_UNSPECIFIED);
switch (len) {
case 1:
pci_set_byte(buf, val);
--
1.8.3.1
- Re: [Qemu-devel] [PATCH v2 02/20] tcg: Replace MO_16 with MO_UW alias, (continued)
- Re: [Qemu-devel] [PATCH v2 02/20] tcg: Replace MO_16 with MO_UW alias, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 02/20] tcg: Replace MO_16 with MO_UW alias, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 03/20] tcg: Replace MO_32 with MO_UL alias, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 04/20] tcg: Replace MO_64 with MO_UQ alias, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 05/20] tcg: Move size+sign+endian from TCGMemOp to MemOp, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 06/20] tcg: Rename get_memop to get_tcgmemop, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 07/20] memory: Access MemoryRegion with MemOp, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 09/20] hw/s390x: Access MemoryRegion with MemOp, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 08/20] target/mips: Access MemoryRegion with MemOp, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 10/20] hw/intc/armv7m_nic: Access MemoryRegion with MemOp, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 11/20] hw/virtio: Access MemoryRegion with MemOp,
tony.nguyen <=
- [Qemu-devel] [PATCH v2 12/20] hw/vfio: Access MemoryRegion with MemOp, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 13/20] exec: Access MemoryRegion with MemOp, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 14/20] cputlb: Access MemoryRegion with MemOp, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 15/20] memory: Access MemoryRegion with MemOp semantics, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 16/20] memory: Single byte swap along the I/O path, tony.nguyen, 2019/07/22
- [Qemu-devel] [PATCH v2 17/20] cpu: TLB_FLAGS_MASK bit to force memory slow path, tony.nguyen, 2019/07/22