On Mon, Feb 04, 2013 at 11:55:53AM +0200, Gleb Natapov wrote:
> On Sun, Feb 03, 2013 at 10:57:00PM -0200, Marcelo Tosatti wrote:
> > On Thu, Jan 31, 2013 at 03:55:56PM +0200, Gleb Natapov wrote:
> > > On Thu, Jan 31, 2013 at 11:44:43AM -0200, Marcelo Tosatti wrote:
> > > > On Thu, Jan 31, 2013 at 03:38:37PM +0200, Gleb Natapov wrote:
> > > > > On Thu, Jan 31, 2013 at 11:32:45AM -0200, Marcelo Tosatti wrote:
> > > > > > On Thu, Jan 31, 2013 at 11:43:48AM +0200, Gleb Natapov wrote:
> > > > > > > On Wed, Jan 30, 2013 at 09:03:11PM -0200, Marcelo Tosatti wrote:
> > > > > > > > Posted interrupt patch:
> > > > > > > > 2) Must move IN_GUEST_MODE assignment after local_irq_disable, in
> > > > > > > > vcpu_enter_guest function. Otherwise:
> > > > > > > >
> > > > > > > > cpu0 vcpu1<->cpu1
> > > > > > > >
> > > > > > > > vcpu->mode = IN_GUEST_MODE
> > > > > > > >
> > > > > > > > if IN_GUEST_MODE == true
> > > > > > > > send IPI
> > > > > > > > local_irq_disable
> > > > > > > >
> > > > > > > > PIR not transferred to VIRR, misses interrupt.
> > > > > > > >
> > > > > > > cpu0 will set KVM_REQ_EVENT, so vmentry will be aborted after
> > > > > > > local_irq_disable() by ->requests check.
> > > > > >
> > > > > > Yes, but you don't want KVM_REQ_EVENT+kick. It defeats the purpose
> > > > > > of posted interrupts. You want
> > > > > >
> > > > > > if vcpu in guest mode
> > > > > > send posted interrupt IPI
> > > > > > else
> > > > > > KVM_REQ_EVENT+kick
> > > > > >
> > > > > I am thinking:
> > > > >
> > > > > set KVM_REQ_EVENT
> > > > > if pi is enabled
> > > > > send posted interrupt IPI
> > > > > else
> > > > > kick
> > > >
> > > > KVM_REQ_EVENT must be after sending posted interrupt IPI. Otherwise on
> > > > the vcpu entry side
> > > >
> > > > test_and_clear(KVM_REQ_EVENT) {
> > > > No bits set in PIR
> > > > }
> > > >
> > > It should be after updating PIR, but before sending posted interrupt
> > > IPI. Otherwise:
> > >
> > > cpu0 cpu1/vcpu
> > >
> > > KVM_REQ_EVENT is not set
> > > set pir
> > > send IPI
> > > irq_disable()
> > > ->request is empty.
> > > set KVM_REQ_EVENT
> > >
> > > That's the same sequence as with IRR update, KVM_REQ_EVENT and kick
> > > today.
> >
> > Can only send IPI if vcpu->mode == IN_GUEST_MODE, which must be set
> > after interrupt flag is cleared as noted.
> >
> > Also KVM_REQ_EVENT is processed outside of interrupt disabled region today.
> But it is checked in interrupt disabled section and vcpu entry is
> aborted if event is pending.
>
> >
> > Or maybe i don't get what you say... write a complete
> > description?
> >
> I am saying that we do not need to move vcpu->mode = IN_GUEST_MODE to
> irq_disable() section to make things work. Just do:
>
> set bit in pir
> set KVM_REQ_EVENT
> if in guest mode do IPI
I see. Yeah, probably.
> > > > What about item 4 below?
> > > >
> > > That's for Yang to answer :)
> >
> > "If more than one interrupt is generated with the same vector number,
> > the local APIC can set the bit for the vector both in the IRR and ISR.
> > This means that for the Pentium 4 and Intel Xeon processors, the IRR
> > and ISR can queue two interrupts for each interrupt vector: one in the
> > IRR and one in the ISR. Any additional interrupts issued for the same
> > interrupt vector are collapsed into the single bit in IRR"
> >
> > Which would mean KVM emulation differs... "Eventually 3 interrupts can
> > be queued: one in IRR, one in ISR, one in PIR".
> I do not think this is the case though. PIR will be folded into IRR
> either during a guest entry or by vcpu itself on receiving of
> notification vector IPI.
>
> >
> > Any example how software relies on such two-interrupts-queued-in-IRR/ISR behaviour?
> Don't know about guests, but KVM relies on it to detect interrupt
> coalescing. So if interrupt is set in IRR but not in PIR interrupt will
> not be reported as coalesced, but it will be coalesced during PIR->IRR
> merge.
Yes, so:
1. IRR=1, ISR=0, PIR=0. Event: set_irq, coalesced=no.
2. IRR=0, ISR=1, PIR=0. Event: IRR->ISR transfer.
3. vcpu outside of guest mode.
4. IRR=1, ISR=1, PIR=0. Event: set_irq, coalesced=no.
5. vcpu enters guest mode.
6. IRR=1, ISR=1, PIR=1. Event: set_irq, coalesced=no.
7. HW transfers PIR into IRR.
set_irq return value at 7 is incorrect, interrupt event was _not_
queued.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
