Re: [PATCH v4 04/29] KVM: PPC: Book3S PR: Move kvmppc_save_tm/kvmppc_restore_tm to separate file

2018-05-30 Thread Simon Guo 
On Wed, May 30, 2018 at 09:40:27AM +1000, Paul Mackerras wrote:
> On Wed, May 23, 2018 at 03:01:47PM +0800, wei.guo.si...@gmail.com wrote:
> > From: Simon Guo 
> > 
> > It is a simple patch just for moving kvmppc_save_tm/kvmppc_restore_tm()
> > functionalities to tm.S. There is no logic change. The reconstruct of
> > those APIs will be done in later patches to improve readability.
> > 
> > It is for preparation of reusing those APIs on both HV/PR PPC KVM.
> > 
> > Some slight change during move the functions includes:
> > - surrounds some HV KVM specific code with CONFIG_KVM_BOOK3S_HV_POSSIBLE
> > for compilation.
> > - use _GLOBAL() to define kvmppc_save_tm/kvmppc_restore_tm()
> > 
> > Signed-off-by: Simon Guo 
> > ---
> >  arch/powerpc/kvm/Makefile   |   3 +
> >  arch/powerpc/kvm/book3s_hv_rmhandlers.S | 322 
> >  arch/powerpc/kvm/tm.S   | 363 
> > 
> >  3 files changed, 366 insertions(+), 322 deletions(-)
> >  create mode 100644 arch/powerpc/kvm/tm.S
> > 
> > diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
> > index 4b19da8..f872c04 100644
> > --- a/arch/powerpc/kvm/Makefile
> > +++ b/arch/powerpc/kvm/Makefile
> > @@ -63,6 +63,9 @@ kvm-pr-y := \
> > book3s_64_mmu.o \
> > book3s_32_mmu.o
> >  
> > +kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \
> > +   tm.o
> > +
> >  ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
> >  kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \
> > book3s_rmhandlers.o
> > diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S 
> > b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > index 5e6e493..4db2b10 100644
> > --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> > @@ -39,8 +39,6 @@ BEGIN_FTR_SECTION;\
> > extsw   reg, reg;   \
> >  END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
> >  
> > -#define VCPU_GPRS_TM(reg) (((reg) * ULONG_SIZE) + VCPU_GPR_TM)
> > -
> >  /* Values in HSTATE_NAPPING(r13) */
> >  #define NAPPING_CEDE   1
> >  #define NAPPING_NOVCPU 2
> > @@ -3119,326 +3117,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
> > mr  r4,r31
> > blr
> >  
> > -#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> > -/*
> > - * Save transactional state and TM-related registers.
> > - * Called with r9 pointing to the vcpu struct.
> > - * This can modify all checkpointed registers, but
> > - * restores r1, r2 and r9 (vcpu pointer) before exit.
> > - */
> > -kvmppc_save_tm:
> > -   mflrr0
> > -   std r0, PPC_LR_STKOFF(r1)
> > -   stdur1, -PPC_MIN_STKFRM(r1)
> > -
> > -   /* Turn on TM. */
> > -   mfmsr   r8
> > -   li  r0, 1
> > -   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
> > -   beq 1f  /* TM not active in guest. */
> > -
> > -   std r1, HSTATE_HOST_R1(r13)
> > -   li  r3, TM_CAUSE_KVM_RESCHED
> > -
> > -BEGIN_FTR_SECTION
> > -   lbz r0, HSTATE_FAKE_SUSPEND(r13) /* Were we fake suspended? */
> > -   cmpwi   r0, 0
> > -   beq 3f
> > -   rldicl. r8, r8, 64 - MSR_TS_S_LG, 62 /* Did we actually hrfid? */
> > -   beq 4f
> > -BEGIN_FTR_SECTION_NESTED(96)
> > -   bl  pnv_power9_force_smt4_catch
> > -END_FTR_SECTION_NESTED(CPU_FTR_P9_TM_XER_SO_BUG, CPU_FTR_P9_TM_XER_SO_BUG, 
> > 96)
> > -   nop
> > -   b   6f
> > -3:
> > -   /* Emulation of the treclaim instruction needs TEXASR before treclaim */
> > -   mfspr   r6, SPRN_TEXASR
> > -   std r6, VCPU_ORIG_TEXASR(r9)
> > -6:
> > -END_FTR_SECTION_IFSET(CPU_FTR_P9_TM_HV_ASSIST)
> 
> It worries me that we now have this TM hypervisor assist stuff in a
> place where it could be active with PR KVM.  I think it would be
> better to factor out the TM assist code into a separate function which
> then calls kvmppc_save_tm if it needs to do an actual treclaim.  I'll
> look at doing that.
Paul,
Thanks for the info. Please let me know if anything I can help.

BR,
- Simon

Re: [PATCH v4 04/29] KVM: PPC: Book3S PR: Move kvmppc_save_tm/kvmppc_restore_tm to separate file

2018-05-29 Thread Paul Mackerras 
On Wed, May 23, 2018 at 03:01:47PM +0800, wei.guo.si...@gmail.com wrote:
> From: Simon Guo 
> 
> It is a simple patch just for moving kvmppc_save_tm/kvmppc_restore_tm()
> functionalities to tm.S. There is no logic change. The reconstruct of
> those APIs will be done in later patches to improve readability.
> 
> It is for preparation of reusing those APIs on both HV/PR PPC KVM.
> 
> Some slight change during move the functions includes:
> - surrounds some HV KVM specific code with CONFIG_KVM_BOOK3S_HV_POSSIBLE
> for compilation.
> - use _GLOBAL() to define kvmppc_save_tm/kvmppc_restore_tm()
> 
> Signed-off-by: Simon Guo 
> ---
>  arch/powerpc/kvm/Makefile   |   3 +
>  arch/powerpc/kvm/book3s_hv_rmhandlers.S | 322 
>  arch/powerpc/kvm/tm.S   | 363 
> 
>  3 files changed, 366 insertions(+), 322 deletions(-)
>  create mode 100644 arch/powerpc/kvm/tm.S
> 
> diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
> index 4b19da8..f872c04 100644
> --- a/arch/powerpc/kvm/Makefile
> +++ b/arch/powerpc/kvm/Makefile
> @@ -63,6 +63,9 @@ kvm-pr-y := \
>   book3s_64_mmu.o \
>   book3s_32_mmu.o
>  
> +kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \
> + tm.o
> +
>  ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
>  kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \
>   book3s_rmhandlers.o
> diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S 
> b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> index 5e6e493..4db2b10 100644
> --- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> +++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
> @@ -39,8 +39,6 @@ BEGIN_FTR_SECTION;  \
>   extsw   reg, reg;   \
>  END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
>  
> -#define VCPU_GPRS_TM(reg) (((reg) * ULONG_SIZE) + VCPU_GPR_TM)
> -
>  /* Values in HSTATE_NAPPING(r13) */
>  #define NAPPING_CEDE 1
>  #define NAPPING_NOVCPU   2
> @@ -3119,326 +3117,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
>   mr  r4,r31
>   blr
>  
> -#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> -/*
> - * Save transactional state and TM-related registers.
> - * Called with r9 pointing to the vcpu struct.
> - * This can modify all checkpointed registers, but
> - * restores r1, r2 and r9 (vcpu pointer) before exit.
> - */
> -kvmppc_save_tm:
> - mflrr0
> - std r0, PPC_LR_STKOFF(r1)
> - stdur1, -PPC_MIN_STKFRM(r1)
> -
> - /* Turn on TM. */
> - mfmsr   r8
> - li  r0, 1
> - 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
> - beq 1f  /* TM not active in guest. */
> -
> - std r1, HSTATE_HOST_R1(r13)
> - li  r3, TM_CAUSE_KVM_RESCHED
> -
> -BEGIN_FTR_SECTION
> - lbz r0, HSTATE_FAKE_SUSPEND(r13) /* Were we fake suspended? */
> - cmpwi   r0, 0
> - beq 3f
> - rldicl. r8, r8, 64 - MSR_TS_S_LG, 62 /* Did we actually hrfid? */
> - beq 4f
> -BEGIN_FTR_SECTION_NESTED(96)
> - bl  pnv_power9_force_smt4_catch
> -END_FTR_SECTION_NESTED(CPU_FTR_P9_TM_XER_SO_BUG, CPU_FTR_P9_TM_XER_SO_BUG, 
> 96)
> - nop
> - b   6f
> -3:
> - /* Emulation of the treclaim instruction needs TEXASR before treclaim */
> - mfspr   r6, SPRN_TEXASR
> - std r6, VCPU_ORIG_TEXASR(r9)
> -6:
> -END_FTR_SECTION_IFSET(CPU_FTR_P9_TM_HV_ASSIST)

It worries me that we now have this TM hypervisor assist stuff in a
place where it could be active with PR KVM.  I think it would be
better to factor out the TM assist code into a separate function which
then calls kvmppc_save_tm if it needs to do an actual treclaim.  I'll
look at doing that.

Paul.

[PATCH v4 04/29] KVM: PPC: Book3S PR: Move kvmppc_save_tm/kvmppc_restore_tm to separate file

2018-05-23 Thread wei . guo . simon 
From: Simon Guo 

It is a simple patch just for moving kvmppc_save_tm/kvmppc_restore_tm()
functionalities to tm.S. There is no logic change. The reconstruct of
those APIs will be done in later patches to improve readability.

It is for preparation of reusing those APIs on both HV/PR PPC KVM.

Some slight change during move the functions includes:
- surrounds some HV KVM specific code with CONFIG_KVM_BOOK3S_HV_POSSIBLE
for compilation.
- use _GLOBAL() to define kvmppc_save_tm/kvmppc_restore_tm()

Signed-off-by: Simon Guo 
---
 arch/powerpc/kvm/Makefile   |   3 +
 arch/powerpc/kvm/book3s_hv_rmhandlers.S | 322 
 arch/powerpc/kvm/tm.S   | 363 
 3 files changed, 366 insertions(+), 322 deletions(-)
 create mode 100644 arch/powerpc/kvm/tm.S

diff --git a/arch/powerpc/kvm/Makefile b/arch/powerpc/kvm/Makefile
index 4b19da8..f872c04 100644
--- a/arch/powerpc/kvm/Makefile
+++ b/arch/powerpc/kvm/Makefile
@@ -63,6 +63,9 @@ kvm-pr-y := \
book3s_64_mmu.o \
book3s_32_mmu.o
 
+kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \
+   tm.o
+
 ifdef CONFIG_KVM_BOOK3S_PR_POSSIBLE
 kvm-book3s_64-builtin-objs-$(CONFIG_KVM_BOOK3S_64_HANDLER) += \
book3s_rmhandlers.o
diff --git a/arch/powerpc/kvm/book3s_hv_rmhandlers.S 
b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
index 5e6e493..4db2b10 100644
--- a/arch/powerpc/kvm/book3s_hv_rmhandlers.S
+++ b/arch/powerpc/kvm/book3s_hv_rmhandlers.S
@@ -39,8 +39,6 @@ BEGIN_FTR_SECTION;\
extsw   reg, reg;   \
 END_FTR_SECTION_IFCLR(CPU_FTR_ARCH_300)
 
-#define VCPU_GPRS_TM(reg) (((reg) * ULONG_SIZE) + VCPU_GPR_TM)
-
 /* Values in HSTATE_NAPPING(r13) */
 #define NAPPING_CEDE   1
 #define NAPPING_NOVCPU 2
@@ -3119,326 +3117,6 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
mr  r4,r31
blr
 
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
-/*
- * Save transactional state and TM-related registers.
- * Called with r9 pointing to the vcpu struct.
- * This can modify all checkpointed registers, but
- * restores r1, r2 and r9 (vcpu pointer) before exit.
- */
-kvmppc_save_tm:
-   mflrr0
-   std r0, PPC_LR_STKOFF(r1)
-   stdur1, -PPC_MIN_STKFRM(r1)
-
-   /* Turn on TM. */
-   mfmsr   r8
-   li  r0, 1
-   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
-   beq 1f  /* TM not active in guest. */
-
-   std r1, HSTATE_HOST_R1(r13)
-   li  r3, TM_CAUSE_KVM_RESCHED
-
-BEGIN_FTR_SECTION
-   lbz r0, HSTATE_FAKE_SUSPEND(r13) /* Were we fake suspended? */
-   cmpwi   r0, 0
-   beq 3f
-   rldicl. r8, r8, 64 - MSR_TS_S_LG, 62 /* Did we actually hrfid? */
-   beq 4f
-BEGIN_FTR_SECTION_NESTED(96)
-   bl  pnv_power9_force_smt4_catch
-END_FTR_SECTION_NESTED(CPU_FTR_P9_TM_XER_SO_BUG, CPU_FTR_P9_TM_XER_SO_BUG, 96)
-   nop
-   b   6f
-3:
-   /* Emulation of the treclaim instruction needs TEXASR before treclaim */
-   mfspr   r6, SPRN_TEXASR
-   std r6, VCPU_ORIG_TEXASR(r9)
-6:
-END_FTR_SECTION_IFSET(CPU_FTR_P9_TM_HV_ASSIST)
-
-   /* Clear the MSR RI since r1, r13 are all going to be foobar. */
-   li  r5, 0
-   mtmsrd  r5, 1
-
-   /* All GPRs are volatile at this point. */
-   TRECLAIM(R3)
-
-   /* Temporarily store r13 and r9 so we have some regs to play with */
-   SET_SCRATCH0(r13)
-   GET_PACA(r13)
-   std r9, PACATMSCRATCH(r13)
-
-   /* If doing TM emulation on POWER9 DD2.2, check for fake suspend mode */
-BEGIN_FTR_SECTION
-   lbz r9, HSTATE_FAKE_SUSPEND(r13)
-   cmpwi   r9, 0
-   beq 2f
-   /*
-* We were in fake suspend, so we are not going to save the
-* register state as the guest checkpointed state (since
-* we already have it), therefore we can now use any volatile GPR.
-*/
-   /* Reload stack pointer and TOC. */
-   ld  r1, HSTATE_HOST_R1(r13)
-   ld  r2, PACATOC(r13)
-   /* Set MSR RI now we have r1 and r13 back. */
-   li  r5, MSR_RI
-   mtmsrd  r5, 1
-   HMT_MEDIUM
-   ld  r6, HSTATE_DSCR(r13)
-   mtspr   SPRN_DSCR, r6
-BEGIN_FTR_SECTION_NESTED(96)
-   bl  pnv_power9_force_smt4_release
-END_FTR_SECTION_NESTED(CPU_FTR_P9_TM_XER_SO_BUG, CPU_FTR_P9_TM_XER_SO_BUG, 96)
-   nop
-
-4:
-   mfspr   r3, SPRN_PSSCR
-   /* PSSCR_FAKE_SUSPEND is a write-only bit, but clear it anyway */
-   li  r0, PSSCR_FAKE_SUSPEND
-   andcr3, r3, r0
-   mtspr   SPRN_PSSCR, r3
-   ld  r9, HSTATE_KVM_VCPU(r13)
-   /* Don't save TEXASR, use value from last exit in real suspend state */
-   b   11f
-2:
-END_FTR_SECTION_IFSET(CPU_FTR_P9_TM_HV_ASSIST)
-
-   ld  r9,