diff mbox

[v3,6/8] intel_iommu: reject broken EIM

Message ID 20160930161013.9832-7-rkrcmar@redhat.com
State New
Headers show

Commit Message

Radim Krčmář Sept. 30, 2016, 4:10 p.m. UTC
Cluster x2APIC cannot work without KVM's x2apic API when the maximal
APIC ID is greater than 8 and only KVM's LAPIC can support x2APIC, so we
forbid other APICs and also the old KVM case with less than 9, to
simplify the code.

There is no point in enabling EIM in forbidden APICs, so we keep it
enabled only for the KVM APIC;  unconditionally, because making the
option depend on KVM version would be a maintanance burden.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
---
v3:
 * use error_setg [Paolo]
 * shorten the code [Peter]
v2:
 * adapt to new intr_eim parameter
 * provide first linux version that has x2apic api
 * disable QEMU's LAPIC
---
 hw/i386/intel_iommu.c  | 15 ++++++++++++++-
 target-i386/kvm-stub.c |  5 +++++
 target-i386/kvm.c      | 13 +++++++++++++
 target-i386/kvm_i386.h |  1 +
 4 files changed, 33 insertions(+), 1 deletion(-)

Comments

Igor Mammedov Oct. 4, 2016, 1:43 p.m. UTC | #1
On Fri, 30 Sep 2016 18:10:11 +0200
Radim Krčmář <rkrcmar@redhat.com> wrote:

> Cluster x2APIC cannot work without KVM's x2apic API when the maximal
> APIC ID is greater than 8 and only KVM's LAPIC can support x2APIC, so we
> forbid other APICs and also the old KVM case with less than 9, to
> simplify the code.
> 
> There is no point in enabling EIM in forbidden APICs, so we keep it
> enabled only for the KVM APIC;  unconditionally, because making the
> option depend on KVM version would be a maintanance burden.
Probably it would be good to mention here that x2APIC API is enabled
if KVM's apic is used and kernel supports x2APIC API + DISABLE_BROADCAST_QUIRK,
and doing that enabling fixes error:

   KVM: injection failed, MSI lost (Operation not permitted)

when guest is started with: kernel-irqchip=split + intremap=on,[eim=on]

> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
> v3:
>  * use error_setg [Paolo]
>  * shorten the code [Peter]
> v2:
>  * adapt to new intr_eim parameter
>  * provide first linux version that has x2apic api
>  * disable QEMU's LAPIC
> ---
>  hw/i386/intel_iommu.c  | 15 ++++++++++++++-
>  target-i386/kvm-stub.c |  5 +++++
>  target-i386/kvm.c      | 13 +++++++++++++
>  target-i386/kvm_i386.h |  1 +
>  4 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 17892b8c336b..efb018b85544 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -33,6 +33,7 @@
>  #include "hw/pci-host/q35.h"
>  #include "sysemu/kvm.h"
>  #include "hw/i386/apic_internal.h"
> +#include "kvm_i386.h"
>  
>  /*#define DEBUG_INTEL_IOMMU*/
>  #ifdef DEBUG_INTEL_IOMMU
> @@ -2472,10 +2473,22 @@ static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
>      }
>  
>      if (s->intr_eim == ON_OFF_AUTO_AUTO) {
> -        s->intr_eim = x86_iommu->intr_supported ?
> +        s->intr_eim = x86_iommu->intr_supported && kvm_irqchip_in_kernel() ?
>                                                ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
>      }
>  
> +    if (s->intr_eim == ON_OFF_AUTO_ON) {
> +        if (kvm_irqchip_in_kernel() && !kvm_enable_x2apic()) {
> +            error_setg(errp, "eim=on requires support on the KVM side"
> +                             "(X2APIC_API, first shipped in v4.7)");
> +            return false;
> +        }
> +        if (!kvm_irqchip_in_kernel()) {
> +            error_setg(errp, "eim=on requires accel=kvm,kernel-irqchip=split");
> +            return false;
> +        }
> +    }
> +
>      return true;
>  }
>  
> diff --git a/target-i386/kvm-stub.c b/target-i386/kvm-stub.c
> index cdf15061091d..bda4dc2f0c57 100644
> --- a/target-i386/kvm-stub.c
> +++ b/target-i386/kvm-stub.c
> @@ -25,6 +25,11 @@ bool kvm_has_smm(void)
>      return 1;
>  }
>  
> +bool kvm_enable_x2apic(void)
> +{
> +    return false;
> +}
> +
>  /* This function is only called inside conditionals which we
>   * rely on the compiler to optimize out when CONFIG_KVM is not
>   * defined.
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index ee1f53e56953..0fd664648665 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -122,6 +122,19 @@ bool kvm_allows_irq0_override(void)
>      return !kvm_irqchip_in_kernel() || kvm_has_gsi_routing();
>  }
>  
> +static bool kvm_x2apic_api_set_flags(uint64_t flags)
> +{
> +    KVMState *s = KVM_STATE(current_machine->accelerator);
> +
> +    return !kvm_vm_enable_cap(s, KVM_CAP_X2APIC_API, 0, flags);
> +}
> +
> +bool kvm_enable_x2apic(void)
> +{
> +    return kvm_x2apic_api_set_flags(KVM_X2APIC_API_USE_32BIT_IDS |
> +                                    KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK);
> +}
> +
>  static int kvm_get_tsc(CPUState *cs)
>  {
>      X86CPU *cpu = X86_CPU(cs);
> diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
> index 36407e0a5dc7..5c369b1c5b40 100644
> --- a/target-i386/kvm_i386.h
> +++ b/target-i386/kvm_i386.h
> @@ -43,4 +43,5 @@ int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
>  
>  void kvm_put_apicbase(X86CPU *cpu, uint64_t value);
>  
> +bool kvm_enable_x2apic(void);
>  #endif
diff mbox

Patch

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 17892b8c336b..efb018b85544 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -33,6 +33,7 @@ 
 #include "hw/pci-host/q35.h"
 #include "sysemu/kvm.h"
 #include "hw/i386/apic_internal.h"
+#include "kvm_i386.h"
 
 /*#define DEBUG_INTEL_IOMMU*/
 #ifdef DEBUG_INTEL_IOMMU
@@ -2472,10 +2473,22 @@  static bool vtd_decide_config(IntelIOMMUState *s, Error **errp)
     }
 
     if (s->intr_eim == ON_OFF_AUTO_AUTO) {
-        s->intr_eim = x86_iommu->intr_supported ?
+        s->intr_eim = x86_iommu->intr_supported && kvm_irqchip_in_kernel() ?
                                               ON_OFF_AUTO_ON : ON_OFF_AUTO_OFF;
     }
 
+    if (s->intr_eim == ON_OFF_AUTO_ON) {
+        if (kvm_irqchip_in_kernel() && !kvm_enable_x2apic()) {
+            error_setg(errp, "eim=on requires support on the KVM side"
+                             "(X2APIC_API, first shipped in v4.7)");
+            return false;
+        }
+        if (!kvm_irqchip_in_kernel()) {
+            error_setg(errp, "eim=on requires accel=kvm,kernel-irqchip=split");
+            return false;
+        }
+    }
+
     return true;
 }
 
diff --git a/target-i386/kvm-stub.c b/target-i386/kvm-stub.c
index cdf15061091d..bda4dc2f0c57 100644
--- a/target-i386/kvm-stub.c
+++ b/target-i386/kvm-stub.c
@@ -25,6 +25,11 @@  bool kvm_has_smm(void)
     return 1;
 }
 
+bool kvm_enable_x2apic(void)
+{
+    return false;
+}
+
 /* This function is only called inside conditionals which we
  * rely on the compiler to optimize out when CONFIG_KVM is not
  * defined.
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index ee1f53e56953..0fd664648665 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -122,6 +122,19 @@  bool kvm_allows_irq0_override(void)
     return !kvm_irqchip_in_kernel() || kvm_has_gsi_routing();
 }
 
+static bool kvm_x2apic_api_set_flags(uint64_t flags)
+{
+    KVMState *s = KVM_STATE(current_machine->accelerator);
+
+    return !kvm_vm_enable_cap(s, KVM_CAP_X2APIC_API, 0, flags);
+}
+
+bool kvm_enable_x2apic(void)
+{
+    return kvm_x2apic_api_set_flags(KVM_X2APIC_API_USE_32BIT_IDS |
+                                    KVM_X2APIC_API_DISABLE_BROADCAST_QUIRK);
+}
+
 static int kvm_get_tsc(CPUState *cs)
 {
     X86CPU *cpu = X86_CPU(cs);
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index 36407e0a5dc7..5c369b1c5b40 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -43,4 +43,5 @@  int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
 
 void kvm_put_apicbase(X86CPU *cpu, uint64_t value);
 
+bool kvm_enable_x2apic(void);
 #endif