Re: [PATCH] KVM: x86: make backwards_tsc_observed a per-VM variable

[Marcelo Tosatti <mtosatti@redhat.com>]

Hi Ladi,

Can you disable masterclock if suspend-to-RAM is performed, in 
case backwards_tsc_observed was observed for any VM ? 
(which indicates its likely to happen again, backwards TSC). 

Because a backwards TSC event, with masterclock in place, is likely
to cause the guest to see time going backwards.

What masterclock requires is that TSCs are:

	1) In sync across CPUs and TSC clocksource is in use by the host.
	2) kvmclock regions updated on VM-entry, after a suspend from RAM (in case it 
	   goes backwards): kvm_gen_update_masterclock.

Is the first condition met?

On Mon, Jun 26, 2017 at 09:56:43AM +0200, Ladi Prosek wrote:
> The backwards_tsc_observed global introduced in commit 16a9602 is never
> reset to false. If a VM happens to be running while the host is suspended
> (a common source of the TSC jumping backwards), master clock will never
> be enabled again for any VM. In contrast, if no VM is running while the
> host is suspended, master clock is unaffected. This is inconsistent and
> unnecessarily strict. Let's track the backwards_tsc_observed variable
> separately and let each VM start with a clean slate.
> 
> Real world impact: My Windows VMs get slower after my laptop undergoes a
> suspend/resume cycle. The only way to get the perf back is unloading and
> reloading the kvm module.
> 
> Signed-off-by: Ladi Prosek <lprosek@redhat.com>
> ---
>  arch/x86/include/asm/kvm_host.h | 1 +
>  arch/x86/kvm/x86.c              | 6 ++----
>  2 files changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index 695605e..d8259c3 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -799,6 +799,7 @@ struct kvm_arch {
>  	int audit_point;
>  	#endif
>  
> +	bool backwards_tsc_observed;
>  	bool boot_vcpu_runs_old_kvmclock;
>  	u32 bsp_vcpu_id;
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 87d3cb9..8586ec6 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -134,8 +134,6 @@ module_param(lapic_timer_advance_ns, uint, S_IRUGO | S_IWUSR);
>  static bool __read_mostly vector_hashing = true;
>  module_param(vector_hashing, bool, S_IRUGO);
>  
> -static bool __read_mostly backwards_tsc_observed = false;
> -
>  #define KVM_NR_SHARED_MSRS 16
>  
>  struct kvm_shared_msrs_global {
> @@ -1719,7 +1717,7 @@ static void pvclock_update_vm_gtod_copy(struct kvm *kvm)
>  					&ka->master_cycle_now);
>  
>  	ka->use_master_clock = host_tsc_clocksource && vcpus_matched
> -				&& !backwards_tsc_observed
> +				&& !ka->backwards_tsc_observed
>  				&& !ka->boot_vcpu_runs_old_kvmclock;
>  
>  	if (ka->use_master_clock)
> @@ -7827,8 +7825,8 @@ int kvm_arch_hardware_enable(void)
>  	 */
>  	if (backwards_tsc) {
>  		u64 delta_cyc = max_tsc - local_tsc;
> -		backwards_tsc_observed = true;
>  		list_for_each_entry(kvm, &vm_list, vm_list) {
> +			kvm->arch.backwards_tsc_observed = true;
>  			kvm_for_each_vcpu(i, vcpu, kvm) {
>  				vcpu->arch.tsc_offset_adjustment += delta_cyc;
>  				vcpu->arch.last_host_tsc = local_tsc;
> -- 
> 2.9.3