Re: [PATCH] x86: restore (optional) forwarding of PCI SERR induced NMI to Dom0

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 21/01/13 16:11, Jan Beulich wrote:
> c/s 22949:54fe1011f86b removed the forwarding of NMIs to Dom0 when they
> were caused by PCI SERR. NMI buttons as well as BMCs (like HP's iLO)
> may however want such events to be seen in Dom0 (e.g. to trigger a
> dump).
>
> Therefore restore most of the functionality which named c/s removed
> (adjusted for subsequent changes, and adjusting the public interface to
> use the modern term, retaining the old one for backwards
> compatibility).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -3089,6 +3089,7 @@ static void nmi_mce_softirq(void)
>  static void pci_serr_softirq(void)
>  {
>      printk("\n\nNMI - PCI system error (SERR)\n");
> +    outb(inb(0x61) & 0x0b, 0x61); /* re-enable the PCI SERR error line. */
>  }
>  
>  void async_exception_cleanup(struct vcpu *curr)
> @@ -3135,9 +3136,20 @@ static void pci_serr_error(struct cpu_us
>  {
>      outb((inb(0x61) & 0x0f) | 0x04, 0x61); /* clear-and-disable the PCI SERR error line. */
>  
> -    /* Would like to print a diagnostic here but can't call printk()
> -       from NMI context -- raise a softirq instead. */
> -    raise_softirq(PCI_SERR_SOFTIRQ);
> +    switch ( opt_nmi[0] )
> +    {
> +    case 'd': /* 'dom0' */
> +        nmi_dom0_report(_XEN_NMIREASON_pci_serr);
> +    case 'i': /* 'ignore' */
> +        /* Would like to print a diagnostic here but can't call printk()
> +           from NMI context -- raise a softirq instead. */
> +        raise_softirq(PCI_SERR_SOFTIRQ);
> +        break;
> +    default:  /* 'fatal' */
> +        console_force_unlock();
> +        printk("\n\nNMI - PCI system error (SERR)\n");
> +        fatal_trap(TRAP_nmi, regs);
> +    }
>  }
>  
>  static void io_check_error(struct cpu_user_regs *regs)
> --- a/xen/include/public/nmi.h
> +++ b/xen/include/public/nmi.h
> @@ -36,9 +36,14 @@
>   /* I/O-check error reported via ISA port 0x61, bit 6. */
>  #define _XEN_NMIREASON_io_error     0
>  #define XEN_NMIREASON_io_error      (1UL << _XEN_NMIREASON_io_error)
> + /* PCI SERR reported via ISA port 0x61, bit 7. */
> +#define _XEN_NMIREASON_pci_serr     1
> +#define XEN_NMIREASON_pci_serr      (1UL << _XEN_NMIREASON_pci_serr)
> +#if __XEN_INTERFACE_VERSION__ < 0x00040300 /* legacy alias of the above */
>   /* Parity error reported via ISA port 0x61, bit 7. */

These seem wrong.  Both SERR and Parity error have the same comment and
definition.

Furthermore, the definition disagrees with the comment.

~Andrew

>  #define _XEN_NMIREASON_parity_error 1
>  #define XEN_NMIREASON_parity_error  (1UL << _XEN_NMIREASON_parity_error)
> +#endif
>   /* Unknown hardware-generated NMI. */
>  #define _XEN_NMIREASON_unknown      2
>  #define XEN_NMIREASON_unknown       (1UL << _XEN_NMIREASON_unknown)
>
>
>