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

[PATCH 0/8] 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.

Changes since RFC (http://lkml.kernel.org/r/cover.1757589490.git.naveen@kernel.org):
- Split the first patch up into the initial three patches (Tom)
- Fix up indents in qom.json (Markus)
- Drop Secure-AVIC flag enablement pending KVM enablement (Tom)
- Collect Tom's reviewed-by tag for patch 4


- Naveen

Naveen N Rao (AMD) (8):
  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: Add support for enabling debug-swap SEV feature
  target/i386: SEV: Enable use of KVM_SEV_INIT2 for SEV-ES guests
  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.h |   4 +-
 target/i386/sev.c | 126 ++++++++++++++++++++++++++++++++++++++++------
 qapi/qom.json     |  16 +++++-
 3 files changed, 128 insertions(+), 18 deletions(-)


base-commit: 6a9fa5ef3230a7d51e0d953a59ee9ef10af705b8
-- 
2.51.0

[PATCH 1/8] 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.

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 2/8] 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.

Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
---
 target/i386/sev.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 2fb1268ed788..c4011a6f2ef7 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -1901,6 +1901,11 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
          * as SEV_STATE_UNINIT.
          */
         if (x86machine->igvm) {
+            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 3/8] 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.

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 c4011a6f2ef7..7c4cd1146b9a 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))) {
@@ -1913,6 +1910,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
@@ -2532,9 +2533,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 4/8] 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 7c4cd1146b9a..f6e4333922ea 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 5/8] 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.

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

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 f6e4333922ea..4f1b0bf6ccc8 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) {
@@ -2741,6 +2746,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)
 {
@@ -2758,6 +2773,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 6/8] target/i386: SEV: Enable use of KVM_SEV_INIT2 for SEV-ES guests

[Naveen N Rao (AMD)]

Now that users can 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.

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

Reviewed-by: Nikunj A Dadhania <nikunj@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 4f1b0bf6ccc8..6b11359f06dd 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -1704,8 +1704,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 7/8] 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

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 6b11359f06dd..679bedb63c3a 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -3117,6 +3117,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)
 {
@@ -3152,6 +3162,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 8/8] 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.

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 | 45 +++++++++++++++++++++++++++++++++++++++++++++
 qapi/qom.json     |  6 +++++-
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 679bedb63c3a..ef54265f4e46 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,18 @@ sev_snp_launch_start(SevCommonState *sev_common)
             return 1;
     }
 
+    if (is_sev_feature_set(sev_common, SVM_SEV_FEAT_SECURE_TSC)) {
+        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) {
@@ -3127,6 +3147,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)
 {
@@ -3165,6 +3207,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

Re: [PATCH 1/8] target/i386: SEV: Generalize handling of SVM_SEV_FEAT_SNP_ACTIVE

[Tom Lendacky]

On 9/18/25 05:26, Naveen N Rao (AMD) wrote:
> Align with IGVM files providing sev features with

s/sev/SEV/

if you have to re-submit...

> 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.
> 
> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  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 */

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

[Tom Lendacky]

On 9/18/25 05:27, Naveen N Rao (AMD) wrote:
> 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.
> 
> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>

One minor comment below, otherwise:

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  target/i386/sev.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 2fb1268ed788..c4011a6f2ef7 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -1901,6 +1901,11 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
>           * as SEV_STATE_UNINIT.
>           */
>          if (x86machine->igvm) {

A comment here about SVM_SEV_FEAT_SNP_ACTIVE being set by default being
the reason it needs to be factored out, would be good to have.

> +            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) {

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

[Tom Lendacky]

On 9/18/25 05:27, Naveen N Rao (AMD) wrote:
> 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.
> 
> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  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 c4011a6f2ef7..7c4cd1146b9a 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))) {
> @@ -1913,6 +1910,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
> @@ -2532,9 +2533,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;

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

[Tom Lendacky]

On 9/18/25 05:27, Naveen N Rao (AMD) wrote:
> 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.
> 
> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>

Looks reasonable.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  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 c4011a6f2ef7..7c4cd1146b9a 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))) {
> @@ -1913,6 +1910,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
> @@ -2532,9 +2533,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;

Re: [PATCH 5/8] target/i386: SEV: Add support for enabling debug-swap SEV feature

[Tom Lendacky]

On 9/18/25 05:27, Naveen N Rao (AMD) wrote:
> 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.
> 
> Sample command-line:
>   -machine q35,confidential-guest-support=sev0 \
>   -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,debug-swap=on
> 
> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>

The actual feature name in the APM is DebugVirtualization, but we have
debug_swap in KVM...  so I guess it's ok to use debug-swap.

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  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 f6e4333922ea..4f1b0bf6ccc8 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) {
> @@ -2741,6 +2746,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)
>  {
> @@ -2758,6 +2773,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:

Re: [PATCH 6/8] target/i386: SEV: Enable use of KVM_SEV_INIT2 for SEV-ES guests

[Tom Lendacky]

On 9/18/25 05:27, Naveen N Rao (AMD) wrote:
> Now that users can 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.
> 
> Sample command-line:
>   -machine q35,confidential-guest-support=sev0 \
>   -object sev-guest,id=sev0,policy=0x5,cbitpos=51,reduced-phys-bits=1,debug-swap=on
> 

Should this patch go before patch #5 from a bisect point of view? Because
won't patch #5 fail because you still aren't using init2?

> Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  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 4f1b0bf6ccc8..6b11359f06dd 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -1704,8 +1704,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)

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

[Tom Lendacky]

On 9/18/25 05:27, Naveen N Rao (AMD) wrote:
> 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
> 
> 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>

Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

> ---
>  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 6b11359f06dd..679bedb63c3a 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -3117,6 +3117,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)
>  {
> @@ -3152,6 +3162,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:

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

[Tom Lendacky]

On 9/18/25 05:27, 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.

A sample command line like in the previous patches would be consistent.

> 
> 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>

It would be nice to have a follow-up patch that cleans up
check_sev_features() so that there aren't a bunch of checks with "if
(sev_snp_enabled() ...". Having all the SNP related checks under one "if"
block might be cleaner as more SNP only features get added. Just a thought.

> ---
>  target/i386/sev.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  qapi/qom.json     |  6 +++++-
>  2 files changed, 50 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 679bedb63c3a..ef54265f4e46 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,18 @@ sev_snp_launch_start(SevCommonState *sev_common)
>              return 1;
>      }
>  
> +    if (is_sev_feature_set(sev_common, SVM_SEV_FEAT_SECURE_TSC)) {
> +        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;
> +        }

It looks like KVM_CAP_VM_TSC_CONTROL is required for Secure TSC. Should
this cap check be part of check_sev_features() then, rather than waiting
until launch start?

And does KVM_SET_TSC_KHZ have to be called if "tsc-frequency" wasn't set?

Thanks,
Tom

> +    }
> +
>      rc = sev_ioctl(sev_common->sev_fd, KVM_SEV_SNP_LAUNCH_START,
>                     start, &fw_error);
>      if (rc < 0) {
> @@ -3127,6 +3147,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)
>  {
> @@ -3165,6 +3207,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 8/8] target/i386: SEV: Add support for setting TSC frequency for Secure TSC

[Nikunj A. Dadhania]

On 9/20/2025 3:36 AM, Tom Lendacky wrote:
> On 9/18/25 05:27, Naveen N Rao (AMD) wrote:

>> @@ -1085,6 +1093,18 @@ sev_snp_launch_start(SevCommonState *sev_common)
>>              return 1;
>>      }
>>  
>> +    if (is_sev_feature_set(sev_common, SVM_SEV_FEAT_SECURE_TSC)) {
>> +        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;
>> +        }
> 
> It looks like KVM_CAP_VM_TSC_CONTROL is required for Secure TSC. Should
> this cap check be part of check_sev_features() then, rather than waiting
> until launch start?

If the user has not provided tsc-frequency, KVM_CAP_VM_TSC_CONTROL is not required.

> 
> And does KVM_SET_TSC_KHZ have to be called if "tsc-frequency" wasn't set?
No, this is not required. This patch has changed a bit from my original version, we should have something like below: 

if (is_sev_feature_set(sev_common, SVM_SEV_FEAT_SECURE_TSC) && sev_snp_guest->stsc_khz) {
...
}

Regards
Nikunj

Re: [PATCH 6/8] target/i386: SEV: Enable use of KVM_SEV_INIT2 for SEV-ES guests

[Naveen N Rao]

On Fri, Sep 19, 2025 at 04:44:34PM -0500, Tom Lendacky wrote:
> On 9/18/25 05:27, Naveen N Rao (AMD) wrote:
> > Now that users can 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.
> > 
> > Sample command-line:
> >   -machine q35,confidential-guest-support=sev0 \
> >   -object sev-guest,id=sev0,policy=0x5,cbitpos=51,reduced-phys-bits=1,debug-swap=on
> > 
> 
> Should this patch go before patch #5 from a bisect point of view? Because
> won't patch #5 fail because you still aren't using init2?

I put this patch after the base debug-swap support since it is not 
possible to exercize this code otherwise. But, as you rightly point out, 
this just means that patch 5/8 is buggy and that is not good from a 
bisect standpoint. I will move this before patch 5/8.

> 
> > Reviewed-by: Nikunj A Dadhania <nikunj@amd.com>
> > Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
> 
> Reviewed-by: Tom Lendacky <thomas.lendacky@amd.com>

Thanks for the review,
- Naveen

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

[Naveen N Rao]

On Tue, Sep 23, 2025 at 09:48:07AM +0530, Nikunj A. Dadhania wrote:
> 
> 
> On 9/20/2025 3:36 AM, Tom Lendacky wrote:
> > On 9/18/25 05:27, Naveen N Rao (AMD) wrote:

<snip>

> > 
> > And does KVM_SET_TSC_KHZ have to be called if "tsc-frequency" wasn't set?
> No, this is not required. This patch has changed a bit from my original version, we should have something like below: 
> 
> if (is_sev_feature_set(sev_common, SVM_SEV_FEAT_SECURE_TSC) && sev_snp_guest->stsc_khz) {
> ...
> }

Right, I suppose I relied on KVM using the default TSC frequency if the 
VMM does KVM_SET_TSC_KHZ with a TSC frequency of zero, which is totally 
unnecessary. I will update this.


Thanks,
Naveen