mirrors/qemu
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix race condition on access to env->interrupt_request

Browse Source 
env->interrupt_request is accessed as the bit level from both main code
and signal handler, making a race condition possible even on CISC CPU.
This causes freeze of QEMU under high load when running the dyntick
clock.

The patch below move the bit corresponding to CPU_INTERRUPT_EXIT in a
separate variable, declared as volatile sig_atomic_t, so it should be
work even on RISC CPU.

We may want to move the cpu_interrupt(env, CPU_INTERRUPT_EXIT) case in
its own function and get rid of CPU_INTERRUPT_EXIT. That can be done
later, I wanted to keep the patch short for easier review.

Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>

git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@6728 c046a42c-6fe2-441c-8c8c-71466251a162
This commit is contained in:
aurel32 2009-03-06 21:48:00 +00:00
parent 28a76be8f4
commit be214e6c05
4 changed files with 19 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
cpu-defs.h
View File

@ -27,6 +27,7 @@
#include "config.h" #include "config.h"
#include <setjmp.h> #include <setjmp.h>
#include <inttypes.h> #include <inttypes.h>
#include <signal.h>
#include "osdep.h" #include "osdep.h"
#include "sys-queue.h" #include "sys-queue.h"
@ -170,6 +171,7 @@ typedef struct CPUWatchpoint {
memory was accessed */ \ memory was accessed */ \
uint32_t halted; /* Nonzero if the CPU is in suspend state */ \ uint32_t halted; /* Nonzero if the CPU is in suspend state */ \
uint32_t interrupt_request; \ uint32_t interrupt_request; \
volatile sig_atomic_t exit_request; \
/* The meaning of the MMU modes is defined in the target code. */ \ /* The meaning of the MMU modes is defined in the target code. */ \
CPUTLBEntry tlb_table[NB_MMU_MODES][CPU_TLB_SIZE]; \ CPUTLBEntry tlb_table[NB_MMU_MODES][CPU_TLB_SIZE]; \
target_phys_addr_t iotlb[NB_MMU_MODES][CPU_TLB_SIZE]; \ target_phys_addr_t iotlb[NB_MMU_MODES][CPU_TLB_SIZE]; \

16
cpu-exec.c
View File

@ -311,7 +311,7 @@ int cpu_exec(CPUState *env1)
env->exception_index = -1; env->exception_index = -1;
} }
#ifdef USE_KQEMU #ifdef USE_KQEMU
if (kqemu_is_ok(env) && env->interrupt_request == 0) { if (kqemu_is_ok(env) && env->interrupt_request == 0 && env->exit_request == 0) {
int ret; int ret;
env->eflags = env->eflags | helper_cc_compute_all(CC_OP) | (DF & DF_MASK); env->eflags = env->eflags | helper_cc_compute_all(CC_OP) | (DF & DF_MASK);
ret = kqemu_cpu_exec(env); ret = kqemu_cpu_exec(env);
@ -326,7 +326,7 @@ int cpu_exec(CPUState *env1)
} else if (ret == 2) { } else if (ret == 2) {
/* softmmu execution needed */ /* softmmu execution needed */
} else { } else {
if (env->interrupt_request != 0) { if (env->interrupt_request != 0 || env->exit_request != 0) {
/* hardware interrupt will be executed just after */ /* hardware interrupt will be executed just after */
} else { } else {
/* otherwise, we restart */ /* otherwise, we restart */
@ -525,11 +525,11 @@ int cpu_exec(CPUState *env1)
the program flow was changed */ the program flow was changed */
next_tb = 0; next_tb = 0;
} }
if (interrupt_request & CPU_INTERRUPT_EXIT) { }
env->interrupt_request &= ~CPU_INTERRUPT_EXIT; if (unlikely(env->exit_request)) {
env->exception_index = EXCP_INTERRUPT; env->exit_request = 0;
cpu_loop_exit(); env->exception_index = EXCP_INTERRUPT;
} cpu_loop_exit();
} }
#ifdef DEBUG_EXEC #ifdef DEBUG_EXEC
if (qemu_loglevel_mask(CPU_LOG_TB_CPU)) { if (qemu_loglevel_mask(CPU_LOG_TB_CPU)) {
@ -599,7 +599,7 @@ int cpu_exec(CPUState *env1)
TB, but before it is linked into a potentially TB, but before it is linked into a potentially
infinite loop and becomes env->current_tb. Avoid infinite loop and becomes env->current_tb. Avoid
starting execution if there is a pending interrupt. */ starting execution if there is a pending interrupt. */
if (unlikely (env->interrupt_request & CPU_INTERRUPT_EXIT)) if (unlikely (env->exit_request))
env->current_tb = NULL; env->current_tb = NULL;
while (env->current_tb) { while (env->current_tb) {

11
exec.c
View File

@ -1501,9 +1501,12 @@ void cpu_interrupt(CPUState *env, int mask)
#endif #endif
int old_mask; int old_mask;
if (mask & CPU_INTERRUPT_EXIT) {
env->exit_request = 1;
mask &= ~CPU_INTERRUPT_EXIT;
}
old_mask = env->interrupt_request; old_mask = env->interrupt_request;
/* FIXME: This is probably not threadsafe. A different thread could
be in the middle of a read-modify-write operation. */
env->interrupt_request |= mask; env->interrupt_request |= mask;
#if defined(USE_NPTL) #if defined(USE_NPTL)
/* FIXME: TB unchaining isn't SMP safe. For now just ignore the /* FIXME: TB unchaining isn't SMP safe. For now just ignore the
@ -1514,10 +1517,8 @@ void cpu_interrupt(CPUState *env, int mask)
if (use_icount) { if (use_icount) {
env->icount_decr.u16.high = 0xffff; env->icount_decr.u16.high = 0xffff;
#ifndef CONFIG_USER_ONLY #ifndef CONFIG_USER_ONLY
/* CPU_INTERRUPT_EXIT isn't a real interrupt. It just means
an async event happened and we need to process it. */
if (!can_do_io(env) if (!can_do_io(env)
&& (mask & ~(old_mask | CPU_INTERRUPT_EXIT)) != 0) { && (mask & ~old_mask) != 0) {
cpu_abort(env, "Raised interrupt while not in I/O function"); cpu_abort(env, "Raised interrupt while not in I/O function");
} }
#endif #endif

6
kvm-all.c
View File

@ -445,7 +445,7 @@ int kvm_cpu_exec(CPUState *env)
do { do {
kvm_arch_pre_run(env, run); kvm_arch_pre_run(env, run);
if ((env->interrupt_request & CPU_INTERRUPT_EXIT)) { if (env->exit_request) {
dprintf("interrupt exit requested\n"); dprintf("interrupt exit requested\n");
ret = 0; ret = 0;
break; break;
@ -512,8 +512,8 @@ int kvm_cpu_exec(CPUState *env)
} }
} while (ret > 0); } while (ret > 0);
if ((env->interrupt_request & CPU_INTERRUPT_EXIT)) { if (env->exit_request) {
env->interrupt_request &= ~CPU_INTERRUPT_EXIT; env->exit_request = 0;
env->exception_index = EXCP_INTERRUPT; env->exception_index = EXCP_INTERRUPT;
} }