[Xen-devel] [PATCH v3] IOMMU: keep disabled until iommu_setup() is called

["Jan Beulich" <JBeulich@xxxxxxxx>]

The iommu is enabled by default when xen is booting and later disabled
in iommu_setup() when no iommu is present.

But under some circumstances iommu code can be called before
iommu_setup() is processed. If there is no iommu available xen crashes.

This can happen for example when panic(...) is called as introduced
with the patch "x86-64: detect processors subject to AMD erratum #121
and refuse to boot" since xen 4.1.3, resulting in
find_iommu_for_device() to be called in the context of
disable_IO_APIC() / __stop_this_cpu().

This patch fixes this by keeping the iommu disabled until iommu_setup()
is entered.

Originally-by: Ronny Hegewald <ronny.hegewald@xxxxxxxxx>

In order for iommu_enable to be off initially, iommu_supports_eim()
must not depend on it anymore, nor must acpi_parse_dmar(). The former
in turn requires that iommu_intremap gets uncoupled from iommu_enabled
(in particular, failure during IOMMU setup should no longer result in
iommu_intremap getting cleared by generic code; IOMMU specific code
can still do so provided in can live with the consequences).

This could have the nice side effect of allowing to use "iommu=off"
even when x2APIC was pre-enabled by the BIOS (in which case interrupt
remapping is a requirement, but DMA translation [obviously] isn't), but
that doesn't currently work (and hence x2apic_bsp_setup() forces the
IOMMU on rather than just interrupt remapping).

For consistency with VT-d, make the AMD IOMMU code also skip all ACPI
table parsing when neither iommu_enable nor iommu_intremap are set.

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -975,6 +975,8 @@ void __init x2apic_bsp_setup(void)
         goto restore_out;
     }
 
+    force_iommu = 1;
+
     genapic = apic_x2apic_probe();
     printk("Switched to APIC driver %s.\n", genapic->name);
 
--- a/xen/drivers/passthrough/amd/pci_amd_iommu.c
+++ b/xen/drivers/passthrough/amd/pci_amd_iommu.c
@@ -164,9 +164,13 @@ int __init amd_iov_detect(void)
 {
     INIT_LIST_HEAD(&amd_iommu_head);
 
+    if ( !iommu_enable && !iommu_intremap )
+        return 0;
+
     if ( (amd_iommu_detect_acpi() !=0) || (iommu_found() == 0) )
     {
         printk("AMD-Vi: IOMMU not found!\n");
+        iommu_intremap = 0;
         return -ENODEV;
     }
 
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -41,7 +41,8 @@ static void iommu_dump_p2m_table(unsigne
  *   no-intremap                Disable VT-d Interrupt Remapping
  */
 custom_param("iommu", parse_iommu_param);
-bool_t __read_mostly iommu_enabled = 1;
+bool_t __initdata iommu_enable = 1;
+bool_t __read_mostly iommu_enabled;
 bool_t __read_mostly force_iommu;
 bool_t __initdata iommu_dom0_strict;
 bool_t __read_mostly iommu_verbose;
@@ -77,7 +78,7 @@ static void __init parse_iommu_param(cha
             *ss = '\0';
 
         if ( !parse_bool(s) )
-            iommu_enabled = 0;
+            iommu_enable = 0;
         else if ( !strcmp(s, "force") || !strcmp(s, "required") )
             force_iommu = val;
         else if ( !strcmp(s, "workaround_bios_bug") )
@@ -395,7 +396,7 @@ int __init iommu_setup(void)
     if ( iommu_dom0_strict )
         iommu_passthrough = 0;
 
-    if ( iommu_enabled )
+    if ( iommu_enable )
     {
         rc = iommu_hardware_setup();
         iommu_enabled = (rc == 0);
@@ -409,8 +410,6 @@ int __init iommu_setup(void)
     if ( !iommu_enabled )
     {
         iommu_snoop = 0;
-        iommu_qinval = 0;
-        iommu_intremap = 0;
         iommu_passthrough = 0;
         iommu_dom0_strict = 0;
     }
@@ -419,6 +418,8 @@ int __init iommu_setup(void)
         printk(" - Dom0 mode: %s\n",
                iommu_passthrough ? "Passthrough" :
                iommu_dom0_strict ? "Strict" : "Relaxed");
+    else if ( iommu_intremap )
+        printk("Interrupt remapping enabled\n");
 
     return rc;
 }
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -744,7 +744,7 @@ static int __init acpi_parse_dmar(struct
     dmar = (struct acpi_table_dmar *)table;
     dmar_flags = dmar->flags;
 
-    if ( !iommu_enabled )
+    if ( !iommu_enable && !iommu_intremap )
     {
         ret = -EINVAL;
         goto out;
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -149,8 +149,7 @@ int iommu_supports_eim(void)
     struct acpi_drhd_unit *drhd;
     int apic;
 
-    if ( !iommu_enabled || !iommu_qinval || !iommu_intremap ||
-         list_empty(&acpi_drhd_units) )
+    if ( !iommu_qinval || !iommu_intremap || list_empty(&acpi_drhd_units) )
         return 0;
 
     /* We MUST have a DRHD unit for each IOAPIC. */
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2086,7 +2086,10 @@ int __init intel_vtd_setup(void)
     int ret;
 
     if ( list_empty(&acpi_drhd_units) )
-        return -ENODEV;
+    {
+        ret = -ENODEV;
+        goto error;
+    }
 
     platform_quirks_init();
 
--- a/xen/include/xen/iommu.h
+++ b/xen/include/xen/iommu.h
@@ -26,7 +26,7 @@
 #include <public/hvm/ioreq.h>
 #include <public/domctl.h>
 
-extern bool_t iommu_enabled;
+extern bool_t iommu_enable, iommu_enabled;
 extern bool_t force_iommu, iommu_verbose;
 extern bool_t iommu_workaround_bios_bug, iommu_passthrough;
 extern bool_t iommu_snoop, iommu_qinval, iommu_intremap;

Attachment: IOMMU-defer-marking-enabled.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel