WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

RE: [Xen-devel] [PATCH] x86/mce: CPU notifiers must not be registered a

To: Jan Beulich <JBeulich@xxxxxxxxxx>
Subject: RE: [Xen-devel] [PATCH] x86/mce: CPU notifiers must not be registered a second time during resume
From: "Liu, Jinsong" <jinsong.liu@xxxxxxxxx>
Date: Mon, 21 Mar 2011 21:18:52 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc: Keir Fraser <keir.xen@xxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Mon, 21 Mar 2011 06:20:31 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4D8718E8020000780003778E@xxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <4D8383340200007800037412@xxxxxxxxxxxxxxxxxx> <C9A92E38.150A6%keir.xen@xxxxxxxxx> <BC00F5384FCFC9499AF06F92E8B78A9E1FD9976459@xxxxxxxxxxxxxxxxxxxxxxxxxxxx> <4D8718E8020000780003778E@xxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcvnoQsv9V382N3TTFC1rlf7YcQUnQAIpK7g
Thread-topic: [Xen-devel] [PATCH] x86/mce: CPU notifiers must not be registered a second time during resume
Jan Beulich wrote:
>>>> On 19.03.11 at 16: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;
>>  }
> 
> This one may be an option, though it then is unclear to me why
> you removed it in 22694.

It would alloc redundant mcabanks when using AMD platform.

> 
>> -----------------------------------------------
>> 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 )
>> -----------------------------------------------
> 
> This one I don't like at all - pre-SMP init calls ought to be allowed
> to assume identify_cpu() was run.
> 
> Further, I wouldn't really like to see anything sufficiently generic
> being pushed ahead of (the final step of) console initialization.

Agree.

> 
>> 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();
>> 
> 
> Here as well I'm not sure this wouldn't have any bad side effects.
> 
> Overall, trying to also answer Keir's subsequent question, I don't
> think this gets us any closer to Linux - I think it'd be more of the
> opposite.
> 
> Jan

Reasonable to me, move do_presmp_initcall aheah is of some risk.
Let's keep current way.

Thanks,
Jinsong


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel