From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [Qemu-devel] KVM: Windows 64-bit troubles with user space irqchip Date: Thu, 03 Feb 2011 15:27:51 +0100 Message-ID: <4D4ABB67.9070208@siemens.com> References: <4D496D77.2010405@siemens.com> <4D496FA6.8070301@siemens.com> <4D49738D.7080404@redhat.com> <4D4979BD.6080900@siemens.com> <20110202154611.GR14984@redhat.com> <4D497DAB.7010901@siemens.com> <4D4A64F2.8010309@redhat.com> <4D4A7629.1010506@siemens.com> <20110203100407.GA2449@amt.cnet> <4D4A7F4B.6050406@siemens.com> <20110203141537.GY14984@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , Avi Kivity , kvm , qemu-devel To: Gleb Natapov Return-path: Received: from goliath.siemens.de ([192.35.17.28]:15325 "EHLO goliath.siemens.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755855Ab1BCO2H (ORCPT ); Thu, 3 Feb 2011 09:28:07 -0500 In-Reply-To: <20110203141537.GY14984@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2011-02-03 15:15, Gleb Natapov wrote: > On Thu, Feb 03, 2011 at 11:11:23AM +0100, Jan Kiszka wrote: >> On 2011-02-03 11:04, Marcelo Tosatti wrote: >>> On Thu, Feb 03, 2011 at 10:32:25AM +0100, Jan Kiszka wrote: >>>> On 2011-02-03 09:18, Avi Kivity wrote: >>>>> On 02/02/2011 05:52 PM, Jan Kiszka wrote: >>>>>>>> >>>>>>> If there is no problem in the logic of this commit (and I do not see >>>>>>> one yet) then we somewhere miss kicking vcpu when interrupt, that should be >>>>>>> handled, arrives? >>>>>> >>>>>> I'm not yet confident about the logic of the kernel patch: mov to cr8 is >>>>>> serializing. If the guest raises the tpr and then signals this with a >>>>>> succeeding, non vm-exiting instruction to the other vcpus, one of those >>>>>> could inject an interrupt with a higher priority than the previous tpr, >>>>>> but a lower one than current tpr. QEMU user space would accept this >>>>>> interrupt - and would likely surprise the guest. Do I miss something? >>>>> >>>>> apic_get_interrupt() is only called from the vcpu thread, so it should >>>>> see a correct tpr. >>>>> >>>>> The only difference I can see with the patch is that we may issue a >>>>> spurious cpu_interrupt(). But that shouldn't do anything bad, should it? >>>> >>>> I tested this yesterday, and it doesn't confuse Windows. It actually >>>> receives quite a few spurious IRQs in normal operation, w/ or w/o the >>>> kernel's tpr optimization. >>> >>> http://www.mail-archive.com/kvm@vger.kernel.org/msg41681.html >> >> Don't get the scenario yet: We do not inject (or set isr) over the >> context of apic_set_irq caller. >> >>> >>> tpr of a vcpu should always be inspected in vcpu context, instead of >>> iothread context? >> >> Maybe this is true for the in-kernel model, but I don't see the issue >> (anymore) for the way user space works. >> > With patch below I can boot Windows7. > > diff --git a/hw/apic.c b/hw/apic.c > index 146deca..fdcac88 100644 > --- a/hw/apic.c > +++ b/hw/apic.c > @@ -600,7 +600,7 @@ int apic_get_interrupt(DeviceState *d) > intno = get_highest_priority_int(s->irr); > if (intno < 0) > return -1; > - if (s->tpr && intno <= s->tpr) > + if ((s->tpr >> 4) && (intno >> 4) <= (s->tpr >> 4)) > return s->spurious_vec & 0xff; > reset_bit(s->irr, intno); > set_bit(s->isr, intno); > -- > Gleb. Cool, /me too. I would just suggest diff --git a/hw/apic.c b/hw/apic.c index 05a115f..13bd7b4 100644 --- a/hw/apic.c +++ b/hw/apic.c @@ -582,6 +582,7 @@ int apic_get_interrupt(DeviceState *d) { APICState *s = DO_UPCAST(APICState, busdev.qdev, d); int intno; + int tpr; /* if the APIC is installed or enabled, we let the 8259 handle the IRQs */ @@ -594,8 +595,10 @@ int apic_get_interrupt(DeviceState *d) intno = get_highest_priority_int(s->irr); if (intno < 0) return -1; - if (s->tpr && intno <= s->tpr) + tpr = s->tpr >> 4; + if (tpr && (intno >> 4) <= tpr) { return s->spurious_vec & 0xff; + } reset_bit(s->irr, intno); set_bit(s->isr, intno); apic_update_irq(s); Unfortunately, that issue was not related to the emulation mode problems of QEMU. Thanks! Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux