[PATCH v2] x86, vt-d: enable x2apic opt out

[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 <youqua...@intel.com>
Reviewed-by: Kay, Allen M <allen...@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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majo...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

[Youquan Song]

> 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

[David Woodhouse]

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

[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

[Valdis.K...@vt.edu]

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...

[Alex Williamson]

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

[Alex Williamson]

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 Williamson]

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,

[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 or ignore the request
by adding kernel option.

Signed-off-by: Youquan Song <youqua...@intel.com>
Reviewed-by: Kay, Allen M <allen...@intel.com>

Acked-by: David Woodhouse <David.W...@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 c603ef7..bc6ec9e 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 a0c46f0..94d1aed 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 f92a8e5..9a86362 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1462,11 +1462,11 @@ void __init enable_IR_x2apic(void)
{
unsigned long flags;
struct IO_APIC_route_entry **ioapic_entries;

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

@@ -1478,6 +1478,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;
}

@@ -1504,7 +1505,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;

@@ -1527,8 +1529,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..c4fe2f9 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

--

[Valdis.K...@vt.edu]

On Mon, 23 May 2011 22:32:28 +0800, Youquan Song said:

> + no_x2apic_optout [Default Off]
> + With this option BIOS x2APIC opt-out request will be
> + ignored.

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

If we're doing a WARN level here, what are the downsides of just automagically
forcing it rather than making them use a kernel parameter and reboot? Will
some systems fail to boot because the BIOS was in fact right in requesting
hat x2apic be turned off?

[Youquan Song]

> If we're doing a WARN level here, what are the downsides of just automagically
> forcing it rather than making them use a kernel parameter and reboot?

As we have discussed before, x2apci opt out feature is requested from OEM that
they want to firmware tell OS to opt out x2apic when the platform,
hardware or BIOS is not ready to support x2apic. So we can not reboot or
force to use kernel parameter.
But as David Woodhouse address that there are some researches about
disabling x2apic will be vulnerable to irq-injection attack, so we need
warn user about it. If user is sensitive to it, they also has alternative
choice to ignore BIOS request by adding no_x2apic_optout kernel parameter.

> Will some systems fail to boot because the BIOS was in fact right in requesting

No. we do not want system boot fail for BIOS requesting it.

Thanks
-Youquan

[Valdis.K...@vt.edu]

On Tue, 24 May 2011 11:36:14 EDT, Youquan Song said:
> > If we're doing a WARN level here, what are the downsides of just automagically
> > forcing it rather than making them use a kernel parameter and reboot?
>
> As we have discussed before, x2apci opt out feature is requested from OEM that
> they want to firmware tell OS to opt out x2apic when the platform,
> hardware or BIOS is not ready to support x2apic. So we can not reboot or
> force to use kernel parameter.

Do we want an actual WARN there, complete with stack traceback and all?

Or did you intend a pr_warn or printk(KERN_WARNING or similar?

[David Woodhouse]

On Tue, 2011-05-24 at 05:18 +0100, Valdis.K...@vt.edu wrote:
> Do we want an actual WARN there, complete with stack traceback and
> all?
>
> Or did you intend a pr_warn or printk(KERN_WARNING or similar?

The traceback has been very useful in other BIOS issues, because it gets
tracked and counted on kerneloops.org along with platform
identification. So we have been able to track down the major offenders
of some of the most egregious BIOS stupidities and make *some* progress
on getting them to improve the untested dross they habitually turn out.

On *this* occasion perhaps a printk might suffice. After all, this
X2APIC_OPT_OUT was invented specifically as a way to allow the BIOS
engineers to get away without fixing their bugs. But then again, it
would also be useful to track how many people are doing it, and I cannot
think of a *legitimate* reason for a BIOS having to set it. So on
balance I'm happier with the WARN().

--
dwmw2

[Valdis.K...@vt.edu]

On Tue, 24 May 2011 08:38:47 BST, David Woodhouse said:
> On Tue, 2011-05-24 at 05:18 +0100, Valdis.K...@vt.edu wrote:
> > Do we want an actual WARN there, complete with stack traceback and
> > all?
> >
> > Or did you intend a pr_warn or printk(KERN_WARNING or similar?
>
> The traceback has been very useful in other BIOS issues, because it gets
> tracked and counted on kerneloops.org along with platform
> identification. So we have been able to track down the major offenders
> of some of the most egregious BIOS stupidities and make *some* progress
> on getting them to improve the untested dross they habitually turn out.

Oh OK... that probably is a good idea then.

[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 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 <youqua...@intel.com>
Reviewed-by: Kay, Allen M <allen...@intel.com>

Signed-off-by: David Woodhouse <David.W...@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

index b862dac..3087ccb 100644

--

[Ingo Molnar]

* Youquan Song <youqua...@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

[Thomas Gleixner]

On Wed, 25 May 2011, Ingo Molnar wrote:

>
> * Youquan Song <youqua...@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

[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