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

Naveen N Rao (AMD) posted 9 patches 4 months, 2 weeks ago
Maintainers: Eric Blake <eblake@redhat.com>, Markus Armbruster <armbru@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eduardo Habkost <eduardo@habkost.net>, Zhao Liu <zhao1.liu@intel.com>, Marcelo Tosatti <mtosatti@redhat.com>
There is a newer version of this series
[PATCH v2 6/9] target/i386: SEV: Add support for enabling debug-swap SEV feature
Posted by Naveen N Rao (AMD) 4 months, 2 weeks ago
Add support for enabling debug-swap VMSA SEV feature in SEV-ES and
SEV-SNP guests through a new "debug-swap" boolean property on SEV guest
objects. Though the boolean property is available for plain SEV guests,
check_sev_features() will reject setting this for plain SEV guests.

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

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

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

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

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

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

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

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

Always appreciated in commit messages.

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

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

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

>  # Since: 9.1
>  ##
>  { 'struct': 'SevCommonProperties',
>    'data': { '*sev-device': 'str',
>              '*cbitpos': 'uint32',
>              'reduced-phys-bits': 'uint32',
> -            '*kernel-hashes': 'bool' } }
> +            '*kernel-hashes': 'bool',
> +            '*debug-swap': 'bool' } }
>  
>  ##
>  # @SevGuestProperties:
Re: [PATCH v2 6/9] target/i386: SEV: Add support for enabling debug-swap SEV feature
Posted by Naveen N Rao 4 months ago
On Tue, Oct 07, 2025 at 08:14:37AM +0200, Markus Armbruster wrote:
> "Naveen N Rao (AMD)" <naveen@kernel.org> writes:
> 
> > Add support for enabling debug-swap VMSA SEV feature in SEV-ES and
> > SEV-SNP guests through a new "debug-swap" boolean property on SEV guest
> > objects. Though the boolean property is available for plain SEV guests,
> > check_sev_features() will reject setting this for plain SEV guests.
> 
> Is this the sev_features && !sev_es_enabled() check there?

Yes, that's the one.

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

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

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

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

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

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

Sure, we can add that.


Thanks,
Naveen