Re: [RFC 4/5] selftests: KVM: SNP IOCTL test

["Pratik R. Sampat" <pratikrajesh.sampat@amd.com>]

On 8/9/2024 10:48 AM, Sean Christopherson wrote:
> On Wed, Jul 10, 2024, Pratik R. Sampat wrote:
>> Introduce testing of SNP ioctl calls. This patch includes both positive
>> and negative tests of various parameters such as flags, page types and
>> policies.
>>
>> Signed-off-by: Pratik R. Sampat <pratikrajesh.sampat@amd.com>
>> ---
>>  .../selftests/kvm/x86_64/sev_smoke_test.c     | 119 +++++++++++++++++-
>>  1 file changed, 118 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
>> index 500c67b3793b..1d5c275c11b3 100644
>> --- a/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
>> +++ b/tools/testing/selftests/kvm/x86_64/sev_smoke_test.c
>> @@ -186,13 +186,130 @@ static void test_sev_launch(void *guest_code, uint32_t type, uint64_t policy)
>>  	kvm_vm_free(vm);
>>  }
>>  
>> +static int spawn_snp_launch_start(uint32_t type, uint64_t policy, uint8_t flags)
>> +{
>> +	struct kvm_vcpu *vcpu;
>> +	struct kvm_vm *vm;
>> +	int ret;
>> +
>> +	vm = vm_sev_create_with_one_vcpu(type, NULL, &vcpu);
> 
> Is a vCPU actually necessary/interesting?
> 

Yes, vcpu is not really needed here.
I could get away with a simple vm_create variant where I can pass the type.

>> +	ret = snp_vm_launch(vm, policy, flags);
>> +	kvm_vm_free(vm);
>> +
>> +	return ret;
>> +}
>> +
>> +static void test_snp_launch_start(uint32_t type, uint64_t policy)
>> +{
>> +	uint8_t i;
>> +	int ret;
>> +
>> +	ret = spawn_snp_launch_start(type, policy, 0);
> 
> s/spawn/__test, because "spawn" implies there's something living after this.
> 

Ack. Changed.

>> +	TEST_ASSERT(!ret,
>> +		    "KVM_SEV_SNP_LAUNCH_START should not fail, invalid flag.");
> 
> This should go away once vm_sev_ioctl() handles the assertion, but this assert
> message is bad (there's no invalid flag).
> 
>> +
>> +	for (i = 1; i < 8; i++) {
>> +		ret = spawn_snp_launch_start(type, policy, BIT(i));
>> +		TEST_ASSERT(ret && errno == EINVAL,
>> +			    "KVM_SEV_SNP_LAUNCH_START should fail, invalid flag.");
> 
> Print the flag, type, and policy.  In general, please think about what information
> would be helpful if this fails.

Right, I'll be more mindful of the error messages shown and try to
display more useful information from them for the overall series.

Thanks!
Pratik