[PATCH 0/2] --disable-kvm currently not working

[PATCH 0/2] --disable-kvm currently not working

[Glauber Costa]

Current compilations with --disable-kvm are broken.
I'm spliting the fix in two patches for clearity. The first one
does some code movement in qemu-kvm-x86.c, while the other just
do fixes to the current code base.

[PATCH] provide a kvm_qemu_memory_alias() function

[Glauber Costa]

Following the pattern we already do, provide a qemu_kvm wrapper to
the memory aliases x86 functions. Reason is that we don't want to have
references to the context spread over qemu.

The destroy alias function is completely removed from libkvm/libkvm.c,
since no one in the code base uses it directly.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 libkvm/libkvm-x86.c |    5 -----
 qemu/qemu-kvm-x86.c |   12 ++++++++++++
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/libkvm/libkvm-x86.c b/libkvm/libkvm-x86.c
index a8cca15..f33038d 100644
--- a/libkvm/libkvm-x86.c
+++ b/libkvm/libkvm-x86.c
@@ -206,11 +206,6 @@ int kvm_create_memory_alias(kvm_context_t kvm,
 	return 0;
 }
 
-int kvm_destroy_memory_alias(kvm_context_t kvm, uint64_t phys_start)
-{
-	return kvm_create_memory_alias(kvm, phys_start, 0, 0);
-}
-
 #ifdef KVM_CAP_IRQCHIP
 
 int kvm_get_lapic(kvm_context_t kvm, int vcpu, struct kvm_lapic_state *s)
diff --git a/qemu/qemu-kvm-x86.c b/qemu/qemu-kvm-x86.c
index 5daedd1..55c520d 100644
--- a/qemu/qemu-kvm-x86.c
+++ b/qemu/qemu-kvm-x86.c
@@ -27,6 +27,18 @@ static int kvm_has_msr_star;
 
 static int lm_capable_kernel;
 
+int kvm_qemu_create_memory_alias(uint64_t phys_start,
+                                 uint64_t len,
+                                 uint64_t target_phys)
+{
+    return kvm_create_memory_alias(kvm_context, phys_start, len, target_phys);
+}
+
+int kvm_qemu_destroy_memory_alias(uint64_t phys_start)
+{
+	return kvm_create_memory_alias(kvm_context, phys_start, 0, 0);
+}
+
 int kvm_arch_qemu_create_context(void)
 {
     int i;
-- 
1.5.5.1

Re: [PATCH] provide a kvm_qemu_memory_alias() function

[Avi Kivity]

Glauber Costa wrote:
> Following the pattern we already do, provide a qemu_kvm wrapper to
> the memory aliases x86 functions. Reason is that we don't want to have
> references to the context spread over qemu.
>
> The destroy alias function is completely removed from libkvm/libkvm.c,
> since no one in the code base uses it directly.
>
>  
> -int kvm_destroy_memory_alias(kvm_context_t kvm, uint64_t phys_start)
> -{
> -	return kvm_create_memory_alias(kvm, phys_start, 0, 0);
> -}
> -
>   

This exists so that readers don't have to wander why you're calling 
kvm_create_memory_alias when you actually want to destroy one.

-- 
I have a truly marvellous patch that fixes the bug which this
signature is too narrow to contain.

Re: [PATCH] provide a kvm_qemu_memory_alias() function

[Glauber Costa]

On Wed, Oct 1, 2008 at 4:38 AM, Avi Kivity <avi@redhat.com> wrote:
> Glauber Costa wrote:
>>
>> Following the pattern we already do, provide a qemu_kvm wrapper to
>> the memory aliases x86 functions. Reason is that we don't want to have
>> references to the context spread over qemu.
>>
>> The destroy alias function is completely removed from libkvm/libkvm.c,
>> since no one in the code base uses it directly.
>>
>>  -int kvm_destroy_memory_alias(kvm_context_t kvm, uint64_t phys_start)
>> -{
>> -       return kvm_create_memory_alias(kvm, phys_start, 0, 0);
>> -}
>> -
>>
>
> This exists so that readers don't have to wander why you're calling
> kvm_create_memory_alias when you actually want to destroy one.

So what? I'm replacing it with a kvm_qemu_destroy... that does the
very same thing, in the very same way.
only difference is the presence/absence of context.

>
> --
> I have a truly marvellous patch that fixes the bug which this
> signature is too narrow to contain.
>
> --
> 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] provide a kvm_qemu_memory_alias() function

[Avi Kivity]

Glauber Costa wrote:
> On Wed, Oct 1, 2008 at 4:38 AM, Avi Kivity <avi@redhat.com> wrote:
>   
>> Glauber Costa wrote:
>>     
>>> Following the pattern we already do, provide a qemu_kvm wrapper to
>>> the memory aliases x86 functions. Reason is that we don't want to have
>>> references to the context spread over qemu.
>>>
>>> The destroy alias function is completely removed from libkvm/libkvm.c,
>>> since no one in the code base uses it directly.
>>>
>>>  -int kvm_destroy_memory_alias(kvm_context_t kvm, uint64_t phys_start)
>>> -{
>>> -       return kvm_create_memory_alias(kvm, phys_start, 0, 0);
>>> -}
>>> -
>>>
>>>       
>> This exists so that readers don't have to wander why you're calling
>> kvm_create_memory_alias when you actually want to destroy one.
>>     
>
> So what? I'm replacing it with a kvm_qemu_destroy... that does the
> very same thing, in the very same way.
> only difference is the presence/absence of context.
>   

libkvm exists to provide a sane API to applications.  Using 
kvm_create_memory_alias() to destroy aliases is not a sane API.


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

[PATCH] fix compilation with --disable-kvm

[Glauber Costa]

Currently, kvm is failing to build with --disable-kvm.
this patch fixes the issue.

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 qemu/gdbstub.c       |   15 ++++++++++-----
 qemu/hw/acpi.c       |    6 +++++-
 qemu/hw/cirrus_vga.c |    7 +++----
 qemu/monitor.c       |    6 ++++--
 4 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/qemu/gdbstub.c b/qemu/gdbstub.c
index d57cd61..d65f1cf 100644
--- a/qemu/gdbstub.c
+++ b/qemu/gdbstub.c
@@ -994,10 +994,12 @@ static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf)
             addr = strtoull(p, (char **)&p, 16);
 #if defined(TARGET_I386)
             env->eip = addr;
-            kvm_load_registers(env);
+            if (kvm_enabled())
+                kvm_load_registers(env);
 #elif defined (TARGET_PPC)
             env->nip = addr;
-            kvm_load_registers(env);
+            if (kvm_enabled())
+                kvm_load_registers(env);
 #elif defined (TARGET_SPARC)
             env->pc = addr;
             env->npc = addr + 4;
@@ -1033,7 +1035,8 @@ static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf)
             addr = strtoull(p, (char **)&p, 16);
 #if defined(TARGET_I386)
             env->eip = addr;
-            kvm_load_registers(env);
+            if (kvm_enabled())
+                kvm_load_registers(env);
 #elif defined (TARGET_PPC)
             env->nip = addr;
             kvm_load_registers(env);
@@ -1078,7 +1081,8 @@ static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf)
         }
         break;
     case 'g':
-        kvm_save_registers(env);
+        if (kvm_enabled())
+            kvm_save_registers(env);
         reg_size = cpu_gdb_read_registers(env, mem_buf);
         memtohex(buf, mem_buf, reg_size);
         put_packet(s, buf);
@@ -1088,7 +1092,8 @@ static int gdb_handle_packet(GDBState *s, CPUState *env, const char *line_buf)
         len = strlen(p) / 2;
         hextomem((uint8_t *)registers, p, len);
         cpu_gdb_write_registers(env, mem_buf, len);
-        kvm_load_registers(env);
+        if (kvm_enabled())
+            kvm_load_registers(env);
         put_packet(s, "OK");
         break;
     case 'm':
diff --git a/qemu/hw/acpi.c b/qemu/hw/acpi.c
index 05f5dc0..a51fcb7 100644
--- a/qemu/hw/acpi.c
+++ b/qemu/hw/acpi.c
@@ -726,7 +726,11 @@ void qemu_system_cpu_hot_add(int cpu, int state)
 {
     CPUState *env;
 
-    if ((state) && (!qemu_kvm_cpu_env(cpu))) {
+    if (state 
+#ifdef USE_KVM
+        && (!qemu_kvm_cpu_env(cpu))
+#endif
+    ) {
         env = pc_new_cpu(cpu, model, 1);
         if (!env) {
             fprintf(stderr, "cpu %d creation failed\n", cpu);
diff --git a/qemu/hw/cirrus_vga.c b/qemu/hw/cirrus_vga.c
index 4f3aef9..dac0731 100644
--- a/qemu/hw/cirrus_vga.c
+++ b/qemu/hw/cirrus_vga.c
@@ -2677,14 +2677,13 @@ static void kvm_update_vga_alias(CirrusVGAState *s, int ok, int bank)
 	if (!s->aliases_enabled
 	    || base != s->aliased_bank_base[bank]
 	    || limit != s->aliased_bank_limit[bank]) {
-	    kvm_create_memory_alias(kvm_context,
-				    0xa0000 + bank * 0x8000,
-				    limit, base);
+	    kvm_qemu_create_memory_alias(0xa0000 + bank * 0x8000,
+                                     limit, base);
 	    s->aliased_bank_base[bank] = base;
 	    s->aliased_bank_limit[bank] = limit;
 	}
     } else {
-	kvm_destroy_memory_alias(kvm_context, 0xa0000 + bank * 0x8000);
+	kvm_qemu_destroy_memory_alias(0xa0000 + bank * 0x8000);
     }
 }
 
diff --git a/qemu/monitor.c b/qemu/monitor.c
index d6b3da6..97b5cbe 100644
--- a/qemu/monitor.c
+++ b/qemu/monitor.c
@@ -292,7 +292,8 @@ static CPUState *mon_get_cpu(void)
         mon_set_cpu(0);
     }
 
-    kvm_save_registers(mon_cpu);
+    if (kvm_enabled())
+        kvm_save_registers(mon_cpu);
 
     return mon_cpu;
 }
@@ -320,7 +321,8 @@ static void do_info_cpus(void)
     mon_get_cpu();
 
     for(env = first_cpu; env != NULL; env = env->next_cpu) {
-        kvm_save_registers(env);
+        if (kvm_enabled())
+            kvm_save_registers(env);
         term_printf("%c CPU #%d:",
                     (env == mon_cpu) ? '*' : ' ',
                     env->cpu_index);
-- 
1.5.5.1

Re: [PATCH 0/2] --disable-kvm currently not working

[Glauber Costa]

On Tue, Sep 30, 2008 at 6:47 PM, Glauber Costa <glommer@redhat.com> wrote:
> Current compilations with --disable-kvm are broken.
> I'm spliting the fix in two patches for clearity. The first one
> does some code movement in qemu-kvm-x86.c, while the other just
> do fixes to the current code base.

oh god, forgot to do format-patch -n _again_ ;-(
avi, should I resend? Since there are only two of them , it's pretty
straightforward.
>
>
> --
> 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."