Nicholas Piggin <npiggin@gmail.com> writes:
> (H)DEC and PURR get reset before icount does, which causes them to be
> skewed and not match the init state. This can cause replay to not
> match the recorded trace exactly. For DEC and HDEC this is usually not
> noticable since they tend to get programmed before affecting the
> target machine. PURR has been observed to cause replay bugs when
> running Linux.
>
> Fix this by resetting using a time of 0.
>
> Cc: qemu-ppc@nongnu.org
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> hw/ppc/ppc.c | 11 ++++++++---
> target/ppc/machine.c | 4 ++++
> 2 files changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index fadb8f5239..f22321779e 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -1112,16 +1112,21 @@ void cpu_ppc_tb_reset(CPUPPCState *env)
> timer_del(tb_env->hdecr_timer);
> ppc_set_irq(cpu, PPC_INTERRUPT_HDECR, 0);
> tb_env->hdecr_next = 0;
> + _cpu_ppc_store_hdecr(cpu, 0, 0, 0, 64);
> }
>
> /*
> * There is a bug in Linux 2.4 kernels:
> * if a decrementer exception is pending when it enables msr_ee at startup,
> * it's not ready to handle it...
> + *
> + * On machine reset, in this is called before icount is reset, so
> + * for icount-mode, setting TB registers using now=qemu_clock_get_ns
> + * results in them being skewed when icount does get reset. Use an
> + * explicit 0 to get a consistent reset state.
> */
> - cpu_ppc_store_decr(env, -1);
> - cpu_ppc_store_hdecr(env, -1);
> - cpu_ppc_store_purr(env, 0x0000000000000000ULL);
> + _cpu_ppc_store_decr(cpu, 0, 0, -1, 64);
> + _cpu_ppc_store_purr(env, 0, 0);
> }
>
> void cpu_ppc_tb_free(CPUPPCState *env)
> diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> index 203fe28e01..4c4294eafe 100644
> --- a/target/ppc/machine.c
> +++ b/target/ppc/machine.c
> @@ -215,6 +215,8 @@ static int cpu_pre_save(void *opaque)
> * it here.
> */
> env->spr[SPR_DECR] = cpu_ppc_load_decr(env);
> + printf("cpu_ppc_pre_save TB:0x%016lx\n", cpu_ppc_load_tbl(env));
> + printf("cpu_ppc_pre_save DEC:0x%016lx\n",
> cpu_ppc_load_decr(env));
I think this is left over debug, which btw needs proper types:
../../target/ppc/machine.c: In function ‘cpu_pre_save’:
../../target/ppc/machine.c:219:45: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘target_ulong’ {aka ‘unsigned int’} [-Werror=format=]
219 | printf("cpu_ppc_pre_save DEC:0x%016lx\n", cpu_ppc_load_decr(env));
| ~~~~~^ ~~~~~~~~~~~~~~~~~~~~~~
| | |
| | target_ulong {aka unsigned int}
| long unsigned int
| %016x
../../target/ppc/machine.c: In function ‘cpu_post_load’:
../../target/ppc/machine.c:339:45: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 2 has type ‘target_ulong’ {aka ‘unsigned int’} [-Werror=format=]
339 | printf("cpu_ppc_post_ld DEC:0x%016lx\n", cpu_ppc_load_decr(env));
| ~~~~~^ ~~~~~~~~~~~~~~~~~~~~~~
| | |
| | target_ulong {aka unsigned int}
| long unsigned int
| %016x
cc1: all warnings being treated as errors
> }
>
> return 0;
> @@ -333,6 +335,8 @@ static int cpu_post_load(void *opaque, int version_id)
> * triggered types (including HDEC) would need to carry more state.
> */
> cpu_ppc_store_decr(env, env->spr[SPR_DECR]);
> + printf("cpu_ppc_post_ld TB:0x%016lx\n", cpu_ppc_load_tbl(env));
> + printf("cpu_ppc_post_ld DEC:0x%016lx\n", cpu_ppc_load_decr(env));
> pmu_mmcr01_updated(env);
> }
--
Alex Bennée
Virtualisation Tech Lead @ Linaro