Re: [PATCH 1/8] target/i386: SEV: Generalize handling of SVM_SEV_FEAT_SNP_ACTIVE

[Tom Lendacky <thomas.lendacky@amd.com>]

On 9/18/25 05:26, Naveen N Rao (AMD) wrote:
> Align with IGVM files providing sev features with

s/sev/SEV/

if you have to re-submit...

> 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.
> 
> Signed-off-by: Naveen N Rao (AMD) <naveen@kernel.org>

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

> ---
>  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 9db1a802f6bb..102546b112d6 100644
> --- a/target/i386/sev.h
> +++ b/target/i386/sev.h
> @@ -44,7 +44,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 1057b8ab2c60..2fb1268ed788 100644
> --- a/target/i386/sev.c
> +++ b/target/i386/sev.c
> @@ -319,6 +319,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)
> @@ -1897,15 +1906,15 @@ static int sev_common_kvm_init(ConfidentialGuestSupport *cgs, Error **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;
>      }
> @@ -3127,6 +3136,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 */