212 lines
7.1 KiB
Diff
212 lines
7.1 KiB
Diff
From 9763d20d318638573f7633637f1c5ff96a0e62a3 Mon Sep 17 00:00:00 2001
|
|
From: Peter Maydell <peter.maydell@linaro.org>
|
|
Date: Wed, 5 Oct 2011 10:04:02 +0100
|
|
Subject: [PATCH] Handle CPU interrupts by inline checking of a flag
|
|
|
|
Fix the nasty TCG race conditions and crashes by implementing cpu_exit
|
|
as setting a flag which is checked at the start of each TB. This is
|
|
slightly slower than the attempt to have cpu_exit alter the graph of
|
|
TBs, but it doesn't crash if a thread or signal handler calls cpu_exit
|
|
while the execution thread is itself modifying the TB graph.
|
|
|
|
This version of the patch includes command line option "-no-stopflag"
|
|
which reverts to the previous racy behaviour. This is intended for
|
|
convenience in testing and comparative benchmarking and won't be
|
|
in the final patch.
|
|
|
|
It's probably worth experimenting with whether the flag-testing
|
|
code has the branch in a sense which confuses branch-prediction
|
|
and thus whether flipping it might change performance.
|
|
|
|
Mostly this needs benchmarking to determine what the actual speed
|
|
hit is, which I never got round to. Feel free to do some :-)
|
|
|
|
[AF: CPUState -> CPUArchState]
|
|
---
|
|
cpu-exec.c | 11 ++++++++++-
|
|
exec.c | 14 ++++++++++++--
|
|
gen-icount.h | 16 ++++++++++++++++
|
|
linux-user/main.c | 8 ++++++++
|
|
qemu-options.hx | 9 +++++++++
|
|
vl.c | 5 +++++
|
|
6 files changed, 60 insertions(+), 3 deletions(-)
|
|
|
|
diff --git a/cpu-exec.c b/cpu-exec.c
|
|
index 134b3c4..e90ea29 100644
|
|
--- a/cpu-exec.c
|
|
+++ b/cpu-exec.c
|
|
@@ -599,7 +599,16 @@ int cpu_exec(CPUArchState *env)
|
|
tc_ptr = tb->tc_ptr;
|
|
/* execute the generated code */
|
|
next_tb = tcg_qemu_tb_exec(env, tc_ptr);
|
|
- if ((next_tb & 3) == 2) {
|
|
+ if ((next_tb & 3) == 3) {
|
|
+ /* hit stopflag check */
|
|
+ tb = (TranslationBlock *)(long)(next_tb & ~3);
|
|
+ /* Restore PC. */
|
|
+ cpu_pc_from_tb(env, tb);
|
|
+ next_tb = 0;
|
|
+ env->exit_request = 0;
|
|
+ env->exception_index = EXCP_INTERRUPT;
|
|
+ cpu_loop_exit(env);
|
|
+ } else if ((next_tb & 3) == 2) {
|
|
/* Instruction counter expired. */
|
|
int insns_left;
|
|
tb = (TranslationBlock *)(next_tb & ~3);
|
|
diff --git a/exec.c b/exec.c
|
|
index 5834766..9cbdc35 100644
|
|
--- a/exec.c
|
|
+++ b/exec.c
|
|
@@ -130,6 +130,8 @@ DEFINE_TLS(CPUArchState *,cpu_single_env);
|
|
1 = Precise instruction counting.
|
|
2 = Adaptive rate instruction counting. */
|
|
int use_icount = 0;
|
|
+/* 1 to do cpu_exit by inline flag check rather than tb link breaking */
|
|
+int use_stopflag = 1;
|
|
|
|
typedef struct PageDesc {
|
|
/* list of TBs intersecting this ram page */
|
|
@@ -1708,7 +1710,13 @@ static void tcg_handle_interrupt(CPUArchState *env, int mask)
|
|
cpu_abort(env, "Raised interrupt while not in I/O function");
|
|
}
|
|
} else {
|
|
- cpu_unlink_tb(env);
|
|
+ // XXX just call cpu_exit ?
|
|
+ if (use_stopflag) {
|
|
+ // XXX is this OK?
|
|
+ env->exit_request = 1;
|
|
+ } else {
|
|
+ cpu_unlink_tb(env);
|
|
+ }
|
|
}
|
|
}
|
|
|
|
@@ -1731,7 +1739,9 @@ void cpu_reset_interrupt(CPUArchState *env, int mask)
|
|
void cpu_exit(CPUArchState *env)
|
|
{
|
|
env->exit_request = 1;
|
|
- cpu_unlink_tb(env);
|
|
+ if (!use_stopflag) {
|
|
+ cpu_unlink_tb(env);
|
|
+ }
|
|
}
|
|
|
|
void cpu_abort(CPUArchState *env, const char *fmt, ...)
|
|
diff --git a/gen-icount.h b/gen-icount.h
|
|
index 430cb44..65a75d9 100644
|
|
--- a/gen-icount.h
|
|
+++ b/gen-icount.h
|
|
@@ -2,13 +2,25 @@
|
|
|
|
/* Helpers for instruction counting code generation. */
|
|
|
|
+extern int use_stopflag;
|
|
+
|
|
static TCGArg *icount_arg;
|
|
static int icount_label;
|
|
+static int stopflag_label;
|
|
|
|
static inline void gen_icount_start(void)
|
|
{
|
|
TCGv_i32 count;
|
|
|
|
+ if (use_stopflag) {
|
|
+ TCGv_i32 flag;
|
|
+ stopflag_label = gen_new_label();
|
|
+ flag = tcg_temp_local_new_i32();
|
|
+ tcg_gen_ld_i32(flag, cpu_env, offsetof(CPUArchState, exit_request));
|
|
+ tcg_gen_brcondi_i32(TCG_COND_NE, flag, 0, stopflag_label);
|
|
+ tcg_temp_free_i32(flag);
|
|
+ }
|
|
+
|
|
if (!use_icount)
|
|
return;
|
|
|
|
@@ -26,6 +38,10 @@ static inline void gen_icount_start(void)
|
|
|
|
static void gen_icount_end(TranslationBlock *tb, int num_insns)
|
|
{
|
|
+ if (use_stopflag) {
|
|
+ gen_set_label(stopflag_label);
|
|
+ tcg_gen_exit_tb((long)tb + 3); // XXX
|
|
+ }
|
|
if (use_icount) {
|
|
*icount_arg = num_insns;
|
|
gen_set_label(icount_label);
|
|
diff --git a/linux-user/main.c b/linux-user/main.c
|
|
index 7dea084..d9814bc 100644
|
|
--- a/linux-user/main.c
|
|
+++ b/linux-user/main.c
|
|
@@ -64,6 +64,7 @@ unsigned long reserved_va;
|
|
#endif
|
|
|
|
static void usage(void);
|
|
+extern int use_stopflag;
|
|
|
|
static const char *interp_prefix = CONFIG_QEMU_INTERP_PREFIX;
|
|
const char *qemu_uname_release = CONFIG_UNAME_RELEASE;
|
|
@@ -3196,6 +3197,11 @@ static void handle_arg_reserved_va(const char *arg)
|
|
}
|
|
#endif
|
|
|
|
+static void handle_arg_nostopflag(const char *arg)
|
|
+{
|
|
+ use_stopflag = 0;
|
|
+}
|
|
+
|
|
static void handle_arg_singlestep(const char *arg)
|
|
{
|
|
singlestep = 1;
|
|
@@ -3251,6 +3257,8 @@ struct qemu_argument arg_table[] = {
|
|
"options", "activate log"},
|
|
{"D", "QEMU_LOG_FILENAME", true, handle_arg_log_filename,
|
|
"logfile", "override default logfile location"},
|
|
+ {"no-stopflag", "QEMU_NOSTOPFLAG", false, handle_arg_nostopflag,
|
|
+ "", "run in singlestep mode"},
|
|
{"p", "QEMU_PAGESIZE", true, handle_arg_pagesize,
|
|
"pagesize", "set the host page size to 'pagesize'"},
|
|
{"singlestep", "QEMU_SINGLESTEP", false, handle_arg_singlestep,
|
|
diff --git a/qemu-options.hx b/qemu-options.hx
|
|
index 3c411c4..af614bf 100644
|
|
--- a/qemu-options.hx
|
|
+++ b/qemu-options.hx
|
|
@@ -1196,6 +1196,15 @@ STEXI
|
|
Disable HPET support.
|
|
ETEXI
|
|
|
|
+DEF("no-stopflag", 0, QEMU_OPTION_no_stopflag,
|
|
+ "-no-stopflag use old behaviour, not inline stopflag checks\n", QEMU_ARCH_ALL)
|
|
+STEXI
|
|
+@item -no-stopflag
|
|
+@findex -no-stopflag
|
|
+Implement cpu-exit by the old tb link breaking method rather than inline checks
|
|
+(this is slightly faster but racy!)
|
|
+ETEXI
|
|
+
|
|
DEF("acpitable", HAS_ARG, QEMU_OPTION_acpitable,
|
|
"-acpitable [sig=str][,rev=n][,oem_id=str][,oem_table_id=str][,oem_rev=n][,asl_compiler_id=str][,asl_compiler_rev=n][,{data|file}=file1[:file2]...]\n"
|
|
" ACPI table description\n", QEMU_ARCH_I386)
|
|
diff --git a/vl.c b/vl.c
|
|
index 67f5813..322fea0 100644
|
|
--- a/vl.c
|
|
+++ b/vl.c
|
|
@@ -176,6 +176,8 @@ int main(int argc, char **argv)
|
|
|
|
#define MAX_VIRTIO_CONSOLES 1
|
|
|
|
+extern int use_stopflag;
|
|
+
|
|
static const char *data_dir;
|
|
const char *bios_name = NULL;
|
|
enum vga_retrace_method vga_retrace_method = VGA_RETRACE_DUMB;
|
|
@@ -3038,6 +3040,9 @@ int main(int argc, char **argv, char **envp)
|
|
qdev_prop_register_global_list(slew_lost_ticks);
|
|
break;
|
|
}
|
|
+ case QEMU_OPTION_no_stopflag:
|
|
+ use_stopflag = 0;
|
|
+ break;
|
|
case QEMU_OPTION_acpitable:
|
|
do_acpitable_option(optarg);
|
|
break;
|