Thanks Jan and Keir!
Sorry for late check email at weekend.
I think a while, how about following solution (draft scheme):
-----------------------------------------------
1. at mce_intel.c, keep old intel_mce_initcall() func (it has been removed at
c/s 22964), and do
--- a/xen/arch/x86/cpu/mcheck/mce_intel.c Fri Feb 25 01:26:01 2011 +0800
+++ b/xen/arch/x86/cpu/mcheck/mce_intel.c Mon Feb 28 19:19:20 2011 +0800
static int __init intel_mce_initcall(void)
{
+ void *hcpu = (void *)(long)smp_processor_id();
+ cpu_callback(&cpu_nfb, CPU_UP_PREPARE, hcpu);
register_cpu_notifier(&cpu_nfb);
return 0;
}
-----------------------------------------------
2. at setup.c, do_presmp_initcalls() at little bit earlier
diff -r 1a364b17d66a xen/arch/x86/setup.c
--- a/xen/arch/x86/setup.c Fri Feb 25 01:26:01 2011 +0800
+++ b/xen/arch/x86/setup.c Mon Feb 28 19:19:20 2011 +0800
@@ -1203,6 +1203,8 @@ void __init __start_xen(unsigned long mb
arch_init_memory();
+ do_presmp_initcalls();
+
identify_cpu(&boot_cpu_data);
if ( cpu_has_fxsr )
set_in_cr4(X86_CR4_OSFXSR);
@@ -1235,8 +1237,6 @@ void __init __start_xen(unsigned long mb
initialize_keytable();
console_init_postirq();
-
- do_presmp_initcalls();
for_each_present_cpu ( i )
-----------------------------------------------
How do you think? it don't need to add bsp para to mcheck_int() as
-void mcheck_init(struct cpuinfo_x86 *c)
+void mcheck_init(struct cpuinfo_x86 *c, bool_t bsp)
BTW, it can go further to unify cpu0 and cpux, like:
----------------------------------------------
diff -r 682880e909db xen/arch/x86/setup.c
--- a/xen/arch/x86/setup.c Mon Feb 28 09:17:40 2011 +0800
+++ b/xen/arch/x86/setup.c Mon Feb 28 09:53:54 2011 +0800
@@ -1205,7 +1205,8 @@ void __init __start_xen(unsigned long mb
do_presmp_initcalls();
- identify_cpu(&boot_cpu_data);
+ smp_prepare_cpus(max_cpus);
+ boot_cpu_data = cpu_data[0];
if ( cpu_has_fxsr )
set_in_cr4(X86_CR4_OSFXSR);
if ( cpu_has_xmm )
@@ -1221,8 +1222,6 @@ void __init __start_xen(unsigned long mb
max_cpus = 0;
iommu_setup(); /* setup iommu if available */
-
- smp_prepare_cpus(max_cpus);
spin_debug_enable();
diff -r 682880e909db xen/arch/x86/smpboot.c
--- a/xen/arch/x86/smpboot.c Mon Feb 28 09:17:40 2011 +0800
+++ b/xen/arch/x86/smpboot.c Mon Feb 28 09:53:54 2011 +0800
@@ -88,9 +88,7 @@ static void smp_store_cpu_info(int id)
{
struct cpuinfo_x86 *c = cpu_data + id;
- *c = boot_cpu_data;
- if ( id != 0 )
- identify_cpu(c);
+ identify_cpu(c);
/* Mask B, Pentium, but not Pentium MMX -- remember it, as it has bugs. */
if ( (c->x86_vendor == X86_VENDOR_INTEL) &&
-----------------------------------------
Thanks,
Jinsong
Keir Fraser wrote:
> On 18/03/2011 15:07, "Jan Beulich" <JBeulich@xxxxxxxxxx> wrote:
>
>> While c/s 22964:f71212f712fd and 23051:93c864c16ab1 fixed issues with
>> CPU onlining, they introduced a problem with resume: mcheck_init() is
>> also being called on that path, and hence checking whether it's
>> running on CPU 0, which is generally not a really good thing, is
>> particularly inappropriate here.
>
> Just have a 'static bool_t early_init_done' or similar in
> intel_mcheck_init().
>
> if ( !early_init_done ) {
> BUG_ON(smp_processor_id() != 0);
> ...
> early_init_done = 1;
> }
>
> It's clearer anyway -- we're simply protecting one-time-only
> early-boot-time initialisation stuff.
>
> -- Keir
>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxxxx>
>>
>> --- a/xen/arch/x86/acpi/power.c
>> +++ b/xen/arch/x86/acpi/power.c
>> @@ -187,7 +187,7 @@ static int enter_state(u32 state)
>>
>> device_power_up();
>>
>> - mcheck_init(&boot_cpu_data);
>> + mcheck_init(&boot_cpu_data, 0);
>> write_cr4(cr4);
>>
>> printk(XENLOG_INFO "Finishing wakeup from ACPI S%d state.\n",
>> state); --- a/xen/arch/x86/cpu/common.c
>> +++ b/xen/arch/x86/cpu/common.c
>> @@ -431,19 +431,13 @@ void __cpuinit identify_cpu(struct cpuin
>> /* AND the already accumulated flags with these */
>> for ( i = 0 ; i < NCAPINTS ; i++ )
>> boot_cpu_data.x86_capability[i] &= c->x86_capability[i];
>> - }
>> -
>> - /* Init Machine Check Exception if available. */
>> - mcheck_init(c);
>>
>> -#if 0
>> - if (c == &boot_cpu_data)
>> - sysenter_setup();
>> - enable_sep_cpu();
>> -#endif
>> + mcheck_init(c, 0);
>> + } else {
>> + mcheck_init(c, 1);
>>
>> - if (c == &boot_cpu_data)
>> mtrr_bp_init();
>> + }
>> }
>>
>> /* cpuid returns the value latched in the HW at reset, not the APIC
>> ID --- a/xen/arch/x86/cpu/mcheck/mce.c
>> +++ b/xen/arch/x86/cpu/mcheck/mce.c
>> @@ -793,7 +793,7 @@ static struct notifier_block cpu_nfb = { };
>>
>> /* This has to be run for each processor */
>> -void mcheck_init(struct cpuinfo_x86 *c)
>> +void mcheck_init(struct cpuinfo_x86 *c, bool_t bsp) {
>> enum mcheck_type inited = mcheck_none;
>>
>> @@ -822,7 +822,7 @@ void mcheck_init(struct cpuinfo_x86 *c)
>> switch (c->x86) { case 6:
>> case 15:
>> - inited = intel_mcheck_init(c);
>> + inited = intel_mcheck_init(c, bsp);
>> break;
>> }
>> break;
>> @@ -844,7 +844,7 @@ void mcheck_init(struct cpuinfo_x86 *c) /*
>> Turn on MCE now */ set_in_cr4(X86_CR4_MCE);
>>
>> - if ( smp_processor_id() == 0 )
>> + if ( bsp )
>> {
>> /* Early MCE initialisation for BSP. */
>> if ( cpu_poll_bankmask_alloc(0) )
>> --- a/xen/arch/x86/cpu/mcheck/mce.h
>> +++ b/xen/arch/x86/cpu/mcheck/mce.h
>> @@ -42,7 +42,7 @@ enum mcheck_type amd_k7_mcheck_init(stru
>> enum mcheck_type amd_k8_mcheck_init(struct cpuinfo_x86 *c);
>> enum mcheck_type amd_f10_mcheck_init(struct cpuinfo_x86 *c);
>>
>> -enum mcheck_type intel_mcheck_init(struct cpuinfo_x86 *c);
>> +enum mcheck_type intel_mcheck_init(struct cpuinfo_x86 *c, bool_t
>> bsp);
>>
>> void intel_mcheck_timer(struct cpuinfo_x86 *c);
>> void mce_intel_feature_init(struct cpuinfo_x86 *c);
>> --- a/xen/arch/x86/cpu/mcheck/mce_intel.c
>> +++ b/xen/arch/x86/cpu/mcheck/mce_intel.c
>> @@ -1297,9 +1297,9 @@ static struct notifier_block cpu_nfb = { };
>>
>> /* p4/p6 family have similar MCA initialization process */
>> -enum mcheck_type intel_mcheck_init(struct cpuinfo_x86 *c)
>> +enum mcheck_type intel_mcheck_init(struct cpuinfo_x86 *c, bool_t
>> bsp) { - if ( smp_processor_id() == 0 )
>> + if ( bsp )
>> {
>> /* Early MCE initialisation for BSP. */
>> if ( cpu_mcabank_alloc(0) )
>> --- a/xen/include/asm-x86/processor.h
>> +++ b/xen/include/asm-x86/processor.h
>> @@ -555,7 +555,7 @@ void compat_show_guest_stack(struct vcpu
>> extern void mtrr_ap_init(void);
>> extern void mtrr_bp_init(void);
>>
>> -void mcheck_init(struct cpuinfo_x86 *c);
>> +void mcheck_init(struct cpuinfo_x86 *c, bool_t bsp);
>>
>> #define DECLARE_TRAP_HANDLER(_name) \
>> asmlinkage void _name(void); \
>>
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home