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

Naveen N Rao (AMD) posted 7 patches 2 weeks, 3 days 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
[RFC PATCH 3/7] target/i386: SEV: Add support for enabling debug-swap SEV feature
Posted by Naveen N Rao (AMD) 2 weeks, 3 days 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.

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
Re: [RFC PATCH 3/7] target/i386: SEV: Add support for enabling debug-swap SEV feature
Posted by Tom Lendacky 2 weeks, 2 days ago
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 3/7] target/i386: SEV: Add support for enabling debug-swap SEV feature
Posted by Naveen N Rao 1 week, 6 days ago
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
Posted by Markus Armbruster 2 weeks, 2 days 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.

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 3/7] target/i386: SEV: Add support for enabling debug-swap SEV feature
Posted by Naveen N Rao 1 week, 6 days ago
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
Posted by Markus Armbruster 1 week, 5 days ago
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
Posted by Daniel P. Berrangé 1 week, 5 days ago
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 :|