[PATCH v2 0/9] target/i386: SEV: Add support for enabling VMSA SEV features

[PATCH v2 0/9] target/i386: SEV: Add support for enabling VMSA SEV features

[Naveen N Rao (AMD)]

This series adds support for enabling VMSA SEV features for SEV-ES and
SEV-SNP guests. Since that is already supported for IGVM files, some of
that code is moved to generic path and reused.

Debug-swap is already supported in KVM today, while patches for enabling
Secure TSC have been accepted for the upcoming kernel release.

Roy,
I haven't been able to test IGVM, so would be great if that is tested to 
confirm there are no unintended changes there.

Changes since v1 (*):
- Move patch enabling use of KVM_SEV_INIT2 for SEV-ES guests before 
  patch enabling use of debug-swap VMSA SEV feature (Tom)
- Only issue KVM_SET_TSC_KHZ if user has specified a tsc-frequency for 
  Secure TSC (Tom)
- Patch 9/9 is new and refactors check_sev_features in preparation for 
  future SEV feature support (Tom)
- Minor updates to commit log and comments (Tom)
- Collect review tags from Tom

(*) http://lkml.kernel.org/r/cover.1758189463.git.naveen@kernel.org


- Naveen

Naveen N Rao (AMD) (9):
  target/i386: SEV: Generalize handling of SVM_SEV_FEAT_SNP_ACTIVE
  target/i386: SEV: Ensure SEV features are only set through qemu cli or
    IGVM
  target/i386: SEV: Consolidate SEV feature validation to common init
    path
  target/i386: SEV: Validate that SEV-ES is enabled when VMSA features
    are used
  target/i386: SEV: Enable use of KVM_SEV_INIT2 for SEV-ES guests
  target/i386: SEV: Add support for enabling debug-swap SEV feature
  target/i386: SEV: Add support for enabling Secure TSC SEV feature
  target/i386: SEV: Add support for setting TSC frequency for Secure TSC
  target/i386: SEV: Refactor check_sev_features()

 target/i386/sev.h |   4 +-
 target/i386/sev.c | 170 +++++++++++++++++++++++++++++++++++++---------
 qapi/qom.json     |  16 ++++-
 3 files changed, 155 insertions(+), 35 deletions(-)


base-commit: 95b9e0d2ade5d633fd13ffba96a54e87c65baf39
-- 
2.51.0

[PATCH v2 1/9] target/i386: SEV: Generalize handling of SVM_SEV_FEAT_SNP_ACTIVE

[Naveen N Rao (AMD)]

Align with IGVM files providing SEV features with
SVM_SEV_FEAT_SNP_ACTIVE set by setting the same when creating a
sev-snp-guest object.

Since KVM sets this feature itself, SVM_SEV_FEAT_SNP_ACTIVE is unset
before KVM_SEV_INIT2 ioctl is invoked. Move that out of IGVM-specific
section to common code.

While at it, convert the existing SVM_SEV_FEAT_SNP_ACTIVE definition to
use the BIT() macro for consistency with upcoming feature flags.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
---
 target/i386/sev.h |  2 +-
 target/i386/sev.c | 24 +++++++++++++++++-------
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index 9db1a802f6bb..102546b112d6 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -44,7 +44,7 @@ bool sev_snp_enabled(void);
 #define SEV_SNP_POLICY_SMT      0x10000
 #define SEV_SNP_POLICY_DBG      0x80000
 
-#define SVM_SEV_FEAT_SNP_ACTIVE 1
+#define SVM_SEV_FEAT_SNP_ACTIVE     BIT(0)
 
 typedef struct SevKernelLoaderContext {
     char *setup_data;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 1057b8ab2c60..2fb1268ed788 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -319,6 +319,15 @@ sev_set_guest_state(SevCommonState *sev_common, SevState new_state)
     sev_common->state = new_state;
 }
 
+static void sev_set_feature(SevCommonState *sev_common, uint64_t feature, bool set)
+{
+    if (set) {
+        sev_common->sev_features |= feature;
+    } else {
+        sev_common->sev_features &= ~feature;
+    }
+}
+
 static void
 sev_ram_block_added(RAMBlockNotifier *n, void *host, size_t size,
                     size_t max_size)
@@ -1897,15 +1906,15 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
                 -1) {
                 return -1;
             }
-            /*
-             * KVM maintains a bitmask of allowed sev_features. This does not
-             * include SVM_SEV_FEAT_SNP_ACTIVE which is set accordingly by KVM
-             * itself. Therefore we need to clear this flag.
-             */
-            args.vmsa_features = sev_common->sev_features &
-                                 ~SVM_SEV_FEAT_SNP_ACTIVE;
         }
 
+        /*
+         * KVM maintains a bitmask of allowed sev_features. This does not
+         * include SVM_SEV_FEAT_SNP_ACTIVE which is set accordingly by KVM
+         * itself. Therefore we need to clear this flag.
+         */
+        args.vmsa_features = sev_common->sev_features & ~SVM_SEV_FEAT_SNP_ACTIVE;
+
         ret = sev_ioctl(sev_common->sev_fd, KVM_SEV_INIT2, &args, &fw_error);
         break;
     }
@@ -3127,6 +3136,7 @@ sev_snp_guest_instance_init(Object *obj)
 
     /* default init/start/finish params for kvm */
     sev_snp_guest->kvm_start_conf.policy = DEFAULT_SEV_SNP_POLICY;
+    sev_set_feature(SEV_COMMON(sev_snp_guest), SVM_SEV_FEAT_SNP_ACTIVE, true);
 }
 
 /* guest info specific to sev-snp */
-- 
2.51.0

[PATCH v2 2/9] target/i386: SEV: Ensure SEV features are only set through qemu cli or IGVM

[Naveen N Rao (AMD)]

In preparation for qemu being able to set SEV features through the cli,
add a check to ensure that SEV features are not also set if using IGVM
files.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
---
 target/i386/sev.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 2fb1268ed788..ddd7c01f5a56 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -1901,6 +1901,15 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
          * as SEV_STATE_UNINIT.
          */
         if (x86machine->igvm) {
+            /*
+             * Test only the user-set SEV features by masking out
+             * SVM_SEV_FEAT_SNP_ACTIVE which is set by default.
+             */
+            if (sev_common->sev_features & ~SVM_SEV_FEAT_SNP_ACTIVE) {
+                error_setg(errp, "%s: SEV features can't be specified when using IGVM files",
+                           __func__);
+                return -1;
+            }
             if (IGVM_CFG_GET_CLASS(x86machine->igvm)
                     ->process(x86machine->igvm, machine->cgs, true, errp) ==
                 -1) {
-- 
2.51.0

[PATCH v2 3/9] target/i386: SEV: Consolidate SEV feature validation to common init path

[Naveen N Rao (AMD)]

Currently, check_sev_features() is called in multiple places when
processing IGVM files: both when processing the initial VMSA SEV
features from IGVM, as well as when validating the full contents of the
VMSA. Move this to a single point in sev_common_kvm_init() to simplify
the flow, as well as to re-use this function when VMSA SEV features are
being set without using IGVM files.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
---
 target/i386/sev.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index ddd7c01f5a56..3b11e61f78d8 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -595,9 +595,6 @@ static int check_vmsa_supported(SevCommonState *sev_common, hwaddr gpa,
     vmsa_check.x87_fcw = 0;
     vmsa_check.mxcsr = 0;
 
-    if (check_sev_features(sev_common, vmsa_check.sev_features, errp) < 0) {
-        return -1;
-    }
     vmsa_check.sev_features = 0;
 
     if (!buffer_is_zero(&vmsa_check, sizeof(vmsa_check))) {
@@ -1917,6 +1914,10 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
             }
         }
 
+        if (check_sev_features(sev_common, sev_common->sev_features, errp) < 0) {
+            return -1;
+        }
+
         /*
          * KVM maintains a bitmask of allowed sev_features. This does not
          * include SVM_SEV_FEAT_SNP_ACTIVE which is set accordingly by KVM
@@ -2536,9 +2537,6 @@ static int cgs_set_guest_state(hwaddr gpa, uint8_t *ptr, uint64_t len,
                            __func__);
                 return -1;
             }
-            if (check_sev_features(sev_common, sa->sev_features, errp) < 0) {
-                return -1;
-            }
             sev_common->sev_features = sa->sev_features;
         }
         return 0;
-- 
2.51.0

[PATCH v2 4/9] target/i386: SEV: Validate that SEV-ES is enabled when VMSA features are used

[Naveen N Rao (AMD)]

SEV features in the VMSA are only meaningful for SEV-ES and SEV-SNP
guests, as they control aspects of the encrypted guest state that are
not relevant for basic SEV guests.

Add a check in check_sev_features() to ensure that SEV-ES or SEV-SNP is
enabled when any SEV features are specified.

Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
---
 target/i386/sev.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 3b11e61f78d8..2f41e1c0b688 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -518,6 +518,12 @@ static int check_sev_features(SevCommonState *sev_common, uint64_t sev_features,
             __func__);
         return -1;
     }
+    if (sev_features && !sev_es_enabled()) {
+        error_setg(errp,
+                   "%s: SEV features require either SEV-ES or SEV-SNP to be enabled",
+                   __func__);
+        return -1;
+    }
     if (sev_features & ~sev_common->supported_sev_features) {
         error_setg(errp,
                    "%s: VMSA contains unsupported sev_features: %lX, "
-- 
2.51.0

[PATCH v2 5/9] target/i386: SEV: Enable use of KVM_SEV_INIT2 for SEV-ES guests

[Naveen N Rao (AMD)]

In preparation for allowing SEV-ES guests to enable VMSA SEV features,
update sev_init2_required() to return true if any SEV features are
requested. This enables qemu to use KVM_SEV_INIT2 for SEV-ES guests when
necessary.

Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
---
 target/i386/sev.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 2f41e1c0b688..88dd0750d481 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -1699,8 +1699,7 @@ sev_vm_state_change(void *opaque, bool running, RunState state)
  */
 static bool sev_init2_required(SevGuestState *sev_guest)
 {
-    /* Currently no KVM_SEV_INIT2-specific options are exposed via QEMU */
-    return false;
+    return !!SEV_COMMON(sev_guest)->sev_features;
 }
 
 static int sev_kvm_type(X86ConfidentialGuest *cg)
-- 
2.51.0

[PATCH v2 6/9] target/i386: SEV: Add support for enabling debug-swap SEV feature

[Naveen N Rao (AMD)]

Add support for enabling debug-swap VMSA SEV feature in SEV-ES and
SEV-SNP guests through a new "debug-swap" boolean property on SEV guest
objects. Though the boolean property is available for plain SEV guests,
check_sev_features() will reject setting this for plain SEV guests.

Though this SEV feature is called "Debug virtualization" in the APM, KVM
calls this "debug swap" so use the same name for consistency.

Sample command-line:
  -machine q35,confidential-guest-support=sev0 \
  -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,debug-swap=on

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
---
 target/i386/sev.h |  1 +
 target/i386/sev.c | 20 ++++++++++++++++++++
 qapi/qom.json     |  6 +++++-
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index 102546b112d6..8e09b2ce1976 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -45,6 +45,7 @@ bool sev_snp_enabled(void);
 #define SEV_SNP_POLICY_DBG      0x80000
 
 #define SVM_SEV_FEAT_SNP_ACTIVE     BIT(0)
+#define SVM_SEV_FEAT_DEBUG_SWAP     BIT(5)
 
 typedef struct SevKernelLoaderContext {
     char *setup_data;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index 88dd0750d481..e9d84ea25571 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -319,6 +319,11 @@ sev_set_guest_state(SevCommonState *sev_common, SevState new_state)
     sev_common->state = new_state;
 }
 
+static bool is_sev_feature_set(SevCommonState *sev_common, uint64_t feature)
+{
+    return !!(sev_common->sev_features & feature);
+}
+
 static void sev_set_feature(SevCommonState *sev_common, uint64_t feature, bool set)
 {
     if (set) {
@@ -2744,6 +2749,16 @@ static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
     return 0;
 }
 
+static bool sev_common_get_debug_swap(Object *obj, Error **errp)
+{
+    return is_sev_feature_set(SEV_COMMON(obj), SVM_SEV_FEAT_DEBUG_SWAP);
+}
+
+static void sev_common_set_debug_swap(Object *obj, bool value, Error **errp)
+{
+    sev_set_feature(SEV_COMMON(obj), SVM_SEV_FEAT_DEBUG_SWAP, value);
+}
+
 static void
 sev_common_class_init(ObjectClass *oc, const void *data)
 {
@@ -2761,6 +2776,11 @@ sev_common_class_init(ObjectClass *oc, const void *data)
                                    sev_common_set_kernel_hashes);
     object_class_property_set_description(oc, "kernel-hashes",
             "add kernel hashes to guest firmware for measured Linux boot");
+    object_class_property_add_bool(oc, "debug-swap",
+                                   sev_common_get_debug_swap,
+                                   sev_common_set_debug_swap);
+    object_class_property_set_description(oc, "debug-swap",
+            "enable virtualization of debug registers");
 }
 
 static void
diff --git a/qapi/qom.json b/qapi/qom.json
index 830cb2ffe781..df962d4a5215 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1010,13 +1010,17 @@
 #     designated guest firmware page for measured boot with -kernel
 #     (default: false) (since 6.2)
 #
+# @debug-swap: enable virtualization of debug registers
+#     (default: false) (since 10.2)
+#
 # Since: 9.1
 ##
 { 'struct': 'SevCommonProperties',
   'data': { '*sev-device': 'str',
             '*cbitpos': 'uint32',
             'reduced-phys-bits': 'uint32',
-            '*kernel-hashes': 'bool' } }
+            '*kernel-hashes': 'bool',
+            '*debug-swap': 'bool' } }
 
 ##
 # @SevGuestProperties:
-- 
2.51.0

[PATCH v2 7/9] target/i386: SEV: Add support for enabling Secure TSC SEV feature

[Naveen N Rao (AMD)]

Add support for enabling Secure TSC VMSA SEV feature in SEV-SNP guests
through a new "secure-tsc" boolean property on SEV-SNP guest objects. By
default, KVM uses the host TSC frequency for Secure TSC.

Sample command-line:
  -machine q35,confidential-guest-support=sev0 \
  -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,secure-tsc=on

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
Co-developed-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
Signed-off-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
Co-developed-by: Nikunj A Dadhania <nikunj@amd.com>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
---
 target/i386/sev.h |  1 +
 target/i386/sev.c | 13 +++++++++++++
 qapi/qom.json     |  6 +++++-
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index 8e09b2ce1976..87e73034ad15 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -46,6 +46,7 @@ bool sev_snp_enabled(void);
 
 #define SVM_SEV_FEAT_SNP_ACTIVE     BIT(0)
 #define SVM_SEV_FEAT_DEBUG_SWAP     BIT(5)
+#define SVM_SEV_FEAT_SECURE_TSC     BIT(9)
 
 typedef struct SevKernelLoaderContext {
     char *setup_data;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index e9d84ea25571..68d193402de3 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -3121,6 +3121,16 @@ sev_snp_guest_set_host_data(Object *obj, const char *value, Error **errp)
     memcpy(finish->host_data, blob, len);
 }
 
+static bool sev_snp_guest_get_secure_tsc(Object *obj, Error **errp)
+{
+    return is_sev_feature_set(SEV_COMMON(obj), SVM_SEV_FEAT_SECURE_TSC);
+}
+
+static void sev_snp_guest_set_secure_tsc(Object *obj, bool value, Error **errp)
+{
+    sev_set_feature(SEV_COMMON(obj), SVM_SEV_FEAT_SECURE_TSC, value);
+}
+
 static void
 sev_snp_guest_class_init(ObjectClass *oc, const void *data)
 {
@@ -3156,6 +3166,9 @@ sev_snp_guest_class_init(ObjectClass *oc, const void *data)
     object_class_property_add_str(oc, "host-data",
                                   sev_snp_guest_get_host_data,
                                   sev_snp_guest_set_host_data);
+    object_class_property_add_bool(oc, "secure-tsc",
+                                  sev_snp_guest_get_secure_tsc,
+                                  sev_snp_guest_set_secure_tsc);
 }
 
 static void
diff --git a/qapi/qom.json b/qapi/qom.json
index df962d4a5215..52c23e85e349 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1100,6 +1100,9 @@
 #     firmware.  Set this to true to disable the use of VCEK.
 #     (default: false) (since: 9.1)
 #
+# @secure-tsc: enable Secure TSC
+#     (default: false) (since 10.2)
+#
 # Since: 9.1
 ##
 { 'struct': 'SevSnpGuestProperties',
@@ -1111,7 +1114,8 @@
             '*id-auth': 'str',
             '*author-key-enabled': 'bool',
             '*host-data': 'str',
-            '*vcek-disabled': 'bool' } }
+            '*vcek-disabled': 'bool',
+            '*secure-tsc': 'bool' } }
 
 ##
 # @TdxGuestProperties:
-- 
2.51.0

[PATCH v2 8/9] target/i386: SEV: Add support for setting TSC frequency for Secure TSC

[Naveen N Rao (AMD)]

Add support for configuring the TSC frequency when Secure TSC is enabled
in SEV-SNP guests through a new "tsc-frequency" property on SEV-SNP
guest objects, similar to the vCPU-specific property used by regular
guests and TDX. A new property is needed since SEV-SNP guests require
the TSC frequency to be specified during early SNP_LAUNCH_START command
before any vCPUs are created.

The user-provided TSC frequency is set through KVM_SET_TSC_KHZ before
issuing KVM_SEV_SNP_LAUNCH_START.

Sample command-line:
  -machine q35,confidential-guest-support=sev0 \
  -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,secure-tsc=on,tsc-frequency=2500000000

Co-developed-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
Signed-off-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
Co-developed-by: Nikunj A Dadhania <nikunj@amd.com>
Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
---
 target/i386/sev.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 qapi/qom.json     |  6 +++++-
 2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 68d193402de3..8bb9faaa7779 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -178,6 +178,7 @@ struct SevSnpGuestState {
     char *id_auth_base64;
     uint8_t *id_auth;
     char *host_data;
+    uint32_t tsc_khz;
 
     struct kvm_sev_snp_launch_start kvm_start_conf;
     struct kvm_sev_snp_launch_finish kvm_finish_conf;
@@ -536,6 +537,13 @@ static int check_sev_features(SevCommonState *sev_common, uint64_t sev_features,
                    __func__, sev_features, sev_common->supported_sev_features);
         return -1;
     }
+    if (sev_snp_enabled() && SEV_SNP_GUEST(sev_common)->tsc_khz &&
+        !(sev_features & SVM_SEV_FEAT_SECURE_TSC)) {
+        error_setg(errp,
+                   "%s: TSC frequency can only be set if Secure TSC is enabled",
+                   __func__);
+        return -1;
+    }
     return 0;
 }
 
@@ -1085,6 +1093,19 @@ sev_snp_launch_start(SevCommonState *sev_common)
             return 1;
     }
 
+    if (is_sev_feature_set(sev_common, SVM_SEV_FEAT_SECURE_TSC) &&
+        sev_snp_guest->tsc_khz) {
+        rc = -EINVAL;
+        if (kvm_check_extension(kvm_state, KVM_CAP_VM_TSC_CONTROL)) {
+            rc = kvm_vm_ioctl(kvm_state, KVM_SET_TSC_KHZ, sev_snp_guest->tsc_khz);
+        }
+        if (rc < 0) {
+            error_report("%s: Unable to set Secure TSC frequency to %u kHz ret=%d",
+                         __func__, sev_snp_guest->tsc_khz, rc);
+            return 1;
+        }
+    }
+
     rc = sev_ioctl(sev_common->sev_fd, KVM_SEV_SNP_LAUNCH_START,
                    start, &fw_error);
     if (rc < 0) {
@@ -3131,6 +3152,28 @@ static void sev_snp_guest_set_secure_tsc(Object *obj, bool value, Error **errp)
     sev_set_feature(SEV_COMMON(obj), SVM_SEV_FEAT_SECURE_TSC, value);
 }
 
+static void
+sev_snp_guest_get_tsc_frequency(Object *obj, Visitor *v, const char *name,
+                                void *opaque, Error **errp)
+{
+    uint32_t value = SEV_SNP_GUEST(obj)->tsc_khz * 1000;
+
+    visit_type_uint32(v, name, &value, errp);
+}
+
+static void
+sev_snp_guest_set_tsc_frequency(Object *obj, Visitor *v, const char *name,
+                                void *opaque, Error **errp)
+{
+    uint32_t value;
+
+    if (!visit_type_uint32(v, name, &value, errp)) {
+        return;
+    }
+
+    SEV_SNP_GUEST(obj)->tsc_khz = value / 1000;
+}
+
 static void
 sev_snp_guest_class_init(ObjectClass *oc, const void *data)
 {
@@ -3169,6 +3212,9 @@ sev_snp_guest_class_init(ObjectClass *oc, const void *data)
     object_class_property_add_bool(oc, "secure-tsc",
                                   sev_snp_guest_get_secure_tsc,
                                   sev_snp_guest_set_secure_tsc);
+    object_class_property_add(oc, "tsc-frequency", "uint32",
+                              sev_snp_guest_get_tsc_frequency,
+                              sev_snp_guest_set_tsc_frequency, NULL, NULL);
 }
 
 static void
diff --git a/qapi/qom.json b/qapi/qom.json
index 52c23e85e349..c01ae70dd43d 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1103,6 +1103,9 @@
 # @secure-tsc: enable Secure TSC
 #     (default: false) (since 10.2)
 #
+# @tsc-frequency: set secure TSC frequency.  Only valid if Secure TSC
+#     is enabled (default: zero) (since 10.2)
+#
 # Since: 9.1
 ##
 { 'struct': 'SevSnpGuestProperties',
@@ -1115,7 +1118,8 @@
             '*author-key-enabled': 'bool',
             '*host-data': 'str',
             '*vcek-disabled': 'bool',
-            '*secure-tsc': 'bool' } }
+            '*secure-tsc': 'bool',
+            '*tsc-frequency': 'uint32' } }
 
 ##
 # @TdxGuestProperties:
-- 
2.51.0

[PATCH v2 9/9] target/i386: SEV: Refactor check_sev_features()

[Naveen N Rao (AMD)]

Refactor check_sev_features() to consolidate SEV-SNP checks to a single
if block. This is also helpful when adding checks for future SEV
features.  While at it, move the comment about the checks being done
outside of the function body and expand it to describe what this
function does. Update error_setg() invocations to use a consistent
format.

No functional change intended.

Suggested-by: Tom Lendacky <thomas.lendacky@amd.com>
Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
---
 target/i386/sev.c | 55 ++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 25 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 8bb9faaa7779..138210e24124 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -502,34 +502,22 @@ static void sev_apply_cpu_context(CPUState *cpu)
     }
 }
 
+/*
+ * Ensure SEV_FEATURES is configured for correct SEV hardware and that
+ * the requested features are supported. In addition, ensure feature
+ * dependencies are satisfied (allow tsc-frequency only if secure-tsc
+ * is also enabled, as an example).
+ */
 static int check_sev_features(SevCommonState *sev_common, uint64_t sev_features,
                               Error **errp)
 {
-    /*
-     * Ensure SEV_FEATURES is configured for correct SEV hardware and that
-     * the requested features are supported. If SEV-SNP is enabled then
-     * that feature must be enabled, otherwise it must be cleared.
-     */
-    if (sev_snp_enabled() && !(sev_features & SVM_SEV_FEAT_SNP_ACTIVE)) {
-        error_setg(
-            errp,
-            "%s: SEV_SNP is enabled but is not enabled in VMSA sev_features",
-            __func__);
-        return -1;
-    } else if (!sev_snp_enabled() &&
-               (sev_features & SVM_SEV_FEAT_SNP_ACTIVE)) {
-        error_setg(
-            errp,
-            "%s: SEV_SNP is not enabled but is enabled in VMSA sev_features",
-            __func__);
-        return -1;
-    }
     if (sev_features && !sev_es_enabled()) {
         error_setg(errp,
                    "%s: SEV features require either SEV-ES or SEV-SNP to be enabled",
                    __func__);
         return -1;
     }
+
     if (sev_features & ~sev_common->supported_sev_features) {
         error_setg(errp,
                    "%s: VMSA contains unsupported sev_features: %lX, "
@@ -537,13 +525,30 @@ static int check_sev_features(SevCommonState *sev_common, uint64_t sev_features,
                    __func__, sev_features, sev_common->supported_sev_features);
         return -1;
     }
-    if (sev_snp_enabled() && SEV_SNP_GUEST(sev_common)->tsc_khz &&
-        !(sev_features & SVM_SEV_FEAT_SECURE_TSC)) {
-        error_setg(errp,
-                   "%s: TSC frequency can only be set if Secure TSC is enabled",
-                   __func__);
-        return -1;
+
+    if (sev_snp_enabled()) {
+        if (!(sev_features & SVM_SEV_FEAT_SNP_ACTIVE)) {
+            error_setg(errp,
+                       "%s: SEV_SNP is enabled but is not enabled in VMSA sev_features",
+                       __func__);
+            return -1;
+        }
+        if (SEV_SNP_GUEST(sev_common)->tsc_khz &&
+            !(sev_features & SVM_SEV_FEAT_SECURE_TSC)) {
+            error_setg(errp,
+                       "%s: TSC frequency can only be set if Secure TSC is enabled",
+                       __func__);
+            return -1;
+        }
+    } else {
+        if (sev_features & SVM_SEV_FEAT_SNP_ACTIVE) {
+            error_setg(errp,
+                       "%s: SEV_SNP is not enabled but is enabled in VMSA sev_features",
+                       __func__);
+            return -1;
+        }
     }
+
     return 0;
 }
 
-- 
2.51.0

Re: [PATCH v2 6/9] target/i386: SEV: Add support for enabling debug-swap SEV feature

[Markus Armbruster]

"Naveen N Rao (AMD)" <naveen@kernel.org> writes:

> Add support for enabling debug-swap VMSA SEV feature in SEV-ES and
> SEV-SNP guests through a new "debug-swap" boolean property on SEV guest
> objects. Though the boolean property is available for plain SEV guests,
> check_sev_features() will reject setting this for plain SEV guests.

Is this the sev_features && !sev_es_enabled() check there?

Does "reject setting this" mean setting it to true is rejected, or does
it mean setting it to any value is rejected?

> Though this SEV feature is called "Debug virtualization" in the APM, KVM
> calls this "debug swap" so use the same name for consistency.
>
> Sample command-line:
>   -machine q35,confidential-guest-support=sev0 \
>   -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,debug-swap=on

Always appreciated in commit messages.

I get "cannot set up private guest memory for sev-snp-guest: KVM
required".  If I add the obvious "-accel kvm", I get "-accel kvm:
vm-type SEV-SNP not supported by KVM".  I figure that's because my
hardware isn't capable.  The error message could be clearer.  Not this
patch's fault.

> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
> ---
>  target/i386/sev.h |  1 +
>  target/i386/sev.c | 20 ++++++++++++++++++++
>  qapi/qom.json     |  6 +++++-
>  3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/sev.h b/target/i386/sev.h
> index 102546b112d6..8e09b2ce1976 100644
> --- a/target/i386/sev.h
> +++ b/target/i386/sev.h
> @@ -45,6 +45,7 @@ bool sev_snp_enabled(void);
>  #define SEV_SNP_POLICY_DBG      0x80000
>  
>  #define SVM_SEV_FEAT_SNP_ACTIVE     BIT(0)
> +#define SVM_SEV_FEAT_DEBUG_SWAP     BIT(5)
>  
>  typedef struct SevKernelLoaderContext {
>      char *setup_data;
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 88dd0750d481..e9d84ea25571 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -319,6 +319,11 @@ sev_set_guest_state(SevCommonState *sev_common, SevState new_state)
>      sev_common->state = new_state;
>  }
>  
> +static bool is_sev_feature_set(SevCommonState *sev_common, uint64_t feature)
> +{
> +    return !!(sev_common->sev_features & feature);
> +}
> +
>  static void sev_set_feature(SevCommonState *sev_common, uint64_t feature, bool set)
>  {
>      if (set) {
> @@ -2744,6 +2749,16 @@ static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
>      return 0;
>  }
>  
> +static bool sev_common_get_debug_swap(Object *obj, Error **errp)
> +{
> +    return is_sev_feature_set(SEV_COMMON(obj), SVM_SEV_FEAT_DEBUG_SWAP);
> +}
> +
> +static void sev_common_set_debug_swap(Object *obj, bool value, Error **errp)
> +{
> +    sev_set_feature(SEV_COMMON(obj), SVM_SEV_FEAT_DEBUG_SWAP, value);
> +}
> +
>  static void
>  sev_common_class_init(ObjectClass *oc, const void *data)
>  {
> @@ -2761,6 +2776,11 @@ sev_common_class_init(ObjectClass *oc, const void *data)
>                                     sev_common_set_kernel_hashes);
>      object_class_property_set_description(oc, "kernel-hashes",
>              "add kernel hashes to guest firmware for measured Linux boot");
> +    object_class_property_add_bool(oc, "debug-swap",
> +                                   sev_common_get_debug_swap,
> +                                   sev_common_set_debug_swap);
> +    object_class_property_set_description(oc, "debug-swap",
> +            "enable virtualization of debug registers");
>  }
>  
>  static void
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 830cb2ffe781..df962d4a5215 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -1010,13 +1010,17 @@
>  #     designated guest firmware page for measured boot with -kernel
>  #     (default: false) (since 6.2)
>  #
> +# @debug-swap: enable virtualization of debug registers
> +#     (default: false) (since 10.2)
> +#

According to the commit message, setting @default-swap works only for
SEV-ES and SEV-SNP guests, i.e. it fails for plain SEV guests.  Should
we document this here?

>  # Since: 9.1
>  ##
>  { 'struct': 'SevCommonProperties',
>    'data': { '*sev-device': 'str',
>              '*cbitpos': 'uint32',
>              'reduced-phys-bits': 'uint32',
> -            '*kernel-hashes': 'bool' } }
> +            '*kernel-hashes': 'bool',
> +            '*debug-swap': 'bool' } }
>  
>  ##
>  # @SevGuestProperties:

Re: [PATCH v2 8/9] target/i386: SEV: Add support for setting TSC frequency for Secure TSC

[Tom Lendacky]

On 9/25/25 05:17, Naveen N Rao (AMD) wrote:
> Add support for configuring the TSC frequency when Secure TSC is enabled
> in SEV-SNP guests through a new "tsc-frequency" property on SEV-SNP
> guest objects, similar to the vCPU-specific property used by regular
> guests and TDX. A new property is needed since SEV-SNP guests require
> the TSC frequency to be specified during early SNP_LAUNCH_START command
> before any vCPUs are created.
> 
> The user-provided TSC frequency is set through KVM_SET_TSC_KHZ before
> issuing KVM_SEV_SNP_LAUNCH_START.
> 
> Sample command-line:
>   -machine q35,confidential-guest-support=sev0 \
>   -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,secure-tsc=on,tsc-frequency=2500000000
> 
> Co-developed-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
> Signed-off-by: Ketan Chaturvedi <Ketan.Chaturvedi@amd.com>
> Co-developed-by: Nikunj A Dadhania <nikunj@amd.com>
> Signed-off-by: Nikunj A Dadhania <nikunj@amd.com>
> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
> ---
>  target/i386/sev.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
>  qapi/qom.json     |  6 +++++-
>  2 files changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 68d193402de3..8bb9faaa7779 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -178,6 +178,7 @@ struct SevSnpGuestState {
>      char *id_auth_base64;
>      uint8_t *id_auth;
>      char *host_data;
> +    uint32_t tsc_khz;
>  
>      struct kvm_sev_snp_launch_start kvm_start_conf;
>      struct kvm_sev_snp_launch_finish kvm_finish_conf;
> @@ -536,6 +537,13 @@ static int check_sev_features(SevCommonState *sev_common, uint64_t sev_features,
>                     __func__, sev_features, sev_common->supported_sev_features);
>          return -1;
>      }
> +    if (sev_snp_enabled() && SEV_SNP_GUEST(sev_common)->tsc_khz &&
> +        !(sev_features & SVM_SEV_FEAT_SECURE_TSC)) {
> +        error_setg(errp,
> +                   "%s: TSC frequency can only be set if Secure TSC is enabled",
> +                   __func__);
> +        return -1;
> +    }
>      return 0;
>  }
>  
> @@ -1085,6 +1093,19 @@ sev_snp_launch_start(SevCommonState *sev_common)
>              return 1;
>      }
>  
> +    if (is_sev_feature_set(sev_common, SVM_SEV_FEAT_SECURE_TSC) &&
> +        sev_snp_guest->tsc_khz) {
> +        rc = -EINVAL;
> +        if (kvm_check_extension(kvm_state, KVM_CAP_VM_TSC_CONTROL)) {
> +            rc = kvm_vm_ioctl(kvm_state, KVM_SET_TSC_KHZ, sev_snp_guest->tsc_khz);
> +        }
> +        if (rc < 0) {
> +            error_report("%s: Unable to set Secure TSC frequency to %u kHz ret=%d",
> +                         __func__, sev_snp_guest->tsc_khz, rc);
> +            return 1;
> +        }
> +    }
> +
>      rc = sev_ioctl(sev_common->sev_fd, KVM_SEV_SNP_LAUNCH_START,
>                     start, &fw_error);
>      if (rc < 0) {
> @@ -3131,6 +3152,28 @@ static void sev_snp_guest_set_secure_tsc(Object *obj, bool value, Error **errp)
>      sev_set_feature(SEV_COMMON(obj), SVM_SEV_FEAT_SECURE_TSC, value);
>  }
>  
> +static void
> +sev_snp_guest_get_tsc_frequency(Object *obj, Visitor *v, const char *name,
> +                                void *opaque, Error **errp)
> +{
> +    uint32_t value = SEV_SNP_GUEST(obj)->tsc_khz * 1000;
> +
> +    visit_type_uint32(v, name, &value, errp);
> +}
> +
> +static void
> +sev_snp_guest_set_tsc_frequency(Object *obj, Visitor *v, const char *name,
> +                                void *opaque, Error **errp)
> +{
> +    uint32_t value;
> +
> +    if (!visit_type_uint32(v, name, &value, errp)) {
> +        return;
> +    }
> +
> +    SEV_SNP_GUEST(obj)->tsc_khz = value / 1000;

This will cause a value that isn't evenly divisible by 1000 to be
rounded down, e.g.: tsc-frequency=2500000999. Should this name instead
just be tsc-khz or secure-tsc-khz (to show it is truly associated with
Secure TSC)?

Also, I think there is already a "tsc-freq" parameter for the -cpu
parameter (?), should there be some kind of error message if both of
these are set? Or a warning saying it is being ignored? Or ...?

Thanks,
Tom

> +}
> +
>  static void
>  sev_snp_guest_class_init(ObjectClass *oc, const void *data)
>  {
> @@ -3169,6 +3212,9 @@ sev_snp_guest_class_init(ObjectClass *oc, const void *data)
>      object_class_property_add_bool(oc, "secure-tsc",
>                                    sev_snp_guest_get_secure_tsc,
>                                    sev_snp_guest_set_secure_tsc);
> +    object_class_property_add(oc, "tsc-frequency", "uint32",
> +                              sev_snp_guest_get_tsc_frequency,
> +                              sev_snp_guest_set_tsc_frequency, NULL, NULL);
>  }
>  
>  static void
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 52c23e85e349..c01ae70dd43d 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -1103,6 +1103,9 @@
>  # @secure-tsc: enable Secure TSC
>  #     (default: false) (since 10.2)
>  #
> +# @tsc-frequency: set secure TSC frequency.  Only valid if Secure TSC
> +#     is enabled (default: zero) (since 10.2)
> +#
>  # Since: 9.1
>  ##
>  { 'struct': 'SevSnpGuestProperties',
> @@ -1115,7 +1118,8 @@
>              '*author-key-enabled': 'bool',
>              '*host-data': 'str',
>              '*vcek-disabled': 'bool',
> -            '*secure-tsc': 'bool' } }
> +            '*secure-tsc': 'bool',
> +            '*tsc-frequency': 'uint32' } }
>  
>  ##
>  # @TdxGuestProperties:

Re: [PATCH v2 6/9] target/i386: SEV: Add support for enabling debug-swap SEV feature

[Naveen N Rao]

On Tue, Oct 07, 2025 at 08:14:37AM +0200, Markus Armbruster wrote:
> "Naveen N Rao (AMD)" <naveen@kernel.org> writes:
> 
> > Add support for enabling debug-swap VMSA SEV feature in SEV-ES and
> > SEV-SNP guests through a new "debug-swap" boolean property on SEV guest
> > objects. Though the boolean property is available for plain SEV guests,
> > check_sev_features() will reject setting this for plain SEV guests.
> 
> Is this the sev_features && !sev_es_enabled() check there?

Yes, that's the one.

> 
> Does "reject setting this" mean setting it to true is rejected, or does
> it mean setting it to any value is rejected?

Right -- we don't allow this to be "enabled". Passing "debug-swap=off" 
should mostly be a no-op.

> 
> > Though this SEV feature is called "Debug virtualization" in the APM, KVM
> > calls this "debug swap" so use the same name for consistency.
> >
> > Sample command-line:
> >   -machine q35,confidential-guest-support=sev0 \
> >   -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,debug-swap=on
> 
> Always appreciated in commit messages.
> 
> I get "cannot set up private guest memory for sev-snp-guest: KVM
> required".  If I add the obvious "-accel kvm", I get "-accel kvm:
> vm-type SEV-SNP not supported by KVM".  I figure that's because my
> hardware isn't capable.  The error message could be clearer.  Not this
> patch's fault.

SEV needs to be explicitly enabled in the BIOS:
https://github.com/AMDESE/AMDSEV/tree/snp-latest?tab=readme-ov-file#prepare-host

Be sure to enable SMEE first to be able to see the other options.

> 
> > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>
> > Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
> > ---
> >  target/i386/sev.h |  1 +
> >  target/i386/sev.c | 20 ++++++++++++++++++++
> >  qapi/qom.json     |  6 +++++-
> >  3 files changed, 26 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/i386/sev.h b/target/i386/sev.h
> > index 102546b112d6..8e09b2ce1976 100644
> > --- a/target/i386/sev.h
> > +++ b/target/i386/sev.h
> > @@ -45,6 +45,7 @@ bool sev_snp_enabled(void);
> >  #define SEV_SNP_POLICY_DBG      0x80000
> >  
> >  #define SVM_SEV_FEAT_SNP_ACTIVE     BIT(0)
> > +#define SVM_SEV_FEAT_DEBUG_SWAP     BIT(5)
> >  
> >  typedef struct SevKernelLoaderContext {
> >      char *setup_data;
> > diff --git a/target/i386/sev.c b/target/i386/sev.c
> > index 88dd0750d481..e9d84ea25571 100644
> > --- a/target/i386/sev.c
> > +++ b/target/i386/sev.c
> > @@ -319,6 +319,11 @@ sev_set_guest_state(SevCommonState *sev_common, SevState new_state)
> >      sev_common->state = new_state;
> >  }
> >  
> > +static bool is_sev_feature_set(SevCommonState *sev_common, uint64_t feature)
> > +{
> > +    return !!(sev_common->sev_features & feature);
> > +}
> > +
> >  static void sev_set_feature(SevCommonState *sev_common, uint64_t feature, bool set)
> >  {
> >      if (set) {
> > @@ -2744,6 +2749,16 @@ static int cgs_set_guest_policy(ConfidentialGuestPolicyType policy_type,
> >      return 0;
> >  }
> >  
> > +static bool sev_common_get_debug_swap(Object *obj, Error **errp)
> > +{
> > +    return is_sev_feature_set(SEV_COMMON(obj), SVM_SEV_FEAT_DEBUG_SWAP);
> > +}
> > +
> > +static void sev_common_set_debug_swap(Object *obj, bool value, Error **errp)
> > +{
> > +    sev_set_feature(SEV_COMMON(obj), SVM_SEV_FEAT_DEBUG_SWAP, value);
> > +}
> > +
> >  static void
> >  sev_common_class_init(ObjectClass *oc, const void *data)
> >  {
> > @@ -2761,6 +2776,11 @@ sev_common_class_init(ObjectClass *oc, const void *data)
> >                                     sev_common_set_kernel_hashes);
> >      object_class_property_set_description(oc, "kernel-hashes",
> >              "add kernel hashes to guest firmware for measured Linux boot");
> > +    object_class_property_add_bool(oc, "debug-swap",
> > +                                   sev_common_get_debug_swap,
> > +                                   sev_common_set_debug_swap);
> > +    object_class_property_set_description(oc, "debug-swap",
> > +            "enable virtualization of debug registers");
> >  }
> >  
> >  static void
> > diff --git a/qapi/qom.json b/qapi/qom.json
> > index 830cb2ffe781..df962d4a5215 100644
> > --- a/qapi/qom.json
> > +++ b/qapi/qom.json
> > @@ -1010,13 +1010,17 @@
> >  #     designated guest firmware page for measured boot with -kernel
> >  #     (default: false) (since 6.2)
> >  #
> > +# @debug-swap: enable virtualization of debug registers
> > +#     (default: false) (since 10.2)
> > +#
> 
> According to the commit message, setting @default-swap works only for
> SEV-ES and SEV-SNP guests, i.e. it fails for plain SEV guests.  Should
> we document this here?

Sure, we can add that.


Thanks,
Naveen

Re: [PATCH v2 8/9] target/i386: SEV: Add support for setting TSC frequency for Secure TSC

[Naveen N Rao]

On Tue, Oct 07, 2025 at 08:31:47AM -0500, Tom Lendacky wrote:
> On 9/25/25 05:17, Naveen N Rao (AMD) wrote:

...

> > +
> > +static void
> > +sev_snp_guest_set_tsc_frequency(Object *obj, Visitor *v, const char *name,
> > +                                void *opaque, Error **errp)
> > +{
> > +    uint32_t value;
> > +
> > +    if (!visit_type_uint32(v, name, &value, errp)) {
> > +        return;
> > +    }
> > +
> > +    SEV_SNP_GUEST(obj)->tsc_khz = value / 1000;
> 
> This will cause a value that isn't evenly divisible by 1000 to be
> rounded down, e.g.: tsc-frequency=2500000999. Should this name instead
> just be tsc-khz or secure-tsc-khz (to show it is truly associated with
> Secure TSC)?

I modeled this after the existing tsc-frequency parameter on the cpu 
object to keep it simple (parameter is the same, just where it is 
specified differs). This also aligns with TDX which re-uses the 
tsc-frequency parameter on the cpu object.

> 
> Also, I think there is already a "tsc-freq" parameter for the -cpu
> parameter (?), should there be some kind of error message if both of
> these are set? Or a warning saying it is being ignored? Or ...?

This is validated when the TSC frequency is being set on the vcpu, so I didn't
add an explicit check.

As an example, with:
  -cpu EPYC-v4,tsc-frequency=2500000000 \
  -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,secure-tsc=on

qemu-system-x86_64: warning: TSC frequency mismatch between VM (2500000 kHz) and host (2596099 kHz), and TSC scaling unavailable
qemu-system-x86_64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument


Thanks,
Naveen

Re: [PATCH v2 0/9] target/i386: SEV: Add support for enabling VMSA SEV features

[Naveen N Rao]

On Thu, Sep 25, 2025 at 03:47:29PM +0530, Naveen N Rao (AMD) wrote:
> This series adds support for enabling VMSA SEV features for SEV-ES and
> SEV-SNP guests. Since that is already supported for IGVM files, some of
> that code is moved to generic path and reused.
> 
> Debug-swap is already supported in KVM today, while patches for enabling
> Secure TSC have been accepted for the upcoming kernel release.

Any other comments on this series?

So far, the only minor change I have on top of this series is the change 
suggested by Markus:

diff --git a/qapi/qom.json b/qapi/qom.json
index 5b830a9ba000..a2b9ccdfe43e 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1010,7 +1010,8 @@
 #     designated guest firmware page for measured boot with -kernel
 #     (default: false) (since 6.2)
 #
-# @debug-swap: enable virtualization of debug registers
+# @debug-swap: enable virtualization of debug registers. This is only
+#     supported on SEV-ES/SEV-SNP guests
 #     (default: false) (since 10.2)
 #
 # Since: 9.1

Otherwise, this series still applies cleanly to current master.

> 
> Roy,
> I haven't been able to test IGVM, so would be great if that is tested to 
> confirm there are no unintended changes there.

I took a stab at this with the buildigvm tool from Roy. I am able to 
boot a Linux guest with an IGVM file generated from that using qemu 
built with this series applied. In addition, with the below change to 
buildigvm, I am able to see Secure TSC being enabled in the guest:

diff --git a/src/vmsa.rs b/src/vmsa.rs
index 3d67a953055e..ac150264c244 100644
--- a/src/vmsa.rs
+++ b/src/vmsa.rs
@@ -70,6 +70,7 @@ fn construct_vmsa(reset_addr: u32, platform: Platform) -> Result<Box<SevVmsa>, B

     if let Platform::SevSnp = platform {
         vmsa.sev_features.set_snp(true);
+        vmsa.sev_features.set_secure_tsc(true);
     }

     Ok(vmsa_box)

I couldn't get it to work with > 1 vCPUs though (I'm possibly missing 
OVMF changes or such).


- Naveen

Re: [PATCH v2 8/9] target/i386: SEV: Add support for setting TSC frequency for Secure TSC

[Tom Lendacky]

On 10/8/25 04:52, Naveen N Rao wrote:
> On Tue, Oct 07, 2025 at 08:31:47AM -0500, Tom Lendacky wrote:
>> On 9/25/25 05:17, Naveen N Rao (AMD) wrote:
> 
> ...
> 
>>> +
>>> +static void
>>> +sev_snp_guest_set_tsc_frequency(Object *obj, Visitor *v, const char *name,
>>> +                                void *opaque, Error **errp)
>>> +{
>>> +    uint32_t value;
>>> +
>>> +    if (!visit_type_uint32(v, name, &value, errp)) {
>>> +        return;
>>> +    }
>>> +
>>> +    SEV_SNP_GUEST(obj)->tsc_khz = value / 1000;
>>
>> This will cause a value that isn't evenly divisible by 1000 to be
>> rounded down, e.g.: tsc-frequency=2500000999. Should this name instead
>> just be tsc-khz or secure-tsc-khz (to show it is truly associated with
>> Secure TSC)?
> 
> I modeled this after the existing tsc-frequency parameter on the cpu 
> object to keep it simple (parameter is the same, just where it is 
> specified differs). This also aligns with TDX which re-uses the 
> tsc-frequency parameter on the cpu object.

So why aren't we using the one on the cpu object instead of creating a
duplicate parameter? There should be some way to get that value, no?

Thanks,
Tom

> 
>>
>> Also, I think there is already a "tsc-freq" parameter for the -cpu
>> parameter (?), should there be some kind of error message if both of
>> these are set? Or a warning saying it is being ignored? Or ...?
> 
> This is validated when the TSC frequency is being set on the vcpu, so I didn't
> add an explicit check.
> 
> As an example, with:
>   -cpu EPYC-v4,tsc-frequency=2500000000 \
>   -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,secure-tsc=on
> 
> qemu-system-x86_64: warning: TSC frequency mismatch between VM (2500000 kHz) and host (2596099 kHz), and TSC scaling unavailable
> qemu-system-x86_64: kvm_init_vcpu: kvm_arch_init_vcpu failed (0): Invalid argument
> 
> 
> Thanks,
> Naveen
> 

Re: [PATCH v2 8/9] target/i386: SEV: Add support for setting TSC frequency for Secure TSC

[Naveen N Rao]

On Fri, Oct 24, 2025 at 10:00:08AM -0500, Tom Lendacky wrote:
> On 10/8/25 04:52, Naveen N Rao wrote:
> > On Tue, Oct 07, 2025 at 08:31:47AM -0500, Tom Lendacky wrote:
> >> On 9/25/25 05:17, Naveen N Rao (AMD) wrote:
> > 
> > ...
> > 
> >>> +
> >>> +static void
> >>> +sev_snp_guest_set_tsc_frequency(Object *obj, Visitor *v, const char *name,
> >>> +                                void *opaque, Error **errp)
> >>> +{
> >>> +    uint32_t value;
> >>> +
> >>> +    if (!visit_type_uint32(v, name, &value, errp)) {
> >>> +        return;
> >>> +    }
> >>> +
> >>> +    SEV_SNP_GUEST(obj)->tsc_khz = value / 1000;
> >>
> >> This will cause a value that isn't evenly divisible by 1000 to be
> >> rounded down, e.g.: tsc-frequency=2500000999. Should this name instead
> >> just be tsc-khz or secure-tsc-khz (to show it is truly associated with
> >> Secure TSC)?
> > 
> > I modeled this after the existing tsc-frequency parameter on the cpu 
> > object to keep it simple (parameter is the same, just where it is 
> > specified differs). This also aligns with TDX which re-uses the 
> > tsc-frequency parameter on the cpu object.
> 
> So why aren't we using the one on the cpu object instead of creating a
> duplicate parameter? There should be some way to get that value, no?

I had spent some time on this, but I couldn't figure out a simple way to 
make that work.

TDX uses a vcpu pre-create hook (similar to KVM) to get access to and 
set the TSC value from the cpu object. For SEV-SNP, we need the TSC 
frequency during SNP_LAUNCH_START which is quite early and we don't have 
access to the cpu object there.

Admittedly, my qemu understanding is limited. If there is a way to 
re-use the cpu tsc-frequency field, then that would be ideal.

Any ideas/suggestions?


Thanks,
Naveen