From 965eb2fcdfe919ecced6c34803535ad32dc1249c Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Wed, 17 Jun 2015 10:40:27 +0200
Subject: [PATCH] exec: do not clamp accesses to MMIO regions
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit
It is common for MMIO registers to overlap, for example a 4 byte register
at 0xcf8 (totally random choice... :)) and a 1 byte register at 0xcf9.
If these registers are implemented via separate MemoryRegions, it is
wrong to clamp the accesses as the value written would be truncated.
Hence for these regions the effects of commit 23820db (exec: Respect
as_translate_internal length clamp, 2015-03-16, previously applied as
commit c3c1bb99) must be skipped.
Tested-by: Hervé Poussineau <hpoussin@reactos.org>
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 8 ++++++--
1 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/exec.c b/exec.c
index 76bfc4a..d00e017 100644
--- a/tools/qemu-xen/exec.c
+++ b/tools/qemu-xen/exec.c
@@ -341,6 +341,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
hwaddr *plen, bool resolve_subpage)
{
MemoryRegionSection *section;
+ MemoryRegion *mr;
Int128 diff;
section = address_space_lookup_region(d, addr, resolve_subpage);
@@ -350,8 +351,11 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
/* Compute offset within MemoryRegion */
*xlat = addr + section->offset_within_region;
- diff = int128_sub(section->mr->size, int128_make64(addr));
- *plen = int128_get64(int128_min(diff, int128_make64(*plen)));
+ mr = section->mr;
+ if (memory_region_is_ram(mr)) {
+ diff = int128_sub(mr->size, int128_make64(addr));
+ *plen = int128_get64(int128_min(diff, int128_make64(*plen)));
+ }
return section;
}
--
1.7.0.4
From e4a511f8cc6f4a46d409fb5c9f72c38ba45f8d83 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Wed, 17 Jun 2015 10:36:54 +0200
Subject: [PATCH] exec: clamp accesses against the MemoryRegionSection
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit
Because the clamping was done against the MemoryRegion,
address_space_rw was effectively broken if a write spanned
multiple sections that are not linear in underlying memory
(with the memory not being under an IOMMU).
This is visible with the MIPS rc4030 IOMMU, which is implemented
as a series of alias memory regions that point to the actual RAM.
Tested-by: Hervé Poussineau <hpoussin@reactos.org>
Tested-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/exec.c b/exec.c
index d00e017..f7883d2 100644
--- a/tools/qemu-xen/exec.c
+++ b/tools/qemu-xen/exec.c
@@ -353,7 +353,7 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
mr = section->mr;
if (memory_region_is_ram(mr)) {
- diff = int128_sub(mr->size, int128_make64(addr));
+ diff = int128_sub(section->size, int128_make64(addr));
*plen = int128_get64(int128_min(diff, int128_make64(*plen)));
}
return section;
--
1.7.0.4
From b242e0e0e2969c044a318e56f7988bbd84de1f63 Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Sat, 4 Jul 2015 00:24:51 +0200
Subject: [PATCH] exec: skip MMIO regions correctly in cpu_physical_memory_write_rom_internal
Loading the BIOS in the mac99 machine is interesting, because there is a
PROM in the middle of the BIOS region (from 16K to 32K). Before memory
region accesses were clamped, when QEMU was asked to load a BIOS from
0xfff00000 to 0xffffffff it would put even those 16K from the BIOS file
into the region. This is weird because those 16K were not actually
visible between 0xfff04000 and 0xfff07fff. However, it worked.
After clamping was added, this also worked. In this case, the
cpu_physical_memory_write_rom_internal function split the write in
three parts: the first 16K were copied, the PROM area (second 16K) were
ignored, then the rest was copied.
Problems then started with commit 965eb2f (exec: do not clamp accesses
to MMIO regions, 2015-06-17). Clamping accesses is not done for MMIO
regions because they can overlap wildly, and MMIO registers can be
expected to perform full-width accesses based only on their address
(with no respect for adjacent registers that could decode to completely
different MemoryRegions). However, this lack of clamping also applied
to the PROM area! cpu_physical_memory_write_rom_internal thus failed
to copy the third range above, i.e. only copied the first 16K of the BIOS.
In effect, address_space_translate is expecting _something else_ to do
the clamping for MMIO regions if the incoming length is large. This
"something else" is memory_access_size in the case of address_space_rw,
so use the same logic in cpu_physical_memory_write_rom_internal.
Reported-by: Alexander Graf <agraf@redhat.com>
Reviewed-by: Laurent Vivier <lvivier@redhat.com>
Tested-by: Laurent Vivier <lvivier@redhat.com>
Fixes: 965eb2f
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 14 +++++++++++++-
1 files changed, 13 insertions(+), 1 deletions(-)
diff --git a/exec.c b/exec.c
index 3457f7e..251dc79 100644
--- a/tools/qemu-xen/exec.c
+++ b/tools/qemu-xen/exec.c
@@ -353,6 +353,18 @@ address_space_translate_internal(AddressSpaceDispatch *d, hwaddr addr, hwaddr *x
*xlat = addr + section->offset_within_region;
mr = section->mr;
+
+ /* MMIO registers can be expected to perform full-width accesses based only
+ * on their address, without considering adjacent registers that could
+ * decode to completely different MemoryRegions. When such registers
+ * exist (e.g. I/O ports 0xcf8 and 0xcf9 on most PC chipsets), MMIO
+ * regions overlap wildly. For this reason we cannot clamp the accesses
+ * here.
+ *
+ * If the length is small (as is the case for address_space_ldl/stl),
+ * everything works fine. If the incoming length is large, however,
+ * the caller really has to do the clamping through memory_access_size.
+ */
if (memory_region_is_ram(mr)) {
diff = int128_sub(section->size, int128_make64(addr));
*plen = int128_get64(int128_min(diff, int128_make64(*plen)));
@@ -2491,7 +2503,7 @@ static inline void cpu_physical_memory_write_rom_internal(AddressSpace *as,
if (!(memory_region_is_ram(mr) ||
memory_region_is_romd(mr))) {
- /* do nothing */
+ l = memory_access_size(mr, l, addr1);
} else {
addr1 += memory_region_get_ram_addr(mr);
/* ROM/RAM case */
--
1.7.0.4
From c3c1bb99d1c11978d9ce94d1bdcf0705378c1459 Mon Sep 17 00:00:00 2001
From: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Date: Mon, 16 Mar 2015 22:35:54 -0700
Subject: [PATCH] exec: Respect as_tranlsate_internal length clamp
address_space_translate_internal will clamp the *plen length argument
based on the size of the memory region being queried. The iommu walker
logic in addresss_space_translate was ignoring this by discarding the
post fn call value of *plen. Fix by just always using *plen as the
length argument throughout the fn, removing the len local variable.
This fixes a bootloader bug when a single elf section spans multiple
QEMU memory regions.
Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com>
Message-Id: <1426570554-15940-1-git-send-email-peter.crosthwaite@xilinx.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
exec.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/exec.c b/exec.c
index e97071a..8b922db 100644
--- a/tools/qemu-xen/exec.c
+++ b/tools/qemu-xen/exec.c
@@ -380,7 +380,6 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
IOMMUTLBEntry iotlb;
MemoryRegionSection *section;
MemoryRegion *mr;
- hwaddr len = *plen;
for (;;) {
section = address_space_translate_internal(as->dispatch, addr, &addr, plen, true);
@@ -395,7 +394,7 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
iotlb = mr->iommu_ops->translate(mr, addr, is_write);
addr = ((iotlb.translated_addr & ~iotlb.addr_mask)
| (addr & iotlb.addr_mask));
- len = MIN(len, (addr | iotlb.addr_mask) - addr + 1);
+ *plen = MIN(*plen, (addr | iotlb.addr_mask) - addr + 1);
if (!(iotlb.perm & (1 << is_write))) {
mr = &io_mem_unassigned;
break;
@@ -406,10 +405,9 @@ MemoryRegion *address_space_translate(AddressSpace *as, hwaddr addr,
if (xen_enabled() && memory_access_is_direct(mr, is_write)) {
hwaddr page = ((addr & TARGET_PAGE_MASK) + TARGET_PAGE_SIZE) - addr;
- len = MIN(page, len);
+ *plen = MIN(page, *plen);
}
- *plen = len;
*xlat = addr;
return mr;
}
--
1.7.0.4