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

From: Scott Wood <[email-masked]>
Date: Tue, 9 Jul 2013 19:06:37 -0500

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

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

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

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

-Scott=