[PATCH 2/2] KVM: PPC: Book3E: Get vcpu's last instruction for emulation

[Scott Wood]

On 07/09/2013 04:44:24 PM, Alexander Graf wrote:
> 
> On 09.07.2013, at 20:46, Scott Wood wrote:
> > I suspect that tlbsx is faster, or at worst similar.  And unlike  
> comparing tlbsx to lwepx (not counting a fix for the threading  
> problem), we don't already have code to search the guest TLB, so  
> testing would be more work.
> 
> We have code to walk the guest TLB for TLB misses. This really is  
> just the TLB miss search without host TLB injection.
> 
> So let's say we're using the shadow TLB. The guest always has its say  
> 64 TLB entries that it can count on - we never evict anything by  
> accident, because we store all of the 64 entries in our guest TLB  
> cache. When the guest faults at an address, the first thing we do is  
> we check the cache whether we have that page already mapped.
> 
> However, with this method we now have 2 enumeration methods for guest  
> TLB searches. We have the tlbsx one which searches the host TLB and  
> we have our guest TLB cache. The guest TLB cache might still contain  
> an entry for an address that we already invalidated on the host.  
> Would that impose a problem?
> 
> I guess not because we're swizzling the exit code around to instead  
> be an instruction miss which means we restore the TLB entry into our  
> host's TLB so that when we resume, we land here and the tlbsx hits.  
> But it feels backwards.

Any better way?  Searching the guest TLB won't work for the LRAT case,  
so we'd need to have this logic around anyway.  We shouldn't add a  
second codepath unless it's a clear performance gain -- and again, I  
suspect it would be the opposite, especially if the entry is not in  
TLB0 or in one of the first few entries searched in TLB1.  The tlbsx  
miss case is not what we should optimize for.

> At least this code has to become something more generic, such as  
> kvmppc_read_guest(vcpu, addr, TYPE_INSN) and move into the host mmu  
> implementation, as it's 100% host mmu specific.

I agree that e500_mmu_host.c is a better place for it (with an ifdef  
for BOOKEHV), but supporting anything other than instruction fetches  
could wait until we have a user for it (it means extra code to figure  
out if permissions are correct).

-Scott