In kvm mode, vm_clock may be read outside BQL. This will make timers_state --the foundation of vm_clock exposed to race condition. Using private lock to protect it.
Note in tcg mode, vm_clock still read inside BQL, so icount is left without change. As for cpu_ticks in timers_state, it is still protected by BQL. Lock rule: private lock innermost, ie BQL->"this lock" Signed-off-by: Liu Ping Fan <pingf...@linux.vnet.ibm.com> --- cpus.c | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/cpus.c b/cpus.c index 85e743d..ab92db9 100644 --- a/cpus.c +++ b/cpus.c @@ -107,12 +107,17 @@ static int64_t qemu_icount; typedef struct TimersState { int64_t cpu_ticks_prev; int64_t cpu_ticks_offset; + /* QemuClock will be read out of BQL, so protect is with private lock. + * As for cpu_ticks, no requirement to read it outside BQL. + * Lock rule: innermost + */ + QemuSeqLock clock_seqlock; int64_t cpu_clock_offset; int32_t cpu_ticks_enabled; int64_t dummy; } TimersState; -TimersState timers_state; +static TimersState timers_state; /* Return the virtual CPU time, based on the instruction counter. */ int64_t cpu_get_icount(void) @@ -132,6 +137,7 @@ int64_t cpu_get_icount(void) } /* return the host CPU cycle counter and handle stop/restart */ +/* cpu_ticks is safely if holding BQL */ int64_t cpu_get_ticks(void) { if (use_icount) { @@ -156,33 +162,46 @@ int64_t cpu_get_ticks(void) int64_t cpu_get_clock(void) { int64_t ti; - if (!timers_state.cpu_ticks_enabled) { - return timers_state.cpu_clock_offset; - } else { - ti = get_clock(); - return ti + timers_state.cpu_clock_offset; - } + unsigned start; + + do { + start = seqlock_read_begin(&timers_state.clock_seqlock); + if (!timers_state.cpu_ticks_enabled) { + ti = timers_state.cpu_clock_offset; + } else { + ti = get_clock(); + ti += timers_state.cpu_clock_offset; + } + } while (seqlock_read_check(&timers_state.clock_seqlock, start); + + return ti; } /* enable cpu_get_ticks() */ void cpu_enable_ticks(void) { + /* Here, the really thing protected by seqlock is cpu_clock. */ + seqlock_write_lock(&timers_state.clock_seqlock); if (!timers_state.cpu_ticks_enabled) { timers_state.cpu_ticks_offset -= cpu_get_real_ticks(); timers_state.cpu_clock_offset -= get_clock(); timers_state.cpu_ticks_enabled = 1; } + seqlock_write_unlock(&timers_state.clock_seqlock); } /* disable cpu_get_ticks() : the clock is stopped. You must not call cpu_get_ticks() after that. */ void cpu_disable_ticks(void) { + /* Here, the really thing protected by seqlock is cpu_clock. */ + seqlock_write_lock(&timers_state.clock_seqlock); if (timers_state.cpu_ticks_enabled) { timers_state.cpu_ticks_offset = cpu_get_ticks(); timers_state.cpu_clock_offset = cpu_get_clock(); timers_state.cpu_ticks_enabled = 0; } + seqlock_write_unlock(&timers_state.clock_seqlock); } /* Correlation between real and virtual time is always going to be @@ -364,6 +383,9 @@ static const VMStateDescription vmstate_timers = { void configure_icount(const char *option) { + QemuMutex *mutex = g_malloc0(sizeof(QemuMutex)); + qemu_mutex_init(mutex); + seqlock_init(&timers_state.clock_seqlock, mutex); vmstate_register(NULL, 0, &vmstate_timers, &timers_state); if (!option) { return; -- 1.8.1.4