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

From: David Woodhouse
Date: Wed Sep 13 2023 - 04:44:59 EST


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.

Attachment: smime.p7s
Description: S/MIME cryptographic signature