Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Date:   Tue, 3 Oct 2023 17:10:22 -0700
In-Reply-To: <96ceb6a6e9c380d329e8cd556ad13a2fcd554882.camel@infradead.org>
Mime-Version: 1.0
References: <20230926230649.67852-1-dongli.zhang@oracle.com>
 <377d9706-cc10-dfb8-5326-96c83c47338d@oracle.com> <36f3dbb1-61d7-e90a-02cf-9f151a1a3d35@oracle.com>
 <ZRWnVDMKNezAzr2m@google.com> <a461bf3f-c17e-9c3f-56aa-726225e8391d@oracle.com>
 <884aa233ef46d5209b2d1c92ce992f50a76bd656.camel@infradead.org>
 <ZRrxtagy7vJO5tgU@google.com> <52a3cea2084482fc67e35a0bf37453f84dcd6297.camel@infradead.org>
 <ZRtl94_rIif3GRpu@google.com> <96ceb6a6e9c380d329e8cd556ad13a2fcd554882.camel@infradead.org>
Message-ID: <ZRytbih4TUVkEhLC@google.com>
Subject: Re: [PATCH RFC 1/1] KVM: x86: add param to update master clock periodically
From:   Sean Christopherson <seanjc@google.com>
To:     David Woodhouse <dwmw2@infradead.org>
Cc:     Dongli Zhang <dongli.zhang@oracle.com>,
        Joe Jin <joe.jin@oracle.com>, x86@kernel.org,
        kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        pbonzini@redhat.com, tglx@linutronix.de, mingo@redhat.com,
        bp@alien8.de, dave.hansen@linux.intel.com
Content-Type: text/plain; charset="us-ascii"
Precedence: bulk

On Tue, Oct 03, 2023, David Woodhouse wrote:
> On Mon, 2023-10-02 at 17:53 -0700, Sean Christopherson wrote:
> > 
> > This is *very* lightly tested, as in it compiles and doesn't explode, but that's
> > about all I've tested.
> 
> I don't think it's working, if I understand what it's supposed to be
> doing.

...

> When I run xen_shinfo_test, the kvmclock still drifts from the
> "monotonic_raw" I get from kvm_get_time_and_clockread(), even with your
> patch.

It "works", in that it does what I intended it to do.  What I missed, and what's
obvious in hindsight, is that the timekeeping code doesn't *just* update what KVM
uses as the "base" when there "big events" like suspend/resume, the timekeeping
code updates the base clock on every tick.

My hack only smoothed away the delta, it didn't address the time that had gotten
moved into the base.  I though the base would be stable if the host was in a
steady state, but see above...