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: Thursday, June 18, 2015 4:10 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 18.06.15 at 10:01, <feng.wu@xxxxxxxxx> wrote:
> 
> >
> >> -----Original Message-----
> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> >> Sent: Thursday, June 18, 2015 3:51 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 18.06.15 at 09:34, <feng.wu@xxxxxxxxx> 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel