diff mbox series

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

Message ID 20240710220540.188239-5-pratikrajesh.sampat@amd.com (mailing list archive)
State New, archived
Headers show
Series SEV Kernel Selftests | expand

Commit Message

Pratik R. Sampat July 10, 2024, 10:05 p.m. UTC
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(-)

Comments

Peter Gonda July 11, 2024, 3:57 p.m. UTC | #1
On Wed, Jul 10, 2024 at 4:06 PM Pratik R. Sampat
<pratikrajesh.sampat@amd.com> 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>

Tested-by: Peter Gonda <pgonda@google.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);
> +       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);
> +       TEST_ASSERT(!ret,
> +                   "KVM_SEV_SNP_LAUNCH_START should not fail, 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.");
> +       }

To save readers sometime do we want to comment that flags must be zero?

> +
> +       ret = spawn_snp_launch_start(type, 0, 0);
> +       TEST_ASSERT(ret && errno == EINVAL,
> +                   "KVM_SEV_SNP_LAUNCH_START should fail, invalid policy.");
> +
> +       ret = spawn_snp_launch_start(type, SNP_POLICY_SMT, 0);
> +       TEST_ASSERT(ret && errno == EINVAL,
> +                   "KVM_SEV_SNP_LAUNCH_START should fail, invalid policy.");
> +
> +       ret = spawn_snp_launch_start(type, SNP_POLICY_RSVD_MBO, 0);
> +       TEST_ASSERT(ret && errno == EINVAL,
> +                   "KVM_SEV_SNP_LAUNCH_START should fail, invalid policy.");

Ditto on SMT comment, this could pass if SMT was disabled right?

> +
> +       ret = spawn_snp_launch_start(type, SNP_POLICY_SMT | SNP_POLICY_RSVD_MBO |
> +                                    (255 * SNP_POLICY_ABI_MAJOR) |
> +                                    (255 * SNP_POLICY_ABI_MINOR), 0);
> +       TEST_ASSERT(ret && errno == EIO,
> +                   "KVM_SEV_SNP_LAUNCH_START should fail, invalid version.");
> +}
> +
> +static void test_snp_launch_update(uint32_t type, uint64_t policy)
> +{
> +       struct kvm_vcpu *vcpu;
> +       struct kvm_vm *vm;
> +       int ret;
> +
> +       for (int pgtype = 0; pgtype <= KVM_SEV_SNP_PAGE_TYPE_CPUID; pgtype++) {

Do we want to test KVM_SEV_SNP_PAGE_TYPE_CPUID+1 to make sure that fails?

> +               vm = vm_sev_create_with_one_vcpu(type, NULL, &vcpu);
> +               snp_vm_launch(vm, policy, 0);
> +               ret = snp_vm_launch_update(vm, pgtype);
> +
> +               switch (pgtype) {
> +               case KVM_SEV_SNP_PAGE_TYPE_NORMAL:
> +               case KVM_SEV_SNP_PAGE_TYPE_ZERO:
> +               case KVM_SEV_SNP_PAGE_TYPE_UNMEASURED:
> +               case KVM_SEV_SNP_PAGE_TYPE_SECRETS:
> +                       TEST_ASSERT(!ret,
> +                                   "KVM_SEV_SNP_LAUNCH_UPDATE should not fail, invalid Page type.");

Double negative maybe: "KVM_SEV_SNP_LAUNCH_UPDATE should succeed..."

> +                       break;
> +               case KVM_SEV_SNP_PAGE_TYPE_CPUID:
> +                       TEST_ASSERT(ret && errno == EIO,
> +                                   "KVM_SEV_SNP_LAUNCH_UPDATE should fail, invalid Page type.");

This is a valid page type right? But I think the error is from the ASP
due to the page being malformed for a CPUID page.

> +                       break;
> +               default:
> +                       TEST_ASSERT(ret && errno == EINVAL,
> +                                   "KVM_SEV_SNP_LAUNCH_UPDATE should fail, invalid Page type.");
> +               }
> +
> +               kvm_vm_free(vm);
> +       }
> +}
> +
> +void test_snp_launch_finish(uint32_t type, uint64_t policy)
> +{
> +       struct kvm_vcpu *vcpu;
> +       struct kvm_vm *vm;
> +       int ret;
> +
> +       vm = vm_sev_create_with_one_vcpu(type, NULL, &vcpu);
> +       snp_vm_launch(vm, policy, 0);
> +       snp_vm_launch_update(vm, KVM_SEV_SNP_PAGE_TYPE_NORMAL);
> +       ret = snp_vm_launch_finish(vm, 0);
> +       TEST_ASSERT(!ret,
> +                   "KVM_SEV_SNP_LAUNCH_FINISH should not fail, invalid flag.");

Comment is wrong, maybe: "KVM_SEV_SNP_LAUNCH_FINISH should not fail."

> +       kvm_vm_free(vm);
> +
> +       for (int i = 1; i < 16; i++) {
> +               vm = vm_sev_create_with_one_vcpu(type, NULL, &vcpu);
> +               snp_vm_launch(vm, policy, 0);
> +               snp_vm_launch_update(vm, KVM_SEV_SNP_PAGE_TYPE_NORMAL);
> +               ret = snp_vm_launch_finish(vm, BIT(i));
> +               TEST_ASSERT(ret && errno == EINVAL,
> +                           "KVM_SEV_SNP_LAUNCH_FINISH should fail, invalid flag.");
> +               kvm_vm_free(vm);

To save readers sometime do we want to comment that flags must be zero?

> +       }
> +}
> +
> +static void test_sev_ioctl(void *guest_code, uint32_t type, uint64_t policy)
> +{
> +       if (type == KVM_X86_SNP_VM) {
> +               test_snp_launch_start(type, policy);
> +               test_snp_launch_update(type, policy);
> +               test_snp_launch_finish(type, policy);
> +
> +               return;
> +       }
> +
> +       test_sev_launch(guest_code, type, policy);
> +}
> +
>  static void test_sev(void *guest_code, uint32_t type, uint64_t policy)
>  {
>         struct kvm_vcpu *vcpu;
>         struct kvm_vm *vm;
>         struct ucall uc;
>
> -       test_sev_launch(guest_code, type, policy);
> +       test_sev_ioctl(guest_code, type, policy);
>
>         vm = vm_sev_create_with_one_vcpu(type, guest_code, &vcpu);
>
> --
> 2.34.1
>
Pratik R. Sampat July 11, 2024, 4:27 p.m. UTC | #2
On 7/11/2024 10:57 AM, Peter Gonda wrote:
> On Wed, Jul 10, 2024 at 4:06 PM Pratik R. Sampat
> <pratikrajesh.sampat@amd.com> 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>
> 
> Tested-by: Peter Gonda <pgonda@google.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);
>> +       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);
>> +       TEST_ASSERT(!ret,
>> +                   "KVM_SEV_SNP_LAUNCH_START should not fail, 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.");
>> +       }
> 
> To save readers sometime do we want to comment that flags must be zero?
> 

Ack. I can add that comment.

>> +
>> +       ret = spawn_snp_launch_start(type, 0, 0);
>> +       TEST_ASSERT(ret && errno == EINVAL,
>> +                   "KVM_SEV_SNP_LAUNCH_START should fail, invalid policy.");
>> +
>> +       ret = spawn_snp_launch_start(type, SNP_POLICY_SMT, 0);
>> +       TEST_ASSERT(ret && errno == EINVAL,
>> +                   "KVM_SEV_SNP_LAUNCH_START should fail, invalid policy.");
>> +
>> +       ret = spawn_snp_launch_start(type, SNP_POLICY_RSVD_MBO, 0);
>> +       TEST_ASSERT(ret && errno == EINVAL,
>> +                   "KVM_SEV_SNP_LAUNCH_START should fail, invalid policy.");
> 
> Ditto on SMT comment, this could pass if SMT was disabled right?
> 

Ack.
Yes, it could. Maybe the check I was speaking about earlier about SMT
can be made here as well and based on that we decide if this should fail
or pass.

>> +
>> +       ret = spawn_snp_launch_start(type, SNP_POLICY_SMT | SNP_POLICY_RSVD_MBO |
>> +                                    (255 * SNP_POLICY_ABI_MAJOR) |
>> +                                    (255 * SNP_POLICY_ABI_MINOR), 0);
>> +       TEST_ASSERT(ret && errno == EIO,
>> +                   "KVM_SEV_SNP_LAUNCH_START should fail, invalid version.");
>> +}
>> +
>> +static void test_snp_launch_update(uint32_t type, uint64_t policy)
>> +{
>> +       struct kvm_vcpu *vcpu;
>> +       struct kvm_vm *vm;
>> +       int ret;
>> +
>> +       for (int pgtype = 0; pgtype <= KVM_SEV_SNP_PAGE_TYPE_CPUID; pgtype++) {
> 
> Do we want to test KVM_SEV_SNP_PAGE_TYPE_CPUID+1 to make sure that fails?
> 

We could. Looking at loop however, we also go through 0x2 which is
undefined so I thought we were already taking care of the negative test
case here. Having said that, I have no issues in adding one more case
that fails.

>> +               vm = vm_sev_create_with_one_vcpu(type, NULL, &vcpu);
>> +               snp_vm_launch(vm, policy, 0);
>> +               ret = snp_vm_launch_update(vm, pgtype);
>> +
>> +               switch (pgtype) {
>> +               case KVM_SEV_SNP_PAGE_TYPE_NORMAL:
>> +               case KVM_SEV_SNP_PAGE_TYPE_ZERO:
>> +               case KVM_SEV_SNP_PAGE_TYPE_UNMEASURED:
>> +               case KVM_SEV_SNP_PAGE_TYPE_SECRETS:
>> +                       TEST_ASSERT(!ret,
>> +                                   "KVM_SEV_SNP_LAUNCH_UPDATE should not fail, invalid Page type.");
> 
> Double negative maybe: "KVM_SEV_SNP_LAUNCH_UPDATE should succeed..."
> 

Ack. This double negative is used in a couple of more places. Will clean
them up too.

>> +                       break;
>> +               case KVM_SEV_SNP_PAGE_TYPE_CPUID:
>> +                       TEST_ASSERT(ret && errno == EIO,
>> +                                   "KVM_SEV_SNP_LAUNCH_UPDATE should fail, invalid Page type.");
> 
> This is a valid page type right? But I think the error is from the ASP
> due to the page being malformed for a CPUID page.
> 

Yes you're absolutely right. It's technically a correct page type just
not set up correctly to be used this way so we should see it fail.

>> +                       break;
>> +               default:
>> +                       TEST_ASSERT(ret && errno == EINVAL,
>> +                                   "KVM_SEV_SNP_LAUNCH_UPDATE should fail, invalid Page type.");
>> +               }
>> +
>> +               kvm_vm_free(vm);
>> +       }
>> +}
>> +
>> +void test_snp_launch_finish(uint32_t type, uint64_t policy)
>> +{
>> +       struct kvm_vcpu *vcpu;
>> +       struct kvm_vm *vm;
>> +       int ret;
>> +
>> +       vm = vm_sev_create_with_one_vcpu(type, NULL, &vcpu);
>> +       snp_vm_launch(vm, policy, 0);
>> +       snp_vm_launch_update(vm, KVM_SEV_SNP_PAGE_TYPE_NORMAL);
>> +       ret = snp_vm_launch_finish(vm, 0);
>> +       TEST_ASSERT(!ret,
>> +                   "KVM_SEV_SNP_LAUNCH_FINISH should not fail, invalid flag.");
> 
> Comment is wrong, maybe: "KVM_SEV_SNP_LAUNCH_FINISH should not fail."
> 

Thanks for catching this. Will fix the comment.

>> +       kvm_vm_free(vm);
>> +
>> +       for (int i = 1; i < 16; i++) {
>> +               vm = vm_sev_create_with_one_vcpu(type, NULL, &vcpu);
>> +               snp_vm_launch(vm, policy, 0);
>> +               snp_vm_launch_update(vm, KVM_SEV_SNP_PAGE_TYPE_NORMAL);
>> +               ret = snp_vm_launch_finish(vm, BIT(i));
>> +               TEST_ASSERT(ret && errno == EINVAL,
>> +                           "KVM_SEV_SNP_LAUNCH_FINISH should fail, invalid flag.");
>> +               kvm_vm_free(vm);
> 
> To save readers sometime do we want to comment that flags must be zero?
> 

Ack.

Thanks again for the review
Sean Christopherson Aug. 9, 2024, 3:48 p.m. UTC | #3
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?

> +	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.

> +	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.
Pratik R. Sampat Aug. 13, 2024, 3:23 p.m. UTC | #4
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
diff mbox series

Patch

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);
+	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);
+	TEST_ASSERT(!ret,
+		    "KVM_SEV_SNP_LAUNCH_START should not fail, 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.");
+	}
+
+	ret = spawn_snp_launch_start(type, 0, 0);
+	TEST_ASSERT(ret && errno == EINVAL,
+		    "KVM_SEV_SNP_LAUNCH_START should fail, invalid policy.");
+
+	ret = spawn_snp_launch_start(type, SNP_POLICY_SMT, 0);
+	TEST_ASSERT(ret && errno == EINVAL,
+		    "KVM_SEV_SNP_LAUNCH_START should fail, invalid policy.");
+
+	ret = spawn_snp_launch_start(type, SNP_POLICY_RSVD_MBO, 0);
+	TEST_ASSERT(ret && errno == EINVAL,
+		    "KVM_SEV_SNP_LAUNCH_START should fail, invalid policy.");
+
+	ret = spawn_snp_launch_start(type, SNP_POLICY_SMT | SNP_POLICY_RSVD_MBO |
+				     (255 * SNP_POLICY_ABI_MAJOR) |
+				     (255 * SNP_POLICY_ABI_MINOR), 0);
+	TEST_ASSERT(ret && errno == EIO,
+		    "KVM_SEV_SNP_LAUNCH_START should fail, invalid version.");
+}
+
+static void test_snp_launch_update(uint32_t type, uint64_t policy)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	int ret;
+
+	for (int pgtype = 0; pgtype <= KVM_SEV_SNP_PAGE_TYPE_CPUID; pgtype++) {
+		vm = vm_sev_create_with_one_vcpu(type, NULL, &vcpu);
+		snp_vm_launch(vm, policy, 0);
+		ret = snp_vm_launch_update(vm, pgtype);
+
+		switch (pgtype) {
+		case KVM_SEV_SNP_PAGE_TYPE_NORMAL:
+		case KVM_SEV_SNP_PAGE_TYPE_ZERO:
+		case KVM_SEV_SNP_PAGE_TYPE_UNMEASURED:
+		case KVM_SEV_SNP_PAGE_TYPE_SECRETS:
+			TEST_ASSERT(!ret,
+				    "KVM_SEV_SNP_LAUNCH_UPDATE should not fail, invalid Page type.");
+			break;
+		case KVM_SEV_SNP_PAGE_TYPE_CPUID:
+			TEST_ASSERT(ret && errno == EIO,
+				    "KVM_SEV_SNP_LAUNCH_UPDATE should fail, invalid Page type.");
+			break;
+		default:
+			TEST_ASSERT(ret && errno == EINVAL,
+				    "KVM_SEV_SNP_LAUNCH_UPDATE should fail, invalid Page type.");
+		}
+
+		kvm_vm_free(vm);
+	}
+}
+
+void test_snp_launch_finish(uint32_t type, uint64_t policy)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	int ret;
+
+	vm = vm_sev_create_with_one_vcpu(type, NULL, &vcpu);
+	snp_vm_launch(vm, policy, 0);
+	snp_vm_launch_update(vm, KVM_SEV_SNP_PAGE_TYPE_NORMAL);
+	ret = snp_vm_launch_finish(vm, 0);
+	TEST_ASSERT(!ret,
+		    "KVM_SEV_SNP_LAUNCH_FINISH should not fail, invalid flag.");
+	kvm_vm_free(vm);
+
+	for (int i = 1; i < 16; i++) {
+		vm = vm_sev_create_with_one_vcpu(type, NULL, &vcpu);
+		snp_vm_launch(vm, policy, 0);
+		snp_vm_launch_update(vm, KVM_SEV_SNP_PAGE_TYPE_NORMAL);
+		ret = snp_vm_launch_finish(vm, BIT(i));
+		TEST_ASSERT(ret && errno == EINVAL,
+			    "KVM_SEV_SNP_LAUNCH_FINISH should fail, invalid flag.");
+		kvm_vm_free(vm);
+	}
+}
+
+static void test_sev_ioctl(void *guest_code, uint32_t type, uint64_t policy)
+{
+	if (type == KVM_X86_SNP_VM) {
+		test_snp_launch_start(type, policy);
+		test_snp_launch_update(type, policy);
+		test_snp_launch_finish(type, policy);
+
+		return;
+	}
+
+	test_sev_launch(guest_code, type, policy);
+}
+
 static void test_sev(void *guest_code, uint32_t type, uint64_t policy)
 {
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
 	struct ucall uc;
 
-	test_sev_launch(guest_code, type, policy);
+	test_sev_ioctl(guest_code, type, policy);
 
 	vm = vm_sev_create_with_one_vcpu(type, guest_code, &vcpu);