diff mbox

[v2] intel_iommu: add "eim" property

Message ID 1470994887-21409-1-git-send-email-peterx@redhat.com
State New
Headers show

Commit Message

Peter Xu Aug. 12, 2016, 9:41 a.m. UTC 
Adding one extra property for intel-iommu device to decide whether we
should support EIM bit for IR.

Now we are throwing high 24 bits of dest_id away directly. This will
cause interrupt issues with guests that:

- enabled x2apic with cluster mode
- have more than 8 vcpus (so dest_id[31:8] might be nonzero)

Let's make xapic the default one, and for the brave people who would
like to try EIM and know the side effects, we can do it by explicitly
enabling EIM using:

  -device intel-iommu,intremap=on,eim=on

Even after we have x2apic support, it'll still be good if we can provide
a way to switch xapic/x2apic from QEMU side for e.g. debugging purpose,
which is an alternative for tuning guest kernel boot parameters.

We can switch the default to "on" after x2apic fully supported.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 hw/i386/intel_iommu.c         | 16 +++++++++++++++-
 include/hw/i386/intel_iommu.h |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

Comments

Peter Xu Sept. 1, 2016, 1:31 a.m. UTC | #1 
On Fri, Aug 12, 2016 at 05:41:27PM +0800, Peter Xu wrote:
> Adding one extra property for intel-iommu device to decide whether we
> should support EIM bit for IR.
> 
> Now we are throwing high 24 bits of dest_id away directly. This will
> cause interrupt issues with guests that:
> 
> - enabled x2apic with cluster mode
> - have more than 8 vcpus (so dest_id[31:8] might be nonzero)
> 
> Let's make xapic the default one, and for the brave people who would
> like to try EIM and know the side effects, we can do it by explicitly
> enabling EIM using:
> 
>   -device intel-iommu,intremap=on,eim=on
> 
> Even after we have x2apic support, it'll still be good if we can provide
> a way to switch xapic/x2apic from QEMU side for e.g. debugging purpose,
> which is an alternative for tuning guest kernel boot parameters.
> 
> We can switch the default to "on" after x2apic fully supported.
> 
> Signed-off-by: Peter Xu <peterx@redhat.com>

Ping.

I'd really appreciate if someone can help have a look on this patch,
and to merge it if possible. Since current x2apic is broken. We should
not allow broken system boot, at least not by default.

Thanks,

-- peterx
diff mbox

Patch

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 28c31a2..a696f71 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -2005,6 +2005,11 @@  static const MemoryRegionOps vtd_mem_ops = {
 
 static Property vtd_properties[] = {
     DEFINE_PROP_UINT32("version", IntelIOMMUState, version, 0),
+    /*
+     * TODO: currently EIM is disabled by default. We can enable this
+     * after fully support x2apic.
+     */
+    DEFINE_PROP_BOOL("eim", IntelIOMMUState, eim_supported, false),
     DEFINE_PROP_END_OF_LIST(),
 };
 
@@ -2364,7 +2369,10 @@  static void vtd_init(IntelIOMMUState *s)
     s->ecap = VTD_ECAP_QI | VTD_ECAP_IRO;
 
     if (x86_iommu->intr_supported) {
-        s->ecap |= VTD_ECAP_IR | VTD_ECAP_EIM | VTD_ECAP_MHMV;
+        s->ecap |= VTD_ECAP_IR | VTD_ECAP_MHMV;
+        if (s->eim_supported) {
+            s->ecap |= VTD_ECAP_EIM;
+        }
     }
 
     vtd_reset_context_cache(s);
@@ -2468,6 +2476,12 @@  static void vtd_realize(DeviceState *dev, Error **errp)
     /* Pseudo address space under root PCI bus. */
     pcms->ioapic_as = vtd_host_dma_iommu(bus, s, Q35_PSEUDO_DEVFN_IOAPIC);
 
+    /* EIM bit requires IR */
+    if (s->eim_supported && !x86_iommu->intr_supported) {
+        error_report("EIM (Extended Interrupt Mode) bit requires intremap=on");
+        exit(1);
+    }
+
     /* Currently Intel IOMMU IR only support "kernel-irqchip={off|split}" */
     if (x86_iommu->intr_supported && kvm_irqchip_in_kernel() &&
         !kvm_irqchip_is_split()) {
diff --git a/include/hw/i386/intel_iommu.h b/include/hw/i386/intel_iommu.h
index a42dbd7..b1bc768 100644
--- a/include/hw/i386/intel_iommu.h
+++ b/include/hw/i386/intel_iommu.h
@@ -289,6 +289,7 @@  struct IntelIOMMUState {
     dma_addr_t intr_root;           /* Interrupt remapping table pointer */
     uint32_t intr_size;             /* Number of IR table entries */
     bool intr_eime;                 /* Extended interrupt mode enabled */
+    bool eim_supported;             /* Whether to allow EIM bit */
 };
 
 /* Find the VTD Address space associated with the given bus pointer,