From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [patch 3/4] KVM: introduce kvm_arch_can_free_memslot, disallow slot deletion if cached cr3 Date: Sun, 10 May 2009 19:40:16 +0300 Message-ID: <4A070370.5000501@redhat.com> References: <20090507210331.370806285@amt.cnet> <20090507210534.058747069@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: mtosatti@redhat.com Return-path: Received: from mx2.redhat.com ([66.187.237.31]:35086 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751634AbZEJQlR (ORCPT ); Sun, 10 May 2009 12:41:17 -0400 Received: from int-mx2.corp.redhat.com (int-mx2.corp.redhat.com [172.16.27.26]) by mx2.redhat.com (8.13.8/8.13.8) with ESMTP id n4AGfIJM027478 for ; Sun, 10 May 2009 12:41:18 -0400 In-Reply-To: <20090507210534.058747069@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: mtosatti@redhat.com wrote: > Disallow the deletion of memory slots (and aliases, for x86 case), if a > vcpu contains a cr3 that points to such slot/alias. > > This complements commit 6c20e1442bb1c62914bb85b7f4a38973d2a423ba. > > v2: > - set KVM_REQ_TRIPLE_FAULT > - use __KVM_HAVE_ARCH_CAN_FREE_MEMSLOT to avoid duplication of stub > > Signed-off-by: Marcelo Tosatti > > Index: kvm-pending/arch/x86/kvm/x86.c > =================================================================== > --- kvm-pending.orig/arch/x86/kvm/x86.c > +++ kvm-pending/arch/x86/kvm/x86.c > @@ -1636,6 +1636,29 @@ gfn_t unalias_gfn(struct kvm *kvm, gfn_t > return gfn; > } > > +static int kvm_root_gfn_in_range(struct kvm *kvm, gfn_t base_gfn, > + gfn_t end_gfn, bool unalias) > +{ > + struct kvm_vcpu *vcpu; > + gfn_t root_gfn; > + int i; > + > + for (i = 0; i < KVM_MAX_VCPUS; ++i) { > + vcpu = kvm->vcpus[i]; > + if (!vcpu) > + continue; > + root_gfn = vcpu->arch.cr3 >> PAGE_SHIFT; > The guest may have changed this by now. > + if (unalias) > + root_gfn = unalias_gfn(kvm, root_gfn); > + if (root_gfn >= base_gfn && root_gfn <= end_gfn) { > + set_bit(KVM_REQ_TRIPLE_FAULT, &vcpu->requests); > + return 1; > + } > + } > + > + return 0; > +} > + > The naming is bad, a function named as a predicate shouldn't have side effects. Also, we should allow deleting the slot. There's no reason to deny userspace something just because the guest is playing around I think this should be enough: - take mmu lock - request an mmu reload from all vcpus - drop the slot - release mmu lock The reload will inject a #GP if cr3 is now out of bounds, should be changed to triple fault, but everything is in place (set_cr3 already checks). -- error compiling committee.c: too many arguments to function