[PATCH 0/2] MIPS: KVM: fixes for KVM on ProAptiv cores

[PATCH 0/2] MIPS: KVM: fixes for KVM on ProAptiv cores

[James Hogan]

ProAptiv support includes support for EHINV (TLB invalidation) and FTLB
(large fixed page size TLBs), both of which cause problems when combined
with KVM. These two patches fix those problems.

These are based on John Crispin's mips-next-3.14 branch where ProAptiv
support is applied. Please consider applying these for v3.14 too.

Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Gleb Natapov <gleb@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org
Cc: Markos Chandras <markos.chandras@imgtec.com>
Cc: Sanjay Lal <sanjayl@kymasys.com>

James Hogan (2):
  MIPS: KVM: use common EHINV aware UNIQUE_ENTRYHI
  MIPS: KVM: remove shadow_tlb code

 arch/mips/include/asm/kvm_host.h |   7 --
 arch/mips/kvm/kvm_mips.c         |   1 -
 arch/mips/kvm/kvm_tlb.c          | 134 +--------------------------------------
 3 files changed, 1 insertion(+), 141 deletions(-)

-- 
1.8.1.2

[PATCH 1/2] MIPS: KVM: use common EHINV aware UNIQUE_ENTRYHI

[James Hogan]

When KVM is enabled and TLB invalidation is supported,
kvm_mips_flush_host_tlb() can cause a machine check exception due to
multiple matching TLB entries. This can occur on shutdown even when KVM
hasn't been actively used.

Commit adb78de9eae8 (MIPS: mm: Move UNIQUE_ENTRYHI macro to a header
file) created a common UNIQUE_ENTRYHI in asm/tlb.h but it didn't update
the copy of UNIQUE_ENTRYHI in kvm_tlb.c to use it.

Commit 36b175451399 (MIPS: tlb: Set the EHINV bit for TLBINVF cores when
invalidating the TLB) later added TLB invalidation (EHINV) support to
the common UNIQUE_ENTRYHI.

Therefore make kvm_tlb.c use the EHINV aware UNIQUE_ENTRYHI
implementation in asm/tlb.h too.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Gleb Natapov <gleb@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org
Cc: Markos Chandras <markos.chandras@imgtec.com>
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
This is based on John Crispin's mips-next-3.14 branch.

I do not object to it being squashed into commit adb78de9eae8 (MIPS: mm:
Move UNIQUE_ENTRYHI macro to a header file) since that commit hasn't
reached mainline yet.
---
 arch/mips/kvm/kvm_tlb.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
index c777dd36d4a8..52083ea7fddd 100644
--- a/arch/mips/kvm/kvm_tlb.c
+++ b/arch/mips/kvm/kvm_tlb.c
@@ -25,6 +25,7 @@
 #include <asm/mmu_context.h>
 #include <asm/pgtable.h>
 #include <asm/cacheflush.h>
+#include <asm/tlb.h>
 
 #undef CONFIG_MIPS_MT
 #include <asm/r4kcache.h>
@@ -35,9 +36,6 @@
 
 #define PRIx64 "llx"
 
-/* Use VZ EntryHi.EHINV to invalidate TLB entries */
-#define UNIQUE_ENTRYHI(idx) (CKSEG0 + ((idx) << (PAGE_SHIFT + 1)))
-
 atomic_t kvm_mips_instance;
 EXPORT_SYMBOL(kvm_mips_instance);
 
-- 
1.8.1.2

[PATCH 2/2] MIPS: KVM: remove shadow_tlb code

[James Hogan]

The initialisation of the shadow host TLB assumes the number of host TLB
entries <= 64, an assumption that can break in the presense of an FTLB
for which support was recently added. This results in an overflow and
amongst other things the comparecount timer callback function pointer
being overwritten which causes a lock up.

The shadow host TLB is actually disabled at present as it is not
necessary, so lets remove that code entirely.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: Gleb Natapov <gleb@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org
Cc: Sanjay Lal <sanjayl@kymasys.com>
---
This is based on John Crispin's mips-next-3.14 branch where FTLB support
is applied.
---
 arch/mips/include/asm/kvm_host.h |   7 ---
 arch/mips/kvm/kvm_mips.c         |   1 -
 arch/mips/kvm/kvm_tlb.c          | 130 ---------------------------------------
 3 files changed, 138 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 32966969f2f9..a995fce87791 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -391,9 +391,6 @@ struct kvm_vcpu_arch {
 	uint32_t guest_kernel_asid[NR_CPUS];
 	struct mm_struct guest_kernel_mm, guest_user_mm;
 
-	struct kvm_mips_tlb shadow_tlb[NR_CPUS][KVM_MIPS_GUEST_TLB_SIZE];
-
-
 	struct hrtimer comparecount_timer;
 
 	int last_sched_cpu;
@@ -529,7 +526,6 @@ extern enum emulation_result kvm_mips_handle_tlbmod(unsigned long cause,
 
 extern void kvm_mips_dump_host_tlbs(void);
 extern void kvm_mips_dump_guest_tlbs(struct kvm_vcpu *vcpu);
-extern void kvm_mips_dump_shadow_tlbs(struct kvm_vcpu *vcpu);
 extern void kvm_mips_flush_host_tlb(int skip_kseg0);
 extern int kvm_mips_host_tlb_inv(struct kvm_vcpu *vcpu, unsigned long entryhi);
 extern int kvm_mips_host_tlb_inv_index(struct kvm_vcpu *vcpu, int index);
@@ -541,10 +537,7 @@ extern unsigned long kvm_mips_translate_guest_kseg0_to_hpa(struct kvm_vcpu *vcpu
 						   unsigned long gva);
 extern void kvm_get_new_mmu_context(struct mm_struct *mm, unsigned long cpu,
 				    struct kvm_vcpu *vcpu);
-extern void kvm_shadow_tlb_put(struct kvm_vcpu *vcpu);
-extern void kvm_shadow_tlb_load(struct kvm_vcpu *vcpu);
 extern void kvm_local_flush_tlb_all(void);
-extern void kvm_mips_init_shadow_tlb(struct kvm_vcpu *vcpu);
 extern void kvm_mips_alloc_new_mmu_context(struct kvm_vcpu *vcpu);
 extern void kvm_mips_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
 extern void kvm_mips_vcpu_put(struct kvm_vcpu *vcpu);
diff --git a/arch/mips/kvm/kvm_mips.c b/arch/mips/kvm/kvm_mips.c
index 73b34827826c..da5186fbd77a 100644
--- a/arch/mips/kvm/kvm_mips.c
+++ b/arch/mips/kvm/kvm_mips.c
@@ -1001,7 +1001,6 @@ int kvm_arch_vcpu_init(struct kvm_vcpu *vcpu)
 	hrtimer_init(&vcpu->arch.comparecount_timer, CLOCK_MONOTONIC,
 		     HRTIMER_MODE_REL);
 	vcpu->arch.comparecount_timer.function = kvm_mips_comparecount_wakeup;
-	kvm_mips_init_shadow_tlb(vcpu);
 	return 0;
 }
 
diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
index 52083ea7fddd..68e6563915cd 100644
--- a/arch/mips/kvm/kvm_tlb.c
+++ b/arch/mips/kvm/kvm_tlb.c
@@ -145,30 +145,6 @@ void kvm_mips_dump_guest_tlbs(struct kvm_vcpu *vcpu)
 	}
 }
 
-void kvm_mips_dump_shadow_tlbs(struct kvm_vcpu *vcpu)
-{
-	int i;
-	volatile struct kvm_mips_tlb tlb;
-
-	printk("Shadow TLBs:\n");
-	for (i = 0; i < KVM_MIPS_GUEST_TLB_SIZE; i++) {
-		tlb = vcpu->arch.shadow_tlb[smp_processor_id()][i];
-		printk("TLB%c%3d Hi 0x%08lx ",
-		       (tlb.tlb_lo0 | tlb.tlb_lo1) & MIPS3_PG_V ? ' ' : '*',
-		       i, tlb.tlb_hi);
-		printk("Lo0=0x%09" PRIx64 " %c%c attr %lx ",
-		       (uint64_t) mips3_tlbpfn_to_paddr(tlb.tlb_lo0),
-		       (tlb.tlb_lo0 & MIPS3_PG_D) ? 'D' : ' ',
-		       (tlb.tlb_lo0 & MIPS3_PG_G) ? 'G' : ' ',
-		       (tlb.tlb_lo0 >> 3) & 7);
-		printk("Lo1=0x%09" PRIx64 " %c%c attr %lx sz=%lx\n",
-		       (uint64_t) mips3_tlbpfn_to_paddr(tlb.tlb_lo1),
-		       (tlb.tlb_lo1 & MIPS3_PG_D) ? 'D' : ' ',
-		       (tlb.tlb_lo1 & MIPS3_PG_G) ? 'G' : ' ',
-		       (tlb.tlb_lo1 >> 3) & 7, tlb.tlb_mask);
-	}
-}
-
 static int kvm_mips_map_page(struct kvm *kvm, gfn_t gfn)
 {
 	int srcu_idx, err = 0;
@@ -655,70 +631,6 @@ kvm_get_new_mmu_context(struct mm_struct *mm, unsigned long cpu,
 	cpu_context(cpu, mm) = asid_cache(cpu) = asid;
 }
 
-void kvm_shadow_tlb_put(struct kvm_vcpu *vcpu)
-{
-	unsigned long flags;
-	unsigned long old_entryhi;
-	unsigned long old_pagemask;
-	int entry = 0;
-	int cpu = smp_processor_id();
-
-	local_irq_save(flags);
-
-	old_entryhi = read_c0_entryhi();
-	old_pagemask = read_c0_pagemask();
-
-	for (entry = 0; entry < current_cpu_data.tlbsize; entry++) {
-		write_c0_index(entry);
-		mtc0_tlbw_hazard();
-		tlb_read();
-		tlbw_use_hazard();
-
-		vcpu->arch.shadow_tlb[cpu][entry].tlb_hi = read_c0_entryhi();
-		vcpu->arch.shadow_tlb[cpu][entry].tlb_lo0 = read_c0_entrylo0();
-		vcpu->arch.shadow_tlb[cpu][entry].tlb_lo1 = read_c0_entrylo1();
-		vcpu->arch.shadow_tlb[cpu][entry].tlb_mask = read_c0_pagemask();
-	}
-
-	write_c0_entryhi(old_entryhi);
-	write_c0_pagemask(old_pagemask);
-	mtc0_tlbw_hazard();
-
-	local_irq_restore(flags);
-
-}
-
-void kvm_shadow_tlb_load(struct kvm_vcpu *vcpu)
-{
-	unsigned long flags;
-	unsigned long old_ctx;
-	int entry;
-	int cpu = smp_processor_id();
-
-	local_irq_save(flags);
-
-	old_ctx = read_c0_entryhi();
-
-	for (entry = 0; entry < current_cpu_data.tlbsize; entry++) {
-		write_c0_entryhi(vcpu->arch.shadow_tlb[cpu][entry].tlb_hi);
-		mtc0_tlbw_hazard();
-		write_c0_entrylo0(vcpu->arch.shadow_tlb[cpu][entry].tlb_lo0);
-		write_c0_entrylo1(vcpu->arch.shadow_tlb[cpu][entry].tlb_lo1);
-
-		write_c0_index(entry);
-		mtc0_tlbw_hazard();
-
-		tlb_write_indexed();
-		tlbw_use_hazard();
-	}
-
-	tlbw_use_hazard();
-	write_c0_entryhi(old_ctx);
-	mtc0_tlbw_hazard();
-	local_irq_restore(flags);
-}
-
-
 void kvm_local_flush_tlb_all(void)
 {
 	unsigned long flags;
@@ -747,30 +659,6 @@ void kvm_local_flush_tlb_all(void)
 	local_irq_restore(flags);
 }
 
-void kvm_mips_init_shadow_tlb(struct kvm_vcpu *vcpu)
-{
-	int cpu, entry;
-
-	for_each_possible_cpu(cpu) {
-		for (entry = 0; entry < current_cpu_data.tlbsize; entry++) {
-			vcpu->arch.shadow_tlb[cpu][entry].tlb_hi =
-			    UNIQUE_ENTRYHI(entry);
-			vcpu->arch.shadow_tlb[cpu][entry].tlb_lo0 = 0x0;
-			vcpu->arch.shadow_tlb[cpu][entry].tlb_lo1 = 0x0;
-			vcpu->arch.shadow_tlb[cpu][entry].tlb_mask =
-			    read_c0_pagemask();
-#ifdef DEBUG
-			kvm_debug
-			    ("shadow_tlb[%d][%d]: tlb_hi: %#lx, lo0: %#lx, lo1: %#lx\n",
-			     cpu, entry,
-			     vcpu->arch.shadow_tlb[cpu][entry].tlb_hi,
-			     vcpu->arch.shadow_tlb[cpu][entry].tlb_lo0,
-			     vcpu->arch.shadow_tlb[cpu][entry].tlb_lo1);
-#endif
-		}
-	}
-}
-
 /* Restore ASID once we are scheduled back after preemption */
 void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 {
@@ -808,14 +696,6 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 			 vcpu->arch.last_sched_cpu, cpu, vcpu->vcpu_id);
 	}
 
-	/* Only reload shadow host TLB if new ASIDs haven't been allocated */
-#if 0
-	if ((atomic_read(&kvm_mips_instance) > 1) && !newasid) {
-		kvm_mips_flush_host_tlb(0);
-		kvm_shadow_tlb_load(vcpu);
-	}
-#endif
-
 	if (!newasid) {
 		/* If we preempted while the guest was executing, then reload the pre-empted ASID */
 		if (current->flags & PF_VCPU) {
@@ -861,12 +741,6 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
 	vcpu->arch.preempt_entryhi = read_c0_entryhi();
 	vcpu->arch.last_sched_cpu = cpu;
 
-#if 0
-	if ((atomic_read(&kvm_mips_instance) > 1)) {
-		kvm_shadow_tlb_put(vcpu);
-	}
-#endif
-
 	if (((cpu_context(cpu, current->mm) ^ asid_cache(cpu)) &
 	     ASID_VERSION_MASK)) {
 		kvm_debug("%s: Dropping MMU Context:  %#lx\n", __func__,
@@ -928,10 +802,8 @@ uint32_t kvm_get_inst(uint32_t *opc, struct kvm_vcpu *vcpu)
 }
 
 EXPORT_SYMBOL(kvm_local_flush_tlb_all);
-EXPORT_SYMBOL(kvm_shadow_tlb_put);
 EXPORT_SYMBOL(kvm_mips_handle_mapped_seg_tlb_fault);
 EXPORT_SYMBOL(kvm_mips_handle_commpage_tlb_fault);
-EXPORT_SYMBOL(kvm_mips_init_shadow_tlb);
 EXPORT_SYMBOL(kvm_mips_dump_host_tlbs);
 EXPORT_SYMBOL(kvm_mips_handle_kseg0_tlb_fault);
 EXPORT_SYMBOL(kvm_mips_host_tlb_lookup);
@@ -939,8 +811,6 @@ EXPORT_SYMBOL(kvm_mips_flush_host_tlb);
 EXPORT_SYMBOL(kvm_mips_guest_tlb_lookup);
 EXPORT_SYMBOL(kvm_mips_host_tlb_inv);
 EXPORT_SYMBOL(kvm_mips_translate_guest_kseg0_to_hpa);
-EXPORT_SYMBOL(kvm_shadow_tlb_load);
-EXPORT_SYMBOL(kvm_mips_dump_shadow_tlbs);
 EXPORT_SYMBOL(kvm_mips_dump_guest_tlbs);
 EXPORT_SYMBOL(kvm_get_inst);
 EXPORT_SYMBOL(kvm_arch_vcpu_load);
-- 
1.8.1.2

Re: [PATCH 1/2] MIPS: KVM: use common EHINV aware UNIQUE_ENTRYHI

[Markos Chandras]

On 01/15/2014 10:11 AM, James Hogan wrote:
> When KVM is enabled and TLB invalidation is supported,
> kvm_mips_flush_host_tlb() can cause a machine check exception due to
> multiple matching TLB entries. This can occur on shutdown even when KVM
> hasn't been actively used.
>
> Commit adb78de9eae8 (MIPS: mm: Move UNIQUE_ENTRYHI macro to a header
> file) created a common UNIQUE_ENTRYHI in asm/tlb.h but it didn't update
> the copy of UNIQUE_ENTRYHI in kvm_tlb.c to use it.
>
> Commit 36b175451399 (MIPS: tlb: Set the EHINV bit for TLBINVF cores when
> invalidating the TLB) later added TLB invalidation (EHINV) support to
> the common UNIQUE_ENTRYHI.
>
> Therefore make kvm_tlb.c use the EHINV aware UNIQUE_ENTRYHI
> implementation in asm/tlb.h too.
>
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> Cc: Gleb Natapov <gleb@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: kvm@vger.kernel.org
> Cc: Markos Chandras <markos.chandras@imgtec.com>
> Cc: Sanjay Lal <sanjayl@kymasys.com>
> ---
> This is based on John Crispin's mips-next-3.14 branch.
>
> I do not object to it being squashed into commit adb78de9eae8 (MIPS: mm:
> Move UNIQUE_ENTRYHI macro to a header file) since that commit hasn't
> reached mainline yet.
> ---
>   arch/mips/kvm/kvm_tlb.c | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/arch/mips/kvm/kvm_tlb.c b/arch/mips/kvm/kvm_tlb.c
> index c777dd36d4a8..52083ea7fddd 100644
> --- a/arch/mips/kvm/kvm_tlb.c
> +++ b/arch/mips/kvm/kvm_tlb.c
> @@ -25,6 +25,7 @@
>   #include <asm/mmu_context.h>
>   #include <asm/pgtable.h>
>   #include <asm/cacheflush.h>
> +#include <asm/tlb.h>
>
>   #undef CONFIG_MIPS_MT
>   #include <asm/r4kcache.h>
> @@ -35,9 +36,6 @@
>
>   #define PRIx64 "llx"
>
> -/* Use VZ EntryHi.EHINV to invalidate TLB entries */
> -#define UNIQUE_ENTRYHI(idx) (CKSEG0 + ((idx) << (PAGE_SHIFT + 1)))
> -
>   atomic_t kvm_mips_instance;
>   EXPORT_SYMBOL(kvm_mips_instance);
>
>

Thanks. That looks good to me.

Reviewed-by: Markos Chandras <markos.chandras@imgtec.com>

-- 
markos

Re: [PATCH 0/2] MIPS: KVM: fixes for KVM on ProAptiv cores

[Paolo Bonzini]

Il 15/01/2014 11:11, James Hogan ha scritto:
> ProAptiv support includes support for EHINV (TLB invalidation) and FTLB
> (large fixed page size TLBs), both of which cause problems when combined
> with KVM. These two patches fix those problems.
> 
> These are based on John Crispin's mips-next-3.14 branch where ProAptiv
> support is applied. Please consider applying these for v3.14 too.
> 
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> Cc: Gleb Natapov <gleb@redhat.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: kvm@vger.kernel.org
> Cc: Markos Chandras <markos.chandras@imgtec.com>
> Cc: Sanjay Lal <sanjayl@kymasys.com>
> 
> James Hogan (2):
>   MIPS: KVM: use common EHINV aware UNIQUE_ENTRYHI
>   MIPS: KVM: remove shadow_tlb code
> 
>  arch/mips/include/asm/kvm_host.h |   7 --
>  arch/mips/kvm/kvm_mips.c         |   1 -
>  arch/mips/kvm/kvm_tlb.c          | 134 +--------------------------------------
>  3 files changed, 1 insertion(+), 141 deletions(-)
> 

Acked-by: Paolo Bonzini <pbonzini@redhat.com>