Re: [PATCH 2/2] kvm/e500v2: mapping guest TLB1 to host TLB0

[Alexander Graf <agraf-l3A5Bk7waGM@public.gmane.org>]

Liu Yu-B13201 wrote:
>  
>
>   
>> -----Original Message-----
>> From: Alexander Graf [mailto:agraf-l3A5Bk7waGM@public.gmane.org] 
>> Sent: Friday, September 10, 2010 7:24 AM
>> To: Liu Yu-B13201
>> Cc: kvm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; kvm-ppc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Subject: Re: [PATCH 2/2] kvm/e500v2: mapping guest TLB1 to host TLB0
>>
>>
>> On 08.09.2010, at 11:40, Liu Yu wrote:
>>
>>     
>>> Current guest TLB1 is mapped to host TLB1.
>>> As host kernel only provides 4K uncontinuous pages,
>>> we have to break guest large mapping into 4K shadow mappings.
>>> These 4K shadow mappings are then mapped into host TLB1 on fly.
>>> As host TLB1 only has 13 free entries, there's serious tlb miss.
>>>
>>> Since e500v2 has a big number of TLB0 entries,
>>> it should be help to map those 4K shadow mappings to host TLB0.
>>> To achieve this, we need to unlink guest tlb and host tlb,
>>> So that guest TLB1 mappings can route to any host TLB0 
>>>       
>> entries freely.
>>     
>>> Pages/mappings are considerred in the same kind as host tlb entry.
>>> This patch remove the link between pages and guest tlb 
>>>       
>> entry to do the unlink.
>>     
>>> And keep host_tlb0_ref in each vcpu to trace pages.
>>> Then it's easy to map guest TLB1 to host TLB0.
>>>
>>> In guest ramdisk boot test(guest mainly uses TLB1),
>>> with this patch, the tlb miss number get down 90%.
>>>
>>> Signed-off-by: Liu Yu <yu.liu-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
>>> ---
>>> arch/powerpc/include/asm/kvm_e500.h |    7 +-
>>> arch/powerpc/kvm/e500.c             |    4 +
>>> arch/powerpc/kvm/e500_tlb.c         |  280 
>>>       
>> ++++++++++++-----------------------
>>     
>>> arch/powerpc/kvm/e500_tlb.h         |    1 +
>>> 4 files changed, 104 insertions(+), 188 deletions(-)
>>>
>>>       
>
>   
>>> -static unsigned int tlb1_entry_num;
>>> +static inline unsigned int get_tlb0_entry_offset(u32 
>>>       
>> eaddr, u32 esel)
>>     
>>> +{
>>> +	return ((eaddr & 0x7F000) >> (12 - host_tlb0_assoc_bit) |
>>> +			(esel & (host_tlb0_assoc - 1)));
>>> +}
>>>
>>> void kvmppc_dump_tlbs(struct kvm_vcpu *vcpu)
>>> {
>>> @@ -62,11 +68,6 @@ static inline unsigned int tlb0_get_next_victim(
>>> 	return victim;
>>> }
>>>
>>> -static inline unsigned int tlb1_max_shadow_size(void)
>>> -{
>>> -	return tlb1_entry_num - tlbcam_index;
>>> -}
>>> -
>>> static inline int tlbe_is_writable(struct tlbe *tlbe)
>>> {
>>> 	return tlbe->mas3 & (MAS3_SW|MAS3_UW);
>>> @@ -100,7 +101,7 @@ static inline u32 
>>>       
>> e500_shadow_mas2_attrib(u32 mas2, int usermode)
>>     
>>> /*
>>>  * writing shadow tlb entry to host TLB
>>>  */
>>> -static inline void __write_host_tlbe(struct tlbe *stlbe)
>>> +static inline void __host_tlbe_write(struct tlbe *stlbe)
>>> {
>>> 	mtspr(SPRN_MAS1, stlbe->mas1);
>>> 	mtspr(SPRN_MAS2, stlbe->mas2);
>>> @@ -109,25 +110,22 @@ static inline void 
>>>       
>> __write_host_tlbe(struct tlbe *stlbe)
>>     
>>> 	__asm__ __volatile__ ("tlbwe\n" : : );
>>> }
>>>
>>> -static inline void write_host_tlbe(struct kvmppc_vcpu_e500 
>>>       
>> *vcpu_e500,
>>     
>>> -		int tlbsel, int esel, struct tlbe *stlbe)
>>> +static inline u32 host_tlb0_write(struct kvmppc_vcpu_e500 
>>>       
>> *vcpu_e500,
>>     
>>> +                                   u32 gvaddr, struct tlbe *stlbe)
>>> {
>>> -	local_irq_disable();
>>> -	if (tlbsel == 0) {
>>> -		__write_host_tlbe(stlbe);
>>> -	} else {
>>> -		unsigned register mas0;
>>> +	unsigned register mas0;
>>>
>>> -		mas0 = mfspr(SPRN_MAS0);
>>> +	local_irq_disable();
>>>       
>> Do you have to disable interrupts for a tlb write? If you get 
>> preempted before the write, the entry you overwrite could be 
>> different. But you don't protect against that either way. And 
>> if you get preempted afterwards, you could lose the entry. 
>> But since you enable interrupts beyond that, that could 
>> happen either way too.
>>
>> So what's the reason for the disable here?
>>
>>     
>
> Hello Alex,
>
> Doesn't local_irq_disable() also disable preempt?
> The reason to disable interrupts is because it's possible to have tlb
> misses in interrupt service route.
>   

Yes, local_irq_disable disables preempt. That's exactly what I was
referring to :).
I don't understand the statement about the service route. A tlb miss
still arrives with local_irq_disable.


Alex