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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.06.15 at 11:26, <feng.wu@xxxxxxxxx> wrote:
>> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> Sent: Thursday, June 18, 2015 4:10 PM
>> >>> On 18.06.15 at 10:01, <feng.wu@xxxxxxxxx> wrote:
>> >> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
>> >> Sent: Thursday, June 18, 2015 3:51 PM
>> >> >>> 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!

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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel