[patch 1/5] x2apic: fix reserved APIC register accesses in print_local_APIC()

[patch 1/5] x2apic: fix reserved APIC register accesses in print_local_APIC()

[Suresh Siddha]

[-- Attachment #1: fix_reserved_apic_reg_access.patch --]
[-- Type: text/plain, Size: 2727 bytes --]

From: Yinghai Lu <yhlu.kernel@gmail.com>
Subject: x2apic: fix reserved APIC register accesses in print_local_APIC()

APIC_ARBPRI is a reserved register for XAPIC and beyond.
APIC_RRR is a reserved register except for 82489DX, APIC for Pentium processors.
APIC_EOI is a write only register.
APIC_DFR is reserved in x2apic mode.

Access to these registers in x2apic will result in #GP fault. Fix these
apic register accesses.

Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Cc: Maciej W. Rozycki <macro@linux-mips.org>
---

Index: tip/arch/x86/kernel/io_apic.c
===================================================================
--- tip.orig/arch/x86/kernel/io_apic.c	2008-09-03 14:02:56.000000000 -0700
+++ tip/arch/x86/kernel/io_apic.c	2008-09-03 14:51:24.000000000 -0700
@@ -1751,21 +1751,30 @@
 	printk(KERN_DEBUG "... APIC TASKPRI: %08x (%02x)\n", v, v & APIC_TPRI_MASK);
 
 	if (APIC_INTEGRATED(ver)) {                     /* !82489DX */
-		v = apic_read(APIC_ARBPRI);
-		printk(KERN_DEBUG "... APIC ARBPRI: %08x (%02x)\n", v,
-			v & APIC_ARBPRI_MASK);
+		if (!APIC_XAPIC(ver)) {
+			v = apic_read(APIC_ARBPRI);
+			printk(KERN_DEBUG "... APIC ARBPRI: %08x (%02x)\n", v,
+			       v & APIC_ARBPRI_MASK);
+		}
 		v = apic_read(APIC_PROCPRI);
 		printk(KERN_DEBUG "... APIC PROCPRI: %08x\n", v);
 	}
 
-	v = apic_read(APIC_EOI);
-	printk(KERN_DEBUG "... APIC EOI: %08x\n", v);
-	v = apic_read(APIC_RRR);
-	printk(KERN_DEBUG "... APIC RRR: %08x\n", v);
+	/*
+	 * Remote read supported only in the 82489DX and local APIC for
+	 * Pentium processors.
+	 */
+	if (!APIC_INTEGRATED(ver) || maxlvt == 3) {
+		v = apic_read(APIC_RRR);
+		printk(KERN_DEBUG "... APIC RRR: %08x\n", v);
+	}
+
 	v = apic_read(APIC_LDR);
 	printk(KERN_DEBUG "... APIC LDR: %08x\n", v);
-	v = apic_read(APIC_DFR);
-	printk(KERN_DEBUG "... APIC DFR: %08x\n", v);
+	if (!x2apic_enabled()) {
+		v = apic_read(APIC_DFR);
+		printk(KERN_DEBUG "... APIC DFR: %08x\n", v);
+	}
 	v = apic_read(APIC_SPIV);
 	printk(KERN_DEBUG "... APIC SPIV: %08x\n", v);
 
Index: tip/include/asm-x86/apic.h
===================================================================
--- tip.orig/include/asm-x86/apic.h	2008-09-03 14:02:56.000000000 -0700
+++ tip/include/asm-x86/apic.h	2008-09-03 14:43:02.000000000 -0700
@@ -98,6 +98,20 @@
 extern void enable_x2apic(void);
 extern void enable_IR_x2apic(void);
 extern void x2apic_icr_write(u32 low, u32 id);
+static inline int x2apic_enabled(void)
+{
+	int msr, msr2;
+
+	if (!cpu_has_x2apic)
+		return 0;
+
+	rdmsr(MSR_IA32_APICBASE, msr, msr2);
+	if (msr & X2APIC_ENABLE)
+		return 1;
+	return 0;
+}
+#else
+#define x2apic_enabled()	0
 #endif
 
 struct apic_ops {

-- 

[patch 2/5] dmar: fix using early fixmap mapping for DMAR table parsing

[Suresh Siddha]

[-- Attachment #1: dmar_fix_fixmap_mapping.patch --]
[-- Type: text/plain, Size: 3011 bytes --]

From: Yinghai Lu <yhlu.kernel@gmail.com>
Subject: dmar: fix using early fixmap mapping for DMAR table parsing

Very early detection of the DMAR tables will setup fixmap mapping. For
parsing these tables later (while enabling dma and/or interrupt remapping),
early fixmap mapping shouldn't be used. Fix it by calling table detection
routines again, which will call generic apci_get_table() for setting up
the correct mapping.

Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
---

Index: tip/drivers/pci/dmar.c
===================================================================
--- tip.orig/drivers/pci/dmar.c	2008-09-02 16:04:04.000000000 -0700
+++ tip/drivers/pci/dmar.c	2008-09-02 16:41:31.000000000 -0700
@@ -289,6 +289,24 @@
 	}
 }
 
+/**
+ * dmar_table_detect - checks to see if the platform supports DMAR devices
+ */
+static int __init dmar_table_detect(void)
+{
+	acpi_status status = AE_OK;
+
+	/* if we could find DMAR table, then there are DMAR devices */
+	status = acpi_get_table(ACPI_SIG_DMAR, 0,
+				(struct acpi_table_header **)&dmar_tbl);
+
+	if (ACPI_SUCCESS(status) && !dmar_tbl) {
+		printk (KERN_WARNING PREFIX "Unable to map DMAR\n");
+		status = AE_NOT_FOUND;
+	}
+
+	return (ACPI_SUCCESS(status) ? 1 : 0);
+}
 
 /**
  * parse_dmar_table - parses the DMA reporting table
@@ -300,6 +318,12 @@
 	struct acpi_dmar_header *entry_header;
 	int ret = 0;
 
+	/*
+	 * Do it again, earlier dmar_tbl mapping could be mapped with
+	 * fixed map.
+	 */
+	dmar_table_detect();
+
 	dmar = (struct acpi_table_dmar *)dmar_tbl;
 	if (!dmar)
 		return -ENODEV;
@@ -430,30 +454,11 @@
 	return 0;
 }
 
-/**
- * early_dmar_detect - checks to see if the platform supports DMAR devices
- */
-int __init early_dmar_detect(void)
-{
-	acpi_status status = AE_OK;
-
-	/* if we could find DMAR table, then there are DMAR devices */
-	status = acpi_get_table(ACPI_SIG_DMAR, 0,
-				(struct acpi_table_header **)&dmar_tbl);
-
-	if (ACPI_SUCCESS(status) && !dmar_tbl) {
-		printk (KERN_WARNING PREFIX "Unable to map DMAR\n");
-		status = AE_NOT_FOUND;
-	}
-
-	return (ACPI_SUCCESS(status) ? 1 : 0);
-}
-
 void __init detect_intel_iommu(void)
 {
 	int ret;
 
-	ret = early_dmar_detect();
+	ret = dmar_table_detect();
 
 #ifdef CONFIG_DMAR
 	{
@@ -479,14 +484,16 @@
 			       " x2apic support\n");
 
 			dmar_disabled = 1;
-			return;
+			goto end;
 		}
 
 		if (ret && !no_iommu && !iommu_detected && !swiotlb &&
 		    !dmar_disabled)
 			iommu_detected = 1;
 	}
+end:
 #endif
+	dmar_tbl = NULL;
 }
 
 
Index: tip/include/linux/dmar.h
===================================================================
--- tip.orig/include/linux/dmar.h	2008-09-02 16:04:04.000000000 -0700
+++ tip/include/linux/dmar.h	2008-09-02 16:40:40.000000000 -0700
@@ -45,7 +45,6 @@
 	list_for_each_entry(drhd, &dmar_drhd_units, list)
 
 extern int dmar_table_init(void);
-extern int early_dmar_detect(void);
 extern int dmar_dev_scope_init(void);
 
 /* Intel IOMMU detection */

-- 

[patch 3/5] dmar: initialize the return value in dmar_parse_dev()

[Suresh Siddha]

[-- Attachment #1: init_ret_value_dmar_parse_dev.patch --]
[-- Type: text/plain, Size: 676 bytes --]

From: Yinghai Lu <yhlu.kernel@gmail.com>
Subject: dmar: initialize the return value in dmar_parse_dev()

initialize the return value in dmar_parse_dev()

Signed-off-by: Yinghai Lu <yhlu.kernel@gmail.com>
Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
---

Index: tip/drivers/pci/dmar.c
===================================================================
--- tip.orig/drivers/pci/dmar.c	2008-09-03 11:48:21.000000000 -0700
+++ tip/drivers/pci/dmar.c	2008-09-03 11:49:34.000000000 -0700
@@ -193,7 +193,7 @@
 {
 	struct acpi_dmar_hardware_unit *drhd;
 	static int include_all;
-	int ret;
+	int ret = 0;
 
 	drhd = (struct acpi_dmar_hardware_unit *) dmaru->hdr;
 

-- 

[patch 4/5] dmar: use list_for_each_entry_safe() in dmar_dev_scope_init()

[Suresh Siddha]

[-- Attachment #1: safe_traversal_in_dmar.patch --]
[-- Type: text/plain, Size: 1270 bytes --]

From: Suresh Siddha <suresh.b.siddha@intel.com>
Subject: dmar: use list_for_each_entry_safe() in dmar_dev_scope_init()

In dmar_dev_scope_init(), functions called under for_each_drhd_unit()/
for_each_rmrr_units() can delete the list entry under some error conditions.

So we should use list_for_each_entry_safe() for safe traversal.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Acked-by: Yinghai Lu <yhlu.kernel@gmail.com>
---

Index: tip/drivers/pci/dmar.c
===================================================================
--- tip.orig/drivers/pci/dmar.c	2008-09-03 14:43:02.000000000 -0700
+++ tip/drivers/pci/dmar.c	2008-09-03 14:50:15.000000000 -0700
@@ -397,10 +397,10 @@
 
 int __init dmar_dev_scope_init(void)
 {
-	struct dmar_drhd_unit *drhd;
+	struct dmar_drhd_unit *drhd, *drhd_n;
 	int ret = -ENODEV;
 
-	for_each_drhd_unit(drhd) {
+	list_for_each_entry_safe(drhd, drhd_n, &dmar_drhd_units, list) {
 		ret = dmar_parse_dev(drhd);
 		if (ret)
 			return ret;
@@ -408,8 +408,8 @@
 
 #ifdef CONFIG_DMAR
 	{
-		struct dmar_rmrr_unit *rmrr;
-		for_each_rmrr_units(rmrr) {
+		struct dmar_rmrr_unit *rmrr, *rmrr_n;
+		list_for_each_entry_safe(rmrr, rmrr_n, &dmar_rmrr_units, list) {
 			ret = rmrr_parse_dev(rmrr);
 			if (ret)
 				return ret;

-- 

[patch 5/5] dmar: fix dmar_parse_dev() devices_cnt error condition check

[Suresh Siddha]

[-- Attachment #1: fix_dmar_parse_dev_error_condition_check.patch --]
[-- Type: text/plain, Size: 964 bytes --]

From: Suresh Siddha <suresh.b.siddha@intel.com>
Subject: dmar: fix dmar_parse_dev() devices_cnt error condition check

It is possible that,
instead of PCI endpoint/sub-hierarchy structures, only IO-APIC/HPET
devices may be reported under device scope structures. Fix the devices_cnt
error check, which cares about only PCI structures and removes the
dma-remapping unit structure (dmaru) when the devices_cnt is zero
and include_all flag is not set.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
Acked-by: Yinghai Lu <yhlu.kernel@gmail.com>
---

Index: tip/drivers/pci/dmar.c
===================================================================
--- tip.orig/drivers/pci/dmar.c	2008-09-03 13:57:28.000000000 -0700
+++ tip/drivers/pci/dmar.c	2008-09-03 13:59:12.000000000 -0700
@@ -212,7 +212,7 @@
 		include_all = 1;
 	}
 
-	if (ret || (dmaru->devices_cnt == 0 && !dmaru->include_all)) {
+	if (ret) {
 		list_del(&dmaru->list);
 		kfree(dmaru);
 	}

-- 

Re: [patch 1/5] x2apic: fix reserved APIC register accesses in print_local_APIC()

[Ingo Molnar]

applied the following patches tip/irq/sparseirq:

 0f48966: dmar: fix dmar_parse_dev() devices_cnt error condition check
 2283240: dmar: use list_for_each_entry_safe() in dmar_dev_scope_init()
 3f1fdb3: dmar: initialize the return value in dmar_parse_dev()
 f12c73e: dmar: fix using early fixmap mapping for DMAR table parsing
 2c72d93: x2apic: fix reserved APIC register accesses in print_local_APIC()

thanks Suresh!

	Ingo

Re: [patch 1/5] x2apic: fix reserved APIC register accesses in print_local_APIC()

[Suresh Siddha]

On Thu, Sep 04, 2008 at 04:06:31AM -0700, Ingo Molnar wrote:
> 
> applied the following patches tip/irq/sparseirq:

Ingo, Shouldn't this go to tip/x86/x2apic ? Thanks.

>  0f48966: dmar: fix dmar_parse_dev() devices_cnt error condition check
>  2283240: dmar: use list_for_each_entry_safe() in dmar_dev_scope_init()
>  3f1fdb3: dmar: initialize the return value in dmar_parse_dev()
>  f12c73e: dmar: fix using early fixmap mapping for DMAR table parsing
>  2c72d93: x2apic: fix reserved APIC register accesses in print_local_APIC()
> 
> thanks Suresh!
> 
>         Ingo

Re: [patch 1/5] x2apic: fix reserved APIC register accesses in print_local_APIC()

[Ingo Molnar]

* Suresh Siddha <suresh.b.siddha@intel.com> wrote:

> On Thu, Sep 04, 2008 at 04:06:31AM -0700, Ingo Molnar wrote:
> > 
> > applied the following patches tip/irq/sparseirq:
> 
> Ingo, Shouldn't this go to tip/x86/x2apic ? Thanks.

normally yes - but this is a special case: there's existing overlap with 
other DMAR changes in irq/sparseirq (intr-remap and ioapic unification 
changes), so these followups have to go there too. Also, since they 
change the generic PCI code it's better they live in a generic topic to 
begin with. The full stack of pending changes is:

0f48966: dmar: fix dmar_parse_dev() devices_cnt error condition check
2283240: dmar: use list_for_each_entry_safe() in dmar_dev_scope_init()
3f1fdb3: dmar: initialize the return value in dmar_parse_dev()
f12c73e: dmar: fix using early fixmap mapping for DMAR table parsing
1cb1158: x64, x2apic/intr-remap: disable DMA-remapping if Interrupt-remapping is detected (temporary quirk)
2ae2101: x64, x2apic/intr-remap: Interrupt remapping infrastructure
fe962e9: x64, x2apic/intr-remap: Queued invalidation infrastructure (part of VT-d)
ad3ad3f: x64, x2apic/intr-remap: parse ioapic scope under vt-d structures
2d6b5f8: x64, x2apic/intr-remap: Fix the need for RMRR in the DMA-remapping detection
aaa9d1d: x64, x2apic/intr-remap: use CONFIG_DMAR for DMA-remapping specific code
1886e8a: x64, x2apic/intr-remap: code re-structuring, to be used by both DMA and Interrupt remapping
c42d9f3: x64, x2apic/intr-remap: fix the need for sequential array allocation of iommus
e61d98d: x64, x2apic/intr-remap: Intel vt-d, IOMMU code reorganization

Jesse, do you have any objections to this approach?

	Ingo

Re: [patch 1/5] x2apic: fix reserved APIC register accesses in print_local_APIC()

[Jesse Barnes]

On Friday, September 05, 2008 1:10 am Ingo Molnar wrote:
> * Suresh Siddha <suresh.b.siddha@intel.com> wrote:
> > On Thu, Sep 04, 2008 at 04:06:31AM -0700, Ingo Molnar wrote:
> > > applied the following patches tip/irq/sparseirq:
> >
> > Ingo, Shouldn't this go to tip/x86/x2apic ? Thanks.
>
> normally yes - but this is a special case: there's existing overlap with
> other DMAR changes in irq/sparseirq (intr-remap and ioapic unification
> changes), so these followups have to go there too. Also, since they
> change the generic PCI code it's better they live in a generic topic to
> begin with. The full stack of pending changes is:
>
> 0f48966: dmar: fix dmar_parse_dev() devices_cnt error condition check
> 2283240: dmar: use list_for_each_entry_safe() in dmar_dev_scope_init()
> 3f1fdb3: dmar: initialize the return value in dmar_parse_dev()
> f12c73e: dmar: fix using early fixmap mapping for DMAR table parsing
> 1cb1158: x64, x2apic/intr-remap: disable DMA-remapping if
> Interrupt-remapping is detected (temporary quirk) 2ae2101: x64,
> x2apic/intr-remap: Interrupt remapping infrastructure fe962e9: x64,
> x2apic/intr-remap: Queued invalidation infrastructure (part of VT-d)
> ad3ad3f: x64, x2apic/intr-remap: parse ioapic scope under vt-d structures
> 2d6b5f8: x64, x2apic/intr-remap: Fix the need for RMRR in the DMA-remapping
> detection aaa9d1d: x64, x2apic/intr-remap: use CONFIG_DMAR for
> DMA-remapping specific code 1886e8a: x64, x2apic/intr-remap: code
> re-structuring, to be used by both DMA and Interrupt remapping c42d9f3:
> x64, x2apic/intr-remap: fix the need for sequential array allocation of
> iommus e61d98d: x64, x2apic/intr-remap: Intel vt-d, IOMMU code
> reorganization
>
> Jesse, do you have any objections to this approach?

Nope, no problems.  I think I have some pending patches that may have 
conflicts with the above, but I'll take care of that when I get back to the 
US (I think Sunday or Monday will be a massive patch review/merge/comment 
day).

Thanks,
Jesse