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

Avi Kivity
2008-09-23T10:46:27Z

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