On 01/25/2011 01:59 PM, Stefan Hajnoczi wrote:
int kvm_cpu_exec(CPUState *env)
{
struct kvm_run *run = env->kvm_run;
int ret;
DPRINTF("kvm_cpu_exec()\n");
do {
This is broken because a signal handler could change env->exit_request
after this check:
#ifndef CONFIG_IOTHREAD
if (env->exit_request) {
DPRINTF("interrupt exit requested\n");
ret = 0;
break;
}
#endif
Yeah, this is classic signal/select race with ioctl(KVM_RUN) subbing in
for select. But this is supposed to be mitigated by the fact that we
block SIG_IPI except for when we execute KVM_RUN which means that we can
reliably send SIG_IPI.
Of course, that doesn't help for SIGALRM unless we send a SIG_IPI from
the SIGALRM handler which we do with the I/O thread but not w/o it. At
any rate, post stable-0.14, I want to enable I/O thread by default so I
don't know that we really need to fix this...
if (kvm_arch_process_irqchip_events(env)) {
ret = 0;
break;
}
if (env->kvm_vcpu_dirty) {
kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE);
env->kvm_vcpu_dirty = 0;
}
kvm_arch_pre_run(env, run);
cpu_single_env = NULL;
qemu_mutex_unlock_iothread();
env->exit_request might be set but we still reenter, possibly without
rearming the timer:
ret = kvm_vcpu_ioctl(env, KVM_RUN, 0);
I can think of two solutions:
1. Block SIGALRM during critical regions, not sure if the necessary
atomic signal mask capabilities are there in KVM. Haven't looked at
TCG yet either.
2. Make a portion of the timer code signal-safe and rearm the timer
from within the SIGLARM handler.
Or, switch to timerfd and stop using a signal based alarm timer.
Doesn't work for !CONFIG_IOTHREAD.
Yeah, we need to get rid of !CONFIG_IOTHREAD. We need to run select()
in parallel with TCG/KVM and interrupt the VCPUs appropriately when
select() returns.
Regards,
Anthony Liguori
Stefan
