Re: [PATCH] target-i386: clear guest TSC on reset

[Paolo Bonzini <pbonzini@redhat.com>]

Il 05/12/2013 16:42, Fernando Luis Vazquez Cao ha scritto:
> (2013/12/05 22:53), Paolo Bonzini wrote:
>> Il 05/12/2013 14:15, Fernando Luis Vazquez Cao ha scritto:
>>>          /*
>>>           * KVM is yet unable to synchronize TSC values of multiple VCPUs on
>>>           * writeback. Until this is fixed, we only write the offset to SMP
>>>           * guests after migration, desynchronizing the VCPUs, but avoiding
>>>           * huge jump-backs that would occur without any writeback at all.
>>>           */
>>> -        if (smp_cpus == 1 || env->tsc != 0) {
>>> +        if (smp_cpus == 1 || env->tsc != 0 || level == KVM_PUT_RESET_STATE) {
>>>              kvm_msr_entry_set(&msrs[n++], MSR_IA32_TSC, env->tsc);
>>>          }
>> This is still a bit ugly, and desynchronizes the VCPUs on reset.
> 
> I agree it is a bit ugly, but in my testing QEMU seemed to loop over all
> the VCPUS fast enough for the kernel side kvm_write_tsc() to do a
> reasonable job of matching the offsets (the Linux guest did not mark
> the TSC unstable due to the TSCs being unsynchronized). Am I missing
> something?

No, probably not.

> I understand the benefits of what you are proposing but, since it is
> wider is scope and it would be more difficult to backport, I would
> prefer to implement it as a follow-up patch, unless you think that
> the current patch as a standalone fix does more harm than good.

It does some harm in that it introduces a case where KVM_PUT_RESET_STATE
restores something, but KVM_PUT_FULL_STATE doesn't.

If it really usually works, there shouldn't be a need for this "if"
statement at all.

Marcelo, what do you think?

Paolo