Messages in this thread | | | Date | Wed, 1 Dec 2021 19:21:56 +0100 | Subject | Re: [PATCH 10/12] KVM: SEV: Prohibit migration of a VM that has mirrors | From | Paolo Bonzini <> |
| |
On 12/1/21 19:17, Peter Gonda wrote: > On Mon, Nov 22, 2021 at 5:50 PM Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> 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: 0b020f5af092 ("KVM: SEV: Add support for SEV-ES intra host migration") >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> arch/x86/kvm/svm/sev.c | 22 ++++++++++- >> arch/x86/kvm/svm/svm.h | 1 + >> .../selftests/kvm/x86_64/sev_migrate_tests.c | 37 +++++++++++++++++++ >> 3 files changed, 59 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c >> index 025d9731b66c..89a716290fac 100644 >> --- a/arch/x86/kvm/svm/sev.c >> +++ b/arch/x86/kvm/svm/sev.c >> @@ -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 we don't change to atomic doesn't this need to happen when we have > the kvm->lock?
Hi,
there can be no concurrency when destroying a VM. (If you do Rust, it's similar to using x.get_mut().get_mut() on an Arc<Mutex<T>>).
Paolo
| |