[RFC,02/14] powerpc/tm: Reclaim on unavailable exception

If there is a FP/VEC/Altivec touch inside a transaction and the facility is
disabled, then a facility unavailable exception is raised and ends up
calling {fp,vec,vsx}_unavailable_tm, which was reclaiming and
recheckpointing.

This is not required anymore, since the checkpointed state was reclaimed in
the exception entrance, and it will be recheckpointed by restore_tm_state
later.

Adding a WARN_ON() warning if we hit the _unavailable_tm() in suspended
mode, i.e, the reclaim was not executed somehow in the trap entrance, and
this is a bug.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 arch/powerpc/include/asm/exception-64s.h |  4 ++++
 arch/powerpc/kernel/exceptions-64s.S     |  3 +++
 arch/powerpc/kernel/traps.c              | 22 ++++------------------
 3 files changed, 11 insertions(+), 18 deletions(-)

On Tue, 2018-11-06 at 10:40 -0200, Breno Leitao wrote:
> If there is a FP/VEC/Altivec touch inside a transaction and the facility is
> disabled, then a facility unavailable exception is raised and ends up
> calling {fp,vec,vsx}_unavailable_tm, which was reclaiming and
> recheckpointing.
> 
> This is not required anymore, since the checkpointed state was reclaimed in
> the exception entrance, and it will be recheckpointed by restore_tm_state
> later.
> 
> Adding a WARN_ON() warning if we hit the _unavailable_tm() in suspended
> mode, i.e, the reclaim was not executed somehow in the trap entrance, and
> this is a bug.

The "why" above is good and the important part of the commit but, 

Can you also add what you're doing?  The above would suggest you're just
removing some things but you're actually adding the TM_KERNEL_ENTRY() macro too.

Mikey

> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>  arch/powerpc/include/asm/exception-64s.h |  4 ++++
>  arch/powerpc/kernel/exceptions-64s.S     |  3 +++
>  arch/powerpc/kernel/traps.c              | 22 ++++------------------
>  3 files changed, 11 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/exception-64s.h
> b/arch/powerpc/include/asm/exception-64s.h
> index 931a74ba037b..80f01d5683c3 100644
> --- a/arch/powerpc/include/asm/exception-64s.h
> +++ b/arch/powerpc/include/asm/exception-64s.h
> @@ -711,6 +711,10 @@ END_FTR_SECTION_IFSET(CPU_FTR_CTRL)
>  	 * Soft disable the IRQs, otherwise it might cause a CPU hang.	\
>  	 */								\
>  	RECONCILE_IRQ_STATE(r10, r11);					\
> +	/*								\
> +	 * Although this cause will be set initially, it might be	\
> +	 * updated later, once the exception is better understood	\
> +	 */								\
>  	li      r3, cause;						\
>  	bl      tm_reclaim_current;					\
>  	li      r3, 1;		/* Reclaim happened */			\
> diff --git a/arch/powerpc/kernel/exceptions-64s.S
> b/arch/powerpc/kernel/exceptions-64s.S
> index 5c685a46202d..47e05b09eed6 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -786,6 +786,7 @@ END_FTR_SECTION_IFSET(CPU_FTR_TM)
>  #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>  2:	/* User process was in a transaction */
>  	bl	save_nvgprs
> +	TM_KERNEL_ENTRY(TM_CAUSE_FAC_UNAV)
>  	RECONCILE_IRQ_STATE(r10, r11)
>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>  	bl	fp_unavailable_tm
> @@ -1128,6 +1129,7 @@ BEGIN_FTR_SECTION
>  #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>  2:	/* User process was in a transaction */
>  	bl	save_nvgprs
> +	TM_KERNEL_ENTRY(TM_CAUSE_FAC_UNAV)
>  	RECONCILE_IRQ_STATE(r10, r11)
>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>  	bl	altivec_unavailable_tm
> @@ -1164,6 +1166,7 @@ BEGIN_FTR_SECTION
>  #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
>  2:	/* User process was in a transaction */
>  	bl	save_nvgprs
> +	TM_KERNEL_ENTRY(TM_CAUSE_FAC_UNAV)
>  	RECONCILE_IRQ_STATE(r10, r11)
>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>  	bl	vsx_unavailable_tm
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 9a86572db1ef..e74b735e974c 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1742,23 +1742,10 @@ void fp_unavailable_tm(struct pt_regs *regs)
>           * transaction, and probably retry but now with FP enabled.  So the
>           * checkpointed FP registers need to be loaded.
>  	 */
> -	tm_reclaim_current(TM_CAUSE_FAC_UNAV);
> -
> -	/*
> -	 * Reclaim initially saved out bogus (lazy) FPRs to ckfp_state, and
> -	 * then it was overwrite by the thr->fp_state by tm_reclaim_thread().
> -	 *
> -	 * At this point, ck{fp,vr}_state contains the exact values we want to
> -	 * recheckpoint.
> -	 */
> +	WARN_ON(MSR_TM_SUSPENDED(mfmsr()));
>  
>  	/* Enable FP for the task: */
>  	current->thread.load_fp = 1;
> -
> -	/*
> -	 * Recheckpoint all the checkpointed ckpt, ck{fp, vr}_state registers.
> -	 */
> -	tm_recheckpoint(&current->thread);
>  }
>  
>  void altivec_unavailable_tm(struct pt_regs *regs)
> @@ -1770,10 +1757,10 @@ void altivec_unavailable_tm(struct pt_regs *regs)
>  	TM_DEBUG("Vector Unavailable trap whilst transactional at 0x%lx,"
>  		 "MSR=%lx\n",
>  		 regs->nip, regs->msr);
> -	tm_reclaim_current(TM_CAUSE_FAC_UNAV);
> +	WARN_ON(MSR_TM_SUSPENDED(mfmsr()));
>  	current->thread.load_vec = 1;
> -	tm_recheckpoint(&current->thread);
>  	current->thread.used_vr = 1;
> +
>  }
>  
>  void vsx_unavailable_tm(struct pt_regs *regs)
> @@ -1792,12 +1779,11 @@ void vsx_unavailable_tm(struct pt_regs *regs)
>  	current->thread.used_vsr = 1;
>  
>  	/* This reclaims FP and/or VR regs if they're already enabled */
> -	tm_reclaim_current(TM_CAUSE_FAC_UNAV);
> +	WARN_ON(MSR_TM_SUSPENDED(mfmsr()));
>  
>  	current->thread.load_vec = 1;
>  	current->thread.load_fp = 1;
>  
> -	tm_recheckpoint(&current->thread);
>  }
>  #endif /* CONFIG_PPC_TRANSACTIONAL_MEM */
>