[PATCH] powerpc64/idle: Fix SP offsets when saving GPRs

[Michael Ellerman]

"Christopher M. Riedl" <cmr at codefail.de> writes:
> The idle entry/exit code saves/restores GPRs in the stack "red zone"
> (Protected Zone according to PowerPC64 ELF ABI v2). However, the offset
> used for the first GPR is incorrect and overwrites the back chain - the
> Protected Zone actually starts below the current SP. In practice this is
> probably not an issue, but it's still incorrect so fix it.

Nice catch.

Corrupting the back chain means you can't backtrace from there, which
could be confusing for debugging one day.

It does make me wonder why we don't just create a stack frame and use
the normal macros? It would use a bit more stack space, but we shouldn't
be short of stack space when going idle.

Nick, was there a particular reason for using the red zone?

cheers


> diff --git a/arch/powerpc/kernel/idle_book3s.S b/arch/powerpc/kernel/idle_book3s.S
> index 22f249b6f58d..80cf35183e9d 100644
> --- a/arch/powerpc/kernel/idle_book3s.S
> +++ b/arch/powerpc/kernel/idle_book3s.S
> @@ -53,27 +53,27 @@ _GLOBAL(isa300_idle_stop_mayloss)
>  	mflr	r4
>  	mfcr	r5
>  	/* use stack red zone rather than a new frame for saving regs */
> -	std	r2,-8*0(r1)
> -	std	r14,-8*1(r1)
> -	std	r15,-8*2(r1)
> -	std	r16,-8*3(r1)
> -	std	r17,-8*4(r1)
> -	std	r18,-8*5(r1)
> -	std	r19,-8*6(r1)
> -	std	r20,-8*7(r1)
> -	std	r21,-8*8(r1)
> -	std	r22,-8*9(r1)
> -	std	r23,-8*10(r1)
> -	std	r24,-8*11(r1)
> -	std	r25,-8*12(r1)
> -	std	r26,-8*13(r1)
> -	std	r27,-8*14(r1)
> -	std	r28,-8*15(r1)
> -	std	r29,-8*16(r1)
> -	std	r30,-8*17(r1)
> -	std	r31,-8*18(r1)
> -	std	r4,-8*19(r1)
> -	std	r5,-8*20(r1)
> +	std	r2,-8*1(r1)
> +	std	r14,-8*2(r1)
> +	std	r15,-8*3(r1)
> +	std	r16,-8*4(r1)
> +	std	r17,-8*5(r1)
> +	std	r18,-8*6(r1)
> +	std	r19,-8*7(r1)
> +	std	r20,-8*8(r1)
> +	std	r21,-8*9(r1)
> +	std	r22,-8*10(r1)
> +	std	r23,-8*11(r1)
> +	std	r24,-8*12(r1)
> +	std	r25,-8*13(r1)
> +	std	r26,-8*14(r1)
> +	std	r27,-8*15(r1)
> +	std	r28,-8*16(r1)
> +	std	r29,-8*17(r1)
> +	std	r30,-8*18(r1)
> +	std	r31,-8*19(r1)
> +	std	r4,-8*20(r1)
> +	std	r5,-8*21(r1)
>  	/* 168 bytes */
>  	PPC_STOP
>  	b	.	/* catch bugs */
> @@ -89,8 +89,8 @@ _GLOBAL(isa300_idle_stop_mayloss)
>   */
>  _GLOBAL(idle_return_gpr_loss)
>  	ld	r1,PACAR1(r13)
> -	ld	r4,-8*19(r1)
> -	ld	r5,-8*20(r1)
> +	ld	r4,-8*20(r1)
> +	ld	r5,-8*21(r1)
>  	mtlr	r4
>  	mtcr	r5
>  	/*
> @@ -98,25 +98,25 @@ _GLOBAL(idle_return_gpr_loss)
>  	 * from PACATOC. This could be avoided for that less common case
>  	 * if KVM saved its r2.
>  	 */
> -	ld	r2,-8*0(r1)
> -	ld	r14,-8*1(r1)
> -	ld	r15,-8*2(r1)
> -	ld	r16,-8*3(r1)
> -	ld	r17,-8*4(r1)
> -	ld	r18,-8*5(r1)
> -	ld	r19,-8*6(r1)
> -	ld	r20,-8*7(r1)
> -	ld	r21,-8*8(r1)
> -	ld	r22,-8*9(r1)
> -	ld	r23,-8*10(r1)
> -	ld	r24,-8*11(r1)
> -	ld	r25,-8*12(r1)
> -	ld	r26,-8*13(r1)
> -	ld	r27,-8*14(r1)
> -	ld	r28,-8*15(r1)
> -	ld	r29,-8*16(r1)
> -	ld	r30,-8*17(r1)
> -	ld	r31,-8*18(r1)
> +	ld	r2,-8*1(r1)
> +	ld	r14,-8*2(r1)
> +	ld	r15,-8*3(r1)
> +	ld	r16,-8*4(r1)
> +	ld	r17,-8*5(r1)
> +	ld	r18,-8*6(r1)
> +	ld	r19,-8*7(r1)
> +	ld	r20,-8*8(r1)
> +	ld	r21,-8*9(r1)
> +	ld	r22,-8*10(r1)
> +	ld	r23,-8*11(r1)
> +	ld	r24,-8*12(r1)
> +	ld	r25,-8*13(r1)
> +	ld	r26,-8*14(r1)
> +	ld	r27,-8*15(r1)
> +	ld	r28,-8*16(r1)
> +	ld	r29,-8*17(r1)
> +	ld	r30,-8*18(r1)
> +	ld	r31,-8*19(r1)
>  	blr
>  
>  /*
> @@ -155,27 +155,27 @@ _GLOBAL(isa206_idle_insn_mayloss)
>  	mflr	r4
>  	mfcr	r5
>  	/* use stack red zone rather than a new frame for saving regs */
> -	std	r2,-8*0(r1)
> -	std	r14,-8*1(r1)
> -	std	r15,-8*2(r1)
> -	std	r16,-8*3(r1)
> -	std	r17,-8*4(r1)
> -	std	r18,-8*5(r1)
> -	std	r19,-8*6(r1)
> -	std	r20,-8*7(r1)
> -	std	r21,-8*8(r1)
> -	std	r22,-8*9(r1)
> -	std	r23,-8*10(r1)
> -	std	r24,-8*11(r1)
> -	std	r25,-8*12(r1)
> -	std	r26,-8*13(r1)
> -	std	r27,-8*14(r1)
> -	std	r28,-8*15(r1)
> -	std	r29,-8*16(r1)
> -	std	r30,-8*17(r1)
> -	std	r31,-8*18(r1)
> -	std	r4,-8*19(r1)
> -	std	r5,-8*20(r1)
> +	std	r2,-8*1(r1)
> +	std	r14,-8*2(r1)
> +	std	r15,-8*3(r1)
> +	std	r16,-8*4(r1)
> +	std	r17,-8*5(r1)
> +	std	r18,-8*6(r1)
> +	std	r19,-8*7(r1)
> +	std	r20,-8*8(r1)
> +	std	r21,-8*9(r1)
> +	std	r22,-8*10(r1)
> +	std	r23,-8*11(r1)
> +	std	r24,-8*12(r1)
> +	std	r25,-8*13(r1)
> +	std	r26,-8*14(r1)
> +	std	r27,-8*15(r1)
> +	std	r28,-8*16(r1)
> +	std	r29,-8*17(r1)
> +	std	r30,-8*18(r1)
> +	std	r31,-8*19(r1)
> +	std	r4,-8*20(r1)
> +	std	r5,-8*21(r1)
>  	cmpwi	r3,PNV_THREAD_NAP
>  	bne	1f
>  	IDLE_STATE_ENTER_SEQ_NORET(PPC_NAP)
> -- 
> 2.26.1