Re: [Xen-devel] [PATCH v2] x86/NMI: Allow processing unknown NMIs when watchdog is enabled

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 27.08.14 at 17:11, <ross.lagerwall@xxxxxxxxxx> wrote:
> @@ -473,14 +496,26 @@ void nmi_watchdog_tick(const struct cpu_user_regs *regs)
>          }
>          else if ( nmi_perfctr_msr == MSR_P6_PERFCTR0 )
>          {
> +            rdmsrl(MSR_P6_PERFCTR0, msr_content);
> +            if ( msr_content & (1ULL << P6_EVENT_WIDTH) )
> +                watchdog_tick = 0;
> +
>              /*
>               * Only P6 based Pentium M need to re-unmask the apic vector but
>               * it doesn't hurt other P6 variants.
>               */
>              apic_write(APIC_LVTPC, APIC_DM_NMI);
>          }
> +        else if ( nmi_perfctr_msr == MSR_K7_PERFCTR0 )
> +        {
> +            rdmsrl(MSR_K7_PERFCTR0, msr_content);
> +            if ( msr_content & (1ULL << K7_EVENT_WIDTH) )
> +                watchdog_tick = 0;
> +        }

Looking at the top counter bits is rather weak a check, but I guess
there's nothing better (and considering a few more of the top
bits would only shrink the window of mis-detection, not eliminate
it)...

> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -3306,6 +3306,7 @@ void do_nmi(const struct cpu_user_regs *regs)
>  {
>      unsigned int cpu = smp_processor_id();
>      unsigned char reason;
> +    bool_t ticked = 1;
>  
>      ++nmi_count(cpu);
>  
> @@ -3313,7 +3314,7 @@ void do_nmi(const struct cpu_user_regs *regs)
>          return;
>  
>      if ( nmi_watchdog )
> -        nmi_watchdog_tick(regs);
> +        ticked = nmi_watchdog_tick(regs);
>  
>      /* Only the BSP gets external NMIs from the system. */
>      if ( cpu == 0 )
> @@ -3323,7 +3324,7 @@ void do_nmi(const struct cpu_user_regs *regs)
>              pci_serr_error(regs);
>          if ( reason & 0x40 )
>              io_check_error(regs);
> -        if ( !(reason & 0xc0) && !nmi_watchdog )
> +        if ( !(reason & 0xc0) && !ticked && watchdog_force )
>              unknown_nmi_error(regs, reason);

So when !nmi_watchdog we now will never get into
unknown_nmi_error()? Also I think the inspection of watchdog_force
would better be done above where the watchdog gets handled, so
it's all in one central place.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel