* Re: [PATCH v3 07/41] clocksource: hyper-v: Register sched_clock save/restore iff it's necessary
From: David Woodhouse @ 2026-05-20 21:27 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-8-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 1346 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Register the Hyper-V reference counter (refcounter) callbacks for saving
> and restoring its PV sched_clock, if and only if the refcounter is
> actually being used for sched_clock. Currently, Hyper-V overrides the
> save/restore hooks if the reference TSC available, whereas the Hyper-V
> refcounter code only overrides sched_clock if the reference TSC is
> available *and* it's not invariant. The flaw is effectively papered over
> by invoking the "old" save/restore callbacks as part of save/restore, but
> that's unnecessary and fragile.
>
> To avoid introducing more complexity, and to allow for additional cleanups
> of the PV sched_clock code, move the save/restore hooks and logic into
> hyperv_timer.c and simply wire up the hooks when overriding sched_clock
> itself.
>
> Note, while the Hyper-V refcounter code is intended to be architecture
> neutral, CONFIG_PARAVIRT is firmly x86-only, i.e. adding a small amount of
> x86 specific code (which will be reduced in future cleanups) doesn't
> meaningfully pollute generic code.
>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> Tested-by: Michael Kelley <mhklinux@outlook.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 02/41] x86/tsc: Add helper to register CPU and TSC freq calibration routines
From: David Woodhouse @ 2026-05-20 21:19 UTC (permalink / raw)
To: Sean Christopherson
Cc: Kiryl Shutsemau, Paolo Bonzini, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
Jan Kiszka, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, Thomas Gleixner, John Stultz,
Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <ag4dMc2B3JQi4vxU@google.com>
[-- Attachment #1: Type: text/plain, Size: 2384 bytes --]
On Wed, 2026-05-20 at 13:44 -0700, Sean Christopherson wrote:
>
> + /*
> + * If the TSC counts at a constant frequency across P/T states, counts
> + * in deep C-states, and the TSC hasn't been marked unstable, treat the
> + * TSC reliable, as guaranteed by KVM. Note, the TSC unstable check
> + * exists purely to honor the TSC being marked unstable via command
> + * line, any runtime detection of an unstable will happen after this.
> + */
> + if (boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
> + boot_cpu_has(X86_FEATURE_NONSTOP_TSC) &&
> + !check_tsc_unstable())
{
> + tsc_properties = TSC_FREQ_KNOWN_AND_RELIABLE;
kvmclock = 0; /* Why use it if the TSC works? The kvmclock exists
*purely* to work around a TSC which *doesn't*
have those properties checked above. */
}
I was going to say PVCLOCK_TSC_STABLE_BIT, and maybe we should check
that *too* for paranoia? But hopefully the checks you have above are
equivalent?
> +
> + kvm_tsc_khz_cpuid = kvm_para_tsc_khz();
> +
> + /*
> + * If provided, use the TSC (and APIC bus) frequency provided in KVM's
> + * PV CPUID leaf even if kvmclock itself is disabled via command line.
> + * The PV CPUID information isn't dependent on kvmclock in any way, and
> + * in fact using the precise information is *more* important when the
> + * user has explicitly disabled kvmclock to force the kernel to use the
> + * TSC as its clocksource.
> + */
> + if (!kvmclock) {
> + if (kvm_tsc_khz_cpuid)
> + tsc_register_calibration_routines(kvm_get_tsc_khz,
> + kvm_get_cpu_khz,
> + tsc_properties);
> + return;
> + }
> +
Regardless of the above, why not just register these here
unconditionally, and remove the later call that does the same?
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 06/41] x86/acrn: Mark TSC frequency as known when using ACRN for calibration
From: Sean Christopherson @ 2026-05-20 21:02 UTC (permalink / raw)
To: David Woodhouse
Cc: Kiryl Shutsemau, Paolo Bonzini, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
Jan Kiszka, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, Thomas Gleixner, John Stultz,
Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <c347d65f555ad1e10a0e87ee57c5879c7046d0e7.camel@infradead.org>
On Wed, May 20, 2026, David Woodhouse wrote:
> On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> > Mark the TSC frequency as known when using ACRN's PV CPUID information.
> > Per commit 81a71f51b89e ("x86/acrn: Set up timekeeping") and common sense,
> > the TSC freq is explicitly provided by the hypervisor.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> > arch/x86/kernel/cpu/acrn.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
> > index c1506cb87d8c..2da3de4d470e 100644
> > --- a/arch/x86/kernel/cpu/acrn.c
> > +++ b/arch/x86/kernel/cpu/acrn.c
> > @@ -29,6 +29,7 @@ static void __init acrn_init_platform(void)
> > /* Install system interrupt handler for ACRN hypervisor callback */
> > sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_acrn_hv_callback);
> >
> > + setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
> > tsc_register_calibration_routines(acrn_get_tsc_khz,
> > acrn_get_tsc_khz);
>
> I'd feel slightly happier doing that from within acrn_get_tsc_khz()
> itself....
Ya, though as you note below, this is really a comment on the overall series.
Waiting to set the cap until the calibration routine is actually run does prevent
the case where the something registers a calibration routine, but its routine is
never run.
However, because the cap is sticky, it doesn't handle the scenario where its
routine _does_ run, but the kernel ultimately throws away its calibration in favor
of something else.
Further complicating things is that ~half of the paravirt flows already force set
caps before their routines are invoked:
snp_secure_tsc_init(), jailhouse_init_platform(), ms_hyperv_init_platform(),
vmware_set_capabilities()
Rather than trying to convince everyone that waiting is better, despite that
approach still being flawed, I chose to handle this by ensuring that once the TSC
is marked known and/or reliable, the kernel won't replace the calibration routine
with a "lesser" source:
x86/tsc: Rejects attempts to override TSC calibration with lesser routine
That doesn't completely prevent the kernel from being stupid, but it should prevent
both the case where X's routine is registered but never run, as well as the case
where it's run but ultimately ignored.
> which I note is 'static inline'. I'm vaguely surprised that even builds
> (although it does).
Heh, KVM x86 (in the host) does stupid things like this too. E.g. kvm_pdptr_read()
is a static inline, but then wired up as a function pointer in three different
places.
> Probably nicer to move it explicitly out of line in acrn.c though.
>
> Most of that should be a comment on patch 2 of the series, I guess?
^ permalink raw reply
* Re: [PATCH v3 02/41] x86/tsc: Add helper to register CPU and TSC freq calibration routines
From: Sean Christopherson @ 2026-05-20 20:44 UTC (permalink / raw)
To: David Woodhouse
Cc: Kiryl Shutsemau, Paolo Bonzini, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
Jan Kiszka, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, Thomas Gleixner, John Stultz,
Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <44e0d60548d317fd59895f18bd17220dfb2f834b.camel@infradead.org>
On Wed, May 20, 2026, David Woodhouse wrote:
> On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> > diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> > index b5991d53fc0e..e9e7394140dd 100644
> > --- a/arch/x86/kernel/kvmclock.c
> > +++ b/arch/x86/kernel/kvmclock.c
> > @@ -321,8 +321,8 @@ void __init kvmclock_init(void)
> > flags = pvclock_read_flags(&hv_clock_boot[0].pvti);
> > kvm_sched_clock_init(flags & PVCLOCK_TSC_STABLE_BIT);
> >
> > - x86_platform.calibrate_tsc = kvm_get_tsc_khz;
> > - x86_platform.calibrate_cpu = kvm_get_tsc_khz;
> > + tsc_register_calibration_routines(kvm_get_tsc_khz, kvm_get_tsc_khz);
> > +
> > x86_platform.get_wallclock = kvm_get_wallclock;
> > x86_platform.set_wallclock = kvm_set_wallclock;
> > #ifdef CONFIG_X86_LOCAL_APIC
>
> Can we move those (and maybe everything in the context there too) up
> *before* the check for no-kvmclock at the top of the function?
Oof, I was going to say "no", but disabling kvmclock is exactly the workaround
I've told people to use to get the kernel to use the TSC instead of kvmclock.
> Probably in a separate patch.
Ya. I think this?
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index 08ee4bc304c8..92a1ebf31e4d 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -27,6 +27,7 @@ static int kvmclock_vsyscall __initdata = 1;
static int msr_kvm_system_time __ro_after_init;
static int msr_kvm_wall_clock __ro_after_init;
static u64 kvm_sched_clock_offset __ro_after_init;
+static unsigned int kvm_tsc_khz_cpuid __ro_after_init;
static int __init parse_no_kvmclock(char *arg)
{
@@ -207,7 +208,7 @@ static unsigned long kvm_get_tsc_khz(void)
lapic_timer_period = apic_khz * 1000 / HZ;
#endif
- return kvm_para_tsc_khz() ? : pvclock_tsc_khz(this_cpu_pvti());
+ return kvm_tsc_khz_cpuid ? : pvclock_tsc_khz(this_cpu_pvti());
}
static unsigned long kvm_get_cpu_khz(void)
@@ -387,9 +388,39 @@ void __init kvmclock_init(void)
enum tsc_properties tsc_properties = TSC_FREQUENCY_KNOWN;
bool stable = false;
- if (!kvm_para_available() || !kvmclock)
+ if (!kvm_para_available())
return;
+ /*
+ * If the TSC counts at a constant frequency across P/T states, counts
+ * in deep C-states, and the TSC hasn't been marked unstable, treat the
+ * TSC reliable, as guaranteed by KVM. Note, the TSC unstable check
+ * exists purely to honor the TSC being marked unstable via command
+ * line, any runtime detection of an unstable will happen after this.
+ */
+ if (boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
+ boot_cpu_has(X86_FEATURE_NONSTOP_TSC) &&
+ !check_tsc_unstable())
+ tsc_properties = TSC_FREQ_KNOWN_AND_RELIABLE;
+
+ kvm_tsc_khz_cpuid = kvm_para_tsc_khz();
+
+ /*
+ * If provided, use the TSC (and APIC bus) frequency provided in KVM's
+ * PV CPUID leaf even if kvmclock itself is disabled via command line.
+ * The PV CPUID information isn't dependent on kvmclock in any way, and
+ * in fact using the precise information is *more* important when the
+ * user has explicitly disabled kvmclock to force the kernel to use the
+ * TSC as its clocksource.
+ */
+ if (!kvmclock) {
+ if (kvm_tsc_khz_cpuid)
+ tsc_register_calibration_routines(kvm_get_tsc_khz,
+ kvm_get_cpu_khz,
+ tsc_properties);
+ return;
+ }
+
if (kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE2)) {
msr_kvm_system_time = MSR_KVM_SYSTEM_TIME_NEW;
msr_kvm_wall_clock = MSR_KVM_WALL_CLOCK_NEW;
@@ -424,21 +455,14 @@ void __init kvmclock_init(void)
}
/*
- * If the TSC counts at a constant frequency across P/T states, counts
- * in deep C-states, and the TSC hasn't been marked unstable, prefer
- * the TSC over kvmclock for sched_clock and drop kvmclock's rating so
- * that TSC is chosen as the clocksource. Note, the TSC unstable check
- * exists purely to honor the TSC being marked unstable via command
- * line, any runtime detection of an unstable will happen after this.
+ * If the TSC is reliable (see above), prefer the TSC over kvmclock for
+ * sched_clock and drop kvmclock's rating so that TSC is chosen as the
+ * clocksource.
*/
- if (boot_cpu_has(X86_FEATURE_CONSTANT_TSC) &&
- boot_cpu_has(X86_FEATURE_NONSTOP_TSC) &&
- !check_tsc_unstable()) {
+ if (tsc_properties & TSC_RELIABLE)
kvm_clock.rating = 299;
- tsc_properties = TSC_FREQ_KNOWN_AND_RELIABLE;
- } else {
+ else
kvm_sched_clock_init(stable);
- }
tsc_register_calibration_routines(kvm_get_tsc_khz, kvm_get_cpu_khz,
tsc_properties);
^ permalink raw reply related
* Re: [PATCH v6 06/43] KVM: x86/mmu: Bug the VM if gmem attributes are queried to determine max mapping level
From: Sean Christopherson @ 2026-05-20 20:39 UTC (permalink / raw)
To: Ackerley Tng
Cc: Fuad Tabba, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, ira.weiny, jmattson, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, willy, wyihan, yan.y.zhao, forkloop, pratyush,
suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Baoquan He, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka, kvm,
linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <CAEvNRgGHHkvfJ-mn9rfDvS+_1ht08YatFWo-Swt+5wFSPnC9Nw@mail.gmail.com>
On Wed, May 20, 2026, Ackerley Tng wrote:
> Sean Christopherson <seanjc@google.com> writes:
> >> > } else {
> >> > + /*
> >> > + * Memory attributes cannot be obtained from guest_memfd while
> >> > + * the MMU lock is held.
> >> > + */
> >> > + if (KVM_BUG_ON(static_call_query(__kvm_get_memory_attributes) ==
> >> > + kvm_gmem_get_memory_attributes, kvm)) {
> >> > + return 0;
> >> > + }
> >> > +
> >>
> >> This directly takes the address of kvm_gmem_get_memory_attributes,
> >> which is only compiled if CONFIG_KVM_GUEST_MEMFD=y. This breaks
> >> ARCH=i386.
> >
> > And this bleeds guest_memfd implementation details into places they don't belong.
> > The right way to deal with this is to use lockdep_assert_not_held() in whatever
> > code mustn't run with mmu_lock held. E.g.
> >
> > diff --git virt/kvm/guest_memfd.c virt/kvm/guest_memfd.c
> > index c9f155c2dc5c..3bea9c1137ef 100644
> > --- virt/kvm/guest_memfd.c
> > +++ virt/kvm/guest_memfd.c
> > @@ -547,6 +547,9 @@ unsigned long kvm_gmem_get_memory_attributes(struct kvm *kvm, gfn_t gfn)
> > struct kvm_memory_slot *slot = gfn_to_memslot(kvm, gfn);
> > struct inode *inode;
> >
> > + /* Comment goes here. */
> > + lockdep_assert_not_held(&kvm->mmu_lock);
> > +
> > /*
> > * If this gfn has no associated memslot, there's no chance of the gfn
> > * being backed by private memory, since guest_memfd must be used for
> >
> > But I'm confused, because kvm_gmem_get_memory_attributes() doesn't actually take
> > filemap_invalidate_lock(), so what exactly is the problem?
> >
>
> Ahh I can drop this patch now. kvm_gmem_get_memory_attributes() used to
> take the filemap_invalidate_lock(), but after Liam pointed out that
> the attributes maple tree should be using MT_FLAGS_USE_RCU, I stopped
> taking filemap_invalidate_lock() and forgot to undo this.
>
> I'll wait a bit for more reviews and then put out another revision
> without this patch.
If this is the only issue with v6, don't send a new version, I'll just drop it
when applying.
^ permalink raw reply
* Re: [PATCH v6 12/43] KVM: guest_memfd: Call arch invalidate hooks on conversion
From: Ackerley Tng @ 2026-05-20 20:35 UTC (permalink / raw)
To: Fuad Tabba
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, jmattson, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, willy, wyihan, yan.y.zhao, forkloop, pratyush,
suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
Sean Christopherson, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, H. Peter Anvin, Steven Rostedt,
Masami Hiramatsu, Mathieu Desnoyers, Jonathan Corbet, Shuah Khan,
Shuah Khan, Vishal Annapurve, Andrew Morton, Chris Li,
Kairui Song, Kemeng Shi, Nhat Pham, Baoquan He, Barry Song,
Axel Rasmussen, Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng,
Shakeel Butt, Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <CA+EHjTwOfJ=nCRoX3m5uDt=CcF_zrqGsZVBBmo4MscmPqrBxOA@mail.gmail.com>
Fuad Tabba <tabba@google.com> writes:
> On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay
> <devnull+ackerleytng.google.com@kernel.org> wrote:
>>
>> From: Ackerley Tng <ackerleytng@google.com>
>>
>> When memory in guest_memfd is converted from private to shared, the
>> platform-specific state associated with the guest-private pages must be
>> invalidated or cleaned up.
>>
>> Iterate over the folios in the affected range and call the
>> kvm_arch_gmem_invalidate() hook for each PFN range. This allows
>> architectures to perform necessary teardown, such as updating hardware
>> metadata or encryption states, before the pages are transitioned to the
>> shared state.
>>
>> Invoke this helper after indicating to KVM's mmu code that an invalidation
>> is in progress to stop in-flight page faults from succeeding.
>>
>> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
>
> Minor nit below, but lgtm.
>
> Reviewed-by: Fuad Tabba <tabba@google.com>
>
> Cheers,
> /fuad
>
>> ---
>> virt/kvm/guest_memfd.c | 41 +++++++++++++++++++++++++++++++++++++++++
>> 1 file changed, 41 insertions(+)
>>
>> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
>> index 9d82642a025e9..baf4b88dead1f 100644
>> --- a/virt/kvm/guest_memfd.c
>> +++ b/virt/kvm/guest_memfd.c
>> @@ -603,6 +603,42 @@ static bool kvm_gmem_is_safe_for_conversion(struct inode *inode, pgoff_t start,
>> return safe;
>> }
>>
>> +#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
>> +static void kvm_gmem_invalidate(struct inode *inode, pgoff_t start, pgoff_t end)
>> +{
>> + struct folio_batch fbatch;
>> + pgoff_t next = start;
>> + int i;
>> +
>> + folio_batch_init(&fbatch);
>> + while (filemap_get_folios(inode->i_mapping, &next, end - 1, &fbatch)) {
>> + for (i = 0; i < folio_batch_count(&fbatch); ++i) {
>> + struct folio *folio = fbatch.folios[i];
>> + pgoff_t start_index, end_index;
>> + kvm_pfn_t start_pfn, end_pfn;
>> +
>> + start_index = max(start, folio->index);
>> + end_index = min(end, folio_next_index(folio));
>> + /*
>> + * end_index is either in folio or points to
>> + * the first page of the next folio. Hence,
>> + * all pages in range [start_index, end_index)
>> + * are contiguous.
>> + */
>> + start_pfn = folio_file_pfn(folio, start_index);
>> + end_pfn = start_pfn + end_index - start_index;
>> +
>> + kvm_arch_gmem_invalidate(start_pfn, end_pfn);
>> + }
>> +
>> + folio_batch_release(&fbatch);
>> + cond_resched();
>> + }
>> +}
>> +#else
>> +static void kvm_gmem_invalidate(struct inode *inode, pgoff_t start, pgoff_t end) {}
>> +#endif
>> +
>> static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>> size_t nr_pages, uint64_t attrs,
>> pgoff_t *err_index)
>> @@ -643,7 +679,12 @@ static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
>> */
>>
>> kvm_gmem_invalidate_begin(inode, start, end);
>> +
>> + if (!to_private)
>> + kvm_gmem_invalidate(inode, start, end);
>> +
>> mas_store_prealloc(&mas, xa_mk_value(attrs));
>> +
>
> Why the unrelated extra space?
>
Hmm this space provides vertical space between invalidate_{begin,end}
and the STUFF it wraps, like
invalidate_begin
STUFF
invalidate_end
More STUFF is going to go here in future patch series, such as splitting
private pages in TDX.
>> kvm_gmem_invalidate_end(inode, start, end);
>> out:
>> filemap_invalidate_unlock(mapping);
>>
>> --
>> 2.54.0.563.g4f69b47b94-goog
>>
>>
^ permalink raw reply
* Re: [PATCH v6 06/43] KVM: x86/mmu: Bug the VM if gmem attributes are queried to determine max mapping level
From: Ackerley Tng @ 2026-05-20 20:25 UTC (permalink / raw)
To: Sean Christopherson, Fuad Tabba
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
ira.weiny, jmattson, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, willy, wyihan, yan.y.zhao, forkloop, pratyush,
suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Baoquan He, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka, kvm,
linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <ag3DawWCcrpCkD0e@google.com>
Sean Christopherson <seanjc@google.com> writes:
>
> [...snip...]
>
>> > @@ -3357,6 +3357,15 @@ int kvm_mmu_max_mapping_level(struct kvm *kvm, struct kvm_page_fault *fault,
>> > max_level = fault->max_level;
>> > is_private = fault->is_private;
>> > } else {
>> > + /*
>> > + * Memory attributes cannot be obtained from guest_memfd while
>> > + * the MMU lock is held.
>> > + */
>> > + if (KVM_BUG_ON(static_call_query(__kvm_get_memory_attributes) ==
>> > + kvm_gmem_get_memory_attributes, kvm)) {
>> > + return 0;
>> > + }
>> > +
>>
>> This directly takes the address of kvm_gmem_get_memory_attributes,
>> which is only compiled if CONFIG_KVM_GUEST_MEMFD=y. This breaks
>> ARCH=i386.
>
> And this bleeds guest_memfd implementation details into places they don't belong.
> The right way to deal with this is to use lockdep_assert_not_held() in whatever
> code mustn't run with mmu_lock held. E.g.
>
> diff --git virt/kvm/guest_memfd.c virt/kvm/guest_memfd.c
> index c9f155c2dc5c..3bea9c1137ef 100644
> --- virt/kvm/guest_memfd.c
> +++ virt/kvm/guest_memfd.c
> @@ -547,6 +547,9 @@ unsigned long kvm_gmem_get_memory_attributes(struct kvm *kvm, gfn_t gfn)
> struct kvm_memory_slot *slot = gfn_to_memslot(kvm, gfn);
> struct inode *inode;
>
> + /* Comment goes here. */
> + lockdep_assert_not_held(&kvm->mmu_lock);
> +
> /*
> * If this gfn has no associated memslot, there's no chance of the gfn
> * being backed by private memory, since guest_memfd must be used for
>
> But I'm confused, because kvm_gmem_get_memory_attributes() doesn't actually take
> filemap_invalidate_lock(), so what exactly is the problem?
>
Ahh I can drop this patch now. kvm_gmem_get_memory_attributes() used to
take the filemap_invalidate_lock(), but after Liam pointed out that
the attributes maple tree should be using MT_FLAGS_USE_RCU, I stopped
taking filemap_invalidate_lock() and forgot to undo this.
I'll wait a bit for more reviews and then put out another revision
without this patch.
>> > max_level = PG_LEVEL_NUM;
>> > is_private = kvm_mem_is_private(kvm, gfn);
>> > }
>> >
>> > --
>> > 2.54.0.563.g4f69b47b94-goog
>> >
>> >
^ permalink raw reply
* Re: [PATCH v3 06/41] x86/acrn: Mark TSC frequency as known when using ACRN for calibration
From: David Woodhouse @ 2026-05-20 20:01 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-7-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 1288 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Mark the TSC frequency as known when using ACRN's PV CPUID information.
> Per commit 81a71f51b89e ("x86/acrn: Set up timekeeping") and common sense,
> the TSC freq is explicitly provided by the hypervisor.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kernel/cpu/acrn.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/x86/kernel/cpu/acrn.c b/arch/x86/kernel/cpu/acrn.c
> index c1506cb87d8c..2da3de4d470e 100644
> --- a/arch/x86/kernel/cpu/acrn.c
> +++ b/arch/x86/kernel/cpu/acrn.c
> @@ -29,6 +29,7 @@ static void __init acrn_init_platform(void)
> /* Install system interrupt handler for ACRN hypervisor callback */
> sysvec_install(HYPERVISOR_CALLBACK_VECTOR, sysvec_acrn_hv_callback);
>
> + setup_force_cpu_cap(X86_FEATURE_TSC_KNOWN_FREQ);
> tsc_register_calibration_routines(acrn_get_tsc_khz,
> acrn_get_tsc_khz);
I'd feel slightly happier doing that from within acrn_get_tsc_khz()
itself.... which I note is 'static inline'. I'm vaguely surprised that
even builds (although it does). Probably nicer to move it explicitly
out of line in acrn.c though.
Most of that should be a comment on patch 2 of the series, I guess?
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 05/41] x86/tdx: Override PV calibration routines with CPUID-based calibration
From: David Woodhouse @ 2026-05-20 19:52 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-6-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 2003 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> When running as a TDX guest, explicitly override the TSC frequency
> calibration routine with CPUID-based calibration instead of potentially
> relying on a hypervisor-controlled PV routine. For TDX guests, CPUID.0x15
> is always emulated by the TDX-Module, i.e. the information from CPUID is
> more trustworthy than the information provided by the hypervisor.
>
> To maintain backwards compatibility with TDX guest kernels that use native
> calibration, and because it's the least awful option, retain
> native_calibrate_tsc()'s stuffing of the local APIC bus period using the
> core crystal frequency. While it's entirely possible for the hypervisor
> to emulate the APIC timer at a different frequency than the core crystal
> frequency, the commonly accepted interpretation of Intel's SDM is that APIC
> timer runs at the core crystal frequency when that latter is enumerated via
> CPUID:
>
> The APIC timer frequency will be the processor’s bus clock or core
> crystal clock frequency (when TSC/core crystal clock ratio is enumerated
> in CPUID leaf 0x15).
>
> If the hypervisor is malicious and deliberately runs the APIC timer at the
> wrong frequency, nothing would stop the hypervisor from modifying the
> frequency at any time, i.e. attempting to manually calibrate the frequency
> out of paranoia would be futile.
>
> Deliberately leave the CPU frequency calibration routine as is, since the
> TDX-Module doesn't provide any guarantees with respect to CPUID.0x16.
>
> Opportunistically add a comment explaining that CoCo TSC initialization
> needs to come after hypervisor specific initialization.
>
> Cc: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
I don't much like stuffing the lapic_timer_period... but I'll give you
'least awful option'. For now.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v10 00/25] Runtime TDX module update support
From: Dave Hansen @ 2026-05-20 19:42 UTC (permalink / raw)
To: Chao Gao, kvm, linux-coco, x86, linux-kernel, linux-rt-devel,
linux-doc
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, Sebastian Andrzej Siewior,
Clark Williams, Steven Rostedt, Jonathan Corbet, Shuah Khan
In-Reply-To: <20260520133909.409394-1-chao.gao@intel.com>
So a little cat|sort|uniq says:
11 Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
17 Reviewed-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
We're on v10 here. There are 6 patches in here with no review tags:
M: Kiryl Shutsemau <kas@kernel.org>
R: Dave Hansen <dave.hansen@linux.intel.com>
R: Rick Edgecombe <rick.p.edgecombe@intel.com>
Are all of those 6 new patches? Or do the reviewers still have some work
to do?
^ permalink raw reply
* Re: [PATCH v10 22/25] x86/virt/tdx: Reject updates during compatibility-sensitive operations
From: Dave Hansen @ 2026-05-20 19:35 UTC (permalink / raw)
To: Chao Gao, kvm, linux-coco, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, H. Peter Anvin
In-Reply-To: <20260520133909.409394-23-chao.gao@intel.com>
On 5/20/26 06:38, Chao Gao wrote:
> int tdx_module_shutdown(void)
> {
> struct tdx_sys_info_handoff handoff = {};
> struct tdx_module_args args = {};
> int ret, cpu;
> + u64 err;
>
> ret = get_tdx_sys_info_handoff(&handoff);
> WARN_ON_ONCE(ret);
> @@ -1288,9 +1291,30 @@ int tdx_module_shutdown(void)
> * module can produce and most likely supported by newer modules.
> */
> args.rcx = handoff.module_hv;
> - ret = seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
> - if (ret)
> - return ret;
> +
> + /*
> + * This flag tells the TDX module to reject shutdown if it races
> + * with a "sensitive" ongoing operation. That eliminates exposure
> + * to a TDX erratum which can corrupt TDX guest states.
> + *
> + * This flag is not supported by all TDX modules and may cause
> + * the shutdown (and subsequent update procedure) to fail.
> + */
> + args.rcx |= TDX_SYS_SHUTDOWN_AVOID_COMPAT_SENSITIVE;
> +
> + err = seamcall(TDH_SYS_SHUTDOWN, &args);
> +
> + /*
> + * The shutdown ran into a "sensitive" ongoing operation. Signal
> + * to userspace that it can retry.
> + */
> + if ((err & TDX_SEAMCALL_STATUS_MASK) == TDX_UPDATE_COMPAT_SENSITIVE)
> + return -EBUSY;
> +
> + if (err) {
> + seamcall_err(TDH_SYS_SHUTDOWN, err, &args);
> + return -EIO;
> + }
This function is pretty tidy. More or less:
ret = get_tdx_sys_info_handoff(&handoff);
if (ret)
return
args.foo = handoff.bar;
ret = seamcall_prerr(TDH_SYS_SHUTDOWN, &args);
if (ret)
return
memset(&tdx_module_state, 0, sizeof(tdx_module_state));
for_each_possible_cpu(cpu)
per_cpu(tdx_lp_initialized, cpu) = false;
The logic's not bad, right? Get the handoff data, hand it off to
something, then go set some fields.
Then what does this patch do? It goes and globs a just huge blob of
TDH_SYS_SHUTDOWN errata handling and implementation details right smack
in the middle. Our tidy little function is no more.
I really with this would trigger folks' gag reflexes. It's *SO* easy to
fix. It's *so* easy to keep the code tidy and hide the dead bodies so
that the logic can still be followed.
I'm probably going to drop this for now.
^ permalink raw reply
* Re: [PATCH v10 24/25] coco/tdx-host: Document TDX module update compatibility criteria
From: Dave Hansen @ 2026-05-20 19:22 UTC (permalink / raw)
To: Chao Gao, kvm, linux-coco, x86, linux-kernel
Cc: binbin.wu, dave.hansen, djbw, ira.weiny, kai.huang, kas,
nik.borisov, paulmck, pbonzini, reinette.chatre, rick.p.edgecombe,
sagis, seanjc, tony.lindgren, vannapurve, vishal.l.verma,
yilun.xu, xiaoyao.li, yan.y.zhao, Dan Williams
In-Reply-To: <20260520133909.409394-25-chao.gao@intel.com>
On 5/20/26 06:38, Chao Gao wrote:
> Note that runtime TDX module updates are an "update at your own risk"
> operation; userspace is responsible for ensuring that the update meets
> the compatibility criteria.
I feel like this was copied right out of some TDX powerpoint
presentation. It's really hard to parse.
What does any of this mean to end users?
Who are they depending on getting this all right?
If it goes wrong, what happens?
This needs to be from the user's perspective.
^ permalink raw reply
* Re: [PATCH v3 04/41] x86/sev: Move check for SNP Secure TSC support to tsc_early_init()
From: David Woodhouse @ 2026-05-20 19:16 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-5-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 572 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Move the check on having a Secure TSC to the common tsc_early_init() so
> that it's obvious that having a Secure TSC is conditional, and to prepare
> for adding TDX to the mix (blindly initializing *both* SNP and TDX TSC
> logic looks especially weird).
>
> No functional change intended.
>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 03/41] x86/sev: Mark TSC as reliable when configuring Secure TSC
From: David Woodhouse @ 2026-05-20 19:06 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-4-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 653 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Move the code to mark the TSC as reliable from sme_early_init() to
> snp_secure_tsc_init(). The only reader of TSC_RELIABLE is the aptly
> named check_system_tsc_reliable(), which runs in tsc_init(), i.e.
> after snp_secure_tsc_init().
>
> This will allow consolidating the handling of TSC_KNOWN_FREQ and
> TSC_RELIABLE when overriding the TSC calibration routine.
>
> Cc: Tom Lendacky <thomas.lendacky@amd.com>
> Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 41/41] x86/kvmclock: Get CPU base frequency from CPUID when it's available
From: Sean Christopherson @ 2026-05-20 19:06 UTC (permalink / raw)
To: David Woodhouse
Cc: Kiryl Shutsemau, Paolo Bonzini, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
Jan Kiszka, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, Thomas Gleixner, John Stultz,
Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <0a3aa07a1d3c4bec2b89f8026093969155b73caa.camel@infradead.org>
On Wed, May 20, 2026, David Woodhouse wrote:
> On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> > If CPUID.0x16 is present and valid, use the CPU frequency provided by
> > CPUID instead of assuming that the virtual CPU runs at the same
> > frequency as TSC and/or kvmclock. Back before constant TSCs were a
> > thing, treating the TSC and CPU frequencies as one and the same was
> > somewhat reasonable, but now it's nonsensical, especially if the
> > hypervisor explicitly enumerates the CPU frequency.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > ---
> > arch/x86/kernel/kvmclock.c | 16 +++++++++++++++-
> > 1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> > index 62c8ea2e6769..7607920ae386 100644
> > --- a/arch/x86/kernel/kvmclock.c
> > +++ b/arch/x86/kernel/kvmclock.c
> > @@ -190,6 +190,20 @@ void kvmclock_cpu_action(enum kvm_guest_cpu_action action)
> > }
> > }
> >
> > +static unsigned long kvm_get_cpu_khz(void)
> > +{
> > + unsigned int cpu_khz;
> > +
> > + /*
> > + * Prefer CPUID over kvmclock when possible, as the base CPU frequency
> > + * isn't necessarily the same as the kvmlock "TSC" frequency.
> > + */
> > + if (!cpuid_get_cpu_freq(&cpu_khz))
> > + return cpu_khz;
> > +
> > + return pvclock_tsc_khz(this_cpu_pvti());
>
> I'm fine with this in principle but shouldn't the fallback be calling
> kvm_get_tsc_khz() instead of directly calling pvclock_tsc_khz()?
Oh, yeah, for this patch, definitely yes, so that there's no side effects. The
question really should be answered in the context of "x86/kvmclock: Obtain TSC
frequency from CPUID if present", which subtly impacts the CPU frequency, but I
think the answer is "yes" there as well.
^ permalink raw reply
* Re: [PATCH v3 02/41] x86/tsc: Add helper to register CPU and TSC freq calibration routines
From: David Woodhouse @ 2026-05-20 19:04 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-3-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 2333 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Add a helper to register non-native, i.e. PV and CoCo, CPU and TSC
> frequency calibration routines. This will allow consolidating handling
> of common TSC properties that are forced by hypervisor (PV routines),
> and will also allow adding sanity checks to guard against overriding a
> TSC calibration routine with a routine that is less robust/trusted.
>
> Make the CPU calibration routine optional, as Xen (very sanely) doesn't
> assume the CPU runs as the same frequency as the TSC.
>
> Wrap the helper in an #ifdef to document that the kernel overrides
> the native routines when running as a VM, and to guard against unwanted
> usage. Add a TODO to call out that AMD_MEM_ENCRYPT is a mess and doesn't
> depend on HYPERVISOR_GUEST because it gates both guest and host code.
>
> No functional change intended.
>
> Reviewed-by: Michael Kelley <mhklinux@outlook.com>
> Tested-by: Michael Kelley <mhklinux@outlook.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Mildly concerned that we might want to support multiple options — does
it have CPUID 0x15? Does it have 0x40000x10? Does it have a pvclock?
There are various permutations of those which are perhaps best handled
by *trying* each one, in some order, and populating a struct with the
answers?
But on the basis that perfect is the enemy of good,
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index b5991d53fc0e..e9e7394140dd 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -321,8 +321,8 @@ void __init kvmclock_init(void)
> flags = pvclock_read_flags(&hv_clock_boot[0].pvti);
> kvm_sched_clock_init(flags & PVCLOCK_TSC_STABLE_BIT);
>
> - x86_platform.calibrate_tsc = kvm_get_tsc_khz;
> - x86_platform.calibrate_cpu = kvm_get_tsc_khz;
> + tsc_register_calibration_routines(kvm_get_tsc_khz, kvm_get_tsc_khz);
> +
> x86_platform.get_wallclock = kvm_get_wallclock;
> x86_platform.set_wallclock = kvm_set_wallclock;
> #ifdef CONFIG_X86_LOCAL_APIC
Can we move those (and maybe everything in the context there too) up
*before* the check for no-kvmclock at the top of the function? Probably
in a separate patch.
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 01/41] x86/tsc: Add a standalone helpers for getting TSC info from CPUID.0x15
From: David Woodhouse @ 2026-05-20 18:59 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-2-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 755 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> Extract retrieval of TSC frequency information from CPUID into standalone
> helpers so that TDX guest support can reuse the logic. Provide a version
> that includes the multiplier math as TDX does NOT want to use
> native_calibrate_tsc()'s fallback logic that derives the TSC frequency
> based on CPUID.0x16, when the core crystal frequency isn't known.
>
> Opportunsitically drop native_calibrate_tsc()'s "== 0" and "!= 0" checks
> in favor of the kernel's preferred style.
"Opportunistically" ? Now that looks wrong too...
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v6 05/43] KVM: guest_memfd: Wire up kvm_get_memory_attributes() to per-gmem attributes
From: Sean Christopherson @ 2026-05-20 18:59 UTC (permalink / raw)
To: Fuad Tabba
Cc: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, ira.weiny, jmattson, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, willy, wyihan, yan.y.zhao, forkloop, pratyush,
suzuki.poulose, aneesh.kumar, liam, Paolo Bonzini,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Shuah Khan,
Vishal Annapurve, Andrew Morton, Chris Li, Kairui Song,
Kemeng Shi, Nhat Pham, Baoquan He, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Jason Gunthorpe, Vlastimil Babka, kvm,
linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <CA+EHjTw-cUM=FrJevtSDtR7K6MwUfGfOx21LMFDn7DAy5bFzYw@mail.gmail.com>
On Wed, May 20, 2026, Fuad Tabba wrote:
> On Thu, 7 May 2026 at 21:22, Ackerley Tng via B4 Relay
> <devnull+ackerleytng.google.com@kernel.org> wrote:
> >
> > From: Sean Christopherson <seanjc@google.com>
> >
> > Implement kvm_gmem_get_memory_attributes() for guest_memfd to allow the KVM
> > core and architecture code to query per-GFN memory attributes.
> >
> > kvm_gmem_get_memory_attributes() finds the memory slot for a given GFN and
> > queries the guest_memfd file's to determine if the page is marked as
> > private.
> >
> > If vm_memory_attributes is not enabled, there is no shared/private tracking
> > at the VM level. Install the guest_memfd implementation as long as
> > guest_memfd is enabled to give guest_memfd a chance to respond on
> > attributes.
> >
> > guest_memfd should look up attributes regardless of whether this memslot is
> > gmem-only since attributes are now tracked by gmem regardless of whether
> > mmap() is enabled.
> >
> > Signed-off-by: Sean Christopherson <seanjc@google.com>
> > Co-developed-by: Ackerley Tng <ackerleytng@google.com>
> > Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> > ---
> > include/linux/kvm_host.h | 2 ++
> > virt/kvm/guest_memfd.c | 31 +++++++++++++++++++++++++++++++
> > virt/kvm/kvm_main.c | 3 +++
> > 3 files changed, 36 insertions(+)
> >
> > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> > index c5ba2cb34e45c..28a54298d27db 100644
> > --- a/include/linux/kvm_host.h
> > +++ b/include/linux/kvm_host.h
> > @@ -2557,6 +2557,8 @@ bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
> > struct kvm_gfn_range *range);
> > #endif /* CONFIG_KVM_VM_MEMORY_ATTRIBUTES */
> >
> > +unsigned long kvm_gmem_get_memory_attributes(struct kvm *kvm, gfn_t gfn);
> > +
> > #ifdef CONFIG_KVM_GUEST_MEMFD
> > int kvm_gmem_get_pfn(struct kvm *kvm, struct kvm_memory_slot *slot,
> > gfn_t gfn, kvm_pfn_t *pfn, struct page **page,
> > diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> > index 5011d38820d0d..f055e058a3f28 100644
> > --- a/virt/kvm/guest_memfd.c
> > +++ b/virt/kvm/guest_memfd.c
> > @@ -509,6 +509,37 @@ static int kvm_gmem_mmap(struct file *file, struct vm_area_struct *vma)
> > return 0;
> > }
> >
> > +unsigned long kvm_gmem_get_memory_attributes(struct kvm *kvm, gfn_t gfn)
> > +{
> > + struct kvm_memory_slot *slot = gfn_to_memslot(kvm, gfn);
> > + struct inode *inode;
> > +
> > + /*
> > + * If this gfn has no associated memslot, there's no chance of the gfn
> > + * being backed by private memory, since guest_memfd must be used for
> > + * private memory, and guest_memfd must be associated with some memslot.
> > + */
> > + if (!slot)
> > + return 0;
> > +
> > + CLASS(gmem_get_file, file)(slot);
> > + if (!file)
> > + return 0;
> > +
> > + inode = file_inode(file);
> > +
> > + /*
> > + * Rely on the maple tree's internal RCU lock to ensure a
> > + * stable result. This result can become stale as soon as the
> > + * lock is dropped, so the caller _must_ still protect
> > + * consumption of private vs. shared by checking
> > + * mmu_invalidate_retry_gfn() under mmu_lock to serialize
> > + * against ongoing attribute updates.
> > + */
> > + return kvm_gmem_get_attributes(inode, kvm_gmem_get_index(slot, gfn));
> > +}
>
> Doesn't this imply that all consumers of kvm_mem_is_private() should
> validate the result using mmu_lock and the invalidation sequence?
> sev_handle_rmp_fault() calls kvm_mem_is_private() without holding
> mmu_lock and without any retry mechanism. Is that a problem?
Yes, but my understanding is that sev_handle_rmp_fault() can tolerate false
positives and false negatives. It's not optimal, but it's "fine", and already
KVM's existing behavior, e.g. KVM gets the PFN and then smashes the RMP, without
ensuring the PFN is fresh.
Mike, is that all correct?
^ permalink raw reply
* Re: [PATCH v3 41/41] x86/kvmclock: Get CPU base frequency from CPUID when it's available
From: David Woodhouse @ 2026-05-20 18:52 UTC (permalink / raw)
To: Sean Christopherson, Kiryl Shutsemau, Paolo Bonzini,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <20260515191942.1892718-42-seanjc@google.com>
[-- Attachment #1: Type: text/plain, Size: 1441 bytes --]
On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> If CPUID.0x16 is present and valid, use the CPU frequency provided by
> CPUID instead of assuming that the virtual CPU runs at the same
> frequency as TSC and/or kvmclock. Back before constant TSCs were a
> thing, treating the TSC and CPU frequencies as one and the same was
> somewhat reasonable, but now it's nonsensical, especially if the
> hypervisor explicitly enumerates the CPU frequency.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
> arch/x86/kernel/kvmclock.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index 62c8ea2e6769..7607920ae386 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -190,6 +190,20 @@ void kvmclock_cpu_action(enum kvm_guest_cpu_action action)
> }
> }
>
> +static unsigned long kvm_get_cpu_khz(void)
> +{
> + unsigned int cpu_khz;
> +
> + /*
> + * Prefer CPUID over kvmclock when possible, as the base CPU frequency
> + * isn't necessarily the same as the kvmlock "TSC" frequency.
> + */
> + if (!cpuid_get_cpu_freq(&cpu_khz))
> + return cpu_khz;
> +
> + return pvclock_tsc_khz(this_cpu_pvti());
I'm fine with this in principle but shouldn't the fallback be calling
kvm_get_tsc_khz() instead of directly calling pvclock_tsc_khz()?
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v3 40/41] x86/tsc: Add standalone helper for getting CPU frequency from CPUID
From: David Woodhouse @ 2026-05-20 18:50 UTC (permalink / raw)
To: Paolo Bonzini, Sean Christopherson, Kiryl Shutsemau,
K. Y. Srinivasan, Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li,
Ajay Kaher, Alexey Makhalov, Jan Kiszka, Dave Hansen,
Andy Lutomirski, Peter Zijlstra, Juergen Gross, Daniel Lezcano,
Thomas Gleixner, John Stultz
Cc: Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <3cb683b4-973a-4b3e-a5d5-a8baa8a70eb0@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 907 bytes --]
On Sat, 2026-05-16 at 09:42 +0200, Paolo Bonzini wrote:
> On 5/15/26 21:19, Sean Christopherson wrote:
> > Extract the guts of cpu_khz_from_cpuid() to a standalone helper that
> > doesn't restrict the usage to Intel CPUs. This will allow sharing the
> > core logic with kvmclock, as (a) CPUID.0x16 may be enumerated alongside
> > kvmclock, and (b) KVM generally doesn't restrict CPUID based on vendor.
>
> Even for native there's no real reason to restrict to Intel, I think.
> native_calibrate_tsc() only limits itself because historically (prior to
> commit 604dc9170f24, "x86/tsc: Use CPUID.0x16 to calculate missing
> crystal frequency", 2019-05-09) it used a hardcoded table of crystal
> frequencies.
>
> Of course paranoia applies, but for virtualization, if the leaf exists
> there is no reason not to trust it.
Agreed.
Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: SVSM Development Call May 20th, 2026
From: Jörg Rödel @ 2026-05-20 18:44 UTC (permalink / raw)
To: coconut-svsm, linux-coco
In-Reply-To: <agymbvQiKu17pQnQ@8bytes.org>
Meeting minutes are now ready:
https://github.com/coconut-svsm/governance/pull/108
-Joerg
^ permalink raw reply
* Re: [PATCH v3 00/41] x86: Try to wrangle PV clocks vs. TSC
From: David Woodhouse @ 2026-05-20 18:30 UTC (permalink / raw)
To: Sean Christopherson
Cc: Kiryl Shutsemau, Paolo Bonzini, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
Jan Kiszka, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, Thomas Gleixner, John Stultz,
Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <ag32mwLvHpWn2vCt@google.com>
[-- Attachment #1: Type: text/plain, Size: 648 bytes --]
On Wed, 2026-05-20 at 10:59 -0700, Sean Christopherson wrote:
>
> > And then it even spent some time at boot actually using the kvmclock as
> > clocksource... when ideally I don't think it would even have *enabled*
> > it at all?
>
> Yeah, that's definitely the ideal state. And I had all the same expectations and
> observations as you when digging in and testing this. But unless this series
> makes things worse, I want punt on achieving the ideal state for the moment, as
> it's proving to be a big lift just to get to a not-awful state.
Ack. Just checking my understanding was correct. Baby steps... 40 patches at a time :)
[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]
^ permalink raw reply
* Re: [PATCH v2 15/15] KVM: x86: Move the bulk of register specific code from x86.c to regs.c
From: Sean Christopherson @ 2026-05-20 18:11 UTC (permalink / raw)
To: Kai Huang
Cc: dwmw2@infradead.org, Rick P Edgecombe,
dave.hansen@linux.intel.com, binbin.wu@linux.intel.com,
vkuznets@redhat.com, x86@kernel.org, kas@kernel.org, paul@xen.org,
yosry@kernel.org, pbonzini@redhat.com, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, linux-kernel@vger.kernel.org
In-Reply-To: <4a918cb38dccd4ef7a2d84d16f5044472d069f7c.camel@intel.com>
On Wed, May 20, 2026, Kai Huang wrote:
> On Tue, 2026-05-19 at 18:25 -0700, Sean Christopherson wrote:
> > On Wed, May 20, 2026, Kai Huang wrote:
> > > I am not sure whether there's a mandatory requirement that "struct kvm_arch" and
> > > "struct kvm_vcpu_arch" must be fully embedded, and it would be kinda painful to
> > > covert to a pointer (e.g., there's kvm_x86_ops::vm_size), but perhaps that is
> > > also an option to consider?
> >
> > The idea I had in the past, and where I was going with things before s390's love
> > for arm64 came along, was to add a kvm_arch.h in arch/<arch>/kvm, and have virt/kvm
> > include _that_ instead of kvm_host.h.
> >
>
> Not sure whether there's other code doing so? :-)
>
> > That way we don't need to make any fundamental
> > changes to structures, but we can still significantly cut down on what's exposed
> > via kvm_host.h.
> >
>
> Yeah.
>
> I saw below from you in [1]:
>
> --
> We've explore several alternatives to the #ifdef __KVM__ approach, and
> they all sucked, hard. What I really wanted (and still want) to do, is to
> bury the bulk of kvm_host.h (and other KVM headers) in virt/kvm, but every
> attempt to do that ended in flames. Even with the __KVM__ guards in place,
> each architecture's kvm_host.h is too intertwined with the common kvm_host.h,
> and trying to extract small-ish pieces just doesn't work (each patch
> inevitably snowballed into a gigantic beast).
>
> The other idea we considered (which I thought of, and feel dirty for even
> proposing it internally), is to move all headers under virt/kvm, add
> virt/kvm/include to the global header path, and then have KVM x86 omit
> virt/kvm/include when configured to hide KVM internals. I hate this idea
> because it sets a bad precedent, and requires a lot of file movement
> without providing any benefit to other architectures. E.g. I hope that
> guarding KVM internals with #ifdef __KVM__ will allow us to slowly clean
> things up so that some day KVM only exposes a handful of APIs to the rest
> of the kernel (probably a pipe dream).
> --
>
> I haven't looked into details of your #ifdef __KVM__ approach yet, but seems you
> don't quite like moving KVM internal staff to virt/kvm/include/ ?
Not for arch code, which is the trickiest bit to handle.
> But if we want to hide KVM internal structures, I don't see any other options
> except virt/kvm/include/ is the place to go?
arch/$(ARCH)/kvm/kvm_arch.h is the obvious approach. Code in virt/kvm can reach
arch/$(ARCH)/kvm, we just need to add it to the include path. That's why I was
working on unifying the include definitions.
> Btw, have you considered reverting the inclusion of "strut kvm" and "struct
> kvm_arch" (and the vcpu structure), i.e., to make "struct kvm_arch" include
> "struct kvm"? I don't have any clue of whether it is feasible or how much
> effort it needs, though -- it's just something came to mind when replying.
>
> [1]: https://lore.kernel.org/all/20230916003118.2540661-1-seanjc@google.com/
>
> > At some point I'll try to take another look; it's really the
> > s390+arm64 combo that's problematic :-/
>
> If you want, I can take a look. I think I'll have bandwidth in near feature.
>
> Given you have tried multiple times so I am not sure what I can achieve, though.
Consolidating includes and creating arch/$(ARCH)/kvm/kvm_arch.h should be more
doable, what has failed spectacularly over and over is effectively trying to hide
most of asm/kvm_host.h, which sounds *really* stupid when I phrase it that way :-)
> Anyway, seems "allow loading a new (or old) KVM module without needing to
> rebuild and reboot the entire kernel" is a good reason to do this.
Note, we've scrapped upstreaming multi-KVM, and I don't see it ever getting accepted
upstream, as live update is far superior, e.g. isn't limited to just KVM, doesn't
have the same orchestration or testing problems, etc.
And FWIW, today, you _can_ reload a new (or old) KVM module without rebuilding
the kernel or rebooting the host, it's just that you can't modify certain structures.
^ permalink raw reply
* Re: [PATCH v3 00/41] x86: Try to wrangle PV clocks vs. TSC
From: Sean Christopherson @ 2026-05-20 17:59 UTC (permalink / raw)
To: David Woodhouse
Cc: Kiryl Shutsemau, Paolo Bonzini, K. Y. Srinivasan, Haiyang Zhang,
Wei Liu, Dexuan Cui, Long Li, Ajay Kaher, Alexey Makhalov,
Jan Kiszka, Dave Hansen, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, Thomas Gleixner, John Stultz,
Rick Edgecombe, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, x86, linux-coco, kvm, linux-hyperv, virtualization,
linux-kernel, xen-devel, Michael Kelley, Tom Lendacky,
Nikunj A Dadhania, Thomas Gleixner
In-Reply-To: <7260682b21c28d1299e58400b9a2f4b8d23bd434.camel@infradead.org>
On Tue, May 19, 2026, David Woodhouse wrote:
> On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> > Dave/Thomas/Peter/Boris, what's the going rate for bribes to take something
> > like this through the tip tree?
> >
> > The bulk of the changes are in kvmclock and TSC, but pretty much every
> > hypervisor's guest-side code gets touched at some point. I am reaonsably
> > confident in the correctness of the KVM changes. Michael tested Hyper-V in
> > v2, and while there were conflicts when rebasing, they were largely
> > superficial (and I've just jinxed myself). For all other hypervisors, assume
> > the code is compile-tested only, but those changes are all quite small and
> > straightforward.
> >
> > The only changes that are questionable/contentious are the last two patches,
> > which have KVM-as-a-guest use CPUID 0x16 to get the CPU frequency, even on
> > AMD (that's the dubious part). I very deliberately put them last, so that
> > they can be dropped at will (I don't care terribly if those patches land).
> > To merge them, I would want explicit Acks from Paolo and David W.
> >
> > So, except for the last two patches, to get the stuff I really care about
> > landed, I think/hope it's just the TSC and guest-side CoCo changes that need
> > reviews/acks?
> >
> > The primary goal of this series is (or at least was, when I started) to
> > fix flaws with SNP and TDX guests where a PV clock provided by the untrusted
> > hypervisor is used instead of the secure/trusted TSC that is controlled by
> > trusted firmware.
> >
> > The secondary goal is to draft off of the SNP and TDX changes to slightly
> > modernize running under KVM. Currently, KVM guests will use TSC for
> > clocksource, but not sched_clock. And they ignore Intel's CPUID-based TSC
> > and CPU frequency enumeration, even when using the TSC instead of kvmclock.
> > And if the host provides the core crystal frequency in CPUID.0x15, then KVM
> > guests can use that for the APIC timer period instead of manually calibrating
> > the frequency.
> >
> > The tertiary goal is to clean up all of the PV clock code to deduplicate logic
> > across hypervisors, and to hopefully make it all easier to maintain going
> > forward.
>
> I booted this in qemu with -cpu host,+invtsc,+vmware-cpuid-freq
>
> I was expecting to see it eschew the kvmclock and use *only* the TSC.
> Is there even any need for 'tsc-early' given that it's *told* the TSC
> frequency in CPUID? Shouldn't it have detected that the TSC is known
> before init_tsc_clocksource() runs?
>
> And then it even spent some time at boot actually using the kvmclock as
> clocksource... when ideally I don't think it would even have *enabled*
> it at all?
Yeah, that's definitely the ideal state. And I had all the same expectations and
observations as you when digging in and testing this. But unless this series
makes things worse, I want punt on achieving the ideal state for the moment, as
it's proving to be a big lift just to get to a not-awful state.
> [ 0.000000] clocksource: kvm-clock: mask: 0xffffffffffffffff max_cycles: 0x1cd42e4dffb, max_idle_ns: 881590591483 ns
> [ 0.000000] tsc: Detected 2400.000 MHz processor
> [ 0.008205] TSC deadline timer available
> [ 0.008270] clocksource: refined-jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 1910969940391419 ns
> [ 0.159085] clocksource: hpet: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 19112604467 ns
> [ 0.164074] clocksource: tsc-early: mask: 0xffffffffffffffff max_cycles: 0x22983777dd9, max_idle_ns: 440795300422 ns
> [ 0.229087] clocksource: jiffies: mask: 0xffffffff max_cycles: 0xffffffff, max_idle_ns: 1911260446275000 ns
> [ 0.337095] clocksource: Switched to clocksource kvm-clock
> [ 0.345246] clocksource: acpi_pm: mask: 0xffffff max_cycles: 0xffffff, max_idle_ns: 2085701024 ns
> [ 0.356201] clocksource: tsc: mask: 0xffffffffffffffff max_cycles: 0x22983777dd9, max_idle_ns: 440795300422 ns
> [ 0.360560] clocksource: Switched to clocksource tsc
>
^ permalink raw reply
* Re: [PATCH v3 02/41] x86/tsc: Add helper to register CPU and TSC freq calibration routines
From: Sean Christopherson @ 2026-05-20 17:56 UTC (permalink / raw)
To: David Woodhouse
Cc: tglx@kernel.org, longli@microsoft.com, luto@kernel.org,
alexey.makhalov@broadcom.com, jstultz@google.com,
dave.hansen@linux.intel.com, ajay.kaher@broadcom.com,
jan.kiszka@siemens.com, haiyangz@microsoft.com, kas@kernel.org,
pbonzini@redhat.com, kys@microsoft.com, decui@microsoft.com,
daniel.lezcano@kernel.org, wei.liu@kernel.org,
peterz@infradead.org, jgross@suse.com, boris.ostrovsky@oracle.com,
linux-coco@lists.linux.dev, kvm@vger.kernel.org,
mhklinux@outlook.com, thomas.lendacky@amd.com,
linux-kernel@vger.kernel.org,
bcm-kernel-feedback-list@broadcom.com, tglx@linutronix.de,
nikunj@amd.com, xen-devel@lists.xenproject.org,
linux-hyperv@vger.kernel.org, vkuznets@redhat.com,
rick.p.edgecombe@intel.com, virtualization@lists.linux.dev,
sboyd@kernel.org, x86@kernel.org
In-Reply-To: <949e39aec749f019b18fa41c2a42bcc9231288b9.camel@amazon.co.uk>
On Mon, May 18, 2026, David Woodhouse wrote:
> On Fri, 2026-05-15 at 12:19 -0700, Sean Christopherson wrote:
> >
> > --- a/arch/x86/xen/time.c
> > +++ b/arch/x86/xen/time.c
> > @@ -569,7 +569,7 @@ static void __init xen_init_time_common(void)
> > static_call_update(pv_steal_clock, xen_steal_clock);
> > paravirt_set_sched_clock(xen_sched_clock);
> >
> > - x86_platform.calibrate_tsc = xen_tsc_khz;
> > + tsc_register_calibration_routines(xen_tsc_khz, NULL);
> > x86_platform.get_wallclock = xen_get_wallclock;
> > }
> >
>
> xen_tsc_khz() doesn't use CPUID but really *should*.
>
> Care to pull in
> https://lore.kernel.org/all/20260509224824.3264567-31-dwmw2@infradead.org/
> to your next round please?
>
> (Without the misplaced changes in kvm/x86.c that should have been in
> two different prior commits, and are now folded into those correctly in
> my kvmclock5 branch ready for the next posting of that).
Ya, will do. What's one more patch...
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox