Received-SPF: pass (google.com: domain of linux-kernel+bounces-155720-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <cdc6c880-c666-4ff2-99f3-56c6b4f7576d@intel.com>
Date: Tue, 23 Apr 2024 10:59:21 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/2] KVM: selftests: Add KVM/PV clock selftest to prove
 timer drift correction
To: David Woodhouse <dwmw2@infradead.org>, Jack Allister
 <jalliste@amazon.com>, Paolo Bonzini <pbonzini@redhat.com>,
 Jonathan Corbet <corbet@lwn.net>, Sean Christopherson <seanjc@google.com>,
 Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>,
 Borislav Petkov <bp@alien8.de>, Dave Hansen <dave.hansen@linux.intel.com>,
 x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
 Shuah Khan <shuah@kernel.org>
Cc: Paul Durrant <paul@xen.org>, kvm@vger.kernel.org,
 linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
 linux-kselftest@vger.kernel.org
References: <20240408220705.7637-1-jalliste@amazon.com>
 <20240408220705.7637-3-jalliste@amazon.com>
 <3664e8ec-1fa1-48c0-a80d-546b7f6cd671@intel.com>
 <17F1A2E9-6BAD-40E7-ACDD-B110CFC124B3@infradead.org>
 <65FF4D51-05A8-42E0-9D07-6E42913CC75E@infradead.org>
 <6dca783b-6532-4fa7-9e04-1c0a382a00b0@intel.com>
 <c863ffcfd4edda9a1a46e3351766a655c5523f7d.camel@infradead.org>
 <71260288-3666-4419-8283-6565e91aaba4@intel.com>
 <06864883c53cc4042e1939c04ecbd5ef0f8acf3b.camel@infradead.org>
Content-Language: en-US
From: "Chen, Zide" <zide.chen@intel.com>
In-Reply-To: <06864883c53cc4042e1939c04ecbd5ef0f8acf3b.camel@infradead.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit


On 4/23/2024 12:49 AM, David Woodhouse wrote:
>> If I restored the KVM_REQ_GLOBAL_CLOCK_UPDATE request from
>> kvm_arch_vcpu_load(), the selftest works for me, and I ran the test for
>> 1000+ iterations, w/ or w/o TSC scaling, the TEST_ASSERT(delta_corrected
>>  <= ±1) never got hit. This is awesome!
>>
>> However, without KVM_REQ_GLOBAL_CLOCK_UPDATE, it still fails on creating
>> a VM. Maybe the init sequence sill needs some rework.
> 
> That one confuses me. The crash is actually in debugfs, as it's
> registering the per-vm or per-vcpu stats. I can't imagine *how* that's
> occurring. Or see why the availability of TSC scaling would cause it to
> show up for you and not me. Can I have your .config please?
> 
> First thought would be that there's some change in the KVM structures
> and you have some stale object files using the old struct, but then I
> realise I forgot to actually *remove* the now-unused
> kvmclock_update_work from x86's struct kvm_arch anyway.
> 
> I'll try to reproduce, as I think I want to *know* what's going on
> here, even if I am going to drop that patch as mentioned in 
> https://lore.kernel.org/kvm/a6723ac9e0169839cb33e8022a47c2de213866ac.camel@infradead.org
> 
> Are you able to load that vmlinux in gdb and
> (gdb) list *start_creating+0x80
> (gdb) list *kvm_create_vm_debugfs+0x28b
> 
> Thanks again.

My apologies, it turns out the KVM_REQ_GLOBAL_CLOCK_UPDATE is not
needed. Today I can't reproduce the issue after removing it.  Yesterday
I thought it may miss something related to pfncache.

To be clear, with the above mentioned change to
kvm_scale_tsc(master_tsc_scaling_ratio), the selftest runs reliably
regardless TSC scaling is enabled or not.