Re: [PATCH v11 1/6] passthrough: don't migrate pirq when it is delivered through VT-d PI

[Chao Gao <chao.gao@intel.com>]

On Fri, Mar 31, 2017 at 04:06:27AM -0600, Jan Beulich wrote:
>>>> On 31.03.17 at 04:42, <chao.gao@intel.com> wrote:
>> On Fri, Mar 31, 2017 at 03:31:47AM -0600, Jan Beulich wrote:
>>>>>> On 29.03.17 at 07:11, <chao.gao@intel.com> wrote:
>>>> @@ -442,17 +397,24 @@ int pt_irq_create_bind(
>>>>          dest_vcpu_id = hvm_girq_dest_2_vcpu_id(d, dest, dest_mode);
>>>>          pirq_dpci->gmsi.dest_vcpu_id = dest_vcpu_id;
>>>>          spin_unlock(&d->event_lock);
>>>> +
>>>> +        pirq_dpci->gmsi.posted = false;
>>>> +        vcpu = (dest_vcpu_id >= 0) ? d->vcpu[dest_vcpu_id] : NULL;
>>>> +        if ( iommu_intpost && (delivery_mode == dest_LowestPrio) )
>>>
>>>Why again would dest_Fixed not allow posted delivery? This needs
>> 
>> No this restriction. For dest_Fixed case, hvm_girq_dest_2_vcpu_id() gives
>> the same output with pi_find_dest_vcpu(). Thus we don't call it again, just
>> use the return value of hvm_girq_dest_2_vcpu_id().
>
>But as pointed out you don't set the new posted field in that case.
>

Indeed.

How about this:
	pirq_dpci->gmsi.posted = false;
        if ( iommu_intpost )
        {
            vcpu = (dest_vcpu_id >= 0) ? d->vcpu[dest_vcpu_id] : NULL;
            if ( delivery_mode == dest_LowestPrio )
                vcpu = vector_hashing_dest(d, dest, dest_mode,
                                           pirq_dpci->gmsi.gvec);
            if ( vcpu )
                pirq_dpci->gmsi.posted = true;
        }

>>>recording in a comment, if there really is such a restriction. Or did
>>>you really mean to place ...
>>>
>>>> +        {
>>>> +            vcpu = vector_hashing_dest(d, dest, dest_mode,
>>>> +                                       pirq_dpci->gmsi.gvec);
>>>> +            if ( vcpu )
>>>> +                pirq_dpci->gmsi.posted = true;
>>>
>>>... this ...
>>>
>>>> +        }
>>>
>>>... after this brace (which then wouldn't be needed anymore)? If
>>>so, is there any point calling vector_hashing_dest() when vcpu is
>>>already non-NULL prior to the if()?
>>>
>>>This then also raises the question whether the call to
>>>hvm_girq_dest_2_vcpu_id() is actually legitimate for lowest
>>>priority delivery mode.
>> 
>> For lowest priority delivery mode, if VT-d PI is enabled, the result (the
>> destination vcpu) is overrided by vector_hashing_dest() to keep the 
>> existing behavior. I think the only point we should keep in mind is
>> for cases other than lowest priority delivery mode, pi_find_dest_vcpu()
>> and hvm_girq_dest_2_vcpu_id() give the same destination vcpu.
>
>Well, the override is done for the iommu_intpost case. The remark
>on hvm_girq_dest_2_vcpu_id(), however, was made in general.

Ok. You meant the method using in hvm_girq_dest_2_vcpu_id() may not match the method
used by real hardware. I will check it.

Thanks
Chao

>
>Jan
>

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