Re: [Xen-devel] [PATCH] x86/mce: CPU notifiers must not be registered a second time during resume

[Keir Fraser <keir.xen@xxxxxxxxx>]

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