* [PATCH 0/3] Fixes for SEV mirror VM tests
@ 2021-12-08 19:16 Peter Gonda
2021-12-08 19:16 ` [PATCH 1/3] selftests: sev_migrate_tests: Fix test_sev_mirror() Peter Gonda
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Peter Gonda @ 2021-12-08 19:16 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: Peter Gonda
Updated patch series for fixing bug in sev_ioctl() which allowed test
to look like a mirror vm could call KVM_SEV_LAUNCH_START. Adds
additional testing to validate mirror vm can only call subset of
commands.
I could not add the patch Seanjc recommended due to issues with
sev_platform_init() not correctly setting the fw error. I'll work ontop
of the INIT_EX patch series to fix this issue with the PSP driver.
Peter Gonda (3):
selftests: sev_migrate_tests: Fix test_sev_mirror()
selftests: sev_migrate_tests: Fix sev_ioctl()
selftests: sev_migrate_tests: Add mirror command tests
.../selftests/kvm/x86_64/sev_migrate_tests.c | 59 ++++++++++++++++---
1 file changed, 52 insertions(+), 7 deletions(-)
--
2.34.1.400.ga245620fadb-goog
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH 1/3] selftests: sev_migrate_tests: Fix test_sev_mirror() 2021-12-08 19:16 [PATCH 0/3] Fixes for SEV mirror VM tests Peter Gonda @ 2021-12-08 19:16 ` Peter Gonda 2021-12-09 5:43 ` Marc Orr 2021-12-08 19:16 ` [PATCH 2/3] selftests: sev_migrate_tests: Fix sev_ioctl() Peter Gonda 2021-12-08 19:16 ` [PATCH 3/3] selftests: sev_migrate_tests: Add mirror command tests Peter Gonda 2 siblings, 1 reply; 10+ messages in thread From: Peter Gonda @ 2021-12-08 19:16 UTC (permalink / raw) To: linux-kernel, kvm Cc: Peter Gonda, Paolo Bonzini, Sean Christopherson, Marc Orr Mirrors should not be able to call LAUNCH_START. Remove the call on the mirror to correct the test before fixing sev_ioctl() to correctly assert on this failed ioctl. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Sean Christopherson <seanjc@google.com> Cc: Marc Orr <marcorr@google.com> Signed-off-by: Peter Gonda <pgonda@google.com> --- tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c index 29b18d565cf4..fbc742b42145 100644 --- a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c +++ b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c @@ -228,9 +228,6 @@ static void sev_mirror_create(int dst_fd, int src_fd) static void test_sev_mirror(bool es) { struct kvm_vm *src_vm, *dst_vm; - struct kvm_sev_launch_start start = { - .policy = es ? SEV_POLICY_ES : 0 - }; int i; src_vm = sev_vm_create(es); @@ -241,7 +238,7 @@ static void test_sev_mirror(bool es) /* Check that we can complete creation of the mirror VM. */ for (i = 0; i < NR_MIGRATE_TEST_VCPUS; ++i) vm_vcpu_add(dst_vm, i); - sev_ioctl(dst_vm->fd, KVM_SEV_LAUNCH_START, &start); + if (es) sev_ioctl(dst_vm->fd, KVM_SEV_LAUNCH_UPDATE_VMSA, NULL); -- 2.34.1.400.ga245620fadb-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/3] selftests: sev_migrate_tests: Fix test_sev_mirror() 2021-12-08 19:16 ` [PATCH 1/3] selftests: sev_migrate_tests: Fix test_sev_mirror() Peter Gonda @ 2021-12-09 5:43 ` Marc Orr 0 siblings, 0 replies; 10+ messages in thread From: Marc Orr @ 2021-12-09 5:43 UTC (permalink / raw) To: Peter Gonda; +Cc: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson On Wed, Dec 8, 2021 at 11:16 AM Peter Gonda <pgonda@google.com> wrote: > > Mirrors should not be able to call LAUNCH_START. Remove the call on the > mirror to correct the test before fixing sev_ioctl() to correctly assert > on this failed ioctl. > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Sean Christopherson <seanjc@google.com> > Cc: Marc Orr <marcorr@google.com> > Signed-off-by: Peter Gonda <pgonda@google.com> > --- > tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > index 29b18d565cf4..fbc742b42145 100644 > --- a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > +++ b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > @@ -228,9 +228,6 @@ static void sev_mirror_create(int dst_fd, int src_fd) > static void test_sev_mirror(bool es) > { > struct kvm_vm *src_vm, *dst_vm; > - struct kvm_sev_launch_start start = { > - .policy = es ? SEV_POLICY_ES : 0 > - }; > int i; > > src_vm = sev_vm_create(es); > @@ -241,7 +238,7 @@ static void test_sev_mirror(bool es) > /* Check that we can complete creation of the mirror VM. */ > for (i = 0; i < NR_MIGRATE_TEST_VCPUS; ++i) > vm_vcpu_add(dst_vm, i); > - sev_ioctl(dst_vm->fd, KVM_SEV_LAUNCH_START, &start); > + > if (es) > sev_ioctl(dst_vm->fd, KVM_SEV_LAUNCH_UPDATE_VMSA, NULL); > > -- > 2.34.1.400.ga245620fadb-goog > Reviewed-by: Marc Orr <marcorr@google.com> ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 2/3] selftests: sev_migrate_tests: Fix sev_ioctl() 2021-12-08 19:16 [PATCH 0/3] Fixes for SEV mirror VM tests Peter Gonda 2021-12-08 19:16 ` [PATCH 1/3] selftests: sev_migrate_tests: Fix test_sev_mirror() Peter Gonda @ 2021-12-08 19:16 ` Peter Gonda 2021-12-09 5:45 ` Marc Orr 2021-12-08 19:16 ` [PATCH 3/3] selftests: sev_migrate_tests: Add mirror command tests Peter Gonda 2 siblings, 1 reply; 10+ messages in thread From: Peter Gonda @ 2021-12-08 19:16 UTC (permalink / raw) To: linux-kernel, kvm Cc: Peter Gonda, Paolo Bonzini, Sean Christopherson, Marc Orr TEST_ASSERT in SEV ioctl was allowing errors because it checked return value was good OR the FW error code was OK. This TEST_ASSERT should require both (aka. AND) values are OK. Removes the LAUNCH_START from the mirror VM because this call correctly fails because mirror VMs cannot call this command. Currently issues with the PSP driver functions mean the firmware error is not always reset to SEV_RET_SUCCESS when a call is successful. Mainly sev_platform_init() doesn't correctly set the fw error if the platform has already been initialized. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Sean Christopherson <seanjc@google.com> Cc: Marc Orr <marcorr@google.com> Signed-off-by: Peter Gonda <pgonda@google.com> --- tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c index fbc742b42145..4bb960ca6486 100644 --- a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c +++ b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c @@ -30,8 +30,9 @@ static void sev_ioctl(int vm_fd, int cmd_id, void *data) }; int ret; + ret = ioctl(vm_fd, KVM_MEMORY_ENCRYPT_OP, &cmd); - TEST_ASSERT((ret == 0 || cmd.error == SEV_RET_SUCCESS), + TEST_ASSERT(ret == 0 && cmd.error == SEV_RET_SUCCESS, "%d failed: return code: %d, errno: %d, fw error: %d", cmd_id, ret, errno, cmd.error); } -- 2.34.1.400.ga245620fadb-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] selftests: sev_migrate_tests: Fix sev_ioctl() 2021-12-08 19:16 ` [PATCH 2/3] selftests: sev_migrate_tests: Fix sev_ioctl() Peter Gonda @ 2021-12-09 5:45 ` Marc Orr 2021-12-09 18:25 ` Peter Gonda 0 siblings, 1 reply; 10+ messages in thread From: Marc Orr @ 2021-12-09 5:45 UTC (permalink / raw) To: Peter Gonda; +Cc: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson On Wed, Dec 8, 2021 at 11:16 AM Peter Gonda <pgonda@google.com> wrote: > > TEST_ASSERT in SEV ioctl was allowing errors because it checked return > value was good OR the FW error code was OK. This TEST_ASSERT should > require both (aka. AND) values are OK. Removes the LAUNCH_START from the > mirror VM because this call correctly fails because mirror VMs cannot > call this command. Currently issues with the PSP driver functions mean This commit description is now stale. The previous patch removes the LAUNCH_START -- not this patch. > the firmware error is not always reset to SEV_RET_SUCCESS when a call is > successful. Mainly sev_platform_init() doesn't correctly set the fw > error if the platform has already been initialized. > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Sean Christopherson <seanjc@google.com> > Cc: Marc Orr <marcorr@google.com> > Signed-off-by: Peter Gonda <pgonda@google.com> > --- > tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > index fbc742b42145..4bb960ca6486 100644 > --- a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > +++ b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > @@ -30,8 +30,9 @@ static void sev_ioctl(int vm_fd, int cmd_id, void *data) > }; > int ret; > > + nit: Looks like you picked up an extra new line. Since you need to fixup the commit description, let's fix this up too. > ret = ioctl(vm_fd, KVM_MEMORY_ENCRYPT_OP, &cmd); > - TEST_ASSERT((ret == 0 || cmd.error == SEV_RET_SUCCESS), > + TEST_ASSERT(ret == 0 && cmd.error == SEV_RET_SUCCESS, > "%d failed: return code: %d, errno: %d, fw error: %d", > cmd_id, ret, errno, cmd.error); > } > -- > 2.34.1.400.ga245620fadb-goog > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/3] selftests: sev_migrate_tests: Fix sev_ioctl() 2021-12-09 5:45 ` Marc Orr @ 2021-12-09 18:25 ` Peter Gonda 0 siblings, 0 replies; 10+ messages in thread From: Peter Gonda @ 2021-12-09 18:25 UTC (permalink / raw) To: Marc Orr; +Cc: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson On Wed, Dec 8, 2021 at 10:45 PM Marc Orr <marcorr@google.com> wrote: > > On Wed, Dec 8, 2021 at 11:16 AM Peter Gonda <pgonda@google.com> wrote: > > > > TEST_ASSERT in SEV ioctl was allowing errors because it checked return > > value was good OR the FW error code was OK. This TEST_ASSERT should > > require both (aka. AND) values are OK. Removes the LAUNCH_START from the > > mirror VM because this call correctly fails because mirror VMs cannot > > call this command. Currently issues with the PSP driver functions mean > > This commit description is now stale. The previous patch removes the > LAUNCH_START -- not this patch. > > > the firmware error is not always reset to SEV_RET_SUCCESS when a call is > > successful. Mainly sev_platform_init() doesn't correctly set the fw > > error if the platform has already been initialized. > > > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Cc: Sean Christopherson <seanjc@google.com> > > Cc: Marc Orr <marcorr@google.com> > > Signed-off-by: Peter Gonda <pgonda@google.com> > > --- > > tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > > index fbc742b42145..4bb960ca6486 100644 > > --- a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > > +++ b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > > @@ -30,8 +30,9 @@ static void sev_ioctl(int vm_fd, int cmd_id, void *data) > > }; > > int ret; > > > > + > > nit: Looks like you picked up an extra new line. Since you need to > fixup the commit description, let's fix this up too. I just sent a V0.1 with these fixes, thanks Marc. Paolo is that an OK way to handle that, I saw you queued the series? > > > ret = ioctl(vm_fd, KVM_MEMORY_ENCRYPT_OP, &cmd); > > - TEST_ASSERT((ret == 0 || cmd.error == SEV_RET_SUCCESS), > > + TEST_ASSERT(ret == 0 && cmd.error == SEV_RET_SUCCESS, > > "%d failed: return code: %d, errno: %d, fw error: %d", > > cmd_id, ret, errno, cmd.error); > > } > > -- > > 2.34.1.400.ga245620fadb-goog > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/3] selftests: sev_migrate_tests: Add mirror command tests 2021-12-08 19:16 [PATCH 0/3] Fixes for SEV mirror VM tests Peter Gonda 2021-12-08 19:16 ` [PATCH 1/3] selftests: sev_migrate_tests: Fix test_sev_mirror() Peter Gonda 2021-12-08 19:16 ` [PATCH 2/3] selftests: sev_migrate_tests: Fix sev_ioctl() Peter Gonda @ 2021-12-08 19:16 ` Peter Gonda 2021-12-09 5:53 ` Marc Orr 2021-12-09 11:27 ` Paolo Bonzini 2 siblings, 2 replies; 10+ messages in thread From: Peter Gonda @ 2021-12-08 19:16 UTC (permalink / raw) To: linux-kernel, kvm Cc: Peter Gonda, Paolo Bonzini, Sean Christopherson, Marc Orr Add tests to confirm mirror vms can only run correct subset of commands. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Sean Christopherson <seanjc@google.com> Cc: Marc Orr <marcorr@google.com> Signed-off-by: Peter Gonda <pgonda@google.com> --- .../selftests/kvm/x86_64/sev_migrate_tests.c | 55 +++++++++++++++++-- 1 file changed, 51 insertions(+), 4 deletions(-) diff --git a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c index 4bb960ca6486..80056bbbb003 100644 --- a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c +++ b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c @@ -21,7 +21,7 @@ #define NR_LOCK_TESTING_THREADS 3 #define NR_LOCK_TESTING_ITERATIONS 10000 -static void sev_ioctl(int vm_fd, int cmd_id, void *data) +static int __sev_ioctl(int vm_fd, int cmd_id, void *data, __u32 *fw_error) { struct kvm_sev_cmd cmd = { .id = cmd_id, @@ -30,11 +30,20 @@ static void sev_ioctl(int vm_fd, int cmd_id, void *data) }; int ret; - ret = ioctl(vm_fd, KVM_MEMORY_ENCRYPT_OP, &cmd); - TEST_ASSERT(ret == 0 && cmd.error == SEV_RET_SUCCESS, + *fw_error = cmd.error; + return ret; +} + +static void sev_ioctl(int vm_fd, int cmd_id, void *data) +{ + int ret; + __u32 fw_error; + + ret = __sev_ioctl(vm_fd, cmd_id, data, &fw_error); + TEST_ASSERT(ret == 0 && fw_error == SEV_RET_SUCCESS, "%d failed: return code: %d, errno: %d, fw error: %d", - cmd_id, ret, errno, cmd.error); + cmd_id, ret, errno, fw_error); } static struct kvm_vm *sev_vm_create(bool es) @@ -226,6 +235,42 @@ static void sev_mirror_create(int dst_fd, int src_fd) TEST_ASSERT(!ret, "Copying context failed, ret: %d, errno: %d\n", ret, errno); } +static void verify_mirror_allowed_cmds(int vm_fd) +{ + struct kvm_sev_guest_status status; + + for (int cmd_id = KVM_SEV_INIT; cmd_id < KVM_SEV_NR_MAX; ++cmd_id) { + int ret; + __u32 fw_error; + + /* + * These commands are allowed for mirror VMs, all others are + * not. + */ + switch (cmd_id) { + case KVM_SEV_LAUNCH_UPDATE_VMSA: + case KVM_SEV_GUEST_STATUS: + case KVM_SEV_DBG_DECRYPT: + case KVM_SEV_DBG_ENCRYPT: + continue; + default: + break; + } + + /* + * These commands should be disallowed before the data + * parameter is examined so NULL is OK here. + */ + ret = __sev_ioctl(vm_fd, cmd_id, NULL, &fw_error); + TEST_ASSERT( + ret == -1 && errno == EINVAL, + "Should not be able call command: %d. ret: %d, errno: %d\n", + cmd_id, ret, errno); + } + + sev_ioctl(vm_fd, KVM_SEV_GUEST_STATUS, &status); +} + static void test_sev_mirror(bool es) { struct kvm_vm *src_vm, *dst_vm; @@ -243,6 +288,8 @@ static void test_sev_mirror(bool es) if (es) sev_ioctl(dst_vm->fd, KVM_SEV_LAUNCH_UPDATE_VMSA, NULL); + verify_mirror_allowed_cmds(dst_vm->fd); + kvm_vm_free(src_vm); kvm_vm_free(dst_vm); } -- 2.34.1.400.ga245620fadb-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] selftests: sev_migrate_tests: Add mirror command tests 2021-12-08 19:16 ` [PATCH 3/3] selftests: sev_migrate_tests: Add mirror command tests Peter Gonda @ 2021-12-09 5:53 ` Marc Orr 2021-12-09 20:45 ` Marc Orr 2021-12-09 11:27 ` Paolo Bonzini 1 sibling, 1 reply; 10+ messages in thread From: Marc Orr @ 2021-12-09 5:53 UTC (permalink / raw) To: Peter Gonda; +Cc: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson On Wed, Dec 8, 2021 at 11:16 AM Peter Gonda <pgonda@google.com> wrote: > > Add tests to confirm mirror vms can only run correct subset of commands. > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Sean Christopherson <seanjc@google.com> > Cc: Marc Orr <marcorr@google.com> > Signed-off-by: Peter Gonda <pgonda@google.com> > --- > .../selftests/kvm/x86_64/sev_migrate_tests.c | 55 +++++++++++++++++-- > 1 file changed, 51 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > index 4bb960ca6486..80056bbbb003 100644 > --- a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > +++ b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > @@ -21,7 +21,7 @@ > #define NR_LOCK_TESTING_THREADS 3 > #define NR_LOCK_TESTING_ITERATIONS 10000 > > -static void sev_ioctl(int vm_fd, int cmd_id, void *data) > +static int __sev_ioctl(int vm_fd, int cmd_id, void *data, __u32 *fw_error) > { > struct kvm_sev_cmd cmd = { > .id = cmd_id, > @@ -30,11 +30,20 @@ static void sev_ioctl(int vm_fd, int cmd_id, void *data) > }; > int ret; > > - > ret = ioctl(vm_fd, KVM_MEMORY_ENCRYPT_OP, &cmd); > - TEST_ASSERT(ret == 0 && cmd.error == SEV_RET_SUCCESS, > + *fw_error = cmd.error; > + return ret; > +} > + > +static void sev_ioctl(int vm_fd, int cmd_id, void *data) > +{ > + int ret; > + __u32 fw_error; > + > + ret = __sev_ioctl(vm_fd, cmd_id, data, &fw_error); > + TEST_ASSERT(ret == 0 && fw_error == SEV_RET_SUCCESS, > "%d failed: return code: %d, errno: %d, fw error: %d", > - cmd_id, ret, errno, cmd.error); > + cmd_id, ret, errno, fw_error); > } > > static struct kvm_vm *sev_vm_create(bool es) > @@ -226,6 +235,42 @@ static void sev_mirror_create(int dst_fd, int src_fd) > TEST_ASSERT(!ret, "Copying context failed, ret: %d, errno: %d\n", ret, errno); > } > > +static void verify_mirror_allowed_cmds(int vm_fd) > +{ > + struct kvm_sev_guest_status status; > + > + for (int cmd_id = KVM_SEV_INIT; cmd_id < KVM_SEV_NR_MAX; ++cmd_id) { > + int ret; > + __u32 fw_error; > + > + /* > + * These commands are allowed for mirror VMs, all others are > + * not. > + */ > + switch (cmd_id) { > + case KVM_SEV_LAUNCH_UPDATE_VMSA: > + case KVM_SEV_GUEST_STATUS: > + case KVM_SEV_DBG_DECRYPT: > + case KVM_SEV_DBG_ENCRYPT: > + continue; > + default: > + break; > + } > + > + /* > + * These commands should be disallowed before the data > + * parameter is examined so NULL is OK here. > + */ > + ret = __sev_ioctl(vm_fd, cmd_id, NULL, &fw_error); > + TEST_ASSERT( > + ret == -1 && errno == EINVAL, > + "Should not be able call command: %d. ret: %d, errno: %d\n", > + cmd_id, ret, errno); > + } > + > + sev_ioctl(vm_fd, KVM_SEV_GUEST_STATUS, &status); Why is this here? I'd either delete it or maybe alternatively move it into the `case KVM_SEV_GUEST_STATUS` with a corresponding TEST_ASSERT to check that the command succeeded. Something like: ... switch (cmd_id) { case KVM_SEV_GUEST_STATUS: sev_ioctl(vm_fd, KVM_SEV_GUEST_STATUS, &status); TEST_ASSERT(ret == 0 && fw_error == SEV_RET_SUCCESS, ...); continue; case KVM_SEV_LAUNCH_UPDATE_VMSA: case KVM_SEV_DBG_DECRYPT: case KVM_SEV_DBG_ENCRYPT: continue; default: break; } > +} > + > static void test_sev_mirror(bool es) > { > struct kvm_vm *src_vm, *dst_vm; > @@ -243,6 +288,8 @@ static void test_sev_mirror(bool es) > if (es) > sev_ioctl(dst_vm->fd, KVM_SEV_LAUNCH_UPDATE_VMSA, NULL); > > + verify_mirror_allowed_cmds(dst_vm->fd); > + > kvm_vm_free(src_vm); > kvm_vm_free(dst_vm); > } > -- > 2.34.1.400.ga245620fadb-goog > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] selftests: sev_migrate_tests: Add mirror command tests 2021-12-09 5:53 ` Marc Orr @ 2021-12-09 20:45 ` Marc Orr 0 siblings, 0 replies; 10+ messages in thread From: Marc Orr @ 2021-12-09 20:45 UTC (permalink / raw) To: Peter Gonda; +Cc: linux-kernel, kvm, Paolo Bonzini, Sean Christopherson On Wed, Dec 8, 2021 at 9:53 PM Marc Orr <marcorr@google.com> wrote: > > On Wed, Dec 8, 2021 at 11:16 AM Peter Gonda <pgonda@google.com> wrote: > > > > Add tests to confirm mirror vms can only run correct subset of commands. > > > > Cc: Paolo Bonzini <pbonzini@redhat.com> > > Cc: Sean Christopherson <seanjc@google.com> > > Cc: Marc Orr <marcorr@google.com> > > Signed-off-by: Peter Gonda <pgonda@google.com> > > --- > > .../selftests/kvm/x86_64/sev_migrate_tests.c | 55 +++++++++++++++++-- > > 1 file changed, 51 insertions(+), 4 deletions(-) > > > > diff --git a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > > index 4bb960ca6486..80056bbbb003 100644 > > --- a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > > +++ b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > > @@ -21,7 +21,7 @@ > > #define NR_LOCK_TESTING_THREADS 3 > > #define NR_LOCK_TESTING_ITERATIONS 10000 > > > > -static void sev_ioctl(int vm_fd, int cmd_id, void *data) > > +static int __sev_ioctl(int vm_fd, int cmd_id, void *data, __u32 *fw_error) > > { > > struct kvm_sev_cmd cmd = { > > .id = cmd_id, > > @@ -30,11 +30,20 @@ static void sev_ioctl(int vm_fd, int cmd_id, void *data) > > }; > > int ret; > > > > - > > ret = ioctl(vm_fd, KVM_MEMORY_ENCRYPT_OP, &cmd); > > - TEST_ASSERT(ret == 0 && cmd.error == SEV_RET_SUCCESS, > > + *fw_error = cmd.error; > > + return ret; > > +} > > + > > +static void sev_ioctl(int vm_fd, int cmd_id, void *data) > > +{ > > + int ret; > > + __u32 fw_error; > > + > > + ret = __sev_ioctl(vm_fd, cmd_id, data, &fw_error); > > + TEST_ASSERT(ret == 0 && fw_error == SEV_RET_SUCCESS, > > "%d failed: return code: %d, errno: %d, fw error: %d", > > - cmd_id, ret, errno, cmd.error); > > + cmd_id, ret, errno, fw_error); > > } > > > > static struct kvm_vm *sev_vm_create(bool es) > > @@ -226,6 +235,42 @@ static void sev_mirror_create(int dst_fd, int src_fd) > > TEST_ASSERT(!ret, "Copying context failed, ret: %d, errno: %d\n", ret, errno); > > } > > > > +static void verify_mirror_allowed_cmds(int vm_fd) > > +{ > > + struct kvm_sev_guest_status status; > > + > > + for (int cmd_id = KVM_SEV_INIT; cmd_id < KVM_SEV_NR_MAX; ++cmd_id) { > > + int ret; > > + __u32 fw_error; > > + > > + /* > > + * These commands are allowed for mirror VMs, all others are > > + * not. > > + */ > > + switch (cmd_id) { > > + case KVM_SEV_LAUNCH_UPDATE_VMSA: > > + case KVM_SEV_GUEST_STATUS: > > + case KVM_SEV_DBG_DECRYPT: > > + case KVM_SEV_DBG_ENCRYPT: > > + continue; > > + default: > > + break; > > + } > > + > > + /* > > + * These commands should be disallowed before the data > > + * parameter is examined so NULL is OK here. > > + */ > > + ret = __sev_ioctl(vm_fd, cmd_id, NULL, &fw_error); > > + TEST_ASSERT( > > + ret == -1 && errno == EINVAL, > > + "Should not be able call command: %d. ret: %d, errno: %d\n", > > + cmd_id, ret, errno); > > + } > > + > > + sev_ioctl(vm_fd, KVM_SEV_GUEST_STATUS, &status); > > Why is this here? I'd either delete it or maybe alternatively move it > into the `case KVM_SEV_GUEST_STATUS` with a corresponding TEST_ASSERT > to check that the command succeeded. Something like: > > ... > switch (cmd_id) { > case KVM_SEV_GUEST_STATUS: > sev_ioctl(vm_fd, KVM_SEV_GUEST_STATUS, &status); > TEST_ASSERT(ret == 0 && fw_error == SEV_RET_SUCCESS, ...); > continue; > case KVM_SEV_LAUNCH_UPDATE_VMSA: > case KVM_SEV_DBG_DECRYPT: > case KVM_SEV_DBG_ENCRYPT: > continue; > default: > break; > } For posterity: Peter pointed out to me offline that `sev_ioctl()` in fact does the TEST_ASSERT internally. Doh! So this line is fine as is. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 3/3] selftests: sev_migrate_tests: Add mirror command tests 2021-12-08 19:16 ` [PATCH 3/3] selftests: sev_migrate_tests: Add mirror command tests Peter Gonda 2021-12-09 5:53 ` Marc Orr @ 2021-12-09 11:27 ` Paolo Bonzini 1 sibling, 0 replies; 10+ messages in thread From: Paolo Bonzini @ 2021-12-09 11:27 UTC (permalink / raw) To: Peter Gonda, linux-kernel, kvm; +Cc: Sean Christopherson, Marc Orr On 12/8/21 20:16, Peter Gonda wrote: > Add tests to confirm mirror vms can only run correct subset of commands. > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Sean Christopherson <seanjc@google.com> > Cc: Marc Orr <marcorr@google.com> > Signed-off-by: Peter Gonda <pgonda@google.com> > --- > .../selftests/kvm/x86_64/sev_migrate_tests.c | 55 +++++++++++++++++-- > 1 file changed, 51 insertions(+), 4 deletions(-) > > diff --git a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > index 4bb960ca6486..80056bbbb003 100644 > --- a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > +++ b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > @@ -21,7 +21,7 @@ > #define NR_LOCK_TESTING_THREADS 3 > #define NR_LOCK_TESTING_ITERATIONS 10000 > > -static void sev_ioctl(int vm_fd, int cmd_id, void *data) > +static int __sev_ioctl(int vm_fd, int cmd_id, void *data, __u32 *fw_error) > { > struct kvm_sev_cmd cmd = { > .id = cmd_id, > @@ -30,11 +30,20 @@ static void sev_ioctl(int vm_fd, int cmd_id, void *data) > }; > int ret; > > - > ret = ioctl(vm_fd, KVM_MEMORY_ENCRYPT_OP, &cmd); > - TEST_ASSERT(ret == 0 && cmd.error == SEV_RET_SUCCESS, > + *fw_error = cmd.error; > + return ret; > +} > + > +static void sev_ioctl(int vm_fd, int cmd_id, void *data) > +{ > + int ret; > + __u32 fw_error; > + > + ret = __sev_ioctl(vm_fd, cmd_id, data, &fw_error); > + TEST_ASSERT(ret == 0 && fw_error == SEV_RET_SUCCESS, > "%d failed: return code: %d, errno: %d, fw error: %d", > - cmd_id, ret, errno, cmd.error); > + cmd_id, ret, errno, fw_error); > } > > static struct kvm_vm *sev_vm_create(bool es) > @@ -226,6 +235,42 @@ static void sev_mirror_create(int dst_fd, int src_fd) > TEST_ASSERT(!ret, "Copying context failed, ret: %d, errno: %d\n", ret, errno); > } > > +static void verify_mirror_allowed_cmds(int vm_fd) > +{ > + struct kvm_sev_guest_status status; > + > + for (int cmd_id = KVM_SEV_INIT; cmd_id < KVM_SEV_NR_MAX; ++cmd_id) { > + int ret; > + __u32 fw_error; > + > + /* > + * These commands are allowed for mirror VMs, all others are > + * not. > + */ > + switch (cmd_id) { > + case KVM_SEV_LAUNCH_UPDATE_VMSA: > + case KVM_SEV_GUEST_STATUS: > + case KVM_SEV_DBG_DECRYPT: > + case KVM_SEV_DBG_ENCRYPT: > + continue; > + default: > + break; > + } > + > + /* > + * These commands should be disallowed before the data > + * parameter is examined so NULL is OK here. > + */ > + ret = __sev_ioctl(vm_fd, cmd_id, NULL, &fw_error); > + TEST_ASSERT( > + ret == -1 && errno == EINVAL, > + "Should not be able call command: %d. ret: %d, errno: %d\n", > + cmd_id, ret, errno); > + } > + > + sev_ioctl(vm_fd, KVM_SEV_GUEST_STATUS, &status); > +} > + > static void test_sev_mirror(bool es) > { > struct kvm_vm *src_vm, *dst_vm; > @@ -243,6 +288,8 @@ static void test_sev_mirror(bool es) > if (es) > sev_ioctl(dst_vm->fd, KVM_SEV_LAUNCH_UPDATE_VMSA, NULL); > > + verify_mirror_allowed_cmds(dst_vm->fd); > + > kvm_vm_free(src_vm); > kvm_vm_free(dst_vm); > } > Queued, thanks. Paolo ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-12-09 20:46 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-08 19:16 [PATCH 0/3] Fixes for SEV mirror VM tests Peter Gonda 2021-12-08 19:16 ` [PATCH 1/3] selftests: sev_migrate_tests: Fix test_sev_mirror() Peter Gonda 2021-12-09 5:43 ` Marc Orr 2021-12-08 19:16 ` [PATCH 2/3] selftests: sev_migrate_tests: Fix sev_ioctl() Peter Gonda 2021-12-09 5:45 ` Marc Orr 2021-12-09 18:25 ` Peter Gonda 2021-12-08 19:16 ` [PATCH 3/3] selftests: sev_migrate_tests: Add mirror command tests Peter Gonda 2021-12-09 5:53 ` Marc Orr 2021-12-09 20:45 ` Marc Orr 2021-12-09 11:27 ` Paolo Bonzini
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).