[RFC 0/9] Memory registration rework

[RFC 0/9] Memory registration rework

[Glauber Costa]

Hi folks,

The following series contain a proposal for our memory registration
framework. This is by no means complete, and rather, a first step only.

This first step, btw, has the goal of taking the kvm-specific memory registration
functions from all over the code, so we can make the merging with qemu easier.

Note that I'm putting kvm_cpu_register_phys_memory() _inside_ cpu_register_phys_memory().
To do that, we need to be resilient against the same region being registered multiple times,
and should be able to interpret the flags embedded in phys_offset in a meaninful way.
Although arguably with some bugs yet unknown, this series does exactly that.

For that to work, we have to be sure that we'll never reach a situation in which we
register a piece of memory, and later on, register another region that contains it. Current
code does that, so we're fine. The oposite situation, namely, registering a large piece of memory
and then re-registering pieces of it, is perfectly valid.

In the to-be-merged version, if it ever exists, I intend to comment all those issues very well,
to get an as predictable interface as possible.

There's another option of doing this, as anthony pointed out in earlier private comments to me,
which is scanning the already registered regions right before starting execution, and building our
maps. While this is valid, we can't run away from doing what I'm doing, because some areas are
manipulated _after_ the machine has started. For example, the pci region, for the hotplug case.

Note that this is not tested in anything but x86. 

Comments welcome.

[PATCH 1/9] add debuging facilities to memory registration at libkvm

[Glauber Costa]

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 libkvm/libkvm.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
index 5edfad7..0bacb43 100644
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -52,6 +52,8 @@
 #include "kvm-s390.h"
 #endif
 
+//#define DEBUG_MEMREG
+
 int kvm_abi = EXPECTED_KVM_API_VERSION;
 int kvm_page_size;
 
@@ -548,6 +550,12 @@ int kvm_create_mem_hole(kvm_context_t kvm, unsigned long phys_start,
 	}
 	free_slot(slot);
 
+#ifdef DEBUG_MEMREG
+	printf("%s, newslot1: gpa: %llx, size: %llx, uaddr: %llx, slot: %x, flags: %lx\n",
+		__func__, newslot1.guest_phys_addr, newslot1.memory_size,
+		newslot1.userspace_addr, newslot1.slot, newslot1.flags);
+#endif
+
 	r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &newslot1);
 	if (r == -1) {
 		fprintf(stderr, "kvm_create_mem_hole: %s\n", strerror(errno));
@@ -557,6 +565,11 @@ int kvm_create_mem_hole(kvm_context_t kvm, unsigned long phys_start,
 		      newslot1.memory_size, 1, newslot1.userspace_addr,
 		      newslot1.flags);
 
+#ifdef DEBUG_MEMREG
+	printf("%s, newslot2: gpa: %llx, size: %llx, uaddr: %llx, slot: %x, flags: %lx\n",
+		__func__, newslot2.guest_phys_addr, newslot2.memory_size,
+		newslot2.userspace_addr, newslot2.slot, newslot2.flags);
+#endif
 	r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &newslot2);
 	if (r == -1) {
 		fprintf(stderr, "kvm_create_mem_hole: %s\n", strerror(errno));
@@ -584,6 +597,11 @@ int kvm_register_userspace_phys_mem(kvm_context_t kvm,
 	int r;
 
 	memory.slot = get_free_slot(kvm);
+#ifdef DEBUG_MEMREG
+	printf("%s, memory: gpa: %llx, size: %llx, uaddr: %llx, slot: %x, flags: %lx\n",
+		__func__, memory.guest_phys_addr, memory.memory_size,
+		memory.userspace_addr, memory.slot, memory.flags);
+#endif
 	r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &memory);
 	if (r == -1) {
 		fprintf(stderr, "create_userspace_phys_mem: %s\n", strerror(errno));
-- 
1.5.5.1

[PATCH 2/9] experimental change to avoid doing the same thing twice

[Glauber Costa]

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 qemu/qemu-kvm.c |   31 +++++++++++--------------------
 1 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index 1d07650..f24c436 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -779,28 +779,19 @@ void kvm_cpu_register_physical_memory(target_phys_addr_t start_addr,
 #ifdef KVM_CAP_USER_MEMORY
     int r = 0;
 
+
     r = kvm_check_extension(kvm_context, KVM_CAP_USER_MEMORY);
     if (r) {
-        if (!(phys_offset & ~TARGET_PAGE_MASK)) {
-                r = kvm_is_allocated_mem(kvm_context, start_addr, size);
-            if (r)
-                return;
-            r = kvm_is_intersecting_mem(kvm_context, start_addr);
-            if (r)
-                kvm_create_mem_hole(kvm_context, start_addr, size);
-            r = kvm_register_userspace_phys_mem(kvm_context, start_addr,
-                                                phys_ram_base + phys_offset,
-                                                size, 0);
-        }
-        if (phys_offset & IO_MEM_ROM) {
-            phys_offset &= ~IO_MEM_ROM;
-            r = kvm_is_intersecting_mem(kvm_context, start_addr);
-            if (r)
-                kvm_create_mem_hole(kvm_context, start_addr, size);
-            r = kvm_register_userspace_phys_mem(kvm_context, start_addr,
-                                                phys_ram_base + phys_offset,
-                                                size, 0);
-        }
+        phys_offset &= ~IO_MEM_ROM;
+        r = kvm_is_allocated_mem(kvm_context, start_addr, size);
+        if (r)
+            return;
+        r = kvm_is_intersecting_mem(kvm_context, start_addr);
+        if (r)
+            kvm_create_mem_hole(kvm_context, start_addr, size);
+        r = kvm_register_userspace_phys_mem(kvm_context, start_addr,
+                                            phys_ram_base + phys_offset,
+                                            size, 0);
         if (r < 0) {
             printf("kvm_cpu_register_physical_memory: failed\n");
             exit(1);
-- 
1.5.5.1

[PATCH 3/9] do not use mem_hole anymore.

[Glauber Costa]

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 libkvm/libkvm.c |   78 -------------------------------------------------------
 qemu/qemu-kvm.c |   11 ++++---
 2 files changed, 6 insertions(+), 83 deletions(-)

diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
index 0bacb43..eb85445 100644
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -504,84 +504,6 @@ int kvm_is_allocated_mem(kvm_context_t kvm, unsigned long phys_start,
 	return 0;
 }
 
-int kvm_create_mem_hole(kvm_context_t kvm, unsigned long phys_start,
-			unsigned long len)
-{
-#ifdef KVM_CAP_USER_MEMORY
-	int slot;
-	int r;
-	struct kvm_userspace_memory_region rmslot;
-	struct kvm_userspace_memory_region newslot1;
-	struct kvm_userspace_memory_region newslot2;
-
-	len = (len + PAGE_SIZE - 1) & PAGE_MASK;
-
-	slot = get_intersecting_slot(phys_start);
-	/* no need to create hole, as there is already hole */
-	if (slot == -1)
-		return 0;
-
-	memset(&rmslot, 0, sizeof(struct kvm_userspace_memory_region));
-	memset(&newslot1, 0, sizeof(struct kvm_userspace_memory_region));
-	memset(&newslot2, 0, sizeof(struct kvm_userspace_memory_region));
-
-	rmslot.guest_phys_addr = slots[slot].phys_addr;
-	rmslot.slot = slot;
-
-	newslot1.guest_phys_addr = slots[slot].phys_addr;
-	newslot1.memory_size = phys_start - slots[slot].phys_addr;
-	newslot1.slot = slot;
-	newslot1.userspace_addr = slots[slot].userspace_addr;
-	newslot1.flags = slots[slot].flags;
-
-	newslot2.guest_phys_addr = newslot1.guest_phys_addr +
-				   newslot1.memory_size + len;
-	newslot2.memory_size = slots[slot].phys_addr +
-			       slots[slot].len - newslot2.guest_phys_addr;
-	newslot2.userspace_addr = newslot1.userspace_addr +
-				  newslot1.memory_size;
-	newslot2.slot = get_free_slot(kvm);
-	newslot2.flags = newslot1.flags;
-
-	r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &rmslot);
-	if (r == -1) {
-		fprintf(stderr, "kvm_create_mem_hole: %s\n", strerror(errno));
-		return -1;
-	}
-	free_slot(slot);
-
-#ifdef DEBUG_MEMREG
-	printf("%s, newslot1: gpa: %llx, size: %llx, uaddr: %llx, slot: %x, flags: %lx\n",
-		__func__, newslot1.guest_phys_addr, newslot1.memory_size,
-		newslot1.userspace_addr, newslot1.slot, newslot1.flags);
-#endif
-
-	r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &newslot1);
-	if (r == -1) {
-		fprintf(stderr, "kvm_create_mem_hole: %s\n", strerror(errno));
-		return -1;
-	}
-	register_slot(newslot1.slot, newslot1.guest_phys_addr,
-		      newslot1.memory_size, 1, newslot1.userspace_addr,
-		      newslot1.flags);
-
-#ifdef DEBUG_MEMREG
-	printf("%s, newslot2: gpa: %llx, size: %llx, uaddr: %llx, slot: %x, flags: %lx\n",
-		__func__, newslot2.guest_phys_addr, newslot2.memory_size,
-		newslot2.userspace_addr, newslot2.slot, newslot2.flags);
-#endif
-	r = ioctl(kvm->vm_fd, KVM_SET_USER_MEMORY_REGION, &newslot2);
-	if (r == -1) {
-		fprintf(stderr, "kvm_create_mem_hole: %s\n", strerror(errno));
-		return -1;
-	}
-	register_slot(newslot2.slot, newslot2.guest_phys_addr,
-		      newslot2.memory_size, 1, newslot2.userspace_addr,
-		      newslot2.flags);
-#endif
-	return 0;
-}
-
 int kvm_register_userspace_phys_mem(kvm_context_t kvm,
 			unsigned long phys_start, void *userspace_addr,
 			unsigned long len, int log)
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index f24c436..e6221f8 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -787,11 +787,12 @@ void kvm_cpu_register_physical_memory(target_phys_addr_t start_addr,
         if (r)
             return;
         r = kvm_is_intersecting_mem(kvm_context, start_addr);
-        if (r)
-            kvm_create_mem_hole(kvm_context, start_addr, size);
-        r = kvm_register_userspace_phys_mem(kvm_context, start_addr,
-                                            phys_ram_base + phys_offset,
-                                            size, 0);
+        if (r) {
+            printf("Ignoring intersecting memory %llx (%lx)\n", start_addr, size);
+        } else
+            r = kvm_register_userspace_phys_mem(kvm_context, start_addr,
+                                                phys_ram_base + phys_offset,
+                                                size, 0);
         if (r < 0) {
             printf("kvm_cpu_register_physical_memory: failed\n");
             exit(1);
-- 
1.5.5.1

[PATCH 4/9] allow intersecting region to be on the boundary.

[Glauber Costa]

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 libkvm/libkvm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
index eb85445..33f00b7 100644
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -134,8 +134,8 @@ int get_intersecting_slot(unsigned long phys_addr)
 	int i;
 
 	for (i = 0; i < KVM_MAX_NUM_MEM_REGIONS ; ++i)
-		if (slots[i].len && slots[i].phys_addr < phys_addr &&
-		    (slots[i].phys_addr + slots[i].len) > phys_addr)
+		if (slots[i].len && slots[i].phys_addr <= phys_addr &&
+		    (slots[i].phys_addr + slots[i].len) >= phys_addr)
 			return i;
 	return -1;
 }
-- 
1.5.5.1

[PATCH 5/9] substitute is_allocated_mem with more general is_containing_region

[Glauber Costa]

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 libkvm/libkvm.c |   34 +++++++++++++++++++++-------------
 libkvm/libkvm.h |    2 +-
 qemu/qemu-kvm.c |    2 +-
 3 files changed, 23 insertions(+), 15 deletions(-)

diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
index 33f00b7..c885dee 100644
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -140,6 +140,27 @@ int get_intersecting_slot(unsigned long phys_addr)
 	return -1;
 }
 
+/* Returns -1 if this slot is not totally contained on any other,
+ * and the number of the slot otherwise */
+int get_container_slot(uint64_t phys_addr, unsigned long size)
+{
+	int i;
+
+	for (i = 0; i < KVM_MAX_NUM_MEM_REGIONS ; ++i)
+		if (slots[i].len && slots[i].phys_addr <= phys_addr &&
+		    (slots[i].phys_addr + slots[i].len) >= phys_addr + size)
+			return i;
+	return -1;
+}
+
+int kvm_is_containing_region(kvm_context_t kvm, unsigned long phys_addr, unsigned long size)
+{
+	int slot = get_container_slot(phys_addr, size);
+	if (slot == -1)
+		return 0;
+	return 1;
+}
+
 /* 
  * dirty pages logging control 
  */
@@ -491,19 +512,6 @@ int kvm_is_intersecting_mem(kvm_context_t kvm, unsigned long phys_start)
 	return get_intersecting_slot(phys_start) != -1;
 }
 
-int kvm_is_allocated_mem(kvm_context_t kvm, unsigned long phys_start,
-			 unsigned long len)
-{
-	int slot;
-
-	slot = get_slot(phys_start);
-	if (slot == -1)
-		return 0;
-	if (slots[slot].len == len)
-		return 1;
-	return 0;
-}
-
 int kvm_register_userspace_phys_mem(kvm_context_t kvm,
 			unsigned long phys_start, void *userspace_addr,
 			unsigned long len, int log)
diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h
index 9f06fcc..d762323 100644
--- a/libkvm/libkvm.h
+++ b/libkvm/libkvm.h
@@ -454,7 +454,7 @@ void *kvm_create_phys_mem(kvm_context_t, unsigned long phys_start,
 			  unsigned long len, int log, int writable);
 void kvm_destroy_phys_mem(kvm_context_t, unsigned long phys_start, 
 			  unsigned long len);
-int kvm_is_intersecting_mem(kvm_context_t kvm, unsigned long phys_start);
+int kvm_is_containing_region(kvm_context_t kvm, unsigned long phys_start, unsigned long size);
 int kvm_is_allocated_mem(kvm_context_t kvm, unsigned long phys_start,
 			 unsigned long len);
 int kvm_create_mem_hole(kvm_context_t kvm, unsigned long phys_start,
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index e6221f8..bfbaacc 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -783,7 +783,7 @@ void kvm_cpu_register_physical_memory(target_phys_addr_t start_addr,
     r = kvm_check_extension(kvm_context, KVM_CAP_USER_MEMORY);
     if (r) {
         phys_offset &= ~IO_MEM_ROM;
-        r = kvm_is_allocated_mem(kvm_context, start_addr, size);
+        r = kvm_is_containing_region(kvm_context, start_addr, size);
         if (r)
             return;
         r = kvm_is_intersecting_mem(kvm_context, start_addr);
-- 
1.5.5.1

[PATCH 6/9] move kvm_cpu_register_memory_area into qemu's

[Glauber Costa]

Turn the explicit calls to kvm_cpu_register_memoy_area()
an empty function. Provide a __kvm_cpu_register_memory_area()
that is called from within cpu_register_memory_area().
To avoid registering mmio regions to the hypervisor, since we depend on
them faulting, we keep track of what regions are mmio regions too.

This is to be bisection friendly. Direct calls are to be removed
in a later commit.

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 libkvm/libkvm.c |   84 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 libkvm/libkvm.h |    6 ++++
 qemu/exec.c     |    3 ++
 qemu/qemu-kvm.c |   22 ++++++++++++++
 4 files changed, 111 insertions(+), 4 deletions(-)

diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
index c885dee..d62cb2a 100644
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -65,14 +65,22 @@ struct slot_info {
 	unsigned flags;
 };
 
+struct mmio_slot_info {
+    uint64_t phys_addr;
+    unsigned int len;
+};
+
 struct slot_info slots[KVM_MAX_NUM_MEM_REGIONS];
+struct mmio_slot_info mmio_slots[KVM_MAX_NUM_MEM_REGIONS];
 
 void init_slots(void)
 {
 	int i;
 
-	for (i = 0; i < KVM_MAX_NUM_MEM_REGIONS; ++i)
+	for (i = 0; i < KVM_MAX_NUM_MEM_REGIONS; ++i) {
 		slots[i].len = 0;
+		mmio_slots[i].len = 0;
+	}
 }
 
 int get_free_slot(kvm_context_t kvm)
@@ -102,6 +110,16 @@ int get_free_slot(kvm_context_t kvm)
 	return -1;
 }
 
+int get_free_mmio_slot(kvm_context_t kvm)
+{
+
+       unsigned int i;
+       for (i = 0; i < KVM_MAX_NUM_MEM_REGIONS; ++i)
+		if (!mmio_slots[i].len)
+			return i;
+       return -1;
+}
+
 void register_slot(int slot, unsigned long phys_addr, unsigned long len,
 		   int user_alloc, unsigned long userspace_addr, unsigned flags)
 {
@@ -153,14 +171,47 @@ int get_container_slot(uint64_t phys_addr, unsigned long size)
 	return -1;
 }
 
+int get_container_mmio_slot(kvm_context_t kvm, uint64_t phys_addr, unsigned long size)
+{
+       int i;
+
+       for (i = 0; i < KVM_MAX_NUM_MEM_REGIONS ; ++i)
+               if (mmio_slots[i].len && mmio_slots[i].phys_addr <= phys_addr &&
+                   (mmio_slots[i].phys_addr + mmio_slots[i].len) >= phys_addr + size)
+                       return i;
+       return -1;
+}
+
+int kvm_register_mmio_slot(kvm_context_t kvm, uint64_t phys_addr, unsigned int size)
+{
+       int slot = get_free_mmio_slot(kvm);
+
+       if (slot == -1)
+               goto out;
+
+#ifdef DEBUG_MEMREG
+	printf("Registering mmio region %llx (%lx)\n", phys_addr, size);
+#endif
+       mmio_slots[slot].phys_addr = phys_addr;
+       mmio_slots[slot].len = size;
+out:
+       return slot;
+}
+
 int kvm_is_containing_region(kvm_context_t kvm, unsigned long phys_addr, unsigned long size)
 {
 	int slot = get_container_slot(phys_addr, size);
-	if (slot == -1)
-		return 0;
-	return 1;
+
+	if (slot != -1)
+		return 1;
+	slot = get_container_mmio_slot(kvm, phys_addr, size);
+	if (slot != -1)
+		return 1;
+
+	return 0;
 }
 
+
 /* 
  * dirty pages logging control 
  */
@@ -576,6 +627,31 @@ void kvm_destroy_phys_mem(kvm_context_t kvm, unsigned long phys_start,
 		kvm_create_kernel_phys_mem(kvm, phys_start, 0, 0, 0);
 }
 
+void kvm_unregister_memory_area(kvm_context_t kvm, uint64_t phys_addr, unsigned long size)
+{
+       
+       int slot = get_container_slot(phys_addr, size);
+
+       if (slot != -1) {
+#ifdef DEBUG_MEMREG
+               printf("Unregistering memory region %llx (%lx)\n", phys_addr, size);
+#endif
+               kvm_destroy_phys_mem(kvm, phys_addr, size);
+               return;
+       }
+
+       slot = get_container_mmio_slot(kvm, phys_addr, size);
+       if (slot != -1) {
+#ifdef DEBUG_MEMREG
+               printf("Unregistering mmio region %llx (%lx)\n", phys_addr, size);
+#endif
+               kvm_unregister_coalesced_mmio(kvm, phys_addr, size);
+               mmio_slots[slot].len = 0;
+       }
+
+       return;
+}
+
 static int kvm_get_map(kvm_context_t kvm, int ioctl_num, int slot, void *buf)
 {
 	int r;
diff --git a/libkvm/libkvm.h b/libkvm/libkvm.h
index d762323..ceadc45 100644
--- a/libkvm/libkvm.h
+++ b/libkvm/libkvm.h
@@ -454,6 +454,10 @@ void *kvm_create_phys_mem(kvm_context_t, unsigned long phys_start,
 			  unsigned long len, int log, int writable);
 void kvm_destroy_phys_mem(kvm_context_t, unsigned long phys_start, 
 			  unsigned long len);
+
+void kvm_unregister_memory_area(kvm_context_t, uint64_t phys_start, 
+			  	unsigned long len);
+
 int kvm_is_containing_region(kvm_context_t kvm, unsigned long phys_start, unsigned long size);
 int kvm_is_allocated_mem(kvm_context_t kvm, unsigned long phys_start,
 			 unsigned long len);
@@ -467,6 +471,8 @@ int kvm_get_dirty_pages_range(kvm_context_t kvm, unsigned long phys_addr,
 			      unsigned long end_addr, void *buf, void*opaque,
 			      int (*cb)(unsigned long start, unsigned long len,
 					void*bitmap, void *opaque));
+int kvm_register_mmio_slot(kvm_context_t kvm,
+				uint64_t addr, uint32_t size);
 int kvm_register_coalesced_mmio(kvm_context_t kvm,
 				uint64_t addr, uint32_t size);
 int kvm_unregister_coalesced_mmio(kvm_context_t kvm,
diff --git a/qemu/exec.c b/qemu/exec.c
index 7a68062..14c3852 100644
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -2196,6 +2196,9 @@ void cpu_register_physical_memory(target_phys_addr_t start_addr,
         kqemu_set_phys_mem(start_addr, size, phys_offset);
     }
 #endif
+
+    __kvm_cpu_register_physical_memory(start_addr, size, phys_offset);
+
     size = (size + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK;
     end_addr = start_addr + (target_phys_addr_t)size;
     for(addr = start_addr; addr != end_addr; addr += TARGET_PAGE_SIZE) {
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index bfbaacc..225fbe6 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -776,16 +776,38 @@ void kvm_cpu_register_physical_memory(target_phys_addr_t start_addr,
                                       unsigned long size,
                                       unsigned long phys_offset)
 {
+}
+
+void __kvm_cpu_register_physical_memory(target_phys_addr_t start_addr,
+                                      unsigned long size,
+                                      unsigned long phys_offset)
+{
 #ifdef KVM_CAP_USER_MEMORY
     int r = 0;
 
 
     r = kvm_check_extension(kvm_context, KVM_CAP_USER_MEMORY);
     if (r) {
+        unsigned long area_flags = phys_offset & ~TARGET_PAGE_MASK;
         phys_offset &= ~IO_MEM_ROM;
+
+        if (area_flags == IO_MEM_UNASSIGNED) {
+            kvm_unregister_memory_area(kvm_context, start_addr, size);
+            return;
+        }
+
         r = kvm_is_containing_region(kvm_context, start_addr, size);
         if (r)
             return;
+
+        if (area_flags >= TLB_MMIO) {
+            r = kvm_register_mmio_slot(kvm_context, start_addr, size);
+            if (r < 0) {
+                printf("No free mmio slots\n");
+                exit(1);
+            }
+            return;
+        }
         r = kvm_is_intersecting_mem(kvm_context, start_addr);
         if (r) {
             printf("Ignoring intersecting memory %llx (%lx)\n", start_addr, size);
-- 
1.5.5.1

[PATCH 7/9] cleanup kvm memory registration

[Glauber Costa]

Remove all references to kvm_cpu_register_physical_memory(),
since it is called from within cpu_register_physical_memory().

FIXME: There are leftovers in ppc and ia64. Technicaly, anyone should
test it in one of those arches, to make sure it work.

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 qemu/exec.c             |    2 +-
 qemu/hw/ipf.c           |    7 -------
 qemu/hw/pc.c            |   23 ++---------------------
 qemu/hw/ppc440_bamboo.c |    2 --
 qemu/qemu-kvm.c         |    6 ------
 5 files changed, 3 insertions(+), 37 deletions(-)

diff --git a/qemu/exec.c b/qemu/exec.c
index 14c3852..0cba1a3 100644
--- a/qemu/exec.c
+++ b/qemu/exec.c
@@ -2197,7 +2197,7 @@ void cpu_register_physical_memory(target_phys_addr_t start_addr,
     }
 #endif
 
-    __kvm_cpu_register_physical_memory(start_addr, size, phys_offset);
+    kvm_cpu_register_physical_memory(start_addr, size, phys_offset);
 
     size = (size + TARGET_PAGE_SIZE - 1) & TARGET_PAGE_MASK;
     end_addr = start_addr + (target_phys_addr_t)size;
diff --git a/qemu/hw/ipf.c b/qemu/hw/ipf.c
index b11e328..80e6315 100644
--- a/qemu/hw/ipf.c
+++ b/qemu/hw/ipf.c
@@ -421,18 +421,14 @@ static void ipf_init1(ram_addr_t ram_size, int vga_ram_size,
 	if (kvm_enabled() && kvm_qemu_check_extension(KVM_CAP_USER_MEMORY)) {
 		ram_addr = qemu_ram_alloc(0xa0000);
 		cpu_register_physical_memory(0, 0xa0000, ram_addr);
-		kvm_cpu_register_physical_memory(0, 0xa0000, ram_addr);
 
 		ram_addr = qemu_ram_alloc(0x20000); // Workaround 0xa0000-0xc0000
 
 		ram_addr = qemu_ram_alloc(0x40000);
 		cpu_register_physical_memory(0xc0000, 0x40000, ram_addr);
-		kvm_cpu_register_physical_memory(0xc0000, 0x40000, ram_addr);
 
 		ram_addr = qemu_ram_alloc(ram_size - 0x100000);
 		cpu_register_physical_memory(0x100000, ram_size - 0x100000, ram_addr);
-		kvm_cpu_register_physical_memory(0x100000, ram_size - 0x100000,
-				ram_addr);
 	} else
 #endif
     {
@@ -446,9 +442,6 @@ static void ipf_init1(ram_addr_t ram_size, int vga_ram_size,
 	if (above_4g_mem_size > 0) {
 		ram_addr = qemu_ram_alloc(above_4g_mem_size);
 		cpu_register_physical_memory(0x100000000, above_4g_mem_size, ram_addr);
-	if (kvm_enabled())
-		kvm_cpu_register_physical_memory(0x100000000, above_4g_mem_size,
-					ram_addr);
 	}
 
 	/*Load firware to its proper position.*/
diff --git a/qemu/hw/pc.c b/qemu/hw/pc.c
index 3a8269b..98c279f 100644
--- a/qemu/hw/pc.c
+++ b/qemu/hw/pc.c
@@ -740,9 +740,6 @@ static int load_option_rom(const char *filename, int offset, int type)
     size = (size + 4095) & ~4095;
     cpu_register_physical_memory(0xd0000 + offset,
 				 size, option_rom_offset | type);
-    if (kvm_enabled())
-	    kvm_cpu_register_physical_memory(0xd0000 + offset,
-                                             size, option_rom_offset | type);
     return size;
 }
 
@@ -817,16 +814,13 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
     if (kvm_enabled() && kvm_qemu_check_extension(KVM_CAP_USER_MEMORY)) {
         ram_addr = qemu_ram_alloc(0xa0000);
         cpu_register_physical_memory(0, 0xa0000, ram_addr);
-        kvm_cpu_register_physical_memory(0, 0xa0000, ram_addr);
 
         ram_addr = qemu_ram_alloc(0x100000 - 0xa0000);   // hole
         ram_addr = qemu_ram_alloc(below_4g_mem_size - 0x100000);
         cpu_register_physical_memory(0x100000,
 				     below_4g_mem_size - 0x100000,
 				     ram_addr);
-        kvm_cpu_register_physical_memory(0x100000,
-					 below_4g_mem_size - 0x100000,
-                                         ram_addr);
+
         /* above 4giga memory allocation */
         if (above_4g_mem_size > 0) {
             ram_addr = qemu_ram_alloc(above_4g_mem_size);
@@ -842,9 +836,6 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
             cpu_register_physical_memory(0x100000000ULL,
                                          above_4g_mem_size,
                                          ram_addr);
-            kvm_cpu_register_physical_memory(0x100000000ULL,
-					     above_4g_mem_size,
-                                             ram_addr);
         }
     } else
 #endif
@@ -899,9 +890,6 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
     /* setup basic memory access */
     cpu_register_physical_memory(0xc0000, 0x10000,
                                  vga_bios_offset | IO_MEM_ROM);
-    if (kvm_enabled())
-        kvm_cpu_register_physical_memory(0xc0000, 0x10000,
-                                         vga_bios_offset | IO_MEM_ROM);
 
     /* map the last 128KB of the BIOS in ISA space */
     isa_bios_size = bios_size;
@@ -913,10 +901,6 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
     cpu_register_physical_memory(0x100000 - isa_bios_size,
                                  isa_bios_size,
                                  (bios_offset + bios_size - isa_bios_size) /* | IO_MEM_ROM */);
-    if (kvm_enabled())
-        kvm_cpu_register_physical_memory(0x100000 - isa_bios_size,
-                                         isa_bios_size,
-                                         (bios_offset + bios_size - isa_bios_size) | IO_MEM_ROM);
 
     /* XXX: for DDIM support, "ROM space" should be writable during
        initialization, and (optionally) marked readonly by the BIOS
@@ -939,10 +923,7 @@ static void pc_init1(ram_addr_t ram_size, int vga_ram_size,
         int r;
 #ifdef KVM_CAP_USER_MEMORY
         r = kvm_qemu_check_extension(KVM_CAP_USER_MEMORY);
-        if (r)
-            kvm_cpu_register_physical_memory((uint32_t)(-bios_size),
-                                           bios_size, bios_offset | IO_MEM_ROM);
-        else
+        if (!r)
 #endif
         {
             bios_mem = kvm_cpu_create_phys_mem((uint32_t)(-bios_size),
diff --git a/qemu/hw/ppc440_bamboo.c b/qemu/hw/ppc440_bamboo.c
index 9ff6f7d..deafc5f 100644
--- a/qemu/hw/ppc440_bamboo.c
+++ b/qemu/hw/ppc440_bamboo.c
@@ -93,8 +93,6 @@ void bamboo_init(ram_addr_t ram_size, int vga_ram_size,
 
 	/* Register mem */
 	cpu_register_physical_memory(0, ram_size, 0);
-	if (kvm_enabled())
-	  kvm_cpu_register_physical_memory(0, ram_size, 0);
 
 	/* load kernel with uboot loader */
 	printf("%s: load kernel\n", __func__);
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index 225fbe6..96b622b 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -776,12 +776,6 @@ void kvm_cpu_register_physical_memory(target_phys_addr_t start_addr,
                                       unsigned long size,
                                       unsigned long phys_offset)
 {
-}
-
-void __kvm_cpu_register_physical_memory(target_phys_addr_t start_addr,
-                                      unsigned long size,
-                                      unsigned long phys_offset)
-{
 #ifdef KVM_CAP_USER_MEMORY
     int r = 0;
 
-- 
1.5.5.1

[PATCH 8/9] coalesce mmio regions without an explicit call

[Glauber Costa]

Try to coalesce mmio regions inside kvm_cpu_register_physical_memory().
Coalescing is done if area has TLB_MMIO flags set, or anything greater than that.

The original explicit function turns into an empty function. This is to be
bisection friendly. Direct calls are to be removed in a later commit.

Signed-off-by: Glauber Costa <gcosta@redhat.com>
---
 libkvm/libkvm.c |    4 ++++
 qemu/qemu-kvm.c |   13 +++----------
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
index d62cb2a..c563bb6 100644
--- a/libkvm/libkvm.c
+++ b/libkvm/libkvm.c
@@ -1107,6 +1107,9 @@ int kvm_register_coalesced_mmio(kvm_context_t kvm, uint64_t addr, uint32_t size)
 			perror("kvm_register_coalesced_mmio_zone");
 			return -errno;
 		}
+#ifdef DEBUG_MEMREG
+		printf("Registered coalesced mmio region for %llx (%lx)\n", addr, size);
+#endif
 		return 0;
 	}
 #endif
@@ -1129,6 +1132,7 @@ int kvm_unregister_coalesced_mmio(kvm_context_t kvm, uint64_t addr, uint32_t siz
 			perror("kvm_unregister_coalesced_mmio_zone");
 			return -errno;
 		}
+		printf("Unregistered coalesced mmio region for %llx (%lx)\n", addr, size);
 		return 0;
 	}
 #endif
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
index 96b622b..29c5c1d 100644
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -800,6 +800,7 @@ void kvm_cpu_register_physical_memory(target_phys_addr_t start_addr,
                 printf("No free mmio slots\n");
                 exit(1);
             }
+            kvm_register_coalesced_mmio(kvm_context, start_addr, size);
             return;
         }
         r = kvm_is_intersecting_mem(kvm_context, start_addr);
@@ -1035,13 +1036,5 @@ void kvm_mutex_lock(void)
     cpu_single_env = NULL;
 }
 
-int qemu_kvm_register_coalesced_mmio(target_phys_addr_t addr, unsigned int size)
-{
-    return kvm_register_coalesced_mmio(kvm_context, addr, size);
-}
-
-int qemu_kvm_unregister_coalesced_mmio(target_phys_addr_t addr,
-				       unsigned int size)
-{
-    return kvm_unregister_coalesced_mmio(kvm_context, addr, size);
-}
+int qemu_kvm_register_coalesced_mmio(target_phys_addr_t addr, unsigned int size) {}
+int qemu_kvm_unregister_coalesced_mmio(target_phys_addr_t addr, unsigned int size) {}
-- 
1.5.5.1

[PATCH 9/9] remove explicit calls to kvm_qemu_register_coalesced_mmio

[Glauber Costa]

It is now done automatically for IO regions inside
kvm_cpu_register_physical_memory().
---
 qemu/hw/cirrus_vga.c |    2 --
 qemu/hw/e1000.c      |   12 ------------
 qemu/hw/pci.c        |    3 ---
 qemu/hw/vga.c        |    4 ----
 4 files changed, 0 insertions(+), 21 deletions(-)

diff --git a/qemu/hw/cirrus_vga.c b/qemu/hw/cirrus_vga.c
index c7e8f2c..42bca4f 100644
--- a/qemu/hw/cirrus_vga.c
+++ b/qemu/hw/cirrus_vga.c
@@ -3291,8 +3291,6 @@ static void cirrus_init_common(CirrusVGAState * s, int device_id, int is_pci)
                                            cirrus_vga_mem_write, s);
     cpu_register_physical_memory(isa_mem_base + 0x000a0000, 0x20000,
                                  vga_io_memory);
-    if (kvm_enabled())
-        qemu_kvm_register_coalesced_mmio(isa_mem_base + 0x000a0000, 0x20000);
 
     s->sr[0x06] = 0x0f;
     if (device_id == CIRRUS_ID_CLGD5446) {
diff --git a/qemu/hw/e1000.c b/qemu/hw/e1000.c
index 8d60ea6..ce3496b 100644
--- a/qemu/hw/e1000.c
+++ b/qemu/hw/e1000.c
@@ -951,18 +951,6 @@ e1000_mmio_map(PCIDevice *pci_dev, int region_num,
 
     d->mmio_base = addr;
     cpu_register_physical_memory(addr, PNPMMIO_SIZE, d->mmio_index);
-
-    if (kvm_enabled()) {
-	int i;
-        uint32_t excluded_regs[] = {
-            E1000_MDIC, E1000_ICR, E1000_ICS, E1000_IMS,
-            E1000_IMC, E1000_TCTL, E1000_TDT, PNPMMIO_SIZE
-        };
-        qemu_kvm_register_coalesced_mmio(addr, excluded_regs[0]);
-        for (i = 0; excluded_regs[i] != PNPMMIO_SIZE; i++)
-            qemu_kvm_register_coalesced_mmio(addr + excluded_regs[i] + 4,
-                             excluded_regs[i + 1] - excluded_regs[i] - 4);
-    }
 }
 
 static int
diff --git a/qemu/hw/pci.c b/qemu/hw/pci.c
index 92683d1..f58c634 100644
--- a/qemu/hw/pci.c
+++ b/qemu/hw/pci.c
@@ -324,9 +324,6 @@ static void pci_update_mappings(PCIDevice *d)
                         cpu_register_physical_memory(pci_to_cpu_addr(r->addr),
                                                      r->size,
                                                      IO_MEM_UNASSIGNED);
-                        if (kvm_enabled())
-                                qemu_kvm_unregister_coalesced_mmio(r->addr,
-                                                                 r->size);
                     }
                 }
                 r->addr = new_addr;
diff --git a/qemu/hw/vga.c b/qemu/hw/vga.c
index 95d6033..17b5a36 100644
--- a/qemu/hw/vga.c
+++ b/qemu/hw/vga.c
@@ -2257,8 +2257,6 @@ void vga_init(VGAState *s)
     vga_io_memory = cpu_register_io_memory(0, vga_mem_read, vga_mem_write, s);
     cpu_register_physical_memory(isa_mem_base + 0x000a0000, 0x20000,
                                  vga_io_memory);
-    if (kvm_enabled())
-        qemu_kvm_register_coalesced_mmio(isa_mem_base + 0x000a0000, 0x20000);
 }
 
 /* Memory mapped interface */
@@ -2334,8 +2332,6 @@ static void vga_mm_init(VGAState *s, target_phys_addr_t vram_base,
     cpu_register_physical_memory(ctrl_base, 0x100000, s_ioport_ctrl);
     s->bank_offset = 0;
     cpu_register_physical_memory(vram_base + 0x000a0000, 0x20000, vga_io_memory);
-    if (kvm_enabled())
-        qemu_kvm_register_coalesced_mmio(vram_base + 0x000a0000, 0x20000);
 }
 
 int isa_vga_init(DisplayState *ds, uint8_t *vga_ram_base,
-- 
1.5.5.1

Re: [PATCH 6/9] move kvm_cpu_register_memory_area into qemu's

[Anthony Liguori]

Glauber Costa wrote:
> Turn the explicit calls to kvm_cpu_register_memoy_area()
> an empty function. Provide a __kvm_cpu_register_memory_area()
> that is called from within cpu_register_memory_area().
> To avoid registering mmio regions to the hypervisor, since we depend on
> them faulting, we keep track of what regions are mmio regions too.
>
> This is to be bisection friendly. Direct calls are to be removed
> in a later commit.
>
> diff --git a/qemu/exec.c b/qemu/exec.c
> index 7a68062..14c3852 100644
> --- a/qemu/exec.c
> +++ b/qemu/exec.c
> @@ -2196,6 +2196,9 @@ void cpu_register_physical_memory(target_phys_addr_t start_addr,
>          kqemu_set_phys_mem(start_addr, size, phys_offset);
>      }
>  #endif
> +
> +    __kvm_cpu_register_physical_memory(start_addr, size, phys_offset);
>
>   

This is a great place to add a callback of some sort (like QEMUAccel).  
Then it can be shared by both kqemu and kvm.

Regards,

Anthony Liguori

Re: [PATCH 6/9] move kvm_cpu_register_memory_area into qemu's

[Glauber Costa]

On Wed, Aug 13, 2008 at 11:11 AM, Anthony Liguori <aliguori@us.ibm.com> wrote:
> Glauber Costa wrote:
>>
>> Turn the explicit calls to kvm_cpu_register_memoy_area()
>> an empty function. Provide a __kvm_cpu_register_memory_area()
>> that is called from within cpu_register_memory_area().
>> To avoid registering mmio regions to the hypervisor, since we depend on
>> them faulting, we keep track of what regions are mmio regions too.
>>
>> This is to be bisection friendly. Direct calls are to be removed
>> in a later commit.
>>
>> diff --git a/qemu/exec.c b/qemu/exec.c
>> index 7a68062..14c3852 100644
>> --- a/qemu/exec.c
>> +++ b/qemu/exec.c
>> @@ -2196,6 +2196,9 @@ void cpu_register_physical_memory(target_phys_addr_t
>> start_addr,
>>         kqemu_set_phys_mem(start_addr, size, phys_offset);
>>     }
>>  #endif
>> +
>> +    __kvm_cpu_register_physical_memory(start_addr, size, phys_offset);
>>
>>
>
> This is a great place to add a callback of some sort (like QEMUAccel).  Then
> it can be shared by both kqemu and kvm.

The current QEMUAccel patchset already has a callback for this exactly here.

If we set up on this, I'll rebase QEMUAccel to include a kvm callback.

>
> Regards,
>
> Anthony Liguori
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Glauber Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

Re: [PATCH 5/9] substitute is_allocated_mem with more general is_containing_region

[Avi Kivity]

Glauber Costa wrote:
> Signed-off-by: Glauber Costa <gcosta@redhat.com>
> ---
>  libkvm/libkvm.c |   34 +++++++++++++++++++++-------------
>  libkvm/libkvm.h |    2 +-
>  qemu/qemu-kvm.c |    2 +-
>  3 files changed, 23 insertions(+), 15 deletions(-)
>
> diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
> index 33f00b7..c885dee 100644
> --- a/libkvm/libkvm.c
> +++ b/libkvm/libkvm.c
> @@ -140,6 +140,27 @@ int get_intersecting_slot(unsigned long phys_addr)
>  	return -1;
>  }
>  
> +/* Returns -1 if this slot is not totally contained on any other,
> + * and the number of the slot otherwise */
> +int get_container_slot(uint64_t phys_addr, unsigned long size)
> +{
> +	int i;
> +
> +	for (i = 0; i < KVM_MAX_NUM_MEM_REGIONS ; ++i)
> +		if (slots[i].len && slots[i].phys_addr <= phys_addr &&
> +		    (slots[i].phys_addr + slots[i].len) >= phys_addr + size)
> +			return i;
> +	return -1;
> +}
> +
>   

What about partially containing (or: overlapping) slots?

-- 
error compiling committee.c: too many arguments to function

Re: [PATCH 5/9] substitute is_allocated_mem with more general is_containing_region

[Glauber Costa]

On Wed, Aug 13, 2008 at 8:41 AM, Avi Kivity <avi@qumranet.com> wrote:
> Glauber Costa wrote:
>>
>> Signed-off-by: Glauber Costa <gcosta@redhat.com>
>> ---
>>  libkvm/libkvm.c |   34 +++++++++++++++++++++-------------
>>  libkvm/libkvm.h |    2 +-
>>  qemu/qemu-kvm.c |    2 +-
>>  3 files changed, 23 insertions(+), 15 deletions(-)
>>
>> diff --git a/libkvm/libkvm.c b/libkvm/libkvm.c
>> index 33f00b7..c885dee 100644
>> --- a/libkvm/libkvm.c
>> +++ b/libkvm/libkvm.c
>> @@ -140,6 +140,27 @@ int get_intersecting_slot(unsigned long phys_addr)
>>        return -1;
>>  }
>>  +/* Returns -1 if this slot is not totally contained on any other,
>> + * and the number of the slot otherwise */
>> +int get_container_slot(uint64_t phys_addr, unsigned long size)
>> +{
>> +       int i;
>> +
>> +       for (i = 0; i < KVM_MAX_NUM_MEM_REGIONS ; ++i)
>> +               if (slots[i].len && slots[i].phys_addr <= phys_addr &&
>> +                   (slots[i].phys_addr + slots[i].len) >= phys_addr +
>> size)
>> +                       return i;
>> +       return -1;
>> +}
>> +
>>
>
> What about partially containing (or: overlapping) slots?

That would be handled by kvm_is_intersecting_memory. Ideally, I would
like to have a single function for both.
Now that the default action for intersecting memory is to do nothing
but return, that would be possible.

I'll make sure I address that in a later version

> --
> error compiling committee.c: too many arguments to function
>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>



-- 
Glauber Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

Re: [RFC 0/9] Memory registration rework

[Avi Kivity]

Glauber Costa wrote:
> Hi folks,
>
> The following series contain a proposal for our memory registration
> framework. This is by no means complete, and rather, a first step only.
>
> This first step, btw, has the goal of taking the kvm-specific memory registration
> functions from all over the code, so we can make the merging with qemu easier.
>
> Note that I'm putting kvm_cpu_register_phys_memory() _inside_ cpu_register_phys_memory().
> To do that, we need to be resilient against the same region being registered multiple times,
> and should be able to interpret the flags embedded in phys_offset in a meaninful way.
> Although arguably with some bugs yet unknown, this series does exactly that.
>
> For that to work, we have to be sure that we'll never reach a situation in which we
> register a piece of memory, and later on, register another region that contains it. Current
> code does that, so we're fine. The oposite situation, namely, registering a large piece of memory
> and then re-registering pieces of it, is perfectly valid.
>
> In the to-be-merged version, if it ever exists, I intend to comment all those issues very well,
> to get an as predictable interface as possible.
>
> There's another option of doing this, as anthony pointed out in earlier private comments to me,
> which is scanning the already registered regions right before starting execution, and building our
> maps. While this is valid, we can't run away from doing what I'm doing, because some areas are
> manipulated _after_ the machine has started. For example, the pci region, for the hotplug case.
>
> Note that this is not tested in anything but x86. 
>
>   

Looks good.  The current duplication of memory registration is very 
annoying.

-- 
error compiling committee.c: too many arguments to function

Re: [RFC 0/9] Memory registration rework

[Anthony Liguori]

Avi Kivity wrote:
> Glauber Costa wrote:
>> Hi folks,
>>
>> Note that this is not tested in anything but x86.
>>   
>
> Looks good.  The current duplication of memory registration is very 
> annoying.

Yeah, I really like the way this series turned out.  I agree that this 
is the right thing to do.

Regards,

Anthony Liguori