linux-kernel - Re: [PATCH v6 9/9] KVM: selftests: Add a basic SEV-SNP smoke test

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <cc0253ec-bf30-49b4-b7fd-15a063c80b9d@amd.com>
Date: Wed, 19 Feb 2025 08:58:44 -0600
From: Pratik Rajesh Sampat <prsampat@....com>
To: Sean Christopherson <seanjc@...gle.com>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org, kvm@...r.kernel.org,
 linux-crypto@...r.kernel.org, linux-kselftest@...r.kernel.org,
 pbonzini@...hat.com, thomas.lendacky@....com, tglx@...utronix.de,
 mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
 shuah@...nel.org, pgonda@...gle.com, ashish.kalra@....com, nikunj@....com,
 pankaj.gupta@....com, michael.roth@....com, sraithal@....com
Subject: Re: [PATCH v6 9/9] KVM: selftests: Add a basic SEV-SNP smoke test



On 2/18/25 6:54 PM, Sean Christopherson wrote:
> On Fri, Feb 14, 2025, Pratik Rajesh Sampat wrote:
>>
>>
>> On 2/11/25 8:31 PM, Sean Christopherson wrote:
>>> On Mon, Feb 03, 2025, Pratik R. Sampat wrote:
>>>> @@ -217,5 +244,20 @@ int main(int argc, char *argv[])
>>>>  		}
>>>>  	}
>>>>  
>>>> +	if (kvm_cpu_has(X86_FEATURE_SEV_SNP)) {
>>>> +		uint64_t snp_policy = snp_default_policy();
>>>> +
>>>> +		test_snp(snp_policy);
>>>> +		/* Test minimum firmware level */
>>>> +		test_snp(snp_policy | SNP_FW_VER_MAJOR(SNP_MIN_API_MAJOR) |
>>>> +			SNP_FW_VER_MINOR(SNP_MIN_API_MINOR));
>>>
>>> Ah, this is where the firmware policy stuff is used.  Refresh me, can userspace
>>> request _any_ major/minor as the min, and expect failure if the version isn't
>>> supported?  If so, the test should iterate over the major/minor combinations that
>>> are guaranteed to fail.  And if userspace can query the supported minor/major,
>>> the test should iterate over all the happy versions too. 
>>>
>>
>> Yes, any policy greater than the min policy (defined in sev-dev.c)
>> should be supported. The sad path tests were intended to be added in the
>> upcoming negative test patch series so that we could have the proper
>> infrastructure to handle and report failures.
>>
>>> Unless there's nothing interesting to test, I would move the major/minor stuff to
>>> a separate patch.
>>
>> Would you rather prefer I do the happy tests here (something like -
>> min_policy and min_policy + 1?) and defer the failure tests for the
>> next patchset? Or, I can remove policy testing from here entirely and
>> introduce it only when the sad path testing infrastructure is ready, so
>> that we can test this completely at once?
> 
> Let's do the latter.  For the initial series, do the bare minimum so that we can
> get that merged, and then focus on the min API version stuff in a separate series.
> The version testing shouldn't be terribly complex, but it doesn't seem like it's
> entirely trivial either, and I don't want it to block the base SNP support.

Sure thing, will do.
Thanks!