Re: [PATCH v3 1/4] KVM: Recover IRTE to remapped mode if the interrupt is not single-destination

[Yang Zhang <yang.zhang.wz@gmail.com>]

On 2016/1/21 13:41, Wu, Feng wrote:
>
>
>> -----Original Message-----
>> From: Yang Zhang [mailto:yang.zhang.wz@gmail.com]
>> Sent: Thursday, January 21, 2016 1:36 PM
>> To: Wu, Feng <feng.wu@intel.com>; pbonzini@redhat.com;
>> rkrcmar@redhat.com
>> Cc: linux-kernel@vger.kernel.org; kvm@vger.kernel.org
>> Subject: Re: [PATCH v3 1/4] KVM: Recover IRTE to remapped mode if the
>> interrupt is not single-destination
>>
>> On 2016/1/21 13:07, Wu, Feng wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Yang Zhang [mailto:yang.zhang.wz@gmail.com]
>>>> Sent: Thursday, January 21, 2016 1:00 PM
>>>> To: Wu, Feng <feng.wu@intel.com>; pbonzini@redhat.com;
>>>> rkrcmar@redhat.com
>>>> Cc: linux-kernel@vger.kernel.org; kvm@vger.kernel.org
>>>> Subject: Re: [PATCH v3 1/4] KVM: Recover IRTE to remapped mode if the
>>>> interrupt is not single-destination
>>>>
>>>> On 2016/1/21 12:42, Wu, Feng wrote:
>>>>>
>>>>>
>>>>>> -----Original Message-----
>>>>>> From: kvm-owner@vger.kernel.org [mailto:kvm-owner@vger.kernel.org]
>>>> On
>>>>>> Behalf Of Yang Zhang
>>>>>> Sent: Thursday, January 21, 2016 11:35 AM
>>>>>> To: Wu, Feng <feng.wu@intel.com>; pbonzini@redhat.com;
>>>>>> rkrcmar@redhat.com
>>>>>> Cc: linux-kernel@vger.kernel.org; kvm@vger.kernel.org
>>>>>> Subject: Re: [PATCH v3 1/4] KVM: Recover IRTE to remapped mode if
>> the
>>>>>> interrupt is not single-destination
>>>>>>
>>>>>> On 2016/1/21 11:14, Wu, Feng wrote:
>>>>>>>
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Yang Zhang [mailto:yang.zhang.wz@gmail.com]
>>>>>>>> Sent: Thursday, January 21, 2016 11:06 AM
>>>>>>>> To: Wu, Feng <feng.wu@intel.com>; pbonzini@redhat.com;
>>>>>>>> rkrcmar@redhat.com
>>>>>>>> Cc: linux-kernel@vger.kernel.org; kvm@vger.kernel.org
>>>>>>>> Subject: Re: [PATCH v3 1/4] KVM: Recover IRTE to remapped mode if
>>>> the
>>>>>>>> interrupt is not single-destination
>>>>>>>>
>>>>>>>> On 2016/1/20 9:42, Feng Wu wrote:
>>>>>>>>> When the interrupt is not single destination any more, we need
>>>>>>>>> to change back IRTE to remapped mode explicitly.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Feng Wu <feng.wu@intel.com>
>>>>>>>>> ---
>>>>>>>>>       arch/x86/kvm/vmx.c | 11 ++++++++++-
>>>>>>>>>       1 file changed, 10 insertions(+), 1 deletion(-)
>>>>>>>>>
>>>>>>>>> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>>>>>>>> index e2951b6..13d14d4 100644
>>>>>>>>> --- a/arch/x86/kvm/vmx.c
>>>>>>>>> +++ b/arch/x86/kvm/vmx.c
>>>>>>>>> @@ -10764,8 +10764,17 @@ static int vmx_update_pi_irte(struct
>> kvm
>>>>>>>> *kvm, unsigned int host_irq,
>>>>>>>>>       		 */
>>>>>>>>>
>>>>>>>>>       		kvm_set_msi_irq(e, &irq);
>>>>>>>>> -		if (!kvm_intr_is_single_vcpu(kvm, &irq, &vcpu))
>>>>>>>>> +		if (!kvm_intr_is_single_vcpu(kvm, &irq, &vcpu)) {
>>>>>>>>> +			/*
>>>>>>>>> +			 * Make sure the IRTE is in remapped mode if
>>>>>>>>> +			 * we don't handle it in posted mode.
>>>>>>>>> +			 */
>>>>>>>>> +			pi_set_sn(vcpu_to_pi_desc(vcpu));
>>>>>>>>> +			ret = irq_set_vcpu_affinity(host_irq, NULL);
>>>>>>>>> +			pi_clear_sn(vcpu_to_pi_desc(vcpu));
>>>>>>>>> +
>>>>>>>>>       			continue;
>>>>>>>>> +		}
>>>>>>>>>
>>>>>>>>>       		vcpu_info.pi_desc_addr =
>>>> __pa(vcpu_to_pi_desc(vcpu));
>>>>>>>>>       		vcpu_info.vector = irq.vector;
>>>>>>>>>
>>>>>>>>
>>>>>>>> I am still feel weird with this change: according the semantic of VT-d
>>>>>>>> posted interrupt, the interrupt will injected to guest through posted
>>>>>>>> notification and /proc/interrupts shows the same meaning. But now,
>>>>>>>> without being aware of user, the interrupt changes to legacy way and
>> it
>>>>>>>> appears on different entry on /proc/interrupts. It looks weird.
>>>>>>>
>>>>>>> I don't think it has problem here, IMO, this is exactly how it works.
>>>>>>> There should be different entry for the interrupts in VT-d PI mode
>>>>>>> and leagcy mode.
>>>>>>
>>>>>> I am not saying any problem here. Just feel weird. From a normal user's
>>>>>> point, he has turned on the VT-d pi and according the semantic of VT-d
>>>>>> pi, he should not observe the interrupt through legacy mode, but now
>> he
>>>>>> do see it. Maybe print out a message here will be helpful, like what you
>>>>>> did for disabled lapic found during irq injection.
>>>>>
>>>>> Even VT-d PI is on, not all interrupts can be handled by it, the reason the
>>>>
>>>> No, we can handle it but we don't do it due to the complexity.For
>>>> example, we can use wake up vector to delivery the interrupt which still
>>>> is in PI mode but doesn't require any mode change.
>>>
>>> I mean, multi-cast and broadcast interrupts cannot be handled in PI mode.
>>
>> We may have different understanding on PI mode. My understanding is if
>> we set the IRTE to PI format, than the subsequent interrupt will be
>> handled in PI mode. multi-cast and broadcast interrupts cannot be
>> injected to guest directly but it doesn't mean cannot be handled in PI
>> mode. As i said, we can handle it in wake up vector or via other
>> approach.But it is much complexity.
>
> For the multicast/broastcast, we cannot set the related IRTE in PI
> mode, since we cannot set only one destination in IRTE. If an interrupt
> is for multiple destination, how can you use VT-d PI to injection it
> to all the destinations?

You may still not get my point. Anyway, it doesn't matter. Rollback to 
legacy mode still is the best choice so far.

-- 
best regards
yang