linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/3] x86, apic, kexec: Add disable_cpu_apic kernel parameter
@ 2013-11-12  9:51 HATAYAMA Daisuke
  2013-11-12  9:51 ` [PATCH v5 1/3] x86, apic: add bsp_physical_apicid HATAYAMA Daisuke
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: HATAYAMA Daisuke @ 2013-11-12  9:51 UTC (permalink / raw)
  To: hpa, ebiederm, vgoyal
  Cc: kexec, linux-kernel, bp, akpm, fengguang.wu, jingbai.ma

This patch set is to allow kdump 2nd kernel to wake up multiple CPUs
even if 1st kernel crashs on some AP, a continueing work from:

  [PATCH v3 0/2] x86, apic, kdump: Disable BSP if boot cpu is AP
  https://lkml.org/lkml/2013/10/16/300.

At v4, basic design has changed. Now users need to figure out initial
APIC ID of BSP in the 1st kernel and configures kernel parameter for
the 2nd kernel manually using disable_cpu_apic kernel parameter to be
newly introduced in this patch set. This design is more flexible than
the previous version in that we no longer have to rely on ACPI/MP
table to get initial APIC ID of BSP.

Sorry, this patch set have not include in-source documentation
requested by Borislav Petkov yet, but I'll post it later separately,
which would be better to focus on documentation reviewing.

ChangeLog

v4 => v5)

- Rebased on top of v3.12

- Introduce bsp_physical_apicid that has the initial APIC ID for the
  processor with BSP flag on IA32_APIC_BASE MSR. Without this,
  boot_cpu_physical_apicid has temporarilly the value around MP table
  related codes, although it's designed to have the initial APIC ID
  for the processor that is doing the boot up. Use the
  bsp_physical_apicid in MP table related codes; no impact on
  semantics at runtime there.

v3 => v4)

- Rebased on top of v3.12-rc6

- Basic design has been changed. Now users need to figure out initial
  APIC ID of BSP in the 1st kernel and configures kernel parameter for
  the 2nd kernel manually using disable_cpu_apic kernel parameter to
  be newly introduced in this patch set. This design is more flexible
  than the previous version in that we no longer have to rely on
  ACPI/MP table to get initial APIC ID of BSP.

v2 => v3)

- Change default value of boot_cpu_is_bsp to true.

- Before executing rdmsr(MSR_IA32_APICBASE), check if the number of
  processor family is larger than or equal to 6 in order to avoid
  invalid opcode exception on processors where MSR_IA32_APICBASE is
  not supported.

v1 => v2)

- Rebased on top of v3.12-rc5.

- Fix linking time error of boot_cpu_is_bsp_init() in case of
  CONFIG_LOCAL_APIC disabled by adding empty static inline function
  instead.

- Fix missing feature check by means of cpu_has_apic macro in
  boot_cpu_is_bsp_init() before calling rdmsr_safe(MSR_IA32_APICBASE).

  NOTE: I've checked local apic-present case only; I don't have any
  x86 processor without local apic.

- Add __init annotation to boot_cpu_is_bsp_init().

Test

- built with and without CONFIG_LOCAL_APIC, CONFIG_SMP and CONFIG_X86_UP_APIC.
- tested x86_64 in case of acpi and MP table
- tested disable_cpu_apicid=<n> to disable both AP and BSP

---

HATAYAMA Daisuke (3):
      x86, apic: add bsp_physical_apicid
      x86, apic: Add disable_cpu_apicid kernel parameter
      Documentation, x86, apic, kexec: Add disable_cpu_apicid kernel parameter


 Documentation/kernel-parameters.txt    |    9 +++++++
 arch/x86/include/asm/mpspec.h          |    7 +++++
 arch/x86/kernel/apic/apic.c            |   44 ++++++++++++++++++++++++++++++++
 arch/x86/kernel/cpu/common.c           |    5 +++-
 arch/x86/kernel/mpparse.c              |    3 +-
 arch/x86/kernel/setup.c                |    2 +
 arch/x86/mm/amdtopology.c              |    6 ++--
 arch/x86/platform/visws/visws_quirks.c |    6 +++-
 8 files changed, 75 insertions(+), 7 deletions(-)

-- 

Thanks.
HATAYAMA, Daisuke

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

* [PATCH v5 1/3] x86, apic: add bsp_physical_apicid
  2013-11-12  9:51 [PATCH v5 0/3] x86, apic, kexec: Add disable_cpu_apic kernel parameter HATAYAMA Daisuke
@ 2013-11-12  9:51 ` HATAYAMA Daisuke
  2013-11-14 11:16   ` HATAYAMA Daisuke
  2013-11-12  9:51 ` [PATCH v5 2/3] x86, apic: Add disable_cpu_apicid kernel parameter HATAYAMA Daisuke
  2013-11-12  9:52 ` [PATCH v5 3/3] Documentation, x86, apic, kexec: " HATAYAMA Daisuke
  2 siblings, 1 reply; 7+ messages in thread
From: HATAYAMA Daisuke @ 2013-11-12  9:51 UTC (permalink / raw)
  To: hpa, ebiederm, vgoyal
  Cc: kexec, linux-kernel, bp, akpm, fengguang.wu, jingbai.ma

Add bsp_physical_apicid variable that has the initial APIC ID for the
processor with BSP flag on IA32_APIC_BASE MSR. Without this,
boot_cpu_physical_apicid temporarily has the value around MP table
related codes such as kernel/mpparse.c, mm/amdtopology.c and
platform/visws/visws_quirks.c until it is rewritten back in
init_apic_mappings() to the initial APIC ID for the processor that is
doing boot up.

This change is also required by the next commit introducing new
disable_cpu_apicid kernel parameter, where the value specified via
disable_cpu_apicid is compared with boot_cpu_physical_apicid in
generic_processor_info(), but in case of using MP table, the
boot_cpu_physical_apicid is rewiriten by MP_processor_info() to the
initial APIC ID of the BSP reported by BIOS, which is problematic if
some AP is specified in disable_cpu_apic.

Then, replace boot_cpu_physical_apicid in the MP table related codes
such as kernel/mpparse.c, mm/amdtopology.c and
platform/visws/visws_quirks.c by newly introduced bsp_physical_apicid,
where no functional change is intended.

If the cpu that is doing boot-up process is BSP, use cpuid to get the
initial APIC ID. If AP, assign the initial APIC ID obtained from MP
table or MADT. This covers the above MP table related codes with no
functional change.

Initialize boot_cpu_data.initial_apicid in early_identify_cpu() since
currently it is initialized after logical cpu number assignment is
performed.

Signed-off-by: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
---
 arch/x86/include/asm/mpspec.h          |    7 +++++++
 arch/x86/kernel/apic/apic.c            |   15 +++++++++++++++
 arch/x86/kernel/cpu/common.c           |    5 ++++-
 arch/x86/kernel/mpparse.c              |    3 ++-
 arch/x86/kernel/setup.c                |    2 ++
 arch/x86/mm/amdtopology.c              |    6 +++---
 arch/x86/platform/visws/visws_quirks.c |    6 ++++--
 7 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/arch/x86/include/asm/mpspec.h b/arch/x86/include/asm/mpspec.h
index 626cf70..894d6df 100644
--- a/arch/x86/include/asm/mpspec.h
+++ b/arch/x86/include/asm/mpspec.h
@@ -47,11 +47,18 @@ extern int mp_bus_id_to_type[MAX_MP_BUSSES];
 extern DECLARE_BITMAP(mp_bus_not_pci, MAX_MP_BUSSES);
 
 extern unsigned int boot_cpu_physical_apicid;
+extern unsigned int bsp_physical_apicid;
 extern unsigned int max_physical_apicid;
 extern int mpc_default_type;
 extern unsigned long mp_lapic_addr;
 
 #ifdef CONFIG_X86_LOCAL_APIC
+extern void bsp_physical_apicid_init(void);
+#else
+static inline void bsp_physical_apicid_init(void) { };
+#endif
+
+#ifdef CONFIG_X86_LOCAL_APIC
 extern int smp_found_config;
 #else
 # define smp_found_config 0
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index a7eb82d..b60ad92 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -63,6 +63,10 @@ unsigned disabled_cpus;
 /* Processor that is doing the boot up */
 unsigned int boot_cpu_physical_apicid = -1U;
 
+/* Processor with BP flag on IA32_APIC_BASE MSR. In case of kexec,
+ * this can differ from the processor that is doing the boot up. */
+unsigned int bsp_physical_apicid = BAD_APICID;
+
 /*
  * The highest APIC ID seen during enumeration.
  */
@@ -2589,3 +2593,14 @@ static int __init lapic_insert_resource(void)
  * that is using request_resource
  */
 late_initcall(lapic_insert_resource);
+
+void __init bsp_physical_apicid_init(void)
+{
+	u32 l, h;
+
+	if (boot_cpu_data.x86 >= 6 && cpu_has_apic) {
+		rdmsr(MSR_IA32_APICBASE, l, h);
+		if (!(l & MSR_IA32_APICBASE_BSP))
+			bsp_physical_apicid = boot_cpu_data.initial_apicid;
+	}
+}
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 2793d1f..28bea2c 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -681,7 +681,7 @@ static void identify_cpu_without_cpuid(struct cpuinfo_x86 *c)
 /*
  * Do minimum CPU detection early.
  * Fields really needed: vendor, cpuid_level, family, model, mask,
- * cache alignment.
+ * cache alignment, initial_apicid.
  * The others are not touched to avoid unwanted side effects.
  *
  * WARNING: this function is only called on the BP.  Don't add code here
@@ -725,6 +725,9 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
 		this_cpu->c_bsp_init(c);
 
 	setup_force_cpu_cap(X86_FEATURE_ALWAYS);
+
+	if (c->cpuid_level >= 0x00000001)
+		c->initial_apicid = (cpuid_ebx(1) >> 24) & 0xFF;
 }
 
 void __init early_cpu_init(void)
diff --git a/arch/x86/kernel/mpparse.c b/arch/x86/kernel/mpparse.c
index d2b5648..209fbf6 100644
--- a/arch/x86/kernel/mpparse.c
+++ b/arch/x86/kernel/mpparse.c
@@ -64,7 +64,8 @@ static void __init MP_processor_info(struct mpc_cpu *m)
 
 	if (m->cpuflag & CPU_BOOTPROCESSOR) {
 		bootup_cpu = " (Bootup-CPU)";
-		boot_cpu_physical_apicid = m->apicid;
+		if (bsp_physical_apicid == BAD_APICID)
+			bsp_physical_apicid = m->apicid;
 	}
 
 	printk(KERN_INFO "Processor #%d%s\n", m->apicid, bootup_cpu);
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index f0de629..8745b24 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -1166,6 +1166,8 @@ void __init setup_arch(char **cmdline_p)
 
 	early_quirks();
 
+	bsp_physical_apicid_init();
+
 	/*
 	 * Read APIC and some other early information from ACPI tables.
 	 */
diff --git a/arch/x86/mm/amdtopology.c b/arch/x86/mm/amdtopology.c
index 2ca15b5..bec16ad 100644
--- a/arch/x86/mm/amdtopology.c
+++ b/arch/x86/mm/amdtopology.c
@@ -183,9 +183,9 @@ int __init amd_numa_init(void)
 
 	/* get the APIC ID of the BSP early for systems with apicid lifting */
 	early_get_boot_cpu_id();
-	if (boot_cpu_physical_apicid > 0) {
-		pr_info("BSP APIC ID: %02x\n", boot_cpu_physical_apicid);
-		apicid_base = boot_cpu_physical_apicid;
+	if (bsp_physical_apicid > 0) {
+		pr_info("BSP APIC ID: %02x\n", bsp_physical_apicid);
+		apicid_base = bsp_physical_apicid;
 	}
 
 	for_each_node_mask(i, numa_nodes_parsed)
diff --git a/arch/x86/platform/visws/visws_quirks.c b/arch/x86/platform/visws/visws_quirks.c
index 94d8a39..aebe821 100644
--- a/arch/x86/platform/visws/visws_quirks.c
+++ b/arch/x86/platform/visws/visws_quirks.c
@@ -165,8 +165,10 @@ static void __init MP_processor_info(struct mpc_cpu *m)
 	       m->apicid, (m->cpufeature & CPU_FAMILY_MASK) >> 8,
 	       (m->cpufeature & CPU_MODEL_MASK) >> 4, m->apicver);
 
-	if (m->cpuflag & CPU_BOOTPROCESSOR)
-		boot_cpu_physical_apicid = m->apicid;
+	if (m->cpuflag & CPU_BOOTPROCESSOR) {
+		if (bsp_physical_apicid == BAD_APICID)
+			bsp_physical_apicid = m->apicid;
+	}
 
 	ver = m->apicver;
 	if ((ver >= 0x14 && m->apicid >= 0xff) || m->apicid >= 0xf) {


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

* [PATCH v5 2/3] x86, apic: Add disable_cpu_apicid kernel parameter
  2013-11-12  9:51 [PATCH v5 0/3] x86, apic, kexec: Add disable_cpu_apic kernel parameter HATAYAMA Daisuke
  2013-11-12  9:51 ` [PATCH v5 1/3] x86, apic: add bsp_physical_apicid HATAYAMA Daisuke
@ 2013-11-12  9:51 ` HATAYAMA Daisuke
  2013-11-12 10:44   ` Borislav Petkov
  2013-11-12  9:52 ` [PATCH v5 3/3] Documentation, x86, apic, kexec: " HATAYAMA Daisuke
  2 siblings, 1 reply; 7+ messages in thread
From: HATAYAMA Daisuke @ 2013-11-12  9:51 UTC (permalink / raw)
  To: hpa, ebiederm, vgoyal
  Cc: kexec, linux-kernel, bp, akpm, fengguang.wu, jingbai.ma

Add disable_cpu_apicid kernel parameter. To use this kernel parameter,
specify an initial APIC ID of the corresponding CPU you want to
disable.

This is mostly used for the kdump 2nd kernel to disable BSP to wake up
multiple CPUs without causing system reset or hang due to sending INIT
from AP to BSP.

Kdump users first figure out initial APIC ID of the BSP, CPU0 in the
1st kernel, for example from /proc/cpuinfo and then set up this kernel
parameter for the 2nd kernel using the obtained APIC ID.

However, doing this procedure at each boot time manually is awkward,
which should be automatically done by user-land service scripts, for
example, kexec-tools on fedora/RHEL distributions.

This design is more flexible than disabling BSP in kernel boot time
automatically in that in kernel boot time we have no choice but
referring to ACPI/MP table to obtain initial APIC ID for BSP, meaning
that the method is not applicable to the systems without such BIOS
tables.

One assumption behind this design is that users get initial APIC ID of
the BSP in still healthy state and so BSP is uniquely kept in
CPU0. Thus, through the kernel parameter, only one initial APIC ID can
be specified.

Signed-off-by: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
---
 arch/x86/kernel/apic/apic.c |   29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index b60ad92..075bf23 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -78,6 +78,13 @@ unsigned int max_physical_apicid;
 physid_mask_t phys_cpu_present_map;
 
 /*
+ * Processor to be disabled specified by kernel parameter
+ * disable_cpu_apicid=<int>, mostly used for the kdump 2nd kernel to
+ * avoid undefined behaviour caused by sending INIT from AP to BSP.
+ */
+unsigned int disabled_cpu_apicid = BAD_APICID;
+
+/*
  * Map cpu index to physical APIC ID
  */
 DEFINE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_cpu_to_apicid, BAD_APICID);
@@ -2117,6 +2124,19 @@ void generic_processor_info(int apicid, int version)
 	bool boot_cpu_detected = physid_isset(boot_cpu_physical_apicid,
 				phys_cpu_present_map);
 
+	if (disabled_cpu_apicid != BAD_APICID &&
+	    disabled_cpu_apicid != boot_cpu_physical_apicid &&
+	    disabled_cpu_apicid == apicid) {
+		int thiscpu = num_processors + disabled_cpus;
+
+		pr_warning("ACPI: Disable specified CPU."
+			   " Processor %d/0x%x ignored.\n",
+			   thiscpu, apicid);
+
+		disabled_cpus++;
+		return;
+	}
+
 	/*
 	 * If boot cpu has not been detected yet, then only allow upto
 	 * nr_cpu_ids - 1 processors and keep one slot free for boot cpu
@@ -2604,3 +2624,12 @@ void __init bsp_physical_apicid_init(void)
 			bsp_physical_apicid = boot_cpu_data.initial_apicid;
 	}
 }
+
+static int __init apic_set_disabled_cpu_apicid(char *arg)
+{
+	if (!arg || !get_option(&arg, &disabled_cpu_apicid))
+		return -EINVAL;
+
+	return 0;
+}
+early_param("disable_cpu_apicid", apic_set_disabled_cpu_apicid);


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

* [PATCH v5 3/3] Documentation, x86, apic, kexec: Add disable_cpu_apicid kernel parameter
  2013-11-12  9:51 [PATCH v5 0/3] x86, apic, kexec: Add disable_cpu_apic kernel parameter HATAYAMA Daisuke
  2013-11-12  9:51 ` [PATCH v5 1/3] x86, apic: add bsp_physical_apicid HATAYAMA Daisuke
  2013-11-12  9:51 ` [PATCH v5 2/3] x86, apic: Add disable_cpu_apicid kernel parameter HATAYAMA Daisuke
@ 2013-11-12  9:52 ` HATAYAMA Daisuke
  2 siblings, 0 replies; 7+ messages in thread
From: HATAYAMA Daisuke @ 2013-11-12  9:52 UTC (permalink / raw)
  To: hpa, ebiederm, vgoyal
  Cc: kexec, linux-kernel, bp, akpm, fengguang.wu, jingbai.ma

Add disable_cpu_apicid kernel parameter to disable the CPU with the
specified number of initial APIC ID, mostly used for the kdump 2nd
kernel to disable BSP to wake up multiple CPUs without causing system
reset or hang due to sending INIT from AP to BSP.

Signed-off-by: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
---
 Documentation/kernel-parameters.txt |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index fcbb736..0ca0902 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -774,6 +774,15 @@ bytes respectively. Such letter suffixes can also be entirely omitted.
 	disable=	[IPV6]
 			See Documentation/networking/ipv6.txt.
 
+	disable_cpu_apicid= [X86,APIC,KEXEC,SMP]
+			Format: <int>
+			The number of initial APIC ID for the
+			corresponding CPU to be disabled at boot,
+			mostly used for the kdump 2nd kernel to
+			disable BSP to wake up multiple CPUs without
+			causing system reset or hang due to sending
+			INIT from AP to BSP.
+
 	disable_ddw     [PPC/PSERIES]
 			Disable Dynamic DMA Window support. Use this if
 			to workaround buggy firmware.


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

* Re: [PATCH v5 2/3] x86, apic: Add disable_cpu_apicid kernel parameter
  2013-11-12  9:51 ` [PATCH v5 2/3] x86, apic: Add disable_cpu_apicid kernel parameter HATAYAMA Daisuke
@ 2013-11-12 10:44   ` Borislav Petkov
  2013-11-14 10:42     ` HATAYAMA Daisuke
  0 siblings, 1 reply; 7+ messages in thread
From: Borislav Petkov @ 2013-11-12 10:44 UTC (permalink / raw)
  To: HATAYAMA Daisuke
  Cc: hpa, ebiederm, vgoyal, kexec, linux-kernel, akpm, fengguang.wu,
	jingbai.ma

On Tue, Nov 12, 2013 at 06:51:58PM +0900, HATAYAMA Daisuke wrote:
> Add disable_cpu_apicid kernel parameter. To use this kernel parameter,
> specify an initial APIC ID of the corresponding CPU you want to
> disable.
> 
> This is mostly used for the kdump 2nd kernel to disable BSP to wake up
> multiple CPUs without causing system reset or hang due to sending INIT
> from AP to BSP.
> 
> Kdump users first figure out initial APIC ID of the BSP, CPU0 in the
> 1st kernel, for example from /proc/cpuinfo and then set up this kernel
> parameter for the 2nd kernel using the obtained APIC ID.
> 
> However, doing this procedure at each boot time manually is awkward,
> which should be automatically done by user-land service scripts, for
> example, kexec-tools on fedora/RHEL distributions.
> 
> This design is more flexible than disabling BSP in kernel boot time
> automatically in that in kernel boot time we have no choice but
> referring to ACPI/MP table to obtain initial APIC ID for BSP, meaning
> that the method is not applicable to the systems without such BIOS
> tables.
> 
> One assumption behind this design is that users get initial APIC ID of
> the BSP in still healthy state and so BSP is uniquely kept in
> CPU0. Thus, through the kernel parameter, only one initial APIC ID can
> be specified.
> 
> Signed-off-by: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
> ---
>  arch/x86/kernel/apic/apic.c |   29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
> 
> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
> index b60ad92..075bf23 100644
> --- a/arch/x86/kernel/apic/apic.c
> +++ b/arch/x86/kernel/apic/apic.c
> @@ -78,6 +78,13 @@ unsigned int max_physical_apicid;
>  physid_mask_t phys_cpu_present_map;
>  
>  /*
> + * Processor to be disabled specified by kernel parameter
> + * disable_cpu_apicid=<int>, mostly used for the kdump 2nd kernel to
> + * avoid undefined behaviour caused by sending INIT from AP to BSP.
> + */
> +unsigned int disabled_cpu_apicid = BAD_APICID;
> +
> +/*
>   * Map cpu index to physical APIC ID
>   */
>  DEFINE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_cpu_to_apicid, BAD_APICID);
> @@ -2117,6 +2124,19 @@ void generic_processor_info(int apicid, int version)
>  	bool boot_cpu_detected = physid_isset(boot_cpu_physical_apicid,
>  				phys_cpu_present_map);
>  
> +	if (disabled_cpu_apicid != BAD_APICID &&
> +	    disabled_cpu_apicid != boot_cpu_physical_apicid &&
> +	    disabled_cpu_apicid == apicid) {
> +		int thiscpu = num_processors + disabled_cpus;
> +
> +		pr_warning("ACPI: Disable specified CPU."
> +			   " Processor %d/0x%x ignored.\n",
> +			   thiscpu, apicid);

How am I to parse this message - that 'thiscpu' is being disabled
currently? What does "Processor ... ignored" mean?

Why not just write:

	ACPI: Disabling requested CPU %d (APIC ID: 0x%x)

and everyone knows what's happening?

Thanks.

-- 
Regards/Gruss,
    Boris.

Sent from a fat crate under my desk. Formatting is fine.
--

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

* Re: [PATCH v5 2/3] x86, apic: Add disable_cpu_apicid kernel parameter
  2013-11-12 10:44   ` Borislav Petkov
@ 2013-11-14 10:42     ` HATAYAMA Daisuke
  0 siblings, 0 replies; 7+ messages in thread
From: HATAYAMA Daisuke @ 2013-11-14 10:42 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: hpa, ebiederm, vgoyal, kexec, linux-kernel, akpm, fengguang.wu,
	jingbai.ma

(2013/11/12 19:44), Borislav Petkov wrote:
> On Tue, Nov 12, 2013 at 06:51:58PM +0900, HATAYAMA Daisuke wrote:
>> Add disable_cpu_apicid kernel parameter. To use this kernel parameter,
>> specify an initial APIC ID of the corresponding CPU you want to
>> disable.
>>
>> This is mostly used for the kdump 2nd kernel to disable BSP to wake up
>> multiple CPUs without causing system reset or hang due to sending INIT
>> from AP to BSP.
>>
>> Kdump users first figure out initial APIC ID of the BSP, CPU0 in the
>> 1st kernel, for example from /proc/cpuinfo and then set up this kernel
>> parameter for the 2nd kernel using the obtained APIC ID.
>>
>> However, doing this procedure at each boot time manually is awkward,
>> which should be automatically done by user-land service scripts, for
>> example, kexec-tools on fedora/RHEL distributions.
>>
>> This design is more flexible than disabling BSP in kernel boot time
>> automatically in that in kernel boot time we have no choice but
>> referring to ACPI/MP table to obtain initial APIC ID for BSP, meaning
>> that the method is not applicable to the systems without such BIOS
>> tables.
>>
>> One assumption behind this design is that users get initial APIC ID of
>> the BSP in still healthy state and so BSP is uniquely kept in
>> CPU0. Thus, through the kernel parameter, only one initial APIC ID can
>> be specified.
>>
>> Signed-off-by: HATAYAMA Daisuke <d.hatayama@jp.fujitsu.com>
>> ---
>>   arch/x86/kernel/apic/apic.c |   29 +++++++++++++++++++++++++++++
>>   1 file changed, 29 insertions(+)
>>
>> diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
>> index b60ad92..075bf23 100644
>> --- a/arch/x86/kernel/apic/apic.c
>> +++ b/arch/x86/kernel/apic/apic.c
>> @@ -78,6 +78,13 @@ unsigned int max_physical_apicid;
>>   physid_mask_t phys_cpu_present_map;
>>
>>   /*
>> + * Processor to be disabled specified by kernel parameter
>> + * disable_cpu_apicid=<int>, mostly used for the kdump 2nd kernel to
>> + * avoid undefined behaviour caused by sending INIT from AP to BSP.
>> + */
>> +unsigned int disabled_cpu_apicid = BAD_APICID;
>> +
>> +/*
>>    * Map cpu index to physical APIC ID
>>    */
>>   DEFINE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_cpu_to_apicid, BAD_APICID);
>> @@ -2117,6 +2124,19 @@ void generic_processor_info(int apicid, int version)
>>   	bool boot_cpu_detected = physid_isset(boot_cpu_physical_apicid,
>>   				phys_cpu_present_map);
>>
>> +	if (disabled_cpu_apicid != BAD_APICID &&
>> +	    disabled_cpu_apicid != boot_cpu_physical_apicid &&
>> +	    disabled_cpu_apicid == apicid) {
>> +		int thiscpu = num_processors + disabled_cpus;
>> +
>> +		pr_warning("ACPI: Disable specified CPU."
>> +			   " Processor %d/0x%x ignored.\n",
>> +			   thiscpu, apicid);
>
> How am I to parse this message - that 'thiscpu' is being disabled
> currently? What does "Processor ... ignored" mean?
>
> Why not just write:
>
> 	ACPI: Disabling requested CPU %d (APIC ID: 0x%x)
>
> and everyone knows what's happening?
>

It seems "Disabling requested CPU %d" part is by far better than mine.
I'll apply this in the next patch.

For the latter part "(APIC ID: 0x%x)", there are other two messages about
disabling processors and I tried to use a message consistent with these.
So, I think "Processor %d/0x%x ignored.\n" should be used.

         /*
          * If boot cpu has not been detected yet, then only allow upto
          * nr_cpu_ids - 1 processors and keep one slot free for boot cpu
          */
         if (!boot_cpu_detected && num_processors >= nr_cpu_ids - 1 &&
             apicid != boot_cpu_physical_apicid) {
                 int thiscpu = max + disabled_cpus - 1;

                 pr_warning(
                         "ACPI: NR_CPUS/possible_cpus limit of %i almost"
                         " reached. Keeping one slot for boot cpu."
                         "  Processor %d/0x%x ignored.\n", max, thiscpu, apicid);

                 disabled_cpus++;
                 return;
         }

         if (num_processors >= nr_cpu_ids) {
                 int thiscpu = max + disabled_cpus;

                 pr_warning(
                         "ACPI: NR_CPUS/possible_cpus limit of %i reached."
                         "  Processor %d/0x%x ignored.\n", max, thiscpu, apicid);

                 disabled_cpus++;
                 return;
         }

-- 
Thanks.
HATAYAMA, Daisuke


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

* Re: [PATCH v5 1/3] x86, apic: add bsp_physical_apicid
  2013-11-12  9:51 ` [PATCH v5 1/3] x86, apic: add bsp_physical_apicid HATAYAMA Daisuke
@ 2013-11-14 11:16   ` HATAYAMA Daisuke
  0 siblings, 0 replies; 7+ messages in thread
From: HATAYAMA Daisuke @ 2013-11-14 11:16 UTC (permalink / raw)
  To: hpa
  Cc: ebiederm, vgoyal, kexec, linux-kernel, bp, akpm, fengguang.wu,
	jingbai.ma

Hello HPA,

I have another question relevant to http://lkml.org/lkml/2013/11/12/311.

(2013/11/12 18:51), HATAYAMA Daisuke wrote:
<cut>
> @@ -2589,3 +2593,14 @@ static int __init lapic_insert_resource(void)
>    * that is using request_resource
>    */
>   late_initcall(lapic_insert_resource);
> +
> +void __init bsp_physical_apicid_init(void)
> +{
> +	u32 l, h;
> +
> +	if (boot_cpu_data.x86 >= 6 && cpu_has_apic) {
> +		rdmsr(MSR_IA32_APICBASE, l, h);
> +		if (!(l & MSR_IA32_APICBASE_BSP))
> +			bsp_physical_apicid = boot_cpu_data.initial_apicid;
> +	}
> +}

So, rigrously, we should not use rdmsr for MSR_IA32_APICBASE here since this can
return wrong value on some clustered system?

At least all the mpparse.c, amdtopology.c and visws_quirks.c has referred to
MP table only, so there's no issue by simply dropping this code.

> diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
> index 2793d1f..28bea2c 100644
> --- a/arch/x86/kernel/cpu/common.c
> +++ b/arch/x86/kernel/cpu/common.c
> @@ -681,7 +681,7 @@ static void identify_cpu_without_cpuid(struct cpuinfo_x86 *c)
>  /*
>   * Do minimum CPU detection early.
>   * Fields really needed: vendor, cpuid_level, family, model, mask,
> - * cache alignment.
> + * cache alignment, initial_apicid.
>   * The others are not touched to avoid unwanted side effects.
>   *
>   * WARNING: this function is only called on the BP.  Don't add code here
> @@ -725,6 +725,9 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c)
>  		this_cpu->c_bsp_init(c);
>
>  	setup_force_cpu_cap(X86_FEATURE_ALWAYS);
> +
> +	if (c->cpuid_level >= 0x00000001)
> +		c->initial_apicid = (cpuid_ebx(1) >> 24) & 0xFF;
>  }

Can the initial APID IDs obtained in this way using cpuid duplicate on some
clustered system?

Can we consider each apic->get_apic_id() method return *initial* APIC ID?
According to 8.4 Multiple-Processor (MP) Initialization, Intel System Programming
Guide, the APID IDs written into MADT and MP table are the initial ones.
So, APID IDs to be compared with these must be the initial ones.

-- 
Thanks.
HATAYAMA, Daisuke


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

end of thread, other threads:[~2013-11-14 11:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-11-12  9:51 [PATCH v5 0/3] x86, apic, kexec: Add disable_cpu_apic kernel parameter HATAYAMA Daisuke
2013-11-12  9:51 ` [PATCH v5 1/3] x86, apic: add bsp_physical_apicid HATAYAMA Daisuke
2013-11-14 11:16   ` HATAYAMA Daisuke
2013-11-12  9:51 ` [PATCH v5 2/3] x86, apic: Add disable_cpu_apicid kernel parameter HATAYAMA Daisuke
2013-11-12 10:44   ` Borislav Petkov
2013-11-14 10:42     ` HATAYAMA Daisuke
2013-11-12  9:52 ` [PATCH v5 3/3] Documentation, x86, apic, kexec: " HATAYAMA Daisuke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).