Subject: Re: [v3 11/15] powerpc/perf: BHRB control to disable BHRB logic when not used

From: Gautham R Shenoy <[email-masked]>
Date: Mon, 20 Jul 2020 15:35:57 +0530

On Fri, Jul 17, 2020 at 10:38:23AM -0400, Athira Rajeev wrote:
> PowerISA v3.1 has few updates for the Branch History Rolling Buffer(BHRB).
> 
> BHRB disable is controlled via Monitor Mode Control Register A (MMCRA)
> bit, namely "BHRB Recording Disable (BHRBRD)". This field controls
> whether BHRB entries are written when BHRB recording is enabled by other
> bits. This patch implements support for this BHRB disable bit.
> 
> By setting 0b1 to this bit will disable the BHRB and by setting 0b0
> to this bit will have BHRB enabled. This addresses backward
> compatibility (for older OS), since this bit will be cleared and
> hardware will be writing to BHRB by default.
> 
> This patch addresses changes to set MMCRA (BHRBRD) at boot for power10
> ( there by the core will run faster) and enable this feature only on
> runtime ie, on explicit need from user. Also save/restore MMCRA in the
> restore path of state-loss idle state to make sure we keep BHRB disabled
> if it was not enabled on request at runtime.
> 
> Signed-off-by: Athira Rajeev <[email-masked]>

For arch/powerpc/platforms/powernv/idle.c
Reviewed-by: Gautham R. Shenoy <[email-masked]>

> ---
>  arch/powerpc/perf/core-book3s.c       | 20 ++++++++++++++++----
>  arch/powerpc/perf/isa207-common.c     | 12 ++++++++++++
>  arch/powerpc/platforms/powernv/idle.c | 22 ++++++++++++++++++++--
>  3 files changed, 48 insertions(+), 6 deletions(-)

[..snip..]

> diff --git a/arch/powerpc/platforms/powernv/idle.c b/arch/powerpc/platforms/powernv/idle.c
> index 2dd4673..1c9d0a9 100644
> --- a/arch/powerpc/platforms/powernv/idle.c
> +++ b/arch/powerpc/platforms/powernv/idle.c
> @@ -611,6 +611,7 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
>  	unsigned long srr1;
>  	unsigned long pls;
>  	unsigned long mmcr0 = 0;
> +	unsigned long mmcra = 0;
>  	struct p9_sprs sprs = {}; /* avoid false used-uninitialised */
>  	bool sprs_saved = false;
> 
> @@ -657,6 +658,21 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
>  		  */
>  		mmcr0		= mfspr(SPRN_MMCR0);
>  	}
> +
> +	if (cpu_has_feature(CPU_FTR_ARCH_31)) {
> +		/*
> +		 * POWER10 uses MMCRA (BHRBRD) as BHRB disable bit.
> +		 * If the user hasn't asked for the BHRB to be
> +		 * written, the value of MMCRA[BHRBRD] is 1.
> +		 * On wakeup from stop, MMCRA[BHRBD] will be 0,
> +		 * since it is previleged resource and will be lost.
> +		 * Thus, if we do not save and restore the MMCRA[BHRBD],
> +		 * hardware will be needlessly writing to the BHRB
> +		 * in problem mode.
> +		 */
> +		mmcra		= mfspr(SPRN_MMCRA);
> +	}
> +
>  	if ((psscr & PSSCR_RL_MASK) >= pnv_first_spr_loss_level) {
>  		sprs.lpcr	= mfspr(SPRN_LPCR);
>  		sprs.hfscr	= mfspr(SPRN_HFSCR);
> @@ -700,8 +716,6 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
>  	WARN_ON_ONCE(mfmsr() & (MSR_IR|MSR_DR));
> 
>  	if ((srr1 & SRR1_WAKESTATE) != SRR1_WS_NOLOSS) {
> -		unsigned long mmcra;
> -
>  		/*
>  		 * We don't need an isync after the mtsprs here because the
>  		 * upcoming mtmsrd is execution synchronizing.
> @@ -721,6 +735,10 @@ static unsigned long power9_idle_stop(unsigned long psscr, bool mmu_on)
>  			mtspr(SPRN_MMCR0, mmcr0);
>  		}
> 
> +		/* Reload MMCRA to restore BHRB disable bit for POWER10 */
> +		if (cpu_has_feature(CPU_FTR_ARCH_31))
> +			mtspr(SPRN_MMCRA, mmcra);
> +
>  		/*
>  		 * DD2.2 and earlier need to set then clear bit 60 in MMCRA
>  		 * to ensure the PMU starts running.
> -- 
> 1.8.3.1
>