| Message ID | 20211208191642.3792819-3-pgonda@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixes for SEV mirror VM tests | expand |
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 >
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 > >
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); }
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(-)