All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] kvm/e500: Do not keep shadow tlb array
@ 2009-04-27  6:58 Liu Yu
  2009-04-28 19:33 ` Hollis Blanchard
  2009-04-29  2:43 ` Liu Yu-B13201
  0 siblings, 2 replies; 3+ messages in thread
From: Liu Yu @ 2009-04-27  6:58 UTC (permalink / raw)
  To: kvm-ppc

Shadow tlb array costs a lot memory
and incurs potential coherence problem.

Remove it and we now translate to shadow mappings directly
from guest TLB entries.

Signed-off-by: Liu Yu <yu.liu@freescale.com>
---
 arch/powerpc/include/asm/kvm_e500.h |    8 +-
 arch/powerpc/kvm/e500_tlb.c         |  277 +++++++++++++----------------------
 2 files changed, 106 insertions(+), 179 deletions(-)

diff --git a/arch/powerpc/include/asm/kvm_e500.h b/arch/powerpc/include/asm/kvm_e500.h
index 9d497ce..ffa111b 100644
--- a/arch/powerpc/include/asm/kvm_e500.h
+++ b/arch/powerpc/include/asm/kvm_e500.h
@@ -29,13 +29,19 @@ struct tlbe{
 	u32 mas7;
 };
 
+struct kvmppc_e500_shadow_ref {
+	struct page *page;
+	u16 gtlb_index;
+	u16 pad;
+};
+
 struct kvmppc_vcpu_e500 {
 	/* Unmodified copy of the guest's TLB. */
 	struct tlbe *guest_tlb[E500_TLB_NUM];
 	/* TLB that's actually used when the guest is running. */
 	struct tlbe *shadow_tlb[E500_TLB_NUM];
 	/* Pages which are referenced in the shadow TLB. */
-	struct page **shadow_pages[E500_TLB_NUM];
+	struct kvmppc_e500_shadow_ref *shadow_refs[E500_TLB_NUM];
 
 	unsigned int guest_tlb_size[E500_TLB_NUM];
 	unsigned int shadow_tlb_size[E500_TLB_NUM];
diff --git a/arch/powerpc/kvm/e500_tlb.c b/arch/powerpc/kvm/e500_tlb.c
index 0e773fc..847dfec 100644
--- a/arch/powerpc/kvm/e500_tlb.c
+++ b/arch/powerpc/kvm/e500_tlb.c
@@ -46,17 +46,6 @@ void kvmppc_dump_tlbs(struct kvm_vcpu *vcpu)
 					tlbe->mas3, tlbe->mas7);
 		}
 	}
-
-	for (tlbsel = 0; tlbsel < 2; tlbsel++) {
-		printk("Shadow TLB%d:\n", tlbsel);
-		for (i = 0; i < vcpu_e500->shadow_tlb_size[tlbsel]; i++) {
-			tlbe = &vcpu_e500->shadow_tlb[tlbsel][i];
-			if (tlbe->mas1 & MAS1_VALID)
-				printk(" S[%d][%3d] |  %08X | %08X | %08X | %08X |\n",
-					tlbsel, i, tlbe->mas1, tlbe->mas2,
-					tlbe->mas3, tlbe->mas7);
-		}
-	}
 }
 
 static inline unsigned int tlb0_get_next_victim(
@@ -119,10 +108,8 @@ static inline void __write_host_tlbe(struct tlbe *stlbe)
 }
 
 static inline void write_host_tlbe(struct kvmppc_vcpu_e500 *vcpu_e500,
-		int tlbsel, int esel)
+		int tlbsel, int esel, struct tlbe *stlbe)
 {
-	struct tlbe *stlbe = &vcpu_e500->shadow_tlb[tlbsel][esel];
-
 	local_irq_disable();
 	if (tlbsel = 0) {
 		__write_host_tlbe(stlbe);
@@ -137,28 +124,13 @@ static inline void write_host_tlbe(struct kvmppc_vcpu_e500 *vcpu_e500,
 		mtspr(SPRN_MAS0, mas0);
 	}
 	local_irq_enable();
+	KVMTRACE_5D(STLB_WRITE, &vcpu_e500->vcpu, index_of(tlbsel, esel),
+			stlbe->mas1, stlbe->mas2, stlbe->mas3, stlbe->mas7,
+			handler);
 }
 
 void kvmppc_e500_tlb_load(struct kvm_vcpu *vcpu, int cpu)
 {
-	struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
-	int i;
-	unsigned register mas0;
-
-	/* Load all valid TLB1 entries to reduce guest tlb miss fault */
-	local_irq_disable();
-	mas0 = mfspr(SPRN_MAS0);
-	for (i = 0; i < tlb1_max_shadow_size(); i++) {
-		struct tlbe *stlbe = &vcpu_e500->shadow_tlb[1][i];
-
-		if (get_tlb_v(stlbe)) {
-			mtspr(SPRN_MAS0, MAS0_TLBSEL(1)
-					| MAS0_ESEL(to_htlb1_esel(i)));
-			__write_host_tlbe(stlbe);
-		}
-	}
-	mtspr(SPRN_MAS0, mas0);
-	local_irq_enable();
 }
 
 void kvmppc_e500_tlb_put(struct kvm_vcpu *vcpu)
@@ -200,16 +172,24 @@ static int kvmppc_e500_tlb_index(struct kvmppc_vcpu_e500 *vcpu_e500,
 }
 
 static void kvmppc_e500_shadow_release(struct kvmppc_vcpu_e500 *vcpu_e500,
-		int tlbsel, int esel)
+		int stlbsel, int sesel)
 {
-	struct tlbe *stlbe = &vcpu_e500->shadow_tlb[tlbsel][esel];
-	struct page *page = vcpu_e500->shadow_pages[tlbsel][esel];
+	struct tlbe *gtlbe;
+	struct kvmppc_e500_shadow_ref *ref;
+	struct page *page;
+	int index;
+
+	ref = &vcpu_e500->shadow_refs[stlbsel][sesel];
+	page = ref->page;
 
 	if (page) {
-		vcpu_e500->shadow_pages[tlbsel][esel] = NULL;
+		index = ref->gtlb_index;
+		gtlbe = &vcpu_e500->guest_tlb[tlbsel_of(index)][esel_of(index)];
 
-		if (get_tlb_v(stlbe)) {
-			if (tlbe_is_writable(stlbe))
+		ref->page = NULL;
+
+		if (get_tlb_v(gtlbe)) {
+			if (tlbe_is_writable(gtlbe))
 				kvm_release_page_dirty(page);
 			else
 				kvm_release_page_clean(page);
@@ -217,44 +197,19 @@ static void kvmppc_e500_shadow_release(struct kvmppc_vcpu_e500 *vcpu_e500,
 	}
 }
 
-static void kvmppc_e500_stlbe_invalidate(struct kvmppc_vcpu_e500 *vcpu_e500,
-		int tlbsel, int esel)
-{
-	struct tlbe *stlbe = &vcpu_e500->shadow_tlb[tlbsel][esel];
-
-	kvmppc_e500_shadow_release(vcpu_e500, tlbsel, esel);
-	stlbe->mas1 = 0;
-	KVMTRACE_5D(STLB_INVAL, &vcpu_e500->vcpu, index_of(tlbsel, esel),
-			stlbe->mas1, stlbe->mas2, stlbe->mas3, stlbe->mas7,
-			handler);
-}
-
 static void kvmppc_e500_tlb1_invalidate(struct kvmppc_vcpu_e500 *vcpu_e500,
-		gva_t eaddr, gva_t eend, u32 tid)
+		int esel)
 {
-	unsigned int pid = tid & 0xff;
+	struct tlbe stlbe;
 	unsigned int i;
 
-	/* XXX Replace loop with fancy data structures. */
-	for (i = 0; i < vcpu_e500->guest_tlb_size[1]; i++) {
-		struct tlbe *stlbe = &vcpu_e500->shadow_tlb[1][i];
-		unsigned int tid;
-
-		if (!get_tlb_v(stlbe))
-			continue;
-
-		if (eend < get_tlb_eaddr(stlbe))
-			continue;
-
-		if (eaddr > get_tlb_end(stlbe))
-			continue;
-
-		tid = get_tlb_tid(stlbe);
-		if (tid && (tid != pid))
-			continue;
+	stlbe.mas1 = 0;
+	for (i = 0; i < KVM_E500_TLB1_SIZE; i++) {
+		struct kvmppc_e500_shadow_ref *ref +			&vcpu_e500->shadow_refs[1][i];
 
-		kvmppc_e500_stlbe_invalidate(vcpu_e500, 1, i);
-		write_host_tlbe(vcpu_e500, 1, i);
+		if (ref->gtlb_index = index_of(1, esel))
+			write_host_tlbe(vcpu_e500, 1, i, &stlbe);
 	}
 }
 
@@ -285,14 +240,29 @@ static inline void kvmppc_e500_deliver_tlb_miss(struct kvm_vcpu *vcpu,
 	vcpu_e500->mas7 = 0;
 }
 
+static inline void kvmppc_e500_setup_stlbe(struct kvmppc_vcpu_e500 *vcpu_e500,
+		struct tlbe *gtlbe, struct kvmppc_e500_shadow_ref *ref,
+		u64 gvaddr, struct tlbe *stlbe)
+{
+	hpa_t hpaddr = page_to_phys(ref->page);
+
+	/* Force TS=1 IPROT=0 TSIZE=4KB for all guest mappings. */
+	stlbe->mas1 = MAS1_TSIZE(BOOKE_PAGESZ_4K)
+		| MAS1_TID(get_tlb_tid(gtlbe)) | MAS1_TS | MAS1_VALID;
+	stlbe->mas2 = (gvaddr & MAS2_EPN)
+		| e500_shadow_mas2_attrib(gtlbe->mas2,
+				vcpu_e500->vcpu.arch.msr & MSR_PR);
+	stlbe->mas3 = (hpaddr & MAS3_RPN)
+		| e500_shadow_mas3_attrib(gtlbe->mas3,
+				vcpu_e500->vcpu.arch.msr & MSR_PR);
+	stlbe->mas7 = (hpaddr >> 32) & MAS7_RPN;
+}
+
 static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
-	u64 gvaddr, gfn_t gfn, struct tlbe *gtlbe, int tlbsel, int esel)
+	gfn_t gfn, struct tlbe *gtlbe, int stlbsel, int sesel)
 {
 	struct page *new_page;
-	struct tlbe *stlbe;
-	hpa_t hpaddr;
-
-	stlbe = &vcpu_e500->shadow_tlb[tlbsel][esel];
+	struct kvmppc_e500_shadow_ref *ref;
 
 	/* Get reference to new page. */
 	new_page = gfn_to_page(vcpu_e500->vcpu.kvm, gfn);
@@ -301,40 +271,23 @@ static inline void kvmppc_e500_shadow_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 		kvm_release_page_clean(new_page);
 		return;
 	}
-	hpaddr = page_to_phys(new_page);
 
 	/* Drop reference to old page. */
-	kvmppc_e500_shadow_release(vcpu_e500, tlbsel, esel);
-
-	vcpu_e500->shadow_pages[tlbsel][esel] = new_page;
-
-	/* Force TS=1 IPROT=0 TSIZE=4KB for all guest mappings. */
-	stlbe->mas1 = MAS1_TSIZE(BOOKE_PAGESZ_4K)
-		| MAS1_TID(get_tlb_tid(gtlbe)) | MAS1_TS | MAS1_VALID;
-	stlbe->mas2 = (gvaddr & MAS2_EPN)
-		| e500_shadow_mas2_attrib(gtlbe->mas2,
-				vcpu_e500->vcpu.arch.msr & MSR_PR);
-	stlbe->mas3 = (hpaddr & MAS3_RPN)
-		| e500_shadow_mas3_attrib(gtlbe->mas3,
-				vcpu_e500->vcpu.arch.msr & MSR_PR);
-	stlbe->mas7 = (hpaddr >> 32) & MAS7_RPN;
+	kvmppc_e500_shadow_release(vcpu_e500, stlbsel, sesel);
 
-	KVMTRACE_5D(STLB_WRITE, &vcpu_e500->vcpu, index_of(tlbsel, esel),
-			stlbe->mas1, stlbe->mas2, stlbe->mas3, stlbe->mas7,
-			handler);
+	ref = &vcpu_e500->shadow_refs[stlbsel][sesel];
+	ref->page = new_page;
 }
 
 /* XXX only map the one-one case, for now use TLB0 */
-static int kvmppc_e500_stlbe_map(struct kvmppc_vcpu_e500 *vcpu_e500,
-		int tlbsel, int esel)
+static int kvmppc_e500_tlb0_map(struct kvmppc_vcpu_e500 *vcpu_e500, int esel)
 {
 	struct tlbe *gtlbe;
 
-	gtlbe = &vcpu_e500->guest_tlb[tlbsel][esel];
+	gtlbe = &vcpu_e500->guest_tlb[0][esel];
 
-	kvmppc_e500_shadow_map(vcpu_e500, get_tlb_eaddr(gtlbe),
-			get_tlb_raddr(gtlbe) >> PAGE_SHIFT,
-			gtlbe, tlbsel, esel);
+	kvmppc_e500_shadow_map(vcpu_e500, get_tlb_raddr(gtlbe) >> PAGE_SHIFT,
+			gtlbe, 0, esel);
 
 	return esel;
 }
@@ -352,7 +305,7 @@ static int kvmppc_e500_tlb1_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 	if (unlikely(vcpu_e500->guest_tlb_nv[1] >= tlb1_max_shadow_size()))
 		vcpu_e500->guest_tlb_nv[1] = 0;
 
-	kvmppc_e500_shadow_map(vcpu_e500, gvaddr, gfn, gtlbe, 1, victim);
+	kvmppc_e500_shadow_map(vcpu_e500, gfn, gtlbe, 1, victim);
 
 	return victim;
 }
@@ -363,33 +316,19 @@ static int kvmppc_e500_tlb1_map(struct kvmppc_vcpu_e500 *vcpu_e500,
 void kvmppc_mmu_priv_switch(struct kvm_vcpu *vcpu, int usermode)
 {
 	if (usermode) {
-		struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
-		int i;
-
-		/* XXX Replace loop with fancy data structures. */
-		for (i = 0; i < tlb1_max_shadow_size(); i++)
-			kvmppc_e500_stlbe_invalidate(vcpu_e500, 1, i);
-
 		_tlbil_all();
 	}
 }
 
-static int kvmppc_e500_gtlbe_invalidate(struct kvmppc_vcpu_e500 *vcpu_e500,
-		int tlbsel, int esel)
+static inline int kvmppc_e500_gtlbe_invalidate(
+				struct kvmppc_vcpu_e500 *vcpu_e500,
+				int tlbsel, int esel)
 {
 	struct tlbe *gtlbe = &vcpu_e500->guest_tlb[tlbsel][esel];
 
 	if (unlikely(get_tlb_iprot(gtlbe)))
 		return -1;
 
-	if (tlbsel = 1) {
-		kvmppc_e500_tlb1_invalidate(vcpu_e500, get_tlb_eaddr(gtlbe),
-				get_tlb_end(gtlbe),
-				get_tlb_tid(gtlbe));
-	} else {
-		kvmppc_e500_stlbe_invalidate(vcpu_e500, tlbsel, esel);
-	}
-
 	gtlbe->mas1 = 0;
 
 	return 0;
@@ -512,23 +451,16 @@ int kvmppc_e500_emul_tlbsx(struct kvm_vcpu *vcpu, int rb)
 int kvmppc_e500_emul_tlbwe(struct kvm_vcpu *vcpu)
 {
 	struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
-	u64 eaddr;
-	u64 raddr;
-	u32 tid;
 	struct tlbe *gtlbe;
-	int tlbsel, esel, stlbsel, sesel;
+	int tlbsel, esel;
 
 	tlbsel = get_tlb_tlbsel(vcpu_e500);
 	esel = get_tlb_esel(vcpu_e500, tlbsel);
 
 	gtlbe = &vcpu_e500->guest_tlb[tlbsel][esel];
 
-	if (get_tlb_v(gtlbe) && tlbsel = 1) {
-		eaddr = get_tlb_eaddr(gtlbe);
-		tid = get_tlb_tid(gtlbe);
-		kvmppc_e500_tlb1_invalidate(vcpu_e500, eaddr,
-				get_tlb_end(gtlbe), tid);
-	}
+	if (get_tlb_v(gtlbe) && tlbsel = 1)
+		kvmppc_e500_tlb1_invalidate(vcpu_e500, esel);
 
 	gtlbe->mas1 = vcpu_e500->mas1;
 	gtlbe->mas2 = vcpu_e500->mas2;
@@ -541,35 +473,36 @@ int kvmppc_e500_emul_tlbwe(struct kvm_vcpu *vcpu)
 
 	/* Invalidate shadow mappings for the about-to-be-clobbered TLBE. */
 	if (tlbe_is_host_safe(vcpu, gtlbe)) {
+		struct tlbe stlbe;
+		int stlbsel, sesel;
+		struct kvmppc_e500_shadow_ref *ref;
+		u64 eaddr;
+
 		switch (tlbsel) {
 		case 0:
 			/* TLB0 */
 			gtlbe->mas1 &= ~MAS1_TSIZE(~0);
 			gtlbe->mas1 |= MAS1_TSIZE(BOOKE_PAGESZ_4K);
 
+			eaddr =  get_tlb_eaddr(gtlbe);
 			stlbsel = 0;
-			sesel = kvmppc_e500_stlbe_map(vcpu_e500, 0, esel);
+			sesel = kvmppc_e500_tlb0_map(vcpu_e500, esel);
 
+			ref = &vcpu_e500->shadow_refs[stlbsel][sesel];
+
+			kvmppc_e500_setup_stlbe(vcpu_e500, gtlbe,
+						ref, eaddr, &stlbe);
+			write_host_tlbe(vcpu_e500, stlbsel, sesel, &stlbe);
 			break;
 
 		case 1:
-			/* TLB1 */
-			eaddr = get_tlb_eaddr(gtlbe);
-			raddr = get_tlb_raddr(gtlbe);
-
-			/* Create a 4KB mapping on the host.
-			 * If the guest wanted a large page,
-			 * only the first 4KB is mapped here and the rest
-			 * are mapped on the fly. */
-			stlbsel = 1;
-			sesel = kvmppc_e500_tlb1_map(vcpu_e500, eaddr,
-					raddr >> PAGE_SHIFT, gtlbe);
+			/* Large page breaks into 4KB pages.
+			 * And all these 4KB pages will be mapped on the fly. */
 			break;
 
 		default:
 			BUG();
 		}
-		write_host_tlbe(vcpu_e500, stlbsel, sesel);
 	}
 
 	return EMULATE_DONE;
@@ -617,11 +550,11 @@ gpa_t kvmppc_mmu_xlate(struct kvm_vcpu *vcpu, unsigned int index,
 void kvmppc_mmu_destroy(struct kvm_vcpu *vcpu)
 {
 	struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
-	int tlbsel, i;
+	int stlbsel, i;
 
-	for (tlbsel = 0; tlbsel < 2; tlbsel++)
-		for (i = 0; i < vcpu_e500->guest_tlb_size[tlbsel]; i++)
-			kvmppc_e500_shadow_release(vcpu_e500, tlbsel, i);
+	for (stlbsel = 0; stlbsel < 2; stlbsel++)
+		for (i = 0; i < vcpu_e500->guest_tlb_size[stlbsel]; i++)
+			kvmppc_e500_shadow_release(vcpu_e500, stlbsel, i);
 
 	/* discard all guest mapping */
 	_tlbil_all();
@@ -631,10 +564,14 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 eaddr, gpa_t gpaddr,
 			unsigned int index)
 {
 	struct kvmppc_vcpu_e500 *vcpu_e500 = to_e500(vcpu);
+	struct kvmppc_e500_shadow_ref *ref;
+	struct tlbe *gtlbe, stlbe;
 	int tlbsel = tlbsel_of(index);
 	int esel = esel_of(index);
 	int stlbsel, sesel;
 
+	gtlbe = &vcpu_e500->guest_tlb[tlbsel][esel];
+
 	switch (tlbsel) {
 	case 0:
 		stlbsel = 0;
@@ -643,8 +580,6 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 eaddr, gpa_t gpaddr,
 
 	case 1: {
 		gfn_t gfn = gpaddr >> PAGE_SHIFT;
-		struct tlbe *gtlbe
-			= &vcpu_e500->guest_tlb[tlbsel][esel];
 
 		stlbsel = 1;
 		sesel = kvmppc_e500_tlb1_map(vcpu_e500, eaddr, gfn, gtlbe);
@@ -655,7 +590,11 @@ void kvmppc_mmu_map(struct kvm_vcpu *vcpu, u64 eaddr, gpa_t gpaddr,
 		BUG();
 		break;
 	}
-	write_host_tlbe(vcpu_e500, stlbsel, sesel);
+
+	ref = &vcpu_e500->shadow_refs[stlbsel][sesel];
+
+	kvmppc_e500_setup_stlbe(vcpu_e500, gtlbe, ref, eaddr, &stlbe);
+	write_host_tlbe(vcpu_e500, stlbsel, sesel, &stlbe);
 }
 
 int kvmppc_e500_tlb_search(struct kvm_vcpu *vcpu,
@@ -702,44 +641,28 @@ int kvmppc_e500_tlb_init(struct kvmppc_vcpu_e500 *vcpu_e500)
 	if (vcpu_e500->guest_tlb[0] = NULL)
 		goto err_out;
 
-	vcpu_e500->shadow_tlb_size[0] = KVM_E500_TLB0_SIZE;
-	vcpu_e500->shadow_tlb[0] -		kzalloc(sizeof(struct tlbe) * KVM_E500_TLB0_SIZE, GFP_KERNEL);
-	if (vcpu_e500->shadow_tlb[0] = NULL)
-		goto err_out_guest0;
-
 	vcpu_e500->guest_tlb_size[1] = KVM_E500_TLB1_SIZE;
 	vcpu_e500->guest_tlb[1]  		kzalloc(sizeof(struct tlbe) * KVM_E500_TLB1_SIZE, GFP_KERNEL);
 	if (vcpu_e500->guest_tlb[1] = NULL)
-		goto err_out_shadow0;
+		goto err_out_guest0;
 
-	vcpu_e500->shadow_tlb_size[1] = tlb1_entry_num;
-	vcpu_e500->shadow_tlb[1] -		kzalloc(sizeof(struct tlbe) * tlb1_entry_num, GFP_KERNEL);
-	if (vcpu_e500->shadow_tlb[1] = NULL)
+	vcpu_e500->shadow_refs[0] = (struct kvmppc_e500_shadow_ref *)
+		kzalloc(sizeof(struct kvmppc_e500_shadow_ref) * KVM_E500_TLB0_SIZE, GFP_KERNEL);
+	if (vcpu_e500->shadow_refs[0] = NULL)
 		goto err_out_guest1;
 
-	vcpu_e500->shadow_pages[0] = (struct page **)
-		kzalloc(sizeof(struct page *) * KVM_E500_TLB0_SIZE, GFP_KERNEL);
-	if (vcpu_e500->shadow_pages[0] = NULL)
-		goto err_out_shadow1;
-
-	vcpu_e500->shadow_pages[1] = (struct page **)
-		kzalloc(sizeof(struct page *) * tlb1_entry_num, GFP_KERNEL);
-	if (vcpu_e500->shadow_pages[1] = NULL)
-		goto err_out_page0;
+	vcpu_e500->shadow_refs[1] = (struct kvmppc_e500_shadow_ref *)
+		kzalloc(sizeof(struct kvmppc_e500_shadow_ref) * KVM_E500_TLB1_SIZE, GFP_KERNEL);
+	if (vcpu_e500->shadow_refs[1] = NULL)
+		goto err_out_ref0;
 
 	return 0;
 
-err_out_page0:
-	kfree(vcpu_e500->shadow_pages[0]);
-err_out_shadow1:
-	kfree(vcpu_e500->shadow_tlb[1]);
+err_out_ref0:
+	kfree(vcpu_e500->shadow_refs[0]);
 err_out_guest1:
 	kfree(vcpu_e500->guest_tlb[1]);
-err_out_shadow0:
-	kfree(vcpu_e500->shadow_tlb[0]);
 err_out_guest0:
 	kfree(vcpu_e500->guest_tlb[0]);
 err_out:
@@ -748,10 +671,8 @@ err_out:
 
 void kvmppc_e500_tlb_uninit(struct kvmppc_vcpu_e500 *vcpu_e500)
 {
-	kfree(vcpu_e500->shadow_pages[1]);
-	kfree(vcpu_e500->shadow_pages[0]);
-	kfree(vcpu_e500->shadow_tlb[1]);
+	kfree(vcpu_e500->shadow_refs[1]);
+	kfree(vcpu_e500->shadow_refs[0]);
 	kfree(vcpu_e500->guest_tlb[1]);
-	kfree(vcpu_e500->shadow_tlb[0]);
 	kfree(vcpu_e500->guest_tlb[0]);
 }
-- 
1.5.4


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH 1/3] kvm/e500: Do not keep shadow tlb array
  2009-04-27  6:58 [PATCH 1/3] kvm/e500: Do not keep shadow tlb array Liu Yu
@ 2009-04-28 19:33 ` Hollis Blanchard
  2009-04-29  2:43 ` Liu Yu-B13201
  1 sibling, 0 replies; 3+ messages in thread
From: Hollis Blanchard @ 2009-04-28 19:33 UTC (permalink / raw)
  To: kvm-ppc

On Mon, 2009-04-27 at 14:58 +0800, Liu Yu wrote:
> Shadow tlb array costs a lot memory
> and incurs potential coherence problem.
> 
> Remove it and we now translate to shadow mappings directly
> from guest TLB entries.
> 
> Signed-off-by: Liu Yu <yu.liu@freescale.com>

Well, funny story... pretty much as soon as we removed the shadow TLB on
440, we put it back. But we use it differently now. Our experience on
440 was that it was a performance win to *also* keep a shadow TLB array,
but use that *only* in the vcpu_put/vcpu_load() path instead of tlbia().
In other words, instead of just clearing all that guest TLB state you
worked so hard to build up, preserve it across host context switches.

We don't use that array for anything other than host context switches.
For normal exits (handled in kernel or handled in qemu) we take a less
precise "shadowy" approach, where host and guest TLB entries can compete
without KVM's knowledge, and we indiscriminately invalidate them.

(Yes, http://www.linux-kvm.org/page/PowerPC_Book_E_MMU is no longer
completely accurate.)

I'm not sure what the coherence problems are that you allude to, but we
never did SMP host support for 440.

-- 
Hollis Blanchard
IBM Linux Technology Center


^ permalink raw reply	[flat|nested] 3+ messages in thread

* RE: [PATCH 1/3] kvm/e500: Do not keep shadow tlb array
  2009-04-27  6:58 [PATCH 1/3] kvm/e500: Do not keep shadow tlb array Liu Yu
  2009-04-28 19:33 ` Hollis Blanchard
@ 2009-04-29  2:43 ` Liu Yu-B13201
  1 sibling, 0 replies; 3+ messages in thread
From: Liu Yu-B13201 @ 2009-04-29  2:43 UTC (permalink / raw)
  To: kvm-ppc



> -----Original Message-----
> From: Hollis Blanchard [mailto:hollisb@us.ibm.com] 
> Sent: Wednesday, April 29, 2009 3:34 AM
> To: Liu Yu-B13201
> Cc: kvm-ppc@vger.kernel.org; rkulkarn@force10networks.com
> Subject: Re: [PATCH 1/3] kvm/e500: Do not keep shadow tlb array
> 
> On Mon, 2009-04-27 at 14:58 +0800, Liu Yu wrote:
> > Shadow tlb array costs a lot memory
> > and incurs potential coherence problem.
> > 
> > Remove it and we now translate to shadow mappings directly
> > from guest TLB entries.
> > 
> > Signed-off-by: Liu Yu <yu.liu@freescale.com>
> 
> Well, funny story... pretty much as soon as we removed the 
> shadow TLB on
> 440, we put it back. But we use it differently now. Our experience on
> 440 was that it was a performance win to *also* keep a shadow 
> TLB array,
> but use that *only* in the vcpu_put/vcpu_load() path instead 
> of tlbia().
> In other words, instead of just clearing all that guest TLB state you
> worked so hard to build up, preserve it across host context switches.
> 
> We don't use that array for anything other than host context switches.
> For normal exits (handled in kernel or handled in qemu) we take a less
> precise "shadowy" approach, where host and guest TLB entries 
> can compete
> without KVM's knowledge, and we indiscriminately invalidate them.
> 
> (Yes, http://www.linux-kvm.org/page/PowerPC_Book_E_MMU is no longer
> completely accurate.)
> 
> I'm not sure what the coherence problems are that you allude 
> to, but we
> never did SMP host support for 440.

It's not a SMP coherence problem.
But that after we apply shadow id trick, the shadow tlb entry may be
expired due to the change of id mapping.
As we always update PID0, PID1 to the latest shadow id, write expired
shadow entry back is useless and may incur other problem.

Moreover after applying shadow id trick, there is no tlbia() in the
vcpu_load()/vcpu_put().
So I think it doesn't affect the performance of e500.



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2009-04-29  2:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-04-27  6:58 [PATCH 1/3] kvm/e500: Do not keep shadow tlb array Liu Yu
2009-04-28 19:33 ` Hollis Blanchard
2009-04-29  2:43 ` Liu Yu-B13201

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.