On Tue, Oct 07, 2025 at 08:14:37AM +0200, Markus Armbruster wrote:
> "Naveen N Rao (AMD)" <[email protected]> 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 <[email protected]>
> > Signed-off-by: Naveen N Rao (AMD) <[email protected]>
> > ---
> >  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


Reply via email to