diff mbox

KVM: PPC: Book3S HV: Save/restore XER in checkpointed register state

Message ID 20161107040958.dzopsvoelow25css@oak.ozlabs.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

Paul Mackerras Nov. 7, 2016, 4:09 a.m. UTC
When switching from/to a guest that has a transaction in progress,
we need to save/restore the checkpointed register state.  Although
XER is part of the CPU state that gets checkpointed, the code that
does this saving and restoring doesn't save/restore XER.

This fixes it by saving and restoring the XER.  To allow userspace
to read/write the checkpointed XER value, we also add a new ONE_REG
specifier.

The visible effect of this bug is that the guest may see its XER
value being corrupted when it uses transactions.

Fixes: e4e38121507a ("KVM: PPC: Book3S HV: Add transactional memory support")
Fixes: 0a8eccefcb34 ("KVM: PPC: Book3S HV: Add missing code for transaction reclaim on guest exit")
Cc: stable@vger.kernel.org # v3.15+
Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
---
 Documentation/virtual/kvm/api.txt       | 1 +
 arch/powerpc/include/asm/kvm_host.h     | 1 +
 arch/powerpc/include/uapi/asm/kvm.h     | 1 +
 arch/powerpc/kernel/asm-offsets.c       | 1 +
 arch/powerpc/kvm/book3s_hv.c            | 6 ++++++
 arch/powerpc/kvm/book3s_hv_rmhandlers.S | 4 ++++
 6 files changed, 14 insertions(+)

Comments

Thomas Huth Nov. 7, 2016, 7:38 a.m. UTC | #1
On 07.11.2016 05:09, Paul Mackerras wrote:
> When switching from/to a guest that has a transaction in progress,
> we need to save/restore the checkpointed register state.  Although
> XER is part of the CPU state that gets checkpointed, the code that
> does this saving and restoring doesn't save/restore XER.
> 
> This fixes it by saving and restoring the XER.  To allow userspace
> to read/write the checkpointed XER value, we also add a new ONE_REG
> specifier.
> 
> The visible effect of this bug is that the guest may see its XER
> value being corrupted when it uses transactions.
> 
> Fixes: e4e38121507a ("KVM: PPC: Book3S HV: Add transactional memory support")
> Fixes: 0a8eccefcb34 ("KVM: PPC: Book3S HV: Add missing code for transaction reclaim on guest exit")
> Cc: stable@vger.kernel.org # v3.15+
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>
> ---
>  Documentation/virtual/kvm/api.txt       | 1 +
>  arch/powerpc/include/asm/kvm_host.h     | 1 +
>  arch/powerpc/include/uapi/asm/kvm.h     | 1 +
>  arch/powerpc/kernel/asm-offsets.c       | 1 +
>  arch/powerpc/kvm/book3s_hv.c            | 6 ++++++
>  arch/powerpc/kvm/book3s_hv_rmhandlers.S | 4 ++++
>  6 files changed, 14 insertions(+)
> 
> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
> index 739db9a..a7596e9 100644
> --- a/Documentation/virtual/kvm/api.txt
> +++ b/Documentation/virtual/kvm/api.txt
> @@ -2039,6 +2039,7 @@ registers, find a list below:
>    PPC   | KVM_REG_PPC_TM_VSCR           | 32
>    PPC   | KVM_REG_PPC_TM_DSCR           | 64
>    PPC   | KVM_REG_PPC_TM_TAR            | 64
> +  PPC   | KVM_REG_PPC_TM_XER            | 64
>          |                               |
>    MIPS  | KVM_REG_MIPS_R0               | 64
>            ...
> diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
> index 28350a2..5e12e19 100644
> --- a/arch/powerpc/include/asm/kvm_host.h
> +++ b/arch/powerpc/include/asm/kvm_host.h
> @@ -546,6 +546,7 @@ struct kvm_vcpu_arch {
>  	u64 tfiar;
>  
>  	u32 cr_tm;
> +	u64 xer_tm;
>  	u64 lr_tm;
>  	u64 ctr_tm;
>  	u64 amr_tm;
> diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
> index c93cf35..0fb1326 100644
> --- a/arch/powerpc/include/uapi/asm/kvm.h
> +++ b/arch/powerpc/include/uapi/asm/kvm.h
> @@ -596,6 +596,7 @@ struct kvm_get_htab_header {
>  #define KVM_REG_PPC_TM_VSCR	(KVM_REG_PPC_TM | KVM_REG_SIZE_U32 | 0x67)
>  #define KVM_REG_PPC_TM_DSCR	(KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x68)
>  #define KVM_REG_PPC_TM_TAR	(KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x69)
> +#define KVM_REG_PPC_TM_XER	(KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x6a)
>  
>  /* PPC64 eXternal Interrupt Controller Specification */
>  #define KVM_DEV_XICS_GRP_SOURCES	1	/* 64-bit source attributes */
> diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
> index caec7bf..c833d88 100644
> --- a/arch/powerpc/kernel/asm-offsets.c
> +++ b/arch/powerpc/kernel/asm-offsets.c
> @@ -569,6 +569,7 @@ int main(void)
>  	DEFINE(VCPU_VRS_TM, offsetof(struct kvm_vcpu, arch.vr_tm.vr));
>  	DEFINE(VCPU_VRSAVE_TM, offsetof(struct kvm_vcpu, arch.vrsave_tm));
>  	DEFINE(VCPU_CR_TM, offsetof(struct kvm_vcpu, arch.cr_tm));
> +	DEFINE(VCPU_XER_TM, offsetof(struct kvm_vcpu, arch.xer_tm));
>  	DEFINE(VCPU_LR_TM, offsetof(struct kvm_vcpu, arch.lr_tm));
>  	DEFINE(VCPU_CTR_TM, offsetof(struct kvm_vcpu, arch.ctr_tm));
>  	DEFINE(VCPU_AMR_TM, offsetof(struct kvm_vcpu, arch.amr_tm));
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 3686471..094deb6 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -1288,6 +1288,9 @@ static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
>  	case KVM_REG_PPC_TM_CR:
>  		*val = get_reg_val(id, vcpu->arch.cr_tm);
>  		break;
> +	case KVM_REG_PPC_TM_XER:
> +		*val = get_reg_val(id, vcpu->arch.xer_tm);
> +		break;
>  	case KVM_REG_PPC_TM_LR:
>  		*val = get_reg_val(id, vcpu->arch.lr_tm);
>  		break;
> @@ -1498,6 +1501,9 @@ static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
>  	case KVM_REG_PPC_TM_CR:
>  		vcpu->arch.cr_tm = set_reg_val(id, *val);
>  		break;
> +	case KVM_REG_PPC_TM_XER:
> +		vcpu->arch.xer_tm = set_reg_val(id, *val);
> +		break;
>  	case KVM_REG_PPC_TM_LR:
>  		vcpu->arch.lr_tm = set_reg_val(id, *val);
>  		break;
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index c3c1d1b..6f81adb 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -2600,11 +2600,13 @@ kvmppc_save_tm:
>  	mfctr	r7
>  	mfspr	r8, SPRN_AMR
>  	mfspr	r10, SPRN_TAR
> +	mfxer	r11
>  	std	r5, VCPU_LR_TM(r9)
>  	stw	r6, VCPU_CR_TM(r9)
>  	std	r7, VCPU_CTR_TM(r9)
>  	std	r8, VCPU_AMR_TM(r9)
>  	std	r10, VCPU_TAR_TM(r9)
> +	std	r11, VCPU_XER_TM(r9)
>  
>  	/* Restore r12 as trap number. */
>  	lwz	r12, VCPU_TRAP(r9)
> @@ -2697,11 +2699,13 @@ kvmppc_restore_tm:
>  	ld	r7, VCPU_CTR_TM(r4)
>  	ld	r8, VCPU_AMR_TM(r4)
>  	ld	r9, VCPU_TAR_TM(r4)
> +	ld	r10, VCPU_XER_TM(r4)
>  	mtlr	r5
>  	mtcr	r6
>  	mtctr	r7
>  	mtspr	SPRN_AMR, r8
>  	mtspr	SPRN_TAR, r9
> +	mtxer	r10
>  
>  	/*
>  	 * Load up PPR and DSCR values but don't put them in the actual SPRs
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Paul Mackerras Nov. 21, 2016, 5:07 a.m. UTC | #2
On Mon, Nov 07, 2016 at 03:09:58PM +1100, Paul Mackerras wrote:
> When switching from/to a guest that has a transaction in progress,
> we need to save/restore the checkpointed register state.  Although
> XER is part of the CPU state that gets checkpointed, the code that
> does this saving and restoring doesn't save/restore XER.
> 
> This fixes it by saving and restoring the XER.  To allow userspace
> to read/write the checkpointed XER value, we also add a new ONE_REG
> specifier.
> 
> The visible effect of this bug is that the guest may see its XER
> value being corrupted when it uses transactions.
> 
> Fixes: e4e38121507a ("KVM: PPC: Book3S HV: Add transactional memory support")
> Fixes: 0a8eccefcb34 ("KVM: PPC: Book3S HV: Add missing code for transaction reclaim on guest exit")
> Cc: stable@vger.kernel.org # v3.15+
> Signed-off-by: Paul Mackerras <paulus@ozlabs.org>

Applied to kvm-ppc-next.

Paul.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 739db9a..a7596e9 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2039,6 +2039,7 @@  registers, find a list below:
   PPC   | KVM_REG_PPC_TM_VSCR           | 32
   PPC   | KVM_REG_PPC_TM_DSCR           | 64
   PPC   | KVM_REG_PPC_TM_TAR            | 64
+  PPC   | KVM_REG_PPC_TM_XER            | 64
         |                               |
   MIPS  | KVM_REG_MIPS_R0               | 64
           ...
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index 28350a2..5e12e19 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -546,6 +546,7 @@  struct kvm_vcpu_arch {
 	u64 tfiar;
 
 	u32 cr_tm;
+	u64 xer_tm;
 	u64 lr_tm;
 	u64 ctr_tm;
 	u64 amr_tm;
diff --git a/arch/powerpc/include/uapi/asm/kvm.h b/arch/powerpc/include/uapi/asm/kvm.h
index c93cf35..0fb1326 100644
--- a/arch/powerpc/include/uapi/asm/kvm.h
+++ b/arch/powerpc/include/uapi/asm/kvm.h
@@ -596,6 +596,7 @@  struct kvm_get_htab_header {
 #define KVM_REG_PPC_TM_VSCR	(KVM_REG_PPC_TM | KVM_REG_SIZE_U32 | 0x67)
 #define KVM_REG_PPC_TM_DSCR	(KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x68)
 #define KVM_REG_PPC_TM_TAR	(KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x69)
+#define KVM_REG_PPC_TM_XER	(KVM_REG_PPC_TM | KVM_REG_SIZE_U64 | 0x6a)
 
 /* PPC64 eXternal Interrupt Controller Specification */
 #define KVM_DEV_XICS_GRP_SOURCES	1	/* 64-bit source attributes */
diff --git a/arch/powerpc/kernel/asm-offsets.c b/arch/powerpc/kernel/asm-offsets.c
index caec7bf..c833d88 100644
--- a/arch/powerpc/kernel/asm-offsets.c
+++ b/arch/powerpc/kernel/asm-offsets.c
@@ -569,6 +569,7 @@  int main(void)
 	DEFINE(VCPU_VRS_TM, offsetof(struct kvm_vcpu, arch.vr_tm.vr));
 	DEFINE(VCPU_VRSAVE_TM, offsetof(struct kvm_vcpu, arch.vrsave_tm));
 	DEFINE(VCPU_CR_TM, offsetof(struct kvm_vcpu, arch.cr_tm));
+	DEFINE(VCPU_XER_TM, offsetof(struct kvm_vcpu, arch.xer_tm));
 	DEFINE(VCPU_LR_TM, offsetof(struct kvm_vcpu, arch.lr_tm));
 	DEFINE(VCPU_CTR_TM, offsetof(struct kvm_vcpu, arch.ctr_tm));
 	DEFINE(VCPU_AMR_TM, offsetof(struct kvm_vcpu, arch.amr_tm));
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index 3686471..094deb6 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -1288,6 +1288,9 @@  static int kvmppc_get_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
 	case KVM_REG_PPC_TM_CR:
 		*val = get_reg_val(id, vcpu->arch.cr_tm);
 		break;
+	case KVM_REG_PPC_TM_XER:
+		*val = get_reg_val(id, vcpu->arch.xer_tm);
+		break;
 	case KVM_REG_PPC_TM_LR:
 		*val = get_reg_val(id, vcpu->arch.lr_tm);
 		break;
@@ -1498,6 +1501,9 @@  static int kvmppc_set_one_reg_hv(struct kvm_vcpu *vcpu, u64 id,
 	case KVM_REG_PPC_TM_CR:
 		vcpu->arch.cr_tm = set_reg_val(id, *val);
 		break;
+	case KVM_REG_PPC_TM_XER:
+		vcpu->arch.xer_tm = set_reg_val(id, *val);
+		break;
 	case KVM_REG_PPC_TM_LR:
 		vcpu->arch.lr_tm = set_reg_val(id, *val);
 		break;
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index c3c1d1b..6f81adb 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -2600,11 +2600,13 @@  kvmppc_save_tm:
 	mfctr	r7
 	mfspr	r8, SPRN_AMR
 	mfspr	r10, SPRN_TAR
+	mfxer	r11
 	std	r5, VCPU_LR_TM(r9)
 	stw	r6, VCPU_CR_TM(r9)
 	std	r7, VCPU_CTR_TM(r9)
 	std	r8, VCPU_AMR_TM(r9)
 	std	r10, VCPU_TAR_TM(r9)
+	std	r11, VCPU_XER_TM(r9)
 
 	/* Restore r12 as trap number. */
 	lwz	r12, VCPU_TRAP(r9)
@@ -2697,11 +2699,13 @@  kvmppc_restore_tm:
 	ld	r7, VCPU_CTR_TM(r4)
 	ld	r8, VCPU_AMR_TM(r4)
 	ld	r9, VCPU_TAR_TM(r4)
+	ld	r10, VCPU_XER_TM(r4)
 	mtlr	r5
 	mtcr	r6
 	mtctr	r7
 	mtspr	SPRN_AMR, r8
 	mtspr	SPRN_TAR, r9
+	mtxer	r10
 
 	/*
 	 * Load up PPR and DSCR values but don't put them in the actual SPRs