]> xenbits.xensource.com Git - people/dariof/xen.git/commitdiff
AMD/IOMMU: without XT, x2APIC needs to be forced into physical mode
authorJan Beulich <jbeulich@suse.com>
Tue, 10 Mar 2020 14:25:58 +0000 (15:25 +0100)
committerJan Beulich <jbeulich@suse.com>
Tue, 10 Mar 2020 14:25:58 +0000 (15:25 +0100)
The wider cluster mode APIC IDs aren't generally representable. Convert
the iommu_intremap variable into a tristate, allowing the AMD IOMMU
driver to signal this special restriction to the apic_x2apic_probe().
(Note: assignments to the variable get adjusted, while existing
consumers - all assuming a boolean property - are left alone.)

While we are not aware of any hardware/firmware with this as a
restriction, it is a situation which could be created on fully x2apic-
capable systems via firmware settings.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Kevin Tian <kevin.tian@intel.com>
xen/arch/x86/genapic/x2apic.c
xen/drivers/passthrough/amd/iommu_init.c
xen/drivers/passthrough/amd/pci_amd_iommu.c
xen/drivers/passthrough/iommu.c
xen/drivers/passthrough/vtd/iommu.c
xen/include/xen/iommu.h

index f9b5e4976161e4059e3ebe9f2934402541ad8040..077a576a7f0e200cc5ffce80cc220e0de1a06981 100644 (file)
@@ -236,12 +236,21 @@ const struct genapic *__init apic_x2apic_probe(void)
         x2apic_phys = !iommu_intremap ||
                       (acpi_gbl_FADT.flags & ACPI_FADT_APIC_PHYSICAL);
     }
-    else if ( !x2apic_phys && !iommu_intremap )
-    {
-        printk("WARNING: x2APIC cluster mode is not supported without interrupt remapping\n"
-               "x2APIC: forcing phys mode\n");
-        x2apic_phys = true;
-    }
+    else if ( !x2apic_phys )
+        switch ( iommu_intremap )
+        {
+        case iommu_intremap_off:
+        case iommu_intremap_restricted:
+            printk("WARNING: x2APIC cluster mode is not supported %s interrupt remapping -"
+                   " forcing phys mode\n",
+                   iommu_intremap == iommu_intremap_off ? "without"
+                                                        : "with restricted");
+            x2apic_phys = true;
+            break;
+
+        case iommu_intremap_full:
+            break;
+        }
 
     if ( x2apic_phys )
         return &apic_x2apic_phys;
index 147ce8ca87fb0da2369976c7cc7f9c7d153f6ba4..034f3b9c2cf5c3f3540f890e87973f21ec3f816b 100644 (file)
@@ -1139,7 +1139,7 @@ static void __init amd_iommu_init_cleanup(void)
 
     iommu_enabled = 0;
     iommu_hwdom_passthrough = false;
-    iommu_intremap = 0;
+    iommu_intremap = iommu_intremap_off;
     iommuv2_enabled = 0;
 }
 
@@ -1413,6 +1413,9 @@ int __init amd_iommu_prepare(bool xt)
         iommu->ctrl.int_cap_xt_en = xt && has_xt;
     }
 
+    if ( iommu_intremap && !has_xt )
+        iommu_intremap = iommu_intremap_restricted;
+
     rc = amd_iommu_update_ivrs_mapping_acpi();
 
  error_out:
index 3112653960b2fcaf17a7f0bb48a609a1653b3ea5..cc0ff00c1e5417e68fff36fcc7e0fb454560feee 100644 (file)
@@ -157,7 +157,7 @@ int __init acpi_ivrs_init(void)
 
     if ( (amd_iommu_detect_acpi() !=0) || (iommu_found() == 0) )
     {
-        iommu_intremap = 0;
+        iommu_intremap = iommu_intremap_off;
         return -ENODEV;
     }
 
index dac1b58fa5a9949ec9a17c9617f20fb637f8cbd4..0977634c34ddafd762d630ec0be3959cdf7473e1 100644 (file)
@@ -35,7 +35,7 @@ bool __read_mostly iommu_quarantine = true;
 bool_t __read_mostly iommu_igfx = 1;
 bool_t __read_mostly iommu_snoop = 1;
 bool_t __read_mostly iommu_qinval = 1;
-bool_t __read_mostly iommu_intremap = 1;
+enum iommu_intremap __read_mostly iommu_intremap = iommu_intremap_full;
 bool_t __read_mostly iommu_crash_disable;
 
 static bool __hwdom_initdata iommu_hwdom_none;
@@ -91,7 +91,7 @@ static int __init parse_iommu_param(const char *s)
         else if ( (val = parse_boolean("qinval", s, ss)) >= 0 )
             iommu_qinval = val;
         else if ( (val = parse_boolean("intremap", s, ss)) >= 0 )
-            iommu_intremap = val;
+            iommu_intremap = val ? iommu_intremap_full : iommu_intremap_off;
         else if ( (val = parse_boolean("intpost", s, ss)) >= 0 )
             iommu_intpost = val;
 #ifdef CONFIG_KEXEC
@@ -475,7 +475,7 @@ int __init iommu_setup(void)
         iommu_enabled = (rc == 0);
     }
     if ( !iommu_enabled )
-        iommu_intremap = 0;
+        iommu_intremap = iommu_intremap_off;
 
     if ( (force_iommu && !iommu_enabled) ||
          (force_intremap && !iommu_intremap) )
@@ -557,7 +557,8 @@ void iommu_crash_shutdown(void)
 
     if ( iommu_enabled )
         iommu_get_ops()->crash_shutdown();
-    iommu_enabled = iommu_intremap = iommu_intpost = 0;
+    iommu_enabled = iommu_intpost = 0;
+    iommu_intremap = iommu_intremap_off;
 }
 
 int iommu_get_reserved_device_memory(iommu_grdm_t *func, void *ctxt)
index 3d60976dd5cb447980a42a5d4e740ec86c7a866b..5d4cc3fd04de6336fe442588f8dc407f301bcced 100644 (file)
@@ -2177,7 +2177,7 @@ static int __must_check init_vtd_hw(void)
         {
             if ( ioapic_to_iommu(IO_APIC_ID(apic)) == NULL )
             {
-                iommu_intremap = 0;
+                iommu_intremap = iommu_intremap_off;
                 dprintk(XENLOG_ERR VTDPREFIX,
                     "ioapic_to_iommu: ioapic %#x (id: %#x) is NULL! "
                     "Will not try to enable Interrupt Remapping.\n",
@@ -2193,7 +2193,7 @@ static int __must_check init_vtd_hw(void)
             iommu = drhd->iommu;
             if ( enable_intremap(iommu, 0) != 0 )
             {
-                iommu_intremap = 0;
+                iommu_intremap = iommu_intremap_off;
                 dprintk(XENLOG_WARNING VTDPREFIX,
                         "Interrupt Remapping not enabled\n");
 
@@ -2295,7 +2295,7 @@ static int __init vtd_setup(void)
             iommu_qinval = 0;
 
         if ( iommu_intremap && !ecap_intr_remap(iommu->ecap) )
-            iommu_intremap = 0;
+            iommu_intremap = iommu_intremap_off;
 
         /*
          * We cannot use posted interrupt if X86_FEATURE_CX16 is
@@ -2320,7 +2320,7 @@ static int __init vtd_setup(void)
 
     if ( !iommu_qinval && iommu_intremap )
     {
-        iommu_intremap = 0;
+        iommu_intremap = iommu_intremap_off;
         dprintk(XENLOG_WARNING VTDPREFIX, "Interrupt Remapping disabled "
             "since Queued Invalidation isn't supported or enabled.\n");
     }
@@ -2347,7 +2347,7 @@ static int __init vtd_setup(void)
     iommu_snoop = 0;
     iommu_hwdom_passthrough = false;
     iommu_qinval = 0;
-    iommu_intremap = 0;
+    iommu_intremap = iommu_intremap_off;
     iommu_intpost = 0;
     return ret;
 }
index 6f79fb79f3d15e239d41e3eca7eb4477a58b18f5..ea8dad69e6c205aa5974dbd7b115f6a2c45666e2 100644 (file)
@@ -54,7 +54,22 @@ static inline bool_t dfn_eq(dfn_t x, dfn_t y)
 
 extern bool_t iommu_enable, iommu_enabled;
 extern bool force_iommu, iommu_quarantine, iommu_verbose, iommu_igfx;
-extern bool_t iommu_snoop, iommu_qinval, iommu_intremap, iommu_intpost;
+extern bool_t iommu_snoop, iommu_qinval, iommu_intpost;
+extern enum __packed iommu_intremap {
+   /*
+    * In order to allow traditional boolean uses of the iommu_intremap
+    * variable, the "off" value has to come first (yielding a value of zero).
+    */
+   iommu_intremap_off,
+#ifdef CONFIG_X86
+   /*
+    * Interrupt remapping enabled, but only able to generate interrupts
+    * with an 8-bit APIC ID.
+    */
+   iommu_intremap_restricted,
+#endif
+   iommu_intremap_full,
+} iommu_intremap;
 
 #if defined(CONFIG_IOMMU_FORCE_PT_SHARE)
 #define iommu_hap_pt_share true