Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754614AbbKLO1c (ORCPT ); Thu, 12 Nov 2015 09:27:32 -0500 Received: from mail-wm0-f50.google.com ([74.125.82.50]:34854 "EHLO mail-wm0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752773AbbKLO1a (ORCPT ); Thu, 12 Nov 2015 09:27:30 -0500 Subject: Re: [PATCH 09/10 RFC] KVM: x86: MMU: Move parent_pte handling from kvm_mmu_get_page() to link_shadow_page() To: Takuya Yoshikawa References: <20151112204849.ba920599a8426d7196a0df73@lab.ntt.co.jp> <20151112205656.33f23effece7e6914c3e2646@lab.ntt.co.jp> Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org From: Paolo Bonzini X-Enigmail-Draft-Status: N1110 Message-ID: <5644A1CE.1040906@redhat.com> Date: Thu, 12 Nov 2015 15:27:26 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <20151112205656.33f23effece7e6914c3e2646@lab.ntt.co.jp> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1667 Lines: 55 On 12/11/2015 12:56, Takuya Yoshikawa wrote: > diff --git a/arch/x86/kvm/paging_tmpl.h b/arch/x86/kvm/paging_tmpl.h > index 9d21b44..f414ca6 100644 > --- a/arch/x86/kvm/paging_tmpl.h > +++ b/arch/x86/kvm/paging_tmpl.h > @@ -598,7 +598,7 @@ static int FNAME(fetch)(struct kvm_vcpu *vcpu, gva_t addr, > goto out_gpte_changed; > > if (sp) > - link_shadow_page(it.sptep, sp, PT_GUEST_ACCESSED_MASK); > + link_shadow_page(vcpu, it.sptep, sp, PT_GUEST_ACCESSED_MASK); > } > Here I think you can remove completely the if (sp) kvm_mmu_put_page(sp, it.sptep); later in FNAME(fetch). Apart from this nit, it's okay. On to kvm_mmu_get_page... if (!direct) { if (rmap_write_protect(vcpu, gfn)) kvm_flush_remote_tlbs(vcpu->kvm); if (level > PT_PAGE_TABLE_LEVEL && need_sync) kvm_sync_pages(vcpu, gfn); This seems fishy. need_sync is set if sp->unsync, but then the parents have not been unsynced yet. On the other hand, all calls to kvm_mmu_get_page except for the roots are followed by link_shadow_page... Perhaps if parent_pte != NULL you can call link_shadow_page directly from kvm_mmu_get_page. The call would go before the "if (!direct)" and it would subsume all the existing calls. We could probably also warn if (parent_pte == NULL) != (level == vcpu->arch.mmu.root_level) in kvm_mmu_get_page. Paolo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/