Skip to contentSkip to conversation

port qemu-kvm's -mem-path and -mem-prealloc to qemu

PATCH Applied Marcelo Tosatti 3 participants 8 messages Feb 24, 2010 – Mar 1, 2010 KVM

lore
Share & tools
lore

Marcelo Tosatti

author
#0 lore raw

cover port qemu-kvm's -mem-path and -mem-prealloc to qemu

-mem-path option allows file backed guest memory.

author
#1 lore raw

PATCH 1/2 Allocate memory below 4GB as one chunk

From: Avi Kivity <[email protected]>

Instead of allocating a separate chunk for the first 640KB and another for 1MB+, allocate one large chunk. This plays well in terms of alignment and size with large pages.

Signed-off-by Avi Kivity <[email protected]>
(cherry picked from commit cfe0cef63988a7876a9bbcb098500a3983e63814)
hw/pc.c +2 -9
Index: qemu-kvm/hw/pc.c
===================================================================
@@ -798,18 +798,11 @@ static void pc_init1(ram_addr_t ram_size
     vmport_init();
 
     /* allocate RAM */
-    ram_addr = qemu_ram_alloc(0xa0000);
+    ram_addr = qemu_ram_alloc(below_4g_mem_size);
     cpu_register_physical_memory(0, 0xa0000, ram_addr);
-
-    /* Allocate, even though we won't register, so we don't break the
-     * phys_ram_base + PA assumption. This range includes vga (0xa0000 - 0xc0000),
-     * and some bios areas, which will be registered later
-     */
-    ram_addr = qemu_ram_alloc(0x100000 - 0xa0000);
-    ram_addr = qemu_ram_alloc(below_4g_mem_size - 0x100000);
     cpu_register_physical_memory(0x100000,
                  below_4g_mem_size - 0x100000,
-                 ram_addr);
+                 ram_addr + 0x100000);
 
     /* above 4giga memory allocation */
     if (above_4g_mem_size > 0) {
#2 lore raw

Re: [patch uq/master 1/2] Allocate memory below 4GB as one chunk

Quoted 9 lines · Marcelo Tosatti

From: Avi Kivity<[email protected]>

Instead of allocating a separate chunk for the first 640KB and another for 1MB+, allocate one large chunk. This plays well in terms of alignment and size with large pages.

Signed-off-by Avi Kivity<[email protected]>
(cherry picked from commit cfe0cef63988a7876a9bbcb098500a3983e63814)

Note: qemu-kvm.git commit hashes are not meaningful in qemu.git, so don't add them to uq commit logs please.

Signature · 1 line
error compiling committee.c: too many arguments to function
author
#3 lore raw

PATCH 2/2 Add option to use file backed guest memory

In reply to Marcelo Tosatti

Port qemu-kvm's -mem-path and -mem-prealloc options. These are useful for backing guest memory with huge pages via hugetlbfs.

Signed-off-by Marcelo Tosatti <[email protected]>
CC john cooper <[email protected]>
cpu-all.h +3 -0
Index: qemu-kvm/cpu-all.h
===================================================================
@@ -850,6 +850,9 @@ extern uint8_t *phys_ram_dirty;
 extern ram_addr_t ram_size;
 extern ram_addr_t last_ram_offset;
 
+extern const char *mem_path;
+extern int mem_prealloc;
+
 /* physical memory access */
 
 /* MMIO pages are identified by a combination of an IO device index and
exec.c +114 -5

136 lines not on this page · raw message

qemu-options.hx +16 -0
Index: qemu-kvm/qemu-options.hx
===================================================================
@@ -314,6 +314,22 @@ a suffix of ``M'' or ``G'' can be used t
 gigabytes respectively.
 ETEXI
 
+DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath,
+    "-mem-path FILE  provide backing storage for guest RAM\n")
+STEXI
+@item -mem-path @var{path}
+Allocate guest RAM from a temporarily created file in @var{path}.
+ETEXI
+
+#ifdef MAP_POPULATE
+DEF("mem-prealloc", 0, QEMU_OPTION_mem_prealloc,
+    "-mem-prealloc   preallocate guest memory (use with -mem-path)\n")
+STEXI
+@item -mem-prealloc
+Preallocate memory when using -mem-path.
+ETEXI
+#endif
+
 DEF("k", HAS_ARG, QEMU_OPTION_k,
     "-k language     use keyboard layout (for example 'fr' for French)\n")
 STEXI
vl.c +11 -0
Index: qemu-kvm/vl.c
===================================================================
@@ -186,6 +186,10 @@ static DisplayState *display_state;
 DisplayType display_type = DT_DEFAULT;
 const char* keyboard_layout = NULL;
 ram_addr_t ram_size;
+const char *mem_path = NULL;
+#ifdef MAP_POPULATE
+int mem_prealloc = 1; /* force preallocation of physical target memory */
+#endif
 int nb_nics;
 NICInfo nd_table[MAX_NICS];
 int vm_running;

@@ -5252,6 +5256,13 @@ int main(int argc, char **argv, char **e
                 ram_size = value;
                 break;
             }
+            case QEMU_OPTION_mempath:
+                mem_path = optarg;
+                break;
+#ifdef MAP_POPULATE
+            case QEMU_OPTION_mem_prealloc:
+                mem_prealloc = !mem_prealloc;
+#endif
             case QEMU_OPTION_d:
                 {
                     int mask;
#4 lore raw

Re: [Qemu-devel] [patch uq/master 2/2] Add option to use file backed guest memory

exec.c void qemu_flush_coalesced_mmio_buffer(vo · line 2592

Quoted 8 lines
exec.c +8 -0
+    /*
+     * ftruncate is not supported by hugetlbfs in older
+     * hosts, so don't bother checking for errors.
+     * If anything goes wrong with it under other filesystems,
+     * mmap will fail.
+     */
+    if (ftruncate(fd, memory))
+       perror("ftruncate");

Code does not match comment.

exec.c void qemu_flush_coalesced_mmio_buffer(vo · line 2572

Quoted 3 lines
exec.c +3 -0
+    if (asprintf(&filename, "%s/kvm.XXXXXX", path) == -1) {
+       return NULL;
+    }

This isn't kvm any more :-)

Quoted 1 line

+ flags = mem_prealloc ? MAP_POPULATE|MAP_SHARED : MAP_PRIVATE;

Missing spaces round logic operator (plus several other occurrences).

exec.c void qemu_flush_coalesced_mmio_buffer(vo · line 2554

Quoted 4 lines
exec.c +4 -0
+static void *file_ram_alloc(ram_addr_t memory, const char *path)
+{
+    return NULL;
+}

Silently ignoring commandline options is bad.
Especially as the other option you added (-mem-prealloc) causes an error if not supported.

exec.c void qemu_flush_coalesced_mmio_buffer(vo · line 2568

Quoted 4 lines
exec.c +3 -1
+    if (kvm_enabled() && !kvm_has_sync_mmu()) {
+        fprintf(stderr, "kvm: host lacks mmu notifiers, disabling
-mem-path\n"); +        return NULL;
+    }

Code does not match error message. Users are liable to see this many times.

Quoted 1 line

+ new_block->host = file_ram_alloc(size, mem_path);

IMHO it would be better to check the mem_path != NULL here, rather that burying the check in file_ram_alloc.

exec.c void qemu_flush_coalesced_mmio_buffer(vo · line 2563

Quoted 3 lines
exec.c +3 -0
+    if (memory < hpagesize) {
+        return NULL;
+    }

Ah, so it's actually "allocate memory in $path, if you feel like it". Good job we aren't relying on this for correctness. At minimum I recommend documenting this heuristic.

Quoted 3 lines

+ if (!new_block->host) {
#if defined(TARGET_S390X) && defined(CONFIG_KVM)
- /* XXX S390 KVM requires the topmost vma of the RAM to be < 256GB */

By my reading this implies -mempath is probably broken on s390 KVM?

qemu-options.hx a suffix of ``M'' or ``G'' can be used t · line 322

Quoted 6 lines
qemu-options.hx +6 -0
+DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath,
+    "-mem-path FILE  provide backing storage for guest RAM\n")
+STEXI
+@item -mem-path @var{path}
+Allocate guest RAM from a temporarily created file in @var{path}.
+ETEXI

You should mention that this is only useful when PATH happens to be a linux hugetlbfs mount.

vl.c int main(int argc, char **argv, char **e · line 5265

Quoted 4 lines
vl.c +4 -0
+#ifdef MAP_POPULATE
+            case QEMU_OPTION_mem_prealloc:
+                mem_prealloc = !mem_prealloc;
+#endif

This looks highly suspect. Having redundant options toggle the sate seems like a particularly bad UI.

Paul

author
#5 lore raw

Re: [Qemu-devel] [patch uq/master 2/2] Add option to use file backed guest memory

Hi Paul,

Thank you for reviewing.

Quoted 10 lines · Paul Brook

IMHO it would be better to check the mem_path != NULL here, rather that burying the check in file_ram_alloc.

Review

Quoted 3 lines
(quoted diff) +3 -0
+    if (memory < hpagesize) {
+        return NULL;
+    }

Ah, so it's actually "allocate memory in $path, if you feel like it". Good job we aren't relying on this for correctness. At minimum I recommend documenting this heuristic.

More like "allocate memory in $path, if it its larger than a hugepage."

Huge pages are an optimization.

Quoted 7 lines
Quoted 4 lines

+ if (!new_block->host) {
#if defined(TARGET_S390X) && defined(CONFIG_KVM)
- /* XXX S390 KVM requires the topmost vma of the RAM to be < 256GB */

By my reading this implies -mempath is probably broken on s390 KVM?

Quoted 9 lines

Review

Quoted 6 lines
(quoted diff) +6 -0
+DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath,
+    "-mem-path FILE  provide backing storage for guest RAM\n")
+STEXI
+@item -mem-path @var{path}
+Allocate guest RAM from a temporarily created file in @var{path}.
+ETEXI

You should mention that this is only useful when PATH happens to be a linux hugetlbfs mount.

It can be used with a file, since its mapped as MAP_PRIVATE.

Can you check whether the patch below properly addresses your concerns.

Add option to use file backed guest memory

Port qemu-kvm's -mem-path and -mem-prealloc options. These are useful for backing guest memory with huge pages via hugetlbfs.

Signed-off-by Marcelo Tosatti <[email protected]>
CC john cooper <[email protected]>
cpu-all.h +3 -0
Index: qemu/cpu-all.h
===================================================================
@@ -847,6 +847,9 @@ extern uint8_t *phys_ram_dirty;
 extern ram_addr_t ram_size;
 extern ram_addr_t last_ram_offset;
 
+extern const char *mem_path;
+extern int mem_prealloc;
+
 /* physical memory access */
 
 /* MMIO pages are identified by a combination of an IO device index and
exec.c +110 -5

132 lines not on this page · raw message

qemu-options.hx +16 -0
Index: qemu/qemu-options.hx
===================================================================
@@ -314,6 +314,22 @@ a suffix of ``M'' or ``G'' can be used t
 gigabytes respectively.
 ETEXI
 
+DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath,
+    "-mem-path FILE  provide backing storage for guest RAM\n")
+STEXI
+@item -mem-path @var{path}
+Allocate guest RAM from a temporarily created file in @var{path}.
+ETEXI
+
+#ifdef MAP_POPULATE
+DEF("mem-prealloc", 0, QEMU_OPTION_mem_prealloc,
+    "-mem-prealloc   preallocate guest memory (use with -mem-path)\n")
+STEXI
+@item -mem-prealloc
+Preallocate memory when using -mem-path.
+ETEXI
+#endif
+
 DEF("k", HAS_ARG, QEMU_OPTION_k,
     "-k language     use keyboard layout (for example 'fr' for French)\n")
 STEXI
vl.c +12 -0
Index: qemu/vl.c
===================================================================
@@ -185,6 +185,10 @@ enum vga_retrace_method vga_retrace_meth
 DisplayType display_type = DT_DEFAULT;
 const char* keyboard_layout = NULL;
 ram_addr_t ram_size;
+const char *mem_path = NULL;
+#ifdef MAP_POPULATE
+int mem_prealloc = 0; /* force preallocation of physical target memory */
+#endif
 int nb_nics;
 NICInfo nd_table[MAX_NICS];
 int vm_running;

@@ -5216,6 +5220,14 @@ int main(int argc, char **argv, char **e
                 ram_size = value;
                 break;
             }
+            case QEMU_OPTION_mempath:
+                mem_path = optarg;
+                break;
+#ifdef MAP_POPULATE
+            case QEMU_OPTION_mem_prealloc:
+                mem_prealloc = 1;
+                break;
+#endif
             case QEMU_OPTION_d:
                 {
                     int mask;
author
#6 lore raw

Re: [Qemu-devel] [patch uq/master 2/2] Add option to use file backed guest memory

Quoted 37 lines · Marcelo Tosatti

Hi Paul,

Thank you for reviewing.

Quoted 10 lines · Paul Brook

IMHO it would be better to check the mem_path != NULL here, rather that burying the check in file_ram_alloc.

exec.c void qemu_flush_coalesced_mmio_buffer(vo · line 2575

Quoted 3 lines
exec.c +3 -0
+    if (memory < hpagesize) {
+        return NULL;
+    }

Ah, so it's actually "allocate memory in $path, if you feel like it". Good job we aren't relying on this for correctness. At minimum I recommend documenting this heuristic.

More like "allocate memory in $path, if it its larger than a hugepage."

Huge pages are an optimization.

Quoted 7 lines
Quoted 4 lines

+ if (!new_block->host) {
#if defined(TARGET_S390X) && defined(CONFIG_KVM)
- /* XXX S390 KVM requires the topmost vma of the RAM to be < 256GB */

By my reading this implies -mempath is probably broken on s390 KVM?

Quoted 9 lines

qemu-options.hx a suffix of ``M'' or ``G'' can be used t · line 322

Quoted 6 lines
qemu-options.hx +6 -0
+DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath,
+    "-mem-path FILE  provide backing storage for guest RAM\n")
+STEXI
+@item -mem-path @var{path}
+Allocate guest RAM from a temporarily created file in @var{path}.
+ETEXI

You should mention that this is only useful when PATH happens to be a linux hugetlbfs mount.

Quoted 1 line

It can be used with a file, since its mapped as MAP_PRIVATE.

I meant non hugetlbfs backed file.

Applied
#7 lore raw
In reply to Marcelo Tosatti
Quoted 2 lines · Marcelo Tosatti

-mem-path option allows file backed guest memory.

Applied, thanks.

Signature · 1 line
error compiling committee.c: too many arguments to function