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

Reply via email to