| Message ID | 1515665499-31710-19-git-send-email-wei.guo.simon@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [01/26] KVM: PPC: Book3S PR: Move kvmppc_save_tm/kvmppc_restore_tm to separate file | expand |
On Thu, Jan 11, 2018 at 06:11:31PM +0800, wei.guo.simon@gmail.com wrote: > From: Simon Guo <wei.guo.simon@gmail.com> > > The mfspr/mtspr on TM SPRs(TEXASR/TFIAR/TFHAR) are non-privileged > instructions and can be executed at PR KVM guest without trapping > into host in problem state. We only emulate mtspr/mfspr > texasr/tfiar/tfhar at guest PR=0 state. > > When we are emulating mtspr tm sprs at guest PR=0 state, the emulation > result need to be visible to guest PR=1 state. That is, the actual TM > SPR val should be loaded into actual registers. > > We already flush TM SPRs into vcpu when switching out of CPU, and load > TM SPRs when switching back. > > This patch corrects mfspr()/mtspr() emulation for TM SPRs to make the > actual source/dest based on actual TM SPRs. > > Signed-off-by: Simon Guo <wei.guo.simon@gmail.com> > --- > arch/powerpc/kvm/book3s_emulate.c | 35 +++++++++++++++++++++++++++-------- > 1 file changed, 27 insertions(+), 8 deletions(-) > > diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c > index e096d01..c2836330 100644 > --- a/arch/powerpc/kvm/book3s_emulate.c > +++ b/arch/powerpc/kvm/book3s_emulate.c > @@ -521,13 +521,26 @@ int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val) > break; > #ifdef CONFIG_PPC_TRANSACTIONAL_MEM > case SPRN_TFHAR: > - vcpu->arch.tfhar = spr_val; > - break; > case SPRN_TEXASR: > - vcpu->arch.texasr = spr_val; > - break; > case SPRN_TFIAR: > - vcpu->arch.tfiar = spr_val; > + if (MSR_TM_ACTIVE(kvmppc_get_msr(vcpu))) { > + /* it is illegal to mtspr() TM regs in > + * other than non-transactional state. > + */ > + kvmppc_core_queue_program(vcpu, SRR1_PROGTM); > + emulated = EMULATE_AGAIN; > + break; > + } We also need to check that the guest has TM enabled in the guest MSR, and give them a facility unavailable interrupt if not. > + > + tm_enable(); > + if (sprn == SPRN_TFHAR) > + mtspr(SPRN_TFHAR, spr_val); > + else if (sprn == SPRN_TEXASR) > + mtspr(SPRN_TEXASR, spr_val); > + else > + mtspr(SPRN_TFIAR, spr_val); > + tm_disable(); I haven't seen any checks that we are on a CPU that has TM. What happens if a guest does a mtmsrd with TM=1 and then a mtspr to TEXASR when running on a POWER7 (assuming the host kernel was compiled with CONFIG_PPC_TRANSACTIONAL_MEM=y)? Ideally, if the host CPU does not have TM functionality, these mtsprs would be treated as no-ops and attempts to set the TM or TS fields in the guest MSR would be ignored. > + > break; > #endif > #endif > @@ -674,13 +687,19 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val > break; > #ifdef CONFIG_PPC_TRANSACTIONAL_MEM > case SPRN_TFHAR: > - *spr_val = vcpu->arch.tfhar; > + tm_enable(); > + *spr_val = mfspr(SPRN_TFHAR); > + tm_disable(); > break; > case SPRN_TEXASR: > - *spr_val = vcpu->arch.texasr; > + tm_enable(); > + *spr_val = mfspr(SPRN_TEXASR); > + tm_disable(); > break; > case SPRN_TFIAR: > - *spr_val = vcpu->arch.tfiar; > + tm_enable(); > + *spr_val = mfspr(SPRN_TFIAR); > + tm_disable(); > break; These need to check MSR_TM in the guest MSR, and become no-ops on machines without TM capability. Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Paul, On Tue, Jan 23, 2018 at 07:17:45PM +1100, Paul Mackerras wrote: > On Thu, Jan 11, 2018 at 06:11:31PM +0800, wei.guo.simon@gmail.com wrote: > > From: Simon Guo <wei.guo.simon@gmail.com> > > > > The mfspr/mtspr on TM SPRs(TEXASR/TFIAR/TFHAR) are non-privileged > > instructions and can be executed at PR KVM guest without trapping > > into host in problem state. We only emulate mtspr/mfspr > > texasr/tfiar/tfhar at guest PR=0 state. > > > > When we are emulating mtspr tm sprs at guest PR=0 state, the emulation > > result need to be visible to guest PR=1 state. That is, the actual TM > > SPR val should be loaded into actual registers. > > > > We already flush TM SPRs into vcpu when switching out of CPU, and load > > TM SPRs when switching back. > > > > This patch corrects mfspr()/mtspr() emulation for TM SPRs to make the > > actual source/dest based on actual TM SPRs. > > > > Signed-off-by: Simon Guo <wei.guo.simon@gmail.com> > > --- > > arch/powerpc/kvm/book3s_emulate.c | 35 +++++++++++++++++++++++++++-------- > > 1 file changed, 27 insertions(+), 8 deletions(-) > > > > diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c > > index e096d01..c2836330 100644 > > --- a/arch/powerpc/kvm/book3s_emulate.c > > +++ b/arch/powerpc/kvm/book3s_emulate.c > > @@ -521,13 +521,26 @@ int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val) > > break; > > #ifdef CONFIG_PPC_TRANSACTIONAL_MEM > > case SPRN_TFHAR: > > - vcpu->arch.tfhar = spr_val; > > - break; > > case SPRN_TEXASR: > > - vcpu->arch.texasr = spr_val; > > - break; > > case SPRN_TFIAR: > > - vcpu->arch.tfiar = spr_val; > > + if (MSR_TM_ACTIVE(kvmppc_get_msr(vcpu))) { > > + /* it is illegal to mtspr() TM regs in > > + * other than non-transactional state. > > + */ > > + kvmppc_core_queue_program(vcpu, SRR1_PROGTM); > > + emulated = EMULATE_AGAIN; > > + break; > > + } > > We also need to check that the guest has TM enabled in the guest MSR, > and give them a facility unavailable interrupt if not. > > > + > > + tm_enable(); > > + if (sprn == SPRN_TFHAR) > > + mtspr(SPRN_TFHAR, spr_val); > > + else if (sprn == SPRN_TEXASR) > > + mtspr(SPRN_TEXASR, spr_val); > > + else > > + mtspr(SPRN_TFIAR, spr_val); > > + tm_disable(); > > I haven't seen any checks that we are on a CPU that has TM. What > happens if a guest does a mtmsrd with TM=1 and then a mtspr to TEXASR > when running on a POWER7 (assuming the host kernel was compiled with > CONFIG_PPC_TRANSACTIONAL_MEM=y)? > > Ideally, if the host CPU does not have TM functionality, these mtsprs > would be treated as no-ops and attempts to set the TM or TS fields in > the guest MSR would be ignored. > > > + > > break; > > #endif > > #endif > > @@ -674,13 +687,19 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val > > break; > > #ifdef CONFIG_PPC_TRANSACTIONAL_MEM > > case SPRN_TFHAR: > > - *spr_val = vcpu->arch.tfhar; > > + tm_enable(); > > + *spr_val = mfspr(SPRN_TFHAR); > > + tm_disable(); > > break; > > case SPRN_TEXASR: > > - *spr_val = vcpu->arch.texasr; > > + tm_enable(); > > + *spr_val = mfspr(SPRN_TEXASR); > > + tm_disable(); > > break; > > case SPRN_TFIAR: > > - *spr_val = vcpu->arch.tfiar; > > + tm_enable(); > > + *spr_val = mfspr(SPRN_TFIAR); > > + tm_disable(); > > break; > > These need to check MSR_TM in the guest MSR, and become no-ops on > machines without TM capability. Thanks for the above catches. I will rework later. BR, - Simon -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/powerpc/kvm/book3s_emulate.c b/arch/powerpc/kvm/book3s_emulate.c index e096d01..c2836330 100644 --- a/arch/powerpc/kvm/book3s_emulate.c +++ b/arch/powerpc/kvm/book3s_emulate.c @@ -521,13 +521,26 @@ int kvmppc_core_emulate_mtspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong spr_val) break; #ifdef CONFIG_PPC_TRANSACTIONAL_MEM case SPRN_TFHAR: - vcpu->arch.tfhar = spr_val; - break; case SPRN_TEXASR: - vcpu->arch.texasr = spr_val; - break; case SPRN_TFIAR: - vcpu->arch.tfiar = spr_val; + if (MSR_TM_ACTIVE(kvmppc_get_msr(vcpu))) { + /* it is illegal to mtspr() TM regs in + * other than non-transactional state. + */ + kvmppc_core_queue_program(vcpu, SRR1_PROGTM); + emulated = EMULATE_AGAIN; + break; + } + + tm_enable(); + if (sprn == SPRN_TFHAR) + mtspr(SPRN_TFHAR, spr_val); + else if (sprn == SPRN_TEXASR) + mtspr(SPRN_TEXASR, spr_val); + else + mtspr(SPRN_TFIAR, spr_val); + tm_disable(); + break; #endif #endif @@ -674,13 +687,19 @@ int kvmppc_core_emulate_mfspr_pr(struct kvm_vcpu *vcpu, int sprn, ulong *spr_val break; #ifdef CONFIG_PPC_TRANSACTIONAL_MEM case SPRN_TFHAR: - *spr_val = vcpu->arch.tfhar; + tm_enable(); + *spr_val = mfspr(SPRN_TFHAR); + tm_disable(); break; case SPRN_TEXASR: - *spr_val = vcpu->arch.texasr; + tm_enable(); + *spr_val = mfspr(SPRN_TEXASR); + tm_disable(); break; case SPRN_TFIAR: - *spr_val = vcpu->arch.tfiar; + tm_enable(); + *spr_val = mfspr(SPRN_TFIAR); + tm_disable(); break; #endif #endif