| Message ID | 20090603012345.GB30777@poweredge.glommer (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Glauber Costa wrote: > On Wed, Jun 03, 2009 at 12:32:04AM +0200, Jan Kiszka wrote: >> Glauber Costa wrote: >>> On Wed, Jun 03, 2009 at 12:01:00AM +0200, Jan Kiszka wrote: >>>> Glauber Costa wrote: >>>>> On Tue, Jun 02, 2009 at 10:35:47PM +0200, Jan Kiszka wrote: >>>>>> Glauber Costa wrote: >>>>>>> This is not kvm specific, and should do fine in plain qemu >>>>>> This is fine with plain qemu already. The problem, IIUC, is that >>>>>> in-kernel kvm irqchip does not have a chance to remove the halted state >>>>>> again. Did you test the effect of this patch on that scenario? What >>>>>> makes it safe to be removed now? >>>>> IIRC, the in kernel irqchip sets halted = 0 in the very beginning of >>>>> the vcpu initialization. >>>>> >>>>> It is tested here with in-kernel irqchip and works, so probably not >>>>> a problem, unless you can spot something. >>>> At least your patch applied alone breaks -smp >1 here. >>>> >>>> But the whole management of env->halted for the in-kernel irqchip in >>>> qemu-kvm is a bit hacky IMHO. Maybe it's time to rethink this. Would be >>>> nice to always see a consistent halted in user space, specifically for >>>> debugging purposes. >>> out of curiosity: did you apply the whole series? >> Meanwhile I did, but it makes no difference. >> > > Can you try putting the following patch before this one? If it helps you to understand the issue, I will do so later. But I *really* suggest to take this chance and develop in-kernel irqchip support that does not mess with halted, rather keeps it consistent (on register sync) and maybe adds exceptions from "if (!env->halted)" tests where required. IMHO, this is mandatory for an upstream merge! Jan
On Wed, Jun 03, 2009 at 01:01:29PM +0200, Jan Kiszka wrote: > Glauber Costa wrote: > > On Wed, Jun 03, 2009 at 12:32:04AM +0200, Jan Kiszka wrote: > >> Glauber Costa wrote: > >>> On Wed, Jun 03, 2009 at 12:01:00AM +0200, Jan Kiszka wrote: > >>>> Glauber Costa wrote: > >>>>> On Tue, Jun 02, 2009 at 10:35:47PM +0200, Jan Kiszka wrote: > >>>>>> Glauber Costa wrote: > >>>>>>> This is not kvm specific, and should do fine in plain qemu > >>>>>> This is fine with plain qemu already. The problem, IIUC, is that > >>>>>> in-kernel kvm irqchip does not have a chance to remove the halted state > >>>>>> again. Did you test the effect of this patch on that scenario? What > >>>>>> makes it safe to be removed now? > >>>>> IIRC, the in kernel irqchip sets halted = 0 in the very beginning of > >>>>> the vcpu initialization. > >>>>> > >>>>> It is tested here with in-kernel irqchip and works, so probably not > >>>>> a problem, unless you can spot something. > >>>> At least your patch applied alone breaks -smp >1 here. > >>>> > >>>> But the whole management of env->halted for the in-kernel irqchip in > >>>> qemu-kvm is a bit hacky IMHO. Maybe it's time to rethink this. Would be > >>>> nice to always see a consistent halted in user space, specifically for > >>>> debugging purposes. > >>> out of curiosity: did you apply the whole series? > >> Meanwhile I did, but it makes no difference. > >> > > > > Can you try putting the following patch before this one? > > If it helps you to understand the issue, I will do so later. > > But I *really* suggest to take this chance and develop in-kernel irqchip > support that does not mess with halted, rather keeps it consistent (on > register sync) and maybe adds exceptions from "if (!env->halted)" tests > where required. IMHO, this is mandatory for an upstream merge! > The difference between kernel/userspace irq chip is that in former case halted cpu sleeps in the kernel and in later case in the usespace (well also in the kernel but not in kvm code). We currently abuse halted to do different sleeps. Cpu loop should be reworked. -- Gleb. -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/qemu-kvm.c b/qemu-kvm.c index 68d3b92..519a6ee 100644 --- a/qemu-kvm.c +++ b/qemu-kvm.c @@ -390,8 +390,6 @@ static int kvm_main_loop_cpu(CPUState *env) setup_kernel_sigmask(env); pthread_mutex_lock(&qemu_mutex); - if (kvm_irqchip_in_kernel(kvm_context)) - env->halted = 0; kvm_qemu_init_env(env); #ifdef TARGET_I386 @@ -402,6 +400,9 @@ static int kvm_main_loop_cpu(CPUState *env) kvm_load_registers(env); while (1) { + + if (kvm_irqchip_in_kernel(kvm_context)) + env->halted = 0; while (!has_work(env)) kvm_main_loop_wait(env, 1000); if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI))