[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
