Subject: Re: [PATCH v4] powerpc/64s: reimplement book3s idle code in C

From: Nicholas Piggin <[email-masked]>
Date: Fri, 28 Sep 2018 10:16:14 +1000

On Wed, 26 Sep 2018 19:39:14 +0530
Akshay Adiga <[email-masked]> wrote:

> On Fri, Sep 14, 2018 at 11:52:40AM +1000, Nicholas Piggin wrote:
> > +
> > +	/*
> > +	 * On POWER9, SRR1 bits do not match exactly as expected.
> > +	 * SRR1_WS_GPRLOSS (10b) can also result in SPR loss, so
> > +	 * always test PSSCR if there is any state loss.
> > +	 */
> > +	if (likely((psscr & PSSCR_RL_MASK) < pnv_first_hv_loss_level)) {  
> 
> Shouldn't we check PLS field to see if the cpu/core woke up from hv loss ?
> 
> Currently, a cpu requested stop4 (RL=4) and exited from a shallower state
> (PLS=2), SPR's are unecessarily restored.
> 
> We can do something like : 
> 
> #define	PSSCR_PLS_SHIFT		60
> if (likely((psscr & PSSCR_PLS) >> PSSCR_PLS_SHIFT) < pnv_first_hv_loss_level)

Ah, that corresponds with the following existing code?

        /*
         * POWER ISA 3. Use PSSCR to determine if we
         * are waking up from deep idle state
         */
        LOAD_REG_ADDRBASE(r5,pnv_first_deep_stop_state)
        ld      r4,ADDROFF(pnv_first_deep_stop_state)(r5)

        /*
         * 0-3 bits correspond to Power-Saving Level Status
         * which indicates the idle state we are waking up from
         */
        mfspr   r5, SPRN_PSSCR
        rldicl  r5,r5,4,60
        li      r0, 0           /* clear requested_psscr to say we're awake */
        std     r0, PACA_REQ_PSSCR(r13)
        cmpd    cr4,r5,r4
        bge     cr4,pnv_wakeup_tb_loss /* returns to caller */

Yes I didn't get that right, good catch.

Thanks,
Nick