Re: [PATCH 2/2] x86, apicv: Add Posted Interrupt supporting

[Marcelo Tosatti <mtosatti@redhat.com>]

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.

Or maybe i don't get what you say... write a complete
description?

> > 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".

Any example how software relies on such two-interrupts-queued-in-IRR/ISR behaviour?