Re: [patch 09/10] KVM: MMU: out of sync shadow core v2

[Avi Kivity <avi@redhat.com>]

Marcelo Tosatti wrote:
>>>   
>>>       
>> 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.
>
>   

Oh okay.  'parent' is never assigned to.  Lack of concentration.

>>> 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.
>
>   

Well.  But I don't see kvm_mmu_zap_page()'s return value used anywhere.

Actually, I think I see an alternative:  set the invalid flag on these 
pages and queue them in a list, like we do for roots in use.  Flush the 
list on some cleanup path.

>>> 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?
>   

I'd like to reexamine this from another angle: what if we allow oos of 
any level?

This will simplify the can_unsync path (always true) and remove a 
special case.  The cost is implementing invlpg and resync for non-leaf 
pages (invlpg has to resync the pte for every level).  Are there other 
problems with this?

-- 
error compiling committee.c: too many arguments to function