[PATCH 07/15] x86, sfi: Use smp_register_lapic_address()

Previous thread: [PATCH 12/15] x86, x2apic: Don't map lapic addr for preenabled x2apic by Yinghai Lu on Saturday, October 23, 2010 - 6:02 pm. (1 message)

Next thread: Re: intel_idle .. Was: __pm_runtime_resume() returns -1 Was: Regression in 2.6.36 by Christian Ruediger Bahls on Saturday, October 23, 2010 - 7:40 pm. (1 message)
From: Yinghai Lu
Date: Saturday, October 23, 2010 - 6:02 pm

1. Merge acpi_register_lapic_address, smp_register_lapic_address, sfi_register_lapic_address

2. nox2apic support: disable x2apic for preenabled system or kexec path.

Thanks

Yinghai Lu

71fee2d: x86: Disabling x2apic if nox2apic is specified
017b8d0: acpi: Reverse uid and apic_id print out for x2apic
0c2a5ca: x86, apic, acpi: Handle xapic/x2apic entries in MADT at same time
88c4f93: x86, x2apic: Don't map lapic addr for preenabled x2apic
7084cf3: x86, ioapic: Only print mapping for ioapic in right place
d4c4263: x86, apic: Set fixmap only one time
19fafd5: x86: on !find_smp_config path use smp_register_lapic_address
74c3593: x86, visws: Set_fixmap in find_smp_config
039ff68: x86, sfi: Use smp_register_lapic_address()
6f13e6a: x86, apic: Use smp_register_lapic_address in init_apic_mapping
48c02b1: x86: Call smp_register_lapic_address for contruct_default mptable path
d2f3d61: x86, apic: Remove early_init_lapic_mapping
3887e09: x86, apic: Merge two register_lapic_address()
703583d: x86, apic: Fix lapic mapping with construct ISA and visws mptable path
a2bd134: x86, apic: Don't write io_apic ID if it is not changed

 arch/x86/include/asm/apic.h    |    8 +++-
 arch/x86/include/asm/apicdef.h |    1 +
 arch/x86/kernel/acpi/boot.c    |   61 +++++++++++++---------
 arch/x86/kernel/apic/apic.c    |  110 +++++++++++++++++++++++++---------------
 arch/x86/kernel/apic/io_apic.c |   19 ++++---
 arch/x86/kernel/mpparse.c      |   24 ++-------
 arch/x86/kernel/sfi.c          |   13 +----
 arch/x86/kernel/visws_quirks.c |    1 +
 arch/x86/mm/k8topology_64.c    |    1 -
 arch/x86/mm/srat_64.c          |   12 ++++-
 drivers/acpi/numa.c            |   18 +++++--
 drivers/acpi/tables.c          |   60 ++++++++++++++++------
 include/linux/acpi.h           |    8 +++
 13 files changed, 207 insertions(+), 129 deletions(-)

--

From: Yinghai Lu
Date: Saturday, October 23, 2010 - 6:02 pm

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

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

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 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    |   54 +++++++++++++++++++++++++++++++--------
 arch/x86/mm/srat_64.c          |   12 ++++++++-
 5 files changed, 69 insertions(+), 14 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 69879dd..522f39b 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -176,6 +176,7 @@ static inline u64 native_x2apic_icr_read(void)
 }
 
 extern int x2apic_phys;
+extern int nox2apic;
 extern void check_x2apic(void);
 extern void enable_x2apic(void);
 extern void x2apic_icr_write(u32 low, u32 id);
@@ -186,6 +187,10 @@ static inline int x2apic_enabled(void)
 	if (!cpu_has_x2apic)
 		return 0;
 
+	/* avoid to read msr */
+	if (nox2apic)
+		return 0;
+
 	rdmsr(MSR_IA32_APICBASE, msr, msr2);
 	if (msr & X2APIC_ENABLE)
 		return 1;
@@ -214,6 +219,7 @@ static inline void x2apic_force_phys(void)
 
 #define	x2apic_preenabled 0
 #define	x2apic_supported()	0
+#define	nox2apic	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 a859ca4..f28feba 100644
--- a/arch/x86/include/asm/apicdef.h
+++ b/arch/x86/include/asm/apicdef.h
@@ -141,6 +141,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 8e13ec8..5a4e67e 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -213,6 +213,8 @@ static int __init
 ...
From: Thomas Gleixner
Date: Sunday, October 24, 2010 - 3:15 am

That comment is useless. I wish you would add comments to complex code
not to obvious one.

Also it can be folded into the above check



Do we really need all these extra checks ? Can't we simply make all
this one variable wich is set to 1 when x2apic is available and not

Why conditional? No need to duplicate the printk. It just needs some
--

From: Yinghai Lu
Date: Sunday, October 24, 2010 - 3:09 pm

need to check that bit with disable_x2apic.

+static void disable_x2apic(void)
+{
+       int msr, msr2;
+
+       if (!cpu_has_x2apic)
+               return;

other concern should be addressed. please check -v2.

Thanks

	Yinghai

[PATCH -v2] x86: Disable x2apic if nox2apic is specified

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 x2apic_disabled instead of nox2apic, Suggested by Thomas
     update x2apic_supported with x2apic_disabled, Suggested by Thomas

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 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    |   46 ++++++++++++++++++++++++++++++++++-------
 arch/x86/mm/srat_64.c          |    7 +++++-
 5 files changed, 58 insertions(+), 12 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,7 @@ static inline u64 native_x2apic_icr_read
 }
 
 extern int x2apic_phys;
+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 +184,7 @@ static inline int x2apic_enabled(void)
 {
 	int msr, msr2;
 
-	if (!cpu_has_x2apic)
+	if (!cpu_has_x2apic || x2apic_disabled)
 		return 0;
 
 	rdmsr(MSR_IA32_APICBASE, msr, msr2);
@@ -192,7 +193,7 @@ 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;
@@ -214,6 +215,7 @@ static inline void x2apic_force_phys(voi
 
 #define	x2apic_preenabled 0
 ...
From: Suresh Siddha
Date: Monday, October 25, 2010 - 10:50 am

Yinghai, Have you validated this patch on a system having apicid's > 255
and when the bios has enabled x2apic? When a bios enables x2apic
(typically when it finds processors with apic id's > 255), it also
enables interrupt-remapping before enabling x2apic in the bios.

So not sure what is the behavior with this patch(along with the nox2apic
boot option). Does the kernel boot with cpu's having < 255 apicid, and
also use the interrupt-remapping but with xapic mode?

Also, the boot cpu's apic id might be already > 8 bit, but by forcing to
use xapic, the lower 8 bit portion of it might conflict with another AP
(whose 32bit apic id value is < 255). So we need to be careful in this
case aswell. Have you validated this scenario aswell? May be we should
(or already does) skip that AP having the same 8bit apic id etc.

thanks,

--

From: Yinghai Lu
Date: Tuesday, October 26, 2010 - 11:27 pm

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 x2apic_disabled instead of 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.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>

---
 arch/x86/include/asm/apic.h      |    6 +++-
 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      |   51 +++++++++++++++++++++++++++++++++------
 arch/x86/kernel/cpu/topology.c   |   21 ++++++++++++++++
 arch/x86/mm/srat_64.c            |    7 ++++-
 7 files changed, 85 insertions(+), 12 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,7 @@ static inline u64 native_x2apic_icr_read
 }
 
 extern int x2apic_phys;
+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 +184,7 @@ static inline int x2apic_enabled(void)
 {
 	int msr, msr2;
 
-	if (!cpu_has_x2apic)
+	if (!cpu_has_x2apic || x2apic_disabled)
 		return 0;
 
 	rdmsr(MSR_IA32_APICBASE, msr, msr2);
@@ -192,7 +193,7 @@ 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;
@@ -214,6 +215,7 @@ static inline void x2apic_force_phys(voi
 
 #define	x2apic_preenabled 0
 #define	x2apic_supported()	0
+#define	x2apic_disabled	1
 #endif
 
 extern void ...
From: Suresh Siddha
Date: Thursday, October 28, 2010 - 10:53 pm

Can we please hold off in merging this patch for a bit? There are some
subtleties in going back to xapic when the platform/cpu's are in x2apic
mode. I will check and get back to you on this after KS.

thanks,
suresh

--

From: Yinghai Lu
Date: Friday, October 29, 2010 - 12:26 am

sure. take you time.

	Yinghai
--

From: Yinghai Lu
Date: Saturday, October 23, 2010 - 6:02 pm

Do need to set lapic mapping for them

in arch/x86/kernel/visws_quirks.c:
we only have visws_find_smp_config() to set mp_lapic_addr to APIC_DEFAULT_PHYS_BASE
visws_get_smp_config() is nop call.
default_get_smp_config/check_physptr/smp_read_mpc is not called in the path.
So smp_register_lapic_address() is not called, and lapic is not mapped.


in arch/x86/kernel/mpparse.c
if mpf->feature1 != 0, it will go through contruct_default_ISA_mptable instead
of check_phystr path, so smp_register_lapic_address is not called.

Those pathes all have smp_found_config set.

So need to remove !smp_found_config checking

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 arch/x86/kernel/apic/apic.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 4638396..193e407 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1677,7 +1677,7 @@ void __init init_apic_mappings(void)
 		 * acpi lapic path already maps that address in
 		 * acpi_register_lapic_address()
 		 */
-		if (!acpi_lapic && !smp_found_config)
+		if (!acpi_lapic)
 			set_fixmap_nocache(FIX_APIC_BASE, apic_phys);
 
 		apic_printk(APIC_VERBOSE, "mapped APIC to %08lx (%08lx)\n",
-- 
1.7.1

--

From: Thomas Gleixner
Date: Sunday, October 24, 2010 - 2:47 am

Need ? You can, because the check is redundant if I understand the
above correctly.

Thanks,

--

From: Yinghai Lu
Date: Saturday, October 23, 2010 - 6:02 pm

For 32bit mptable path, setup_ids_from_mpc() always write io apic id
register, even there is no change needed.

So try to do that when they are different bewteen reading out and mptable

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 arch/x86/kernel/apic/io_apic.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
index ce3c6fb..0579b3b 100644
--- a/arch/x86/kernel/apic/io_apic.c
+++ b/arch/x86/kernel/apic/io_apic.c
@@ -2006,7 +2006,6 @@ void __init setup_ioapic_ids_from_mpc(void)
 			physids_or(phys_id_present_map, phys_id_present_map, tmp);
 		}
 
-
 		/*
 		 * We need to adjust the IRQ routing table
 		 * if the ID changed.
@@ -2018,9 +2017,12 @@ void __init setup_ioapic_ids_from_mpc(void)
 						= mp_ioapics[apic_id].apicid;
 
 		/*
-		 * Read the right value from the MPC table and
-		 * write it into the ID register.
+		 * Update the ID register according to the right value from
+		 *  the MPC table if they are different.
 		 */
+		if (mp_ioapics[apic_id].apicid == reg_00.bits.ID)
+			continue;
+
 		apic_printk(APIC_VERBOSE, KERN_INFO
 			"...changing IO-APIC physical APIC ID to %d ...",
 			mp_ioapics[apic_id].apicid);
-- 
1.7.1

--

From: Yinghai Lu
Date: Saturday, October 23, 2010 - 6:02 pm

It is almost the same as smp_register_lapic_addr()

Just need to make smp_read_mpc() call smp_register_lapic_addr when early==1.

also add printing out in smp_register_lapic_address()

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 arch/x86/include/asm/apic.h |    1 -
 arch/x86/kernel/apic/apic.c |   24 ++----------------------
 arch/x86/kernel/mpparse.c   |    8 ++------
 arch/x86/mm/k8topology_64.c |    1 -
 4 files changed, 4 insertions(+), 30 deletions(-)

diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index dcc5f48..69879dd 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -245,7 +245,6 @@ extern int apic_force_enable(void);
  * On 32bit this is mach-xxx local
  */
 #ifdef CONFIG_X86_64
-extern void early_init_lapic_mapping(void);
 extern int apic_is_clustered_box(void);
 #else
 static inline int apic_is_clustered_box(void)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index 4827c17..05b8bf7 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -1631,28 +1631,6 @@ no_apic:
 }
 #endif
 
-#ifdef CONFIG_X86_64
-void __init early_init_lapic_mapping(void)
-{
-	/*
-	 * If no local APIC can be found then go out
-	 * : it means there is no mpatable and MADT
-	 */
-	if (!smp_found_config)
-		return;
-
-	set_fixmap_nocache(FIX_APIC_BASE, mp_lapic_addr);
-	apic_printk(APIC_VERBOSE, "mapped APIC to %16lx (%16lx)\n",
-		    APIC_BASE, mp_lapic_addr);
-
-	/*
-	 * Fetch the APIC ID of the BSP in case we have a
-	 * default configuration (or the MP table is broken).
-	 */
-	boot_cpu_physical_apicid = read_apic_id();
-}
-#endif
-
 /**
  * init_apic_mappings - initialize APIC mappings
  */
@@ -1708,6 +1686,8 @@ void __init smp_register_lapic_address(unsigned long address)
 	mp_lapic_addr = address;
 
 	set_fixmap_nocache(FIX_APIC_BASE, address);
+	apic_printk(APIC_VERBOSE, "mapped APIC to %16lx (%16lx)\n",
+		    APIC_BASE, mp_lapic_addr);
 	if ...
From: Thomas Gleixner
Date: Sunday, October 24, 2010 - 2:53 am

How is this restricted to early == 1 ? It's called unconditionally
which is nonsense.

Thanks,

	tglx
--

From: Yinghai Lu
Date: Saturday, October 23, 2010 - 6:02 pm

kill mp_sfi_register_lapic_address, they are the same

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 arch/x86/kernel/sfi.c |   13 +------------
 1 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/arch/x86/kernel/sfi.c b/arch/x86/kernel/sfi.c
index dd4c281..4dc8065 100644
--- a/arch/x86/kernel/sfi.c
+++ b/arch/x86/kernel/sfi.c
@@ -34,17 +34,6 @@
 #ifdef CONFIG_X86_LOCAL_APIC
 static unsigned long sfi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE;
 
-static void __init mp_sfi_register_lapic_address(unsigned long address)
-{
-	mp_lapic_addr = address;
-
-	set_fixmap_nocache(FIX_APIC_BASE, mp_lapic_addr);
-	if (boot_cpu_physical_apicid == -1U)
-		boot_cpu_physical_apicid = read_apic_id();
-
-	pr_info("Boot CPU = %d\n", boot_cpu_physical_apicid);
-}
-
 /* All CPUs enumerated by SFI must be present and enabled */
 static void __cpuinit mp_sfi_register_lapic(u8 id)
 {
@@ -110,7 +99,7 @@ static int __init sfi_parse_ioapic(struct sfi_table_header *table)
 int __init sfi_platform_init(void)
 {
 #ifdef CONFIG_X86_LOCAL_APIC
-	mp_sfi_register_lapic_address(sfi_lapic_addr);
+	smp_register_lapic_address(sfi_lapic_addr);
 	sfi_table_parse(SFI_SIG_CPUS, NULL, NULL, sfi_parse_cpus);
 #endif
 #ifdef CONFIG_X86_IO_APIC
-- 
1.7.1

--

From: Thomas Gleixner
Date: Sunday, October 24, 2010 - 2:56 am

That pr_info is not really important, but it makes the function
--

From: Yinghai Lu
Date: Saturday, October 23, 2010 - 6:02 pm

So make it have same sequence like old apic.

Signed-off-by: Yinghai Lu <yinghai@kernel.org>
---
 drivers/acpi/tables.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 3e4a60e..4dffce0 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -67,8 +67,8 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header)
 			struct acpi_madt_local_x2apic *p =
 			    (struct acpi_madt_local_x2apic *)header;
 			printk(KERN_INFO PREFIX
-			       "X2APIC (apic_id[0x%02x] uid[0x%02x] %s)\n",
-			       p->local_apic_id, p->uid,
+			       "X2APIC (uid[0x%02x] apic_id[0x%02x] %s)\n",
+			       p->uid, p->local_apic_id,
 			       (p->lapic_flags & ACPI_MADT_ENABLED) ?
 			       "enabled" : "disabled");
 		}
-- 
1.7.1

--

Previous thread: [PATCH 12/15] x86, x2apic: Don't map lapic addr for preenabled x2apic by Yinghai Lu on Saturday, October 23, 2010 - 6:02 pm. (1 message)

Next thread: Re: intel_idle .. Was: __pm_runtime_resume() returns -1 Was: Regression in 2.6.36 by Christian Ruediger Bahls on Saturday, October 23, 2010 - 7:40 pm. (1 message)