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

[PATCH v3 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 through the Qemu command line. This is already supported 
for IGVM files, so some of that code has been generalized and reused.

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

I have tested this with a simple IGVM file generated by the buildigvm 
tool, and it did not show any issues with IGVM compatibility.


- Naveen

---
v2: http://lore.kernel.org/r/cover.1758794556.git.naveen@kernel.org

Changes since v2:
- Clarify that debug-swap property only works for SEV-ES/SEV-SNP guests, 
  and update commit log to clarify the same (Markus)
- Update commit log to clarify need for a new tsc-frequency property and 
  its interaction with the one on the cpu object (Tom)


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 | 171 +++++++++++++++++++++++++++++++++++++---------
 qapi/qom.json     |  17 ++++-
 3 files changed, 157 insertions(+), 35 deletions(-)


base-commit: 36076d24f04ea9dc3357c0fbe7bb14917375819c
-- 
2.51.0

[PATCH v3 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 v3 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 | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 2fb1268ed788..7cfd15238703 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -1901,6 +1901,16 @@ 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 v3 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 7cfd15238703..89cde2c6464a 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))) {
@@ -1918,6 +1915,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
@@ -2537,9 +2538,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 v3 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 89cde2c6464a..35df7be4f67c 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 v3 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 35df7be4f67c..0508b8998997 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 v3 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() has a check that rejects attempts to enable any SEV
feature for a plain SEV guest.

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     |  7 ++++++-
 3 files changed, 27 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 0508b8998997..2cea2661cc03 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) {
@@ -2745,6 +2750,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)
 {
@@ -2762,6 +2777,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..e89d11ce45ad 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1010,13 +1010,18 @@
 #     designated guest firmware page for measured boot with -kernel
 #     (default: false) (since 6.2)
 #
+# @debug-swap: enable virtualization of debug registers,
+#     only supported on SEV-ES and SEV-SNP guests
+#     (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 v3 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 2cea2661cc03..af8222b8ceb3 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -3122,6 +3122,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)
 {
@@ -3157,6 +3167,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 e89d11ce45ad..c7dd2dd1b095 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1101,6 +1101,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',
@@ -1112,7 +1115,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 v3 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.

Attempts to set TSC frequency on both the SEV_SNP object and the cpu
object result in an error from KVM (on the vCPU ioctl), so do not add
separate checks for the same.

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 af8222b8ceb3..56d7cc9e6901 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) {
@@ -3132,6 +3153,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)
 {
@@ -3170,6 +3213,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 c7dd2dd1b095..5daaf065b6b7 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1104,6 +1104,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',
@@ -1116,7 +1119,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 v3 8/9] target/i386: SEV: Add support for setting TSC frequency for Secure TSC

[Markus Armbruster]

Pardon my ignorance...

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

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

Which property exactly?

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

Sounds awkward.

Do the two properties set the same thing at different times?

> The user-provided TSC frequency is set through KVM_SET_TSC_KHZ before
> issuing KVM_SEV_SNP_LAUNCH_START.
>
> Attempts to set TSC frequency on both the SEV_SNP object and the cpu
> object result in an error from KVM (on the vCPU ioctl), so do not add
> separate checks for the same.
>
> 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>

[...]

> diff --git a/qapi/qom.json b/qapi/qom.json
> index c7dd2dd1b095..5daaf065b6b7 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -1104,6 +1104,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)

Is this likely to remain the only property that's only valied when
@secure-tsc is true?

> +#
>  # Since: 9.1
>  ##
>  { 'struct': 'SevSnpGuestProperties',
> @@ -1116,7 +1119,8 @@
>              '*author-key-enabled': 'bool',
>              '*host-data': 'str',
>              '*vcek-disabled': 'bool',
> -            '*secure-tsc': 'bool' } }
> +            '*secure-tsc': 'bool',
> +            '*tsc-frequency': 'uint32' } }
>  
>  ##
>  # @TdxGuestProperties:

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

[Naveen N Rao]

On Thu, Nov 06, 2025 at 01:09:37PM +0100, Markus Armbruster wrote:
> Pardon my ignorance...
> 
> "Naveen N Rao (AMD)" <naveen@kernel.org> writes:
> 
> > 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.
> 
> Which property exactly?

Same name: tsc-frequency specified with '-cpu'

> 
> >                 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.
> 
> Sounds awkward.
> 
> Do the two properties set the same thing at different times?

Yes. For regular guests, TSC frequency is set using a vCPU ioctl.  
However, TDX and SEV-SNP (with Secure TSC) require the TSC frequency to 
be set as a VM property (there is a VM ioctl for this purpose).

This was Tom's question too (see v2): is there any way to re-use 
'tsc-frequency' specified with '-cpu' for Secure TSC.

> 
> > The user-provided TSC frequency is set through KVM_SET_TSC_KHZ before
> > issuing KVM_SEV_SNP_LAUNCH_START.
> >
> > Attempts to set TSC frequency on both the SEV_SNP object and the cpu
> > object result in an error from KVM (on the vCPU ioctl), so do not add
> > separate checks for the same.
> >
> > 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>
> 
> [...]
> 
> > diff --git a/qapi/qom.json b/qapi/qom.json
> > index c7dd2dd1b095..5daaf065b6b7 100644
> > --- a/qapi/qom.json
> > +++ b/qapi/qom.json
> > @@ -1104,6 +1104,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)
> 
> Is this likely to remain the only property that's only valied when
> @secure-tsc is true?

At this stage, yes. I am not aware of anything else that is specific to 
Secure TSC.


- Naveen

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

[Markus Armbruster]

Naveen N Rao <naveen@kernel.org> writes:

> On Thu, Nov 06, 2025 at 01:09:37PM +0100, Markus Armbruster wrote:
>> Pardon my ignorance...
>> 
>> "Naveen N Rao (AMD)" <naveen@kernel.org> writes:
>> 
>> > 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.
>> 
>> Which property exactly?
>
> Same name: tsc-frequency specified with '-cpu'

Thanks.  It's x86_64-cpu property tsc-frequency.

>> 
>> >                 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.
>> 
>> Sounds awkward.
>> 
>> Do the two properties set the same thing at different times?
>
> Yes. For regular guests, TSC frequency is set using a vCPU ioctl.  
> However, TDX and SEV-SNP (with Secure TSC) require the TSC frequency to 
> be set as a VM property (there is a VM ioctl for this purpose).
>
> This was Tom's question too (see v2): is there any way to re-use 
> 'tsc-frequency' specified with '-cpu' for Secure TSC.

Hmm, let's see whether I can guess how this stuff works.  Please correct
my misunderstandings.

When machine property confidential-guest-support is null, it's a regular
guest.

If it points to a sev-guest object, it's SEV.

If it points to a sev-snp-guest object, it's SEV-SNP.

If it points to a tdx-guest object, it's TDX.

Normally, the TSC frequency is specified with x86_64-cpu property
tsc-frequency.

Can different CPUs have different frequencies?

In certain cases (SEV-SNP or TDX guest with Secure TSC), tsc-frequency
needs to be configured before any CPUs are created.  You're implementing
this for SEV-SNP, and you chose to create a sev-snp property
tsc-frequency for this.

What happens when I enable Secure TSC with sev-snp property
"secure-tsc": true, but don't set property tsc-frequency?

What happens when I do set it, and then also set the CPU property?  To
the same frequency?  To a different frequency?

>> > The user-provided TSC frequency is set through KVM_SET_TSC_KHZ before
>> > issuing KVM_SEV_SNP_LAUNCH_START.
>> >
>> > Attempts to set TSC frequency on both the SEV_SNP object and the cpu
>> > object result in an error from KVM (on the vCPU ioctl), so do not add
>> > separate checks for the same.
>> >
>> > 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>
>> 
>> [...]
>> 
>> > diff --git a/qapi/qom.json b/qapi/qom.json
>> > index c7dd2dd1b095..5daaf065b6b7 100644
>> > --- a/qapi/qom.json
>> > +++ b/qapi/qom.json
>> > @@ -1104,6 +1104,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)
>> 
>> Is this likely to remain the only property that's only valied when
>> @secure-tsc is true?
>
> At this stage, yes. I am not aware of anything else that is specific to 
> Secure TSC.

Alright, this makes "only valid if" reasonable.

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

[Naveen N Rao]

On Fri, Nov 07, 2025 at 10:49:30AM +0100, Markus Armbruster wrote:
> Naveen N Rao <naveen@kernel.org> writes:
> 
> > On Thu, Nov 06, 2025 at 01:09:37PM +0100, Markus Armbruster wrote:
> >> Pardon my ignorance...
> >> 
> >> "Naveen N Rao (AMD)" <naveen@kernel.org> writes:
> >> 
> >> > 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.
> >> 
> >> Which property exactly?
> >
> > Same name: tsc-frequency specified with '-cpu'
> 
> Thanks.  It's x86_64-cpu property tsc-frequency.

Ok.

> 
> >> 
> >> >                 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.
> >> 
> >> Sounds awkward.
> >> 
> >> Do the two properties set the same thing at different times?
> >
> > Yes. For regular guests, TSC frequency is set using a vCPU ioctl.  
> > However, TDX and SEV-SNP (with Secure TSC) require the TSC frequency to 
> > be set as a VM property (there is a VM ioctl for this purpose).
> >
> > This was Tom's question too (see v2): is there any way to re-use 
> > 'tsc-frequency' specified with '-cpu' for Secure TSC.
> 
> Hmm, let's see whether I can guess how this stuff works.  Please correct
> my misunderstandings.
> 
> When machine property confidential-guest-support is null, it's a regular
> guest.
> 
> If it points to a sev-guest object, it's SEV.

Yes, or a SEV-ES guest.

> 
> If it points to a sev-snp-guest object, it's SEV-SNP.
> 
> If it points to a tdx-guest object, it's TDX.
> 
> Normally, the TSC frequency is specified with x86_64-cpu property
> tsc-frequency.
> 
> Can different CPUs have different frequencies?

I believe Daniel has answered this in his reply.

> 
> In certain cases (SEV-SNP or TDX guest with Secure TSC), tsc-frequency

nit: TDX guest, or SEV-SNP with Secure TSC.

> needs to be configured before any CPUs are created.  You're implementing
> this for SEV-SNP, and you chose to create a sev-snp property
> tsc-frequency for this.
> 
> What happens when I enable Secure TSC with sev-snp property
> "secure-tsc": true, but don't set property tsc-frequency?

KVM uses the host default if tsc-frequency is not explicitly specified.

> 
> What happens when I do set it, and then also set the CPU property?  To
> the same frequency?  To a different frequency?

From the commit log:
  Attempts to set TSC frequency on both the SEV_SNP object and the cpu
  object result in an error from KVM (on the vCPU ioctl), so do not add
  separate checks for the same.

If the same frequency is specified in both places, then no error is 
thrown.


Thanks,
Naveen

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

[Daniel P. Berrangé]

On Fri, Nov 07, 2025 at 02:21:24PM +0530, Naveen N Rao wrote:
> On Thu, Nov 06, 2025 at 01:09:37PM +0100, Markus Armbruster wrote:
> > Pardon my ignorance...
> > 
> > "Naveen N Rao (AMD)" <naveen@kernel.org> writes:
> > 
> > > 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.
> > 
> > Which property exactly?
> 
> Same name: tsc-frequency specified with '-cpu'
> 
> > 
> > >                 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.
> > 
> > Sounds awkward.
> > 
> > Do the two properties set the same thing at different times?
> 
> Yes. For regular guests, TSC frequency is set using a vCPU ioctl.  
> However, TDX and SEV-SNP (with Secure TSC) require the TSC frequency to 
> be set as a VM property (there is a VM ioctl for this purpose).

The '-cpu' arg is global to the VM, so even though the ioctl is per-VCPU,
a single '-cpu ...,tsc-frequency=NNN' argument applies universally to all
the vCPUs in regular guests. 

> This was Tom's question too (see v2): is there any way to re-use 
> 'tsc-frequency' specified with '-cpu' for Secure TSC.

I see no reason why we can't simply use the existing '-cpu tsc-frequency'
value. Fetch the CPU 0 object and query its "tsc-frequency" property,
and just assume all non-0 CPUs have the same tsc-frequency, since we
don't provide a way to set it differently per-CPU IIUC.


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

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

[Naveen N Rao]

On Fri, Nov 07, 2025 at 09:59:46AM +0000, Daniel P. Berrangé wrote:
> On Fri, Nov 07, 2025 at 02:21:24PM +0530, Naveen N Rao wrote:
> > On Thu, Nov 06, 2025 at 01:09:37PM +0100, Markus Armbruster wrote:
> > > Pardon my ignorance...
> > > 
> > > "Naveen N Rao (AMD)" <naveen@kernel.org> writes:
> > > 
> > > > 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.
> > > 
> > > Which property exactly?
> > 
> > Same name: tsc-frequency specified with '-cpu'
> > 
> > > 
> > > >                 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.
> > > 
> > > Sounds awkward.
> > > 
> > > Do the two properties set the same thing at different times?
> > 
> > Yes. For regular guests, TSC frequency is set using a vCPU ioctl.  
> > However, TDX and SEV-SNP (with Secure TSC) require the TSC frequency to 
> > be set as a VM property (there is a VM ioctl for this purpose).
> 
> The '-cpu' arg is global to the VM, so even though the ioctl is per-VCPU,
> a single '-cpu ...,tsc-frequency=NNN' argument applies universally to all
> the vCPUs in regular guests. 
> 
> > This was Tom's question too (see v2): is there any way to re-use 
> > 'tsc-frequency' specified with '-cpu' for Secure TSC.
> 
> I see no reason why we can't simply use the existing '-cpu tsc-frequency'
> value. Fetch the CPU 0 object and query its "tsc-frequency" property,
> and just assume all non-0 CPUs have the same tsc-frequency, since we
> don't provide a way to set it differently per-CPU IIUC.

I might be missing something obvious, but did you mean using 
'qemu_get_cpu(0)' or 'first_cpu' in sev_snp_launch_start()?  That 
doesn't seem to work (both are NULL).


Thanks,
Naveen

[PATCH v3 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 56d7cc9e6901..50d9e5714408 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