Thread (16 messages) flat view 16 messages, 3 authors, 2013-10-04

Re: [PATCH 5/6 v5] kvm: booke: clear host tlb reference flag on guest tlb invalidation

From: Scott Wood <hidden>
Date: 2013-09-19 21:08:18
Also in: kvm

On Thu, 2013-09-19 at 11:32 +0530, Bharat Bhushan wrote:
quoted hunk ↗ jump to hunk
On booke, "struct tlbe_ref" contains host tlb mapping information
(pfn: for guest-pfn to pfn, flags: attribute associated with this mapping)
for a guest tlb entry. So when a guest creates a TLB entry then
"struct tlbe_ref" is set to point to valid "pfn" and set attributes in
"flags" field of the above said structure. When a guest TLB entry is
invalidated then flags field of corresponding "struct tlbe_ref" is
updated to point that this is no more valid, also we selectively clear
some other attribute bits, example: if E500_TLB_BITMAP was set then we clear
E500_TLB_BITMAP, if E500_TLB_TLB0 is set then we clear this.

Ideally we should clear complete "flags" as this entry is invalid and does not
have anything to re-used. The other part of the problem is that when we use
the same entry again then also we do not clear (started doing or-ing etc).

So far it was working because the selectively clearing mentioned above
actually clears "flags" what was set during TLB mapping. But the problem
starts coming when we add more attributes to this then we need to selectively
clear them and which is not needed.

This patch we do both
        - Clear "flags" when invalidating;
        - Clear "flags" when reusing same entry later

Signed-off-by: Bharat Bhushan <redacted>
---
v3-> v5
 - New patch (found this issue when doing vfio-pci development)

 arch/powerpc/kvm/e500_mmu_host.c |   12 +++++++-----
 1 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/arch/powerpc/kvm/e500_mmu_host.c b/arch/powerpc/kvm/e500_mmu_host.c
index 1c6a9d7..60f5a3c 100644
--- a/arch/powerpc/kvm/e500_mmu_host.c
+++ b/arch/powerpc/kvm/e500_mmu_host.c
@@ -217,7 +217,8 @@ void inval_gtlbe_on_host(struct kvmppc_vcpu_e500 *vcpu_e500, int tlbsel,
 		}
 		mb();
 		vcpu_e500->g2h_tlb1_map[esel] = 0;
-		ref->flags &= ~(E500_TLB_BITMAP | E500_TLB_VALID);
+		/* Clear flags as TLB is not backed by the host anymore */
+		ref->flags = 0;
 		local_irq_restore(flags);
 	}
This breaks when you have both E500_TLB_BITMAP and E500_TLB_TLB0 set.

Instead, just convert the final E500_TLB_VALID clearing at the end into
ref->flags = 0, and convert the early return a few lines earlier into
conditional execution of the tlbil_one().

-Scott
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help