>>> On 12.06.15 at 11:40, <feng...@intel.com> wrote: > >> -----Original Message----- >> From: Jan Beulich [mailto:jbeul...@suse.com] >> Sent: Wednesday, June 10, 2015 2:49 PM >> To: Wu, Feng >> Cc: andrew.coop...@citrix.com; george.dun...@eu.citrix.com; Tian, Kevin; >> Zhang, Yang Z; xen-devel@lists.xen.org; k...@xen.org >> Subject: Re: [RFC v2 14/15] Suppress posting interrupts when 'SN' is set >> >> >>> On 08.05.15 at 11:07, <feng...@intel.com> 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". >> 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@lists.xen.org http://lists.xen.org/xen-devel
