[PATCH] Cleanup cpu loop

[PATCH] Cleanup cpu loop

[Gleb Natapov]

Rearrange cpu loop to be (hopefully) more readable. Put difference
between kernel/userspace irqchip in one place.

Signed-off-by: Gleb Natapov <gleb@redhat.com>
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 4129fe2..72a081d 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -1746,15 +1746,9 @@ int kvm_cpu_exec(CPUState *env)
     return 0;
 }
 
-static int has_work(CPUState *env)
+static int is_cpu_stopped(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);
+    return !vm_running || env->kvm_cpu_state.stopped;
 }
 
 static void flush_queued_work(CPUState *env)
@@ -1877,6 +1871,8 @@ static void update_regs_for_init(CPUState *env)
 #endif
 
     cpu_reset(env);
+    /* cpu_reset() clears env->halted, cpu should be halted after init */
+    env->halted = 1;
 
 #ifdef TARGET_I386
     /* restore SIPI vector */
@@ -1920,6 +1916,16 @@ static void qemu_kvm_system_reset(void)
     resume_all_threads();
 }
 
+static void process_irqchip_events(CPUState *env)
+{
+    if (env->kvm_cpu_state.init)
+        update_regs_for_init(env);
+    if (env->kvm_cpu_state.sipi_needed)
+        update_regs_for_sipi(env);
+    if (kvm_arch_has_work(env))
+        env->halted = 0;
+}
+
 static int kvm_main_loop_cpu(CPUState *env)
 {
     setup_kernel_sigmask(env);
@@ -1935,19 +1941,14 @@ static int kvm_main_loop_cpu(CPUState *env)
     kvm_arch_load_regs(env);
 
     while (1) {
-	while (!has_work(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);
+        int run_cpu = is_cpu_stopped(env) ? 0 : 1;
+        if (run_cpu && !kvm_irqchip_in_kernel(kvm_context)) {
+            process_irqchip_events(env);
+            run_cpu = !env->halted;
         }
-	if (!env->halted || kvm_irqchip_in_kernel(kvm_context))
-	    kvm_cpu_exec(env);
-	kvm_main_loop_wait(env, 0);
+        kvm_main_loop_wait(env, run_cpu ? 0 : 1000);
+        if (run_cpu)
+            kvm_cpu_exec(env);
     }
     pthread_mutex_unlock(&qemu_mutex);
     return 0;
--
			Gleb.

Re: [PATCH] Cleanup cpu loop

[Avi Kivity]

On 06/18/2009 03:22 PM, Gleb Natapov wrote:
> Rearrange cpu loop to be (hopefully) more readable. Put difference
> between kernel/userspace irqchip in one place.
>
>
>   static void flush_queued_work(CPUState *env)
> @@ -1877,6 +1871,8 @@ static void update_regs_for_init(CPUState *env)
>   #endif
>
>       cpu_reset(env);
> +    /* cpu_reset() clears env->halted, cpu should be halted after init */
> +    env->halted = 1;
>    

How does that work for the boot cpu?

>   static int kvm_main_loop_cpu(CPUState *env)
>   {
>       setup_kernel_sigmask(env);
> @@ -1935,19 +1941,14 @@ static int kvm_main_loop_cpu(CPUState *env)
>       kvm_arch_load_regs(env);
>
>       while (1) {
> -	while (!has_work(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);
> +        int run_cpu = is_cpu_stopped(env) ? 0 : 1;
>    

run_cpu = !is_cpu_stopped()

> +        if (run_cpu&&  !kvm_irqchip_in_kernel(kvm_context)) {
> +            process_irqchip_events(env);
> +            run_cpu = !env->halted;
>           }
> -	if (!env->halted || kvm_irqchip_in_kernel(kvm_context))
> -	    kvm_cpu_exec(env);
> -	kvm_main_loop_wait(env, 0);
> +        kvm_main_loop_wait(env, run_cpu ? 0 : 1000);
> +        if (run_cpu)
> +            kvm_cpu_exec(env);
>       }
>    

A single conditional may be clearer:

if (run_cpu)
kvm_main_loop_wait(0)
kvm_cpu_exec()
else
kvm_main_loop_wait(1000)

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

Re: [PATCH] Cleanup cpu loop

[Gleb Natapov]

On Mon, Jun 22, 2009 at 04:30:51PM +0300, Avi Kivity wrote:
> On 06/18/2009 03:22 PM, Gleb Natapov wrote:
>> Rearrange cpu loop to be (hopefully) more readable. Put difference
>> between kernel/userspace irqchip in one place.
>>
>>
>>   static void flush_queued_work(CPUState *env)
>> @@ -1877,6 +1871,8 @@ static void update_regs_for_init(CPUState *env)
>>   #endif
>>
>>       cpu_reset(env);
>> +    /* cpu_reset() clears env->halted, cpu should be halted after init */
>> +    env->halted = 1;
>>    
>
> How does that work for the boot cpu?

void kvm_apic_init(CPUState *env)
{
    if (env->cpu_index != 0)
        env->kvm_cpu_state.init = 1;
    kvm_update_interrupt_request(env);
}

Boot cpu does not get here.

--
			Gleb.