Commit 17d44a96 authored by Paolo Bonzini's avatar Paolo Bonzini
Browse files

KVM: SEV: Prohibit migration of a VM that has mirrors 



VMs that mirror an encryption context rely on the owner to keep the
ASID allocated.  Performing a KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM
would cause a dangling ASID:

1. copy context from A to B (gets ref to A)
2. move context from A to L (moves ASID from A to L)
3. close L (releases ASID from L, B still references it)

The right way to do the handoff instead is to create a fresh mirror VM
on the destination first:

1. copy context from A to B (gets ref to A)
[later] 2. close B (releases ref to A)
3. move context from A to L (moves ASID from A to L)
4. copy context from L to M

So, catch the situation by adding a count of how many VMs are
mirroring this one's encryption context.

Fixes: 0b020f5a ("KVM: SEV: Add support for SEV-ES intra host migration")
Message-Id: <20211123005036.2954379-11-pbonzini@redhat.com>
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent bf42b02b

arch/x86/kvm/svm/sev.c

+21 −1
Original line number Diff line number Diff line
@@ -1696,6 +1696,16 @@ int svm_vm_migrate_from(struct kvm *kvm, unsigned int source_fd) 
	}



	src_sev = &to_kvm_svm(source_kvm)->sev_info;



	/*

	 * VMs mirroring src's encryption context rely on it to keep the

	 * ASID allocated, but below we are clearing src_sev->asid.

	 */

	if (src_sev->num_mirrored_vms) {

		ret = -EBUSY;

		goto out_unlock;

	}



	dst_sev->misc_cg = get_current_misc_cg();

	cg_cleanup_sev = dst_sev;

	if (dst_sev->misc_cg != src_sev->misc_cg) {
@@ -1987,6 +1997,7 @@ int svm_vm_copy_asid_from(struct kvm *kvm, unsigned int source_fd) 
	 */

	source_sev = &to_kvm_svm(source_kvm)->sev_info;

	kvm_get_kvm(source_kvm);

	source_sev->num_mirrored_vms++;



	/* Set enc_context_owner and copy its encryption context over */

	mirror_sev = &to_kvm_svm(kvm)->sev_info;
@@ -2019,12 +2030,21 @@ void sev_vm_destroy(struct kvm *kvm) 
	struct list_head *head = &sev->regions_list;

	struct list_head *pos, *q;



	WARN_ON(sev->num_mirrored_vms);



	if (!sev_guest(kvm))

		return;



	/* If this is a mirror_kvm release the enc_context_owner and skip sev cleanup */

	if (is_mirroring_enc_context(kvm)) {

		kvm_put_kvm(sev->enc_context_owner);

		struct kvm *owner_kvm = sev->enc_context_owner;

		struct kvm_sev_info *owner_sev = &to_kvm_svm(owner_kvm)->sev_info;



		mutex_lock(&owner_kvm->lock);

		if (!WARN_ON(!owner_sev->num_mirrored_vms))

			owner_sev->num_mirrored_vms--;

		mutex_unlock(&owner_kvm->lock);

		kvm_put_kvm(owner_kvm);

		return;

	}

arch/x86/kvm/svm/svm.h

+1 −0
Original line number Diff line number Diff line
@@ -79,6 +79,7 @@ struct kvm_sev_info { 
	struct list_head regions_list;  /* List of registered regions */

	u64 ap_jump_table;	/* SEV-ES AP Jump Table address */

	struct kvm *enc_context_owner; /* Owner of copied encryption context */

	unsigned long num_mirrored_vms; /* Number of VMs sharing this ASID */

	struct misc_cg *misc_cg; /* For misc cgroup accounting */

	atomic_t migration_in_progress;

};

tools/testing/selftests/kvm/x86_64/sev_migrate_tests.c

+37 −0
Original line number Diff line number Diff line
@@ -294,6 +294,41 @@ static void test_sev_mirror_parameters(void) 
	kvm_vm_free(vm_no_vcpu);

}



static void test_sev_move_copy(void)

{

	struct kvm_vm *dst_vm, *sev_vm, *mirror_vm, *dst_mirror_vm;

	int ret;



	sev_vm = sev_vm_create(/* es= */ false);

	dst_vm = aux_vm_create(true);

	mirror_vm = aux_vm_create(false);

	dst_mirror_vm = aux_vm_create(false);



	sev_mirror_create(mirror_vm->fd, sev_vm->fd);

	ret = __sev_migrate_from(dst_vm->fd, sev_vm->fd);

	TEST_ASSERT(ret == -1 && errno == EBUSY,

		    "Cannot migrate VM that has mirrors. ret %d, errno: %d\n", ret,

		    errno);



	/* The mirror itself can be migrated.  */

	sev_migrate_from(dst_mirror_vm->fd, mirror_vm->fd);

	ret = __sev_migrate_from(dst_vm->fd, sev_vm->fd);

	TEST_ASSERT(ret == -1 && errno == EBUSY,

		    "Cannot migrate VM that has mirrors. ret %d, errno: %d\n", ret,

		    errno);



	/*

	 * mirror_vm is not a mirror anymore, dst_mirror_vm is.  Thus,

	 * the owner can be copied as soon as dst_mirror_vm is gone.

	 */

	kvm_vm_free(dst_mirror_vm);

	sev_migrate_from(dst_vm->fd, sev_vm->fd);



	kvm_vm_free(mirror_vm);

	kvm_vm_free(dst_vm);

	kvm_vm_free(sev_vm);

}



int main(int argc, char *argv[])

{

	if (kvm_check_cap(KVM_CAP_VM_MOVE_ENC_CONTEXT_FROM)) {
@@ -301,6 +336,8 @@ int main(int argc, char *argv[]) 
		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_move_copy();

	}

	if (kvm_check_cap(KVM_CAP_VM_COPY_ENC_CONTEXT_FROM)) {

		test_sev_mirror(/* es= */ false);