Re: [PATCH 2/2] KVM: SEV: Add support for IBPB-on-Entry

[Kim Phillips]

On 1/27/26 12:38 AM, Nikunj A. Dadhania wrote:

Hi Nikunj,

> On 1/27/2026 4:12 AM, Kim Phillips wrote:
> > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > index ea515cf41168..8a6d25db0c00 100644
> > --- a/arch/x86/kvm/svm/sev.c
> > +++ b/arch/x86/kvm/svm/sev.c
> > @@ -3165,8 +3165,15 @@ void __init sev_hardware_setup(void)
> >   	    cpu_feature_enabled(X86_FEATURE_NO_NESTED_DATA_BP))
> >   		sev_supported_vmsa_features |= SVM_SEV_FEAT_DEBUG_SWAP;
> >   
> > -	if (sev_snp_enabled && tsc_khz && cpu_feature_enabled(X86_FEATURE_SNP_SECURE_TSC))
> > +	if (!sev_snp_enabled)
> > +		return;
> > +	/* the following feature bit checks are SNP specific */
> > +
>
> The early return seems to split up the SNP features unnecessarily.
>
> Keeping everything under `if (sev_snp_enabled)` is cleaner IMO -
> it's clear that these features belong together. Plus, when
> someone adds the next SNP feature, they won't have to think about
> whether it goes before or after the return. The comment about
> "SNP specific" features becomes redundant as well.
The SNP 'togetherness' semantics are maintained whether under an
'if (sev_snp_enabled)' body, or after an 'if (!sev_snp_enabled) return;'.

Only SNP-specific things are being done in the trailing part of the function,
so it naturally lends itself to do the early return.  It  makes it more
readable by eliminating the unnecessary indentation created by an
'if (sev_snp_enabled)' body.

Meanwhile, I agree with your comments on the first patch in the series.

Thanks for your review,

Kim