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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 31.03.17 at 05:27, <chao.gao@xxxxxxxxx> wrote:
> On Fri, Mar 31, 2017 at 04:06:27AM -0600, Jan Beulich wrote:
>>>>> On 31.03.17 at 04:42, <chao.gao@xxxxxxxxx> wrote:
>>> On Fri, Mar 31, 2017 at 03:31:47AM -0600, Jan Beulich wrote:
>>>>>>> On 29.03.17 at 07:11, <chao.gao@xxxxxxxxx> 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;
>         }

I think the setting of vcpu needs to stay outside the if(), but other
than that the above looks reasonable at a first glance.

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

I mean that the method used there is pretty clearly "fixed" mode
handling. Thanks for checking.

Jan


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