From cb85f7ab045e8c05ee182b3573c9aba8e287e36b Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Mon, 8 Jul 2013 09:44:04 +0100 Subject: [PATCH 1/4] exec.c: Pass correct pointer type to qemu_ram_ptr_length Commit e3127ae0 introduced a problem where we're passing a hwaddr* to qemu_ram_ptr_length() but it wants a ram_addr_t*; this will cause problems on 32 bit hosts and in any case provokes a clang warning on MacOSX: CC arm-softmmu/exec.o exec.c:2164:46: warning: incompatible pointer types passing 'hwaddr *' (aka 'unsigned long long *') to parameter of type 'ram_addr_t *' (aka 'unsigned long *') [-Wincompatible-pointer-types] return qemu_ram_ptr_length(raddr + base, plen); ^~~~ exec.c:1392:63: note: passing argument to parameter 'size' here static void *qemu_ram_ptr_length(ram_addr_t addr, ram_addr_t *size) ^ Since this function is only used in one place, change its prototype to pass a hwaddr* rather than a ram_addr_t*, rather than contorting the calling code to get the type right. Signed-off-by: Peter Maydell Tested-by: Riku Voipio Tested-by: Peter Crosthwaite Signed-off-by: Paolo Bonzini --- exec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/exec.c b/exec.c index c99a8839c5..d312bb4f3f 100644 --- a/exec.c +++ b/exec.c @@ -1379,7 +1379,7 @@ static void *qemu_safe_ram_ptr(ram_addr_t addr) /* Return a host pointer to guest's ram. Similar to qemu_get_ram_ptr * but takes a size argument */ -static void *qemu_ram_ptr_length(ram_addr_t addr, ram_addr_t *size) +static void *qemu_ram_ptr_length(ram_addr_t addr, hwaddr *size) { if (*size == 0) { return NULL; From b4afea11aafe85975e74dd562bb94f7ce3de1ef1 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 15 Jul 2013 15:48:50 +0200 Subject: [PATCH 2/4] memory: actually set the owner Brown paper bag for me. Originally commit 803c0816 came before commit 2c9b15c. When the order was inverted, I left in the NULL initialization of mr->owner. Reviewed-by: Hu Tao Signed-off-by: Paolo Bonzini --- memory.c | 1 - 1 file changed, 1 deletion(-) diff --git a/memory.c b/memory.c index c8f9a2bfb3..9938b6ba45 100644 --- a/memory.c +++ b/memory.c @@ -805,7 +805,6 @@ void memory_region_init(MemoryRegion *mr, mr->owner = owner; mr->iommu_ops = NULL; mr->parent = NULL; - mr->owner = NULL; mr->size = int128_make64(size); if (size == UINT64_MAX) { mr->size = int128_2_64(); From 9b8c69243585a32d14b9bb9fcd52c37b0b5a1b71 Mon Sep 17 00:00:00 2001 From: Jan Kiszka Date: Tue, 16 Jul 2013 14:45:16 +0200 Subject: [PATCH 3/4] memory: Return -1 again on reads from unsigned regions This restore the behavior prior to b018ddf633 which accidentally changed the return code to 0. Specifically guests probing for register existence were affected by this. Signed-off-by: Jan Kiszka Signed-off-by: Paolo Bonzini --- memory.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/memory.c b/memory.c index 9938b6ba45..34a088e90f 100644 --- a/memory.c +++ b/memory.c @@ -840,7 +840,7 @@ static uint64_t unassigned_mem_read(void *opaque, hwaddr addr, if (current_cpu != NULL) { cpu_unassigned_access(current_cpu, addr, false, false, 0, size); } - return 0; + return -1ULL; } static void unassigned_mem_write(void *opaque, hwaddr addr, From e1622f4b15391bd44eb0f99a244fdf19a20fd981 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Wed, 17 Jul 2013 13:17:41 +0200 Subject: [PATCH 4/4] exec: fix incorrect assumptions in memory_access_size access_size_min can be 1 because erroneous accesses must not crash QEMU, they should trigger exceptions in the guest or just return garbage (depending on the CPU). I am not sure I understand the comment: placing a 4-byte field at the last byte of a region makes no sense (unless impl.unaligned is true), and that is why memory.c:access_with_adjusted_size does not bother with minimums larger than the remaining length. access_size_max can be mr->ops->valid.max_access_size because memory.c can and will still break accesses bigger than mr->ops->impl.max_access_size. Reported-by: Markus Armbruster Tested-by: Markus Armbruster Signed-off-by: Paolo Bonzini --- exec.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/exec.c b/exec.c index d312bb4f3f..c8658c6f9d 100644 --- a/exec.c +++ b/exec.c @@ -1898,14 +1898,10 @@ static inline bool memory_access_is_direct(MemoryRegion *mr, bool is_write) static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) { - unsigned access_size_min = mr->ops->impl.min_access_size; - unsigned access_size_max = mr->ops->impl.max_access_size; + unsigned access_size_max = mr->ops->valid.max_access_size; /* Regions are assumed to support 1-4 byte accesses unless otherwise specified. */ - if (access_size_min == 0) { - access_size_min = 1; - } if (access_size_max == 0) { access_size_max = 4; } @@ -1922,9 +1918,6 @@ static int memory_access_size(MemoryRegion *mr, unsigned l, hwaddr addr) if (l > access_size_max) { l = access_size_max; } - /* ??? The users of this function are wrong, not supporting minimums larger - than the remaining length. C.f. memory.c:access_with_adjusted_size. */ - assert(l >= access_size_min); return l; }