Skip to contentSkip to conversation

Cleanup cpu loop

PATCH Superseded Gleb Natapov 2 participants 3 messages Jun 18, 2009 – Jun 22, 2009 KVM

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Cleanup cpu loop

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

qemu-kvm.c +21 -20
index 4129fe2..72a081d 100644
@@ -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.

#1 lore raw

qemu-kvm.c static void update_regs_for_init(CPUState *env) · line 1876

Quoted 12 lines · Gleb Natapov

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

static void flush_queued_work(CPUState *env)

qemu-kvm.c +2 -0
@@ -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?

Quoted 8 lines
qemu-kvm.c +0 -0
  static int kvm_main_loop_cpu(CPUState *env)
  {
      setup_kernel_sigmask(env);
qemu-kvm.c +0 -1
@@ -1935,19 +1941,14 @@ static int kvm_main_loop_cpu(CPUState *env)
      kvm_arch_load_regs(env);
 
      while (1) {
-	while (!has_work(env))

qemu-kvm.c static int kvm_main_loop_cpu(CPUState *env) · line 1945

Quoted 10 lines
qemu-kvm.c +1 -8
-	    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()

qemu-kvm.c static int kvm_main_loop_cpu(CPUState *env)

Quoted 12 lines
qemu-kvm.c +6 -3
+        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)

Signature · 1 line
error compiling committee.c: too many arguments to function
#2 lore raw
Quoted 9 lines · Avi Kivity
Quoted 8 lines · Gleb Natapov

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

static void flush_queued_work(CPUState *env)

(quoted diff) +0 -0
@@ -1877,6 +1871,8 @@ static void update_regs_for_init(CPUState *env)
  #endif
 
Quoted 6 lines

qemu-kvm.c static void update_regs_for_init(CPUState *env) · line 1876

Quoted 4 lines
qemu-kvm.c +2 -0
      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.