On Thu, Jan 11, 2018 at 06:11:31PM +0800, wei.guo.simon@xxxxxxxxx wrote: > From: Simon Guo <wei.guo.simon@xxxxxxxxx> > > 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@xxxxxxxxx> > --- > 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.