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