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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <727a7d5d-53f2-4575-b55a-f2b5422e5674@amd.com>
Date: Tue, 27 Jan 2026 14:56:02 -0600
From: Kim Phillips <kim.phillips@....com>
To: "Nikunj A. Dadhania" <nikunj@....com>, linux-kernel@...r.kernel.org,
 kvm@...r.kernel.org, linux-coco@...ts.linux.dev, x86@...nel.org
Cc: Sean Christopherson <seanjc@...gle.com>,
 Paolo Bonzini <pbonzini@...hat.com>, K Prateek Nayak
 <kprateek.nayak@....com>, Tom Lendacky <thomas.lendacky@....com>,
 Michael Roth <michael.roth@....com>,
 Borislav Petkov <borislav.petkov@....com>, Borislav Petkov <bp@...en8.de>,
 Naveen Rao <naveen.rao@....com>, David Kaplan <david.kaplan@....com>
Subject: Re: [PATCH 2/2] KVM: SEV: Add support for IBPB-on-Entry

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