From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH] KVM: MMU: Segregate mmu pages created with different cr4.pge settings Date: Tue, 06 Jan 2009 12:41:40 +0200 Message-ID: <49633564.7070403@redhat.com> References: <20081221184146.8E00B250012@cleopatra.tlv.redhat.com> <49621FA9.5080903@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: "kvm@vger.kernel.org" , joerg.roedel@amd.com, Marcelo Tosatti To: Alexander Graf Return-path: Received: from mx2.redhat.com ([66.187.237.31]:34824 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750719AbZAFKls (ORCPT ); Tue, 6 Jan 2009 05:41:48 -0500 In-Reply-To: <49621FA9.5080903@suse.de> Sender: kvm-owner@vger.kernel.org List-ID: Alexander Graf wrote: > Avi Kivity wrote: > >> From: Avi Kivity >> >> Don't allow a vcpu with cr4.pge cleared to use a shadow page created with >> cr4.pge set; this might cause a cr3 switch not to sync ptes that have the >> global bit set (the global bit has no effect if !cr4.pge). >> >> This can only occur on smp with different cr4.pge settings for different >> vcpus (since a cr4 change will resync the shadow ptes), but there's no >> cost to being correct here. >> >> Signed-off-by: Avi Kivity >> >> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h >> index f49bfd0..ab8ef1d 100644 >> --- a/arch/x86/include/asm/kvm_host.h >> +++ b/arch/x86/include/asm/kvm_host.h >> @@ -183,6 +183,7 @@ union kvm_mmu_page_role { >> unsigned metaphysical:1; >> unsigned access:3; >> unsigned invalid:1; >> + unsigned cr4_pge:1; >> }; >> }; >> >> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c >> index c4da7fb..aa4575c 100644 >> --- a/arch/x86/kvm/x86.c >> +++ b/arch/x86/kvm/x86.c >> @@ -363,6 +363,7 @@ void kvm_set_cr4(struct kvm_vcpu *vcpu, unsigned long cr4) >> } >> kvm_x86_ops->set_cr4(vcpu, cr4); >> vcpu->arch.cr4 = cr4; >> + vcpu->arch.mmu.base_role.cr4_pge = !!(cr4 & X86_CR4_PGE); >> >> > > This line broke VMware ESX bootup using NPT on one VCPU for me. If I > comment it out and apply my patches to actually make ESX run, it boots > again. > I think this might be the problem: > static int mmu_need_write_protect(struct kvm_vcpu *vcpu, gfn_t gfn, > bool can_unsync) > { > struct kvm_mmu_page *shadow; > > shadow = kvm_mmu_lookup_page(vcpu->kvm, gfn); > if (shadow) { > if (shadow->role.level != PT_PAGE_TABLE_LEVEL) > return 1; > if (shadow->unsync) > return 0; > if (can_unsync && oos_shadow) > return kvm_unsync_page(vcpu, shadow); > return 1; > } > return 0; > } > lookup_page() is not deterministic if there are multiple shadows for a page, and the patch increases multiple shadows. Marcelo, shouldn't there be a for_each in there? -- error compiling committee.c: too many arguments to function