All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] KVM: MIPS: Miscellaneous 4.9 fixes
@ 2016-10-25 15:08 ` James Hogan
  0 siblings, 0 replies; 9+ messages in thread
From: James Hogan @ 2016-10-25 15:08 UTC (permalink / raw)
  To: linux-mips
  Cc: James Hogan, Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, kvm, stable

A few more fixes intended for v4.9. Patches 2 & 3 are tagged for stable.

- The first fixes lazy user ASID regeneration which was introduced in
  4.9-rc1 and still wasn't quite right for SMP hosts.

- The second is a minor incorrect behaviour in ERET emulation when both
  ERL and EXL are set (i.e. unlikely to hit in practice), which has been
  wrong since MIPS KVM was added in v3.10.

- The third fixes a slightly risky completion of an MMIO load in branch
  delay slot, where it'll try and read guest code outside of the proper
  context. Currently we should only be able to hit this if the MMIO load
  in branch delay slot is in guest TLB mapped (i.e. kernel module) code.

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

James Hogan (3):
  KVM: MIPS: Fix lazy user ASID regenerate for SMP
  KVM: MIPS: Make ERET handle ERL before EXL
  KVM: MIPS: Precalculate MMIO load resume PC

 arch/mips/include/asm/kvm_host.h |  7 ++++---
 arch/mips/kvm/emulate.c          | 32 +++++++++++++++++++-------------
 arch/mips/kvm/mips.c             |  5 ++++-
 arch/mips/kvm/mmu.c              |  4 ----
 4 files changed, 27 insertions(+), 21 deletions(-)

-- 
git-series 0.8.10

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

* [PATCH 0/3] KVM: MIPS: Miscellaneous 4.9 fixes
@ 2016-10-25 15:08 ` James Hogan
  0 siblings, 0 replies; 9+ messages in thread
From: James Hogan @ 2016-10-25 15:08 UTC (permalink / raw)
  To: linux-mips
  Cc: James Hogan, Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, kvm, stable

A few more fixes intended for v4.9. Patches 2 & 3 are tagged for stable.

- The first fixes lazy user ASID regeneration which was introduced in
  4.9-rc1 and still wasn't quite right for SMP hosts.

- The second is a minor incorrect behaviour in ERET emulation when both
  ERL and EXL are set (i.e. unlikely to hit in practice), which has been
  wrong since MIPS KVM was added in v3.10.

- The third fixes a slightly risky completion of an MMIO load in branch
  delay slot, where it'll try and read guest code outside of the proper
  context. Currently we should only be able to hit this if the MMIO load
  in branch delay slot is in guest TLB mapped (i.e. kernel module) code.

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

James Hogan (3):
  KVM: MIPS: Fix lazy user ASID regenerate for SMP
  KVM: MIPS: Make ERET handle ERL before EXL
  KVM: MIPS: Precalculate MMIO load resume PC

 arch/mips/include/asm/kvm_host.h |  7 ++++---
 arch/mips/kvm/emulate.c          | 32 +++++++++++++++++++-------------
 arch/mips/kvm/mips.c             |  5 ++++-
 arch/mips/kvm/mmu.c              |  4 ----
 4 files changed, 27 insertions(+), 21 deletions(-)

-- 
git-series 0.8.10

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

* [PATCH 1/3] KVM: MIPS: Fix lazy user ASID regenerate for SMP
@ 2016-10-25 15:08   ` James Hogan
  0 siblings, 0 replies; 9+ messages in thread
From: James Hogan @ 2016-10-25 15:08 UTC (permalink / raw)
  To: linux-mips
  Cc: James Hogan, Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, kvm

kvm_mips_check_asids() runs before entering the guest and performs lazy
regeneration of host ASID for guest usermode, using last_user_gasid to
track the last guest ASID in the VCPU that was used by guest usermode on
any host CPU.

last_user_gasid is reset after performing the lazy ASID regeneration on
the current CPU, and by kvm_arch_vcpu_load() if the host ASID for guest
usermode is regenerated due to staleness (to cancel outstanding lazy
ASID regenerations). Unfortunately neither case handles SMP hosts
correctly:

 - When the lazy ASID regeneration is performed it should apply to all
   CPUs (as last_user_gasid does), so reset the ASID on other CPUs to
   zero to trigger regeneration when the VCPU is next loaded on those
   CPUs.

 - When the ASID is found to be stale on the current CPU, we should not
   cancel lazy ASID regenerations globally, so drop the reset of
   last_user_gasid altogether here.

Both cases would require a guest ASID change and two host CPU migrations
(and in the latter case one of the CPUs to start a new ASID cycle)
before guest usermode could potentially access stale user pages from a
previously running ASID in the same VCPU.

Fixes: 25b08c7fb0e4 ("KVM: MIPS: Invalidate TLB by regenerating ASIDs")
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
---
 arch/mips/kvm/mips.c | 5 ++++-
 arch/mips/kvm/mmu.c  | 4 ----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 622037d851a3..06a60b19acfb 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -426,7 +426,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
 static void kvm_mips_check_asids(struct kvm_vcpu *vcpu)
 {
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
-	int cpu = smp_processor_id();
+	int i, cpu = smp_processor_id();
 	unsigned int gasid;
 
 	/*
@@ -442,6 +442,9 @@ static void kvm_mips_check_asids(struct kvm_vcpu *vcpu)
 						vcpu);
 			vcpu->arch.guest_user_asid[cpu] =
 				vcpu->arch.guest_user_mm.context.asid[cpu];
+			for_each_possible_cpu(i)
+				if (i != cpu)
+					vcpu->arch.guest_user_asid[cpu] = 0;
 			vcpu->arch.last_user_gasid = gasid;
 		}
 	}
diff --git a/arch/mips/kvm/mmu.c b/arch/mips/kvm/mmu.c
index 03883ba806e2..3b677c851be0 100644
--- a/arch/mips/kvm/mmu.c
+++ b/arch/mips/kvm/mmu.c
@@ -260,13 +260,9 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 
 	if ((vcpu->arch.guest_user_asid[cpu] ^ asid_cache(cpu)) &
 						asid_version_mask(cpu)) {
-		u32 gasid = kvm_read_c0_guest_entryhi(vcpu->arch.cop0) &
-				KVM_ENTRYHI_ASID;
-
 		kvm_get_new_mmu_context(&vcpu->arch.guest_user_mm, cpu, vcpu);
 		vcpu->arch.guest_user_asid[cpu] =
 		    vcpu->arch.guest_user_mm.context.asid[cpu];
-		vcpu->arch.last_user_gasid = gasid;
 		newasid++;
 
 		kvm_debug("[%d]: cpu_context: %#lx\n", cpu,
-- 
git-series 0.8.10

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

* [PATCH 1/3] KVM: MIPS: Fix lazy user ASID regenerate for SMP
@ 2016-10-25 15:08   ` James Hogan
  0 siblings, 0 replies; 9+ messages in thread
From: James Hogan @ 2016-10-25 15:08 UTC (permalink / raw)
  To: linux-mips
  Cc: James Hogan, Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, kvm

kvm_mips_check_asids() runs before entering the guest and performs lazy
regeneration of host ASID for guest usermode, using last_user_gasid to
track the last guest ASID in the VCPU that was used by guest usermode on
any host CPU.

last_user_gasid is reset after performing the lazy ASID regeneration on
the current CPU, and by kvm_arch_vcpu_load() if the host ASID for guest
usermode is regenerated due to staleness (to cancel outstanding lazy
ASID regenerations). Unfortunately neither case handles SMP hosts
correctly:

 - When the lazy ASID regeneration is performed it should apply to all
   CPUs (as last_user_gasid does), so reset the ASID on other CPUs to
   zero to trigger regeneration when the VCPU is next loaded on those
   CPUs.

 - When the ASID is found to be stale on the current CPU, we should not
   cancel lazy ASID regenerations globally, so drop the reset of
   last_user_gasid altogether here.

Both cases would require a guest ASID change and two host CPU migrations
(and in the latter case one of the CPUs to start a new ASID cycle)
before guest usermode could potentially access stale user pages from a
previously running ASID in the same VCPU.

Fixes: 25b08c7fb0e4 ("KVM: MIPS: Invalidate TLB by regenerating ASIDs")
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
---
 arch/mips/kvm/mips.c | 5 ++++-
 arch/mips/kvm/mmu.c  | 4 ----
 2 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index 622037d851a3..06a60b19acfb 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -426,7 +426,7 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
 static void kvm_mips_check_asids(struct kvm_vcpu *vcpu)
 {
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
-	int cpu = smp_processor_id();
+	int i, cpu = smp_processor_id();
 	unsigned int gasid;
 
 	/*
@@ -442,6 +442,9 @@ static void kvm_mips_check_asids(struct kvm_vcpu *vcpu)
 						vcpu);
 			vcpu->arch.guest_user_asid[cpu] =
 				vcpu->arch.guest_user_mm.context.asid[cpu];
+			for_each_possible_cpu(i)
+				if (i != cpu)
+					vcpu->arch.guest_user_asid[cpu] = 0;
 			vcpu->arch.last_user_gasid = gasid;
 		}
 	}
diff --git a/arch/mips/kvm/mmu.c b/arch/mips/kvm/mmu.c
index 03883ba806e2..3b677c851be0 100644
--- a/arch/mips/kvm/mmu.c
+++ b/arch/mips/kvm/mmu.c
@@ -260,13 +260,9 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
 
 	if ((vcpu->arch.guest_user_asid[cpu] ^ asid_cache(cpu)) &
 						asid_version_mask(cpu)) {
-		u32 gasid = kvm_read_c0_guest_entryhi(vcpu->arch.cop0) &
-				KVM_ENTRYHI_ASID;
-
 		kvm_get_new_mmu_context(&vcpu->arch.guest_user_mm, cpu, vcpu);
 		vcpu->arch.guest_user_asid[cpu] =
 		    vcpu->arch.guest_user_mm.context.asid[cpu];
-		vcpu->arch.last_user_gasid = gasid;
 		newasid++;
 
 		kvm_debug("[%d]: cpu_context: %#lx\n", cpu,
-- 
git-series 0.8.10

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

* [PATCH 2/3] KVM: MIPS: Make ERET handle ERL before EXL
@ 2016-10-25 15:11   ` James Hogan
  0 siblings, 0 replies; 9+ messages in thread
From: James Hogan @ 2016-10-25 15:11 UTC (permalink / raw)
  To: linux-mips
  Cc: Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, kvm, stable, James Hogan

The ERET instruction to return from exception is used for returning from
exception level (Status.EXL) and error level (Status.ERL). If both bits
are set however we should be returning from ERL first, as ERL can
interrupt EXL, for example when an NMI is taken. KVM however checks EXL
first.

Fix the order of the checks to match the pseudocode in the instruction
set manual.

Fixes: e685c689f3a8 ("KVM/MIPS32: Privileged instruction/target branch emulation.")
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-
---
 arch/mips/kvm/emulate.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
index 8770f32c9e0b..c45ef0f13dfa 100644
--- a/arch/mips/kvm/emulate.c
+++ b/arch/mips/kvm/emulate.c
@@ -790,15 +790,15 @@ enum emulation_result kvm_mips_emul_eret(struct kvm_vcpu *vcpu)
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
 	enum emulation_result er = EMULATE_DONE;
 
-	if (kvm_read_c0_guest_status(cop0) & ST0_EXL) {
+	if (kvm_read_c0_guest_status(cop0) & ST0_ERL) {
+		kvm_clear_c0_guest_status(cop0, ST0_ERL);
+		vcpu->arch.pc = kvm_read_c0_guest_errorepc(cop0);
+	} else if (kvm_read_c0_guest_status(cop0) & ST0_EXL) {
 		kvm_debug("[%#lx] ERET to %#lx\n", vcpu->arch.pc,
 			  kvm_read_c0_guest_epc(cop0));
 		kvm_clear_c0_guest_status(cop0, ST0_EXL);
 		vcpu->arch.pc = kvm_read_c0_guest_epc(cop0);
 
-	} else if (kvm_read_c0_guest_status(cop0) & ST0_ERL) {
-		kvm_clear_c0_guest_status(cop0, ST0_ERL);
-		vcpu->arch.pc = kvm_read_c0_guest_errorepc(cop0);
 	} else {
 		kvm_err("[%#lx] ERET when MIPS_SR_EXL|MIPS_SR_ERL == 0\n",
 			vcpu->arch.pc);
-- 
git-series 0.8.10

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

* [PATCH 2/3] KVM: MIPS: Make ERET handle ERL before EXL
@ 2016-10-25 15:11   ` James Hogan
  0 siblings, 0 replies; 9+ messages in thread
From: James Hogan @ 2016-10-25 15:11 UTC (permalink / raw)
  To: linux-mips
  Cc: Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, kvm, stable, James Hogan

The ERET instruction to return from exception is used for returning from
exception level (Status.EXL) and error level (Status.ERL). If both bits
are set however we should be returning from ERL first, as ERL can
interrupt EXL, for example when an NMI is taken. KVM however checks EXL
first.

Fix the order of the checks to match the pseudocode in the instruction
set manual.

Fixes: e685c689f3a8 ("KVM/MIPS32: Privileged instruction/target branch emulation.")
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-
---
 arch/mips/kvm/emulate.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
index 8770f32c9e0b..c45ef0f13dfa 100644
--- a/arch/mips/kvm/emulate.c
+++ b/arch/mips/kvm/emulate.c
@@ -790,15 +790,15 @@ enum emulation_result kvm_mips_emul_eret(struct kvm_vcpu *vcpu)
 	struct mips_coproc *cop0 = vcpu->arch.cop0;
 	enum emulation_result er = EMULATE_DONE;
 
-	if (kvm_read_c0_guest_status(cop0) & ST0_EXL) {
+	if (kvm_read_c0_guest_status(cop0) & ST0_ERL) {
+		kvm_clear_c0_guest_status(cop0, ST0_ERL);
+		vcpu->arch.pc = kvm_read_c0_guest_errorepc(cop0);
+	} else if (kvm_read_c0_guest_status(cop0) & ST0_EXL) {
 		kvm_debug("[%#lx] ERET to %#lx\n", vcpu->arch.pc,
 			  kvm_read_c0_guest_epc(cop0));
 		kvm_clear_c0_guest_status(cop0, ST0_EXL);
 		vcpu->arch.pc = kvm_read_c0_guest_epc(cop0);
 
-	} else if (kvm_read_c0_guest_status(cop0) & ST0_ERL) {
-		kvm_clear_c0_guest_status(cop0, ST0_ERL);
-		vcpu->arch.pc = kvm_read_c0_guest_errorepc(cop0);
 	} else {
 		kvm_err("[%#lx] ERET when MIPS_SR_EXL|MIPS_SR_ERL == 0\n",
 			vcpu->arch.pc);
-- 
git-series 0.8.10

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

* [PATCH 3/3] KVM: MIPS: Precalculate MMIO load resume PC
@ 2016-10-25 15:11   ` James Hogan
  0 siblings, 0 replies; 9+ messages in thread
From: James Hogan @ 2016-10-25 15:11 UTC (permalink / raw)
  To: linux-mips
  Cc: Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, kvm, stable, James Hogan

The advancing of the PC when completing an MMIO load is done before
re-entering the guest, i.e. before restoring the guest ASID. However if
the load is in a branch delay slot it may need to access guest code to
read the prior branch instruction. This isn't safe in TLB mapped code at
the moment, nor in the future when we'll access unmapped guest segments
using direct user accessors too, as it could read the branch from host
user memory instead.

Therefore calculate the resume PC in advance while we're still in the
right context and save it in the new vcpu->arch.io_pc (replacing the no
longer needed vcpu->arch.pending_load_cause), and restore it on MMIO
completion.

Fixes: e685c689f3a8 ("KVM/MIPS32: Privileged instruction/target branch emulation.")
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-
---
 arch/mips/include/asm/kvm_host.h |  7 ++++---
 arch/mips/kvm/emulate.c          | 24 +++++++++++++++---------
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 07f58cfc1ab9..bebec370324f 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -293,7 +293,10 @@ struct kvm_vcpu_arch {
 	/* Host KSEG0 address of the EI/DI offset */
 	void *kseg0_commpage;
 
-	u32 io_gpr;		/* GPR used as IO source/target */
+	/* Resume PC after MMIO completion */
+	unsigned long io_pc;
+	/* GPR used as IO source/target */
+	u32 io_gpr;
 
 	struct hrtimer comparecount_timer;
 	/* Count timer control KVM register */
@@ -315,8 +318,6 @@ struct kvm_vcpu_arch {
 	/* Bitmask of pending exceptions to be cleared */
 	unsigned long pending_exceptions_clr;
 
-	u32 pending_load_cause;
-
 	/* Save/Restore the entryhi register when are are preempted/scheduled back in */
 	unsigned long preempt_entryhi;
 
diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
index c45ef0f13dfa..aa0937423e28 100644
--- a/arch/mips/kvm/emulate.c
+++ b/arch/mips/kvm/emulate.c
@@ -1528,13 +1528,25 @@ enum emulation_result kvm_mips_emulate_load(union mips_instruction inst,
 					    struct kvm_vcpu *vcpu)
 {
 	enum emulation_result er = EMULATE_DO_MMIO;
+	unsigned long curr_pc;
 	u32 op, rt;
 	u32 bytes;
 
 	rt = inst.i_format.rt;
 	op = inst.i_format.opcode;
 
-	vcpu->arch.pending_load_cause = cause;
+	/*
+	 * Find the resume PC now while we have safe and easy access to the
+	 * prior branch instruction, and save it for
+	 * kvm_mips_complete_mmio_load() to restore later.
+	 */
+	curr_pc = vcpu->arch.pc;
+	er = update_pc(vcpu, cause);
+	if (er == EMULATE_FAIL)
+		return er;
+	vcpu->arch.io_pc = vcpu->arch.pc;
+	vcpu->arch.pc = curr_pc;
+
 	vcpu->arch.io_gpr = rt;
 
 	switch (op) {
@@ -2494,9 +2506,8 @@ enum emulation_result kvm_mips_complete_mmio_load(struct kvm_vcpu *vcpu,
 		goto done;
 	}
 
-	er = update_pc(vcpu, vcpu->arch.pending_load_cause);
-	if (er == EMULATE_FAIL)
-		return er;
+	/* Restore saved resume PC */
+	vcpu->arch.pc = vcpu->arch.io_pc;
 
 	switch (run->mmio.len) {
 	case 4:
@@ -2518,11 +2529,6 @@ enum emulation_result kvm_mips_complete_mmio_load(struct kvm_vcpu *vcpu,
 		break;
 	}
 
-	if (vcpu->arch.pending_load_cause & CAUSEF_BD)
-		kvm_debug("[%#lx] Completing %d byte BD Load to gpr %d (0x%08lx) type %d\n",
-			  vcpu->arch.pc, run->mmio.len, vcpu->arch.io_gpr, *gpr,
-			  vcpu->mmio_needed);
-
 done:
 	return er;
 }
-- 
git-series 0.8.10

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

* [PATCH 3/3] KVM: MIPS: Precalculate MMIO load resume PC
@ 2016-10-25 15:11   ` James Hogan
  0 siblings, 0 replies; 9+ messages in thread
From: James Hogan @ 2016-10-25 15:11 UTC (permalink / raw)
  To: linux-mips
  Cc: Paolo Bonzini, Radim Krčmář,
	Ralf Baechle, kvm, stable, James Hogan

The advancing of the PC when completing an MMIO load is done before
re-entering the guest, i.e. before restoring the guest ASID. However if
the load is in a branch delay slot it may need to access guest code to
read the prior branch instruction. This isn't safe in TLB mapped code at
the moment, nor in the future when we'll access unmapped guest segments
using direct user accessors too, as it could read the branch from host
user memory instead.

Therefore calculate the resume PC in advance while we're still in the
right context and save it in the new vcpu->arch.io_pc (replacing the no
longer needed vcpu->arch.pending_load_cause), and restore it on MMIO
completion.

Fixes: e685c689f3a8 ("KVM/MIPS32: Privileged instruction/target branch emulation.")
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-
---
 arch/mips/include/asm/kvm_host.h |  7 ++++---
 arch/mips/kvm/emulate.c          | 24 +++++++++++++++---------
 2 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/arch/mips/include/asm/kvm_host.h b/arch/mips/include/asm/kvm_host.h
index 07f58cfc1ab9..bebec370324f 100644
--- a/arch/mips/include/asm/kvm_host.h
+++ b/arch/mips/include/asm/kvm_host.h
@@ -293,7 +293,10 @@ struct kvm_vcpu_arch {
 	/* Host KSEG0 address of the EI/DI offset */
 	void *kseg0_commpage;
 
-	u32 io_gpr;		/* GPR used as IO source/target */
+	/* Resume PC after MMIO completion */
+	unsigned long io_pc;
+	/* GPR used as IO source/target */
+	u32 io_gpr;
 
 	struct hrtimer comparecount_timer;
 	/* Count timer control KVM register */
@@ -315,8 +318,6 @@ struct kvm_vcpu_arch {
 	/* Bitmask of pending exceptions to be cleared */
 	unsigned long pending_exceptions_clr;
 
-	u32 pending_load_cause;
-
 	/* Save/Restore the entryhi register when are are preempted/scheduled back in */
 	unsigned long preempt_entryhi;
 
diff --git a/arch/mips/kvm/emulate.c b/arch/mips/kvm/emulate.c
index c45ef0f13dfa..aa0937423e28 100644
--- a/arch/mips/kvm/emulate.c
+++ b/arch/mips/kvm/emulate.c
@@ -1528,13 +1528,25 @@ enum emulation_result kvm_mips_emulate_load(union mips_instruction inst,
 					    struct kvm_vcpu *vcpu)
 {
 	enum emulation_result er = EMULATE_DO_MMIO;
+	unsigned long curr_pc;
 	u32 op, rt;
 	u32 bytes;
 
 	rt = inst.i_format.rt;
 	op = inst.i_format.opcode;
 
-	vcpu->arch.pending_load_cause = cause;
+	/*
+	 * Find the resume PC now while we have safe and easy access to the
+	 * prior branch instruction, and save it for
+	 * kvm_mips_complete_mmio_load() to restore later.
+	 */
+	curr_pc = vcpu->arch.pc;
+	er = update_pc(vcpu, cause);
+	if (er == EMULATE_FAIL)
+		return er;
+	vcpu->arch.io_pc = vcpu->arch.pc;
+	vcpu->arch.pc = curr_pc;
+
 	vcpu->arch.io_gpr = rt;
 
 	switch (op) {
@@ -2494,9 +2506,8 @@ enum emulation_result kvm_mips_complete_mmio_load(struct kvm_vcpu *vcpu,
 		goto done;
 	}
 
-	er = update_pc(vcpu, vcpu->arch.pending_load_cause);
-	if (er == EMULATE_FAIL)
-		return er;
+	/* Restore saved resume PC */
+	vcpu->arch.pc = vcpu->arch.io_pc;
 
 	switch (run->mmio.len) {
 	case 4:
@@ -2518,11 +2529,6 @@ enum emulation_result kvm_mips_complete_mmio_load(struct kvm_vcpu *vcpu,
 		break;
 	}
 
-	if (vcpu->arch.pending_load_cause & CAUSEF_BD)
-		kvm_debug("[%#lx] Completing %d byte BD Load to gpr %d (0x%08lx) type %d\n",
-			  vcpu->arch.pc, run->mmio.len, vcpu->arch.io_gpr, *gpr,
-			  vcpu->mmio_needed);
-
 done:
 	return er;
 }
-- 
git-series 0.8.10

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

* Re: [PATCH 0/3] KVM: MIPS: Miscellaneous 4.9 fixes
  2016-10-25 15:08 ` James Hogan
                   ` (3 preceding siblings ...)
  (?)
@ 2016-10-25 15:57 ` Paolo Bonzini
  -1 siblings, 0 replies; 9+ messages in thread
From: Paolo Bonzini @ 2016-10-25 15:57 UTC (permalink / raw)
  To: James Hogan, linux-mips
  Cc: Radim Krčmář, Ralf Baechle, kvm, stable



On 25/10/2016 17:08, James Hogan wrote:
> A few more fixes intended for v4.9. Patches 2 & 3 are tagged for stable.
> 
> - The first fixes lazy user ASID regeneration which was introduced in
>   4.9-rc1 and still wasn't quite right for SMP hosts.
> 
> - The second is a minor incorrect behaviour in ERET emulation when both
>   ERL and EXL are set (i.e. unlikely to hit in practice), which has been
>   wrong since MIPS KVM was added in v3.10.
> 
> - The third fixes a slightly risky completion of an MMIO load in branch
>   delay slot, where it'll try and read guest code outside of the proper
>   context. Currently we should only be able to hit this if the MMIO load
>   in branch delay slot is in guest TLB mapped (i.e. kernel module) code.
> 
> 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
> 
> James Hogan (3):
>   KVM: MIPS: Fix lazy user ASID regenerate for SMP
>   KVM: MIPS: Make ERET handle ERL before EXL
>   KVM: MIPS: Precalculate MMIO load resume PC
> 
>  arch/mips/include/asm/kvm_host.h |  7 ++++---
>  arch/mips/kvm/emulate.c          | 32 +++++++++++++++++++-------------
>  arch/mips/kvm/mips.c             |  5 ++++-
>  arch/mips/kvm/mmu.c              |  4 ----
>  4 files changed, 27 insertions(+), 21 deletions(-)
> 

Applied to kvm/master, thanks.

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

end of thread, other threads:[~2016-10-25 15:57 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-25 15:08 [PATCH 0/3] KVM: MIPS: Miscellaneous 4.9 fixes James Hogan
2016-10-25 15:08 ` James Hogan
2016-10-25 15:08 ` [PATCH 1/3] KVM: MIPS: Fix lazy user ASID regenerate for SMP James Hogan
2016-10-25 15:08   ` James Hogan
2016-10-25 15:11 ` [PATCH 2/3] KVM: MIPS: Make ERET handle ERL before EXL James Hogan
2016-10-25 15:11   ` James Hogan
2016-10-25 15:11 ` [PATCH 3/3] KVM: MIPS: Precalculate MMIO load resume PC James Hogan
2016-10-25 15:11   ` James Hogan
2016-10-25 15:57 ` [PATCH 0/3] KVM: MIPS: Miscellaneous 4.9 fixes Paolo Bonzini

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.