2005-04-12 06:03:07

by Shaohua Li

[permalink] [raw]
Subject: [PATCH 1/6]sep initializing rework

Hi,
These patches (together with 5 patches followed this one) are updated
suspend/resume SMP patches. The patches fixed some bugs and do clean up
as suggested. Now they work for both suspend-to-ram and suspend-to-disk.
Patches are against 2.6.12-rc2-mm3.

Thanks,
Shaohua

---
Make SEP init per-cpu, so it is hotplug safed.

Signed-off-by: Li Shaohua<[email protected]>

---

linux-2.6.11-root/arch/i386/kernel/smpboot.c | 6 ++++++
linux-2.6.11-root/arch/i386/kernel/sysenter.c | 12 +++++++-----
linux-2.6.11-root/arch/i386/mach-voyager/voyager_smp.c | 4 ++++
linux-2.6.11-root/arch/i386/power/cpu.c | 4 +---
linux-2.6.11-root/include/asm-i386/smp.h | 3 +++
5 files changed, 21 insertions(+), 8 deletions(-)

diff -puN arch/i386/kernel/smpboot.c~sep_init_cleanup arch/i386/kernel/smpboot.c
--- linux-2.6.11/arch/i386/kernel/smpboot.c~sep_init_cleanup 2005-04-12 10:36:00.164171464 +0800
+++ linux-2.6.11-root/arch/i386/kernel/smpboot.c 2005-04-12 10:36:00.174169944 +0800
@@ -443,6 +443,9 @@ static void __init start_secondary(void
* the local TLBs too.
*/
local_flush_tlb();
+
+ /* Note: this must be done before __cpu_up finish */
+ enable_sep_cpu();
cpu_set(smp_processor_id(), cpu_online_map);

/* We can take interrupts now: we're officially "up". */
@@ -920,6 +923,9 @@ static void __init smp_boot_cpus(unsigne
cpus_clear(cpu_core_map[0]);
cpu_set(0, cpu_core_map[0]);

+ sysenter_setup();
+ enable_sep_cpu();
+
/*
* If we couldn't find an SMP configuration at boot time,
* get out of here now!
diff -puN arch/i386/kernel/sysenter.c~sep_init_cleanup arch/i386/kernel/sysenter.c
--- linux-2.6.11/arch/i386/kernel/sysenter.c~sep_init_cleanup 2005-04-12 10:36:00.165171312 +0800
+++ linux-2.6.11-root/arch/i386/kernel/sysenter.c 2005-04-12 10:36:00.174169944 +0800
@@ -21,11 +21,16 @@

extern asmlinkage void sysenter_entry(void);

-void enable_sep_cpu(void *info)
+void enable_sep_cpu(void)
{
int cpu = get_cpu();
struct tss_struct *tss = &per_cpu(init_tss, cpu);

+ if (!boot_cpu_has(X86_FEATURE_SEP)) {
+ put_cpu();
+ return;
+ }
+
tss->ss1 = __KERNEL_CS;
tss->esp1 = sizeof(struct tss_struct) + (unsigned long) tss;
wrmsr(MSR_IA32_SYSENTER_CS, __KERNEL_CS, 0);
@@ -41,7 +46,7 @@ void enable_sep_cpu(void *info)
extern const char vsyscall_int80_start, vsyscall_int80_end;
extern const char vsyscall_sysenter_start, vsyscall_sysenter_end;

-static int __init sysenter_setup(void)
+int __init sysenter_setup(void)
{
void *page = (void *)get_zeroed_page(GFP_ATOMIC);

@@ -58,8 +63,5 @@ static int __init sysenter_setup(void)
&vsyscall_sysenter_start,
&vsyscall_sysenter_end - &vsyscall_sysenter_start);

- on_each_cpu(enable_sep_cpu, NULL, 1, 1);
return 0;
}
-
-__initcall(sysenter_setup);
diff -puN arch/i386/mach-voyager/voyager_smp.c~sep_init_cleanup arch/i386/mach-voyager/voyager_smp.c
--- linux-2.6.11/arch/i386/mach-voyager/voyager_smp.c~sep_init_cleanup 2005-04-12 10:36:00.167171008 +0800
+++ linux-2.6.11-root/arch/i386/mach-voyager/voyager_smp.c 2005-04-12 10:36:00.175169792 +0800
@@ -499,6 +499,7 @@ start_secondary(void *unused)
while (!cpu_isset(cpuid, smp_commenced_mask))
rep_nop();
local_irq_enable();
+ enable_sep_cpu();

local_flush_tlb();

@@ -696,6 +697,9 @@ smp_boot_cpus(void)
printk("CPU%d: ", boot_cpu_id);
print_cpu_info(&cpu_data[boot_cpu_id]);

+ sysenter_setup();
+ enable_sep_cpu();
+
if(is_cpu_quad()) {
/* booting on a Quad CPU */
printk("VOYAGER SMP: Boot CPU is Quad\n");
diff -puN arch/i386/power/cpu.c~sep_init_cleanup arch/i386/power/cpu.c
--- linux-2.6.11/arch/i386/power/cpu.c~sep_init_cleanup 2005-04-12 10:36:00.168170856 +0800
+++ linux-2.6.11-root/arch/i386/power/cpu.c 2005-04-12 10:36:00.175169792 +0800
@@ -33,8 +33,6 @@ unsigned long saved_context_esp, saved_c
unsigned long saved_context_esi, saved_context_edi;
unsigned long saved_context_eflags;

-extern void enable_sep_cpu(void *);
-
void __save_processor_state(struct saved_context *ctxt)
{
kernel_fpu_begin();
@@ -136,7 +134,7 @@ void __restore_processor_state(struct sa
* sysenter MSRs
*/
if (boot_cpu_has(X86_FEATURE_SEP))
- enable_sep_cpu(NULL);
+ enable_sep_cpu();

fix_processor_context();
do_fpu_end();
diff -puN include/asm-i386/smp.h~sep_init_cleanup include/asm-i386/smp.h
--- linux-2.6.11/include/asm-i386/smp.h~sep_init_cleanup 2005-04-12 10:36:00.170170552 +0800
+++ linux-2.6.11-root/include/asm-i386/smp.h 2005-04-12 10:36:00.176169640 +0800
@@ -37,6 +37,9 @@ extern int smp_num_siblings;
extern cpumask_t cpu_sibling_map[];
extern cpumask_t cpu_core_map[];

+extern int sysenter_setup(void);
+extern void enable_sep_cpu(void);
+
extern void smp_flush_tlb(void);
extern void smp_message_irq(int cpl, void *dev_id, struct pt_regs *regs);
extern void smp_invalidate_rcv(void); /* Process an NMI */
_



2005-04-12 12:07:46

by Zwane Mwaikambo

[permalink] [raw]
Subject: Re: [PATCH 1/6]sep initializing rework

Hello Shaohua,

On Tue, 12 Apr 2005, Li Shaohua wrote:

> These patches (together with 5 patches followed this one) are updated
> suspend/resume SMP patches. The patches fixed some bugs and do clean up
> as suggested. Now they work for both suspend-to-ram and suspend-to-disk.
> Patches are against 2.6.12-rc2-mm3.

These patches look good and i think we should go ahead with them. I've
also cross checked with physical hotplug cpu patches for ES7xxx from
Natalie (added to Cc) and it does indeed look like a lot of the code will
work for her too, but i'd appreciate it if she also does a double check.
Obviously this won't work for other upcoming users of hotplug cpu like Xen
(Ryan added to Cc) but i think we can abstract things later on to cover
other special users.

Thanks Shaohua,
Zwane

2005-04-13 01:49:55

by Shaohua Li

[permalink] [raw]
Subject: RE: [PATCH 1/6]sep initializing rework

On Wed, 2005-04-13 at 01:57, Protasevich, Natalie wrote:
> Hello,
> This is a hotplug CPU patch for i386, done against 2.6.12-rc2-mm3.
> Somewhat alternative to the one posted by Li Shaohua, but not really
> (and I didn't mean that :). If you look closer, our patches are
> different and can complement each other I think. Li did great job on
> sep, after-offline cleanup, __devinit etc., and I have some radical
> changes in the AP bringup mechanism. I left alone __init to __devinit
> part (I was going through it lately, but I think even though I had few
> more than Li did, he covered it sufficiently perhaps). I started
> having
> doubts in free_initmem() vs __devinit because look how many of
> __init's
> left! just a few :).
Looks quite smart, but people will argue it will keep all __init
sections in this way. I'd like we keep the default behavior of __init.

> I got rid of do_boot_cpu loop in smpboot.c because
> the loop
> static void __init smp_init(void)
> {
> unsigned int i;
>
> /* FIXME: This should be done in userspace --RR */
> for_each_present_cpu(i) {
> if (num_online_cpus() >= max_cpus)
> break;
> if (!cpu_online(i))
> cpu_up(i);
> }
> ...
> does it again so why leave it in smpboot.c to boot AP's twice.
This is what IA64 does. In this way, you must clean up the bogomips
message, TSC synchronization. And CPU_UP could be called in user
context, so fork_idle possibly should be in workqueue. And please make
sure it doesn't break other things like check_nmi_watchdog. I just
select an easy way (add smp_prepare_cpu) and it doesn't break anything.

> I also
> found that my system fails sooner or later when I try not to synch
> runtime booted processor with others, so I changed tsc synchronization
> to only sync between booting CPU and the one that boots it.
IA64 also does like this. It synchronizes one AP's ITC against BP's one
time. But in IA32, TSC's upper 32 bits can be written only on prescott
and above. In earlier CPU, upper 32 bits will become 0 after any write.

> The patch
> works for me on Intel 8x generic box, and on ES7000. I was asked to
> separate my patch into smaller ones by the theme, but I'm posting the
> entire patch for now, because I think it is probably not the final
> one.
> I think (I hope) I will sync up with Li later on.
> My idea was that if we find a CPU core in ACPI (enabled or disabled),
> we
> encounter for it in sibling map and create a sysfs node accordingly,
> and
> cpu_possible_map will reflect that. We take processors up/down
> depending
> on physical presence using the existing node. That's the scenario
> implemented on ES7000 that reports all possible cores in ACPI marking
> absent processors as disabled. Runtime enablement/disablement depends
> on
> sysfs only and the driving agent can be anything (ACPI or user) that
> triggers sysfs node for this processor.
You possibly can refer to IA64's implementation. The goal of my patches
are to support suspend/resume, which actually doesn't really hotremove a
CPU, so I just ignored the sysfs/ACPI issues.

Thanks,
Shaohua

>
> -----Original Message-----
> From: Zwane Mwaikambo [mailto:[email protected]]
> Sent: Tuesday, April 12, 2005 6:08 AM
> To: Li Shaohua
> Cc: lkml; ACPI-DEV; Len Brown; Pavel Machek; Andrew Morton;
> Protasevich,
> Natalie; Ryan Harper
> Subject: Re: [PATCH 1/6]sep initializing rework
>
> Hello Shaohua,
>
> On Tue, 12 Apr 2005, Li Shaohua wrote:
>
> > These patches (together with 5 patches followed this one) are
> updated
> > suspend/resume SMP patches. The patches fixed some bugs and do clean
> > up as suggested. Now they work for both suspend-to-ram and
> suspend-to-disk.
> > Patches are against 2.6.12-rc2-mm3.
>
> These patches look good and i think we should go ahead with them. I've
> also cross checked with physical hotplug cpu patches for ES7xxx from
> Natalie (added to Cc) and it does indeed look like a lot of the code
> will work for her too, but i'd appreciate it if she also does a double
> check.
> Obviously this won't work for other upcoming users of hotplug cpu like
> Xen (Ryan added to Cc) but i think we can abstract things later on to
> cover other special users.
>
> Thanks Shaohua,
> Zwane
>

2005-04-12 18:35:13

by Protasevich, Natalie

[permalink] [raw]
Subject: RE: [PATCH 1/6]sep initializing rework

Hello,
This is a hotplug CPU patch for i386, done against 2.6.12-rc2-mm3.
Somewhat alternative to the one posted by Li Shaohua, but not really
(and I didn't mean that :). If you look closer, our patches are
different and can complement each other I think. Li did great job on
sep, after-offline cleanup, __devinit etc., and I have some radical
changes in the AP bringup mechanism. I left alone __init to __devinit
part (I was going through it lately, but I think even though I had few
more than Li did, he covered it sufficiently perhaps). I started having
doubts in free_initmem() vs __devinit because look how many of __init's
left! just a few :). I got rid of do_boot_cpu loop in smpboot.c because
the loop
static void __init smp_init(void)
{
unsigned int i;

/* FIXME: This should be done in userspace --RR */
for_each_present_cpu(i) {
if (num_online_cpus() >= max_cpus)
break;
if (!cpu_online(i))
cpu_up(i);
}
...
does it again so why leave it in smpboot.c to boot AP's twice. I also
found that my system fails sooner or later when I try not to synch
runtime booted processor with others, so I changed tsc synchronization
to only sync between booting CPU and the one that boots it. The patch
works for me on Intel 8x generic box, and on ES7000. I was asked to
separate my patch into smaller ones by the theme, but I'm posting the
entire patch for now, because I think it is probably not the final one.
I think (I hope) I will sync up with Li later on.
My idea was that if we find a CPU core in ACPI (enabled or disabled), we
encounter for it in sibling map and create a sysfs node accordingly, and
cpu_possible_map will reflect that. We take processors up/down depending
on physical presence using the existing node. That's the scenario
implemented on ES7000 that reports all possible cores in ACPI marking
absent processors as disabled. Runtime enablement/disablement depends on
sysfs only and the driving agent can be anything (ACPI or user) that
triggers sysfs node for this processor.
Thanks,
--Natalie

-----Original Message-----
From: Zwane Mwaikambo [mailto:[email protected]]
Sent: Tuesday, April 12, 2005 6:08 AM
To: Li Shaohua
Cc: lkml; ACPI-DEV; Len Brown; Pavel Machek; Andrew Morton; Protasevich,
Natalie; Ryan Harper
Subject: Re: [PATCH 1/6]sep initializing rework

Hello Shaohua,

On Tue, 12 Apr 2005, Li Shaohua wrote:

> These patches (together with 5 patches followed this one) are updated
> suspend/resume SMP patches. The patches fixed some bugs and do clean
> up as suggested. Now they work for both suspend-to-ram and
suspend-to-disk.
> Patches are against 2.6.12-rc2-mm3.

These patches look good and i think we should go ahead with them. I've
also cross checked with physical hotplug cpu patches for ES7xxx from
Natalie (added to Cc) and it does indeed look like a lot of the code
will work for her too, but i'd appreciate it if she also does a double
check.
Obviously this won't work for other upcoming users of hotplug cpu like
Xen (Ryan added to Cc) but i think we can abstract things later on to
cover other special users.

Thanks Shaohua,
Zwane


Attachments:
hotcpu-2.6.11.diff (10.08 kB)
hotcpu-2.6.11.diff

2005-04-30 12:07:15

by Benoit Boissinot

[permalink] [raw]
Subject: Re: [PATCH 1/6]sep initializing rework

On 4/12/05, Li Shaohua <[email protected]> wrote:
> Hi,
> These patches (together with 5 patches followed this one) are updated
> suspend/resume SMP patches. The patches fixed some bugs and do clean up
> as suggested. Now they work for both suspend-to-ram and suspend-to-disk.
> Patches are against 2.6.12-rc2-mm3.
>
> Thanks,
> Shaohua
>
> ---
> Make SEP init per-cpu, so it is hotplug safed.
>
> Signed-off-by: Li Shaohua<[email protected]>
>
> ---
> +++ linux-2.6.11-root/arch/i386/power/cpu.c 2005-04-12 10:36:00.175169792 +0800
> @@ -33,8 +33,6 @@ unsigned long saved_context_esp, saved_c
> unsigned long saved_context_esi, saved_context_edi;
> unsigned long saved_context_eflags;
>
> -extern void enable_sep_cpu(void *);
> -
> void __save_processor_state(struct saved_context *ctxt)
> {
> kernel_fpu_begin();


> diff -puN include/asm-i386/smp.h~sep_init_cleanup include/asm-i386/smp.h
> --- linux-2.6.11/include/asm-i386/smp.h~sep_init_cleanup 2005-04-12 10:36:00.170170552 +0800
> +++ linux-2.6.11-root/include/asm-i386/smp.h 2005-04-12 10:36:00.176169640 +0800
> @@ -37,6 +37,9 @@ extern int smp_num_siblings;
> extern cpumask_t cpu_sibling_map[];
> extern cpumask_t cpu_core_map[];
>
> +extern int sysenter_setup(void);
> +extern void enable_sep_cpu(void);
> +
> extern void smp_flush_tlb(void);
> extern void smp_message_irq(int cpl, void *dev_id, struct pt_regs *regs);
> extern void smp_invalidate_rcv(void); /* Process an NMI */
> _

This change adds a warning when CONFIG_SMP is not set:

arch/i386/power/cpu.c: In function '__restore_processor_state':
arch/i386/power/cpu.c:137: warning: implicit declaration of function
'enable_sep_cpu'
Maybe those functions should be defined somewhere else.

regards,

Benoit