Re: [Xen-devel] [PATCH for-4.13] x86/vmx: always sync PIR to IRR before vmentry

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Sent: Thursday, November 21, 2019 5:26 PM
> 
> On Mon, Nov 18, 2019 at 05:00:29PM +0100, Jan Beulich wrote:
> > On 18.11.2019 15:20, Roger Pau Monné  wrote:
> > > On Mon, Nov 18, 2019 at 03:00:00PM +0100, Jan Beulich wrote:
> > >> On 18.11.2019 14:46, Roger Pau Monné  wrote:
> > >>> On Mon, Nov 18, 2019 at 01:01:58PM +0100, Jan Beulich wrote:
> > >>>> On 18.11.2019 11:16, Roger Pau Monne wrote:
> > >>>>> When using posted interrupts on Intel hardware it's possible that the
> > >>>>> vCPU resumes execution with a stale local APIC IRR register because
> > >>>>> depending on the interrupts to be injected vlapic_has_pending_irq
> > >>>>> might not be called, and thus PIR won't be synced into IRR.
> > >>>>>
> > >>>>> Fix this by making sure PIR is always synced to IRR in vmx_intr_assist
> > >>>>> regardless of what interrupts are pending.
> > >>>>
> > >>>> For this part, did you consider pulling ahead to the beginning
> > >>>> of hvm_vcpu_has_pending_irq() its call to vlapic_has_pending_irq()?
> > >>>
> > >>> I assumed the order in hvm_vcpu_has_pending_irq is there for a
> reason.
> > >>> I could indeed move vlapic_has_pending_irq to the top, but then either
> > >>> the result is discarded if for example a NMI is pending injection
> > >>> (in which case there's no need to go through all the logic in
> > >>> vlapic_has_pending_irq), or we invert the priority of event
> > >>> injection.
> > >>
> > >> Changing the order of events injected is not an option afaict. The
> > >> pointless processing done is a valid concern, yet the suggestion
> > >> was specifically to have (part of) this processing to occur early.
> > >> The discarding of the result, in turn, is not a problem afaict, as
> > >> a subsequent call will return the same result (unless a higher
> > >> priority interrupt has surfaced in the meantime).
> > >
> > > Yes, that's fine. So you would prefer to move the call to
> > > vlapic_has_pending_irq before any exit path in
> > > hvm_vcpu_has_pending_irq?
> >
> > "Prefer" isn't really the way I would put it. I'd like this to be
> > considered as an alternative because, as said, I think the current
> > placement look more like a plaster than a cure. I'm also open for
> > other suggestions. But first of all I'd like to see what the VMX
> > maintainers think.
> 
> Kevin/Jun, can we please get your opinion on the above item?
> 

putting the sync within hvm_vcpu_has_pending_irq sounds better,
implying that all intermediate states must be synced back to 
architectural states anytime when software wants to check virtual
interrupt.

Thanks
Kevin

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel