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

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

The primary change w.r.t v3 (apart from rebasing and targeting v11.1) is 
a change to patch 6/9 to restrict debug-swap to SEV-SNP guests due to a 
compatibility issue with SEV-ES guests and pflash, discussed here:
http://lore.kernel.org/r/fcqjl5a7m27f2mfpblnhgmozbipdjmvpdyk3m5hhzwcenp4cpg@m2ooa7ykrcvs

I plan to post a workaround for that separately. For that reason, I have 
retained patch 5/9 so that enabling debug-swap for SEV-ES guests will be 
a simpler subsequent change.

The only other concern has been around patch 8/9 adding a new 
tsc-frequency property on sev-snp-guest object (and not re-using the one 
on the cpu object). If that's required/possible, I would appreciate some 
help since I've been unable to get that working.

Tom,
I have dropped your reviewed-by tag on patch 6/9 due to this change.  
Kindly take a look once.

v3: http://lore.kernel.org/r/cover.1761648149.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 | 177 +++++++++++++++++++++++++++++++++++++---------
 qapi/qom.json     |  17 ++++-
 3 files changed, 163 insertions(+), 35 deletions(-)


base-commit: e89049b3ba5f1f0468bc0d294173345597514a1b
-- 
2.54.0

[PATCH v4 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 4358df40e48b..b84ca3ce0b67 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -46,7 +46,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 b44b5a1c2b94..22c350fe14b7 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -328,6 +328,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)
@@ -1903,15 +1912,15 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **errp)
                     ->process(x86machine->igvm, machine, true, 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;
     }
@@ -3192,6 +3201,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.54.0

[PATCH v4 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 22c350fe14b7..641a295c42b7 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -1908,6 +1908,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, true, errp) == -1) {
                 return -1;
-- 
2.54.0

[PATCH v4 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 641a295c42b7..9857921e5a55 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -604,9 +604,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))) {
@@ -1924,6 +1921,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
@@ -2584,9 +2585,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.54.0

[PATCH v4 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 9857921e5a55..196a70b35c51 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -527,6 +527,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.54.0

[PATCH v4 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 196a70b35c51..4553fe4d6e4a 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -1701,8 +1701,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.54.0

[PATCH v4 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

Restrict debug-swap to SEV-SNP guests at this time due to a
compatibility issue with SEV-ES pflash devices.

Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
---
 target/i386/sev.h |  1 +
 target/i386/sev.c | 26 ++++++++++++++++++++++++++
 qapi/qom.json     |  7 ++++++-
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index b84ca3ce0b67..d19a39669747 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -47,6 +47,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 4553fe4d6e4a..4532b1b6a484 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -328,6 +328,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) {
@@ -527,6 +532,12 @@ static int check_sev_features(SevCommonState *sev_common, uint64_t sev_features,
             __func__);
         return -1;
     }
+    if (sev_features && sev_es_enabled() && !sev_snp_enabled()) {
+        error_setg(errp,
+                   "%s: SEV features are not supported with SEV-ES at this time",
+                   __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",
@@ -2800,6 +2811,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)
 {
@@ -2825,6 +2846,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 dd45ac1087c3..e2bb716b603e 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1017,6 +1017,10 @@
 #     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 11.1)
+#
 # Features:
 #
 # @confidential-guest-reset: If present, the hypervisor supports
@@ -1028,7 +1032,8 @@
   'data': { '*sev-device': 'str',
             '*cbitpos': 'uint32',
             'reduced-phys-bits': 'uint32',
-            '*kernel-hashes': 'bool' },
+            '*kernel-hashes': 'bool',
+            '*debug-swap': 'bool' },
   'features': ['confidential-guest-reset']}
 
 ##
-- 
2.54.0

[PATCH v4 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 d19a39669747..7725f92e1959 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -48,6 +48,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 4532b1b6a484..518f0eb91aa1 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -3193,6 +3193,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)
 {
@@ -3228,6 +3238,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 e2bb716b603e..9c2966b68e8f 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1113,6 +1113,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 11.1)
+#
 # Since: 9.1
 ##
 { 'struct': 'SevSnpGuestProperties',
@@ -1124,7 +1127,8 @@
             '*id-auth': 'str',
             '*author-key-enabled': 'bool',
             '*host-data': 'str',
-            '*vcek-disabled': 'bool' } }
+            '*vcek-disabled': 'bool',
+            '*secure-tsc': 'bool' } }
 
 ##
 # @TdxGuestProperties:
-- 
2.54.0

[PATCH v4 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 518f0eb91aa1..0009b03bdf7d 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -187,6 +187,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;
@@ -551,6 +552,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;
 }
 
@@ -1099,6 +1107,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) {
@@ -3203,6 +3224,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)
 {
@@ -3241,6 +3284,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 9c2966b68e8f..ef7df6b05a0b 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1116,6 +1116,9 @@
 # @secure-tsc: enable Secure TSC
 #     (default: false) (since 11.1)
 #
+# @tsc-frequency: set secure TSC frequency.  Only valid if Secure TSC
+#     is enabled (default: zero) (since 11.1)
+#
 # Since: 9.1
 ##
 { 'struct': 'SevSnpGuestProperties',
@@ -1128,7 +1131,8 @@
             '*author-key-enabled': 'bool',
             '*host-data': 'str',
             '*vcek-disabled': 'bool',
-            '*secure-tsc': 'bool' } }
+            '*secure-tsc': 'bool',
+            '*tsc-frequency': 'uint32' } }
 
 ##
 # @TdxGuestProperties:
-- 
2.54.0

[PATCH v4 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 | 67 +++++++++++++++++++++++++----------------------
 1 file changed, 36 insertions(+), 31 deletions(-)

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 0009b03bdf7d..f04ae4e91f3e 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -511,40 +511,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() && !sev_snp_enabled()) {
-        error_setg(errp,
-                   "%s: SEV features are not supported with SEV-ES at this time",
-                   __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, "
@@ -552,13 +534,36 @@ 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 && sev_es_enabled()) {
+            error_setg(errp,
+                       "%s: SEV features are not supported with SEV-ES at this time",
+                       __func__);
+            return -1;
+        }
+        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.54.0

Re: [PATCH v4 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() has a check that rejects attempts to enable any SEV
> feature for a plain SEV guest.

Why does it make sense to add the property to objects where it's not
supported?

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

Thanks for explaining this.

> Sample command-line:
>   -machine q35,confidential-guest-support=sev0 \
>   -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,debug-swap=on
>
> Restrict debug-swap to SEV-SNP guests at this time due to a
> compatibility issue with SEV-ES pflash devices.

What are these issues?

> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
> ---
>  target/i386/sev.h |  1 +
>  target/i386/sev.c | 26 ++++++++++++++++++++++++++
>  qapi/qom.json     |  7 ++++++-
>  3 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/target/i386/sev.h b/target/i386/sev.h
> index b84ca3ce0b67..d19a39669747 100644
> --- a/target/i386/sev.h
> +++ b/target/i386/sev.h
> @@ -47,6 +47,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 4553fe4d6e4a..4532b1b6a484 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -328,6 +328,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) {
> @@ -527,6 +532,12 @@ static int check_sev_features(SevCommonState *sev_common, uint64_t sev_features,
>              __func__);
>          return -1;
>      }
> +    if (sev_features && sev_es_enabled() && !sev_snp_enabled()) {
> +        error_setg(errp,
> +                   "%s: SEV features are not supported with SEV-ES at this time",
> +                   __func__);

Once again, I'm confused about SEV.  Remind me: what kinds of SEV guests
exist?  The commit message suggests "plain", "ES", "SNP".  What do
sev_es_enabled() and sev_snp_enabled() return for each of these?

What are the "SEV features"?  Can we expect users to know?  I suspect
they need to know to fix the problem!

Are they exactly debug-swap?

__func__ in an error message meant for users is an anti-pattern.  Not an
objection to this patch; it matches existing errors around here.

> +        return -1;
> +    }
>      if (sev_features && !sev_es_enabled()) {
>          error_setg(errp,
>                     "%s: SEV features require either SEV-ES or SEV-SNP to be enabled",
> @@ -2800,6 +2811,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)
>  {
> @@ -2825,6 +2846,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 dd45ac1087c3..e2bb716b603e 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -1017,6 +1017,10 @@
>  #     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 11.1)
> +#
>  # Features:
>  #
>  # @confidential-guest-reset: If present, the hypervisor supports
> @@ -1028,7 +1032,8 @@
>    'data': { '*sev-device': 'str',
>              '*cbitpos': 'uint32',
>              'reduced-phys-bits': 'uint32',
> -            '*kernel-hashes': 'bool' },
> +            '*kernel-hashes': 'bool',
> +            '*debug-swap': 'bool' },
>    'features': ['confidential-guest-reset']}
>  
>  ##

This is SevCommonProperties, the common base type of SevGuestProperties
(configuration for sev-guest QOM objects) and SevSnpGuestProperties
(sev-snp-guest objects).

So, all SEV objects have property @debug-swap.  I figure the check you
add to check_sev_features() rejects it for certain SEV objects.  I don't
quite understand for which ones.

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

[Naveen N Rao]

On Mon, Jun 01, 2026 at 01:52:00PM +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() has a check that rejects attempts to enable any SEV
> > feature for a plain SEV guest.
> 
> Why does it make sense to add the property to objects where it's not
> supported?

Primarily to minimize changes. The restriction is only until the 
workaround is implemented for which I have posted a patch just now:
https://lore.kernel.org/qemu-devel/20260602071213.2084388-1-naveen@kernel.org/T/#u

I didn't want to move the property to sev-snp-object only to move it 
back again subsequently.

> 
> > Though this SEV feature is called "Debug virtualization" in the APM, KVM
> > calls this "debug swap" so use the same name for consistency.
> 
> Thanks for explaining this.
> 
> > Sample command-line:
> >   -machine q35,confidential-guest-support=sev0 \
> >   -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,debug-swap=on
> >
> > Restrict debug-swap to SEV-SNP guests at this time due to a
> > compatibility issue with SEV-ES pflash devices.
> 
> What are these issues?

Explained in the patch I posted today (please see above for the link).

> 
> > Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
> > ---
> >  target/i386/sev.h |  1 +
> >  target/i386/sev.c | 26 ++++++++++++++++++++++++++
> >  qapi/qom.json     |  7 ++++++-
> >  3 files changed, 33 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/i386/sev.h b/target/i386/sev.h
> > index b84ca3ce0b67..d19a39669747 100644
> > --- a/target/i386/sev.h
> > +++ b/target/i386/sev.h
> > @@ -47,6 +47,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 4553fe4d6e4a..4532b1b6a484 100644
> > --- a/target/i386/sev.c
> > +++ b/target/i386/sev.c
> > @@ -328,6 +328,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) {
> > @@ -527,6 +532,12 @@ static int check_sev_features(SevCommonState *sev_common, uint64_t sev_features,
> >              __func__);
> >          return -1;
> >      }
> > +    if (sev_features && sev_es_enabled() && !sev_snp_enabled()) {
> > +        error_setg(errp,
> > +                   "%s: SEV features are not supported with SEV-ES at this time",
> > +                   __func__);
> 
> Once again, I'm confused about SEV.  Remind me: what kinds of SEV guests
> exist?  The commit message suggests "plain", "ES", "SNP".  What do
> sev_es_enabled() and sev_snp_enabled() return for each of these?

Right, SEV (plain) is encrypted-memory guests, SEV-ES is SEV with 
Encrypted register State, SEV-SNP is SEV-ES with Secure Nested Paging.

sev_es_enabled() returns true for SEV-ES and SEV-SNP guests.

> 
> What are the "SEV features"?  Can we expect users to know?  I suspect
> they need to know to fix the problem!
> 
> Are they exactly debug-swap?

Yes, users should know that since they need to enable those explicitly.  
Guests will need to support those features too.

Debug-swap is one, Secure-TSC is another (support for which is also 
added in this series), then there is Secure-AVIC and many others 
(documented in AMD APM).

> 
> __func__ in an error message meant for users is an anti-pattern.  Not an
> objection to this patch; it matches existing errors around here.
> 
> > +        return -1;
> > +    }
> >      if (sev_features && !sev_es_enabled()) {
> >          error_setg(errp,
> >                     "%s: SEV features require either SEV-ES or SEV-SNP to be enabled",
> > @@ -2800,6 +2811,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)
> >  {
> > @@ -2825,6 +2846,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 dd45ac1087c3..e2bb716b603e 100644
> > --- a/qapi/qom.json
> > +++ b/qapi/qom.json
> > @@ -1017,6 +1017,10 @@
> >  #     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 11.1)
> > +#
> >  # Features:
> >  #
> >  # @confidential-guest-reset: If present, the hypervisor supports
> > @@ -1028,7 +1032,8 @@
> >    'data': { '*sev-device': 'str',
> >              '*cbitpos': 'uint32',
> >              'reduced-phys-bits': 'uint32',
> > -            '*kernel-hashes': 'bool' },
> > +            '*kernel-hashes': 'bool',
> > +            '*debug-swap': 'bool' },
> >    'features': ['confidential-guest-reset']}
> >  
> >  ##
> 
> This is SevCommonProperties, the common base type of SevGuestProperties
> (configuration for sev-guest QOM objects) and SevSnpGuestProperties
> (sev-snp-guest objects).
> 
> So, all SEV objects have property @debug-swap.  I figure the check you
> add to check_sev_features() rejects it for certain SEV objects.  I don't
> quite understand for which ones.

The check above rejects it for SEV-ES guests. There is a generic check 
added in patch 4/9 which rejects any and add SEV features for plain SEV 
guests.


- Naveen