[PATCH 1/6] env->kvm_cpu_state.init is always zero here.

[PATCH 1/6] env->kvm_cpu_state.init is always zero here.

[Gleb Natapov]

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 qemu-kvm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 2aeb17c..ec911ef 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -412,7 +412,7 @@ static int kvm_main_loop_cpu(CPUState *env)
 	    if (env->kvm_cpu_state.sipi_needed)
 	        update_regs_for_sipi(env);
     }
-	if (!env->halted && !env->kvm_cpu_state.init)
+	if (!env->halted)
 	    kvm_cpu_exec(env);
 	env->exit_request = 0;
         env->exception_index = EXCP_INTERRUPT;
-- 
1.6.2.1

[PATCH 2/6] Do not use env->halted to decide where halted state should be handled.

[Gleb Natapov]

Use kvm_irqchip_in_kernel() for that. If irq chip is not handled by
userspace kernel should be entered even when CPU is halted.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 hw/apic.c  |    3 +--
 qemu-kvm.c |    6 +++---
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/hw/apic.c b/hw/apic.c
index c5d97b2..f186202 100644
--- a/hw/apic.c
+++ b/hw/apic.c
@@ -467,8 +467,7 @@ static void apic_init_ipi(APICState *s)
 
     cpu_reset(s->cpu_env);
 
-    if (!(s->apicbase & MSR_IA32_APICBASE_BSP) &&
-        (!kvm_enabled() || !qemu_kvm_irqchip_in_kernel()))
+    if (!(s->apicbase & MSR_IA32_APICBASE_BSP))
         s->cpu_env->halted = 1;
 
     if (kvm_enabled() && !qemu_kvm_irqchip_in_kernel())
diff --git a/qemu-kvm.c b/qemu-kvm.c
index ec911ef..7676e02 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -217,6 +217,8 @@ static int has_work(CPUState *env)
 {
     if (!vm_running || (env && env->kvm_cpu_state.stopped))
 	return 0;
+    if (kvm_irqchip_in_kernel(kvm_context))
+        return 1;
     if (!env->halted)
 	return 1;
     return kvm_arch_has_work(env);
@@ -390,8 +392,6 @@ static int kvm_main_loop_cpu(CPUState *env)
     setup_kernel_sigmask(env);
 
     pthread_mutex_lock(&qemu_mutex);
-    if (kvm_irqchip_in_kernel(kvm_context))
-	env->halted = 0;
 
     kvm_qemu_init_env(env);
 #ifdef TARGET_I386
@@ -412,7 +412,7 @@ static int kvm_main_loop_cpu(CPUState *env)
 	    if (env->kvm_cpu_state.sipi_needed)
 	        update_regs_for_sipi(env);
     }
-	if (!env->halted)
+	if (!env->halted || kvm_irqchip_in_kernel(kvm_context))
 	    kvm_cpu_exec(env);
 	env->exit_request = 0;
         env->exception_index = EXCP_INTERRUPT;
-- 
1.6.2.1

[PATCH 3/6] Call kvm_arch_load_regs() instead of kvm_load_registers()

[Gleb Natapov]

The call is done from vcpu thread.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 qemu-kvm.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 7676e02..5fa7154 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -399,7 +399,7 @@ static int kvm_main_loop_cpu(CPUState *env)
 #endif
 
     cpu_single_env = env;
-    kvm_load_registers(env);
+    kvm_arch_load_regs(env);
 
     while (1) {
 	while (!has_work(env))
-- 
1.6.2.1

[PATCH 4/6] Handle vcpu init/sipi by calling a function on vcpu

[Gleb Natapov]

Instead of having special case in vcpu event loop.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 cpu-defs.h |    2 --
 qemu-kvm.c |   51 +++++++++++++++++----------------------------------
 2 files changed, 17 insertions(+), 36 deletions(-)

diff --git a/cpu-defs.h b/cpu-defs.h
index e17209a..7570096 100644
--- a/cpu-defs.h
+++ b/cpu-defs.h
@@ -140,8 +140,6 @@ typedef struct CPUWatchpoint {
 struct qemu_work_item;
 
 struct KVMCPUState {
-    int sipi_needed;
-    int init;
     pthread_t thread;
     int signalled;
     int stop;
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 5fa7154..af3fd91 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -134,19 +134,6 @@ void kvm_update_interrupt_request(CPUState *env)
     }
 }
 
-void kvm_update_after_sipi(CPUState *env)
-{
-    env->kvm_cpu_state.sipi_needed = 1;
-    kvm_update_interrupt_request(env);
-}
-
-void kvm_apic_init(CPUState *env)
-{
-    if (env->cpu_index != 0)
-	env->kvm_cpu_state.init = 1;
-    kvm_update_interrupt_request(env);
-}
-
 #include <signal.h>
 
 static int kvm_try_push_interrupts(void *opaque)
@@ -331,30 +318,32 @@ static void kvm_vm_state_change_handler(void *context, int running, int reason)
 	pause_all_threads();
 }
 
-static void update_regs_for_sipi(CPUState *env)
+static void update_regs_for_sipi(void *data)
 {
-    kvm_arch_update_regs_for_sipi(env);
-    env->kvm_cpu_state.sipi_needed = 0;
+    kvm_arch_update_regs_for_sipi(data);
 }
 
-static void update_regs_for_init(CPUState *env)
+void kvm_update_after_sipi(CPUState *env)
 {
-#ifdef TARGET_I386
-    SegmentCache cs = env->segs[R_CS];
-#endif
-
-    cpu_reset(env);
+    on_vcpu(env, update_regs_for_sipi, env);
+}
 
-#ifdef TARGET_I386
-    /* restore SIPI vector */
-    if(env->kvm_cpu_state.sipi_needed)
-        env->segs[R_CS] = cs;
-#endif
+static void update_regs_for_init(void *data)
+{
+    CPUState *env = data;
 
-    env->kvm_cpu_state.init = 0;
     kvm_arch_load_regs(env);
 }
 
+void kvm_apic_init(CPUState *env)
+{
+    if (env->cpu_index != 0) {
+        if (env->kvm_cpu_state.created)
+            on_vcpu(env, update_regs_for_init, env);
+    } else
+        kvm_update_interrupt_request(env);
+}
+
 static void setup_kernel_sigmask(CPUState *env)
 {
     sigset_t set;
@@ -406,12 +395,6 @@ static int kvm_main_loop_cpu(CPUState *env)
 	    kvm_main_loop_wait(env, 1000);
 	if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI))
 	    env->halted = 0;
-    if (!kvm_irqchip_in_kernel(kvm_context)) {
-	    if (env->kvm_cpu_state.init)
-	        update_regs_for_init(env);
-	    if (env->kvm_cpu_state.sipi_needed)
-	        update_regs_for_sipi(env);
-    }
 	if (!env->halted || kvm_irqchip_in_kernel(kvm_context))
 	    kvm_cpu_exec(env);
 	env->exit_request = 0;
-- 
1.6.2.1

Re: [PATCH 4/6] Handle vcpu init/sipi by calling a function on vcpu

[Avi Kivity]

On 06/14/2009 01:52 PM, Gleb Natapov wrote:
> Instead of having special case in vcpu event loop.
>
>    

I'm a little worried about two vcpus INITing each other simultaneously 
and deadlocking.  INIT/SIPI are async events, the initiator should not 
wait for them.

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

Re: [PATCH 4/6] Handle vcpu init/sipi by calling a function on vcpu

[Gleb Natapov]

On Mon, Jun 15, 2009 at 01:03:39PM +0300, Avi Kivity wrote:
> On 06/14/2009 01:52 PM, Gleb Natapov wrote:
>> Instead of having special case in vcpu event loop.
>>
>>    
>
> I'm a little worried about two vcpus INITing each other simultaneously  
> and deadlocking.  INIT/SIPI are async events, the initiator should not  
> wait for them.
>
I thought to add on_vcpu_async() for that (if this case is worth warring about).

--
			Gleb.

Re: [PATCH 4/6] Handle vcpu init/sipi by calling a function on vcpu

[Avi Kivity]

On 06/15/2009 01:11 PM, Gleb Natapov wrote:
> On Mon, Jun 15, 2009 at 01:03:39PM +0300, Avi Kivity wrote:
>    
>> On 06/14/2009 01:52 PM, Gleb Natapov wrote:
>>      
>>> Instead of having special case in vcpu event loop.
>>>
>>>
>>>        
>> I'm a little worried about two vcpus INITing each other simultaneously
>> and deadlocking.  INIT/SIPI are async events, the initiator should not
>> wait for them.
>>
>>      
> I thought to add on_vcpu_async() for that (if this case is worth warring about).
>    

A generic on_vcpu_async() would need to allocate, that might be expoitable.

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

Re: [PATCH 4/6] Handle vcpu init/sipi by calling a function on vcpu

[Gleb Natapov]

On Mon, Jun 15, 2009 at 01:14:21PM +0300, Avi Kivity wrote:
> On 06/15/2009 01:11 PM, Gleb Natapov wrote:
>> On Mon, Jun 15, 2009 at 01:03:39PM +0300, Avi Kivity wrote:
>>    
>>> On 06/14/2009 01:52 PM, Gleb Natapov wrote:
>>>      
>>>> Instead of having special case in vcpu event loop.
>>>>
>>>>
>>>>        
>>> I'm a little worried about two vcpus INITing each other simultaneously
>>> and deadlocking.  INIT/SIPI are async events, the initiator should not
>>> wait for them.
>>>
>>>      
>> I thought to add on_vcpu_async() for that (if this case is worth warring about).
>>    
>
> A generic on_vcpu_async() would need to allocate, that might be expoitable.
>
Then what about processing events while waiting in on_vcpu()?

--
			Gleb.

Re: [PATCH 4/6] Handle vcpu init/sipi by calling a function on vcpu

[Avi Kivity]

On 06/15/2009 01:16 PM, Gleb Natapov wrote:
>> A generic on_vcpu_async() would need to allocate, that might be expoitable.
>>
>>      
> Then what about processing events while waiting in on_vcpu()?
>
>    

Could work, but prefer a simpler solution.

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

[PATCH 5/6] Rename kvm_(load|save)_mpstate to kvm_arch_(load|save)_mpstate

[Gleb Natapov]

To be consistent with other function naming.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 qemu-kvm-ia64.c       |    4 ++--
 qemu-kvm-x86.c        |    4 ++--
 qemu-kvm.h            |    2 ++
 target-i386/machine.c |    6 +++---
 target-ia64/machine.c |    4 ++--
 5 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/qemu-kvm-ia64.c b/qemu-kvm-ia64.c
index d33c1c3..234602c 100644
--- a/qemu-kvm-ia64.c
+++ b/qemu-kvm-ia64.c
@@ -98,7 +98,7 @@ void kvm_arch_update_guest_debug(CPUState *env, struct kvm_guest_debug *dbg)
 {
 }
 
-void kvm_save_mpstate(CPUState *env)
+void kvm_arch_save_mpstate(CPUState *env)
 {
 #ifdef KVM_CAP_MP_STATE
     int r;
@@ -112,7 +112,7 @@ void kvm_save_mpstate(CPUState *env)
 #endif
 }
 
-void kvm_load_mpstate(CPUState *env)
+void kvm_arch_load_mpstate(CPUState *env)
 {
 #ifdef KVM_CAP_MP_STATE
     struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
diff --git a/qemu-kvm-x86.c b/qemu-kvm-x86.c
index 729d600..8e6fb75 100644
--- a/qemu-kvm-x86.c
+++ b/qemu-kvm-x86.c
@@ -295,7 +295,7 @@ void kvm_load_tsc(CPUState *env)
         perror("kvm_set_tsc FAILED.\n");
 }
 
-void kvm_save_mpstate(CPUState *env)
+void kvm_arch_save_mpstate(CPUState *env)
 {
 #ifdef KVM_CAP_MP_STATE
     int r;
@@ -309,7 +309,7 @@ void kvm_save_mpstate(CPUState *env)
 #endif
 }
 
-void kvm_load_mpstate(CPUState *env)
+void kvm_arch_load_mpstate(CPUState *env)
 {
 #ifdef KVM_CAP_MP_STATE
     struct kvm_mp_state mp_state = { .mp_state = env->mp_state };
diff --git a/qemu-kvm.h b/qemu-kvm.h
index fa40542..3b73fe9 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -69,6 +69,8 @@ int kvm_arch_qemu_create_context(void);
 
 void kvm_arch_save_regs(CPUState *env);
 void kvm_arch_load_regs(CPUState *env);
+void kvm_arch_load_mpstate(CPUState *env);
+void kvm_arch_save_mpstate(CPUState *env);
 int kvm_arch_qemu_init_env(CPUState *cenv);
 void kvm_arch_pre_kvm_run(void *opaque, CPUState *env);
 void kvm_arch_post_kvm_run(void *opaque, CPUState *env);
diff --git a/target-i386/machine.c b/target-i386/machine.c
index 07df1e1..14942c0 100644
--- a/target-i386/machine.c
+++ b/target-i386/machine.c
@@ -34,7 +34,7 @@ void cpu_save(QEMUFile *f, void *opaque)
 
     if (kvm_enabled()) {
         kvm_save_registers(env);
-        kvm_save_mpstate(env);
+        kvm_arch_save_mpstate(env);
     }
 
     for(i = 0; i < CPU_NB_REGS; i++)
@@ -369,12 +369,12 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
             kvm_load_tsc(env);
             if (version_id >= 5) {
                 qemu_get_be32s(f, &env->mp_state);
-                kvm_load_mpstate(env);
+                kvm_arch_load_mpstate(env);
             }
         } else {
             kvm_load_registers(env);
             kvm_load_tsc(env);
-            kvm_load_mpstate(env);
+            kvm_arch_load_mpstate(env);
         }
     }
     return 0;
diff --git a/target-ia64/machine.c b/target-ia64/machine.c
index dd205c5..70ef379 100644
--- a/target-ia64/machine.c
+++ b/target-ia64/machine.c
@@ -10,7 +10,7 @@ void cpu_save(QEMUFile *f, void *opaque)
 
     if (kvm_enabled()) {
         kvm_save_registers(env);
-        kvm_save_mpstate(env);
+        kvm_arch_save_mpstate(env);
     }
 }
 
@@ -20,7 +20,7 @@ int cpu_load(QEMUFile *f, void *opaque, int version_id)
 
     if (kvm_enabled()) {
         kvm_load_registers(env);
-        kvm_load_mpstate(env);
+        kvm_arch_load_mpstate(env);
     }
     return 0;
 }
-- 
1.6.2.1

[PATCH 6/6] Retrieve mp state info in cpu_synchronize_state()

[Gleb Natapov]

And set env->halted based on the value to show accurate vcpu state in
QEMU monitor.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
---
 qemu-kvm.c |   27 +++++++++++++++++++++++++++
 qemu-kvm.h |    2 ++
 2 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index af3fd91..41dcdc3 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -187,6 +187,33 @@ void kvm_save_registers(CPUState *env)
         on_vcpu(env, kvm_do_save_registers, env);
 }
 
+static void kvm_do_load_mpstate(void *_env)
+{
+    CPUState *env = _env;
+
+    kvm_arch_load_mpstate(env);
+}
+
+void kvm_load_mpstate(CPUState *env)
+{
+    if (kvm_enabled() && qemu_system_ready)
+        on_vcpu(env, kvm_do_load_mpstate, env);
+}
+
+static void kvm_do_save_mpstate(void *_env)
+{
+    CPUState *env = _env;
+
+    kvm_arch_save_mpstate(env);
+    env->halted = (env->mp_state == KVM_MP_STATE_HALTED);
+}
+
+void kvm_save_mpstate(CPUState *env)
+{
+    if (kvm_enabled())
+        on_vcpu(env, kvm_do_save_mpstate, env);
+}
+
 int kvm_cpu_exec(CPUState *env)
 {
     int r;
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 3b73fe9..22452e9 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -221,11 +221,13 @@ static inline int kvm_sync_vcpus(void) { return 0; }
 static inline void kvm_arch_get_registers(CPUState *env)
 {
     kvm_save_registers(env);
+    kvm_save_mpstate(env);
 }
 
 static inline void kvm_arch_put_registers(CPUState *env)
 {
     kvm_load_registers(env);
+    kvm_load_mpstate(env);
 }
 
 static inline void cpu_synchronize_state(CPUState *env, int modified)
-- 
1.6.2.1

Re: [PATCH 1/6] env->kvm_cpu_state.init is always zero here.

[Avi Kivity]

On 06/14/2009 01:52 PM, Gleb Natapov wrote:
> Signed-off-by: Gleb Natapov<gleb@redhat.com>
> ---
>   qemu-kvm.c |    2 +-
>   1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/qemu-kvm.c b/qemu-kvm.c
> index 2aeb17c..ec911ef 100644
> --- a/qemu-kvm.c
> +++ b/qemu-kvm.c
> @@ -412,7 +412,7 @@ static int kvm_main_loop_cpu(CPUState *env)
>   	    if (env->kvm_cpu_state.sipi_needed)
>   	        update_regs_for_sipi(env);
>       }
> -	if (!env->halted&&  !env->kvm_cpu_state.init)
> +	if (!env->halted)
>   	    kvm_cpu_exec(env);
>   	env->exit_request = 0;
>           env->exception_index = EXCP_INTERRUPT;
>    

Are you sure?  Can't a reset reenable it?


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

Re: [PATCH 1/6] env->kvm_cpu_state.init is always zero here.

[Gleb Natapov]

On Mon, Jun 15, 2009 at 12:55:27PM +0300, Avi Kivity wrote:
> On 06/14/2009 01:52 PM, Gleb Natapov wrote:
>> Signed-off-by: Gleb Natapov<gleb@redhat.com>
>> ---
>>   qemu-kvm.c |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/qemu-kvm.c b/qemu-kvm.c
>> index 2aeb17c..ec911ef 100644
>> --- a/qemu-kvm.c
>> +++ b/qemu-kvm.c
>> @@ -412,7 +412,7 @@ static int kvm_main_loop_cpu(CPUState *env)
>>   	    if (env->kvm_cpu_state.sipi_needed)
>>   	        update_regs_for_sipi(env);
>>       }
>> -	if (!env->halted&&  !env->kvm_cpu_state.init)
>> +	if (!env->halted)
>>   	    kvm_cpu_exec(env);
>>   	env->exit_request = 0;
>>           env->exception_index = EXCP_INTERRUPT;
>>    
>
> Are you sure?  Can't a reset reenable it?
>
The thing is used only with userspace irq chip. If
env->kvm_cpu_state.init == 1 update_regs_for_init() is called
three line above and it will set kvm_cpu_state.init to zero.

--
			Gleb.

Re: [PATCH 1/6] env->kvm_cpu_state.init is always zero here.

[Avi Kivity]

On 06/15/2009 12:58 PM, Gleb Natapov wrote:
> On Mon, Jun 15, 2009 at 12:55:27PM +0300, Avi Kivity wrote:
>    
>> On 06/14/2009 01:52 PM, Gleb Natapov wrote:
>>      
>>> Signed-off-by: Gleb Natapov<gleb@redhat.com>
>>> ---
>>>    qemu-kvm.c |    2 +-
>>>    1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/qemu-kvm.c b/qemu-kvm.c
>>> index 2aeb17c..ec911ef 100644
>>> --- a/qemu-kvm.c
>>> +++ b/qemu-kvm.c
>>> @@ -412,7 +412,7 @@ static int kvm_main_loop_cpu(CPUState *env)
>>>    	    if (env->kvm_cpu_state.sipi_needed)
>>>    	        update_regs_for_sipi(env);
>>>        }
>>> -	if (!env->halted&&   !env->kvm_cpu_state.init)
>>> +	if (!env->halted)
>>>    	    kvm_cpu_exec(env);
>>>    	env->exit_request = 0;
>>>            env->exception_index = EXCP_INTERRUPT;
>>>
>>>        
>> Are you sure?  Can't a reset reenable it?
>>
>>      
> The thing is used only with userspace irq chip. If
> env->kvm_cpu_state.init == 1 update_regs_for_init() is called
> three line above and it will set kvm_cpu_state.init to zero.
>    

Right, and nothing can sleep in between.

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