[Qemu-devel] [PATCH 06/35] x86: Refine error reporting of MCE injection services

[Marcelo Tosatti]

From: Jan Kiszka <address@hidden>

As this service is used by the human monitor, make sure that errors get
reported to the right channel, and also raise the verbosity.

This requires to move Monitor typedef in qemu-common.h to resolve the
include dependency.

Signed-off-by: Jan Kiszka <address@hidden>
CC: Huang Ying <address@hidden>
CC: Hidetoshi Seto <address@hidden>
CC: Jin Dongming <address@hidden>
Signed-off-by: Marcelo Tosatti <address@hidden>
---
 monitor.c            |    4 +-
 qemu-common.h        |    6 ++--
 target-i386/cpu.h    |    6 ++--
 target-i386/helper.c |   79 +++++++++++++++++++++++++++++---------------------
 4 files changed, 54 insertions(+), 41 deletions(-)

diff --git a/monitor.c b/monitor.c
index 45b0cc2..662df7c 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2712,8 +2712,8 @@ static void do_inject_mce(Monitor *mon, const QDict 
*qdict)
     int broadcast = qdict_get_try_bool(qdict, "broadcast", 0);
 
     for (cenv = first_cpu; cenv != NULL; cenv = cenv->next_cpu) {
-        if (cenv->cpu_index == cpu_index && cenv->mcg_cap) {
-            cpu_x86_inject_mce(cenv, bank, status, mcg_status, addr, misc,
+        if (cenv->cpu_index == cpu_index) {
+            cpu_x86_inject_mce(mon, cenv, bank, status, mcg_status, addr, misc,
                                broadcast);
             break;
         }
diff --git a/qemu-common.h b/qemu-common.h
index 3fe1719..7a96dd1 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -18,6 +18,9 @@ typedef struct QEMUFile QEMUFile;
 typedef struct QEMUBH QEMUBH;
 typedef struct DeviceState DeviceState;
 
+struct Monitor;
+typedef struct Monitor Monitor;
+
 /* we put basic includes here to avoid repeating them in device drivers */
 #include <stdlib.h>
 #include <stdio.h>
@@ -327,9 +330,6 @@ void qemu_iovec_memset(QEMUIOVector *qiov, int c, size_t 
count);
 void qemu_iovec_memset_skip(QEMUIOVector *qiov, int c, size_t count,
                             size_t skip);
 
-struct Monitor;
-typedef struct Monitor Monitor;
-
 /* Convert a byte between binary and BCD.  */
 static inline uint8_t to_bcd(uint8_t val)
 {
diff --git a/target-i386/cpu.h b/target-i386/cpu.h
index 52bb48e..486af1d 100644
--- a/target-i386/cpu.h
+++ b/target-i386/cpu.h
@@ -987,8 +987,8 @@ static inline void cpu_get_tb_cpu_state(CPUState *env, 
target_ulong *pc,
 void do_cpu_init(CPUState *env);
 void do_cpu_sipi(CPUState *env);
 
-void cpu_x86_inject_mce(CPUState *cenv, int bank, uint64_t status,
-                        uint64_t mcg_status, uint64_t addr, uint64_t misc,
-                        int broadcast);
+void cpu_x86_inject_mce(Monitor *mon, CPUState *cenv, int bank,
+                        uint64_t status, uint64_t mcg_status, uint64_t addr,
+                        uint64_t misc, int broadcast);
 
 #endif /* CPU_I386_H */
diff --git a/target-i386/helper.c b/target-i386/helper.c
index ba3bed9..462d332 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -30,6 +30,7 @@
 #include "kvm_x86.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu.h"
+#include "monitor.h"
 #endif
 
 //#define DEBUG_MMU
@@ -1067,33 +1068,38 @@ static void breakpoint_handler(CPUState *env)
 }
 
 static void
-qemu_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
+qemu_inject_x86_mce(Monitor *mon, CPUState *cenv, int bank, uint64_t status,
                     uint64_t mcg_status, uint64_t addr, uint64_t misc)
 {
     uint64_t mcg_cap = cenv->mcg_cap;
-    uint64_t *banks = cenv->mce_banks;
-
-    /*
-     * if MSR_MCG_CTL is not all 1s, the uncorrected error
-     * reporting is disabled
-     */
-    if ((status & MCI_STATUS_UC) && (mcg_cap & MCG_CTL_P) &&
-        cenv->mcg_ctl != ~(uint64_t)0) {
-        return;
-    }
-    banks += 4 * bank;
-    /*
-     * if MSR_MCi_CTL is not all 1s, the uncorrected error
-     * reporting is disabled for the bank
-     */
-    if ((status & MCI_STATUS_UC) && banks[0] != ~(uint64_t)0) {
-        return;
-    }
+    uint64_t *banks = cenv->mce_banks + 4 * bank;
+
     if (status & MCI_STATUS_UC) {
+        /*
+         * if MSR_MCG_CTL is not all 1s, the uncorrected error
+         * reporting is disabled
+         */
+        if ((mcg_cap & MCG_CTL_P) && cenv->mcg_ctl != ~(uint64_t)0) {
+            monitor_printf(mon,
+                           "CPU %d: Uncorrected error reporting disabled\n",
+                           cenv->cpu_index);
+            return;
+        }
+
+        /*
+         * if MSR_MCi_CTL is not all 1s, the uncorrected error
+         * reporting is disabled for the bank
+         */
+        if (banks[0] != ~(uint64_t)0) {
+            monitor_printf(mon, "CPU %d: Uncorrected error reporting disabled "
+                           "for bank %d\n", cenv->cpu_index, bank);
+            return;
+        }
+
         if ((cenv->mcg_status & MCG_STATUS_MCIP) ||
             !(cenv->cr[4] & CR4_MCE_MASK)) {
-            fprintf(stderr, "injects mce exception while previous "
-                    "one is in progress!\n");
+            monitor_printf(mon, "CPU %d: Previous MCE still in progress, "
+                                "raising triple fault\n", cenv->cpu_index);
             qemu_log_mask(CPU_LOG_RESET, "Triple fault\n");
             qemu_system_reset_request();
             return;
@@ -1119,23 +1125,29 @@ qemu_inject_x86_mce(CPUState *cenv, int bank, uint64_t 
status,
     }
 }
 
-void cpu_x86_inject_mce(CPUState *cenv, int bank, uint64_t status,
-                        uint64_t mcg_status, uint64_t addr, uint64_t misc,
-                        int broadcast)
+void cpu_x86_inject_mce(Monitor *mon, CPUState *cenv, int bank,
+                        uint64_t status, uint64_t mcg_status, uint64_t addr,
+                        uint64_t misc, int broadcast)
 {
     unsigned bank_num = cenv->mcg_cap & 0xff;
     CPUState *env;
     int flag = 0;
 
-    if (bank >= bank_num || !(status & MCI_STATUS_VAL)) {
+    if (!cenv->mcg_cap) {
+        monitor_printf(mon, "MCE injection not supported\n");
         return;
     }
-
-    if (broadcast) {
-        if (!cpu_x86_support_mca_broadcast(cenv)) {
-            fprintf(stderr, "Current CPU does not support broadcast\n");
-            return;
-        }
+    if (bank >= bank_num) {
+        monitor_printf(mon, "Invalid MCE bank number\n");
+        return;
+    }
+    if (!(status & MCI_STATUS_VAL)) {
+        monitor_printf(mon, "Invalid MCE status code\n");
+        return;
+    }
+    if (broadcast && !cpu_x86_support_mca_broadcast(cenv)) {
+        monitor_printf(mon, "Guest CPU does not support MCA broadcast\n");
+        return;
     }
 
     if (kvm_enabled()) {
@@ -1145,13 +1157,14 @@ void cpu_x86_inject_mce(CPUState *cenv, int bank, 
uint64_t status,
 
         kvm_inject_x86_mce(cenv, bank, status, mcg_status, addr, misc, flag);
     } else {
-        qemu_inject_x86_mce(cenv, bank, status, mcg_status, addr, misc);
+        qemu_inject_x86_mce(mon, cenv, bank, status, mcg_status, addr, misc);
         if (broadcast) {
             for (env = first_cpu; env != NULL; env = env->next_cpu) {
                 if (cenv == env) {
                     continue;
                 }
-                qemu_inject_x86_mce(env, 1, MCI_STATUS_VAL | MCI_STATUS_UC,
+                qemu_inject_x86_mce(mon, env, 1,
+                                    MCI_STATUS_VAL | MCI_STATUS_UC,
                                     MCG_STATUS_MCIP | MCG_STATUS_RIPV, 0, 0);
             }
         }
-- 
1.7.4