On Tue, Sep 14, 2021 at 9:47 AM Peter Gonda [off-list ref] wrote:

I should've suggested this in my original review. But is it worth
moving `sev_vm_create()` and `sev_ioctl()` into the broader selftests
library, so others can leverage this function to write selftests?

+
+static struct kvm_vm *__vm_create(void)
+{
+       struct kvm_vm *vm;
+       int i;
+
+       vm = vm_create(VM_MODE_DEFAULT, 0, O_RDWR);
+       for (i = 0; i < NR_MIGRATE_TEST_VCPUS; ++i)
+               vm_vcpu_add(vm, i);
+
+       return vm;
+}
+
+static int __sev_migrate_from(int dst_fd, int src_fd)
+{
+       struct kvm_enable_cap cap = {
+               .cap = KVM_CAP_VM_MIGRATE_PROTECTED_VM_FROM,
+               .args = { src_fd }
+       };
+
+       return ioctl(dst_fd, KVM_ENABLE_CAP, &cap);
+}
+
+
+static void sev_migrate_from(int dst_fd, int src_fd)
+{
+       int ret;
+
+       ret = __sev_migrate_from(dst_fd, src_fd);
+       TEST_ASSERT(!ret, "Migration failed, ret: %d, errno: %d\n", ret, errno);
+}
+
+static void test_sev_migrate_from(bool es)
+{
+       struct kvm_vm *src_vm;
+       struct kvm_vm *dst_vms[NR_MIGRATE_TEST_VMS];
+       int i;
+
+       src_vm = sev_vm_create(es);
+       for (i = 0; i < NR_MIGRATE_TEST_VMS; ++i)
+               dst_vms[i] = __vm_create();
+
+       /* Initial migration from the src to the first dst. */
+       sev_migrate_from(dst_vms[0]->fd, src_vm->fd);
+
+       for (i = 1; i < NR_MIGRATE_TEST_VMS; i++)
+               sev_migrate_from(dst_vms[i]->fd, dst_vms[i - 1]->fd);
+
+       /* Migrate the guest back to the original VM. */
+       sev_migrate_from(src_vm->fd, dst_vms[NR_MIGRATE_TEST_VMS - 1]->fd);
+
+       kvm_vm_free(src_vm);
+       for (i = 0; i < NR_MIGRATE_TEST_VMS; ++i)
+               kvm_vm_free(dst_vms[i]);
+}
+
+struct locking_thread_input {
+       struct kvm_vm *vm;
+       int source_fds[NR_LOCK_TESTING_THREADS];
+};
+
+static void *locking_test_thread(void *arg)
+{
+       int i, j;
+       struct locking_thread_input *input = (struct locking_test_thread *)arg;
+
+       for (i = 0; i < NR_LOCK_TESTING_ITERATIONS; ++i) {
+               j = i % NR_LOCK_TESTING_THREADS;
+               __sev_migrate_from(input->vm->fd, input->source_fds[j]);
+       }
+
+       return NULL;
+}
+
+static void test_sev_migrate_locking(void)
+{
+       struct locking_thread_input input[NR_LOCK_TESTING_THREADS];
+       pthread_t pt[NR_LOCK_TESTING_THREADS];
+       int i;
+
+       for (i = 0; i < NR_LOCK_TESTING_THREADS; ++i) {
+               input[i].vm = sev_vm_create(/* es= */ false);
+               input[0].source_fds[i] = input[i].vm->fd;
+       }
+       for (i = 1; i < NR_LOCK_TESTING_THREADS; ++i)
+               memcpy(input[i].source_fds, input[0].source_fds,
+                      sizeof(input[i].source_fds));
+
+       for (i = 0; i < NR_LOCK_TESTING_THREADS; ++i)
+               pthread_create(&pt[i], NULL, locking_test_thread, &input[i]);
+
+       for (i = 0; i < NR_LOCK_TESTING_THREADS; ++i)
+               pthread_join(pt[i], NULL);
+}
+
+static void test_sev_migrate_parameters(void)
+{
+       struct kvm_vm *sev_vm, *sev_es_vm, *vm_no_vcpu, *vm_no_sev,
+               *sev_es_vm_no_vmsa;
+       int ret;
+
+       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();
+       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);
+
+
+       ret = __sev_migrate_from(sev_vm->fd, sev_es_vm->fd);
+       TEST_ASSERT(
+               ret == -1 && errno == EINVAL,
+               "Should not be able migrate to SEV enabled VM. ret: %d, errno: %d\n",
+               ret, errno);
+
+       ret = __sev_migrate_from(sev_es_vm->fd, sev_vm->fd);
+       TEST_ASSERT(
+               ret == -1 && errno == EINVAL,
+               "Should not be able migrate to SEV-ES enabled VM. ret: %d, errno: %d\n",
+               ret, errno);
+
+       ret = __sev_migrate_from(vm_no_vcpu->fd, sev_es_vm->fd);
+       TEST_ASSERT(
+               ret == -1 && errno == EINVAL,
+               "SEV-ES migrations require same number of vCPUS. ret: %d, errno: %d\n",
+               ret, errno);

How do we know that this failed because `vm_no_vcpu` has no vCPUs or
because it's not a SEV-ES VM?

+
+       ret = __sev_migrate_from(vm_no_vcpu->fd, sev_es_vm_no_vmsa->fd);
+       TEST_ASSERT(
+               ret == -1 && errno == EINVAL,
+               "SEV-ES migrations require UPDATE_VMSA. ret %d, errno: %d\n",
+               ret, errno);

Same question. How do we know why this failed? `sev_es_vm_no_vmsa` did
not have any vCPUs added. Would it be cleaner to add an additional
param to `sev_vm_create()` to skip calling UPDATE_VMSA? Then,
`sev_es_vm_no_vmsa` can be created from `sev_vm_create()` and it's
obvious to the read that the VMs are identical except for this aspect.

+
+       ret = __sev_migrate_from(vm_no_vcpu->fd, vm_no_sev->fd);
+       TEST_ASSERT(ret == -1 && errno == EINVAL,
+                   "Migrations require SEV enabled. ret %d, errno: %d\n", ret,
+                   errno);

`vm_no_sev` has vCPUs. Therefore, how do we know why this failed --
(a) differing vCPU counts or (b) no SEV?

+}
+
+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();
+       return 0;
+}
--
2.33.0.309.g3052b89438-goog