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

[RFC PATCH 0/7] 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 and Secure AVIC have been posted. 


- Naveen



Naveen N Rao (AMD) (7):
  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: Add support for enabling Secure AVIC SEV feature

 target/i386/sev.h |   5 +-
 target/i386/sev.c | 139 +++++++++++++++++++++++++++++++++++++++++-----
 qapi/qom.json     |  18 +++++-
 3 files changed, 144 insertions(+), 18 deletions(-)


base-commit: 6a9fa5ef3230a7d51e0d953a59ee9ef10af705b8
-- 
2.50.1

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

Since check_sev_features() relies on SVM_SEV_FEAT_SNP_ACTIVE being set
in VMSA SEV features depending on the guest type, set this flag by
default when creating SEV-SNP guests. When using IGVM files, this field
is anyway over-written so that validation in check_sev_features() is
still relevant.

Finally, add a check to ensure SEV features aren't also set through qemu
cli if using IGVM files.

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

diff --git a/target/i386/sev.c b/target/i386/sev.c
index 1057b8ab2c60..243e9493ba8d 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -586,9 +586,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))) {
@@ -1892,20 +1889,29 @@ 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) {
                 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;
         }
 
+        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
+         * 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;
     }
@@ -2518,9 +2524,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;
@@ -3127,6 +3130,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_COMMON(sev_snp_guest)->sev_features |= SVM_SEV_FEAT_SNP_ACTIVE;
 }
 
 /* guest info specific to sev-snp */
-- 
2.50.1

[RFC PATCH 2/7] 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>
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 243e9493ba8d..fa23b5c38e9b 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -509,6 +509,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.50.1

[RFC PATCH 3/7] 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.

Add helpers for setting and querying the VMSA SEV features so that they
can be re-used for subsequent VMSA SEV features, and convert the
existing SVM_SEV_FEAT_SNP_ACTIVE definition to use the BIT() macro for
consistency with the new feature flag.

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 |  3 ++-
 target/i386/sev.c | 29 +++++++++++++++++++++++++++++
 qapi/qom.json     |  6 +++++-
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index 9db1a802f6bb..8e09b2ce1976 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -44,7 +44,8 @@ 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)
+#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 fa23b5c38e9b..b3e4d0f2c1d5 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -319,6 +319,20 @@ 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 value)
+{
+    if (value) {
+        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)
@@ -2732,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)
 {
@@ -2749,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..71cd8ad588b5 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.50.1

[RFC PATCH 4/7] 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 b3e4d0f2c1d5..3063ad2d077a 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.50.1

[RFC PATCH 5/7] 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.

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     |  5 ++++-
 3 files changed, 18 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 3063ad2d077a..8f88df19a408 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 71cd8ad588b5..b05a475ef499 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1100,6 +1100,8 @@
 #     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 +1113,8 @@
             '*id-auth': 'str',
             '*author-key-enabled': 'bool',
             '*host-data': 'str',
-            '*vcek-disabled': 'bool' } }
+            '*vcek-disabled': 'bool',
+            '*secure-tsc': 'bool' } }
 
 ##
 # @TdxGuestProperties:
-- 
2.50.1

[RFC PATCH 6/7] 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 8f88df19a408..facf51c810d9 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 b05a475ef499..5b99148cb790 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1102,6 +1102,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',
@@ -1114,7 +1117,8 @@
             '*author-key-enabled': 'bool',
             '*host-data': 'str',
             '*vcek-disabled': 'bool',
-            '*secure-tsc': 'bool' } }
+            '*secure-tsc': 'bool',
+            '*tsc-frequency': 'uint32' } }
 
 ##
 # @TdxGuestProperties:
-- 
2.50.1

[RFC PATCH 7/7] target/i386: SEV: Add support for enabling Secure AVIC SEV feature

[Naveen N Rao (AMD)]

Add support for enabling Secure AVIC VMSA SEV feature in SEV-SNP guests
through a new "secure-avic" boolean property on SEV-SNP guest objects.

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

Reviewed-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     |  5 ++++-
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/target/i386/sev.h b/target/i386/sev.h
index 87e73034ad15..a374c144bccd 100644
--- a/target/i386/sev.h
+++ b/target/i386/sev.h
@@ -47,6 +47,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)
+#define SVM_SEV_FEAT_SECURE_AVIC    BIT(16)
 
 typedef struct SevKernelLoaderContext {
     char *setup_data;
diff --git a/target/i386/sev.c b/target/i386/sev.c
index facf51c810d9..f9170e21ca57 100644
--- a/target/i386/sev.c
+++ b/target/i386/sev.c
@@ -3147,6 +3147,16 @@ 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 bool sev_snp_guest_get_secure_avic(Object *obj, Error **errp)
+{
+    return is_sev_feature_set(SEV_COMMON(obj), SVM_SEV_FEAT_SECURE_AVIC);
+}
+
+static void sev_snp_guest_set_secure_avic(Object *obj, bool value, Error **errp)
+{
+    sev_set_feature(SEV_COMMON(obj), SVM_SEV_FEAT_SECURE_AVIC, value);
+}
+
 static void
 sev_snp_guest_get_tsc_frequency(Object *obj, Visitor *v, const char *name,
                                 void *opaque, Error **errp)
@@ -3210,6 +3220,9 @@ sev_snp_guest_class_init(ObjectClass *oc, const void *data)
     object_class_property_add(oc, "tsc-frequency", "uint32",
                               sev_snp_guest_get_tsc_frequency,
                               sev_snp_guest_set_tsc_frequency, NULL, NULL);
+    object_class_property_add_bool(oc, "secure-avic",
+                                  sev_snp_guest_get_secure_avic,
+                                  sev_snp_guest_set_secure_avic);
 }
 
 static void
diff --git a/qapi/qom.json b/qapi/qom.json
index 5b99148cb790..5dce560a2f54 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -1105,6 +1105,8 @@
 # @tsc-frequency: set secure TSC frequency. Only valid if Secure TSC
 #     is enabled (default: zero) (since 10.2)
 #
+# @secure-avic: enable Secure AVIC (default: false) (since 10.2)
+#
 # Since: 9.1
 ##
 { 'struct': 'SevSnpGuestProperties',
@@ -1118,7 +1120,8 @@
             '*host-data': 'str',
             '*vcek-disabled': 'bool',
             '*secure-tsc': 'bool',
-            '*tsc-frequency': 'uint32' } }
+            '*tsc-frequency': 'uint32',
+            '*secure-avic': 'bool' } }
 
 ##
 # @TdxGuestProperties:
-- 
2.50.1

Re: [RFC PATCH 3/7] target/i386: SEV: Add support for enabling debug-swap SEV feature

[Markus Armbruster]

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

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

Let's see whether I understand...

It's a property of sev-guest and sev-snp-guest objects.  These are the
"SEV guest objects".

I guess a sev-snp-guest object implies it's a SEV-SNP guest, and setting
@debug-swap on such an object just works.

With a sev-guest object, it's either a "plain SEV guest" or a "SEV-ES"
guest.

If it's the latter, setting @debug-swap just works.

If it's the former, and you set @debug-swap to true, then KVM
accelerator initialization will fail later on.  This might trigger
fallback to TCG.

Am I confused?

> Add helpers for setting and querying the VMSA SEV features so that they
> can be re-used for subsequent VMSA SEV features, and convert the
> existing SVM_SEV_FEAT_SNP_ACTIVE definition to use the BIT() macro for
> consistency with the new feature flag.
>
> 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>

[...]

> diff --git a/qapi/qom.json b/qapi/qom.json
> index 830cb2ffe781..71cd8ad588b5 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)

Please indent like this:

   # @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: [RFC PATCH 6/7] target/i386: SEV: Add support for setting TSC frequency for Secure TSC

[Markus Armbruster]

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

[...]

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

Two spaces between sentences for consistency, please.

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

Re: [RFC PATCH 1/7] target/i386: SEV: Consolidate SEV feature validation to common init path

[Tom Lendacky]

On 9/11/25 06:54, 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.
> 
> Since check_sev_features() relies on SVM_SEV_FEAT_SNP_ACTIVE being set
> in VMSA SEV features depending on the guest type, set this flag by
> default when creating SEV-SNP guests. When using IGVM files, this field
> is anyway over-written so that validation in check_sev_features() is
> still relevant.

There seem to be multiple things going on in this patch and I wonder if it
would be best to split it up into separate smaller patches.

You have setting of SVM_SEV_FEAT_SNP_ACTIVE in sev_features, you have a
new check for sev_features being set when using an IGVM file and you have
the consolidation.

Thanks,
Tom

> 
> Finally, add a check to ensure SEV features aren't also set through qemu
> cli if using IGVM files.
> 
> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>
> ---
>  target/i386/sev.c | 30 +++++++++++++++++-------------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 1057b8ab2c60..243e9493ba8d 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -586,9 +586,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))) {
> @@ -1892,20 +1889,29 @@ 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) {
>                  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;
>          }
>  
> +        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
> +         * 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;
>      }
> @@ -2518,9 +2524,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;
> @@ -3127,6 +3130,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_COMMON(sev_snp_guest)->sev_features |= SVM_SEV_FEAT_SNP_ACTIVE;
>  }
>  
>  /* guest info specific to sev-snp */

Re: [RFC PATCH 2/7] target/i386: SEV: Validate that SEV-ES is enabled when VMSA features are used

[Tom Lendacky]

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

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

> ---
>  target/i386/sev.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index 243e9493ba8d..fa23b5c38e9b 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -509,6 +509,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, "

Re: [RFC PATCH 3/7] target/i386: SEV: Add support for enabling debug-swap SEV feature

[Tom Lendacky]

On 9/11/25 06:54, 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.
> 
> Add helpers for setting and querying the VMSA SEV features so that they
> can be re-used for subsequent VMSA SEV features, and convert the
> existing SVM_SEV_FEAT_SNP_ACTIVE definition to use the BIT() macro for
> consistency with the new feature flag.
> 
> 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>

Should you convert the setting/checking of SVM_SEV_FEAT_SNP_ACTIVE in the
first patch (and wherever else it might be used), too?

If you do, then it would split this into two patches, one that adds the
helpers and converts existing accesses to sev_features and then the new
debug_swap parameter.

Thanks,
Tom

> ---
>  target/i386/sev.h |  3 ++-
>  target/i386/sev.c | 29 +++++++++++++++++++++++++++++
>  qapi/qom.json     |  6 +++++-
>  3 files changed, 36 insertions(+), 2 deletions(-)
> 
> diff --git a/target/i386/sev.h b/target/i386/sev.h
> index 9db1a802f6bb..8e09b2ce1976 100644
> --- a/target/i386/sev.h
> +++ b/target/i386/sev.h
> @@ -44,7 +44,8 @@ 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)
> +#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 fa23b5c38e9b..b3e4d0f2c1d5 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -319,6 +319,20 @@ 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 value)
> +{
> +    if (value) {
> +        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)
> @@ -2732,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)
>  {
> @@ -2749,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..71cd8ad588b5 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: [RFC PATCH 5/7] target/i386: SEV: Add support for enabling Secure TSC SEV feature

[Tom Lendacky]

On 9/11/25 06:54, 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.
> 
> Sample command-line:
>   -machine q35,confidential-guest-support=sev0 \
>   -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,secure-tsc=on
> 

Since the next patch talks about setting a TSC value, it would be a good
idea to document the default TSC value that is used when you specify just
this parameter.

Thanks,
Tom

> 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     |  5 ++++-
>  3 files changed, 18 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 3063ad2d077a..8f88df19a408 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 71cd8ad588b5..b05a475ef499 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -1100,6 +1100,8 @@
>  #     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 +1113,8 @@
>              '*id-auth': 'str',
>              '*author-key-enabled': 'bool',
>              '*host-data': 'str',
> -            '*vcek-disabled': 'bool' } }
> +            '*vcek-disabled': 'bool',
> +            '*secure-tsc': 'bool' } }
>  
>  ##
>  # @TdxGuestProperties:

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

[Tom Lendacky]

On 9/11/25 06:54, Naveen N Rao (AMD) wrote:
> Add support for enabling Secure AVIC VMSA SEV feature in SEV-SNP guests
> through a new "secure-avic" boolean property on SEV-SNP guest objects.
> 
> Sample command-line:
>   -machine q35,confidential-guest-support=sev0 \
>   -object sev-snp-guest,id=sev0,cbitpos=51,reduced-phys-bits=1,secure-avic=on

Since the hypervisor support for Secure AVIC is not accepted in KVM, yet,
this should not be included yet until we know what the full VMM
requirements might be.

Thanks,
Tom

> 
> Reviewed-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     |  5 ++++-
>  3 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/target/i386/sev.h b/target/i386/sev.h
> index 87e73034ad15..a374c144bccd 100644
> --- a/target/i386/sev.h
> +++ b/target/i386/sev.h
> @@ -47,6 +47,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)
> +#define SVM_SEV_FEAT_SECURE_AVIC    BIT(16)
>  
>  typedef struct SevKernelLoaderContext {
>      char *setup_data;
> diff --git a/target/i386/sev.c b/target/i386/sev.c
> index facf51c810d9..f9170e21ca57 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -3147,6 +3147,16 @@ 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 bool sev_snp_guest_get_secure_avic(Object *obj, Error **errp)
> +{
> +    return is_sev_feature_set(SEV_COMMON(obj), SVM_SEV_FEAT_SECURE_AVIC);
> +}
> +
> +static void sev_snp_guest_set_secure_avic(Object *obj, bool value, Error **errp)
> +{
> +    sev_set_feature(SEV_COMMON(obj), SVM_SEV_FEAT_SECURE_AVIC, value);
> +}
> +
>  static void
>  sev_snp_guest_get_tsc_frequency(Object *obj, Visitor *v, const char *name,
>                                  void *opaque, Error **errp)
> @@ -3210,6 +3220,9 @@ sev_snp_guest_class_init(ObjectClass *oc, const void *data)
>      object_class_property_add(oc, "tsc-frequency", "uint32",
>                                sev_snp_guest_get_tsc_frequency,
>                                sev_snp_guest_set_tsc_frequency, NULL, NULL);
> +    object_class_property_add_bool(oc, "secure-avic",
> +                                  sev_snp_guest_get_secure_avic,
> +                                  sev_snp_guest_set_secure_avic);
>  }
>  
>  static void
> diff --git a/qapi/qom.json b/qapi/qom.json
> index 5b99148cb790..5dce560a2f54 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -1105,6 +1105,8 @@
>  # @tsc-frequency: set secure TSC frequency. Only valid if Secure TSC
>  #     is enabled (default: zero) (since 10.2)
>  #
> +# @secure-avic: enable Secure AVIC (default: false) (since 10.2)
> +#
>  # Since: 9.1
>  ##
>  { 'struct': 'SevSnpGuestProperties',
> @@ -1118,7 +1120,8 @@
>              '*host-data': 'str',
>              '*vcek-disabled': 'bool',
>              '*secure-tsc': 'bool',
> -            '*tsc-frequency': 'uint32' } }
> +            '*tsc-frequency': 'uint32',
> +            '*secure-avic': 'bool' } }
>  
>  ##
>  # @TdxGuestProperties:

Re: [RFC PATCH 1/7] target/i386: SEV: Consolidate SEV feature validation to common init path

[Naveen N Rao]

Hi Tom,

On Fri, Sep 12, 2025 at 08:39:09AM -0500, Tom Lendacky wrote:
> On 9/11/25 06:54, 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.
> > 
> > Since check_sev_features() relies on SVM_SEV_FEAT_SNP_ACTIVE being set
> > in VMSA SEV features depending on the guest type, set this flag by
> > default when creating SEV-SNP guests. When using IGVM files, this field
> > is anyway over-written so that validation in check_sev_features() is
> > still relevant.
> 
> There seem to be multiple things going on in this patch and I wonder if it
> would be best to split it up into separate smaller patches.
> 
> You have setting of SVM_SEV_FEAT_SNP_ACTIVE in sev_features, you have a
> new check for sev_features being set when using an IGVM file and you have
> the consolidation.

Sure, I started with the premise of unifying the call to 
check_sev_features() which necessitated the other changes. I will move 
those as pre-req patches.

Thanks for the review,
- Naveen

Re: [RFC PATCH 3/7] target/i386: SEV: Add support for enabling debug-swap SEV feature

[Naveen N Rao]

Hi Markus,

On Fri, Sep 12, 2025 at 01:20:43PM +0200, Markus Armbruster wrote:
> "Naveen N Rao (AMD)" <naveen@kernel.org> writes:
> 
> > Add support for enabling debug-swap VMSA SEV feature in SEV-ES and
> > SEV-SNP guests through a new "debug-swap" boolean property on SEV guest
> > objects. Though the boolean property is available for plain SEV guests,
> > check_sev_features() will reject setting this for plain SEV guests.
> 
> Let's see whether I understand...
> 
> It's a property of sev-guest and sev-snp-guest objects.  These are the
> "SEV guest objects".
> 
> I guess a sev-snp-guest object implies it's a SEV-SNP guest, and setting
> @debug-swap on such an object just works.
> 
> With a sev-guest object, it's either a "plain SEV guest" or a "SEV-ES"
> guest.
> 
> If it's the latter, setting @debug-swap just works.
> 
> If it's the former, and you set @debug-swap to true, then KVM
> accelerator initialization will fail later on.  This might trigger
> fallback to TCG.
> 
> Am I confused?

You're spot on, except that in the last case above (plain old SEV 
guest), qemu throws an error:
	qemu-system-x86_64: check_sev_features: SEV features require either SEV-ES or SEV-SNP to be enabled

> 
> > Add helpers for setting and querying the VMSA SEV features so that they
> > can be re-used for subsequent VMSA SEV features, and convert the
> > existing SVM_SEV_FEAT_SNP_ACTIVE definition to use the BIT() macro for
> > consistency with the new feature flag.
> >
> > 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>
> 
> [...]
> 
> > diff --git a/qapi/qom.json b/qapi/qom.json
> > index 830cb2ffe781..71cd8ad588b5 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)
> 
> Please indent like this:
> 
>    # @debug-swap: enable virtualization of debug registers
>    #     (default: false) (since 10.2)

Sure.

Thanks for the review,
- Naveen

Re: [RFC PATCH 3/7] target/i386: SEV: Add support for enabling debug-swap SEV feature

[Naveen N Rao]

On Fri, Sep 12, 2025 at 08:50:28AM -0500, Tom Lendacky wrote:
> On 9/11/25 06:54, 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.
> > 
> > Add helpers for setting and querying the VMSA SEV features so that they
> > can be re-used for subsequent VMSA SEV features, and convert the
> > existing SVM_SEV_FEAT_SNP_ACTIVE definition to use the BIT() macro for
> > consistency with the new feature flag.
> > 
> > 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>
> 
> Should you convert the setting/checking of SVM_SEV_FEAT_SNP_ACTIVE in the
> first patch (and wherever else it might be used), too?
> 
> If you do, then it would split this into two patches, one that adds the
> helpers and converts existing accesses to sev_features and then the new
> debug_swap parameter.

Sure, I'll do that.

Thanks,
Naveen

Re: [RFC PATCH 3/7] target/i386: SEV: Add support for enabling debug-swap SEV feature

[Markus Armbruster]

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

> Hi Markus,
>
> On Fri, Sep 12, 2025 at 01:20:43PM +0200, Markus Armbruster wrote:
>> "Naveen N Rao (AMD)" <naveen@kernel.org> writes:
>> 
>> > Add support for enabling debug-swap VMSA SEV feature in SEV-ES and
>> > SEV-SNP guests through a new "debug-swap" boolean property on SEV guest
>> > objects. Though the boolean property is available for plain SEV guests,
>> > check_sev_features() will reject setting this for plain SEV guests.
>> 
>> Let's see whether I understand...
>> 
>> It's a property of sev-guest and sev-snp-guest objects.  These are the
>> "SEV guest objects".
>> 
>> I guess a sev-snp-guest object implies it's a SEV-SNP guest, and setting
>> @debug-swap on such an object just works.
>> 
>> With a sev-guest object, it's either a "plain SEV guest" or a "SEV-ES"
>> guest.
>> 
>> If it's the latter, setting @debug-swap just works.
>> 
>> If it's the former, and you set @debug-swap to true, then KVM
>> accelerator initialization will fail later on.  This might trigger
>> fallback to TCG.
>> 
>> Am I confused?
>
> You're spot on, except that in the last case above (plain old SEV 
> guest), qemu throws an error:
> 	qemu-system-x86_64: check_sev_features: SEV features require either SEV-ES or SEV-SNP to be enabled

Okay.

Can you (or anyone) explain to me why SEV-SNP gets its own object type,
but SEV-ES does not?

[...]

Re: [RFC PATCH 3/7] target/i386: SEV: Add support for enabling debug-swap SEV feature

[Daniel P. Berrangé]

On Tue, Sep 16, 2025 at 02:46:27PM +0200, Markus Armbruster wrote:
> Naveen N Rao <naveen@kernel.org> writes:
> 
> > Hi Markus,
> >
> > On Fri, Sep 12, 2025 at 01:20:43PM +0200, Markus Armbruster wrote:
> >> "Naveen N Rao (AMD)" <naveen@kernel.org> writes:
> >> 
> >> > Add support for enabling debug-swap VMSA SEV feature in SEV-ES and
> >> > SEV-SNP guests through a new "debug-swap" boolean property on SEV guest
> >> > objects. Though the boolean property is available for plain SEV guests,
> >> > check_sev_features() will reject setting this for plain SEV guests.
> >> 
> >> Let's see whether I understand...
> >> 
> >> It's a property of sev-guest and sev-snp-guest objects.  These are the
> >> "SEV guest objects".
> >> 
> >> I guess a sev-snp-guest object implies it's a SEV-SNP guest, and setting
> >> @debug-swap on such an object just works.
> >> 
> >> With a sev-guest object, it's either a "plain SEV guest" or a "SEV-ES"
> >> guest.
> >> 
> >> If it's the latter, setting @debug-swap just works.
> >> 
> >> If it's the former, and you set @debug-swap to true, then KVM
> >> accelerator initialization will fail later on.  This might trigger
> >> fallback to TCG.
> >> 
> >> Am I confused?
> >
> > You're spot on, except that in the last case above (plain old SEV 
> > guest), qemu throws an error:
> > 	qemu-system-x86_64: check_sev_features: SEV features require either SEV-ES or SEV-SNP to be enabled
> 
> Okay.
> 
> Can you (or anyone) explain to me why SEV-SNP gets its own object type,
> but SEV-ES does not?

SEV-ES is a minor incremental enhancement over SEV, with the user provided
configuration in QEMU largely common between the two.

SEV-SNP is a significant improvement that requires new/different user
config data to be provided to QEMU. It also changes the way attestation
is driven, moving out of host/QEMU, into the guest.

It made more sense to separate the configuration for SEV-SNP from that
used for SEV/SEV-ES. It also helps reinforce the message that SEV-SNP
is where the long term focus should be, with SEV/SEV-ES (ideally) only
used on old platforms that predate SNP, or running OS that lack the
more recent software support for SNP.

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