All of lore.kernel.org
 help / color / mirror / Atom feed
* [BACKPORT PATCH 3.10..3.16] KVM: MIPS: Drop other CPU ASIDs on guest MMU changes
@ 2016-11-09 14:46 ` James Hogan
  0 siblings, 0 replies; 13+ messages in thread
From: James Hogan @ 2016-11-09 14:46 UTC (permalink / raw)
  To: Jiri Slaby, stable
  Cc: Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, linux-mips, kvm, James Hogan

commit 91e4f1b6073dd680d86cdb7e42d7cccca9db39d8 upstream.

When a guest TLB entry is replaced by TLBWI or TLBWR, we only invalidate
TLB entries on the local CPU. This doesn't work correctly on an SMP host
when the guest is migrated to a different physical CPU, as it could pick
up stale TLB mappings from the last time the vCPU ran on that physical
CPU.

Therefore invalidate both user and kernel host ASIDs on other CPUs,
which will cause new ASIDs to be generated when it next runs on those
CPUs.

We're careful only to do this if the TLB entry was already valid, and
only for the kernel ASID where the virtual address it mapped is outside
of the guest user address range.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Radim Krčmář" <rkrcmar@redhat.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: kvm@vger.kernel.org
Cc: <stable@vger.kernel.org> # 3.10.x-
Cc: Jiri Slaby <jslaby@suse.cz>
[james.hogan@imgtec.com: Backport to 3.10..3.16]
Signed-off-by: James Hogan <james.hogan@imgtec.com>
---
Unfortunately the original commit went in to v3.12.65 as commit
168e5ebbd63e, without fixing up the references to tlb_lo[0/1] to
tlb_lo0/1 which broke the MIPS KVM build, and I didn't twig that I
already had a correct backport outstanding (sorry!). That commit should
be reverted before applying this backport to 3.12.
---
 arch/mips/kvm/kvm_mips_emul.c | 61 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 53 insertions(+), 8 deletions(-)

diff --git a/arch/mips/kvm/kvm_mips_emul.c b/arch/mips/kvm/kvm_mips_emul.c
index 1983678883c9..d0eb34279955 100644
--- a/arch/mips/kvm/kvm_mips_emul.c
+++ b/arch/mips/kvm/kvm_mips_emul.c
@@ -817,6 +817,47 @@ enum emulation_result kvm_mips_emul_tlbr(struct kvm_vcpu *vcpu)
 	return er;
 }
 
+/**
+ * kvm_mips_invalidate_guest_tlb() - Indicates a change in guest MMU map.
+ * @vcpu:	VCPU with changed mappings.
+ * @tlb:	TLB entry being removed.
+ *
+ * This is called to indicate a single change in guest MMU mappings, so that we
+ * can arrange TLB flushes on this and other CPUs.
+ */
+static void kvm_mips_invalidate_guest_tlb(struct kvm_vcpu *vcpu,
+					  struct kvm_mips_tlb *tlb)
+{
+	int cpu, i;
+	bool user;
+
+	/* No need to flush for entries which are already invalid */
+	if (!((tlb->tlb_lo0 | tlb->tlb_lo1) & MIPS3_PG_V))
+		return;
+	/* User address space doesn't need flushing for KSeg2/3 changes */
+	user = tlb->tlb_hi < KVM_GUEST_KSEG0;
+
+	preempt_disable();
+
+	/*
+	 * Probe the shadow host TLB for the entry being overwritten, if one
+	 * matches, invalidate it
+	 */
+	kvm_mips_host_tlb_inv(vcpu, tlb->tlb_hi);
+
+	/* Invalidate the whole ASID on other CPUs */
+	cpu = smp_processor_id();
+	for_each_possible_cpu(i) {
+		if (i == cpu)
+			continue;
+		if (user)
+			vcpu->arch.guest_user_asid[i] = 0;
+		vcpu->arch.guest_kernel_asid[i] = 0;
+	}
+
+	preempt_enable();
+}
+
 /* Write Guest TLB Entry @ Index */
 enum emulation_result kvm_mips_emul_tlbwi(struct kvm_vcpu *vcpu)
 {
@@ -838,10 +879,8 @@ enum emulation_result kvm_mips_emul_tlbwi(struct kvm_vcpu *vcpu)
 	}
 
 	tlb = &vcpu->arch.guest_tlb[index];
-#if 1
-	/* Probe the shadow host TLB for the entry being overwritten, if one matches, invalidate it */
-	kvm_mips_host_tlb_inv(vcpu, tlb->tlb_hi);
-#endif
+
+	kvm_mips_invalidate_guest_tlb(vcpu, tlb);
 
 	tlb->tlb_mask = kvm_read_c0_guest_pagemask(cop0);
 	tlb->tlb_hi = kvm_read_c0_guest_entryhi(cop0);
@@ -880,10 +919,7 @@ enum emulation_result kvm_mips_emul_tlbwr(struct kvm_vcpu *vcpu)
 
 	tlb = &vcpu->arch.guest_tlb[index];
 
-#if 1
-	/* Probe the shadow host TLB for the entry being overwritten, if one matches, invalidate it */
-	kvm_mips_host_tlb_inv(vcpu, tlb->tlb_hi);
-#endif
+	kvm_mips_invalidate_guest_tlb(vcpu, tlb);
 
 	tlb->tlb_mask = kvm_read_c0_guest_pagemask(cop0);
 	tlb->tlb_hi = kvm_read_c0_guest_entryhi(cop0);
@@ -926,6 +962,7 @@ kvm_mips_emulate_CP0(uint32_t inst, uint32_t *opc, uint32_t cause,
 	int32_t rt, rd, copz, sel, co_bit, op;
 	uint32_t pc = vcpu->arch.pc;
 	unsigned long curr_pc;
+	int cpu, i;
 
 	/*
 	 * Update PC and hold onto current PC in case there is
@@ -1037,8 +1074,16 @@ kvm_mips_emulate_CP0(uint32_t inst, uint32_t *opc, uint32_t cause,
 					     ASID_MASK,
 					     vcpu->arch.gprs[rt] & ASID_MASK);
 
+					preempt_disable();
 					/* Blow away the shadow host TLBs */
 					kvm_mips_flush_host_tlb(1);
+					cpu = smp_processor_id();
+					for_each_possible_cpu(i)
+						if (i != cpu) {
+							vcpu->arch.guest_user_asid[i] = 0;
+							vcpu->arch.guest_kernel_asid[i] = 0;
+						}
+					preempt_enable();
 				}
 				kvm_write_c0_guest_entryhi(cop0,
 							   vcpu->arch.gprs[rt]);
-- 
2.10.1


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

* [BACKPORT PATCH 3.10..3.16] KVM: MIPS: Drop other CPU ASIDs on guest MMU changes
@ 2016-11-09 14:46 ` James Hogan
  0 siblings, 0 replies; 13+ messages in thread
From: James Hogan @ 2016-11-09 14:46 UTC (permalink / raw)
  To: Jiri Slaby, stable
  Cc: Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, linux-mips, kvm, James Hogan

commit 91e4f1b6073dd680d86cdb7e42d7cccca9db39d8 upstream.

When a guest TLB entry is replaced by TLBWI or TLBWR, we only invalidate
TLB entries on the local CPU. This doesn't work correctly on an SMP host
when the guest is migrated to a different physical CPU, as it could pick
up stale TLB mappings from the last time the vCPU ran on that physical
CPU.

Therefore invalidate both user and kernel host ASIDs on other CPUs,
which will cause new ASIDs to be generated when it next runs on those
CPUs.

We're careful only to do this if the TLB entry was already valid, and
only for the kernel ASID where the virtual address it mapped is outside
of the guest user address range.

Signed-off-by: James Hogan <james.hogan@imgtec.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: "Radim Krčmář" <rkrcmar@redhat.com>
Cc: Ralf Baechle <ralf@linux-mips.org>
Cc: linux-mips@linux-mips.org
Cc: kvm@vger.kernel.org
Cc: <stable@vger.kernel.org> # 3.10.x-
Cc: Jiri Slaby <jslaby@suse.cz>
[james.hogan@imgtec.com: Backport to 3.10..3.16]
Signed-off-by: James Hogan <james.hogan@imgtec.com>
---
Unfortunately the original commit went in to v3.12.65 as commit
168e5ebbd63e, without fixing up the references to tlb_lo[0/1] to
tlb_lo0/1 which broke the MIPS KVM build, and I didn't twig that I
already had a correct backport outstanding (sorry!). That commit should
be reverted before applying this backport to 3.12.
---
 arch/mips/kvm/kvm_mips_emul.c | 61 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 53 insertions(+), 8 deletions(-)

diff --git a/arch/mips/kvm/kvm_mips_emul.c b/arch/mips/kvm/kvm_mips_emul.c
index 1983678883c9..d0eb34279955 100644
--- a/arch/mips/kvm/kvm_mips_emul.c
+++ b/arch/mips/kvm/kvm_mips_emul.c
@@ -817,6 +817,47 @@ enum emulation_result kvm_mips_emul_tlbr(struct kvm_vcpu *vcpu)
 	return er;
 }
 
+/**
+ * kvm_mips_invalidate_guest_tlb() - Indicates a change in guest MMU map.
+ * @vcpu:	VCPU with changed mappings.
+ * @tlb:	TLB entry being removed.
+ *
+ * This is called to indicate a single change in guest MMU mappings, so that we
+ * can arrange TLB flushes on this and other CPUs.
+ */
+static void kvm_mips_invalidate_guest_tlb(struct kvm_vcpu *vcpu,
+					  struct kvm_mips_tlb *tlb)
+{
+	int cpu, i;
+	bool user;
+
+	/* No need to flush for entries which are already invalid */
+	if (!((tlb->tlb_lo0 | tlb->tlb_lo1) & MIPS3_PG_V))
+		return;
+	/* User address space doesn't need flushing for KSeg2/3 changes */
+	user = tlb->tlb_hi < KVM_GUEST_KSEG0;
+
+	preempt_disable();
+
+	/*
+	 * Probe the shadow host TLB for the entry being overwritten, if one
+	 * matches, invalidate it
+	 */
+	kvm_mips_host_tlb_inv(vcpu, tlb->tlb_hi);
+
+	/* Invalidate the whole ASID on other CPUs */
+	cpu = smp_processor_id();
+	for_each_possible_cpu(i) {
+		if (i == cpu)
+			continue;
+		if (user)
+			vcpu->arch.guest_user_asid[i] = 0;
+		vcpu->arch.guest_kernel_asid[i] = 0;
+	}
+
+	preempt_enable();
+}
+
 /* Write Guest TLB Entry @ Index */
 enum emulation_result kvm_mips_emul_tlbwi(struct kvm_vcpu *vcpu)
 {
@@ -838,10 +879,8 @@ enum emulation_result kvm_mips_emul_tlbwi(struct kvm_vcpu *vcpu)
 	}
 
 	tlb = &vcpu->arch.guest_tlb[index];
-#if 1
-	/* Probe the shadow host TLB for the entry being overwritten, if one matches, invalidate it */
-	kvm_mips_host_tlb_inv(vcpu, tlb->tlb_hi);
-#endif
+
+	kvm_mips_invalidate_guest_tlb(vcpu, tlb);
 
 	tlb->tlb_mask = kvm_read_c0_guest_pagemask(cop0);
 	tlb->tlb_hi = kvm_read_c0_guest_entryhi(cop0);
@@ -880,10 +919,7 @@ enum emulation_result kvm_mips_emul_tlbwr(struct kvm_vcpu *vcpu)
 
 	tlb = &vcpu->arch.guest_tlb[index];
 
-#if 1
-	/* Probe the shadow host TLB for the entry being overwritten, if one matches, invalidate it */
-	kvm_mips_host_tlb_inv(vcpu, tlb->tlb_hi);
-#endif
+	kvm_mips_invalidate_guest_tlb(vcpu, tlb);
 
 	tlb->tlb_mask = kvm_read_c0_guest_pagemask(cop0);
 	tlb->tlb_hi = kvm_read_c0_guest_entryhi(cop0);
@@ -926,6 +962,7 @@ kvm_mips_emulate_CP0(uint32_t inst, uint32_t *opc, uint32_t cause,
 	int32_t rt, rd, copz, sel, co_bit, op;
 	uint32_t pc = vcpu->arch.pc;
 	unsigned long curr_pc;
+	int cpu, i;
 
 	/*
 	 * Update PC and hold onto current PC in case there is
@@ -1037,8 +1074,16 @@ kvm_mips_emulate_CP0(uint32_t inst, uint32_t *opc, uint32_t cause,
 					     ASID_MASK,
 					     vcpu->arch.gprs[rt] & ASID_MASK);
 
+					preempt_disable();
 					/* Blow away the shadow host TLBs */
 					kvm_mips_flush_host_tlb(1);
+					cpu = smp_processor_id();
+					for_each_possible_cpu(i)
+						if (i != cpu) {
+							vcpu->arch.guest_user_asid[i] = 0;
+							vcpu->arch.guest_kernel_asid[i] = 0;
+						}
+					preempt_enable();
 				}
 				kvm_write_c0_guest_entryhi(cop0,
 							   vcpu->arch.gprs[rt]);
-- 
2.10.1

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

* Re: [BACKPORT PATCH 3.10..3.16] KVM: MIPS: Drop other CPU ASIDs on guest MMU changes
  2016-11-09 14:46 ` James Hogan
  (?)
@ 2016-11-09 17:34 ` Willy Tarreau
  -1 siblings, 0 replies; 13+ messages in thread
From: Willy Tarreau @ 2016-11-09 17:34 UTC (permalink / raw)
  To: James Hogan
  Cc: Jiri Slaby, stable, Paolo Bonzini, Radim Kr??má??,
	Ralf Baechle, linux-mips, kvm

On Wed, Nov 09, 2016 at 02:46:24PM +0000, James Hogan wrote:
> commit 91e4f1b6073dd680d86cdb7e42d7cccca9db39d8 upstream.
(...)

And queued as well for 3.10, thanks James!

Willy

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

* Re: [BACKPORT PATCH 3.10..3.16] KVM: MIPS: Drop other CPU ASIDs on guest MMU changes
  2016-11-09 14:46 ` James Hogan
  (?)
  (?)
@ 2016-11-09 21:22 ` Jiri Slaby
  2016-11-09 22:00   ` James Hogan
  -1 siblings, 1 reply; 13+ messages in thread
From: Jiri Slaby @ 2016-11-09 21:22 UTC (permalink / raw)
  To: James Hogan, stable
  Cc: Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, linux-mips, kvm

On 11/09/2016, 03:46 PM, James Hogan wrote:
> commit 91e4f1b6073dd680d86cdb7e42d7cccca9db39d8 upstream.
> 
> When a guest TLB entry is replaced by TLBWI or TLBWR, we only invalidate
> TLB entries on the local CPU. This doesn't work correctly on an SMP host
> when the guest is migrated to a different physical CPU, as it could pick
> up stale TLB mappings from the last time the vCPU ran on that physical
> CPU.
> 
> Therefore invalidate both user and kernel host ASIDs on other CPUs,
> which will cause new ASIDs to be generated when it next runs on those
> CPUs.
> 
> We're careful only to do this if the TLB entry was already valid, and
> only for the kernel ASID where the virtual address it mapped is outside
> of the guest user address range.
> 
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Radim Krčmář" <rkrcmar@redhat.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> Cc: kvm@vger.kernel.org
> Cc: <stable@vger.kernel.org> # 3.10.x-
> Cc: Jiri Slaby <jslaby@suse.cz>
> [james.hogan@imgtec.com: Backport to 3.10..3.16]
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> ---
> Unfortunately the original commit went in to v3.12.65 as commit
> 168e5ebbd63e, without fixing up the references to tlb_lo[0/1] to
> tlb_lo0/1 which broke the MIPS KVM build, and I didn't twig that I
> already had a correct backport outstanding (sorry!). That commit should
> be reverted before applying this backport to 3.12.

Thanks, reverted and applied. I wonder the builders didn't break given 4
mips configurations are tested. I indeed could reproduce locally.


-- 
js
suse labs

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

* Re: [BACKPORT PATCH 3.10..3.16] KVM: MIPS: Drop other CPU ASIDs on guest MMU changes
  2016-11-09 21:22 ` Jiri Slaby
@ 2016-11-09 22:00   ` James Hogan
  2016-11-10  6:08     ` Greg KH
  0 siblings, 1 reply; 13+ messages in thread
From: James Hogan @ 2016-11-09 22:00 UTC (permalink / raw)
  To: Jiri Slaby
  Cc: stable, Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, linux-mips, kvm

[-- Attachment #1: Type: text/plain, Size: 1987 bytes --]

On Wed, Nov 09, 2016 at 10:22:01PM +0100, Jiri Slaby wrote:
> On 11/09/2016, 03:46 PM, James Hogan wrote:
> > commit 91e4f1b6073dd680d86cdb7e42d7cccca9db39d8 upstream.
> > 
> > When a guest TLB entry is replaced by TLBWI or TLBWR, we only invalidate
> > TLB entries on the local CPU. This doesn't work correctly on an SMP host
> > when the guest is migrated to a different physical CPU, as it could pick
> > up stale TLB mappings from the last time the vCPU ran on that physical
> > CPU.
> > 
> > Therefore invalidate both user and kernel host ASIDs on other CPUs,
> > which will cause new ASIDs to be generated when it next runs on those
> > CPUs.
> > 
> > We're careful only to do this if the TLB entry was already valid, and
> > only for the kernel ASID where the virtual address it mapped is outside
> > of the guest user address range.
> > 
> > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: "Radim Krčmář" <rkrcmar@redhat.com>
> > Cc: Ralf Baechle <ralf@linux-mips.org>
> > Cc: linux-mips@linux-mips.org
> > Cc: kvm@vger.kernel.org
> > Cc: <stable@vger.kernel.org> # 3.10.x-
> > Cc: Jiri Slaby <jslaby@suse.cz>
> > [james.hogan@imgtec.com: Backport to 3.10..3.16]
> > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > ---
> > Unfortunately the original commit went in to v3.12.65 as commit
> > 168e5ebbd63e, without fixing up the references to tlb_lo[0/1] to
> > tlb_lo0/1 which broke the MIPS KVM build, and I didn't twig that I
> > already had a correct backport outstanding (sorry!). That commit should
> > be reverted before applying this backport to 3.12.
> 
> Thanks, reverted and applied. I wonder the builders didn't break given 4
> mips configurations are tested. I indeed could reproduce locally.

I'm guessing malta_kvm_defconfig isn't one of those defconfigs (and the
imgtec buildbots don't yet test stable branches). Which builders do you
use?

Thanks
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [BACKPORT PATCH 3.10..3.16] KVM: MIPS: Drop other CPU ASIDs on guest MMU changes
  2016-11-09 22:00   ` James Hogan
@ 2016-11-10  6:08     ` Greg KH
  2016-11-10 17:37         ` James Hogan
  0 siblings, 1 reply; 13+ messages in thread
From: Greg KH @ 2016-11-10  6:08 UTC (permalink / raw)
  To: James Hogan
  Cc: Jiri Slaby, stable, Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, linux-mips, kvm

On Wed, Nov 09, 2016 at 10:00:43PM +0000, James Hogan wrote:
> On Wed, Nov 09, 2016 at 10:22:01PM +0100, Jiri Slaby wrote:
> > On 11/09/2016, 03:46 PM, James Hogan wrote:
> > > commit 91e4f1b6073dd680d86cdb7e42d7cccca9db39d8 upstream.
> > > 
> > > When a guest TLB entry is replaced by TLBWI or TLBWR, we only invalidate
> > > TLB entries on the local CPU. This doesn't work correctly on an SMP host
> > > when the guest is migrated to a different physical CPU, as it could pick
> > > up stale TLB mappings from the last time the vCPU ran on that physical
> > > CPU.
> > > 
> > > Therefore invalidate both user and kernel host ASIDs on other CPUs,
> > > which will cause new ASIDs to be generated when it next runs on those
> > > CPUs.
> > > 
> > > We're careful only to do this if the TLB entry was already valid, and
> > > only for the kernel ASID where the virtual address it mapped is outside
> > > of the guest user address range.
> > > 
> > > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > > Cc: "Radim Krčmář" <rkrcmar@redhat.com>
> > > Cc: Ralf Baechle <ralf@linux-mips.org>
> > > Cc: linux-mips@linux-mips.org
> > > Cc: kvm@vger.kernel.org
> > > Cc: <stable@vger.kernel.org> # 3.10.x-
> > > Cc: Jiri Slaby <jslaby@suse.cz>
> > > [james.hogan@imgtec.com: Backport to 3.10..3.16]
> > > Signed-off-by: James Hogan <james.hogan@imgtec.com>
> > > ---
> > > Unfortunately the original commit went in to v3.12.65 as commit
> > > 168e5ebbd63e, without fixing up the references to tlb_lo[0/1] to
> > > tlb_lo0/1 which broke the MIPS KVM build, and I didn't twig that I
> > > already had a correct backport outstanding (sorry!). That commit should
> > > be reverted before applying this backport to 3.12.
> > 
> > Thanks, reverted and applied. I wonder the builders didn't break given 4
> > mips configurations are tested. I indeed could reproduce locally.
> 
> I'm guessing malta_kvm_defconfig isn't one of those defconfigs (and the
> imgtec buildbots don't yet test stable branches). Which builders do you
> use?

I use 0-day for these types of things, and it is not showing up any
errors for the 4.4-stable kernel.  Can you get these configurations
added to it so that we can ensure it doesn't regress?

thanks,

greg k-h

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

* Re: [BACKPORT PATCH 3.10..3.16] KVM: MIPS: Drop other CPU ASIDs on guest MMU changes
@ 2016-11-10 17:37         ` James Hogan
  0 siblings, 0 replies; 13+ messages in thread
From: James Hogan @ 2016-11-10 17:37 UTC (permalink / raw)
  To: fengguang.wu
  Cc: Greg KH, Jiri Slaby, stable, Paolo Bonzini,
	Radim Krčmář,
	Ralf Baechle, linux-mips, kvm, Paul Burton

[-- Attachment #1: Type: text/plain, Size: 1847 bytes --]

Hi Fengguang,

On Thu, Nov 10, 2016 at 07:08:43AM +0100, Greg KH wrote:
> On Wed, Nov 09, 2016 at 10:00:43PM +0000, James Hogan wrote:
> > On Wed, Nov 09, 2016 at 10:22:01PM +0100, Jiri Slaby wrote:
> > > On 11/09/2016, 03:46 PM, James Hogan wrote:
> > > > Unfortunately the original commit went in to v3.12.65 as commit
> > > > 168e5ebbd63e, without fixing up the references to tlb_lo[0/1] to
> > > > tlb_lo0/1 which broke the MIPS KVM build, and I didn't twig that I
> > > > already had a correct backport outstanding (sorry!). That commit should
> > > > be reverted before applying this backport to 3.12.
> > > 
> > > Thanks, reverted and applied. I wonder the builders didn't break given 4
> > > mips configurations are tested. I indeed could reproduce locally.
> > 
> > I'm guessing malta_kvm_defconfig isn't one of those defconfigs (and the
> > imgtec buildbots don't yet test stable branches). Which builders do you
> > use?
> 
> I use 0-day for these types of things, and it is not showing up any
> errors for the 4.4-stable kernel.  Can you get these configurations
> added to it so that we can ensure it doesn't regress?

Can we please get a few MIPS defconfigs added to the 0-day testing?

- malta_kvm_defconfig
  this probably doesn't need to be a high priority build, but other
  configs don't yet cover MIPS KVM so its worth having (that bit us
  recently with 3.12 and 4.4 stable branches).

- 64r6el_defconfig and 32r2_defconfig (4.9 and later)
  these are just a couple of the new generic/multiplatform kernel
  configurations added in 4.9 (Paul Burton Cc'd). There are others too,
  but these will probably give decent coverage. These are likely to be
  increasingly relevant as more/new platforms are converted to use it.
  (note, the r6 one may require a newish toolchain).

Thanks
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [BACKPORT PATCH 3.10..3.16] KVM: MIPS: Drop other CPU ASIDs on guest MMU changes
@ 2016-11-10 17:37         ` James Hogan
  0 siblings, 0 replies; 13+ messages in thread
From: James Hogan @ 2016-11-10 17:37 UTC (permalink / raw)
  To: fengguang.wu
  Cc: Greg KH, Jiri Slaby, stable, Paolo Bonzini,
	Radim Krčmář,
	Ralf Baechle, linux-mips, kvm, Paul Burton

[-- Attachment #1: Type: text/plain, Size: 1847 bytes --]

Hi Fengguang,

On Thu, Nov 10, 2016 at 07:08:43AM +0100, Greg KH wrote:
> On Wed, Nov 09, 2016 at 10:00:43PM +0000, James Hogan wrote:
> > On Wed, Nov 09, 2016 at 10:22:01PM +0100, Jiri Slaby wrote:
> > > On 11/09/2016, 03:46 PM, James Hogan wrote:
> > > > Unfortunately the original commit went in to v3.12.65 as commit
> > > > 168e5ebbd63e, without fixing up the references to tlb_lo[0/1] to
> > > > tlb_lo0/1 which broke the MIPS KVM build, and I didn't twig that I
> > > > already had a correct backport outstanding (sorry!). That commit should
> > > > be reverted before applying this backport to 3.12.
> > > 
> > > Thanks, reverted and applied. I wonder the builders didn't break given 4
> > > mips configurations are tested. I indeed could reproduce locally.
> > 
> > I'm guessing malta_kvm_defconfig isn't one of those defconfigs (and the
> > imgtec buildbots don't yet test stable branches). Which builders do you
> > use?
> 
> I use 0-day for these types of things, and it is not showing up any
> errors for the 4.4-stable kernel.  Can you get these configurations
> added to it so that we can ensure it doesn't regress?

Can we please get a few MIPS defconfigs added to the 0-day testing?

- malta_kvm_defconfig
  this probably doesn't need to be a high priority build, but other
  configs don't yet cover MIPS KVM so its worth having (that bit us
  recently with 3.12 and 4.4 stable branches).

- 64r6el_defconfig and 32r2_defconfig (4.9 and later)
  these are just a couple of the new generic/multiplatform kernel
  configurations added in 4.9 (Paul Burton Cc'd). There are others too,
  but these will probably give decent coverage. These are likely to be
  increasingly relevant as more/new platforms are converted to use it.
  (note, the r6 one may require a newish toolchain).

Thanks
James

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [BACKPORT PATCH 3.10..3.16] KVM: MIPS: Drop other CPU ASIDs on guest MMU changes
  2016-11-10 17:37         ` James Hogan
  (?)
@ 2016-11-11  2:56         ` Fengguang Wu
  2016-11-11  8:42           ` Jiri Slaby
  2016-11-11  8:58           ` Paul Burton
  -1 siblings, 2 replies; 13+ messages in thread
From: Fengguang Wu @ 2016-11-11  2:56 UTC (permalink / raw)
  To: James Hogan
  Cc: Greg KH, Jiri Slaby, stable, Paolo Bonzini,
	Radim Krčmář,
	Ralf Baechle, linux-mips, kvm, Paul Burton

Hi James,

On Thu, Nov 10, 2016 at 05:37:22PM +0000, James Hogan wrote:
>Hi Fengguang,
>
>On Thu, Nov 10, 2016 at 07:08:43AM +0100, Greg KH wrote:
>> On Wed, Nov 09, 2016 at 10:00:43PM +0000, James Hogan wrote:
>> > On Wed, Nov 09, 2016 at 10:22:01PM +0100, Jiri Slaby wrote:
>> > > On 11/09/2016, 03:46 PM, James Hogan wrote:
>> > > > Unfortunately the original commit went in to v3.12.65 as commit
>> > > > 168e5ebbd63e, without fixing up the references to tlb_lo[0/1] to
>> > > > tlb_lo0/1 which broke the MIPS KVM build, and I didn't twig that I
>> > > > already had a correct backport outstanding (sorry!). That commit should
>> > > > be reverted before applying this backport to 3.12.
>> > >
>> > > Thanks, reverted and applied. I wonder the builders didn't break given 4
>> > > mips configurations are tested. I indeed could reproduce locally.
>> >
>> > I'm guessing malta_kvm_defconfig isn't one of those defconfigs (and the
>> > imgtec buildbots don't yet test stable branches). Which builders do you
>> > use?
>>
>> I use 0-day for these types of things, and it is not showing up any
>> errors for the 4.4-stable kernel.  Can you get these configurations
>> added to it so that we can ensure it doesn't regress?
>
>Can we please get a few MIPS defconfigs added to the 0-day testing?

The 0-day build bot should already cover the below configs (not
necessarily in the early hours, but very likely in the first day after
your git push) since they are included in arch/*/configs/. I wonder
where you test your patches?  Let me check how they missed the test
coverage.

>- malta_kvm_defconfig
>  this probably doesn't need to be a high priority build, but other
>  configs don't yet cover MIPS KVM so its worth having (that bit us
>  recently with 3.12 and 4.4 stable branches).
>
>- 64r6el_defconfig and 32r2_defconfig (4.9 and later)
>  these are just a couple of the new generic/multiplatform kernel
>  configurations added in 4.9 (Paul Burton Cc'd). There are others too,
>  but these will probably give decent coverage. These are likely to be
>  increasingly relevant as more/new platforms are converted to use it.
>  (note, the r6 one may require a newish toolchain).

Thanks,
Fengguang

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

* Re: [BACKPORT PATCH 3.10..3.16] KVM: MIPS: Drop other CPU ASIDs on guest MMU changes
  2016-11-11  2:56         ` Fengguang Wu
@ 2016-11-11  8:42           ` Jiri Slaby
  2016-11-11  8:58           ` Paul Burton
  1 sibling, 0 replies; 13+ messages in thread
From: Jiri Slaby @ 2016-11-11  8:42 UTC (permalink / raw)
  To: Fengguang Wu, James Hogan
  Cc: Greg KH, stable, Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, linux-mips, kvm, Paul Burton

On 11/11/2016, 03:56 AM, Fengguang Wu wrote:
>> Can we please get a few MIPS defconfigs added to the 0-day testing?
> 
> The 0-day build bot should already cover the below configs (not
> necessarily in the early hours, but very likely in the first day after
> your git push) since they are included in arch/*/configs/. I wonder
> where you test your patches?  Let me check how they missed the test
> coverage.

Hello, it is mostly about my stable-3.12 at:
git://git.kernel.org/pub/scm/linux/kernel/git/jirislaby/linux-stable

The status mail stated:
The following configs have been built successfully.
More configs may be tested in the coming days.
...
mips                                   jz4740
mips                              allnoconfig
mips                      fuloong2e_defconfig
mips                                     txx9

thanks,
-- 
js
suse labs

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

* Re: [BACKPORT PATCH 3.10..3.16] KVM: MIPS: Drop other CPU ASIDs on guest MMU changes
  2016-11-11  2:56         ` Fengguang Wu
  2016-11-11  8:42           ` Jiri Slaby
@ 2016-11-11  8:58           ` Paul Burton
  2016-11-11 23:11             ` Fengguang Wu
  1 sibling, 1 reply; 13+ messages in thread
From: Paul Burton @ 2016-11-11  8:58 UTC (permalink / raw)
  To: Fengguang Wu
  Cc: James Hogan, Greg KH, Jiri Slaby, stable, Paolo Bonzini,
	Radim Krčmář,
	Ralf Baechle, linux-mips, kvm

[-- Attachment #1: Type: text/plain, Size: 1718 bytes --]

Hi Fengguang,

On Friday, 11 November 2016 10:56:21 GMT Fengguang Wu wrote:
> The 0-day build bot should already cover the below configs (not
> necessarily in the early hours, but very likely in the first day after
> your git push) since they are included in arch/*/configs/.

<snip>

> >- malta_kvm_defconfig
> >
> >  this probably doesn't need to be a high priority build, but other
> >  configs don't yet cover MIPS KVM so its worth having (that bit us
> >  recently with 3.12 and 4.4 stable branches).
> >
> >- 64r6el_defconfig and 32r2_defconfig (4.9 and later)
> >
> >  these are just a couple of the new generic/multiplatform kernel
> >  configurations added in 4.9 (Paul Burton Cc'd). There are others too,
> >  but these will probably give decent coverage. These are likely to be
> >  increasingly relevant as more/new platforms are converted to use it.
> >  (note, the r6 one may require a newish toolchain).

Just wanted to note that this isn't true for the 64r6el_defconfig & 
32r2_defconfig (or similarly other variations of architecture revision & 
endianness). They aren't files under arch/mips/configs/, instead these configs 
are generated by merging a bunch of config fragments atop arch/mips/configs/
generic_defconfig. This allows us to have these minor variations on the base 
generic_defconfig without duplicating its content & incurring the maintenance 
headache of keeping them all in sync. For reference powerpc do similar things 
to generate variations upon their real defconfigs.

If you do have a way to get 64r6el_defconfig & 32r2_defconfig included that 
would be great, since more MIPS boards should be covered by these generic 
configurations over the next few cycles.

Thanks,
    Paul

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

* Re: [BACKPORT PATCH 3.10..3.16] KVM: MIPS: Drop other CPU ASIDs on guest MMU changes
  2016-11-11  8:58           ` Paul Burton
@ 2016-11-11 23:11             ` Fengguang Wu
  0 siblings, 0 replies; 13+ messages in thread
From: Fengguang Wu @ 2016-11-11 23:11 UTC (permalink / raw)
  To: Paul Burton
  Cc: James Hogan, Greg KH, Jiri Slaby, stable, Paolo Bonzini,
	Radim Krčmář,
	Ralf Baechle, linux-mips, kvm

Hi Paul,

On Fri, Nov 11, 2016 at 08:58:54AM +0000, Paul Burton wrote:
>Hi Fengguang,
>
>On Friday, 11 November 2016 10:56:21 GMT Fengguang Wu wrote:
>> The 0-day build bot should already cover the below configs (not
>> necessarily in the early hours, but very likely in the first day after
>> your git push) since they are included in arch/*/configs/.
>
><snip>
>
>> >- malta_kvm_defconfig
>> >
>> >  this probably doesn't need to be a high priority build, but other
>> >  configs don't yet cover MIPS KVM so its worth having (that bit us
>> >  recently with 3.12 and 4.4 stable branches).
>> >
>> >- 64r6el_defconfig and 32r2_defconfig (4.9 and later)
>> >
>> >  these are just a couple of the new generic/multiplatform kernel
>> >  configurations added in 4.9 (Paul Burton Cc'd). There are others too,
>> >  but these will probably give decent coverage. These are likely to be
>> >  increasingly relevant as more/new platforms are converted to use it.
>> >  (note, the r6 one may require a newish toolchain).
>
>Just wanted to note that this isn't true for the 64r6el_defconfig &
>32r2_defconfig (or similarly other variations of architecture revision &
>endianness). They aren't files under arch/mips/configs/, instead these configs
>are generated by merging a bunch of config fragments atop arch/mips/configs/
>generic_defconfig. This allows us to have these minor variations on the base
>generic_defconfig without duplicating its content & incurring the maintenance
>headache of keeping them all in sync. For reference powerpc do similar things
>to generate variations upon their real defconfigs.
>
>If you do have a way to get 64r6el_defconfig & 32r2_defconfig included that
>would be great, since more MIPS boards should be covered by these generic
>configurations over the next few cycles.

Got it, thank you for the explanations! It's very helpful to get the
tips on what set of kconfigs should be included in the minimal test
coverage.

I'll add 64r6el_defconfig and 32r2_defconfig to the list.
It looks will merge several configs together:

wfg@inn ~/linux/obj-compiletest% make ARCH=mips 64r6el_defconfig
/usr/bin/make -C source O=/home/wfg/linux/obj-compiletest ARCH=mips CROSS_COMPILE=/usr/local/gcc-4.6.3-nolibc/mips-linux/bin/mips-linux- -j32 ARCH=mips 64r6el_defconfig
make: Entering directory '/c/wfg/linux'
make[1]: Entering directory '/c/wfg/linux/obj-compiletest'
Using ../arch/mips/configs/generic_defconfig as base
Merging ../arch/mips/configs/generic/64r6.config
Merging ../arch/mips/configs/generic/el.config
Merging ../arch/mips/configs/generic/board-sead-3.config
#
# merged configuration written to .config (needs make)
#
scripts/kconfig/conf  --olddefconfig Kconfig
#
# configuration written to .config
#
make[1]: Leaving directory '/c/wfg/linux/obj-compiletest'
make: Leaving directory '/c/wfg/linux'

Regards,
Fengguang

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

* Re: [BACKPORT PATCH 3.10..3.16] KVM: MIPS: Drop other CPU ASIDs on guest MMU changes
  2016-11-09 14:46 ` James Hogan
                   ` (2 preceding siblings ...)
  (?)
@ 2016-11-12  3:08 ` Ben Hutchings
  -1 siblings, 0 replies; 13+ messages in thread
From: Ben Hutchings @ 2016-11-12  3:08 UTC (permalink / raw)
  To: James Hogan, Jiri Slaby, stable
  Cc: Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, linux-mips, kvm

[-- Attachment #1: Type: text/plain, Size: 5482 bytes --]

On Wed, 2016-11-09 at 14:46 +0000, James Hogan wrote:
> commit 91e4f1b6073dd680d86cdb7e42d7cccca9db39d8 upstream.
> 
> When a guest TLB entry is replaced by TLBWI or TLBWR, we only invalidate
> TLB entries on the local CPU. This doesn't work correctly on an SMP host
> when the guest is migrated to a different physical CPU, as it could pick
> up stale TLB mappings from the last time the vCPU ran on that physical
> CPU.
> 
> Therefore invalidate both user and kernel host ASIDs on other CPUs,
> which will cause new ASIDs to be generated when it next runs on those
> CPUs.
> 
> We're careful only to do this if the TLB entry was already valid, and
> only for the kernel ASID where the virtual address it mapped is outside
> of the guest user address range.
> 
> Signed-off-by: James Hogan <james.hogan@imgtec.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: "Radim Krčmář" <rkrcmar@redhat.com>
> Cc: Ralf Baechle <ralf@linux-mips.org>
> Cc: linux-mips@linux-mips.org
> Cc: kvm@vger.kernel.org
> Cc: <stable@vger.kernel.org> # 3.10.x-
> Cc: Jiri Slaby <jslaby@suse.cz>
> [james.hogan@imgtec.com: Backport to 3.10..3.16]
> Signed-off-by: James Hogan <james.hogan@imgtec.com>

Queued up for 3.16, thanks.

Ben.

> ---
> Unfortunately the original commit went in to v3.12.65 as commit
> 168e5ebbd63e, without fixing up the references to tlb_lo[0/1] to
> tlb_lo0/1 which broke the MIPS KVM build, and I didn't twig that I
> already had a correct backport outstanding (sorry!). That commit should
> be reverted before applying this backport to 3.12.
> ---
>  arch/mips/kvm/kvm_mips_emul.c | 61 +++++++++++++++++++++++++++++++++++++------
>  1 file changed, 53 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/mips/kvm/kvm_mips_emul.c b/arch/mips/kvm/kvm_mips_emul.c
> index 1983678883c9..d0eb34279955 100644
> --- a/arch/mips/kvm/kvm_mips_emul.c
> +++ b/arch/mips/kvm/kvm_mips_emul.c
> @@ -817,6 +817,47 @@ enum emulation_result kvm_mips_emul_tlbr(struct kvm_vcpu *vcpu)
> >  	return er;
>  }
>  
> +/**
> + * kvm_mips_invalidate_guest_tlb() - Indicates a change in guest MMU map.
> > + * @vcpu:	VCPU with changed mappings.
> > + * @tlb:	TLB entry being removed.
> + *
> + * This is called to indicate a single change in guest MMU mappings, so that we
> + * can arrange TLB flushes on this and other CPUs.
> + */
> +static void kvm_mips_invalidate_guest_tlb(struct kvm_vcpu *vcpu,
> > +					  struct kvm_mips_tlb *tlb)
> +{
> > +	int cpu, i;
> > +	bool user;
> +
> > +	/* No need to flush for entries which are already invalid */
> > +	if (!((tlb->tlb_lo0 | tlb->tlb_lo1) & MIPS3_PG_V))
> > +		return;
> > +	/* User address space doesn't need flushing for KSeg2/3 changes */
> > +	user = tlb->tlb_hi < KVM_GUEST_KSEG0;
> +
> > +	preempt_disable();
> +
> > +	/*
> > +	 * Probe the shadow host TLB for the entry being overwritten, if one
> > +	 * matches, invalidate it
> > +	 */
> > +	kvm_mips_host_tlb_inv(vcpu, tlb->tlb_hi);
> +
> > +	/* Invalidate the whole ASID on other CPUs */
> > +	cpu = smp_processor_id();
> > +	for_each_possible_cpu(i) {
> > +		if (i == cpu)
> > +			continue;
> > +		if (user)
> > +			vcpu->arch.guest_user_asid[i] = 0;
> > +		vcpu->arch.guest_kernel_asid[i] = 0;
> > +	}
> +
> > +	preempt_enable();
> +}
> +
>  /* Write Guest TLB Entry @ Index */
>  enum emulation_result kvm_mips_emul_tlbwi(struct kvm_vcpu *vcpu)
>  {
> @@ -838,10 +879,8 @@ enum emulation_result kvm_mips_emul_tlbwi(struct kvm_vcpu *vcpu)
> >  	}
>  
> >  	tlb = &vcpu->arch.guest_tlb[index];
> -#if 1
> > -	/* Probe the shadow host TLB for the entry being overwritten, if one matches, invalidate it */
> > -	kvm_mips_host_tlb_inv(vcpu, tlb->tlb_hi);
> -#endif
> +
> > +	kvm_mips_invalidate_guest_tlb(vcpu, tlb);
>  
> >  	tlb->tlb_mask = kvm_read_c0_guest_pagemask(cop0);
> >  	tlb->tlb_hi = kvm_read_c0_guest_entryhi(cop0);
> @@ -880,10 +919,7 @@ enum emulation_result kvm_mips_emul_tlbwr(struct kvm_vcpu *vcpu)
>  
> >  	tlb = &vcpu->arch.guest_tlb[index];
>  
> -#if 1
> > -	/* Probe the shadow host TLB for the entry being overwritten, if one matches, invalidate it */
> > -	kvm_mips_host_tlb_inv(vcpu, tlb->tlb_hi);
> -#endif
> > +	kvm_mips_invalidate_guest_tlb(vcpu, tlb);
>  
> >  	tlb->tlb_mask = kvm_read_c0_guest_pagemask(cop0);
> >  	tlb->tlb_hi = kvm_read_c0_guest_entryhi(cop0);
> @@ -926,6 +962,7 @@ kvm_mips_emulate_CP0(uint32_t inst, uint32_t *opc, uint32_t cause,
> >  	int32_t rt, rd, copz, sel, co_bit, op;
> >  	uint32_t pc = vcpu->arch.pc;
> >  	unsigned long curr_pc;
> > +	int cpu, i;
>  
> >  	/*
> >  	 * Update PC and hold onto current PC in case there is
> @@ -1037,8 +1074,16 @@ kvm_mips_emulate_CP0(uint32_t inst, uint32_t *opc, uint32_t cause,
> >  					     ASID_MASK,
> >  					     vcpu->arch.gprs[rt] & ASID_MASK);
>  
> > +					preempt_disable();
> >  					/* Blow away the shadow host TLBs */
> >  					kvm_mips_flush_host_tlb(1);
> > +					cpu = smp_processor_id();
> > +					for_each_possible_cpu(i)
> > +						if (i != cpu) {
> > +							vcpu->arch.guest_user_asid[i] = 0;
> > +							vcpu->arch.guest_kernel_asid[i] = 0;
> > +						}
> > +					preempt_enable();
> >  				}
> >  				kvm_write_c0_guest_entryhi(cop0,
>  							   vcpu->arch.gprs[rt]);
-- 
Ben Hutchings
If you seem to know what you are doing, you'll be given more to do.


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

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

end of thread, other threads:[~2016-11-12  3:08 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-09 14:46 [BACKPORT PATCH 3.10..3.16] KVM: MIPS: Drop other CPU ASIDs on guest MMU changes James Hogan
2016-11-09 14:46 ` James Hogan
2016-11-09 17:34 ` Willy Tarreau
2016-11-09 21:22 ` Jiri Slaby
2016-11-09 22:00   ` James Hogan
2016-11-10  6:08     ` Greg KH
2016-11-10 17:37       ` James Hogan
2016-11-10 17:37         ` James Hogan
2016-11-11  2:56         ` Fengguang Wu
2016-11-11  8:42           ` Jiri Slaby
2016-11-11  8:58           ` Paul Burton
2016-11-11 23:11             ` Fengguang Wu
2016-11-12  3:08 ` Ben Hutchings

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.