xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86: restore (optional) forwarding of PCI SERR induced NMI to Dom0
@ 2013-01-21 16:11 Jan Beulich
  2013-01-21 16:16 ` Andrew Cooper
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Jan Beulich @ 2013-01-21 16:11 UTC (permalink / raw)
  To: xen-devel; +Cc: Stefano Stabellini

[-- Attachment #1: Type: text/plain, Size: 2573 bytes --]

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. */
 #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)




[-- Attachment #2: x86-forward-serr.patch --]
[-- Type: text/plain, Size: 2637 bytes --]

x86: restore (optional) forwarding of PCI SERR induced NMI to Dom0

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. */
 #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)

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: restore (optional) forwarding of PCI SERR induced NMI to Dom0
  2013-01-21 16:11 [PATCH] x86: restore (optional) forwarding of PCI SERR induced NMI to Dom0 Jan Beulich
@ 2013-01-21 16:16 ` Andrew Cooper
  2013-01-21 16:57   ` Jan Beulich
  2013-01-21 16:20 ` Stefano Stabellini
  2013-01-21 16:28 ` Keir Fraser
  2 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2013-01-21 16:16 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Stefano Stabellini, xen-devel

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)
>
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: restore (optional) forwarding of PCI SERR induced NMI to Dom0
  2013-01-21 16:11 [PATCH] x86: restore (optional) forwarding of PCI SERR induced NMI to Dom0 Jan Beulich
  2013-01-21 16:16 ` Andrew Cooper
@ 2013-01-21 16:20 ` Stefano Stabellini
  2013-01-21 16:28 ` Keir Fraser
  2 siblings, 0 replies; 6+ messages in thread
From: Stefano Stabellini @ 2013-01-21 16:20 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Stefano Stabellini, xen-devel

On Mon, 21 Jan 2013, 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>

Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.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. */
>  #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)
> 
> 
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: restore (optional) forwarding of PCI SERR induced NMI to Dom0
  2013-01-21 16:11 [PATCH] x86: restore (optional) forwarding of PCI SERR induced NMI to Dom0 Jan Beulich
  2013-01-21 16:16 ` Andrew Cooper
  2013-01-21 16:20 ` Stefano Stabellini
@ 2013-01-21 16:28 ` Keir Fraser
  2 siblings, 0 replies; 6+ messages in thread
From: Keir Fraser @ 2013-01-21 16:28 UTC (permalink / raw)
  To: Jan Beulich, xen-devel; +Cc: Stefano Stabellini

On 21/01/2013 16:11, "Jan Beulich" <JBeulich@suse.com> 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>

Acked-by: Keir Fraser <keir@xen.org>

> --- 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. */
>  #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)
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: restore (optional) forwarding of PCI SERR induced NMI to Dom0
  2013-01-21 16:16 ` Andrew Cooper
@ 2013-01-21 16:57   ` Jan Beulich
  2013-01-21 17:16     ` Andrew Cooper
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2013-01-21 16:57 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: xen-devel, Stefano Stabellini

>>> On 21.01.13 at 17:16, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> On 21/01/13 16:11, Jan Beulich wrote:
>> --- 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.

Sure - did you look at the history of this (particularly the c/s
mentioned in the description)?

> Furthermore, the definition disagrees with the comment.

In what way?

Jan

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] x86: restore (optional) forwarding of PCI SERR induced NMI to Dom0
  2013-01-21 16:57   ` Jan Beulich
@ 2013-01-21 17:16     ` Andrew Cooper
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Cooper @ 2013-01-21 17:16 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, Stefano Stabellini

On 21/01/13 16:57, Jan Beulich wrote:
>>>> On 21.01.13 at 17:16, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> On 21/01/13 16:11, Jan Beulich wrote:
>>> --- 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.
> Sure - did you look at the history of this (particularly the c/s
> mentioned in the description)?
>
>> Furthermore, the definition disagrees with the comment.
> In what way?
>
> Jan
>

Ah - I missed that being in public, and was thinking that you were
attempting to use (1UL<<1) to test against bit 7.  My mistake - sorry.

~Andrew

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-01-21 17:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-21 16:11 [PATCH] x86: restore (optional) forwarding of PCI SERR induced NMI to Dom0 Jan Beulich
2013-01-21 16:16 ` Andrew Cooper
2013-01-21 16:57   ` Jan Beulich
2013-01-21 17:16     ` Andrew Cooper
2013-01-21 16:20 ` Stefano Stabellini
2013-01-21 16:28 ` Keir Fraser

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).