Skip to contentSkip to conversation

A handful of KVM/ARM fixes

PATCH New Marc Zyngier 3 participants 9 messages Jun 21, 2013 – Jun 22, 2013 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

v2 cover A handful of KVM/ARM fixes

This is a small patch series that I accumulated while tracking what turned out to be an unrelated issue.

The first patch has already been posted a while ago and included here for completeness. The others are fairly simple missing barriers/clear exclusives that are actually required.

This has been tested on TC2, and is also available in my tree:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git kvm-arm/fixes-3.10-rc6

Quoted 1 line

From v1:

- Dropped two barrier related patches after Will convinced me that they were not required at all.
- Changed dsb to "dsb ishst" in the second patch to reflect that we only care about stores.

Marc Zyngier (3):
ARM: KVM: perform save/restore of PAR
ARM: KVM: add missing dsb before invalidating Stage-2 TLBs
ARM: KVM: clear exclusive monitor on all exception returns

 arch/arm/include/asm/kvm_asm.h | 22 ++++++++++++----------
 arch/arm/kvm/coproc.c          |  4 ++++
 arch/arm/kvm/interrupts.S      | 16 +++++++++++++++-
 arch/arm/kvm/interrupts_head.S | 10 ++++++++--
 4 files changed, 39 insertions(+), 13 deletions(-)
Signature · 1 line
1.8.2.3
#1 lore raw

PATCH v2 1/3 ARM: KVM: perform save/restore of PAR

Not saving PAR is an unfortunate oversight. If the guest performs an AT* operation and gets scheduled out before reading the result of the translation from PAR, it could become corrupted by another guest or the host.

Saving this register is made slightly more complicated as KVM also uses it on the permission fault handling path, leading to an ugly "stash and restore" sequence. Fortunately, this is already a slow path so we don't really care. Also, Linux doesn't do any AT* operation, so Linux guests are not impacted by this bug.

---
 arch/arm/include/asm/kvm_asm.h | 22 ++++++++++++----------
 arch/arm/kvm/coproc.c          |  4 ++++
 arch/arm/kvm/interrupts.S      | 12 +++++++++++-
 arch/arm/kvm/interrupts_head.S | 10 ++++++++--
 4 files changed, 35 insertions(+), 13 deletions(-)
arch/arm/include/asm/kvm_asm.h +12 -10
index 18d5032..4bb08e3 100644
@@ -37,16 +37,18 @@
 #define c5_AIFSR	15	/* Auxilary Instrunction Fault Status R */
 #define c6_DFAR		16	/* Data Fault Address Register */
 #define c6_IFAR		17	/* Instruction Fault Address Register */
-#define c9_L2CTLR	18	/* Cortex A15 L2 Control Register */
-#define c10_PRRR	19	/* Primary Region Remap Register */
-#define c10_NMRR	20	/* Normal Memory Remap Register */
-#define c12_VBAR	21	/* Vector Base Address Register */
-#define c13_CID		22	/* Context ID Register */
-#define c13_TID_URW	23	/* Thread ID, User R/W */
-#define c13_TID_URO	24	/* Thread ID, User R/O */
-#define c13_TID_PRIV	25	/* Thread ID, Privileged */
-#define c14_CNTKCTL	26	/* Timer Control Register (PL1) */
-#define NR_CP15_REGS	27	/* Number of regs (incl. invalid) */
+#define c7_PAR		18	/* Physical Address Register */
+#define c7_PAR_high	19	/* PAR top 32 bits */
+#define c9_L2CTLR	20	/* Cortex A15 L2 Control Register */
+#define c10_PRRR	21	/* Primary Region Remap Register */
+#define c10_NMRR	22	/* Normal Memory Remap Register */
+#define c12_VBAR	23	/* Vector Base Address Register */
+#define c13_CID		24	/* Context ID Register */
+#define c13_TID_URW	25	/* Thread ID, User R/W */
+#define c13_TID_URO	26	/* Thread ID, User R/O */
+#define c13_TID_PRIV	27	/* Thread ID, Privileged */
+#define c14_CNTKCTL	28	/* Timer Control Register (PL1) */
+#define NR_CP15_REGS	29	/* Number of regs (incl. invalid) */
 
 #define ARM_EXCEPTION_RESET	  0
 #define ARM_EXCEPTION_UNDEFINED   1
arch/arm/kvm/coproc.c +4 -0
index 8eea97b..4a51990 100644
@@ -180,6 +180,10 @@ static const struct coproc_reg cp15_regs[] = {
 			NULL, reset_unknown, c6_DFAR },
 	{ CRn( 6), CRm( 0), Op1( 0), Op2( 2), is32,
 			NULL, reset_unknown, c6_IFAR },
+
+	/* PAR swapped by interrupt.S */
+	{ CRn( 7), Op1( 0), is64, NULL, reset_unknown64, c7_PAR },
+
 	/*
 	 * DC{C,I,CI}SW operations:
 	 */
arch/arm/kvm/interrupts.S +11 -1
index f7793df..d0a8fa3 100644
@@ -414,6 +414,10 @@ guest_trap:
 	mrcne	p15, 4, r2, c6, c0, 4	@ HPFAR
 	bne	3f
 
+	/* Preserve PAR */
+	mrrc	p15, 0, r0, r1, c7	@ PAR
+	push	{r0, r1}
+
 	/* Resolve IPA using the xFAR */
 	mcr	p15, 0, r2, c7, c8, 0	@ ATS1CPR
 	isb
@@ -424,13 +428,19 @@ guest_trap:
 	lsl	r2, r2, #4
 	orr	r2, r2, r1, lsl #24
 
+	/* Restore PAR */
+	pop	{r0, r1}
+	mcrr	p15, 0, r0, r1, c7	@ PAR
+
 3:	load_vcpu			@ Load VCPU pointer to r0
 	str	r2, [r0, #VCPU_HPFAR]
 
 1:	mov	r1, #ARM_EXCEPTION_HVC
 	b	__kvm_vcpu_return
 
-4:	pop	{r0, r1, r2}		@ Failed translation, return to guest
+4:	pop	{r0, r1}		@ Failed translation, return to guest
+	mcrr	p15, 0, r0, r1, c7	@ PAR
+	pop	{r0, r1, r2}
 	eret
 
 /*
arch/arm/kvm/interrupts_head.S +8 -2
index 3c8f2f0..2478af1 100644
@@ -302,11 +302,14 @@ vcpu	.req	r0		@ vcpu pointer always in r0
 	.endif
 
 	mrc	p15, 0, r2, c14, c1, 0	@ CNTKCTL
+	mrrc	p15, 0, r3, r4, c7	@ PAR
 
 	.if \store_to_vcpu == 0
-	push	{r2}
+	push	{r2-r4}
 	.else
 	str	r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
+	add	r12, vcpu, #CP15_OFFSET(c7_PAR)
+	strd	r3, r4, [r12]
 	.endif
 .endm
 
@@ -319,12 +322,15 @@ vcpu	.req	r0		@ vcpu pointer always in r0
  */
 .macro write_cp15_state read_from_vcpu
 	.if \read_from_vcpu == 0
-	pop	{r2}
+	pop	{r2-r4}
 	.else
 	ldr	r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
+	add	r12, vcpu, #CP15_OFFSET(c7_PAR)
+	ldrd	r3, r4, [r12]
 	.endif
 
 	mcr	p15, 0, r2, c14, c1, 0	@ CNTKCTL
+	mcrr	p15, 0, r3, r4, c7	@ PAR
 
 	.if \read_from_vcpu == 0
 	pop	{r2-r12}
Signature · 1 line
1.8.2.3
#2 lore raw

PATCH v2 1/3 ARM: KVM: perform save/restore of PAR

Quoted 110 lines · Marc Zyngier

Not saving PAR is an unfortunate oversight. If the guest performs an AT* operation and gets scheduled out before reading the result of the translation from PAR, it could become corrupted by another guest or the host.

Saving this register is made slightly more complicated as KVM also uses it on the permission fault handling path, leading to an ugly "stash and restore" sequence. Fortunately, this is already a slow path so we don't really care. Also, Linux doesn't do any AT* operation, so Linux guests are not impacted by this bug.

---
 arch/arm/include/asm/kvm_asm.h | 22 ++++++++++++----------
 arch/arm/kvm/coproc.c          |  4 ++++
 arch/arm/kvm/interrupts.S      | 12 +++++++++++-
 arch/arm/kvm/interrupts_head.S | 10 ++++++++--
 4 files changed, 35 insertions(+), 13 deletions(-)
arch/arm/include/asm/kvm_asm.h +12 -10
index 18d5032..4bb08e3 100644
@@ -37,16 +37,18 @@
 #define c5_AIFSR	15	/* Auxilary Instrunction Fault Status R */
 #define c6_DFAR		16	/* Data Fault Address Register */
 #define c6_IFAR		17	/* Instruction Fault Address Register */
-#define c9_L2CTLR	18	/* Cortex A15 L2 Control Register */
-#define c10_PRRR	19	/* Primary Region Remap Register */
-#define c10_NMRR	20	/* Normal Memory Remap Register */
-#define c12_VBAR	21	/* Vector Base Address Register */
-#define c13_CID		22	/* Context ID Register */
-#define c13_TID_URW	23	/* Thread ID, User R/W */
-#define c13_TID_URO	24	/* Thread ID, User R/O */
-#define c13_TID_PRIV	25	/* Thread ID, Privileged */
-#define c14_CNTKCTL	26	/* Timer Control Register (PL1) */
-#define NR_CP15_REGS	27	/* Number of regs (incl. invalid) */
+#define c7_PAR		18	/* Physical Address Register */
+#define c7_PAR_high	19	/* PAR top 32 bits */
+#define c9_L2CTLR	20	/* Cortex A15 L2 Control Register */
+#define c10_PRRR	21	/* Primary Region Remap Register */
+#define c10_NMRR	22	/* Normal Memory Remap Register */
+#define c12_VBAR	23	/* Vector Base Address Register */
+#define c13_CID		24	/* Context ID Register */
+#define c13_TID_URW	25	/* Thread ID, User R/W */
+#define c13_TID_URO	26	/* Thread ID, User R/O */
+#define c13_TID_PRIV	27	/* Thread ID, Privileged */
+#define c14_CNTKCTL	28	/* Timer Control Register (PL1) */
+#define NR_CP15_REGS	29	/* Number of regs (incl. invalid) */
 
 #define ARM_EXCEPTION_RESET	  0
 #define ARM_EXCEPTION_UNDEFINED   1
arch/arm/kvm/coproc.c +4 -0
index 8eea97b..4a51990 100644
@@ -180,6 +180,10 @@ static const struct coproc_reg cp15_regs[] = {
 			NULL, reset_unknown, c6_DFAR },
 	{ CRn( 6), CRm( 0), Op1( 0), Op2( 2), is32,
 			NULL, reset_unknown, c6_IFAR },
+
+	/* PAR swapped by interrupt.S */
+	{ CRn( 7), Op1( 0), is64, NULL, reset_unknown64, c7_PAR },
+
 	/*
 	 * DC{C,I,CI}SW operations:
 	 */
arch/arm/kvm/interrupts.S +11 -1
index f7793df..d0a8fa3 100644
@@ -414,6 +414,10 @@ guest_trap:
 	mrcne	p15, 4, r2, c6, c0, 4	@ HPFAR
 	bne	3f
 
+	/* Preserve PAR */
+	mrrc	p15, 0, r0, r1, c7	@ PAR
+	push	{r0, r1}
+
 	/* Resolve IPA using the xFAR */
 	mcr	p15, 0, r2, c7, c8, 0	@ ATS1CPR
 	isb
@@ -424,13 +428,19 @@ guest_trap:
 	lsl	r2, r2, #4
 	orr	r2, r2, r1, lsl #24
 
+	/* Restore PAR */
+	pop	{r0, r1}
+	mcrr	p15, 0, r0, r1, c7	@ PAR
+
 3:	load_vcpu			@ Load VCPU pointer to r0
 	str	r2, [r0, #VCPU_HPFAR]
 
 1:	mov	r1, #ARM_EXCEPTION_HVC
 	b	__kvm_vcpu_return
 
-4:	pop	{r0, r1, r2}		@ Failed translation, return to guest
+4:	pop	{r0, r1}		@ Failed translation, return to guest
+	mcrr	p15, 0, r0, r1, c7	@ PAR
+	pop	{r0, r1, r2}
 	eret
 
 /*
arch/arm/kvm/interrupts_head.S +0 -0
index 3c8f2f0..2478af1 100644
@@ -302,11 +302,14 @@ vcpu	.req	r0		@ vcpu pointer always in r0
 	.endif
 

arch/arm/kvm/interrupts_head.S vcpu .req r0 @ vcpu pointer always in r0 · line 312

Quoted 10 lines
arch/arm/kvm/interrupts_head.S +4 -1
 	mrc	p15, 0, r2, c14, c1, 0	@ CNTKCTL
+	mrrc	p15, 0, r3, r4, c7	@ PAR
 
 	.if \store_to_vcpu == 0
-	push	{r2}
+	push	{r2-r4}
 	.else
 	str	r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
+	add	r12, vcpu, #CP15_OFFSET(c7_PAR)
+	strd	r3, r4, [r12]

my compiler complains that the load should start with an even register, so I've changed this with the patch below.

arch/arm/kvm/interrupts_head.S vcpu .req r0 @ vcpu pointer always in r0 · line 329

Quoted 13 lines
arch/arm/kvm/interrupts_head.S +3 -1
 	.endif
 .endm
 
@@ -319,12 +322,15 @@ vcpu	.req	r0		@ vcpu pointer always in r0
  */
 .macro write_cp15_state read_from_vcpu
 	.if \read_from_vcpu == 0
-	pop	{r2}
+	pop	{r2-r4}
 	.else
 	ldr	r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
+	add	r12, vcpu, #CP15_OFFSET(c7_PAR)
+	ldrd	r3, r4, [r12]

ditto

Quoted 10 lines
arch/arm/kvm/interrupts_head.S +1 -0
 	.endif
 
 	mcr	p15, 0, r2, c14, c1, 0	@ CNTKCTL
+	mcrr	p15, 0, r3, r4, c7	@ PAR
 
 	.if \read_from_vcpu == 0
 	pop	{r2-r12}
Signature · 1 line
1.8.2.3

Here's the fixup patch:

arch/arm/kvm/interrupts_head.S +6 -6
index 2478af1..2b44b95 100644
@@ -302,14 +302,14 @@ vcpu	.req	r0		@ vcpu pointer always in r0
 	.endif
 
 	mrc	p15, 0, r2, c14, c1, 0	@ CNTKCTL
-	mrrc	p15, 0, r3, r4, c7	@ PAR
+	mrrc	p15, 0, r4, r5, c7	@ PAR
 
 	.if \store_to_vcpu == 0
-	push	{r2-r4}
+	push	{r2,r4-r5}
 	.else
 	str	r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
 	add	r12, vcpu, #CP15_OFFSET(c7_PAR)
-	strd	r3, r4, [r12]
+	strd	r4, r5, [r12]
 	.endif
 .endm
 
@@ -322,15 +322,15 @@ vcpu	.req	r0		@ vcpu pointer always in r0
  */
 .macro write_cp15_state read_from_vcpu
 	.if \read_from_vcpu == 0
-	pop	{r2-r4}
+	pop	{r2,r4-r5}
 	.else
 	ldr	r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
 	add	r12, vcpu, #CP15_OFFSET(c7_PAR)
-	ldrd	r3, r4, [r12]
+	ldrd	r4, r5, [r12]
 	.endif
 
 	mcr	p15, 0, r2, c14, c1, 0	@ CNTKCTL
-	mcrr	p15, 0, r3, r4, c7	@ PAR
+	mcrr	p15, 0, r4, r5, c7	@ PAR
 
 	.if \read_from_vcpu == 0
 	pop	{r2-r12}

-Christoffer

#3 lore raw

PATCH v2 1/3 ARM: KVM: perform save/restore of PAR

Quoted 116 lines · Christoffer Dall
Quoted 115 lines · Marc Zyngier

Not saving PAR is an unfortunate oversight. If the guest performs an AT* operation and gets scheduled out before reading the result of the translation from PAR, it could become corrupted by another guest or the host.

Saving this register is made slightly more complicated as KVM also uses it on the permission fault handling path, leading to an ugly "stash and restore" sequence. Fortunately, this is already a slow path so we don't really care. Also, Linux doesn't do any AT* operation, so Linux guests are not impacted by this bug.

---
 arch/arm/include/asm/kvm_asm.h | 22 ++++++++++++----------
 arch/arm/kvm/coproc.c          |  4 ++++
 arch/arm/kvm/interrupts.S      | 12 +++++++++++-
 arch/arm/kvm/interrupts_head.S | 10 ++++++++--
 4 files changed, 35 insertions(+), 13 deletions(-)
arch/arm/include/asm/kvm_asm.h +12 -10
index 18d5032..4bb08e3 100644
@@ -37,16 +37,18 @@
 #define c5_AIFSR	15	/* Auxilary Instrunction Fault Status R */
 #define c6_DFAR		16	/* Data Fault Address Register */
 #define c6_IFAR		17	/* Instruction Fault Address Register */
-#define c9_L2CTLR	18	/* Cortex A15 L2 Control Register */
-#define c10_PRRR	19	/* Primary Region Remap Register */
-#define c10_NMRR	20	/* Normal Memory Remap Register */
-#define c12_VBAR	21	/* Vector Base Address Register */
-#define c13_CID		22	/* Context ID Register */
-#define c13_TID_URW	23	/* Thread ID, User R/W */
-#define c13_TID_URO	24	/* Thread ID, User R/O */
-#define c13_TID_PRIV	25	/* Thread ID, Privileged */
-#define c14_CNTKCTL	26	/* Timer Control Register (PL1) */
-#define NR_CP15_REGS	27	/* Number of regs (incl. invalid) */
+#define c7_PAR		18	/* Physical Address Register */
+#define c7_PAR_high	19	/* PAR top 32 bits */
+#define c9_L2CTLR	20	/* Cortex A15 L2 Control Register */
+#define c10_PRRR	21	/* Primary Region Remap Register */
+#define c10_NMRR	22	/* Normal Memory Remap Register */
+#define c12_VBAR	23	/* Vector Base Address Register */
+#define c13_CID		24	/* Context ID Register */
+#define c13_TID_URW	25	/* Thread ID, User R/W */
+#define c13_TID_URO	26	/* Thread ID, User R/O */
+#define c13_TID_PRIV	27	/* Thread ID, Privileged */
+#define c14_CNTKCTL	28	/* Timer Control Register (PL1) */
+#define NR_CP15_REGS	29	/* Number of regs (incl. invalid) */
 
 #define ARM_EXCEPTION_RESET	  0
 #define ARM_EXCEPTION_UNDEFINED   1
arch/arm/kvm/coproc.c +4 -0
index 8eea97b..4a51990 100644
@@ -180,6 +180,10 @@ static const struct coproc_reg cp15_regs[] = {
 			NULL, reset_unknown, c6_DFAR },
 	{ CRn( 6), CRm( 0), Op1( 0), Op2( 2), is32,
 			NULL, reset_unknown, c6_IFAR },
+
+	/* PAR swapped by interrupt.S */
+	{ CRn( 7), Op1( 0), is64, NULL, reset_unknown64, c7_PAR },
+
 	/*
 	 * DC{C,I,CI}SW operations:
 	 */
arch/arm/kvm/interrupts.S +11 -1
index f7793df..d0a8fa3 100644
@@ -414,6 +414,10 @@ guest_trap:
 	mrcne	p15, 4, r2, c6, c0, 4	@ HPFAR
 	bne	3f
 
+	/* Preserve PAR */
+	mrrc	p15, 0, r0, r1, c7	@ PAR
+	push	{r0, r1}
+
 	/* Resolve IPA using the xFAR */
 	mcr	p15, 0, r2, c7, c8, 0	@ ATS1CPR
 	isb
@@ -424,13 +428,19 @@ guest_trap:
 	lsl	r2, r2, #4
 	orr	r2, r2, r1, lsl #24
 
+	/* Restore PAR */
+	pop	{r0, r1}
+	mcrr	p15, 0, r0, r1, c7	@ PAR
+
 3:	load_vcpu			@ Load VCPU pointer to r0
 	str	r2, [r0, #VCPU_HPFAR]
 
 1:	mov	r1, #ARM_EXCEPTION_HVC
 	b	__kvm_vcpu_return
 
-4:	pop	{r0, r1, r2}		@ Failed translation, return to guest
+4:	pop	{r0, r1}		@ Failed translation, return to guest
+	mcrr	p15, 0, r0, r1, c7	@ PAR
+	pop	{r0, r1, r2}
 	eret
 
 /*
arch/arm/kvm/interrupts_head.S +1 -0
index 3c8f2f0..2478af1 100644
@@ -302,11 +302,14 @@ vcpu	.req	r0		@ vcpu pointer always in r0
 	.endif
 
 	mrc	p15, 0, r2, c14, c1, 0	@ CNTKCTL
+	mrrc	p15, 0, r3, r4, c7	@ PAR
 
Quoted 10 lines

arch/arm/kvm/interrupts_head.S vcpu .req r0 @ vcpu pointer always in r0 · line 312

Quoted 7 lines
arch/arm/kvm/interrupts_head.S +3 -1
 	.if \store_to_vcpu == 0
-	push	{r2}
+	push	{r2-r4}
 	.else
 	str	r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
+	add	r12, vcpu, #CP15_OFFSET(c7_PAR)
+	strd	r3, r4, [r12]

my compiler complains that the load should start with an even register, so I've changed this with the patch below.

Ah, ARM kernel - I get bitten by this all the time. Note to self: test ARM builds as well as Thumb2... ;-)

Thanks for noticing this.

Quoted 61 lines

arch/arm/kvm/interrupts_head.S vcpu .req r0 @ vcpu pointer always in r0 · line 329

Quoted 13 lines
arch/arm/kvm/interrupts_head.S +3 -1
 	.endif
 .endm
 
@@ -319,12 +322,15 @@ vcpu	.req	r0		@ vcpu pointer always in r0
  */
 .macro write_cp15_state read_from_vcpu
 	.if \read_from_vcpu == 0
-	pop	{r2}
+	pop	{r2-r4}
 	.else
 	ldr	r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
+	add	r12, vcpu, #CP15_OFFSET(c7_PAR)
+	ldrd	r3, r4, [r12]

ditto

Quoted 10 lines
arch/arm/kvm/interrupts_head.S +1 -0
 	.endif
 
 	mcr	p15, 0, r2, c14, c1, 0	@ CNTKCTL
+	mcrr	p15, 0, r3, r4, c7	@ PAR
 
 	.if \read_from_vcpu == 0
 	pop	{r2-r12}
Signature · 1 line
1.8.2.3

Here's the fixup patch:

arch/arm/kvm/interrupts_head.S +4 -4
index 2478af1..2b44b95 100644
@@ -302,14 +302,14 @@ vcpu	.req	r0		@ vcpu pointer always in r0
 	.endif
 
 	mrc	p15, 0, r2, c14, c1, 0	@ CNTKCTL
-	mrrc	p15, 0, r3, r4, c7	@ PAR
+	mrrc	p15, 0, r4, r5, c7	@ PAR
 
 	.if \store_to_vcpu == 0
-	push	{r2-r4}
+	push	{r2,r4-r5}
 	.else
 	str	r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
 	add	r12, vcpu, #CP15_OFFSET(c7_PAR)
-	strd	r3, r4, [r12]
+	strd	r4, r5, [r12]
 	.endif
 .endm
 
@@ -322,15 +322,15 @@ vcpu	.req	r0		@ vcpu pointer always in r0
  */
 .macro write_cp15_state read_from_vcpu
 	.if \read_from_vcpu == 0
-	pop	{r2-r4}
+	pop	{r2,r4-r5}
 	.else
 	ldr	r2, [vcpu, #CP15_OFFSET(c14_CNTKCTL)]
 	add	r12, vcpu, #CP15_OFFSET(c7_PAR)

arch/arm/kvm/interrupts_head.S vcpu .req r0 @ vcpu pointer always in r0 · line 336

Quoted 10 lines
arch/arm/kvm/interrupts_head.S +2 -2
-	ldrd	r3, r4, [r12]
+	ldrd	r4, r5, [r12]
 	.endif
 
 	mcr	p15, 0, r2, c14, c1, 0	@ CNTKCTL
-	mcrr	p15, 0, r3, r4, c7	@ PAR
+	mcrr	p15, 0, r4, r5, c7	@ PAR
 
 	.if \read_from_vcpu == 0
 	pop	{r2-r12}

Looks good. Thanks for taking care of this.

        M.
Signature · 1 line
Fast, cheap, reliable. Pick two.
#4 lore raw

PATCH v2 2/3 ARM: KVM: add missing dsb before invalidating Stage-2 TLBs

In reply to Marc Zyngier

When performing a Stage-2 TLB invalidation, it is necessary to make sure the write to the page tables is observable by all CPUs.

For this purpose, add a dsb instruction to __kvm_tlb_flush_vmid_ipa before doing the TLB invalidation itself.

---
 arch/arm/kvm/interrupts.S | 1 +
 1 file changed, 1 insertion(+)
arch/arm/kvm/interrupts.S +1 -0
index d0a8fa3..20e03d9 100644
@@ -49,6 +49,7 @@ __kvm_hyp_code_start:
 ENTRY(__kvm_tlb_flush_vmid_ipa)
 	push	{r2, r3}
 
+	dsb	ishst
 	add	r0, r0, #KVM_VTTBR
 	ldrd	r2, r3, [r0]
 	mcrr	p15, 6, r2, r3, c2	@ Write VTTBR
Signature · 1 line
1.8.2.3
#5 lore raw

PATCH v2 3/3 ARM: KVM: clear exclusive monitor on all exception returns

In reply to Marc Zyngier

Make sure we clear the exclusive movitor on all exception returns, which otherwise could lead to lock corruptions.

---
 arch/arm/kvm/interrupts.S | 3 +++
 1 file changed, 3 insertions(+)
arch/arm/kvm/interrupts.S +3 -0
index 20e03d9..16cd4ba 100644
@@ -292,6 +292,7 @@ THUMB(	orr	r2, r2, #PSR_T_BIT	)
 	ldr	r2, =BSYM(panic)
 	msr	ELR_hyp, r2
 	ldr	r0, =\panic_str
+	clrex				@ Clear exclusive monitor
 	eret
 .endm
 
@@ -441,6 +442,7 @@ guest_trap:
 
 4:	pop	{r0, r1}		@ Failed translation, return to guest
 	mcrr	p15, 0, r0, r1, c7	@ PAR
+	clrex
 	pop	{r0, r1, r2}
 	eret
 
@@ -467,6 +469,7 @@ switch_to_guest_vfp:
 
 	pop	{r3-r7}
 	pop	{r0-r2}
+	clrex
 	eret
 #endif
 
Signature · 1 line
1.8.2.3
#6 lore raw

PATCH v2 3/3 ARM: KVM: clear exclusive monitor on all exception returns

Hello.

Quoted 1 line · Marc Zyngier

Make sure we clear the exclusive movitor on all exception returns,

Only "monitor". Perhaps a committer could fix...

Quoted 3 lines

which otherwise could lead to lock corruptions.

WBR, Sergei

#7 lore raw

PATCH v2 3/3 ARM: KVM: clear exclusive monitor on all exception returns

Quoted 8 lines · Sergei Shtylyov

Hello.

Quoted 1 line · Marc Zyngier

Make sure we clear the exclusive movitor on all exception returns,

Only "monitor". Perhaps a committer could fix...

fixed,
-Christoffer

#8 lore raw

v2 cover A handful of KVM/ARM fixes

In reply to Marc Zyngier
Quoted 10 lines · Marc Zyngier

This is a small patch series that I accumulated while tracking what turned out to be an unrelated issue.

The first patch has already been posted a while ago and included here for completeness. The others are fairly simple missing barriers/clear exclusives that are actually required.

This has been tested on TC2, and is also available in my tree:
git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git kvm-arm/fixes-3.10-rc6

Applied with the minor fixes addressed on the individual patch mails.

Thanks,
-Christoffer