All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] IOMMU errata treatment adjustments
@ 2013-03-19 16:19 Jan Beulich
  2013-03-19 16:46 ` [PATCH 1/3] IOMMU: properly check whether interrupt remapping is enabled Jan Beulich
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Jan Beulich @ 2013-03-19 16:19 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Malcolm Crossley, Jacob Shin, xiantao.zhang,
	suravee.suthikulpanit

1: IOMMU: properly check whether interrupt remapping is enabled
2: AMD IOMMU: only disable when certain IVRS consistency checks fail
3: VT-d: deal with 5500/5520/X58 errata

Patch 1 and 2 are version 2 of a previously submitted, then
withdrawn patch following up after XSA-36. Patch 3 is version 3 of
a patch previously sent by Malcolm and Andrew.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] IOMMU: properly check whether interrupt remapping is enabled
  2013-03-19 16:19 [PATCH 0/3] IOMMU errata treatment adjustments Jan Beulich
@ 2013-03-19 16:46 ` Jan Beulich
  2013-03-19 16:47 ` [PATCH 2/3] AMD IOMMU: only disable when certain IVRS consistency checks fail Jan Beulich
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Jan Beulich @ 2013-03-19 16:46 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Malcolm Crossley, Jacob Shin, xiantao.zhang,
	suravee.suthikulpanit

[-- Attachment #1: Type: text/plain, Size: 5275 bytes --]

... rather than the IOMMU as a whole.

That in turn required to make sure iommu_intremap gets properly
cleared when the respective initialization fails (or isn't being
done at all).

Along with making sure interrupt remapping doesn't get inconsistently
enabled on some IOMMUs and not on others in the VT-d code, this in turn
allowed quite a bit of cleanup on the VT-d side (if desired, that
cleanup could of course be broken out into a separate patch).

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -210,7 +210,7 @@ static void read_msi_msg(struct msi_desc
         BUG();
     }
 
-    if ( iommu_enabled )
+    if ( iommu_intremap )
         iommu_read_msi_from_ire(entry, msg);
 }
 
@@ -218,7 +218,7 @@ static void write_msi_msg(struct msi_des
 {
     entry->msg = *msg;
 
-    if ( iommu_enabled )
+    if ( iommu_intremap )
     {
         ASSERT(msg != &entry->msg);
         iommu_update_ire_from_msi(entry, msg);
@@ -492,7 +492,7 @@ int msi_free_irq(struct msi_desc *entry)
     }
 
     /* Free the unused IRTE if intr remap enabled */
-    if ( iommu_enabled )
+    if ( iommu_intremap )
         iommu_update_ire_from_msi(entry, NULL);
 
     list_del(&entry->list);
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -470,6 +470,8 @@ int __init iommu_setup(void)
         rc = iommu_hardware_setup();
         iommu_enabled = (rc == 0);
     }
+    if ( !iommu_enabled )
+        iommu_intremap = 0;
 
     if ( (force_iommu && !iommu_enabled) ||
          (force_intremap && !iommu_intremap) )
@@ -484,9 +486,12 @@ int __init iommu_setup(void)
     }
     printk("I/O virtualisation %sabled\n", iommu_enabled ? "en" : "dis");
     if ( iommu_enabled )
+    {
         printk(" - Dom0 mode: %s\n",
                iommu_passthrough ? "Passthrough" :
                iommu_dom0_strict ? "Strict" : "Relaxed");
+        printk("Interrupt remapping %sabled\n", iommu_intremap ? "en" : "dis");
+    }
 
     return rc;
 }
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -373,7 +373,7 @@ unsigned int io_apic_read_remap_rte(
     struct iommu *iommu = ioapic_to_iommu(IO_APIC_ID(apic));
     struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
 
-    if ( !ir_ctrl || !ir_ctrl->iremap_maddr || !ir_ctrl->iremap_num ||
+    if ( !ir_ctrl->iremap_num ||
         ( (index = apic_pin_2_ir_idx[apic][ioapic_pin]) < 0 ) )
         return __io_apic_read(apic, reg);
 
@@ -396,15 +396,8 @@ void io_apic_write_remap_rte(
     struct IO_APIC_route_remap_entry *remap_rte;
     unsigned int rte_upper = (reg & 1) ? 1 : 0;
     struct iommu *iommu = ioapic_to_iommu(IO_APIC_ID(apic));
-    struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
     int saved_mask;
 
-    if ( !ir_ctrl || !ir_ctrl->iremap_maddr )
-    {
-        __io_apic_write(apic, reg, value);
-        return;
-    }
-
     old_rte = __ioapic_read_entry(apic, ioapic_pin, 1);
 
     remap_rte = (struct IO_APIC_route_remap_entry *) &old_rte;
@@ -653,20 +646,11 @@ void msi_msg_read_remap_rte(
 {
     struct pci_dev *pdev = msi_desc->dev;
     struct acpi_drhd_unit *drhd = NULL;
-    struct iommu *iommu = NULL;
-    struct ir_ctrl *ir_ctrl;
 
     drhd = pdev ? acpi_find_matched_drhd_unit(pdev)
                 : hpet_to_drhd(msi_desc->hpet_id);
-    if ( !drhd )
-        return;
-    iommu = drhd->iommu;
-
-    ir_ctrl = iommu_ir_ctrl(iommu);
-    if ( !ir_ctrl || !ir_ctrl->iremap_maddr )
-        return;
-
-    remap_entry_to_msi_msg(iommu, msg);
+    if ( drhd )
+        remap_entry_to_msi_msg(drhd->iommu, msg);
 }
 
 void msi_msg_write_remap_rte(
@@ -674,20 +658,11 @@ void msi_msg_write_remap_rte(
 {
     struct pci_dev *pdev = msi_desc->dev;
     struct acpi_drhd_unit *drhd = NULL;
-    struct iommu *iommu = NULL;
-    struct ir_ctrl *ir_ctrl;
 
     drhd = pdev ? acpi_find_matched_drhd_unit(pdev)
                 : hpet_to_drhd(msi_desc->hpet_id);
-    if ( !drhd )
-        return;
-    iommu = drhd->iommu;
-
-    ir_ctrl = iommu_ir_ctrl(iommu);
-    if ( !ir_ctrl || !ir_ctrl->iremap_maddr )
-        return;
-
-    msi_msg_to_remap_entry(iommu, pdev, msi_desc, msg);
+    if ( drhd )
+        msi_msg_to_remap_entry(drhd->iommu, pdev, msi_desc, msg);
 }
 
 int __init intel_setup_hpet_msi(struct msi_desc *msi_desc)
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2065,6 +2065,9 @@ static int init_vtd_hw(void)
                 break;
             }
         }
+        if ( !iommu_intremap )
+            for_each_drhd_unit ( drhd )
+                disable_intremap(drhd->iommu);
     }
 
     /*
--- a/xen/include/asm-x86/io_apic.h
+++ b/xen/include/asm-x86/io_apic.h
@@ -129,7 +129,7 @@ struct IO_APIC_route_entry {
 extern struct mpc_config_ioapic mp_ioapics[MAX_IO_APICS];
 
 /* Only need to remap ioapic RTE (reg: 10~3Fh) */
-#define ioapic_reg_remapped(reg) (iommu_enabled && ((reg) >= 0x10))
+#define ioapic_reg_remapped(reg) (iommu_intremap && ((reg) >= 0x10))
 
 static inline unsigned int __io_apic_read(unsigned int apic, unsigned int reg)
 {



[-- Attachment #2: IOMMU-intremap-gating.patch --]
[-- Type: text/plain, Size: 5335 bytes --]

IOMMU: properly check whether interrupt remapping is enabled

... rather than the IOMMU as a whole.

That in turn required to make sure iommu_intremap gets properly
cleared when the respective initialization fails (or isn't being
done at all).

Along with making sure interrupt remapping doesn't get inconsistently
enabled on some IOMMUs and not on others in the VT-d code, this in turn
allowed quite a bit of cleanup on the VT-d side (if desired, that
cleanup could of course be broken out into a separate patch).

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/arch/x86/msi.c
+++ b/xen/arch/x86/msi.c
@@ -210,7 +210,7 @@ static void read_msi_msg(struct msi_desc
         BUG();
     }
 
-    if ( iommu_enabled )
+    if ( iommu_intremap )
         iommu_read_msi_from_ire(entry, msg);
 }
 
@@ -218,7 +218,7 @@ static void write_msi_msg(struct msi_des
 {
     entry->msg = *msg;
 
-    if ( iommu_enabled )
+    if ( iommu_intremap )
     {
         ASSERT(msg != &entry->msg);
         iommu_update_ire_from_msi(entry, msg);
@@ -492,7 +492,7 @@ int msi_free_irq(struct msi_desc *entry)
     }
 
     /* Free the unused IRTE if intr remap enabled */
-    if ( iommu_enabled )
+    if ( iommu_intremap )
         iommu_update_ire_from_msi(entry, NULL);
 
     list_del(&entry->list);
--- a/xen/drivers/passthrough/iommu.c
+++ b/xen/drivers/passthrough/iommu.c
@@ -470,6 +470,8 @@ int __init iommu_setup(void)
         rc = iommu_hardware_setup();
         iommu_enabled = (rc == 0);
     }
+    if ( !iommu_enabled )
+        iommu_intremap = 0;
 
     if ( (force_iommu && !iommu_enabled) ||
          (force_intremap && !iommu_intremap) )
@@ -484,9 +486,12 @@ int __init iommu_setup(void)
     }
     printk("I/O virtualisation %sabled\n", iommu_enabled ? "en" : "dis");
     if ( iommu_enabled )
+    {
         printk(" - Dom0 mode: %s\n",
                iommu_passthrough ? "Passthrough" :
                iommu_dom0_strict ? "Strict" : "Relaxed");
+        printk("Interrupt remapping %sabled\n", iommu_intremap ? "en" : "dis");
+    }
 
     return rc;
 }
--- a/xen/drivers/passthrough/vtd/intremap.c
+++ b/xen/drivers/passthrough/vtd/intremap.c
@@ -373,7 +373,7 @@ unsigned int io_apic_read_remap_rte(
     struct iommu *iommu = ioapic_to_iommu(IO_APIC_ID(apic));
     struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
 
-    if ( !ir_ctrl || !ir_ctrl->iremap_maddr || !ir_ctrl->iremap_num ||
+    if ( !ir_ctrl->iremap_num ||
         ( (index = apic_pin_2_ir_idx[apic][ioapic_pin]) < 0 ) )
         return __io_apic_read(apic, reg);
 
@@ -396,15 +396,8 @@ void io_apic_write_remap_rte(
     struct IO_APIC_route_remap_entry *remap_rte;
     unsigned int rte_upper = (reg & 1) ? 1 : 0;
     struct iommu *iommu = ioapic_to_iommu(IO_APIC_ID(apic));
-    struct ir_ctrl *ir_ctrl = iommu_ir_ctrl(iommu);
     int saved_mask;
 
-    if ( !ir_ctrl || !ir_ctrl->iremap_maddr )
-    {
-        __io_apic_write(apic, reg, value);
-        return;
-    }
-
     old_rte = __ioapic_read_entry(apic, ioapic_pin, 1);
 
     remap_rte = (struct IO_APIC_route_remap_entry *) &old_rte;
@@ -653,20 +646,11 @@ void msi_msg_read_remap_rte(
 {
     struct pci_dev *pdev = msi_desc->dev;
     struct acpi_drhd_unit *drhd = NULL;
-    struct iommu *iommu = NULL;
-    struct ir_ctrl *ir_ctrl;
 
     drhd = pdev ? acpi_find_matched_drhd_unit(pdev)
                 : hpet_to_drhd(msi_desc->hpet_id);
-    if ( !drhd )
-        return;
-    iommu = drhd->iommu;
-
-    ir_ctrl = iommu_ir_ctrl(iommu);
-    if ( !ir_ctrl || !ir_ctrl->iremap_maddr )
-        return;
-
-    remap_entry_to_msi_msg(iommu, msg);
+    if ( drhd )
+        remap_entry_to_msi_msg(drhd->iommu, msg);
 }
 
 void msi_msg_write_remap_rte(
@@ -674,20 +658,11 @@ void msi_msg_write_remap_rte(
 {
     struct pci_dev *pdev = msi_desc->dev;
     struct acpi_drhd_unit *drhd = NULL;
-    struct iommu *iommu = NULL;
-    struct ir_ctrl *ir_ctrl;
 
     drhd = pdev ? acpi_find_matched_drhd_unit(pdev)
                 : hpet_to_drhd(msi_desc->hpet_id);
-    if ( !drhd )
-        return;
-    iommu = drhd->iommu;
-
-    ir_ctrl = iommu_ir_ctrl(iommu);
-    if ( !ir_ctrl || !ir_ctrl->iremap_maddr )
-        return;
-
-    msi_msg_to_remap_entry(iommu, pdev, msi_desc, msg);
+    if ( drhd )
+        msi_msg_to_remap_entry(drhd->iommu, pdev, msi_desc, msg);
 }
 
 int __init intel_setup_hpet_msi(struct msi_desc *msi_desc)
--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2065,6 +2065,9 @@ static int init_vtd_hw(void)
                 break;
             }
         }
+        if ( !iommu_intremap )
+            for_each_drhd_unit ( drhd )
+                disable_intremap(drhd->iommu);
     }
 
     /*
--- a/xen/include/asm-x86/io_apic.h
+++ b/xen/include/asm-x86/io_apic.h
@@ -129,7 +129,7 @@ struct IO_APIC_route_entry {
 extern struct mpc_config_ioapic mp_ioapics[MAX_IO_APICS];
 
 /* Only need to remap ioapic RTE (reg: 10~3Fh) */
-#define ioapic_reg_remapped(reg) (iommu_enabled && ((reg) >= 0x10))
+#define ioapic_reg_remapped(reg) (iommu_intremap && ((reg) >= 0x10))
 
 static inline unsigned int __io_apic_read(unsigned int apic, unsigned int reg)
 {

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/3] AMD IOMMU: only disable when certain IVRS consistency checks fail
  2013-03-19 16:19 [PATCH 0/3] IOMMU errata treatment adjustments Jan Beulich
  2013-03-19 16:46 ` [PATCH 1/3] IOMMU: properly check whether interrupt remapping is enabled Jan Beulich
@ 2013-03-19 16:47 ` Jan Beulich
  2013-03-19 16:48 ` [PATCH 3/3] VT-d: deal with 5500/5520/X58 errata Jan Beulich
  2013-03-25 10:17 ` Ping: [PATCH 0/3] IOMMU errata treatment adjustments Jan Beulich
  3 siblings, 0 replies; 8+ messages in thread
From: Jan Beulich @ 2013-03-19 16:47 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Malcolm Crossley, Jacob Shin, xiantao.zhang,
	suravee.suthikulpanit

[-- Attachment #1: Type: text/plain, Size: 2536 bytes --]

After some more thought on the XSA-36 and specifically the comments we
got regarding disabling the IOMMU in this situation altogether making
things worse instead of better, I came to the conclusion that we can
actually restrict the action in affected cases to just disabling
interrupt remapping. That doesn't make the situation worse than prior
to the XSA-36 fixes (where interrupt remapping didn't really protect
domains from one another), but allows at least DMA isolation to still
be utilized.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Split off generic and VT-d code changes to a separate, prerequisite
    patch.
    Rather than disabling interrupt remapping here, do the respective
    checks only when interrupt remapping is to be enabled, thus
    allowing "iommu=no-intremap" to be used to enable DMA remapping
    inspite of the errata detected here.

--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -659,6 +659,8 @@ static u16 __init parse_ivhd_device_spec
     switch ( special->variety )
     {
     case ACPI_IVHD_IOAPIC:
+        if ( !iommu_intremap )
+            break;
         /*
          * Some BIOSes have IOAPIC broken entries so we check for IVRS
          * consistency here --- whether entry's IOAPIC ID is valid and
@@ -921,7 +923,7 @@ static int __init parse_ivrs_table(struc
     }
 
     /* Each IO-APIC must have been mentioned in the table. */
-    for ( apic = 0; !error && apic < nr_ioapics; ++apic )
+    for ( apic = 0; !error && iommu_intremap && apic < nr_ioapics; ++apic )
     {
         if ( !nr_ioapic_entries[apic] ||
              ioapic_sbdf[IO_APIC_ID(apic)].pin_setup )
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -1194,7 +1194,8 @@ int __init amd_iommu_init(void)
 
     BUG_ON( !iommu_found() );
 
-    if ( amd_iommu_perdev_intremap && amd_sp5100_erratum28() )
+    if ( iommu_intremap && amd_iommu_perdev_intremap &&
+         amd_sp5100_erratum28() )
         goto error_out;
 
     ivrs_bdf_entries = amd_iommu_get_ivrs_dev_entries();
@@ -1211,7 +1212,7 @@ int __init amd_iommu_init(void)
         goto error_out;
 
     /* initialize io-apic interrupt remapping entries */
-    if ( amd_iommu_setup_ioapic_remapping() != 0 )
+    if ( iommu_intremap && amd_iommu_setup_ioapic_remapping() != 0 )
         goto error_out;
 
     /* allocate and initialize a global device table shared by all iommus */




[-- Attachment #2: AMD-IOMMU-disable-intremap-only.patch --]
[-- Type: text/plain, Size: 2599 bytes --]

AMD IOMMU: only disable when certain IVRS consistency checks fail

After some more thought on the XSA-36 and specifically the comments we
got regarding disabling the IOMMU in this situation altogether making
things worse instead of better, I came to the conclusion that we can
actually restrict the action in affected cases to just disabling
interrupt remapping. That doesn't make the situation worse than prior
to the XSA-36 fixes (where interrupt remapping didn't really protect
domains from one another), but allows at least DMA isolation to still
be utilized.

Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Split off generic and VT-d code changes to a separate, prerequisite
    patch.
    Rather than disabling interrupt remapping here, do the respective
    checks only when interrupt remapping is to be enabled, thus
    allowing "iommu=no-intremap" to be used to enable DMA remapping
    inspite of the errata detected here.

--- a/xen/drivers/passthrough/amd/iommu_acpi.c
+++ b/xen/drivers/passthrough/amd/iommu_acpi.c
@@ -659,6 +659,8 @@ static u16 __init parse_ivhd_device_spec
     switch ( special->variety )
     {
     case ACPI_IVHD_IOAPIC:
+        if ( !iommu_intremap )
+            break;
         /*
          * Some BIOSes have IOAPIC broken entries so we check for IVRS
          * consistency here --- whether entry's IOAPIC ID is valid and
@@ -921,7 +923,7 @@ static int __init parse_ivrs_table(struc
     }
 
     /* Each IO-APIC must have been mentioned in the table. */
-    for ( apic = 0; !error && apic < nr_ioapics; ++apic )
+    for ( apic = 0; !error && iommu_intremap && apic < nr_ioapics; ++apic )
     {
         if ( !nr_ioapic_entries[apic] ||
              ioapic_sbdf[IO_APIC_ID(apic)].pin_setup )
--- a/xen/drivers/passthrough/amd/iommu_init.c
+++ b/xen/drivers/passthrough/amd/iommu_init.c
@@ -1194,7 +1194,8 @@ int __init amd_iommu_init(void)
 
     BUG_ON( !iommu_found() );
 
-    if ( amd_iommu_perdev_intremap && amd_sp5100_erratum28() )
+    if ( iommu_intremap && amd_iommu_perdev_intremap &&
+         amd_sp5100_erratum28() )
         goto error_out;
 
     ivrs_bdf_entries = amd_iommu_get_ivrs_dev_entries();
@@ -1211,7 +1212,7 @@ int __init amd_iommu_init(void)
         goto error_out;
 
     /* initialize io-apic interrupt remapping entries */
-    if ( amd_iommu_setup_ioapic_remapping() != 0 )
+    if ( iommu_intremap && amd_iommu_setup_ioapic_remapping() != 0 )
         goto error_out;
 
     /* allocate and initialize a global device table shared by all iommus */

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/3] VT-d: deal with 5500/5520/X58 errata
  2013-03-19 16:19 [PATCH 0/3] IOMMU errata treatment adjustments Jan Beulich
  2013-03-19 16:46 ` [PATCH 1/3] IOMMU: properly check whether interrupt remapping is enabled Jan Beulich
  2013-03-19 16:47 ` [PATCH 2/3] AMD IOMMU: only disable when certain IVRS consistency checks fail Jan Beulich
@ 2013-03-19 16:48 ` Jan Beulich
  2013-04-12  6:52   ` Zhang, Xiantao
  2013-03-25 10:17 ` Ping: [PATCH 0/3] IOMMU errata treatment adjustments Jan Beulich
  3 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2013-03-19 16:48 UTC (permalink / raw)
  To: xen-devel
  Cc: Andrew Cooper, Malcolm Crossley, Jacob Shin, xiantao.zhang,
	suravee.suthikulpanit

[-- Attachment #1: Type: text/plain, Size: 2871 bytes --]

http://www.intel.com/content/www/us/en/chipsets/5520-and-5500-chipset-ioh-specification-update.html 

Stepping B-3 has two errata (#47 and #53) related to Interrupt
remapping, to which the workaround is for the BIOS to completely disable
interrupt remapping.  These errata are fixed in stepping C-2.

Unfortunately this chipset stepping is very common and many BIOSes are
not disabling interrupt remapping on this stepping .  We can detect this in
Xen and prevent Xen from using the problematic interrupt remapping feature.

The Intel 5500/5520/X58 chipset does not support VT-d
Extended Interrupt Mode(EIM). This means the iommu_supports_eim() check
always fails and so x2apic mode cannot be enabled in Xen before this quirk
disables the interrupt remapping feature.

Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Gate the function call to check the quirk on interrupt remapping being
requested to get enabled, and upon failure disable the IOMMU to be in
line with what the changes for XSA-36 (plus follow-ups) did.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2125,6 +2125,11 @@ int __init intel_vtd_setup(void)
     }
 
     platform_quirks_init();
+    if ( !iommu_enable )
+    {
+        ret = -ENODEV;
+        goto error;
+    }
 
     /* We enable the following features only if they are supported by all VT-d
      * engines: Snoop Control, DMA passthrough, Queued Invalidation and
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -244,6 +244,29 @@ void vtd_ops_postamble_quirk(struct iomm
     }
 }
 
+/* 5500/5520/X58 Chipset Interrupt remapping errata, for stepping B-3.
+ * Fixed in stepping C-2. */
+static void __init tylersburg_intremap_quirk(void)
+{
+    uint32_t bus, device;
+    uint8_t rev;
+
+    for ( bus = 0; bus < 0x100; bus++ )
+    {
+        /* Match on System Management Registers on Device 20 Function 0 */
+        device = pci_conf_read32(0, bus, 20, 0, PCI_VENDOR_ID);
+        rev = pci_conf_read8(0, bus, 20, 0, PCI_REVISION_ID);
+
+        if ( rev == 0x13 && device == 0x342e8086 )
+        {
+            printk(XENLOG_WARNING VTDPREFIX
+                   "Disabling IOMMU due to Intel 5500/5520/X58 Chipset errata #47, #53\n");
+            iommu_enable = 0;
+            break;
+        }
+    }
+}
+
 /* initialize platform identification flags */
 void __init platform_quirks_init(void)
 {
@@ -264,6 +287,10 @@ void __init platform_quirks_init(void)
 
     /* ioremap IGD MMIO+0x2000 page */
     map_igd_reg();
+
+    /* Tylersburg interrupt remap quirk */
+    if ( iommu_intremap )
+        tylersburg_intremap_quirk();
 }
 
 /*




[-- Attachment #2: VT-d-5500-5700-X58-errata.patch --]
[-- Type: text/plain, Size: 2905 bytes --]

VT-d: deal with 5500/5520/X58 errata

http://www.intel.com/content/www/us/en/chipsets/5520-and-5500-chipset-ioh-specification-update.html 

Stepping B-3 has two errata (#47 and #53) related to Interrupt
remapping, to which the workaround is for the BIOS to completely disable
interrupt remapping.  These errata are fixed in stepping C-2.

Unfortunately this chipset stepping is very common and many BIOSes are
not disabling interrupt remapping on this stepping .  We can detect this in
Xen and prevent Xen from using the problematic interrupt remapping feature.

The Intel 5500/5520/X58 chipset does not support VT-d
Extended Interrupt Mode(EIM). This means the iommu_supports_eim() check
always fails and so x2apic mode cannot be enabled in Xen before this quirk
disables the interrupt remapping feature.

Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Gate the function call to check the quirk on interrupt remapping being
requested to get enabled, and upon failure disable the IOMMU to be in
line with what the changes for XSA-36 (plus follow-ups) did.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/xen/drivers/passthrough/vtd/iommu.c
+++ b/xen/drivers/passthrough/vtd/iommu.c
@@ -2125,6 +2125,11 @@ int __init intel_vtd_setup(void)
     }
 
     platform_quirks_init();
+    if ( !iommu_enable )
+    {
+        ret = -ENODEV;
+        goto error;
+    }
 
     /* We enable the following features only if they are supported by all VT-d
      * engines: Snoop Control, DMA passthrough, Queued Invalidation and
--- a/xen/drivers/passthrough/vtd/quirks.c
+++ b/xen/drivers/passthrough/vtd/quirks.c
@@ -244,6 +244,29 @@ void vtd_ops_postamble_quirk(struct iomm
     }
 }
 
+/* 5500/5520/X58 Chipset Interrupt remapping errata, for stepping B-3.
+ * Fixed in stepping C-2. */
+static void __init tylersburg_intremap_quirk(void)
+{
+    uint32_t bus, device;
+    uint8_t rev;
+
+    for ( bus = 0; bus < 0x100; bus++ )
+    {
+        /* Match on System Management Registers on Device 20 Function 0 */
+        device = pci_conf_read32(0, bus, 20, 0, PCI_VENDOR_ID);
+        rev = pci_conf_read8(0, bus, 20, 0, PCI_REVISION_ID);
+
+        if ( rev == 0x13 && device == 0x342e8086 )
+        {
+            printk(XENLOG_WARNING VTDPREFIX
+                   "Disabling IOMMU due to Intel 5500/5520/X58 Chipset errata #47, #53\n");
+            iommu_enable = 0;
+            break;
+        }
+    }
+}
+
 /* initialize platform identification flags */
 void __init platform_quirks_init(void)
 {
@@ -264,6 +287,10 @@ void __init platform_quirks_init(void)
 
     /* ioremap IGD MMIO+0x2000 page */
     map_igd_reg();
+
+    /* Tylersburg interrupt remap quirk */
+    if ( iommu_intremap )
+        tylersburg_intremap_quirk();
 }
 
 /*

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Ping: [PATCH 0/3] IOMMU errata treatment adjustments
  2013-03-19 16:19 [PATCH 0/3] IOMMU errata treatment adjustments Jan Beulich
                   ` (2 preceding siblings ...)
  2013-03-19 16:48 ` [PATCH 3/3] VT-d: deal with 5500/5520/X58 errata Jan Beulich
@ 2013-03-25 10:17 ` Jan Beulich
  2013-03-25 11:11   ` Zhang, Xiantao
  3 siblings, 1 reply; 8+ messages in thread
From: Jan Beulich @ 2013-03-25 10:17 UTC (permalink / raw)
  To: Jacob Shin, suravee.suthikulpanit, xiantao.zhang
  Cc: Andrew Cooper, Malcolm Crossley, xen-devel

>>> On 19.03.13 at 17:19, "Jan Beulich" <JBeulich@suse.com> wrote:
> 1: IOMMU: properly check whether interrupt remapping is enabled
> 2: AMD IOMMU: only disable when certain IVRS consistency checks fail
> 3: VT-d: deal with 5500/5520/X58 errata
> 
> Patch 1 and 2 are version 2 of a previously submitted, then
> withdrawn patch following up after XSA-36. Patch 3 is version 3 of
> a patch previously sent by Malcolm and Andrew.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Maintainers of that code - any opinion? ACKs? NAKs?

Thanks, Jan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Ping: [PATCH 0/3] IOMMU errata treatment adjustments
  2013-03-25 10:17 ` Ping: [PATCH 0/3] IOMMU errata treatment adjustments Jan Beulich
@ 2013-03-25 11:11   ` Zhang, Xiantao
  2013-03-25 15:49     ` Suravee Suthikulanit
  0 siblings, 1 reply; 8+ messages in thread
From: Zhang, Xiantao @ 2013-03-25 11:11 UTC (permalink / raw)
  To: Jan Beulich, Jacob Shin, suravee.suthikulpanit
  Cc: Andrew Cooper, Malcolm Crossley, Zhang, Xiantao, xen-devel

Sorry for late.   They make sense to me.  Acked!   Thanks!
Xiantao

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Monday, March 25, 2013 6:18 PM
> To: Jacob Shin; suravee.suthikulpanit@amd.com; Zhang, Xiantao
> Cc: Andrew Cooper; Malcolm Crossley; xen-devel
> Subject: Ping: [PATCH 0/3] IOMMU errata treatment adjustments
> 
> >>> On 19.03.13 at 17:19, "Jan Beulich" <JBeulich@suse.com> wrote:
> > 1: IOMMU: properly check whether interrupt remapping is enabled
> > 2: AMD IOMMU: only disable when certain IVRS consistency checks fail
> > 3: VT-d: deal with 5500/5520/X58 errata
> >
> > Patch 1 and 2 are version 2 of a previously submitted, then
> > withdrawn patch following up after XSA-36. Patch 3 is version 3 of
> > a patch previously sent by Malcolm and Andrew.
> >
> > Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Maintainers of that code - any opinion? ACKs? NAKs?
> 
> Thanks, Jan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: Ping: [PATCH 0/3] IOMMU errata treatment adjustments
  2013-03-25 11:11   ` Zhang, Xiantao
@ 2013-03-25 15:49     ` Suravee Suthikulanit
  0 siblings, 0 replies; 8+ messages in thread
From: Suravee Suthikulanit @ 2013-03-25 15:49 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Andrew Cooper, Malcolm Crossley, Jacob Shin, Zhang, Xiantao, xen-devel

Sorry for late reply.  This works looks ok and works fine.  Acked.

Thank you

Suravee
On 3/25/2013 6:11 AM, Zhang, Xiantao wrote:
> Sorry for late.   They make sense to me.  Acked!   Thanks!
> Xiantao
>
>> -----Original Message-----
>> From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: Monday, March 25, 2013 6:18 PM
>> To: Jacob Shin; suravee.suthikulpanit@amd.com; Zhang, Xiantao
>> Cc: Andrew Cooper; Malcolm Crossley; xen-devel
>> Subject: Ping: [PATCH 0/3] IOMMU errata treatment adjustments
>>
>>>>> On 19.03.13 at 17:19, "Jan Beulich" <JBeulich@suse.com> wrote:
>>> 1: IOMMU: properly check whether interrupt remapping is enabled
>>> 2: AMD IOMMU: only disable when certain IVRS consistency checks fail
>>> 3: VT-d: deal with 5500/5520/X58 errata
>>>
>>> Patch 1 and 2 are version 2 of a previously submitted, then
>>> withdrawn patch following up after XSA-36. Patch 3 is version 3 of
>>> a patch previously sent by Malcolm and Andrew.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> Maintainers of that code - any opinion? ACKs? NAKs?
>>
>> Thanks, Jan
>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 3/3] VT-d: deal with 5500/5520/X58 errata
  2013-03-19 16:48 ` [PATCH 3/3] VT-d: deal with 5500/5520/X58 errata Jan Beulich
@ 2013-04-12  6:52   ` Zhang, Xiantao
  0 siblings, 0 replies; 8+ messages in thread
From: Zhang, Xiantao @ 2013-04-12  6:52 UTC (permalink / raw)
  To: Jan Beulich, xen-devel
  Cc: Andrew Cooper, Malcolm Crossley, Jacob Shin, Zhang, Xiantao,
	suravee.suthikulpanit

Looks fine, Acked! Thanks! 
Xiantao

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Wednesday, March 20, 2013 12:48 AM
> To: xen-devel
> Cc: Jacob Shin; suravee.suthikulpanit@amd.com; Andrew Cooper; Malcolm
> Crossley; Zhang, Xiantao
> Subject: [PATCH 3/3] VT-d: deal with 5500/5520/X58 errata
> 
> http://www.intel.com/content/www/us/en/chipsets/5520-and-5500-chipset-ioh-
> specification-update.html
> 
> Stepping B-3 has two errata (#47 and #53) related to Interrupt
> remapping, to which the workaround is for the BIOS to completely disable
> interrupt remapping.  These errata are fixed in stepping C-2.
> 
> Unfortunately this chipset stepping is very common and many BIOSes are
> not disabling interrupt remapping on this stepping .  We can detect this in
> Xen and prevent Xen from using the problematic interrupt remapping feature.
> 
> The Intel 5500/5520/X58 chipset does not support VT-d
> Extended Interrupt Mode(EIM). This means the iommu_supports_eim() check
> always fails and so x2apic mode cannot be enabled in Xen before this quirk
> disables the interrupt remapping feature.
> 
> Signed-off-by: Malcolm Crossley <malcolm.crossley@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Gate the function call to check the quirk on interrupt remapping being
> requested to get enabled, and upon failure disable the IOMMU to be in
> line with what the changes for XSA-36 (plus follow-ups) did.
> 
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -2125,6 +2125,11 @@ int __init intel_vtd_setup(void)
>      }
> 
>      platform_quirks_init();
> +    if ( !iommu_enable )
> +    {
> +        ret = -ENODEV;
> +        goto error;
> +    }
> 
>      /* We enable the following features only if they are supported by all VT-d
>       * engines: Snoop Control, DMA passthrough, Queued Invalidation and
> --- a/xen/drivers/passthrough/vtd/quirks.c
> +++ b/xen/drivers/passthrough/vtd/quirks.c
> @@ -244,6 +244,29 @@ void vtd_ops_postamble_quirk(struct iomm
>      }
>  }
> 
> +/* 5500/5520/X58 Chipset Interrupt remapping errata, for stepping B-3.
> + * Fixed in stepping C-2. */
> +static void __init tylersburg_intremap_quirk(void)
> +{
> +    uint32_t bus, device;
> +    uint8_t rev;
> +
> +    for ( bus = 0; bus < 0x100; bus++ )
> +    {
> +        /* Match on System Management Registers on Device 20 Function 0 */
> +        device = pci_conf_read32(0, bus, 20, 0, PCI_VENDOR_ID);
> +        rev = pci_conf_read8(0, bus, 20, 0, PCI_REVISION_ID);
> +
> +        if ( rev == 0x13 && device == 0x342e8086 )
> +        {
> +            printk(XENLOG_WARNING VTDPREFIX
> +                   "Disabling IOMMU due to Intel 5500/5520/X58 Chipset errata #47,
> #53\n");
> +            iommu_enable = 0;
> +            break;
> +        }
> +    }
> +}
> +
>  /* initialize platform identification flags */
>  void __init platform_quirks_init(void)
>  {
> @@ -264,6 +287,10 @@ void __init platform_quirks_init(void)
> 
>      /* ioremap IGD MMIO+0x2000 page */
>      map_igd_reg();
> +
> +    /* Tylersburg interrupt remap quirk */
> +    if ( iommu_intremap )
> +        tylersburg_intremap_quirk();
>  }
> 
>  /*
> 
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2013-04-12  6:52 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-19 16:19 [PATCH 0/3] IOMMU errata treatment adjustments Jan Beulich
2013-03-19 16:46 ` [PATCH 1/3] IOMMU: properly check whether interrupt remapping is enabled Jan Beulich
2013-03-19 16:47 ` [PATCH 2/3] AMD IOMMU: only disable when certain IVRS consistency checks fail Jan Beulich
2013-03-19 16:48 ` [PATCH 3/3] VT-d: deal with 5500/5520/X58 errata Jan Beulich
2013-04-12  6:52   ` Zhang, Xiantao
2013-03-25 10:17 ` Ping: [PATCH 0/3] IOMMU errata treatment adjustments Jan Beulich
2013-03-25 11:11   ` Zhang, Xiantao
2013-03-25 15:49     ` Suravee Suthikulanit

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.