* [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP
@ 2025-08-19 23:48 Sean Christopherson
2025-08-19 23:48 ` [PATCH v11 1/8] KVM: SEV: Drop GHCB_VERSION_DEFAULT and open code it Sean Christopherson
` (9 more replies)
0 siblings, 10 replies; 22+ messages in thread
From: Sean Christopherson @ 2025-08-19 23:48 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth,
Nikunj A Dadhania, Borislav Petkov, Vaishali Thakkar,
Ketan Chaturvedi, Kai Huang
This is a combination of Nikunk's series to enable secure TSC support and to
fix the GHCB version issues, along with some code refactorings to move SEV+
setup code into sev.c (we've managed to grow something like 4 flows that all
do more or less the same thing).
Note, I haven't tested SNP functionality in any way.
v11:
- Shuffle code around so that snp_is_secure_tsc_enabled() doesn't need to
be exposed outside of sev.c.
- Explicitly modify the intercept for MSR_AMD64_GUEST_TSC_FREQ (paranoia is
cheap in this case).
- Trim the changelog for the GHCB version enforcement patch.
- Continue on with snp_launch_start() if default_tsc_khz is '0'. AFAICT,
continuing on doesn't put the host at (any moer) risk. [Kai]
v10: https://lore.kernel.org/all/20250804103751.7760-1-nikunj@amd.com
v3 (GHCB): https://lore.kernel.org/all/20250804090945.267199-1-nikunj@amd.com
Nikunj A Dadhania (4):
KVM: SEV: Drop GHCB_VERSION_DEFAULT and open code it
KVM: SEV: Enforce minimum GHCB version requirement for SEV-SNP guests
x86/cpufeatures: Add SNP Secure TSC
KVM: SVM: Enable Secure TSC for SNP guests
Sean Christopherson (4):
KVM: SVM: Move SEV-ES VMSA allocation to a dedicated sev_vcpu_create()
helper
KVM: SEV: Move init of SNP guest state into sev_init_vmcb()
KVM: SEV: Set RESET GHCB MSR value during sev_es_init_vmcb()
KVM: SEV: Fold sev_es_vcpu_reset() into sev_vcpu_create()
arch/x86/include/asm/cpufeatures.h | 1 +
arch/x86/include/asm/svm.h | 1 +
arch/x86/kvm/svm/sev.c | 108 ++++++++++++++++++++---------
arch/x86/kvm/svm/svm.c | 37 +++-------
arch/x86/kvm/svm/svm.h | 7 +-
5 files changed, 92 insertions(+), 62 deletions(-)
base-commit: c17b750b3ad9f45f2b6f7e6f7f4679844244f0b9
--
2.51.0.rc1.167.g924127e9c0-goog
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH v11 1/8] KVM: SEV: Drop GHCB_VERSION_DEFAULT and open code it
2025-08-19 23:48 [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Sean Christopherson
@ 2025-08-19 23:48 ` Sean Christopherson
2025-08-19 23:48 ` [PATCH v11 2/8] KVM: SEV: Enforce minimum GHCB version requirement for SEV-SNP guests Sean Christopherson
` (8 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Sean Christopherson @ 2025-08-19 23:48 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth,
Nikunj A Dadhania, Borislav Petkov, Vaishali Thakkar,
Ketan Chaturvedi, Kai Huang
From: Nikunj A Dadhania <nikunj@amd.com>
Remove the GHCB_VERSION_DEFAULT macro and open code it with '2'. The macro
is used conditionally and is not a true default. KVM ABI does not
advertise/emumerates the default GHCB version. Any future change to this
macro would silently alter the ABI and potentially break existing
deployments that rely on the current behavior.
Additionally, move the GHCB version assignment earlier in the code flow and
update the comment to clarify that KVM_SEV_INIT2 defaults to version 2,
while KVM_SEV_INIT forces version 1.
No functional change intended.
Cc: Thomas Lendacky <thomas.lendacky@amd.com>
Cc: Michael Roth <michael.roth@amd.com>
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/svm/sev.c | 17 ++++++++---------
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 2fbdebf79fbb..212f790eedd4 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -37,7 +37,6 @@
#include "trace.h"
#define GHCB_VERSION_MAX 2ULL
-#define GHCB_VERSION_DEFAULT 2ULL
#define GHCB_VERSION_MIN 1ULL
#define GHCB_HV_FT_SUPPORTED (GHCB_HV_FT_SNP | GHCB_HV_FT_SNP_AP_CREATION)
@@ -421,6 +420,14 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
if (data->ghcb_version > GHCB_VERSION_MAX || (!es_active && data->ghcb_version))
return -EINVAL;
+ /*
+ * KVM supports the full range of mandatory features defined by version
+ * 2 of the GHCB protocol, so default to that for SEV-ES guests created
+ * via KVM_SEV_INIT2 (KVM_SEV_INIT forces version 1).
+ */
+ if (es_active && !data->ghcb_version)
+ data->ghcb_version = 2;
+
if (unlikely(sev->active))
return -EINVAL;
@@ -429,14 +436,6 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
sev->vmsa_features = data->vmsa_features;
sev->ghcb_version = data->ghcb_version;
- /*
- * Currently KVM supports the full range of mandatory features defined
- * by version 2 of the GHCB protocol, so default to that for SEV-ES
- * guests created via KVM_SEV_INIT2.
- */
- if (sev->es_active && !sev->ghcb_version)
- sev->ghcb_version = GHCB_VERSION_DEFAULT;
-
if (vm_type == KVM_X86_SNP_VM)
sev->vmsa_features |= SVM_SEV_FEAT_SNP_ACTIVE;
--
2.51.0.rc1.167.g924127e9c0-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v11 2/8] KVM: SEV: Enforce minimum GHCB version requirement for SEV-SNP guests
2025-08-19 23:48 [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Sean Christopherson
2025-08-19 23:48 ` [PATCH v11 1/8] KVM: SEV: Drop GHCB_VERSION_DEFAULT and open code it Sean Christopherson
@ 2025-08-19 23:48 ` Sean Christopherson
2025-08-19 23:48 ` [PATCH v11 3/8] x86/cpufeatures: Add SNP Secure TSC Sean Christopherson
` (7 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Sean Christopherson @ 2025-08-19 23:48 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth,
Nikunj A Dadhania, Borislav Petkov, Vaishali Thakkar,
Ketan Chaturvedi, Kai Huang
From: Nikunj A Dadhania <nikunj@amd.com>
Require a minimum GHCB version of 2 when starting SEV-SNP guests through
KVM_SEV_INIT2. When a VMM attempts to start an SEV-SNP guest with an
incompatible GHCB version (less than 2), reject the request early rather
than allowing the guest kernel to start with an incorrect protocol version
and fail later with GHCB_SNP_UNSUPPORTED guest termination.
Not enforcing the minimum version typically causes the guest to request
termination with GHCB_SNP_UNSUPPORTED error code:
kvm_amd: SEV-ES guest requested termination: 0x0:0x2
Fixes: 4af663c2f64a ("KVM: SEV: Allow per-guest configuration of GHCB protocol version")
Cc: Thomas Lendacky <thomas.lendacky@amd.com>
Cc: Sean Christopherson <seanjc@google.com>
Cc: Michael Roth <michael.roth@amd.com>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/svm/sev.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 212f790eedd4..e88dce598785 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -405,6 +405,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
struct sev_platform_init_args init_args = {0};
bool es_active = vm_type != KVM_X86_SEV_VM;
+ bool snp_active = vm_type == KVM_X86_SNP_VM;
u64 valid_vmsa_features = es_active ? sev_supported_vmsa_features : 0;
int ret;
@@ -428,6 +429,9 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
if (es_active && !data->ghcb_version)
data->ghcb_version = 2;
+ if (snp_active && data->ghcb_version < 2)
+ return -EINVAL;
+
if (unlikely(sev->active))
return -EINVAL;
@@ -436,7 +440,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
sev->vmsa_features = data->vmsa_features;
sev->ghcb_version = data->ghcb_version;
- if (vm_type == KVM_X86_SNP_VM)
+ if (snp_active)
sev->vmsa_features |= SVM_SEV_FEAT_SNP_ACTIVE;
ret = sev_asid_new(sev);
@@ -454,7 +458,7 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
}
/* This needs to happen after SEV/SNP firmware initialization. */
- if (vm_type == KVM_X86_SNP_VM) {
+ if (snp_active) {
ret = snp_guest_req_init(kvm);
if (ret)
goto e_free;
--
2.51.0.rc1.167.g924127e9c0-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v11 3/8] x86/cpufeatures: Add SNP Secure TSC
2025-08-19 23:48 [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Sean Christopherson
2025-08-19 23:48 ` [PATCH v11 1/8] KVM: SEV: Drop GHCB_VERSION_DEFAULT and open code it Sean Christopherson
2025-08-19 23:48 ` [PATCH v11 2/8] KVM: SEV: Enforce minimum GHCB version requirement for SEV-SNP guests Sean Christopherson
@ 2025-08-19 23:48 ` Sean Christopherson
2025-08-19 23:48 ` [PATCH v11 4/8] KVM: SVM: Move SEV-ES VMSA allocation to a dedicated sev_vcpu_create() helper Sean Christopherson
` (6 subsequent siblings)
9 siblings, 0 replies; 22+ messages in thread
From: Sean Christopherson @ 2025-08-19 23:48 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth,
Nikunj A Dadhania, Borislav Petkov, Vaishali Thakkar,
Ketan Chaturvedi, Kai Huang
From: Nikunj A Dadhania <nikunj@amd.com>
The Secure TSC feature for SEV-SNP allows guests to securely use the RDTSC
and RDTSCP instructions, ensuring that the parameters used cannot be
altered by the hypervisor once the guest is launched. For more details,
refer to the AMD64 APM Vol 2, Section "Secure TSC".
Acked-by: Borislav Petkov (AMD) <bp@alien8.de>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Tested-by: Vaishali Thakkar <vaishali.thakkar@suse.com>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/cpufeatures.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 06fc0479a23f..f53d4943ea63 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -444,6 +444,7 @@
#define X86_FEATURE_VM_PAGE_FLUSH (19*32+ 2) /* VM Page Flush MSR is supported */
#define X86_FEATURE_SEV_ES (19*32+ 3) /* "sev_es" Secure Encrypted Virtualization - Encrypted State */
#define X86_FEATURE_SEV_SNP (19*32+ 4) /* "sev_snp" Secure Encrypted Virtualization - Secure Nested Paging */
+#define X86_FEATURE_SNP_SECURE_TSC (19*32+ 8) /* SEV-SNP Secure TSC */
#define X86_FEATURE_V_TSC_AUX (19*32+ 9) /* Virtual TSC_AUX */
#define X86_FEATURE_SME_COHERENT (19*32+10) /* hardware-enforced cache coherency */
#define X86_FEATURE_DEBUG_SWAP (19*32+14) /* "debug_swap" SEV-ES full debug state swap support */
--
2.51.0.rc1.167.g924127e9c0-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v11 4/8] KVM: SVM: Move SEV-ES VMSA allocation to a dedicated sev_vcpu_create() helper
2025-08-19 23:48 [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Sean Christopherson
` (2 preceding siblings ...)
2025-08-19 23:48 ` [PATCH v11 3/8] x86/cpufeatures: Add SNP Secure TSC Sean Christopherson
@ 2025-08-19 23:48 ` Sean Christopherson
2025-08-20 9:00 ` Nikunj A. Dadhania
2025-08-19 23:48 ` [PATCH v11 5/8] KVM: SEV: Move init of SNP guest state into sev_init_vmcb() Sean Christopherson
` (5 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Sean Christopherson @ 2025-08-19 23:48 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth,
Nikunj A Dadhania, Borislav Petkov, Vaishali Thakkar,
Ketan Chaturvedi, Kai Huang
Add a dedicated sev_vcpu_create() helper to allocate the VMSA page for
SEV-ES+ vCPUs, and to allow for consolidating a variety of related SEV+
code in the near future.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/svm/sev.c | 20 ++++++++++++++++++++
arch/x86/kvm/svm/svm.c | 25 +++++++------------------
arch/x86/kvm/svm/svm.h | 2 ++
3 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index e88dce598785..c17cc4eb0fe1 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -4561,6 +4561,26 @@ void sev_init_vmcb(struct vcpu_svm *svm)
sev_es_init_vmcb(svm);
}
+int sev_vcpu_create(struct kvm_vcpu *vcpu)
+{
+ struct vcpu_svm *svm = to_svm(vcpu);
+ struct page *vmsa_page;
+
+ if (!sev_es_guest(vcpu->kvm))
+ return 0;
+
+ /*
+ * SEV-ES guests require a separate (from the VMCB) VMSA page used to
+ * contain the encrypted register state of the guest.
+ */
+ vmsa_page = snp_safe_alloc_page();
+ if (!vmsa_page)
+ return -ENOMEM;
+
+ svm->sev_es.vmsa = page_address(vmsa_page);
+ return 0;
+}
+
void sev_es_vcpu_reset(struct vcpu_svm *svm)
{
struct kvm_vcpu *vcpu = &svm->vcpu;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index d9931c6c4bc6..3d4c14e0244f 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1275,7 +1275,6 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm;
struct page *vmcb01_page;
- struct page *vmsa_page = NULL;
int err;
BUILD_BUG_ON(offsetof(struct vcpu_svm, vcpu) != 0);
@@ -1286,24 +1285,18 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu)
if (!vmcb01_page)
goto out;
- if (sev_es_guest(vcpu->kvm)) {
- /*
- * SEV-ES guests require a separate VMSA page used to contain
- * the encrypted register state of the guest.
- */
- vmsa_page = snp_safe_alloc_page();
- if (!vmsa_page)
- goto error_free_vmcb_page;
- }
+ err = sev_vcpu_create(vcpu);
+ if (err)
+ goto error_free_vmcb_page;
err = avic_init_vcpu(svm);
if (err)
- goto error_free_vmsa_page;
+ goto error_free_sev;
svm->msrpm = svm_vcpu_alloc_msrpm();
if (!svm->msrpm) {
err = -ENOMEM;
- goto error_free_vmsa_page;
+ goto error_free_sev;
}
svm->x2avic_msrs_intercepted = true;
@@ -1312,16 +1305,12 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu)
svm->vmcb01.pa = __sme_set(page_to_pfn(vmcb01_page) << PAGE_SHIFT);
svm_switch_vmcb(svm, &svm->vmcb01);
- if (vmsa_page)
- svm->sev_es.vmsa = page_address(vmsa_page);
-
svm->guest_state_loaded = false;
return 0;
-error_free_vmsa_page:
- if (vmsa_page)
- __free_page(vmsa_page);
+error_free_sev:
+ sev_free_vcpu(vcpu);
error_free_vmcb_page:
__free_page(vmcb01_page);
out:
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 58b9d168e0c8..cf2569b5451a 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -854,6 +854,7 @@ static inline struct page *snp_safe_alloc_page(void)
return snp_safe_alloc_page_node(numa_node_id(), GFP_KERNEL_ACCOUNT);
}
+int sev_vcpu_create(struct kvm_vcpu *vcpu);
void sev_free_vcpu(struct kvm_vcpu *vcpu);
void sev_vm_destroy(struct kvm *kvm);
void __init sev_set_cpu_caps(void);
@@ -880,6 +881,7 @@ static inline struct page *snp_safe_alloc_page(void)
return snp_safe_alloc_page_node(numa_node_id(), GFP_KERNEL_ACCOUNT);
}
+static inline int sev_vcpu_create(struct kvm_vcpu *vcpu) { return 0; }
static inline void sev_free_vcpu(struct kvm_vcpu *vcpu) {}
static inline void sev_vm_destroy(struct kvm *kvm) {}
static inline void __init sev_set_cpu_caps(void) {}
--
2.51.0.rc1.167.g924127e9c0-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v11 5/8] KVM: SEV: Move init of SNP guest state into sev_init_vmcb()
2025-08-19 23:48 [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Sean Christopherson
` (3 preceding siblings ...)
2025-08-19 23:48 ` [PATCH v11 4/8] KVM: SVM: Move SEV-ES VMSA allocation to a dedicated sev_vcpu_create() helper Sean Christopherson
@ 2025-08-19 23:48 ` Sean Christopherson
2025-08-20 9:21 ` Nikunj A. Dadhania
2025-08-19 23:48 ` [PATCH v11 6/8] KVM: SEV: Set RESET GHCB MSR value during sev_es_init_vmcb() Sean Christopherson
` (4 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Sean Christopherson @ 2025-08-19 23:48 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth,
Nikunj A Dadhania, Borislav Petkov, Vaishali Thakkar,
Ketan Chaturvedi, Kai Huang
Move the initialization of SNP guest state from svm_vcpu_reset() into
sev_init_vmcb() to reduce the number of paths that deal with INIT/RESET
for SEV+ vCPUs from 4+ to 1. Plumb in @init_event as necessary.
Opportunistically check for an SNP guest outside of
sev_snp_init_protected_guest_state() so that sev_init_vmcb() is consistent
with respect to checking for SEV-ES+ and SNP+ guests.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/svm/sev.c | 16 +++++++++-------
arch/x86/kvm/svm/svm.c | 9 +++------
arch/x86/kvm/svm/svm.h | 4 +---
3 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index c17cc4eb0fe1..c5726b091680 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -1975,7 +1975,7 @@ static void sev_migrate_from(struct kvm *dst_kvm, struct kvm *src_kvm)
kvm_for_each_vcpu(i, dst_vcpu, dst_kvm) {
dst_svm = to_svm(dst_vcpu);
- sev_init_vmcb(dst_svm);
+ sev_init_vmcb(dst_svm, false);
if (!dst->es_active)
continue;
@@ -3887,7 +3887,7 @@ static int snp_begin_psc(struct vcpu_svm *svm, struct psc_buffer *psc)
/*
* Invoked as part of svm_vcpu_reset() processing of an init event.
*/
-void sev_snp_init_protected_guest_state(struct kvm_vcpu *vcpu)
+static void sev_snp_init_protected_guest_state(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm = to_svm(vcpu);
struct kvm_memory_slot *slot;
@@ -3895,9 +3895,6 @@ void sev_snp_init_protected_guest_state(struct kvm_vcpu *vcpu)
kvm_pfn_t pfn;
gfn_t gfn;
- if (!sev_snp_guest(vcpu->kvm))
- return;
-
guard(mutex)(&svm->sev_es.snp_vmsa_mutex);
if (!svm->sev_es.snp_ap_waiting_for_reset)
@@ -4546,8 +4543,10 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
svm_clr_intercept(svm, INTERCEPT_XSETBV);
}
-void sev_init_vmcb(struct vcpu_svm *svm)
+void sev_init_vmcb(struct vcpu_svm *svm, bool init_event)
{
+ struct kvm_vcpu *vcpu = &svm->vcpu;
+
svm->vmcb->control.nested_ctl |= SVM_NESTED_CTL_SEV_ENABLE;
clr_exception_intercept(svm, UD_VECTOR);
@@ -4557,7 +4556,10 @@ void sev_init_vmcb(struct vcpu_svm *svm)
*/
clr_exception_intercept(svm, GP_VECTOR);
- if (sev_es_guest(svm->vcpu.kvm))
+ if (init_event && sev_snp_guest(vcpu->kvm))
+ sev_snp_init_protected_guest_state(vcpu);
+
+ if (sev_es_guest(vcpu->kvm))
sev_es_init_vmcb(svm);
}
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 3d4c14e0244f..8ed135dbd649 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1083,7 +1083,7 @@ static void svm_recalc_intercepts_after_set_cpuid(struct kvm_vcpu *vcpu)
svm_recalc_msr_intercepts(vcpu);
}
-static void init_vmcb(struct kvm_vcpu *vcpu)
+static void init_vmcb(struct kvm_vcpu *vcpu, bool init_event)
{
struct vcpu_svm *svm = to_svm(vcpu);
struct vmcb *vmcb = svm->vmcb01.ptr;
@@ -1221,7 +1221,7 @@ static void init_vmcb(struct kvm_vcpu *vcpu)
svm_set_intercept(svm, INTERCEPT_BUSLOCK);
if (sev_guest(vcpu->kvm))
- sev_init_vmcb(svm);
+ sev_init_vmcb(svm, init_event);
svm_hv_init_vmcb(vmcb);
@@ -1256,10 +1256,7 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
svm->spec_ctrl = 0;
svm->virt_spec_ctrl = 0;
- if (init_event)
- sev_snp_init_protected_guest_state(vcpu);
-
- init_vmcb(vcpu);
+ init_vmcb(vcpu, init_event);
if (!init_event)
__svm_vcpu_reset(vcpu);
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index cf2569b5451a..321480ebe62f 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -826,7 +826,7 @@ void avic_refresh_virtual_apic_mode(struct kvm_vcpu *vcpu);
/* sev.c */
int pre_sev_run(struct vcpu_svm *svm, int cpu);
-void sev_init_vmcb(struct vcpu_svm *svm);
+void sev_init_vmcb(struct vcpu_svm *svm, bool init_event);
void sev_vcpu_after_set_cpuid(struct vcpu_svm *svm);
int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in);
void sev_es_vcpu_reset(struct vcpu_svm *svm);
@@ -864,7 +864,6 @@ int sev_cpu_init(struct svm_cpu_data *sd);
int sev_dev_get_attr(u32 group, u64 attr, u64 *val);
extern unsigned int max_sev_asid;
void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code);
-void sev_snp_init_protected_guest_state(struct kvm_vcpu *vcpu);
int sev_gmem_prepare(struct kvm *kvm, kvm_pfn_t pfn, gfn_t gfn, int max_order);
void sev_gmem_invalidate(kvm_pfn_t start, kvm_pfn_t end);
int sev_private_max_mapping_level(struct kvm *kvm, kvm_pfn_t pfn);
@@ -891,7 +890,6 @@ static inline int sev_cpu_init(struct svm_cpu_data *sd) { return 0; }
static inline int sev_dev_get_attr(u32 group, u64 attr, u64 *val) { return -ENXIO; }
#define max_sev_asid 0
static inline void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code) {}
-static inline void sev_snp_init_protected_guest_state(struct kvm_vcpu *vcpu) {}
static inline int sev_gmem_prepare(struct kvm *kvm, kvm_pfn_t pfn, gfn_t gfn, int max_order)
{
return 0;
--
2.51.0.rc1.167.g924127e9c0-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v11 6/8] KVM: SEV: Set RESET GHCB MSR value during sev_es_init_vmcb()
2025-08-19 23:48 [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Sean Christopherson
` (4 preceding siblings ...)
2025-08-19 23:48 ` [PATCH v11 5/8] KVM: SEV: Move init of SNP guest state into sev_init_vmcb() Sean Christopherson
@ 2025-08-19 23:48 ` Sean Christopherson
2025-08-20 9:32 ` Nikunj A. Dadhania
2025-08-19 23:48 ` [PATCH v11 7/8] KVM: SEV: Fold sev_es_vcpu_reset() into sev_vcpu_create() Sean Christopherson
` (3 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Sean Christopherson @ 2025-08-19 23:48 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth,
Nikunj A Dadhania, Borislav Petkov, Vaishali Thakkar,
Ketan Chaturvedi, Kai Huang
Set the RESET value for the GHCB "MSR" during sev_es_init_vmcb() instead
of sev_es_vcpu_reset() to allow for dropping sev_es_vcpu_reset() entirely.
Note, the call to sev_init_vmcb() from sev_migrate_from() also kinda sorta
emulates a RESET, but sev_migrate_from() immediate overwrites ghcb_gpa
with the source's current value, so whether or not stuffing the GHCB
version is correct/desirable is moot.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/svm/sev.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index c5726b091680..ee7a05843548 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -4480,7 +4480,7 @@ void sev_vcpu_after_set_cpuid(struct vcpu_svm *svm)
vcpu->arch.reserved_gpa_bits &= ~(1UL << (best->ebx & 0x3f));
}
-static void sev_es_init_vmcb(struct vcpu_svm *svm)
+static void sev_es_init_vmcb(struct vcpu_svm *svm, bool init_event)
{
struct kvm_sev_info *sev = to_kvm_sev_info(svm->vcpu.kvm);
struct vmcb *vmcb = svm->vmcb01.ptr;
@@ -4541,6 +4541,15 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
/* Can't intercept XSETBV, HV can't modify XCR0 directly */
svm_clr_intercept(svm, INTERCEPT_XSETBV);
+
+ /*
+ * Set the GHCB MSR value as per the GHCB specification when emulating
+ * vCPU RESET for an SEV-ES guest.
+ */
+ if (!init_event)
+ set_ghcb_msr(svm, GHCB_MSR_SEV_INFO((__u64)sev->ghcb_version,
+ GHCB_VERSION_MIN,
+ sev_enc_bit));
}
void sev_init_vmcb(struct vcpu_svm *svm, bool init_event)
@@ -4560,7 +4569,7 @@ void sev_init_vmcb(struct vcpu_svm *svm, bool init_event)
sev_snp_init_protected_guest_state(vcpu);
if (sev_es_guest(vcpu->kvm))
- sev_es_init_vmcb(svm);
+ sev_es_init_vmcb(svm, init_event);
}
int sev_vcpu_create(struct kvm_vcpu *vcpu)
@@ -4585,17 +4594,6 @@ int sev_vcpu_create(struct kvm_vcpu *vcpu)
void sev_es_vcpu_reset(struct vcpu_svm *svm)
{
- struct kvm_vcpu *vcpu = &svm->vcpu;
- struct kvm_sev_info *sev = to_kvm_sev_info(vcpu->kvm);
-
- /*
- * Set the GHCB MSR value as per the GHCB specification when emulating
- * vCPU RESET for an SEV-ES guest.
- */
- set_ghcb_msr(svm, GHCB_MSR_SEV_INFO((__u64)sev->ghcb_version,
- GHCB_VERSION_MIN,
- sev_enc_bit));
-
mutex_init(&svm->sev_es.snp_vmsa_mutex);
}
--
2.51.0.rc1.167.g924127e9c0-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v11 7/8] KVM: SEV: Fold sev_es_vcpu_reset() into sev_vcpu_create()
2025-08-19 23:48 [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Sean Christopherson
` (5 preceding siblings ...)
2025-08-19 23:48 ` [PATCH v11 6/8] KVM: SEV: Set RESET GHCB MSR value during sev_es_init_vmcb() Sean Christopherson
@ 2025-08-19 23:48 ` Sean Christopherson
2025-08-20 9:33 ` Nikunj A. Dadhania
2025-08-19 23:48 ` [PATCH v11 8/8] KVM: SVM: Enable Secure TSC for SNP guests Sean Christopherson
` (2 subsequent siblings)
9 siblings, 1 reply; 22+ messages in thread
From: Sean Christopherson @ 2025-08-19 23:48 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth,
Nikunj A Dadhania, Borislav Petkov, Vaishali Thakkar,
Ketan Chaturvedi, Kai Huang
Fold the remaining line of sev_es_vcpu_reset() into sev_vcpu_create() as
there's no need for a dedicated RESET hook just to init a mutex, and the
mutex should be initialized as early as possible anyways.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/kvm/svm/sev.c | 7 ++-----
arch/x86/kvm/svm/svm.c | 3 ---
arch/x86/kvm/svm/svm.h | 1 -
3 files changed, 2 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index ee7a05843548..7d1d34e45310 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -4577,6 +4577,8 @@ int sev_vcpu_create(struct kvm_vcpu *vcpu)
struct vcpu_svm *svm = to_svm(vcpu);
struct page *vmsa_page;
+ mutex_init(&svm->sev_es.snp_vmsa_mutex);
+
if (!sev_es_guest(vcpu->kvm))
return 0;
@@ -4592,11 +4594,6 @@ int sev_vcpu_create(struct kvm_vcpu *vcpu)
return 0;
}
-void sev_es_vcpu_reset(struct vcpu_svm *svm)
-{
- mutex_init(&svm->sev_es.snp_vmsa_mutex);
-}
-
void sev_es_prepare_switch_to_guest(struct vcpu_svm *svm, struct sev_es_save_area *hostsa)
{
struct kvm *kvm = svm->vcpu.kvm;
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 8ed135dbd649..b237b4081c91 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -1244,9 +1244,6 @@ static void __svm_vcpu_reset(struct kvm_vcpu *vcpu)
svm->nmi_masked = false;
svm->awaiting_iret_completion = false;
-
- if (sev_es_guest(vcpu->kvm))
- sev_es_vcpu_reset(svm);
}
static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index 321480ebe62f..3c7f208b7935 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -829,7 +829,6 @@ int pre_sev_run(struct vcpu_svm *svm, int cpu);
void sev_init_vmcb(struct vcpu_svm *svm, bool init_event);
void sev_vcpu_after_set_cpuid(struct vcpu_svm *svm);
int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in);
-void sev_es_vcpu_reset(struct vcpu_svm *svm);
void sev_es_recalc_msr_intercepts(struct kvm_vcpu *vcpu);
void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector);
void sev_es_prepare_switch_to_guest(struct vcpu_svm *svm, struct sev_es_save_area *hostsa);
--
2.51.0.rc1.167.g924127e9c0-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH v11 8/8] KVM: SVM: Enable Secure TSC for SNP guests
2025-08-19 23:48 [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Sean Christopherson
` (6 preceding siblings ...)
2025-08-19 23:48 ` [PATCH v11 7/8] KVM: SEV: Fold sev_es_vcpu_reset() into sev_vcpu_create() Sean Christopherson
@ 2025-08-19 23:48 ` Sean Christopherson
2025-08-20 4:53 ` Nikunj A. Dadhania
2025-08-20 8:48 ` [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Nikunj A. Dadhania
2025-08-21 21:35 ` Sean Christopherson
9 siblings, 1 reply; 22+ messages in thread
From: Sean Christopherson @ 2025-08-19 23:48 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth,
Nikunj A Dadhania, Borislav Petkov, Vaishali Thakkar,
Ketan Chaturvedi, Kai Huang
From: Nikunj A Dadhania <nikunj@amd.com>
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.
Co-developed-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
Signed-off-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
Reviewed-by: Kai Huang <kai.huang@intel.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
[sean: contain Secure TSC to sev.c]
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
arch/x86/include/asm/svm.h | 1 +
arch/x86/kvm/svm/sev.c | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h
index ffc27f676243..17f6c3fedeee 100644
--- a/arch/x86/include/asm/svm.h
+++ b/arch/x86/include/asm/svm.h
@@ -299,6 +299,7 @@ static_assert((X2AVIC_MAX_PHYSICAL_ID & AVIC_PHYSICAL_MAX_INDEX_MASK) == X2AVIC_
#define SVM_SEV_FEAT_RESTRICTED_INJECTION BIT(3)
#define SVM_SEV_FEAT_ALTERNATE_INJECTION BIT(4)
#define SVM_SEV_FEAT_DEBUG_SWAP BIT(5)
+#define SVM_SEV_FEAT_SECURE_TSC BIT(9)
#define VMCB_ALLOWED_SEV_FEATURES_VALID BIT_ULL(63)
diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
index 7d1d34e45310..fb45a96e0159 100644
--- a/arch/x86/kvm/svm/sev.c
+++ b/arch/x86/kvm/svm/sev.c
@@ -146,6 +146,14 @@ static bool sev_vcpu_has_debug_swap(struct vcpu_svm *svm)
return sev->vmsa_features & SVM_SEV_FEAT_DEBUG_SWAP;
}
+static bool snp_is_secure_tsc_enabled(struct kvm *kvm)
+{
+ struct kvm_sev_info *sev = to_kvm_sev_info(kvm);
+
+ return (sev->vmsa_features & SVM_SEV_FEAT_SECURE_TSC) &&
+ !WARN_ON_ONCE(!sev_snp_guest(kvm));
+}
+
/* Must be called with the sev_bitmap_lock held */
static bool __sev_recycle_asids(unsigned int min_asid, unsigned int max_asid)
{
@@ -415,6 +423,9 @@ static int __sev_guest_init(struct kvm *kvm, struct kvm_sev_cmd *argp,
if (data->flags)
return -EINVAL;
+ if (!snp_active)
+ valid_vmsa_features &= ~SVM_SEV_FEAT_SECURE_TSC;
+
if (data->vmsa_features & ~valid_vmsa_features)
return -EINVAL;
@@ -2195,6 +2206,12 @@ 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_is_secure_tsc_enabled(kvm)) {
+ WARN_ON_ONCE(!kvm->arch.default_tsc_khz);
+ 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) {
@@ -3085,6 +3102,9 @@ void __init sev_hardware_setup(void)
sev_supported_vmsa_features = 0;
if (sev_es_debug_swap_enabled)
sev_supported_vmsa_features |= SVM_SEV_FEAT_DEBUG_SWAP;
+
+ if (sev_snp_enabled && tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
+ sev_supported_vmsa_features |= SVM_SEV_FEAT_SECURE_TSC;
}
void sev_hardware_unsetup(void)
@@ -4452,6 +4472,9 @@ void sev_es_recalc_msr_intercepts(struct kvm_vcpu *vcpu)
!guest_cpu_cap_has(vcpu, X86_FEATURE_RDTSCP) &&
!guest_cpu_cap_has(vcpu, X86_FEATURE_RDPID));
+ svm_set_intercept_for_msr(vcpu, MSR_AMD64_GUEST_TSC_FREQ, MSR_TYPE_R,
+ !snp_is_secure_tsc_enabled(vcpu->kvm));
+
/*
* For SEV-ES, accesses to MSR_IA32_XSS should not be intercepted if
* the host/guest supports its use.
@@ -4591,6 +4614,9 @@ int sev_vcpu_create(struct kvm_vcpu *vcpu)
return -ENOMEM;
svm->sev_es.vmsa = page_address(vmsa_page);
+
+ vcpu->arch.guest_tsc_protected = snp_is_secure_tsc_enabled(vcpu->kvm);
+
return 0;
}
--
2.51.0.rc1.167.g924127e9c0-goog
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH v11 8/8] KVM: SVM: Enable Secure TSC for SNP guests
2025-08-19 23:48 ` [PATCH v11 8/8] KVM: SVM: Enable Secure TSC for SNP guests Sean Christopherson
@ 2025-08-20 4:53 ` Nikunj A. Dadhania
2025-08-20 13:01 ` Sean Christopherson
0 siblings, 1 reply; 22+ messages in thread
From: Nikunj A. Dadhania @ 2025-08-20 4:53 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth, Borislav Petkov,
Vaishali Thakkar, Kai Huang, David.Kaplan
On 8/20/2025 5:18 AM, Sean Christopherson wrote:
> From: Nikunj A Dadhania <nikunj@amd.com>
>
> @@ -2195,6 +2206,12 @@ 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_is_secure_tsc_enabled(kvm)) {
> + WARN_ON_ONCE(!kvm->arch.default_tsc_khz);
Any particular reason to drop the the following change:
+ if (WARN_ON(!kvm->arch.default_tsc_khz)) {
+ rc = -EINVAL;
+ goto e_free_context;
+ }
As this is an unsupported configuration as per the SEV SNP Firmware ABI Specification:
8.16 SNP_LAUNCH_START
DESIRED_TSC_FREQ
Hypervisor-desired mean TSC frequency in KHz of the guest. This field has no effect if guests
do not enable Secure TSC in the VMSA. The hypervisor should set this field to 0h if it
*does not support Secure TSC* for this guest.
> + start.desired_tsc_khz = kvm->arch.default_tsc_khz;
> + }
> +
Regards,Nikunj
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP
2025-08-19 23:48 [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Sean Christopherson
` (7 preceding siblings ...)
2025-08-19 23:48 ` [PATCH v11 8/8] KVM: SVM: Enable Secure TSC for SNP guests Sean Christopherson
@ 2025-08-20 8:48 ` Nikunj A. Dadhania
2025-08-20 11:25 ` Huang, Kai
2025-08-21 21:35 ` Sean Christopherson
9 siblings, 1 reply; 22+ messages in thread
From: Nikunj A. Dadhania @ 2025-08-20 8:48 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth, Borislav Petkov,
Vaishali Thakkar, Ketan Chaturvedi, Kai Huang
On 8/20/2025 5:18 AM, Sean Christopherson wrote:
> This is a combination of Nikunk's series to enable secure TSC support and to
*Nikunj's 😊 (close though!)
> fix the GHCB version issues, along with some code refactorings to move SEV+
> setup code into sev.c (we've managed to grow something like 4 flows that all
> do more or less the same thing).
>
> Note, I haven't tested SNP functionality in any way.
Tested SNP with and without Secure TSC, guest works as expected.
>
> v11:
> - Shuffle code around so that snp_is_secure_tsc_enabled() doesn't need to
> be exposed outside of sev.c.
> - Explicitly modify the intercept for MSR_AMD64_GUEST_TSC_FREQ (paranoia is
> cheap in this case).
> - Trim the changelog for the GHCB version enforcement patch.
> - Continue on with snp_launch_start() if default_tsc_khz is '0'. AFAICT,
> continuing on doesn't put the host at (any moer) risk. [Kai]
If I hack default_tsc_khz as '0', SNP guest kernel with SecureTSC spits out
couple of warnings and finally panics:
Persistent clock returned invalid value
------------[ cut here ]------------
Missing cycle counter and fallback timer; RNG entropy collection will consequently suffer.
WARNING: CPU: 0 PID: 0 at drivers/char/random.c:931 random_init+0xe7/0xf0
RIP: 0010:random_init+0xe7/0xf0
Call Trace:
<TASK>
start_kernel+0x5e9/0xb80
x86_64_start_reservations+0x18/0x30
x86_64_start_kernel+0xf5/0x140
common_startup_64+0x13e/0x141
</TASK>
...
WARNING: CPU: 0 PID: 0 at arch/x86/kernel/tsc.c:1464 determine_cpu_tsc_frequencies+0x118/0x120
CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Tainted: G W 6.17.0-rc2-stsc+ #1063 PREEMPT(voluntary)
RIP: 0010:determine_cpu_tsc_frequencies+0x118/0x120
Call Trace:
<TASK>
tsc_init+0x2ba/0x430
x86_late_time_init+0x29/0x40
start_kernel+0x70b/0xb80
x86_64_start_reservations+0x18/0x30
x86_64_start_kernel+0xf5/0x140
common_startup_64+0x13e/0x141
</TASK>
...
Oops: divide error: 0000 [#1] SMP NOPTI
CPU: 0 UID: 0 PID: 0 Comm: swapper/0 Tainted: G W 6.17.0-rc2-stsc+ #1063 PREEMPT(voluntary)
Tainted: [W]=WARN
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS unknown 02/02/2022
RIP: 0010:pit_hpet_ptimer_calibrate_cpu+0x1be/0x410
Call Trace:
<TASK>
determine_cpu_tsc_frequencies+0xc1/0x120
tsc_init+0x2ba/0x430
x86_late_time_init+0x29/0x40
start_kernel+0x70b/0xb80
x86_64_start_reservations+0x18/0x30
x86_64_start_kernel+0xf5/0x140
common_startup_64+0x13e/0x141
</TASK>
Regards,
Nikunj
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v11 4/8] KVM: SVM: Move SEV-ES VMSA allocation to a dedicated sev_vcpu_create() helper
2025-08-19 23:48 ` [PATCH v11 4/8] KVM: SVM: Move SEV-ES VMSA allocation to a dedicated sev_vcpu_create() helper Sean Christopherson
@ 2025-08-20 9:00 ` Nikunj A. Dadhania
0 siblings, 0 replies; 22+ messages in thread
From: Nikunj A. Dadhania @ 2025-08-20 9:00 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth, Borislav Petkov,
Vaishali Thakkar, Ketan Chaturvedi, Kai Huang
On 8/20/2025 5:18 AM, Sean Christopherson wrote:
> Add a dedicated sev_vcpu_create() helper to allocate the VMSA page for
> SEV-ES+ vCPUs, and to allow for consolidating a variety of related SEV+
> code in the near future.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
> ---
> arch/x86/kvm/svm/sev.c | 20 ++++++++++++++++++++
> arch/x86/kvm/svm/svm.c | 25 +++++++------------------
> arch/x86/kvm/svm/svm.h | 2 ++
> 3 files changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index e88dce598785..c17cc4eb0fe1 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -4561,6 +4561,26 @@ void sev_init_vmcb(struct vcpu_svm *svm)
> sev_es_init_vmcb(svm);
> }
>
> +int sev_vcpu_create(struct kvm_vcpu *vcpu)
> +{
> + struct vcpu_svm *svm = to_svm(vcpu);
> + struct page *vmsa_page;
> +
> + if (!sev_es_guest(vcpu->kvm))
> + return 0;
> +
> + /*
> + * SEV-ES guests require a separate (from the VMCB) VMSA page used to
> + * contain the encrypted register state of the guest.
> + */
> + vmsa_page = snp_safe_alloc_page();
> + if (!vmsa_page)
> + return -ENOMEM;
> +
> + svm->sev_es.vmsa = page_address(vmsa_page);
> + return 0;
> +}
> +
> void sev_es_vcpu_reset(struct vcpu_svm *svm)
> {
> struct kvm_vcpu *vcpu = &svm->vcpu;
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index d9931c6c4bc6..3d4c14e0244f 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -1275,7 +1275,6 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu)
> {
> struct vcpu_svm *svm;
> struct page *vmcb01_page;
> - struct page *vmsa_page = NULL;
> int err;
>
> BUILD_BUG_ON(offsetof(struct vcpu_svm, vcpu) != 0);
> @@ -1286,24 +1285,18 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu)
> if (!vmcb01_page)
> goto out;
>
> - if (sev_es_guest(vcpu->kvm)) {
> - /*
> - * SEV-ES guests require a separate VMSA page used to contain
> - * the encrypted register state of the guest.
> - */
> - vmsa_page = snp_safe_alloc_page();
> - if (!vmsa_page)
> - goto error_free_vmcb_page;
> - }
> + err = sev_vcpu_create(vcpu);
> + if (err)
> + goto error_free_vmcb_page;
>
> err = avic_init_vcpu(svm);
> if (err)
> - goto error_free_vmsa_page;
> + goto error_free_sev;
>
> svm->msrpm = svm_vcpu_alloc_msrpm();
> if (!svm->msrpm) {
> err = -ENOMEM;
> - goto error_free_vmsa_page;
> + goto error_free_sev;
> }
>
> svm->x2avic_msrs_intercepted = true;
> @@ -1312,16 +1305,12 @@ static int svm_vcpu_create(struct kvm_vcpu *vcpu)
> svm->vmcb01.pa = __sme_set(page_to_pfn(vmcb01_page) << PAGE_SHIFT);
> svm_switch_vmcb(svm, &svm->vmcb01);
>
> - if (vmsa_page)
> - svm->sev_es.vmsa = page_address(vmsa_page);
> -
> svm->guest_state_loaded = false;
>
> return 0;
>
> -error_free_vmsa_page:
> - if (vmsa_page)
> - __free_page(vmsa_page);
> +error_free_sev:
> + sev_free_vcpu(vcpu);
> error_free_vmcb_page:
> __free_page(vmcb01_page);
> out:
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 58b9d168e0c8..cf2569b5451a 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -854,6 +854,7 @@ static inline struct page *snp_safe_alloc_page(void)
> return snp_safe_alloc_page_node(numa_node_id(), GFP_KERNEL_ACCOUNT);
> }
>
> +int sev_vcpu_create(struct kvm_vcpu *vcpu);
> void sev_free_vcpu(struct kvm_vcpu *vcpu);
> void sev_vm_destroy(struct kvm *kvm);
> void __init sev_set_cpu_caps(void);
> @@ -880,6 +881,7 @@ static inline struct page *snp_safe_alloc_page(void)
> return snp_safe_alloc_page_node(numa_node_id(), GFP_KERNEL_ACCOUNT);
> }
>
> +static inline int sev_vcpu_create(struct kvm_vcpu *vcpu) { return 0; }
> static inline void sev_free_vcpu(struct kvm_vcpu *vcpu) {}
> static inline void sev_vm_destroy(struct kvm *kvm) {}
> static inline void __init sev_set_cpu_caps(void) {}
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v11 5/8] KVM: SEV: Move init of SNP guest state into sev_init_vmcb()
2025-08-19 23:48 ` [PATCH v11 5/8] KVM: SEV: Move init of SNP guest state into sev_init_vmcb() Sean Christopherson
@ 2025-08-20 9:21 ` Nikunj A. Dadhania
0 siblings, 0 replies; 22+ messages in thread
From: Nikunj A. Dadhania @ 2025-08-20 9:21 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth, Borislav Petkov,
Vaishali Thakkar, Kai Huang
On 8/20/2025 5:18 AM, Sean Christopherson wrote:
> Move the initialization of SNP guest state from svm_vcpu_reset() into
> sev_init_vmcb() to reduce the number of paths that deal with INIT/RESET
> for SEV+ vCPUs from 4+ to 1. Plumb in @init_event as necessary.
>
> Opportunistically check for an SNP guest outside of
> sev_snp_init_protected_guest_state() so that sev_init_vmcb() is consistent
> with respect to checking for SEV-ES+ and SNP+ guests.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
> ---
> arch/x86/kvm/svm/sev.c | 16 +++++++++-------
> arch/x86/kvm/svm/svm.c | 9 +++------
> arch/x86/kvm/svm/svm.h | 4 +---
> 3 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index c17cc4eb0fe1..c5726b091680 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -1975,7 +1975,7 @@ static void sev_migrate_from(struct kvm *dst_kvm, struct kvm *src_kvm)
> kvm_for_each_vcpu(i, dst_vcpu, dst_kvm) {
> dst_svm = to_svm(dst_vcpu);
>
> - sev_init_vmcb(dst_svm);
> + sev_init_vmcb(dst_svm, false);
>
> if (!dst->es_active)
> continue;
> @@ -3887,7 +3887,7 @@ static int snp_begin_psc(struct vcpu_svm *svm, struct psc_buffer *psc)
> /*
> * Invoked as part of svm_vcpu_reset() processing of an init event.
> */
> -void sev_snp_init_protected_guest_state(struct kvm_vcpu *vcpu)
> +static void sev_snp_init_protected_guest_state(struct kvm_vcpu *vcpu)
> {
> struct vcpu_svm *svm = to_svm(vcpu);
> struct kvm_memory_slot *slot;
> @@ -3895,9 +3895,6 @@ void sev_snp_init_protected_guest_state(struct kvm_vcpu *vcpu)
> kvm_pfn_t pfn;
> gfn_t gfn;
>
> - if (!sev_snp_guest(vcpu->kvm))
> - return;
> -
> guard(mutex)(&svm->sev_es.snp_vmsa_mutex);
>
> if (!svm->sev_es.snp_ap_waiting_for_reset)
> @@ -4546,8 +4543,10 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
> svm_clr_intercept(svm, INTERCEPT_XSETBV);
> }
>
> -void sev_init_vmcb(struct vcpu_svm *svm)
> +void sev_init_vmcb(struct vcpu_svm *svm, bool init_event)
> {
> + struct kvm_vcpu *vcpu = &svm->vcpu;
> +
> svm->vmcb->control.nested_ctl |= SVM_NESTED_CTL_SEV_ENABLE;
> clr_exception_intercept(svm, UD_VECTOR);
>
> @@ -4557,7 +4556,10 @@ void sev_init_vmcb(struct vcpu_svm *svm)
> */
> clr_exception_intercept(svm, GP_VECTOR);
>
> - if (sev_es_guest(svm->vcpu.kvm))
> + if (init_event && sev_snp_guest(vcpu->kvm))
> + sev_snp_init_protected_guest_state(vcpu);
> +
> + if (sev_es_guest(vcpu->kvm))
> sev_es_init_vmcb(svm);
> }
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 3d4c14e0244f..8ed135dbd649 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -1083,7 +1083,7 @@ static void svm_recalc_intercepts_after_set_cpuid(struct kvm_vcpu *vcpu)
> svm_recalc_msr_intercepts(vcpu);
> }
>
> -static void init_vmcb(struct kvm_vcpu *vcpu)
> +static void init_vmcb(struct kvm_vcpu *vcpu, bool init_event)
> {
> struct vcpu_svm *svm = to_svm(vcpu);
> struct vmcb *vmcb = svm->vmcb01.ptr;
> @@ -1221,7 +1221,7 @@ static void init_vmcb(struct kvm_vcpu *vcpu)
> svm_set_intercept(svm, INTERCEPT_BUSLOCK);
>
> if (sev_guest(vcpu->kvm))
> - sev_init_vmcb(svm);
> + sev_init_vmcb(svm, init_event);
>
> svm_hv_init_vmcb(vmcb);
>
> @@ -1256,10 +1256,7 @@ static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> svm->spec_ctrl = 0;
> svm->virt_spec_ctrl = 0;
>
> - if (init_event)
> - sev_snp_init_protected_guest_state(vcpu);
> -
> - init_vmcb(vcpu);
> + init_vmcb(vcpu, init_event);
>
> if (!init_event)
> __svm_vcpu_reset(vcpu);
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index cf2569b5451a..321480ebe62f 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -826,7 +826,7 @@ void avic_refresh_virtual_apic_mode(struct kvm_vcpu *vcpu);
> /* sev.c */
>
> int pre_sev_run(struct vcpu_svm *svm, int cpu);
> -void sev_init_vmcb(struct vcpu_svm *svm);
> +void sev_init_vmcb(struct vcpu_svm *svm, bool init_event);
> void sev_vcpu_after_set_cpuid(struct vcpu_svm *svm);
> int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in);
> void sev_es_vcpu_reset(struct vcpu_svm *svm);
> @@ -864,7 +864,6 @@ int sev_cpu_init(struct svm_cpu_data *sd);
> int sev_dev_get_attr(u32 group, u64 attr, u64 *val);
> extern unsigned int max_sev_asid;
> void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code);
> -void sev_snp_init_protected_guest_state(struct kvm_vcpu *vcpu);
> int sev_gmem_prepare(struct kvm *kvm, kvm_pfn_t pfn, gfn_t gfn, int max_order);
> void sev_gmem_invalidate(kvm_pfn_t start, kvm_pfn_t end);
> int sev_private_max_mapping_level(struct kvm *kvm, kvm_pfn_t pfn);
> @@ -891,7 +890,6 @@ static inline int sev_cpu_init(struct svm_cpu_data *sd) { return 0; }
> static inline int sev_dev_get_attr(u32 group, u64 attr, u64 *val) { return -ENXIO; }
> #define max_sev_asid 0
> static inline void sev_handle_rmp_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u64 error_code) {}
> -static inline void sev_snp_init_protected_guest_state(struct kvm_vcpu *vcpu) {}
> static inline int sev_gmem_prepare(struct kvm *kvm, kvm_pfn_t pfn, gfn_t gfn, int max_order)
> {
> return 0;
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v11 6/8] KVM: SEV: Set RESET GHCB MSR value during sev_es_init_vmcb()
2025-08-19 23:48 ` [PATCH v11 6/8] KVM: SEV: Set RESET GHCB MSR value during sev_es_init_vmcb() Sean Christopherson
@ 2025-08-20 9:32 ` Nikunj A. Dadhania
0 siblings, 0 replies; 22+ messages in thread
From: Nikunj A. Dadhania @ 2025-08-20 9:32 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth, Borislav Petkov,
Vaishali Thakkar, Ketan Chaturvedi, Kai Huang
On 8/20/2025 5:18 AM, Sean Christopherson wrote:
> Set the RESET value for the GHCB "MSR" during sev_es_init_vmcb() instead
> of sev_es_vcpu_reset() to allow for dropping sev_es_vcpu_reset() entirely.
>
> Note, the call to sev_init_vmcb() from sev_migrate_from() also kinda sorta
> emulates a RESET, but sev_migrate_from() immediate overwrites ghcb_gpa
s/immediate/immediately/
> with the source's current value, so whether or not stuffing the GHCB
> version is correct/desirable is moot.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
> ---
> arch/x86/kvm/svm/sev.c | 24 +++++++++++-------------
> 1 file changed, 11 insertions(+), 13 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index c5726b091680..ee7a05843548 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -4480,7 +4480,7 @@ void sev_vcpu_after_set_cpuid(struct vcpu_svm *svm)
> vcpu->arch.reserved_gpa_bits &= ~(1UL << (best->ebx & 0x3f));
> }
>
> -static void sev_es_init_vmcb(struct vcpu_svm *svm)
> +static void sev_es_init_vmcb(struct vcpu_svm *svm, bool init_event)
> {
> struct kvm_sev_info *sev = to_kvm_sev_info(svm->vcpu.kvm);
> struct vmcb *vmcb = svm->vmcb01.ptr;
> @@ -4541,6 +4541,15 @@ static void sev_es_init_vmcb(struct vcpu_svm *svm)
>
> /* Can't intercept XSETBV, HV can't modify XCR0 directly */
> svm_clr_intercept(svm, INTERCEPT_XSETBV);
> +
> + /*
> + * Set the GHCB MSR value as per the GHCB specification when emulating
> + * vCPU RESET for an SEV-ES guest.
> + */
> + if (!init_event)
> + set_ghcb_msr(svm, GHCB_MSR_SEV_INFO((__u64)sev->ghcb_version,
> + GHCB_VERSION_MIN,
> + sev_enc_bit));
> }
>
> void sev_init_vmcb(struct vcpu_svm *svm, bool init_event)
> @@ -4560,7 +4569,7 @@ void sev_init_vmcb(struct vcpu_svm *svm, bool init_event)
> sev_snp_init_protected_guest_state(vcpu);
>
> if (sev_es_guest(vcpu->kvm))
> - sev_es_init_vmcb(svm);
> + sev_es_init_vmcb(svm, init_event);
> }
>
> int sev_vcpu_create(struct kvm_vcpu *vcpu)
> @@ -4585,17 +4594,6 @@ int sev_vcpu_create(struct kvm_vcpu *vcpu)
>
> void sev_es_vcpu_reset(struct vcpu_svm *svm)
> {
> - struct kvm_vcpu *vcpu = &svm->vcpu;
> - struct kvm_sev_info *sev = to_kvm_sev_info(vcpu->kvm);
> -
> - /*
> - * Set the GHCB MSR value as per the GHCB specification when emulating
> - * vCPU RESET for an SEV-ES guest.
> - */
> - set_ghcb_msr(svm, GHCB_MSR_SEV_INFO((__u64)sev->ghcb_version,
> - GHCB_VERSION_MIN,
> - sev_enc_bit));
> -
> mutex_init(&svm->sev_es.snp_vmsa_mutex);
> }
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v11 7/8] KVM: SEV: Fold sev_es_vcpu_reset() into sev_vcpu_create()
2025-08-19 23:48 ` [PATCH v11 7/8] KVM: SEV: Fold sev_es_vcpu_reset() into sev_vcpu_create() Sean Christopherson
@ 2025-08-20 9:33 ` Nikunj A. Dadhania
0 siblings, 0 replies; 22+ messages in thread
From: Nikunj A. Dadhania @ 2025-08-20 9:33 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth, Borislav Petkov,
Vaishali Thakkar, Kai Huang
On 8/20/2025 5:18 AM, Sean Christopherson wrote:
> Fold the remaining line of sev_es_vcpu_reset() into sev_vcpu_create() as
> there's no need for a dedicated RESET hook just to init a mutex, and the
> mutex should be initialized as early as possible anyways.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
> ---
> arch/x86/kvm/svm/sev.c | 7 ++-----
> arch/x86/kvm/svm/svm.c | 3 ---
> arch/x86/kvm/svm/svm.h | 1 -
> 3 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index ee7a05843548..7d1d34e45310 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -4577,6 +4577,8 @@ int sev_vcpu_create(struct kvm_vcpu *vcpu)
> struct vcpu_svm *svm = to_svm(vcpu);
> struct page *vmsa_page;
>
> + mutex_init(&svm->sev_es.snp_vmsa_mutex);
> +
> if (!sev_es_guest(vcpu->kvm))
> return 0;
>
> @@ -4592,11 +4594,6 @@ int sev_vcpu_create(struct kvm_vcpu *vcpu)
> return 0;
> }
>
> -void sev_es_vcpu_reset(struct vcpu_svm *svm)
> -{
> - mutex_init(&svm->sev_es.snp_vmsa_mutex);
> -}
> -
> void sev_es_prepare_switch_to_guest(struct vcpu_svm *svm, struct sev_es_save_area *hostsa)
> {
> struct kvm *kvm = svm->vcpu.kvm;
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 8ed135dbd649..b237b4081c91 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -1244,9 +1244,6 @@ static void __svm_vcpu_reset(struct kvm_vcpu *vcpu)
>
> svm->nmi_masked = false;
> svm->awaiting_iret_completion = false;
> -
> - if (sev_es_guest(vcpu->kvm))
> - sev_es_vcpu_reset(svm);
> }
>
> static void svm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
> index 321480ebe62f..3c7f208b7935 100644
> --- a/arch/x86/kvm/svm/svm.h
> +++ b/arch/x86/kvm/svm/svm.h
> @@ -829,7 +829,6 @@ int pre_sev_run(struct vcpu_svm *svm, int cpu);
> void sev_init_vmcb(struct vcpu_svm *svm, bool init_event);
> void sev_vcpu_after_set_cpuid(struct vcpu_svm *svm);
> int sev_es_string_io(struct vcpu_svm *svm, int size, unsigned int port, int in);
> -void sev_es_vcpu_reset(struct vcpu_svm *svm);
> void sev_es_recalc_msr_intercepts(struct kvm_vcpu *vcpu);
> void sev_vcpu_deliver_sipi_vector(struct kvm_vcpu *vcpu, u8 vector);
> void sev_es_prepare_switch_to_guest(struct vcpu_svm *svm, struct sev_es_save_area *hostsa);
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP
2025-08-20 8:48 ` [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Nikunj A. Dadhania
@ 2025-08-20 11:25 ` Huang, Kai
2025-08-20 11:30 ` Nikunj A. Dadhania
0 siblings, 1 reply; 22+ messages in thread
From: Huang, Kai @ 2025-08-20 11:25 UTC (permalink / raw)
To: pbonzini@redhat.com, seanjc@google.com, nikunj@amd.com
Cc: thomas.lendacky@amd.com, kvm@vger.kernel.org,
vaishali.thakkar@suse.com, Ketan.Chaturvedi@amd.com,
linux-kernel@vger.kernel.org, michael.roth@amd.com, bp@alien8.de
On Wed, 2025-08-20 at 14:18 +0530, Nikunj A. Dadhania wrote:
> > - Continue on with snp_launch_start() if default_tsc_khz is '0'. AFAICT,
> > continuing on doesn't put the host at (any moer) risk. [Kai]
>
> If I hack default_tsc_khz as '0', SNP guest kernel with SecureTSC spits out
> couple of warnings and finally panics:
>
It's a surprise that the SEV_CMD_SNP_LAUNCH_START didn't fail in such
configuration. :-)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP
2025-08-20 11:25 ` Huang, Kai
@ 2025-08-20 11:30 ` Nikunj A. Dadhania
2025-08-20 15:10 ` Sean Christopherson
0 siblings, 1 reply; 22+ messages in thread
From: Nikunj A. Dadhania @ 2025-08-20 11:30 UTC (permalink / raw)
To: Huang, Kai, pbonzini@redhat.com, seanjc@google.com
Cc: thomas.lendacky@amd.com, kvm@vger.kernel.org,
vaishali.thakkar@suse.com, Ketan.Chaturvedi@amd.com,
linux-kernel@vger.kernel.org, michael.roth@amd.com, bp@alien8.de,
david.kaplan
On 8/20/2025 4:55 PM, Huang, Kai wrote:
> On Wed, 2025-08-20 at 14:18 +0530, Nikunj A. Dadhania wrote:
>>> - Continue on with snp_launch_start() if default_tsc_khz is '0'. AFAICT,
>>> continuing on doesn't put the host at (any moer) risk. [Kai]
>>
>> If I hack default_tsc_khz as '0', SNP guest kernel with SecureTSC spits out
>> couple of warnings and finally panics:
>>
>
> It's a surprise that the SEV_CMD_SNP_LAUNCH_START didn't fail in such
> configuration. :-)
As mentioned here[1], this is an unsupported configuration as per the SNP
Firmware ABI.
Regards,
Nikunj
1. https://lore.kernel.org/all/c3e638e9-631f-47af-b0d2-06cea949ec1e@amd.com/
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v11 8/8] KVM: SVM: Enable Secure TSC for SNP guests
2025-08-20 4:53 ` Nikunj A. Dadhania
@ 2025-08-20 13:01 ` Sean Christopherson
2025-08-20 13:11 ` Nikunj A. Dadhania
0 siblings, 1 reply; 22+ messages in thread
From: Sean Christopherson @ 2025-08-20 13:01 UTC (permalink / raw)
To: Nikunj A. Dadhania
Cc: Paolo Bonzini, kvm, linux-kernel, Thomas Lendacky, Michael Roth,
Borislav Petkov, Vaishali Thakkar, Kai Huang, David.Kaplan
On Wed, Aug 20, 2025, Nikunj A. Dadhania wrote:
>
>
> On 8/20/2025 5:18 AM, Sean Christopherson wrote:
> > From: Nikunj A Dadhania <nikunj@amd.com>
> >
> > @@ -2195,6 +2206,12 @@ 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_is_secure_tsc_enabled(kvm)) {
> > + WARN_ON_ONCE(!kvm->arch.default_tsc_khz);
>
> Any particular reason to drop the the following change:
>
> + if (WARN_ON(!kvm->arch.default_tsc_khz)) {
> + rc = -EINVAL;
> + goto e_free_context;
> + }
Based on this conversation[*], both Kai and I expected KVM to let firmware deal
with the should-be-impossible situation.
On Tue, Jul 8, 2025 at 9:15 PM Nikunj A. Dadhania <nikunj@amd.com> wrote:
> On 7/8/2025 8:04 PM, Sean Christopherson wrote:
> > On Tue, Jul 08, 2025, Kai Huang wrote:
> >>>> Even some bug results in the default_tsc_khz being 0, will the
> >>>> SNP_LAUNCH_START command catch this and return error?
> >>>
> >>> No, that is an invalid configuration, desired_tsc_khz is set to 0 when
> >>> SecureTSC is disabled. If SecureTSC is enabled, desired_tsc_khz should
> >>> have correct value.
> >>
> >> So it's an invalid configuration that when Secure TSC is enabled and
> >> desired_tsc_khz is 0. Assuming the SNP_LAUNCH_START will return an error
> >> if such configuration is used, wouldn't it be simpler if you remove the
> >> above check and depend on the SNP_LAUNCH_START command to catch the
> >> invalid configuration?
> >
> > Support for secure TSC should depend on tsc_khz being non-zero. That way it'll
> > be impossible for arch.default_tsc_khz to be zero at runtime. Then KVM can WARN
> > on arch.default_tsc_khz being zero during SNP_LAUNCH_START.
>
> Sure.
https://lore.kernel.org/all/c327df02-c2eb-41e7-9402-5a16aa211265@amd.com
>
> As this is an unsupported configuration as per the SEV SNP Firmware ABI Specification:
Right, but what happens if KVM manages to pass in '0' for the frequency? Does
SNP_LAUNCH_START fail? If so, bailing from KVM doesn't seem to add any value.
>
> 8.16 SNP_LAUNCH_START
>
> DESIRED_TSC_FREQ
> Hypervisor-desired mean TSC frequency in KHz of the guest. This field has no
> effect if guests do not enable Secure TSC in the VMSA. The hypervisor should
> set this field to 0h if it *does not support Secure TSC* for this guest.
>
> > + start.desired_tsc_khz = kvm->arch.default_tsc_khz;
> > + }
> > +
> Regards,Nikunj
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v11 8/8] KVM: SVM: Enable Secure TSC for SNP guests
2025-08-20 13:01 ` Sean Christopherson
@ 2025-08-20 13:11 ` Nikunj A. Dadhania
0 siblings, 0 replies; 22+ messages in thread
From: Nikunj A. Dadhania @ 2025-08-20 13:11 UTC (permalink / raw)
To: Sean Christopherson
Cc: Paolo Bonzini, kvm, linux-kernel, Thomas Lendacky, Michael Roth,
Borislav Petkov, Vaishali Thakkar, Kai Huang, David.Kaplan
On 8/20/2025 6:31 PM, Sean Christopherson wrote:
> On Wed, Aug 20, 2025, Nikunj A. Dadhania wrote:
>>
>>
>> On 8/20/2025 5:18 AM, Sean Christopherson wrote:
>>> From: Nikunj A Dadhania <nikunj@amd.com>
>>>
>>> @@ -2195,6 +2206,12 @@ 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_is_secure_tsc_enabled(kvm)) {
>>> + WARN_ON_ONCE(!kvm->arch.default_tsc_khz);
>>
>> Any particular reason to drop the the following change:
>>
>> + if (WARN_ON(!kvm->arch.default_tsc_khz)) {
>> + rc = -EINVAL;
>> + goto e_free_context;
>> + }
>
> Based on this conversation[*], both Kai and I expected KVM to let firmware deal
> with the should-be-impossible situation.
>
> On Tue, Jul 8, 2025 at 9:15 PM Nikunj A. Dadhania <nikunj@amd.com> wrote:
> > On 7/8/2025 8:04 PM, Sean Christopherson wrote:
> > > On Tue, Jul 08, 2025, Kai Huang wrote:
> > >>>> Even some bug results in the default_tsc_khz being 0, will the
> > >>>> SNP_LAUNCH_START command catch this and return error?
> > >>>
> > >>> No, that is an invalid configuration, desired_tsc_khz is set to 0 when
> > >>> SecureTSC is disabled. If SecureTSC is enabled, desired_tsc_khz should
> > >>> have correct value.
> > >>
> > >> So it's an invalid configuration that when Secure TSC is enabled and
> > >> desired_tsc_khz is 0. Assuming the SNP_LAUNCH_START will return an error
> > >> if such configuration is used, wouldn't it be simpler if you remove the
> > >> above check and depend on the SNP_LAUNCH_START command to catch the
> > >> invalid configuration?
> > >
> > > Support for secure TSC should depend on tsc_khz being non-zero. That way it'll
> > > be impossible for arch.default_tsc_khz to be zero at runtime. Then KVM can WARN
> > > on arch.default_tsc_khz being zero during SNP_LAUNCH_START.
> >
> > Sure.
>
> https://lore.kernel.org/all/c327df02-c2eb-41e7-9402-5a16aa211265@amd.com
>
>>
>> As this is an unsupported configuration as per the SEV SNP Firmware ABI Specification:
>
> Right, but what happens if KVM manages to pass in '0' for the frequency? Does
> SNP_LAUNCH_START fail?
SNP_LAUNCH_START succeeds, and the guest kernel starts and panics during early boot [*]
> If so, bailing from KVM doesn't seem to add any value.
As firmware does not bail out, I had kept this check.
RegardsNikunjhttps://lore.kernel.org/all/afcf9a0b-7450-4df7-a21b-80b56264fc15@amd.com
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP
2025-08-20 11:30 ` Nikunj A. Dadhania
@ 2025-08-20 15:10 ` Sean Christopherson
0 siblings, 0 replies; 22+ messages in thread
From: Sean Christopherson @ 2025-08-20 15:10 UTC (permalink / raw)
To: Nikunj A. Dadhania
Cc: Kai Huang, pbonzini@redhat.com, thomas.lendacky@amd.com,
kvm@vger.kernel.org, vaishali.thakkar@suse.com,
Ketan.Chaturvedi@amd.com, linux-kernel@vger.kernel.org,
michael.roth@amd.com, bp@alien8.de, david.kaplan
On Wed, Aug 20, 2025, Nikunj A. Dadhania wrote:
> On 8/20/2025 4:55 PM, Huang, Kai wrote:
> > On Wed, 2025-08-20 at 14:18 +0530, Nikunj A. Dadhania wrote:
> >>> - Continue on with snp_launch_start() if default_tsc_khz is '0'. AFAICT,
> >>> continuing on doesn't put the host at (any moer) risk. [Kai]
> >>
> >> If I hack default_tsc_khz as '0', SNP guest kernel with SecureTSC spits out
> >> couple of warnings and finally panics:
> >>
> >
> > It's a surprise that the SEV_CMD_SNP_LAUNCH_START didn't fail in such
> > configuration. :-)
>
> As mentioned here[1], this is an unsupported configuration as per the SNP
> Firmware ABI.
Yeah, but outside of AMD hardware/firmware, the usual response to an unsupported
configuration is to return an error :-D
Anyways, I'll fixup to the -EINVAL version from v10 when applying.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP
2025-08-19 23:48 [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Sean Christopherson
` (8 preceding siblings ...)
2025-08-20 8:48 ` [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Nikunj A. Dadhania
@ 2025-08-21 21:35 ` Sean Christopherson
2025-08-25 5:37 ` Nikunj A. Dadhania
9 siblings, 1 reply; 22+ messages in thread
From: Sean Christopherson @ 2025-08-21 21:35 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth,
Nikunj A Dadhania, Borislav Petkov, Vaishali Thakkar,
Ketan Chaturvedi, Kai Huang
On Tue, 19 Aug 2025 16:48:25 -0700, Sean Christopherson wrote:
> This is a combination of Nikunk's series to enable secure TSC support and to
> fix the GHCB version issues, along with some code refactorings to move SEV+
> setup code into sev.c (we've managed to grow something like 4 flows that all
> do more or less the same thing).
>
> Note, I haven't tested SNP functionality in any way.
>
> [...]
Applied to kvm-x86 svm. Nikunj, can you give this one last sanity check when
you get the chance? No rush. I moved the "!kvm->arch.default_tsc_khz" check
up slightly so that it could use a direct return instead of a goto, just want
to make sure I didn't pull a stupid.
Thanks!
[1/8] KVM: SEV: Drop GHCB_VERSION_DEFAULT and open code it
https://github.com/kvm-x86/linux/commit/c78af20374a1
[2/8] KVM: SEV: Enforce minimum GHCB version requirement for SEV-SNP guests
https://github.com/kvm-x86/linux/commit/00f0b959ffb0
[3/8] x86/cpufeatures: Add SNP Secure TSC
https://github.com/kvm-x86/linux/commit/7b59c73fd611
[4/8] KVM: SVM: Move SEV-ES VMSA allocation to a dedicated sev_vcpu_create() helper
https://github.com/kvm-x86/linux/commit/34bd82aab15b
[5/8] KVM: SEV: Move init of SNP guest state into sev_init_vmcb()
https://github.com/kvm-x86/linux/commit/3d4e882e3439
[6/8] KVM: SEV: Set RESET GHCB MSR value during sev_es_init_vmcb()
https://github.com/kvm-x86/linux/commit/baf6ed177290
[7/8] KVM: SEV: Fold sev_es_vcpu_reset() into sev_vcpu_create()
https://github.com/kvm-x86/linux/commit/f7b1f0c1620d
[8/8] KVM: SVM: Enable Secure TSC for SNP guests
https://github.com/kvm-x86/linux/commit/a311fce2b694
--
https://github.com/kvm-x86/linux/tree/next
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP
2025-08-21 21:35 ` Sean Christopherson
@ 2025-08-25 5:37 ` Nikunj A. Dadhania
0 siblings, 0 replies; 22+ messages in thread
From: Nikunj A. Dadhania @ 2025-08-25 5:37 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: kvm, linux-kernel, Thomas Lendacky, Michael Roth, Borislav Petkov,
Vaishali Thakkar, Kai Huang
On 8/22/2025 3:05 AM, Sean Christopherson wrote:
> On Tue, 19 Aug 2025 16:48:25 -0700, Sean Christopherson wrote:
>> This is a combination of Nikunk's series to enable secure TSC support and to
>> fix the GHCB version issues, along with some code refactorings to move SEV+
>> setup code into sev.c (we've managed to grow something like 4 flows that all
>> do more or less the same thing).
>>
>> Note, I haven't tested SNP functionality in any way.
>>
>> [...]
>
> Applied to kvm-x86 svm.
Thanks Sean !
> Nikunj, can you give this one last sanity check when
> you get the chance? No rush. I moved the "!kvm->arch.default_tsc_khz" check
> up slightly so that it could use a direct return instead of a goto, just want
> to make sure I didn't pull a stupid.
Tested the branch with SEV, SEV-ES, SNP and SNP with SecureTSC guests,
working as expected.
>
> Thanks!
>
> [1/8] KVM: SEV: Drop GHCB_VERSION_DEFAULT and open code it
> https://github.com/kvm-x86/linux/commit/c78af20374a1
> [2/8] KVM: SEV: Enforce minimum GHCB version requirement for SEV-SNP guests
> https://github.com/kvm-x86/linux/commit/00f0b959ffb0
> [3/8] x86/cpufeatures: Add SNP Secure TSC
> https://github.com/kvm-x86/linux/commit/7b59c73fd611
> [4/8] KVM: SVM: Move SEV-ES VMSA allocation to a dedicated sev_vcpu_create() helper
> https://github.com/kvm-x86/linux/commit/34bd82aab15b
> [5/8] KVM: SEV: Move init of SNP guest state into sev_init_vmcb()
> https://github.com/kvm-x86/linux/commit/3d4e882e3439
> [6/8] KVM: SEV: Set RESET GHCB MSR value during sev_es_init_vmcb()
> https://github.com/kvm-x86/linux/commit/baf6ed177290
> [7/8] KVM: SEV: Fold sev_es_vcpu_reset() into sev_vcpu_create()
> https://github.com/kvm-x86/linux/commit/f7b1f0c1620d
> [8/8] KVM: SVM: Enable Secure TSC for SNP guests
> https://github.com/kvm-x86/linux/commit/a311fce2b694
>
> --
> https://github.com/kvm-x86/linux/tree/next
Regards,
Nikunj
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2025-08-25 5:37 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-19 23:48 [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Sean Christopherson
2025-08-19 23:48 ` [PATCH v11 1/8] KVM: SEV: Drop GHCB_VERSION_DEFAULT and open code it Sean Christopherson
2025-08-19 23:48 ` [PATCH v11 2/8] KVM: SEV: Enforce minimum GHCB version requirement for SEV-SNP guests Sean Christopherson
2025-08-19 23:48 ` [PATCH v11 3/8] x86/cpufeatures: Add SNP Secure TSC Sean Christopherson
2025-08-19 23:48 ` [PATCH v11 4/8] KVM: SVM: Move SEV-ES VMSA allocation to a dedicated sev_vcpu_create() helper Sean Christopherson
2025-08-20 9:00 ` Nikunj A. Dadhania
2025-08-19 23:48 ` [PATCH v11 5/8] KVM: SEV: Move init of SNP guest state into sev_init_vmcb() Sean Christopherson
2025-08-20 9:21 ` Nikunj A. Dadhania
2025-08-19 23:48 ` [PATCH v11 6/8] KVM: SEV: Set RESET GHCB MSR value during sev_es_init_vmcb() Sean Christopherson
2025-08-20 9:32 ` Nikunj A. Dadhania
2025-08-19 23:48 ` [PATCH v11 7/8] KVM: SEV: Fold sev_es_vcpu_reset() into sev_vcpu_create() Sean Christopherson
2025-08-20 9:33 ` Nikunj A. Dadhania
2025-08-19 23:48 ` [PATCH v11 8/8] KVM: SVM: Enable Secure TSC for SNP guests Sean Christopherson
2025-08-20 4:53 ` Nikunj A. Dadhania
2025-08-20 13:01 ` Sean Christopherson
2025-08-20 13:11 ` Nikunj A. Dadhania
2025-08-20 8:48 ` [PATCH v11 0/8] KVM: SVM: Enable Secure TSC for SEV-SNP Nikunj A. Dadhania
2025-08-20 11:25 ` Huang, Kai
2025-08-20 11:30 ` Nikunj A. Dadhania
2025-08-20 15:10 ` Sean Christopherson
2025-08-21 21:35 ` Sean Christopherson
2025-08-25 5:37 ` Nikunj A. Dadhania
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).