[Qemu-devel] [PATCH RFC 5/8] s390: Make qemu_ram_remap() consistent with allocation

[Markus Armbruster <armbru@redhat.com>]

Old S390 KVM wants guest RAM mapped in a peculiar way.

Commit fdec991 changed qemu_ram_alloc_from_ptr() to do this only when
necessary, but forgot to update qemu_ram_remap().  If
qemu_ram_alloc_from_ptr() maps RAM the normal way, but
qemu_ram_remap() remaps it the peculiar way, remapping changes
protection and flags, which it shouldn't.

Fortunately, this can't happen right now, as we never remap on S390.
Fix it anyway.

Thanks to Christian Borntraeger for help with assessing the bug's
(non-)impact.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 exec.c               | 19 +++++++------------
 include/sysemu/kvm.h |  1 +
 kvm-all.c            |  1 +
 target-s390x/kvm.c   | 15 ++++++++++-----
 4 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/exec.c b/exec.c
index 7552e3c..8810d33 100644
--- a/exec.c
+++ b/exec.c
@@ -1209,27 +1209,22 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
             } else if (xen_enabled()) {
                 abort();
             } else {
-                flags = MAP_FIXED;
                 munmap(vaddr, length);
                 if (block->fd >= 0) {
 #ifdef MAP_POPULATE
-                    flags |= mem_prealloc ? MAP_POPULATE | MAP_SHARED :
+                    flags = mem_prealloc ? MAP_POPULATE | MAP_SHARED :
                         MAP_PRIVATE;
 #else
-                    flags |= MAP_PRIVATE;
+                    flags = MAP_PRIVATE;
 #endif
                     area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
-                                flags, block->fd, offset);
+                                MAP_FIXED | flags, block->fd, offset);
+                } else if (kvm_enabled() && kvm_arch_ram_remap) {
+                    area = kvm_arch_ram_remap(vaddr, length);
                 } else {
-#if defined(TARGET_S390X) && defined(CONFIG_KVM)
-                    flags |= MAP_SHARED | MAP_ANONYMOUS;
-                    area = mmap(vaddr, length, PROT_EXEC|PROT_READ|PROT_WRITE,
-                                flags, -1, 0);
-#else
-                    flags |= MAP_PRIVATE | MAP_ANONYMOUS;
                     area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
-                                flags, -1, 0);
-#endif
+                                MAP_FIXED | MAP_PRIVATE | MAP_ANONYMOUS,
+                                -1, 0);
                 }
                 if (area != vaddr) {
                     fprintf(stderr, "Could not remap addr: "
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index dd79914..fdfa7ba 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -153,6 +153,7 @@ int kvm_cpu_exec(CPUArchState *env);
 
 #if !defined(CONFIG_USER_ONLY)
 extern void *(*kvm_arch_ram_alloc)(ram_addr_t size);
+extern void *(*kvm_arch_ram_remap)(void *vaddr, ram_addr_t size);
 #endif
 
 void kvm_setup_guest_memory(void *start, size_t size);
diff --git a/kvm-all.c b/kvm-all.c
index 5e0cc9b..daa4d40 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -120,6 +120,7 @@ static const KVMCapabilityInfo kvm_required_capabilites[] = {
 };
 
 void *(*kvm_arch_ram_alloc)(ram_addr_t size);
+void *(*kvm_arch_ram_remap)(void *vaddr, ram_addr_t size);
 
 static KVMSlot *kvm_alloc_slot(KVMState *s)
 {
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index be802ab..aa45e06 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -93,6 +93,7 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 static int cap_sync_regs;
 
 static void *legacy_s390_alloc(ram_addr_t size);
+static void *legacy_s390_mmap(void *vaddr, ram_addr_t length);
 
 int kvm_arch_init(KVMState *s)
 {
@@ -100,6 +101,7 @@ int kvm_arch_init(KVMState *s)
     if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
         || !kvm_check_extension(s, KVM_CAP_S390_COW)) {
         kvm_arch_ram_alloc = legacy_s390_alloc;
+        kvm_arch_ram_remap = legacy_s390_mmap;
     }
     return 0;
 }
@@ -324,13 +326,16 @@ int kvm_s390_get_registers_partial(CPUState *cs)
  * to grow. We also have to use MAP parameters that avoid
  * read-only mapping of guest pages.
  */
-static void *legacy_s390_alloc(ram_addr_t size)
+
+static void *legacy_s390_mmap(void *vaddr, ram_addr_t size)
 {
-    void *mem;
+    return mmap(vaddr, size, PROT_EXEC | PROT_READ | PROT_WRITE,
+                MAP_FIXED | MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+}
 
-    mem = mmap((void *) 0x800000000ULL, size,
-               PROT_EXEC|PROT_READ|PROT_WRITE,
-               MAP_SHARED | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
+static void *legacy_s390_alloc(ram_addr_t size)
+{
+    void *mem = legacy_s390_mmap((void *) 0x800000000ULL, size);
     if (mem == MAP_FAILED) {
         fprintf(stderr, "Allocating RAM failed\n");
         abort();
-- 
1.7.11.7