[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1ab4f306-2338-91c2-0329-caa750594977@csgroup.eu>
Date: Fri, 4 Nov 2022 18:00:47 +0000
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Andrew Donnellan <ajd@...ux.ibm.com>,
"linuxppc-dev@...ts.ozlabs.org" <linuxppc-dev@...ts.ozlabs.org>
CC: "ruscur@...sell.cc" <ruscur@...sell.cc>,
"cmr@...escreens.de" <cmr@...escreens.de>,
"linux-hardening@...r.kernel.org" <linux-hardening@...r.kernel.org>
Subject: Re: [RFC PATCH 3/6] powerpc/powernv: Keep MSR in register across OPAL
entry/return path
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@...ux.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
Powered by blists - more mailing lists