Guest memory allocation fixes & cleanup

RFC PATCH In review Markus Armbruster 6 participants 23 messages Jun 13, 2013 – Jun 15, 2013

Share & tools

Markus Armbruster

author

2013-06-13 07:02 UTC #0 lore raw

cover Guest memory allocation fixes & cleanup

All I wanted to do is exit(1) instead of abort() on guest memory allocation failure [07/08]. But that lead me into a minor #ifdef bog, and here's what I brought back. Enjoy!

Markus Armbruster (8):
exec: Fix Xen RAM allocation with unusual options
exec: Clean up fall back when -mem-path allocation fails
exec: Reduce ifdeffery around -mem-path
s390: Simplify the RAM allocation hook
s390: Make qemu_ram_remap() consistent with allocation
exec: Clean up unnecessary S390 ifdeffery
exec: Don't abort when we can't allocate guest memory
pc_sysfw: Fix ISA BIOS init for ridiculously big flash

 exec.c                 | 117 ++++++++++++++++++++++++-------------------------
 hw/block/pc_sysfw.c    |   5 +--
 include/exec/cpu-all.h |   2 -
 include/sysemu/kvm.h   |   4 +-
 kvm-all.c              |  16 ++-----
 target-s390x/kvm.c     |  34 +++++++-------
 util/oslib-posix.c     |   4 +-
 util/oslib-win32.c     |   5 +--
 8 files changed, 81 insertions(+), 106 deletions(-)

Signature · 1 line

1.7.11.7

Markus Armbruster

author

2013-06-13 07:02 UTC #1 lore raw

RFC PATCH 1/8 exec: Fix Xen RAM allocation with unusual options

Issues:

* We try to obey -mem-path even though it can't work with Xen.

* To implement -machine mem-merge, we call memory_try_enable_merging(new_block->host, size). But with Xen, new_block->host remains null. Oops.

Fix by separating Xen allocation from normal allocation.

Signed-off-by Markus Armbruster <[email protected]>

---
 exec.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

exec.c +12 -8

index 5b8b40d..b424e12 100644
@@ -1081,6 +1081,12 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
     if (host) {
         new_block->host = host;
         new_block->flags |= RAM_PREALLOC_MASK;
+    } else if (xen_enabled()) {
+        if (mem_path) {
+            fprintf(stderr, "-mem-path not supported with Xen\n");
+            exit(1);
+        }
+        xen_ram_alloc(new_block->offset, size, mr);
     } else {
         if (mem_path) {
 #if defined (__linux__) && !defined(TARGET_S390X)

@@ -1094,9 +1100,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
             exit(1);
 #endif
         } else {
-            if (xen_enabled()) {
-                xen_ram_alloc(new_block->offset, size, mr);
-            } else if (kvm_enabled()) {
+            if (kvm_enabled()) {
                 /* some s390/kvm configurations have special constraints */
                 new_block->host = kvm_ram_alloc(size);
             } else {

@@ -1174,6 +1178,8 @@ void qemu_ram_free(ram_addr_t addr)
             ram_list.version++;
             if (block->flags & RAM_PREALLOC_MASK) {
                 ;
+            } else if (xen_enabled()) {
+                xen_invalidate_map_cache_entry(block->host);
             } else if (mem_path) {
 #if defined (__linux__) && !defined(TARGET_S390X)
                 if (block->fd) {

@@ -1186,11 +1192,7 @@ void qemu_ram_free(ram_addr_t addr)
                 abort();
 #endif
             } else {
-                if (xen_enabled()) {
-                    xen_invalidate_map_cache_entry(block->host);
-                } else {
-                    qemu_anon_ram_free(block->host, block->length);
-                }
+                qemu_anon_ram_free(block->host, block->length);
             }
             g_free(block);
             break;

@@ -1214,6 +1216,8 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
             vaddr = block->host + offset;
             if (block->flags & RAM_PREALLOC_MASK) {
                 ;
+            } else if (xen_enabled()) {
+                abort();
             } else {
                 flags = MAP_FIXED;
                 munmap(vaddr, length);

Signature · 1 line

1.7.11.7

Stefano Stabellini

Acked-by on v1 1/8

2013-06-13 11:54 UTC #2 lore raw

Re: [Qemu-devel] [PATCH RFC 1/8] exec: Fix Xen RAM allocation with unusual options

Quoted 11 lines · Markus Armbruster

Issues:
* We try to obey -mem-path even though it can't work with Xen.
* To implement -machine mem-merge, we call memory_try_enable_merging(new_block->host, size). But with Xen, new_block->host remains null. Oops.
Fix by separating Xen allocation from normal allocation.
Signed-off-by Markus Armbruster <[email protected]>

Acked-by Stefano Stabellini <[email protected]>

Quoted 65 lines

 exec.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

exec.c +12 -8

index 5b8b40d..b424e12 100644
@@ -1081,6 +1081,12 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
     if (host) {
         new_block->host = host;
         new_block->flags |= RAM_PREALLOC_MASK;
+    } else if (xen_enabled()) {
+        if (mem_path) {
+            fprintf(stderr, "-mem-path not supported with Xen\n");
+            exit(1);
+        }
+        xen_ram_alloc(new_block->offset, size, mr);
     } else {
         if (mem_path) {
 #if defined (__linux__) && !defined(TARGET_S390X)

@@ -1094,9 +1100,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
             exit(1);
 #endif
         } else {
-            if (xen_enabled()) {
-                xen_ram_alloc(new_block->offset, size, mr);
-            } else if (kvm_enabled()) {
+            if (kvm_enabled()) {
                 /* some s390/kvm configurations have special constraints */
                 new_block->host = kvm_ram_alloc(size);
             } else {

@@ -1174,6 +1178,8 @@ void qemu_ram_free(ram_addr_t addr)
             ram_list.version++;
             if (block->flags & RAM_PREALLOC_MASK) {
                 ;
+            } else if (xen_enabled()) {
+                xen_invalidate_map_cache_entry(block->host);
             } else if (mem_path) {
 #if defined (__linux__) && !defined(TARGET_S390X)
                 if (block->fd) {

@@ -1186,11 +1192,7 @@ void qemu_ram_free(ram_addr_t addr)
                 abort();
 #endif
             } else {
-                if (xen_enabled()) {
-                    xen_invalidate_map_cache_entry(block->host);
-                } else {
-                    qemu_anon_ram_free(block->host, block->length);
-                }
+                qemu_anon_ram_free(block->host, block->length);
             }
             g_free(block);
             break;

@@ -1214,6 +1216,8 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
             vaddr = block->host + offset;
             if (block->flags & RAM_PREALLOC_MASK) {
                 ;
+            } else if (xen_enabled()) {
+                abort();
             } else {
                 flags = MAP_FIXED;
                 munmap(vaddr, length);

Signature · 1 line

1.7.11.7

Markus Armbruster

author

2013-06-13 07:02 UTC #3 lore raw

RFC PATCH 2/8 exec: Clean up fall back when -mem-path allocation fails

In reply to Markus Armbruster

With -mem-path, qemu_ram_alloc_from_ptr() first tries to allocate accordingly, but when it fails, it falls back to normal allocation.

The fall back allocation code used to be effectively identical to the "-mem-path not given" code, until it started to diverge incommit 432d268. I believe the code still works, but clean it up anyway: drop the special fall back allocation code, and fall back to the ordinary "-mem-path not given" code instead.

Signed-off-by Markus Armbruster <[email protected]>

---
 exec.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

exec.c +2 -5

index b424e12..56c31a9 100644
@@ -1091,15 +1091,12 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
         if (mem_path) {
 #if defined (__linux__) && !defined(TARGET_S390X)
             new_block->host = file_ram_alloc(new_block, size, mem_path);
-            if (!new_block->host) {
-                new_block->host = qemu_anon_ram_alloc(size);
-                memory_try_enable_merging(new_block->host, size);
-            }
 #else
             fprintf(stderr, "-mem-path option unsupported\n");
             exit(1);
 #endif
-        } else {
+        }
+        if (!new_block->host) {
             if (kvm_enabled()) {
                 /* some s390/kvm configurations have special constraints */
                 new_block->host = kvm_ram_alloc(size);

Signature · 1 line

1.7.11.7

Paolo Bonzini

Reviewed-by on v1 2/8

2013-06-13 22:12 UTC #4 lore raw

Re: [Qemu-devel] [PATCH RFC 2/8] exec: Clean up fall back when -mem-path allocation fails

Il 13/06/2013 03:02, Markus Armbruster ha scritto:

Quoted 27 lines

With -mem-path, qemu_ram_alloc_from_ptr() first tries to allocate accordingly, but when it fails, it falls back to normal allocation.
The fall back allocation code used to be effectively identical to the "-mem-path not given" code, until it started to diverge incommit 432d268. I believe the code still works, but clean it up anyway: drop the special fall back allocation code, and fall back to the ordinary "-mem-path not given" code instead.
Signed-off-by Markus Armbruster <[email protected]>
---
 exec.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
exec.c +0 -4
index b424e12..56c31a9 100644
@@ -1091,15 +1091,12 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
         if (mem_path) {
 #if defined (__linux__) && !defined(TARGET_S390X)
             new_block->host = file_ram_alloc(new_block, size, mem_path);
-            if (!new_block->host) {
-                new_block->host = qemu_anon_ram_alloc(size);
-                memory_try_enable_merging(new_block->host, size);
-            }

exec.c ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host, · line 1102

Quoted 10 lines

exec.c +2 -1

 #else
             fprintf(stderr, "-mem-path option unsupported\n");
             exit(1);
 #endif
-        } else {
+        }
+        if (!new_block->host) {
             if (kvm_enabled()) {
                 /* some s390/kvm configurations have special constraints */
                 new_block->host = kvm_ram_alloc(size);

Reviewed-by Paolo Bonzini <[email protected]>

Markus Armbruster

author

2013-06-13 07:02 UTC #5 lore raw

RFC PATCH 3/8 exec: Reduce ifdeffery around -mem-path

In reply to Markus Armbruster

Instead of spreading its ifdeffery everywhere, confine it to qemu_ram_alloc_from_ptr(). Everywhere else, simply test block->fd, which is valid exactly when block uses -mem-path.

Signed-off-by Markus Armbruster <[email protected]>

---
 exec.c                 | 37 ++++++++++---------------------------
 include/exec/cpu-all.h |  2 --
 2 files changed, 10 insertions(+), 29 deletions(-)

exec.c +10 ~~-27~~

index 56c31a9..4dbb0f1 100644
@@ -1073,6 +1073,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
 
     size = TARGET_PAGE_ALIGN(size);
     new_block = g_malloc0(sizeof(*new_block));
+    new_block->fd = -1;
 
     /* This assumes the iothread lock is taken here too.  */
     qemu_mutex_lock_ramlist();

@@ -1177,17 +1178,9 @@ void qemu_ram_free(ram_addr_t addr)
                 ;
             } else if (xen_enabled()) {
                 xen_invalidate_map_cache_entry(block->host);
-            } else if (mem_path) {
-#if defined (__linux__) && !defined(TARGET_S390X)
-                if (block->fd) {
-                    munmap(block->host, block->length);
-                    close(block->fd);
-                } else {
-                    qemu_anon_ram_free(block->host, block->length);
-                }
-#else
-                abort();
-#endif
+            } else if (block->fd >= 0) {
+                munmap(block->host, block->length);
+                close(block->fd);
             } else {
                 qemu_anon_ram_free(block->host, block->length);
             }

@@ -1218,25 +1211,15 @@ void qemu_ram_remap(ram_addr_t addr, ram_addr_t length)
             } else {
                 flags = MAP_FIXED;
                 munmap(vaddr, length);
-                if (mem_path) {
-#if defined(__linux__) && !defined(TARGET_S390X)
-                    if (block->fd) {
+                if (block->fd >= 0) {
 #ifdef MAP_POPULATE
-                        flags |= mem_prealloc ? MAP_POPULATE | MAP_SHARED :
-                            MAP_PRIVATE;
+                    flags |= mem_prealloc ? MAP_POPULATE | MAP_SHARED :
+                        MAP_PRIVATE;
 #else
-                        flags |= MAP_PRIVATE;
-#endif
-                        area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
-                                    flags, block->fd, offset);
-                    } else {
-                        flags |= MAP_PRIVATE | MAP_ANONYMOUS;
-                        area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
-                                    flags, -1, 0);
-                    }
-#else
-                    abort();
+                    flags |= MAP_PRIVATE;
 #endif
+                    area = mmap(vaddr, length, PROT_READ | PROT_WRITE,
+                                flags, block->fd, offset);
                 } else {
 #if defined(TARGET_S390X) && defined(CONFIG_KVM)
                     flags |= MAP_SHARED | MAP_ANONYMOUS;

include/exec/cpu-all.h +0 -2

index e9c3717..c369b25 100644
@@ -476,9 +476,7 @@ typedef struct RAMBlock {
      * Writes must take both locks.
      */
     QTAILQ_ENTRY(RAMBlock) next;
-#if defined(__linux__) && !defined(TARGET_S390X)
     int fd;
-#endif
 } RAMBlock;
 
 typedef struct RAMList {

Signature · 1 line

1.7.11.7

Markus Armbruster

author

2013-06-13 07:02 UTC #6 lore raw

RFC PATCH 4/8 s390: Simplify the RAM allocation hook

In reply to Markus Armbruster

Less code and ifdeffery.

Signed-off-by Markus Armbruster <[email protected]>

---
 exec.c               |  4 ++--
 include/sysemu/kvm.h |  3 +--
 kvm-all.c            | 15 ++-------------
 target-s390x/kvm.c   | 17 ++++++-----------
 4 files changed, 11 insertions(+), 28 deletions(-)

exec.c +2 -2

index 4dbb0f1..7552e3c 100644
@@ -1098,9 +1098,9 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
 #endif
         }
         if (!new_block->host) {
-            if (kvm_enabled()) {
+            if (kvm_enabled() && kvm_arch_ram_alloc) {
                 /* some s390/kvm configurations have special constraints */
-                new_block->host = kvm_ram_alloc(size);
+                new_block->host = kvm_arch_ram_alloc(size);
             } else {
                 new_block->host = qemu_anon_ram_alloc(size);
             }

include/sysemu/kvm.h +1 -2

index 8b19322..dd79914 100644
@@ -152,8 +152,7 @@ int kvm_init_vcpu(CPUState *cpu);
 int kvm_cpu_exec(CPUArchState *env);
 
 #if !defined(CONFIG_USER_ONLY)
-void *kvm_ram_alloc(ram_addr_t size);
-void *kvm_arch_ram_alloc(ram_addr_t size);
+extern void *(*kvm_arch_ram_alloc)(ram_addr_t size);
 #endif
 
 void kvm_setup_guest_memory(void *start, size_t size);

kvm-all.c +2 ~~-13~~

index 405480e..5e0cc9b 100644
@@ -119,6 +119,8 @@ static const KVMCapabilityInfo kvm_required_capabilites[] = {
     KVM_CAP_LAST_INFO
 };
 
+void *(*kvm_arch_ram_alloc)(ram_addr_t size);
+
 static KVMSlot *kvm_alloc_slot(KVMState *s)
 {
     int i;

@@ -1816,19 +1818,6 @@ int kvm_has_intx_set_mask(void)
     return kvm_state->intx_set_mask;
 }
 
-void *kvm_ram_alloc(ram_addr_t size)
-{
-#ifdef TARGET_S390X
-    void *mem;
-
-    mem = kvm_arch_ram_alloc(size);
-    if (mem) {
-        return mem;
-    }
-#endif
-    return qemu_anon_ram_alloc(size);
-}
-
 void kvm_setup_guest_memory(void *start, size_t size)
 {
 #ifdef CONFIG_VALGRIND_H

target-s390x/kvm.c +6 ~~-11~~

index 862fb12..be802ab 100644
@@ -92,9 +92,15 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 
 static int cap_sync_regs;
 
+static void *legacy_s390_alloc(ram_addr_t size);
+
 int kvm_arch_init(KVMState *s)
 {
     cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
+    if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
+        || !kvm_check_extension(s, KVM_CAP_S390_COW)) {
+        kvm_arch_ram_alloc = legacy_s390_alloc;
+    }
     return 0;
 }

@@ -332,17 +338,6 @@ static void *legacy_s390_alloc(ram_addr_t size)
     return mem;
 }
 
-void *kvm_arch_ram_alloc(ram_addr_t size)
-{
-    /* Can we use the standard allocation ? */
-    if (kvm_check_extension(kvm_state, KVM_CAP_S390_GMAP) &&
-        kvm_check_extension(kvm_state, KVM_CAP_S390_COW)) {
-        return NULL;
-    } else {
-        return legacy_s390_alloc(size);
-    }
-}
-
 int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
 {
     S390CPU *cpu = S390_CPU(cs);

Signature · 1 line

1.7.11.7

Andreas Färber

Reviewed-by on v1 4/8

2013-06-13 08:19 UTC #7 lore raw

Re: [Qemu-devel] [PATCH RFC 4/8] s390: Simplify the RAM allocation hook

Am 13.06.2013 09:02, schrieb Markus Armbruster:

Quoted 3 lines

Less code and ifdeffery.
Signed-off-by Markus Armbruster <[email protected]>

Reviewed-by Andreas Färber <[email protected]>

Andreas

Signature · 2 lines

SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Paolo Bonzini

2013-06-13 22:14 UTC #8 lore raw

Re: [Qemu-devel] [PATCH RFC 4/8] s390: Simplify the RAM allocation hook

In reply to Markus Armbruster

Il 13/06/2013 03:02, Markus Armbruster ha scritto:

Quoted 1 line

Less code and ifdeffery.

Less ifdeffery is of course fine, but please add instead kvm_arch_ram_alloc to the other target-*/kvm.c files. It can just return NULL. I think this may affect the karma that comes from a negative diffstat, but it is more similar to other kvm_arch_ hooks.

Paolo

Quoted 110 lines

Signed-off-by Markus Armbruster <[email protected]>

---
 exec.c               |  4 ++--
 include/sysemu/kvm.h |  3 +--
 kvm-all.c            | 15 ++-------------
 target-s390x/kvm.c   | 17 ++++++-----------
 4 files changed, 11 insertions(+), 28 deletions(-)

exec.c +2 -2

index 4dbb0f1..7552e3c 100644
@@ -1098,9 +1098,9 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
 #endif
         }
         if (!new_block->host) {
-            if (kvm_enabled()) {
+            if (kvm_enabled() && kvm_arch_ram_alloc) {
                 /* some s390/kvm configurations have special constraints */
-                new_block->host = kvm_ram_alloc(size);
+                new_block->host = kvm_arch_ram_alloc(size);
             } else {
                 new_block->host = qemu_anon_ram_alloc(size);
             }

include/sysemu/kvm.h +1 -2

index 8b19322..dd79914 100644
@@ -152,8 +152,7 @@ int kvm_init_vcpu(CPUState *cpu);
 int kvm_cpu_exec(CPUArchState *env);
 
 #if !defined(CONFIG_USER_ONLY)
-void *kvm_ram_alloc(ram_addr_t size);
-void *kvm_arch_ram_alloc(ram_addr_t size);
+extern void *(*kvm_arch_ram_alloc)(ram_addr_t size);
 #endif
 
 void kvm_setup_guest_memory(void *start, size_t size);

kvm-all.c +2 ~~-13~~

index 405480e..5e0cc9b 100644
@@ -119,6 +119,8 @@ static const KVMCapabilityInfo kvm_required_capabilites[] = {
     KVM_CAP_LAST_INFO
 };
 
+void *(*kvm_arch_ram_alloc)(ram_addr_t size);
+
 static KVMSlot *kvm_alloc_slot(KVMState *s)
 {
     int i;

@@ -1816,19 +1818,6 @@ int kvm_has_intx_set_mask(void)
     return kvm_state->intx_set_mask;
 }
 
-void *kvm_ram_alloc(ram_addr_t size)
-{
-#ifdef TARGET_S390X
-    void *mem;
-
-    mem = kvm_arch_ram_alloc(size);
-    if (mem) {
-        return mem;
-    }
-#endif
-    return qemu_anon_ram_alloc(size);
-}
-
 void kvm_setup_guest_memory(void *start, size_t size)
 {
 #ifdef CONFIG_VALGRIND_H

target-s390x/kvm.c +6 ~~-11~~

index 862fb12..be802ab 100644
@@ -92,9 +92,15 @@ const KVMCapabilityInfo kvm_arch_required_capabilities[] = {
 
 static int cap_sync_regs;
 
+static void *legacy_s390_alloc(ram_addr_t size);
+
 int kvm_arch_init(KVMState *s)
 {
     cap_sync_regs = kvm_check_extension(s, KVM_CAP_SYNC_REGS);
+    if (!kvm_check_extension(s, KVM_CAP_S390_GMAP)
+        || !kvm_check_extension(s, KVM_CAP_S390_COW)) {
+        kvm_arch_ram_alloc = legacy_s390_alloc;
+    }
     return 0;
 }

@@ -332,17 +338,6 @@ static void *legacy_s390_alloc(ram_addr_t size)
     return mem;
 }
 
-void *kvm_arch_ram_alloc(ram_addr_t size)
-{
-    /* Can we use the standard allocation ? */
-    if (kvm_check_extension(kvm_state, KVM_CAP_S390_GMAP) &&
-        kvm_check_extension(kvm_state, KVM_CAP_S390_COW)) {
-        return NULL;
-    } else {
-        return legacy_s390_alloc(size);
-    }
-}
-
 int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)
 {
     S390CPU *cpu = S390_CPU(cs);

Markus Armbruster

author

2013-06-13 07:02 UTC #9 lore raw

RFC PATCH 5/8 s390: Make qemu_ram_remap() consistent with allocation

In reply to Markus Armbruster

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 <[email protected]>

---
 exec.c               | 19 +++++++------------
 include/sysemu/kvm.h |  1 +
 kvm-all.c            |  1 +
 target-s390x/kvm.c   | 15 ++++++++++-----
 4 files changed, 19 insertions(+), 17 deletions(-)

exec.c +7 ~~-12~~

index 7552e3c..8810d33 100644
@@ -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: "

include/sysemu/kvm.h +1 -0

index dd79914..fdfa7ba 100644
@@ -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);

kvm-all.c +1 -0

index 5e0cc9b..daa4d40 100644
@@ -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)
 {

target-s390x/kvm.c +10 -5

index be802ab..aa45e06 100644
@@ -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();

Signature · 1 line

1.7.11.7

Paolo Bonzini

2013-06-13 22:20 UTC #10 lore raw

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

Il 13/06/2013 03:02, Markus Armbruster ha scritto:

target-s390x/kvm.c int kvm_s390_get_registers_partial(CPUState *cs) · line 333

Quoted 12 lines

target-s390x/kvm.c +5 -5

-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

Markus Armbruster

author

2013-06-13 07:02 UTC #11 lore raw

RFC PATCH 6/8 exec: Clean up unnecessary S390 ifdeffery

In reply to Markus Armbruster

Another issue missed in commit fdec991 is -mem-path: it needs to be rejected only for old S390 KVM, not for any S390. Not that I personally care, but the ifdeffery in qemu_ram_alloc_from_ptr() annoys me.

Note that this doesn't actually make -mem-path work, as the kernel doesn't (yet?) support large pages in the host for KVM guests. Clean it up anyway.

Thanks to Christian Borntraeger for pointing out the S390 kernel limitations.

Signed-off-by Markus Armbruster <[email protected]>

---
 exec.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

exec.c +19 ~~-12~~

index 8810d33..9f0355b 100644
@@ -849,7 +849,7 @@ void qemu_mutex_unlock_ramlist(void)
     qemu_mutex_unlock(&ram_list.mutex);
 }
 
-#if defined(__linux__) && !defined(TARGET_S390X)
+#ifdef __linux__
 
 #include <sys/vfs.h>

@@ -952,6 +952,14 @@ static void *file_ram_alloc(RAMBlock *block,
     block->fd = fd;
     return area;
 }
+#else
+static void *file_ram_alloc(RAMBlock *block,
+                            ram_addr_t memory,
+                            const char *path)
+{
+    fprintf(stderr, "-mem-path not supported on this host\n");
+    exit(1);
+}
 #endif
 
 static ram_addr_t find_ram_offset(ram_addr_t size)

@@ -1088,22 +1096,21 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
             exit(1);
         }
         xen_ram_alloc(new_block->offset, size, mr);
+    } else if (kvm_enabled() && kvm_arch_ram_alloc) {
+        /* some s390/kvm configurations have special constraints */
+        if (mem_path) {
+            fprintf(stderr,
+                    "-mem-path not supported with this version of KVM\n");
+            exit(1);
+        }
+        new_block->host = kvm_arch_ram_alloc(size);
+        memory_try_enable_merging(new_block->host, size);
     } else {
         if (mem_path) {
-#if defined (__linux__) && !defined(TARGET_S390X)
             new_block->host = file_ram_alloc(new_block, size, mem_path);
-#else
-            fprintf(stderr, "-mem-path option unsupported\n");
-            exit(1);
-#endif
         }
         if (!new_block->host) {
-            if (kvm_enabled() && kvm_arch_ram_alloc) {
-                /* some s390/kvm configurations have special constraints */
-                new_block->host = kvm_arch_ram_alloc(size);
-            } else {
-                new_block->host = qemu_anon_ram_alloc(size);
-            }
+            new_block->host = qemu_anon_ram_alloc(size);
             memory_try_enable_merging(new_block->host, size);
         }
     }

Signature · 1 line

1.7.11.7

Paolo Bonzini

2013-06-13 22:21 UTC #12 lore raw

Re: [Qemu-devel] [PATCH RFC 6/8] exec: Clean up unnecessary S390 ifdeffery

Il 13/06/2013 03:02, Markus Armbruster ha scritto:

exec.c ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host, · line 1107

Quoted 9 lines

exec.c +9 -0

+    } else if (kvm_enabled() && kvm_arch_ram_alloc) {
+        /* some s390/kvm configurations have special constraints */
+        if (mem_path) {
+            fprintf(stderr,
+                    "-mem-path not supported with this version of KVM\n");
+            exit(1);
+        }
+        new_block->host = kvm_arch_ram_alloc(size);
+        memory_try_enable_merging(new_block->host, size);

Uh oh, now I see why you wanted the hook as a function pointer. Can you instead pass the file descriptor to kvm_ram_alloc?

Paolo

Markus Armbruster

author

2013-06-14 08:06 UTC #13 lore raw

Re: [Qemu-devel] [PATCH RFC 6/8] exec: Clean up unnecessary S390 ifdeffery

Quoted 11 lines · Paolo Bonzini

Il 13/06/2013 03:02, Markus Armbruster ha scritto:

exec.c ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host, · line 1107

Quoted 9 lines

exec.c +9 -0

+    } else if (kvm_enabled() && kvm_arch_ram_alloc) {
+        /* some s390/kvm configurations have special constraints */
+        if (mem_path) {
+            fprintf(stderr,
+                    "-mem-path not supported with this version of KVM\n");
+            exit(1);
+        }
+        new_block->host = kvm_arch_ram_alloc(size);
+        memory_try_enable_merging(new_block->host, size);

Quoted 2 lines

Uh oh, now I see why you wanted the hook as a function pointer. Can you instead pass the file descriptor to kvm_ram_alloc?

I'm not sure I understand what you're suggesting. Here's my best guess.

I made kvm_arch_ram_alloc() an *optional* hook, implemented as function pointer that may be null. It's actually non-null only with old S390 KVM.

Non-null value also suppresses -mem-path. Admittedly not the cleanest possible solution, but (1) it's better than what we have now, and (2) the whole thing should go away forever once we stop supporting old S390 kernels.

You seem to suggest to factor the actual allocation of guest memory out of this part of qemu_ram_alloc_from_ptr():

    if (host) {
        new_block->host = host;
        new_block->flags |= RAM_PREALLOC_MASK;
    } else if (xen_enabled()) {
        if (mem_path) {
            fprintf(stderr, "-mem-path not supported with Xen\n");
            exit(1);
        }
        xen_ram_alloc(new_block->offset, size, mr);
    } else if (kvm_enabled() && kvm_arch_ram_alloc) {
        /* some s390/kvm configurations have special constraints */
        if (mem_path) {
            fprintf(stderr,
                    "-mem-path not supported with this version of KVM\n");
            exit(1);
        }
        new_block->host = kvm_arch_ram_alloc(size);
        if (!new_block->host) {
            no_guest_mem(new_block);
        }
        memory_try_enable_merging(new_block->host, size);
    } else {
        if (mem_path) {
            new_block->host = file_ram_alloc(new_block, size, mem_path);
        }
        if (!new_block->host) {
            new_block->host = qemu_anon_ram_alloc(size);
            if (!new_block->host) {
                no_guest_mem(new_block);
            }
            memory_try_enable_merging(new_block->host, size);
        }
    }

Three functions, one each for Xen, S390 KVM, and generic. Except the one for Xen has only one caller, so we better leave that one alone.

Put a trivial wrapper around generic into each target-*/kvm.c other than
s390: arm, i386, ppc.

qemu_ram_alloc_from_ptr() then looks roughly like this:

    if (host) {
        new_block->host = host;
        new_block->flags |= RAM_PREALLOC_MASK;
    } else if (xen_enabled()) {
        if (mem_path) {
            fprintf(stderr, "-mem-path not supported with Xen\n");
            exit(1);
        }
        xen_ram_alloc(new_block->offset, size, mr);
    } else if (kvm_enabled()) {
        new->block->host = kvm_arch_ram_alloc(size, new_block->fd);
        if (!new_block->host) {
            no_guest_mem(new_block);
        }
    } else {
        new_block->host = qemu_guest_mem_alloc(size, new_block->fd);
        if (!new_block->host) {
            no_guest_mem(new_block);
        }
    }

Except new_block->fd still needs to be set. Need to split file_ram_alloc() into two: first part yields the file descriptor, to be called by qemu_ram_alloc_from_ptr(), second part does the actual allocation, to be called by the allocation functions. The code in qemu_ram_alloc_from_ptr() becomes:

    if (host) {
        new_block->host = host;
        new_block->flags |= RAM_PREALLOC_MASK;
    } else if (xen_enabled()) {
        if (mem_path) {
            fprintf(stderr, "-mem-path not supported with Xen\n");
            exit(1);
        }
        xen_ram_alloc(new_block->offset, size, mr);
    } else {
        new->block->fd = file_ram_fd(mem_path);
        new->block->host = kvm_enabled()
            ? kvm_arch_ram_alloc(size, new_block->fd);
            : qemu_guest_mem_alloc(size, new_block->fd);
        if (!new_block->host) {
            no_guest_mem(new_block);
        }
    }

Is that what you have in mind?

It's a lot of restructuring and notational overhead just for this stupid hack to support old S390 kernels.

Here's another way to keep the KVM hooks regular: make my function pointers exec.c hooks instead of KVM hooks ;-P

Paolo Bonzini

2013-06-15 17:26 UTC #14 lore raw

Re: [Qemu-devel] [PATCH RFC 6/8] exec: Clean up unnecessary S390 ifdeffery

Il 14/06/2013 04:06, Markus Armbruster ha scritto:

Quoted 2 lines

Here's another way to keep the KVM hooks regular: make my function pointers exec.c hooks instead of KVM hooks ;-P

I can buy this one!

Paolo

Markus Armbruster

author

2013-06-13 07:02 UTC #15 lore raw

RFC PATCH 7/8 exec: Don't abort when we can't allocate guest memory

In reply to Markus Armbruster

We abort() on memory allocation failure. abort() is appropriate for programming errors. Maybe most memory allocation failures are programming errors, maybe not. But guest memory allocation failure isn't, and aborting when the user asks for more memory than we can provide is not nice. exit(1) instead, and do it in just one place, so the error message is consistent.

Signed-off-by Markus Armbruster <[email protected]>

---
 exec.c             | 15 ++++++++++++++-
 target-s390x/kvm.c |  6 +-----
 util/oslib-posix.c |  4 +---
 util/oslib-win32.c |  5 +----
 4 files changed, 17 insertions(+), 13 deletions(-)

exec.c +14 -1

index 9f0355b..7432465 100644
@@ -851,6 +851,13 @@ void qemu_mutex_unlock_ramlist(void)
 
 #ifdef __linux__
 
+static void no_guest_mem(RAMBlock *block)
+{
+    fprintf(stderr, "Cannot set up guest memory '%s': %s\n",
+            block->mr->name, strerror(errno));
+    exit(1);
+}
+
 #include <sys/vfs.h>
 
 #define HUGETLBFS_MAGIC       0x958458f6

@@ -945,7 +952,7 @@ static void *file_ram_alloc(RAMBlock *block,
     area = mmap(0, memory, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
 #endif
     if (area == MAP_FAILED) {
-        perror("file_ram_alloc: can't mmap RAM pages");
+        no_guest_mem(block);
         close(fd);
         return (NULL);
     }

@@ -1104,6 +1111,9 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
             exit(1);
         }
         new_block->host = kvm_arch_ram_alloc(size);
+        if (!new_block->host) {
+            no_guest_mem(new_block);
+        }
         memory_try_enable_merging(new_block->host, size);
     } else {
         if (mem_path) {

@@ -1111,6 +1121,9 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
         }
         if (!new_block->host) {
             new_block->host = qemu_anon_ram_alloc(size);
+            if (!new_block->host) {
+                no_guest_mem(new_block);
+            }
             memory_try_enable_merging(new_block->host, size);
         }
     }

target-s390x/kvm.c +1 -5

index aa45e06..472a66f 100644
@@ -336,11 +336,7 @@ static void *legacy_s390_mmap(void *vaddr, ram_addr_t size)
 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();
-    }
-    return mem;
+    return mem == MAP_FAILED ? NULL : mem;
 }
 
 int kvm_arch_insert_sw_breakpoint(CPUState *cs, struct kvm_sw_breakpoint *bp)

util/oslib-posix.c +1 -3

index 3dc8b1b..253bc3d 100644
@@ -112,9 +112,7 @@ void *qemu_anon_ram_alloc(size_t size)
     size_t offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr;
 
     if (ptr == MAP_FAILED) {
-        fprintf(stderr, "Failed to allocate %zu B: %s\n",
-                size, strerror(errno));
-        abort();
+        return NULL;
     }
 
     ptr += offset;

util/oslib-win32.c +1 -4

index 961fbf5..983b7a2 100644
@@ -65,10 +65,7 @@ void *qemu_anon_ram_alloc(size_t size)
     /* FIXME: this is not exactly optimal solution since VirtualAlloc
        has 64Kb granularity, but at least it guarantees us that the
        memory is page aligned. */
-    if (!size) {
-        abort();
-    }
-    ptr = qemu_oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
+    ptr = VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE);
     trace_qemu_anon_ram_alloc(size, ptr);
     return ptr;
 }

Signature · 1 line

1.7.11.7

Andreas Färber

Acked-by on v1 7/8

2013-06-13 08:33 UTC #16 lore raw

Re: [Qemu-devel] [PATCH RFC 7/8] exec: Don't abort when we can't allocate guest memory

Am 13.06.2013 09:02, schrieb Markus Armbruster:

Quoted 8 lines

We abort() on memory allocation failure. abort() is appropriate for programming errors. Maybe most memory allocation failures are programming errors, maybe not. But guest memory allocation failure isn't, and aborting when the user asks for more memory than we can provide is not nice. exit(1) instead, and do it in just one place, so the error message is consistent.
Signed-off-by Markus Armbruster <[email protected]>

Acked-by Andreas Färber <[email protected]>

Thanks for looking into this!

Andreas

Signature · 2 lines

SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Richard Henderson

2013-06-13 16:02 UTC #17 lore raw

Re: [Qemu-devel] [PATCH RFC 7/8] exec: Don't abort when we can't allocate guest memory

In reply to Markus Armbruster

exec.c static void *file_ram_alloc(RAMBlock *block, · line 958

Quoted 9 lines · Markus Armbruster

exec.c +1 -1

@@ -945,7 +952,7 @@ static void *file_ram_alloc(RAMBlock *block,
     area = mmap(0, memory, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
 #endif
     if (area == MAP_FAILED) {
-        perror("file_ram_alloc: can't mmap RAM pages");
+        no_guest_mem(block);
         close(fd);
         return (NULL);
     }

Dead code after no_guest_mem.

Markus Armbruster

author

2013-06-13 17:27 UTC #18 lore raw

Re: [Qemu-devel] [PATCH RFC 7/8] exec: Don't abort when we can't allocate guest memory

Quoted 12 lines · Richard Henderson

exec.c static void *file_ram_alloc(RAMBlock *block, · line 958

Quoted 9 lines · Markus Armbruster

exec.c +1 -1

@@ -945,7 +952,7 @@ static void *file_ram_alloc(RAMBlock *block,
     area = mmap(0, memory, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
 #endif
     if (area == MAP_FAILED) {
-        perror("file_ram_alloc: can't mmap RAM pages");
+        no_guest_mem(block);
         close(fd);
         return (NULL);
     }

Dead code after no_guest_mem.

Right you are.

Before: if allocation obeying -mem-path fails, we retry without -mem-path. Some failures are silent.

After: if mmap() fails, we give up. Functional change not mentioned in commit message. I'll either revert it or document it.

I'm not sure falling back to non-mem-path allocation is a good idea in all failure cases.

Thanks!

Peter Maydell

2013-06-13 16:21 UTC #19 lore raw

Re: [Qemu-devel] [PATCH RFC 7/8] exec: Don't abort when we can't allocate guest memory

In reply to Markus Armbruster

exec.c void qemu_mutex_unlock_ramlist(void) · line 859

Quoted 8 lines · Markus Armbruster

#ifdef __linux__

exec.c +6 -0

+static void no_guest_mem(RAMBlock *block)
+{
+    fprintf(stderr, "Cannot set up guest memory '%s': %s\n",
+            block->mr->name, strerror(errno));
+    exit(1);
+}

This new error message is inside an #ifdef __linux__...

util/oslib-posix.c void *qemu_anon_ram_alloc(size_t size) · line 115

Quoted 10 lines

util/oslib-posix.c +1 -3

@@ -112,9 +112,7 @@ void *qemu_anon_ram_alloc(size_t size)
     size_t offset = QEMU_ALIGN_UP((uintptr_t)ptr, align) - (uintptr_t)ptr;
 
     if (ptr == MAP_FAILED) {
-        fprintf(stderr, "Failed to allocate %zu B: %s\n",
-                size, strerror(errno));
-        abort();
+        return NULL;

...but this error message which got removed is not. Shouldn't we be reporting errors in a non-host-specific bit of code?

thanks
-- PMM

Markus Armbruster

author

2013-06-13 17:27 UTC #20 lore raw

Re: [Qemu-devel] [PATCH RFC 7/8] exec: Don't abort when we can't allocate guest memory

Quoted 11 lines · Peter Maydell

exec.c void qemu_mutex_unlock_ramlist(void) · line 859
Quoted 8 lines · Markus Armbruster
#ifdef __linux__
exec.c +6 -0
+static void no_guest_mem(RAMBlock *block)
+{
+    fprintf(stderr, "Cannot set up guest memory '%s': %s\n",
+            block->mr->name, strerror(errno));
+    exit(1);
+}
This new error message is inside an #ifdef __linux__...

I'm afraid this won't compile on non-linux hosts. Will respin. Thanks!

[...]

Markus Armbruster

author

2013-06-13 07:02 UTC #21 lore raw

RFC PATCH 8/8 pc_sysfw: Fix ISA BIOS init for ridiculously big flash

In reply to Markus Armbruster

pc_isa_bios_init() suffers integer overflow for flash larger than INT_MAX.

Signed-off-by Markus Armbruster <[email protected]>

---
 hw/block/pc_sysfw.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

hw/block/pc_sysfw.c +1 -4

index 412d1b0..aebefc9 100644
@@ -54,10 +54,7 @@ static void pc_isa_bios_init(MemoryRegion *rom_memory,
     flash_size = memory_region_size(flash_mem);
 
     /* map the last 128KB of the BIOS in ISA space */
-    isa_bios_size = flash_size;
-    if (isa_bios_size > (128 * 1024)) {
-        isa_bios_size = 128 * 1024;
-    }
+    isa_bios_size = MIN(flash_size, 128 * 1024);
     isa_bios = g_malloc(sizeof(*isa_bios));
     memory_region_init_ram(isa_bios, "isa-bios", isa_bios_size);
     vmstate_register_ram_global(isa_bios);

Signature · 1 line

1.7.11.7

Markus Armbruster

author

2013-06-13 07:05 UTC #22 lore raw

In reply to Markus Armbruster

Err, the RFC is accidental. Please review anyway. I'll respin for commit afterwards.