From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marcelo Tosatti Subject: Re: [patch 09/10] KVM: MMU: out of sync shadow core v2 Date: Mon, 22 Sep 2008 18:55:03 -0300 Message-ID: <20080922215503.GA27744@dmt.cnet> References: <20080918212749.800177179@localhost.localdomain> <20080918213337.148804603@localhost.localdomain> <48D4506C.5070804@redhat.com> <20080921004515.GC10120@dmt.cnet> <48D802EA.9070807@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: kvm@vger.kernel.org, "David S. Ahern" To: Avi Kivity Return-path: Received: from mx2.redhat.com ([66.187.237.31]:55210 "EHLO mx2.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754190AbYIVV4d (ORCPT ); Mon, 22 Sep 2008 17:56:33 -0400 Content-Disposition: inline In-Reply-To: <48D802EA.9070807@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On Mon, Sep 22, 2008 at 11:41:14PM +0300, Avi Kivity wrote: > Marcelo Tosatti wrote: >>>> + while (parent->unsync_children) { >>>> + for (i = 0; i < PT64_ENT_PER_PAGE; ++i) { >>>> + u64 ent = sp->spt[i]; >>>> + >>>> + if (is_shadow_present_pte(ent)) { >>>> + struct kvm_mmu_page *child; >>>> + child = page_header(ent & PT64_BASE_ADDR_MASK); >>>> >>> What does this do? >>> >> >> Walks all children of given page with no efficiency. Its replaced later >> by the bitmap version. >> >> > > I don't understand how the variables sp, child, and parent interact. You > either need recursion or an explicit stack? It restarts at parent level whenever finishing any children: + if (i == PT64_ENT_PER_PAGE) { + sp->unsync_children = 0; + sp = parent; + } No efficiency. >> Yes. The next element for_each_entry_safe saved could have been zapped. >> >> > > Ouch. Ouch. > > I hate doing this. Can see no alternative though. Me neither. >>>> + /* don't unsync if pagetable is shadowed with multiple roles */ >>>> + hlist_for_each_entry_safe(s, node, n, bucket, hash_link) { >>>> + if (s->gfn != sp->gfn || s->role.metaphysical) >>>> + continue; >>>> + if (s->role.word != sp->role.word) >>>> + return 1; >>>> + } >>>> >>> This will happen for nonpae paging. But why not allow it? Zap all >>> unsynced pages on mode switch. >>> >>> Oh, if a page is both a page directory and page table, yes. >> >> Yes. >> >> >>> So to allow nonpae oos, check the level instead. >>> >> >> Windows 2008 64-bit has all sorts of sharing a pagetable at multiple >> levels too. >> >> > > We still want to allow oos for the two quadrants of a nonpae shadow page. Sure, can be an optimization step later?