linux-kernel - Re: [PATCH v2 1/3] KVM: x86: implement KVM_{GET|SET}_TSC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sun, 06 Dec 2020 17:19:16 +0100
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Maxim Levitsky <mlevitsk@...hat.com>, kvm@...r.kernel.org
Cc:     "H. Peter Anvin" <hpa@...or.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Jonathan Corbet <corbet@....net>,
        Jim Mattson <jmattson@...gle.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        "open list\:KERNEL SELFTEST FRAMEWORK" 
        <linux-kselftest@...r.kernel.org>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Marcelo Tosatti <mtosatti@...hat.com>,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        open list <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...hat.com>,
        "maintainer\:X86 ARCHITECTURE \(32-BIT AND 64-BIT\)" <x86@...nel.org>,
        Joerg Roedel <joro@...tes.org>, Borislav Petkov <bp@...en8.de>,
        Shuah Khan <shuah@...nel.org>,
        Maxim Levitsky <mlevitsk@...hat.com>,
        Andrew Jones <drjones@...hat.com>,
        Oliver Upton <oupton@...gle.com>,
        "open list\:DOCUMENTATION" <linux-doc@...r.kernel.org>
Subject: Re: [PATCH v2 1/3] KVM: x86: implement KVM_{GET|SET}_TSC_STATE

On Thu, Dec 03 2020 at 19:11, Maxim Levitsky wrote:
> +	case KVM_SET_TSC_STATE: {
> +		struct kvm_tsc_state __user *user_tsc_state = argp;
> +		struct kvm_tsc_state tsc_state;
> +		u64 host_tsc, wall_nsec;
> +
> +		u64 new_guest_tsc, new_guest_tsc_offset;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&tsc_state, user_tsc_state, sizeof(tsc_state)))
> +			goto out;
> +
> +		kvm_get_walltime(&wall_nsec, &host_tsc);
> +		new_guest_tsc = tsc_state.tsc;
> +
> +		if (tsc_state.flags & KVM_TSC_STATE_TIMESTAMP_VALID) {
> +			s64 diff = wall_nsec - tsc_state.nsec;
> +			if (diff >= 0)
> +				new_guest_tsc += nsec_to_cycles(vcpu, diff);
> +			else
> +				new_guest_tsc -= nsec_to_cycles(vcpu, -diff);
> +		}
> +
> +		new_guest_tsc_offset = new_guest_tsc - kvm_scale_tsc(vcpu, host_tsc);
> +		kvm_vcpu_write_tsc_offset(vcpu, new_guest_tsc_offset);

>From a timekeeping POV and the guests expectation of TSC this is
fundamentally wrong:

      tscguest = scaled(hosttsc) + offset

The TSC has to be viewed systemwide and not per CPU. It's systemwide
used for timekeeping and for that to work it has to be synchronized. 

Why would this be different on virt? Just because it's virt or what? 

Migration is a guest wide thing and you're not migrating single vCPUs.

This hackery just papers over he underlying design fail that KVM looks
at the TSC per vCPU which is the root cause and that needs to be fixed.

Thanks,

        tglx