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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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.

>>> +        }
>>>          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.

Jan


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