* Re: [PATCH] kvm: work around inability of older kvm modules to destroy memory regions
[not found] <20081209161057.DC28A25006D@cleopatra.tlv.redhat.com>
@ 2008-12-10 14:08 ` Christian Ehrhardt
2008-12-17 12:26 ` Avi Kivity
0 siblings, 1 reply; 4+ messages in thread
From: Christian Ehrhardt @ 2008-12-10 14:08 UTC (permalink / raw)
To: Avi Kivity; +Cc: kvm, Hollis Blanchard, Carsten Otte, Christian Borntraeger
[-- Attachment #1: Type: text/plain, Size: 7079 bytes --]
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 <avi@redhat.com>
>
> 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 <avi@redhat.com>
>
> 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
[-- Attachment #2: memory-aliases.diff --]
[-- Type: text/x-diff, Size: 3545 bytes --]
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;
}
^ permalink raw reply [flat|nested] 4+ messages in thread