Re: [PATCH] powerpc/64s: fix idle wakeup potential to clobber registers

[Gautham R Shenoy <ego@linux.vnet.ibm.com>]

Hi ,


On Fri, Mar 17, 2017 at 03:13:20PM +1000, Nicholas Piggin wrote:
> We concluded there may be a window where the idle wakeup code could
> get to pnv_wakeup_tb_loss (which clobbers non-volatile GPRs), but the
> hardware may set SRR1[46:47] to 01b (no state loss) which would
> result in the wakeup code failing to restore non-volatile GPRs.
> 
> I was not able to trigger this condition with trivial tests on
> real hardware or simulator, but the ISA (at least 2.07) seems to
> allow for it, and Gautham says that it can happen if there is an
> exception pending when the sleep/winkle instruction is executed.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Acked-by: Gautham R. Shenoy <ego@linux.vnet.ibm.com>

This fix should go into stable v4.8,v4.9 and v4.10.

Prior to commit 83289f909a72 ("powerpc/powernv: Rename idle_power7.S
to idle_book3s.S"), pnv_wakeup_tb_loss was explicitly restoring all
the GPRs to the saved value. So we are good for all the previous
kernel versions.

> ---
>  arch/powerpc/kernel/idle_book3s.S | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 995728736677..6fd08219248d 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -449,9 +449,23 @@ END_FTR_SECTION_IFSET(CPU_FTR_ARCH_300)
>  _GLOBAL(pnv_wakeup_tb_loss)
>  	ld	r1,PACAR1(r13)
>  	/*
> -	 * Before entering any idle state, the NVGPRs are saved in the stack
> -	 * and they are restored before switching to the process context. Hence
> -	 * until they are restored, they are free to be used.
> +	 * Before entering any idle state, the NVGPRs are saved in the stack.
> +	 * If there was a state loss, or PACA_NAPSTATELOST was set, then the
> +	 * NVGPRs are restored. If we are here, it is likely that state is lost,
> +	 * but not guaranteed -- neither ISA207 nor ISA300 tests to reach
> +	 * here are the same as the test to restore NVGPRS:
> +	 * PACA_THREAD_IDLE_STATE test for ISA207, PSSCR test for ISA300,
> +	 * and SRR1 test for restoring NVGPRs.
> +	 *
> +	 * We are about to clobber NVGPRs now, so set NAPSTATELOST to
> +	 * guarantee they will always be restored. This might be tightened
> +	 * with careful reading of specs (particularly for ISA300) but this
> +	 * is already a slow wakeup path and it's simpler to be safe.
> +	 */
> +	li	r0,1
> +	stb	r0,PACA_NAPSTATELOST(r13)
> +
> +	/*
>  	 *
>  	 * Save SRR1 and LR in NVGPRs as they might be clobbered in
>  	 * opal_call() (called in CHECK_HMI_INTERRUPT). SRR1 is required
> -- 
> 2.11.0
> 
--
Thanks and Regards
gautham.