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>, "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Subject: Re: [Xen-devel] [PATCH] x86/mce: CPU notifiers must not be registered a second time during resume
From: Keir Fraser <keir.xen@xxxxxxxxx>
Date: Fri, 18 Mar 2011 15:43:52 +0000
Cc: Jinsong Liu <jinsong.liu@xxxxxxxxx>
Delivery-date: Fri, 18 Mar 2011 08:44:32 -0700
Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:user-agent:date:subject:from:to:cc:message-id :thread-topic:thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; bh=xi21yGwVfKlvlMItTNW3U4zYUUwxBWyAqyLZSzm1l78=; b=S0RfFQ7VZKzchSUa0pWinZ29c9E56KL2sL+GK5QjoB3Hw7kkBW/t8pWnF4TlpRya/6 CBuN2DiIm9/FRbb0S2CEJuBFPfmeLrx1SeudkuGna/slavlkrJ635m6MWiq2GQvzoMv5 SSFs/Mken652TDbdPy25oLWR24TxscCtmpCpc=
Domainkey-signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=user-agent:date:subject:from:to:cc:message-id:thread-topic :thread-index:in-reply-to:mime-version:content-type :content-transfer-encoding; b=HEe1yHjr+K0YygsT63jmOPAAl2iBWqhvuJ9p0MYU/U6z9KkC3VvjZWV1LzxMo/1RY2 dz1MtYVbxOVeYcMsDBLykI7zoxEImLIpkmoMkh8yA/uapPZjEZmdAstu6yk4ATOrSdPo AkWGdz8MUy6m/CqkMm9A02hLkvV8+fsYm7p8o=
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <4D8383340200007800037412@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>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: Acvlg0fnfftgVGNkgUuqSd8PG8dvfg==
Thread-topic: [Xen-devel] [PATCH] x86/mce: CPU notifiers must not be registered a second time during resume
User-agent: Microsoft-Entourage/12.28.0.101117
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