Re: [Xen-devel] [PATCH v10 5/6] passthrough/io: don't migrate pirq when it is delivered through VT-d PI

[Chao Gao <chao.gao@xxxxxxxxx>]

On Mon, Mar 20, 2017 at 03:18:18AM -0600, Jan Beulich wrote:
>>>> On 20.03.17 at 02:59, <chao.gao@xxxxxxxxx> wrote:
>> On Fri, Mar 17, 2017 at 04:43:08AM -0600, Jan Beulich wrote:
>>>>>> On 15.03.17 at 06:11, <chao.gao@xxxxxxxxx> wrote:
>>>> +        if ( iommu_intpost )
>>>> +        {
>>>> +            vcpu = pi_find_dest_vcpu(d, dest, dest_mode, delivery_mode,
>>>> +                                     pirq_dpci->gmsi.gvec);
>>>
>>>This is now outside of the event_lock-ed region - is this safe?
>> 
>> do you mean it is __inside__ the event_lock-ed region?
>
>Oops, indeed.
>
>> I think it is safe
>> for the functions called by pi_find_dest_vcpu() are almost same with
>> hvm_girq_dest_2_vcpu_id.
>
>The question then needs to be put differently: Is this needed?
>You shouldn't move into a locked region what doesn't need to
>be there.

Yes. For we rely on the result to set @via_pi which needs to be 
protected by the lock.

>
>>>> +        }
>>>>          spin_unlock(&d->event_lock);
>>>>          if ( dest_vcpu_id >= 0 )
>>>>              hvm_migrate_pirqs(d->vcpu[dest_vcpu_id]);
>>>
>>>(continuing from above) This could then use vcpu too.
>> 
>> I don't understand. In this patch, vcpu is always null when VT-d PI is not
>> enabled. Do you mean something like below: 
>> 
>> if ( dest_vcpu_id >= 0 )
>>     vcpu = d->vcpu[dest_vcpu_id];
>> if ( iommu_intpost && (!vcpu) && (delivery_mode == dest_LowestPrio) )
>> {
>>     vcpu = vector_hashing_dest(d, dest, dest_mode,pirq_dpci->gmsi.gvec);
>> ...
>> }
>> spin_unlock(&d->event_lock);
>> if ( vcpu )
>>     hvm_migrate_pirqs(vcpu);
>
>Yes, along these lines, albeit I think the first if() is more complicated
>than it needs to be.

We can make it simple like this:

const struct *vcpu vcpu;
...

vcpu = (dest_vcpu_id >= 0) ? d->vcpu[dest_vcpu_id] : NULL;

Thanks
Chao

>
>Jan
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel