From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] KVM portability split : move mmu code to arch Date: Sun, 18 Nov 2007 13:31:28 +0200 Message-ID: <47402290.2030604@qumranet.com> References: <42DFA526FC41B1429CE7279EF83C6BDC9A064F@pdsmsx415.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, carsteno-tA70FqPdS9bQT0dZR+AlfA@public.gmane.org, Hollis Blanchard To: "Zhang, Xiantao" Return-path: In-Reply-To: <42DFA526FC41B1429CE7279EF83C6BDC9A064F-wq7ZOvIWXbMAbVU2wMM1CrfspsVTdybXVpNB7YpNyf8@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org Zhang, Xiantao wrote: > According to privious decisions, mmu code should be moved to arch. > > >From 43a629932b6babcbf2af75299192c1d77c2393d5 Mon Sep 17 00:00:00 2001 > From: Zhang Xiantao > Date: Fri, 16 Nov 2007 13:16:40 +0800 > Subject: [PATCH] KVM: MMU split for portability. > Since mmu code is very arch-denpendent, move them to archs. > 1. For kvm_vm_ioctl_get_dirty_log, this ioctl should be first transfered > to arch-specific code, and will > call the common code implemtend in kvm_get_dirty_log to perform common > task. > 2. For set_memory_region, I moved the mmu-speicific operations to an > arch-ops. > Signed-off-by: Zhang Xiantao > --- > drivers/kvm/kvm.h | 8 ++++ > drivers/kvm/kvm_main.c | 49 +++------------------------ > drivers/kvm/x86.c | 85 > +++++++++++++++++++++++++++++++++++++++++++++--- > 3 files changed, 94 insertions(+), 48 deletions(-) > > diff --git a/drivers/kvm/kvm_main.c b/drivers/kvm/kvm_main.c > index 0414e92..84eec47 100644 > --- a/drivers/kvm/kvm_main.c > +++ b/drivers/kvm/kvm_main.c > @@ -362,29 +362,9 @@ int __kvm_set_memory_region(struct kvm *kvm, > if (mem->slot >= kvm->nmemslots) > kvm->nmemslots = mem->slot + 1; > > - if (!kvm->n_requested_mmu_pages) { > - unsigned int n_pages; > - > - if (npages) { > - n_pages = npages * KVM_PERMILLE_MMU_PAGES / > 1000; > - kvm_mmu_change_mmu_pages(kvm, > kvm->n_alloc_mmu_pages + > - n_pages); > - } else { > - unsigned int nr_mmu_pages; > - > - n_pages = old.npages * KVM_PERMILLE_MMU_PAGES / > We rely on the values in 'old' here... > 1000; > - nr_mmu_pages = kvm->n_alloc_mmu_pages - n_pages; > - nr_mmu_pages = max(nr_mmu_pages, > - (unsigned int) > KVM_MIN_ALLOC_MMU_PAGES); > - kvm_mmu_change_mmu_pages(kvm, nr_mmu_pages); > - } > - } > > *memslot = new; > > - kvm_mmu_slot_remove_write_access(kvm, mem->slot); > - kvm_flush_remote_tlbs(kvm); > - > kvm_free_physmem_slot(&old, &new); > return 0; > > @@ -404,6 +384,8 @@ int kvm_set_memory_region(struct kvm *kvm, > > mutex_lock(&kvm->lock); > r = __kvm_set_memory_region(kvm, mem, user_alloc); > + if (r == 0) > + kvm_arch_set_memory_region(kvm, mem, user_alloc); > But 'old' does not longer exist here. You recalculate it from kvm->memslots[] but the value is not longer identical to the 'old' value. Suggest changing this code not to rely on 'old' at all; instead recalculate nr_mmu_pages from kvm->memslots[] each time. That will make the code more robust, enabling the movement later. -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2005. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/