Re: [PATCH 1/11] QEMU/KVM: Fix deadlocks in monitor and debugger

Jan Kiszka
2008-05-27T13:01:07Z

Avi Kivity wrote:
> Jan Kiszka wrote:
>> Some monitor commands as well as the vm_stop() issued by the gdbstub on
>> external interruption so far deadlock on vcpu locks in the kernel. Patch
>> below resolves the issue by temporarily or permanently stopping all vcpu
>> threads before issuing the related KVM IOCTLs. It enables, e.g., to
>> break into guest code spinning in the vcpu and to use things like "info
>> cpus" in the monitor.
>>
>
> I implemented the alternative on_vcpu() approach for this (similar to
> smp_call_function_single in the kernel) which solves the livelock
> without resorting to stopping the VM.

I assume this pattern should then be applied to kvm_guest_debug (and
later on kvm_set_guest_debug) as well? You missed to fix that bug.

Here is a quick patch to complete the work, will post rebased versions
of my remaining patches later today:
-----------

Use on_vcpu to call into kvm_guest_debug. Fix on_vpuc usage in
kvm_load_registers.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
qemu/qemu-kvm.c | 32 +++++++++++++++++++++++---------
1 file changed, 23 insertions(+), 9 deletions(-)

Index: b/qemu/qemu-kvm.c
===================================================================
--- a/qemu/qemu-kvm.c
+++ b/qemu/qemu-kvm.c
@@ -187,7 +187,7 @@ static void kvm_do_load_registers(void *
void kvm_load_registers(CPUState *env)
{
if (kvm_enabled())
- on_vcpu(env->cpu_index, kvm_do_load_registers, env);
+ on_vcpu(env, kvm_do_load_registers, env);
}

static void kvm_do_save_registers(void *_env)
@@ -816,23 +816,37 @@ int kvm_qemu_init_env(CPUState *cenv)
return kvm_arch_qemu_init_env(cenv);
}

+struct kvm_guest_debug_data {
+ struct kvm_debug_guest dbg;
+ int err;
+};
+
+void kvm_invoke_guest_debug(void *data)
+{
+ struct kvm_guest_debug_data *dbg_data = data;
+
+ dbg_data->err = kvm_guest_debug(kvm_context, cpu_single_env->cpu_index,
+ &dbg_data->dbg);
+}
+
int kvm_update_debugger(CPUState *env)
{
- struct kvm_debug_guest dbg;
+ struct kvm_guest_debug_data data;
int i;

- memset(dbg.breakpoints, 0, sizeof(dbg.breakpoints));
+ memset(data.dbg.breakpoints, 0, sizeof(data.dbg.breakpoints));

- dbg.enabled = 0;
+ data.dbg.enabled = 0;
if (env->nb_breakpoints || env->singlestep_enabled) {
- dbg.enabled = 1;
+ data.dbg.enabled = 1;
for (i = 0; i < 4 && i < env->nb_breakpoints; ++i) {
- dbg.breakpoints[i].enabled = 1;
- dbg.breakpoints[i].address = env->breakpoints[i];
+ data.dbg.breakpoints[i].enabled = 1;
+ data.dbg.breakpoints[i].address = env->breakpoints[i];
}
- dbg.singlestep = env->singlestep_enabled;
+ data.dbg.singlestep = env->singlestep_enabled;
}
- return kvm_guest_debug(kvm_context, env->cpu_index, &dbg);
+ on_vcpu(env, kvm_invoke_guest_debug, &data);
+ return data.err;
}