One issue is reverse-step on one gdb thread (vCPU) only steps back one
icount, so if another thread is currently running then it is that one
which goes back one instruction and the selected thread doesn't move. I
would call this a separate issue from the record-replay mechanism, which
is in the replay-debugging policy. I think we could record in each vCPU
an icount of the last instruction it executed before switching, then
reverse step for that vCPU could replay to there. I think that's not so
important yet until this mechanism is solid. But if you test and rsi is
not going backwards, then check your other threads.
Thanks,
Nick
accel/tcg/tcg-accel-ops-icount.c | 9 +++-
accel/tcg/tcg-accel-ops-rr.c | 73 +++++++++++++++++++++++++++++---
include/exec/replay-core.h | 3 ++
replay/replay-internal.h | 1 +
replay/replay.c | 34 ++++++++++++++-
scripts/replay-dump.py | 5 +++
softmmu/vl.c | 4 --
7 files changed, 115 insertions(+), 14 deletions(-)
diff --git a/accel/tcg/tcg-accel-ops-icount.c b/accel/tcg/tcg-accel-ops-icount.c
index 3d2cfbbc97..c26782a56a 100644
--- a/accel/tcg/tcg-accel-ops-icount.c
+++ b/accel/tcg/tcg-accel-ops-icount.c
@@ -93,10 +93,15 @@ void icount_handle_deadline(void)
int64_t icount_percpu_budget(int cpu_count)
{
int64_t limit = icount_get_limit();
- int64_t timeslice = limit / cpu_count;
+ int64_t timeslice;
- if (timeslice == 0) {
+ if (replay_mode == REPLAY_MODE_PLAY) {
timeslice = limit;
+ } else {
+ timeslice = limit / cpu_count;
+ if (timeslice == 0) {
+ timeslice = limit;
+ }
}
return timeslice;
diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index 212d6f8df4..ce040a687e 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -27,6 +27,7 @@
#include "qemu/lockable.h"
#include "sysemu/tcg.h"
#include "sysemu/replay.h"
+#include "sysemu/reset.h"
#include "sysemu/cpu-timers.h"
#include "qemu/main-loop.h"
#include "qemu/notify.h"
@@ -61,6 +62,22 @@ void rr_kick_vcpu_thread(CPUState *unused)
static QEMUTimer *rr_kick_vcpu_timer;
static CPUState *rr_current_cpu;
+static CPUState *rr_next_cpu;
+static CPUState *rr_last_cpu;
+
+/*
+ * Reset the vCPU scheduler to the initial state.
+ */
+static void record_replay_reset(void *param)
+{
+ if (rr_kick_vcpu_timer) {
+ timer_del(rr_kick_vcpu_timer);
+ }
+ g_assert(!rr_current_cpu);
+ rr_next_cpu = NULL;
+ rr_last_cpu = first_cpu;
+ current_cpu = NULL;
+}
static inline int64_t rr_next_kick_time(void)
{
@@ -184,6 +201,8 @@ static void *rr_cpu_thread_fn(void *arg)
Notifier force_rcu;
CPUState *cpu = arg;
+ qemu_register_reset(record_replay_reset, NULL);
+
assert(tcg_enabled());
rcu_register_thread();
force_rcu.notify = rr_force_rcu;
@@ -238,14 +257,20 @@ static void *rr_cpu_thread_fn(void *arg)
cpu_budget = icount_percpu_budget(cpu_count);
}
+ if (!rr_next_cpu) {
+ qatomic_set_mb(&rr_next_cpu, first_cpu);
+ }
+ cpu = rr_next_cpu;
+
+ if (cpu != rr_last_cpu) {
+ replay_switch_cpu();
+ qatomic_set_mb(&rr_last_cpu, cpu);
+ }
+
rr_start_kick_timer();
replay_mutex_unlock();
- if (!cpu) {
- cpu = first_cpu;
- }
-
while (cpu && cpu_work_list_empty(cpu) && !cpu->exit_request) {
/* Store rr_current_cpu before evaluating cpu_can_run(). */
qatomic_set_mb(&rr_current_cpu, cpu);
@@ -284,7 +309,34 @@ static void *rr_cpu_thread_fn(void *arg)
break;
}
- cpu = CPU_NEXT(cpu);
+ if (replay_mode == REPLAY_MODE_NONE) {
+ cpu = CPU_NEXT(cpu);
+ } else if (replay_mode == REPLAY_MODE_RECORD) {
+ /*
+ * Exit the loop immediately so CPU switch events can be
+ * recorded. This may be able to be improved to record
+ * switch events here.
+ */
+ cpu = CPU_NEXT(cpu);
+ break;
+ } else if (replay_mode == REPLAY_MODE_PLAY) {
+ /*
+ * Play can exit from tcg_cpus_exec at different times than
+ * record, because icount budget is set to next non-insn
+ * event which could be an exception or something that
+ * tcg_cpus_exec can process internally if icount limit was
+ * not reached. So we don't necessarily switch CPU here,
+ * only if there is a switch in the trace.
+ */
+ qemu_mutex_unlock_iothread();
+ replay_mutex_lock();
+ qemu_mutex_lock_iothread();
+ if (replay_has_switch_cpu()) {
+ cpu = CPU_NEXT(cpu);
+ }
+ replay_mutex_unlock();
+ break;
+ }
} /* while (cpu && !cpu->exit_request).. */
/* Does not need a memory barrier because a spurious wakeup is okay. */
@@ -294,9 +346,9 @@ static void *rr_cpu_thread_fn(void *arg)
qatomic_set_mb(&cpu->exit_request, 0);
}
- if (all_cpu_threads_idle()) {
- rr_stop_kick_timer();
+ qatomic_set(&rr_next_cpu, cpu);
+ if (all_cpu_threads_idle()) {
if (icount_enabled()) {
/*
* When all cpus are sleeping (e.g in WFI), to avoid a deadlock
@@ -304,6 +356,13 @@ static void *rr_cpu_thread_fn(void *arg)
* timer.
*/
qemu_notify_event();
+ } else {
+ /*
+ * icount timer won't expire when not executing instructions
+ * so no need to stop it. Avoiding restarts is also important
+ * for record-replay to avoid clock events.
+ */
+ rr_stop_kick_timer();
}
}
diff --git a/include/exec/replay-core.h b/include/exec/replay-core.h
index 244c77acce..543c129a1d 100644
--- a/include/exec/replay-core.h
+++ b/include/exec/replay-core.h
@@ -52,6 +52,9 @@ void replay_gdb_attached(void);
/* Interrupts and exceptions */
+bool replay_switch_cpu(void);
+bool replay_has_switch_cpu(void);
+
/* Called by exception handler to write or read exception processing events */
bool replay_exception(void);
/*
diff --git a/replay/replay-internal.h b/replay/replay-internal.h
index b6836354ac..95849e7461 100644
--- a/replay/replay-internal.h
+++ b/replay/replay-internal.h
@@ -58,6 +58,7 @@ enum ReplayEvents {
/* some of greater codes are reserved for checkpoints */
EVENT_CHECKPOINT,
EVENT_CHECKPOINT_LAST = EVENT_CHECKPOINT + CHECKPOINT_COUNT - 1,
+ EVENT_SWITCH_CPU,
/* end of log event */
EVENT_END,
EVENT_COUNT
diff --git a/replay/replay.c b/replay/replay.c
index 0f7d766efe..e2d77ab644 100644
--- a/replay/replay.c
+++ b/replay/replay.c
@@ -98,9 +98,41 @@ void replay_account_executed_instructions(void)
}
}
-bool replay_exception(void)
+bool replay_switch_cpu(void)
+{
+ if (replay_mode == REPLAY_MODE_RECORD) {
+ g_assert(replay_mutex_locked());
+ replay_save_instructions();
+ replay_put_event(EVENT_SWITCH_CPU);
+ return true;
+ } else if (replay_mode == REPLAY_MODE_PLAY) {
+ bool res = replay_has_switch_cpu();
+ if (res) {
+ replay_finish_event();
+ } else {
+ g_assert_not_reached();
+ }
+ return res;
+ }
+
+ return true;
+}
+
+bool replay_has_switch_cpu(void)
{
+ bool res = false;
+ if (replay_mode == REPLAY_MODE_PLAY) {
+ g_assert(replay_mutex_locked());
+ replay_account_executed_instructions();
+ res = replay_next_event_is(EVENT_SWITCH_CPU);
+ }
+
+ return res;
+}
+
+bool replay_exception(void)
+{
if (replay_mode == REPLAY_MODE_RECORD) {
g_assert(replay_mutex_locked());
replay_save_instructions();
diff --git a/softmmu/vl.c b/softmmu/vl.c
index b0b96f67fa..946b22d1de 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -1869,10 +1869,6 @@ static void qemu_apply_machine_options(QDict *qdict)
/* fall back to the -kernel/-append */
semihosting_arg_fallback(current_machine->kernel_filename,
current_machine->kernel_cmdline);
}
-
- if (current_machine->smp.cpus > 1) {
- replay_add_blocker("smp");
- }
}
static void qemu_create_early_backends(void)