linux-kernel - Re: [PATCH v2 2/2] KVM: x86: Add lowest-priority support for vt-d posted-interrupts

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20151223165012.GB19037@potion.brq.redhat.com>
Date:	Wed, 23 Dec 2015 17:50:13 +0100
From:	"rkrcmar@...hat.com" <rkrcmar@...hat.com>
To:	Yang Zhang <yang.zhang.wz@...il.com>
Cc:	"Wu, Feng" <feng.wu@...el.com>,
	"pbonzini@...hat.com" <pbonzini@...hat.com>,
	"kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 2/2] KVM: x86: Add lowest-priority support for vt-d
 posted-interrupts

2015-12-22 14:42+0800, Yang Zhang:
> On 2015/12/22 12:36, Wu, Feng wrote:
>>>From: Yang Zhang [mailto:yang.zhang.wz@...il.com]
>>>On 2015/12/21 9:55, Wu, Feng wrote:
>>>>>From: linux-kernel-owner@...r.kernel.org [mailto:linux-kernel-
>>>>>On 2015/12/16 9:37, Feng Wu wrote:
>>>>>>diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
>>>>>>@@ -10702,8 +10702,16 @@ 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))
>>>>>>-			continue;
>>>>>>+
>>>>>>+		if (!kvm_intr_is_single_vcpu(kvm, &irq, &vcpu)) {
>>>>>>+			if (!kvm_vector_hashing_enabled() ||
>>>>>>+					irq.delivery_mode !=
>>>>>APIC_DM_LOWEST)
>>>>>>+				continue;
>>>>>>+
>>>>>>+			vcpu = kvm_intr_vector_hashing_dest(kvm, &irq);
>>>>>>+			if (!vcpu)
>>>>>>+				continue;
>>>>>>+		}
>>>>>
>>>>>I am a little confused with the 'continue'. If the destination is not
>>>>>single vcpu, shouldn't we rollback to use non-PI mode?
>>>>
>>>>Here is the logic:
>>>>- If it is single destination, we will use PI no matter it is fixed or lowest-priority.
>>>>- If it is not single destination:
>>>>	a) It is fixed, we will use non-PI
>>>>	b) It is lowest-priority and vector-hashing is enabled, we will use PI
>>>>	c) otherwise, use non-PI
>>>
>>>If it is single destination previously, then change to no-single mode.
>>>Can current code cover this case?
>>
>>In my test, before setting irq affinity (change single vcpu to non-single vcpu
>>in this case), the guest will mask the interrupt first, so before getting here, IRTE
>>has been changed back to remapped mode already(when guest masks the MSIx,
>>we will change back to remapped mode), hence nothing needed here.
>>
>>Digging into the linux code (guest) a bit more, I found that if interrupt remapping
>>is not enabled in the guest (IR is not supported for guest anyway), it will always
>>mask the MSI/MSIx before setting the irq affinity. So the code should work
>>well currently.
> 
> We should not rely on guest's behavior. From code level, it need be fixed.
> 
>>However, for robustness, I think explicitly changing IRTE back to remapped
>>mode for the 'continue' case should be a good idea.
> 
> This is what i am looking for.

I agree, that would be a nice addition.

IIRC, the masking is optional -- if the guest can handle interrupts that
are generated while the device is half-configured, it doesn't need to
disable MSIs.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/