[v4,3/3] xen/x86: issue pci_serr error message via NMI continuation

Commit Message

Jürgen Groß Nov. 9, 2020, 9:50 a.m. UTC

Instead of using a softirq pci_serr_error() can use NMI continuation
for issuing an error message.

Signed-off-by: Juergen Gross <[e-mail]>
---
V4:
- rework to less generic approach
---
 xen/arch/x86/traps.c          | 21 +++++++++++++++------
 xen/include/asm-x86/softirq.h |  5 ++---
 2 files changed, 17 insertions(+), 9 deletions(-)

Comments

Jan Beulich Nov. 12, 2020, 9:29 a.m. UTC | #1

On 09.11.2020 10:50, Juergen Gross wrote:
> Instead of using a softirq pci_serr_error() can use NMI continuation
> for issuing an error message.
> 
> Signed-off-by: Juergen Gross <[e-mail]>

Reviewed-by: Jan Beulich <[e-mail]>
with one minor change to be considered:

> @@ -1808,6 +1816,9 @@ bool nmi_check_continuation(void)
>      if ( nmi_oprofile_send_virq() )
>          ret = true;
>  
> +    if ( pci_serr_nmicont() )
> +        ret = true;
> +
>      return ret;
>  }

As the likely more important part, wouldn't it be better to insert
this ahead of the oprofile check?

Jan

Jürgen Groß Nov. 12, 2020, 10:50 a.m. UTC | #2

On 12.11.20 10:29, Jan Beulich wrote:
> On 09.11.2020 10:50, Juergen Gross wrote:
>> Instead of using a softirq pci_serr_error() can use NMI continuation
>> for issuing an error message.
>>
>> Signed-off-by: Juergen Gross <[e-mail]>
> 
> Reviewed-by: Jan Beulich <[e-mail]>
> with one minor change to be considered:
> 
>> @@ -1808,6 +1816,9 @@ bool nmi_check_continuation(void)
>>       if ( nmi_oprofile_send_virq() )
>>           ret = true;
>>   
>> +    if ( pci_serr_nmicont() )
>> +        ret = true;
>> +
>>       return ret;
>>   }
> 
> As the likely more important part, wouldn't it be better to insert
> this ahead of the oprofile check?

Fine with me.


Juergen

Patch

diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 7cb7d7e09c..6aeccef32d 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1660,10 +1660,18 @@  void do_general_protection(struct cpu_user_regs *regs)
     panic("GENERAL PROTECTION FAULT\n[error_code=%04x]\n", regs->error_code);
 }
 
-static void pci_serr_softirq(void)
+static bool pci_serr_cont;
+
+static bool pci_serr_nmicont(void)
 {
+    if ( !pci_serr_cont )
+        return false;
+
+    pci_serr_cont = false;
     printk("\n\nNMI - PCI system error (SERR)\n");
     outb(inb(0x61) & 0x0b, 0x61); /* re-enable the PCI SERR error line. */
+
+    return true;
 }
 
 static void nmi_hwdom_report(unsigned int reason_idx)
@@ -1688,9 +1696,9 @@  static void pci_serr_error(const struct cpu_user_regs *regs)
         nmi_hwdom_report(_XEN_NMIREASON_pci_serr);
         /* fallthrough */
     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);
+        /* Issue error message in NMI continuation. */
+        pci_serr_cont = true;
+        trigger_nmi_continuation();
         break;
     default:  /* 'fatal' */
         console_force_unlock();
@@ -1808,6 +1816,9 @@  bool nmi_check_continuation(void)
     if ( nmi_oprofile_send_virq() )
         ret = true;
 
+    if ( pci_serr_nmicont() )
+        ret = true;
+
     return ret;
 }
 
@@ -2154,8 +2165,6 @@  void __init trap_init(void)
     percpu_traps_init();
 
     cpu_init();
-
-    open_softirq(PCI_SERR_SOFTIRQ, pci_serr_softirq);
 }
 
 void activate_debugregs(const struct vcpu *curr)
diff --git a/xen/include/asm-x86/softirq.h b/xen/include/asm-x86/softirq.h
index 0b7a77f11f..415ee866c7 100644
--- a/xen/include/asm-x86/softirq.h
+++ b/xen/include/asm-x86/softirq.h
@@ -6,9 +6,8 @@ 
 #define VCPU_KICK_SOFTIRQ      (NR_COMMON_SOFTIRQS + 2)
 
 #define MACHINE_CHECK_SOFTIRQ  (NR_COMMON_SOFTIRQS + 3)
-#define PCI_SERR_SOFTIRQ       (NR_COMMON_SOFTIRQS + 4)
-#define HVM_DPCI_SOFTIRQ       (NR_COMMON_SOFTIRQS + 5)
-#define NR_ARCH_SOFTIRQS       6
+#define HVM_DPCI_SOFTIRQ       (NR_COMMON_SOFTIRQS + 4)
+#define NR_ARCH_SOFTIRQS       5
 
 bool arch_skip_send_event_check(unsigned int cpu);