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

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

On Tue, Nov 26, 2019 at 05:50:32PM +0100, Jan Beulich wrote:
> On 26.11.2019 14:26, Roger Pau Monne wrote:
> > --- a/xen/arch/x86/hvm/irq.c
> > +++ b/xen/arch/x86/hvm/irq.c
> > @@ -515,7 +515,11 @@ void hvm_set_callback_via(struct domain *d, uint64_t 
> > via)
> >  struct hvm_intack hvm_vcpu_has_pending_irq(struct vcpu *v)
> >  {
> >      struct hvm_domain *plat = &v->domain->arch.hvm;
> > -    int vector;
> > +    /*
> > +     * Always call vlapic_has_pending_irq so that PIR is synced into IRR 
> > when
> > +     * using posted interrupts.
> > +     */
> > +    int vector = vlapic_has_pending_irq(v);
> 
> Did you consider doing this conditionally either here ...
> 
> > @@ -530,7 +534,6 @@ struct hvm_intack hvm_vcpu_has_pending_irq(struct vcpu 
> > *v)
> >      if ( vlapic_accept_pic_intr(v) && plat->vpic[0].int_output )
> >          return hvm_intack_pic(0);
> >  
> > -    vector = vlapic_has_pending_irq(v);
> >      if ( vector != -1 )
> >          return hvm_intack_lapic(vector);
> 
> ... or here?

I'm afraid I don't follow. The whole point of this change is to ensure
vlapic_has_pending_irq is unconditionally called in
hvm_vcpu_has_pending_irq, so I'm not sure what you mean by "doing this
conditionally...".

> I ask not only because the function isn't exactly
> cheap to call (as iirc you did also mention during the v2
> discussion), but also because of its interaction with Viridian
> and nested mode. In case of problems there, avoiding the use
> of interrupt posting would be a workaround in such cases then.

Would you like me to export something like vlapic_sync_pir_to_irr and
call it unconditionally instead of calling vlapic_has_pending_irq?

Thanks, Roger.

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