Re: [PATCH 4 of 5] kvm: ppc: Write only modified shadow entries into

[Christian Ehrhardt <ehrhardt@linux.vnet.ibm.com>]

Liu Yu wrote:
>> -----Original Message-----
>> From: Christian Ehrhardt [mailto:ehrhardt@linux.vnet.ibm.com] 
>> Sent: Monday, August 11, 2008 4:33 PM
>> To: Liu Yu
>> Cc: Hollis Blanchard; kvm-ppc@vger.kernel.org
>> Subject: Re: [PATCH 4 of 5] kvm: ppc: Write only modified 
>> shadow entries into theTLB on exit
>>
>> Liu Yu wrote:
>>     
>>>   
>>>       
>>>> -----Original Message-----
>>>> From: kvm-ppc-owner@vger.kernel.org
>>>> [mailto:kvm-ppc-owner@vger.kernel.org] On Behalf Of Hollis 
>>>>         
>> Blanchard
>>     
>>>> Sent: Saturday, July 26, 2008 2:55 AM
>>>> To: avi@qumranet.com
>>>> Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org
>>>> Subject: [PATCH 4 of 5] kvm: ppc: Write only modified 
>>>>         
>> shadow entries 
>>     
>>>> into theTLB on exit
>>>>
>>>> @@ -229,6 +235,7 @@ void kvmppc_mmu_priv_switch(struct kvm_v
>>>>  
>>>>  		kvmppc_44x_shadow_release(vcpu, i);
>>>>  		stlbe->word0 = 0;
>>>> +		kvmppc_tlbe_set_modified(vcpu, i);
>>>>  		KVMTRACE_5D(STLB_INVAL, vcpu, i,
>>>>  				stlbe->tid, stlbe->word0, stlbe->word1,
>>>>  				stlbe->word2, handler);
>>>>     
>>>>         
>>> Can this place use "_tlbia()" instead?
>>>   
>>>       
>> Hi Liu,
>> when we exit&reenter a guest on the lightweight path this 
>> patch saves us from rebuilding too much of the tlb.
>> It's just the point of the patch to not invalidate all tlb 
>> entries on every exit, but keeping as much as possible.
>> Additionally I'm not even sure if tlbia is available on all 
>> cores used.
>>
>> Am I overlooking something ? - what do you think would be the 
>> benefit of using _tlbia() here?
>> Do you mean the loop to clear up to the hwater mark where 
>> this patch makes changes and not this patch itself?
>>
>>     
>
> Yes.
>
> When you set nearly all tlb entries modified in this loop,
> you have to write all of them back before entering the guest in booke_interrupts.S.
>
> So, why not "_tlbia()" here outside this loop, and don't set them modified.
> Then, you needn't write them back.
> In fact, the shadow tlb entries are still consistent with the hardware, as all of them are invalidated.
>
> Am I overlooking something ?
>   
As Hollis wrote in his patch comment usually just one tlb entry is 
modified which saves a lot of updates.
If we invalidate all we later on would have to satisfy the misses we 
cause by doing so.

With some free time we could test _tlbia() here, but I expect it to be 
slower by causing the need for tlb re-population compared to what we 
save here.

-- 

Grüsse / regards, 
Christian Ehrhardt
IBM Linux Technology Center, Open Virtualization