Skip to content

Commit

Permalink
hw/ppc: Avoid decrementer rounding errors
Browse files Browse the repository at this point in the history
The decrementer register contains a relative time in timebase units.
When writing to DECR this is converted and stored as an absolute value
in nanosecond units, reading DECR converts back to relative timebase.

The tb<->ns conversion of the relative part can cause rounding such that
a value writen to the decrementer can read back a different, with time
held constant. This is a particular problem for a deterministic icount
and record-replay trace.

Fix this by storing the absolute value in timebase units rather than
nanoseconds. The math before:
  store:  decr_next = now_ns + decr * ns_per_sec / tb_per_sec
  load:        decr = (decr_next - now_ns) * tb_per_sec / ns_per_sec
  load(store): decr = decr * ns_per_sec / tb_per_sec * tb_per_sec /
                      ns_per_sec

After:
  store:  decr_next = now_ns * tb_per_sec / ns_per_sec + decr
  load:        decr = decr_next - now_ns * tb_per_sec / ns_per_sec
  load(store): decr = decr

Fixes: 9fddaa0 ("PowerPC merge: real time TB and decrementer - faster and simpler exception handling (Jocelyn Mayer)")
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Cédric Le Goater <clg@kaod.org>
(cherry picked from commit 8e0a5ac)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
  • Loading branch information
npiggin authored and Michael Tokarev committed Sep 25, 2023
1 parent 73b7a81 commit 0e35c81
Showing 1 changed file with 24 additions and 15 deletions.
39 changes: 24 additions & 15 deletions hw/ppc/ppc.c
Original file line number Diff line number Diff line change
Expand Up @@ -707,16 +707,17 @@ bool ppc_decr_clear_on_delivery(CPUPPCState *env)
static inline int64_t _cpu_ppc_load_decr(CPUPPCState *env, uint64_t next)
{
ppc_tb_t *tb_env = env->tb_env;
int64_t decr, diff;
uint64_t now, n;
int64_t decr;

diff = next - qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
if (diff >= 0) {
decr = ns_to_tb(tb_env->decr_freq, diff);
} else if (tb_env->flags & PPC_TIMER_BOOKE) {
now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
n = ns_to_tb(tb_env->decr_freq, now);
if (next > n && tb_env->flags & PPC_TIMER_BOOKE) {
decr = 0;
} else {
decr = -ns_to_tb(tb_env->decr_freq, -diff);
} else {
decr = next - n;
}

trace_ppc_decr_load(decr);

return decr;
Expand Down Expand Up @@ -853,13 +854,18 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
(*lower_excp)(cpu);
}

/* Calculate the next timer event */
/*
* Calculate the next decrementer event and set a timer.
* decr_next is in timebase units to keep rounding simple. Note it is
* not adjusted by tb_offset because if TB changes via tb_offset changing,
* decrementer does not change, so not directly comparable with TB.
*/
now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
next = now + tb_to_ns_round_up(tb_env->decr_freq, value);
next = ns_to_tb(tb_env->decr_freq, now) + value;
*nextp = next;

/* Adjust timer */
timer_mod(timer, next);
timer_mod(timer, tb_to_ns_round_up(tb_env->decr_freq, next));
}

static inline void _cpu_ppc_store_decr(PowerPCCPU *cpu, target_ulong decr,
Expand Down Expand Up @@ -1172,12 +1178,15 @@ static void start_stop_pit (CPUPPCState *env, ppc_tb_t *tb_env, int is_excp)
} else {
trace_ppc4xx_pit_start(ppc40x_timer->pit_reload);
now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
next = now + tb_to_ns_round_up(tb_env->decr_freq,
ppc40x_timer->pit_reload);
if (is_excp)
next += tb_env->decr_next - now;

if (is_excp) {
tb_env->decr_next += ppc40x_timer->pit_reload;
} else {
tb_env->decr_next = ns_to_tb(tb_env->decr_freq, now)
+ ppc40x_timer->pit_reload;
}
next = tb_to_ns_round_up(tb_env->decr_freq, tb_env->decr_next);
timer_mod(tb_env->decr_timer, next);
tb_env->decr_next = next;
}
}

Expand Down

0 comments on commit 0e35c81

Please sign in to comment.