Re: [Xen-devel] [RFC v2 14/15] Suppress posting interrupts when 'SN' is set

["Wu, Feng" <feng.wu@xxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Friday, June 12, 2015 6:47 PM
> To: Wu, Feng
> Cc: andrew.cooper3@xxxxxxxxxx; george.dunlap@xxxxxxxxxxxxx; Tian, Kevin;
> Zhang, Yang Z; xen-devel@xxxxxxxxxxxxx; keir@xxxxxxx
> Subject: RE: [RFC v2 14/15] Suppress posting interrupts when 'SN' is set
> 
> >>> On 12.06.15 at 11:40, <feng.wu@xxxxxxxxx> wrote:
> 
> >
> >> -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> Sent: Wednesday, June 10, 2015 2:49 PM
> >> To: Wu, Feng
> >> Cc: andrew.cooper3@xxxxxxxxxx; george.dunlap@xxxxxxxxxxxxx; Tian, Kevin;
> >> Zhang, Yang Z; xen-devel@xxxxxxxxxxxxx; keir@xxxxxxx
> >> Subject: Re: [RFC v2 14/15] Suppress posting interrupts when 'SN' is set
> >>
> >> >>> On 08.05.15 at 11:07, <feng.wu@xxxxxxxxx> wrote:
> >> > --- a/xen/arch/x86/hvm/vmx/vmx.c
> >> > +++ b/xen/arch/x86/hvm/vmx/vmx.c
> >> > @@ -1664,9 +1664,20 @@ static void
> >> __vmx_deliver_posted_interrupt(struct vcpu *v)
> >> >
> >> >  static void vmx_deliver_posted_intr(struct vcpu *v, u8 vector)
> >> >  {
> >> > +    int r, sn;
> >> > +
> >> >      if ( pi_test_and_set_pir(vector, &v->arch.hvm_vmx.pi_desc) )
> >> >          return;
> >> >
> >> > +    /*
> >> > +     * Currently, we don't support urgent interrupt, all interrupts
> >> > +     * are recognized as non-urgent interrupt, so we cannot send
> >> > +     * posted-interrupt when 'SN' is set.
> >> > +     */
> >> > +
> >> > +    sn = v->arch.hvm_vmx.pi_desc.sn;
> >> > +    r = pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc);
> >>
> >> I'm probably misunderstanding something here, but to me this looks
> >> like a change that would need to be done quite a bit earlier in the
> >> series (i.e. at this point it looks like it's fixing a bug/oversight of an
> >> earlier patch).
> >
> > From hardware p.o.v, if 'SN' is set, the hardware doesn't send notification
> > event.
> > vmx_deliver_posted_intr() is the software way to delivery posted-interrupts,
> > so
> > we need to follow the HW's behavior. Hence we check 'SN' first, and not send
> > notification event if it is set.
> 
> Right, that's what I understood; my question wasn't "why", but
> "doesn't this need to be done earlier in the series".

Yes, it should be done earlier.

Thanks,
Feng

> 
> >> Apart from that I'm also not understanding the synchronization
> >> aspect here: What if SN gets set after having been latched above,
> >> but before the latched value gets looked at below?
> >
> > Yes, that is a question. Here is the scenario your described above, right?
> >
> >     ......
> >
> >     sn = v->arch.hvm_vmx.pi_desc.sn; /*sn gets 0 here*/
> >
> >     v->arch.hvm_vmx.pi_desc.sn gets set by others
> >
> >     else if ( !r && !sn ) /*Oops, sn cannot reflect the real
> v->arch.hvm_vmx.pi_desc.sn here*/
> >
> >     ......
> 
> Yes.
> 
> > Maybe I need think about how to handle this.
> 
> Apparently, unless you can find a reason why this is not a problem.
> 
> Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel