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!
