From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [PATCH RFC] KVM MMU: fix hashing for TDP and non-paging modes Date: Fri, 23 Apr 2010 14:11:02 +0300 Message-ID: <4BD18046.8020905@redhat.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Eric Northup Return-path: Received: from mx1.redhat.com ([209.132.183.28]:26801 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756155Ab0DWLLI (ORCPT ); Fri, 23 Apr 2010 07:11:08 -0400 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 04/23/2010 12:15 AM, Eric Northup wrote: > I've been reading the x86's mmu.c recently and had been wondering > about something. Avi's recent mmu documentation (thanks!) seems to > have confirmed my understanding of how the shadow paging is supposed > to be working. Wasn't it a lot more fun to understand it from the code? reading the documentation is way to easy. > In TDP mode, when mmu_alloc_roots() calls > kvm_mmu_get_page(), why does it pass (vcpu->arch.cr3>> PAGE_SHIFT) or > (vcpu->arch.mmu.pae_root[i]) as gfn? > Historical accident. Luckily, no harmful effects other than wasting a bit of cpu cycles every now and then. > It seems to me that in TDP mode, gfn should be either zero for the > root page table, or 0/1GB/2GB/3GB (for PAE page tables). > > The existing behavior can lead to multiple, semantically-identical TDP > roots being created by mmu_alloc_roots, depending on the VCPU's CR3 at > the time that mmu_alloc_roots was called. But the nested page tables > should be* independent of the VCPU state. That wastes some memory and > causes extra page faults while populating the extra copies of the page > tables. > Yeah. > *assuming that we aren't modeling per-VCPU state that might change the > physical address map as seen by that VCPU, such as setting the APIC > base to an address overlapping RAM. > We don't actually support that. > All feedback would be welcome, since I'm new to this system! A > strawman patch follows. > > Patch is correct, but I have already fixed this in a more extensive patch set (that also folds the 32-bit and 64-bit cases together, etc.) and I'm too lazy to rebase on top of yours. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic.