From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 7E179EDEC6C for ; Wed, 13 Sep 2023 14:50:33 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S236639AbjIMOug (ORCPT ); Wed, 13 Sep 2023 10:50:36 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38010 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232416AbjIMOue (ORCPT ); Wed, 13 Sep 2023 10:50:34 -0400 Received: from mail-pg1-x549.google.com (mail-pg1-x549.google.com [IPv6:2607:f8b0:4864:20::549]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 31E3FB7 for ; Wed, 13 Sep 2023 07:50:30 -0700 (PDT) Received: by mail-pg1-x549.google.com with SMTP id 41be03b00d2f7-570096f51acso869808a12.0 for ; Wed, 13 Sep 2023 07:50:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1694616629; x=1695221429; darn=vger.kernel.org; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:from:to:cc:subject:date:message-id :reply-to; bh=Q41llD3aO1ZBdbxd4yEGR7UuhY6UnD3ZTPq+ctmbTzk=; b=j2XQEjJAwF3a79kZbyVFFjs/4gUpLaNauNZrrSeekpWXr8Srj4P9psd0BOpPqQJUGL GGjvUXWrC1ZsSRIt9ztMssW8VK7XG8Y7oWL0UiTObCKHo7iIEZ9b4xZgYODHWDCdpE0S oOaFdZwOi7/ikDi+PMaL5dfddR2nilFkLPiaM4Du+FIJPUYIfX6Qdadir/Y5JlHvZlSj XzTXB0lo9jxb9d6V9XJIanTSBKEiKjGpfVos/9Rc0JP2QyjrxrQGPwvHbjm7H5stdUfa cuv46TNBQwnaACDN08ettIVGmQ1DeJNtXBKkhmF5HWL5T9LxGR/z99ogdG9a6kKfaozK EM0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1694616629; x=1695221429; h=content-transfer-encoding:cc:to:from:subject:message-id:references :mime-version:in-reply-to:date:x-gm-message-state:from:to:cc:subject :date:message-id:reply-to; bh=Q41llD3aO1ZBdbxd4yEGR7UuhY6UnD3ZTPq+ctmbTzk=; b=Y1p0nXQJWIsd3vQJcYzPZqaXYjYp5BD0vagd2rrgRMUgVyd0OoAfceRtYoztOPAQRy A3CYI/QLaoosQMKcb9R9Sx4zrUqXIrEqDRfJ/tcM7B5lEUXRgwSQ3Ki2FMbKnQUlWI15 FKTZl2TAKop1x3ebgThS9yUFNKbhyncoCXxn5ubXErUfYB9ijozZqS6faVGlK5yngTQH vItHtL70M2w8hH72JArpbI9tgAfqC7+zfqckrrMMDZMFVGU/eoXbPIKxCRmaZrrE0ZFJ NGoIwOavPoiXNAxAHil9z3g5Tk2ue+MRGYrn1CD9OekfNvmBsix0QpR/7MT6mpCF9iG8 ENCA== X-Gm-Message-State: AOJu0YyG/AIKI1m0vqEvY/uWqpiwrHWRC3VdHWqFIlltJRJVzcfjKA1n USS/Srx3ahUANWYfVX1CUXnOYhR9TVA= X-Google-Smtp-Source: AGHT+IF6OLHFs3rc4zlDaaJJlMjjFatCSnlFXp9JVMtBirdNZQaMQNAvUPI9DJwp3ilRUBYHX9sLm/FMmBE= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a63:348c:0:b0:563:dced:3f3a with SMTP id b134-20020a63348c000000b00563dced3f3amr127344pga.0.1694616629594; Wed, 13 Sep 2023 07:50:29 -0700 (PDT) Date: Wed, 13 Sep 2023 07:50:27 -0700 In-Reply-To: <055482bec09cae1ea56f979893c6b67e9d6b26a2.camel@infradead.org> Mime-Version: 1.0 References: <20230801034524.64007-1-likexu@tencent.com> <055482bec09cae1ea56f979893c6b67e9d6b26a2.camel@infradead.org> Message-ID: Subject: Re: [PATCH v4] KVM: x86/tsc: Don't sync user changes to TSC with KVM-initiated change From: Sean Christopherson To: David Woodhouse Cc: Like Xu , Paolo Bonzini , Oliver Upton , kvm@vger.kernel.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Sep 13, 2023, David Woodhouse wrote: > On Fri, 2023-08-11 at 15:59 -0700, Sean Christopherson wrote: > > On Tue, Aug 01, 2023, Like Xu wrote: > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > > index 278dbd37dab2..eeaf4ad9174d 100644 > > > --- a/arch/x86/kvm/x86.c > > > +++ b/arch/x86/kvm/x86.c > > > @@ -2713,7 +2713,7 @@ static void __kvm_synchronize_tsc(struct kvm_vc= pu *vcpu, u64 offset, u64 tsc, > > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0kvm_track_tsc_matchin= g(vcpu); > > > =C2=A0 } > > > =C2=A0=20 > > > -static void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 data) > > > +static void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 data, boo= l user_initiated) > >=20 > > Rather than pass two somewhat magic values for the KVM-internal call, w= hat about > > making @data a pointer and passing NULL? >=20 > Why change that at all? >=20 > Userspace used to be able to force a sync by writing zero. You are > removing that from the ABI without any explanation about why; No, my suggestion did not remove that from the ABI. A @user_value of '0' w= ould still force synchronization. -static void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 data) +static void kvm_synchronize_tsc(struct kvm_vcpu *vcpu, u64 *user_value) { + u64 data =3D user_value ? *user_value : 0; <=3D=3D=3D "*user_value= " is '0' struct kvm *kvm =3D vcpu->kvm; u64 offset, ns, elapsed; unsigned long flags; @@ -2712,14 +2713,17 @@ static void kvm_synchronize_tsc(struct kvm_vcpu *vc= pu, u64 data) elapsed =3D ns - kvm->arch.last_tsc_nsec; if (vcpu->arch.virtual_tsc_khz) { + /* + * Force synchronization when creating or hotplugging a vCP= U, + * i.e. when the TSC value is '0', to help keep clocks stab= le. + * If this is NOT a hotplug/creation case, skip synchroniza= tion + * on the first write from userspace so as not to misconstr= ue + * state restoration after live migration as an attempt fro= m + * userspace to synchronize. + */ if (data =3D=3D 0) { <=3D=3D "data" still '0', still forces= synchronization - /* - * detection of vcpu initialization -- need to sync - * with other vCPUs. This particularly helps to kee= p - * kvm_clock stable after CPU hotplug - */ synchronizing =3D true; > it doesn't seem necessary for fixing the original issue. It's necessary for "user_set_tsc" to be an accurate name. The code in v6 y= ields "user_set_tsc_to_non_zero_value". And I don't think it's just a naming iss= ue, e.g. if userspace writes '0' immediately after creating, and then later wri= tes a small delta, the v6 code wouldn't trigger synchronization because "user_set= _tsc" would be left unseft by the write of '0'.