[patch 0/4] fix SMP migration and savevm/loadvm

[patch 0/4] fix SMP migration and savevm/loadvm

[Marcelo Tosatti]

-- 


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

[patch 1/4] QEMU/KVM: only use KVM apic registers if vm is running

[Marcelo Tosatti]

[-- Attachment #1: fix-migration-1 --]
[-- Type: text/plain, Size: 1206 bytes --]

In the -incoming case the apic regs are not initialized and therefore
bogus.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm-userspace.io/qemu/qemu-kvm-x86.c
===================================================================
--- kvm-userspace.io.orig/qemu/qemu-kvm-x86.c
+++ kvm-userspace.io/qemu/qemu-kvm-x86.c
@@ -27,6 +27,8 @@ static int kvm_has_msr_star;
 
 static int lm_capable_kernel;
 
+extern int vm_running;
+
 int kvm_arch_qemu_create_context(void)
 {
     int i;
@@ -248,7 +250,7 @@ void kvm_arch_load_regs(CPUState *env)
     sregs.cr3 = env->cr[3];
     sregs.cr4 = env->cr[4];
 
-    if (kvm_irqchip_in_kernel(kvm_context)) {
+    if (kvm_irqchip_in_kernel(kvm_context) && vm_running) {
         sregs.cr8 = kvm_get_cr8(kvm_context, env->cpu_index);
         sregs.apic_base = kvm_get_apic_base(kvm_context, env->cpu_index);
     } else {

-- 


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [patch 1/4] QEMU/KVM: only use KVM apic registers if vm is running

[Avi Kivity]

Marcelo Tosatti wrote:
> In the -incoming case the apic regs are not initialized and therefore
> bogus.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> Index: kvm-userspace.io/qemu/qemu-kvm-x86.c
> ===================================================================
> --- kvm-userspace.io.orig/qemu/qemu-kvm-x86.c
> +++ kvm-userspace.io/qemu/qemu-kvm-x86.c
> @@ -27,6 +27,8 @@ static int kvm_has_msr_star;
>  
>  static int lm_capable_kernel;
>  
> +extern int vm_running;
> +
>  int kvm_arch_qemu_create_context(void)
>  {
>      int i;
> @@ -248,7 +250,7 @@ void kvm_arch_load_regs(CPUState *env)
>      sregs.cr3 = env->cr[3];
>      sregs.cr4 = env->cr[4];
>  
> -    if (kvm_irqchip_in_kernel(kvm_context)) {
> +    if (kvm_irqchip_in_kernel(kvm_context) && vm_running) {
>          sregs.cr8 = kvm_get_cr8(kvm_context, env->cpu_index);
>          sregs.apic_base = kvm_get_apic_base(kvm_context, env->cpu_index);
>      } else {
>   

Fishy.   What if the vm is stopped using the monitor?

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


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

[patch 2/4] QEMU/KVM: save and load mp state

[Marcelo Tosatti]

[-- Attachment #1: fix-smp-migration --]
[-- Type: text/plain, Size: 4459 bytes --]

Use the new ioctl's to save and restore the MP_STATE for all vcpu's.

Fixes SMP migration.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm-userspace.io/libkvm/libkvm.c
===================================================================
--- kvm-userspace.io.orig/libkvm/libkvm.c
+++ kvm-userspace.io/libkvm/libkvm.c
@@ -776,6 +776,17 @@ int kvm_set_sregs(kvm_context_t kvm, int
     return ioctl(kvm->vcpu_fd[vcpu], KVM_SET_SREGS, sregs);
 }
 
+int kvm_get_mpstate(kvm_context_t kvm, int vcpu, int *mpstate)
+{
+    return ioctl(kvm->vcpu_fd[vcpu], KVM_GET_MP_STATE, mpstate);
+}
+
+int kvm_set_mpstate(kvm_context_t kvm, int vcpu, int *mpstate)
+{
+    return ioctl(kvm->vcpu_fd[vcpu], KVM_SET_MP_STATE, mpstate);
+}
+
+
 static int handle_mmio(kvm_context_t kvm, struct kvm_run *kvm_run)
 {
 	unsigned long addr = kvm_run->mmio.phys_addr;
Index: kvm-userspace.io/libkvm/libkvm.h
===================================================================
--- kvm-userspace.io.orig/libkvm/libkvm.h
+++ kvm-userspace.io/libkvm/libkvm.h
@@ -301,6 +301,18 @@ int kvm_get_sregs(kvm_context_t kvm, int
 int kvm_set_sregs(kvm_context_t kvm, int vcpu, struct kvm_sregs *regs);
 
 /*!
+ *  * \brief Read VCPU MP state
+ *
+ */
+int kvm_get_mpstate(kvm_context_t kvm, int vcpu, int *mpstate);
+
+/*!
+ *  * \brief Write VCPU MP state
+ *
+ */
+int kvm_set_mpstate(kvm_context_t kvm, int vcpu, int *mpstate);
+
+/*!
  * \brief Simulate an external vectored interrupt
  *
  * This allows you to simulate an external vectored interrupt.
Index: kvm-userspace.io/qemu/qemu-kvm-x86.c
===================================================================
--- kvm-userspace.io.orig/qemu/qemu-kvm-x86.c
+++ kvm-userspace.io/qemu/qemu-kvm-x86.c
@@ -282,6 +282,15 @@ void kvm_arch_load_regs(CPUState *env)
         perror("kvm_set_msrs FAILED");
 }
 
+void kvm_save_mpstate(CPUState *env)
+{
+    kvm_get_mpstate(kvm_context, env->cpu_index, &env->mp_state);
+}
+
+void kvm_load_mpstate(CPUState *env)
+{
+    kvm_set_mpstate(kvm_context, env->cpu_index, &env->mp_state);
+}
 
 void kvm_arch_save_regs(CPUState *env)
 {
Index: kvm-userspace.io/qemu/qemu-kvm.h
===================================================================
--- kvm-userspace.io.orig/qemu/qemu-kvm.h
+++ kvm-userspace.io/qemu/qemu-kvm.h
@@ -18,6 +18,8 @@ int kvm_init_ap(void);
 void kvm_qemu_destroy(void);
 void kvm_load_registers(CPUState *env);
 void kvm_save_registers(CPUState *env);
+void kvm_load_mpstate(CPUState *env);
+void kvm_save_mpstate(CPUState *env);
 int kvm_cpu_exec(CPUState *env);
 int kvm_update_debugger(CPUState *env);
 int kvm_qemu_init_env(CPUState *env);
Index: kvm-userspace.io/qemu/target-i386/cpu.h
===================================================================
--- kvm-userspace.io.orig/qemu/target-i386/cpu.h
+++ kvm-userspace.io/qemu/target-i386/cpu.h
@@ -599,6 +599,7 @@ typedef struct CPUX86State {
     /* in order to simplify APIC support, we leave this pointer to the
        user */
     struct APICState *apic_state;
+    int mp_state;
 } CPUX86State;
 
 CPUX86State *cpu_x86_init(const char *cpu_model);
Index: kvm-userspace.io/qemu/vl.c
===================================================================
--- kvm-userspace.io.orig/qemu/vl.c
+++ kvm-userspace.io/qemu/vl.c
@@ -6655,8 +6655,10 @@ void cpu_save(QEMUFile *f, void *opaque)
     uint32_t hflags;
     int i;
 
-    if (kvm_enabled())
+    if (kvm_enabled()) {
         kvm_save_registers(env);
+        kvm_save_mpstate(env);
+    }
 
     for(i = 0; i < CPU_NB_REGS; i++)
         qemu_put_betls(f, &env->regs[i]);
@@ -6748,6 +6750,7 @@ void cpu_save(QEMUFile *f, void *opaque)
             qemu_put_be32s(f, &env->kvm_interrupt_bitmap[i]);
         }
         qemu_put_be64s(f, &env->tsc);
+        qemu_put_be32s(f, &env->mp_state);
     }
 }
 
@@ -6899,7 +6902,9 @@ int cpu_load(QEMUFile *f, void *opaque, 
             qemu_get_be32s(f, &env->kvm_interrupt_bitmap[i]);
         }
         qemu_get_be64s(f, &env->tsc);
+        qemu_get_be32s(f, &env->mp_state);
         kvm_load_registers(env);
+        kvm_load_mpstate(env);
     }
     return 0;
 }

-- 


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [patch 2/4] QEMU/KVM: save and load mp state

[Avi Kivity]

Marcelo Tosatti wrote:
> Use the new ioctl's to save and restore the MP_STATE for all vcpu's.
>
> Fixes SMP migration.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> Index: kvm-userspace.io/libkvm/libkvm.c
> ===================================================================
> --- kvm-userspace.io.orig/libkvm/libkvm.c
> +++ kvm-userspace.io/libkvm/libkvm.c
> @@ -776,6 +776,17 @@ int kvm_set_sregs(kvm_context_t kvm, int
>      return ioctl(kvm->vcpu_fd[vcpu], KVM_SET_SREGS, sregs);
>  }
>  
> +int kvm_get_mpstate(kvm_context_t kvm, int vcpu, int *mpstate)
> +{
> +    return ioctl(kvm->vcpu_fd[vcpu], KVM_GET_MP_STATE, mpstate);
> +}
> +
> +int kvm_set_mpstate(kvm_context_t kvm, int vcpu, int *mpstate)
> +{
> +    return ioctl(kvm->vcpu_fd[vcpu], KVM_SET_MP_STATE, mpstate);
> +}
> +
>   

I missed the kernel part of this.  The functions need KVM_CAP 
compile-time and run-time protection.  Please separate the libkvm and 
qemu parts.

>  
>  CPUX86State *cpu_x86_init(const char *cpu_model);
> Index: kvm-userspace.io/qemu/vl.c
> ===================================================================
> --- kvm-userspace.io.orig/qemu/vl.c
> +++ kvm-userspace.io/qemu/vl.c
> @@ -6655,8 +6655,10 @@ void cpu_save(QEMUFile *f, void *opaque)
>      uint32_t hflags;
>      int i;
>  
> -    if (kvm_enabled())
> +    if (kvm_enabled()) {
>          kvm_save_registers(env);
> +        kvm_save_mpstate(env);
> +    }
>   

Perhaps fold kvm_save_mpstate() into kvm_save_registers()?  We can 
regard the mp state as a hidden register.

>  
>      for(i = 0; i < CPU_NB_REGS; i++)
>          qemu_put_betls(f, &env->regs[i]);
> @@ -6748,6 +6750,7 @@ void cpu_save(QEMUFile *f, void *opaque)
>              qemu_put_be32s(f, &env->kvm_interrupt_bitmap[i]);
>          }
>          qemu_put_be64s(f, &env->tsc);
> +        qemu_put_be32s(f, &env->mp_state);
>      }
>  }
>  

Bump the format version number and introduce compatibility code, please.


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


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

[patch 3/4] QEMU/KVM: ignore SIG_IPI signals in userspace

[Marcelo Tosatti]

[-- Attachment #1: ignore-sipi --]
[-- Type: text/plain, Size: 1002 bytes --]

Otherwise a signal can be received in userspace and a vcpu goes back
to the kernel while it should stay still.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm-userspace.io/qemu/qemu-kvm.c
===================================================================
--- kvm-userspace.io.orig/qemu/qemu-kvm.c
+++ kvm-userspace.io/qemu/qemu-kvm.c
@@ -350,7 +350,6 @@ static void *ap_main_loop(void *_env)
     vcpu->env = env;
     vcpu->env->thread_id = kvm_get_thread_id();
     sigfillset(&signals);
-    sigdelset(&signals, SIG_IPI);
     sigprocmask(SIG_BLOCK, &signals, NULL);
     kvm_create_vcpu(kvm_context, env->cpu_index);
     kvm_qemu_init_env(env);

-- 


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [patch 3/4] QEMU/KVM: ignore SIG_IPI signals in userspace

[Avi Kivity]

Marcelo Tosatti wrote:
> Otherwise a signal can be received in userspace and a vcpu goes back
> to the kernel while it should stay still.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> Index: kvm-userspace.io/qemu/qemu-kvm.c
> ===================================================================
> --- kvm-userspace.io.orig/qemu/qemu-kvm.c
> +++ kvm-userspace.io/qemu/qemu-kvm.c
> @@ -350,7 +350,6 @@ static void *ap_main_loop(void *_env)
>      vcpu->env = env;
>      vcpu->env->thread_id = kvm_get_thread_id();
>      sigfillset(&signals);
> -    sigdelset(&signals, SIG_IPI);
>      sigprocmask(SIG_BLOCK, &signals, NULL);
>      kvm_create_vcpu(kvm_context, env->cpu_index);
>      kvm_qemu_init_env(env);
>
>   

Does this work with -no-kvm-irqchip?

I think we need to fix the kernel to handle random signals.  Otherwise 
even attaching a debugger can change guest behavior (I think).

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


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [patch 3/4] QEMU/KVM: ignore SIG_IPI signals in userspace

[Marcelo Tosatti]

On Sun, Apr 06, 2008 at 12:15:07PM +0300, Avi Kivity wrote:
> Marcelo Tosatti wrote:
> >Otherwise a signal can be received in userspace and a vcpu goes back
> >to the kernel while it should stay still.
> >
> >Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> >
> >Index: kvm-userspace.io/qemu/qemu-kvm.c
> >===================================================================
> >--- kvm-userspace.io.orig/qemu/qemu-kvm.c
> >+++ kvm-userspace.io/qemu/qemu-kvm.c
> >@@ -350,7 +350,6 @@ static void *ap_main_loop(void *_env)
> >     vcpu->env = env;
> >     vcpu->env->thread_id = kvm_get_thread_id();
> >     sigfillset(&signals);
> >-    sigdelset(&signals, SIG_IPI);
> >     sigprocmask(SIG_BLOCK, &signals, NULL);
> >     kvm_create_vcpu(kvm_context, env->cpu_index);
> >     kvm_qemu_init_env(env);
> >
> >  
> 
> Does this work with -no-kvm-irqchip?

Yes. SIG_IPI was blocked before the IO thread.

> I think we need to fix the kernel to handle random signals.  Otherwise 
> even attaching a debugger can change guest behavior (I think).

Well ptrace forces signals so SIGSTOP is delivered even though the child
has blocked them.

Attaching a debugger does change behaviour since SIGSTOP will send a
vcpu back to userspace.

Can you be more specific?


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [patch 3/4] QEMU/KVM: ignore SIG_IPI signals in userspace

[Avi Kivity]

Marcelo Tosatti wrote:
> On Sun, Apr 06, 2008 at 12:15:07PM +0300, Avi Kivity wrote:
>   
>> Marcelo Tosatti wrote:
>>     
>>> Otherwise a signal can be received in userspace and a vcpu goes back
>>> to the kernel while it should stay still.
>>>
>>> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>>>
>>> Index: kvm-userspace.io/qemu/qemu-kvm.c
>>> ===================================================================
>>> --- kvm-userspace.io.orig/qemu/qemu-kvm.c
>>> +++ kvm-userspace.io/qemu/qemu-kvm.c
>>> @@ -350,7 +350,6 @@ static void *ap_main_loop(void *_env)
>>>     vcpu->env = env;
>>>     vcpu->env->thread_id = kvm_get_thread_id();
>>>     sigfillset(&signals);
>>> -    sigdelset(&signals, SIG_IPI);
>>>     sigprocmask(SIG_BLOCK, &signals, NULL);
>>>     kvm_create_vcpu(kvm_context, env->cpu_index);
>>>     kvm_qemu_init_env(env);
>>>
>>>  
>>>       
>> Does this work with -no-kvm-irqchip?
>>     
>
> Yes. SIG_IPI was blocked before the IO thread.
>
>   

Okay (of course; it is blocked, but still dequeued by sigtimedwait).

>> I think we need to fix the kernel to handle random signals.  Otherwise 
>> even attaching a debugger can change guest behavior (I think).
>>     
>
> Well ptrace forces signals so SIGSTOP is delivered even though the child
> has blocked them.
>
> Attaching a debugger does change behaviour since SIGSTOP will send a
> vcpu back to userspace.
>
> Can you be more specific?
>
>   

I misunderstood. I thought something about a spurious signal being 
received in the kernel, dropping it out of hlt state, and confusing the 
guest; but the real issue is the signal getting lost completely if it is 
delivered to userspace instead of sigtimedwait(), which this patch fixes.

-- 
Any sufficiently difficult bug is indistinguishable from a feature.


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

[patch 4/4] QEMU/KVM: dont read any bits from userspace APIC emulation if its done in-kernel

[Marcelo Tosatti]

[-- Attachment #1: apic-save --]
[-- Type: text/plain, Size: 1226 bytes --]

Fixes loadvm/savem on SMP.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm-userspace.io/qemu/hw/apic.c
===================================================================
--- kvm-userspace.io.orig/qemu/hw/apic.c
+++ kvm-userspace.io/qemu/hw/apic.c
@@ -248,8 +248,11 @@ void cpu_set_apic_base(CPUState *env, ui
 #ifdef DEBUG_APIC
     printf("cpu_set_apic_base: %016" PRIx64 "\n", val);
 #endif
-    s->apicbase = (val & 0xfffff000) |
-        (s->apicbase & (MSR_IA32_APICBASE_BSP | MSR_IA32_APICBASE_ENABLE));
+    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel())
+        s->apicbase = val;
+    else
+        s->apicbase = (val & 0xfffff000) |
+            (s->apicbase & (MSR_IA32_APICBASE_BSP | MSR_IA32_APICBASE_ENABLE));
     /* if disabled, cannot be enabled again */
     if (!(val & MSR_IA32_APICBASE_ENABLE)) {
         s->apicbase &= ~MSR_IA32_APICBASE_ENABLE;

-- 


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [patch 4/4] QEMU/KVM: dont read any bits from userspace APIC emulation if its done in-kernel

[Avi Kivity]

Marcelo Tosatti wrote:
> Fixes loadvm/savem on SMP.
>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
>
> Index: kvm-userspace.io/qemu/hw/apic.c
> ===================================================================
> --- kvm-userspace.io.orig/qemu/hw/apic.c
> +++ kvm-userspace.io/qemu/hw/apic.c
> @@ -248,8 +248,11 @@ void cpu_set_apic_base(CPUState *env, ui
>  #ifdef DEBUG_APIC
>      printf("cpu_set_apic_base: %016" PRIx64 "\n", val);
>  #endif
> -    s->apicbase = (val & 0xfffff000) |
> -        (s->apicbase & (MSR_IA32_APICBASE_BSP | MSR_IA32_APICBASE_ENABLE));
> +    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel())
> +        s->apicbase = val;
> +    else
> +        s->apicbase = (val & 0xfffff000) |
> +            (s->apicbase & (MSR_IA32_APICBASE_BSP | MSR_IA32_APICBASE_ENABLE));
>      /* if disabled, cannot be enabled again */
>      if (!(val & MSR_IA32_APICBASE_ENABLE)) {
>          s->apicbase &= ~MSR_IA32_APICBASE_ENABLE;
>
>   

Can you explain how the existing code fails?

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


-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

Re: [patch 4/4] QEMU/KVM: dont read any bits from userspace APIC emulation if its done in-kernel

[Marcelo Tosatti]

On Sun, Apr 06, 2008 at 12:18:20PM +0300, Avi Kivity wrote:
> Marcelo Tosatti wrote:
> >Fixes loadvm/savem on SMP.
> >
> >Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> >
> >Index: kvm-userspace.io/qemu/hw/apic.c
> >===================================================================
> >--- kvm-userspace.io.orig/qemu/hw/apic.c
> >+++ kvm-userspace.io/qemu/hw/apic.c
> >@@ -248,8 +248,11 @@ void cpu_set_apic_base(CPUState *env, ui
> > #ifdef DEBUG_APIC
> >     printf("cpu_set_apic_base: %016" PRIx64 "\n", val);
> > #endif
> >-    s->apicbase = (val & 0xfffff000) |
> >-        (s->apicbase & (MSR_IA32_APICBASE_BSP | 
> >MSR_IA32_APICBASE_ENABLE));
> >+    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel())
> >+        s->apicbase = val;
> >+    else
> >+        s->apicbase = (val & 0xfffff000) |
> >+            (s->apicbase & (MSR_IA32_APICBASE_BSP | 
> >MSR_IA32_APICBASE_ENABLE));
> >     /* if disabled, cannot be enabled again */
> >     if (!(val & MSR_IA32_APICBASE_ENABLE)) {
> >         s->apicbase &= ~MSR_IA32_APICBASE_ENABLE;
> >
> >  
> 
> Can you explain how the existing code fails?

It fails because apic_load (qemu/hw/apic.c), which runs
after cpu_load, will set the apic base value saved by
QEMU. And for some reason cpu_set_apic_base() uses the
MSR_IA32_APICBASE_BSP/MSR_IA32_APICBASE_ENABLE bits from s->apicbase
instead of what the caller passed. That breaks if in-kernel APIC
emulation is being used.

So this is the same bug which caused vmport to disable the APIC 
for SMP guests on X11.

I think it is better to revert that partial fix and instead make sure
that cpu_get_apic_base() will return the proper value.

This also fixes
https://sourceforge.net/tracker/?func=detail&atid=893831&aid=1936539&group_id=180599


QEMU/KVM: properly copy the in-kernel apicbase value

The MSR_IA32_APICBASE_ENABLE/MSR_IA32_APICBASE_BSP bits in s->apicbase
are not initialized if in-kernel APIC emulation is used, so save the 
actual value passed by cpu_set_apic_base() caller.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>

Index: kvm-userspace.io/qemu/hw/apic.c
===================================================================
--- kvm-userspace.io/qemu/hw/apic.c
+++ kvm-userspace.io/qemu/hw/apic.c
@@ -248,8 +248,11 @@ void cpu_set_apic_base(CPUState *env, ui
 #ifdef DEBUG_APIC
     printf("cpu_set_apic_base: %016" PRIx64 "\n", val);
 #endif
-    s->apicbase = (val & 0xfffff000) |
-        (s->apicbase & (MSR_IA32_APICBASE_BSP | MSR_IA32_APICBASE_ENABLE));
+    if (kvm_enabled() && qemu_kvm_irqchip_in_kernel())
+        s->apicbase = val;
+    else
+        s->apicbase = (val & 0xfffff000) |
+            (s->apicbase & (MSR_IA32_APICBASE_BSP | MSR_IA32_APICBASE_ENABLE));
     /* if disabled, cannot be enabled again */
     if (!(val & MSR_IA32_APICBASE_ENABLE)) {
         s->apicbase &= ~MSR_IA32_APICBASE_ENABLE;
Index: marcelo/git/kvm-userspace.io/qemu/qemu-kvm-x86.c
===================================================================
--- marcelo.orig/git/kvm-userspace.io/qemu/qemu-kvm-x86.c
+++ marcelo/git/kvm-userspace.io/qemu/qemu-kvm-x86.c
@@ -248,13 +248,8 @@ void kvm_arch_load_regs(CPUState *env)
     sregs.cr3 = env->cr[3];
     sregs.cr4 = env->cr[4];
 
-    if (kvm_irqchip_in_kernel(kvm_context)) {
-        sregs.cr8 = kvm_get_cr8(kvm_context, env->cpu_index);
-        sregs.apic_base = kvm_get_apic_base(kvm_context, env->cpu_index);
-    } else {
-        sregs.cr8 = cpu_get_apic_tpr(env);
-        sregs.apic_base = cpu_get_apic_base(env);
-    }
+    sregs.cr8 = cpu_get_apic_tpr(env);
+    sregs.apic_base = cpu_get_apic_base(env);
 
     sregs.efer = env->efer;
 

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Register now and save $200. Hurry, offer ends at 11:59 p.m., 
Monday, April 7! Use priority code J8TLD2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone