2012-05-10 16:28:25

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v6 0/12] x86: Arbitrary CPU hot(un)plug support

From: Fenghua Yu <[email protected]>

CPU0 or BSP (Bootstrap Processor) has been the last processor that can not be
hot removed on x86. This patch set implements CPU0 or BSP online and offline
and removes this obstacle to CPU hotplug.

RAS needs the feature. If socket0 needs to be hotplugged for any reason (any
thread on socket0 is bad, shared cache issue, uncore issue, etc), CPU0 is
required to be offline or hot replaced to keep the system run. For example,
starting with Core Duo, if you have a system that is reporting cache problems
via the "yellow" status in the MCi_STATUS msr, then there is benefit in simply
soft off-lining the cores that share that cache - assuming that leaves you at
least one online core. A single socket system with L3 cache troubles is not
helped - but problems in L1/L2 cache, or on multi-socket systems can be avoided.
They are already being avoided for the cases where CPU0 is not involved.
This patchset can help L1/L2 cache problem in CPU0 or L3 cache problem on the
socket with CPU0 in a multi-socket system.

v6: If CPU0 is offlined during boot time in CPU0 hotplug debug mode, put CPU0
online again before resuming from hibernation and disable x2apic and xapic.Don't
set __CPUINIT for start_cpu0() in head_32.S. Clean up CPU0 wake up nmi handler
after callin and callout sync. In a period (3 seconds), check if CPU0 wake up
NMI is handled after offlined CPU0 exits from mwait.

v5: Add CONFIG_BOOTPARAM_HOTPLUG_CPU0 and CONFIG_DEBUG_HOTPLUG_CPU0. Simplify
duplicate xstate_size init check. Wakeup CPU0 via nmi instead INITs. Add
mcheck_cpu_init when CPU0 online. Change variable bsp_hotpluggable to
cpu0_hotpluggable with __initdata qualifier.

v4: Add __read_mostly for internal bsp_hotpluggable variable. Add my email
address in cpu-hotplug.txt document. A wording change in comment.

v3: Register a pm notifier to check if CPU0 is online before hibernate/suspend.
Small wording changes in document and print info.

v2: Add locking changes between cpu hotplug and hibernate/suspend. Change PIC
irq bound to CPU0 detection.

Fenghua Yu (12):
Documentations/cpu-hotplug.tx, kernel-parameters.txt: Add x86 CPU0
online/offline feature
x86/Kconfig: Add config switch for CPU0 hotplug
x86/topology.c: Support functions for CPU0 online/offline
x86/smpboot.c: Don't offline CPU0 if any irq can not be migrated out
of it and remove CPU0 check in smp_callin()
x86/power/cpu.c: Don't hibernate/suspend if CPU0 is offline
x86/head_64.S: Define start_cpu0
x86/head_32.S: Define start_cpu0
x86/smpboot.c: Wake up CPU0 via NMI instead of INITs
x86/common.c: Init CPU0 data during CPU0 online
x86/mtrr/main.c: Ask the first online CPU to save mtrr
x86/i387.c: Thread xstate is initialized only on CPU0 once
x86/topology.c: debug CPU0 hotplug

Documentation/cpu-hotplug.txt | 24 ++++++
Documentation/kernel-parameters.txt | 14 +++
arch/x86/Kconfig | 44 ++++++++++
arch/x86/include/asm/cpu.h | 4 +
arch/x86/include/asm/processor.h | 1 +
arch/x86/kernel/cpu/common.c | 17 +++-
arch/x86/kernel/cpu/mtrr/main.c | 9 ++-
arch/x86/kernel/head_32.S | 12 +++
arch/x86/kernel/head_64.S | 15 ++++
arch/x86/kernel/i387.c | 6 +-
arch/x86/kernel/smpboot.c | 154 ++++++++++++++++++++++++++++++++---
arch/x86/kernel/topology.c | 78 ++++++++++++++++--
arch/x86/power/cpu.c | 82 +++++++++++++++++++
13 files changed, 434 insertions(+), 26 deletions(-)


2012-05-10 16:28:32

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v6 11/12] x86/i387.c: Thread xstate is initialized only on CPU0 once

From: Fenghua Yu <[email protected]>

init_thread_xstate() is only called once to avoid overriding xstate_size during
boot time or during CPU hotplug.

Signed-off-by: Fenghua Yu <[email protected]>
---
arch/x86/kernel/i387.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index 2d6e649..9d9ce97 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -188,7 +188,11 @@ void __cpuinit fpu_init(void)
cr0 |= X86_CR0_EM;
write_cr0(cr0);

- if (!smp_processor_id())
+ /*
+ * init_thread_xstate is only called once to avoid overriding
+ * xstate_size during boot time or during CPU hotplug.
+ */
+ if (xstate_size == 0)
init_thread_xstate();

mxcsr_feature_mask_init();
--
1.6.0.3

2012-05-10 16:28:30

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v6 12/12] x86/topology.c: debug CPU0 hotplug

From: Fenghua Yu <[email protected]>

CONFIG_DEBUG_HOTPLUG_CPU0 is for debuging the CPU0 hotplug feature. The switch
offlines CPU0 as soon as possible and boots userspace up with CPU0 offlined.
User can online CPU0 back after boot time. The default value of the switch is
off.

To debug CPU0 hotplug, you need to enable CPU0 offline/online feature by either
turning on CONFIG_BOOTPARAM_HOTPLUG_CPU0 during compilation or giving
cpu0_hotplug kernel parameter at boot.

It's safe and early place to take down CPU0 after all hotplug notifiers
are installed and SMP is booted.

Signed-off-by: Fenghua Yu <[email protected]>
---
arch/x86/Kconfig | 15 +++++++++++++
arch/x86/include/asm/cpu.h | 3 ++
arch/x86/kernel/topology.c | 51 ++++++++++++++++++++++++++++++++++++++++++++
arch/x86/power/cpu.c | 38 ++++++++++++++++++++++++++++++++
4 files changed, 107 insertions(+), 0 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index b48bd21..904bf7c 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1748,6 +1748,21 @@ config BOOTPARAM_HOTPLUG_CPU0
You still can enable the CPU0 hotplug feature at boot by kernel
parameter cpu0_hotplug.

+config DEBUG_HOTPLUG_CPU0
+ def_bool n
+ prompt "Debug CPU0 hotplug"
+ depends on HOTPLUG_CPU && EXPERIMENTAL
+ ---help---
+ Enabling this option offlines CPU0 (if CPU0 can be offlined) as
+ soon as possible and boots up userspace with CPU0 offlined. User
+ can online CPU0 back after boot time.
+
+ To debug CPU0 hotplug, you need to enable CPU0 offline/online
+ feature by either turning on CONFIG_BOOTPARAM_HOTPLUG_CPU0 during
+ compilation or giving cpu0_hotplug kernel parameter at boot.
+
+ If unuser, say N.
+
config COMPAT_VDSO
def_bool y
prompt "Compat VDSO support"
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index a119572..5f9a124 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -29,6 +29,9 @@ struct x86_cpu {
extern int arch_register_cpu(int num);
extern void arch_unregister_cpu(int);
extern void __cpuinit start_cpu0(void);
+#ifdef CONFIG_DEBUG_HOTPLUG_CPU0
+extern int _debug_hotplug_cpu(int cpu, int action);
+#endif
#endif

DECLARE_PER_CPU(int, cpu_state);
diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index 8cec15c..7cc7990 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -49,6 +49,57 @@ static int __init enable_cpu0_hotplug(char *str)
__setup("cpu0_hotplug", enable_cpu0_hotplug);
#endif

+#ifdef CONFIG_DEBUG_HOTPLUG_CPU0
+/*
+ * This function offlines a CPU as early as possible and allows userspace to
+ * boot up without the CPU. The CPU can be onlined back by user after boot.
+ *
+ * This is only called for debugging CPU offline/online feature.
+ */
+int __ref _debug_hotplug_cpu(int cpu, int action)
+{
+ struct device *dev = get_cpu_device(cpu);
+ int ret;
+
+ if (!cpu_is_hotpluggable(cpu))
+ return -EINVAL;
+
+ cpu_hotplug_driver_lock();
+
+ switch (action) {
+ case 0:
+ ret = cpu_down(cpu);
+ if (!ret) {
+ pr_info("CPU %u is now offline\n", cpu);
+ kobject_uevent(&dev->kobj, KOBJ_OFFLINE);
+ } else
+ pr_debug("Can't offline CPU%d.\n", cpu);
+ break;
+ case 1:
+ ret = cpu_up(cpu);
+ if (!ret)
+ kobject_uevent(&dev->kobj, KOBJ_ONLINE);
+ else
+ pr_debug("Can't online CPU%d.\n", cpu);
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+ cpu_hotplug_driver_unlock();
+
+ return ret;
+}
+
+static int __init debug_hotplug_cpu(void)
+{
+ _debug_hotplug_cpu(0, 0);
+ return 0;
+}
+
+device_initcall_sync(debug_hotplug_cpu);
+#endif /* CONFIG_DEBUG_HOTPLUG_CPU0 */
+
int __ref arch_register_cpu(int num)
{
/*
diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c
index 3338609..abc748a 100644
--- a/arch/x86/power/cpu.c
+++ b/arch/x86/power/cpu.c
@@ -21,6 +21,7 @@
#include <asm/suspend.h>
#include <asm/debugreg.h>
#include <asm/fpu-internal.h> /* pcntxt_mask */
+#include <asm/cpu.h>

#ifdef CONFIG_X86_32
static struct saved_context saved_context;
@@ -263,6 +264,43 @@ static int bsp_pm_callback(struct notifier_block *nb, unsigned long action,
case PM_HIBERNATION_PREPARE:
ret = bsp_check();
break;
+#ifdef CONFIG_DEBUG_HOTPLUG_CPU0
+ case PM_RESTORE_PREPARE:
+ /*
+ * When system resumes from hibernation, online CPU0 because
+ * 1. it's required for resume and
+ * 2. the CPU was online before hibernation
+ */
+ if (!cpu_online(0))
+ _debug_hotplug_cpu(0, 1);
+ break;
+ case PM_POST_RESTORE:
+ /*
+ * When a resume really happens, this code won't be called.
+ *
+ * This code is called only when user space hibernation software
+ * prepares for snapshot device during boot time. So we just
+ * call _debug_hotplug_cpu() to restore to CPU0's state prior to
+ * preparing the snapshot device.
+ *
+ * This works for normal boot case in our CPU0 hotplug debug
+ * mode, i.e. CPU0 is offline and user mode hibernation
+ * software initializes during boot time.
+ *
+ * If CPU0 is online and user application accesses snapshot
+ * device after boot time, this will offline CPU0 and user may
+ * see different CPU0 state before and after accessing
+ * the snapshot device. But hopefully this is not a case when
+ * user debugging CPU0 hotplug. Even if users hit this case,
+ * they can easily online CPU0 back.
+ *
+ * To simplify this debug code, we only consider normal boot
+ * case. Otherwise we need to remember CPU0's state and restore
+ * to that state and resolve racy conditions etc.
+ */
+ _debug_hotplug_cpu(0, 0);
+ break;
+#endif
default:
break;
}
--
1.6.0.3

2012-05-10 16:29:02

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v6 10/12] x86/mtrr/main.c: Ask the first online CPU to save mtrr

From: Fenghua Yu <[email protected]>

Ask the first online CPU to save mtrr instead of asking BSP. BSP could be
offline when mtrr_save_state() is called.

Signed-off-by: Fenghua Yu <[email protected]>
---
arch/x86/kernel/cpu/mtrr/main.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
index 6b96110..e4c1a41 100644
--- a/arch/x86/kernel/cpu/mtrr/main.c
+++ b/arch/x86/kernel/cpu/mtrr/main.c
@@ -695,11 +695,16 @@ void mtrr_ap_init(void)
}

/**
- * Save current fixed-range MTRR state of the BSP
+ * Save current fixed-range MTRR state of the first cpu in cpu_online_mask.
*/
void mtrr_save_state(void)
{
- smp_call_function_single(0, mtrr_save_fixed_ranges, NULL, 1);
+ int first_cpu;
+
+ get_online_cpus();
+ first_cpu = cpumask_first(cpu_online_mask);
+ smp_call_function_single(first_cpu, mtrr_save_fixed_ranges, NULL, 1);
+ put_online_cpus();
}

void set_mtrr_aps_delayed_init(void)
--
1.6.0.3

2012-05-10 16:28:27

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v6 01/12] Documentations/cpu-hotplug.tx, kernel-parameters.txt: Add x86 CPU0 online/offline feature

From: Fenghua Yu <[email protected]>

If CONFIG_BOOTPARAM_HOTPLUG_CPU0 is turned on, CPU0 is hotpluggable. Otherwise,
by default CPU0 is not hotpluggable and kernel parameter cpu0_hotplug enables
CPU0 online/offline feature.

The documentations point out two known CPU0 dependencies. First, resume from
hibernate or suspend always starts from CPU0. So hibernate and suspend are
prevented if CPU0 is offline. Another dependency is PIC interrupts always go
to CPU0.

It's said that some machines may depend on CPU0 to poweroff/reboot. But I
haven't seen such dependency on a few tested machines.

Please let me know if you see any CPU0 dependencies on your machine.

Signed-off-by: Fenghua Yu <[email protected]>
---
Documentation/cpu-hotplug.txt | 24 ++++++++++++++++++++++++
Documentation/kernel-parameters.txt | 14 ++++++++++++++
2 files changed, 38 insertions(+), 0 deletions(-)

diff --git a/Documentation/cpu-hotplug.txt b/Documentation/cpu-hotplug.txt
index 66ef8f3..9f40135 100644
--- a/Documentation/cpu-hotplug.txt
+++ b/Documentation/cpu-hotplug.txt
@@ -207,6 +207,30 @@ by making it not-removable.

In such cases you will also notice that the online file is missing under cpu0.

+Q: Is CPU0 removable on X86?
+A: Yes. If kernel is compiled with CONFIG_BOOTPARAM_HOTPLUG_CPU0=y, CPU0 is
+removable by default. Otherwise, CPU0 is also removable by kernel option
+cpu0_hotplug.
+
+But some features depend on CPU0. Two known dependencies are:
+
+1. Resume from hibernate/suspend depends on CPU0. Hibernate/suspend will fail if
+CPU0 is offline and you need to online CPU0 before hibernate/suspend can
+continue.
+2. PIC interrupts also depend on CPU0. CPU0 can't be removed if a PIC interrupt
+is detected.
+
+It's said poweroff/reboot may depend on CPU0 on some machines although I haven't
+seen any poweroff/reboot failure so far after CPU0 is offline on a few tested
+machines.
+
+Please let me know if you know or see any other dependencies of CPU0.
+
+If the dependencies are under your control, you can turn on CPU0 hotplug feature
+either by CONFIG_BOOTPARAM_HOTPLUG_CPU0 or by kernel parameter cpu0_hotplug.
+
+--Fenghua Yu <[email protected]>
+
Q: How do i find out if a particular CPU is not removable?
A: Depending on the implementation, some architectures may show this by the
absence of the "online" file. This is done if it can be determined ahead of
diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index c1601e5..1dc732f 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -1923,6 +1923,20 @@ bytes respectively. Such letter suffixes can also be entirely omitted.

nox2apic [X86-64,APIC] Do not enable x2APIC mode.

+ cpu0_hotplug [X86] Turn on CPU0 hotplug feature when
+ CONFIG_BOOTPARAM_HOTPLUG_CPU0 is off.
+ Some features depend on CPU0. Known dependencies are:
+ 1. Resume from suspend/hibernate depends on CPU0.
+ Suspend/hibernate will fail if CPU0 is offline and you
+ need to online CPU0 before suspend/hibernate.
+ 2. PIC interrupts also depend on CPU0. CPU0 can't be
+ removed if a PIC interrupt is detected.
+ It's said poweroff/reboot may depend on CPU0 on some
+ machines although I haven't seen such issues so far
+ after CPU0 is offline on a few tested machines.
+ If the dependencies are under your control, you can
+ turn on cpu0_hotplug.
+
nptcg= [IA-64] Override max number of concurrent global TLB
purges which is reported from either PAL_VM_SUMMARY or
SAL PALO.
--
1.6.0.3

2012-05-10 16:29:05

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v6 09/12] x86/common.c: Init CPU0 data during CPU0 online

From: Fenghua Yu <[email protected]>

During CPU0 online, enable x2apic, re-init mcheck, set_numa_node, add numa mask,
and enable sep cpu for CPU0.

Signed-off-by: Fenghua Yu <[email protected]>
---
arch/x86/include/asm/processor.h | 1 +
arch/x86/kernel/cpu/common.c | 17 ++++++++++++++---
2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index ccbb1ea..81d6993 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -169,6 +169,7 @@ extern struct pt_regs *idle_regs(struct pt_regs *);

extern void early_cpu_init(void);
extern void identify_boot_cpu(void);
+extern void identify_boot_cpu_online(void);
extern void identify_secondary_cpu(struct cpuinfo_x86 *);
extern void print_cpu_info(struct cpuinfo_x86 *);
void print_cpu_msr(struct cpuinfo_x86 *);
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index cf79302..7f9f481 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -913,6 +913,18 @@ void __init identify_boot_cpu(void)
#endif
}

+void __cpuinit identify_boot_cpu_online(void)
+{
+ mcheck_cpu_init(&boot_cpu_data);
+
+#ifdef CONFIG_NUMA
+ numa_add_cpu(smp_processor_id());
+#endif
+#ifdef CONFIG_X86_32
+ enable_sep_cpu();
+#endif
+}
+
void __cpuinit identify_secondary_cpu(struct cpuinfo_x86 *c)
{
BUG_ON(c == &boot_cpu_data);
@@ -1185,7 +1197,7 @@ void __cpuinit cpu_init(void)
oist = &per_cpu(orig_ist, cpu);

#ifdef CONFIG_NUMA
- if (cpu != 0 && percpu_read(numa_node) == 0 &&
+ if (percpu_read(numa_node) == 0 &&
early_cpu_to_node(cpu) != NUMA_NO_NODE)
set_numa_node(early_cpu_to_node(cpu));
#endif
@@ -1217,8 +1229,7 @@ void __cpuinit cpu_init(void)
barrier();

x86_configure_nx();
- if (cpu != 0)
- enable_x2apic();
+ enable_x2apic();

/*
* set up and load the per-CPU TSS
--
1.6.0.3

2012-05-10 16:30:59

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v6 06/12] x86/head_64.S: Define start_cpu0

From: Fenghua Yu <[email protected]>

start_cpu0() is defined in head_64.S for 64-bit. The function sets up stack and
jumps to start_secondary() for CPU0 wake up.

Signed-off-by: Fenghua Yu <[email protected]>
---
arch/x86/kernel/head_64.S | 15 +++++++++++++++
1 files changed, 15 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 7a40f24..a91ce03 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -256,6 +256,21 @@ ENTRY(secondary_startup_64)
pushq %rax # target address in negative space
lretq

+#ifdef CONFIG_HOTPLUG_CPU
+/*
+ * Boot CPU0 entry point. It's called from play_dead(). Everything has been set
+ * up already except stack. We just set up stack here. Then call
+ * start_secondary().
+ */
+ENTRY(start_cpu0)
+ movq stack_start(%rip),%rsp
+ movq initial_code(%rip),%rax
+ pushq $0 # fake return address to stop unwinder
+ pushq $__KERNEL_CS # set correct cs
+ pushq %rax # target address in negative space
+ lretq
+#endif
+
/* SMP bootup changes these two */
__REFDATA
.align 8
--
1.6.0.3

2012-05-10 16:30:57

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v6 07/12] x86/head_32.S: Define start_cpu0

From: Fenghua Yu <[email protected]>

start_cpu0() is defined in head_32.S for 32-bit. The function sets up stack and
jumps to start_secondary() for CPU0 wake up.

Signed-off-by: Fenghua Yu <[email protected]>
---
arch/x86/kernel/head_32.S | 12 ++++++++++++
1 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 463c979..a441303 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -266,6 +266,18 @@ num_subarch_entries = (. - subarch_entries) / 4
jmp default_entry
#endif /* CONFIG_PARAVIRT */

+#ifdef CONFIG_HOTPLUG_CPU
+/*
+ * Boot CPU0 entry point. It's called from play_dead(). Everything has been set
+ * up already except stack. We just set up stack here. Then call
+ * start_secondary().
+ */
+ENTRY(start_cpu0)
+ movl stack_start, %ecx
+ movl %ecx, %esp
+ jmp *(initial_code)
+#endif
+
/*
* Non-boot CPU entry point; entered from trampoline.S
* We can't lgdt here, because lgdt itself uses a data segment, but
--
1.6.0.3

2012-05-10 16:30:55

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v6 08/12] x86/smpboot.c: Wake up CPU0 via NMI instead of INITs

From: Fenghua Yu <[email protected]>

Instead of waiting for STARTUP after INITs, BSP will execute
the BIOS boot-strap code which is not a desired behavior for
waking up BSP. To avoid the boot-strap code, wake up CPU0 by
NMI instead.

This works to wake up soft offlined CPU0 only. If CPU0 is hard offlined (i.e.
physically hot removed and then hot added), NMI won't wake it up. We'll change
this code in the future to wake up hard offlined CPU0 if real platform and
request are available.

AP is still waken up as before by INIT, INIT, STARTUP sequence.

Signed-off-by: Fenghua Yu <[email protected]>
---
arch/x86/include/asm/cpu.h | 1 +
arch/x86/kernel/smpboot.c | 112 +++++++++++++++++++++++++++++++++++++++++--
2 files changed, 107 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index 4564c8e..a119572 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -28,6 +28,7 @@ struct x86_cpu {
#ifdef CONFIG_HOTPLUG_CPU
extern int arch_register_cpu(int num);
extern void arch_unregister_cpu(int);
+extern void __cpuinit start_cpu0(void);
#endif

DECLARE_PER_CPU(int, cpu_state);
diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index fa3822e..fba37ad 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -134,15 +134,17 @@ static void __cpuinit smp_callin(void)
* we may get here before an INIT-deassert IPI reaches
* our local APIC. We have to wait for the IPI or we'll
* lock up on an APIC access.
+ *
+ * Since CPU0 is not wakened up by INIT, it doesn't wait for the IPI.
*/
- if (apic->wait_for_init_deassert)
+ cpuid = smp_processor_id();
+ if (apic->wait_for_init_deassert && cpuid != 0)
apic->wait_for_init_deassert(&init_deasserted);

/*
* (This works even if the APIC is not enabled.)
*/
phys_id = read_apic_id();
- cpuid = smp_processor_id();
if (cpumask_test_cpu(cpuid, cpu_callin_mask)) {
panic("%s: phys CPU#%d, CPU#%d already present??\n", __func__,
phys_id, cpuid);
@@ -231,6 +233,8 @@ static void __cpuinit smp_callin(void)
cpumask_set_cpu(cpuid, cpu_callin_mask);
}

+static int cpu0_logical_apicid;
+static int enable_start_cpu0;
/*
* Activate a secondary processor.
*/
@@ -246,6 +250,8 @@ notrace static void __cpuinit start_secondary(void *unused)
preempt_disable();
smp_callin();

+ enable_start_cpu0 = 0;
+
#ifdef CONFIG_X86_32
/* switch away from the initial page table */
load_cr3(swapper_pg_dir);
@@ -472,7 +478,7 @@ void __inquire_remote_apic(int apicid)
* won't ... remember to clear down the APIC, etc later.
*/
int __cpuinit
-wakeup_secondary_cpu_via_nmi(int logical_apicid, unsigned long start_eip)
+wakeup_secondary_cpu_via_nmi(int apicid, unsigned long start_eip)
{
unsigned long send_status, accept_status = 0;
int maxlvt;
@@ -480,7 +486,7 @@ wakeup_secondary_cpu_via_nmi(int logical_apicid, unsigned long start_eip)
/* Target chip */
/* Boot on the stack */
/* Kick the second */
- apic_icr_write(APIC_DM_NMI | apic->dest_logical, logical_apicid);
+ apic_icr_write(APIC_DM_NMI | apic->dest_logical, apicid);

pr_debug("Waiting for send to finish...\n");
send_status = safe_apic_wait_icr_idle();
@@ -640,6 +646,17 @@ static void __cpuinit announce_cpu(int cpu, int apicid)
node, cpu, apicid);
}

+static int wakeup_cpu0_nmi(unsigned int cmd, struct pt_regs *regs)
+{
+ int cpu;
+
+ cpu = smp_processor_id();
+ if (cpu == 0 && !cpu_online(cpu) && enable_start_cpu0)
+ return NMI_HANDLED;
+
+ return NMI_DONE;
+}
+
/*
* NOTE - on most systems this is a PHYSICAL apic ID, but on multiquad
* (ie clustered apic addressing mode), this is a LOGICAL apic ID.
@@ -651,6 +668,7 @@ static int __cpuinit do_boot_cpu(int apicid, int cpu, struct task_struct *idle)
unsigned long boot_error = 0;
unsigned long start_ip;
int timeout;
+ int cpu0_nmi_registered = 0;

alternatives_smp_switch(1);

@@ -705,8 +723,47 @@ static int __cpuinit do_boot_cpu(int apicid, int cpu, struct task_struct *idle)
*/
if (apic->wakeup_secondary_cpu)
boot_error = apic->wakeup_secondary_cpu(apicid, start_ip);
- else
- boot_error = wakeup_secondary_cpu_via_init(apicid, start_ip);
+ else {
+ if (cpu)
+ /*
+ * Wake up AP by INIT, INIT, STARTUP sequence.
+ */
+ boot_error = wakeup_secondary_cpu_via_init(apicid,
+ start_ip);
+ else {
+ /*
+ * Instead of waiting for STARTUP after INITs, BSP will
+ * execute the BIOS boot-strap code which is not a
+ * desired behavior for waking up BSP. To avoid the
+ * boot-strap code, wake up CPU0 by NMI instead.
+ *
+ * This works to wake up soft offlined CPU0 only. If
+ * CPU0 is hard offlined (i.e. physically hot removed
+ * and then hot added), NMI won't wake it up. We'll
+ * change this code in the future to wake up hard
+ * offlined CPU0 if real platform and request are
+ * available.
+ */
+ int id;
+
+ enable_start_cpu0 = 1;
+ /*
+ * Register a NMI handler to help wake up CPU0.
+ */
+ boot_error = register_nmi_handler(NMI_LOCAL,
+ wakeup_cpu0_nmi, 0, "wake_cpu0");
+
+ if (!boot_error) {
+ cpu0_nmi_registered = 1;
+ if (apic->dest_logical == APIC_DEST_LOGICAL)
+ id = cpu0_logical_apicid;
+ else
+ id = apicid;
+ boot_error = wakeup_secondary_cpu_via_nmi(id,
+ start_ip);
+ }
+ }
+ }

if (!boot_error) {
/*
@@ -772,6 +829,13 @@ static int __cpuinit do_boot_cpu(int apicid, int cpu, struct task_struct *idle)
*/
smpboot_restore_warm_reset_vector();
}
+ /*
+ * Clean up the nmi handler. Do this after the callin and callout sync
+ * to avoid impact of possible long unregister time.
+ */
+ if (cpu0_nmi_registered)
+ unregister_nmi_handler(NMI_LOCAL, "wake_cpu0");
+
return boot_error;
}

@@ -1021,6 +1085,8 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)
*/
setup_local_APIC();

+ cpu0_logical_apicid = GET_APIC_LOGICAL_ID(apic_read(APIC_LDR));
+
/*
* Enable IO APIC before setting up error vector
*/
@@ -1301,6 +1367,30 @@ void play_dead_common(void)
local_irq_disable();
}

+static bool wakeup_cpu0(void)
+{
+ unsigned int timeout;
+
+ if (smp_processor_id())
+ return false;
+
+ /*
+ * Wait up to 3 seconds to check if CPU0 wakeup NMI is handled.
+ * If there is no CPU0 wakeup NMI, go back to sleep.
+ */
+ for (timeout = 0; timeout < 30000; timeout++) {
+ /*
+ * Check if CPU0 wakeup NMI is issued and handled.
+ */
+ if (enable_start_cpu0)
+ return true;
+
+ udelay(100);
+ }
+
+ return false;
+}
+
/*
* We need to flush the caches before going to sleep, lest we have
* dirty data in our caches when we come back up.
@@ -1364,6 +1454,11 @@ static inline void mwait_play_dead(void)
__monitor(mwait_ptr, 0, 0);
mb();
__mwait(eax, 0);
+ /*
+ * If NMI wants to wake up CPU0, start CPU0.
+ */
+ if (wakeup_cpu0())
+ start_cpu0();
}
}

@@ -1374,6 +1469,11 @@ static inline void hlt_play_dead(void)

while (1) {
native_halt();
+ /*
+ * If NMI wants to wake up CPU0, start CPU0.
+ */
+ if (wakeup_cpu0())
+ start_cpu0();
}
}

--
1.6.0.3

2012-05-10 16:32:09

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v6 04/12] x86/smpboot.c: Don't offline CPU0 if any irq can not be migrated out of it and remove CPU0 check in smp_callin()

From: Fenghua Yu <[email protected]>

Don't offline CPU0 if any irq can not be migrated out of it.

Call identify_boot_cpu_online() for CPU0 in smp_callin() and continue to online
CPU0 in native_cpu_up().

Signed-off-by: Fenghua Yu <[email protected]>
---
arch/x86/kernel/smpboot.c | 42 +++++++++++++++++++++++++++++++++++-------
1 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index e543e02..fa3822e 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -121,8 +121,8 @@ EXPORT_PER_CPU_SYMBOL(cpu_info);
atomic_t init_deasserted;

/*
- * Report back to the Boot Processor.
- * Running on AP.
+ * Report back to the Boot Processor during boot time or to the caller processor
+ * during CPU online.
*/
static void __cpuinit smp_callin(void)
{
@@ -210,6 +210,13 @@ static void __cpuinit smp_callin(void)
pr_debug("Stack at about %p\n", &cpuid);

/*
+ * This function won't run on the BSP during boot time. It run
+ * on BSP only when BSP is offlined and onlined again.
+ */
+ if (cpuid == 0)
+ identify_boot_cpu_online();
+
+ /*
* This must be done before setting cpu_online_mask
* or calling notify_cpu_starting.
*/
@@ -778,7 +785,7 @@ int __cpuinit native_cpu_up(unsigned int cpu, struct task_struct *tidle)

pr_debug("++++++++++++++++++++=_---CPU UP %u\n", cpu);

- if (apicid == BAD_APICID || apicid == boot_cpu_physical_apicid ||
+ if (apicid == BAD_APICID ||
!physid_isset(apicid, phys_cpu_present_map) ||
!apic->apic_id_valid(apicid)) {
printk(KERN_ERR "%s: bad cpu %d\n", __func__, cpu);
@@ -1224,12 +1231,33 @@ int native_cpu_disable(void)
* Perhaps use cpufreq to drop frequency, but that could go
* into generic code.
*
- * We won't take down the boot processor on i386 due to some
+ * We won't take down the boot processor on x86 if some
* interrupts only being able to be serviced by the BSP.
- * Especially so if we're not using an IOAPIC -zwane
+ * Especially so if we're not using an IOAPIC
*/
- if (cpu == 0)
- return -EBUSY;
+ if (cpu == 0) {
+ int irq;
+ struct irq_desc *desc;
+ struct irq_data *data;
+ struct irq_chip *chip;
+
+ for_each_irq_desc(irq, desc) {
+ raw_spin_lock(&desc->lock);
+ if (!irq_has_action(irq)) {
+ raw_spin_unlock(&desc->lock);
+ continue;
+ }
+
+ data = irq_desc_get_irq_data(desc);
+ chip = irq_data_get_irq_chip(data);
+ if (!chip->irq_set_affinity) {
+ pr_debug("irq%d can't move out of BSP\n", irq);
+ raw_spin_unlock(&desc->lock);
+ return -EBUSY;
+ }
+ raw_spin_unlock(&desc->lock);
+ }
+ }

clear_local_APIC();

--
1.6.0.3

2012-05-10 16:32:05

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v6 05/12] x86/power/cpu.c: Don't hibernate/suspend if CPU0 is offline

From: Fenghua Yu <[email protected]>

Because x86 BIOS requires CPU0 to resume from sleep, suspend or hibernate
can't be executed if CPU0 is detected offline. To make suspend or hibernate and
further resume succeed, CPU0 must be online.

Signed-off-by: Fenghua Yu <[email protected]>
---
arch/x86/power/cpu.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
1 files changed, 44 insertions(+), 0 deletions(-)

diff --git a/arch/x86/power/cpu.c b/arch/x86/power/cpu.c
index 218cdb1..3338609 100644
--- a/arch/x86/power/cpu.c
+++ b/arch/x86/power/cpu.c
@@ -237,3 +237,47 @@ void restore_processor_state(void)
#ifdef CONFIG_X86_32
EXPORT_SYMBOL(restore_processor_state);
#endif
+
+/*
+ * When bsp_check() is called in hibernate and suspend, cpu hotplug
+ * is disabled already. So it's unnessary to handle race condition between
+ * cpumask query and cpu hotplug.
+ */
+static int bsp_check(void)
+{
+ if (cpumask_first(cpu_online_mask) != 0) {
+ printk(KERN_WARNING "CPU0 is offline.\n");
+ return -ENODEV;
+ }
+
+ return 0;
+}
+
+static int bsp_pm_callback(struct notifier_block *nb, unsigned long action,
+ void *ptr)
+{
+ int ret = 0;
+
+ switch (action) {
+ case PM_SUSPEND_PREPARE:
+ case PM_HIBERNATION_PREPARE:
+ ret = bsp_check();
+ break;
+ default:
+ break;
+ }
+ return notifier_from_errno(ret);
+}
+
+static int __init bsp_pm_check_init(void)
+{
+ /*
+ * Set this bsp_pm_callback as lower priority than
+ * cpu_hotplug_pm_callback. So cpu_hotplug_pm_callback will be called
+ * earlier to disable cpu hotplug before bsp online check.
+ */
+ pm_notifier(bsp_pm_callback, -INT_MAX);
+ return 0;
+}
+
+core_initcall(bsp_pm_check_init);
--
1.6.0.3

2012-05-10 16:33:05

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v6 02/12] x86/Kconfig: Add config switch for CPU0 hotplug

From: Fenghua Yu <[email protected]>

New config switch CONFIG_BOOTPARAM_HOTPLUG_CPU0 sets default state of whether
the CPU0 hotplug is on or off.

If the switch is off, CPU0 is not hotpluggable by default. But the CPU0 hotplug
feature can still be turned on by kernel parameter cpu0_hotplug at boot.

If the switch is on, CPU0 is always hotpluggable. The default value of
the switch is off.

Signed-off-by: Fenghua Yu <[email protected]>
---
arch/x86/Kconfig | 29 +++++++++++++++++++++++++++++
1 files changed, 29 insertions(+), 0 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 5e99fa6..b48bd21 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1719,6 +1719,35 @@ config HOTPLUG_CPU
automatically on SMP systems. )
Say N if you want to disable CPU hotplug.

+config BOOTPARAM_HOTPLUG_CPU0
+ bool "Set default setting of cpu0_hotpluggable"
+ default n
+ depends on HOTPLUG_CPU && EXPERIMENTAL
+ ---help---
+ Set whether default state of cpu0_hotpluggable is on or off.
+
+ Say Y here to enable CPU0 hotplug by default. If this switch
+ is turned on, there is no need to give cpu0_hotplug kernel
+ parameter and the CPU0 hotplug feature is enabled by default.
+
+ Please note: there are two known CPU0 dependencies if you want
+ to enable the CPU0 hotplug feature either by this switch or by
+ cpu0_hotplug kernel parameter.
+
+ First, resume from hibernate or suspend always starts from CPU0.
+ So hibernate and suspend are prevented if CPU0 is offline.
+
+ Second dependency is PIC interrupts always go to CPU0. CPU0 can not
+ offline if any interrupt can not migrate out of CPU0. There may
+ be other CPU0 dependencies.
+
+ Please make sure the dependencies are under your control before
+ you enable this feature.
+
+ Say N if you don't want to enable CPU0 hotplug feature by default.
+ You still can enable the CPU0 hotplug feature at boot by kernel
+ parameter cpu0_hotplug.
+
config COMPAT_VDSO
def_bool y
prompt "Compat VDSO support"
--
1.6.0.3

2012-05-10 16:33:02

by Fenghua Yu

[permalink] [raw]
Subject: [PATCH v6 03/12] x86/topology.c: Support functions for CPU0 online/offline

From: Fenghua Yu <[email protected]>

If CONFIG_BOOTPARAM_HOTPLUG_CPU is turned on, CPU0 hotplug feature is enabled
by default.

If CONFIG_BOOTPARAM_HOTPLUG_CPU is not turned on, CPU0 hotplug feature is not
enabled by default. The kernel parameter cpu0_hotplug can enable CPU0 hotplug
feature at boot.

Signed-off-by: Fenghua Yu <[email protected]>
---
arch/x86/kernel/topology.c | 27 ++++++++++++++++++++-------
1 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index 76ee977..8cec15c 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -35,18 +35,31 @@
static DEFINE_PER_CPU(struct x86_cpu, cpu_devices);

#ifdef CONFIG_HOTPLUG_CPU
+
+#ifdef CONFIG_BOOTPARAM_HOTPLUG_CPU0
+static int __initdata cpu0_hotpluggable = 1;
+#else
+static int __initdata cpu0_hotpluggable;
+static int __init enable_cpu0_hotplug(char *str)
+{
+ cpu0_hotpluggable = 1;
+ return 1;
+}
+
+__setup("cpu0_hotplug", enable_cpu0_hotplug);
+#endif
+
int __ref arch_register_cpu(int num)
{
/*
- * CPU0 cannot be offlined due to several
- * restrictions and assumptions in kernel. This basically
- * doesn't add a control file, one cannot attempt to offline
- * BSP.
+ * Two known BSP/CPU0 dependencies: Resume from suspend/hibernate
+ * depends on BSP. PIC interrupts depend on BSP.
*
- * Also certain PCI quirks require not to enable hotplug control
- * for all CPU's.
+ * If the BSP depencies are under control, one can tell kernel to
+ * enable BSP hotplug. This basically adds a control file and
+ * one can attempt to offline BSP.
*/
- if (num)
+ if (num || cpu0_hotpluggable)
per_cpu(cpu_devices, num).cpu.hotpluggable = 1;

return register_cpu(&per_cpu(cpu_devices, num).cpu, num);
--
1.6.0.3

2012-05-10 17:32:06

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v6 07/12] x86/head_32.S: Define start_cpu0

Can you check how this interacts with tip:x86/realmode?

Fenghua Yu <[email protected]> wrote:

>From: Fenghua Yu <[email protected]>
>
>start_cpu0() is defined in head_32.S for 32-bit. The function sets up
>stack and
>jumps to start_secondary() for CPU0 wake up.
>
>Signed-off-by: Fenghua Yu <[email protected]>
>---
> arch/x86/kernel/head_32.S | 12 ++++++++++++
> 1 files changed, 12 insertions(+), 0 deletions(-)
>
>diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
>index 463c979..a441303 100644
>--- a/arch/x86/kernel/head_32.S
>+++ b/arch/x86/kernel/head_32.S
>@@ -266,6 +266,18 @@ num_subarch_entries = (. - subarch_entries) / 4
> jmp default_entry
> #endif /* CONFIG_PARAVIRT */
>
>+#ifdef CONFIG_HOTPLUG_CPU
>+/*
>+ * Boot CPU0 entry point. It's called from play_dead(). Everything has
>been set
>+ * up already except stack. We just set up stack here. Then call
>+ * start_secondary().
>+ */
>+ENTRY(start_cpu0)
>+ movl stack_start, %ecx
>+ movl %ecx, %esp
>+ jmp *(initial_code)
>+#endif
>+
> /*
> * Non-boot CPU entry point; entered from trampoline.S
> * We can't lgdt here, because lgdt itself uses a data segment, but
>--
>1.6.0.3

--
Sent from my mobile phone. Please excuse brevity and lack of formatting.

2012-05-10 20:48:42

by Fenghua Yu

[permalink] [raw]
Subject: RE: [PATCH v6 07/12] x86/head_32.S: Define start_cpu0

> -----Original Message-----
> From: H. Peter Anvin [mailto:[email protected]]
> Sent: Thursday, May 10, 2012 10:31 AM
> To: Yu, Fenghua; Ingo Molnar; Thomas Gleixner; Linus Torvalds; Andrew
> Morton; Mallick, Asit K; Luck, Tony; Arjan Dan De Ven; Siddha, Suresh B;
> Brown, Len; Srivatssa S. Bhat; Randy Dunlap; Chen Gong; linux-kernel;
> linux-pm; x86
> Subject: Re: [PATCH v6 07/12] x86/head_32.S: Define start_cpu0
>
> Can you check how this interacts with tip:x86/realmode?

When CPU0 is waken up by NMI, it's in protect mode and jumps to start_cpu0() which only sets up stack and goes to initial_code. CPU0 wake up doesn't go through trampline and starts from real mode. This is different from waking up AP when AP goes through INIT/STARTUP sequence and starts from real mode.

Thanks.

-Fenghua
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2012-05-10 23:54:24

by H. Peter Anvin

[permalink] [raw]
Subject: RE: [PATCH v6 07/12] x86/head_32.S: Define start_cpu0

I just meant: please check if there are any conflicts ..

"Yu, Fenghua" <[email protected]> wrote:

>> -----Original Message-----
>> From: H. Peter Anvin [mailto:[email protected]]
>> Sent: Thursday, May 10, 2012 10:31 AM
>> To: Yu, Fenghua; Ingo Molnar; Thomas Gleixner; Linus Torvalds; Andrew
>> Morton; Mallick, Asit K; Luck, Tony; Arjan Dan De Ven; Siddha, Suresh
>B;
>> Brown, Len; Srivatssa S. Bhat; Randy Dunlap; Chen Gong; linux-kernel;
>> linux-pm; x86
>> Subject: Re: [PATCH v6 07/12] x86/head_32.S: Define start_cpu0
>>
>> Can you check how this interacts with tip:x86/realmode?
>
>When CPU0 is waken up by NMI, it's in protect mode and jumps to
>start_cpu0() which only sets up stack and goes to initial_code. CPU0
>wake up doesn't go through trampline and starts from real mode. This is
>different from waking up AP when AP goes through INIT/STARTUP sequence
>and starts from real mode.
>
>Thanks.
>
>-Fenghua

--
Sent from my mobile phone. Please excuse brevity and lack of formatting.

2012-05-11 12:06:04

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [PATCH v6 04/12] x86/smpboot.c: Don't offline CPU0 if any irq can not be migrated out of it and remove CPU0 check in smp_callin()

On Thu, 10 May 2012, Fenghua Yu wrote:
> From: Fenghua Yu <[email protected]>
>
> Don't offline CPU0 if any irq can not be migrated out of it.
>
> Call identify_boot_cpu_online() for CPU0 in smp_callin() and continue to online
> CPU0 in native_cpu_up().
>
> Signed-off-by: Fenghua Yu <[email protected]>
> ---
> arch/x86/kernel/smpboot.c | 42 +++++++++++++++++++++++++++++++++++-------
> 1 files changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
> index e543e02..fa3822e 100644
> --- a/arch/x86/kernel/smpboot.c
> +++ b/arch/x86/kernel/smpboot.c
> @@ -121,8 +121,8 @@ EXPORT_PER_CPU_SYMBOL(cpu_info);
> atomic_t init_deasserted;
>
> /*
> - * Report back to the Boot Processor.
> - * Running on AP.
> + * Report back to the Boot Processor during boot time or to the caller processor
> + * during CPU online.
> */
> static void __cpuinit smp_callin(void)
> {
> @@ -210,6 +210,13 @@ static void __cpuinit smp_callin(void)
> pr_debug("Stack at about %p\n", &cpuid);
>
> /*
> + * This function won't run on the BSP during boot time. It run
> + * on BSP only when BSP is offlined and onlined again.
> + */
> + if (cpuid == 0)
> + identify_boot_cpu_online();

No, this is not how we do things.

If this is required, then hell we do it at boot time and not at a
random place in the cpu hotplug code.

> +
> + /*
> * This must be done before setting cpu_online_mask
> * or calling notify_cpu_starting.
> */
> @@ -778,7 +785,7 @@ int __cpuinit native_cpu_up(unsigned int cpu, struct task_struct *tidle)
>
> pr_debug("++++++++++++++++++++=_---CPU UP %u\n", cpu);
>
> - if (apicid == BAD_APICID || apicid == boot_cpu_physical_apicid ||
> + if (apicid == BAD_APICID ||
> !physid_isset(apicid, phys_cpu_present_map) ||
> !apic->apic_id_valid(apicid)) {
> printk(KERN_ERR "%s: bad cpu %d\n", __func__, cpu);
> @@ -1224,12 +1231,33 @@ int native_cpu_disable(void)
> * Perhaps use cpufreq to drop frequency, but that could go
> * into generic code.
> *
> - * We won't take down the boot processor on i386 due to some
> + * We won't take down the boot processor on x86 if some
> * interrupts only being able to be serviced by the BSP.
> - * Especially so if we're not using an IOAPIC -zwane
> + * Especially so if we're not using an IOAPIC
> */
> - if (cpu == 0)
> - return -EBUSY;
> + if (cpu == 0) {
> + int irq;
> + struct irq_desc *desc;
> + struct irq_data *data;
> + struct irq_chip *chip;
> +
> + for_each_irq_desc(irq, desc) {
> + raw_spin_lock(&desc->lock);
> + if (!irq_has_action(irq)) {
> + raw_spin_unlock(&desc->lock);
> + continue;
> + }
> +
> + data = irq_desc_get_irq_data(desc);
> + chip = irq_data_get_irq_chip(data);
> + if (!chip->irq_set_affinity) {
> + pr_debug("irq%d can't move out of BSP\n", irq);
> + raw_spin_unlock(&desc->lock);
> + return -EBUSY;
> + }
> + raw_spin_unlock(&desc->lock);
> + }

This is as fricking wrong as it gets. We already know that at boot
time when we initialize the irq chips. There is no need to iterate
over the whole irq descriptors just to figure that out.

And of course you'd detect cascaded irq chips behind a PCIe master
interrupt as non migratable even if that's not affecting hotplug.


I haven't looked at the other patches, but given the gems in this one
I can't be bothered to see the other magic fixups which are just
hacked into the code at random places to make it somehow work.

NAK.

tglx

2012-05-11 18:42:25

by Tony Luck

[permalink] [raw]
Subject: Re: [PATCH v6 04/12] x86/smpboot.c: Don't offline CPU0 if any irq can not be migrated out of it and remove CPU0 check in smp_callin()

On Fri, May 11, 2012 at 5:05 AM, Thomas Gleixner <[email protected]> wrote:
>> + ? ? if (cpuid == 0)
>> + ? ? ? ? ? ? identify_boot_cpu_online();
>
> No, this is not how we do things.
>
> If this is required, then hell we do it at boot time and not at a
> random place in the cpu hotplug code.

Open to better suggestions for placement. Sometimes we do need
special cases because on x86 the system isn't symmetrical, cpu0
really is treated differently by h/w and f/w and we have to live with this.
Biggest code impact of that is the extra code to bring cpu0 back online
using NMI instead of INIT. We can't use INIT because if cpu0 gets one,
it just resets the whole machine :-( But obviously we'd like to avoid
special cases where there is a sane way to do so.

> This is as fricking wrong as it gets. We already know that at boot
> time when we initialize the irq chips. There is no need to iterate
> over the whole irq descriptors just to figure that out.

We *knew* it at boot time ... but that information will be stale if
we hot plugged new devices and bound their interrupts to cpu0.

Do we need to maintain some parallel table of this information, or
is there a better way to check this?

-Tony

2012-05-14 12:17:34

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v6 04/12] x86/smpboot.c: Don't offline CPU0 if any irq can not be migrated out of it and remove CPU0 check in smp_callin()


* Tony Luck <[email protected]> wrote:

> Biggest code impact of that is the extra code to bring cpu0
> back online using NMI instead of INIT. We can't use INIT
> because if cpu0 gets one, it just resets the whole machine :-(
> But obviously we'd like to avoid special cases where there is
> a sane way to do so.

Could we just standardize on NMI bringup during regular bootup?

Thanks,

Ingo

2012-05-14 16:40:47

by Tony Luck

[permalink] [raw]
Subject: RE: [PATCH v6 04/12] x86/smpboot.c: Don't offline CPU0 if any irq can not be migrated out of it and remove CPU0 check in smp_callin()

> Could we just standardize on NMI bringup during regular bootup?

I don't know for sure (Fenghua?) but I'd guess not. CPU#0 is being brought *back* online
by this NMI ... i.e. it has been online before, so interrupt vector tables and CR3
etc. have been set to Linux-y values. When bringing other cpus online for the first
time they have nothing set so we'd still have to use INIT.

-Tony

2012-05-14 20:03:59

by Fenghua Yu

[permalink] [raw]
Subject: RE: [PATCH v6 04/12] x86/smpboot.c: Don't offline CPU0 if any irq can not be migrated out of it and remove CPU0 check in smp_callin()

> From: Luck, Tony
> Sent: Monday, May 14, 2012 9:41 AM
> To: Ingo Molnar
> Cc: Thomas Gleixner; Yu, Fenghua; Ingo Molnar; H Peter Anvin; Linus
> Torvalds; Andrew Morton; Mallick, Asit K; Arjan Dan De Ven; Siddha,
> Suresh B; Brown, Len; Srivatssa S. Bhat; Randy Dunlap; Chen Gong;
> linux-kernel; linux-pm; x86; Peter Zijlstra
> Subject: RE: [PATCH v6 04/12] x86/smpboot.c: Don't offline CPU0 if any
> irq can not be migrated out of it and remove CPU0 check in smp_callin()
>
> > Could we just standardize on NMI bringup during regular bootup?
>
> I don't know for sure (Fenghua?) but I'd guess not. CPU#0 is being
> brought *back* online
> by this NMI ... i.e. it has been online before, so interrupt vector
> tables and CR3
> etc. have been set to Linux-y values. When bringing other cpus online
> for the first
> time they have nothing set so we'd still have to use INIT.

Yes, on platforms of apic_physflat, apic_flat, x2apicapic, etc, the INIT sequence is necessary for bringing up AP's because AP's are in BIOS initialized unknown status without interrupt tables and CR3 etc.

Thanks.

-Fenghua

2012-05-17 22:49:07

by Suresh Siddha

[permalink] [raw]
Subject: Re: [PATCH v6 04/12] x86/smpboot.c: Don't offline CPU0 if any irq can not be migrated out of it and remove CPU0 check in smp_callin()

On Mon, 2012-05-14 at 14:17 +0200, Ingo Molnar wrote:
> * Tony Luck <[email protected]> wrote:
>
> > Biggest code impact of that is the extra code to bring cpu0
> > back online using NMI instead of INIT. We can't use INIT
> > because if cpu0 gets one, it just resets the whole machine :-(
> > But obviously we'd like to avoid special cases where there is
> > a sane way to do so.
>
> Could we just standardize on NMI bringup during regular bootup?

May be we can use the NMI or even better monitor/mwait based bringup to
re-online an AP that is currently offline in play_dead(), specifically
if we haven't lost that AP processor state because of suspend/resume
event etc.

This can speed-up online if we want to use light weight online/offline
of the AP's for saving power on mobile platforms.

thanks,
suresh

2012-05-17 22:53:28

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v6 04/12] x86/smpboot.c: Don't offline CPU0 if any irq can not be migrated out of it and remove CPU0 check in smp_callin()

On 05/14/2012 05:17 AM, Ingo Molnar wrote:
>
> * Tony Luck <[email protected]> wrote:
>
>> Biggest code impact of that is the extra code to bring cpu0
>> back online using NMI instead of INIT. We can't use INIT
>> because if cpu0 gets one, it just resets the whole machine :-(
>> But obviously we'd like to avoid special cases where there is
>> a sane way to do so.
>
> Could we just standardize on NMI bringup during regular bootup?
>

The first instance has to be SIPI because you can't give the CPU an NMI
without the IDT being setup. We *could* assume that the processor is
parked in real mode with the valid real-mode IDT set up (in which case
the vector at physical address 8 applies) but that seems to assume a lot
from the BIOS for little gain. In particular I suspect that UEFI-based
BIOSes may very well park the CPU in protected or long mode.

-hpa


--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.

2012-05-18 01:47:13

by Fenghua Yu

[permalink] [raw]
Subject: RE: [PATCH v6 04/12] x86/smpboot.c: Don't offline CPU0 if any irq can not be migrated out of it and remove CPU0 check in smp_callin()

> -----Original Message-----
> From: Siddha, Suresh B
> Sent: Thursday, May 17, 2012 3:47 PM
> To: Ingo Molnar
> Cc: Luck, Tony; Thomas Gleixner; Yu, Fenghua; Ingo Molnar; H Peter
> Anvin; Linus Torvalds; Andrew Morton; Mallick, Asit K; Arjan Dan De Ven;
> Brown, Len; Srivatssa S. Bhat; Randy Dunlap; Chen Gong; linux-kernel;
> linux-pm; x86; Peter Zijlstra
> Subject: Re: [PATCH v6 04/12] x86/smpboot.c: Don't offline CPU0 if any
> irq can not be migrated out of it and remove CPU0 check in smp_callin()
>
> On Mon, 2012-05-14 at 14:17 +0200, Ingo Molnar wrote:
> > * Tony Luck <[email protected]> wrote:
> >
> > > Biggest code impact of that is the extra code to bring cpu0
> > > back online using NMI instead of INIT. We can't use INIT
> > > because if cpu0 gets one, it just resets the whole machine :-(
> > > But obviously we'd like to avoid special cases where there is
> > > a sane way to do so.
> >
> > Could we just standardize on NMI bringup during regular bootup?
>
> May be we can use the NMI or even better monitor/mwait based bringup to
> re-online an AP that is currently offline in play_dead(), specifically
> if we haven't lost that AP processor state because of suspend/resume
> event etc.
>
> This can speed-up online if we want to use light weight online/offline
> of the AP's for saving power on mobile platforms.

Then booting up AP via INIT and putting AP online back via NMI/mwait will have different wakeup method and code. We probably need a way to tell if it is first AP bringup or online AP/BSP during run time.

And if offlined CPU is not in mwait (e.g. CPU without mwait feature), mwait based bringup won't work. Do we need to fall back to NMI wakeup if offlined CPU is not in mwait?

Thanks.

-Fenghua
????{.n?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2012-05-18 03:34:59

by H. Peter Anvin

[permalink] [raw]
Subject: Re: [PATCH v6 04/12] x86/smpboot.c: Don't offline CPU0 if any irq can not be migrated out of it and remove CPU0 check in smp_callin()

On 05/17/2012 06:47 PM, Yu, Fenghua wrote:
>
> Then booting up AP via INIT and putting AP online back via NMI/mwait
> will have different wakeup method and code. We probably need a way to
> tell if it is first AP bringup or online AP/BSP during run time.
>
> And if offlined CPU is not in mwait (e.g. CPU without mwait feature),
> mwait based bringup won't work. Do we need to fall back to NMI wakeup
> if offlined CPU is not in mwait?
>

I guess we need NMI (or just re-SIPI) in that case, since we're spinning
in CLI; HLT...

-hpa


--
H. Peter Anvin, Intel Open Source Technology Center
I work for Intel. I don't speak on their behalf.

2012-05-18 06:46:11

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v6 04/12] x86/smpboot.c: Don't offline CPU0 if any irq can not be migrated out of it and remove CPU0 check in smp_callin()


* H. Peter Anvin <[email protected]> wrote:

> On 05/14/2012 05:17 AM, Ingo Molnar wrote:
> >
> > * Tony Luck <[email protected]> wrote:
> >
> >> Biggest code impact of that is the extra code to bring cpu0
> >> back online using NMI instead of INIT. We can't use INIT
> >> because if cpu0 gets one, it just resets the whole machine :-(
> >> But obviously we'd like to avoid special cases where there is
> >> a sane way to do so.
> >
> > Could we just standardize on NMI bringup during regular bootup?
> >
>
> The first instance has to be SIPI because you can't give the
> CPU an NMI without the IDT being setup. We *could* assume
> that the processor is parked in real mode with the valid
> real-mode IDT set up (in which case the vector at physical
> address 8 applies) but that seems to assume a lot from the
> BIOS for little gain. In particular I suspect that UEFI-based
> BIOSes may very well park the CPU in protected or long mode.

Ok, fair enough.

Thanks,

Ingo