[PATCH v2 30/52] powerpc/64s/exception: optimise system_reset for idle, clean up non-idle case

[Nicholas Piggin]

Nicholas Piggin's on June 20, 2019 3:14 pm:
> The idle wake up code in the system reset interrupt is not very
> optimal. There are two requirements: perform idle wake up quickly;
> and save everything including CFAR for non-idle interrupts, with
> no performance requirement.
> 
> The problem with placing the idle test in the middle of the handler
> and using the normal handler code to save CFAR, is that it's quite
> costly (e.g., mfcfar is serialising, speculative workarounds get
> applied, SRR1 has to be reloaded, etc). It also prevents the standard
> interrupt handler boilerplate being used.
> 
> This pain can be avoided by using a dedicated idle interrupt handler
> at the start of the interrupt handler, which restores all registers
> back to the way they were in case it was not an idle wake up. CFAR
> is preserved without saving it before the non-idle case by making that
> the fall-through, and idle is a taken branch.
> 
> Performance seems to be in the noise, but possibly around 0.5% faster,
> the executed instructions certainly look better. The bigger benefit is
> being able to drop in standard interrupt handlers after the idle code,
> which helps with subsequent cleanup and consolidation.
> 
> Signed-off-by: Nicholas Piggin <npiggin at gmail.com>
> ---
>  arch/powerpc/kernel/exceptions-64s.S | 89 ++++++++++++++--------------
>  1 file changed, 44 insertions(+), 45 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
> index e0492912ea79..f582ae30f3f7 100644
> --- a/arch/powerpc/kernel/exceptions-64s.S
> +++ b/arch/powerpc/kernel/exceptions-64s.S
> @@ -241,7 +241,7 @@ END_FTR_SECTION_NESTED(ftr,ftr,943)
>   * load KBASE for a slight optimisation.
>   */
>  #define BRANCH_TO_C000(reg, label)					\
> -	__LOAD_HANDLER(reg, label);					\
> +	__LOAD_FAR_HANDLER(reg, label);					\
>  	mtctr	reg;							\
>  	bctr
>  
> @@ -784,16 +784,6 @@ EXC_VIRT_NONE(0x4000, 0x100)
>  
>  
>  EXC_REAL_BEGIN(system_reset, 0x100, 0x100)
> -	SET_SCRATCH0(r13)
> -	EXCEPTION_PROLOG_0 PACA_EXNMI
> -
> -	/* This is EXCEPTION_PROLOG_1 with the idle feature section added */
> -	OPT_SAVE_REG_TO_PACA(PACA_EXNMI+EX_PPR, r9, CPU_FTR_HAS_PPR)
> -	OPT_SAVE_REG_TO_PACA(PACA_EXNMI+EX_CFAR, r10, CPU_FTR_CFAR)
> -	INTERRUPT_TO_KERNEL
> -	SAVE_CTR(r10, PACA_EXNMI)
> -	mfcr	r9
> -
>  #ifdef CONFIG_PPC_P7_NAP
>  	/*
>  	 * If running native on arch 2.06 or later, check if we are waking up
> @@ -801,45 +791,67 @@ EXC_REAL_BEGIN(system_reset, 0x100, 0x100)
>  	 * bits 46:47. A non-0 value indicates that we are coming from a power
>  	 * saving state. The idle wakeup handler initially runs in real mode,
>  	 * but we branch to the 0xc000... address so we can turn on relocation
> -	 * with mtmsr.
> +	 * with mtmsrd later, after SPRs are restored.
> +	 *
> +	 * Careful to minimise cost for the fast path (idle wakeup) while
> +	 * also avoiding clobbering CFAR for the non-idle case. Once we know
> +	 * it is an idle wake, volatiles don't matter, which is why we use
> +	 * those here, and then re-do the entry in case of non-idle (without
> +	 * branching for the non-idle case, to keep CFAR).
>  	 */
>  BEGIN_FTR_SECTION
> -	mfspr	r10,SPRN_SRR1
> -	rlwinm.	r10,r10,47-31,30,31
> -	beq-	1f
> -	cmpwi	cr1,r10,2
> +	SET_SCRATCH0(r13)
> +	GET_PACA(r13)
> +	std	r3,PACA_EXNMI+0*8(r13)
> +	std	r4,PACA_EXNMI+1*8(r13)
> +	std	r5,PACA_EXNMI+2*8(r13)
>  	mfspr	r3,SPRN_SRR1
> -	bltlr	cr1	/* no state loss, return to idle caller */
> -	BRANCH_TO_C000(r10, system_reset_idle_common)
> -1:
> +	mfocrf	r4,0x80
> +	rlwinm.	r5,r3,47-31,30,31
> +	bne+	system_reset_idle_wake
> +	/* Not powersave wakeup. Restore regs for regular interrupt handler. */
> +	mtocrf	0x80,r4
> +	ld	r12,PACA_EXNMI+0*8(r13)
> +	ld	r4,PACA_EXNMI+1*8(r13)
> +	ld	r5,PACA_EXNMI+2*8(r13)
> +	GET_SCRATCH0(r13)

For the love of... that should be 'ld r3', not 'ld r12', sorry.

Thanks,
Nick