From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christian Ehrhardt Subject: Re: [PATCH] kvm: work around inability of older kvm modules to destroy memory regions Date: Wed, 10 Dec 2008 15:08:26 +0100 Message-ID: <493FCD5A.7010308@linux.vnet.ibm.com> References: <20081209161057.DC28A25006D@cleopatra.tlv.redhat.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------000108020804040907070001" Cc: kvm , Hollis Blanchard , Carsten Otte , Christian Borntraeger To: Avi Kivity Return-path: Received: from mtagate6.de.ibm.com ([195.212.29.155]:63063 "EHLO mtagate6.de.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752309AbYLJOIf (ORCPT ); Wed, 10 Dec 2008 09:08:35 -0500 Received: from d12nrmr1607.megacenter.de.ibm.com (d12nrmr1607.megacenter.de.ibm.com [9.149.167.49]) by mtagate6.de.ibm.com (8.13.8/8.13.8) with ESMTP id mBAE8UME059148 for ; Wed, 10 Dec 2008 14:08:30 GMT Received: from d12av01.megacenter.de.ibm.com (d12av01.megacenter.de.ibm.com [9.149.165.212]) by d12nrmr1607.megacenter.de.ibm.com (8.13.8/8.13.8/NCO v9.1) with ESMTP id mBAE8UAk2781304 for ; Wed, 10 Dec 2008 15:08:30 +0100 Received: from d12av01.megacenter.de.ibm.com (loopback [127.0.0.1]) by d12av01.megacenter.de.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id mBAE8THm031262 for ; Wed, 10 Dec 2008 15:08:30 +0100 In-Reply-To: <20081209161057.DC28A25006D@cleopatra.tlv.redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: This is a multi-part message in MIME format. --------------000108020804040907070001 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 8bit 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 ? I don't want to foist more urgent work on you, so if you are too busy for the right patch now and unapplying is no option because it is too important for x86 you can use the ugly++ patch I attached as interim solution. It is my test patch and uses ifdef's enabling this code only for x86, ugly but works until we have found the right split. regards, Christian Avi Kivity wrote: > From: Avi Kivity > > instead of removing and adding memory regions, keep the vga framebuffer mapped, > and use aliases for the vga windows at 0xa0000. > > Signed-off-by: Avi Kivity > > diff --git a/qemu/qemu-kvm.c b/qemu/qemu-kvm.c > index 619a166..067cf03 100644 > --- a/qemu/qemu-kvm.c > +++ b/qemu/qemu-kvm.c > @@ -766,6 +766,8 @@ int kvm_qemu_init() > return 0; > } > > +static int destroy_region_works = 0; > + > int kvm_qemu_create_context(void) > { > int r; > @@ -782,6 +784,7 @@ int kvm_qemu_create_context(void) > r = kvm_arch_qemu_create_context(); > if(r <0) > kvm_qemu_destroy(); > + destroy_region_works = kvm_destroy_memory_region_works(kvm_context); > return 0; > } > > @@ -790,16 +793,80 @@ void kvm_qemu_destroy(void) > kvm_finalize(kvm_context); > } > > +static int must_use_aliases_source(target_phys_addr_t addr) > +{ > + if (destroy_region_works) > + return false; > + if (addr == 0xa0000 || addr == 0xa8000) > + return true; > + return false; > +} > + > +static int must_use_aliases_target(target_phys_addr_t addr) > +{ > + if (destroy_region_works) > + return false; > + if (addr >= 0xe000000 && addr < 0x100000000ull) > + return true; > + return false; > +} > + > +static struct mapping { > + target_phys_addr_t phys; > + ram_addr_t ram; > + ram_addr_t len; > +} mappings[50]; > +static int nr_mappings; > + > +static struct mapping *find_ram_mapping(ram_addr_t ram_addr) > +{ > + struct mapping *p; > + > + for (p = mappings; p < mappings + nr_mappings; ++p) { > + if (p->ram <= ram_addr && ram_addr < p->ram + p->len) { > + return p; > + } > + } > + return NULL; > +} > + > +static struct mapping *find_mapping(target_phys_addr_t start_addr) > +{ > + struct mapping *p; > + > + for (p = mappings; p < mappings + nr_mappings; ++p) { > + if (p->phys <= start_addr && start_addr < p->phys + p->len) { > + return p; > + } > + } > + return NULL; > +} > + > +static void drop_mapping(target_phys_addr_t start_addr) > +{ > + struct mapping *p = find_mapping(start_addr); > + > + if (p) > + *p = mappings[--nr_mappings]; > +} > + > void kvm_cpu_register_physical_memory(target_phys_addr_t start_addr, > unsigned long size, > unsigned long phys_offset) > { > int r = 0; > unsigned long area_flags = phys_offset & ~TARGET_PAGE_MASK; > + struct mapping *p; > > phys_offset &= ~IO_MEM_ROM; > > if (area_flags == IO_MEM_UNASSIGNED) { > + if (must_use_aliases_source(start_addr)) { > + kvm_destroy_memory_alias(kvm_context, start_addr); > + return; > + } > + if (must_use_aliases_target(start_addr)) > + return; > kvm_unregister_memory_area(kvm_context, start_addr, size); > return; > } > @@ -811,6 +878,15 @@ void kvm_cpu_register_physical_memory(target_phys_addr_t start_addr, > if (area_flags >= TLB_MMIO) > return; > > + if (must_use_aliases_source(start_addr)) { > + p = find_ram_mapping(phys_offset); > + if (p) { > + kvm_create_memory_alias(kvm_context, start_addr, size, > + p->phys + (phys_offset - p->ram)); > + } > + return; > + } > + > r = kvm_register_phys_mem(kvm_context, start_addr, > phys_ram_base + phys_offset, > size, 0); > @@ -818,6 +894,13 @@ void kvm_cpu_register_physical_memory(target_phys_addr_t start_addr, > printf("kvm_cpu_register_physical_memory: failed\n"); > exit(1); > } > + > + drop_mapping(start_addr); > + p = &mappings[nr_mappings++]; > + p->phys = start_addr; > + p->ram = phys_offset; > + p->len = size; > + > return; > } > > @@ -984,8 +1067,11 @@ void kvm_qemu_log_memory(target_phys_addr_t start, target_phys_addr_t size, > { > if (log) > kvm_dirty_pages_log_enable_slot(kvm_context, start, size); > - else > + else { > + if (must_use_aliases_target(start)) > + return; > kvm_dirty_pages_log_disable_slot(kvm_context, start, size); > + } > } > > int kvm_get_phys_ram_page_bitmap(unsigned char *bitmap) > @@ -1071,6 +1157,9 @@ void kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, target_phys_a > { > void *buf; > > + if (must_use_aliases_source(start_addr)) > + return; > + > buf = qemu_malloc((end_addr - start_addr) / 8 + 2); > kvm_get_dirty_pages_range(kvm_context, start_addr, end_addr - start_addr, > buf, NULL, kvm_get_dirty_bitmap_cb); > @@ -1080,6 +1169,8 @@ void kvm_physical_sync_dirty_bitmap(target_phys_addr_t start_addr, target_phys_a > int kvm_log_start(target_phys_addr_t phys_addr, target_phys_addr_t len) > { > #ifndef TARGET_IA64 > + if (must_use_aliases_source(phys_addr)) > + return 0; > kvm_qemu_log_memory(phys_addr, len, 1); > #endif > return 0; > @@ -1088,6 +1179,8 @@ int kvm_log_start(target_phys_addr_t phys_addr, target_phys_addr_t len) > int kvm_log_stop(target_phys_addr_t phys_addr, target_phys_addr_t len) > { > #ifndef TARGET_IA64 > + if (must_use_aliases_source(phys_addr)) > + return 0; > kvm_qemu_log_memory(phys_addr, len, 0); > #endif > return 0; > -- > To unsubscribe from this list: send the line "unsubscribe kvm-commits" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- Grüsse / regards, Christian Ehrhardt IBM Linux Technology Center, Open Virtualization --------------000108020804040907070001 Content-Type: text/x-diff; name="memory-aliases.diff" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="memory-aliases.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; } --------------000108020804040907070001--