All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/18] [uq/master] MCE & IO exit fixes, prepare for VCPU loop reuse
@ 2011-01-10  8:31 ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:31 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti
  Cc: kvm, qemu-devel, Alexander Graf, Gleb Natapov, Huang Ying

This series has three major topics:
 - add required kernel reentry after IO exits
 - provide MCE forwarding under !CONFIG_IOTHREAD
 - prepare kvm_cpu_exec for qemu-kvm reuse

Along these lines, several cleanups and simplifcations are applied to
cpus.c and the KVM VCPU execution bits. The first patch should of course
be dropped if uq/master is applied without "kvm: Drop return value of
kvm_cpu_exec".

Note that I did not yet to test the MCE support. Is there an easy way to
trigger valid MCE events at kernel level? Or do I need to fake SIGBUS at
qemu level?

Jan Kiszka (18):
  Revert "kvm: Drop return value of kvm_cpu_exec"
  kvm: Drop redundant kvm_enabled from kvm_cpu_thread_fn
  kvm: Provide sigbus services arch-independently
  Refactor signal setup functions in cpus.c
  kvm: Set up signal mask also for !CONFIG_IOTHREAD
  kvm: Refactor qemu_kvm_eat_signals
  kvm: Add MCE signal support for !CONFIG_IOTHREAD
  kvm: Handle kvm_init_vcpu errors
  Refactor kvm&tcg function names in cpus.c
  Fix a few coding style violations in cpus.c
  Introduce VCPU self-signaling service
  kvm: Move irqchip event processing out of inner loop
  kvm: Unconditionally reenter kernel after IO exits
  kvm: Remove static return code of kvm_handle_io
  kvm: Leave kvm_cpu_exec directly after KVM_EXIT_SHUTDOWN
  kvm: Separate TCG from KVM cpu execution
  kvm: x86: Prepare VCPU loop for in-kernel irqchip
  kvm: Drop return values from kvm_arch_pre/post_run

 cpu-exec.c         |   19 +--
 cpus.c             |  521 +++++++++++++++++++++++++++++++---------------------
 kvm-all.c          |   76 +++++----
 kvm-stub.c         |    9 +-
 kvm.h              |   14 +-
 qemu-common.h      |    1 +
 target-i386/kvm.c  |   90 +++++-----
 target-ppc/kvm.c   |   16 ++-
 target-s390x/kvm.c |   16 ++-
 9 files changed, 444 insertions(+), 318 deletions(-)


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

* [Qemu-devel] [PATCH 00/18] [uq/master] MCE & IO exit fixes, prepare for VCPU loop reuse
@ 2011-01-10  8:31 ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:31 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti
  Cc: Gleb Natapov, Huang Ying, qemu-devel, kvm, Alexander Graf

This series has three major topics:
 - add required kernel reentry after IO exits
 - provide MCE forwarding under !CONFIG_IOTHREAD
 - prepare kvm_cpu_exec for qemu-kvm reuse

Along these lines, several cleanups and simplifcations are applied to
cpus.c and the KVM VCPU execution bits. The first patch should of course
be dropped if uq/master is applied without "kvm: Drop return value of
kvm_cpu_exec".

Note that I did not yet to test the MCE support. Is there an easy way to
trigger valid MCE events at kernel level? Or do I need to fake SIGBUS at
qemu level?

Jan Kiszka (18):
  Revert "kvm: Drop return value of kvm_cpu_exec"
  kvm: Drop redundant kvm_enabled from kvm_cpu_thread_fn
  kvm: Provide sigbus services arch-independently
  Refactor signal setup functions in cpus.c
  kvm: Set up signal mask also for !CONFIG_IOTHREAD
  kvm: Refactor qemu_kvm_eat_signals
  kvm: Add MCE signal support for !CONFIG_IOTHREAD
  kvm: Handle kvm_init_vcpu errors
  Refactor kvm&tcg function names in cpus.c
  Fix a few coding style violations in cpus.c
  Introduce VCPU self-signaling service
  kvm: Move irqchip event processing out of inner loop
  kvm: Unconditionally reenter kernel after IO exits
  kvm: Remove static return code of kvm_handle_io
  kvm: Leave kvm_cpu_exec directly after KVM_EXIT_SHUTDOWN
  kvm: Separate TCG from KVM cpu execution
  kvm: x86: Prepare VCPU loop for in-kernel irqchip
  kvm: Drop return values from kvm_arch_pre/post_run

 cpu-exec.c         |   19 +--
 cpus.c             |  521 +++++++++++++++++++++++++++++++---------------------
 kvm-all.c          |   76 +++++----
 kvm-stub.c         |    9 +-
 kvm.h              |   14 +-
 qemu-common.h      |    1 +
 target-i386/kvm.c  |   90 +++++-----
 target-ppc/kvm.c   |   16 ++-
 target-s390x/kvm.c |   16 ++-
 9 files changed, 444 insertions(+), 318 deletions(-)

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

* [PATCH 01/18] Revert "kvm: Drop return value of kvm_cpu_exec"
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:31   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:31 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

From: Jan Kiszka <jan.kiszka@siemens.com>

This reverts commit e60806755e2b74a34813c73ec61c33d38d102286.
---
 kvm-all.c  |    6 ++++--
 kvm-stub.c |    4 ++--
 kvm.h      |    2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index a5e9246..4ab5f5c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -817,7 +817,7 @@ void kvm_cpu_synchronize_post_init(CPUState *env)
     env->kvm_vcpu_dirty = 0;
 }
 
-void kvm_cpu_exec(CPUState *env)
+int kvm_cpu_exec(CPUState *env)
 {
     struct kvm_run *run = env->kvm_run;
     int ret;
@@ -906,7 +906,7 @@ void kvm_cpu_exec(CPUState *env)
 #ifdef KVM_CAP_SET_GUEST_DEBUG
             if (kvm_arch_debug(&run->debug.arch)) {
                 env->exception_index = EXCP_DEBUG;
-                return;
+                return 0;
             }
             /* re-enter, this exception was guest-internal */
             ret = 1;
@@ -928,6 +928,8 @@ void kvm_cpu_exec(CPUState *env)
         env->exit_request = 0;
         env->exception_index = EXCP_INTERRUPT;
     }
+
+    return ret;
 }
 
 int kvm_ioctl(int type, ...)
diff --git a/kvm-stub.c b/kvm-stub.c
index e00d7df..1fcfc1e 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -79,9 +79,9 @@ void kvm_cpu_synchronize_post_init(CPUState *env)
 {
 }
 
-void kvm_cpu_exec(CPUState *env)
+int kvm_cpu_exec(CPUState *env)
 {
-    abort();
+    abort ();
 }
 
 int kvm_has_sync_mmu(void)
diff --git a/kvm.h b/kvm.h
index 153d7b9..7bf9cc8 100644
--- a/kvm.h
+++ b/kvm.h
@@ -54,7 +54,7 @@ int kvm_has_xcrs(void);
 #ifdef NEED_CPU_H
 int kvm_init_vcpu(CPUState *env);
 
-void kvm_cpu_exec(CPUState *env);
+int kvm_cpu_exec(CPUState *env);
 
 #if !defined(CONFIG_USER_ONLY)
 int kvm_log_start(target_phys_addr_t phys_addr, ram_addr_t size);
-- 
1.7.1


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

* [Qemu-devel] [PATCH 01/18] Revert "kvm: Drop return value of kvm_cpu_exec"
@ 2011-01-10  8:31   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:31 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

This reverts commit e60806755e2b74a34813c73ec61c33d38d102286.
---
 kvm-all.c  |    6 ++++--
 kvm-stub.c |    4 ++--
 kvm.h      |    2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index a5e9246..4ab5f5c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -817,7 +817,7 @@ void kvm_cpu_synchronize_post_init(CPUState *env)
     env->kvm_vcpu_dirty = 0;
 }
 
-void kvm_cpu_exec(CPUState *env)
+int kvm_cpu_exec(CPUState *env)
 {
     struct kvm_run *run = env->kvm_run;
     int ret;
@@ -906,7 +906,7 @@ void kvm_cpu_exec(CPUState *env)
 #ifdef KVM_CAP_SET_GUEST_DEBUG
             if (kvm_arch_debug(&run->debug.arch)) {
                 env->exception_index = EXCP_DEBUG;
-                return;
+                return 0;
             }
             /* re-enter, this exception was guest-internal */
             ret = 1;
@@ -928,6 +928,8 @@ void kvm_cpu_exec(CPUState *env)
         env->exit_request = 0;
         env->exception_index = EXCP_INTERRUPT;
     }
+
+    return ret;
 }
 
 int kvm_ioctl(int type, ...)
diff --git a/kvm-stub.c b/kvm-stub.c
index e00d7df..1fcfc1e 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -79,9 +79,9 @@ void kvm_cpu_synchronize_post_init(CPUState *env)
 {
 }
 
-void kvm_cpu_exec(CPUState *env)
+int kvm_cpu_exec(CPUState *env)
 {
-    abort();
+    abort ();
 }
 
 int kvm_has_sync_mmu(void)
diff --git a/kvm.h b/kvm.h
index 153d7b9..7bf9cc8 100644
--- a/kvm.h
+++ b/kvm.h
@@ -54,7 +54,7 @@ int kvm_has_xcrs(void);
 #ifdef NEED_CPU_H
 int kvm_init_vcpu(CPUState *env);
 
-void kvm_cpu_exec(CPUState *env);
+int kvm_cpu_exec(CPUState *env);
 
 #if !defined(CONFIG_USER_ONLY)
 int kvm_log_start(target_phys_addr_t phys_addr, ram_addr_t size);
-- 
1.7.1

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

* [PATCH 02/18] kvm: Drop redundant kvm_enabled from kvm_cpu_thread_fn
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:31   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:31 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

From: Jan Kiszka <jan.kiszka@siemens.com>

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index 4c9928e..8af53a9 100644
--- a/cpus.c
+++ b/cpus.c
@@ -597,8 +597,8 @@ static void *kvm_cpu_thread_fn(void *arg)
 
     qemu_mutex_lock(&qemu_global_mutex);
     qemu_thread_self(env->thread);
-    if (kvm_enabled())
-        kvm_init_vcpu(env);
+
+    kvm_init_vcpu(env);
 
     kvm_init_ipi(env);
 
-- 
1.7.1


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

* [Qemu-devel] [PATCH 02/18] kvm: Drop redundant kvm_enabled from kvm_cpu_thread_fn
@ 2011-01-10  8:31   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:31 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index 4c9928e..8af53a9 100644
--- a/cpus.c
+++ b/cpus.c
@@ -597,8 +597,8 @@ static void *kvm_cpu_thread_fn(void *arg)
 
     qemu_mutex_lock(&qemu_global_mutex);
     qemu_thread_self(env->thread);
-    if (kvm_enabled())
-        kvm_init_vcpu(env);
+
+    kvm_init_vcpu(env);
 
     kvm_init_ipi(env);
 
-- 
1.7.1

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

* [PATCH 03/18] kvm: Provide sigbus services arch-independently
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:31   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:31 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel, Huang Ying, Alexander Graf

From: Jan Kiszka <jan.kiszka@siemens.com>

Provide arch-independent kvm_on_sigbus* stubs to remove the #ifdef'ery
from cpus.c. This patch also fixes --disable-kvm build by providing the
missing kvm_on_sigbus_vcpu kvm-stub.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
CC: Huang Ying <ying.huang@intel.com>
CC: Alexander Graf <agraf@suse.de>
---
 cpus.c             |   10 ++++------
 kvm-all.c          |   10 ++++++++++
 kvm-stub.c         |    5 +++++
 kvm.h              |    7 +++++--
 target-i386/kvm.c  |    4 ++--
 target-ppc/kvm.c   |   10 ++++++++++
 target-s390x/kvm.c |   10 ++++++++++
 7 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/cpus.c b/cpus.c
index 8af53a9..081ac30 100644
--- a/cpus.c
+++ b/cpus.c
@@ -527,10 +527,9 @@ static void sigbus_reraise(void)
 static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
                            void *ctx)
 {
-#if defined(TARGET_I386)
-    if (kvm_on_sigbus(siginfo->ssi_code, (void *)(intptr_t)siginfo->ssi_addr))
-#endif
+    if (kvm_on_sigbus(siginfo->ssi_code, (void *)(intptr_t)siginfo->ssi_addr)) {
         sigbus_reraise();
+    }
 }
 
 static void qemu_kvm_eat_signal(CPUState *env, int timeout)
@@ -563,10 +562,9 @@ static void qemu_kvm_eat_signal(CPUState *env, int timeout)
 
         switch (r) {
         case SIGBUS:
-#ifdef TARGET_I386
-            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr))
-#endif
+            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) {
                 sigbus_reraise();
+            }
             break;
         default:
             break;
diff --git a/kvm-all.c b/kvm-all.c
index 4ab5f5c..a8e9f2c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1284,3 +1284,13 @@ int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool assign)
     return -ENOSYS;
 #endif
 }
+
+int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+{
+    return kvm_on_sigbus_vcpu(env, code, addr);
+}
+
+int kvm_on_sigbus(int code, void *addr)
+{
+    return kvm_on_sigbus(code, addr);
+}
diff --git a/kvm-stub.c b/kvm-stub.c
index 1fcfc1e..572380c 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -142,6 +142,11 @@ int kvm_set_ioeventfd_mmio_long(int fd, uint32_t adr, uint32_t val, bool assign)
     return -ENOSYS;
 }
 
+int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+{
+    return 1;
+}
+
 int kvm_on_sigbus(int code, void *addr)
 {
     return 1;
diff --git a/kvm.h b/kvm.h
index 7bf9cc8..0a4db28 100644
--- a/kvm.h
+++ b/kvm.h
@@ -80,6 +80,9 @@ int kvm_set_signal_mask(CPUState *env, const sigset_t *sigset);
 int kvm_pit_in_kernel(void);
 int kvm_irqchip_in_kernel(void);
 
+int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr);
+int kvm_on_sigbus(int code, void *addr);
+
 /* internal API */
 
 int kvm_ioctl(int type, ...);
@@ -117,8 +120,8 @@ 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);
+int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr);
+int kvm_arch_on_sigbus(int code, void *addr);
 
 struct kvm_guest_debug;
 struct kvm_debug_exit_arch;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index af79526..a7acd53 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1918,7 +1918,7 @@ static void kvm_mce_inj_srao_memscrub2(CPUState *env, target_phys_addr_t paddr)
 
 #endif
 
-int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr)
 {
 #if defined(KVM_CAP_MCE)
     void *vaddr;
@@ -1968,7 +1968,7 @@ int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
     return 0;
 }
 
-int kvm_on_sigbus(int code, void *addr)
+int kvm_arch_on_sigbus(int code, void *addr)
 {
 #if defined(KVM_CAP_MCE)
     if ((first_cpu->mcg_cap & MCG_SER_P) && addr && code == BUS_MCEERR_AO) {
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 7918426..8349045 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -404,3 +404,13 @@ bool kvm_arch_stop_on_emulation_error(CPUState *env)
 {
     return true;
 }
+
+int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+{
+    return 1;
+}
+
+int kvm_arch_on_sigbus(int code, void *addr)
+{
+    return 1;
+}
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 29fcd46..b7f644b 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -501,3 +501,13 @@ bool kvm_arch_stop_on_emulation_error(CPUState *env)
 {
     return true;
 }
+
+int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+{
+    return 1;
+}
+
+int kvm_arch_on_sigbus(int code, void *addr)
+{
+    return 1;
+}
-- 
1.7.1


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

* [Qemu-devel] [PATCH 03/18] kvm: Provide sigbus services arch-independently
@ 2011-01-10  8:31   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:31 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: Alexander Graf, qemu-devel, kvm, Huang Ying

From: Jan Kiszka <jan.kiszka@siemens.com>

Provide arch-independent kvm_on_sigbus* stubs to remove the #ifdef'ery
from cpus.c. This patch also fixes --disable-kvm build by providing the
missing kvm_on_sigbus_vcpu kvm-stub.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
CC: Huang Ying <ying.huang@intel.com>
CC: Alexander Graf <agraf@suse.de>
---
 cpus.c             |   10 ++++------
 kvm-all.c          |   10 ++++++++++
 kvm-stub.c         |    5 +++++
 kvm.h              |    7 +++++--
 target-i386/kvm.c  |    4 ++--
 target-ppc/kvm.c   |   10 ++++++++++
 target-s390x/kvm.c |   10 ++++++++++
 7 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/cpus.c b/cpus.c
index 8af53a9..081ac30 100644
--- a/cpus.c
+++ b/cpus.c
@@ -527,10 +527,9 @@ static void sigbus_reraise(void)
 static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
                            void *ctx)
 {
-#if defined(TARGET_I386)
-    if (kvm_on_sigbus(siginfo->ssi_code, (void *)(intptr_t)siginfo->ssi_addr))
-#endif
+    if (kvm_on_sigbus(siginfo->ssi_code, (void *)(intptr_t)siginfo->ssi_addr)) {
         sigbus_reraise();
+    }
 }
 
 static void qemu_kvm_eat_signal(CPUState *env, int timeout)
@@ -563,10 +562,9 @@ static void qemu_kvm_eat_signal(CPUState *env, int timeout)
 
         switch (r) {
         case SIGBUS:
-#ifdef TARGET_I386
-            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr))
-#endif
+            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) {
                 sigbus_reraise();
+            }
             break;
         default:
             break;
diff --git a/kvm-all.c b/kvm-all.c
index 4ab5f5c..a8e9f2c 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1284,3 +1284,13 @@ int kvm_set_ioeventfd_pio_word(int fd, uint16_t addr, uint16_t val, bool assign)
     return -ENOSYS;
 #endif
 }
+
+int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+{
+    return kvm_on_sigbus_vcpu(env, code, addr);
+}
+
+int kvm_on_sigbus(int code, void *addr)
+{
+    return kvm_on_sigbus(code, addr);
+}
diff --git a/kvm-stub.c b/kvm-stub.c
index 1fcfc1e..572380c 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -142,6 +142,11 @@ int kvm_set_ioeventfd_mmio_long(int fd, uint32_t adr, uint32_t val, bool assign)
     return -ENOSYS;
 }
 
+int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+{
+    return 1;
+}
+
 int kvm_on_sigbus(int code, void *addr)
 {
     return 1;
diff --git a/kvm.h b/kvm.h
index 7bf9cc8..0a4db28 100644
--- a/kvm.h
+++ b/kvm.h
@@ -80,6 +80,9 @@ int kvm_set_signal_mask(CPUState *env, const sigset_t *sigset);
 int kvm_pit_in_kernel(void);
 int kvm_irqchip_in_kernel(void);
 
+int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr);
+int kvm_on_sigbus(int code, void *addr);
+
 /* internal API */
 
 int kvm_ioctl(int type, ...);
@@ -117,8 +120,8 @@ 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);
+int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr);
+int kvm_arch_on_sigbus(int code, void *addr);
 
 struct kvm_guest_debug;
 struct kvm_debug_exit_arch;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index af79526..a7acd53 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1918,7 +1918,7 @@ static void kvm_mce_inj_srao_memscrub2(CPUState *env, target_phys_addr_t paddr)
 
 #endif
 
-int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr)
 {
 #if defined(KVM_CAP_MCE)
     void *vaddr;
@@ -1968,7 +1968,7 @@ int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
     return 0;
 }
 
-int kvm_on_sigbus(int code, void *addr)
+int kvm_arch_on_sigbus(int code, void *addr)
 {
 #if defined(KVM_CAP_MCE)
     if ((first_cpu->mcg_cap & MCG_SER_P) && addr && code == BUS_MCEERR_AO) {
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 7918426..8349045 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -404,3 +404,13 @@ bool kvm_arch_stop_on_emulation_error(CPUState *env)
 {
     return true;
 }
+
+int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+{
+    return 1;
+}
+
+int kvm_arch_on_sigbus(int code, void *addr)
+{
+    return 1;
+}
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 29fcd46..b7f644b 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -501,3 +501,13 @@ bool kvm_arch_stop_on_emulation_error(CPUState *env)
 {
     return true;
 }
+
+int kvm_arch_on_sigbus_vcpu(CPUState *env, int code, void *addr)
+{
+    return 1;
+}
+
+int kvm_arch_on_sigbus(int code, void *addr)
+{
+    return 1;
+}
-- 
1.7.1

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

* [PATCH 04/18] Refactor signal setup functions in cpus.c
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:31   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:31 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

From: Jan Kiszka <jan.kiszka@siemens.com>

Move {tcg,kvm}_init_ipi and block_io_signals to avoid prototypes, rename
the former two to clarify that they deal with more than SIG_IPI. No
functional changes.

The forward declaration of sigbus_handler is just temporarily, it will
be moved in a succeeding patch. qemu_kvm_init_cpu_signals is moved into
the !_WIN32 block as we will soon need it also for !CONFIG_IOTHREAD.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |  162 ++++++++++++++++++++++++++++++++-------------------------------
 1 files changed, 82 insertions(+), 80 deletions(-)

diff --git a/cpus.c b/cpus.c
index 081ac30..a21e2d6 100644
--- a/cpus.c
+++ b/cpus.c
@@ -222,7 +222,35 @@ fail:
     close(fds[1]);
     return err;
 }
-#else
+
+#ifdef CONFIG_IOTHREAD
+static void dummy_signal(int sig)
+{
+}
+
+static void qemu_kvm_init_cpu_signals(CPUState *env)
+{
+    int r;
+    sigset_t set;
+    struct sigaction sigact;
+
+    memset(&sigact, 0, sizeof(sigact));
+    sigact.sa_handler = dummy_signal;
+    sigaction(SIG_IPI, &sigact, NULL);
+
+    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));
+        exit(1);
+    }
+}
+#endif
+
+#else /* _WIN32 */
+
 HANDLE qemu_event_handle;
 
 static void dummy_event_handler(void *opaque)
@@ -248,7 +276,7 @@ static void qemu_event_increment(void)
         exit (1);
     }
 }
-#endif
+#endif /* _WIN32 */
 
 #ifndef CONFIG_IOTHREAD
 int qemu_init_main_loop(void)
@@ -337,10 +365,6 @@ static QemuCond qemu_system_cond;
 static QemuCond qemu_pause_cond;
 static QemuCond qemu_work_cond;
 
-static void tcg_init_ipi(void);
-static void kvm_init_ipi(CPUState *env);
-static sigset_t block_io_signals(void);
-
 /* If we have signalfd, we mask out the signals we want to handle and then
  * use signalfd to listen for them.  We rely on whatever the current signal
  * handler is to dispatch the signals when we receive them.
@@ -376,6 +400,56 @@ static void sigfd_handler(void *opaque)
     }
 }
 
+static void cpu_signal(int sig)
+{
+    if (cpu_single_env)
+        cpu_exit(cpu_single_env);
+    exit_request = 1;
+}
+
+static void qemu_tcg_init_cpu_signals(void)
+{
+    sigset_t set;
+    struct sigaction sigact;
+
+    memset(&sigact, 0, sizeof(sigact));
+    sigact.sa_handler = cpu_signal;
+    sigaction(SIG_IPI, &sigact, NULL);
+
+    sigemptyset(&set);
+    sigaddset(&set, SIG_IPI);
+    pthread_sigmask(SIG_UNBLOCK, &set, NULL);
+}
+
+static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
+                           void *ctx);
+
+static sigset_t block_io_signals(void)
+{
+    sigset_t set;
+    struct sigaction action;
+
+    /* SIGUSR2 used by posix-aio-compat.c */
+    sigemptyset(&set);
+    sigaddset(&set, SIGUSR2);
+    pthread_sigmask(SIG_UNBLOCK, &set, NULL);
+
+    sigemptyset(&set);
+    sigaddset(&set, SIGIO);
+    sigaddset(&set, SIGALRM);
+    sigaddset(&set, SIG_IPI);
+    sigaddset(&set, SIGBUS);
+    pthread_sigmask(SIG_BLOCK, &set, NULL);
+
+    memset(&action, 0, sizeof(action));
+    action.sa_flags = SA_SIGINFO;
+    action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler;
+    sigaction(SIGBUS, &action, NULL);
+    prctl(PR_MCE_KILL, 1, 1, 0, 0);
+
+    return set;
+}
+
 static int qemu_signalfd_init(sigset_t mask)
 {
     int sigfd;
@@ -597,8 +671,7 @@ static void *kvm_cpu_thread_fn(void *arg)
     qemu_thread_self(env->thread);
 
     kvm_init_vcpu(env);
-
-    kvm_init_ipi(env);
+    qemu_kvm_init_cpu_signals(env);
 
     /* signal CPU creation */
     env->created = 1;
@@ -621,7 +694,7 @@ static void *tcg_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
 
-    tcg_init_ipi();
+    qemu_tcg_init_cpu_signals();
     qemu_thread_self(env->thread);
 
     /* signal CPU creation */
@@ -659,77 +732,6 @@ int qemu_cpu_self(void *_env)
     return qemu_thread_equal(&this, env->thread);
 }
 
-static void cpu_signal(int sig)
-{
-    if (cpu_single_env)
-        cpu_exit(cpu_single_env);
-    exit_request = 1;
-}
-
-static void tcg_init_ipi(void)
-{
-    sigset_t set;
-    struct sigaction sigact;
-
-    memset(&sigact, 0, sizeof(sigact));
-    sigact.sa_handler = cpu_signal;
-    sigaction(SIG_IPI, &sigact, NULL);
-
-    sigemptyset(&set);
-    sigaddset(&set, SIG_IPI);
-    pthread_sigmask(SIG_UNBLOCK, &set, NULL);
-}
-
-static void dummy_signal(int sig)
-{
-}
-
-static void kvm_init_ipi(CPUState *env)
-{
-    int r;
-    sigset_t set;
-    struct sigaction sigact;
-
-    memset(&sigact, 0, sizeof(sigact));
-    sigact.sa_handler = dummy_signal;
-    sigaction(SIG_IPI, &sigact, NULL);
-
-    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));
-        exit(1);
-    }
-}
-
-static sigset_t block_io_signals(void)
-{
-    sigset_t set;
-    struct sigaction action;
-
-    /* SIGUSR2 used by posix-aio-compat.c */
-    sigemptyset(&set);
-    sigaddset(&set, SIGUSR2);
-    pthread_sigmask(SIG_UNBLOCK, &set, NULL);
-
-    sigemptyset(&set);
-    sigaddset(&set, SIGIO);
-    sigaddset(&set, SIGALRM);
-    sigaddset(&set, SIG_IPI);
-    sigaddset(&set, SIGBUS);
-    pthread_sigmask(SIG_BLOCK, &set, NULL);
-
-    memset(&action, 0, sizeof(action));
-    action.sa_flags = SA_SIGINFO;
-    action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler;
-    sigaction(SIGBUS, &action, NULL);
-    prctl(PR_MCE_KILL, 1, 1, 0, 0);
-
-    return set;
-}
-
 void qemu_mutex_lock_iothread(void)
 {
     if (kvm_enabled()) {
-- 
1.7.1


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

* [Qemu-devel] [PATCH 04/18] Refactor signal setup functions in cpus.c
@ 2011-01-10  8:31   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:31 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

Move {tcg,kvm}_init_ipi and block_io_signals to avoid prototypes, rename
the former two to clarify that they deal with more than SIG_IPI. No
functional changes.

The forward declaration of sigbus_handler is just temporarily, it will
be moved in a succeeding patch. qemu_kvm_init_cpu_signals is moved into
the !_WIN32 block as we will soon need it also for !CONFIG_IOTHREAD.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |  162 ++++++++++++++++++++++++++++++++-------------------------------
 1 files changed, 82 insertions(+), 80 deletions(-)

diff --git a/cpus.c b/cpus.c
index 081ac30..a21e2d6 100644
--- a/cpus.c
+++ b/cpus.c
@@ -222,7 +222,35 @@ fail:
     close(fds[1]);
     return err;
 }
-#else
+
+#ifdef CONFIG_IOTHREAD
+static void dummy_signal(int sig)
+{
+}
+
+static void qemu_kvm_init_cpu_signals(CPUState *env)
+{
+    int r;
+    sigset_t set;
+    struct sigaction sigact;
+
+    memset(&sigact, 0, sizeof(sigact));
+    sigact.sa_handler = dummy_signal;
+    sigaction(SIG_IPI, &sigact, NULL);
+
+    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));
+        exit(1);
+    }
+}
+#endif
+
+#else /* _WIN32 */
+
 HANDLE qemu_event_handle;
 
 static void dummy_event_handler(void *opaque)
@@ -248,7 +276,7 @@ static void qemu_event_increment(void)
         exit (1);
     }
 }
-#endif
+#endif /* _WIN32 */
 
 #ifndef CONFIG_IOTHREAD
 int qemu_init_main_loop(void)
@@ -337,10 +365,6 @@ static QemuCond qemu_system_cond;
 static QemuCond qemu_pause_cond;
 static QemuCond qemu_work_cond;
 
-static void tcg_init_ipi(void);
-static void kvm_init_ipi(CPUState *env);
-static sigset_t block_io_signals(void);
-
 /* If we have signalfd, we mask out the signals we want to handle and then
  * use signalfd to listen for them.  We rely on whatever the current signal
  * handler is to dispatch the signals when we receive them.
@@ -376,6 +400,56 @@ static void sigfd_handler(void *opaque)
     }
 }
 
+static void cpu_signal(int sig)
+{
+    if (cpu_single_env)
+        cpu_exit(cpu_single_env);
+    exit_request = 1;
+}
+
+static void qemu_tcg_init_cpu_signals(void)
+{
+    sigset_t set;
+    struct sigaction sigact;
+
+    memset(&sigact, 0, sizeof(sigact));
+    sigact.sa_handler = cpu_signal;
+    sigaction(SIG_IPI, &sigact, NULL);
+
+    sigemptyset(&set);
+    sigaddset(&set, SIG_IPI);
+    pthread_sigmask(SIG_UNBLOCK, &set, NULL);
+}
+
+static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
+                           void *ctx);
+
+static sigset_t block_io_signals(void)
+{
+    sigset_t set;
+    struct sigaction action;
+
+    /* SIGUSR2 used by posix-aio-compat.c */
+    sigemptyset(&set);
+    sigaddset(&set, SIGUSR2);
+    pthread_sigmask(SIG_UNBLOCK, &set, NULL);
+
+    sigemptyset(&set);
+    sigaddset(&set, SIGIO);
+    sigaddset(&set, SIGALRM);
+    sigaddset(&set, SIG_IPI);
+    sigaddset(&set, SIGBUS);
+    pthread_sigmask(SIG_BLOCK, &set, NULL);
+
+    memset(&action, 0, sizeof(action));
+    action.sa_flags = SA_SIGINFO;
+    action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler;
+    sigaction(SIGBUS, &action, NULL);
+    prctl(PR_MCE_KILL, 1, 1, 0, 0);
+
+    return set;
+}
+
 static int qemu_signalfd_init(sigset_t mask)
 {
     int sigfd;
@@ -597,8 +671,7 @@ static void *kvm_cpu_thread_fn(void *arg)
     qemu_thread_self(env->thread);
 
     kvm_init_vcpu(env);
-
-    kvm_init_ipi(env);
+    qemu_kvm_init_cpu_signals(env);
 
     /* signal CPU creation */
     env->created = 1;
@@ -621,7 +694,7 @@ static void *tcg_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
 
-    tcg_init_ipi();
+    qemu_tcg_init_cpu_signals();
     qemu_thread_self(env->thread);
 
     /* signal CPU creation */
@@ -659,77 +732,6 @@ int qemu_cpu_self(void *_env)
     return qemu_thread_equal(&this, env->thread);
 }
 
-static void cpu_signal(int sig)
-{
-    if (cpu_single_env)
-        cpu_exit(cpu_single_env);
-    exit_request = 1;
-}
-
-static void tcg_init_ipi(void)
-{
-    sigset_t set;
-    struct sigaction sigact;
-
-    memset(&sigact, 0, sizeof(sigact));
-    sigact.sa_handler = cpu_signal;
-    sigaction(SIG_IPI, &sigact, NULL);
-
-    sigemptyset(&set);
-    sigaddset(&set, SIG_IPI);
-    pthread_sigmask(SIG_UNBLOCK, &set, NULL);
-}
-
-static void dummy_signal(int sig)
-{
-}
-
-static void kvm_init_ipi(CPUState *env)
-{
-    int r;
-    sigset_t set;
-    struct sigaction sigact;
-
-    memset(&sigact, 0, sizeof(sigact));
-    sigact.sa_handler = dummy_signal;
-    sigaction(SIG_IPI, &sigact, NULL);
-
-    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));
-        exit(1);
-    }
-}
-
-static sigset_t block_io_signals(void)
-{
-    sigset_t set;
-    struct sigaction action;
-
-    /* SIGUSR2 used by posix-aio-compat.c */
-    sigemptyset(&set);
-    sigaddset(&set, SIGUSR2);
-    pthread_sigmask(SIG_UNBLOCK, &set, NULL);
-
-    sigemptyset(&set);
-    sigaddset(&set, SIGIO);
-    sigaddset(&set, SIGALRM);
-    sigaddset(&set, SIG_IPI);
-    sigaddset(&set, SIGBUS);
-    pthread_sigmask(SIG_BLOCK, &set, NULL);
-
-    memset(&action, 0, sizeof(action));
-    action.sa_flags = SA_SIGINFO;
-    action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler;
-    sigaction(SIGBUS, &action, NULL);
-    prctl(PR_MCE_KILL, 1, 1, 0, 0);
-
-    return set;
-}
-
 void qemu_mutex_lock_iothread(void)
 {
     if (kvm_enabled()) {
-- 
1.7.1

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

* [PATCH 05/18] kvm: Set up signal mask also for !CONFIG_IOTHREAD
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:31   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:31 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

From: Jan Kiszka <jan.kiszka@siemens.com>

Block SIG_IPI and SIGBUS, unblock them during KVM_RUN, just like in
io-thread mode. This will be required to process SIGBUS and for
self-IPIs. As Windows doesn't support signal services, we need to
provide a stub for the init function.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/cpus.c b/cpus.c
index a21e2d6..bf0fb85 100644
--- a/cpus.c
+++ b/cpus.c
@@ -223,7 +223,6 @@ fail:
     return err;
 }
 
-#ifdef CONFIG_IOTHREAD
 static void dummy_signal(int sig)
 {
 }
@@ -238,6 +237,13 @@ static void qemu_kvm_init_cpu_signals(CPUState *env)
     sigact.sa_handler = dummy_signal;
     sigaction(SIG_IPI, &sigact, NULL);
 
+#ifndef CONFIG_IOTHREAD
+    sigemptyset(&set);
+    sigaddset(&set, SIG_IPI);
+    sigaddset(&set, SIGBUS);
+    pthread_sigmask(SIG_BLOCK, &set, NULL);
+#endif
+
     pthread_sigmask(SIG_BLOCK, NULL, &set);
     sigdelset(&set, SIG_IPI);
     sigdelset(&set, SIGBUS);
@@ -247,7 +253,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *env)
         exit(1);
     }
 }
-#endif
 
 #else /* _WIN32 */
 
@@ -276,6 +281,10 @@ static void qemu_event_increment(void)
         exit (1);
     }
 }
+
+static void qemu_kvm_init_cpu_signals(CPUState *env)
+{
+}
 #endif /* _WIN32 */
 
 #ifndef CONFIG_IOTHREAD
@@ -296,8 +305,10 @@ void qemu_init_vcpu(void *_env)
 
     env->nr_cores = smp_cores;
     env->nr_threads = smp_threads;
-    if (kvm_enabled())
+    if (kvm_enabled()) {
         kvm_init_vcpu(env);
+        qemu_kvm_init_cpu_signals(env);
+    }
     return;
 }
 
-- 
1.7.1


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

* [Qemu-devel] [PATCH 05/18] kvm: Set up signal mask also for !CONFIG_IOTHREAD
@ 2011-01-10  8:31   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:31 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

Block SIG_IPI and SIGBUS, unblock them during KVM_RUN, just like in
io-thread mode. This will be required to process SIGBUS and for
self-IPIs. As Windows doesn't support signal services, we need to
provide a stub for the init function.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |   17 ++++++++++++++---
 1 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/cpus.c b/cpus.c
index a21e2d6..bf0fb85 100644
--- a/cpus.c
+++ b/cpus.c
@@ -223,7 +223,6 @@ fail:
     return err;
 }
 
-#ifdef CONFIG_IOTHREAD
 static void dummy_signal(int sig)
 {
 }
@@ -238,6 +237,13 @@ static void qemu_kvm_init_cpu_signals(CPUState *env)
     sigact.sa_handler = dummy_signal;
     sigaction(SIG_IPI, &sigact, NULL);
 
+#ifndef CONFIG_IOTHREAD
+    sigemptyset(&set);
+    sigaddset(&set, SIG_IPI);
+    sigaddset(&set, SIGBUS);
+    pthread_sigmask(SIG_BLOCK, &set, NULL);
+#endif
+
     pthread_sigmask(SIG_BLOCK, NULL, &set);
     sigdelset(&set, SIG_IPI);
     sigdelset(&set, SIGBUS);
@@ -247,7 +253,6 @@ static void qemu_kvm_init_cpu_signals(CPUState *env)
         exit(1);
     }
 }
-#endif
 
 #else /* _WIN32 */
 
@@ -276,6 +281,10 @@ static void qemu_event_increment(void)
         exit (1);
     }
 }
+
+static void qemu_kvm_init_cpu_signals(CPUState *env)
+{
+}
 #endif /* _WIN32 */
 
 #ifndef CONFIG_IOTHREAD
@@ -296,8 +305,10 @@ void qemu_init_vcpu(void *_env)
 
     env->nr_cores = smp_cores;
     env->nr_threads = smp_threads;
-    if (kvm_enabled())
+    if (kvm_enabled()) {
         kvm_init_vcpu(env);
+        qemu_kvm_init_cpu_signals(env);
+    }
     return;
 }
 
-- 
1.7.1

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

* [PATCH 06/18] kvm: Refactor qemu_kvm_eat_signals
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:31   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:31 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

From: Jan Kiszka <jan.kiszka@siemens.com>

We do not use the timeout, so drop its logic. As we always poll our
signals, we do not need to drop the global lock. Removing those calls
allows some further simplifications. Also fix the error processing of
sigpending at this chance.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |   23 +++++++----------------
 1 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/cpus.c b/cpus.c
index bf0fb85..6da0f8f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -617,31 +617,22 @@ static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
     }
 }
 
-static void qemu_kvm_eat_signal(CPUState *env, int timeout)
+static void qemu_kvm_eat_signals(CPUState *env)
 {
-    struct timespec ts;
-    int r, e;
+    struct timespec ts = { 0, 0 };
     siginfo_t siginfo;
     sigset_t waitset;
     sigset_t chkset;
-
-    ts.tv_sec = timeout / 1000;
-    ts.tv_nsec = (timeout % 1000) * 1000000;
+    int r;
 
     sigemptyset(&waitset);
     sigaddset(&waitset, SIG_IPI);
     sigaddset(&waitset, SIGBUS);
 
     do {
-        qemu_mutex_unlock(&qemu_global_mutex);
-
         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));
+        if (r == -1 && !(errno == EAGAIN || errno == EINTR)) {
+            perror("sigtimedwait");
             exit(1);
         }
 
@@ -657,7 +648,7 @@ static void qemu_kvm_eat_signal(CPUState *env, int timeout)
 
         r = sigpending(&chkset);
         if (r == -1) {
-            fprintf(stderr, "sigpending: %s\n", strerror(e));
+            perror("sigpending");
             exit(1);
         }
     } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS));
@@ -668,7 +659,7 @@ static void qemu_kvm_wait_io_event(CPUState *env)
     while (!cpu_has_work(env))
         qemu_cond_timedwait(env->halt_cond, &qemu_global_mutex, 1000);
 
-    qemu_kvm_eat_signal(env, 0);
+    qemu_kvm_eat_signals(env);
     qemu_wait_io_event_common(env);
 }
 
-- 
1.7.1


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

* [Qemu-devel] [PATCH 06/18] kvm: Refactor qemu_kvm_eat_signals
@ 2011-01-10  8:31   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:31 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

We do not use the timeout, so drop its logic. As we always poll our
signals, we do not need to drop the global lock. Removing those calls
allows some further simplifications. Also fix the error processing of
sigpending at this chance.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |   23 +++++++----------------
 1 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/cpus.c b/cpus.c
index bf0fb85..6da0f8f 100644
--- a/cpus.c
+++ b/cpus.c
@@ -617,31 +617,22 @@ static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
     }
 }
 
-static void qemu_kvm_eat_signal(CPUState *env, int timeout)
+static void qemu_kvm_eat_signals(CPUState *env)
 {
-    struct timespec ts;
-    int r, e;
+    struct timespec ts = { 0, 0 };
     siginfo_t siginfo;
     sigset_t waitset;
     sigset_t chkset;
-
-    ts.tv_sec = timeout / 1000;
-    ts.tv_nsec = (timeout % 1000) * 1000000;
+    int r;
 
     sigemptyset(&waitset);
     sigaddset(&waitset, SIG_IPI);
     sigaddset(&waitset, SIGBUS);
 
     do {
-        qemu_mutex_unlock(&qemu_global_mutex);
-
         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));
+        if (r == -1 && !(errno == EAGAIN || errno == EINTR)) {
+            perror("sigtimedwait");
             exit(1);
         }
 
@@ -657,7 +648,7 @@ static void qemu_kvm_eat_signal(CPUState *env, int timeout)
 
         r = sigpending(&chkset);
         if (r == -1) {
-            fprintf(stderr, "sigpending: %s\n", strerror(e));
+            perror("sigpending");
             exit(1);
         }
     } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS));
@@ -668,7 +659,7 @@ static void qemu_kvm_wait_io_event(CPUState *env)
     while (!cpu_has_work(env))
         qemu_cond_timedwait(env->halt_cond, &qemu_global_mutex, 1000);
 
-    qemu_kvm_eat_signal(env, 0);
+    qemu_kvm_eat_signals(env);
     qemu_wait_io_event_common(env);
 }
 
-- 
1.7.1

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

* [PATCH 07/18] kvm: Add MCE signal support for !CONFIG_IOTHREAD
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:32   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel, Huang Ying

From: Jan Kiszka <jan.kiszka@siemens.com>

Currently, we only configure and process MCE-related SIGBUS events if
CONFIG_IOTHREAD is enabled. Fix this by factoring out the required
handler registration and system configuration. Make sure that events
happening over a VCPU context in non-threaded mode get dispatched as
VCPU MCEs.

We also need to call qemu_kvm_eat_signals in non-threaded mode now, so
move it (unmodified) and add the required Windows stub.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
CC: Huang Ying <ying.huang@intel.com>
---
 cpus.c |  200 +++++++++++++++++++++++++++++++++++++++------------------------
 1 files changed, 124 insertions(+), 76 deletions(-)

diff --git a/cpus.c b/cpus.c
index 6da0f8f..b6f1cfb 100644
--- a/cpus.c
+++ b/cpus.c
@@ -34,9 +34,6 @@
 
 #include "cpus.h"
 #include "compatfd.h"
-#ifdef CONFIG_LINUX
-#include <sys/prctl.h>
-#endif
 
 #ifdef SIGRTMIN
 #define SIG_IPI (SIGRTMIN+4)
@@ -44,10 +41,24 @@
 #define SIG_IPI SIGUSR1
 #endif
 
+#ifdef CONFIG_LINUX
+
+#include <sys/prctl.h>
+
 #ifndef PR_MCE_KILL
 #define PR_MCE_KILL 33
 #endif
 
+#ifndef PR_MCE_KILL_SET
+#define PR_MCE_KILL_SET 1
+#endif
+
+#ifndef PR_MCE_KILL_EARLY
+#define PR_MCE_KILL_EARLY 1
+#endif
+
+#endif /* CONFIG_LINUX */
+
 static CPUState *next_cpu;
 
 /***********************************************************/
@@ -158,6 +169,62 @@ static void cpu_debug_handler(CPUState *env)
     vm_stop(EXCP_DEBUG);
 }
 
+#ifdef CONFIG_LINUX
+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();
+}
+
+static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
+                           void *ctx)
+{
+#ifndef CONFIG_IOTHREAD
+    if (cpu_single_env) {
+        if (kvm_on_sigbus_vcpu(cpu_single_env, siginfo->ssi_code,
+                               (void *)(intptr_t)siginfo->ssi_addr)) {
+            sigbus_reraise();
+        }
+        return;
+    }
+#endif
+
+    if (kvm_on_sigbus(siginfo->ssi_code,
+                      (void *)(intptr_t)siginfo->ssi_addr)) {
+        sigbus_reraise();
+    }
+}
+
+static void qemu_init_sigbus(void)
+{
+    struct sigaction action;
+
+    memset(&action, 0, sizeof(action));
+    action.sa_flags = SA_SIGINFO;
+    action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler;
+    sigaction(SIGBUS, &action, NULL);
+
+    prctl(PR_MCE_KILL, PR_MCE_KILL_SET, PR_MCE_KILL_EARLY, 0, 0);
+}
+
+#else /* !CONFIG_LINUX */
+
+static void qemu_init_sigbus(void)
+{
+}
+#endif /* !CONFIG_LINUX */
+
 #ifndef _WIN32
 static int io_thread_fd = -1;
 
@@ -254,6 +321,43 @@ static void qemu_kvm_init_cpu_signals(CPUState *env)
     }
 }
 
+static void qemu_kvm_eat_signals(CPUState *env)
+{
+    struct timespec ts = { 0, 0 };
+    siginfo_t siginfo;
+    sigset_t waitset;
+    sigset_t chkset;
+    int r;
+
+    sigemptyset(&waitset);
+    sigaddset(&waitset, SIG_IPI);
+    sigaddset(&waitset, SIGBUS);
+
+    do {
+        r = sigtimedwait(&waitset, &siginfo, &ts);
+        if (r == -1 && !(errno == EAGAIN || errno == EINTR)) {
+            perror("sigtimedwait");
+            exit(1);
+        }
+
+        switch (r) {
+        case SIGBUS:
+            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) {
+                sigbus_reraise();
+            }
+            break;
+        default:
+            break;
+        }
+
+        r = sigpending(&chkset);
+        if (r == -1) {
+            perror("sigpending");
+            exit(1);
+        }
+    } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS));
+}
+
 #else /* _WIN32 */
 
 HANDLE qemu_event_handle;
@@ -285,6 +389,10 @@ static void qemu_event_increment(void)
 static void qemu_kvm_init_cpu_signals(CPUState *env)
 {
 }
+
+static void qemu_kvm_eat_signals(CPUState *env)
+{
+}
 #endif /* _WIN32 */
 
 #ifndef CONFIG_IOTHREAD
@@ -292,6 +400,8 @@ int qemu_init_main_loop(void)
 {
     cpu_set_debug_excp_handler(cpu_debug_handler);
 
+    qemu_init_sigbus();
+
     return qemu_event_init();
 }
 
@@ -432,13 +542,9 @@ static void qemu_tcg_init_cpu_signals(void)
     pthread_sigmask(SIG_UNBLOCK, &set, NULL);
 }
 
-static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
-                           void *ctx);
-
 static sigset_t block_io_signals(void)
 {
     sigset_t set;
-    struct sigaction action;
 
     /* SIGUSR2 used by posix-aio-compat.c */
     sigemptyset(&set);
@@ -449,15 +555,11 @@ static sigset_t block_io_signals(void)
     sigaddset(&set, SIGIO);
     sigaddset(&set, SIGALRM);
     sigaddset(&set, SIG_IPI);
+#ifdef CONFIG_LINUX
     sigaddset(&set, SIGBUS);
+#endif
     pthread_sigmask(SIG_BLOCK, &set, NULL);
 
-    memset(&action, 0, sizeof(action));
-    action.sa_flags = SA_SIGINFO;
-    action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler;
-    sigaction(SIGBUS, &action, NULL);
-    prctl(PR_MCE_KILL, 1, 1, 0, 0);
-
     return set;
 }
 
@@ -486,6 +588,8 @@ int qemu_init_main_loop(void)
 
     cpu_set_debug_excp_handler(cpu_debug_handler);
 
+    qemu_init_sigbus();
+
     blocked_signals = block_io_signals();
 
     ret = qemu_signalfd_init(blocked_signals);
@@ -592,68 +696,6 @@ 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();
-}
-
-static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
-                           void *ctx)
-{
-    if (kvm_on_sigbus(siginfo->ssi_code, (void *)(intptr_t)siginfo->ssi_addr)) {
-        sigbus_reraise();
-    }
-}
-
-static void qemu_kvm_eat_signals(CPUState *env)
-{
-    struct timespec ts = { 0, 0 };
-    siginfo_t siginfo;
-    sigset_t waitset;
-    sigset_t chkset;
-    int r;
-
-    sigemptyset(&waitset);
-    sigaddset(&waitset, SIG_IPI);
-    sigaddset(&waitset, SIGBUS);
-
-    do {
-        r = sigtimedwait(&waitset, &siginfo, &ts);
-        if (r == -1 && !(errno == EAGAIN || errno == EINTR)) {
-            perror("sigtimedwait");
-            exit(1);
-        }
-
-        switch (r) {
-        case SIGBUS:
-            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) {
-                sigbus_reraise();
-            }
-            break;
-        default:
-            break;
-        }
-
-        r = sigpending(&chkset);
-        if (r == -1) {
-            perror("sigpending");
-            exit(1);
-        }
-    } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS));
-}
-
 static void qemu_kvm_wait_io_event(CPUState *env)
 {
     while (!cpu_has_work(env))
@@ -912,6 +954,8 @@ static int qemu_cpu_exec(CPUState *env)
 
 bool cpu_exec_all(void)
 {
+    int r;
+
     if (next_cpu == NULL)
         next_cpu = first_cpu;
     for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
@@ -923,7 +967,11 @@ bool cpu_exec_all(void)
         if (qemu_alarm_pending())
             break;
         if (cpu_can_run(env)) {
-            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
+            r = qemu_cpu_exec(env);
+            if (kvm_enabled()) {
+                qemu_kvm_eat_signals(env);
+            }
+            if (r == EXCP_DEBUG) {
                 break;
             }
         } else if (env->stop) {
-- 
1.7.1


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

* [Qemu-devel] [PATCH 07/18] kvm: Add MCE signal support for !CONFIG_IOTHREAD
@ 2011-01-10  8:32   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm, Huang Ying

From: Jan Kiszka <jan.kiszka@siemens.com>

Currently, we only configure and process MCE-related SIGBUS events if
CONFIG_IOTHREAD is enabled. Fix this by factoring out the required
handler registration and system configuration. Make sure that events
happening over a VCPU context in non-threaded mode get dispatched as
VCPU MCEs.

We also need to call qemu_kvm_eat_signals in non-threaded mode now, so
move it (unmodified) and add the required Windows stub.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
CC: Huang Ying <ying.huang@intel.com>
---
 cpus.c |  200 +++++++++++++++++++++++++++++++++++++++------------------------
 1 files changed, 124 insertions(+), 76 deletions(-)

diff --git a/cpus.c b/cpus.c
index 6da0f8f..b6f1cfb 100644
--- a/cpus.c
+++ b/cpus.c
@@ -34,9 +34,6 @@
 
 #include "cpus.h"
 #include "compatfd.h"
-#ifdef CONFIG_LINUX
-#include <sys/prctl.h>
-#endif
 
 #ifdef SIGRTMIN
 #define SIG_IPI (SIGRTMIN+4)
@@ -44,10 +41,24 @@
 #define SIG_IPI SIGUSR1
 #endif
 
+#ifdef CONFIG_LINUX
+
+#include <sys/prctl.h>
+
 #ifndef PR_MCE_KILL
 #define PR_MCE_KILL 33
 #endif
 
+#ifndef PR_MCE_KILL_SET
+#define PR_MCE_KILL_SET 1
+#endif
+
+#ifndef PR_MCE_KILL_EARLY
+#define PR_MCE_KILL_EARLY 1
+#endif
+
+#endif /* CONFIG_LINUX */
+
 static CPUState *next_cpu;
 
 /***********************************************************/
@@ -158,6 +169,62 @@ static void cpu_debug_handler(CPUState *env)
     vm_stop(EXCP_DEBUG);
 }
 
+#ifdef CONFIG_LINUX
+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();
+}
+
+static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
+                           void *ctx)
+{
+#ifndef CONFIG_IOTHREAD
+    if (cpu_single_env) {
+        if (kvm_on_sigbus_vcpu(cpu_single_env, siginfo->ssi_code,
+                               (void *)(intptr_t)siginfo->ssi_addr)) {
+            sigbus_reraise();
+        }
+        return;
+    }
+#endif
+
+    if (kvm_on_sigbus(siginfo->ssi_code,
+                      (void *)(intptr_t)siginfo->ssi_addr)) {
+        sigbus_reraise();
+    }
+}
+
+static void qemu_init_sigbus(void)
+{
+    struct sigaction action;
+
+    memset(&action, 0, sizeof(action));
+    action.sa_flags = SA_SIGINFO;
+    action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler;
+    sigaction(SIGBUS, &action, NULL);
+
+    prctl(PR_MCE_KILL, PR_MCE_KILL_SET, PR_MCE_KILL_EARLY, 0, 0);
+}
+
+#else /* !CONFIG_LINUX */
+
+static void qemu_init_sigbus(void)
+{
+}
+#endif /* !CONFIG_LINUX */
+
 #ifndef _WIN32
 static int io_thread_fd = -1;
 
@@ -254,6 +321,43 @@ static void qemu_kvm_init_cpu_signals(CPUState *env)
     }
 }
 
+static void qemu_kvm_eat_signals(CPUState *env)
+{
+    struct timespec ts = { 0, 0 };
+    siginfo_t siginfo;
+    sigset_t waitset;
+    sigset_t chkset;
+    int r;
+
+    sigemptyset(&waitset);
+    sigaddset(&waitset, SIG_IPI);
+    sigaddset(&waitset, SIGBUS);
+
+    do {
+        r = sigtimedwait(&waitset, &siginfo, &ts);
+        if (r == -1 && !(errno == EAGAIN || errno == EINTR)) {
+            perror("sigtimedwait");
+            exit(1);
+        }
+
+        switch (r) {
+        case SIGBUS:
+            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) {
+                sigbus_reraise();
+            }
+            break;
+        default:
+            break;
+        }
+
+        r = sigpending(&chkset);
+        if (r == -1) {
+            perror("sigpending");
+            exit(1);
+        }
+    } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS));
+}
+
 #else /* _WIN32 */
 
 HANDLE qemu_event_handle;
@@ -285,6 +389,10 @@ static void qemu_event_increment(void)
 static void qemu_kvm_init_cpu_signals(CPUState *env)
 {
 }
+
+static void qemu_kvm_eat_signals(CPUState *env)
+{
+}
 #endif /* _WIN32 */
 
 #ifndef CONFIG_IOTHREAD
@@ -292,6 +400,8 @@ int qemu_init_main_loop(void)
 {
     cpu_set_debug_excp_handler(cpu_debug_handler);
 
+    qemu_init_sigbus();
+
     return qemu_event_init();
 }
 
@@ -432,13 +542,9 @@ static void qemu_tcg_init_cpu_signals(void)
     pthread_sigmask(SIG_UNBLOCK, &set, NULL);
 }
 
-static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
-                           void *ctx);
-
 static sigset_t block_io_signals(void)
 {
     sigset_t set;
-    struct sigaction action;
 
     /* SIGUSR2 used by posix-aio-compat.c */
     sigemptyset(&set);
@@ -449,15 +555,11 @@ static sigset_t block_io_signals(void)
     sigaddset(&set, SIGIO);
     sigaddset(&set, SIGALRM);
     sigaddset(&set, SIG_IPI);
+#ifdef CONFIG_LINUX
     sigaddset(&set, SIGBUS);
+#endif
     pthread_sigmask(SIG_BLOCK, &set, NULL);
 
-    memset(&action, 0, sizeof(action));
-    action.sa_flags = SA_SIGINFO;
-    action.sa_sigaction = (void (*)(int, siginfo_t*, void*))sigbus_handler;
-    sigaction(SIGBUS, &action, NULL);
-    prctl(PR_MCE_KILL, 1, 1, 0, 0);
-
     return set;
 }
 
@@ -486,6 +588,8 @@ int qemu_init_main_loop(void)
 
     cpu_set_debug_excp_handler(cpu_debug_handler);
 
+    qemu_init_sigbus();
+
     blocked_signals = block_io_signals();
 
     ret = qemu_signalfd_init(blocked_signals);
@@ -592,68 +696,6 @@ 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();
-}
-
-static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
-                           void *ctx)
-{
-    if (kvm_on_sigbus(siginfo->ssi_code, (void *)(intptr_t)siginfo->ssi_addr)) {
-        sigbus_reraise();
-    }
-}
-
-static void qemu_kvm_eat_signals(CPUState *env)
-{
-    struct timespec ts = { 0, 0 };
-    siginfo_t siginfo;
-    sigset_t waitset;
-    sigset_t chkset;
-    int r;
-
-    sigemptyset(&waitset);
-    sigaddset(&waitset, SIG_IPI);
-    sigaddset(&waitset, SIGBUS);
-
-    do {
-        r = sigtimedwait(&waitset, &siginfo, &ts);
-        if (r == -1 && !(errno == EAGAIN || errno == EINTR)) {
-            perror("sigtimedwait");
-            exit(1);
-        }
-
-        switch (r) {
-        case SIGBUS:
-            if (kvm_on_sigbus_vcpu(env, siginfo.si_code, siginfo.si_addr)) {
-                sigbus_reraise();
-            }
-            break;
-        default:
-            break;
-        }
-
-        r = sigpending(&chkset);
-        if (r == -1) {
-            perror("sigpending");
-            exit(1);
-        }
-    } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS));
-}
-
 static void qemu_kvm_wait_io_event(CPUState *env)
 {
     while (!cpu_has_work(env))
@@ -912,6 +954,8 @@ static int qemu_cpu_exec(CPUState *env)
 
 bool cpu_exec_all(void)
 {
+    int r;
+
     if (next_cpu == NULL)
         next_cpu = first_cpu;
     for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
@@ -923,7 +967,11 @@ bool cpu_exec_all(void)
         if (qemu_alarm_pending())
             break;
         if (cpu_can_run(env)) {
-            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
+            r = qemu_cpu_exec(env);
+            if (kvm_enabled()) {
+                qemu_kvm_eat_signals(env);
+            }
+            if (r == EXCP_DEBUG) {
                 break;
             }
         } else if (env->stop) {
-- 
1.7.1

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

* [PATCH 08/18] kvm: Handle kvm_init_vcpu errors
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:32   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

From: Jan Kiszka <jan.kiszka@siemens.com>

Do not ignore errors of kvm_init_vcpu, they are fatal.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index b6f1cfb..33b604e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -412,14 +412,19 @@ void qemu_main_loop_start(void)
 void qemu_init_vcpu(void *_env)
 {
     CPUState *env = _env;
+    int r;
 
     env->nr_cores = smp_cores;
     env->nr_threads = smp_threads;
     if (kvm_enabled()) {
         kvm_init_vcpu(env);
+        r = kvm_init_vcpu(env);
+        if (r < 0) {
+            fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(r));
+            exit(1);
+        }
         qemu_kvm_init_cpu_signals(env);
     }
-    return;
 }
 
 int qemu_cpu_self(void *env)
@@ -710,11 +715,16 @@ static int qemu_cpu_exec(CPUState *env);
 static void *kvm_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
+    int r;
 
     qemu_mutex_lock(&qemu_global_mutex);
     qemu_thread_self(env->thread);
 
-    kvm_init_vcpu(env);
+    r = kvm_init_vcpu(env);
+    if (r < 0) {
+        fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(r));
+        exit(1);
+    }
     qemu_kvm_init_cpu_signals(env);
 
     /* signal CPU creation */
-- 
1.7.1


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

* [Qemu-devel] [PATCH 08/18] kvm: Handle kvm_init_vcpu errors
@ 2011-01-10  8:32   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

Do not ignore errors of kvm_init_vcpu, they are fatal.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |   14 ++++++++++++--
 1 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index b6f1cfb..33b604e 100644
--- a/cpus.c
+++ b/cpus.c
@@ -412,14 +412,19 @@ void qemu_main_loop_start(void)
 void qemu_init_vcpu(void *_env)
 {
     CPUState *env = _env;
+    int r;
 
     env->nr_cores = smp_cores;
     env->nr_threads = smp_threads;
     if (kvm_enabled()) {
         kvm_init_vcpu(env);
+        r = kvm_init_vcpu(env);
+        if (r < 0) {
+            fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(r));
+            exit(1);
+        }
         qemu_kvm_init_cpu_signals(env);
     }
-    return;
 }
 
 int qemu_cpu_self(void *env)
@@ -710,11 +715,16 @@ static int qemu_cpu_exec(CPUState *env);
 static void *kvm_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
+    int r;
 
     qemu_mutex_lock(&qemu_global_mutex);
     qemu_thread_self(env->thread);
 
-    kvm_init_vcpu(env);
+    r = kvm_init_vcpu(env);
+    if (r < 0) {
+        fprintf(stderr, "kvm_init_vcpu failed: %s\n", strerror(r));
+        exit(1);
+    }
     qemu_kvm_init_cpu_signals(env);
 
     /* signal CPU creation */
-- 
1.7.1

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

* [PATCH 09/18] Refactor kvm&tcg function names in cpus.c
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:32   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

From: Jan Kiszka <jan.kiszka@siemens.com>

Pure interface cosmetics: Ensure that only kvm core services (as
declared in kvm.h) start with "kvm_". Prepend "qemu_" to those that
violate this rule in cpus.c. Also rename the corresponding tcg functions
for the sake of consistency.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/cpus.c b/cpus.c
index 33b604e..e482fdb 100644
--- a/cpus.c
+++ b/cpus.c
@@ -712,7 +712,7 @@ static void qemu_kvm_wait_io_event(CPUState *env)
 
 static int qemu_cpu_exec(CPUState *env);
 
-static void *kvm_cpu_thread_fn(void *arg)
+static void *qemu_kvm_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
     int r;
@@ -744,7 +744,7 @@ static void *kvm_cpu_thread_fn(void *arg)
     return NULL;
 }
 
-static void *tcg_cpu_thread_fn(void *arg)
+static void *qemu_tcg_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
 
@@ -850,7 +850,7 @@ void resume_all_vcpus(void)
     }
 }
 
-static void tcg_init_vcpu(void *_env)
+static void qemu_tcg_init_vcpu(void *_env)
 {
     CPUState *env = _env;
     /* share a single thread for all cpus with TCG */
@@ -858,7 +858,7 @@ static void tcg_init_vcpu(void *_env)
         env->thread = qemu_mallocz(sizeof(QemuThread));
         env->halt_cond = qemu_mallocz(sizeof(QemuCond));
         qemu_cond_init(env->halt_cond);
-        qemu_thread_create(env->thread, tcg_cpu_thread_fn, env);
+        qemu_thread_create(env->thread, qemu_tcg_cpu_thread_fn, env);
         while (env->created == 0)
             qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100);
         tcg_cpu_thread = env->thread;
@@ -869,12 +869,12 @@ static void tcg_init_vcpu(void *_env)
     }
 }
 
-static void kvm_start_vcpu(CPUState *env)
+static void qemu_kvm_start_vcpu(CPUState *env)
 {
     env->thread = qemu_mallocz(sizeof(QemuThread));
     env->halt_cond = qemu_mallocz(sizeof(QemuCond));
     qemu_cond_init(env->halt_cond);
-    qemu_thread_create(env->thread, kvm_cpu_thread_fn, env);
+    qemu_thread_create(env->thread, qemu_kvm_cpu_thread_fn, env);
     while (env->created == 0)
         qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100);
 }
@@ -886,9 +886,9 @@ void qemu_init_vcpu(void *_env)
     env->nr_cores = smp_cores;
     env->nr_threads = smp_threads;
     if (kvm_enabled())
-        kvm_start_vcpu(env);
+        qemu_kvm_start_vcpu(env);
     else
-        tcg_init_vcpu(env);
+        qemu_tcg_init_vcpu(env);
 }
 
 void qemu_notify_event(void)
-- 
1.7.1


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

* [Qemu-devel] [PATCH 09/18] Refactor kvm&tcg function names in cpus.c
@ 2011-01-10  8:32   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

Pure interface cosmetics: Ensure that only kvm core services (as
declared in kvm.h) start with "kvm_". Prepend "qemu_" to those that
violate this rule in cpus.c. Also rename the corresponding tcg functions
for the sake of consistency.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |   16 ++++++++--------
 1 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/cpus.c b/cpus.c
index 33b604e..e482fdb 100644
--- a/cpus.c
+++ b/cpus.c
@@ -712,7 +712,7 @@ static void qemu_kvm_wait_io_event(CPUState *env)
 
 static int qemu_cpu_exec(CPUState *env);
 
-static void *kvm_cpu_thread_fn(void *arg)
+static void *qemu_kvm_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
     int r;
@@ -744,7 +744,7 @@ static void *kvm_cpu_thread_fn(void *arg)
     return NULL;
 }
 
-static void *tcg_cpu_thread_fn(void *arg)
+static void *qemu_tcg_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
 
@@ -850,7 +850,7 @@ void resume_all_vcpus(void)
     }
 }
 
-static void tcg_init_vcpu(void *_env)
+static void qemu_tcg_init_vcpu(void *_env)
 {
     CPUState *env = _env;
     /* share a single thread for all cpus with TCG */
@@ -858,7 +858,7 @@ static void tcg_init_vcpu(void *_env)
         env->thread = qemu_mallocz(sizeof(QemuThread));
         env->halt_cond = qemu_mallocz(sizeof(QemuCond));
         qemu_cond_init(env->halt_cond);
-        qemu_thread_create(env->thread, tcg_cpu_thread_fn, env);
+        qemu_thread_create(env->thread, qemu_tcg_cpu_thread_fn, env);
         while (env->created == 0)
             qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100);
         tcg_cpu_thread = env->thread;
@@ -869,12 +869,12 @@ static void tcg_init_vcpu(void *_env)
     }
 }
 
-static void kvm_start_vcpu(CPUState *env)
+static void qemu_kvm_start_vcpu(CPUState *env)
 {
     env->thread = qemu_mallocz(sizeof(QemuThread));
     env->halt_cond = qemu_mallocz(sizeof(QemuCond));
     qemu_cond_init(env->halt_cond);
-    qemu_thread_create(env->thread, kvm_cpu_thread_fn, env);
+    qemu_thread_create(env->thread, qemu_kvm_cpu_thread_fn, env);
     while (env->created == 0)
         qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100);
 }
@@ -886,9 +886,9 @@ void qemu_init_vcpu(void *_env)
     env->nr_cores = smp_cores;
     env->nr_threads = smp_threads;
     if (kvm_enabled())
-        kvm_start_vcpu(env);
+        qemu_kvm_start_vcpu(env);
     else
-        tcg_init_vcpu(env);
+        qemu_tcg_init_vcpu(env);
 }
 
 void qemu_notify_event(void)
-- 
1.7.1

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

* [PATCH 10/18] Fix a few coding style violations in cpus.c
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:32   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

From: Jan Kiszka <jan.kiszka@siemens.com>

No functional changes.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |   96 ++++++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 58 insertions(+), 38 deletions(-)

diff --git a/cpus.c b/cpus.c
index e482fdb..89b4bd7 100644
--- a/cpus.c
+++ b/cpus.c
@@ -130,25 +130,26 @@ static void do_vm_stop(int reason)
 
 static int cpu_can_run(CPUState *env)
 {
-    if (env->stop)
+    if (env->stop) {
         return 0;
-    if (env->stopped || !vm_running)
+    }
+    if (env->stopped || !vm_running) {
         return 0;
+    }
     return 1;
 }
 
 static int cpu_has_work(CPUState *env)
 {
-    if (env->stop)
+    if (env->stop || env->queued_work_first) {
         return 1;
-    if (env->queued_work_first)
-        return 1;
-    if (env->stopped || !vm_running)
+    }
+    if (env->stopped || !vm_running) {
         return 0;
-    if (!env->halted)
-        return 1;
-    if (qemu_cpu_has_work(env))
+    }
+    if (!env->halted || qemu_cpu_has_work(env)) {
         return 1;
+    }
     return 0;
 }
 
@@ -156,9 +157,11 @@ static int any_cpu_has_work(void)
 {
     CPUState *env;
 
-    for (env = first_cpu; env != NULL; env = env->next_cpu)
-        if (cpu_has_work(env))
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
+        if (cpu_has_work(env)) {
             return 1;
+        }
+    }
     return 0;
 }
 
@@ -234,9 +237,9 @@ static void qemu_event_increment(void)
     static const uint64_t val = 1;
     ssize_t ret;
 
-    if (io_thread_fd == -1)
+    if (io_thread_fd == -1) {
         return;
-
+    }
     do {
         ret = write(io_thread_fd, &val, sizeof(val));
     } while (ret < 0 && errno == EINTR);
@@ -267,17 +270,17 @@ static int qemu_event_init(void)
     int fds[2];
 
     err = qemu_eventfd(fds);
-    if (err == -1)
+    if (err == -1) {
         return -errno;
-
+    }
     err = fcntl_setfl(fds[0], O_NONBLOCK);
-    if (err < 0)
+    if (err < 0) {
         goto fail;
-
+    }
     err = fcntl_setfl(fds[1], O_NONBLOCK);
-    if (err < 0)
+    if (err < 0) {
         goto fail;
-
+    }
     qemu_set_fd_handler2(fds[0], NULL, qemu_event_read, NULL,
                          (void *)(unsigned long)fds[0]);
 
@@ -598,13 +601,15 @@ int qemu_init_main_loop(void)
     blocked_signals = block_io_signals();
 
     ret = qemu_signalfd_init(blocked_signals);
-    if (ret)
+    if (ret) {
         return ret;
+    }
 
     /* Note eventfd must be drained before signalfd handlers run */
     ret = qemu_event_init();
-    if (ret)
+    if (ret) {
         return ret;
+    }
 
     qemu_cond_init(&qemu_pause_cond);
     qemu_cond_init(&qemu_system_cond);
@@ -634,10 +639,11 @@ void run_on_cpu(CPUState *env, void (*func)(void *data), void *data)
 
     wi.func = func;
     wi.data = data;
-    if (!env->queued_work_first)
+    if (!env->queued_work_first) {
         env->queued_work_first = &wi;
-    else
+    } else {
         env->queued_work_last->next = &wi;
+    }
     env->queued_work_last = &wi;
     wi.next = NULL;
     wi.done = false;
@@ -655,8 +661,9 @@ static void flush_queued_work(CPUState *env)
 {
     struct qemu_work_item *wi;
 
-    if (!env->queued_work_first)
+    if (!env->queued_work_first) {
         return;
+    }
 
     while ((wi = env->queued_work_first)) {
         env->queued_work_first = wi->next;
@@ -681,8 +688,9 @@ static void qemu_tcg_wait_io_event(void)
 {
     CPUState *env;
 
-    while (!any_cpu_has_work())
+    while (!any_cpu_has_work()) {
         qemu_cond_timedwait(tcg_halt_cond, &qemu_global_mutex, 1000);
+    }
 
     qemu_mutex_unlock(&qemu_global_mutex);
 
@@ -703,9 +711,9 @@ static void qemu_tcg_wait_io_event(void)
 
 static void qemu_kvm_wait_io_event(CPUState *env)
 {
-    while (!cpu_has_work(env))
+    while (!cpu_has_work(env)) {
         qemu_cond_timedwait(env->halt_cond, &qemu_global_mutex, 1000);
-
+    }
     qemu_kvm_eat_signals(env);
     qemu_wait_io_event_common(env);
 }
@@ -732,12 +740,14 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
     qemu_cond_signal(&qemu_cpu_cond);
 
     /* and wait for machine initialization */
-    while (!qemu_system_ready)
+    while (!qemu_system_ready) {
         qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100);
+    }
 
     while (1) {
-        if (cpu_can_run(env))
+        if (cpu_can_run(env)) {
             qemu_cpu_exec(env);
+        }
         qemu_kvm_wait_io_event(env);
     }
 
@@ -753,13 +763,15 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 
     /* signal CPU creation */
     qemu_mutex_lock(&qemu_global_mutex);
-    for (env = first_cpu; env != NULL; env = env->next_cpu)
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
         env->created = 1;
+    }
     qemu_cond_signal(&qemu_cpu_cond);
 
     /* and wait for machine initialization */
-    while (!qemu_system_ready)
+    while (!qemu_system_ready) {
         qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100);
+    }
 
     while (1) {
         cpu_exec_all();
@@ -772,6 +784,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 void qemu_cpu_kick(void *_env)
 {
     CPUState *env = _env;
+
     qemu_cond_broadcast(env->halt_cond);
     qemu_thread_signal(env->thread, SIG_IPI);
 }
@@ -810,8 +823,9 @@ static int all_vcpus_paused(void)
     CPUState *penv = first_cpu;
 
     while (penv) {
-        if (!penv->stopped)
+        if (!penv->stopped) {
             return 0;
+        }
         penv = (CPUState *)penv->next_cpu;
     }
 
@@ -853,14 +867,16 @@ void resume_all_vcpus(void)
 static void qemu_tcg_init_vcpu(void *_env)
 {
     CPUState *env = _env;
+
     /* share a single thread for all cpus with TCG */
     if (!tcg_cpu_thread) {
         env->thread = qemu_mallocz(sizeof(QemuThread));
         env->halt_cond = qemu_mallocz(sizeof(QemuCond));
         qemu_cond_init(env->halt_cond);
         qemu_thread_create(env->thread, qemu_tcg_cpu_thread_fn, env);
-        while (env->created == 0)
+        while (env->created == 0) {
             qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100);
+        }
         tcg_cpu_thread = env->thread;
         tcg_halt_cond = env->halt_cond;
     } else {
@@ -875,8 +891,9 @@ static void qemu_kvm_start_vcpu(CPUState *env)
     env->halt_cond = qemu_mallocz(sizeof(QemuCond));
     qemu_cond_init(env->halt_cond);
     qemu_thread_create(env->thread, qemu_kvm_cpu_thread_fn, env);
-    while (env->created == 0)
+    while (env->created == 0) {
         qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100);
+    }
 }
 
 void qemu_init_vcpu(void *_env)
@@ -885,10 +902,11 @@ void qemu_init_vcpu(void *_env)
 
     env->nr_cores = smp_cores;
     env->nr_threads = smp_threads;
-    if (kvm_enabled())
+    if (kvm_enabled()) {
         qemu_kvm_start_vcpu(env);
-    else
+    } else {
         qemu_tcg_init_vcpu(env);
+    }
 }
 
 void qemu_notify_event(void)
@@ -966,16 +984,18 @@ bool cpu_exec_all(void)
 {
     int r;
 
-    if (next_cpu == NULL)
+    if (next_cpu == NULL) {
         next_cpu = first_cpu;
+    }
     for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
         CPUState *env = next_cpu;
 
         qemu_clock_enable(vm_clock,
                           (env->singlestep_enabled & SSTEP_NOTIMER) == 0);
 
-        if (qemu_alarm_pending())
+        if (qemu_alarm_pending()) {
             break;
+        }
         if (cpu_can_run(env)) {
             r = qemu_cpu_exec(env);
             if (kvm_enabled()) {
-- 
1.7.1


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

* [Qemu-devel] [PATCH 10/18] Fix a few coding style violations in cpus.c
@ 2011-01-10  8:32   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

No functional changes.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c |   96 ++++++++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 58 insertions(+), 38 deletions(-)

diff --git a/cpus.c b/cpus.c
index e482fdb..89b4bd7 100644
--- a/cpus.c
+++ b/cpus.c
@@ -130,25 +130,26 @@ static void do_vm_stop(int reason)
 
 static int cpu_can_run(CPUState *env)
 {
-    if (env->stop)
+    if (env->stop) {
         return 0;
-    if (env->stopped || !vm_running)
+    }
+    if (env->stopped || !vm_running) {
         return 0;
+    }
     return 1;
 }
 
 static int cpu_has_work(CPUState *env)
 {
-    if (env->stop)
+    if (env->stop || env->queued_work_first) {
         return 1;
-    if (env->queued_work_first)
-        return 1;
-    if (env->stopped || !vm_running)
+    }
+    if (env->stopped || !vm_running) {
         return 0;
-    if (!env->halted)
-        return 1;
-    if (qemu_cpu_has_work(env))
+    }
+    if (!env->halted || qemu_cpu_has_work(env)) {
         return 1;
+    }
     return 0;
 }
 
@@ -156,9 +157,11 @@ static int any_cpu_has_work(void)
 {
     CPUState *env;
 
-    for (env = first_cpu; env != NULL; env = env->next_cpu)
-        if (cpu_has_work(env))
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
+        if (cpu_has_work(env)) {
             return 1;
+        }
+    }
     return 0;
 }
 
@@ -234,9 +237,9 @@ static void qemu_event_increment(void)
     static const uint64_t val = 1;
     ssize_t ret;
 
-    if (io_thread_fd == -1)
+    if (io_thread_fd == -1) {
         return;
-
+    }
     do {
         ret = write(io_thread_fd, &val, sizeof(val));
     } while (ret < 0 && errno == EINTR);
@@ -267,17 +270,17 @@ static int qemu_event_init(void)
     int fds[2];
 
     err = qemu_eventfd(fds);
-    if (err == -1)
+    if (err == -1) {
         return -errno;
-
+    }
     err = fcntl_setfl(fds[0], O_NONBLOCK);
-    if (err < 0)
+    if (err < 0) {
         goto fail;
-
+    }
     err = fcntl_setfl(fds[1], O_NONBLOCK);
-    if (err < 0)
+    if (err < 0) {
         goto fail;
-
+    }
     qemu_set_fd_handler2(fds[0], NULL, qemu_event_read, NULL,
                          (void *)(unsigned long)fds[0]);
 
@@ -598,13 +601,15 @@ int qemu_init_main_loop(void)
     blocked_signals = block_io_signals();
 
     ret = qemu_signalfd_init(blocked_signals);
-    if (ret)
+    if (ret) {
         return ret;
+    }
 
     /* Note eventfd must be drained before signalfd handlers run */
     ret = qemu_event_init();
-    if (ret)
+    if (ret) {
         return ret;
+    }
 
     qemu_cond_init(&qemu_pause_cond);
     qemu_cond_init(&qemu_system_cond);
@@ -634,10 +639,11 @@ void run_on_cpu(CPUState *env, void (*func)(void *data), void *data)
 
     wi.func = func;
     wi.data = data;
-    if (!env->queued_work_first)
+    if (!env->queued_work_first) {
         env->queued_work_first = &wi;
-    else
+    } else {
         env->queued_work_last->next = &wi;
+    }
     env->queued_work_last = &wi;
     wi.next = NULL;
     wi.done = false;
@@ -655,8 +661,9 @@ static void flush_queued_work(CPUState *env)
 {
     struct qemu_work_item *wi;
 
-    if (!env->queued_work_first)
+    if (!env->queued_work_first) {
         return;
+    }
 
     while ((wi = env->queued_work_first)) {
         env->queued_work_first = wi->next;
@@ -681,8 +688,9 @@ static void qemu_tcg_wait_io_event(void)
 {
     CPUState *env;
 
-    while (!any_cpu_has_work())
+    while (!any_cpu_has_work()) {
         qemu_cond_timedwait(tcg_halt_cond, &qemu_global_mutex, 1000);
+    }
 
     qemu_mutex_unlock(&qemu_global_mutex);
 
@@ -703,9 +711,9 @@ static void qemu_tcg_wait_io_event(void)
 
 static void qemu_kvm_wait_io_event(CPUState *env)
 {
-    while (!cpu_has_work(env))
+    while (!cpu_has_work(env)) {
         qemu_cond_timedwait(env->halt_cond, &qemu_global_mutex, 1000);
-
+    }
     qemu_kvm_eat_signals(env);
     qemu_wait_io_event_common(env);
 }
@@ -732,12 +740,14 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
     qemu_cond_signal(&qemu_cpu_cond);
 
     /* and wait for machine initialization */
-    while (!qemu_system_ready)
+    while (!qemu_system_ready) {
         qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100);
+    }
 
     while (1) {
-        if (cpu_can_run(env))
+        if (cpu_can_run(env)) {
             qemu_cpu_exec(env);
+        }
         qemu_kvm_wait_io_event(env);
     }
 
@@ -753,13 +763,15 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 
     /* signal CPU creation */
     qemu_mutex_lock(&qemu_global_mutex);
-    for (env = first_cpu; env != NULL; env = env->next_cpu)
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
         env->created = 1;
+    }
     qemu_cond_signal(&qemu_cpu_cond);
 
     /* and wait for machine initialization */
-    while (!qemu_system_ready)
+    while (!qemu_system_ready) {
         qemu_cond_timedwait(&qemu_system_cond, &qemu_global_mutex, 100);
+    }
 
     while (1) {
         cpu_exec_all();
@@ -772,6 +784,7 @@ static void *qemu_tcg_cpu_thread_fn(void *arg)
 void qemu_cpu_kick(void *_env)
 {
     CPUState *env = _env;
+
     qemu_cond_broadcast(env->halt_cond);
     qemu_thread_signal(env->thread, SIG_IPI);
 }
@@ -810,8 +823,9 @@ static int all_vcpus_paused(void)
     CPUState *penv = first_cpu;
 
     while (penv) {
-        if (!penv->stopped)
+        if (!penv->stopped) {
             return 0;
+        }
         penv = (CPUState *)penv->next_cpu;
     }
 
@@ -853,14 +867,16 @@ void resume_all_vcpus(void)
 static void qemu_tcg_init_vcpu(void *_env)
 {
     CPUState *env = _env;
+
     /* share a single thread for all cpus with TCG */
     if (!tcg_cpu_thread) {
         env->thread = qemu_mallocz(sizeof(QemuThread));
         env->halt_cond = qemu_mallocz(sizeof(QemuCond));
         qemu_cond_init(env->halt_cond);
         qemu_thread_create(env->thread, qemu_tcg_cpu_thread_fn, env);
-        while (env->created == 0)
+        while (env->created == 0) {
             qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100);
+        }
         tcg_cpu_thread = env->thread;
         tcg_halt_cond = env->halt_cond;
     } else {
@@ -875,8 +891,9 @@ static void qemu_kvm_start_vcpu(CPUState *env)
     env->halt_cond = qemu_mallocz(sizeof(QemuCond));
     qemu_cond_init(env->halt_cond);
     qemu_thread_create(env->thread, qemu_kvm_cpu_thread_fn, env);
-    while (env->created == 0)
+    while (env->created == 0) {
         qemu_cond_timedwait(&qemu_cpu_cond, &qemu_global_mutex, 100);
+    }
 }
 
 void qemu_init_vcpu(void *_env)
@@ -885,10 +902,11 @@ void qemu_init_vcpu(void *_env)
 
     env->nr_cores = smp_cores;
     env->nr_threads = smp_threads;
-    if (kvm_enabled())
+    if (kvm_enabled()) {
         qemu_kvm_start_vcpu(env);
-    else
+    } else {
         qemu_tcg_init_vcpu(env);
+    }
 }
 
 void qemu_notify_event(void)
@@ -966,16 +984,18 @@ bool cpu_exec_all(void)
 {
     int r;
 
-    if (next_cpu == NULL)
+    if (next_cpu == NULL) {
         next_cpu = first_cpu;
+    }
     for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
         CPUState *env = next_cpu;
 
         qemu_clock_enable(vm_clock,
                           (env->singlestep_enabled & SSTEP_NOTIMER) == 0);
 
-        if (qemu_alarm_pending())
+        if (qemu_alarm_pending()) {
             break;
+        }
         if (cpu_can_run(env)) {
             r = qemu_cpu_exec(env);
             if (kvm_enabled()) {
-- 
1.7.1

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

* [PATCH 11/18] Introduce VCPU self-signaling service
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:32   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

From: Jan Kiszka <jan.kiszka@siemens.com>

Introduce qemu_cpu_kick_self to send SIG_IPI to the calling VCPU
context. First user will be kvm.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c        |   19 ++++++++++++++++++-
 qemu-common.h |    1 +
 2 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/cpus.c b/cpus.c
index 89b4bd7..4a4d130 100644
--- a/cpus.c
+++ b/cpus.c
@@ -450,7 +450,17 @@ void pause_all_vcpus(void)
 
 void qemu_cpu_kick(void *env)
 {
-    return;
+}
+
+void qemu_cpu_kick_self(void)
+{
+#ifndef _WIN32
+    assert(cpu_single_env);
+
+    raise(SIG_IPI);
+#else
+    abort();
+#endif
 }
 
 void qemu_notify_event(void)
@@ -789,6 +799,13 @@ void qemu_cpu_kick(void *_env)
     qemu_thread_signal(env->thread, SIG_IPI);
 }
 
+void qemu_cpu_kick_self(void)
+{
+    assert(cpu_single_env);
+
+    qemu_thread_signal(cpu_single_env->thread, SIG_IPI);
+}
+
 int qemu_cpu_self(void *_env)
 {
     CPUState *env = _env;
diff --git a/qemu-common.h b/qemu-common.h
index 63d9943..220c8c8 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -287,6 +287,7 @@ void qemu_notify_event(void);
 
 /* Unblock cpu */
 void qemu_cpu_kick(void *env);
+void qemu_cpu_kick_self(void);
 int qemu_cpu_self(void *env);
 
 /* work queue */
-- 
1.7.1


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

* [Qemu-devel] [PATCH 11/18] Introduce VCPU self-signaling service
@ 2011-01-10  8:32   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

Introduce qemu_cpu_kick_self to send SIG_IPI to the calling VCPU
context. First user will be kvm.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpus.c        |   19 ++++++++++++++++++-
 qemu-common.h |    1 +
 2 files changed, 19 insertions(+), 1 deletions(-)

diff --git a/cpus.c b/cpus.c
index 89b4bd7..4a4d130 100644
--- a/cpus.c
+++ b/cpus.c
@@ -450,7 +450,17 @@ void pause_all_vcpus(void)
 
 void qemu_cpu_kick(void *env)
 {
-    return;
+}
+
+void qemu_cpu_kick_self(void)
+{
+#ifndef _WIN32
+    assert(cpu_single_env);
+
+    raise(SIG_IPI);
+#else
+    abort();
+#endif
 }
 
 void qemu_notify_event(void)
@@ -789,6 +799,13 @@ void qemu_cpu_kick(void *_env)
     qemu_thread_signal(env->thread, SIG_IPI);
 }
 
+void qemu_cpu_kick_self(void)
+{
+    assert(cpu_single_env);
+
+    qemu_thread_signal(cpu_single_env->thread, SIG_IPI);
+}
+
 int qemu_cpu_self(void *_env)
 {
     CPUState *env = _env;
diff --git a/qemu-common.h b/qemu-common.h
index 63d9943..220c8c8 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -287,6 +287,7 @@ void qemu_notify_event(void);
 
 /* Unblock cpu */
 void qemu_cpu_kick(void *env);
+void qemu_cpu_kick_self(void);
 int qemu_cpu_self(void *env);
 
 /* work queue */
-- 
1.7.1

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

* [PATCH 12/18] kvm: Move irqchip event processing out of inner loop
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:32   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

From: Jan Kiszka <jan.kiszka@siemens.com>

Align with qemu-kvm and prepare for IO exit fix: There is no need to run
kvm_arch_process_irqchip_events in the inner VCPU loop. Any state change
this service processes will first cause an exit from kvm_cpu_exec
anyway. And we will have to reenter the kernel on IO exits
unconditionally, something that the current logic prevents.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index a8e9f2c..f3c8375 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -824,6 +824,12 @@ int kvm_cpu_exec(CPUState *env)
 
     DPRINTF("kvm_cpu_exec()\n");
 
+    if (kvm_arch_process_irqchip_events(env)) {
+        env->exit_request = 0;
+        env->exception_index = EXCP_HLT;
+        return 0;
+    }
+
     do {
 #ifndef CONFIG_IOTHREAD
         if (env->exit_request) {
@@ -833,11 +839,6 @@ int kvm_cpu_exec(CPUState *env)
         }
 #endif
 
-        if (kvm_arch_process_irqchip_events(env)) {
-            ret = 0;
-            break;
-        }
-
         if (env->kvm_vcpu_dirty) {
             kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE);
             env->kvm_vcpu_dirty = 0;
-- 
1.7.1


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

* [Qemu-devel] [PATCH 12/18] kvm: Move irqchip event processing out of inner loop
@ 2011-01-10  8:32   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

Align with qemu-kvm and prepare for IO exit fix: There is no need to run
kvm_arch_process_irqchip_events in the inner VCPU loop. Any state change
this service processes will first cause an exit from kvm_cpu_exec
anyway. And we will have to reenter the kernel on IO exits
unconditionally, something that the current logic prevents.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index a8e9f2c..f3c8375 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -824,6 +824,12 @@ int kvm_cpu_exec(CPUState *env)
 
     DPRINTF("kvm_cpu_exec()\n");
 
+    if (kvm_arch_process_irqchip_events(env)) {
+        env->exit_request = 0;
+        env->exception_index = EXCP_HLT;
+        return 0;
+    }
+
     do {
 #ifndef CONFIG_IOTHREAD
         if (env->exit_request) {
@@ -833,11 +839,6 @@ int kvm_cpu_exec(CPUState *env)
         }
 #endif
 
-        if (kvm_arch_process_irqchip_events(env)) {
-            ret = 0;
-            break;
-        }
-
         if (env->kvm_vcpu_dirty) {
             kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE);
             env->kvm_vcpu_dirty = 0;
-- 
1.7.1

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

* [PATCH 13/18] kvm: Unconditionally reenter kernel after IO exits
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:32   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel, Gleb Natapov

From: Jan Kiszka <jan.kiszka@siemens.com>

KVM requires to reenter the kernel after IO exits in order to complete
instruction emulation. Failing to do so will leave the kernel state
inconsistently behind. To ensure that we will get back ASAP, we issue a
self-signal that will cause KVM_RUN to return once the pending
operations are completed.

This patch also fixes the missing exit_request check in kvm_cpu_exec in
the CONFIG_IOTHREAD case.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
CC: Gleb Natapov <gleb@redhat.com>
---
 kvm-all.c |   20 +++++++++++---------
 1 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index f3c8375..429ab7a 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -188,7 +188,6 @@ int kvm_pit_in_kernel(void)
     return kvm_state.pit_in_kernel;
 }
 
-
 int kvm_init_vcpu(CPUState *env)
 {
     long mmap_size;
@@ -831,23 +830,26 @@ int kvm_cpu_exec(CPUState *env)
     }
 
     do {
-#ifndef CONFIG_IOTHREAD
-        if (env->exit_request) {
-            DPRINTF("interrupt exit requested\n");
-            ret = 0;
-            break;
-        }
-#endif
-
         if (env->kvm_vcpu_dirty) {
             kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE);
             env->kvm_vcpu_dirty = 0;
         }
 
         kvm_arch_pre_run(env, run);
+        if (env->exit_request) {
+            DPRINTF("interrupt exit requested\n");
+            /*
+             * KVM requires us to reenter the kernel after IO exits to complete
+             * instruction emulation. This self-signal will ensure that we
+             * leave ASAP again.
+             */
+            qemu_cpu_kick_self();
+        }
         cpu_single_env = NULL;
         qemu_mutex_unlock_iothread();
+
         ret = kvm_vcpu_ioctl(env, KVM_RUN, 0);
+
         qemu_mutex_lock_iothread();
         cpu_single_env = env;
         kvm_arch_post_run(env, run);
-- 
1.7.1


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

* [Qemu-devel] [PATCH 13/18] kvm: Unconditionally reenter kernel after IO exits
@ 2011-01-10  8:32   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: Gleb Natapov, qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

KVM requires to reenter the kernel after IO exits in order to complete
instruction emulation. Failing to do so will leave the kernel state
inconsistently behind. To ensure that we will get back ASAP, we issue a
self-signal that will cause KVM_RUN to return once the pending
operations are completed.

This patch also fixes the missing exit_request check in kvm_cpu_exec in
the CONFIG_IOTHREAD case.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
CC: Gleb Natapov <gleb@redhat.com>
---
 kvm-all.c |   20 +++++++++++---------
 1 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index f3c8375..429ab7a 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -188,7 +188,6 @@ int kvm_pit_in_kernel(void)
     return kvm_state.pit_in_kernel;
 }
 
-
 int kvm_init_vcpu(CPUState *env)
 {
     long mmap_size;
@@ -831,23 +830,26 @@ int kvm_cpu_exec(CPUState *env)
     }
 
     do {
-#ifndef CONFIG_IOTHREAD
-        if (env->exit_request) {
-            DPRINTF("interrupt exit requested\n");
-            ret = 0;
-            break;
-        }
-#endif
-
         if (env->kvm_vcpu_dirty) {
             kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE);
             env->kvm_vcpu_dirty = 0;
         }
 
         kvm_arch_pre_run(env, run);
+        if (env->exit_request) {
+            DPRINTF("interrupt exit requested\n");
+            /*
+             * KVM requires us to reenter the kernel after IO exits to complete
+             * instruction emulation. This self-signal will ensure that we
+             * leave ASAP again.
+             */
+            qemu_cpu_kick_self();
+        }
         cpu_single_env = NULL;
         qemu_mutex_unlock_iothread();
+
         ret = kvm_vcpu_ioctl(env, KVM_RUN, 0);
+
         qemu_mutex_lock_iothread();
         cpu_single_env = env;
         kvm_arch_post_run(env, run);
-- 
1.7.1

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

* [PATCH 14/18] kvm: Remove static return code of kvm_handle_io
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:32   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

From: Jan Kiszka <jan.kiszka@siemens.com>

Improve the readability of the exit dispatcher by moving the static
return value of kvm_handle_io to its caller.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 429ab7a..fb4d73e 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -703,8 +703,8 @@ err:
     return ret;
 }
 
-static int kvm_handle_io(uint16_t port, void *data, int direction, int size,
-                         uint32_t count)
+static void kvm_handle_io(uint16_t port, void *data, int direction, int size,
+                          uint32_t count)
 {
     int i;
     uint8_t *ptr = data;
@@ -738,8 +738,6 @@ static int kvm_handle_io(uint16_t port, void *data, int direction, int size,
 
         ptr += size;
     }
-
-    return 1;
 }
 
 #ifdef KVM_CAP_INTERNAL_ERROR_DATA
@@ -872,11 +870,12 @@ int kvm_cpu_exec(CPUState *env)
         switch (run->exit_reason) {
         case KVM_EXIT_IO:
             DPRINTF("handle_io\n");
-            ret = kvm_handle_io(run->io.port,
-                                (uint8_t *)run + run->io.data_offset,
-                                run->io.direction,
-                                run->io.size,
-                                run->io.count);
+            kvm_handle_io(run->io.port,
+                          (uint8_t *)run + run->io.data_offset,
+                          run->io.direction,
+                          run->io.size,
+                          run->io.count);
+            ret = 1;
             break;
         case KVM_EXIT_MMIO:
             DPRINTF("handle_mmio\n");
-- 
1.7.1


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

* [Qemu-devel] [PATCH 14/18] kvm: Remove static return code of kvm_handle_io
@ 2011-01-10  8:32   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

Improve the readability of the exit dispatcher by moving the static
return value of kvm_handle_io to its caller.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c |   17 ++++++++---------
 1 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 429ab7a..fb4d73e 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -703,8 +703,8 @@ err:
     return ret;
 }
 
-static int kvm_handle_io(uint16_t port, void *data, int direction, int size,
-                         uint32_t count)
+static void kvm_handle_io(uint16_t port, void *data, int direction, int size,
+                          uint32_t count)
 {
     int i;
     uint8_t *ptr = data;
@@ -738,8 +738,6 @@ static int kvm_handle_io(uint16_t port, void *data, int direction, int size,
 
         ptr += size;
     }
-
-    return 1;
 }
 
 #ifdef KVM_CAP_INTERNAL_ERROR_DATA
@@ -872,11 +870,12 @@ int kvm_cpu_exec(CPUState *env)
         switch (run->exit_reason) {
         case KVM_EXIT_IO:
             DPRINTF("handle_io\n");
-            ret = kvm_handle_io(run->io.port,
-                                (uint8_t *)run + run->io.data_offset,
-                                run->io.direction,
-                                run->io.size,
-                                run->io.count);
+            kvm_handle_io(run->io.port,
+                          (uint8_t *)run + run->io.data_offset,
+                          run->io.direction,
+                          run->io.size,
+                          run->io.count);
+            ret = 1;
             break;
         case KVM_EXIT_MMIO:
             DPRINTF("handle_mmio\n");
-- 
1.7.1

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

* [PATCH 15/18] kvm: Leave kvm_cpu_exec directly after KVM_EXIT_SHUTDOWN
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:32   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

From: Jan Kiszka <jan.kiszka@siemens.com>

The reset we issue on KVM_EXIT_SHUTDOWN implies that we should also
leave the VCPU loop. As we now check for exit_request which is set by
qemu_system_reset_request, this bug is no longer critical. Still it's an
unneeded extra turn.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index fb4d73e..0abe088 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -891,7 +891,6 @@ int kvm_cpu_exec(CPUState *env)
         case KVM_EXIT_SHUTDOWN:
             DPRINTF("shutdown\n");
             qemu_system_reset_request();
-            ret = 1;
             break;
         case KVM_EXIT_UNKNOWN:
             fprintf(stderr, "KVM: unknown exit, hardware reason %" PRIx64 "\n",
-- 
1.7.1


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

* [Qemu-devel] [PATCH 15/18] kvm: Leave kvm_cpu_exec directly after KVM_EXIT_SHUTDOWN
@ 2011-01-10  8:32   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

The reset we issue on KVM_EXIT_SHUTDOWN implies that we should also
leave the VCPU loop. As we now check for exit_request which is set by
qemu_system_reset_request, this bug is no longer critical. Still it's an
unneeded extra turn.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 kvm-all.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index fb4d73e..0abe088 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -891,7 +891,6 @@ int kvm_cpu_exec(CPUState *env)
         case KVM_EXIT_SHUTDOWN:
             DPRINTF("shutdown\n");
             qemu_system_reset_request();
-            ret = 1;
             break;
         case KVM_EXIT_UNKNOWN:
             fprintf(stderr, "KVM: unknown exit, hardware reason %" PRIx64 "\n",
-- 
1.7.1

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

* [PATCH 16/18] kvm: Separate TCG from KVM cpu execution
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:32   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

From: Jan Kiszka <jan.kiszka@siemens.com>

Mixing up TCG bits with KVM already led to problems around eflags
emulation on x86. Moreover, quite some code that TCG requires on cpu
enty/exit is useless for KVM. So dispatch between tcg_cpu_exec and
kvm_cpu_exec as early as possible.

The core logic of cpu_halted from cpu_exec is added to
kvm_arch_process_irqchip_events. Moving away from cpu_exec makes
exception_index meaningless for KVM, we can simply pass the exit reason
directly (only "EXCP_DEBUG vs. rest" is relevant).

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-exec.c        |   19 ++++++-------------
 cpus.c            |   10 +++++-----
 kvm-all.c         |   19 +++++++++----------
 target-i386/kvm.c |    6 +++---
 4 files changed, 23 insertions(+), 31 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 8c9fb8b..e4fe4f8 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -248,13 +248,11 @@ int cpu_exec(CPUState *env1)
     }
 
 #if defined(TARGET_I386)
-    if (!kvm_enabled()) {
-        /* put eflags in CPU temporary format */
-        CC_SRC = env->eflags & (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
-        DF = 1 - (2 * ((env->eflags >> 10) & 1));
-        CC_OP = CC_OP_EFLAGS;
-        env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
-    }
+    /* put eflags in CPU temporary format */
+    CC_SRC = env->eflags & (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
+    DF = 1 - (2 * ((env->eflags >> 10) & 1));
+    CC_OP = CC_OP_EFLAGS;
+    env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
 #elif defined(TARGET_SPARC)
 #elif defined(TARGET_M68K)
     env->cc_op = CC_OP_FLAGS;
@@ -279,7 +277,7 @@ int cpu_exec(CPUState *env1)
         if (setjmp(env->jmp_env) == 0) {
 #if defined(__sparc__) && !defined(CONFIG_SOLARIS)
 #undef env
-                    env = cpu_single_env;
+            env = cpu_single_env;
 #define env cpu_single_env
 #endif
             /* if an exception is pending, we execute it here */
@@ -340,11 +338,6 @@ int cpu_exec(CPUState *env1)
                 }
             }
 
-            if (kvm_enabled()) {
-                kvm_cpu_exec(env);
-                longjmp(env->jmp_env, 1);
-            }
-
             next_tb = 0; /* force lookup of first TB */
             for(;;) {
                 interrupt_request = env->interrupt_request;
diff --git a/cpus.c b/cpus.c
index 4a4d130..e4ff314 100644
--- a/cpus.c
+++ b/cpus.c
@@ -728,8 +728,6 @@ static void qemu_kvm_wait_io_event(CPUState *env)
     qemu_wait_io_event_common(env);
 }
 
-static int qemu_cpu_exec(CPUState *env);
-
 static void *qemu_kvm_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
@@ -756,7 +754,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
 
     while (1) {
         if (cpu_can_run(env)) {
-            qemu_cpu_exec(env);
+            kvm_cpu_exec(env);
         }
         qemu_kvm_wait_io_event(env);
     }
@@ -959,7 +957,7 @@ void vm_stop(int reason)
 
 #endif
 
-static int qemu_cpu_exec(CPUState *env)
+static int tcg_cpu_exec(CPUState *env)
 {
     int ret;
 #ifdef CONFIG_PROFILER
@@ -1014,9 +1012,11 @@ bool cpu_exec_all(void)
             break;
         }
         if (cpu_can_run(env)) {
-            r = qemu_cpu_exec(env);
             if (kvm_enabled()) {
+                r = kvm_cpu_exec(env);
                 qemu_kvm_eat_signals(env);
+            } else {
+                r = tcg_cpu_exec(env);
             }
             if (r == EXCP_DEBUG) {
                 break;
diff --git a/kvm-all.c b/kvm-all.c
index 0abe088..fd5a137 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -823,10 +823,11 @@ int kvm_cpu_exec(CPUState *env)
 
     if (kvm_arch_process_irqchip_events(env)) {
         env->exit_request = 0;
-        env->exception_index = EXCP_HLT;
-        return 0;
+        return EXCP_HLT;
     }
 
+    cpu_single_env = env;
+
     do {
         if (env->kvm_vcpu_dirty) {
             kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE);
@@ -855,7 +856,6 @@ int kvm_cpu_exec(CPUState *env)
         kvm_flush_coalesced_mmio_buffer();
 
         if (ret == -EINTR || ret == -EAGAIN) {
-            cpu_exit(env);
             DPRINTF("io window exit\n");
             ret = 0;
             break;
@@ -906,8 +906,8 @@ int kvm_cpu_exec(CPUState *env)
             DPRINTF("kvm_exit_debug\n");
 #ifdef KVM_CAP_SET_GUEST_DEBUG
             if (kvm_arch_debug(&run->debug.arch)) {
-                env->exception_index = EXCP_DEBUG;
-                return 0;
+                ret = EXCP_DEBUG;
+                goto out;
             }
             /* re-enter, this exception was guest-internal */
             ret = 1;
@@ -923,13 +923,12 @@ int kvm_cpu_exec(CPUState *env)
     if (ret < 0) {
         cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE);
         vm_stop(0);
-        env->exit_request = 1;
-    }
-    if (env->exit_request) {
-        env->exit_request = 0;
-        env->exception_index = EXCP_INTERRUPT;
     }
+    ret = EXCP_INTERRUPT;
 
+out:
+    env->exit_request = 0;
+    cpu_single_env = NULL;
     return ret;
 }
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index a7acd53..16dacb8 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1563,12 +1563,13 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 
 int kvm_arch_process_irqchip_events(CPUState *env)
 {
+    if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
+        env->halted = 0;
+    }
     if (env->interrupt_request & CPU_INTERRUPT_INIT) {
         kvm_cpu_synchronize_state(env);
         do_cpu_init(env);
-        env->exception_index = EXCP_HALTED;
     }
-
     if (env->interrupt_request & CPU_INTERRUPT_SIPI) {
         kvm_cpu_synchronize_state(env);
         do_cpu_sipi(env);
@@ -1583,7 +1584,6 @@ static int kvm_handle_halt(CPUState *env)
           (env->eflags & IF_MASK)) &&
         !(env->interrupt_request & CPU_INTERRUPT_NMI)) {
         env->halted = 1;
-        env->exception_index = EXCP_HLT;
         return 0;
     }
 
-- 
1.7.1


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

* [Qemu-devel] [PATCH 16/18] kvm: Separate TCG from KVM cpu execution
@ 2011-01-10  8:32   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

Mixing up TCG bits with KVM already led to problems around eflags
emulation on x86. Moreover, quite some code that TCG requires on cpu
enty/exit is useless for KVM. So dispatch between tcg_cpu_exec and
kvm_cpu_exec as early as possible.

The core logic of cpu_halted from cpu_exec is added to
kvm_arch_process_irqchip_events. Moving away from cpu_exec makes
exception_index meaningless for KVM, we can simply pass the exit reason
directly (only "EXCP_DEBUG vs. rest" is relevant).

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 cpu-exec.c        |   19 ++++++-------------
 cpus.c            |   10 +++++-----
 kvm-all.c         |   19 +++++++++----------
 target-i386/kvm.c |    6 +++---
 4 files changed, 23 insertions(+), 31 deletions(-)

diff --git a/cpu-exec.c b/cpu-exec.c
index 8c9fb8b..e4fe4f8 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -248,13 +248,11 @@ int cpu_exec(CPUState *env1)
     }
 
 #if defined(TARGET_I386)
-    if (!kvm_enabled()) {
-        /* put eflags in CPU temporary format */
-        CC_SRC = env->eflags & (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
-        DF = 1 - (2 * ((env->eflags >> 10) & 1));
-        CC_OP = CC_OP_EFLAGS;
-        env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
-    }
+    /* put eflags in CPU temporary format */
+    CC_SRC = env->eflags & (CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
+    DF = 1 - (2 * ((env->eflags >> 10) & 1));
+    CC_OP = CC_OP_EFLAGS;
+    env->eflags &= ~(DF_MASK | CC_O | CC_S | CC_Z | CC_A | CC_P | CC_C);
 #elif defined(TARGET_SPARC)
 #elif defined(TARGET_M68K)
     env->cc_op = CC_OP_FLAGS;
@@ -279,7 +277,7 @@ int cpu_exec(CPUState *env1)
         if (setjmp(env->jmp_env) == 0) {
 #if defined(__sparc__) && !defined(CONFIG_SOLARIS)
 #undef env
-                    env = cpu_single_env;
+            env = cpu_single_env;
 #define env cpu_single_env
 #endif
             /* if an exception is pending, we execute it here */
@@ -340,11 +338,6 @@ int cpu_exec(CPUState *env1)
                 }
             }
 
-            if (kvm_enabled()) {
-                kvm_cpu_exec(env);
-                longjmp(env->jmp_env, 1);
-            }
-
             next_tb = 0; /* force lookup of first TB */
             for(;;) {
                 interrupt_request = env->interrupt_request;
diff --git a/cpus.c b/cpus.c
index 4a4d130..e4ff314 100644
--- a/cpus.c
+++ b/cpus.c
@@ -728,8 +728,6 @@ static void qemu_kvm_wait_io_event(CPUState *env)
     qemu_wait_io_event_common(env);
 }
 
-static int qemu_cpu_exec(CPUState *env);
-
 static void *qemu_kvm_cpu_thread_fn(void *arg)
 {
     CPUState *env = arg;
@@ -756,7 +754,7 @@ static void *qemu_kvm_cpu_thread_fn(void *arg)
 
     while (1) {
         if (cpu_can_run(env)) {
-            qemu_cpu_exec(env);
+            kvm_cpu_exec(env);
         }
         qemu_kvm_wait_io_event(env);
     }
@@ -959,7 +957,7 @@ void vm_stop(int reason)
 
 #endif
 
-static int qemu_cpu_exec(CPUState *env)
+static int tcg_cpu_exec(CPUState *env)
 {
     int ret;
 #ifdef CONFIG_PROFILER
@@ -1014,9 +1012,11 @@ bool cpu_exec_all(void)
             break;
         }
         if (cpu_can_run(env)) {
-            r = qemu_cpu_exec(env);
             if (kvm_enabled()) {
+                r = kvm_cpu_exec(env);
                 qemu_kvm_eat_signals(env);
+            } else {
+                r = tcg_cpu_exec(env);
             }
             if (r == EXCP_DEBUG) {
                 break;
diff --git a/kvm-all.c b/kvm-all.c
index 0abe088..fd5a137 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -823,10 +823,11 @@ int kvm_cpu_exec(CPUState *env)
 
     if (kvm_arch_process_irqchip_events(env)) {
         env->exit_request = 0;
-        env->exception_index = EXCP_HLT;
-        return 0;
+        return EXCP_HLT;
     }
 
+    cpu_single_env = env;
+
     do {
         if (env->kvm_vcpu_dirty) {
             kvm_arch_put_registers(env, KVM_PUT_RUNTIME_STATE);
@@ -855,7 +856,6 @@ int kvm_cpu_exec(CPUState *env)
         kvm_flush_coalesced_mmio_buffer();
 
         if (ret == -EINTR || ret == -EAGAIN) {
-            cpu_exit(env);
             DPRINTF("io window exit\n");
             ret = 0;
             break;
@@ -906,8 +906,8 @@ int kvm_cpu_exec(CPUState *env)
             DPRINTF("kvm_exit_debug\n");
 #ifdef KVM_CAP_SET_GUEST_DEBUG
             if (kvm_arch_debug(&run->debug.arch)) {
-                env->exception_index = EXCP_DEBUG;
-                return 0;
+                ret = EXCP_DEBUG;
+                goto out;
             }
             /* re-enter, this exception was guest-internal */
             ret = 1;
@@ -923,13 +923,12 @@ int kvm_cpu_exec(CPUState *env)
     if (ret < 0) {
         cpu_dump_state(env, stderr, fprintf, CPU_DUMP_CODE);
         vm_stop(0);
-        env->exit_request = 1;
-    }
-    if (env->exit_request) {
-        env->exit_request = 0;
-        env->exception_index = EXCP_INTERRUPT;
     }
+    ret = EXCP_INTERRUPT;
 
+out:
+    env->exit_request = 0;
+    cpu_single_env = NULL;
     return ret;
 }
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index a7acd53..16dacb8 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1563,12 +1563,13 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 
 int kvm_arch_process_irqchip_events(CPUState *env)
 {
+    if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
+        env->halted = 0;
+    }
     if (env->interrupt_request & CPU_INTERRUPT_INIT) {
         kvm_cpu_synchronize_state(env);
         do_cpu_init(env);
-        env->exception_index = EXCP_HALTED;
     }
-
     if (env->interrupt_request & CPU_INTERRUPT_SIPI) {
         kvm_cpu_synchronize_state(env);
         do_cpu_sipi(env);
@@ -1583,7 +1584,6 @@ static int kvm_handle_halt(CPUState *env)
           (env->eflags & IF_MASK)) &&
         !(env->interrupt_request & CPU_INTERRUPT_NMI)) {
         env->halted = 1;
-        env->exception_index = EXCP_HLT;
         return 0;
     }
 
-- 
1.7.1

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

* [PATCH 17/18] kvm: x86: Prepare VCPU loop for in-kernel irqchip
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:32   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

From: Jan Kiszka <jan.kiszka@siemens.com>

Effectively no functional change yet as kvm_irqchip_in_kernel still only
returns 0, but this patch will allow qemu-kvm to adopt the VCPU loop of
upsteam KVM.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 target-i386/kvm.c |   78 ++++++++++++++++++++++++++++-------------------------
 1 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 16dacb8..da7482f 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1515,35 +1515,38 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
         kvm_vcpu_ioctl(env, KVM_NMI);
     }
 
-    /* Try to inject an interrupt if the guest can accept it */
-    if (run->ready_for_interrupt_injection &&
-        (env->interrupt_request & CPU_INTERRUPT_HARD) &&
-        (env->eflags & IF_MASK)) {
-        int irq;
-
-        env->interrupt_request &= ~CPU_INTERRUPT_HARD;
-        irq = cpu_get_pic_interrupt(env);
-        if (irq >= 0) {
-            struct kvm_interrupt intr;
-            intr.irq = irq;
-            /* FIXME: errors */
-            DPRINTF("injected interrupt %d\n", irq);
-            kvm_vcpu_ioctl(env, KVM_INTERRUPT, &intr);
+    if (!kvm_irqchip_in_kernel()) {
+        /* Try to inject an interrupt if the guest can accept it */
+        if (run->ready_for_interrupt_injection &&
+            (env->interrupt_request & CPU_INTERRUPT_HARD) &&
+            (env->eflags & IF_MASK)) {
+            int irq;
+
+            env->interrupt_request &= ~CPU_INTERRUPT_HARD;
+            irq = cpu_get_pic_interrupt(env);
+            if (irq >= 0) {
+                struct kvm_interrupt intr;
+
+                intr.irq = irq;
+                /* FIXME: errors */
+                DPRINTF("injected interrupt %d\n", irq);
+                kvm_vcpu_ioctl(env, KVM_INTERRUPT, &intr);
+            }
         }
-    }
 
-    /* If we have an interrupt but the guest is not ready to receive an
-     * interrupt, request an interrupt window exit.  This will
-     * cause a return to userspace as soon as the guest is ready to
-     * receive interrupts. */
-    if ((env->interrupt_request & CPU_INTERRUPT_HARD)) {
-        run->request_interrupt_window = 1;
-    } else {
-        run->request_interrupt_window = 0;
-    }
+        /* If we have an interrupt but the guest is not ready to receive an
+         * interrupt, request an interrupt window exit.  This will
+         * cause a return to userspace as soon as the guest is ready to
+         * receive interrupts. */
+        if ((env->interrupt_request & CPU_INTERRUPT_HARD)) {
+            run->request_interrupt_window = 1;
+        } else {
+            run->request_interrupt_window = 0;
+        }
 
-    DPRINTF("setting tpr\n");
-    run->cr8 = cpu_get_apic_tpr(env->apic_state);
+        DPRINTF("setting tpr\n");
+        run->cr8 = cpu_get_apic_tpr(env->apic_state);
+    }
 
     return 0;
 }
@@ -1563,18 +1566,19 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 
 int kvm_arch_process_irqchip_events(CPUState *env)
 {
-    if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
-        env->halted = 0;
-    }
-    if (env->interrupt_request & CPU_INTERRUPT_INIT) {
-        kvm_cpu_synchronize_state(env);
-        do_cpu_init(env);
-    }
-    if (env->interrupt_request & CPU_INTERRUPT_SIPI) {
-        kvm_cpu_synchronize_state(env);
-        do_cpu_sipi(env);
+    if (!kvm_irqchip_in_kernel()) {
+        if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
+            env->halted = 0;
+        }
+        if (env->interrupt_request & CPU_INTERRUPT_INIT) {
+            kvm_cpu_synchronize_state(env);
+            do_cpu_init(env);
+        }
+        if (env->interrupt_request & CPU_INTERRUPT_SIPI) {
+            kvm_cpu_synchronize_state(env);
+            do_cpu_sipi(env);
+        }
     }
-
     return env->halted;
 }
 
-- 
1.7.1


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

* [Qemu-devel] [PATCH 17/18] kvm: x86: Prepare VCPU loop for in-kernel irqchip
@ 2011-01-10  8:32   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

From: Jan Kiszka <jan.kiszka@siemens.com>

Effectively no functional change yet as kvm_irqchip_in_kernel still only
returns 0, but this patch will allow qemu-kvm to adopt the VCPU loop of
upsteam KVM.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 target-i386/kvm.c |   78 ++++++++++++++++++++++++++++-------------------------
 1 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 16dacb8..da7482f 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1515,35 +1515,38 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
         kvm_vcpu_ioctl(env, KVM_NMI);
     }
 
-    /* Try to inject an interrupt if the guest can accept it */
-    if (run->ready_for_interrupt_injection &&
-        (env->interrupt_request & CPU_INTERRUPT_HARD) &&
-        (env->eflags & IF_MASK)) {
-        int irq;
-
-        env->interrupt_request &= ~CPU_INTERRUPT_HARD;
-        irq = cpu_get_pic_interrupt(env);
-        if (irq >= 0) {
-            struct kvm_interrupt intr;
-            intr.irq = irq;
-            /* FIXME: errors */
-            DPRINTF("injected interrupt %d\n", irq);
-            kvm_vcpu_ioctl(env, KVM_INTERRUPT, &intr);
+    if (!kvm_irqchip_in_kernel()) {
+        /* Try to inject an interrupt if the guest can accept it */
+        if (run->ready_for_interrupt_injection &&
+            (env->interrupt_request & CPU_INTERRUPT_HARD) &&
+            (env->eflags & IF_MASK)) {
+            int irq;
+
+            env->interrupt_request &= ~CPU_INTERRUPT_HARD;
+            irq = cpu_get_pic_interrupt(env);
+            if (irq >= 0) {
+                struct kvm_interrupt intr;
+
+                intr.irq = irq;
+                /* FIXME: errors */
+                DPRINTF("injected interrupt %d\n", irq);
+                kvm_vcpu_ioctl(env, KVM_INTERRUPT, &intr);
+            }
         }
-    }
 
-    /* If we have an interrupt but the guest is not ready to receive an
-     * interrupt, request an interrupt window exit.  This will
-     * cause a return to userspace as soon as the guest is ready to
-     * receive interrupts. */
-    if ((env->interrupt_request & CPU_INTERRUPT_HARD)) {
-        run->request_interrupt_window = 1;
-    } else {
-        run->request_interrupt_window = 0;
-    }
+        /* If we have an interrupt but the guest is not ready to receive an
+         * interrupt, request an interrupt window exit.  This will
+         * cause a return to userspace as soon as the guest is ready to
+         * receive interrupts. */
+        if ((env->interrupt_request & CPU_INTERRUPT_HARD)) {
+            run->request_interrupt_window = 1;
+        } else {
+            run->request_interrupt_window = 0;
+        }
 
-    DPRINTF("setting tpr\n");
-    run->cr8 = cpu_get_apic_tpr(env->apic_state);
+        DPRINTF("setting tpr\n");
+        run->cr8 = cpu_get_apic_tpr(env->apic_state);
+    }
 
     return 0;
 }
@@ -1563,18 +1566,19 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 
 int kvm_arch_process_irqchip_events(CPUState *env)
 {
-    if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
-        env->halted = 0;
-    }
-    if (env->interrupt_request & CPU_INTERRUPT_INIT) {
-        kvm_cpu_synchronize_state(env);
-        do_cpu_init(env);
-    }
-    if (env->interrupt_request & CPU_INTERRUPT_SIPI) {
-        kvm_cpu_synchronize_state(env);
-        do_cpu_sipi(env);
+    if (!kvm_irqchip_in_kernel()) {
+        if (env->interrupt_request & (CPU_INTERRUPT_HARD | CPU_INTERRUPT_NMI)) {
+            env->halted = 0;
+        }
+        if (env->interrupt_request & CPU_INTERRUPT_INIT) {
+            kvm_cpu_synchronize_state(env);
+            do_cpu_init(env);
+        }
+        if (env->interrupt_request & CPU_INTERRUPT_SIPI) {
+            kvm_cpu_synchronize_state(env);
+            do_cpu_sipi(env);
+        }
     }
-
     return env->halted;
 }
 
-- 
1.7.1

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

* [PATCH 18/18] kvm: Drop return values from kvm_arch_pre/post_run
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10  8:32   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel, Alexander Graf

From: Jan Kiszka <jan.kiszka@siemens.com>

We do not check them, and the only arch with non-empty implementations
always returns 0 (this is also true for qemu-kvm).

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
CC: Alexander Graf <agraf@suse.de>
---
 kvm.h              |    5 ++---
 target-i386/kvm.c  |    8 ++------
 target-ppc/kvm.c   |    6 ++----
 target-s390x/kvm.c |    6 ++----
 4 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/kvm.h b/kvm.h
index 0a4db28..80c54bc 100644
--- a/kvm.h
+++ b/kvm.h
@@ -95,12 +95,11 @@ int kvm_vcpu_ioctl(CPUState *env, int type, ...);
 
 extern const KVMCapabilityInfo kvm_arch_required_capabilities[];
 
-int kvm_arch_post_run(CPUState *env, struct kvm_run *run);
+void kvm_arch_pre_run(CPUState *env, struct kvm_run *run);
+void kvm_arch_post_run(CPUState *env, struct kvm_run *run);
 
 int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run);
 
-int kvm_arch_pre_run(CPUState *env, struct kvm_run *run);
-
 int kvm_arch_process_irqchip_events(CPUState *env);
 
 int kvm_arch_get_registers(CPUState *env);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index da7482f..214b1bc 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1506,7 +1506,7 @@ int kvm_arch_get_registers(CPUState *env)
     return 0;
 }
 
-int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
+void kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
 {
     /* Inject NMI */
     if (env->interrupt_request & CPU_INTERRUPT_NMI) {
@@ -1547,11 +1547,9 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
         DPRINTF("setting tpr\n");
         run->cr8 = cpu_get_apic_tpr(env->apic_state);
     }
-
-    return 0;
 }
 
-int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
+void kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 {
     if (run->if_flag) {
         env->eflags |= IF_MASK;
@@ -1560,8 +1558,6 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
     }
     cpu_set_apic_tpr(env->apic_state, run->cr8);
     cpu_set_apic_base(env->apic_state, run->apic_base);
-
-    return 0;
 }
 
 int kvm_arch_process_irqchip_events(CPUState *env)
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 8349045..e885f8d 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -256,14 +256,12 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
     return 0;
 }
 
-int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
+void kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 {
-    return 0;
 }
 
-int kvm_arch_process_irqchip_events(CPUState *env)
+void kvm_arch_process_irqchip_events(CPUState *env)
 {
-    return 0;
 }
 
 static int kvmppc_handle_halt(CPUState *env)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index b7f644b..6f316df 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -169,14 +169,12 @@ int kvm_arch_remove_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
     return 0;
 }
 
-int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
+void kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
 {
-    return 0;
 }
 
-int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
+void kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 {
-    return 0;
 }
 
 int kvm_arch_process_irqchip_events(CPUState *env)
-- 
1.7.1


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

* [Qemu-devel] [PATCH 18/18] kvm: Drop return values from kvm_arch_pre/post_run
@ 2011-01-10  8:32   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10  8:32 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm, Alexander Graf

From: Jan Kiszka <jan.kiszka@siemens.com>

We do not check them, and the only arch with non-empty implementations
always returns 0 (this is also true for qemu-kvm).

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
CC: Alexander Graf <agraf@suse.de>
---
 kvm.h              |    5 ++---
 target-i386/kvm.c  |    8 ++------
 target-ppc/kvm.c   |    6 ++----
 target-s390x/kvm.c |    6 ++----
 4 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/kvm.h b/kvm.h
index 0a4db28..80c54bc 100644
--- a/kvm.h
+++ b/kvm.h
@@ -95,12 +95,11 @@ int kvm_vcpu_ioctl(CPUState *env, int type, ...);
 
 extern const KVMCapabilityInfo kvm_arch_required_capabilities[];
 
-int kvm_arch_post_run(CPUState *env, struct kvm_run *run);
+void kvm_arch_pre_run(CPUState *env, struct kvm_run *run);
+void kvm_arch_post_run(CPUState *env, struct kvm_run *run);
 
 int kvm_arch_handle_exit(CPUState *env, struct kvm_run *run);
 
-int kvm_arch_pre_run(CPUState *env, struct kvm_run *run);
-
 int kvm_arch_process_irqchip_events(CPUState *env);
 
 int kvm_arch_get_registers(CPUState *env);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index da7482f..214b1bc 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -1506,7 +1506,7 @@ int kvm_arch_get_registers(CPUState *env)
     return 0;
 }
 
-int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
+void kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
 {
     /* Inject NMI */
     if (env->interrupt_request & CPU_INTERRUPT_NMI) {
@@ -1547,11 +1547,9 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
         DPRINTF("setting tpr\n");
         run->cr8 = cpu_get_apic_tpr(env->apic_state);
     }
-
-    return 0;
 }
 
-int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
+void kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 {
     if (run->if_flag) {
         env->eflags |= IF_MASK;
@@ -1560,8 +1558,6 @@ int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
     }
     cpu_set_apic_tpr(env->apic_state, run->cr8);
     cpu_set_apic_base(env->apic_state, run->apic_base);
-
-    return 0;
 }
 
 int kvm_arch_process_irqchip_events(CPUState *env)
diff --git a/target-ppc/kvm.c b/target-ppc/kvm.c
index 8349045..e885f8d 100644
--- a/target-ppc/kvm.c
+++ b/target-ppc/kvm.c
@@ -256,14 +256,12 @@ int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
     return 0;
 }
 
-int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
+void kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 {
-    return 0;
 }
 
-int kvm_arch_process_irqchip_events(CPUState *env)
+void kvm_arch_process_irqchip_events(CPUState *env)
 {
-    return 0;
 }
 
 static int kvmppc_handle_halt(CPUState *env)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index b7f644b..6f316df 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -169,14 +169,12 @@ int kvm_arch_remove_sw_breakpoint(CPUState *env, struct kvm_sw_breakpoint *bp)
     return 0;
 }
 
-int kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
+void kvm_arch_pre_run(CPUState *env, struct kvm_run *run)
 {
-    return 0;
 }
 
-int kvm_arch_post_run(CPUState *env, struct kvm_run *run)
+void kvm_arch_post_run(CPUState *env, struct kvm_run *run)
 {
-    return 0;
 }
 
 int kvm_arch_process_irqchip_events(CPUState *env)
-- 
1.7.1

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

* Re: [PATCH 00/18] [uq/master] MCE & IO exit fixes, prepare for VCPU loop reuse
  2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10 10:10   ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10 10:10 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, qemu-devel

Am 10.01.2011 09:31, Jan Kiszka wrote:
> This series has three major topics:
>  - add required kernel reentry after IO exits
>  - provide MCE forwarding under !CONFIG_IOTHREAD
>  - prepare kvm_cpu_exec for qemu-kvm reuse
> 
> Along these lines, several cleanups and simplifcations are applied to
> cpus.c and the KVM VCPU execution bits. The first patch should of course
> be dropped if uq/master is applied without "kvm: Drop return value of
> kvm_cpu_exec".
> 
> Note that I did not yet to test the MCE support. Is there an easy way to
> trigger valid MCE events at kernel level? Or do I need to fake SIGBUS at
> qemu level?
> 
> Jan Kiszka (18):
>   Revert "kvm: Drop return value of kvm_cpu_exec"
>   kvm: Drop redundant kvm_enabled from kvm_cpu_thread_fn
>   kvm: Provide sigbus services arch-independently
>   Refactor signal setup functions in cpus.c
>   kvm: Set up signal mask also for !CONFIG_IOTHREAD
>   kvm: Refactor qemu_kvm_eat_signals
>   kvm: Add MCE signal support for !CONFIG_IOTHREAD
>   kvm: Handle kvm_init_vcpu errors

Ugh, I did it again: Forgot to test this "trivial" patch. Promptly, it
turned out to be buggy, and it also revealed some potential issues in
the kvm init code. Need to look into this later. The rest of the series
is unaffected, though.

Jan

>   Refactor kvm&tcg function names in cpus.c
>   Fix a few coding style violations in cpus.c
>   Introduce VCPU self-signaling service
>   kvm: Move irqchip event processing out of inner loop
>   kvm: Unconditionally reenter kernel after IO exits
>   kvm: Remove static return code of kvm_handle_io
>   kvm: Leave kvm_cpu_exec directly after KVM_EXIT_SHUTDOWN
>   kvm: Separate TCG from KVM cpu execution
>   kvm: x86: Prepare VCPU loop for in-kernel irqchip
>   kvm: Drop return values from kvm_arch_pre/post_run
> 
>  cpu-exec.c         |   19 +--
>  cpus.c             |  521 +++++++++++++++++++++++++++++++---------------------
>  kvm-all.c          |   76 +++++----
>  kvm-stub.c         |    9 +-
>  kvm.h              |   14 +-
>  qemu-common.h      |    1 +
>  target-i386/kvm.c  |   90 +++++-----
>  target-ppc/kvm.c   |   16 ++-
>  target-s390x/kvm.c |   16 ++-
>  9 files changed, 444 insertions(+), 318 deletions(-)
> 

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

* [Qemu-devel] Re: [PATCH 00/18] [uq/master] MCE & IO exit fixes, prepare for VCPU loop reuse
@ 2011-01-10 10:10   ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10 10:10 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: qemu-devel, kvm

Am 10.01.2011 09:31, Jan Kiszka wrote:
> This series has three major topics:
>  - add required kernel reentry after IO exits
>  - provide MCE forwarding under !CONFIG_IOTHREAD
>  - prepare kvm_cpu_exec for qemu-kvm reuse
> 
> Along these lines, several cleanups and simplifcations are applied to
> cpus.c and the KVM VCPU execution bits. The first patch should of course
> be dropped if uq/master is applied without "kvm: Drop return value of
> kvm_cpu_exec".
> 
> Note that I did not yet to test the MCE support. Is there an easy way to
> trigger valid MCE events at kernel level? Or do I need to fake SIGBUS at
> qemu level?
> 
> Jan Kiszka (18):
>   Revert "kvm: Drop return value of kvm_cpu_exec"
>   kvm: Drop redundant kvm_enabled from kvm_cpu_thread_fn
>   kvm: Provide sigbus services arch-independently
>   Refactor signal setup functions in cpus.c
>   kvm: Set up signal mask also for !CONFIG_IOTHREAD
>   kvm: Refactor qemu_kvm_eat_signals
>   kvm: Add MCE signal support for !CONFIG_IOTHREAD
>   kvm: Handle kvm_init_vcpu errors

Ugh, I did it again: Forgot to test this "trivial" patch. Promptly, it
turned out to be buggy, and it also revealed some potential issues in
the kvm init code. Need to look into this later. The rest of the series
is unaffected, though.

Jan

>   Refactor kvm&tcg function names in cpus.c
>   Fix a few coding style violations in cpus.c
>   Introduce VCPU self-signaling service
>   kvm: Move irqchip event processing out of inner loop
>   kvm: Unconditionally reenter kernel after IO exits
>   kvm: Remove static return code of kvm_handle_io
>   kvm: Leave kvm_cpu_exec directly after KVM_EXIT_SHUTDOWN
>   kvm: Separate TCG from KVM cpu execution
>   kvm: x86: Prepare VCPU loop for in-kernel irqchip
>   kvm: Drop return values from kvm_arch_pre/post_run
> 
>  cpu-exec.c         |   19 +--
>  cpus.c             |  521 +++++++++++++++++++++++++++++++---------------------
>  kvm-all.c          |   76 +++++----
>  kvm-stub.c         |    9 +-
>  kvm.h              |   14 +-
>  qemu-common.h      |    1 +
>  target-i386/kvm.c  |   90 +++++-----
>  target-ppc/kvm.c   |   16 ++-
>  target-s390x/kvm.c |   16 ++-
>  9 files changed, 444 insertions(+), 318 deletions(-)
> 

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

* Re: [PATCH 03/18] kvm: Provide sigbus services arch-independently
  2011-01-10  8:31   ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10 10:13     ` Paolo Bonzini
  -1 siblings, 0 replies; 60+ messages in thread
From: Paolo Bonzini @ 2011-01-10 10:13 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Avi Kivity, Marcelo Tosatti, kvm, qemu-devel, Huang Ying, Alexander Graf

On 01/10/2011 09:31 AM, Jan Kiszka wrote:
> +int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
> +{
> +    return kvm_on_sigbus_vcpu(env, code, addr);
> +}
> +
> +int kvm_on_sigbus(int code, void *addr)
> +{
> +    return kvm_on_sigbus(code, addr);
> +}

Missing arch_ here?

Paolo

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

* [Qemu-devel] Re: [PATCH 03/18] kvm: Provide sigbus services arch-independently
@ 2011-01-10 10:13     ` Paolo Bonzini
  0 siblings, 0 replies; 60+ messages in thread
From: Paolo Bonzini @ 2011-01-10 10:13 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: kvm, Marcelo Tosatti, Alexander Graf, qemu-devel, Avi Kivity, Huang Ying

On 01/10/2011 09:31 AM, Jan Kiszka wrote:
> +int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
> +{
> +    return kvm_on_sigbus_vcpu(env, code, addr);
> +}
> +
> +int kvm_on_sigbus(int code, void *addr)
> +{
> +    return kvm_on_sigbus(code, addr);
> +}

Missing arch_ here?

Paolo

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

* Re: [PATCH 06/18] kvm: Refactor qemu_kvm_eat_signals
  2011-01-10  8:31   ` [Qemu-devel] " Jan Kiszka
@ 2011-01-10 10:15     ` Paolo Bonzini
  -1 siblings, 0 replies; 60+ messages in thread
From: Paolo Bonzini @ 2011-01-10 10:15 UTC (permalink / raw)
  To: kvm; +Cc: qemu-devel

On 01/10/2011 09:31 AM, Jan Kiszka wrote:
> From: Jan Kiszka<jan.kiszka@siemens.com>
>
> We do not use the timeout, so drop its logic. As we always poll our
> signals, we do not need to drop the global lock. Removing those calls
> allows some further simplifications. Also fix the error processing of
> sigpending at this chance.
>
> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
> ---
>   cpus.c |   23 +++++++----------------
>   1 files changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index bf0fb85..6da0f8f 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -617,31 +617,22 @@ static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
>       }
>   }
>
> -static void qemu_kvm_eat_signal(CPUState *env, int timeout)
> +static void qemu_kvm_eat_signals(CPUState *env)
>   {
> -    struct timespec ts;
> -    int r, e;
> +    struct timespec ts = { 0, 0 };
>       siginfo_t siginfo;
>       sigset_t waitset;
>       sigset_t chkset;
> -
> -    ts.tv_sec = timeout / 1000;
> -    ts.tv_nsec = (timeout % 1000) * 1000000;
> +    int r;
>
>       sigemptyset(&waitset);
>       sigaddset(&waitset, SIG_IPI);
>       sigaddset(&waitset, SIGBUS);
>
>       do {
> -        qemu_mutex_unlock(&qemu_global_mutex);
> -
>           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));
> +        if (r == -1&&  !(errno == EAGAIN || errno == EINTR)) {
> +            perror("sigtimedwait");
>               exit(1);
>           }
>
> @@ -657,7 +648,7 @@ static void qemu_kvm_eat_signal(CPUState *env, int timeout)
>
>           r = sigpending(&chkset);
>           if (r == -1) {
> -            fprintf(stderr, "sigpending: %s\n", strerror(e));
> +            perror("sigpending");
>               exit(1);
>           }
>       } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS));
> @@ -668,7 +659,7 @@ static void qemu_kvm_wait_io_event(CPUState *env)
>       while (!cpu_has_work(env))
>           qemu_cond_timedwait(env->halt_cond,&qemu_global_mutex, 1000);
>
> -    qemu_kvm_eat_signal(env, 0);
> +    qemu_kvm_eat_signals(env);
>       qemu_wait_io_event_common(env);
>   }
>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo


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

* [Qemu-devel] Re: [PATCH 06/18] kvm: Refactor qemu_kvm_eat_signals
@ 2011-01-10 10:15     ` Paolo Bonzini
  0 siblings, 0 replies; 60+ messages in thread
From: Paolo Bonzini @ 2011-01-10 10:15 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Marcelo Tosatti, Avi Kivity, kvm, qemu-devel

On 01/10/2011 09:31 AM, Jan Kiszka wrote:
> From: Jan Kiszka<jan.kiszka@siemens.com>
>
> We do not use the timeout, so drop its logic. As we always poll our
> signals, we do not need to drop the global lock. Removing those calls
> allows some further simplifications. Also fix the error processing of
> sigpending at this chance.
>
> Signed-off-by: Jan Kiszka<jan.kiszka@siemens.com>
> ---
>   cpus.c |   23 +++++++----------------
>   1 files changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index bf0fb85..6da0f8f 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -617,31 +617,22 @@ static void sigbus_handler(int n, struct qemu_signalfd_siginfo *siginfo,
>       }
>   }
>
> -static void qemu_kvm_eat_signal(CPUState *env, int timeout)
> +static void qemu_kvm_eat_signals(CPUState *env)
>   {
> -    struct timespec ts;
> -    int r, e;
> +    struct timespec ts = { 0, 0 };
>       siginfo_t siginfo;
>       sigset_t waitset;
>       sigset_t chkset;
> -
> -    ts.tv_sec = timeout / 1000;
> -    ts.tv_nsec = (timeout % 1000) * 1000000;
> +    int r;
>
>       sigemptyset(&waitset);
>       sigaddset(&waitset, SIG_IPI);
>       sigaddset(&waitset, SIGBUS);
>
>       do {
> -        qemu_mutex_unlock(&qemu_global_mutex);
> -
>           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));
> +        if (r == -1&&  !(errno == EAGAIN || errno == EINTR)) {
> +            perror("sigtimedwait");
>               exit(1);
>           }
>
> @@ -657,7 +648,7 @@ static void qemu_kvm_eat_signal(CPUState *env, int timeout)
>
>           r = sigpending(&chkset);
>           if (r == -1) {
> -            fprintf(stderr, "sigpending: %s\n", strerror(e));
> +            perror("sigpending");
>               exit(1);
>           }
>       } while (sigismember(&chkset, SIG_IPI) || sigismember(&chkset, SIGBUS));
> @@ -668,7 +659,7 @@ static void qemu_kvm_wait_io_event(CPUState *env)
>       while (!cpu_has_work(env))
>           qemu_cond_timedwait(env->halt_cond,&qemu_global_mutex, 1000);
>
> -    qemu_kvm_eat_signal(env, 0);
> +    qemu_kvm_eat_signals(env);
>       qemu_wait_io_event_common(env);
>   }
>

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

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

* Re: [PATCH 03/18] kvm: Provide sigbus services arch-independently
  2011-01-10 10:13     ` [Qemu-devel] " Paolo Bonzini
@ 2011-01-10 10:30       ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10 10:30 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Avi Kivity, Marcelo Tosatti, kvm, qemu-devel, Huang Ying, Alexander Graf

Am 10.01.2011 11:13, Paolo Bonzini wrote:
> On 01/10/2011 09:31 AM, Jan Kiszka wrote:
>> +int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
>> +{
>> +    return kvm_on_sigbus_vcpu(env, code, addr);
>> +}
>> +
>> +int kvm_on_sigbus(int code, void *addr)
>> +{
>> +    return kvm_on_sigbus(code, addr);
>> +}
> 
> Missing arch_ here?

Well, I said that I didn't test those bits... :)

Thanks, will fix.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

* [Qemu-devel] Re: [PATCH 03/18] kvm: Provide sigbus services arch-independently
@ 2011-01-10 10:30       ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-10 10:30 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: kvm, Marcelo Tosatti, Alexander Graf, qemu-devel, Avi Kivity, Huang Ying

Am 10.01.2011 11:13, Paolo Bonzini wrote:
> On 01/10/2011 09:31 AM, Jan Kiszka wrote:
>> +int kvm_on_sigbus_vcpu(CPUState *env, int code, void *addr)
>> +{
>> +    return kvm_on_sigbus_vcpu(env, code, addr);
>> +}
>> +
>> +int kvm_on_sigbus(int code, void *addr)
>> +{
>> +    return kvm_on_sigbus(code, addr);
>> +}
> 
> Missing arch_ here?

Well, I said that I didn't test those bits... :)

Thanks, will fix.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

* Re: [PATCH 07/18] kvm: Add MCE signal support for !CONFIG_IOTHREAD
  2011-01-10  8:32   ` [Qemu-devel] " Jan Kiszka
@ 2011-01-24 11:17     ` Marcelo Tosatti
  -1 siblings, 0 replies; 60+ messages in thread
From: Marcelo Tosatti @ 2011-01-24 11:17 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Avi Kivity, kvm, qemu-devel, Huang Ying

On Mon, Jan 10, 2011 at 09:32:00AM +0100, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Currently, we only configure and process MCE-related SIGBUS events if
> CONFIG_IOTHREAD is enabled. Fix this by factoring out the required
> handler registration and system configuration. Make sure that events
> happening over a VCPU context in non-threaded mode get dispatched as
> VCPU MCEs.
> 
> We also need to call qemu_kvm_eat_signals in non-threaded mode now, so
> move it (unmodified) and add the required Windows stub.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> CC: Huang Ying <ying.huang@intel.com>
> ---
>  cpus.c |  200 +++++++++++++++++++++++++++++++++++++++------------------------
>  1 files changed, 124 insertions(+), 76 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 6da0f8f..b6f1cfb 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -34,9 +34,6 @@
>  
>  #include "cpus.h"
>  #include "compatfd.h"
> -#ifdef CONFIG_LINUX
> -#include <sys/prctl.h>
> -#endif
>  
>  #ifdef SIGRTMIN
>  #define SIG_IPI (SIGRTMIN+4)
> @@ -44,10 +41,24 @@
>  #define SIG_IPI SIGUSR1
>  #endif
>  

> @@ -912,6 +954,8 @@ static int qemu_cpu_exec(CPUState *env)
>  
>  bool cpu_exec_all(void)
>  {
> +    int r;
> +
>      if (next_cpu == NULL)
>          next_cpu = first_cpu;
>      for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
> @@ -923,7 +967,11 @@ bool cpu_exec_all(void)
>          if (qemu_alarm_pending())
>              break;
>          if (cpu_can_run(env)) {
> -            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
> +            r = qemu_cpu_exec(env);
> +            if (kvm_enabled()) {
> +                qemu_kvm_eat_signals(env);
> +            }
> +            if (r == EXCP_DEBUG) {
>                  break;
>              }

SIGBUS should be processed outside of vcpu execution context, think of a
non MCE SIGBUS while vm is stopped. Could use signalfd for that.

But the SIGBUS handler for !IOTHREAD case should not ignore Action
Required, since it might have been generated in vcpu context.


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

* [Qemu-devel] Re: [PATCH 07/18] kvm: Add MCE signal support for !CONFIG_IOTHREAD
@ 2011-01-24 11:17     ` Marcelo Tosatti
  0 siblings, 0 replies; 60+ messages in thread
From: Marcelo Tosatti @ 2011-01-24 11:17 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Huang Ying, Avi Kivity, kvm, qemu-devel

On Mon, Jan 10, 2011 at 09:32:00AM +0100, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Currently, we only configure and process MCE-related SIGBUS events if
> CONFIG_IOTHREAD is enabled. Fix this by factoring out the required
> handler registration and system configuration. Make sure that events
> happening over a VCPU context in non-threaded mode get dispatched as
> VCPU MCEs.
> 
> We also need to call qemu_kvm_eat_signals in non-threaded mode now, so
> move it (unmodified) and add the required Windows stub.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> CC: Huang Ying <ying.huang@intel.com>
> ---
>  cpus.c |  200 +++++++++++++++++++++++++++++++++++++++------------------------
>  1 files changed, 124 insertions(+), 76 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 6da0f8f..b6f1cfb 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -34,9 +34,6 @@
>  
>  #include "cpus.h"
>  #include "compatfd.h"
> -#ifdef CONFIG_LINUX
> -#include <sys/prctl.h>
> -#endif
>  
>  #ifdef SIGRTMIN
>  #define SIG_IPI (SIGRTMIN+4)
> @@ -44,10 +41,24 @@
>  #define SIG_IPI SIGUSR1
>  #endif
>  

> @@ -912,6 +954,8 @@ static int qemu_cpu_exec(CPUState *env)
>  
>  bool cpu_exec_all(void)
>  {
> +    int r;
> +
>      if (next_cpu == NULL)
>          next_cpu = first_cpu;
>      for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
> @@ -923,7 +967,11 @@ bool cpu_exec_all(void)
>          if (qemu_alarm_pending())
>              break;
>          if (cpu_can_run(env)) {
> -            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
> +            r = qemu_cpu_exec(env);
> +            if (kvm_enabled()) {
> +                qemu_kvm_eat_signals(env);
> +            }
> +            if (r == EXCP_DEBUG) {
>                  break;
>              }

SIGBUS should be processed outside of vcpu execution context, think of a
non MCE SIGBUS while vm is stopped. Could use signalfd for that.

But the SIGBUS handler for !IOTHREAD case should not ignore Action
Required, since it might have been generated in vcpu context.

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

* Re: [PATCH 11/18] Introduce VCPU self-signaling service
  2011-01-10  8:32   ` [Qemu-devel] " Jan Kiszka
@ 2011-01-24 11:47     ` Marcelo Tosatti
  -1 siblings, 0 replies; 60+ messages in thread
From: Marcelo Tosatti @ 2011-01-24 11:47 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Avi Kivity, kvm, qemu-devel

On Mon, Jan 10, 2011 at 09:32:04AM +0100, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Introduce qemu_cpu_kick_self to send SIG_IPI to the calling VCPU
> context. First user will be kvm.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

For the updated patch, can't see where thread_kicked is cleared.


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

* [Qemu-devel] Re: [PATCH 11/18] Introduce VCPU self-signaling service
@ 2011-01-24 11:47     ` Marcelo Tosatti
  0 siblings, 0 replies; 60+ messages in thread
From: Marcelo Tosatti @ 2011-01-24 11:47 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Avi Kivity, kvm, qemu-devel

On Mon, Jan 10, 2011 at 09:32:04AM +0100, Jan Kiszka wrote:
> From: Jan Kiszka <jan.kiszka@siemens.com>
> 
> Introduce qemu_cpu_kick_self to send SIG_IPI to the calling VCPU
> context. First user will be kvm.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

For the updated patch, can't see where thread_kicked is cleared.

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

* Re: [PATCH 11/18] Introduce VCPU self-signaling service
  2011-01-24 11:47     ` [Qemu-devel] " Marcelo Tosatti
@ 2011-01-24 12:36       ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-24 12:36 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Avi Kivity, kvm, qemu-devel

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

On 2011-01-24 12:47, Marcelo Tosatti wrote:
> On Mon, Jan 10, 2011 at 09:32:04AM +0100, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Introduce qemu_cpu_kick_self to send SIG_IPI to the calling VCPU
>> context. First user will be kvm.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> For the updated patch, can't see where thread_kicked is cleared.
> 

"Prevent abortion on multiple VCPU kicks", 6 patches earlier (assuming
you are hopefully looking at the patch queue in my git, not some older
postings).

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

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

* [Qemu-devel] Re: [PATCH 11/18] Introduce VCPU self-signaling service
@ 2011-01-24 12:36       ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-24 12:36 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Avi Kivity, kvm, qemu-devel

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

On 2011-01-24 12:47, Marcelo Tosatti wrote:
> On Mon, Jan 10, 2011 at 09:32:04AM +0100, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Introduce qemu_cpu_kick_self to send SIG_IPI to the calling VCPU
>> context. First user will be kvm.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> For the updated patch, can't see where thread_kicked is cleared.
> 

"Prevent abortion on multiple VCPU kicks", 6 patches earlier (assuming
you are hopefully looking at the patch queue in my git, not some older
postings).

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

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

* Re: [PATCH 07/18] kvm: Add MCE signal support for !CONFIG_IOTHREAD
  2011-01-24 11:17     ` [Qemu-devel] " Marcelo Tosatti
@ 2011-01-24 12:36       ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-24 12:36 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Avi Kivity, kvm, qemu-devel, Huang Ying

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

On 2011-01-24 12:17, Marcelo Tosatti wrote:
> On Mon, Jan 10, 2011 at 09:32:00AM +0100, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Currently, we only configure and process MCE-related SIGBUS events if
>> CONFIG_IOTHREAD is enabled. Fix this by factoring out the required
>> handler registration and system configuration. Make sure that events
>> happening over a VCPU context in non-threaded mode get dispatched as
>> VCPU MCEs.
>>
>> We also need to call qemu_kvm_eat_signals in non-threaded mode now, so
>> move it (unmodified) and add the required Windows stub.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> CC: Huang Ying <ying.huang@intel.com>
>> ---
>>  cpus.c |  200 +++++++++++++++++++++++++++++++++++++++------------------------
>>  1 files changed, 124 insertions(+), 76 deletions(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index 6da0f8f..b6f1cfb 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -34,9 +34,6 @@
>>  
>>  #include "cpus.h"
>>  #include "compatfd.h"
>> -#ifdef CONFIG_LINUX
>> -#include <sys/prctl.h>
>> -#endif
>>  
>>  #ifdef SIGRTMIN
>>  #define SIG_IPI (SIGRTMIN+4)
>> @@ -44,10 +41,24 @@
>>  #define SIG_IPI SIGUSR1
>>  #endif
>>  
> 
>> @@ -912,6 +954,8 @@ static int qemu_cpu_exec(CPUState *env)
>>  
>>  bool cpu_exec_all(void)
>>  {
>> +    int r;
>> +
>>      if (next_cpu == NULL)
>>          next_cpu = first_cpu;
>>      for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
>> @@ -923,7 +967,11 @@ bool cpu_exec_all(void)
>>          if (qemu_alarm_pending())
>>              break;
>>          if (cpu_can_run(env)) {
>> -            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
>> +            r = qemu_cpu_exec(env);
>> +            if (kvm_enabled()) {
>> +                qemu_kvm_eat_signals(env);
>> +            }
>> +            if (r == EXCP_DEBUG) {
>>                  break;
>>              }
> 
> SIGBUS should be processed outside of vcpu execution context, think of a
> non MCE SIGBUS while vm is stopped. Could use signalfd for that.

signalfd - that's the missing bit. I was thinking of how to handle
SIGBUS events raised outside the vcpu context. We need to handle them
synchronously, and signalfd should allow this.

> 
> But the SIGBUS handler for !IOTHREAD case should not ignore Action
> Required, since it might have been generated in vcpu context.
> 

Yes, the sigbus handler will require some rework when we actually start
using it for !IOTHREAD.

Will have a look, thanks,
Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

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

* [Qemu-devel] Re: [PATCH 07/18] kvm: Add MCE signal support for !CONFIG_IOTHREAD
@ 2011-01-24 12:36       ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-24 12:36 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Huang Ying, Avi Kivity, kvm, qemu-devel

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

On 2011-01-24 12:17, Marcelo Tosatti wrote:
> On Mon, Jan 10, 2011 at 09:32:00AM +0100, Jan Kiszka wrote:
>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> Currently, we only configure and process MCE-related SIGBUS events if
>> CONFIG_IOTHREAD is enabled. Fix this by factoring out the required
>> handler registration and system configuration. Make sure that events
>> happening over a VCPU context in non-threaded mode get dispatched as
>> VCPU MCEs.
>>
>> We also need to call qemu_kvm_eat_signals in non-threaded mode now, so
>> move it (unmodified) and add the required Windows stub.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> CC: Huang Ying <ying.huang@intel.com>
>> ---
>>  cpus.c |  200 +++++++++++++++++++++++++++++++++++++++------------------------
>>  1 files changed, 124 insertions(+), 76 deletions(-)
>>
>> diff --git a/cpus.c b/cpus.c
>> index 6da0f8f..b6f1cfb 100644
>> --- a/cpus.c
>> +++ b/cpus.c
>> @@ -34,9 +34,6 @@
>>  
>>  #include "cpus.h"
>>  #include "compatfd.h"
>> -#ifdef CONFIG_LINUX
>> -#include <sys/prctl.h>
>> -#endif
>>  
>>  #ifdef SIGRTMIN
>>  #define SIG_IPI (SIGRTMIN+4)
>> @@ -44,10 +41,24 @@
>>  #define SIG_IPI SIGUSR1
>>  #endif
>>  
> 
>> @@ -912,6 +954,8 @@ static int qemu_cpu_exec(CPUState *env)
>>  
>>  bool cpu_exec_all(void)
>>  {
>> +    int r;
>> +
>>      if (next_cpu == NULL)
>>          next_cpu = first_cpu;
>>      for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
>> @@ -923,7 +967,11 @@ bool cpu_exec_all(void)
>>          if (qemu_alarm_pending())
>>              break;
>>          if (cpu_can_run(env)) {
>> -            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
>> +            r = qemu_cpu_exec(env);
>> +            if (kvm_enabled()) {
>> +                qemu_kvm_eat_signals(env);
>> +            }
>> +            if (r == EXCP_DEBUG) {
>>                  break;
>>              }
> 
> SIGBUS should be processed outside of vcpu execution context, think of a
> non MCE SIGBUS while vm is stopped. Could use signalfd for that.

signalfd - that's the missing bit. I was thinking of how to handle
SIGBUS events raised outside the vcpu context. We need to handle them
synchronously, and signalfd should allow this.

> 
> But the SIGBUS handler for !IOTHREAD case should not ignore Action
> Required, since it might have been generated in vcpu context.
> 

Yes, the sigbus handler will require some rework when we actually start
using it for !IOTHREAD.

Will have a look, thanks,
Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

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

* Re: [PATCH 07/18] kvm: Add MCE signal support for !CONFIG_IOTHREAD
  2011-01-24 12:36       ` [Qemu-devel] " Jan Kiszka
@ 2011-01-26  8:09         ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-26  8:09 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Avi Kivity, kvm, qemu-devel, Huang Ying

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

On 2011-01-24 13:36, Jan Kiszka wrote:
> On 2011-01-24 12:17, Marcelo Tosatti wrote:
>> On Mon, Jan 10, 2011 at 09:32:00AM +0100, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> Currently, we only configure and process MCE-related SIGBUS events if
>>> CONFIG_IOTHREAD is enabled. Fix this by factoring out the required
>>> handler registration and system configuration. Make sure that events
>>> happening over a VCPU context in non-threaded mode get dispatched as
>>> VCPU MCEs.
>>>
>>> We also need to call qemu_kvm_eat_signals in non-threaded mode now, so
>>> move it (unmodified) and add the required Windows stub.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> CC: Huang Ying <ying.huang@intel.com>
>>> ---
>>>  cpus.c |  200 +++++++++++++++++++++++++++++++++++++++------------------------
>>>  1 files changed, 124 insertions(+), 76 deletions(-)
>>>
>>> diff --git a/cpus.c b/cpus.c
>>> index 6da0f8f..b6f1cfb 100644
>>> --- a/cpus.c
>>> +++ b/cpus.c
>>> @@ -34,9 +34,6 @@
>>>  
>>>  #include "cpus.h"
>>>  #include "compatfd.h"
>>> -#ifdef CONFIG_LINUX
>>> -#include <sys/prctl.h>
>>> -#endif
>>>  
>>>  #ifdef SIGRTMIN
>>>  #define SIG_IPI (SIGRTMIN+4)
>>> @@ -44,10 +41,24 @@
>>>  #define SIG_IPI SIGUSR1
>>>  #endif
>>>  
>>
>>> @@ -912,6 +954,8 @@ static int qemu_cpu_exec(CPUState *env)
>>>  
>>>  bool cpu_exec_all(void)
>>>  {
>>> +    int r;
>>> +
>>>      if (next_cpu == NULL)
>>>          next_cpu = first_cpu;
>>>      for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
>>> @@ -923,7 +967,11 @@ bool cpu_exec_all(void)
>>>          if (qemu_alarm_pending())
>>>              break;
>>>          if (cpu_can_run(env)) {
>>> -            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
>>> +            r = qemu_cpu_exec(env);
>>> +            if (kvm_enabled()) {
>>> +                qemu_kvm_eat_signals(env);
>>> +            }
>>> +            if (r == EXCP_DEBUG) {
>>>                  break;
>>>              }
>>
>> SIGBUS should be processed outside of vcpu execution context, think of a
>> non MCE SIGBUS while vm is stopped. Could use signalfd for that.
> 
> signalfd - that's the missing bit. I was thinking of how to handle
> SIGBUS events raised outside the vcpu context. We need to handle them
> synchronously, and signalfd should allow this.

This was straightforward. But now I wonder what actually makes this
pattern work. Doesn't the kernel force-inject SIGBUS, i.e. ignores any
blocking? Or does this only apply to BUS_MCEERR_AR?

> 
>>
>> But the SIGBUS handler for !IOTHREAD case should not ignore Action
>> Required, since it might have been generated in vcpu context.
>>
> 
> Yes, the sigbus handler will require some rework when we actually start
> using it for !IOTHREAD.

And this no longer makes sense to me. The current version simply uses
the same sigbus handler for both modes, an that forwards the error code
properly. What did you mean?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

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

* [Qemu-devel] Re: [PATCH 07/18] kvm: Add MCE signal support for !CONFIG_IOTHREAD
@ 2011-01-26  8:09         ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-26  8:09 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Huang Ying, Avi Kivity, kvm, qemu-devel

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

On 2011-01-24 13:36, Jan Kiszka wrote:
> On 2011-01-24 12:17, Marcelo Tosatti wrote:
>> On Mon, Jan 10, 2011 at 09:32:00AM +0100, Jan Kiszka wrote:
>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> Currently, we only configure and process MCE-related SIGBUS events if
>>> CONFIG_IOTHREAD is enabled. Fix this by factoring out the required
>>> handler registration and system configuration. Make sure that events
>>> happening over a VCPU context in non-threaded mode get dispatched as
>>> VCPU MCEs.
>>>
>>> We also need to call qemu_kvm_eat_signals in non-threaded mode now, so
>>> move it (unmodified) and add the required Windows stub.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> CC: Huang Ying <ying.huang@intel.com>
>>> ---
>>>  cpus.c |  200 +++++++++++++++++++++++++++++++++++++++------------------------
>>>  1 files changed, 124 insertions(+), 76 deletions(-)
>>>
>>> diff --git a/cpus.c b/cpus.c
>>> index 6da0f8f..b6f1cfb 100644
>>> --- a/cpus.c
>>> +++ b/cpus.c
>>> @@ -34,9 +34,6 @@
>>>  
>>>  #include "cpus.h"
>>>  #include "compatfd.h"
>>> -#ifdef CONFIG_LINUX
>>> -#include <sys/prctl.h>
>>> -#endif
>>>  
>>>  #ifdef SIGRTMIN
>>>  #define SIG_IPI (SIGRTMIN+4)
>>> @@ -44,10 +41,24 @@
>>>  #define SIG_IPI SIGUSR1
>>>  #endif
>>>  
>>
>>> @@ -912,6 +954,8 @@ static int qemu_cpu_exec(CPUState *env)
>>>  
>>>  bool cpu_exec_all(void)
>>>  {
>>> +    int r;
>>> +
>>>      if (next_cpu == NULL)
>>>          next_cpu = first_cpu;
>>>      for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
>>> @@ -923,7 +967,11 @@ bool cpu_exec_all(void)
>>>          if (qemu_alarm_pending())
>>>              break;
>>>          if (cpu_can_run(env)) {
>>> -            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
>>> +            r = qemu_cpu_exec(env);
>>> +            if (kvm_enabled()) {
>>> +                qemu_kvm_eat_signals(env);
>>> +            }
>>> +            if (r == EXCP_DEBUG) {
>>>                  break;
>>>              }
>>
>> SIGBUS should be processed outside of vcpu execution context, think of a
>> non MCE SIGBUS while vm is stopped. Could use signalfd for that.
> 
> signalfd - that's the missing bit. I was thinking of how to handle
> SIGBUS events raised outside the vcpu context. We need to handle them
> synchronously, and signalfd should allow this.

This was straightforward. But now I wonder what actually makes this
pattern work. Doesn't the kernel force-inject SIGBUS, i.e. ignores any
blocking? Or does this only apply to BUS_MCEERR_AR?

> 
>>
>> But the SIGBUS handler for !IOTHREAD case should not ignore Action
>> Required, since it might have been generated in vcpu context.
>>
> 
> Yes, the sigbus handler will require some rework when we actually start
> using it for !IOTHREAD.

And this no longer makes sense to me. The current version simply uses
the same sigbus handler for both modes, an that forwards the error code
properly. What did you mean?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

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

* Re: [PATCH 07/18] kvm: Add MCE signal support for !CONFIG_IOTHREAD
  2011-01-26  8:09         ` [Qemu-devel] " Jan Kiszka
@ 2011-01-26 12:01           ` Marcelo Tosatti
  -1 siblings, 0 replies; 60+ messages in thread
From: Marcelo Tosatti @ 2011-01-26 12:01 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Avi Kivity, kvm, qemu-devel, Huang Ying

On Wed, Jan 26, 2011 at 09:09:25AM +0100, Jan Kiszka wrote:
> On 2011-01-24 13:36, Jan Kiszka wrote:
> > On 2011-01-24 12:17, Marcelo Tosatti wrote:
> >> On Mon, Jan 10, 2011 at 09:32:00AM +0100, Jan Kiszka wrote:
> >>> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>>
> >>> Currently, we only configure and process MCE-related SIGBUS events if
> >>> CONFIG_IOTHREAD is enabled. Fix this by factoring out the required
> >>> handler registration and system configuration. Make sure that events
> >>> happening over a VCPU context in non-threaded mode get dispatched as
> >>> VCPU MCEs.
> >>>
> >>> We also need to call qemu_kvm_eat_signals in non-threaded mode now, so
> >>> move it (unmodified) and add the required Windows stub.
> >>>
> >>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>> CC: Huang Ying <ying.huang@intel.com>
> >>> ---
> >>>  cpus.c |  200 +++++++++++++++++++++++++++++++++++++++------------------------
> >>>  1 files changed, 124 insertions(+), 76 deletions(-)
> >>>
> >>> diff --git a/cpus.c b/cpus.c
> >>> index 6da0f8f..b6f1cfb 100644
> >>> --- a/cpus.c
> >>> +++ b/cpus.c
> >>> @@ -34,9 +34,6 @@
> >>>  
> >>>  #include "cpus.h"
> >>>  #include "compatfd.h"
> >>> -#ifdef CONFIG_LINUX
> >>> -#include <sys/prctl.h>
> >>> -#endif
> >>>  
> >>>  #ifdef SIGRTMIN
> >>>  #define SIG_IPI (SIGRTMIN+4)
> >>> @@ -44,10 +41,24 @@
> >>>  #define SIG_IPI SIGUSR1
> >>>  #endif
> >>>  
> >>
> >>> @@ -912,6 +954,8 @@ static int qemu_cpu_exec(CPUState *env)
> >>>  
> >>>  bool cpu_exec_all(void)
> >>>  {
> >>> +    int r;
> >>> +
> >>>      if (next_cpu == NULL)
> >>>          next_cpu = first_cpu;
> >>>      for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
> >>> @@ -923,7 +967,11 @@ bool cpu_exec_all(void)
> >>>          if (qemu_alarm_pending())
> >>>              break;
> >>>          if (cpu_can_run(env)) {
> >>> -            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
> >>> +            r = qemu_cpu_exec(env);
> >>> +            if (kvm_enabled()) {
> >>> +                qemu_kvm_eat_signals(env);
> >>> +            }
> >>> +            if (r == EXCP_DEBUG) {
> >>>                  break;
> >>>              }
> >>
> >> SIGBUS should be processed outside of vcpu execution context, think of a
> >> non MCE SIGBUS while vm is stopped. Could use signalfd for that.
> > 
> > signalfd - that's the missing bit. I was thinking of how to handle
> > SIGBUS events raised outside the vcpu context. We need to handle them
> > synchronously, and signalfd should allow this.
> 
> This was straightforward. But now I wonder what actually makes this
> pattern work. Doesn't the kernel force-inject SIGBUS, i.e. ignores any
> blocking? Or does this only apply to BUS_MCEERR_AR?

SIGBUS is only forced if BUS_MCEERR_AR and the poisoned memory was not accessed 
on behalf of the guest (say directly by qemu).

> >> But the SIGBUS handler for !IOTHREAD case should not ignore Action
> >> Required, since it might have been generated in vcpu context.
> >>
> > 
> > Yes, the sigbus handler will require some rework when we actually start
> > using it for !IOTHREAD.
> 
> And this no longer makes sense to me. The current version simply uses
> the same sigbus handler for both modes, an that forwards the error code
> properly. What did you mean?

There are two handlers, kvm_on_sigbus and kvm_on_sigbus_vcpu.
kvm_on_sigbus, the handler for iothread, dies on BUS_MCEERR_AR (which
will be generated if poisoned memory is accessed on behalf of vcpu). It
should be handled with !CONFIG_IOTHREAD.


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

* [Qemu-devel] Re: [PATCH 07/18] kvm: Add MCE signal support for !CONFIG_IOTHREAD
@ 2011-01-26 12:01           ` Marcelo Tosatti
  0 siblings, 0 replies; 60+ messages in thread
From: Marcelo Tosatti @ 2011-01-26 12:01 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Huang Ying, Avi Kivity, kvm, qemu-devel

On Wed, Jan 26, 2011 at 09:09:25AM +0100, Jan Kiszka wrote:
> On 2011-01-24 13:36, Jan Kiszka wrote:
> > On 2011-01-24 12:17, Marcelo Tosatti wrote:
> >> On Mon, Jan 10, 2011 at 09:32:00AM +0100, Jan Kiszka wrote:
> >>> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>>
> >>> Currently, we only configure and process MCE-related SIGBUS events if
> >>> CONFIG_IOTHREAD is enabled. Fix this by factoring out the required
> >>> handler registration and system configuration. Make sure that events
> >>> happening over a VCPU context in non-threaded mode get dispatched as
> >>> VCPU MCEs.
> >>>
> >>> We also need to call qemu_kvm_eat_signals in non-threaded mode now, so
> >>> move it (unmodified) and add the required Windows stub.
> >>>
> >>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>> CC: Huang Ying <ying.huang@intel.com>
> >>> ---
> >>>  cpus.c |  200 +++++++++++++++++++++++++++++++++++++++------------------------
> >>>  1 files changed, 124 insertions(+), 76 deletions(-)
> >>>
> >>> diff --git a/cpus.c b/cpus.c
> >>> index 6da0f8f..b6f1cfb 100644
> >>> --- a/cpus.c
> >>> +++ b/cpus.c
> >>> @@ -34,9 +34,6 @@
> >>>  
> >>>  #include "cpus.h"
> >>>  #include "compatfd.h"
> >>> -#ifdef CONFIG_LINUX
> >>> -#include <sys/prctl.h>
> >>> -#endif
> >>>  
> >>>  #ifdef SIGRTMIN
> >>>  #define SIG_IPI (SIGRTMIN+4)
> >>> @@ -44,10 +41,24 @@
> >>>  #define SIG_IPI SIGUSR1
> >>>  #endif
> >>>  
> >>
> >>> @@ -912,6 +954,8 @@ static int qemu_cpu_exec(CPUState *env)
> >>>  
> >>>  bool cpu_exec_all(void)
> >>>  {
> >>> +    int r;
> >>> +
> >>>      if (next_cpu == NULL)
> >>>          next_cpu = first_cpu;
> >>>      for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
> >>> @@ -923,7 +967,11 @@ bool cpu_exec_all(void)
> >>>          if (qemu_alarm_pending())
> >>>              break;
> >>>          if (cpu_can_run(env)) {
> >>> -            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
> >>> +            r = qemu_cpu_exec(env);
> >>> +            if (kvm_enabled()) {
> >>> +                qemu_kvm_eat_signals(env);
> >>> +            }
> >>> +            if (r == EXCP_DEBUG) {
> >>>                  break;
> >>>              }
> >>
> >> SIGBUS should be processed outside of vcpu execution context, think of a
> >> non MCE SIGBUS while vm is stopped. Could use signalfd for that.
> > 
> > signalfd - that's the missing bit. I was thinking of how to handle
> > SIGBUS events raised outside the vcpu context. We need to handle them
> > synchronously, and signalfd should allow this.
> 
> This was straightforward. But now I wonder what actually makes this
> pattern work. Doesn't the kernel force-inject SIGBUS, i.e. ignores any
> blocking? Or does this only apply to BUS_MCEERR_AR?

SIGBUS is only forced if BUS_MCEERR_AR and the poisoned memory was not accessed 
on behalf of the guest (say directly by qemu).

> >> But the SIGBUS handler for !IOTHREAD case should not ignore Action
> >> Required, since it might have been generated in vcpu context.
> >>
> > 
> > Yes, the sigbus handler will require some rework when we actually start
> > using it for !IOTHREAD.
> 
> And this no longer makes sense to me. The current version simply uses
> the same sigbus handler for both modes, an that forwards the error code
> properly. What did you mean?

There are two handlers, kvm_on_sigbus and kvm_on_sigbus_vcpu.
kvm_on_sigbus, the handler for iothread, dies on BUS_MCEERR_AR (which
will be generated if poisoned memory is accessed on behalf of vcpu). It
should be handled with !CONFIG_IOTHREAD.

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

* Re: [PATCH 07/18] kvm: Add MCE signal support for !CONFIG_IOTHREAD
  2011-01-26 12:01           ` [Qemu-devel] " Marcelo Tosatti
@ 2011-01-26 12:06             ` Jan Kiszka
  -1 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-26 12:06 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Avi Kivity, kvm, qemu-devel, Huang Ying

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

On 2011-01-26 13:01, Marcelo Tosatti wrote:
> On Wed, Jan 26, 2011 at 09:09:25AM +0100, Jan Kiszka wrote:
>> On 2011-01-24 13:36, Jan Kiszka wrote:
>>> On 2011-01-24 12:17, Marcelo Tosatti wrote:
>>>> On Mon, Jan 10, 2011 at 09:32:00AM +0100, Jan Kiszka wrote:
>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>
>>>>> Currently, we only configure and process MCE-related SIGBUS events if
>>>>> CONFIG_IOTHREAD is enabled. Fix this by factoring out the required
>>>>> handler registration and system configuration. Make sure that events
>>>>> happening over a VCPU context in non-threaded mode get dispatched as
>>>>> VCPU MCEs.
>>>>>
>>>>> We also need to call qemu_kvm_eat_signals in non-threaded mode now, so
>>>>> move it (unmodified) and add the required Windows stub.
>>>>>
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> CC: Huang Ying <ying.huang@intel.com>
>>>>> ---
>>>>>  cpus.c |  200 +++++++++++++++++++++++++++++++++++++++------------------------
>>>>>  1 files changed, 124 insertions(+), 76 deletions(-)
>>>>>
>>>>> diff --git a/cpus.c b/cpus.c
>>>>> index 6da0f8f..b6f1cfb 100644
>>>>> --- a/cpus.c
>>>>> +++ b/cpus.c
>>>>> @@ -34,9 +34,6 @@
>>>>>  
>>>>>  #include "cpus.h"
>>>>>  #include "compatfd.h"
>>>>> -#ifdef CONFIG_LINUX
>>>>> -#include <sys/prctl.h>
>>>>> -#endif
>>>>>  
>>>>>  #ifdef SIGRTMIN
>>>>>  #define SIG_IPI (SIGRTMIN+4)
>>>>> @@ -44,10 +41,24 @@
>>>>>  #define SIG_IPI SIGUSR1
>>>>>  #endif
>>>>>  
>>>>
>>>>> @@ -912,6 +954,8 @@ static int qemu_cpu_exec(CPUState *env)
>>>>>  
>>>>>  bool cpu_exec_all(void)
>>>>>  {
>>>>> +    int r;
>>>>> +
>>>>>      if (next_cpu == NULL)
>>>>>          next_cpu = first_cpu;
>>>>>      for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
>>>>> @@ -923,7 +967,11 @@ bool cpu_exec_all(void)
>>>>>          if (qemu_alarm_pending())
>>>>>              break;
>>>>>          if (cpu_can_run(env)) {
>>>>> -            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
>>>>> +            r = qemu_cpu_exec(env);
>>>>> +            if (kvm_enabled()) {
>>>>> +                qemu_kvm_eat_signals(env);
>>>>> +            }
>>>>> +            if (r == EXCP_DEBUG) {
>>>>>                  break;
>>>>>              }
>>>>
>>>> SIGBUS should be processed outside of vcpu execution context, think of a
>>>> non MCE SIGBUS while vm is stopped. Could use signalfd for that.
>>>
>>> signalfd - that's the missing bit. I was thinking of how to handle
>>> SIGBUS events raised outside the vcpu context. We need to handle them
>>> synchronously, and signalfd should allow this.
>>
>> This was straightforward. But now I wonder what actually makes this
>> pattern work. Doesn't the kernel force-inject SIGBUS, i.e. ignores any
>> blocking? Or does this only apply to BUS_MCEERR_AR?
> 
> SIGBUS is only forced if BUS_MCEERR_AR and the poisoned memory was not accessed 
> on behalf of the guest (say directly by qemu).

OK. I didn't find this detail in the kernel code on first glance, only
force_sig(SIGBUS, current).

> 
>>>> But the SIGBUS handler for !IOTHREAD case should not ignore Action
>>>> Required, since it might have been generated in vcpu context.
>>>>
>>>
>>> Yes, the sigbus handler will require some rework when we actually start
>>> using it for !IOTHREAD.
>>
>> And this no longer makes sense to me. The current version simply uses
>> the same sigbus handler for both modes, an that forwards the error code
>> properly. What did you mean?
> 
> There are two handlers, kvm_on_sigbus and kvm_on_sigbus_vcpu.
> kvm_on_sigbus, the handler for iothread, dies on BUS_MCEERR_AR (which
> will be generated if poisoned memory is accessed on behalf of vcpu). It
> should be handled with !CONFIG_IOTHREAD.

Just as with iothread, vcpu-triggered SIGBUS events are processes by
qemu_kvm_eat_signals, not the signal handler.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

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

* [Qemu-devel] Re: [PATCH 07/18] kvm: Add MCE signal support for !CONFIG_IOTHREAD
@ 2011-01-26 12:06             ` Jan Kiszka
  0 siblings, 0 replies; 60+ messages in thread
From: Jan Kiszka @ 2011-01-26 12:06 UTC (permalink / raw)
  To: Marcelo Tosatti; +Cc: Huang Ying, Avi Kivity, kvm, qemu-devel

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

On 2011-01-26 13:01, Marcelo Tosatti wrote:
> On Wed, Jan 26, 2011 at 09:09:25AM +0100, Jan Kiszka wrote:
>> On 2011-01-24 13:36, Jan Kiszka wrote:
>>> On 2011-01-24 12:17, Marcelo Tosatti wrote:
>>>> On Mon, Jan 10, 2011 at 09:32:00AM +0100, Jan Kiszka wrote:
>>>>> From: Jan Kiszka <jan.kiszka@siemens.com>
>>>>>
>>>>> Currently, we only configure and process MCE-related SIGBUS events if
>>>>> CONFIG_IOTHREAD is enabled. Fix this by factoring out the required
>>>>> handler registration and system configuration. Make sure that events
>>>>> happening over a VCPU context in non-threaded mode get dispatched as
>>>>> VCPU MCEs.
>>>>>
>>>>> We also need to call qemu_kvm_eat_signals in non-threaded mode now, so
>>>>> move it (unmodified) and add the required Windows stub.
>>>>>
>>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>>> CC: Huang Ying <ying.huang@intel.com>
>>>>> ---
>>>>>  cpus.c |  200 +++++++++++++++++++++++++++++++++++++++------------------------
>>>>>  1 files changed, 124 insertions(+), 76 deletions(-)
>>>>>
>>>>> diff --git a/cpus.c b/cpus.c
>>>>> index 6da0f8f..b6f1cfb 100644
>>>>> --- a/cpus.c
>>>>> +++ b/cpus.c
>>>>> @@ -34,9 +34,6 @@
>>>>>  
>>>>>  #include "cpus.h"
>>>>>  #include "compatfd.h"
>>>>> -#ifdef CONFIG_LINUX
>>>>> -#include <sys/prctl.h>
>>>>> -#endif
>>>>>  
>>>>>  #ifdef SIGRTMIN
>>>>>  #define SIG_IPI (SIGRTMIN+4)
>>>>> @@ -44,10 +41,24 @@
>>>>>  #define SIG_IPI SIGUSR1
>>>>>  #endif
>>>>>  
>>>>
>>>>> @@ -912,6 +954,8 @@ static int qemu_cpu_exec(CPUState *env)
>>>>>  
>>>>>  bool cpu_exec_all(void)
>>>>>  {
>>>>> +    int r;
>>>>> +
>>>>>      if (next_cpu == NULL)
>>>>>          next_cpu = first_cpu;
>>>>>      for (; next_cpu != NULL && !exit_request; next_cpu = next_cpu->next_cpu) {
>>>>> @@ -923,7 +967,11 @@ bool cpu_exec_all(void)
>>>>>          if (qemu_alarm_pending())
>>>>>              break;
>>>>>          if (cpu_can_run(env)) {
>>>>> -            if (qemu_cpu_exec(env) == EXCP_DEBUG) {
>>>>> +            r = qemu_cpu_exec(env);
>>>>> +            if (kvm_enabled()) {
>>>>> +                qemu_kvm_eat_signals(env);
>>>>> +            }
>>>>> +            if (r == EXCP_DEBUG) {
>>>>>                  break;
>>>>>              }
>>>>
>>>> SIGBUS should be processed outside of vcpu execution context, think of a
>>>> non MCE SIGBUS while vm is stopped. Could use signalfd for that.
>>>
>>> signalfd - that's the missing bit. I was thinking of how to handle
>>> SIGBUS events raised outside the vcpu context. We need to handle them
>>> synchronously, and signalfd should allow this.
>>
>> This was straightforward. But now I wonder what actually makes this
>> pattern work. Doesn't the kernel force-inject SIGBUS, i.e. ignores any
>> blocking? Or does this only apply to BUS_MCEERR_AR?
> 
> SIGBUS is only forced if BUS_MCEERR_AR and the poisoned memory was not accessed 
> on behalf of the guest (say directly by qemu).

OK. I didn't find this detail in the kernel code on first glance, only
force_sig(SIGBUS, current).

> 
>>>> But the SIGBUS handler for !IOTHREAD case should not ignore Action
>>>> Required, since it might have been generated in vcpu context.
>>>>
>>>
>>> Yes, the sigbus handler will require some rework when we actually start
>>> using it for !IOTHREAD.
>>
>> And this no longer makes sense to me. The current version simply uses
>> the same sigbus handler for both modes, an that forwards the error code
>> properly. What did you mean?
> 
> There are two handlers, kvm_on_sigbus and kvm_on_sigbus_vcpu.
> kvm_on_sigbus, the handler for iothread, dies on BUS_MCEERR_AR (which
> will be generated if poisoned memory is accessed on behalf of vcpu). It
> should be handled with !CONFIG_IOTHREAD.

Just as with iothread, vcpu-triggered SIGBUS events are processes by
qemu_kvm_eat_signals, not the signal handler.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]

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

end of thread, other threads:[~2011-01-26 12:06 UTC | newest]

Thread overview: 60+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-10  8:31 [PATCH 00/18] [uq/master] MCE & IO exit fixes, prepare for VCPU loop reuse Jan Kiszka
2011-01-10  8:31 ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:31 ` [PATCH 01/18] Revert "kvm: Drop return value of kvm_cpu_exec" Jan Kiszka
2011-01-10  8:31   ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:31 ` [PATCH 02/18] kvm: Drop redundant kvm_enabled from kvm_cpu_thread_fn Jan Kiszka
2011-01-10  8:31   ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:31 ` [PATCH 03/18] kvm: Provide sigbus services arch-independently Jan Kiszka
2011-01-10  8:31   ` [Qemu-devel] " Jan Kiszka
2011-01-10 10:13   ` Paolo Bonzini
2011-01-10 10:13     ` [Qemu-devel] " Paolo Bonzini
2011-01-10 10:30     ` Jan Kiszka
2011-01-10 10:30       ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:31 ` [PATCH 04/18] Refactor signal setup functions in cpus.c Jan Kiszka
2011-01-10  8:31   ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:31 ` [PATCH 05/18] kvm: Set up signal mask also for !CONFIG_IOTHREAD Jan Kiszka
2011-01-10  8:31   ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:31 ` [PATCH 06/18] kvm: Refactor qemu_kvm_eat_signals Jan Kiszka
2011-01-10  8:31   ` [Qemu-devel] " Jan Kiszka
2011-01-10 10:15   ` Paolo Bonzini
2011-01-10 10:15     ` [Qemu-devel] " Paolo Bonzini
2011-01-10  8:32 ` [PATCH 07/18] kvm: Add MCE signal support for !CONFIG_IOTHREAD Jan Kiszka
2011-01-10  8:32   ` [Qemu-devel] " Jan Kiszka
2011-01-24 11:17   ` Marcelo Tosatti
2011-01-24 11:17     ` [Qemu-devel] " Marcelo Tosatti
2011-01-24 12:36     ` Jan Kiszka
2011-01-24 12:36       ` [Qemu-devel] " Jan Kiszka
2011-01-26  8:09       ` Jan Kiszka
2011-01-26  8:09         ` [Qemu-devel] " Jan Kiszka
2011-01-26 12:01         ` Marcelo Tosatti
2011-01-26 12:01           ` [Qemu-devel] " Marcelo Tosatti
2011-01-26 12:06           ` Jan Kiszka
2011-01-26 12:06             ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:32 ` [PATCH 08/18] kvm: Handle kvm_init_vcpu errors Jan Kiszka
2011-01-10  8:32   ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:32 ` [PATCH 09/18] Refactor kvm&tcg function names in cpus.c Jan Kiszka
2011-01-10  8:32   ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:32 ` [PATCH 10/18] Fix a few coding style violations " Jan Kiszka
2011-01-10  8:32   ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:32 ` [PATCH 11/18] Introduce VCPU self-signaling service Jan Kiszka
2011-01-10  8:32   ` [Qemu-devel] " Jan Kiszka
2011-01-24 11:47   ` Marcelo Tosatti
2011-01-24 11:47     ` [Qemu-devel] " Marcelo Tosatti
2011-01-24 12:36     ` Jan Kiszka
2011-01-24 12:36       ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:32 ` [PATCH 12/18] kvm: Move irqchip event processing out of inner loop Jan Kiszka
2011-01-10  8:32   ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:32 ` [PATCH 13/18] kvm: Unconditionally reenter kernel after IO exits Jan Kiszka
2011-01-10  8:32   ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:32 ` [PATCH 14/18] kvm: Remove static return code of kvm_handle_io Jan Kiszka
2011-01-10  8:32   ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:32 ` [PATCH 15/18] kvm: Leave kvm_cpu_exec directly after KVM_EXIT_SHUTDOWN Jan Kiszka
2011-01-10  8:32   ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:32 ` [PATCH 16/18] kvm: Separate TCG from KVM cpu execution Jan Kiszka
2011-01-10  8:32   ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:32 ` [PATCH 17/18] kvm: x86: Prepare VCPU loop for in-kernel irqchip Jan Kiszka
2011-01-10  8:32   ` [Qemu-devel] " Jan Kiszka
2011-01-10  8:32 ` [PATCH 18/18] kvm: Drop return values from kvm_arch_pre/post_run Jan Kiszka
2011-01-10  8:32   ` [Qemu-devel] " Jan Kiszka
2011-01-10 10:10 ` [PATCH 00/18] [uq/master] MCE & IO exit fixes, prepare for VCPU loop reuse Jan Kiszka
2011-01-10 10:10   ` [Qemu-devel] " Jan Kiszka

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.