diff mbox

[RFC,5/8] s390: Make qemu_ram_remap() consistent with allocation

Message ID 1371106939-6968-6-git-send-email-armbru@redhat.com
State New
Headers show

Commit Message

Markus Armbruster June 13, 2013, 7:02 a.m. UTC 
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(-)

Comments

Paolo Bonzini June 13, 2013, 10:19 p.m. UTC | #1 
Il 13/06/2013 03:02, Markus Armbruster ha scritto:
> -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);

You can just turn kvm_arch_mem_alloc to kvm_arch_ram_mmap, and call it
from both kvm_mem_alloc and a new kvm_mem_remap.  Then s390 can do

    mmap(vaddr ? vaddr : (void *) 0x800000000ULL,
         size, PROT_EXEC | PROT_READ | PROT_WRITE,
         MAP_FIXED | MAP_SHARED | MAP_ANONYMOUS, -1, 0);

and other arches can still return NULL.

Paolo
diff mbox

Patch

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