[Xen-devel] [RFC v2 14/15] Suppress posting interrupts when 'SN' is set
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. Signed-off-by: Feng Wu --- xen/arch/x86/hvm/vmx/vmx.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c index cdcc012..77a7897 100644 --- 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); + if ( unlikely(v->arch.hvm_vmx.eoi_exitmap_changed) ) { /* @@ -1676,7 +1687,7 @@ static void vmx_deliver_posted_intr(struct vcpu *v, u8 vector) */ pi_set_on(&v->arch.hvm_vmx.pi_desc); } -else if ( !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) ) +else if ( !r && !sn ) { __vmx_deliver_posted_interrupt(v); return; -- 2.1.0 ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC v2 14/15] Suppress posting interrupts when 'SN' is set
>>> On 08.05.15 at 11:07, 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). 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? Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC v2 14/15] Suppress posting interrupts when 'SN' is set
> -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, 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. > > 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*/ .. Maybe I need think about how to handle this. Thanks, Feng > > Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC v2 14/15] Suppress posting interrupts when 'SN' is set
>>> On 12.06.15 at 11:40, 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, 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
Re: [Xen-devel] [RFC v2 14/15] Suppress posting interrupts when 'SN' is set
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: Friday, June 12, 2015 6:47 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 12.06.15 at 11:40, 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, 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@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC v2 14/15] Suppress posting interrupts when 'SN' is set
> -Original Message- > From: Wu, Feng > Sent: Monday, June 15, 2015 5:20 PM > To: Jan Beulich > Cc: andrew.coop...@citrix.com; george.dun...@eu.citrix.com; Tian, Kevin; > Zhang, Yang Z; xen-devel@lists.xen.org; k...@xen.org; Wu, Feng > Subject: RE: [RFC v2 14/15] Suppress posting interrupts when 'SN' is set > > > > > -Original Message- > > From: Jan Beulich [mailto:jbeul...@suse.com] > > Sent: Friday, June 12, 2015 6:47 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 12.06.15 at 11:40, 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, 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. Seems it is a little tricky here. What we need to do for this is like something below: .. else if ( !pi_test_sn(&v->arch.hvm_vmx.pi_desc) || !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) ) { __vmx_deliver_posted_interrupt(v); return; } .. But how to handle this if 'SN' is set between pi_test_sn() and pi_test_and_set_on() ? Seems we cannot guarantee this two operations are atomic in software way.But thinking a bit more, maybe this solution is acceptable, 'SN' is only set when the vCPU's state is going to be runnable, which means the vCPU is not running in non-root, in this case, it doesn't matter whether we send notification event or not, as long as the interrupt is stored in PIR, and they will be delivered to guest during the next vm-entry. Any ideas about this? Thanks, Feng > > > > Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC v2 14/15] Suppress posting interrupts when 'SN' is set
>>> On 18.06.15 at 09:34, wrote: > Seems it is a little tricky here. What we need to do for this is > like something below: > > .. > > else if ( !pi_test_sn(&v->arch.hvm_vmx.pi_desc) || > !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) ) > { > __vmx_deliver_posted_interrupt(v); > return; > } > > .. > > But how to handle this if 'SN' is set between pi_test_sn() and > pi_test_and_set_on() ? Seems we cannot guarantee this two > operations are atomic in software way.But thinking a bit > more, maybe this solution is acceptable, 'SN' is only set when > the vCPU's state is going to be runnable, which means the > vCPU is not running in non-root, in this case, it doesn't matter > whether we send notification event or not, as long as the > interrupt is stored in PIR, and they will be delivered to guest > during the next vm-entry. > > Any ideas about this? Unless the two functions don't do what their name suggests I don't see why you need to invoke pi_test_sn() first - the purpose of pi_test_and_set_on() after all is what its name says: Test and set the flag atomically, returning the previous state. Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC v2 14/15] Suppress posting interrupts when 'SN' is set
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: Thursday, June 18, 2015 3:51 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 18.06.15 at 09:34, wrote: > > Seems it is a little tricky here. What we need to do for this is > > like something below: > > > > .. > > > > else if ( !pi_test_sn(&v->arch.hvm_vmx.pi_desc) || > > !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) ) > > { > > __vmx_deliver_posted_interrupt(v); > > return; > > } > > > > .. > > > > But how to handle this if 'SN' is set between pi_test_sn() and > > pi_test_and_set_on() ? Seems we cannot guarantee this two > > operations are atomic in software way.But thinking a bit > > more, maybe this solution is acceptable, 'SN' is only set when > > the vCPU's state is going to be runnable, which means the > > vCPU is not running in non-root, in this case, it doesn't matter > > whether we send notification event or not, as long as the > > interrupt is stored in PIR, and they will be delivered to guest > > during the next vm-entry. > > > > Any ideas about this? > > Unless the two functions don't do what their name suggests I > don't see why you need to invoke pi_test_sn() first - the purpose > of pi_test_and_set_on() after all is what its name says: Test > and set the flag atomically, returning the previous state. If 'SN' is set, then interrupts are suppress, we cannot send notification event, then we don't need to test and set 'ON', and it the purpose of this patch,right? Thanks, Feng > > Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC v2 14/15] Suppress posting interrupts when 'SN' is set
>>> On 18.06.15 at 10:01, wrote: > >> -Original Message- >> From: Jan Beulich [mailto:jbeul...@suse.com] >> Sent: Thursday, June 18, 2015 3:51 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 18.06.15 at 09:34, wrote: >> > Seems it is a little tricky here. What we need to do for this is >> > like something below: >> > >> > .. >> > >> > else if ( !pi_test_sn(&v->arch.hvm_vmx.pi_desc) || >> > !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) ) >> > { >> > __vmx_deliver_posted_interrupt(v); >> > return; >> > } >> > >> > .. >> > >> > But how to handle this if 'SN' is set between pi_test_sn() and >> > pi_test_and_set_on() ? Seems we cannot guarantee this two >> > operations are atomic in software way.But thinking a bit >> > more, maybe this solution is acceptable, 'SN' is only set when >> > the vCPU's state is going to be runnable, which means the >> > vCPU is not running in non-root, in this case, it doesn't matter >> > whether we send notification event or not, as long as the >> > interrupt is stored in PIR, and they will be delivered to guest >> > during the next vm-entry. >> > >> > Any ideas about this? >> >> Unless the two functions don't do what their name suggests I >> don't see why you need to invoke pi_test_sn() first - the purpose >> of pi_test_and_set_on() after all is what its name says: Test >> and set the flag atomically, returning the previous state. > > If 'SN' is set, then interrupts are suppress, we cannot send > notification event, then we don't need to test and set 'ON', > and it the purpose of this patch,right? Oh, sorry, the single character difference in the name didn't catch my attention. Testing the state of one bit and setting (perhaps also along with testing it) another one should be done with cmpxchg(). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC v2 14/15] Suppress posting interrupts when 'SN' is set
> -Original Message- > From: Jan Beulich [mailto:jbeul...@suse.com] > Sent: Thursday, June 18, 2015 4:10 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 18.06.15 at 10:01, wrote: > > > > >> -Original Message- > >> From: Jan Beulich [mailto:jbeul...@suse.com] > >> Sent: Thursday, June 18, 2015 3:51 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 18.06.15 at 09:34, wrote: > >> > Seems it is a little tricky here. What we need to do for this is > >> > like something below: > >> > > >> > .. > >> > > >> > else if ( !pi_test_sn(&v->arch.hvm_vmx.pi_desc) || > >> > !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) ) > >> > { > >> > __vmx_deliver_posted_interrupt(v); > >> > return; > >> > } > >> > > >> > .. > >> > > >> > But how to handle this if 'SN' is set between pi_test_sn() and > >> > pi_test_and_set_on() ? Seems we cannot guarantee this two > >> > operations are atomic in software way.But thinking a bit > >> > more, maybe this solution is acceptable, 'SN' is only set when > >> > the vCPU's state is going to be runnable, which means the > >> > vCPU is not running in non-root, in this case, it doesn't matter > >> > whether we send notification event or not, as long as the > >> > interrupt is stored in PIR, and they will be delivered to guest > >> > during the next vm-entry. > >> > > >> > Any ideas about this? > >> > >> Unless the two functions don't do what their name suggests I > >> don't see why you need to invoke pi_test_sn() first - the purpose > >> of pi_test_and_set_on() after all is what its name says: Test > >> and set the flag atomically, returning the previous state. > > > > If 'SN' is set, then interrupts are suppress, we cannot send > > notification event, then we don't need to test and set 'ON', > > and it the purpose of this patch,right? > > Oh, sorry, the single character difference in the name didn't > catch my attention. Testing the state of one bit and setting > (perhaps also along with testing it) another one should be done > with cmpxchg(). Yes, we can use cmpxchg(), but what happened if other bits are changed during this process. Please see the following pseudo code and scenario: uint64_t flag = pi_desc.control; uint64_t old = flag & ( ~(POSTED_INTR_ON | POSTED_INTR_SN) ); uint64_t new = flag | POSTED_INTR_ON; /* What should we do if other bits except ON and SN are changed here?*/ cmpxchg(&flag, old, new); I think about this for some time, unfortunately I don't get a good solution for this. Could you please give more hints? Thanks a lot! Thanks, Feng > > Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [RFC v2 14/15] Suppress posting interrupts when 'SN' is set
>>> On 23.06.15 at 11:26, wrote: >> From: Jan Beulich [mailto:jbeul...@suse.com] >> Sent: Thursday, June 18, 2015 4:10 PM >> >>> On 18.06.15 at 10:01, wrote: >> >> From: Jan Beulich [mailto:jbeul...@suse.com] >> >> Sent: Thursday, June 18, 2015 3:51 PM >> >> >>> On 18.06.15 at 09:34, wrote: >> >> > Seems it is a little tricky here. What we need to do for this is >> >> > like something below: >> >> > >> >> > .. >> >> > >> >> > else if ( !pi_test_sn(&v->arch.hvm_vmx.pi_desc) || >> >> > !pi_test_and_set_on(&v->arch.hvm_vmx.pi_desc) ) >> >> > { >> >> > __vmx_deliver_posted_interrupt(v); >> >> > return; >> >> > } >> >> > >> >> > .. >> >> > >> >> > But how to handle this if 'SN' is set between pi_test_sn() and >> >> > pi_test_and_set_on() ? Seems we cannot guarantee this two >> >> > operations are atomic in software way.But thinking a bit >> >> > more, maybe this solution is acceptable, 'SN' is only set when >> >> > the vCPU's state is going to be runnable, which means the >> >> > vCPU is not running in non-root, in this case, it doesn't matter >> >> > whether we send notification event or not, as long as the >> >> > interrupt is stored in PIR, and they will be delivered to guest >> >> > during the next vm-entry. >> >> > >> >> > Any ideas about this? >> >> >> >> Unless the two functions don't do what their name suggests I >> >> don't see why you need to invoke pi_test_sn() first - the purpose >> >> of pi_test_and_set_on() after all is what its name says: Test >> >> and set the flag atomically, returning the previous state. >> > >> > If 'SN' is set, then interrupts are suppress, we cannot send >> > notification event, then we don't need to test and set 'ON', >> > and it the purpose of this patch,right? >> >> Oh, sorry, the single character difference in the name didn't >> catch my attention. Testing the state of one bit and setting >> (perhaps also along with testing it) another one should be done >> with cmpxchg(). > > Yes, we can use cmpxchg(), but what happened if other bits are > changed during this process. Please see the following pseudo > code and scenario: > > uint64_t flag = pi_desc.control; > uint64_t old = flag & ( ~(POSTED_INTR_ON | POSTED_INTR_SN) ); > uint64_t new = flag | POSTED_INTR_ON; > > /* What should we do if other bits except ON and SN are > changed here?*/ > > cmpxchg(&flag, old, new); > > I think about this for some time, unfortunately I don't get a good > solution for this. Could you please give more hints? Thanks a lot! This is not a problem unique to your code. Just go look elsewhere - you simply need to loop over cmpxchg() (with some guarantee to be had that this loop will eventually be exited). Jan ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel