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);

         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