On 08/03/2022, 14:50:41, Nicholas Piggin wrote:
> This moves MSR save/restore and some real-mode juggling out of asm and
> into C code, simplifying things.
>
> Signed-off-by: Nicholas Piggin <npig...@gmail.com>
> ---
> arch/powerpc/kernel/rtas.c | 15 ++++++++++++---
> arch/powerpc/kernel/rtas_entry.S | 32 +++++---------------------------
> 2 files changed, 17 insertions(+), 30 deletions(-)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 6b5892d6a56b..87ede1877816 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -47,13 +47,22 @@
> /* This is here deliberately so it's only used in this file */
> void enter_rtas(unsigned long);
>
> -static inline void do_enter_rtas(unsigned long args)
> +static noinline void do_enter_rtas(unsigned long args)
> {
> BUG_ON(!irqs_disabled());
>
> - hard_irq_disable(); /* Ensure MSR[EE] is disabled on PPC64 */
> + if (IS_ENABLED(CONFIG_PPC64)) {
> + unsigned long msr;
>
> - enter_rtas(args);
> + hard_irq_disable();
> +
> + msr = mfmsr();
> + mtmsr(msr & ~(MSR_IR|MSR_DR));
> + enter_rtas(args);
> + mtmsr(msr);
> + } else {
> + enter_rtas(args);
> + }
>
> srr_regs_clobbered(); /* rtas uses SRRs, invalidate */
> }
> diff --git a/arch/powerpc/kernel/rtas_entry.S
> b/arch/powerpc/kernel/rtas_entry.S
> index 5f65ea4436c6..292551684bbd 100644
> --- a/arch/powerpc/kernel/rtas_entry.S
> +++ b/arch/powerpc/kernel/rtas_entry.S
> @@ -84,14 +84,11 @@ _GLOBAL(enter_rtas)
> li r0,0
> mtcr r0
>
> - mfmsr r6
> -
> - /* Unfortunately, the stack pointer and the MSR are also clobbered,
> - * so they are saved in the PACA which allows us to restore
> - * our original state after RTAS returns.
> + /*
> + * The stack pointer is clobbered, so it is saved in the PACA which
> + * allows us to restore our original state after RTAS returns.
> */
> std r1,PACAR1(r13)
> - std r6,PACASAVEDMSR(r13)
>
> /* Setup our real return addr */
> LOAD_REG_ADDR(r4,rtas_return_loc)
> @@ -100,7 +97,6 @@ _GLOBAL(enter_rtas)
>
> LOAD_REG_IMMEDIATE(r6, MSR_ME)
>
> -__enter_rtas:
> LOAD_REG_ADDR(r4, rtas)
> ld r5,RTASENTRY(r4) /* get the rtas->entry value */
> ld r4,RTASBASE(r4) /* get the rtas->base value */
> @@ -112,6 +108,7 @@ __enter_rtas:
> mtspr SPRN_SRR1,r6
> RFI_TO_KERNEL
> b . /* prevent speculative execution */
> +_ASM_NOKPROBE_SYMBOL(enter_rtas)
>
> rtas_return_loc:
> FIXUP_ENDIAN
> @@ -127,29 +124,10 @@ rtas_return_loc:
> sync
> mtmsrd r6
Since MSR plumbing is still needed in the asm, what is the benefit of doing
the real mode switching in the C code?
What if the MSR is saved in the PACA before switching to real mode, and
restored in rtas_return_loc?
>
> - /* relocation is off at this point */
> GET_PACA(r13)
>
> - bcl 20,31,$+4
> -0: mflr r3
> - ld r3,(1f-0b)(r3) /* get &rtas_restore_regs */
> -
> ld r1,PACAR1(r13) /* Restore our SP */
> - ld r4,PACASAVEDMSR(r13) /* Restore our MSR */
>
> - mtspr SPRN_SRR0,r3
> - mtspr SPRN_SRR1,r4
> - RFI_TO_KERNEL
rfid is not more called to restore MSR.
Noob question, is there any impact of using mtmsrd instead of rfid to
restore the MSR?
> - b . /* prevent speculative execution */
> -_ASM_NOKPROBE_SYMBOL(enter_rtas)
> -_ASM_NOKPROBE_SYMBOL(__enter_rtas)
> -_ASM_NOKPROBE_SYMBOL(rtas_return_loc)
> -
> - .align 3
> -1: .8byte rtas_restore_regs
> -
> -rtas_restore_regs:
> - /* relocation is on at this point */
> REST_GPR(2, r1) /* Restore the TOC */
> REST_NVGPRS(r1) /* Restore the non-volatiles */
>
> @@ -169,5 +147,5 @@ rtas_restore_regs:
>
> mtlr r0
> blr /* return to caller */
> -
> +_ASM_NOKPROBE_SYMBOL(rtas_return_loc)
> #endif /* CONFIG_PPC32 */
