linux-kernel - Re: [PATCH v4] KVM: x86/tsc: Don't sync user changes to TSC with KVM-initiated change

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:   Wed, 13 Sep 2023 10:44:52 +0200
From:   David Woodhouse <dwmw2@...radead.org>
To:     Like Xu <like.xu.linux@...il.com>
Cc:     Paolo Bonzini <pbonzini@...hat.com>,
        Oliver Upton <oliver.upton@...ux.dev>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Sean Christopherson <seanjc@...gle.com>
Subject: Re: [PATCH v4] KVM: x86/tsc: Don't sync user changes to TSC with
 KVM-initiated change

On Wed, 2023-09-13 at 16:41 +0800, Like Xu wrote:
> On 13/9/2023 4:10 pm, David Woodhouse wrote:
> > On Fri, 2023-08-11 at 15:59 -0700, Sean Christopherson wrote:
> > > The problem isn't that the sync code doesn't differentiate between kernel and
> > > user-initiated writes, because parts of the code *do* differentiate.  I think it's
> > > more accurate to say that the problem is that the sync code doesn't differentiate
> > > between userspace initializing the TSC and userspace attempting to synchronize the
> > > TSC.
> > 
> > I'm not utterly sure that *I* differentiate between userspace
> > "initializing the TSC" and attempting to "synchronize the TSC". What
> > *is* the difference?
> 
> I'd be more inclined to Oliver's explanation in this version of the changelog
> that different tsc_offsets are used to calculate guest_tsc value between the vcpu
> is created and when it is first set by usersapce. This extra synchronization is not
> expected for guest based on user's bugzilla report.
> 

Yes, it's about the kernel's default startup values (first vCPU
starting at TSC 0, others syncing to that on creation), and the fact
that the *first* userspace write (to any vCPU) should actually be
honoured even if it *does* happen to be within 1 second of the kernel's
startup values.


> Two hands in favor. Using the new KVM_VCPU_TSC_OFFSET API and a little
> fix on the legacy API is not conflict. Thank you for reviewing it.

I'm slightly dubious about making *changes* to an established userspace
ABI, especially when there's already a better way to do it. But I
suppose this specific change, if you *don't* also take away the ability
for userspace to explicitly write zero to force a sync (qv), is OK.

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5965 bytes)