I checked in Jan's patch as it is more suitable for 4.1 anyway, compared
with moving do_presmp_initcalls(). We could consider replacing it with your
approach below in xen-unstable however. If it makes the ordering of setup
closer to that in Linux, that would probably be a good thing.
-- Keir
On 19/03/2011 15:53, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
> 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