Re: [RFC PATCH 3/6] powerpc/powernv: Keep MSR in register across OPAL entry/return path

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 04/11/2022 à 18:27, Andrew Donnellan a écrit :
> When we enter and return from an OPAL call, there's three pieces of state
> we have to save and restore: the stack pointer, the PACA pointer, and the
> MSR. However, there's only two registers that OPAL is guaranteed to
> preserve for us (r1 for the stack pointer and r13 for the PACA), so the MSR
> gets saved on the stack.
> 
> This becomes problematic when we enable VMAP_STACK, as we need to re-enable
> translation in order to access the virtually mapped stack... and to
> re-enable translation, we need to restore the MSR.

Do you need to restore MSR really ? Can't you just set MSR_DR to access 
the stack then restore MSR ? Or maybe you don't want to do it in two 
steps for performance reason ?

> 
> Keep the MSR in r13, and instead store the PACA pointer on the stack - we
> can restore the MSR first, then restore the PACA into r13.
> 
> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
> ---
>   .../powerpc/platforms/powernv/opal-wrappers.S | 43 +++++++++++--------
>   1 file changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/powernv/opal-wrappers.S b/arch/powerpc/platforms/powernv/opal-wrappers.S
> index 0ed95f753416..d692869ee0ce 100644
> --- a/arch/powerpc/platforms/powernv/opal-wrappers.S
> +++ b/arch/powerpc/platforms/powernv/opal-wrappers.S
> @@ -23,40 +23,49 @@
>   _GLOBAL_TOC(__opal_call)
>   	mflr	r0
>   	std	r0,PPC_LR_STKOFF(r1)
> -	ld	r12,STK_PARAM(R12)(r1)
> -	li	r0,MSR_IR|MSR_DR|MSR_LE
> -	andc	r12,r12,r0
>   	LOAD_REG_ADDR(r11, opal_return)
>   	mtlr	r11
>   	LOAD_REG_ADDR(r11, opal)
>   	ld	r2,0(r11)
>   	ld	r11,8(r11)
>   	mtspr	SPRN_HSRR0,r11
> -	mtspr	SPRN_HSRR1,r12
> +
>   	/* set token to r0 */
>   	ld	r0,STK_PARAM(R11)(r1)
> +
> +	/*
> +	 * We need to keep the MSR value in a register that is preserved by
> +	 * OPAL, so that we don't need to access the stack before we restore
> +	 * the MSR, as the stack may be vmalloced and thus require MMU.
> +	 *
> +	 * Move the PACA from R13 into the stack red zone, and put MSR in R13.
> +	 */
> +	std	r13,-8(r1)
> +	ld	r13,STK_PARAM(R12)(r1)
> +
> +	/* Switch off MMU, LE */
> +	li	r11,MSR_IR|MSR_DR|MSR_LE
> +	andc	r11,r13,r11
> +
> +	mtspr	SPRN_HSRR1,r11
>   	hrfid
>   opal_return:
>   	/*
>   	 * Restore MSR on OPAL return. The MSR is set to big-endian.
>   	 */
>   #ifdef __BIG_ENDIAN__
> -	ld	r11,STK_PARAM(R12)(r1)
> -	mtmsrd	r11
> +	mtmsrd	r13
>   #else
>   	/* Endian can only be switched with rfi, must byte reverse MSR load */
> -	.short 0x4039	 /* li r10,STK_PARAM(R12)		*/
> -	.byte (STK_PARAM(R12) >> 8) & 0xff
> -	.byte STK_PARAM(R12) & 0xff
> -
> -	.long 0x280c6a7d /* ldbrx r11,r10,r1			*/
> -	.long 0x05009f42 /* bcl 20,31,$+4			*/
> -	.long 0xa602487d /* mflr r10				*/
> -	.long 0x14004a39 /* addi r10,r10,20			*/
> -	.long 0xa64b5a7d /* mthsrr0 r10				*/
> -	.long 0xa64b7b7d /* mthsrr1 r11				*/
> -	.long 0x2402004c /* hrfid				*/
> +	.long 0x05009f42 /* bcl 20,31,$+4   (LR <- next insn addr)	*/
> +	.long 0xa602487d /* mflr r10					*/
> +	.long 0x14004a39 /* addi r10,r10,20 (r10 <- addr after #endif)	*/
> +	.long 0xa64b5a7d /* mthsrr0 r10	    (new NIP)			*/
> +	.long 0xa64bbb7d /* mthsrr1 r13	    (new MSR)			*/
> +	.long 0x2402004c /* hrfid					*/
>   #endif
> +	/* Restore PACA */
> +	ld	r13,-8(r1)
>   	LOAD_PACA_TOC()
>   	ld	r0,PPC_LR_STKOFF(r1)
>   	mtlr	r0