| Message ID | 20211123005036.2954379-9-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fixes for KVM_CAP_VM_MOVE/COPY_ENC_CONTEXT_FROM | expand |
On Mon, Nov 22, 2021 at 5:50 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > I am putting the tests in sev_migrate_tests because the failure conditions are > very similar and some of the setup code can be reused, too. > > The tests cover both successful creation of a mirror VM, and error > conditions. > > Cc: Peter Gonda <pgonda@google.com> > Cc: Sean Christopherson <seanjc@google.com> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > .../selftests/kvm/x86_64/sev_migrate_tests.c | 112 ++++++++++++++++-- > 1 file changed, 105 insertions(+), 7 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 0cd7e2eaa895..d265cea5de85 100644 > --- a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > +++ b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > @@ -54,12 +54,15 @@ static struct kvm_vm *sev_vm_create(bool es) > return vm; > } > > -static struct kvm_vm *__vm_create(void) > +static struct kvm_vm *aux_vm_create(bool with_vcpus) > { > struct kvm_vm *vm; > int i; > > vm = vm_create(VM_MODE_DEFAULT, 0, O_RDWR); > + if (!with_vcpus) > + return vm; > + > for (i = 0; i < NR_MIGRATE_TEST_VCPUS; ++i) > vm_vcpu_add(vm, i); > > @@ -93,7 +96,7 @@ static void test_sev_migrate_from(bool es) > > src_vm = sev_vm_create(es); > for (i = 0; i < NR_MIGRATE_TEST_VMS; ++i) > - dst_vms[i] = __vm_create(); > + dst_vms[i] = aux_vm_create(true); > > /* Initial migration from the src to the first dst. */ > sev_migrate_from(dst_vms[0]->fd, src_vm->fd); > @@ -162,7 +165,7 @@ static void test_sev_migrate_parameters(void) > sev_vm = sev_vm_create(/* es= */ false); > sev_es_vm = sev_vm_create(/* es= */ true); > vm_no_vcpu = vm_create(VM_MODE_DEFAULT, 0, O_RDWR); > - vm_no_sev = __vm_create(); > + vm_no_sev = aux_vm_create(true); > sev_es_vm_no_vmsa = vm_create(VM_MODE_DEFAULT, 0, O_RDWR); > sev_ioctl(sev_es_vm_no_vmsa->fd, KVM_SEV_ES_INIT, NULL); > vm_vcpu_add(sev_es_vm_no_vmsa, 1); > @@ -203,11 +206,106 @@ static void test_sev_migrate_parameters(void) > kvm_vm_free(vm_no_sev); > } > > +static int __sev_mirror_create(int dst_fd, int src_fd) > +{ > + struct kvm_enable_cap cap = { > + .cap = KVM_CAP_VM_COPY_ENC_CONTEXT_FROM, > + .args = { src_fd } > + }; > + > + return ioctl(dst_fd, KVM_ENABLE_CAP, &cap); > +} > + > + > +static void sev_mirror_create(int dst_fd, int src_fd) > +{ > + int ret; > + > + ret = __sev_mirror_create(dst_fd, src_fd); > + TEST_ASSERT(!ret, "Copying context failed, ret: %d, errno: %d\n", ret, errno); > +} > + > +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); > + dst_vm = aux_vm_create(false); > + > + sev_mirror_create(dst_vm->fd, src_vm->fd); > + > + /* 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); I don't think this should be called on the mirror and I think it should be an error. In is_cmd_allowed_from_mirror() KVM_SEV_LAUNCH_START should not be allowed: if (cmd_id == KVM_SEV_LAUNCH_UPDATE_VMSA || cmd_id == KVM_SEV_GUEST_STATUS || cmd_id == KVM_SEV_DBG_DECRYPT || cmd_id == KVM_SEV_DBG_ENCRYPT) return true; This overrides the mirrored values and sets up the VM as a new SEV context. I would have thought the sev_bind_asid() in sev_launch_start() would fail because the asid is already used by the source. > + if (es) > + sev_ioctl(dst_vm->fd, KVM_SEV_LAUNCH_UPDATE_VMSA, NULL); > + > + kvm_vm_free(src_vm); > + kvm_vm_free(dst_vm); > +} > + > +static void test_sev_mirror_parameters(void) > +{ > + struct kvm_vm *sev_vm, *sev_es_vm, *vm_no_vcpu, *vm_with_vcpu; > + int ret; > + > + sev_vm = sev_vm_create(/* es= */ false); > + sev_es_vm = sev_vm_create(/* es= */ true); > + vm_with_vcpu = aux_vm_create(true); > + vm_no_vcpu = aux_vm_create(false); > + > + ret = __sev_mirror_create(sev_vm->fd, sev_vm->fd); > + TEST_ASSERT( > + ret == -1 && errno == EINVAL, > + "Should not be able copy context to self. ret: %d, errno: %d\n", > + ret, errno); > + > + ret = __sev_mirror_create(sev_vm->fd, sev_es_vm->fd); > + TEST_ASSERT( > + ret == -1 && errno == EINVAL, > + "Should not be able copy context to SEV enabled VM. ret: %d, errno: %d\n", > + ret, errno); > + > + ret = __sev_mirror_create(sev_es_vm->fd, sev_vm->fd); > + TEST_ASSERT( > + ret == -1 && errno == EINVAL, > + "Should not be able copy context to SEV-ES enabled VM. ret: %d, errno: %d\n", > + ret, errno); > + > + ret = __sev_mirror_create(vm_no_vcpu->fd, vm_with_vcpu->fd); > + TEST_ASSERT(ret == -1 && errno == EINVAL, > + "Copy context requires SEV enabled. ret %d, errno: %d\n", ret, > + errno); > + > + ret = __sev_mirror_create(vm_with_vcpu->fd, sev_vm->fd); > + TEST_ASSERT( > + ret == -1 && errno == EINVAL, > + "SEV copy context requires no vCPUS on the destination. ret: %d, errno: %d\n", > + ret, errno); > + > + kvm_vm_free(sev_vm); > + kvm_vm_free(sev_es_vm); > + kvm_vm_free(vm_with_vcpu); > + kvm_vm_free(vm_no_vcpu); > +} > + > int main(int argc, char *argv[]) > { > - test_sev_migrate_from(/* es= */ false); > - test_sev_migrate_from(/* es= */ true); > - test_sev_migrate_locking(); > - test_sev_migrate_parameters(); > + if (kvm_check_cap(KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM)) { > + test_sev_migrate_from(/* es= */ false); > + test_sev_migrate_from(/* es= */ true); > + test_sev_migrate_locking(); > + test_sev_migrate_parameters(); > + } > + if (kvm_check_cap(KVM_CAP_VM_COPY_ENC_CONTEXT_FROM)) { > + test_sev_mirror(/* es= */ false); > + test_sev_mirror(/* es= */ true); > + test_sev_mirror_parameters(); > + } > return 0; > } > -- > 2.27.0 > >
On Wed, Dec 1, 2021 at 11:09 AM Peter Gonda <pgonda@google.com> wrote: > > On Mon, Nov 22, 2021 at 5:50 PM Paolo Bonzini <pbonzini@redhat.com> wrote: > > > > I am putting the tests in sev_migrate_tests because the failure conditions are > > very similar and some of the setup code can be reused, too. > > > > The tests cover both successful creation of a mirror VM, and error > > conditions. > > > > Cc: Peter Gonda <pgonda@google.com> > > Cc: Sean Christopherson <seanjc@google.com> > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > > --- > > .../selftests/kvm/x86_64/sev_migrate_tests.c | 112 ++++++++++++++++-- > > 1 file changed, 105 insertions(+), 7 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 0cd7e2eaa895..d265cea5de85 100644 > > --- a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > > +++ b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c > > @@ -54,12 +54,15 @@ static struct kvm_vm *sev_vm_create(bool es) > > return vm; > > } > > > > -static struct kvm_vm *__vm_create(void) > > +static struct kvm_vm *aux_vm_create(bool with_vcpus) > > { > > struct kvm_vm *vm; > > int i; > > > > vm = vm_create(VM_MODE_DEFAULT, 0, O_RDWR); > > + if (!with_vcpus) > > + return vm; > > + > > for (i = 0; i < NR_MIGRATE_TEST_VCPUS; ++i) > > vm_vcpu_add(vm, i); > > > > @@ -93,7 +96,7 @@ static void test_sev_migrate_from(bool es) > > > > src_vm = sev_vm_create(es); > > for (i = 0; i < NR_MIGRATE_TEST_VMS; ++i) > > - dst_vms[i] = __vm_create(); > > + dst_vms[i] = aux_vm_create(true); > > > > /* Initial migration from the src to the first dst. */ > > sev_migrate_from(dst_vms[0]->fd, src_vm->fd); > > @@ -162,7 +165,7 @@ static void test_sev_migrate_parameters(void) > > sev_vm = sev_vm_create(/* es= */ false); > > sev_es_vm = sev_vm_create(/* es= */ true); > > vm_no_vcpu = vm_create(VM_MODE_DEFAULT, 0, O_RDWR); > > - vm_no_sev = __vm_create(); > > + vm_no_sev = aux_vm_create(true); > > sev_es_vm_no_vmsa = vm_create(VM_MODE_DEFAULT, 0, O_RDWR); > > sev_ioctl(sev_es_vm_no_vmsa->fd, KVM_SEV_ES_INIT, NULL); > > vm_vcpu_add(sev_es_vm_no_vmsa, 1); > > @@ -203,11 +206,106 @@ static void test_sev_migrate_parameters(void) > > kvm_vm_free(vm_no_sev); > > } > > > > +static int __sev_mirror_create(int dst_fd, int src_fd) > > +{ > > + struct kvm_enable_cap cap = { > > + .cap = KVM_CAP_VM_COPY_ENC_CONTEXT_FROM, > > + .args = { src_fd } > > + }; > > + > > + return ioctl(dst_fd, KVM_ENABLE_CAP, &cap); > > +} > > + > > + > > +static void sev_mirror_create(int dst_fd, int src_fd) > > +{ > > + int ret; > > + > > + ret = __sev_mirror_create(dst_fd, src_fd); > > + TEST_ASSERT(!ret, "Copying context failed, ret: %d, errno: %d\n", ret, errno); > > +} > > + > > +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); > > + dst_vm = aux_vm_create(false); > > + > > + sev_mirror_create(dst_vm->fd, src_vm->fd); > > + > > + /* 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); > > I don't think this should be called on the mirror and I think it > should be an error. > > In is_cmd_allowed_from_mirror() KVM_SEV_LAUNCH_START should not be allowed: > > if (cmd_id == KVM_SEV_LAUNCH_UPDATE_VMSA || > cmd_id == KVM_SEV_GUEST_STATUS || cmd_id == KVM_SEV_DBG_DECRYPT || > cmd_id == KVM_SEV_DBG_ENCRYPT) > return true; > > This overrides the mirrored values and sets up the VM as a new SEV > context. I would have thought the sev_bind_asid() in > sev_launch_start() would fail because the asid is already used by the > source. Since you already queue'd this I sent another patch to fix the issue with sev_ioctl() and remove this call. > > > + if (es) > > + sev_ioctl(dst_vm->fd, KVM_SEV_LAUNCH_UPDATE_VMSA, NULL); > > + > > + kvm_vm_free(src_vm); > > + kvm_vm_free(dst_vm); > > +} > > + > > +static void test_sev_mirror_parameters(void) > > +{ > > + struct kvm_vm *sev_vm, *sev_es_vm, *vm_no_vcpu, *vm_with_vcpu; > > + int ret; > > + > > + sev_vm = sev_vm_create(/* es= */ false); > > + sev_es_vm = sev_vm_create(/* es= */ true); > > + vm_with_vcpu = aux_vm_create(true); > > + vm_no_vcpu = aux_vm_create(false); > > + > > + ret = __sev_mirror_create(sev_vm->fd, sev_vm->fd); > > + TEST_ASSERT( > > + ret == -1 && errno == EINVAL, > > + "Should not be able copy context to self. ret: %d, errno: %d\n", > > + ret, errno); > > + > > + ret = __sev_mirror_create(sev_vm->fd, sev_es_vm->fd); > > + TEST_ASSERT( > > + ret == -1 && errno == EINVAL, > > + "Should not be able copy context to SEV enabled VM. ret: %d, errno: %d\n", > > + ret, errno); > > + > > + ret = __sev_mirror_create(sev_es_vm->fd, sev_vm->fd); > > + TEST_ASSERT( > > + ret == -1 && errno == EINVAL, > > + "Should not be able copy context to SEV-ES enabled VM. ret: %d, errno: %d\n", > > + ret, errno); > > + > > + ret = __sev_mirror_create(vm_no_vcpu->fd, vm_with_vcpu->fd); > > + TEST_ASSERT(ret == -1 && errno == EINVAL, > > + "Copy context requires SEV enabled. ret %d, errno: %d\n", ret, > > + errno); > > + > > + ret = __sev_mirror_create(vm_with_vcpu->fd, sev_vm->fd); > > + TEST_ASSERT( > > + ret == -1 && errno == EINVAL, > > + "SEV copy context requires no vCPUS on the destination. ret: %d, errno: %d\n", > > + ret, errno); > > + > > + kvm_vm_free(sev_vm); > > + kvm_vm_free(sev_es_vm); > > + kvm_vm_free(vm_with_vcpu); > > + kvm_vm_free(vm_no_vcpu); > > +} > > + > > int main(int argc, char *argv[]) > > { > > - test_sev_migrate_from(/* es= */ false); > > - test_sev_migrate_from(/* es= */ true); > > - test_sev_migrate_locking(); > > - test_sev_migrate_parameters(); > > + if (kvm_check_cap(KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM)) { > > + test_sev_migrate_from(/* es= */ false); > > + test_sev_migrate_from(/* es= */ true); > > + test_sev_migrate_locking(); > > + test_sev_migrate_parameters(); > > + } > > + if (kvm_check_cap(KVM_CAP_VM_COPY_ENC_CONTEXT_FROM)) { > > + test_sev_mirror(/* es= */ false); > > + test_sev_mirror(/* es= */ true); > > + test_sev_mirror_parameters(); > > + } > > return 0; > > } > > -- > > 2.27.0 > > > >
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 0cd7e2eaa895..d265cea5de85 100644 --- a/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c +++ b/tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c @@ -54,12 +54,15 @@ static struct kvm_vm *sev_vm_create(bool es) return vm; } -static struct kvm_vm *__vm_create(void) +static struct kvm_vm *aux_vm_create(bool with_vcpus) { struct kvm_vm *vm; int i; vm = vm_create(VM_MODE_DEFAULT, 0, O_RDWR); + if (!with_vcpus) + return vm; + for (i = 0; i < NR_MIGRATE_TEST_VCPUS; ++i) vm_vcpu_add(vm, i); @@ -93,7 +96,7 @@ static void test_sev_migrate_from(bool es) src_vm = sev_vm_create(es); for (i = 0; i < NR_MIGRATE_TEST_VMS; ++i) - dst_vms[i] = __vm_create(); + dst_vms[i] = aux_vm_create(true); /* Initial migration from the src to the first dst. */ sev_migrate_from(dst_vms[0]->fd, src_vm->fd); @@ -162,7 +165,7 @@ static void test_sev_migrate_parameters(void) sev_vm = sev_vm_create(/* es= */ false); sev_es_vm = sev_vm_create(/* es= */ true); vm_no_vcpu = vm_create(VM_MODE_DEFAULT, 0, O_RDWR); - vm_no_sev = __vm_create(); + vm_no_sev = aux_vm_create(true); sev_es_vm_no_vmsa = vm_create(VM_MODE_DEFAULT, 0, O_RDWR); sev_ioctl(sev_es_vm_no_vmsa->fd, KVM_SEV_ES_INIT, NULL); vm_vcpu_add(sev_es_vm_no_vmsa, 1); @@ -203,11 +206,106 @@ static void test_sev_migrate_parameters(void) kvm_vm_free(vm_no_sev); } +static int __sev_mirror_create(int dst_fd, int src_fd) +{ + struct kvm_enable_cap cap = { + .cap = KVM_CAP_VM_COPY_ENC_CONTEXT_FROM, + .args = { src_fd } + }; + + return ioctl(dst_fd, KVM_ENABLE_CAP, &cap); +} + + +static void sev_mirror_create(int dst_fd, int src_fd) +{ + int ret; + + ret = __sev_mirror_create(dst_fd, src_fd); + TEST_ASSERT(!ret, "Copying context failed, ret: %d, errno: %d\n", ret, errno); +} + +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); + dst_vm = aux_vm_create(false); + + sev_mirror_create(dst_vm->fd, src_vm->fd); + + /* 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); + + kvm_vm_free(src_vm); + kvm_vm_free(dst_vm); +} + +static void test_sev_mirror_parameters(void) +{ + struct kvm_vm *sev_vm, *sev_es_vm, *vm_no_vcpu, *vm_with_vcpu; + int ret; + + sev_vm = sev_vm_create(/* es= */ false); + sev_es_vm = sev_vm_create(/* es= */ true); + vm_with_vcpu = aux_vm_create(true); + vm_no_vcpu = aux_vm_create(false); + + ret = __sev_mirror_create(sev_vm->fd, sev_vm->fd); + TEST_ASSERT( + ret == -1 && errno == EINVAL, + "Should not be able copy context to self. ret: %d, errno: %d\n", + ret, errno); + + ret = __sev_mirror_create(sev_vm->fd, sev_es_vm->fd); + TEST_ASSERT( + ret == -1 && errno == EINVAL, + "Should not be able copy context to SEV enabled VM. ret: %d, errno: %d\n", + ret, errno); + + ret = __sev_mirror_create(sev_es_vm->fd, sev_vm->fd); + TEST_ASSERT( + ret == -1 && errno == EINVAL, + "Should not be able copy context to SEV-ES enabled VM. ret: %d, errno: %d\n", + ret, errno); + + ret = __sev_mirror_create(vm_no_vcpu->fd, vm_with_vcpu->fd); + TEST_ASSERT(ret == -1 && errno == EINVAL, + "Copy context requires SEV enabled. ret %d, errno: %d\n", ret, + errno); + + ret = __sev_mirror_create(vm_with_vcpu->fd, sev_vm->fd); + TEST_ASSERT( + ret == -1 && errno == EINVAL, + "SEV copy context requires no vCPUS on the destination. ret: %d, errno: %d\n", + ret, errno); + + kvm_vm_free(sev_vm); + kvm_vm_free(sev_es_vm); + kvm_vm_free(vm_with_vcpu); + kvm_vm_free(vm_no_vcpu); +} + int main(int argc, char *argv[]) { - test_sev_migrate_from(/* es= */ false); - test_sev_migrate_from(/* es= */ true); - test_sev_migrate_locking(); - test_sev_migrate_parameters(); + if (kvm_check_cap(KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM)) { + test_sev_migrate_from(/* es= */ false); + test_sev_migrate_from(/* es= */ true); + test_sev_migrate_locking(); + test_sev_migrate_parameters(); + } + if (kvm_check_cap(KVM_CAP_VM_COPY_ENC_CONTEXT_FROM)) { + test_sev_mirror(/* es= */ false); + test_sev_mirror(/* es= */ true); + test_sev_mirror_parameters(); + } return 0; }
I am putting the tests in sev_migrate_tests because the failure conditions are very similar and some of the setup code can be reused, too. The tests cover both successful creation of a mirror VM, and error conditions. Cc: Peter Gonda <pgonda@google.com> Cc: Sean Christopherson <seanjc@google.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- .../selftests/kvm/x86_64/sev_migrate_tests.c | 112 ++++++++++++++++-- 1 file changed, 105 insertions(+), 7 deletions(-)