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? -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html