Re: [Qemu-ppc] [PULL 095/107] spapr: clock should count only if vm is running

[Laurent Vivier]

On 13/12/2017 20:19, Alexander Graf wrote:
> 
> 
> On 02.02.17 06:14, David Gibson wrote:
>> From: Laurent Vivier <address@hidden>
>>
>> This is a port to ppc of the i386 commit:
>>     00f4d64 kvmclock: clock should count only if vm is running
>>
>> We remove timebase_post_load function, and use the VM state
>> change handler to save and restore the guest_timebase (on stop
>> and continue).
>>
>> We keep timebase_pre_save to reduce the clock difference on
>> migration like in:
>>     6053a86 kvmclock: reduce kvmclock difference on migration
>>
>> Time base offset has originally been introduced by commit
>>     98a8b52 spapr: Add support for time base offset migration
>>
>> So while VM is paused, the time is stopped. This allows to have
>> the same result with date (based on Time Base Register) and
>> hwclock (based on "get-time-of-day" RTAS call).
>>
>> Moreover in TCG mode, the Time Base is always paused, so this
>> patch also adjust the behavior between TCG and KVM.
>>
>> VM state field "time_of_the_day_ns" is now useless but we keep
>> it to be able to migrate to older version of the machine.
>>
>> As vmstate_ppc_timebase structure (with timebase_pre_save() and
>> timebase_post_load() functions) was only used by vmstate_spapr,
>> we register the VM state change handler only in ppc_spapr_init().
>>
>> Signed-off-by: Laurent Vivier <address@hidden>
>> Signed-off-by: David Gibson <address@hidden>
> 
> Just a small heads-up: I've been debugging an OpenQA regression lately
> where our automated testing regressed with QEMU 2.9. With stock 2.9.1, I
> get a failure rate of "weird" effects (probably TB divergence between
> vcpus) of ~30%. With this patch reverted it's back to 0%.
> 
> I *think* something here causes the TB offset of multiple threads (I'm
> running -smp 2,threads=2) to diverge.
> 
> I'll keep debugging things tomorrow, but I'll be happy to see anyone
> else beat me to analyze what is going wrong ;).

Don't know if it can be related, but for migration we need:

http://patchwork.ozlabs.org/patch/840170/

diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index b9c49c22f2..4e11e6f489 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -8081,10 +8081,10 @@  static void gen_spr_power8_ebb(CPUPPCState *env)
 /* Virtual Time Base */
 static void gen_spr_vtb(CPUPPCState *env)
 {
-    spr_register(env, SPR_VTB, "VTB",
+    spr_register_kvm(env, SPR_VTB, "VTB",
                  SPR_NOACCESS, SPR_NOACCESS,
                  &spr_read_tbl, SPR_NOACCESS,
-                 0x00000000);
+                 KVM_REG_PPC_VTB, 0x00000000);
 }

 static void gen_spr_power8_fscr(CPUPPCState *env)

Thanks,
Laurent