From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [patch] kvm: make cr3 loading more robust Date: Wed, 03 Jan 2007 14:06:52 +0200 Message-ID: <459B9C5C.9060008@qumranet.com> References: <20070103021057.GA11316@elte.hu> <459B695C.5090004@qumranet.com> <20070103115911.GA2786@elte.hu> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel Return-path: To: Ingo Molnar In-Reply-To: <20070103115911.GA2786-X9Un+BFzKDI@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 Ingo Molnar wrote: > * Avi Kivity wrote: > > >>> [ Avi: this patch needs kvm_mmu_get_page() NULL return to not be ignored >>> by mmu_alloc_roots()/nonpaging_init_context()/paging_new_cr3()/etc. >>> and passed further down - i assume you are working on those >>> issues already, or should i fix those too? ] >>> >> kvm_mmu_get_page() cannot return NULL (before this patch), as we >> ensure there are free pages before entry to page_fault. >> > > hm, where do we ensure that? kvm_mmu_get_page() calls > kvm_mmu_alloc_page(), which might return NULL: > > static struct kvm_mmu_page *kvm_mmu_alloc_page(struct kvm_vcpu *vcpu, > u64 *parent_pte) > { > struct kvm_mmu_page *page; > > if (list_empty(&vcpu->free_pages)) > return NULL; > > and i dont see where we ensure that ->free_pages is not empty. The > relevant callchain would be kvm_vmx_return() -> handle_cr() -> set_cr3() > -> paging_new_cr3() -> mmu_alloc_roots() -> kvm_mmu_get_page(). > > is there some accounting that makes a NULL pointer return impossible? > The only one i can see is the direct quering of vcpu->free_pages or the > querying of n_free_mmu_pages, but neither seems to be done in this > codepath. (but i might have missed it) > > The page fault path does this: static inline int kvm_mmu_page_fault(struct kvm_vcpu *vcpu, gva_t gva, u32 error_code) { if (unlikely(vcpu->kvm->n_free_mmu_pages < KVM_MIN_FREE_MMU_PAGES)) kvm_mmu_free_some_pages(vcpu); return vcpu->mmu.page_fault(vcpu, gva, error_code); } The context switch paths need to do the same. -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV