On 08/01/16 02:47, Alexey Kardashevskiy wrote:
On 01/07/2016 05:22 AM, Mark Cave-Ayland wrote:
During local testing with TCG, intermittent errors were found when
trying to
migrate Darwin OS images.
The underlying cause was that Darwin resets the decrementer value to
fairly
small values on each interrupt. cpu_ppc_set_tb_clk() sets the default
value
of the decrementer to 0xffffffff during initialisation which typically
corresponds to several seconds. Hence when restoring the image, the guest
would effectively "lose" decrementer interrupts during this time causing
confusion in the guest.
NOTE: there does seem to be some overlap here with the
vmstate_ppc_timebase
code, however it doesn't seem to handle multiple CPUs which is why
I've gone
for an independent implementation.
It does handle multiple CPUs:
static int timebase_post_load(void *opaque, int version_id)
{
...
/* Set new offset to all CPUs */
CPU_FOREACH(cpu) {
PowerPCCPU *pcpu = POWERPC_CPU(cpu);
pcpu->env.tb_env->tb_offset = tb_off_adj;
}
It does not transfer DECR though, it transfers the offset instead, one
for all CPUs.
The easier solution would be just adding this instead of the whole patch:
spr_register(env, SPR_DECR, "DECR",
SPR_NOACCESS, SPR_NOACCESS,
&spr_read_decr, &spr_write_decr,
0x00000000);
somewhere in target-ppc/translate_init.c for CPUs you want to support,
gen_tbl() seems to be the right place for this. As long as it is just
spr_register() and not spr_register_kvm(), I suppose it should not break
KVM and pseries.
I've just tried adding that but it then gives the following error on
startup:
Error: Trying to register SPR 22 (016) twice !
Based upon this, the existing registration seems fine. I think the
problem is that I can't see anything in __cpu_ppc_store_decr() that
updates the spr[SPR_DECR] value when the decrementer register is
changed, so it needs to be explicitly added to
cpu_pre_save()/cpu_post_load():
diff --git a/target-ppc/machine.c b/target-ppc/machine.c
index 251a84b..495e58d 100644
--- a/target-ppc/machine.c
+++ b/target-ppc/machine.c
@@ -141,6 +141,7 @@ static void cpu_pre_save(void *opaque)
env->spr[SPR_CFAR] = env->cfar;
#endif
env->spr[SPR_BOOKE_SPEFSCR] = env->spe_fscr;
+ env->spr[SPR_DECR] = cpu_ppc_load_decr(env);
for (i = 0; (i < 4) && (i < env->nb_BATs); i++) {
env->spr[SPR_DBAT0U + 2*i] = env->DBAT[0][i];
@@ -175,6 +176,7 @@ static int cpu_post_load(void *opaque, int version_id)
env->cfar = env->spr[SPR_CFAR];
#endif
env->spe_fscr = env->spr[SPR_BOOKE_SPEFSCR];
+ cpu_ppc_store_decr(env, env->spr[SPR_DECR]);
for (i = 0; (i < 4) && (i < env->nb_BATs); i++) {
env->DBAT[0][i] = env->spr[SPR_DBAT0U + 2*i];
I've just tried the diff above instead of my original version and
repeated my savevm/loadvm pair test with a Darwin installation and it
also fixes the random hang I was seeing on loadvm.
Seemingly this should work on KVM in that cpu_ppc_load_decr() and
cpu_ppc_store_decr() become no-ops as long as KVM maintains
env->spr[SPR_DECR], but a second set of eyeballs would be useful here.
If you can let me know if this is suitable then I'll update the patchset
based upon your feedback and send out a v2.