From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: KVM: MMU: update sp->gfns on pte update path Date: Tue, 25 Jan 2011 19:36:02 +0200 Message-ID: <4D3F0A02.3060902@redhat.com> References: <20110125130733.GA5645@amt.cnet> <4D3EDEFD.9060706@redhat.com> <20110125171236.GA10388@amt.cnet> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm , Nicolas Prochazka To: Marcelo Tosatti Return-path: Received: from mx1.redhat.com ([209.132.183.28]:42462 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753242Ab1AYRgI (ORCPT ); Tue, 25 Jan 2011 12:36:08 -0500 In-Reply-To: <20110125171236.GA10388@amt.cnet> Sender: kvm-owner@vger.kernel.org List-ID: On 01/25/2011 07:12 PM, Marcelo Tosatti wrote: > > > > Should be done by a call to kvm_mmu_page_set_gfn(). But I don't > > understand how it could become inconsistent in the first place. > > > > if (is_rmap_spte(*sptep)) { > > /* > > * If we overwrite a PTE page pointer with a 2MB PMD, unlink > > * the parent of the now unreachable PTE. > > */ > > if (level> PT_PAGE_TABLE_LEVEL&& > > !is_large_pte(*sptep)) { > > struct kvm_mmu_page *child; > > u64 pte = *sptep; > > > > child = page_header(pte& PT64_BASE_ADDR_MASK); > > mmu_page_remove_parent_pte(child, sptep); > > __set_spte(sptep, shadow_trap_nonpresent_pte); > > kvm_flush_remote_tlbs(vcpu->kvm); > > } else if (pfn != spte_to_pfn(*sptep)) { > > pgprintk("hfn old %llx new %llx\n", > > spte_to_pfn(*sptep), pfn); > > drop_spte(vcpu->kvm, sptep, shadow_trap_nonpresent_pte); > > kvm_flush_remote_tlbs(vcpu->kvm); > > } else > > was_rmapped = 1; > > } > > > > If we set was_rmapped, that means rmap_add() was previously called > > for this spte/gfn/pfn pair, and all that changes is permissions, no? > > What if pfn is the same but gfn differs? Could be. Any way to verify if this was the case? Isn't it nicer to have it detected by the test above and do the drop_spte()/kvm_flush_remote_tlbs() thing instead? -- error compiling committee.c: too many arguments to function