All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] x86, vt-d: enable x2apic opt out
@ 2011-04-14  7:06 Youquan Song
  2011-05-11 15:07 ` Woodhouse, David
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Youquan Song @ 2011-04-14  7:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: david.woodhouse, akpm, mingo, tglx, hpa, hpa, allen.m.kay,
	suresh.b.siddha, kent.liu, Youquan Song, Youquan Song

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.

Signed-off-by: Youquan Song <youquan.song@intel.com>
Reviewed-by: Kay, Allen M <allen.m.kay@intel.com>
---
 arch/x86/include/asm/apic.h |    2 --
 arch/x86/kernel/apic/apic.c |   13 +++++++++----
 drivers/pci/dmar.c          |   28 ++++++++++++++++++++++++++--
 include/linux/dmar.h        |    3 +++
 include/linux/intel-iommu.h |    4 ++++
 5 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 3c89694..aa3fc82 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -192,7 +192,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;
@@ -213,7 +212,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 76b96d7..6c96c45 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1449,11 +1449,11 @@ void __init enable_IR_x2apic(void)
 {
 	unsigned long flags;
 	struct IO_APIC_route_entry **ioapic_entries = NULL;
-	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;
 
 	ioapic_entries = alloc_ioapic_entries();
@@ -1465,6 +1465,7 @@ void __init enable_IR_x2apic(void)
 	ret = save_IO_APIC_setup(ioapic_entries);
 	if (ret) {
 		pr_info("Saving IO-APIC state failed: %d\n", ret);
+		ret = 0;
 		goto out;
 	}
 
@@ -1491,7 +1492,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;
@@ -1514,8 +1516,11 @@ 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)
+		pr_info("Not enabling x2apic, firmware requests OS opt-out "
+			  "x2apic.\n");
 }
 
 #ifdef CONFIG_X86_64
diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
index 09933eb..a35dca9 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,30 @@ 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,bit of x2APIC_OPT_OUT at "DMAR flags"
+ *  is not set which means firmware does not tell OS opt out x2apic
+ */
+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 | DMAR_X2APIC_OPT_OUT;
+	return ((dmar->flags & flags) == DMAR_INTR_REMAP);
+}
+
 IOMMU_INIT_POST(detect_intel_iommu);
diff --git a/include/linux/dmar.h b/include/linux/dmar.h
index 7b776d7..73c9bff 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 related	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] x86, vt-d: enable x2apic opt out
  2011-04-14  7:06 [PATCH v2] x86, vt-d: enable x2apic opt out Youquan Song
@ 2011-05-11 15:07 ` Woodhouse, David
  2011-05-12  3:39   ` Youquan Song
  2011-05-16 20:32   ` Alex Williamson
  2011-05-11 17:21 ` Woodhouse, David
  2011-05-12  1:52 ` Youquan Song
  2 siblings, 2 replies; 12+ messages in thread
From: Woodhouse, David @ 2011-05-11 15:07 UTC (permalink / raw)
  To: Song, Youquan
  Cc: linux-kernel, akpm, mingo, tglx, hpa, hpa, Kay, Allen M, Siddha,
	Suresh B, Liu, Kent, Youquan Song

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

On Thu, 2011-04-14 at 08:06 +0100, Song, Youquan 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.
> 
> 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. 

Given that x2apic is *required* to be safe from irq injection tricks,
why would we ever want to manually disable it?

Is this just a workaround for a crappy BIOS? What is the *actual* reason
for wanting to disable x2apic?

-- 
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] 12+ messages in thread

* Re: [PATCH v2] x86, vt-d: enable x2apic opt out
  2011-05-12  3:39   ` Youquan Song
@ 2011-05-11 15:39     ` David Woodhouse
  2011-05-11 22:38       ` Valdis.Kletnieks
  0 siblings, 1 reply; 12+ messages in thread
From: David Woodhouse @ 2011-05-11 15:39 UTC (permalink / raw)
  To: Youquan Song
  Cc: Song, Youquan, linux-kernel, akpm, mingo, tglx, hpa, hpa, Kay,
	Allen M, Siddha, Suresh B, Liu, Kent, Sankaran, Rajesh, Mallick,
	Asit K

On Thu, 2011-05-12 at 04:39 +0100, Youquan Song wrote:
> The VT-d 1.3 version specification add this new feature because OEM
> request it. If OEM platform has issues to support x2apic or BIOS is 
> buggy to support x2apic, there is alternative way to opt out x2apic. 

Why not just *fix* the BIOS? Why must we hard-code workarounds into the
"hardware" specification to allow them to get away without doing their
job competently? This is madness.

Can't we just line up the substandard BIOS engineers against the wall,
and shoot them?

We should get Coreboot working on our new hardware as a matter of
course, to catch up with our main competitor and ensure that our
customers are not held hostage by these morons.

-- 
dwmw2


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

* Re: [PATCH v2] x86, vt-d: enable x2apic opt out
  2011-04-14  7:06 [PATCH v2] x86, vt-d: enable x2apic opt out Youquan Song
  2011-05-11 15:07 ` Woodhouse, David
@ 2011-05-11 17:21 ` Woodhouse, David
  2011-05-12  1:52 ` Youquan Song
  2 siblings, 0 replies; 12+ messages in thread
From: Woodhouse, David @ 2011-05-11 17:21 UTC (permalink / raw)
  To: Song, Youquan
  Cc: linux-kernel, akpm, mingo, tglx, hpa, hpa, Kay, Allen M, Siddha,
	Suresh B, Liu, Kent, Youquan Song

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

On Thu, 2011-04-14 at 08:06 +0100, Song, Youquan wrote:
> +               pr_info("Not enabling x2apic, firmware requests OS opt-out "
> +                         "x2apic.\n"); 

This output is far too innocuous. At the very least, it should have a
clear statement that this should leave you vulnerable to IRQ injection
attacks that intr-remapping + x2apic would have protected against.

It should probably look more like:

 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");


-- 
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] 12+ messages in thread

* Re: [PATCH v2] x86, vt-d: enable x2apic opt out
  2011-05-11 15:39     ` David Woodhouse
@ 2011-05-11 22:38       ` Valdis.Kletnieks
  0 siblings, 0 replies; 12+ messages in thread
From: Valdis.Kletnieks @ 2011-05-11 22:38 UTC (permalink / raw)
  To: David Woodhouse
  Cc: Youquan Song, Song, Youquan, linux-kernel, akpm, mingo, tglx,
	hpa, hpa, Kay, Allen M, Siddha, Suresh B, Liu, Kent, Sankaran,
	Rajesh, Mallick, Asit K

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

On Wed, 11 May 2011 16:39:38 BST, David Woodhouse said:

> Can't we just line up the substandard BIOS engineers against the wall,
> and shoot them?

Linus has stated "We do not trust BIOS tables, because BIOS writers are
invariably totally incompetent crack-addicted monkeys."

Which means if we shoot them, the ASPCA and PETA will be on our case for it...

[-- Attachment #2: Type: application/pgp-signature, Size: 227 bytes --]

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

* Re: [PATCH v2] x86, vt-d: enable x2apic opt out
  2011-04-14  7:06 [PATCH v2] x86, vt-d: enable x2apic opt out Youquan Song
  2011-05-11 15:07 ` Woodhouse, David
  2011-05-11 17:21 ` Woodhouse, David
@ 2011-05-12  1:52 ` Youquan Song
  2 siblings, 0 replies; 12+ messages in thread
From: Youquan Song @ 2011-05-12  1:52 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-kernel, david.woodhouse, akpm, mingo, tglx, hpa, hpa,
	allen.m.kay, suresh.b.siddha, kent.liu, Youquan Song


The path has been verified from customer platform and it works well.
This patch is also desirable from Linux OSV distributions.
please take it and get it into 2.6.40 merge window.

Thanks
-Youquan


On Thu, Apr 14, 2011 at 03:06:23PM +0800, Youquan Song 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.
> 
> 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.
> 
> Signed-off-by: Youquan Song <youquan.song@intel.com>
> Reviewed-by: Kay, Allen M <allen.m.kay@intel.com>
> ---
>  arch/x86/include/asm/apic.h |    2 --
>  arch/x86/kernel/apic/apic.c |   13 +++++++++----
>  drivers/pci/dmar.c          |   28 ++++++++++++++++++++++++++--
>  include/linux/dmar.h        |    3 +++
>  include/linux/intel-iommu.h |    4 ++++
>  5 files changed, 42 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> index 3c89694..aa3fc82 100644
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -192,7 +192,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;
> @@ -213,7 +212,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 76b96d7..6c96c45 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -1449,11 +1449,11 @@ void __init enable_IR_x2apic(void)
>  {
>  	unsigned long flags;
>  	struct IO_APIC_route_entry **ioapic_entries = NULL;
> -	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;
>  
>  	ioapic_entries = alloc_ioapic_entries();
> @@ -1465,6 +1465,7 @@ void __init enable_IR_x2apic(void)
>  	ret = save_IO_APIC_setup(ioapic_entries);
>  	if (ret) {
>  		pr_info("Saving IO-APIC state failed: %d\n", ret);
> +		ret = 0;
>  		goto out;
>  	}
>  
> @@ -1491,7 +1492,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;
> @@ -1514,8 +1516,11 @@ 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)
> +		pr_info("Not enabling x2apic, firmware requests OS opt-out "
> +			  "x2apic.\n");
>  }
>  
>  #ifdef CONFIG_X86_64
> diff --git a/drivers/pci/dmar.c b/drivers/pci/dmar.c
> index 09933eb..a35dca9 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,30 @@ 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,bit of x2APIC_OPT_OUT at "DMAR flags"
> + *  is not set which means firmware does not tell OS opt out x2apic
> + */
> +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 | DMAR_X2APIC_OPT_OUT;
> +	return ((dmar->flags & flags) == DMAR_INTR_REMAP);
> +}
> +
>  IOMMU_INIT_POST(detect_intel_iommu);
> diff --git a/include/linux/dmar.h b/include/linux/dmar.h
> index 7b776d7..73c9bff 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] 12+ messages in thread

* Re: [PATCH v2] x86, vt-d: enable x2apic opt out
  2011-05-11 15:07 ` Woodhouse, David
@ 2011-05-12  3:39   ` Youquan Song
  2011-05-11 15:39     ` David Woodhouse
  2011-05-16 20:32   ` Alex Williamson
  1 sibling, 1 reply; 12+ messages in thread
From: Youquan Song @ 2011-05-12  3:39 UTC (permalink / raw)
  To: Woodhouse, David
  Cc: Song, Youquan, linux-kernel, akpm, mingo, tglx, hpa, hpa, Kay,
	Allen M, Siddha, Suresh B, Liu, Kent, Youquan Song,
	rajesh.sankaran, asit.k.mallick

 
> Given that x2apic is *required* to be safe from irq injection tricks,
> why would we ever want to manually disable it?
> Is this just a workaround for a crappy BIOS? What is the *actual* reason
> for wanting to disable x2apic?

The VT-d 1.3 version specification add this new feature because OEM
request it. If OEM platform has issues to support x2apic or BIOS is 
buggy to support x2apic, there is alternative way to opt out x2apic. 

Refer to VT-d 1.3 specification Chpater 8.1.

Thanks
-Youquan


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

* Re: [PATCH v2] x86, vt-d: enable x2apic opt out
  2011-05-11 15:07 ` Woodhouse, David
  2011-05-12  3:39   ` Youquan Song
@ 2011-05-16 20:32   ` Alex Williamson
  2011-05-17  9:49     ` Woodhouse, David
  2011-05-18 22:58     ` Woodhouse, David
  1 sibling, 2 replies; 12+ messages in thread
From: Alex Williamson @ 2011-05-16 20:32 UTC (permalink / raw)
  To: Woodhouse, David
  Cc: Song, Youquan, linux-kernel, akpm, mingo, tglx, hpa, hpa, Kay,
	Allen M, Siddha, Suresh B, Liu, Kent, Youquan Song

On Wed, 2011-05-11 at 16:07 +0100, Woodhouse, David wrote:
> On Thu, 2011-04-14 at 08:06 +0100, Song, Youquan 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.
> > 
> > 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. 
> 
> Given that x2apic is *required* to be safe from irq injection tricks,
> why would we ever want to manually disable it?
> 
> Is this just a workaround for a crappy BIOS? What is the *actual* reason
> for wanting to disable x2apic?

Just a guess, but the OEM probably hasn't updated their SMI handlers to
understand x2apic yet and won't before the product ships because some
other OS doesn't bother to use x2apic.  We can still enable interrupt
remapping w/o x2apic though, so I'm curious what other irq injection
tricks you're referring to.  Thanks,

Alex


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

* Re: [PATCH v2] x86, vt-d: enable x2apic opt out
  2011-05-16 20:32   ` Alex Williamson
@ 2011-05-17  9:49     ` Woodhouse, David
  2011-05-17 12:50       ` Alex Williamson
  2011-05-18 22:58     ` Woodhouse, David
  1 sibling, 1 reply; 12+ messages in thread
From: Woodhouse, David @ 2011-05-17  9:49 UTC (permalink / raw)
  To: Alex Williamson
  Cc: Song, Youquan, linux-kernel, akpm, mingo, tglx, hpa, hpa, Kay,
	Allen M, Siddha, Suresh B, Liu, Kent, Youquan Song

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

On Mon, 2011-05-16 at 21:32 +0100, Alex Williamson wrote:
> Just a guess, but the OEM probably hasn't updated their SMI handlers to
> understand x2apic yet and won't before the product ships because some
> other OS doesn't bother to use x2apic. 

Yay, crappy closed-source BIOS holds back the platform yet again.

>  We can still enable interrupt remapping w/o x2apic though, so I'm
> curious what other irq injection tricks you're referring to.  Thanks,

http://www.invisiblethingslab.com/resources/2011/Software%20Attacks%20on%20Intel%20VT-d.pdf

-- 
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] 12+ messages in thread

* Re: [PATCH v2] x86, vt-d: enable x2apic opt out
  2011-05-17  9:49     ` Woodhouse, David
@ 2011-05-17 12:50       ` Alex Williamson
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Williamson @ 2011-05-17 12:50 UTC (permalink / raw)
  To: Woodhouse, David
  Cc: Song, Youquan, linux-kernel, akpm, mingo, tglx, hpa, hpa, Kay,
	Allen M, Siddha, Suresh B, Liu, Kent, Youquan Song

On Tue, 2011-05-17 at 10:49 +0100, Woodhouse, David wrote:
> On Mon, 2011-05-16 at 21:32 +0100, Alex Williamson wrote:
> > Just a guess, but the OEM probably hasn't updated their SMI handlers to
> > understand x2apic yet and won't before the product ships because some
> > other OS doesn't bother to use x2apic. 
> 
> Yay, crappy closed-source BIOS holds back the platform yet again.
> 
> >  We can still enable interrupt remapping w/o x2apic though, so I'm
> > curious what other irq injection tricks you're referring to.  Thanks,
> 
> http://www.invisiblethingslab.com/resources/2011/Software%20Attacks%20on%20Intel%20VT-d.pdf

Yep, I just read that, and while x2apic guarantees that we must turn on
the interrupt remapper and block compatibility format, lack of x2apic
doesn't preclude us from doing the same.  Right?  Thanks,

Alex


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

* Re: [PATCH v2] x86, vt-d: enable x2apic opt out
  2011-05-16 20:32   ` Alex Williamson
  2011-05-17  9:49     ` Woodhouse, David
@ 2011-05-18 22:58     ` Woodhouse, David
  2011-05-19  5:43       ` Alex Williamson
  1 sibling, 1 reply; 12+ messages in thread
From: Woodhouse, David @ 2011-05-18 22:58 UTC (permalink / raw)
  To: Alex Williamson
  Cc: Song, Youquan, linux-kernel, akpm, mingo, tglx, hpa, hpa, Kay,
	Allen M, Siddha, Suresh B, Liu, Kent, Youquan Song

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

On Mon, 2011-05-16 at 21:32 +0100, Alex Williamson wrote:
> 
> > Is this just a workaround for a crappy BIOS? What is the *actual* reason
> > for wanting to disable x2apic?
> 
> Just a guess, but the OEM probably hasn't updated their SMI handlers to
> understand x2apic yet and won't before the product ships because some
> other OS doesn't bother to use x2apic. 

But I think we'll still use x2apic if interrupt remapping isn't enabled
(for example if VT-d is disabled in the BIOS and the whole DMAR table is
absent), or if we're booted with 'iommu=off' or indeed if the
distribution vendor has hacked the kernel so that iommu=off is the
*default*, because they've seen so many broken BIOSes.

AFAICT although CONFIG_X86_X2APIC depends on CONFIG_INTR_REMAP, we can
still enable x2apic at runtime if interrupt remapping is not operating?
We end up hitting this code in enable_IR_x2apic():

	if (!ret) {
		/* IR is required if there is APIC ID > 255 even when running
		 * under KVM
		 */
		if (max_physical_apicid > 255 ||
		    !hypervisor_x2apic_available())
			goto nox2apic;
		/*
		 * without IR all CPUs can be addressed by IOAPIC/MSI
		 * only in physical mode
		 */
		x2apic_force_phys();
	}

So if that *is* the reason, this doesn't seem like a viable solution.

-- 
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] 12+ messages in thread

* Re: [PATCH v2] x86, vt-d: enable x2apic opt out
  2011-05-18 22:58     ` Woodhouse, David
@ 2011-05-19  5:43       ` Alex Williamson
  0 siblings, 0 replies; 12+ messages in thread
From: Alex Williamson @ 2011-05-19  5:43 UTC (permalink / raw)
  To: Woodhouse, David
  Cc: Song, Youquan, linux-kernel, akpm, mingo, tglx, hpa, hpa, Kay,
	Allen M, Siddha, Suresh B, Liu, Kent, Youquan Song

On Wed, 2011-05-18 at 23:58 +0100, Woodhouse, David wrote:
> On Mon, 2011-05-16 at 21:32 +0100, Alex Williamson wrote:
> > 
> > > Is this just a workaround for a crappy BIOS? What is the *actual* reason
> > > for wanting to disable x2apic?
> > 
> > Just a guess, but the OEM probably hasn't updated their SMI handlers to
> > understand x2apic yet and won't before the product ships because some
> > other OS doesn't bother to use x2apic. 
> 
> But I think we'll still use x2apic if interrupt remapping isn't enabled
> (for example if VT-d is disabled in the BIOS and the whole DMAR table is
> absent), or if we're booted with 'iommu=off' or indeed if the
> distribution vendor has hacked the kernel so that iommu=off is the
> *default*, because they've seen so many broken BIOSes.
> 
> AFAICT although CONFIG_X86_X2APIC depends on CONFIG_INTR_REMAP, we can
> still enable x2apic at runtime if interrupt remapping is not operating?
> We end up hitting this code in enable_IR_x2apic():
> 
> 	if (!ret) {
> 		/* IR is required if there is APIC ID > 255 even when running
> 		 * under KVM
> 		 */
> 		if (max_physical_apicid > 255 ||
> 		    !hypervisor_x2apic_available())
> 			goto nox2apic;
> 		/*
> 		 * without IR all CPUs can be addressed by IOAPIC/MSI
> 		 * only in physical mode
> 		 */
> 		x2apic_force_phys();
> 	}
> 
> So if that *is* the reason, this doesn't seem like a viable solution.

I've always been under the impression that interrupt remapping is
required to enable x2apic because it enables IOxAPICs to work in that
mode.  Particularly this excerpt from the x2apic spec (sec 1.2):

        Similarly no modifications are required to the IOxAPIC. The
        routing of interrupts from these devices in x2APIC mode
        leverages the interrupt remapping architecture specified in the
        Intel Virtualization Technology for Directed I/O, Rev 1.1
        specification.

KVM wants to enable x2apic because it's easier to virtualize and
provides better performance than xapic.  We won't be taking the above
x2apic physical mode path on real hardware though, so I'm not sure that
code snippet is relevant here.

AIUI, platforms that require x2apic (>255 APIC IDs) probably aren't
going to have an option to disable VT-d in the BIOS.  If such a platform
hands off in xapic mode with iommu=off, you stay in xapic mode and don't
bring all the processors online.  If it hands off in x2apic mode with
iommu=off, hopefully we can switch back to xapic mode and lose
processors, but ISTR the x2apic->xapic transition isn't particularly
easy, so you may just be screwed.  Thanks,

Alex


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

end of thread, other threads:[~2011-05-19  5:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-14  7:06 [PATCH v2] x86, vt-d: enable x2apic opt out Youquan Song
2011-05-11 15:07 ` Woodhouse, David
2011-05-12  3:39   ` Youquan Song
2011-05-11 15:39     ` David Woodhouse
2011-05-11 22:38       ` Valdis.Kletnieks
2011-05-16 20:32   ` Alex Williamson
2011-05-17  9:49     ` Woodhouse, David
2011-05-17 12:50       ` Alex Williamson
2011-05-18 22:58     ` Woodhouse, David
2011-05-19  5:43       ` Alex Williamson
2011-05-11 17:21 ` Woodhouse, David
2011-05-12  1:52 ` Youquan Song

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.