* [PATCH v5 04/51] x86/tsc: Restrict recalibrate_cpu_khz() export to p4-clockmod and powernow-k7
From: Sean Christopherson @ 2026-07-01 19:31 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Export recalibrate_cpu_khz() only for its two users, p4-clockmod.ko and
powernow-k7.ko, to help document that recalibration is relevant only to
ancient CPUs.
For all intents and purposes, no functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/tsc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 4393902c0ddd..482cc3a8999a 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -943,7 +943,7 @@ void recalibrate_cpu_khz(void)
cpu_khz_old, cpu_khz);
#endif
}
-EXPORT_SYMBOL_GPL(recalibrate_cpu_khz);
+EXPORT_SYMBOL_FOR_MODULES(recalibrate_cpu_khz, "p4-clockmod,powernow-k7");
static unsigned long long cyc2ns_suspend;
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 03/51] x86/tsc: Ensure that TSC recalibration doesn't run if TSC frequency is known
From: Sean Christopherson @ 2026-07-01 19:31 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
When attempting TSC recalibration post-boot, which is only done for ancient
CPUS (P4 and K7) on SMP=n kernels, assert that the TSC frequency isn't
known (explicitly provided by hardware) by way of MSR or CPUID, and bail if
the impossible happens. In practice, recalibration and TSC_KNOWN_FREQ are
mutually exclusive, as TSC_KNOWN_FREQ will only be set when running on
hardware that was released decades after recalibration was obsoleted, but
but it's hard to see that, especially when looking at just the TSC code.
Note, the WARN can likely be tripped by running in a virtual machine and
concocting an impossible CPU model, e.g. by combining a P4 signature with
CPUID 0x15. This is working as intended, as such a virtual CPU model is
wildly out-of-spec and is not supported.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kernel/tsc.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 4d6a446645c0..4393902c0ddd 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -930,6 +930,9 @@ void recalibrate_cpu_khz(void)
if (!boot_cpu_has(X86_FEATURE_TSC))
return;
+ if (WARN_ON_ONCE(cpu_feature_enabled(X86_FEATURE_TSC_KNOWN_FREQ)))
+ return;
+
cpu_khz = x86_platform.calibrate_cpu();
tsc_khz = x86_platform.calibrate_tsc();
if (tsc_khz == 0)
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 02/51] x86/apic: Add CONFIG_X86_LOCAL_APIC=n stubs for apic_set_timer_period_{,k}hz()
From: Sean Christopherson @ 2026-07-01 19:31 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Add stubs for the apic_set_timer_period_{,k}hz() APIs when the kernel is
built without support for a local APIC, and drop #ifdefs in callers that
don't need to check CONFIG_X86_LOCAL_APIC for other reasons.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/apic.h | 2 ++
arch/x86/kernel/cpu/vmware.c | 2 --
arch/x86/kernel/tsc.c | 2 --
arch/x86/kernel/tsc_msr.c | 2 --
4 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index cd84a94688a2..035998555e99 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -189,6 +189,8 @@ static inline void disable_local_APIC(void) { }
# define setup_boot_APIC_clock x86_init_noop
# define setup_secondary_APIC_clock x86_init_noop
static inline void lapic_update_tsc_freq(void) { }
+static inline void apic_set_timer_period_hz(u64 period_hz, const char *source) { }
+static inline void apic_set_timer_period_khz(u64 period_khz, const char *source) { }
static inline void init_bsp_APIC(void) { }
static inline void apic_intr_mode_select(void) { }
static inline void apic_intr_mode_init(void) { }
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 36f779dd311d..13b97265c535 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -422,10 +422,8 @@ static void __init vmware_platform_setup(void)
x86_platform.calibrate_tsc = vmware_get_tsc_khz;
x86_platform.calibrate_cpu = vmware_get_tsc_khz;
-#ifdef CONFIG_X86_LOCAL_APIC
/* Skip lapic calibration since we know the bus frequency. */
apic_set_timer_period_hz(ecx, "VMware hypervisor");
-#endif
} else {
pr_warn("Failed to get TSC freq from the hypervisor\n");
}
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index f9ecc9256863..4d6a446645c0 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -710,7 +710,6 @@ unsigned long native_calibrate_tsc(void)
if (boot_cpu_data.x86_vfm == INTEL_ATOM_GOLDMONT)
setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
-#ifdef CONFIG_X86_LOCAL_APIC
/*
* The local APIC appears to be fed by the core crystal clock
* (which sounds entirely sensible). We can set the global
@@ -718,7 +717,6 @@ unsigned long native_calibrate_tsc(void)
* timer later.
*/
apic_set_timer_period_khz(crystal_khz, "CPUID 0x15/0x16");
-#endif
return crystal_khz * ebx_numerator / eax_denominator;
}
diff --git a/arch/x86/kernel/tsc_msr.c b/arch/x86/kernel/tsc_msr.c
index 7e990871e041..aece062aee7e 100644
--- a/arch/x86/kernel/tsc_msr.c
+++ b/arch/x86/kernel/tsc_msr.c
@@ -210,9 +210,7 @@ unsigned long cpu_khz_from_msr(void)
if (freq == 0)
pr_err("Error MSR_FSB_FREQ index %d is unknown\n", index);
-#ifdef CONFIG_X86_LOCAL_APIC
apic_set_timer_period_khz(freq, "MSR_FSB_FREQ");
-#endif
/*
* TSC frequency determined by MSR is always considered "known"
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 01/51] x86/apic: Provide helpers to set local APIC timer period in hz and khz
From: Sean Christopherson @ 2026-07-01 19:31 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
In-Reply-To: <20260701193212.749551-1-seanjc@google.com>
Add and use APIs to set the local APIC timer period instead of open coding
the subtle HZ math in a all external callers, and make lapic_timer_period
local to apic.c. Provide APIs to specify the frequency in both hertz and
kilohertz so that Hyper-V and VMware code aren't forced to lose precision.
Opportunistically use mul_u64_u32_div() to harden against the possibility
that the period in Khz is greater than 4294967, i.e. if the APIC timer runs
at ~4.29 GHz. As pointed out by Sashiko, 4294968 * 1000 == 0x1_000002c0,
and thus a Khz period of 4294968 would silently overflow the 32-bit
unsigned integer used by most callers.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/apic.h | 3 ++-
arch/x86/kernel/apic/apic.c | 12 +++++++++++-
arch/x86/kernel/cpu/mshyperv.c | 5 +----
arch/x86/kernel/cpu/vmware.c | 4 +---
arch/x86/kernel/jailhouse.c | 2 +-
arch/x86/kernel/tsc.c | 2 +-
arch/x86/kernel/tsc_msr.c | 2 +-
7 files changed, 18 insertions(+), 12 deletions(-)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 9cd493d467d4..cd84a94688a2 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -63,7 +63,6 @@ extern int apic_verbosity;
extern int local_apic_timer_c2_ok;
extern bool apic_is_disabled;
-extern unsigned int lapic_timer_period;
extern enum apic_intr_mode_id apic_intr_mode;
enum apic_intr_mode_id {
@@ -138,6 +137,8 @@ void register_lapic_address(unsigned long address);
extern void setup_boot_APIC_clock(void);
extern void setup_secondary_APIC_clock(void);
extern void lapic_update_tsc_freq(void);
+extern void apic_set_timer_period_hz(u64 period_hz, const char *source);
+extern void apic_set_timer_period_khz(u64 period_khz, const char *source);
#ifdef CONFIG_X86_64
static inline bool apic_force_enable(unsigned long addr)
diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c
index aa1e19979aa8..8d3d930576fd 100644
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -176,7 +176,7 @@ static struct resource lapic_resource = {
};
/* Measured in ticks per HZ. */
-unsigned int lapic_timer_period = 0;
+static unsigned int lapic_timer_period;
static void apic_pm_activate(void);
@@ -796,6 +796,16 @@ bool __init apic_needs_pit(void)
return lapic_timer_period == 0;
}
+void apic_set_timer_period_khz(u64 period_khz, const char *source)
+{
+ lapic_timer_period = mul_u64_u32_div(period_khz, 1000, HZ);
+}
+
+void apic_set_timer_period_hz(u64 period_hz, const char *source)
+{
+ lapic_timer_period = div_u64(period_hz, HZ);
+}
+
static int __init calibrate_APIC_clock(void)
{
struct clock_event_device *levt = this_cpu_ptr(&lapic_events);
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 185d4f677ec0..87beecec76f0 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -646,10 +646,7 @@ static void __init ms_hyperv_init_platform(void)
u64 hv_lapic_frequency;
rdmsrq(HV_X64_MSR_APIC_FREQUENCY, hv_lapic_frequency);
- hv_lapic_frequency = div_u64(hv_lapic_frequency, HZ);
- lapic_timer_period = hv_lapic_frequency;
- pr_info("Hyper-V: LAPIC Timer Frequency: %#x\n",
- lapic_timer_period);
+ apic_set_timer_period_hz(hv_lapic_frequency, "Hyper-V hypervisor");
}
register_nmi_handler(NMI_UNKNOWN, hv_nmi_unknown, NMI_FLAG_FIRST,
diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 34b73573b108..36f779dd311d 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -424,9 +424,7 @@ static void __init vmware_platform_setup(void)
#ifdef CONFIG_X86_LOCAL_APIC
/* Skip lapic calibration since we know the bus frequency. */
- lapic_timer_period = ecx / HZ;
- pr_info("Host bus clock speed read from hypervisor : %u Hz\n",
- ecx);
+ apic_set_timer_period_hz(ecx, "VMware hypervisor");
#endif
} else {
pr_warn("Failed to get TSC freq from the hypervisor\n");
diff --git a/arch/x86/kernel/jailhouse.c b/arch/x86/kernel/jailhouse.c
index f58ce9220e0f..f2d4ef89c085 100644
--- a/arch/x86/kernel/jailhouse.c
+++ b/arch/x86/kernel/jailhouse.c
@@ -65,7 +65,7 @@ static void jailhouse_get_wallclock(struct timespec64 *now)
static void __init jailhouse_timer_init(void)
{
- lapic_timer_period = setup_data.v1.apic_khz * (1000 / HZ);
+ apic_set_timer_period_khz(setup_data.v1.apic_khz, "Jailhouse hypervisor");
}
static unsigned long jailhouse_get_tsc(void)
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index ce10ae4b298b..f9ecc9256863 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -717,7 +717,7 @@ unsigned long native_calibrate_tsc(void)
* lapic_timer_period here to avoid having to calibrate the APIC
* timer later.
*/
- lapic_timer_period = crystal_khz * 1000 / HZ;
+ apic_set_timer_period_khz(crystal_khz, "CPUID 0x15/0x16");
#endif
return crystal_khz * ebx_numerator / eax_denominator;
diff --git a/arch/x86/kernel/tsc_msr.c b/arch/x86/kernel/tsc_msr.c
index 48e6cc1cb017..7e990871e041 100644
--- a/arch/x86/kernel/tsc_msr.c
+++ b/arch/x86/kernel/tsc_msr.c
@@ -211,7 +211,7 @@ unsigned long cpu_khz_from_msr(void)
pr_err("Error MSR_FSB_FREQ index %d is unknown\n", index);
#ifdef CONFIG_X86_LOCAL_APIC
- lapic_timer_period = (freq * 1000) / HZ;
+ apic_set_timer_period_khz(freq, "MSR_FSB_FREQ");
#endif
/*
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH v5 00/51] x86: Try to wrangle PV clocks vs. TSC
From: Sean Christopherson @ 2026-07-01 19:31 UTC (permalink / raw)
To: Jonathan Corbet, Paolo Bonzini, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, Dave Hansen, x86, Kiryl Shutsemau,
Rick Edgecombe, Sean Christopherson, K. Y. Srinivasan,
Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Ajay Kaher,
Alexey Makhalov, Jan Kiszka, Andy Lutomirski, Peter Zijlstra,
Juergen Gross, Daniel Lezcano, John Stultz
Cc: Shuah Khan, H. Peter Anvin, Vitaly Kuznetsov,
Broadcom internal kernel review list, Boris Ostrovsky,
Stephen Boyd, linux-doc, kvm, linux-kernel, linux-coco,
linux-hyperv, virtualization, xen-devel, Tom Lendacky,
Nikunj A Dadhania, David Woodhouse, David Woodhouse,
Michael Kelley, Thomas Gleixner
The primary goal of this series to fix flaws with SNP and TDX guests where a
PV clock provided by the untrusted hypervisor is used instead of the secure
TSC that is controlled by trusted firmware.
The secondary goal is modernize running under KVM. Currently, KVM guests will
use TSC for clocksource, but not sched_clock. And Linux-as-a-KVM-guest doesn't
support paravirt enumeration of the TSC/APIC frequencies, even though QEMU
provides that information by default.
The tertiary goal is to clean up the PV clock code to deduplicate logic across
hypervisors, and to hopefully make it all easier to maintain going forward.
The quaternary goal is to clean up the TSC calibration code, which was made
stupidly hard to follow by hypervisor code mixing in with the native
calibration routines, instead of being implemented as a pure alternative.
Note, the VMware and Xen changes still probably should get acks from those
maintainers, as my understanding of what they're trying to do may be flawed.
Lots more background on the SNP/TDX motiviation:
https://lore.kernel.org/all/20250106124633.1418972-13-nikunj@amd.com
As before, I deliberately omitted jailhouse-dev@googlegroups.com from the To/Cc,
as those emails bounced on v1, AFAICT nothing has changed.
v5:
- Use cpu_feature_enabled() instead of boot_cpu_has(). [Boris]
- WARN if recalibrate_cpu_khz() runs on a system with TSC_KNOWN_FREQ. [Thomas]
- Opportunistically drop a line break in native_calibrate_tsc(). [Thomas]
- Rely on callers of cpuid_get_tsc_info() to check the result instead of
unnecessarily zeroing the structure. [Boris]
- Ignore tsc_early_khz if the TSC frequency is provided by trusted firmware
or by the hypervisor. [Thomas, Sashiko]
- Cache CPUID output in acrn_init_platform() to avoid introducing a transient
bug where TSC_KNOWN_FREQ could be set even if the ACRN hypervisor didn't
actually provide the frequency. [Sashiko]
- Drop kvmclock's useless/dead check_tsc_unstable() call (it occurs before the
command line parameter is parsed). [Sashiko]
- Add helpers to set lapic_timer_period, to fix not-so-theoretical overflow
in the various "khz * 1000 / HZ" patterns. [Sashiko]
- Drop the "x86/xen: Obtain TSC frequency from CPUID if present" patch as it
doesn't have any dependencies/conflicts on/with this series, and Sashiko had
concerns about the assumptions it was making. [Sashiko]
- Collect reviews. [David] (Kirill's got dropped because the patch he reviewed
got completely rewritten).
v4:
- Use x86_init_noop() to skip save/restore on VMware and Xen instead of
nullifying x86_platform.{save,restore}_sched_clock_state. [Sashiko]
- Use '0' to indicate "failure" when getting the CPU frequency from CPUID, to
avoid using an out-param and thus make it all but impossible to
unintentionally clobber the global cpu_khz (which v3 did). [Sashiko]
- Rename cpuid_get_cpu_freq() => __cpu_khz_from_cpuid() to capture its
relationship with cpu_khz_from_cpuid().
- Compute lapic_timer_period in units of ticks, not Khz. [Sashiko]
- Kill off x86_platform_ops.calibrate_{cpu,tsc}(), and instead use dedicated
hooks for hypervisor code, and direct calls for TDX and SNP. [David, loosely]
- Drop SNP's secure TSC override of _CPU_ calibration, as there's zero
evidence it's justified or a net positive.
- Collect reviews/acks. [David, Wei]
- Decouple getting TSC/APIC frequencies from KVM PV CPUID from kvmclock. [David]
- Fix an amusing number of Opportunistically misspellings. [David]
- Set kvm_sched_clock_offset _before_ registering kvmclock as sched_clock,
and add a comment to guard against future goofs. [Sashiko]
- Keep "setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE)" in Hyper-V's handling
of HV_ACCESS_TSC_INVARIANT, as it's technically possible to have a VM
with HV_ACCESS_TSC_INVARIANT but not HV_ACCESS_FREQUENCY_MSRS. Though as
a _very_ nice side effect of using dedicated sequencing for selecting the
TSC frequency source, this would have naturally happened anyways. [Sashiko]
v3:
- https://lore.kernel.org/all/20260515191942.1892718-1-seanjc@google.com
- Collect reviews. [Michael, Thomas]
- Use Hyper-V reference counter / refcounter instead of Hyper-V timer. [Michael]
- Use the paravirt CPUID interface first proposed by VMware for KVM's
"official" mechanism for communicating frequency to KVM-aware guests,
instead of abusing Intel's CPUID leafs. [David]
- Deal with paravirt code being moved into asm/timers.h and
arch/x86/kernel/tsc.c.
v2:
- https://lore.kernel.org/all/Z8YWttWDtvkyCtdJ@google.com
- Add struct to hold the TSC CPUID output. [Boris]
- Don't pointlessly inline the TSC CPUID helpers. [Boris]
- Fix a variable goof in a helper, hopefully for real this time. [Dan]
- Collect reviews. [Nikunj]
- Override the sched_clock save/restore hooks if and only if a PV clock
is successfully registered.
- During resome, restore clocksources before reading persistent time.
- Clean up more warts created by kvmclock.
- Fix more bugs in kvmclock's suspend/resume handling.
- Try to harden kvmclock against future bugs.
v1: https://lore.kernel.org/all/20250201021718.699411-1-seanjc@google.com
David Woodhouse (2):
KVM: x86: Officially define CPUID 0x40000010 as PV Timing Info (TSC
and Bus)
x86/kvm: Obtain TSC frequency from PV CPUID if present
Sean Christopherson (49):
x86/apic: Provide helpers to set local APIC timer period in hz and khz
x86/apic: Add CONFIG_X86_LOCAL_APIC=n stubs for
apic_set_timer_period_{,k}hz()
x86/tsc: Ensure that TSC recalibration doesn't run if TSC frequency is
known
x86/tsc: Restrict recalibrate_cpu_khz() export to p4-clockmod and
powernow-k7
x86/sev: Mark TSC as reliable when configuring Secure TSC
x86/sev: Don't override CPU frequency calibration for SNP's Secure TSC
x86/sev: Move check for SNP Secure TSC support to tsc_early_init()
x86/sev: Shove SNP's secure/trusted TSC frequency directly into
"calibration"
x86/tsc: Add a standalone helper for getting TSC info from CPUID.0x15
x86/tdx: Force TSC frequency with CPUID-based info provided by the
TDX-Module
x86/tsc: Add dedicated hypervisor hooks for getting known TSC/CPU
frequencies
x86/acrn: Register TSC/CPU frequency callbacks iff frequency is
actually in CPUID
x86/acrn: Mark TSC frequency as known when using ACRN for calibration
x86/tsc: Consolidate forcing of X86_FEATURE_TSC_KNOWN_FREQ for PV code
x86/tsc: Kill off x86_platform_ops.calibrate_{cpu,tsc}() hooks
x86/tsc: Rename pit_hpet_ptimer_calibrate_cpu() =>
native_calibrate_cpu_late()
x86/tsc: Fold native_calibrate_cpu() into recalibrate_cpu_khz()
x86/kvmclock: Rename kvm_get_tsc_khz() to kvmclock_get_tsc_khz()
x86/kvmclock: Drop dead check on TSC being unstable during
kvmclock_init()
x86/kvm: Mark TSC as reliable when it's constant and nonstop
x86/tsc: Add standalone helper for getting CPU frequency from CPUID
x86/kvm: Get CPU base frequency from CPUID when it's available
clocksource: hyper-v: Register sched_clock save/restore iff it's
necessary
clocksource: hyper-v: Drop wrappers to sched_clock save/restore
helpers
clocksource: hyper-v: Don't save/restore TSC offset when using HV
sched_clock
x86/kvmclock: Setup kvmclock for secondary CPUs iff CONFIG_SMP=y
x86/kvm: Don't disable kvmclock on BSP in syscore_suspend()
x86/paravirt: Remove unnecessary PARAVIRT=n stub for
paravirt_set_sched_clock()
x86/paravirt: Move handling of unstable PV clocks into
paravirt_set_sched_clock()
x86/kvmclock: Move sched_clock save/restore helpers up in kvmclock.c
x86/xen/time: NOP-ify x86_platform's sched_clock save/restore hooks
x86/vmware: NOP-ify save/restore hooks when using VMware's sched_clock
x86/tsc: WARN if TSC sched_clock save/restore used with PV sched_clock
x86/paravirt: Pass sched_clock save/restore helpers during
registration
x86/kvmclock: Move kvm_sched_clock_init() down in kvmclock.c
x86/xen/time: Mark xen_setup_vsyscall_time_info() as __init
x86/pvclock: Mark setup helpers and related various as
__init/__ro_after_init
x86/pvclock: WARN if pvclock's valid_flags are overwritten
x86/kvmclock: Refactor handling of PVCLOCK_TSC_STABLE_BIT during
kvmclock_init()
timekeeping: Resume clocksources before reading persistent clock
x86/kvmclock: Hook clocksource.suspend/resume when kvmclock isn't
sched_clock
x86/kvmclock: WARN if wall clock is read while kvmclock is suspended
x86/paravirt: Mark __paravirt_set_sched_clock() as __init
x86/paravirt: Plumb a return code into __paravirt_set_sched_clock()
x86/paravirt: Don't use a PV sched_clock in CoCo guests with trusted
TSC
x86/kvmclock: Use TSC for sched_clock if it's constant and non-stop
x86/kvmclock: Plumb in AP-online and BSP-resume to kvmlock, for
documentation
x86/paravirt: Move using_native_sched_clock() stub into timer.h
x86/kvm: Get local APIC bus frequency from PV CPUID Timing Info
.../admin-guide/kernel-parameters.txt | 5 +
Documentation/virt/kvm/x86/cpuid.rst | 12 +
arch/x86/coco/sev/core.c | 21 +-
arch/x86/coco/tdx/tdx.c | 19 +-
arch/x86/include/asm/acrn.h | 5 -
arch/x86/include/asm/apic.h | 5 +-
arch/x86/include/asm/kvm_para.h | 12 +-
arch/x86/include/asm/sev.h | 4 +-
arch/x86/include/asm/tdx.h | 2 +
arch/x86/include/asm/timer.h | 15 +-
arch/x86/include/asm/tsc.h | 10 +-
arch/x86/include/asm/x86_init.h | 8 +-
arch/x86/include/uapi/asm/kvm_para.h | 11 +
arch/x86/kernel/apic/apic.c | 12 +-
arch/x86/kernel/cpu/acrn.c | 14 +-
arch/x86/kernel/cpu/mshyperv.c | 70 +-----
arch/x86/kernel/cpu/vmware.c | 19 +-
arch/x86/kernel/jailhouse.c | 9 +-
arch/x86/kernel/kvm.c | 101 ++++++--
arch/x86/kernel/kvmclock.c | 208 +++++++++++------
arch/x86/kernel/pvclock.c | 9 +-
arch/x86/kernel/tsc.c | 218 +++++++++++-------
arch/x86/kernel/tsc_msr.c | 4 +-
arch/x86/kernel/x86_init.c | 2 -
arch/x86/mm/mem_encrypt_amd.c | 3 -
arch/x86/xen/time.c | 14 +-
drivers/clocksource/hyperv_timer.c | 38 ++-
include/clocksource/hyperv_timer.h | 2 -
kernel/time/timekeeping.c | 9 +-
29 files changed, 540 insertions(+), 321 deletions(-)
base-commit: dc59e4fea9d83f03bad6bddf3fa2e52491777482
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply
* Re: [PATCH v2 16/17] KVM: TDX: Add in-kernel Quote generation
From: Edgecombe, Rick P @ 2026-07-01 18:45 UTC (permalink / raw)
To: seanjc@google.com, djbw@kernel.org
Cc: kvm@vger.kernel.org, linux-coco@lists.linux.dev, Li, Xiaoyao,
Hansen, Dave, dave.hansen@linux.intel.com,
baolu.lu@linux.intel.com, Hunter, Adrian, kas@kernel.org,
tony.lindgren@linux.intel.com, Xu, Yilun,
linux-kernel@vger.kernel.org, Mehta, Sohil, Fang, Peter,
Duan, Zhenzhong, Maloor, Kishen, yilun.xu@linux.intel.com,
x86@kernel.org
In-Reply-To: <akVNdxCzk-n4bgMv@google.com>
On Wed, 2026-07-01 at 10:25 -0700, Sean Christopherson wrote:
> > > That is a good question. The answer is partly historical reasons, but I
> > > think the pros/cons don’t really move the needle too much.
> > >
> > > The main benefit of doing it with the host in the loop is that the guest
> > > side TDVMCALL quoting interface can stay the same. There is also a wrinkle
> > > in that there is a limited HW resource involved in the quoting,
>
> What is this magical resource?
It's a HW crypto thing. I'll let Peter explain more.
>
> > > so we want to do these operations one at a time. Having a mutex on the
> > > host is the KISS way of accomplishing some level of fairness for DOS
> > > prevention.
>
> At the risk of unintentionally causing effecitvely DoS by introducing a
> system-wide lock.
The DoS that people are interested in trying to prevent is a TD getting starved
of quotes forever. So the lock is supposed to give some eventual fairness.
Eventually the guest gets its quote even if it has to wait.
If instead contention throws a BUSY code to the guest, the guest spins trying.
That wouldn't have the guarantees.
So I think the two options are: have some lock in the TDX module, or have a lock
of some sort in the host. I think actually a waiting lock in the TDX module is
possible. But I think there are tradeoffs besides where the extra code is. If
it's in Linux we get a lot more control, lockdep, etc.
BTW the "historical reasons" part I mentioned involves a past effort to create
configurable host controlled policies around managing these resources. So part
of the simplification is doing a simple eventually fair global lock instead of
something more complicated.
>
> > > We should've explained this more, but TBH this solution is *way* simpler
> > > than the initial one that never saw the light of day. So this extra host
> > > work seemed quite small compared to what we have been staring at and we
> > > kinda overlooked it.
>
> Simpler for what?
Linux of course.
>
> > > The other relevant tidbit is that the TDX module folks have some problems
> > > to solve before they can support TDG calls to TDX module extensions. I
> > > think we can get them to though. The question is probably really: do we
> > > want the guest trying/selecting multiple interfaces, or the host.
> >
> > tl;dr: FWIW, host, if only because this is one of many blob transports
> > across multiple archs that need host coordination.
> >
> > While TDG calls to do the same helps a TDX problem it does not generally
> > reduce the Linux problem of multiple archs having multiple blob protocols
> > to shuffle data from host to guest in shared memory.
>
> Why is that KVM's problem?
I'm slightly lost on this too, so will leave it to Dan.
>
> > The current direction for other archs (CCA, SEV-SNP) and blobs (e.g. PCI
> > attestation collateral) is shift the burden from arch/*/kvm/ to
> > drivers/. There might be a later opportunity to consolidate some of
> > those drivers' internals on a cross-arch generic transport (AF_VSOCK),
> > but there remains a requirement to talk to the host.
>
> Why?
>
> Bluntly, this series is unreviewable. I've speed read through most of the
> patches, and I have *zero* clue as to *why* any of the design decisions were
> made. E.g. why is there a common buffer that's apparently shared by all CPUs
> and VMs, only for the API to immediately allocate per-quote memory and copy
> out the buffer?
We can keep discussing it, but...
>
> The changelogs are all heavy on the "what", but explanations of why any of
> this is being done, and in the exact way it's being done, are non-existent.
>
> So, back up to the beginning and explain why I should spend any time
> deciphering this series, because as-is I don't have the interest or the time.
...it sounds like it would be better if we tried again with a proper
explanation, rather than try to back out of where the current presentation left
you?
This should be a pretty simple feature. But there is a higher level theme of
what is responsible for locks and scheduling in the overall TDX solution. This
spills into a lot of future stuff. We can try to find the right level.
^ permalink raw reply
* Re: [PATCH v2 16/17] KVM: TDX: Add in-kernel Quote generation
From: Sean Christopherson @ 2026-07-01 17:25 UTC (permalink / raw)
To: Dan Williams (nvidia)
Cc: Rick P Edgecombe, Peter Fang, kvm@vger.kernel.org,
linux-coco@lists.linux.dev, Xiaoyao Li, Dave Hansen,
dave.hansen@linux.intel.com, baolu.lu@linux.intel.com,
Adrian Hunter, kas@kernel.org, linux-kernel@vger.kernel.org,
Yilun Xu, tony.lindgren@linux.intel.com, Sohil Mehta,
Zhenzhong Duan, Kishen Maloor, yilun.xu@linux.intel.com,
x86@kernel.org
In-Reply-To: <6a445e4be6b12_3a3568100db@djbw-dev.notmuch>
On Tue, Jun 30, 2026, Dan Williams (nvidia) wrote:
> Edgecombe, Rick P wrote:
> > On Mon, 2026-06-29 at 17:42 -0700, Sean Christopherson wrote:
> > > Answering my own question (though probably poorly), IIUC the answer is that
> > > DICE-based quoting is done through the TDX Module, whereas existing quoting is
> > > done through an SGX enclave and so was routed through userspace.
> > >
> > > If that's all there is too this, then why is KVM involved? I.e. why doesn't the
> > > TDX Module provide the quote directly to the guest?
> >
> > That is a good question. The answer is partly historical reasons, but I think
> > the pros/cons don’t really move the needle too much.
> >
> > The main benefit of doing it with the host in the loop is that the guest side
> > TDVMCALL quoting interface can stay the same. There is also a wrinkle in that
> > there is a limited HW resource involved in the quoting,
What is this magical resource?
> > so we want to do these operations one at a time. Having a mutex on the host
> > is the KISS way of accomplishing some level of fairness for DOS prevention.
At the risk of unintentionally causing effecitvely DoS by introducing a system-
wide lock.
> > We should've explained this more, but TBH this solution is *way* simpler than
> > the initial one that never saw the light of day. So this extra host work seemed
> > quite small compared to what we have been staring at and we kinda overlooked it.
Simpler for what?
> > The other relevant tidbit is that the TDX module folks have some problems to
> > solve before they can support TDG calls to TDX module extensions. I think we can
> > get them to though. The question is probably really: do we want the guest
> > trying/selecting multiple interfaces, or the host.
>
> tl;dr: FWIW, host, if only because this is one of many blob transports
> across multiple archs that need host coordination.
>
> While TDG calls to do the same helps a TDX problem it does not generally
> reduce the Linux problem of multiple archs having multiple blob protocols
> to shuffle data from host to guest in shared memory.
Why is that KVM's problem?
> The current direction for other archs (CCA, SEV-SNP) and blobs (e.g. PCI
> attestation collateral) is shift the burden from arch/*/kvm/ to
> drivers/. There might be a later opportunity to consolidate some of
> those drivers' internals on a cross-arch generic transport (AF_VSOCK),
> but there remains a requirement to talk to the host.
Why?
Bluntly, this series is unreviewable. I've speed read through most of the patches,
and I have *zero* clue as to *why* any of the design decisions were made. E.g.
why is there a common buffer that's apparently shared by all CPUs and VMs, only
for the API to immediately allocate per-quote memory and copy out the buffer?
The changelogs are all heavy on the "what", but explanations of why any of this
is being done, and in the exact way it's being done, are non-existent.
So, back up to the beginning and explain why I should spend any time deciphering
this series, because as-is I don't have the interest or the time.
^ permalink raw reply
* Re: [PATCH v2 2/2] KVM: TDX: Return EINVAL, not EOPNOTSUPP, for NULL INIT_MEM_REGION source
From: Sean Christopherson @ 2026-07-01 17:12 UTC (permalink / raw)
To: Binbin Wu
Cc: Paolo Bonzini, Kiryl Shutsemau, Dave Hansen, Rick Edgecombe, kvm,
x86, linux-coco, linux-kernel, Sashiko Bot, Joerg Roedel,
Yan Zhao, Ackerley Tng
In-Reply-To: <323a9a1a-6cf9-4a68-b92a-867f497b3d34@linux.intel.com>
On Wed, Jul 01, 2026, Binbin Wu wrote:
> On 7/1/2026 5:37 AM, Sean Christopherson wrote:
> > Return EINVAL instead of EOPNOTSUPP if userspace attempts to pass a NULL
> > pointer for the source page of INIT_MEM_REGION, so that KVM's ABI is
> > consistent between TDX and SNP (for LAUNCH_UPDATE). EOPNOTSUPP was chosen
> > to be a forward-looking error code for when guest_memfd supports in-place
> > conversion, but even when in-place conversion comes along, it's an awkward
> > error code as KVM is deliberately choosing to disallow virtual address '0',
> > which is technically a legal userspace address. I.e. it's not so much a
> > lack of support as it is that KVM reserves address '0' to simplify KVM's
> > internal implementation.
>
> Nit:
> Do you think it's worth calling this out in the documentation?
Yes, though that can be done separate since this series doesn't change ABI.
E.g. we can probably do it opportunistically as part of the in-place conversion
series?
^ permalink raw reply
* Re: [PATCH v5 2/3] x86/insn-eval: Add insn_assign_reg() helper
From: David Laight @ 2026-07-01 17:00 UTC (permalink / raw)
To: Sean Christopherson
Cc: Kiryl Shutsemau, Dave Hansen, Thomas Gleixner, Ingo Molnar,
Borislav Petkov, x86, Paolo Bonzini, Kuppuswamy Sathyanarayanan,
Kai Huang, Xiaoyao Li, Rick Edgecombe, Binbin Wu, Andi Kleen,
Dan Williams, Borys Tsyrulnikov, kvm, linux-coco, linux-kernel,
stable, Kiryl Shutsemau (Meta)
In-Reply-To: <akUrORhAmRur-lHP@google.com>
On Wed, 1 Jul 2026 07:59:05 -0700
Sean Christopherson <seanjc@google.com> wrote:
> On Wed, Jul 01, 2026, Kiryl Shutsemau wrote:
> > From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
> >
> > KVM's instruction emulator has a small helper, assign_register(), that
> > writes a value into a sub-register with x86 partial-register-write
> > semantics: 1- and 2-byte writes leave the upper bits of the destination
> > untouched, 4-byte writes zero-extend to 64 bits, 8-byte writes overwrite
> > the full register.
> >
> > The TDX guest #VE handler needs the same logic for port I/O emulation
> > to get 32-bit zero-extension right. Rather than copy-pasting the
> > helper, lift it to <asm/insn-eval.h> as insn_assign_reg() so both can
> > use it.
> >
> > Add <asm/insn.h> to the header's includes so it builds standalone in
> > callers that have not pulled it in transitively.
> >
> > No functional change.
> >
> > Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> > Cc: stable@vger.kernel.org # prerequisite for the following 32-bit port I/O zero-extension fix
> > ---
> > arch/x86/include/asm/insn-eval.h | 30 ++++++++++++++++++++++++++++++
> > arch/x86/kvm/emulate.c | 26 ++++----------------------
> > 2 files changed, 34 insertions(+), 22 deletions(-)
> >
> > diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
> > index 4733e9064ee5..0c87759816d3 100644
> > --- a/arch/x86/include/asm/insn-eval.h
> > +++ b/arch/x86/include/asm/insn-eval.h
> > @@ -9,6 +9,7 @@
> > #include <linux/compiler.h>
> > #include <linux/bug.h>
> > #include <linux/err.h>
> > +#include <asm/insn.h>
> > #include <asm/ptrace.h>
> >
> > #define INSN_CODE_SEG_ADDR_SZ(params) ((params >> 4) & 0xf)
> > @@ -46,4 +47,33 @@ enum insn_mmio_type insn_decode_mmio(struct insn *insn, int *bytes);
> >
> > bool insn_is_nop(struct insn *insn);
> >
> > +/*
> > + * Write @val into *@reg with x86 partial-register-write semantics: a 1-
> > + * or 2-byte write leaves the upper bits of the destination untouched; a
> > + * 4-byte write zero-extends to 64 bits (matching IN[BWL], MOV[BWL]
>
> The placement of the "(matching IN[BWL], MOV[BWL] etc.)" blurb is confusing. I
> *think* you're trying to say this behavior matches that of MOVB, MOVW, and MOVL
> instruction mnemonics, but the blurb is buried in the snippet that specifically
> describes the 4-byte write behavior.
>
> FWIW, I think giving examples does more harm than good, because the behavior isn't
> instruction specific, it's architectural behavior that applies to all writes to
> GPRs, as defined in "3.4.1.1 General-Purpose Registers in 64-Bit Mode". E.g. for
> a MOV instruction that sign-extends a 32-bit immediate to a 64-bit registers, it's
> not that the instruction is exempt from the normal GPR semenatics, it's that the
> instruction performs a 64-bit write to the destination even though the source is
> only 32 bits.
>
> And the B/W/L terminology isn't architectural, it's AT&T syntax. E.g. trying
> to encode "movl" with NASM yields "error: instruction expected, found `movl dword'".
> Yes, the kernel uses AT&T syntax for assembly, but I think this helper should very
> explicitly document that it's emulating architectural behavior.
>
> > + * etc.); an 8-byte write overwrites the full register.
> > + *
> > + * @reg need not be 8-byte aligned: KVM's instruction emulator points
> > + * into the middle of a register slot to address the high-byte
^ it isn't really the 'middle'.
> > + * registers (AH, CH, DH, BH). Use narrow stores for the sub-word
> > + * cases so that the access width matches @bytes.
> > + */
> > +static inline void insn_assign_reg(unsigned long *reg, u64 val, int bytes)
> > +{
> > + switch (bytes) {
> > + case 1:
> > + *(u8 *)reg = (u8)val;
> > + break;
> > + case 2:
> > + *(u16 *)reg = (u16)val;
> > + break;
> > + case 4:
> > + *reg = (u32)val;
>
> IMO, it's worth keeping a short comment here, because even with the explanation
> above, I suspect most people will think the code is buggy. E.g.
>
> /* As above, zero-extend 4-byte writes on 64-bit CPUs. */
> *reg = (u32)val;
Or be even more specific and use '& 0xffffffff' rather than a cast.
Particularly since the casts of the RHS in the byte/short cases aren't
needed at all.
-- David
>
> > + break;
> > + case 8:
> > + *reg = val;
> > + break;
> > + }
> > +}
^ permalink raw reply
* Re: [PATCH v8 06/46] KVM: Enumerate support for PRIVATE memory iff kvm_arch_has_private_mem is defined
From: Sean Christopherson @ 2026-07-01 16:55 UTC (permalink / raw)
To: Xiaoyao Li
Cc: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
tabba, 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, Barry Song, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <739e6834-40b3-405b-ada4-d31c38d8416a@intel.com>
On Wed, Jul 01, 2026, Xiaoyao Li wrote:
> On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> > From: Ackerley Tng <ackerleytng@google.com>
> >
> > Explicitly guard reporting support for KVM_MEMORY_ATTRIBUTE_PRIVATE based
> > on kvm_arch_has_private_mem being #defined in anticipation of decoupling
> > kvm_supported_mem_attributes() from CONFIG_KVM_VM_MEMORY_ATTRIBUTES.
>
> Well, after this series, kvm_supported_mem_attributes() is renamed to
> kvm_supported_vm_mem_attributes(), and it's still under
> CONFIG_KVM_VM_MEMORY_ATTRIBUTES.
>
> > guest_memfd support for memory attributes will be unconditional to avoid
> > yet more macros (all architectures that support guest_memfd are expected to
> > use per-gmem attributes at some point), at which point enumerating support
> > KVM_MEMORY_ATTRIBUTE_PRIVATE based solely on memory attributes being
> > supported _somewhere_ would result in KVM over-reporting support on arm64.
>
> I don't understand it. This patch only changes the behavior of
> kvm_supported_mem_attributes(), the usage of which is guarded by
> CONFIG_KVM_VM_MEMORY_ATTRIBUTESq. This is config is only visible to x86 due
> to patch 03. How does it affect arm64?
Hrm, yeah, this is messed up. Ahh, I think Ackerley shuffled things around and
"broke" stuff in the process. In v7[1] and earlier, the diff was this:
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 091f201251159..68142bc962953 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -722,7 +722,7 @@ static inline int kvm_arch_vcpu_memslots_id(struct kvm_vcpu *vcpu)
}
#endif
-#ifndef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
+#ifndef kvm_arch_has_private_mem
static inline bool kvm_arch_has_private_mem(struct kvm *kvm)
{
return false;
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 306153abbafa5..abb9cfa3eb04d 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2421,8 +2421,10 @@ static int kvm_vm_ioctl_clear_dirty_log(struct kvm *kvm,
#ifdef CONFIG_KVM_VM_MEMORY_ATTRIBUTES
static u64 kvm_supported_mem_attributes(struct kvm *kvm)
{
+#ifdef kvm_arch_has_private_mem
if (!kvm || kvm_arch_has_private_mem(kvm))
return KVM_MEMORY_ATTRIBUTE_PRIVATE;
+#endif
return 0;
}
which makes a *lot* more sense given the changelog (and IMO for the ordering in
general). In v8 here, Ackerley combined part of a change[2] (that I provided
off-list) with part of this commit, to create patch 4, "KVM: Decouple
kvm_has_arch_private_mem from CONFIG_KVM_VM_MEMORY_ATTRIBUTESthe".
Ackerley, the cover letter says:
+ Reshuffled the earlier commits that deal with preparing KVM to stop
seeing VM memory attributes as the only source of attributes.
but there's no explanation for *why* the reshuffling was done. Reorganizing
code like this at v8 of a series this size is a big "no-no" unless there's a
*really* good reason to do so. In addition to the resulting confusion, changes
like this invalidate Fuad's Reviewed-by. And since it's obviously quite difficult
to tease out exactly what changed, it's not realistic to re-review things without
doing a deep audit of the series, which no one wants to do for a series that is/was
so close to being fully ready. And without such an audit, I can't accept the
patches, because I can't trust that what I am accepting is what I and others have
reviewed.
So, except where there is/was a *need* to shuffle things around relative to v7,
I think we should revert back to the v7 ordering for v9. And where there is a
need to rework things, each and every one of those needs to be explicitly
documented, because "Reshuffled the earlier commits" is grossly insufficient.
[1] https://lore.kernel.org/all/20260522-gmem-inplace-conversion-v7-3-2f0fae496530@google.com
[2] https://github.com/sean-jc/linux/commit/8a475b1bcf89f1cf776ed9ce7d6bb587aab0d421
^ permalink raw reply related
* Re: [PATCH v10 3/6] x86/sev: Disable CPU hotplug while SNP is active
From: K Prateek Nayak @ 2026-07-01 16:39 UTC (permalink / raw)
To: Jethro Beekman, Ashish Kalra, tglx, mingo, bp, dave.hansen, x86,
hpa, seanjc, peterz, thomas.lendacky, herbert, davem, ardb
Cc: pbonzini, aik, Michael.Roth, Tycho.Andersen, Nathan.Fontenot,
ackerleytng, jackyli, pgonda, rientjes, jacobhxu, xin,
pawan.kumar.gupta, babu.moger, dyoung, nikunj, john.allen, darwi,
linux-kernel, linux-crypto, kvm, linux-coco
In-Reply-To: <80f3f279-d70e-44d7-a179-c52068115e46@fortanix.com>
Hello Jethro,
On 7/1/2026 3:10 PM, Jethro Beekman wrote:
> I don't believe my concern has been addressed
>
> https://lore.kernel.org/lkml/0df3b665-3a9c-4c46-a7aa-14388e8e1577@fortanix.com/
Quoting your question:
> I think this is too broad. If I have a hypervisor that supports SNP
> virtualization, a (non-confidential) L1 guest running Linux should
> still support CPU hotplug while also running confidential L2 guests.
Ashish, Tom, correct me if I'm wrong, but I don't think KVM exposes SNP
support to L1, at least as per
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kvm/cpuid.c?h=v7.2-rc1#n1221
and only SNP initialization disables hotplug - not the other variants.
L1, running a confidential guest (SEV/SEV-ES) should still be able to
support hotplug since it doesn't go through SNP init. Only the base
hypervisor can setup the RMP tables and go through snp_prepare().
Also bsp_determine_snp() should clear CC_ATTR_HOST_SEV_SNP if it
detects X86_FEATURE_HYPERVISOR so I don't see how this can be a
problem for hotplug in L1.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/kernel/cpu/amd.c?h=v7.2-rc1#n368
--
Thanks and Regards,
Prateek
^ permalink raw reply
* Re: [PATCH v8 17/46] KVM: guest_memfd: Advertise KVM_SET_MEMORY_ATTRIBUTES2 ioctl
From: Sean Christopherson @ 2026-07-01 16:09 UTC (permalink / raw)
To: Xiaoyao Li
Cc: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
tabba, 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, Barry Song, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <584a8f9a-1538-4f8b-b576-75ef0fa961c7@intel.com>
On Wed, Jul 01, 2026, Xiaoyao Li wrote:
> On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> > @@ -4969,6 +4973,11 @@ static int kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
> > return 1;
> > case KVM_CAP_GUEST_MEMFD_FLAGS:
> > return kvm_gmem_get_supported_flags(kvm);
> > + case KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES:
> > + if (!gmem_in_place_conversion || !kvm_supports_private_mem(kvm))
> > + return 0;
> > +
> > + return KVM_MEMORY_ATTRIBUTE_PRIVATE;
> > #endif
> > default:
> > break;
>
> this looks inconsistent with the
>
> case KVM_SET_MEMORY_ATTRIBUTES2:
> if (!gmem_in_place_conversion)
> return -ENOTTY;
>
> Well, the check of
>
> if (!kvm_arch_has_private_mem(f->kvm))
> return -EINVAL;
>
> is buried in the following kvm_gmem_set_attributes(). How about moving of
> kvm_arch_has_private_mem() check to put it along with
> gmem_in_place_conversion check in kvm_gmem_ioctl() in Patch 13?
Me confused, patch 13 already adds the kvm_arch_has_private_mem() in
kvm_gmem_set_attributes().
That said, the ordering here is wonky and misleading. A cursory read of the series
would make one think that waiting to advertise KVM_CAP_GUEST_MEMFD_MEMORY_ATTRIBUTES
makes it safe/ok for KVM to plumb in support for KVM_SET_MEMORY_ATTRIBUTES2 over
multiple patches. But that's not actually true, because the ioctl becomes live
the instant the code exists, userspace doesn't need to wait for KVM to formally
advertise support.
To further confuse matters, it is actually safe/ok to iteratively add support,
because it's all effectively dead code until "Let userspace disable per-VM mem
attributes, enable per-gmem attributes".
So, I think we should go a step further than what I think Xiaoyao is suggesting,
and fully squash patch 17 into patch 13. That way the reader doesn't have to jump
through as many mental hoops to piece together what is happening. It'll obviously
be a bigger patch, but should be easier to review/understand overall.
Oh, and that combined patch should carve out error_offset straightaway, so that
the full uAPI can be reviewed in a single patch.
^ permalink raw reply
* Re: [PATCH v8 13/46] KVM: guest_memfd: Add base support for KVM_SET_MEMORY_ATTRIBUTES2
From: Sean Christopherson @ 2026-07-01 15:35 UTC (permalink / raw)
To: Ackerley Tng
Cc: aik, andrew.jones, binbin.wu, brauner, chao.p.peng, david,
jmattson, jthoughton, michael.roth, oupton, pankaj.gupta, qperret,
rick.p.edgecombe, rientjes, shivankg, steven.price, tabba, 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, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-13-9d2959357853@google.com>
On Thu, Jun 18, 2026, Ackerley Tng wrote:
> Introduce base support for KVM_SET_MEMORY_ATTRIBUTES2 in guest_memfd, which
> just updates attributes tracked by guest_memfd.
>
> Validate input fields in general. Guard usage of KVM_SET_MEMORY_ATTRIBUTES2
> by making sure requested attributes are supported for this instance of kvm.
>
> A new KVM_SET_MEMORY_ATTRIBUTES2 is defined to support writes (unlike
Phrase this as a command using imperative mood. The wording is also weird,
because "support writes" makes it sound like it allows controlling WRITE attributes,
whereas what you mean by "support writes" is "allowing KVM to write back error
information to the struct without technically violating the semantics embedded
in the ioctl". It's doubly confusing because the macros use a different polarity:
IOW means userspace is writing, but this implicitly refers to IOW as "reads".
> KVM_SET_MEMORY_ATTRIBUTES) in addition to reads so it can provide error
> details to userspace. This will be used in a later patch.
>
> The two ioctls use their corresponding structs with no overlap, but
> backward compatibility is baked in for future support of
> KVM_SET_MEMORY_ATTRIBUTES2 and struct kvm_memory_attributes2 in the VM
> ioctl.
I don't understand what this paragraph is trying to say with respect to backwards
compatibility. It's a new ioctl and struct, there's no compatibility in sight.
E.g.
Add a new ioctl (and matching struct), KVM_SET_MEMORY_ATTRIBUTES2, using
the same base ioctl number (0xd2), but with R/W semantics for the kernel
instead of just read semantics. "Officially" documenting that KVM writes
to the payload will allow KVM to support partial/incremental conversions,
instead of all-or-nothing updates (which requires complex unwinding), by
recording the failing offset if an error occurs.
Opportunistically add a new struct as well, even though KVM could squeeze
the error offset into "struct kvm_memory_attributes", as there's no cost
to doing so in practice. Pad the struct with a pile of extra space to try
and avoid ending up with "struct kvm_memory_attributes3" in the future.
Use the same layout for the fields that common to version 1 of the struct,
e.g. to ease upgrading userspace, and to provide flexibility in KVM ever
adds support for KVM_SET_MEMORY_ATTRIBUTES2 at VM scope.
> The process of setting memory attributes is set up such that the later half
> will not fail due to allocation. Any necessary checks are performed before
> the point of no return.
Explain *why*. Readers can usually understand the "what" by reading the code,
but it's much harder to discern *why* things were done a certain way. Some things
go without saying, e.g. "validate input fields", but in that case, just drop the
changelog blurb (if we _weren't_ validating input, *that* would be interesting and
worth calling out).
> Co-developed-by: Vishal Annapurve <vannapurve@google.com>
> Signed-off-by: Vishal Annapurve <vannapurve@google.com>
> Co-developed-by: Sean Christoperson <seanjc@google.com>
> Signed-off-by: Sean Christoperson <seanjc@google.com>
> Reviewed-by: Fuad Tabba <tabba@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> ---
> include/uapi/linux/kvm.h | 13 ++++++
> virt/kvm/Kconfig | 1 +
> virt/kvm/guest_memfd.c | 116 +++++++++++++++++++++++++++++++++++++++++++++++
> virt/kvm/kvm_main.c | 12 +++++
> 4 files changed, 142 insertions(+)
>
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 419011097fa8e..956877a6aab05 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1649,6 +1649,19 @@ struct kvm_memory_attributes {
> __u64 flags;
> };
>
> +#define KVM_SET_MEMORY_ATTRIBUTES2 _IOWR(KVMIO, 0xd2, struct kvm_memory_attributes2)
> +
> +struct kvm_memory_attributes2 {
> + union {
> + __u64 address;
> + __u64 offset;
> + };
> + __u64 size;
> + __u64 attributes;
> + __u64 flags;
> + __u64 reserved[12];
> +};
> +
> #define KVM_MEMORY_ATTRIBUTE_PRIVATE (1ULL << 3)
>
> #define KVM_CREATE_GUEST_MEMFD _IOWR(KVMIO, 0xd4, struct kvm_create_guest_memfd)
> diff --git a/virt/kvm/Kconfig b/virt/kvm/Kconfig
> index 297e4399fbd49..cfa2c78ba5fb9 100644
> --- a/virt/kvm/Kconfig
> +++ b/virt/kvm/Kconfig
> @@ -102,6 +102,7 @@ config KVM_MMU_LOCKLESS_AGING
>
> config KVM_GUEST_MEMFD
> select XARRAY_MULTI
> + select KVM_MEMORY_ATTRIBUTES
> bool
>
> config HAVE_KVM_ARCH_GMEM_PREPARE
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 65ce795c090d9..0d14548c1ed22 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -541,11 +541,127 @@ bool kvm_gmem_is_private(struct kvm *kvm, gfn_t gfn)
> }
> EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gmem_is_private);
>
> +/*
> + * Preallocate memory for attributes to be stored on a maple tree, pointed to
> + * by mas. Adjacent ranges with attributes identical to the new attributes
> + * will be merged. Also sets mas's bounds up for storing attributes.
> + *
> + * This maintains the invariant that ranges with the same attributes will
> + * always be merged.
> + */
> +static int kvm_gmem_mas_preallocate(struct ma_state *mas, u64 attributes,
> + pgoff_t start, size_t nr_pages)
> +{
> + pgoff_t end = start + nr_pages;
> + pgoff_t last = end - 1;
> + void *entry;
> +
> + /* Try extending range. entry is NULL on overflow/wrap-around. */
> + mas_set(mas, end);
> + entry = mas_find(mas, end);
> + if (entry && xa_to_value(entry) == attributes)
> + last = mas->last;
> +
> + if (start > 0) {
> + mas_set(mas, start - 1);
> + entry = mas_find(mas, start - 1);
> + if (entry && xa_to_value(entry) == attributes)
> + start = mas->index;
> + }
> +
> + mas_set_range(mas, start, last);
> + return mas_preallocate(mas, xa_mk_value(attributes), GFP_KERNEL);
> +}
> +
> +static int __kvm_gmem_set_attributes(struct inode *inode, pgoff_t start,
> + size_t nr_pages, uint64_t attrs)
> +{
> + struct address_space *mapping = inode->i_mapping;
> + struct gmem_inode *gi = GMEM_I(inode);
> + pgoff_t end = start + nr_pages;
> + struct maple_tree *mt;
> + struct ma_state mas;
> + int r;
> +
> + mt = &gi->attributes;
> +
> + filemap_invalidate_lock(mapping);
> +
> + mas_init(&mas, mt, start);
> + r = kvm_gmem_mas_preallocate(&mas, attrs, start, nr_pages);
> + if (r)
> + goto out;
> +
> + /*
> + * From this point on guest_memfd has performed necessary
> + * checks and can proceed to do guest-breaking changes.
> + */
> +
> + kvm_gmem_invalidate_start(inode, start, end);
> + mas_store_prealloc(&mas, xa_mk_value(attrs));
> + kvm_gmem_invalidate_end(inode, start, end);
> +out:
> + filemap_invalidate_unlock(mapping);
> + return r;
> +}
> +
> +static long kvm_gmem_set_attributes(struct file *file, void __user *argp)
> +{
> + struct gmem_file *f = file->private_data;
> + struct inode *inode = file_inode(file);
> + struct kvm_memory_attributes2 attrs;
> + size_t nr_pages;
> + pgoff_t index;
> + int i;
> +
> + if (copy_from_user(&attrs, argp, sizeof(attrs)))
> + return -EFAULT;
> +
> + if (attrs.flags)
> + return -EINVAL;
> + for (i = 0; i < ARRAY_SIZE(attrs.reserved); i++) {
> + if (attrs.reserved[i])
> + return -EINVAL;
> + }
> + if (!kvm_arch_has_private_mem(f->kvm))
> + return -EINVAL;
> + if (attrs.attributes & ~KVM_MEMORY_ATTRIBUTE_PRIVATE)
> + return -EINVAL;
> + if (attrs.size == 0 || attrs.offset + attrs.size < attrs.offset)
> + return -EINVAL;
> + if (!PAGE_ALIGNED(attrs.offset) || !PAGE_ALIGNED(attrs.size))
> + return -EINVAL;
> +
> + if (attrs.offset >= i_size_read(inode) ||
> + attrs.offset + attrs.size > i_size_read(inode))
> + return -EINVAL;
> +
> + nr_pages = attrs.size >> PAGE_SHIFT;
> + index = attrs.offset >> PAGE_SHIFT;
> + return __kvm_gmem_set_attributes(inode, index, nr_pages,
> + attrs.attributes);
> +}
> +
> +static long kvm_gmem_ioctl(struct file *file, unsigned int ioctl,
> + unsigned long arg)
> +{
> + switch (ioctl) {
> + case KVM_SET_MEMORY_ATTRIBUTES2:
> + if (!gmem_in_place_conversion)
> + return -ENOTTY;
> +
> + return kvm_gmem_set_attributes(file, (void __user *)arg);
> + default:
> + return -ENOTTY;
> + }
> +}
> +
> static struct file_operations kvm_gmem_fops = {
> .mmap = kvm_gmem_mmap,
> .open = generic_file_open,
> .release = kvm_gmem_release,
> .fallocate = kvm_gmem_fallocate,
> + .unlocked_ioctl = kvm_gmem_ioctl,
> };
>
> static int kvm_gmem_migrate_folio(struct address_space *mapping,
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 01761f6e25d25..a08b518cdb175 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -105,6 +105,18 @@ module_param(allow_unsafe_mappings, bool, 0444);
> bool __ro_after_init gmem_in_place_conversion = false;
> #endif
>
> +#define MEMORY_ATTRIBUTES_MATCH(one, two) \
Use the same terminology as the memory region asserts, i.e.
SANITY_CHECK_MEM_ATTRIBUTES_FIELD. MEMORY_ATTRIBUTES_MATCH() reads like a helper
that checks if the two objects have the same attributes.
And put the checks where it actually matters, i.e. in the case-statement for
KVM_SET_MEMORY_ATTRIBUTES (again, same as KVM_SET_USER_MEMORY_REGION). Because
the only reason it matters for KVM is if we want to add VM-scoped support for
KVM_SET_MEMORY_ATTRIBUTES2 in the future, at which point we'll want to use the
same overlay shenanigans that we did for KVM_SET_USER_MEMORY_REGION2.
> + static_assert(offsetof(struct kvm_memory_attributes, one) == \
> + offsetof(struct kvm_memory_attributes2, two)); \
And then once these are landed in function scope, use BUILD_BUG_ON() with a
do { ... } while (0).
> + static_assert(sizeof_field(struct kvm_memory_attributes, one) ==\
> + sizeof_field(struct kvm_memory_attributes2, two))
> +
> +/* Ensure the common parts of the two structs are identical. */
> +MEMORY_ATTRIBUTES_MATCH(address, address);
> +MEMORY_ATTRIBUTES_MATCH(size, size);
> +MEMORY_ATTRIBUTES_MATCH(attributes, attributes);
> +MEMORY_ATTRIBUTES_MATCH(flags, flags);
Please put these asserts in the location where the overlay matters. Actually, I
don't think we need to enforce this?
^ permalink raw reply
* Re: [PATCH v8 07/46] KVM: Rename memory attribute APIs to prepare for in-place gmem conversion
From: Sean Christopherson @ 2026-07-01 15:07 UTC (permalink / raw)
To: Xiaoyao Li
Cc: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
tabba, 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, Barry Song, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <e5876e41-a11a-4d5e-958f-9e247c19d387@intel.com>
On Wed, Jul 01, 2026, Xiaoyao Li wrote:
> On 7/1/2026 1:30 AM, Sean Christopherson wrote:
> > On Tue, Jun 30, 2026, Xiaoyao Li wrote:
> > > On 6/19/2026 8:31 AM, Ackerley Tng via B4 Relay wrote:
> > > > -bool kvm_range_has_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
> > > > - unsigned long mask, unsigned long attrs);
> > > > +bool kvm_range_has_vm_memory_attributes(struct kvm *kvm, gfn_t start, gfn_t end,
> > > > + unsigned long mask, unsigned long attrs);
> > > > bool kvm_arch_pre_set_memory_attributes(struct kvm *kvm,
> > > > struct kvm_gfn_range *range);
> > > > bool kvm_arch_post_set_memory_attributes(struct kvm *kvm,
> > >
> > > We have
> > >
> > > - kvm_pre_set_memory_attributes()
> > > - kvm_arch_pre_set_memory_attributes()
> > > - kvm_arch_post_set_memory_attributes()
> >
> > Yeah, that's probably for the best.
> >
> > > left, do they need to be renamed as well?
> > >
> > > then the interesting one is kvm_vm_set_mem_attributes(), which contains "vm"
> > > already while it means "vm ioctl". Do we need to rename it to
> > > kvm_vm_set_vm_mem_attributes()?
> >
> > I say "no" on this last one, the fact that the function is scoped to a VM ioctl
> > is enough to communicate that it applies to per-VM attributes.
> >
> > Actually, since it's a local helper, we could go with kvm_set_vm_mem_attributes()
> > to be consistent with the other functions. That just leaves
> > kvm_vm_ioctl_set_mem_attributes(), which I think it appropriately scoped.
>
> If we finally choose to rename kvm_vm_set_mem_attributes() to
> kvm_set_vm_mem_attributes(), I think the trace
> trace_kvm_vm_set_mem_attributes() needs to be renamed to keep it consistent?
Ya, good catch!
^ permalink raw reply
* Re: [PATCH v5 2/3] x86/insn-eval: Add insn_assign_reg() helper
From: Sean Christopherson @ 2026-07-01 14:59 UTC (permalink / raw)
To: Kiryl Shutsemau
Cc: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86,
Paolo Bonzini, Kuppuswamy Sathyanarayanan, Kai Huang, Xiaoyao Li,
Rick Edgecombe, Binbin Wu, David Laight, Andi Kleen, Dan Williams,
Borys Tsyrulnikov, kvm, linux-coco, linux-kernel, stable,
Kiryl Shutsemau (Meta)
In-Reply-To: <20260701110547.764083-3-kirill@shutemov.name>
On Wed, Jul 01, 2026, Kiryl Shutsemau wrote:
> From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
>
> KVM's instruction emulator has a small helper, assign_register(), that
> writes a value into a sub-register with x86 partial-register-write
> semantics: 1- and 2-byte writes leave the upper bits of the destination
> untouched, 4-byte writes zero-extend to 64 bits, 8-byte writes overwrite
> the full register.
>
> The TDX guest #VE handler needs the same logic for port I/O emulation
> to get 32-bit zero-extension right. Rather than copy-pasting the
> helper, lift it to <asm/insn-eval.h> as insn_assign_reg() so both can
> use it.
>
> Add <asm/insn.h> to the header's includes so it builds standalone in
> callers that have not pulled it in transitively.
>
> No functional change.
>
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> Cc: stable@vger.kernel.org # prerequisite for the following 32-bit port I/O zero-extension fix
> ---
> arch/x86/include/asm/insn-eval.h | 30 ++++++++++++++++++++++++++++++
> arch/x86/kvm/emulate.c | 26 ++++----------------------
> 2 files changed, 34 insertions(+), 22 deletions(-)
>
> diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
> index 4733e9064ee5..0c87759816d3 100644
> --- a/arch/x86/include/asm/insn-eval.h
> +++ b/arch/x86/include/asm/insn-eval.h
> @@ -9,6 +9,7 @@
> #include <linux/compiler.h>
> #include <linux/bug.h>
> #include <linux/err.h>
> +#include <asm/insn.h>
> #include <asm/ptrace.h>
>
> #define INSN_CODE_SEG_ADDR_SZ(params) ((params >> 4) & 0xf)
> @@ -46,4 +47,33 @@ enum insn_mmio_type insn_decode_mmio(struct insn *insn, int *bytes);
>
> bool insn_is_nop(struct insn *insn);
>
> +/*
> + * Write @val into *@reg with x86 partial-register-write semantics: a 1-
> + * or 2-byte write leaves the upper bits of the destination untouched; a
> + * 4-byte write zero-extends to 64 bits (matching IN[BWL], MOV[BWL]
The placement of the "(matching IN[BWL], MOV[BWL] etc.)" blurb is confusing. I
*think* you're trying to say this behavior matches that of MOVB, MOVW, and MOVL
instruction mnemonics, but the blurb is buried in the snippet that specifically
describes the 4-byte write behavior.
FWIW, I think giving examples does more harm than good, because the behavior isn't
instruction specific, it's architectural behavior that applies to all writes to
GPRs, as defined in "3.4.1.1 General-Purpose Registers in 64-Bit Mode". E.g. for
a MOV instruction that sign-extends a 32-bit immediate to a 64-bit registers, it's
not that the instruction is exempt from the normal GPR semenatics, it's that the
instruction performs a 64-bit write to the destination even though the source is
only 32 bits.
And the B/W/L terminology isn't architectural, it's AT&T syntax. E.g. trying
to encode "movl" with NASM yields "error: instruction expected, found `movl dword'".
Yes, the kernel uses AT&T syntax for assembly, but I think this helper should very
explicitly document that it's emulating architectural behavior.
> + * etc.); an 8-byte write overwrites the full register.
> + *
> + * @reg need not be 8-byte aligned: KVM's instruction emulator points
> + * into the middle of a register slot to address the high-byte
> + * registers (AH, CH, DH, BH). Use narrow stores for the sub-word
> + * cases so that the access width matches @bytes.
> + */
> +static inline void insn_assign_reg(unsigned long *reg, u64 val, int bytes)
> +{
> + switch (bytes) {
> + case 1:
> + *(u8 *)reg = (u8)val;
> + break;
> + case 2:
> + *(u16 *)reg = (u16)val;
> + break;
> + case 4:
> + *reg = (u32)val;
IMO, it's worth keeping a short comment here, because even with the explanation
above, I suspect most people will think the code is buggy. E.g.
/* As above, zero-extend 4-byte writes on 64-bit CPUs. */
*reg = (u32)val;
> + break;
> + case 8:
> + *reg = val;
> + break;
> + }
> +}
^ permalink raw reply
* Re: [PATCH v13 13/22] KVM: selftests: Set first memory region as shared if guest_memfd
From: Sean Christopherson @ 2026-07-01 14:00 UTC (permalink / raw)
To: Xiaoyao Li
Cc: Ackerley Tng, Lisa Wang, Andrew Jones, Binbin Wu, Chao Gao,
Chenyi Qiang, Dave Hansen, Erdem Aktas, Ira Weiny, Isaku Yamahata,
Kiryl Shutsemau, linux-kselftest, Paolo Bonzini, Pratik R. Sampat,
Reinette Chatre, Rick Edgecombe, Roger Wang, Ryan Afranji,
Sagi Shahar, Shuah Khan, Oliver Upton, Jeremiah McReynolds, kvm,
linux-coco, linux-kernel, x86
In-Reply-To: <81caa42d-9d07-4bed-892d-c24e0b298a1d@intel.com>
On Wed, Jul 01, 2026, Xiaoyao Li wrote:
> On 7/1/2026 2:18 AM, Ackerley Tng wrote:
> > > > I think this patch should fully buy into in-place conversions, so we
> > > > need to also set GUEST_MEMFD_FLAG_MMAP:
> > > >
> > > > @@ -483,6 +483,7 @@ struct kvm_vm *__vm_create(struct vm_shape shape,
> > > > u32 nr_runnable_vcpus,
> > > > {
> > > > u64 nr_pages = vm_nr_pages_required(shape.mode, nr_runnable_vcpus,
> > > > nr_extra_pages);
> > > > + enum vm_mem_backing_src_type src_type = VM_MEM_SRC_ANONYMOUS;
> > > > struct userspace_mem_region *slot0;
> > > > u64 gmem_flags = 0;
> > > > struct kvm_vm *vm;
> > > > @@ -503,10 +504,16 @@ struct kvm_vm *__vm_create(struct vm_shape
> > > > shape, u32 nr_runnable_vcpus,
> > > > */
> > > > if (is_guest_memfd_required(shape)) {
> > > > flags |= KVM_MEM_GUEST_MEMFD;
> > > > - gmem_flags |= GUEST_MEMFD_FLAG_INIT_SHARED;
> > > > + gmem_flags |= GUEST_MEMFD_FLAG_INIT_SHARED | GUEST_MEMFD_FLAG_MMAP;
> > > GUEST_MEMFD_FLAG_INIT_SHARED is valid only when the memory attributes is
> > > per-gmem.
> > >
> > GUEST_MEMFD_FLAG_INIT_SHARED was introduced before guest_memfd in-place
> > conversions, so I think it's orthogonal to whether memory attributes is
> > per-gmem.
>
> But before gmem in-place conversion, i.e., per-gmem attribute,
> GUEST_MEMFD_FLAG_INIT_SHARED is not supported/valid for Coco VMs.
> > > we need to check KVM_CAP_GUEST_MEMFD_FLAGS or kvm_has_gmem_attributes.
> > I think we do want to deprecate the non-in-place-conversions setup, so
> > how about inserting a TEST_REQIRE(kvm_has_gmem_attributes) here?
No. Unless supporting out-of-place conversion requires orders of magnitude more
effort than just supporting in-place conversion, selftests should play nice with
both.
> Well, then all the TDX and SNP selftest will be skipped on the kernel with
> gmem_in_place_conversion=false.
Exactly. They'll also be unusable on previous kernels.
Deprecating per-VM PRIVATE tracking does not mean dropping support. We won't be
able to drop support for years, if ever. And so from a testing perspective, we
absolutely need to validate both models (again, within reason).
^ permalink raw reply
* Re: [PATCH v14 29/44] arm64: RMI: Runtime faulting of memory
From: Lorenzo Pieralisi @ 2026-07-01 13:58 UTC (permalink / raw)
To: Gavin Shan
Cc: Suzuki K Poulose, Steven Price, kvm, kvmarm, Catalin Marinas,
Marc Zyngier, Will Deacon, James Morse, Oliver Upton, Zenghui Yu,
linux-arm-kernel, linux-kernel, Joey Gouly, Alexandru Elisei,
Christoffer Dall, Fuad Tabba, linux-coco, Ganapatrao Kulkarni,
Shanker Donthineni, Alper Gun, Aneesh Kumar K . V, Emi Kisanuki,
Vishal Annapurve, WeiLin.Chang, Lorenzo.Pieralisi2
In-Reply-To: <901398bb-ed6c-4997-b3cd-ce2829b09c87@redhat.com>
On Sun, Jun 28, 2026 at 08:33:01PM +1000, Gavin Shan wrote:
> On 6/27/26 2:44 AM, Lorenzo Pieralisi wrote:
> > On Fri, Jun 26, 2026 at 09:43:03PM +1000, Gavin Shan wrote:
> > > On 6/26/26 6:47 PM, Suzuki K Poulose wrote:
> > > > On 26/06/2026 08:43, Gavin Shan wrote:
> > > > > On 6/26/26 1:58 AM, Suzuki K Poulose wrote:
> > > > > > On 25/06/2026 14:53, Gavin Shan wrote:
> > > > > > > On 6/6/26 12:35 AM, Lorenzo Pieralisi wrote:
> > > > > > > > On Fri, Jun 05, 2026 at 06:11:11PM +1000, Gavin Shan wrote:
> > > > > > > > > On 6/5/26 5:28 PM, Lorenzo Pieralisi wrote:
> > > > > > > > > > On Fri, Jun 05, 2026 at 04:23:15PM +1000, Gavin Shan wrote:
> > > > >
> > > > > [...]
> > > > >
> > > > > > > > >
> > > > > > > > > I tried to rebase Jean's latest QEMU series [1] to upstream QEMU, and found
> > > > > > > > > that memory slots backed by THP are broken. With THP disabled on the host and
> > > > > > > > > other fixes (mentioned in my prevous replies) applied on the top of this (v14)
> > > > > > > > > series, I'm able to boot a realm guest with rebased QEMU series [2], plus more
> > > > > > > > > fxies on the top.
> > > > > > > > >
> > > > > > > > > [1] https://git.codelinaro.org/linaro/dcap/qemu.git (branch: cca/ latest)
> > > > > > > > > [2] https://git.qemu.org/git/qemu.git (branch: cca/ gavin)
> > > > > > > > >
> > > > > > > > > Lorenzo, You may be saying there is someone making QEMU to support ARM/CCA?
> > > > > > > >
> > > > > > > > Mathieu and I are working on that yes and with Steven/Suzuki to fix the THP
> > > > > > > > issues you pointed out above.
> > > > > > > >
> > > > > > > > > If so, I'm not sure if there is a QEMU repository for me to try?
> > > > > > > >
> > > > > > > > We should be able to submit patches by end of June - we shall let you know
> > > > > > > > whether we can make something available earlier.
> > > > > > > >
> > > > > > >
> > > > > > > Not sure if there are other known issues in this series. It seems the stage2
> > > > > > > page fault handling on the shared space isn't working well. In my test, the
> > > > > > > vring (struct vring_desc) of virtio-net-pci is updated by the guest, and the
> > > > > > > data isn't seen by QEMU, I'm suspecting if the host-page-frame-number is properly
> > > > > > > resolved in the s2 page fault handler for shared (unprotected) space.
> > > > > > >
> > > > > > > - I rebased Jean's latest qemu branch to the upstream qemu;
> > > > > > >
> > > > > > > - On the host, which is emulated by qemu/tcg, the THP (transparent huge page) is
> > > > > > > disabled.
> > > > > > >
> > > > > > > - On the guest, I can see the virtio vring (struct vring_desc) is updated. The
> > > > > > > S1 page-table entry looks correct because the corresponding physical address
> > > > > > > 0x10046880000 is a sane shared (unprotected) space address.
> > > > > > >
> > > > > > > [ 52.094143] software IO TLB: Memory encryption is active and system is using DMA bounce buffers
> > > > > > > [ 52.289746] virtqueue_add_desc_split: desc[0]@0xffff000006880000, [00000100b983f000 00000640 0002 0001]
> > > > > > > [ 52.432150] PTE 0x00e8010046880707 at address 0xffff000006880000
> > > > > > >
> > > > > > > - On the host, the s2 page-table-entry is unmapped due to attribute transition (private -> shared).
> > > > > > > A subsequent S2 page fault is raised against the adress and the s2 page-table-entry is built.
> > > > > > >
> > > > > > > [ 109.259077] ====> realm_unmap_shared_range: tracked_unprot_addr=0x10046880000
> > > > > > > [ 109.260249] realm_unmap_shared_range: unmapped shared range at 0x10046880000
> > > > > > > [ 109.317786] realm_unmap_shared_range: unmapped shared range at 0x10046880000
> > > > > > > [ 109.629939] ====> kvm_handle_guest_abort: fault_ipa=0x10046880000, esr=0x92000007
> > > > > > > [ 109.630245] realm_map_non_secure: ipa=0x10046880000, pfn=0xb8b59, size=0x1000, prot=0xf
> > > > > > > [ 109.630331] realm_map_non_secure: ipa=0x10046880000, ipa_top=0x10046881000, flags=0x1e0001, range_desc=0xb8b59004
> > > > > >
> > > > > > Are you able to correlate the order of the transitions and the Guest
> > > > > > access with RMM log ? We haven't seen this from our end. We are aware
> > > > > > of permission fault issues with Unprotected IPA when backing the memslot
> > > > > > with MAP_PRIVATE areas. But this looks different.
> > > > > >
> > > > > > Lorenzo, have you run into this ?
> > > > > >
> > > > >
> > > > > It's hard to correlate the order since the logs are collected from two separate
> > > > > consoles. For the write permission, I add code to the host where the permission
> > > > > is always added for all s2 page faults in the shared space. Otherwise, qemu can
> > > > > be killed by -EFAULT or similar error.
> > > >
> > > > This is the problem. We can't add WRITE permission by default. I believe
> > > > you may have MAP_PRIVATE mapping and it has to be mapped as READ only
> > > > and on a permission fault, we replace it with a writable page. By
> > > > overriding the WRITE permission, you let the guest write to a page
> > > > that may not be seen by the VMM.
> > > >
> > > > We identified this as a bug in the KVM driver in this series (reported
> > > > by Lorenzo) and there is a corresponding tf-RMM change that is required
> > > > to get this working. So, please could you wait until the next series
> > > > when this will be addressed ? Or you could switch to using MAP_SHARED
> > > > for the "shared" memory in the memslot.
> > > >
> > >
> > > Exactly. the syntax for MAP_PRIVATE is broken if the write permission is
> > > enforced for a read fault in the shared space. In my case, the host page can
> > > be the zero page and eventually multiple s2 page-table entries (for multiple
> > > unprotected or shared pages) point to the zero page. It's why clearing the
> > > 3rd queue (Ctrl queue) also clears the first queue (Rx queue) in my case.
> > >
> > > Yes, this issue can be avoid by using a shared memory backend in qemu, something
> > > like below. With this, I'm able to see virtio-net-pci starts to work...
> > >
> > > -object memory-backend-ram,id=mem0,size=2G,share=yes
> >
> > Yes, as Suzuki said that's what we have been fixing. QEmu patches
> > will be on the mailing lists very shortly - the KVM/tf-RMM fixes
> > to make MAP_PRIVATE work will be included in the next posting.
> >
> > Feel free to drop your QEmu command line so that I can give it
> > a shot and check whether the fixes solve the problem you hit
> > (I think so because that's precisely the kind of issue I got
> > into when I started debugging THP/MAP_PRIVATE but it is better
> > to check).
> >
>
> The virtio-net-pci doesn't work with the following command lines. The guest
> kernel image is built from upstream kernel (v7.1.rc7).
>
> qemu-system-aarch64 -enable-kvm -object rme-guest,id=rme0, \
> -machine virt,gic-version=3,confidential-guest-support=rme0 \
> -cpu host,pmu=off \
> -smp maxcpus=2,cpus=2,sockets=1,clusters=1,cores=1,threads=2 \
> -m 2G -object memory-backend-ram,id=mem0,size=2G \
> -numa node,nodeid=0,cpus=0-1,memdev=mem0 \
> -serial mon:stdio -monitor none -nographic -nodefaults \
> -kernel /mnt/linux/arch/arm64/boot/Image \
> -initrd /mnt/buildroot/output/images/rootfs.cpio.xz \
> -append earlycon=pl011,mmio,0x10009000000 \
> -device pcie-root-port,bus=pcie.0,chassis=1,id=pcie.1 \
> -device pcie-root-port,bus=pcie.0,chassis=2,id=pcie.2 \
> -device pcie-root-port,bus=pcie.0,chassis=3,id=pcie.3 \
> -device pcie-root-port,bus=pcie.0,chassis=4,id=pcie.4 \
> -netdev tap,id=tap1,vhost=on,script=/etc/qemu-ifup,downscript=/etc/qemu-ifdown \
> -device virtio-net-pci,bus=pcie.2,netdev=tap1,mac=b8:3f:d2:1d:3e:c0
Tested with private RAM backend, THP left enabled (and additional KVM and
RMM patches that will be included in v15) looks like the net interface
comes up cleanly.
Probably it is best to sync up on the upcoming v15 posting that I am
testing in the background to make sure you don't spend more time chasing
it.
Thanks,
Lorenzo
>
> The virtio-net-pci starts to work with the shareable memory-backend.
>
> -object memory-backend-ram,id=mem0,size=2G,share=yes
>
> Note that THP is disabled on my host.
>
> root@host:~# cat /sys/kernel/mm/transparent_hugepage/enabled
> always madvise [never]
^ permalink raw reply
* Re: [PATCH v8 24/46] KVM: guest_memfd: Make in-place conversion the default\
From: Sean Christopherson @ 2026-07-01 13:53 UTC (permalink / raw)
To: Xiaoyao Li
Cc: Yan Zhao, Ackerley Tng, aik, andrew.jones, binbin.wu, brauner,
chao.p.peng, david, jmattson, jthoughton, michael.roth, oupton,
pankaj.gupta, qperret, rick.p.edgecombe, rientjes, shivankg,
steven.price, tabba, willy, wyihan, 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, Barry Song, Axel Rasmussen, Yuanchu Xie,
Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt, Kiryl Shutsemau,
Baoquan He, Jason Gunthorpe, Vlastimil Babka, kvm, linux-kernel,
linux-trace-kernel, linux-doc, linux-kselftest, linux-mm,
linux-coco
In-Reply-To: <25fdb77d-20f6-4b3a-8b3a-dbba0dc47046@intel.com>
On Wed, Jul 01, 2026, Xiaoyao Li wrote:
> On 6/27/2026 3:06 AM, Sean Christopherson wrote:
> > On Fri, Jun 26, 2026, Yan Zhao wrote:
> > > My first impression of gmem_in_place_conversion=true was that it enforces gmem
> > > in-place conversion. However, it actually only enforces per-gmem private/shared
> > > attribute.
> > > My worry was that people might think it's a kernel bug if userspace can still
> > > have shared memory from other sources after they configured
> > > gmem_in_place_conversion=true.
> > Ah, I see where you're coming from. FWIW, truly enforcing in-place conversion
> > is flat out impossible. E.g. userspace can simply replace the memslot, at which
> > point the memory effectively reverts to shared.
>
> would something like below enforce the in-place conversion?
No.
> Userspace can create a memslot without gmem fd, but that memslot can only
> serve as shared memory and cannot be converted. So it doesn't violate the
> in-place conversion.
But userspace can delete said memslot and replace it with a memslot pointing at
a guest_memfd instance that was created without INIT_SHARED, at which point
userspace has effected a shared=>private conversion.
^ permalink raw reply
* Re: [PATCH v13 13/22] KVM: selftests: Set first memory region as shared if guest_memfd
From: Xiaoyao Li @ 2026-07-01 13:36 UTC (permalink / raw)
To: Ackerley Tng, Lisa Wang, Andrew Jones, Binbin Wu, Chao Gao,
Chenyi Qiang, Dave Hansen, Erdem Aktas, Ira Weiny, Isaku Yamahata,
Kiryl Shutsemau, linux-kselftest, Paolo Bonzini, Pratik R. Sampat,
Reinette Chatre, Rick Edgecombe, Roger Wang, Ryan Afranji,
Sagi Shahar, Sean Christopherson, Shuah Khan, Oliver Upton
Cc: Jeremiah McReynolds, kvm, linux-coco, linux-kernel, x86
In-Reply-To: <CAEvNRgFLjt+-cO-XT0s8uW2xXgyd+VVZ0g0Os6V0+LBFQzx8cQ@mail.gmail.com>
On 7/1/2026 2:18 AM, Ackerley Tng wrote:
>>> I think this patch should fully buy into in-place conversions, so we
>>> need to also set GUEST_MEMFD_FLAG_MMAP:
>>>
>>> @@ -483,6 +483,7 @@ struct kvm_vm *__vm_create(struct vm_shape shape,
>>> u32 nr_runnable_vcpus,
>>> {
>>> u64 nr_pages = vm_nr_pages_required(shape.mode, nr_runnable_vcpus,
>>> nr_extra_pages);
>>> + enum vm_mem_backing_src_type src_type = VM_MEM_SRC_ANONYMOUS;
>>> struct userspace_mem_region *slot0;
>>> u64 gmem_flags = 0;
>>> struct kvm_vm *vm;
>>> @@ -503,10 +504,16 @@ struct kvm_vm *__vm_create(struct vm_shape
>>> shape, u32 nr_runnable_vcpus,
>>> */
>>> if (is_guest_memfd_required(shape)) {
>>> flags |= KVM_MEM_GUEST_MEMFD;
>>> - gmem_flags |= GUEST_MEMFD_FLAG_INIT_SHARED;
>>> + gmem_flags |= GUEST_MEMFD_FLAG_INIT_SHARED | GUEST_MEMFD_FLAG_MMAP;
>> GUEST_MEMFD_FLAG_INIT_SHARED is valid only when the memory attributes is
>> per-gmem.
>>
> GUEST_MEMFD_FLAG_INIT_SHARED was introduced before guest_memfd in-place
> conversions, so I think it's orthogonal to whether memory attributes is
> per-gmem.
But before gmem in-place conversion, i.e., per-gmem attribute,
GUEST_MEMFD_FLAG_INIT_SHARED is not supported/valid for Coco VMs.
>> we need to check KVM_CAP_GUEST_MEMFD_FLAGS or kvm_has_gmem_attributes.
> I think we do want to deprecate the non-in-place-conversions setup, so
> how about inserting a TEST_REQIRE(kvm_has_gmem_attributes) here?
Well, then all the TDX and SNP selftest will be skipped on the kernel
with gmem_in_place_conversion=false.
I don't object it. But we still need to get Sean's answer.
^ permalink raw reply
* Re: [PATCH v4 2/3] x86/insn-eval: Add insn_assign_reg() helper
From: Sean Christopherson @ 2026-07-01 13:25 UTC (permalink / raw)
To: Kiryl Shutsemau (Meta)
Cc: tglx, mingo, bp, dave.hansen, pbonzini,
sathyanarayanan.kuppuswamy, kai.huang, xiaoyao.li, binbin.wu,
rick.p.edgecombe, david.laight.linux, ak, djbw, tsyrulnikov.borys,
x86, kvm, linux-coco, linux-kernel
In-Reply-To: <daac026e677c10b68740b16ed8ad2556bd9583f8.1780584300.git.kas@kernel.org>
In the shortlog, please capture that code is being moved out of KVM. This isn't
"adding" a helper, it's moving and renaming an existing helper. The misleading
shortlog is quite literally why I didn't look at this for a ~month, I thought it
was entirely outside of my normal scope.
On Thu, Jun 04, 2026, Kiryl Shutsemau (Meta) wrote:
> KVM's instruction emulator has a small helper, assign_register(), that
> writes a value into a sub-register with x86 partial-register-write
> semantics: 1- and 2-byte writes leave the upper bits of the destination
> untouched, 4-byte writes zero-extend to 64 bits, 8-byte writes overwrite
> the full register.
>
> The TDX guest #VE handler needs the same logic for port I/O emulation
> to get 32-bit zero-extension right. Rather than copy-pasting the helper,
> lift it to <asm/insn-eval.h> as insn_assign_reg() so both can use it.
>
> Rewrite the body using arithmetic instead of pointer punning so the
> helper does not depend on -fno-strict-aliasing
Meh, building with -fno-strict-aliasing is 100% mandatory for the kernel. IMO,
even so much as suggesting that strict aliasing is something worth shooting for
is a misleading and wrong.
https://lore.kernel.org/all/CAHk-=wh921g_+TJ33JRxRGFM2uruMdqG-SONfFOD9UOsLQJ_uw@mail.gmail.com
> or little-endian byte order,
Oh come on, this is low level x86 code used to write registers.
> and add <asm/insn.h> to the header's includes so it builds standalone in
> callers that have not pulled it in transitively.
>
> No functional change.
>
> Signed-off-by: Kiryl Shutsemau <kas@kernel.org>
> ---
> arch/x86/include/asm/insn-eval.h | 25 +++++++++++++++++++++++++
> arch/x86/kvm/emulate.c | 26 ++++----------------------
> 2 files changed, 29 insertions(+), 22 deletions(-)
>
> diff --git a/arch/x86/include/asm/insn-eval.h b/arch/x86/include/asm/insn-eval.h
> index 4733e9064ee5..85251e718a77 100644
> --- a/arch/x86/include/asm/insn-eval.h
> +++ b/arch/x86/include/asm/insn-eval.h
> @@ -9,6 +9,7 @@
> #include <linux/compiler.h>
> #include <linux/bug.h>
> #include <linux/err.h>
> +#include <asm/insn.h>
> #include <asm/ptrace.h>
>
> #define INSN_CODE_SEG_ADDR_SZ(params) ((params >> 4) & 0xf)
> @@ -46,4 +47,28 @@ enum insn_mmio_type insn_decode_mmio(struct insn *insn, int *bytes);
>
> bool insn_is_nop(struct insn *insn);
>
> +/*
> + * Write @val into *@reg with x86 partial-register-write semantics: a 1-
> + * or 2-byte write leaves the upper bits of the destination untouched; a
Careful with the "upper bits" wording. As Sashiko pointed out, this is used for
{A,B,C,D}H sub-registers as well, in which case the *lower* bits are also left
untouched.
> + * 4-byte write zero-extends to 64 bits (matching IN[BWL], MOV[BWL]
> + * etc.); an 8-byte write overwrites the full register.
> + */
> +static inline void insn_assign_reg(unsigned long *reg, u64 val, int bytes)
> +{
> + switch (bytes) {
> + case 1:
> + *reg = (*reg & ~0xfful) | (val & 0xff);
Sashiko's feedback aside, I strongly prefer KVM's approach as I find it much more
intuitive. And its far, far more consistent with respect to the 4-byte and 8-byte
cases.
> + break;
> + case 2:
> + *reg = (*reg & ~0xfffful) | (val & 0xffff);
> + break;
> + case 4:
> + *reg = (u32)val;
> + break;
> + case 8:
> + *reg = val;
> + break;
> + }
> +}
^ permalink raw reply
* Re: [RFC PATCH 09/15] x86/virt/tdx: Add interface to generate a Quote
From: Nikolay Borisov @ 2026-07-01 11:46 UTC (permalink / raw)
To: Xu Yilun, kas, djbw, rick.p.edgecombe, x86, peter.fang
Cc: linux-coco, linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan, xiaoyao.li
In-Reply-To: <20260522034128.3144354-10-yilun.xu@linux.intel.com>
On 5/22/26 06:41, Xu Yilun wrote:
> From: Peter Fang <peter.fang@intel.com>
>
> Use the TDX Quoting extension's TDH.QUOTE.GET SEAMCALL to generate a
> Quote. Since the interface is shared across all KVM instances,
> serialize access to the SEAMCALL buffer with a mutex.
>
> Allocate and return a per-call buffer containing the generated Quote so
> callers don't need to size the Quote buffer themselves. The caller is
> responsible for freeing the returned buffer.
>
> Signed-off-by: Peter Fang <peter.fang@intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> arch/x86/include/asm/tdx.h | 2 +
> arch/x86/virt/vmx/tdx/tdx.h | 1 +
> arch/x86/virt/vmx/tdx/tdx.c | 82 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 85 insertions(+)
>
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 7b257088aa1e..bc512a00a0d0 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -177,6 +177,8 @@ struct tdx_vp {
> };
>
> bool tdx_quote_enabled(void);
> +void *tdx_quote_generate(struct tdx_td *td, void *in_data, u32 in_data_len,
> + u32 *quote_len);
>
> static inline u64 mk_keyed_paddr(u16 hkid, struct page *page)
> {
> diff --git a/arch/x86/virt/vmx/tdx/tdx.h b/arch/x86/virt/vmx/tdx/tdx.h
> index 3849f4f9cc78..01a7d7d8ada9 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.h
> +++ b/arch/x86/virt/vmx/tdx/tdx.h
> @@ -49,6 +49,7 @@
> #define TDH_EXT_INIT 60
> #define TDH_EXT_MEM_ADD 61
> #define TDH_SYS_DISABLE 69
> +#define TDH_QUOTE_GET 98
> #define TDH_QUOTE_INIT 100
>
> /*
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index b305fa5aab5c..821f677e9a86 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -62,6 +62,8 @@ static LIST_HEAD(tdx_memlist);
> static struct tdx_sys_info tdx_sysinfo __ro_after_init;
> static bool tdx_module_initialized __ro_after_init;
>
> +static DEFINE_MUTEX(tdx_quote_lock);
> +
> static struct quote_data {
> void *buf;
> u64 buf_len;
> @@ -1228,6 +1230,86 @@ bool tdx_quote_enabled(void)
> }
> EXPORT_SYMBOL_FOR_KVM(tdx_quote_enabled);
>
> +#define QUOTE_ID_MASK GENMASK_U64(47, 32)
> +
> +static u64 tdx_quote_get(struct tdx_td *td, u64 in_data_pa, u64 in_data_len,
> + u64 hpa_list_pa, u64 total_len, u64 *quote_len)
> +{
> + struct tdx_module_args args = {
> + .rcx = tdx_tdr_pa(td),
> + /* Don't bother specifying the quote id */
> + .rdx = QUOTE_ID_MASK & (u64)-1,
This is simply equal to QUOTE_ID_MASK, so why not create a special value
meaning "ANY QUOTE" i.e
#define QUOTE_ID_MASK ....
#define ANY_QUOTE QUOTE_ID_MASK
or some such .
> + .r8 = in_data_pa,
> + .r9 = in_data_len,
> + .r10 = hpa_list_pa,
> + .r11 = total_len,
> + };
> + u64 r;
> +
> + do {
> + r = seamcall_ret(TDH_QUOTE_GET, &args);
> + } while (r == TDX_INTERRUPTED_RESUMABLE);
nit: This pattern seems to repeat a lot, might be worth it to consider
introducing a wrapper similar to existing sc_retry?
> +
> + *quote_len = args.rcx;
> +
> + return r;
> +}
> +
> +/**
> + * tdx_quote_generate() - Generate a quote for a TD
> + * @td: The TD to generate the quote for.
> + * @in_data: Input data for the quote request.
> + * @in_data_len: Size of the input data in bytes.
> + * @quote_len: Returned size of the generated quote in bytes.
> + *
> + * Use the TDX Quoting extension to generate a TD quote. Pass the input data
> + * through the shared quote buffer and return the quote.
> + *
> + * Return: Newly allocated quote buffer or %NULL on failure.
> + * The caller must free the returned buffer with kvfree().
> + */
> +void *tdx_quote_generate(struct tdx_td *td, void *in_data, u32 in_data_len,
> + u32 *quote_len)
> +{
> + void *quote_dup = NULL;
> + u64 r, out_len;
> +
> + if (!tdx_quote_enabled())
> + return NULL;
> +
> + /* TDH.QUOTE.GET expects the input data to fit in a page */
> + if (in_data_len > PAGE_SIZE)
> + return NULL;
> +
> + mutex_lock(&tdx_quote_lock);
> +
> + /*
> + * Use the first page of the quote buffer for input data. The buffer
> + * must be at least one page in size. @in_data may not be page-aligned,
> + * but TDH.QUOTE.GET expects page-aligned addresses.
> + */
> + memcpy(quote_data.buf, in_data, (size_t)in_data_len);
Perhaps you can use min(PAGE_SIZE, in_data_len) and that way you can
eliminate the in_data_len check above and copy up-to PAGE_SIZE data, if
the data is longer - you will copy PAGE_SIZE which will likely result in
error on generating the quote?
> +
> + r = tdx_quote_get(td, quote_data.hpa_list[0], (u64)in_data_len,
> + quote_data.hpa_list_pa, quote_data.buf_len, &out_len);
> + if (r || !out_len || out_len > quote_data.buf_len)
> + goto out;
> +
> + /*
> + * The quote buffer is a shared resource, so use it only for the
> + * SEAMCALL and copy the data out as soon as possible.
> + */
> + quote_dup = kvmemdup(quote_data.buf, out_len, GFP_KERNEL);
> +
> +out:
> + mutex_unlock(&tdx_quote_lock);
> +
> + *quote_len = (u32)out_len;
> +
> + return quote_dup;
> +}
> +EXPORT_SYMBOL_FOR_KVM(tdx_quote_generate);
> +
> #define HPAS_PER_PAGE (PAGE_SIZE / sizeof(u64))
>
> static int tdx_quote_create_buf(unsigned int nr_pages, struct quote_data *qdata)
^ permalink raw reply
* Re: [RFC PATCH 08/15] x86/virt/tdx: Add interface to check Quoting availability
From: Nikolay Borisov @ 2026-07-01 11:25 UTC (permalink / raw)
To: Xu Yilun, kas, djbw, rick.p.edgecombe, x86, peter.fang
Cc: linux-coco, linux-kernel, kvm, sohil.mehta, yilun.xu, baolu.lu,
zhenzhong.duan, xiaoyao.li
In-Reply-To: <20260522034128.3144354-9-yilun.xu@linux.intel.com>
On 5/22/26 06:41, Xu Yilun wrote:
> From: Peter Fang <peter.fang@intel.com>
>
> KVM needs to know if the Quoting extension is available to determine
> whether userspace must be involved in Quote generation.
>
> Since the Quote buffer is always created during Quoting extension
> bringup, checking whether the buffer exists is sufficient.
>
> Signed-off-by: Peter Fang <peter.fang@intel.com>
> Signed-off-by: Xu Yilun <yilun.xu@linux.intel.com>
> ---
> arch/x86/include/asm/tdx.h | 2 ++
> arch/x86/virt/vmx/tdx/tdx.c | 15 +++++++++++++++
> 2 files changed, 17 insertions(+)
>
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 15eac89b0afb..7b257088aa1e 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -176,6 +176,8 @@ struct tdx_vp {
> struct page **tdcx_pages;
> };
>
> +bool tdx_quote_enabled(void);
> +
> static inline u64 mk_keyed_paddr(u16 hkid, struct page *page)
> {
> u64 ret;
> diff --git a/arch/x86/virt/vmx/tdx/tdx.c b/arch/x86/virt/vmx/tdx/tdx.c
> index 9d04293394d7..b305fa5aab5c 100644
> --- a/arch/x86/virt/vmx/tdx/tdx.c
> +++ b/arch/x86/virt/vmx/tdx/tdx.c
> @@ -1213,6 +1213,21 @@ static inline u64 tdx_tdr_pa(struct tdx_td *td)
> return page_to_phys(td->tdr_page);
> }
>
> +/**
> + * tdx_quote_enabled() - Check whether TDX Quoting extension is available
> + *
> + * Return: %true if the Quoting extension is available, otherwise %false.
> + */
> +bool tdx_quote_enabled(void)
nit: Probably rename the function to tdx_quoting_ext_enabled or
tdx_quote_ext_enabled, so it's abundantly clear it's about an extension
and not the quoting functionality in general.
> +{
> + /*
> + * No need for locking here. The quote buffer is initialized as part of
> + * core TDX bringup, which comes before KVM is ready for userspace.
> + */
> + return !!quote_data.buf;
While this works it feels a bit like a hack, perhaps have a static
boolean variable being set by the init code which is simply returned by
this function.
> +}
> +EXPORT_SYMBOL_FOR_KVM(tdx_quote_enabled);
> +
> #define HPAS_PER_PAGE (PAGE_SIZE / sizeof(u64))
>
> static int tdx_quote_create_buf(unsigned int nr_pages, struct quote_data *qdata)
^ permalink raw reply
* Re: [PATCH v8 25/46] KVM: guest_memfd: Enable INIT_SHARED on guest_memfd for x86 Coco VMs
From: Xiaoyao Li @ 2026-07-01 11:18 UTC (permalink / raw)
To: ackerleytng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
tabba, 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, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka
Cc: kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <20260618-gmem-inplace-conversion-v8-25-9d2959357853@google.com>
On 6/19/2026 8:32 AM, Ackerley Tng via B4 Relay wrote:
> From: Sean Christopherson <seanjc@google.com>
>
> Now that guest_memfd supports tracking private vs. shared within gmem
> itself, allow userspace to specify INIT_SHARED on a guest_memfd instance
> for x86 Confidential Computing (CoCo) VMs, so long as in-place conversion
> is enabled, i.e. when it's actually possible for a guest_memfd instance to
> contain shared memory.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> Reviewed-by: Fuad Tabba <tabba@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
> ---
> arch/x86/kvm/x86.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 2fde594e86d72..57a543dadb851 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -14116,14 +14116,15 @@ bool kvm_arch_no_poll(struct kvm_vcpu *vcpu)
> }
>
> #ifdef CONFIG_KVM_GUEST_MEMFD
> -/*
> - * KVM doesn't yet support initializing guest_memfd memory as shared for VMs
> - * with private memory (the private vs. shared tracking needs to be moved into
> - * guest_memfd).
> - */
> bool kvm_arch_supports_gmem_init_shared(struct kvm *kvm)
> {
> - return !kvm_arch_has_private_mem(kvm);
> + /*
> + * INIT_SHARED is supported if in-place conversion is enabled, or if
> + * the VM doesn't support private memory. If the VM has private memory
> + * and in-place conversion is disabled, then guest_memfd can _only_ be
> + * used for private memory.
> + */
> + return gmem_in_place_conversion || !kvm_arch_has_private_mem(kvm);
> }
>
> #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_PREPARE
>
^ permalink raw reply
* Re: [PATCH v8 24/46] KVM: guest_memfd: Make in-place conversion the default\
From: Xiaoyao Li @ 2026-07-01 11:07 UTC (permalink / raw)
To: Sean Christopherson, Yan Zhao
Cc: Ackerley Tng, aik, andrew.jones, binbin.wu, brauner, chao.p.peng,
david, jmattson, jthoughton, michael.roth, oupton, pankaj.gupta,
qperret, rick.p.edgecombe, rientjes, shivankg, steven.price,
tabba, willy, wyihan, 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, Barry Song, Axel Rasmussen,
Yuanchu Xie, Wei Xu, Youngjun Park, Qi Zheng, Shakeel Butt,
Kiryl Shutsemau, Baoquan He, Jason Gunthorpe, Vlastimil Babka,
kvm, linux-kernel, linux-trace-kernel, linux-doc, linux-kselftest,
linux-mm, linux-coco
In-Reply-To: <aj7NwCRwWEfLK-gQ@google.com>
On 6/27/2026 3:06 AM, Sean Christopherson wrote:
> On Fri, Jun 26, 2026, Yan Zhao wrote:
>> My first impression of gmem_in_place_conversion=true was that it enforces gmem
>> in-place conversion. However, it actually only enforces per-gmem private/shared
>> attribute.
>> My worry was that people might think it's a kernel bug if userspace can still
>> have shared memory from other sources after they configured
>> gmem_in_place_conversion=true.
> Ah, I see where you're coming from. FWIW, truly enforcing in-place conversion
> is flat out impossible. E.g. userspace can simply replace the memslot, at which
> point the memory effectively reverts to shared.
would something like below enforce the in-place conversion?
Userspace can create a memslot without gmem fd, but that memslot can
only serve as shared memory and cannot be converted. So it doesn't
violate the in-place conversion.
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2122,6 +2122,8 @@ static int kvm_set_memory_region(struct kvm *kvm,
new->flags = mem->flags;
new->userspace_addr = mem->userspace_addr;
if (mem->flags & KVM_MEM_GUEST_MEMFD) {
+ if (gmem_in_place_conversion)
+ new->flags |= KVM_MEMSLOT_GMEM_ONLY;
r = kvm_gmem_bind(kvm, new, mem->guest_memfd,
mem->guest_memfd_offset);
if (r)
goto out;
^ permalink raw reply
* [PATCH v5 3/3] x86/tdx: Fix zero-extension for 32-bit port I/O
From: Kiryl Shutsemau @ 2026-07-01 11:05 UTC (permalink / raw)
To: Dave Hansen, Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86
Cc: Sean Christopherson, Paolo Bonzini, Kuppuswamy Sathyanarayanan,
Kai Huang, Xiaoyao Li, Rick Edgecombe, Binbin Wu, David Laight,
Andi Kleen, Dan Williams, Borys Tsyrulnikov, kvm, linux-coco,
linux-kernel, stable, Kiryl Shutsemau (Meta)
In-Reply-To: <20260701110547.764083-1-kirill@shutemov.name>
From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>
According to x86 architecture rules, 32-bit operations zero-extend the
result to 64 bits. The current implementation of handle_in() only masks
the lower 32 bits, which preserves the upper 32 bits of RAX when a
32-bit port IN instruction is emulated.
Use insn_assign_reg() to write the result back into RAX with proper
partial-register-write semantics: 1- and 2-byte forms leave the upper
bits untouched, the 4-byte form zero-extends to the full register.
Fixes: 03149948832a ("x86/tdx: Port I/O: Add runtime hypercalls")
Reported-by: Borys Tsyrulnikov <tsyrulnikov.borys@gmail.com>
Link: https://lore.kernel.org/all/CAKw_Dz96rfSQc6Rn+9QBcUFHhmkK+9zu+P=bxowfZwxrATCBRg@mail.gmail.com/
Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
Reviewed-by: Binbin Wu <binbin.wu@linux.intel.com>
Cc: stable@vger.kernel.org
---
arch/x86/coco/tdx/tdx.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index b8bbd715fb62..f904a636d449 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -694,8 +694,8 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
.r13 = PORT_READ,
.r14 = port,
};
- u64 mask = GENMASK(BITS_PER_BYTE * size - 1, 0);
bool success;
+ u64 val;
/*
* Emulate the I/O read via hypercall. More info about ABI can be found
@@ -703,11 +703,9 @@ static bool handle_in(struct pt_regs *regs, int size, int port)
* "TDG.VP.VMCALL<Instruction.IO>".
*/
success = !__tdx_hypercall(&args);
+ val = success ? args.r11 : 0;
- /* Update part of the register affected by the emulated instruction */
- regs->ax &= ~mask;
- if (success)
- regs->ax |= args.r11 & mask;
+ insn_assign_reg(®s->ax, val, size);
return success;
}
--
2.54.0
^ permalink raw reply related
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