All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH -v8] x86: Disable x2apic if nox2apic is specified or intr-remap can not be enabled
@ 2011-12-17  0:59 Yinghai Lu
  2011-12-17  2:07 ` Suresh Siddha
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Yinghai Lu @ 2011-12-17  0:59 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, H. Peter Anvin
  Cc: Suresh Siddha, linux-kernel, Yinghai Lu

For
1. x2apic preenabled system
2. first kernel have x2apic enabled, and try to kexec second kernel with "nox2apic"

Will put back cpu with apic id < 255 into xapic mode, instead of panic.

-v2: use variable x2apic_disabled instead of variable nox2apic, Suggested by Thomas
     update x2apic_supported with x2apic_disabled, Suggested by Thomas

-v3: add checking for boot cpu apic id > 255. in that case will just panic
     --- pointed out by Suresh.

-v4: according to Ingo, for x2apic pre-enabled system, if intr-remap can not
	be enabled, try to disable x2apic instead of panic and request to
	specify nox2apic for next boot.

-v5: fix the lapic mapping for falling path.

-v6: add warning when check_timer fails in strange case when BIOS enable x2apic
     and intr_remapping but does not provide dmar table. requested by Suresh.

-v7: fix compiling with x2apic_preenabled

-v8: fix compiling with x2apic_disabled assigned with !X86_X2APIC

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Tested-by: "Berck E. Nash" <flyboy@gmail.com>

---
 arch/x86/include/asm/apic.h         |    9 +++-
 arch/x86/include/asm/apicdef.h      |    1 
 arch/x86/include/asm/processor.h    |    1 
 arch/x86/kernel/acpi/boot.c         |   10 +++-
 arch/x86/kernel/apic/apic.c         |   79 ++++++++++++++++++++++++++----------
 arch/x86/kernel/apic/apic_flat_64.c |    7 ++-
 arch/x86/kernel/apic/io_apic.c      |    4 +
 arch/x86/kernel/cpu/topology.c      |   21 +++++++++
 arch/x86/mm/srat.c                  |    7 ++-
 9 files changed, 112 insertions(+), 27 deletions(-)

Index: linux-2.6/arch/x86/include/asm/apic.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/apic.h
+++ linux-2.6/arch/x86/include/asm/apic.h
@@ -176,6 +176,8 @@ static inline u64 native_x2apic_icr_read
 }
 
 extern int x2apic_phys;
+extern int x2apic_preenabled;
+extern int x2apic_disabled;
 extern void check_x2apic(void);
 extern void enable_x2apic(void);
 extern void x2apic_icr_write(u32 low, u32 id);
@@ -183,7 +185,7 @@ static inline int x2apic_enabled(void)
 {
 	u64 msr;
 
-	if (!cpu_has_x2apic)
+	if (!cpu_has_x2apic || x2apic_disabled)
 		return 0;
 
 	rdmsrl(MSR_IA32_APICBASE, msr);
@@ -192,12 +194,15 @@ static inline int x2apic_enabled(void)
 	return 0;
 }
 
-#define x2apic_supported()	(cpu_has_x2apic)
+#define x2apic_supported()	(cpu_has_x2apic && !x2apic_disabled)
 static inline void x2apic_force_phys(void)
 {
 	x2apic_phys = 1;
 }
 #else
+static inline void disable_x2apic(void)
+{
+}
 static inline void check_x2apic(void)
 {
 }
@@ -214,6 +219,7 @@ static inline void x2apic_force_phys(voi
 
 #define	x2apic_preenabled 0
 #define	x2apic_supported()	0
+#define	x2apic_disabled	1
 #endif
 
 extern void enable_IR_x2apic(void);
Index: linux-2.6/arch/x86/kernel/acpi/boot.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/acpi/boot.c
+++ linux-2.6/arch/x86/kernel/acpi/boot.c
@@ -219,6 +219,8 @@ static int __init
 acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
 {
 	struct acpi_madt_local_x2apic *processor = NULL;
+	int apic_id;
+	u8 enabled;
 
 	processor = (struct acpi_madt_local_x2apic *)header;
 
@@ -227,6 +229,8 @@ acpi_parse_x2apic(struct acpi_subtable_h
 
 	acpi_table_print_madt_entry(header);
 
+	apic_id = processor->local_apic_id;
+	enabled = processor->lapic_flags & ACPI_MADT_ENABLED;
 #ifdef CONFIG_X86_X2APIC
 	/*
 	 * We need to register disabled CPU as well to permit
@@ -235,8 +239,10 @@ acpi_parse_x2apic(struct acpi_subtable_h
 	 * to not preallocating memory for all NR_CPUS
 	 * when we use CPU hotplug.
 	 */
-	acpi_register_lapic(processor->local_apic_id,	/* APIC ID */
-			    processor->lapic_flags & ACPI_MADT_ENABLED);
+	if (x2apic_disabled && (apic_id >= 0xff) && enabled)
+		printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
+	else
+		acpi_register_lapic(apic_id, enabled);
 #else
 	printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
 #endif
Index: linux-2.6/arch/x86/kernel/apic/apic.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/apic.c
+++ linux-2.6/arch/x86/kernel/apic/apic.c
@@ -151,16 +151,15 @@ __setup("apicpmtimer", setup_apicpmtimer
 int x2apic_mode;
 #ifdef CONFIG_X86_X2APIC
 /* x2apic enabled before OS handover */
-static int x2apic_preenabled;
+int x2apic_preenabled;
+int x2apic_disabled;
 static __init int setup_nox2apic(char *str)
 {
-	if (x2apic_enabled()) {
-		pr_warning("Bios already enabled x2apic, "
-			   "can't enforce nox2apic");
-		return 0;
-	}
+	if (x2apic_enabled())
+		pr_warning("Bios already enabled x2apic, will disable it");
+
+	x2apic_disabled = 1;
 
-	setup_clear_cpu_cap(X86_FEATURE_X2APIC);
 	return 0;
 }
 early_param("nox2apic", setup_nox2apic);
@@ -1437,8 +1436,40 @@ void __init bsp_end_local_APIC_setup(voi
 }
 
 #ifdef CONFIG_X86_X2APIC
+
+static void disable_x2apic(void)
+{
+	int msr, msr2;
+
+	if (!cpu_has_x2apic)
+		return;
+
+	rdmsr(MSR_IA32_APICBASE, msr, msr2);
+	if (msr & X2APIC_ENABLE) {
+		u32 x2apic_id = x2apic_cpuid_initial_apicid();
+
+		if (x2apic_id > 255)
+			panic("Can not disable x2apic, id: %08x\n", x2apic_id);
+
+		pr_info("Disabling x2apic\n");
+		/*
+		 * Need to disable xapic and x2apic at the same time at first
+		 *  then enable xapic
+		 */
+		wrmsr(MSR_IA32_APICBASE, msr & ~(X2APIC_ENABLE | XAPIC_ENABLE),
+			 0);
+		wrmsr(MSR_IA32_APICBASE, msr & ~X2APIC_ENABLE, 0);
+
+		x2apic_disabled = 1;
+	}
+}
 void check_x2apic(void)
 {
+	if (x2apic_disabled) {
+		disable_x2apic();
+		return;
+	}
+
 	if (x2apic_enabled()) {
 		pr_info("x2apic enabled by BIOS, switching to x2apic ops\n");
 		x2apic_preenabled = x2apic_mode = 1;
@@ -1449,6 +1480,11 @@ void enable_x2apic(void)
 {
 	int msr, msr2;
 
+	if (x2apic_disabled) {
+		disable_x2apic();
+		return;
+	}
+
 	if (!x2apic_mode)
 		return;
 
@@ -1499,7 +1535,7 @@ void __init enable_IR_x2apic(void)
 	ret = save_ioapic_entries();
 	if (ret) {
 		pr_info("Saving IO-APIC state failed: %d\n", ret);
-		goto out;
+		return;
 	}
 
 	local_irq_save(flags);
@@ -1511,13 +1547,23 @@ void __init enable_IR_x2apic(void)
 	else
 		ret = enable_IR();
 
+	if (x2apic_disabled)
+		goto nox2apic;
+
 	if (ret < 0) {
 		/* IR is required if there is APIC ID > 255 even when running
 		 * under KVM
 		 */
 		if (max_physical_apicid > 255 ||
-		    !hypervisor_x2apic_available())
+		    !hypervisor_x2apic_available()) {
+			if (x2apic_preenabled) {
+				disable_x2apic();
+				x2apic_mode = 0;
+				/* need to map lapic address */
+				register_lapic_address(mp_lapic_addr);
+			}
 			goto nox2apic;
+		}
 		/*
 		 * without IR all CPUs can be addressed by IOAPIC/MSI
 		 * only in physical mode
@@ -1525,8 +1571,10 @@ void __init enable_IR_x2apic(void)
 		x2apic_force_phys();
 	}
 
-	if (ret == IRQ_REMAP_XAPIC_MODE)
+	if (ret == IRQ_REMAP_XAPIC_MODE) {
+		pr_info("x2apic not enabled, IRQ remapping is in xapic mode\n");
 		goto nox2apic;
+	}
 
 	x2apic_enabled = 1;
 
@@ -1541,17 +1589,6 @@ nox2apic:
 		restore_ioapic_entries();
 	legacy_pic->restore_mask();
 	local_irq_restore(flags);
-
-out:
-	if (x2apic_enabled || !x2apic_supported())
-		return;
-
-	if (x2apic_preenabled)
-		panic("x2apic: enabled by BIOS but kernel init failed.");
-	else if (ret == IRQ_REMAP_XAPIC_MODE)
-		pr_info("x2apic not enabled, IRQ remapping is in xapic mode\n");
-	else if (ret < 0)
-		pr_info("x2apic not enabled, IRQ remapping init failed\n");
 }
 
 #ifdef CONFIG_X86_64
Index: linux-2.6/arch/x86/mm/srat.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/srat.c
+++ linux-2.6/arch/x86/mm/srat.c
@@ -69,6 +69,12 @@ acpi_numa_x2apic_affinity_init(struct ac
 	if ((pa->flags & ACPI_SRAT_CPU_ENABLED) == 0)
 		return;
 	pxm = pa->proximity_domain;
+	apic_id = pa->apic_id;
+	if (x2apic_disabled && (apic_id >= 0xff)) {
+		printk(KERN_INFO "SRAT: PXM %u -> X2APIC 0x%04x ignored\n",
+			 pxm, apic_id);
+		return;
+	}
 	node = setup_node(pxm);
 	if (node < 0) {
 		printk(KERN_ERR "SRAT: Too many proximity domains %x\n", pxm);
@@ -76,7 +82,6 @@ acpi_numa_x2apic_affinity_init(struct ac
 		return;
 	}
 
-	apic_id = pa->apic_id;
 	if (apic_id >= MAX_LOCAL_APIC) {
 		printk(KERN_INFO "SRAT: PXM %u -> APIC 0x%04x -> Node %u skipped apicid that is too big\n", pxm, apic_id, node);
 		return;
Index: linux-2.6/arch/x86/include/asm/apicdef.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/apicdef.h
+++ linux-2.6/arch/x86/include/asm/apicdef.h
@@ -144,6 +144,7 @@
 
 #define APIC_BASE (fix_to_virt(FIX_APIC_BASE))
 #define APIC_BASE_MSR	0x800
+#define XAPIC_ENABLE	(1UL << 11)
 #define X2APIC_ENABLE	(1UL << 10)
 
 #ifdef CONFIG_X86_32
Index: linux-2.6/arch/x86/include/asm/processor.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/processor.h
+++ linux-2.6/arch/x86/include/asm/processor.h
@@ -168,6 +168,7 @@ extern void init_scattered_cpuid_feature
 extern unsigned int init_intel_cacheinfo(struct cpuinfo_x86 *c);
 extern unsigned short num_cache_leaves;
 
+u32 x2apic_cpuid_initial_apicid(void);
 extern void detect_extended_topology(struct cpuinfo_x86 *c);
 extern void detect_ht(struct cpuinfo_x86 *c);
 
Index: linux-2.6/arch/x86/kernel/cpu/topology.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/cpu/topology.c
+++ linux-2.6/arch/x86/kernel/cpu/topology.c
@@ -21,6 +21,27 @@
 #define BITS_SHIFT_NEXT_LEVEL(eax)	((eax) & 0x1f)
 #define LEVEL_MAX_SIBLINGS(ebx)		((ebx) & 0xffff)
 
+u32 x2apic_cpuid_initial_apicid(void)
+{
+	unsigned int eax, ebx, ecx, edx;
+
+	if (boot_cpu_data.cpuid_level < 0xb)
+		return 0;
+
+	cpuid_count(0xb, SMT_LEVEL, &eax, &ebx, &ecx, &edx);
+
+	/*
+	 * check if the cpuid leaf 0xb is actually implemented.
+	 */
+	if (ebx == 0 || (LEAFB_SUBTYPE(ecx) != SMT_TYPE))
+		return 0;
+
+	/*
+	 * initial apic id, which also represents 32-bit extended x2apic id.
+	 */
+	return edx;
+}
+
 /*
  * Check for extended topology enumeration cpuid leaf 0xb and if it
  * exists, use it for populating initial_apicid and cpu topology
Index: linux-2.6/arch/x86/kernel/apic/apic_flat_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/apic_flat_64.c
+++ linux-2.6/arch/x86/kernel/apic/apic_flat_64.c
@@ -171,9 +171,14 @@ static int flat_phys_pkg_id(int initial_
 	return initial_apic_id >> index_msb;
 }
 
+static int flat_probe(void)
+{
+	return 1;
+}
+
 static struct apic apic_flat =  {
 	.name				= "flat",
-	.probe				= NULL,
+	.probe				= flat_probe,
 	.acpi_madt_oem_check		= flat_acpi_madt_oem_check,
 	.apic_id_registered		= flat_apic_id_registered,
 
Index: linux-2.6/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6/arch/x86/kernel/apic/io_apic.c
@@ -2947,6 +2947,10 @@ static inline void __init check_timer(vo
 	}
 	local_irq_disable();
 	apic_printk(APIC_QUIET, KERN_INFO "..... failed :(.\n");
+	if (x2apic_preenabled && !x2apic_mode)
+		pr_info("Your system has x2apic pre-enabled, but kernel can not enable intr-remapping.\n"
+			"So kernel try to disable x2apic, but interrupt still have problem.\n"
+			"You could disable x2apic from BIOS setup\n");
 	panic("IO-APIC + timer doesn't work!  Boot with apic=debug and send a "
 		"report.  Then try booting with the 'noapic' option.\n");
 out:

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

* Re: [PATCH -v8] x86: Disable x2apic if nox2apic is specified or intr-remap can not be enabled
  2011-12-17  0:59 [PATCH -v8] x86: Disable x2apic if nox2apic is specified or intr-remap can not be enabled Yinghai Lu
@ 2011-12-17  2:07 ` Suresh Siddha
  2011-12-20  6:51 ` Suresh Siddha
       [not found] ` <A75BCAD09CE00A4280CDD4429D85F1F92621BD7595@orsmsx501.amr.corp.intel.com>
  2 siblings, 0 replies; 8+ messages in thread
From: Suresh Siddha @ 2011-12-17  2:07 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel

On Fri, 2011-12-16 at 16:59 -0800, Yinghai Lu wrote:
> For
> 1. x2apic preenabled system
> 2. first kernel have x2apic enabled, and try to kexec second kernel with "nox2apic"
> 
> Will put back cpu with apic id < 255 into xapic mode, instead of panic.
> 

Yinghai, I didn't get a chance to review this patch completely. But few
things.

a. Need to split this patch into multiple patches, specially the cpuid
mechanism to get the initial apic id, flat_probe etc. While I understand
the reason behind the flat_probe change, it isn't quite obvious from
this changelog.

b. for nox2apic, as you try to disable x2apic even if the bios has
enabled it along with interrupt-remapping, this will probably not work
unless we disable interrupt-remapping. So either we need to ignore the
nox2apic when OS can find that interrupt-remapping is already enabled or
disable interrupt-remapping. Let me think a bit more about this and get
back to you by monday with detailed review of this.

thanks,
suresh


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

* Re: [PATCH -v8] x86: Disable x2apic if nox2apic is specified or intr-remap can not be enabled
  2011-12-17  0:59 [PATCH -v8] x86: Disable x2apic if nox2apic is specified or intr-remap can not be enabled Yinghai Lu
  2011-12-17  2:07 ` Suresh Siddha
@ 2011-12-20  6:51 ` Suresh Siddha
  2011-12-21  1:49   ` Yinghai Lu
       [not found] ` <A75BCAD09CE00A4280CDD4429D85F1F92621BD7595@orsmsx501.amr.corp.intel.com>
  2 siblings, 1 reply; 8+ messages in thread
From: Suresh Siddha @ 2011-12-20  6:51 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel

Yinghai, Here is the detailed review of your patch:

On Fri, 2011-12-16 at 16:59 -0800, Yinghai Lu wrote:
> Index: linux-2.6/arch/x86/include/asm/apic.h
> ===================================================================
> --- linux-2.6.orig/arch/x86/include/asm/apic.h
> +++ linux-2.6/arch/x86/include/asm/apic.h
> @@ -176,6 +176,8 @@ static inline u64 native_x2apic_icr_read
>  }
>  
>  extern int x2apic_phys;
> +extern int x2apic_preenabled;
> +extern int x2apic_disabled;
>  extern void check_x2apic(void);
>  extern void enable_x2apic(void);
>  extern void x2apic_icr_write(u32 low, u32 id);
> @@ -183,7 +185,7 @@ static inline int x2apic_enabled(void)
>  {
>  	u64 msr;
>  
> -	if (!cpu_has_x2apic)
> +	if (!cpu_has_x2apic || x2apic_disabled)
>  		return 0;
>  
>  	rdmsrl(MSR_IA32_APICBASE, msr);

Not sure why we need x2apic_disabled check here.

> @@ -192,12 +194,15 @@ static inline int x2apic_enabled(void)
>  	return 0;
>  }
>  
> -#define x2apic_supported()	(cpu_has_x2apic)
> +#define x2apic_supported()	(cpu_has_x2apic && !x2apic_disabled)

Here also not sure why we need this. Right thing is to clear the x2apic
capability from the cpuid features when the user selects nox2apic. 

> @@ -235,8 +239,10 @@ acpi_parse_x2apic(struct acpi_subtable_h
>  	 * to not preallocating memory for all NR_CPUS
>  	 * when we use CPU hotplug.
>  	 */
> -	acpi_register_lapic(processor->local_apic_id,	/* APIC ID */
> -			    processor->lapic_flags & ACPI_MADT_ENABLED);
> +	if (x2apic_disabled && (apic_id >= 0xff) && enabled)
> +		printk(KERN_WARNING PREFIX "x2apic entry ignored\n");

x2apic MADT entries are specifically for apic-id's >= 255. So no need to
check for the value again. We can ignore all the x2apic entries. Also
print once indicating why all x2apic entries are being ignored should be
enough.

Also when we disable x2apic in the case not initiated by the user (like
no dmar tables etc), we would have already parsed the MADT entries but
we need to skip those entries with id >= 255 duing smp boot. I don't
think I see that code in this patch.


>  static __init int setup_nox2apic(char *str)
>  {
> -	if (x2apic_enabled()) {
> -		pr_warning("Bios already enabled x2apic, "
> -			   "can't enforce nox2apic");
> -		return 0;
> -	}
> +	if (x2apic_enabled())
> +		pr_warning("Bios already enabled x2apic, will disable it");
> +
> +	x2apic_disabled = 1;
>  
> -	setup_clear_cpu_cap(X86_FEATURE_X2APIC);
>  	return 0;
>  }

Not sure why you removed this clear_cpu_cap here. Right thing to do here
is to delay the clearing when x2apic is pre-enabled by the bios and
clear it here when x2apic is not pre-enabled by the bios.

> +
> +static void disable_x2apic(void)
> +{
> +	int msr, msr2;
> +
> +	if (!cpu_has_x2apic)
> +		return;
> +
> +	rdmsr(MSR_IA32_APICBASE, msr, msr2);
> +	if (msr & X2APIC_ENABLE) {
> +		u32 x2apic_id = x2apic_cpuid_initial_apicid();

I think we should be able to do read_apic_id() here and thus avoid the
need for getting apicid from cpuid.

> +
> +		if (x2apic_id > 255)
> +			panic("Can not disable x2apic, id: %08x\n", x2apic_id);
> +
> +		pr_info("Disabling x2apic\n");
> +		/*
> +		 * Need to disable xapic and x2apic at the same time at first
> +		 *  then enable xapic
> +		 */
> +		wrmsr(MSR_IA32_APICBASE, msr & ~(X2APIC_ENABLE | XAPIC_ENABLE),
> +			 0);
> +		wrmsr(MSR_IA32_APICBASE, msr & ~X2APIC_ENABLE, 0);
> +
> +		x2apic_disabled = 1;
> +	}
> +}
>  void check_x2apic(void)
>  {
> +	if (x2apic_disabled) {
> +		disable_x2apic();
> +		return;
> +	}

We shouldn't call disable_x2apic() from here. We should be following the
same sequence, like masking interrupts at the cpu, pic, io-apic level
before we disable x2apic and re-enable xapic. So the right place to do
this disable is at enable_IR_x2apic(). 

> @@ -1449,6 +1480,11 @@ void enable_x2apic(void)
>  {
>  	int msr, msr2;
>  
> +	if (x2apic_disabled) {
> +		disable_x2apic();

enable_x2apic() gets called from both BP and AP. AP's need not do the
apic-id being >= 255 checks etc. As that is done during smp bringup
(which is missing currently from the current patch) or during MADT
parsing. So all we need to do is a simple x2apic->xapic conversion.

>  
>  #ifdef CONFIG_X86_64
> Index: linux-2.6/arch/x86/mm/srat.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/mm/srat.c
> +++ linux-2.6/arch/x86/mm/srat.c
> @@ -69,6 +69,12 @@ acpi_numa_x2apic_affinity_init(struct ac
>  	if ((pa->flags & ACPI_SRAT_CPU_ENABLED) == 0)
>  		return;
>  	pxm = pa->proximity_domain;
> +	apic_id = pa->apic_id;
> +	if (x2apic_disabled && (apic_id >= 0xff)) {

Same comments like in the above MADT parsing.

>  
> Index: linux-2.6/arch/x86/kernel/cpu/topology.c
> ===================================================================
> --- linux-2.6.orig/arch/x86/kernel/cpu/topology.c
> +++ linux-2.6/arch/x86/kernel/cpu/topology.c
> @@ -21,6 +21,27 @@
>  #define BITS_SHIFT_NEXT_LEVEL(eax)	((eax) & 0x1f)
>  #define LEVEL_MAX_SIBLINGS(ebx)		((ebx) & 0xffff)
>  
> +u32 x2apic_cpuid_initial_apicid(void)
> +{

I think we can do with out this function.

While reviewing the code, I ended up doing some of this cleanup of your
patch. I have appended the modified version. Can you please incorporate
the smpboot checks before the AP bringup (apic-id should be less than
255 in !x2apic_mode etc) and split the patch into couple of patches
(apic_probe etc) along with other needed cleanups.

Thanks.
---
 arch/x86/include/asm/apic.h         |    6 ++
 arch/x86/include/asm/apicdef.h      |    1 +
 arch/x86/kernel/acpi/boot.c         |   10 +++-
 arch/x86/kernel/apic/apic.c         |   92 ++++++++++++++++++++++++++---------
 arch/x86/kernel/apic/apic_flat_64.c |    7 ++-
 arch/x86/kernel/apic/io_apic.c      |    4 ++
 arch/x86/mm/srat.c                  |    7 ++-
 7 files changed, 99 insertions(+), 28 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index a0f541a..5d43993 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -176,6 +176,8 @@ static inline u64 native_x2apic_icr_read(void)
 }
 
 extern int x2apic_phys;
+extern int x2apic_preenabled;
+extern int x2apic_disabled;
 extern void check_x2apic(void);
 extern void enable_x2apic(void);
 extern void x2apic_icr_write(u32 low, u32 id);
@@ -198,6 +200,9 @@ static inline void x2apic_force_phys(void)
 	x2apic_phys = 1;
 }
 #else
+static inline void disable_x2apic(void)
+{
+}
 static inline void check_x2apic(void)
 {
 }
@@ -214,6 +219,7 @@ static inline void x2apic_force_phys(void)
 
 #define	x2apic_preenabled 0
 #define	x2apic_supported()	0
+#define	x2apic_disabled	1
 #endif
 
 extern void enable_IR_x2apic(void);
diff --git a/arch/x86/include/asm/apicdef.h b/arch/x86/include/asm/apicdef.h
index 3925d80..134bba0 100644
--- a/arch/x86/include/asm/apicdef.h
+++ b/arch/x86/include/asm/apicdef.h
@@ -144,6 +144,7 @@
 
 #define APIC_BASE (fix_to_virt(FIX_APIC_BASE))
 #define APIC_BASE_MSR	0x800
+#define XAPIC_ENABLE	(1UL << 11)
 #define X2APIC_ENABLE	(1UL << 10)
 
 #ifdef CONFIG_X86_32
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 4558f0d..1c54a9e 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -219,6 +219,8 @@ static int __init
 acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
 {
 	struct acpi_madt_local_x2apic *processor = NULL;
+	int apic_id;
+	u8 enabled;
 
 	processor = (struct acpi_madt_local_x2apic *)header;
 
@@ -227,6 +229,8 @@ acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
 
 	acpi_table_print_madt_entry(header);
 
+	apic_id = processor->local_apic_id;
+	enabled = processor->lapic_flags & ACPI_MADT_ENABLED;
 #ifdef CONFIG_X86_X2APIC
 	/*
 	 * We need to register disabled CPU as well to permit
@@ -235,8 +239,10 @@ acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
 	 * to not preallocating memory for all NR_CPUS
 	 * when we use CPU hotplug.
 	 */
-	acpi_register_lapic(processor->local_apic_id,	/* APIC ID */
-			    processor->lapic_flags & ACPI_MADT_ENABLED);
+	if (x2apic_disabled && (apic_id >= 0xff) && enabled)
+		printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
+	else
+		acpi_register_lapic(apic_id, enabled);
 #else
 	printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
 #endif
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 6acd05a..a8a0094 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -146,16 +146,16 @@ __setup("apicpmtimer", setup_apicpmtimer);
 int x2apic_mode;
 #ifdef CONFIG_X86_X2APIC
 /* x2apic enabled before OS handover */
-static int x2apic_preenabled;
+int x2apic_preenabled;
+int x2apic_disabled;
 static __init int setup_nox2apic(char *str)
 {
-	if (x2apic_enabled()) {
-		pr_warning("Bios already enabled x2apic, "
-			   "can't enforce nox2apic");
-		return 0;
-	}
+	if (x2apic_enabled())
+		pr_warning("Bios already enabled x2apic, will disable it");
+	else
+		setup_clear_cpu_cap(X86_FEATURE_X2APIC);
+	x2apic_disabled = 1;
 
-	setup_clear_cpu_cap(X86_FEATURE_X2APIC);
 	return 0;
 }
 early_param("nox2apic", setup_nox2apic);
@@ -1432,6 +1432,45 @@ void __init bsp_end_local_APIC_setup(void)
 }
 
 #ifdef CONFIG_X86_X2APIC
+
+static inline void __disable_x2apic(u64 msr)
+{
+	wrmsrl(MSR_IA32_APICBASE,
+	       msr & ~(X2APIC_ENABLE | XAPIC_ENABLE));
+	wrmsrl(MSR_IA32_APICBASE, msr & ~X2APIC_ENABLE);
+}
+
+static void disable_x2apic(void)
+{
+	u64 msr;
+
+	if (!cpu_has_x2apic)
+		return;
+
+	rdmsrl(MSR_IA32_APICBASE, msr);
+	if (msr & X2APIC_ENABLE) {
+		u32 x2apic_id = read_apic_id();
+
+		if (x2apic_id > 255)
+			panic("Can not disable x2apic, id: %08x\n", x2apic_id);
+
+		pr_info("Disabling x2apic\n");
+		/*
+		 * Need to disable xapic and x2apic at the same time at first
+		 *  then enable xapic
+		 */
+		__disable_x2apic(msr);
+
+		if (x2apic_disabled)
+			setup_clear_cpu_cap(X86_FEATURE_X2APIC);
+
+		x2apic_disabled = 1;
+		x2apic_mode = 0;
+
+		register_lapic_address(mp_lapic_addr);
+	}
+}
+
 void check_x2apic(void)
 {
 	if (x2apic_enabled()) {
@@ -1442,15 +1481,20 @@ void check_x2apic(void)
 
 void enable_x2apic(void)
 {
-	int msr, msr2;
+	u64 msr;
+
+	rdmsrl(MSR_IA32_APICBASE, msr);
+	if (x2apic_disabled) {
+		__disable_x2apic(msr);
+		return;
+	}
 
 	if (!x2apic_mode)
 		return;
 
-	rdmsr(MSR_IA32_APICBASE, msr, msr2);
 	if (!(msr & X2APIC_ENABLE)) {
 		printk_once(KERN_INFO "Enabling x2apic\n");
-		wrmsr(MSR_IA32_APICBASE, msr | X2APIC_ENABLE, msr2);
+		wrmsrl(MSR_IA32_APICBASE, msr | X2APIC_ENABLE);
 	}
 }
 #endif /* CONFIG_X86_X2APIC */
@@ -1487,25 +1531,34 @@ void __init enable_IR_x2apic(void)
 	ret = save_ioapic_entries();
 	if (ret) {
 		pr_info("Saving IO-APIC state failed: %d\n", ret);
-		goto out;
+		return;
 	}
 
 	local_irq_save(flags);
 	legacy_pic->mask_all();
 	mask_ioapic_entries();
 
+	if (x2apic_disabled)
+		disable_x2apic();
+
 	if (dmar_table_init_ret)
 		ret = -1;
 	else
 		ret = enable_IR();
 
+	if (!x2apic_supported())
+		goto nox2apic;
+
 	if (ret < 0) {
 		/* IR is required if there is APIC ID > 255 even when running
 		 * under KVM
 		 */
 		if (max_physical_apicid > 255 ||
-		    !hypervisor_x2apic_available())
+		    !hypervisor_x2apic_available()) {
+			if (x2apic_preenabled && !x2apic_disabled)
+				disable_x2apic();
 			goto nox2apic;
+		}
 		/*
 		 * without IR all CPUs can be addressed by IOAPIC/MSI
 		 * only in physical mode
@@ -1513,8 +1566,10 @@ void __init enable_IR_x2apic(void)
 		x2apic_force_phys();
 	}
 
-	if (ret == IRQ_REMAP_XAPIC_MODE)
+	if (ret == IRQ_REMAP_XAPIC_MODE) {
+		pr_info("x2apic not enabled, IRQ remapping is in xapic mode\n");
 		goto nox2apic;
+	}
 
 	x2apic_enabled = 1;
 
@@ -1529,17 +1584,6 @@ nox2apic:
 		restore_ioapic_entries();
 	legacy_pic->restore_mask();
 	local_irq_restore(flags);
-
-out:
-	if (x2apic_enabled || !x2apic_supported())
-		return;
-
-	if (x2apic_preenabled)
-		panic("x2apic: enabled by BIOS but kernel init failed.");
-	else if (ret == IRQ_REMAP_XAPIC_MODE)
-		pr_info("x2apic not enabled, IRQ remapping is in xapic mode\n");
-	else if (ret < 0)
-		pr_info("x2apic not enabled, IRQ remapping init failed\n");
 }
 
 #ifdef CONFIG_X86_64
diff --git a/arch/x86/kernel/apic/apic_flat_64.c b/arch/x86/kernel/apic/apic_flat_64.c
index 57c1f41..8c3cdde 100644
--- a/arch/x86/kernel/apic/apic_flat_64.c
+++ b/arch/x86/kernel/apic/apic_flat_64.c
@@ -171,9 +171,14 @@ static int flat_phys_pkg_id(int initial_apic_id, int index_msb)
 	return initial_apic_id >> index_msb;
 }
 
+static int flat_probe(void)
+{
+	return 1;
+}
+
 static struct apic apic_flat =  {
 	.name				= "flat",
-	.probe				= NULL,
+	.probe				= flat_probe,
 	.acpi_madt_oem_check		= flat_acpi_madt_oem_check,
 	.apic_id_registered		= flat_apic_id_registered,
 
diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index 8980555..f2977a2 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2948,6 +2948,10 @@ static inline void __init check_timer(void)
 	}
 	local_irq_disable();
 	apic_printk(APIC_QUIET, KERN_INFO "..... failed :(.\n");
+	if (x2apic_preenabled && !x2apic_mode)
+		pr_info("Your system has x2apic pre-enabled, but kernel can not enable intr-remapping.\n"
+			"So kernel try to disable x2apic, but interrupt still have problem.\n"
+			"You could disable x2apic from BIOS setup\n");
 	panic("IO-APIC + timer doesn't work!  Boot with apic=debug and send a "
 		"report.  Then try booting with the 'noapic' option.\n");
 out:
diff --git a/arch/x86/mm/srat.c b/arch/x86/mm/srat.c
index 81dbfde..6dd2b09 100644
--- a/arch/x86/mm/srat.c
+++ b/arch/x86/mm/srat.c
@@ -69,6 +69,12 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
 	if ((pa->flags & ACPI_SRAT_CPU_ENABLED) == 0)
 		return;
 	pxm = pa->proximity_domain;
+	apic_id = pa->apic_id;
+	if (x2apic_disabled && (apic_id >= 0xff)) {
+		printk(KERN_INFO "SRAT: PXM %u -> X2APIC 0x%04x ignored\n",
+			 pxm, apic_id);
+		return;
+	}
 	node = setup_node(pxm);
 	if (node < 0) {
 		printk(KERN_ERR "SRAT: Too many proximity domains %x\n", pxm);
@@ -76,7 +82,6 @@ acpi_numa_x2apic_affinity_init(struct acpi_srat_x2apic_cpu_affinity *pa)
 		return;
 	}
 
-	apic_id = pa->apic_id;
 	if (apic_id >= MAX_LOCAL_APIC) {
 		printk(KERN_INFO "SRAT: PXM %u -> APIC 0x%04x -> Node %u skipped apicid that is too big\n", pxm, apic_id, node);
 		return;



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

* Re: [PATCH -v8] x86: Disable x2apic if nox2apic is specified or intr-remap can not be enabled
       [not found] ` <A75BCAD09CE00A4280CDD4429D85F1F92621BD7595@orsmsx501.amr.corp.intel.com>
@ 2011-12-20  6:54   ` Suresh Siddha
  0 siblings, 0 replies; 8+ messages in thread
From: Suresh Siddha @ 2011-12-20  6:54 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel

On Fri, 2011-12-16 at 18:07 -0800, Suresh Siddha wrote:
> b. for nox2apic, as you try to disable x2apic even if the bios has
> enabled it along with interrupt-remapping, this will probably not work
> unless we disable interrupt-remapping.

Looking at the code, this condition should be handled correctly by the
OS by enabling interrupt-remapping in non extended interrupt-mode. So
this case should be ok.

thanks,
suresh


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

* Re: [PATCH -v8] x86: Disable x2apic if nox2apic is specified or intr-remap can not be enabled
  2011-12-20  6:51 ` Suresh Siddha
@ 2011-12-21  1:49   ` Yinghai Lu
  2011-12-21  2:24     ` Yinghai Lu
  0 siblings, 1 reply; 8+ messages in thread
From: Yinghai Lu @ 2011-12-21  1:49 UTC (permalink / raw)
  To: Suresh Siddha; +Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel

On Mon, Dec 19, 2011 at 10:51 PM, Suresh Siddha
<suresh.b.siddha@intel.com> wrote:
> Yinghai, Here is the detailed review of your patch:
>
> On Fri, 2011-12-16 at 16:59 -0800, Yinghai Lu wrote:
>> Index: linux-2.6/arch/x86/include/asm/apic.h
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/include/asm/apic.h
>> +++ linux-2.6/arch/x86/include/asm/apic.h
>> @@ -176,6 +176,8 @@ static inline u64 native_x2apic_icr_read
>>  }
>>
>>  extern int x2apic_phys;
>> +extern int x2apic_preenabled;
>> +extern int x2apic_disabled;
>>  extern void check_x2apic(void);
>>  extern void enable_x2apic(void);
>>  extern void x2apic_icr_write(u32 low, u32 id);
>> @@ -183,7 +185,7 @@ static inline int x2apic_enabled(void)
>>  {
>>       u64 msr;
>>
>> -     if (!cpu_has_x2apic)
>> +     if (!cpu_has_x2apic || x2apic_disabled)
>>               return 0;
>>
>>       rdmsrl(MSR_IA32_APICBASE, msr);
>
> Not sure why we need x2apic_disabled check here.

skip the extra msr reading, because cpu x2apic feature is kept.

>
>> @@ -192,12 +194,15 @@ static inline int x2apic_enabled(void)
>>       return 0;
>>  }
>>
>> -#define x2apic_supported()   (cpu_has_x2apic)
>> +#define x2apic_supported()   (cpu_has_x2apic && !x2apic_disabled)
>
> Here also not sure why we need this. Right thing is to clear the x2apic
> capability from the cpuid features when the user selects nox2apic.

just want to leave that feature alone.

>
>> @@ -235,8 +239,10 @@ acpi_parse_x2apic(struct acpi_subtable_h
>>        * to not preallocating memory for all NR_CPUS
>>        * when we use CPU hotplug.
>>        */
>> -     acpi_register_lapic(processor->local_apic_id,   /* APIC ID */
>> -                         processor->lapic_flags & ACPI_MADT_ENABLED);
>> +     if (x2apic_disabled && (apic_id >= 0xff) && enabled)
>> +             printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
>
> x2apic MADT entries are specifically for apic-id's >= 255. So no need to
> check for the value again. We can ignore all the x2apic entries. Also
> print once indicating why all x2apic entries are being ignored should be
> enough.

no, some vendor still keep x2apic entries even apic id < 255.
they have different understanding of x2apic entries.

>
> Also when we disable x2apic in the case not initiated by the user (like
> no dmar tables etc), we would have already parsed the MADT entries but
> we need to skip those entries with id >= 255 duing smp boot. I don't
> think I see that code in this patch.

it will panic.

>
>
>>  static __init int setup_nox2apic(char *str)
>>  {
>> -     if (x2apic_enabled()) {
>> -             pr_warning("Bios already enabled x2apic, "
>> -                        "can't enforce nox2apic");
>> -             return 0;
>> -     }
>> +     if (x2apic_enabled())
>> +             pr_warning("Bios already enabled x2apic, will disable it");
>> +
>> +     x2apic_disabled = 1;
>>
>> -     setup_clear_cpu_cap(X86_FEATURE_X2APIC);
>>       return 0;
>>  }
>
> Not sure why you removed this clear_cpu_cap here. Right thing to do here
> is to delay the clearing when x2apic is pre-enabled by the bios and
> clear it here when x2apic is not pre-enabled by the bios.

that param (nox2apic) is analyzed earlier than check_x2apic()

so can not x2apic_preenabled to check clear the x2apic cpuid features.

>
>> +
>> +static void disable_x2apic(void)
>> +{
>> +     int msr, msr2;
>> +
>> +     if (!cpu_has_x2apic)
>> +             return;
>> +
>> +     rdmsr(MSR_IA32_APICBASE, msr, msr2);
>> +     if (msr & X2APIC_ENABLE) {
>> +             u32 x2apic_id = x2apic_cpuid_initial_apicid();
>
> I think we should be able to do read_apic_id() here and thus avoid the
> need for getting apicid from cpuid.

during BP enable_IR_x2apic and later apic ops could be change back to xapic...

that may not read out whole x2apic id.

>
>> +
>> +             if (x2apic_id > 255)
>> +                     panic("Can not disable x2apic, id: %08x\n", x2apic_id);
>> +
>> +             pr_info("Disabling x2apic\n");
>> +             /*
>> +              * Need to disable xapic and x2apic at the same time at first
>> +              *  then enable xapic
>> +              */
>> +             wrmsr(MSR_IA32_APICBASE, msr & ~(X2APIC_ENABLE | XAPIC_ENABLE),
>> +                      0);
>> +             wrmsr(MSR_IA32_APICBASE, msr & ~X2APIC_ENABLE, 0);
>> +
>> +             x2apic_disabled = 1;
>> +     }
>> +}
>>  void check_x2apic(void)
>>  {
>> +     if (x2apic_disabled) {
>> +             disable_x2apic();
>> +             return;
>> +     }
>
> We shouldn't call disable_x2apic() from here. We should be following the
> same sequence, like masking interrupts at the cpu, pic, io-apic level
> before we disable x2apic and re-enable xapic. So the right place to do
> this disable is at enable_IR_x2apic().

Disabling early as possible looks like more cleanly.

>
>> @@ -1449,6 +1480,11 @@ void enable_x2apic(void)
>>  {
>>       int msr, msr2;
>>
>> +     if (x2apic_disabled) {
>> +             disable_x2apic();
>
> enable_x2apic() gets called from both BP and AP. AP's need not do the
> apic-id being >= 255 checks etc. As that is done during smp bringup
> (which is missing currently from the current patch) or during MADT
> parsing. So all we need to do is a simple x2apic->xapic conversion.

yes, that is intended for APs too, so it will panic the system. go
back to clean up process_info could be extra work.

>
>>
>>  #ifdef CONFIG_X86_64
>> Index: linux-2.6/arch/x86/mm/srat.c
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/mm/srat.c
>> +++ linux-2.6/arch/x86/mm/srat.c
>> @@ -69,6 +69,12 @@ acpi_numa_x2apic_affinity_init(struct ac
>>       if ((pa->flags & ACPI_SRAT_CPU_ENABLED) == 0)
>>               return;
>>       pxm = pa->proximity_domain;
>> +     apic_id = pa->apic_id;
>> +     if (x2apic_disabled && (apic_id >= 0xff)) {
>
> Same comments like in the above MADT parsing.
>
>>
>> Index: linux-2.6/arch/x86/kernel/cpu/topology.c
>> ===================================================================
>> --- linux-2.6.orig/arch/x86/kernel/cpu/topology.c
>> +++ linux-2.6/arch/x86/kernel/cpu/topology.c
>> @@ -21,6 +21,27 @@
>>  #define BITS_SHIFT_NEXT_LEVEL(eax)   ((eax) & 0x1f)
>>  #define LEVEL_MAX_SIBLINGS(ebx)              ((ebx) & 0xffff)
>>
>> +u32 x2apic_cpuid_initial_apicid(void)
>> +{
>
> I think we can do with out this function.
>
> While reviewing the code, I ended up doing some of this cleanup of your
> patch. I have appended the modified version. Can you please incorporate
> the smpboot checks before the AP bringup (apic-id should be less than
> 255 in !x2apic_mode etc) and split the patch into couple of patches
> (apic_probe etc) along with other needed cleanups.

We could handle AP's apic id > 255 panic problem later if needed in
another patch.

Actually on system with apic id > 255, intr-remapping aka DMAR must be
working, otherwise those big box can not be sold.

And in this case, nox2apic must be passed to disable x2apic, because
dmar is correct in both case kexec or first boot.

when nox2apic is passed,  those entries will be skipped early. and BSP
x2apic get disabled early.
and code path is more like bios enable xapic only.

Can we let this patch get into tip and have test cycle at first?
at least we should not let it break any working set up.

Thanks

Yinghai Lu

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

* Re: [PATCH -v8] x86: Disable x2apic if nox2apic is specified or intr-remap can not be enabled
  2011-12-21  1:49   ` Yinghai Lu
@ 2011-12-21  2:24     ` Yinghai Lu
  2011-12-22  1:54       ` Suresh Siddha
  0 siblings, 1 reply; 8+ messages in thread
From: Yinghai Lu @ 2011-12-21  2:24 UTC (permalink / raw)
  To: Suresh Siddha; +Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel

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

On Tue, Dec 20, 2011 at 5:49 PM, Yinghai Lu <yinghai@kernel.org> wrote:
> On Mon, Dec 19, 2011 at 10:51 PM, Suresh Siddha
> <suresh.b.siddha@intel.com> wrote:
>> While reviewing the code, I ended up doing some of this cleanup of your
>> patch. I have appended the modified version. Can you please incorporate
>> the smpboot checks before the AP bringup (apic-id should be less than
>> 255 in !x2apic_mode etc) and split the patch into couple of patches
>> (apic_probe etc) along with other needed cleanups.
>
> We could handle AP's apic id > 255 panic problem later if needed in
> another patch.
>
> Actually on system with apic id > 255, intr-remapping aka DMAR must be
> working, otherwise those big box can not be sold.
>
> And in this case, nox2apic must be passed to disable x2apic, because
> dmar is correct in both case kexec or first boot.
>
> when nox2apic is passed,  those entries will be skipped early. and BSP
> x2apic get disabled early.
> and code path is more like bios enable xapic only.
>
> Can we let this patch get into tip and have test cycle at first?
> at least we should not let it break any working set up.

put your updated version as v9.

like to let ingo or hpa or thomas to decide v8 or v9 to be used.

        Changes:
                 1. clear x2apic cpu features
                 2. disable x2apic later instead of check_x2apic()

Thanks

Yinghai

[-- Attachment #2: nox2apic_v9.patch --]
[-- Type: text/x-patch, Size: 10358 bytes --]

From: Yinghai Lu <yinghai@kernel.org>
Subject: [PATCH -v9] x86: Disable x2apic if nox2apic is specified or intr-remap can not be enabled

For
1. x2apic preenabled system
2. first kernel have x2apic enabled, and try to kexec second kernel with "nox2apic"

Will put back cpu with apic id < 255 into xapic mode, instead of panic.

-v2: use variable x2apic_disabled instead of variable nox2apic, Suggested by Thomas
     update x2apic_supported with x2apic_disabled, Suggested by Thomas

-v3: add checking for boot cpu apic id > 255. in that case will just panic
     --- pointed out by Suresh.

-v4: according to Ingo, for x2apic pre-enabled system, if intr-remap can not
        be enabled, try to disable x2apic instead of panic and request to
        specify nox2apic for next boot.

-v5: fix the lapic mapping for falling path.

-v6: add warning when check_timer fails in strange case when BIOS enable x2apic
     and intr_remapping but does not provide dmar table. requested by Suresh.

-v7: fix compiling with x2apic_preenabled

-v8: fix compiling with x2apic_disabled assigned with !X86_X2APIC

-v9: updated version from Suresh
	Changes: 1. clear x2apic cpu features
		 2. disable x2apic later instead of check_x2apic()

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
Tested-by: "Berck E. Nash" <flyboy@gmail.com>

---
 arch/x86/include/asm/apic.h         |    6 ++
 arch/x86/include/asm/apicdef.h      |    1 
 arch/x86/kernel/acpi/boot.c         |   10 +++
 arch/x86/kernel/apic/apic.c         |   92 ++++++++++++++++++++++++++----------
 arch/x86/kernel/apic/apic_flat_64.c |    7 ++
 arch/x86/kernel/apic/io_apic.c      |    4 +
 arch/x86/mm/srat.c                  |    7 ++
 7 files changed, 99 insertions(+), 28 deletions(-)

Index: linux-2.6/arch/x86/include/asm/apic.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/apic.h
+++ linux-2.6/arch/x86/include/asm/apic.h
@@ -176,6 +176,8 @@ static inline u64 native_x2apic_icr_read
 }
 
 extern int x2apic_phys;
+extern int x2apic_preenabled;
+extern int x2apic_disabled;
 extern void check_x2apic(void);
 extern void enable_x2apic(void);
 extern void x2apic_icr_write(u32 low, u32 id);
@@ -198,6 +200,9 @@ static inline void x2apic_force_phys(voi
 	x2apic_phys = 1;
 }
 #else
+static inline void disable_x2apic(void)
+{
+}
 static inline void check_x2apic(void)
 {
 }
@@ -214,6 +219,7 @@ static inline void x2apic_force_phys(voi
 
 #define	x2apic_preenabled 0
 #define	x2apic_supported()	0
+#define	x2apic_disabled	1
 #endif
 
 extern void enable_IR_x2apic(void);
Index: linux-2.6/arch/x86/include/asm/apicdef.h
===================================================================
--- linux-2.6.orig/arch/x86/include/asm/apicdef.h
+++ linux-2.6/arch/x86/include/asm/apicdef.h
@@ -144,6 +144,7 @@
 
 #define APIC_BASE (fix_to_virt(FIX_APIC_BASE))
 #define APIC_BASE_MSR	0x800
+#define XAPIC_ENABLE	(1UL << 11)
 #define X2APIC_ENABLE	(1UL << 10)
 
 #ifdef CONFIG_X86_32
Index: linux-2.6/arch/x86/kernel/acpi/boot.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/acpi/boot.c
+++ linux-2.6/arch/x86/kernel/acpi/boot.c
@@ -219,6 +219,8 @@ static int __init
 acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
 {
 	struct acpi_madt_local_x2apic *processor = NULL;
+	int apic_id;
+	u8 enabled;
 
 	processor = (struct acpi_madt_local_x2apic *)header;
 
@@ -227,6 +229,8 @@ acpi_parse_x2apic(struct acpi_subtable_h
 
 	acpi_table_print_madt_entry(header);
 
+	apic_id = processor->local_apic_id;
+	enabled = processor->lapic_flags & ACPI_MADT_ENABLED;
 #ifdef CONFIG_X86_X2APIC
 	/*
 	 * We need to register disabled CPU as well to permit
@@ -235,8 +239,10 @@ acpi_parse_x2apic(struct acpi_subtable_h
 	 * to not preallocating memory for all NR_CPUS
 	 * when we use CPU hotplug.
 	 */
-	acpi_register_lapic(processor->local_apic_id,	/* APIC ID */
-			    processor->lapic_flags & ACPI_MADT_ENABLED);
+	if (x2apic_disabled && (apic_id >= 0xff) && enabled)
+		printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
+	else
+		acpi_register_lapic(apic_id, enabled);
 #else
 	printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
 #endif
Index: linux-2.6/arch/x86/kernel/apic/apic.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/apic.c
+++ linux-2.6/arch/x86/kernel/apic/apic.c
@@ -146,16 +146,16 @@ __setup("apicpmtimer", setup_apicpmtimer
 int x2apic_mode;
 #ifdef CONFIG_X86_X2APIC
 /* x2apic enabled before OS handover */
-static int x2apic_preenabled;
+int x2apic_preenabled;
+int x2apic_disabled;
 static __init int setup_nox2apic(char *str)
 {
-	if (x2apic_enabled()) {
-		pr_warning("Bios already enabled x2apic, "
-			   "can't enforce nox2apic");
-		return 0;
-	}
+	if (x2apic_enabled())
+		pr_warning("Bios already enabled x2apic, will disable it");
+	else
+		setup_clear_cpu_cap(X86_FEATURE_X2APIC);
+	x2apic_disabled = 1;
 
-	setup_clear_cpu_cap(X86_FEATURE_X2APIC);
 	return 0;
 }
 early_param("nox2apic", setup_nox2apic);
@@ -1432,6 +1432,45 @@ void __init bsp_end_local_APIC_setup(voi
 }
 
 #ifdef CONFIG_X86_X2APIC
+
+static inline void __disable_x2apic(u64 msr)
+{
+	wrmsrl(MSR_IA32_APICBASE,
+	       msr & ~(X2APIC_ENABLE | XAPIC_ENABLE));
+	wrmsrl(MSR_IA32_APICBASE, msr & ~X2APIC_ENABLE);
+}
+
+static __init void disable_x2apic(void)
+{
+	u64 msr;
+
+	if (!cpu_has_x2apic)
+		return;
+
+	rdmsrl(MSR_IA32_APICBASE, msr);
+	if (msr & X2APIC_ENABLE) {
+		u32 x2apic_id = read_apic_id();
+
+		if (x2apic_id > 255)
+			panic("Can not disable x2apic, id: %08x\n", x2apic_id);
+
+		pr_info("Disabling x2apic\n");
+		/*
+		 * Need to disable xapic and x2apic at the same time at first
+		 *  then enable xapic
+		 */
+		__disable_x2apic(msr);
+
+		if (x2apic_disabled)
+			setup_clear_cpu_cap(X86_FEATURE_X2APIC);
+
+		x2apic_disabled = 1;
+		x2apic_mode = 0;
+
+		register_lapic_address(mp_lapic_addr);
+	}
+}
+
 void check_x2apic(void)
 {
 	if (x2apic_enabled()) {
@@ -1442,15 +1481,20 @@ void check_x2apic(void)
 
 void enable_x2apic(void)
 {
-	int msr, msr2;
+	u64 msr;
+
+	rdmsrl(MSR_IA32_APICBASE, msr);
+	if (x2apic_disabled) {
+		__disable_x2apic(msr);
+		return;
+	}
 
 	if (!x2apic_mode)
 		return;
 
-	rdmsr(MSR_IA32_APICBASE, msr, msr2);
 	if (!(msr & X2APIC_ENABLE)) {
 		printk_once(KERN_INFO "Enabling x2apic\n");
-		wrmsr(MSR_IA32_APICBASE, msr | X2APIC_ENABLE, msr2);
+		wrmsrl(MSR_IA32_APICBASE, msr | X2APIC_ENABLE);
 	}
 }
 #endif /* CONFIG_X86_X2APIC */
@@ -1494,25 +1538,34 @@ void __init enable_IR_x2apic(void)
 	ret = save_ioapic_entries();
 	if (ret) {
 		pr_info("Saving IO-APIC state failed: %d\n", ret);
-		goto out;
+		return;
 	}
 
 	local_irq_save(flags);
 	legacy_pic->mask_all();
 	mask_ioapic_entries();
 
+	if (x2apic_disabled)
+		disable_x2apic();
+
 	if (dmar_table_init_ret)
 		ret = -1;
 	else
 		ret = enable_IR();
 
+	if (!x2apic_supported())
+		goto nox2apic;
+
 	if (ret < 0) {
 		/* IR is required if there is APIC ID > 255 even when running
 		 * under KVM
 		 */
 		if (max_physical_apicid > 255 ||
-		    !hypervisor_x2apic_available())
+		    !hypervisor_x2apic_available()) {
+			if (x2apic_preenabled && !x2apic_disabled)
+				disable_x2apic();
 			goto nox2apic;
+		}
 		/*
 		 * without IR all CPUs can be addressed by IOAPIC/MSI
 		 * only in physical mode
@@ -1520,8 +1573,10 @@ void __init enable_IR_x2apic(void)
 		x2apic_force_phys();
 	}
 
-	if (ret == IRQ_REMAP_XAPIC_MODE)
+	if (ret == IRQ_REMAP_XAPIC_MODE) {
+		pr_info("x2apic not enabled, IRQ remapping is in xapic mode\n");
 		goto nox2apic;
+	}
 
 	x2apic_enabled = 1;
 
@@ -1536,17 +1591,6 @@ nox2apic:
 		restore_ioapic_entries();
 	legacy_pic->restore_mask();
 	local_irq_restore(flags);
-
-out:
-	if (x2apic_enabled || !x2apic_supported())
-		return;
-
-	if (x2apic_preenabled)
-		panic("x2apic: enabled by BIOS but kernel init failed.");
-	else if (ret == IRQ_REMAP_XAPIC_MODE)
-		pr_info("x2apic not enabled, IRQ remapping is in xapic mode\n");
-	else if (ret < 0)
-		pr_info("x2apic not enabled, IRQ remapping init failed\n");
 }
 
 #ifdef CONFIG_X86_64
Index: linux-2.6/arch/x86/kernel/apic/apic_flat_64.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/apic_flat_64.c
+++ linux-2.6/arch/x86/kernel/apic/apic_flat_64.c
@@ -171,9 +171,14 @@ static int flat_phys_pkg_id(int initial_
 	return initial_apic_id >> index_msb;
 }
 
+static int flat_probe(void)
+{
+	return 1;
+}
+
 static struct apic apic_flat =  {
 	.name				= "flat",
-	.probe				= NULL,
+	.probe				= flat_probe,
 	.acpi_madt_oem_check		= flat_acpi_madt_oem_check,
 	.apic_id_registered		= flat_apic_id_registered,
 
Index: linux-2.6/arch/x86/kernel/apic/io_apic.c
===================================================================
--- linux-2.6.orig/arch/x86/kernel/apic/io_apic.c
+++ linux-2.6/arch/x86/kernel/apic/io_apic.c
@@ -2947,6 +2947,10 @@ static inline void __init check_timer(vo
 	}
 	local_irq_disable();
 	apic_printk(APIC_QUIET, KERN_INFO "..... failed :(.\n");
+	if (x2apic_preenabled && !x2apic_mode)
+		pr_info("Your system has x2apic pre-enabled, but kernel can not enable intr-remapping.\n"
+			"So kernel try to disable x2apic, but interrupt still have problem.\n"
+			"You could disable x2apic from BIOS setup\n");
 	panic("IO-APIC + timer doesn't work!  Boot with apic=debug and send a "
 		"report.  Then try booting with the 'noapic' option.\n");
 out:
Index: linux-2.6/arch/x86/mm/srat.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/srat.c
+++ linux-2.6/arch/x86/mm/srat.c
@@ -69,6 +69,12 @@ acpi_numa_x2apic_affinity_init(struct ac
 	if ((pa->flags & ACPI_SRAT_CPU_ENABLED) == 0)
 		return;
 	pxm = pa->proximity_domain;
+	apic_id = pa->apic_id;
+	if (x2apic_disabled && (apic_id >= 0xff)) {
+		printk(KERN_INFO "SRAT: PXM %u -> X2APIC 0x%04x ignored\n",
+			 pxm, apic_id);
+		return;
+	}
 	node = setup_node(pxm);
 	if (node < 0) {
 		printk(KERN_ERR "SRAT: Too many proximity domains %x\n", pxm);
@@ -76,7 +82,6 @@ acpi_numa_x2apic_affinity_init(struct ac
 		return;
 	}
 
-	apic_id = pa->apic_id;
 	if (apic_id >= MAX_LOCAL_APIC) {
 		printk(KERN_INFO "SRAT: PXM %u -> APIC 0x%04x -> Node %u skipped apicid that is too big\n", pxm, apic_id, node);
 		return;

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

* Re: [PATCH -v8] x86: Disable x2apic if nox2apic is specified or intr-remap can not be enabled
  2011-12-21  2:24     ` Yinghai Lu
@ 2011-12-22  1:54       ` Suresh Siddha
  2011-12-22  7:25         ` Yinghai Lu
  0 siblings, 1 reply; 8+ messages in thread
From: Suresh Siddha @ 2011-12-22  1:54 UTC (permalink / raw)
  To: Yinghai Lu; +Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel

On Tue, 2011-12-20 at 18:24 -0800, Yinghai Lu wrote:
> put your updated version as v9.
> 
> like to let ingo or hpa or thomas to decide v8 or v9 to be used.
> 
>         Changes:
>                  1. clear x2apic cpu features
>                  2. disable x2apic later instead of check_x2apic()

Yinghai, It is not the question of just cleanup. changing the apic mode
has to follow some sequence.

Anyways, I have done all the changes and other minor cleanups. Also
split up your patch into multiple patches and posted it now.

Please let me know if you see any issue.

thanks,
suresh




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

* Re: [PATCH -v8] x86: Disable x2apic if nox2apic is specified or intr-remap can not be enabled
  2011-12-22  1:54       ` Suresh Siddha
@ 2011-12-22  7:25         ` Yinghai Lu
  0 siblings, 0 replies; 8+ messages in thread
From: Yinghai Lu @ 2011-12-22  7:25 UTC (permalink / raw)
  To: Suresh Siddha; +Cc: Ingo Molnar, Thomas Gleixner, H. Peter Anvin, linux-kernel

On Wed, Dec 21, 2011 at 5:54 PM, Suresh Siddha
<suresh.b.siddha@intel.com> wrote:
> On Tue, 2011-12-20 at 18:24 -0800, Yinghai Lu wrote:
>> put your updated version as v9.
>>
>> like to let ingo or hpa or thomas to decide v8 or v9 to be used.
>>
>>         Changes:
>>                  1. clear x2apic cpu features
>>                  2. disable x2apic later instead of check_x2apic()
>
> Yinghai, It is not the question of just cleanup. changing the apic mode
> has to follow some sequence.
>
> Anyways, I have done all the changes and other minor cleanups. Also
> split up your patch into multiple patches and posted it now.
>
> Please let me know if you see any issue.

updated 4/5 for some minor problem with applying and compiling.

Please check that.

Thanks

Yinghai

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

end of thread, other threads:[~2011-12-22  7:25 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-17  0:59 [PATCH -v8] x86: Disable x2apic if nox2apic is specified or intr-remap can not be enabled Yinghai Lu
2011-12-17  2:07 ` Suresh Siddha
2011-12-20  6:51 ` Suresh Siddha
2011-12-21  1:49   ` Yinghai Lu
2011-12-21  2:24     ` Yinghai Lu
2011-12-22  1:54       ` Suresh Siddha
2011-12-22  7:25         ` Yinghai Lu
     [not found] ` <A75BCAD09CE00A4280CDD4429D85F1F92621BD7595@orsmsx501.amr.corp.intel.com>
2011-12-20  6:54   ` Suresh Siddha

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.