mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v4] x86, vt-d: enable x2apic opt out
@ 2011-05-25 16:36 Youquan Song
  2011-05-25 20:58 ` Ingo Molnar
  0 siblings, 1 reply; 5+ messages in thread
From: Youquan Song @ 2011-05-25 16:36 UTC (permalink / raw)
  To: linux-kernel, mingo
  Cc: david.woodhouse, akpm, tglx, hpa, hpa, allen.m.kay,
	suresh.b.siddha, rajesh.sankaran, asit.k.mallick, kent.liu,
	Youquan Song, Youquan Song, David Woodhouse

New version of VT-d2 specification (http://download.intel.com/technology
/computing/vptech/Intel(r)_VT_for_Direct_IO.pdf) includes a new feature that
provide firmware a way to request system software to opt out of enable x2APIC
mode. DMAR ACPI table newly define flags.1 bit: x2APIC_OPT_OUT which is set to
request System software opt out xAPIC mode if flags.0 bit:INTR_REMAP is also
set.

This patch enable the feature. Also re-define x2apic_supported() to address
platform x2apic support needs 1)processor has x2apic capability 2)interrupt
remapping support 3)firmware does not request opt-out or ignore the request
by adding kernel option.

[dwmw2: This seems like a fundamentally broken approach, pandering to BIOSes
        which can't cope with x2apic being enabled. But aren't there code
        paths which will enable x2apic even if we don't *have* a DMAR table
        because the BIOS has disabled VT-d? But it's in the spec now,
        unfortunately, so I suppose we need to support it :( ]

Signed-off-by: Youquan Song <youquan.song@intel.com>
Reviewed-by: Kay, Allen M <allen.m.kay@intel.com>
Signed-off-by: David Woodhouse <David.Woodhouse@intel.com>
---
 Documentation/kernel-parameters.txt |    3 +++
 arch/x86/include/asm/apic.h         |    2 --
 arch/x86/kernel/apic/apic.c         |   16 ++++++++++++----
 drivers/pci/dmar.c                  |   29 +++++++++++++++++++++++++++--
 drivers/pci/intel-iommu.c           |    6 ++++++
 include/linux/dma_remapping.h       |    1 +
 include/linux/dmar.h                |    3 +++
 include/linux/intel-iommu.h         |    4 ++++
 8 files changed, 56 insertions(+), 8 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index 7c6624e..cea8fa2 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -999,6 +999,9 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 			With this option on every unmap_single operation will
 			result in a hardware IOTLB flush operation as opposed
 			to batching them for performance.
+		no_x2apic_optout [Default Off]
+			With this option BIOS x2APIC opt-out request will be
+			ignored.
 
 	intremap=	[X86-64, Intel-IOMMU]
 			Format: { on (default) | off | nosid }
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 4a0b7c7..ca779fb 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -191,7 +191,6 @@ static inline int x2apic_enabled(void)
 	return 0;
 }
 
-#define x2apic_supported()	(cpu_has_x2apic)
 static inline void x2apic_force_phys(void)
 {
 	x2apic_phys = 1;
@@ -212,7 +211,6 @@ static inline void x2apic_force_phys(void)
 }
 
 #define	x2apic_preenabled 0
-#define	x2apic_supported()	0
 #endif
 
 extern void enable_IR_x2apic(void);
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index b961af8..5b137c4 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1461,16 +1461,17 @@ int __init enable_IR(void)
 void __init enable_IR_x2apic(void)
 {
 	unsigned long flags;
-	int ret, x2apic_enabled = 0;
+	int ret = 0, x2apic_enabled = 0;
 	int dmar_table_init_ret;
 
 	dmar_table_init_ret = dmar_table_init();
-	if (dmar_table_init_ret && !x2apic_supported())
+	if (dmar_table_init_ret && !cpu_has_x2apic)
 		return;
 
 	ret = save_ioapic_entries();
 	if (ret) {
 		pr_info("Saving IO-APIC state failed: %d\n", ret);
+		ret = 0;
 		goto out;
 	}
 
@@ -1497,7 +1498,8 @@ void __init enable_IR_x2apic(void)
 		x2apic_force_phys();
 	}
 
-	x2apic_enabled = 1;
+	if (x2apic_supported())
+		x2apic_enabled = 1;
 
 	if (x2apic_supported() && !x2apic_mode) {
 		x2apic_mode = 1;
@@ -1517,8 +1519,14 @@ out:
 
 	if (x2apic_preenabled)
 		panic("x2apic: enabled by BIOS but kernel init failed.");
-	else if (cpu_has_x2apic)
+	else if (!ret && cpu_has_x2apic) /* IR enabling failed */
 		pr_info("Not enabling x2apic, Intr-remapping init failed.\n");
+	else if (!x2apic_supported() && cpu_has_x2apic)
+		WARN(1, "Your BIOS is broken and requested that x2apic be "
+			"disabled.\n This will leave your machine vulnerable to"
+			" irq-injection attacks\n"
+			"Use 'intel_iommu=no_x2apic_optout' to override BIOS "
+			"request\n");
 }
 
 #ifdef CONFIG_X86_64
diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
index 12e02bf..9a64581 100644
--- a/drivers/pci/dmar.c
+++ b/drivers/pci/dmar.c
@@ -705,7 +705,7 @@ int __init detect_intel_iommu(void)
 		 * is added, we will not need this any more.
 		 */
 		dmar = (struct acpi_table_dmar *) dmar_tbl;
-		if (ret && cpu_has_x2apic && dmar->flags & 0x1)
+		if (ret && x2apic_supported() && dmar->flags & DMAR_INTR_REMAP)
 			printk(KERN_INFO
 			       "Queued invalidation will be enabled to support "
 			       "x2apic and Intr-remapping.\n");
@@ -1461,6 +1461,31 @@ int __init dmar_ir_support(void)
 	dmar = (struct acpi_table_dmar *)dmar_tbl;
 	if (!dmar)
 		return 0;
-	return dmar->flags & 0x1;
+	return dmar->flags & DMAR_INTR_REMAP;
 }
+
+/*
+ * Check if the platform support x2apic
+ * three necessary conditions:
+ * a. processor support x2apic
+ * b. interrupt remapping support
+ * c. when interrupt reamapping support,firmware does not request to opt out
+ *  x2apic by not set x2APIC_OPT_OUT bit at DMAR flags or ignore the request
+ *  by adding kernel option.
+ */
+int __init x2apic_supported(void)
+{
+	struct acpi_table_dmar *dmar;
+	unsigned int flags = 0;
+
+	if (!cpu_has_x2apic)
+		return 0;
+
+	dmar = (struct acpi_table_dmar *)dmar_tbl;
+	if (!dmar)
+		return 0;
+	flags = DMAR_INTR_REMAP | (no_x2apic_optout ? 0 : DMAR_X2APIC_OPT_OUT);
+	return ((dmar->flags & flags) == DMAR_INTR_REMAP);
+}
+
 IOMMU_INIT_POST(detect_intel_iommu);
diff --git a/drivers/pci/intel-iommu.c b/drivers/pci/intel-iommu.c
index 6af6b62..aa9c69f 100644
--- a/drivers/pci/intel-iommu.c
+++ b/drivers/pci/intel-iommu.c
@@ -383,6 +383,7 @@ int dmar_disabled = 0;
 #else
 int dmar_disabled = 1;
 #endif /*CONFIG_DMAR_DEFAULT_ON*/
+int no_x2apic_optout = 0;
 
 static int dmar_map_gfx = 1;
 static int dmar_forcedac;
@@ -417,6 +418,11 @@ static int __init intel_iommu_setup(char *str)
 			printk(KERN_INFO
 				"Intel-IOMMU: disable batched IOTLB flush\n");
 			intel_iommu_strict = 1;
+		} else if (!strncmp(str, "no_x2apic_optout", 16)) {
+			printk(KERN_INFO
+				"Intel-IOMMU: ignore BIOS x2apic opt out "
+				"request\n");
+			no_x2apic_optout = 1;
 		}
 
 		str += strcspn(str, ",");
diff --git a/include/linux/dma_remapping.h b/include/linux/dma_remapping.h
index 5619f85..d2d93d4 100644
--- a/include/linux/dma_remapping.h
+++ b/include/linux/dma_remapping.h
@@ -38,5 +38,6 @@ static inline int iommu_calculate_max_sagaw(struct intel_iommu *iommu)
 #endif
 
 extern int dmar_disabled;
+extern int no_x2apic_optout;
 
 #endif
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index b862dac..3087ccb 100644
--- a/include/linux/dmar.h
+++ b/include/linux/dmar.h
@@ -228,8 +228,11 @@ struct dmar_atsr_unit {
 };
 
 extern int intel_iommu_init(void);
+extern int x2apic_supported(void);
+
 #else /* !CONFIG_DMAR: */
 static inline int intel_iommu_init(void) { return -ENODEV; }
+static inline int x2apic_supported(void) { return 0; }
 #endif /* CONFIG_DMAR */
 
 #endif /* __DMAR_H__ */
diff --git a/include/linux/intel-iommu.h b/include/linux/intel-iommu.h
index 9310c69..2d086e5 100644
--- a/include/linux/intel-iommu.h
+++ b/include/linux/intel-iommu.h
@@ -29,6 +29,10 @@
 #include <asm/cacheflush.h>
 #include <asm/iommu.h>
 
+/* DMAR Flags bits */
+#define DMAR_INTR_REMAP 0x1
+#define DMAR_X2APIC_OPT_OUT 0x2
+
 /*
  * Intel IOMMU register specification per version 1.0 public spec.
  */
-- 
1.6.4.2


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

* Re: [PATCH v4] x86, vt-d: enable x2apic opt out
  2011-05-25 16:36 [PATCH v4] x86, vt-d: enable x2apic opt out Youquan Song
@ 2011-05-25 20:58 ` Ingo Molnar
  2011-05-25 22:01   ` Thomas Gleixner
  0 siblings, 1 reply; 5+ messages in thread
From: Ingo Molnar @ 2011-05-25 20:58 UTC (permalink / raw)
  To: Youquan Song
  Cc: linux-kernel, david.woodhouse, akpm, tglx, hpa, hpa, allen.m.kay,
	suresh.b.siddha, rajesh.sankaran, asit.k.mallick, kent.liu,
	Youquan Song


* Youquan Song <youquan.song@intel.com> wrote:

> New version of VT-d2 specification 
> (http://download.intel.com/technology 
> /computing/vptech/Intel(r)_VT_for_Direct_IO.pdf) includes a new 
> feature that provide firmware a way to request system software to 
> opt out of enable x2APIC mode. DMAR ACPI table newly define flags.1 
> bit: x2APIC_OPT_OUT which is set to request System software opt out 
> xAPIC mode if flags.0 bit:INTR_REMAP is also set.

So why isnt the x2apic disabled in the CPUID? That's the canonical 
way to unsupport a particular non-working CPU hw feature.

Thanks,

	Ingo

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

* Re: [PATCH v4] x86, vt-d: enable x2apic opt out
  2011-05-25 20:58 ` Ingo Molnar
@ 2011-05-25 22:01   ` Thomas Gleixner
  2011-05-25 23:37     ` Woodhouse, David
  0 siblings, 1 reply; 5+ messages in thread
From: Thomas Gleixner @ 2011-05-25 22:01 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Youquan Song, linux-kernel, david.woodhouse, akpm, hpa, hpa,
	allen.m.kay, suresh.b.siddha, rajesh.sankaran, asit.k.mallick,
	kent.liu, Youquan Song

On Wed, 25 May 2011, Ingo Molnar wrote:

> 
> * Youquan Song <youquan.song@intel.com> wrote:
> 
> > New version of VT-d2 specification 
> > (http://download.intel.com/technology 
> > /computing/vptech/Intel(r)_VT_for_Direct_IO.pdf) includes a new 
> > feature that provide firmware a way to request system software to 
> > opt out of enable x2APIC mode. DMAR ACPI table newly define flags.1 
> > bit: x2APIC_OPT_OUT which is set to request System software opt out 
> > xAPIC mode if flags.0 bit:INTR_REMAP is also set.
> 
> So why isnt the x2apic disabled in the CPUID? That's the canonical 
> way to unsupport a particular non-working CPU hw feature.

Because some committee decided to make it an ACPI feature. That's
broken by design, but you can't change the stupid spec retroactively.

Thanks,

	tglx

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

* Re: [PATCH v4] x86, vt-d: enable x2apic opt out
  2011-05-25 22:01   ` Thomas Gleixner
@ 2011-05-25 23:37     ` Woodhouse, David
  2011-06-09 15:34       ` Youquan Song
  0 siblings, 1 reply; 5+ messages in thread
From: Woodhouse, David @ 2011-05-25 23:37 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Ingo Molnar, Song, Youquan, linux-kernel, akpm, hpa, hpa, Kay,
	Allen M, Siddha, Suresh B, Sankaran, Rajesh, Mallick, Asit K,
	Liu, Kent, Youquan Song

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

On Wed, 2011-05-25 at 23:01 +0100, Thomas Gleixner wrote:
> On Wed, 25 May 2011, Ingo Molnar wrote:
> > So why isnt the x2apic disabled in the CPUID? That's the canonical 
> > way to unsupport a particular non-working CPU hw feature.

A valid question. Rajesh?

> Because some committee decided to make it an ACPI feature.
> That's broken by design, but you can't change the stupid spec retroactively.

You can ask for clarification of the stupid spec though. :)

Is it *really* tied to interrupt-remapping, as the wording in the spec
implies?

In particular, what about the case where VT-d has been disabled in the
BIOS so there is *no* DMAR table at all, and hence nowhere for this 'opt
out' bit to be set?

Currently, it looks like we still enable x2apic in that case. We have a
*build* time dependency which means you can't build x2apic support
unless you also build interrupt-remapping support. But unless there's a
lot of dead code in our x2apic support, it looks like we still enable
x2apic at run time if we didn't enable IR for various reasons.

Is that going to make these broken BIOSes fall over too? If so, it
really does look like the placement of this bit in the DMAR table is
entirely wrong.

Rajesh, can you tell use *exactly* what is the BIOS brokenness that this
hack was invented to work around?

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

[-- Attachment #2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 6242 bytes --]

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

* Re: [PATCH v4] x86, vt-d: enable x2apic opt out
  2011-05-25 23:37     ` Woodhouse, David
@ 2011-06-09 15:34       ` Youquan Song
  0 siblings, 0 replies; 5+ messages in thread
From: Youquan Song @ 2011-06-09 15:34 UTC (permalink / raw)
  To: Woodhouse, David, Ingo Molnar
  Cc: Thomas Gleixner, Ingo Molnar, Song, Youquan, linux-kernel, akpm,
	hpa, hpa, Kay, Allen M, Siddha, Suresh B, Sankaran, Rajesh,
	Mallick, Asit K, Liu, Kent, Youquan Song

 
> Is that going to make these broken BIOSes fall over too? If so, it
> really does look like the placement of this bit in the DMAR table is
> entirely wrong.
> 
> Rajesh, can you tell use *exactly* what is the BIOS brokenness that this
> hack was invented to work around?
> 
Hi Ingo and David,

IMO, the OPT-OUT flag is not good solution to workround BIOS or platform
issues. 
But some custmoers really requires it to 1) meet their business target.
2) some components in OEM platforms have issue if they generate
platform-events like SMI, or SMM handlers, some interrupt from LAPIC,
because they does not go through VT-d,so they do not know CPU in x2APIC
or xAPIC. 

Can you take it to maintainer tree? 
It will be better for OSVs integration. 
   
Thanks
-Youquan

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

end of thread, other threads:[~2011-06-09  3:20 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-05-25 16:36 [PATCH v4] x86, vt-d: enable x2apic opt out Youquan Song
2011-05-25 20:58 ` Ingo Molnar
2011-05-25 22:01   ` Thomas Gleixner
2011-05-25 23:37     ` Woodhouse, David
2011-06-09 15:34       ` Youquan Song

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome