From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrea Arcangeli Subject: [PATCH 3/6] kvm.git allow browsing memslots with mmu_lock Date: Fri, 25 Jul 2008 16:32:03 +0200 Message-ID: <20080725143203.GC8217@duo.random> References: <20080725142452.GA8217@duo.random> <20080725142639.GB8217@duo.random> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Marcelo Tosatti , kvm@vger.kernel.org To: Avi Kivity Return-path: Received: from host36-195-149-62.serverdedicati.aruba.it ([62.149.195.36]:37963 "EHLO mx.cpushare.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751146AbYGYOcE (ORCPT ); Fri, 25 Jul 2008 10:32:04 -0400 Content-Disposition: inline In-Reply-To: <20080725142639.GB8217@duo.random> Sender: kvm-owner@vger.kernel.org List-ID: This allows reading memslots with only the mmu_lock hold for mmu notifiers that runs in atomic context and with mmu_lock held. Compared to previous versions I had to move this: - if (mem->slot >= kvm->nmemslots) - kvm->nmemslots = mem->slot + 1; after the call to kvm_arch_flush_shadow but that's ok as they're mutually exclusive: npages being null asks to remove the memslot and you can't remove a memslot that doesn't exist yet even if there's no explicit check that forbids it. In any case the ordering didn't seem meaningful if slots is set higher than nmemslots. But please Marcelo double check, thanks! Signed-off-by: Andrea Arcangeli diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index f7726a0..cf04489 100644 --- a/arch/x86/kvm/x86.c +++ b/arch/x86/kvm/x86.c @@ -3999,16 +3999,23 @@ int kvm_arch_set_memory_region(struct kvm *kvm, */ if (!user_alloc) { if (npages && !old.rmap) { + unsigned long userspace_addr; + down_write(¤t->mm->mmap_sem); - memslot->userspace_addr = do_mmap(NULL, 0, - npages * PAGE_SIZE, - PROT_READ | PROT_WRITE, - MAP_SHARED | MAP_ANONYMOUS, - 0); + userspace_addr = do_mmap(NULL, 0, + npages * PAGE_SIZE, + PROT_READ | PROT_WRITE, + MAP_SHARED | MAP_ANONYMOUS, + 0); up_write(¤t->mm->mmap_sem); - if (IS_ERR((void *)memslot->userspace_addr)) - return PTR_ERR((void *)memslot->userspace_addr); + if (IS_ERR((void *)userspace_addr)) + return PTR_ERR((void *)userspace_addr); + + /* set userspace_addr atomically for kvm_hva_to_rmapp */ + spin_lock(&kvm->mmu_lock); + memslot->userspace_addr = userspace_addr; + spin_unlock(&kvm->mmu_lock); } else { if (!old.user_alloc && old.rmap) { int ret; diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 904d7b7..132a26c 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -375,7 +375,15 @@ int __kvm_set_memory_region(struct kvm *kvm, memset(new.rmap, 0, npages * sizeof(*new.rmap)); new.user_alloc = user_alloc; - new.userspace_addr = mem->userspace_addr; + /* + * hva_to_rmmap() serialzies with the mmu_lock and to be + * safe it has to ignore memslots with !user_alloc && + * !userspace_addr. + */ + if (user_alloc) + new.userspace_addr = mem->userspace_addr; + else + new.userspace_addr = 0; } if (npages && !new.lpage_info) { int largepages = npages / KVM_PAGES_PER_HPAGE; @@ -408,17 +416,21 @@ int __kvm_set_memory_region(struct kvm *kvm, } #endif /* not defined CONFIG_S390 */ - if (mem->slot >= kvm->nmemslots) - kvm->nmemslots = mem->slot + 1; - if (!npages) kvm_arch_flush_shadow(kvm); + spin_lock(&kvm->mmu_lock); + if (mem->slot >= kvm->nmemslots) + kvm->nmemslots = mem->slot + 1; + *memslot = new; + spin_unlock(&kvm->mmu_lock); r = kvm_arch_set_memory_region(kvm, mem, old, user_alloc); if (r) { + spin_lock(&kvm->mmu_lock); *memslot = old; + spin_unlock(&kvm->mmu_lock); goto out_free; }