From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pg1-f199.google.com (mail-pg1-f199.google.com [209.85.215.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4977947D951 for ; Tue, 11 Aug 2026 21:46:33 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.215.199 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786484795; cv=none; b=MHpABrfXqSCkt+taIbCZCsrXHZZ1ptMp0yZsmcgO72ZbKzd9g5yKm+RsRKeE7fty4BRFQHn+Bh+oSYE2TALo1lAtyzVTvRieAMZKp34TuE6aRKYzubuROxz/QzEIY9V+aq8vmTt9PxiO5cD9E2tl67HPI61AcD/J7EjUW6fg86Q= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786484795; c=relaxed/simple; bh=Bzrq/VKcfobXcbNSIJ48oES/vQOba3cFYNFGC6yNXhQ=; h=Date:In-Reply-To:Mime-Version:References:Message-ID:Subject:From: To:Cc:Content-Type; b=KpNNy2WSIZDmiM6rmUNHd99dISGvFkexqvz5H6/Z+GDZnO/q5f6f05DuDFugJDiLfJQ4AlZwxqrJEbBx7I+Dn6ooSn+B7nVIKBksmCwnRu179ZdS9YwCXcd1UPMeFPjadvuUt+eWX3yjOVJtnmRYHOpGfHFnw9XL0vQFefR2tEE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=V4Ss0UvV; arc=none smtp.client-ip=209.85.215.199 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="V4Ss0UvV" Received: by mail-pg1-f199.google.com with SMTP id 41be03b00d2f7-cb5cc1e13f8so230623a12.3 for ; Tue, 11 Aug 2026 14:46:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20251104; t=1786484792; x=1787089592; darn=vger.kernel.org; h=content-transfer-encoding:content-type:cc:to:from:subject :message-id:references:mime-version:in-reply-to:date:from:to:cc :subject:date:message-id:reply-to:content-type; bh=GNP84U5NVANNewsvSEzbCgg+MDCqvRSKiMtE5jPKweo=; b=V4Ss0UvVLYZDBtkpuwM9otI200+jqPqEFJftlM4PNtzd8rjir/s2fPmhMOUYZO4bnT rUs/EzczADWpKnCzG4da+7/C3Sp/hM+U/kRuclHztpX1fVGv1DNhlvegl/UxqYHg19Oy sj33bXn8AkV8A62U4GIg5Ivx5/A0cac8mWhkdKLIuMGTayeEo6fNyB9omuz+uuaxZrS5 JNX8qDXbQaFogvLxn1yR42mepWAkd475AUPMtrcom+k7WZOEnfKcsxC2qPTpIwh19yxl LQDQAuXLWk5WbTrFwo2OVsi5mC/SKZOGDHwRul3WfqynfL952DJMRpK6sm6JmRHF+CL+ 8V1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786484792; x=1787089592; h=content-transfer-encoding:content-type: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 :content-type; bh=GNP84U5NVANNewsvSEzbCgg+MDCqvRSKiMtE5jPKweo=; b=Kqb3e1jJApe6JgulE6iT/jwsIM5JDl/1IRaMDNQ8sjp7/JhbiS3hsmcPkNXZ4e1Q8Y woaYvQhs4jXnlrpawcrfjG/tv8V45nL/6CC2FyiLvRZSwTHFB3WiBXl0JSp213hckFLJ 2mkRGUnJgq/7RJ5sndZkvt2k1NjqAEqtyZLHvjdAwY72xkbCSDQmjixL+YPm6VkJ7IIR 7as7sJP+hAfa56/OSW5IlPF1zdV1v0XlLN4p6fIJHHyBnSGIfnV7ZQOiqsaJJ1kaTrL0 8WUZqFne4uLHwsG62U77yTnSk+bPUOCL2cyJzfjAJyoPgFkM9lDLPbv4vGYGTtNyvhvR 8X+Q== X-Forwarded-Encrypted: i=1; AHgh+RoFTctdGcqAPLDEeTR3Mzei2XEqlpY0/27UeRb+sH0HXKE/avUaccFHbrwmCHu63nsMQbxc3BVOXs/0XTQ=@vger.kernel.org X-Gm-Message-State: AOJu0Yybs7nKO4xFBeMxWDEmG9WB1FgAop9vWEaZ0iMLXGD2Uuoy5ZeA /+/gIde98F2Oz1zretGsE3ZnMgrHd9RuFdJ+zociiYa1V5edDIt9vbPaicHWVlbWZaaZ6nUYvgl R1LaOAw== X-Received: from pfbgp12.prod.google.com ([2002:a05:6a00:3b8c:b0:84b:4480:8b65]) (user=seanjc job=prod-delivery.src-stubby-dispatcher) by 2002:a05:6a00:2ea8:b0:847:98ff:4af5 with SMTP id d2e1a72fcca58-84fb54c5d00mr290195b3a.26.1786484792108; Tue, 11 Aug 2026 14:46:32 -0700 (PDT) Date: Tue, 11 Aug 2026 14:46:31 -0700 In-Reply-To: <3dc73f745b6abfd1eb53e7d3fce2067eaa3b3c92.camel@infradead.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <6ab49538675d97f1f4bf01574b1b066aae0bc05c.camel@infradead.org> <3dc73f745b6abfd1eb53e7d3fce2067eaa3b3c92.camel@infradead.org> Message-ID: Subject: Re: [PATCH v7 17/36] KVM: x86: Allow KVM master clock mode when TSCs are offset from each other From: Sean Christopherson To: David Woodhouse Cc: Paolo Bonzini , Jonathan Corbet , Shuah Khan , Thomas Gleixner , Ingo Molnar , Borislav Petkov , Dave Hansen , x86@kernel.org, "H. Peter Anvin" , Vitaly Kuznetsov , Juergen Gross , Boris Ostrovsky , Paul Durrant , Jonathan Cameron , Sascha Bischoff , Marc Zyngier , Joey Gouly , Jack Allister , Dongli Zhang , joe.jin@oracle.com, kvm@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, xen-devel@lists.xenproject.org, linux-kselftest@vger.kernel.org Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable On Tue, Aug 11, 2026, David Woodhouse wrote: > On Tue, 2026-08-11 at 11:41 -0700, Sean Christopherson wrote: > > OMG, I hate this code.=C2=A0 After literally hours of staring at this, = and even typing > > up a lengthy example of why guest time would go off the rails, I finall= y spotted > > that l1_tsc_offset is accounted for by the call to kvm_read_l1_tsc().= =C2=A0 FML. > >=20 > > Thanks for being patient and not flaming me too much :-) >=20 > Haha, no judgement. We *all* hate this code. That's why I threw my toys > out of the pram and went on this crusade to clean it up a bit. >=20 > > > > > And your variant just added a dependency on wallclock time back i= nto it > > > >=20 > > > > Can you elaborate?=C2=A0 I'm guessing I don't entirely understand w= hat you mean by > > > > wallclock time. > > >=20 > > > The system_time field? The unspecified might-be-UTC-might-have-leap-s= econds one :) > >=20 > > Ok, I think I finally understand the goal.=C2=A0 I got turned around by= the combination > > of the name SET_CLOCK_GUEST and the full pvclock structure being passed= to the > > guest.=C2=A0 I was expecting SET_CLOCK_GUEST to literally set the entir= e clock, e.g. > > mul+shift, timestamp, etc. >=20 > That's an implementation detail.=C2=A0 Yes and no. If the payload didn't literally have all the assets needed to = set the kvmclock fields, then I wouldn't care. But I don't think I'd be the on= ly person to see a GET+SET pair and expect GET to return exactly what was writ= ten via SET. > It is literally getting the clock as the guest sees it, and setting it > again on the destination from the same guest-ABI pvclock structure. > From the userspace point of view those actions *are* symmetrical. Only if userspace holds it just so. > I'd actually *like* it to just be a memcpy at both ends, even on the > SET side, just copying what userspace provides into what we offer to > the guest as its pvclock. >=20 > But as well as wanting to do some sanity checking, we also live in a > world where we might have to switch to the non-masterclock mode at any > time, and we have to ingest the information into the per-VM kvmclock > setup in a way that the kernel "understands", and that's why it ends up > implemented the way it is. >=20 > I looked at rewriting the masterclock base information from what > userspace is providing, but there are *host* TSC values in there, and > it ended up in some cases wanting to set ka->master_cycle_now to a > value which is *negative* on the new host, and I didn't want to > exercise that wrap-around path. So instead we just adjust > ka->kvmclock_offset to give appropriate results based on the existing > masterclock base. >=20 > Each vCPU's pvclock is then *regenerated* from the VM-side kvmclock > data, giving rise to that annoying =C2=B11ns discrepancy that I whined ab= out > a while back, but didn't give in to my perfectionism and eliminate... > yet. >=20 > As far as userspace is concerned, KVM_SET_CLOCK_GUEST *does* set the > entire clock (at least the relationship between guest TSC and kvmclock > nanoseconds, which is what it's for). It's just that the kernel then > "tweaks" it a little bit to give a slightly different y=3Dm(x-x')+c > equation which is still within the noise of the original. Again, if and only if the fields match what has been written previoiusly. = To me, that's not a SET operation given the full inputs. I completely agree that conceptually this is intended to SET the entire clo= ck, but as you note above, reality doesn't allow for that. > And the kernel actually does that kind of 'tweak' all the time. > Although we're working on narrowing them down because "within the > noise" is in the eye of the beholder; Dongli had some patches for that > which I think I rounded up and included? >=20 > > But all of that metadata is just a means to an end: the one and only go= al is to > > calculate the per-VM kvmclock_offset for the "new" host's TSC+time snap= shot, by > > computing the nanoseconds delta for the new snapshot as if it the guest= observed > > the TSC while running on the old host. >=20 > That is currently how it is implemented. It isn't the API contract. >=20 > > And that is done in the kernel instead of in userspace to minimize the = amount of > > slop introduced due to delay between taking the snapshot and computing = the offset. >=20 > Huh? There should be no delays here. If *anything* in this new code is > done with something other than a *simultaneous* reading of TSC and > ktime that I sweated blood and tears and got shouted at by Thomas for, > then that *is* something I care about... Sorry, I didn't mean to imply there would be delay on the SET side. What I= was trying to say is that if this were punted to userspace, then there _would_ = be a ton of slop because it would be practically impossible for userspace to pro= vide the correct offset. (I was walking myself through why KVM needed to provide= uAPI to compute the offset, as opposed to provide uAPI to let userspace jam in w= hatever value it wanted).