qemu-kvm.git uq/master queue

Port qemu-kvm's MCE support

commit c68b2374c9048812f488e00ffb95db66c0bc07a7

Author: Huang Ying<[email protected]>
Date:   Mon Jul 20 10:00:53 2009 +0800

Add MCE simulation support to qemu/kvm

KVM ioctls are used to initialize MCE simulation and inject MCE. The real MCE simulation is implemented in Linux kernel. The Kernel part has been merged.

Signed-off-by Marcelo Tosatti<[email protected]>

Signed-off-by Avi Kivity<[email protected]>

---
  target-i386/helper.c  |    6 +++
  target-i386/kvm.c     |   84 +++++++++++++++++++++++++++++++++++++++++++++++++
  target-i386/kvm_x86.h |   21 ++++++++++++
  3 files changed, 111 insertions(+), 0 deletions(-)

create mode 100644 target-i386/kvm_x86.h

target-i386/helper.c +6 -0

index e134340..4b430dd 100644
@@ -27,6 +27,7 @@
  #include "exec-all.h"
  #include "qemu-common.h"
  #include "kvm.h"
+#include "kvm_x86.h"
 
  //#define DEBUG_MMU
 

@@ -1030,6 +1031,11 @@ void cpu_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
      if (bank>= bank_num || !(status&  MCI_STATUS_VAL))
          return;
 
+    if (kvm_enabled()) {
+        kvm_inject_x86_mce(cenv, bank, status, mcg_status, addr, misc);
+        return;
+    }
+
      /*
       * if MSR_MCG_CTL is not all 1s, the uncorrected error
       * reporting is disabled

target-i386/kvm.c +21 -0

index 74e7b4f..343fb02 100644
@@ -27,6 +27,7 @@
  #include "hw/pc.h"
  #include "hw/apic.h"
  #include "ioport.h"
+#include "kvm_x86.h"
 
  #ifdef CONFIG_KVM_PARA
  #include<linux/kvm_para.h>

@@ -167,6 +168,67 @@ static int get_para_features(CPUState *env)
  }
  #endif
 
+#ifdef KVM_CAP_MCE
+static int kvm_get_mce_cap_supported(KVMState *s, uint64_t *mce_cap,
+                                     int *max_banks)
+{
+    int r;
+
+    r = kvm_ioctl(s, KVM_CHECK_EXTENSION, KVM_CAP_MCE);
+    if (r>  0) {
+        *max_banks = r;
+        return kvm_ioctl(s, KVM_X86_GET_MCE_CAP_SUPPORTED, mce_cap);
+    }
+    return -ENOSYS;
+}
+
+static int kvm_setup_mce(CPUState *env, uint64_t *mcg_cap)
+{
+    return kvm_vcpu_ioctl(env, KVM_X86_SETUP_MCE, mcg_cap);
+}
+
+static int kvm_set_mce(CPUState *env, struct kvm_x86_mce *m)

target-i386/kvm.c +32 -0

+}
+#endif
+
+void kvm_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
+                        uint64_t mcg_status, uint64_t addr, uint64_t misc)
+{
+#ifdef KVM_CAP_MCE
+    struct kvm_x86_mce mce = {
+        .bank = bank,
+        .status = status,
+        .mcg_status = mcg_status,
+        .addr = addr,
+        .misc = misc,
+    };
+    struct kvm_x86_mce_data data = {
+            .env = cenv,
+            .mce =&mce,
+    };
+
+    run_on_cpu(cenv, kvm_do_inject_x86_mce,&data);
+#endif
+}
+
  int kvm_arch_init_vcpu(CPUState *env)
  {
      struct {

@@ -277,6 +339,28 @@ int kvm_arch_init_vcpu(CPUState *env)
 
      cpuid_data.cpuid.nent = cpuid_i;
 
+#ifdef KVM_CAP_MCE
+    if (((env->cpuid_version>>  8)&0xF)>= 6
+&&  (env->cpuid_features&(CPUID_MCE|CPUID_MCA)) == (CPUID_MCE|CPUID_MCA)
+&&  kvm_check_extension(env->kvm_state, KVM_CAP_MCE)>  0) {
+        uint64_t mcg_cap;
+        int banks;
+
+        if (kvm_get_mce_cap_supported(env->kvm_state,&mcg_cap,&banks))
+            perror("kvm_get_mce_cap_supported FAILED");

target-i386/kvm.c +4 -0

+        }
+    }
+#endif
+
      return kvm_vcpu_ioctl(env, KVM_SET_CPUID2,&cpuid_data);
  }

target-i386/kvm_x86.h new file +21 -0

new file mode 100644
index 0000000..c1ebd24
@@ -0,0 +1,21 @@
+/*
+ * QEMU KVM support
+ *
+ * Copyright (C) 2009 Red Hat Inc.
+ * Copyright IBM, Corp. 2008
+ *
+ * Authors:
+ *  Anthony Liguori<[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef __KVM_X86_H__
+#define __KVM_X86_H__
+
+void kvm_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
+                        uint64_t mcg_status, uint64_t addr, uint64_t misc);
+
+#endif

From: Huang Ying<[email protected]>

In QEMU-KVM, physical address != RAM address. While MCE simulation needs physical address instead of RAM address. So kvm_physical_memory_addr_from_ram() is implemented to do the conversion, and it is invoked before being filled in the IA32_MCi_ADDR MSR.

Reported-by Dean Nelson<[email protected]>

Signed-off-by Huang Ying<[email protected]>

Signed-off-by Marcelo Tosatti<[email protected]>

Signed-off-by Avi Kivity<[email protected]>

---
  kvm-all.c |   18 ++++++++++++++++++
  kvm.h     |    6 ++++++
  2 files changed, 24 insertions(+), 0 deletions(-)

kvm-all.c +5 -0

index 1cc696f..37b99c7 100644
@@ -137,6 +137,24 @@ static KVMSlot *kvm_lookup_overlapping_slot(KVMState *s,
      return found;
  }
 
+int kvm_physical_memory_addr_from_ram(KVMState *s, ram_addr_t ram_addr,
+                                      target_phys_addr_t *phys_addr)
+{
+    int i;
+

kvm-all.c +3 -0

+    return 0;
+}
+
  static int kvm_set_user_memory_region(KVMState *s, KVMSlot *slot)
  {
      struct kvm_userspace_memory_region mem;

kvm.h +6 -0

index 50b6c01..b2fb3af 100644
@@ -174,6 +174,12 @@ static inline void cpu_synchronize_post_init(CPUState *env)
      }
  }
 
+
+#if !defined(CONFIG_USER_ONLY)
+int kvm_physical_memory_addr_from_ram(KVMState *s, ram_addr_t ram_addr,
+                                      target_phys_addr_t *phys_addr);
+#endif
+
  #endif
  int kvm_set_ioeventfd_mmio_long(int fd, uint32_t adr, uint32_t val, bool assign);
 

Port qemu-kvm's

commit 4b62fff1101a7ad77553147717a8bd3bf79df7ef

Author: Huang Ying<[email protected]>
Date:   Mon Sep 21 10:43:25 2009 +0800

MCE: Relay UCR MCE to guest

UCR (uncorrected recovery) MCE is supported in recent Intel CPUs, where some hardware error such as some memory error can be reported without PCC (processor context corrupted). To recover from such MCE, the corresponding memory will be unmapped, and all processes accessing the memory will be killed via SIGBUS.

For KVM, if QEMU/KVM is killed, all guest processes will be killed too. So we relay SIGBUS from host OS to guest system via a UCR MCE injection. Then guest OS can isolate corresponding memory and kill necessary guest processes only. SIGBUS sent to main thread (not VCPU threads) will be broadcast to all VCPU threads as UCR MCE.

Signed-off-by Marcelo Tosatti<[email protected]>

Signed-off-by Avi Kivity<[email protected]>

---
  cpus.c                |   82 ++++++++++++++++++++--
  kvm-stub.c            |    5 ++
  kvm.h                 |    3 +
  target-i386/cpu.h     |   20 +++++-
  target-i386/helper.c  |    2 +-
  target-i386/kvm.c     |  178 ++++++++++++++++++++++++++++++++++++++++++++++++-
  target-i386/kvm_x86.h |    3 +-
  7 files changed, 279 insertions(+), 14 deletions(-)

cpus.c +0 -0

index 3875657..ad58c55 100644

cpus.c +19 -0

  #ifdef SIGRTMIN
  #define SIG_IPI (SIGRTMIN+4)

@@ -41,6 +45,10 @@
  #define SIG_IPI SIGUSR1
  #endif
 
+#ifndef PR_MCE_KILL
+#define PR_MCE_KILL 33
+#endif
+
  static CPUState *next_cpu;
 
  /***********************************************************/

@@ -498,28 +506,77 @@ static void qemu_tcg_wait_io_event(void)
      }
  }
 
+static void sigbus_reraise(void)
+{
+    sigset_t set;
+    struct sigaction action;
+
+    memset(&action, 0, sizeof(action));
+    action.sa_handler = SIG_DFL;
+    if (!sigaction(SIGBUS,&action, NULL)) {
+        raise(SIGBUS);
+        sigemptyset(&set);
+        sigaddset(&set, SIGBUS);
+        sigprocmask(SIG_UNBLOCK,&set, NULL);
+    }
+    perror("Failed to re-raise SIGBUS!\n");
+    abort();

cpus.c +25 -8

+}
+
  static void qemu_kvm_eat_signal(CPUState *env, int timeout)
  {
      struct timespec ts;
      int r, e;
      siginfo_t siginfo;
      sigset_t waitset;
+    sigset_t chkset;
 
      ts.tv_sec = timeout / 1000;
      ts.tv_nsec = (timeout % 1000) * 1000000;
 
      sigemptyset(&waitset);
      sigaddset(&waitset, SIG_IPI);
+    sigaddset(&waitset, SIGBUS);
 
-    qemu_mutex_unlock(&qemu_global_mutex);
-    r = sigtimedwait(&waitset,&siginfo,&ts);
-    e = errno;
-    qemu_mutex_lock(&qemu_global_mutex);
+    do {
+        qemu_mutex_unlock(&qemu_global_mutex);
 
-    if (r == -1&&  !(e == EAGAIN || e == EINTR)) {
-        fprintf(stderr, "sigtimedwait: %s\n", strerror(e));
-        exit(1);
-    }
+        r = sigtimedwait(&waitset,&siginfo,&ts);
+        e = errno;
+
+        qemu_mutex_lock(&qemu_global_mutex);
+
+        if (r == -1&&  !(e == EAGAIN || e == EINTR)) {
+            fprintf(stderr, "sigtimedwait: %s\n", strerror(e));
+            exit(1);
+        }
+
+        switch (r) {
+        case SIGBUS:
+#ifdef TARGET_I386
+            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr))
+#endif
+                sigbus_reraise();
+            break;
+        default:
+            break;

static void qemu_kvm_wait_io_event(CPUState *env)

cpus.c +2 -0

@@ -640,6 +697,7 @@ static void kvm_init_ipi(CPUState *env)
 
      pthread_sigmask(SIG_BLOCK, NULL,&set);
      sigdelset(&set, SIG_IPI);
+    sigdelset(&set, SIGBUS);
      r = kvm_set_signal_mask(env,&set);
      if (r) {
          fprintf(stderr, "kvm_set_signal_mask: %s\n", strerror(r));

@@ -650,6 +708,7 @@ static void kvm_init_ipi(CPUState *env)
  static sigset_t block_io_signals(void)
  {
      sigset_t set;
+    struct sigaction action;
 
      /* SIGUSR2 used by posix-aio-compat.c */
      sigemptyset(&set);

@@ -660,8 +719,15 @@ static sigset_t block_io_signals(void)
      sigaddset(&set, SIGIO);
      sigaddset(&set, SIGALRM);

cpus.c +0 -0

      return set;
  }

kvm-stub.c +5 -0

index d45f9fa..5384a4b 100644
@@ -141,3 +141,8 @@ int kvm_set_ioeventfd_mmio_long(int fd, uint32_t adr, uint32_t val, bool assign)
  {
      return -ENOSYS;
  }
+
+int kvm_on_sigbus(int code, void *addr)
+{
+    return 1;
+}

kvm.h +3 -0

index b2fb3af..60a9b42 100644
@@ -110,6 +110,9 @@ int kvm_arch_init_vcpu(CPUState *env);
 
  void kvm_arch_reset_vcpu(CPUState *env);
 
+int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr);
+int kvm_on_sigbus(int code, void *addr);
+
  struct kvm_guest_debug;
  struct kvm_debug_exit_arch;
 

target-i386/cpu.h +18 -2

index 77eeab1..85ed30f 100644
@@ -250,16 +250,32 @@
  #define PG_ERROR_RSVD_MASK 0x08
  #define PG_ERROR_I_D_MASK  0x10
 
-#define MCG_CTL_P	(1UL<<8)   /* MCG_CAP register available */
+#define MCG_CTL_P	(1ULL<<8)   /* MCG_CAP register available */
+#define MCG_SER_P	(1ULL<<24) /* MCA recovery/new status bits */
 
-#define MCE_CAP_DEF	MCG_CTL_P
+#define MCE_CAP_DEF	(MCG_CTL_P|MCG_SER_P)
  #define MCE_BANKS_DEF	10
 
+#define MCG_STATUS_RIPV	(1ULL<<0)   /* restart ip valid */
+#define MCG_STATUS_EIPV	(1ULL<<1)   /* ip points to correct instruction */
  #define MCG_STATUS_MCIP	(1ULL<<2)   /* machine check in progress */
 
  #define MCI_STATUS_VAL	(1ULL<<63)  /* valid error */
  #define MCI_STATUS_OVER	(1ULL<<62)  /* previous errors lost */
  #define MCI_STATUS_UC	(1ULL<<61)  /* uncorrected error */
+#define MCI_STATUS_EN	(1ULL<<60)  /* error enabled */
+#define MCI_STATUS_MISCV (1ULL<<59) /* misc error reg. valid */
+#define MCI_STATUS_ADDRV (1ULL<<58) /* addr reg. valid */
+#define MCI_STATUS_PCC	(1ULL<<57)  /* processor context corrupt */
+#define MCI_STATUS_S	(1ULL<<56)  /* Signaled machine check */
+#define MCI_STATUS_AR	(1ULL<<55)  /* Action required */
+
+/* MISC register defines */
+#define MCM_ADDR_SEGOFF	0	/* segment offset */
+#define MCM_ADDR_LINEAR	1	/* linear address */
+#define MCM_ADDR_PHYS	2	/* physical address */
+#define MCM_ADDR_MEM	3	/* memory address */
+#define MCM_ADDR_GENERIC 7	/* generic */
 
  #define MSR_IA32_TSC                    0x10
  #define MSR_IA32_APICBASE               0x1b

target-i386/helper.c +1 -1

index 4b430dd..4fff4a8 100644
@@ -1032,7 +1032,7 @@ void cpu_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
          return;
 
      if (kvm_enabled()) {
-        kvm_inject_x86_mce(cenv, bank, status, mcg_status, addr, misc);
+        kvm_inject_x86_mce(cenv, bank, status, mcg_status, addr, misc, 0);
          return;
      }
 

target-i386/kvm.c +7 -0

index 343fb02..8e26bc4 100644
@@ -46,6 +46,13 @@
  #define MSR_KVM_WALL_CLOCK  0x11
  #define MSR_KVM_SYSTEM_TIME 0x12
 
+#ifndef BUS_MCEERR_AR
+#define BUS_MCEERR_AR 4
+#endif
+#ifndef BUS_MCEERR_AO
+#define BUS_MCEERR_AO 5
+#endif
+
  #ifdef KVM_CAP_EXT_CPUID
 
  static struct kvm_cpuid2 *try_get_cpuid(KVMState *s, int max)

target-i386/kvm.c +26 -0

+    int r;
+
+    kmsrs->nmsrs = n;
+    memcpy(kmsrs->entries, msrs, n * sizeof *msrs);
+    r = kvm_vcpu_ioctl(env, KVM_GET_MSRS, kmsrs);
+    memcpy(msrs, kmsrs->entries, n * sizeof *msrs);
+    free(kmsrs);
+    return r;
+}
+
+/* FIXME: kill this and kvm_get_msr, use env->mcg_status instead */
+static int kvm_mce_in_exception(CPUState *env)
+{
+    struct kvm_msr_entry msr_mcg_status = {
+        .index = MSR_MCG_STATUS,
+    };
+    int r;
+
+    r = kvm_get_msr(env,&msr_mcg_status, 1);
+    if (r == -1 || r == 0) {
+        return -1;
+    }
+    return !!(msr_mcg_status.data&  MCG_STATUS_MCIP);
+}
+
  struct kvm_x86_mce_data
  {
      CPUState *env;
      struct kvm_x86_mce *mce;
+    int abort_on_error;
  };
 

static void kvm_do_inject_x86_mce(void *_data)

target-i386/kvm.c +0 -0

@@ -203,14 +239,26 @@ static void kvm_do_inject_x86_mce(void *_data)
      struct kvm_x86_mce_data *data = _data;

target-i386/kvm.c +7 -2

      r = kvm_set_mce(data->env, data->mce);
-    if (r<  0)
+    if (r<  0) {
          perror("kvm_set_mce FAILED");
+        if (data->abort_on_error) {
+            abort();
+        }
+    }
  }
  #endif
 
  void kvm_inject_x86_mce(CPUState *cenv, int bank, uint64_t status,
-                        uint64_t mcg_status, uint64_t addr, uint64_t misc)
+                        uint64_t mcg_status, uint64_t addr, uint64_t misc,
+                        int abort_on_error)
  {
  #ifdef KVM_CAP_MCE

      struct kvm_x86_mce mce = {

target-i386/kvm.c +34 -0

+int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+{
+#if defined(KVM_CAP_MCE)
+    struct kvm_x86_mce mce = {
+            .bank = 9,
+    };
+    void *vaddr;
+    ram_addr_t ram_addr;
+    target_phys_addr_t paddr;
+    int r;
+
+    if ((env->mcg_cap&  MCG_SER_P)&&  addr
+&&  (code == BUS_MCEERR_AR
+            || code == BUS_MCEERR_AO)) {
+        if (code == BUS_MCEERR_AR) {
+            /* Fake an Intel architectural Data Load SRAR UCR */
+            mce.status = MCI_STATUS_VAL | MCI_STATUS_UC | MCI_STATUS_EN
+                | MCI_STATUS_MISCV | MCI_STATUS_ADDRV | MCI_STATUS_S
+                | MCI_STATUS_AR | 0x134;
+            mce.misc = (MCM_ADDR_PHYS<<  6) | 0xc;
+            mce.mcg_status = MCG_STATUS_MCIP | MCG_STATUS_EIPV;
+        } else {
+            /*
+             * If there is an MCE excpetion being processed, ignore
+             * this SRAO MCE
+             */
+            r = kvm_mce_in_exception(env);
+            if (r == -1) {
+                fprintf(stderr, "Failed to get MCE status\n");
+            } else if (r) {
+                return 0;
+            }
+            /* Fake an Intel architectural Memory scrubbing UCR */
+            mce.status = MCI_STATUS_VAL | MCI_STATUS_UC | MCI_STATUS_EN

target-i386/kvm.c +13 -0

+        }
+    } else
+#endif
+    {
+        if (code == BUS_MCEERR_AO) {
+            return 0;
+        } else if (code == BUS_MCEERR_AR) {
+            hardware_memory_error();
+        } else {
+            return 1;
+        }
+    }
+    return 0;

target-i386/kvm.c +25 -0

+            return 0;
+        }
+        status = MCI_STATUS_VAL | MCI_STATUS_UC | MCI_STATUS_EN
+            | MCI_STATUS_MISCV | MCI_STATUS_ADDRV | MCI_STATUS_S
+            | 0xc0;
+        kvm_inject_x86_mce(first_cpu, 9, status,
+                           MCG_STATUS_MCIP | MCG_STATUS_RIPV, paddr,
+                           (MCM_ADDR_PHYS<<  6) | 0xc, 1);
+        for (cenv = first_cpu->next_cpu; cenv != NULL; cenv = cenv->next_cpu) {
+            kvm_inject_x86_mce(cenv, 1, MCI_STATUS_VAL | MCI_STATUS_UC,
+                               MCG_STATUS_MCIP | MCG_STATUS_RIPV, 0, 0, 1);
+        }
+    } else
+#endif
+    {
+        if (code == BUS_MCEERR_AO) {
+            return 0;
+        } else if (code == BUS_MCEERR_AR) {
+            hardware_memory_error();
+        } else {
+            return 1;
+        }
+    }
+    return 0;
+}

target-i386/kvm_x86.h +0 -0

index c1ebd24..04932cf 100644

Quoted 57 lines

cpus.c +20 -8

+}
+
  static void qemu_kvm_eat_signal(CPUState *env, int timeout)
  {
      struct timespec ts;
      int r, e;
      siginfo_t siginfo;
      sigset_t waitset;
+    sigset_t chkset;
 
      ts.tv_sec = timeout / 1000;
      ts.tv_nsec = (timeout % 1000) * 1000000;
 
      sigemptyset(&waitset);
      sigaddset(&waitset, SIG_IPI);
+    sigaddset(&waitset, SIGBUS);
 
-    qemu_mutex_unlock(&qemu_global_mutex);
-    r = sigtimedwait(&waitset,&siginfo,&ts);
-    e = errno;
-    qemu_mutex_lock(&qemu_global_mutex);
+    do {
+        qemu_mutex_unlock(&qemu_global_mutex);
 
-    if (r == -1&&  !(e == EAGAIN || e == EINTR)) {
-        fprintf(stderr, "sigtimedwait: %s\n", strerror(e));
-        exit(1);
-    }
+        r = sigtimedwait(&waitset,&siginfo,&ts);
+        e = errno;
+
+        qemu_mutex_lock(&qemu_global_mutex);
+
+        if (r == -1&&  !(e == EAGAIN || e == EINTR)) {
+            fprintf(stderr, "sigtimedwait: %s\n", strerror(e));
+            exit(1);
+        }
+
+        switch (r) {
+        case SIGBUS:
+#ifdef TARGET_I386
+            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, 

siginfo.si_addr))

cpus.c +13 -0

+#endif
+                sigbus_reraise();
+            break;
+        default:
+            break;
+        }
+
+        r = sigpending(&chkset);
+        if (r == -1) {
+            fprintf(stderr, "sigpending: %s\n", strerror(e));
+            exit(1);
+        }
+    } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, 

Quoted 2 lines

SIGBUS));
}

I don't understand why this loop is needed but we specifically wait for a signal to get delivered that's either SIG_IPI or SIGBUS. We then check whether a SIG_IPI or SIGBUS is pending and loop waiting for signals again.

Shouldn't we be looping on just sigismember(SIGBUS)?

BTW, we're no longer respecting timeout because we're not adjusting ts after each iteration.

Quoted 65 lines · Anthony Liguori

Quoted 57 lines

cpus.c +20 -8

+}
+
 static void qemu_kvm_eat_signal(CPUState *env, int timeout)
 {
     struct timespec ts;
     int r, e;
     siginfo_t siginfo;
     sigset_t waitset;
+    sigset_t chkset;
 
     ts.tv_sec = timeout / 1000;
     ts.tv_nsec = (timeout % 1000) * 1000000;
 
     sigemptyset(&waitset);
     sigaddset(&waitset, SIG_IPI);
+    sigaddset(&waitset, SIGBUS);
 
-    qemu_mutex_unlock(&qemu_global_mutex);
-    r = sigtimedwait(&waitset,&siginfo,&ts);
-    e = errno;
-    qemu_mutex_lock(&qemu_global_mutex);
+    do {
+        qemu_mutex_unlock(&qemu_global_mutex);
 
-    if (r == -1&&  !(e == EAGAIN || e == EINTR)) {
-        fprintf(stderr, "sigtimedwait: %s\n", strerror(e));
-        exit(1);
-    }
+        r = sigtimedwait(&waitset,&siginfo,&ts);
+        e = errno;
+
+        qemu_mutex_lock(&qemu_global_mutex);
+
+        if (r == -1&&  !(e == EAGAIN || e == EINTR)) {
+            fprintf(stderr, "sigtimedwait: %s\n", strerror(e));
+            exit(1);
+        }
+
+        switch (r) {
+        case SIGBUS:
+#ifdef TARGET_I386
+            if (kvm_on_sigbus_vcpu(env, siginfo.si_code,

siginfo.si_addr))

cpus.c +13 -0

+#endif
+                sigbus_reraise();
+            break;
+        default:
+            break;
+        }
+
+        r = sigpending(&chkset);
+        if (r == -1) {
+            fprintf(stderr, "sigpending: %s\n", strerror(e));
+            exit(1);
+        }
+    } while (sigismember(&chkset, SIG_IPI) ||

Quoted 2 lines

sigismember(&chkset, SIGBUS));
}

I don't understand why this loop is needed but we specifically wait for a signal to get delivered that's either SIG_IPI or SIGBUS. We then check whether a SIG_IPI or SIGBUS is pending and loop waiting for signals again.

Quoted 4 lines

Shouldn't we be looping on just sigismember(SIGBUS)?

BTW, we're no longer respecting timeout because we're not adjusting ts after each iteration.

I think this is important too. The last time I went through the code and played around here, it wasn't possible to set timeout to a very, very large value because there are still things that we poll
for (like whether shutdown has occurred). If we loop indefinitely
without reducing ts, we can potentially recreate an infinite timeout which means we won't catch any of the events we poll for. This would be a very, very subtle bug to track down.

Quoted 17 lines

cpus.c +17 -0

+        e = errno;
+
+        qemu_mutex_lock(&qemu_global_mutex);
+
+        if (r == -1&&  !(e == EAGAIN || e == EINTR)) {
+            fprintf(stderr, "sigtimedwait: %s\n", strerror(e));
+            exit(1);
+        }
+
+        switch (r) {
+        case SIGBUS:
+#ifdef TARGET_I386
+            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr))
+#endif
+                sigbus_reraise();
+            break;
+        default:

cpus.c static void qemu_tcg_wait_io_event(void) · line 580

Quoted 10 lines

cpus.c +9 -0

+            break;
+        }
+
+        r = sigpending(&chkset);
+        if (r == -1) {
+            fprintf(stderr, "sigpending: %s\n", strerror(e));
+            exit(1);
+        }
+    } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS));
 }

I don't understand why this loop is needed but we specifically wait for a signal to get delivered that's either SIG_IPI or SIGBUS. We then check whether a SIG_IPI or SIGBUS is pending and loop waiting for signals again.

Shouldn't we be looping on just sigismember(SIGBUS)?

BTW, we're no longer respecting timeout because we're not adjusting ts after each iteration.

I don't understand why this loop is needed but we specifically wait for a signal to get delivered that's either SIG_IPI or SIGBUS. We then check whether a SIG_IPI or SIGBUS is pending and loop waiting for signals again.

Shouldn't we be looping on just sigismember(SIGBUS)?

BTW, we're no longer respecting timeout because we're not adjusting ts after each iteration.

Quoted 4 lines · Anthony Liguori

I don't understand why this loop is needed but we specifically wait for a signal to get delivered that's either SIG_IPI or SIGBUS. We then check whether a SIG_IPI or SIGBUS is pending and loop waiting for signals again.

Quoted 1 line

Shouldn't we be looping on just sigismember(SIGBUS)?

You mean because SIG_IPI is a real-time signal and standard signals are delivered first? OTOH, real-time signals can be queued multiple times so it makes sense to loop on SIG_IPI as well.

Quoted 2 lines

BTW, we're no longer respecting timeout because we're not adjusting ts after each iteration.

The timeout of qemu_kvm_eat_signal is always zero.

Paolo

Quoted 2 lines

The timeout of qemu_kvm_eat_signal is always zero.

So then qemu_kvm_eat_signal purely polls and it will happily keep polling as long as there is a signal pending.

So what's the point of doing a sigtimedwait() and dropping qemu_mutex?

Why not just check sigpending in a loop?

Port qemu-kvm's

commit 1bab5d11545d8de5facf46c28630085a2f9651ae

Author: Huang Ying<[email protected]>
Date:   Wed Mar 3 16:52:46 2010 +0800

Add savevm/loadvm support for MCE

MCE registers are saved/load into/from CPUState in kvm_arch_save/load_regs. To simulate the MCG_STATUS clearing upon reset, MSR_MCG_STATUS is set to 0 for KVM_PUT_RESET_STATE.

Signed-off-by Marcelo Tosatti<[email protected]>

Signed-off-by Avi Kivity<[email protected]>

---
  target-i386/kvm.c |   39 ++++++++++++++++++++++++++++++++++++++-
  1 files changed, 38 insertions(+), 1 deletions(-)

target-i386/kvm.c +4 -1

index 8e26bc4..1701cb9 100644
@@ -777,7 +777,7 @@ static int kvm_put_msrs(CPUState *env, int level)
          struct kvm_msr_entry entries[100];
      } msr_data;
      struct kvm_msr_entry *msrs = msr_data.entries;
-    int n = 0;
+    int i, n = 0;
 
      kvm_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_CS, env->sysenter_cs);
      kvm_msr_entry_set(&msrs[n++], MSR_IA32_SYSENTER_ESP, env->sysenter_esp);

@@ -797,6 +797,18 @@ static int kvm_put_msrs(CPUState *env, int level)
                            env->system_time_msr);
          kvm_msr_entry_set(&msrs[n++], MSR_KVM_WALL_CLOCK, env->wall_clock_msr);
      }
+#ifdef KVM_CAP_MCE
+    if (env->mcg_cap) {
+        if (level == KVM_PUT_RESET_STATE)

target-i386/kvm.c +0 -0

      msr_data.info.nmsrs = n;
 

@@ -1004,6 +1016,15 @@ static int kvm_get_msrs(CPUState *env)
      msrs[n++].index = MSR_KVM_SYSTEM_TIME;
      msrs[n++].index = MSR_KVM_WALL_CLOCK;
 

target-i386/kvm.c +0 -0

      msr_data.info.nmsrs = n;
      ret = kvm_vcpu_ioctl(env, KVM_GET_MSRS,&msr_data);

      if (ret<  0)

target-i386/kvm.c +16 -0

@@ -1046,6 +1067,22 @@ static int kvm_get_msrs(CPUState *env)
          case MSR_KVM_WALL_CLOCK:
              env->wall_clock_msr = msrs[i].data;
              break;
+#ifdef KVM_CAP_MCE
+        case MSR_MCG_STATUS:
+            env->mcg_status = msrs[i].data;
+            break;
+        case MSR_MCG_CTL:
+            env->mcg_ctl = msrs[i].data;
+            break;
+#endif
+        default:
+#ifdef KVM_CAP_MCE
+            if (msrs[i].index>= MSR_MC0_CTL&&
+                msrs[i].index<  MSR_MC0_CTL + (env->mcg_cap&  0xff) * 4) {
+                env->mce_banks[msrs[i].index - MSR_MC0_CTL] = msrs[i].data;
+                break;
+            }
+#endif
          }
      }
 

The following changes since commit 38cc9b607f85017b095793cab6c129bc9844f441:

   issue snd_pcm_start() when capturing audio (2010-10-18 00:39:06 +0400)

are available in the git repository at:
git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master

Huang Ying (1):

       Add RAM ->  physical addr mapping in MCE simulation

Joerg Roedel (2):
Set cpuid definition to 0 before initializing it Add svm cpuid features

Marcelo Tosatti (7):
signalfd compatibility
iothread: use signalfd
kvm: x86: add mce support
Export qemu_ram_addr_from_host
MCE: Relay UCR MCE to guest
Add savevm/loadvm support for MCE
Fix memory leak in register save load due to xsave support

  Makefile.objs         |    1 +
  compatfd.c            |  117 ++++++++++++++++++
  compatfd.h            |   43 +++++++
  configure             |   18 +++
  cpu-common.h          |    3 +-
  cpus.c                |  156 ++++++++++++++++++++++--
  exec-all.h            |    2 +-
  exec.c                |   26 +++--
  kvm-all.c             |   18 +++
  kvm-stub.c            |    5 +
  kvm.h                 |    9 ++
  target-i386/cpu.h     |   32 +++++-
  target-i386/cpuid.c   |   79 ++++++++++---
  target-i386/helper.c  |    6 +
  target-i386/kvm.c     |  311 ++++++++++++++++++++++++++++++++++++++++++++++++-
  target-i386/kvm_x86.h |   22 ++++
  16 files changed, 801 insertions(+), 47 deletions(-)

create mode 100644 compatfd.c
create mode 100644 compatfd.h
create mode 100644 target-i386/kvm_x86.h
--
To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 14 lines · Marcelo Tosatti

The following changes since commit 38cc9b607f85017b095793cab6c129bc9844f441:

issue snd_pcm_start() when capturing audio (2010-10-18 00:39:06 +0400)

are available in the git repository at:
git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master

Huang Ying (1):

      Add RAM ->  physical addr mapping in MCE simulation

Joerg Roedel (2):
Set cpuid definition to 0 before initializing it Add svm cpuid features

Quoted 8 lines

Marcelo Tosatti (7):
signalfd compatibility
iothread: use signalfd
kvm: x86: add mce support
Export qemu_ram_addr_from_host
MCE: Relay UCR MCE to guest
Add savevm/loadvm support for MCE
Fix memory leak in register save load due to xsave support

Does this fix the second build issue too is this is just with v2 of hte Relay UCR MCE to guest?

Quoted 25 lines · Anthony Liguori

Quoted 23 lines · Marcelo Tosatti

The following changes since commit 38cc9b607f85017b095793cab6c129bc9844f441:

   issue snd_pcm_start() when capturing audio (2010-10-18 00:39:06 +0400)

are available in the git repository at:
git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master

Huang Ying (1):

       Add RAM ->   physical addr mapping in MCE simulation

Joerg Roedel (2):
Set cpuid definition to 0 before initializing it Add svm cpuid features

Marcelo Tosatti (7):
signalfd compatibility
iothread: use signalfd
kvm: x86: add mce support
Export qemu_ram_addr_from_host
MCE: Relay UCR MCE to guest
Add savevm/loadvm support for MCE
Fix memory leak in register save load due to xsave support

Quoted 3 lines

Does this fix the second build issue too is this is just with v2 of hte Relay UCR MCE to guest?

Yes, second and third...

The following changes since commit 38cc9b607f85017b095793cab6c129bc9844f441:

   issue snd_pcm_start() when capturing audio (2010-10-18 00:39:06 +0400)

are available in the git repository at:
git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master

Huang Ying (1):

       Add RAM ->  physical addr mapping in MCE simulation

Joerg Roedel (2):
Set cpuid definition to 0 before initializing it Add svm cpuid features

Marcelo Tosatti (7):
signalfd compatibility
iothread: use signalfd
kvm: x86: add mce support
Export qemu_ram_addr_from_host
MCE: Relay UCR MCE to guest
Add savevm/loadvm support for MCE
Fix memory leak in register save load due to xsave support

  Makefile.objs         |    1 +
  compatfd.c            |  117 ++++++++++++++++++
  compatfd.h            |   43 +++++++
  configure             |   18 +++
  cpu-common.h          |    3 +-
  cpus.c                |  156 ++++++++++++++++++++++--
  exec-all.h            |    2 +-
  exec.c                |   26 +++--
  kvm-all.c             |   18 +++
  kvm-stub.c            |    5 +
  kvm.h                 |    9 ++
  target-i386/cpu.h     |   32 +++++-
  target-i386/cpuid.c   |   79 ++++++++++---
  target-i386/helper.c  |    6 +
  target-i386/kvm.c     |  311 ++++++++++++++++++++++++++++++++++++++++++++++++-
  target-i386/kvm_x86.h |   22 ++++
  16 files changed, 801 insertions(+), 47 deletions(-)

create mode 100644 compatfd.c
create mode 100644 compatfd.h
create mode 100644 target-i386/kvm_x86.h

The following changes since commit 38cc9b607f85017b095793cab6c129bc9844f441:

   issue snd_pcm_start() when capturing audio (2010-10-18 00:39:06 +0400)

are available in the git repository at:
git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master

Huang Ying (1):

       Add RAM ->  physical addr mapping in MCE simulation

Joerg Roedel (2):
Set cpuid definition to 0 before initializing it Add svm cpuid features

Marcelo Tosatti (7):
signalfd compatibility
iothread: use signalfd
kvm: x86: add mce support
Export qemu_ram_addr_from_host
MCE: Relay UCR MCE to guest
Add savevm/loadvm support for MCE
Fix memory leak in register save load due to xsave support

  Makefile.objs         |    1 +
  compatfd.c            |  117 ++++++++++++++++++
  compatfd.h            |   43 +++++++
  configure             |   18 +++
  cpu-common.h          |    3 +-
  cpus.c                |  156 ++++++++++++++++++++++--
  exec-all.h            |    2 +-
  exec.c                |   26 +++--
  kvm-all.c             |   18 +++
  kvm-stub.c            |    5 +
  kvm.h                 |    9 ++
  target-i386/cpu.h     |   32 +++++-
  target-i386/cpuid.c   |   79 ++++++++++---
  target-i386/helper.c  |    6 +
  target-i386/kvm.c     |  311 ++++++++++++++++++++++++++++++++++++++++++++++++-
  target-i386/kvm_x86.h |   22 ++++
  16 files changed, 801 insertions(+), 47 deletions(-)

create mode 100644 compatfd.c
create mode 100644 compatfd.h
create mode 100644 target-i386/kvm_x86.h