lkml.org 
[lkml]   [2021]   [Dec]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH 10/12] KVM: SEV: Prohibit migration of a VM that has mirrors
From
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

\
 
 \ /
  Last update: 2021-12-01 19:24    [W:0.093 / U:0.156 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site