Re: [PATCH] kvm: work around inability of older kvm modules to destroy memory regions

[Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 1304 bytes --]

Avi Kivity wrote:
> Christian Ehrhardt wrote:
>> Hi, this patch breaks all non x86 architectures as 
>> libkvm/libkvm-x86.c has the only implementation of the alias 
>> functionality.
>> Until now only qemu-kvm-x86 has called that functions, but since this 
>> patch the generic qemu-kvm.c calls them which leads to unresolved 
>> symbols for powerpc, s390 and surely ia64 too.
>>
>> Well we could insert stubs for these call, but when looking on the 
>> kernel side x86 is also the only implementer of the 
>> KVM_SET_MEMORY_ALIAS ioctl. Until more arch support that there is no 
>> reason to create these functions for non-x86 in libkvm. Also the 
>> assumptions which addresses must be aliased base on hardware specific 
>> assumptions e.g. vga card -> arch specific too.
>>
>> For now I hold a no-op stub in my private queue to test powerpc, but 
>> eventually this mechanism should be arch dependent and this 
>> implementation x86 only.
>> Avi could you modify your patch to work for the other arch's too ?
>
> Your band aid should be fine.  Yes, it's ugly, but this will be in 
> flux as we merge with upstream qemu.  Please resend it with a signoff.
>
still ugly, but in a proper patch style now :-)


-- 

Grüsse / regards, 
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization


[-- Attachment #2: memory-aliases.diff --]
[-- Type: text/x-diff, Size: 3979 bytes --]

Subject: [PATCH] kvm-userspace: guard destroy memory regions to x86 only

From: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>

Some parts of the changes made were arch specific e..g vga mem.
This is a quick, but ugly workaround until a real arch split is found.

Signed-off-by: Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>
---

[diffstat]
 qemu-kvm.c |   26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

[diff]
diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -766,7 +766,9 @@ int kvm_qemu_init()
     return 0;
 }

+#ifdef TARGET_I386
 static int destroy_region_works = 0;
+#endif

 int kvm_qemu_create_context(void)
 {
@@ -784,7 +786,9 @@ int kvm_qemu_create_context(void)
     r = kvm_arch_qemu_create_context();
     if(r <0)
 	kvm_qemu_destroy();
+#ifdef TARGET_I386
     destroy_region_works = kvm_destroy_memory_region_works(kvm_context);
+#endif
     return 0;
 }

@@ -793,6 +797,7 @@ void kvm_qemu_destroy(void)
     kvm_finalize(kvm_context);
 }

+#ifdef TARGET_I386
 static int must_use_aliases_source(target_phys_addr_t addr)
 {
     if (destroy_region_works)
@@ -849,6 +854,7 @@ static void drop_mapping(target_phys_add
     if (p)
         *p = mappings[--nr_mappings];
 }
+#endif

 void kvm_cpu_register_physical_memory(target_phys_addr_t start_addr,
                                       unsigned long size,
@@ -856,17 +862,21 @@ void kvm_cpu_register_physical_memory(ta
 {
     int r = 0;
     unsigned long area_flags = phys_offset & ~TARGET_PAGE_MASK;
+#ifdef TARGET_I386
     struct mapping *p;
+#endif

     phys_offset &= ~IO_MEM_ROM;

     if (area_flags == IO_MEM_UNASSIGNED) {
+#ifdef TARGET_I386
         if (must_use_aliases_source(start_addr)) {
             kvm_destroy_memory_alias(kvm_context, start_addr);
             return;
         }
         if (must_use_aliases_target(start_addr))
             return;
+#endif
         kvm_unregister_memory_area(kvm_context, start_addr, size);
         return;
     }
@@ -878,6 +888,7 @@ void kvm_cpu_register_physical_memory(ta
     if (area_flags >= TLB_MMIO)
         return;

+#ifdef TARGET_I386
     if (must_use_aliases_source(start_addr)) {
         p = find_ram_mapping(phys_offset);
         if (p) {
@@ -886,6 +897,7 @@ void kvm_cpu_register_physical_memory(ta
         }
         return;
     }
+#endif

     r = kvm_register_phys_mem(kvm_context, start_addr,
                               phys_ram_base + phys_offset,
@@ -895,11 +907,13 @@ void kvm_cpu_register_physical_memory(ta
         exit(1);
     }

+#ifdef TARGET_I386
     drop_mapping(start_addr);
     p = &mappings[nr_mappings++];
     p->phys = start_addr;
     p->ram = phys_offset;
     p->len = size;
+#endif

     return;
 }
@@ -1068,8 +1082,10 @@ void kvm_qemu_log_memory(target_phys_add
     if (log)
 	kvm_dirty_pages_log_enable_slot(kvm_context, start, size);
     else {
+#ifdef TARGET_I386
         if (must_use_aliases_target(start))
             return;
+#endif
 	kvm_dirty_pages_log_disable_slot(kvm_context, start, size);
     }
 }
@@ -1157,8 +1173,10 @@ void kvm_physical_sync_dirty_bitmap(targ
 {
     void *buf;

+#ifdef TARGET_I386
     if (must_use_aliases_source(start_addr))
         return;
+#endif

     buf = qemu_malloc((end_addr - start_addr) / 8 + 2);
     kvm_get_dirty_pages_range(kvm_context, start_addr, end_addr - start_addr,
@@ -1168,21 +1186,21 @@ void kvm_physical_sync_dirty_bitmap(targ

 int kvm_log_start(target_phys_addr_t phys_addr, target_phys_addr_t len)
 {
-#ifndef TARGET_IA64
+#ifdef TARGET_I386
     if (must_use_aliases_source(phys_addr))
         return 0;
+#endif
     kvm_qemu_log_memory(phys_addr, len, 1);
-#endif
     return 0;
 }

 int kvm_log_stop(target_phys_addr_t phys_addr, target_phys_addr_t len)
 {
-#ifndef TARGET_IA64
+#ifdef TARGET_I386
     if (must_use_aliases_source(phys_addr))
         return 0;
+#endif
     kvm_qemu_log_memory(phys_addr, len, 0);
-#endif
     return 0;
 }