Skip to contentSkip to conversation

fix "info cpus" halted state display

PATCH In review Gleb Natapov 3 participants 5 messages May 13, 2010 – May 23, 2010 KVM

lore
Share & tools
lore

Gleb Natapov

author
#0 lore raw

PATCH 1/1 fix "info cpus" halted state display

When in-kernel irqchip is used env->halted is never used for anything except "info cpus" command. Halted state is synced in kvm_arch_save_mpstate() and showed by do_info_cpus() but otherwise never looked at. Zeroing it here breaks "info cpus" since before do_info_cpus() outputs env->halted in io thread it is zeroed here when vcpu thread reenters kernel.

Signed-off-by Gleb Natapov <[email protected]>
qemu-kvm-x86.c +0 -4
index 61d9331..0ec2881 100644
@@ -922,10 +922,6 @@ void kvm_arch_load_regs(CPUState *env, int level)
         if (env->kvm_vcpu_update_vapic)
             kvm_tpr_enable_vapic(env);
     }
-    if (kvm_irqchip_in_kernel()) {
-        /* Avoid deadlock: no user space IRQ will ever clear it. */
-        env->halted = 0;
-    }
 
     kvm_put_vcpu_events(env, level);
     kvm_put_debugregs(env);

--
Gleb.

#1 lore raw
Quoted 2 lines · Gleb Natapov

When in-kernel irqchip is used env->halted is never used for anything except "info cpus" command.

In fact, it's used in a few more places, namely cpu_dump_state and the gdbstub.

Quoted 5 lines

Halted state is synced in
kvm_arch_save_mpstate() and showed by do_info_cpus() but otherwise never looked at. Zeroing it here breaks "info cpus" since before do_info_cpus() outputs env->halted in io thread it is zeroed here when vcpu thread reenters kernel.

Looks good for current qemu-kvm.

Execution of kvm_cpu_exec once depended on env->halted, even for in-kernel irqchip, right? Anyway, there are not such traces left here. We will just need to look at it again when pushing in-kernel irqchips upstream as its kvm loop looks different.

Jan

Quoted 19 lines
Signed-off-by Gleb Natapov <[email protected]>
qemu-kvm-x86.c +0 -5
index 61d9331..0ec2881 100644
@@ -922,10 +922,6 @@ void kvm_arch_load_regs(CPUState *env, int level)
         if (env->kvm_vcpu_update_vapic)
             kvm_tpr_enable_vapic(env);
     }
-    if (kvm_irqchip_in_kernel()) {
-        /* Avoid deadlock: no user space IRQ will ever clear it. */
-        env->halted = 0;
-    }
 
     kvm_put_vcpu_events(env, level);
     kvm_put_debugregs(env);
--

Gleb.

author
#2 lore raw
Quoted 7 lines · Jan Kiszka
Quoted 2 lines · Gleb Natapov

When in-kernel irqchip is used env->halted is never used for anything except "info cpus" command.

In fact, it's used in a few more places, namely cpu_dump_state and the gdbstub.

Both of those places use env->halted the same way "info cpus" does:
print out cpu state. And the both call for cpu_synchronize_state() before using env->halted.

Quoted 10 lines
Quoted 5 lines

Halted state is synced in
kvm_arch_save_mpstate() and showed by do_info_cpus() but otherwise never looked at. Zeroing it here breaks "info cpus" since before do_info_cpus() outputs env->halted in io thread it is zeroed here when vcpu thread reenters kernel.

Looks good for current qemu-kvm.

Execution of kvm_cpu_exec once depended on env->halted, even for in-kernel irqchip, right?

Never in qemu-kvm AFAIR. May be at some point during merge between upstream qemu and qemu-kvm such bug was introduced,

Quoted 21 lines

Anyway, there are not such traces left here.
We will just need to look at it again when pushing in-kernel irqchips upstream as its kvm loop looks different.

Jan

Quoted 15 lines
Signed-off-by Gleb Natapov <[email protected]>
qemu-kvm-x86.c +0 -4
index 61d9331..0ec2881 100644
@@ -922,10 +922,6 @@ void kvm_arch_load_regs(CPUState *env, int level)
         if (env->kvm_vcpu_update_vapic)
             kvm_tpr_enable_vapic(env);
     }
-    if (kvm_irqchip_in_kernel()) {
-        /* Avoid deadlock: no user space IRQ will ever clear it. */
-        env->halted = 0;
-    }
Quoted 6 lines
Quoted 4 lines
qemu-kvm-x86.c +0 -1
     kvm_put_vcpu_events(env, level);
     kvm_put_debugregs(env);
--

Gleb.

--
Gleb.

author
#3 lore raw
In reply to Gleb Natapov
Quoted 7 lines · Gleb Natapov

When in-kernel irqchip is used env->halted is never used for anything except "info cpus" command. Halted state is synced in kvm_arch_save_mpstate() and showed by do_info_cpus() but otherwise never looked at. Zeroing it here breaks "info cpus" since before do_info_cpus() outputs env->halted in io thread it is zeroed here when vcpu thread reenters kernel.

Avi, what about this patch?

Quoted 17 lines
Signed-off-by Gleb Natapov <[email protected]>
qemu-kvm-x86.c +0 -5
index 61d9331..0ec2881 100644
@@ -922,10 +922,6 @@ void kvm_arch_load_regs(CPUState *env, int level)
         if (env->kvm_vcpu_update_vapic)
             kvm_tpr_enable_vapic(env);
     }
-    if (kvm_irqchip_in_kernel()) {
-        /* Avoid deadlock: no user space IRQ will ever clear it. */
-        env->halted = 0;
-    }
 
     kvm_put_vcpu_events(env, level);
     kvm_put_debugregs(env);
--
Quoted 5 lines

Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
Gleb.

#4 lore raw
Quoted 10 lines · Gleb Natapov
Quoted 8 lines · Gleb Natapov

When in-kernel irqchip is used env->halted is never used for anything except "info cpus" command. Halted state is synced in kvm_arch_save_mpstate() and showed by do_info_cpus() but otherwise never looked at. Zeroing it here breaks "info cpus" since before do_info_cpus() outputs env->halted in io thread it is zeroed here when vcpu thread reenters kernel.

Quoted 3 lines

Avi, what about this patch?

Sorry, missed. Applied now.

Signature · 1 line
error compiling committee.c: too many arguments to function