Hi Paul, On Tue, Jan 23, 2018 at 04:42:09PM +1100, Paul Mackerras wrote: > On Thu, Jan 11, 2018 at 06:11:15PM +0800, wei.guo.simon@xxxxxxxxx wrote: > > From: Simon Guo <wei.guo.simon@xxxxxxxxx> > > > > HV KVM and PR KVM need different MSR source to indicate whether > > treclaim. or trecheckpoint. is necessary. > > > > This patch add new parameter (guest MSR) for these kvmppc_save_tm/ > > kvmppc_restore_tm() APIs: > > - For HV KVM, it is VCPU_MSR > > - For PR KVM, it is current host MSR or VCPU_SHADOW_SRR1 > > > > This enhancement enables these 2 APIs to be reused by PR KVM later. > > And the patch keeps HV KVM logic unchanged. > > > > This patch also reworks kvmppc_save_tm()/kvmppc_restore_tm() to > > have a clean ABI: r3 for vcpu and r4 for guest_msr. > > > > Signed-off-by: Simon Guo <wei.guo.simon@xxxxxxxxx> > > Question: why do you switch from using HSTATE_HOST_R1 to HSTATE_SCRATCH2 > > > @@ -42,11 +45,11 @@ _GLOBAL(kvmppc_save_tm) > > rldimi r8, r0, MSR_TM_LG, 63-MSR_TM_LG > > mtmsrd r8 > > > > - ld r5, VCPU_MSR(r9) > > - rldicl. r5, r5, 64 - MSR_TS_S_LG, 62 > > + rldicl. r4, r4, 64 - MSR_TS_S_LG, 62 > > beq 1f /* TM not active in guest. */ > > > > - std r1, HSTATE_HOST_R1(r13) > > + std r1, HSTATE_SCRATCH2(r13) > > ... here? > > > @@ -166,17 +173,17 @@ _GLOBAL(kvmppc_restore_tm) > > * The user may change these outside of a transaction, so they must > > * always be context switched. > > */ > > - ld r5, VCPU_TFHAR(r4) > > - ld r6, VCPU_TFIAR(r4) > > - ld r7, VCPU_TEXASR(r4) > > + ld r5, VCPU_TFHAR(r3) > > + ld r6, VCPU_TFIAR(r3) > > + ld r7, VCPU_TEXASR(r3) > > mtspr SPRN_TFHAR, r5 > > mtspr SPRN_TFIAR, r6 > > mtspr SPRN_TEXASR, r7 > > > > - ld r5, VCPU_MSR(r4) > > + mr r5, r4 > > rldicl. r5, r5, 64 - MSR_TS_S_LG, 62 > > beqlr /* TM not active in guest */ > > - std r1, HSTATE_HOST_R1(r13) > > + std r1, HSTATE_SCRATCH2(r13) > > and here? > > Please add a paragraph to the patch description explaining why you are > making that change. In subsequent patches, kvmppc_save_tm/kvmppc_restore_tm() will be invoked by wrapper function who setup addtional stack frame and update R1(then update HSTATE_HOST_R1 with addtional offset). Although HSTATE_HOST_R1 is now used safely(always PPC_STL before entering guest and PPC_LL in kvmppc_interrupt_pr()), I worried a future usage will take an assumption on HSTATE_HOST_R1 value and bring trouble. As a result, in kvmppc_save_tm/kvmppc_restore_tm() case, I choose HSTATE_SCRATCH2 to restore the r1. I will update the commit message. Thanks, - Simon > > Paul. -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
