On 7/8/2025 10:21 AM, Huang, Kai wrote:
On Mon, 2025-07-07 at 15:40 +0530, Nikunj A Dadhania wrote:
Add support for Secure TSC, allowing userspace to configure the Secure TSC
feature for SNP guests. Use the SNP specification's desired TSC frequency
parameter during the SNP_LAUNCH_START command to set the mean TSC
frequency in KHz for Secure TSC enabled guests.
Always use kvm->arch.arch.default_tsc_khz as the TSC frequency that is
passed to SNP guests in the SNP_LAUNCH_START command. The default value
is the host TSC frequency. The userspace can optionally change the TSC
frequency via the KVM_SET_TSC_KHZ ioctl before calling the
SNP_LAUNCH_START ioctl.
Introduce the read-only MSR GUEST_TSC_FREQ (0xc0010134) that returns
guest's effective frequency in MHZ when Secure TSC is enabled for SNP
guests. Disable interception of this MSR when Secure TSC is enabled. Note
that GUEST_TSC_FREQ MSR is accessible only to the guest and not from the
hypervisor context.
Signed-off-by: Nikunj A Dadhania <nikunj@xxxxxxx>
This SoB isn't needed.
Co-developed-by: Ketan Chaturvedi <Ketan.Chaturvedi@xxxxxxx>
Signed-off-by: Ketan Chaturvedi <Ketan.Chaturvedi@xxxxxxx>
Co-developed-by: Sean Christopherson <seanjc@xxxxxxxxxx>
Signed-off-by: Nikunj A Dadhania <nikunj@xxxxxxx>
[...]
@@ -2146,6 +2158,14 @@ static int snp_launch_start(struct kvm *kvm, struct kvm_sev_cmd *argp)
start.gctx_paddr = __psp_pa(sev->snp_context);
start.policy = params.policy;
+
+ if (snp_secure_tsc_enabled(kvm)) {
+ if (!kvm->arch.default_tsc_khz)
+ return -EINVAL;
Here snp_context_create() has been called successfully therefore IIUC you
need to use
goto e_free_context;
instead.
Btw, IIUC it shouldn't be possible for the kvm->arch.default_tsc_khz to be
0. Perhaps we can just remove the check.
Even some bug results in the default_tsc_khz being 0, will the
SNP_LAUNCH_START command catch this and return error?
+
+ start.desired_tsc_khz = kvm->arch.default_tsc_khz;
+ }
+
memcpy(start.gosvw, params.gosvw, sizeof(params.gosvw));
rc = __sev_issue_cmd(argp->sev_fd, SEV_CMD_SNP_LAUNCH_START, &start, &argp->error);
if (rc) {
@@ -2386,7 +2406,9 @@ static int snp_launch_update_vmsa(struct kvm *kvm, struct kvm_sev_cmd *argp)
return ret;
}
- svm->vcpu.arch.guest_state_protected = true;
+ vcpu->arch.guest_state_protected = true;
+ vcpu->arch.guest_tsc_protected = snp_secure_tsc_enabled(kvm);
+
+ Xiaoyao.
The KVM_SET_TSC_KHZ can also be a vCPU ioctl (in fact, the support of VM
ioctl of it was added later). I am wondering whether we should reject
this vCPU ioctl for TSC protected guests, like:
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2806f7104295..699ca5e74bba 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -6186,6 +6186,10 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
u32 user_tsc_khz;
r = -EINVAL;
+
+ if (vcpu->arch.guest_tsc_protected)
+ goto out;
+
user_tsc_khz = (u32)arg;
if (kvm_caps.has_tsc_control &&
It seems to need to be opt-in since it changes the ABI somehow. E.g., it
at least works before when the VMM calls KVM_SET_TSC_KHZ at vcpu with
the same value passed to KVM_SET_TSC_KHZ at vm. But with the above
change, it would fail.
Well, in reality, it's OK for QEMU since QEMU explicitly doesn't call
KVM_SET_TSC_KHZ at vcpu for TDX VMs. But I'm not sure about the impact
on other VMMs. Considering KVM TDX support just gets in from v6.16-rc1,
maybe it doesn't have real impact for other VMMs as well?
