[PATCH 0/2] KVM: Only use posted interrupts for Fixes/LowPrio MSIs

[PATCH 0/2] KVM: Only use posted interrupts for Fixes/LowPrio MSIs

[Alexander Graf]

The MSI-X descriptor has a "delivery mode" field which can be set to
various different targets, such as "Fixed" (default), SMI, NMI or INIT.

Usually when we pass devices into guests, we only ever see this MSI-X
descriptor configured as Fixed, so nobody realized that the other modes
were broken when using posted interrupts.

With posted interrupts, we end up configuring these special modes just
the same as a Fixed interrupt. That means instead of generating an SMI,
we inject a normal GSI into the guest.

Of course, that if completely broken. These two patches attempt to fix
the situation for x86 systems. If anyone has a great idea how to generalize
the filtering though, I'm all ears.


Alex

Alexander Graf (2):
  KVM: VMX: Disable posted interrupts for odd IRQs
  KVM: SVM: Disable posted interrupts for odd IRQs

 arch/x86/kvm/svm.c     | 16 ++++++++++++++++
 arch/x86/kvm/vmx/vmx.c | 22 ++++++++++++++++++++++
 2 files changed, 38 insertions(+)

-- 
2.17.1




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879

[PATCH 1/2] KVM: VMX: Disable posted interrupts for odd IRQs

[Alexander Graf]

We can easily route hardware interrupts directly into VM context when
they target the "Fixed" or "LowPriority" delivery modes.

However, on modes such as "SMI" or "Init", we need to go via KVM code
to actually put the vCPU into a different mode of operation, so we can
not post the interrupt

Add code in the VMX PI logic to explicitly refuse to establish posted
mappings for advanced IRQ deliver modes.

This fixes a bug I have with code which configures real hardware to
inject virtual SMIs into my guest.

Signed-off-by: Alexander Graf <graf@amazon.com>
---
 arch/x86/kvm/vmx/vmx.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 570a233e272b..d16c4ae8f685 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7401,6 +7401,28 @@ static int vmx_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
 			continue;
 		}
 
+		switch (irq.delivery_mode) {
+		case dest_Fixed:
+		case dest_LowestPrio:
+			break;
+		default:
+			/*
+			 * For non-trivial interrupt events, we need to go
+			 * through the full KVM IRQ code, so refuse to take
+			 * any direct PI assignments here.
+			 */
+
+			ret = irq_set_vcpu_affinity(host_irq, NULL);
+			if (ret < 0) {
+				printk(KERN_INFO
+				   "failed to back to remapped mode, irq: %u\n",
+				   host_irq);
+				goto out;
+			}
+
+			continue;
+		}
+
 		vcpu_info.pi_desc_addr = __pa(vcpu_to_pi_desc(vcpu));
 		vcpu_info.vector = irq.vector;
 
-- 
2.17.1




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879

[PATCH 2/2] KVM: SVM: Disable posted interrupts for odd IRQs

[Alexander Graf]

We can easily route hardware interrupts directly into VM context when
they target the "Fixed" or "LowPriority" delivery modes.

However, on modes such as "SMI" or "Init", we need to go via KVM code
to actually put the vCPU into a different mode of operation, so we can
not post the interrupt

Add code in the SVM PI logic to explicitly refuse to establish posted
mappings for advanced IRQ deliver modes.

This fixes a bug I have with code which configures real hardware to
inject virtual SMIs into my guest.

Signed-off-by: Alexander Graf <graf@amazon.com>
---
 arch/x86/kvm/svm.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 1f220a85514f..9a6ea78c3239 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -5266,6 +5266,21 @@ get_pi_vcpu_info(struct kvm *kvm, struct kvm_kernel_irq_routing_entry *e,
 		return -1;
 	}
 
+	switch (irq.delivery_mode) {
+	case dest_Fixed:
+	case dest_LowestPrio:
+		break;
+	default:
+		/*
+		 * For non-trivial interrupt events, we need to go
+		 * through the full KVM IRQ code, so refuse to take
+		 * any direct PI assignments here.
+		 */
+		pr_debug("SVM: %s: use legacy intr remap mode for irq %u\n",
+			 __func__, irq.vector);
+		return -1;
+	}
+
 	pr_debug("SVM: %s: use GA mode for irq %u\n", __func__,
 		 irq.vector);
 	*svm = to_svm(vcpu);
@@ -5314,6 +5329,7 @@ static int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
 		 * 1. When cannot target interrupt to a specific vcpu.
 		 * 2. Unsetting posted interrupt.
 		 * 3. APIC virtialization is disabled for the vcpu.
+		 * 4. IRQ has extended delivery mode (SMI, INIT, etc)
 		 */
 		if (!get_pi_vcpu_info(kvm, e, &vcpu_info, &svm) && set &&
 		    kvm_vcpu_apicv_active(&svm->vcpu)) {
-- 
2.17.1




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879

Re: [PATCH 1/2] KVM: VMX: Disable posted interrupts for odd IRQs

[Liran Alon]

> On 3 Sep 2019, at 17:29, Alexander Graf <graf@amazon.com> wrote:
> 
> We can easily route hardware interrupts directly into VM context when
> they target the "Fixed" or "LowPriority" delivery modes.
> 
> However, on modes such as "SMI" or "Init", we need to go via KVM code
> to actually put the vCPU into a different mode of operation, so we can
> not post the interrupt

I would also mention in commit message that one can see this is also
true in KVM’s vLAPIC code. i.e. __apic_accept_irq() can call
kvm_x86_ops->deliver_posted_interrupt() only in case deliver-mode is
either “Fixed” or “LowPriority”. 

> 
> Add code in the VMX PI logic to explicitly refuse to establish posted
> mappings for advanced IRQ deliver modes.
> 
> This fixes a bug I have with code which configures real hardware to
> inject virtual SMIs into my guest.
> 
> Signed-off-by: Alexander Graf <graf@amazon.com>

With some small improvements I written inline below:
Reviewed-by: Liran Alon <liran.alon@oracle.com>

> ---
> arch/x86/kvm/vmx/vmx.c | 22 ++++++++++++++++++++++
> 1 file changed, 22 insertions(+)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 570a233e272b..d16c4ae8f685 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7401,6 +7401,28 @@ static int vmx_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
> 			continue;
> 		}
> 
> +		switch (irq.delivery_mode) {
> +		case dest_Fixed:
> +		case dest_LowestPrio:
> +			break;
> +		default:
> +			/*
> +			 * For non-trivial interrupt events, we need to go
> +			 * through the full KVM IRQ code, so refuse to take
> +			 * any direct PI assignments here.
> +			 */
> +
> +			ret = irq_set_vcpu_affinity(host_irq, NULL);
> +			if (ret < 0) {
> +				printk(KERN_INFO
> +				   "failed to back to remapped mode, irq: %u\n",
> +				   host_irq);
> +				goto out;

I recommend we will chose to print here a string that is different than the !kvm_intr_is_single_vcpu()
case to make it easier to diagnose which case exactly failed.

-Liran

> +			}
> +
> +			continue;
> +		}
> +
> 		vcpu_info.pi_desc_addr = __pa(vcpu_to_pi_desc(vcpu));
> 		vcpu_info.vector = irq.vector;
> 
> -- 
> 2.17.1
> 
> 
> 
> 
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
> Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> Sitz: Berlin
> Ust-ID: DE 289 237 879
> 
> 
> 

Re: [PATCH 2/2] KVM: SVM: Disable posted interrupts for odd IRQs

[Liran Alon]

> On 3 Sep 2019, at 17:29, Alexander Graf <graf@amazon.com> wrote:
> 
> We can easily route hardware interrupts directly into VM context when
> they target the "Fixed" or "LowPriority" delivery modes.
> 
> However, on modes such as "SMI" or "Init", we need to go via KVM code
> to actually put the vCPU into a different mode of operation, so we can
> not post the interrupt
> 
> Add code in the SVM PI logic to explicitly refuse to establish posted
> mappings for advanced IRQ deliver modes.
> 
> This fixes a bug I have with code which configures real hardware to
> inject virtual SMIs into my guest.
> 
> Signed-off-by: Alexander Graf <graf@amazon.com>

Nit: I prefer to squash both commits into one that change both VMX & SVM.
As it’s exactly the same change.

> ---
> arch/x86/kvm/svm.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
> 
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 1f220a85514f..9a6ea78c3239 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -5266,6 +5266,21 @@ get_pi_vcpu_info(struct kvm *kvm, struct kvm_kernel_irq_routing_entry *e,
> 		return -1;
> 	}
> 
> +	switch (irq.delivery_mode) {
> +	case dest_Fixed:
> +	case dest_LowestPrio:
> +		break;
> +	default:
> +		/*
> +		 * For non-trivial interrupt events, we need to go
> +		 * through the full KVM IRQ code, so refuse to take
> +		 * any direct PI assignments here.
> +		 */
> +		pr_debug("SVM: %s: use legacy intr remap mode for irq %u\n",
> +			 __func__, irq.vector);
> +		return -1;
> +	}
> +

Prefer changing printed string to something different than the !kvm_intr_is_single_vcpu() case.
To assist debugging.

Having said that,
Reviewed-by: Liran Alon <liran.alon@oracle.com>

-Liran

> 	pr_debug("SVM: %s: use GA mode for irq %u\n", __func__,
> 		 irq.vector);
> 	*svm = to_svm(vcpu);
> @@ -5314,6 +5329,7 @@ static int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
> 		 * 1. When cannot target interrupt to a specific vcpu.
> 		 * 2. Unsetting posted interrupt.
> 		 * 3. APIC virtialization is disabled for the vcpu.
> +		 * 4. IRQ has extended delivery mode (SMI, INIT, etc)
> 		 */
> 		if (!get_pi_vcpu_info(kvm, e, &vcpu_info, &svm) && set &&
> 		    kvm_vcpu_apicv_active(&svm->vcpu)) {
> -- 
> 2.17.1
> 
> 
> 
> 
> Amazon Development Center Germany GmbH
> Krausenstr. 38
> 10117 Berlin
> Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
> Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
> Sitz: Berlin
> Ust-ID: DE 289 237 879
> 
> 
> 

Re: [PATCH 2/2] KVM: SVM: Disable posted interrupts for odd IRQs

[Alexander Graf]

On 04.09.19 01:20, Liran Alon wrote:
> 
> 
>> On 3 Sep 2019, at 17:29, Alexander Graf <graf@amazon.com> wrote:
>>
>> We can easily route hardware interrupts directly into VM context when
>> they target the "Fixed" or "LowPriority" delivery modes.
>>
>> However, on modes such as "SMI" or "Init", we need to go via KVM code
>> to actually put the vCPU into a different mode of operation, so we can
>> not post the interrupt
>>
>> Add code in the SVM PI logic to explicitly refuse to establish posted
>> mappings for advanced IRQ deliver modes.
>>
>> This fixes a bug I have with code which configures real hardware to
>> inject virtual SMIs into my guest.
>>
>> Signed-off-by: Alexander Graf <graf@amazon.com>
> 
> Nit: I prefer to squash both commits into one that change both VMX & SVM.
> As it’s exactly the same change.

It's the same change (hence the same patch set), but they touch 
different files and so for bisectability it's still convenient to have 
them as different commits. I'd really prefer to have them stay separate.

Thanks a lot for the review! :)


Alex



Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Ralf Herbrich
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879