Re: [Xen-devel] [PATCH for-4.13 v3 1/2] x86/vmx: add ASSERT to prevent syncing PIR to IRR...

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Nov 26, 2019 at 05:32:04PM +0100, Jan Beulich wrote:
> On 26.11.2019 14:26, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/hvm/vmx/vmx.c
> > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> > @@ -2054,6 +2054,19 @@ static void vmx_sync_pir_to_irr(struct vcpu *v)
> >      unsigned int group, i;
> >      DECLARE_BITMAP(pending_intr, NR_VECTORS);
> >  
> > +    if ( v != current && !atomic_read(&v->pause_count) )
> > +    {
> > +        /*
> > +         * Syncing PIR to IRR must not be done behind the back of the CPU,
> > +         * since the IRR is controlled by the hardware when the vCPU is
> > +         * executing. Only allow Xen to do such sync if the vCPU is the 
> > current
> > +         * one or if it's paused: that's required in order to sync the 
> > lapic
> > +         * state before saving it.
> > +         */
> 
> Is this stated this way by the SDM anywhere?

No, I think the SDM is not very clear on this, there's a paragraph
about PIR:

"The logical processor performs a logical-OR of PIR into VIRR and
clears PIR. No other agent can read or write a PIR bit (or group of
bits) between the time it is read (to determine what to OR into VIRR)
and when it is cleared."

> I ask because the
> comment then really doesn't apply to just this function, but to
> vlapic_{,test_and_}{set,clear}_vector() more generally. It's
> not clear to me at all whether the CPU caches (in an incoherent
> fashion) IRR (and maybe other APIC page elements), rather than
> honoring the atomic updates these macros do.

IMO syncing PIR to IRR when the vCPU is running on a different pCPU is
likely to at least defeat the purpose of posted interrupts: when the
CPU receives the posted interrupt vector it won't see the
outstanding-notification bit in the posted-interrupt descriptor
because the sync done from a different pCPU would have cleared it, at
which point it's not clear to me that the processor will check vIRR
for pending interrupts. The description in section 29.6
POSTED-INTERRUPT PROCESSING doesn't explicitly mention whether the
value of the outstanding-notification bit affects the logic of posted
interrupt processing.

Thanks, Roger.

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