* [PATCH] powerpc/traps: Declare unrecoverable_exception() as __noreturn
@ 2021-02-11 6:34 Christophe Leroy
2021-02-11 7:17 ` Christophe Leroy
2021-02-11 7:47 ` Gabriel Paubert
0 siblings, 2 replies; 4+ messages in thread
From: Christophe Leroy @ 2021-02-11 6:34 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin
Cc: linux-kernel, linuxppc-dev
unrecoverable_exception() is never expected to return, most callers
have an infiniteloop in case it returns.
Ensure it really never returns by terminating it with a BUG(), and
declare it __no_return.
It always GCC to really simplify functions calling it. In the exemple below,
it avoids the stack frame in the likely fast path and avoids code duplication
for the exit.
With this patch:
00000348 <interrupt_exit_kernel_prepare>:
348: 81 43 00 84 lwz r10,132(r3)
34c: 71 48 00 02 andi. r8,r10,2
350: 41 82 00 2c beq 37c <interrupt_exit_kernel_prepare+0x34>
354: 71 4a 40 00 andi. r10,r10,16384
358: 40 82 00 20 bne 378 <interrupt_exit_kernel_prepare+0x30>
35c: 80 62 00 70 lwz r3,112(r2)
360: 74 63 00 01 andis. r3,r3,1
364: 40 82 00 28 bne 38c <interrupt_exit_kernel_prepare+0x44>
368: 7d 40 00 a6 mfmsr r10
36c: 7c 11 13 a6 mtspr 81,r0
370: 7c 12 13 a6 mtspr 82,r0
374: 4e 80 00 20 blr
378: 48 00 00 00 b 378 <interrupt_exit_kernel_prepare+0x30>
37c: 94 21 ff f0 stwu r1,-16(r1)
380: 7c 08 02 a6 mflr r0
384: 90 01 00 14 stw r0,20(r1)
388: 48 00 00 01 bl 388 <interrupt_exit_kernel_prepare+0x40>
388: R_PPC_REL24 unrecoverable_exception
38c: 38 e2 00 70 addi r7,r2,112
390: 3d 00 00 01 lis r8,1
394: 7c c0 38 28 lwarx r6,0,r7
398: 7c c6 40 78 andc r6,r6,r8
39c: 7c c0 39 2d stwcx. r6,0,r7
3a0: 40 a2 ff f4 bne 394 <interrupt_exit_kernel_prepare+0x4c>
3a4: 38 60 00 01 li r3,1
3a8: 4b ff ff c0 b 368 <interrupt_exit_kernel_prepare+0x20>
Without this patch:
00000348 <interrupt_exit_kernel_prepare>:
348: 94 21 ff f0 stwu r1,-16(r1)
34c: 93 e1 00 0c stw r31,12(r1)
350: 7c 7f 1b 78 mr r31,r3
354: 81 23 00 84 lwz r9,132(r3)
358: 71 2a 00 02 andi. r10,r9,2
35c: 41 82 00 34 beq 390 <interrupt_exit_kernel_prepare+0x48>
360: 71 29 40 00 andi. r9,r9,16384
364: 40 82 00 28 bne 38c <interrupt_exit_kernel_prepare+0x44>
368: 80 62 00 70 lwz r3,112(r2)
36c: 74 63 00 01 andis. r3,r3,1
370: 40 82 00 3c bne 3ac <interrupt_exit_kernel_prepare+0x64>
374: 7d 20 00 a6 mfmsr r9
378: 7c 11 13 a6 mtspr 81,r0
37c: 7c 12 13 a6 mtspr 82,r0
380: 83 e1 00 0c lwz r31,12(r1)
384: 38 21 00 10 addi r1,r1,16
388: 4e 80 00 20 blr
38c: 48 00 00 00 b 38c <interrupt_exit_kernel_prepare+0x44>
390: 7c 08 02 a6 mflr r0
394: 90 01 00 14 stw r0,20(r1)
398: 48 00 00 01 bl 398 <interrupt_exit_kernel_prepare+0x50>
398: R_PPC_REL24 unrecoverable_exception
39c: 80 01 00 14 lwz r0,20(r1)
3a0: 81 3f 00 84 lwz r9,132(r31)
3a4: 7c 08 03 a6 mtlr r0
3a8: 4b ff ff b8 b 360 <interrupt_exit_kernel_prepare+0x18>
3ac: 39 02 00 70 addi r8,r2,112
3b0: 3d 40 00 01 lis r10,1
3b4: 7c e0 40 28 lwarx r7,0,r8
3b8: 7c e7 50 78 andc r7,r7,r10
3bc: 7c e0 41 2d stwcx. r7,0,r8
3c0: 40 a2 ff f4 bne 3b4 <interrupt_exit_kernel_prepare+0x6c>
3c4: 38 60 00 01 li r3,1
3c8: 4b ff ff ac b 374 <interrupt_exit_kernel_prepare+0x2c>
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/include/asm/interrupt.h | 2 +-
arch/powerpc/kernel/interrupt.c | 1 -
arch/powerpc/kernel/traps.c | 2 ++
3 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
index dcff30e3919b..fa8bfb91f8df 100644
--- a/arch/powerpc/include/asm/interrupt.h
+++ b/arch/powerpc/include/asm/interrupt.h
@@ -411,7 +411,7 @@ DECLARE_INTERRUPT_HANDLER(altivec_assist_exception);
DECLARE_INTERRUPT_HANDLER(CacheLockingException);
DECLARE_INTERRUPT_HANDLER(SPEFloatingPointException);
DECLARE_INTERRUPT_HANDLER(SPEFloatingPointRoundException);
-DECLARE_INTERRUPT_HANDLER(unrecoverable_exception);
+DECLARE_INTERRUPT_HANDLER(unrecoverable_exception) __noreturn;
DECLARE_INTERRUPT_HANDLER(WatchdogException);
DECLARE_INTERRUPT_HANDLER(kernel_bad_stack);
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index eca3be36c18c..7e7106641ca9 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -440,7 +440,6 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
return ret;
}
-void unrecoverable_exception(struct pt_regs *regs);
void preempt_schedule_irq(void);
notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index 2afa05ad21c8..1ff776e9e8e3 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -2173,6 +2173,8 @@ DEFINE_INTERRUPT_HANDLER(unrecoverable_exception)
pr_emerg("Unrecoverable exception %lx at %lx (msr=%lx)\n",
regs->trap, regs->nip, regs->msr);
die("Unrecoverable exception", regs, SIGABRT);
+ /* die() should not return */
+ BUG();
}
NOKPROBE_SYMBOL(unrecoverable_exception);
--
2.25.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/traps: Declare unrecoverable_exception() as __noreturn
2021-02-11 6:34 [PATCH] powerpc/traps: Declare unrecoverable_exception() as __noreturn Christophe Leroy
@ 2021-02-11 7:17 ` Christophe Leroy
2021-02-11 7:47 ` Gabriel Paubert
1 sibling, 0 replies; 4+ messages in thread
From: Christophe Leroy @ 2021-02-11 7:17 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin
Cc: linuxppc-dev, linux-kernel
Le 11/02/2021 à 07:34, Christophe Leroy a écrit :
> unrecoverable_exception() is never expected to return, most callers
> have an infiniteloop in case it returns.
>
> Ensure it really never returns by terminating it with a BUG(), and
> declare it __no_return.
Not so easy, gcc complains about DEFINE_INTERRUPT_HANDLER() returning while the function is declared
__noreturn, __noreturn is needed there too.
>
> It always GCC to really simplify functions calling it. In the exemple below,
> it avoids the stack frame in the likely fast path and avoids code duplication
> for the exit.
>
> With this patch:
>
> 00000348 <interrupt_exit_kernel_prepare>:
> 348: 81 43 00 84 lwz r10,132(r3)
> 34c: 71 48 00 02 andi. r8,r10,2
> 350: 41 82 00 2c beq 37c <interrupt_exit_kernel_prepare+0x34>
> 354: 71 4a 40 00 andi. r10,r10,16384
> 358: 40 82 00 20 bne 378 <interrupt_exit_kernel_prepare+0x30>
> 35c: 80 62 00 70 lwz r3,112(r2)
> 360: 74 63 00 01 andis. r3,r3,1
> 364: 40 82 00 28 bne 38c <interrupt_exit_kernel_prepare+0x44>
> 368: 7d 40 00 a6 mfmsr r10
> 36c: 7c 11 13 a6 mtspr 81,r0
> 370: 7c 12 13 a6 mtspr 82,r0
> 374: 4e 80 00 20 blr
> 378: 48 00 00 00 b 378 <interrupt_exit_kernel_prepare+0x30>
> 37c: 94 21 ff f0 stwu r1,-16(r1)
> 380: 7c 08 02 a6 mflr r0
> 384: 90 01 00 14 stw r0,20(r1)
> 388: 48 00 00 01 bl 388 <interrupt_exit_kernel_prepare+0x40>
> 388: R_PPC_REL24 unrecoverable_exception
> 38c: 38 e2 00 70 addi r7,r2,112
> 390: 3d 00 00 01 lis r8,1
> 394: 7c c0 38 28 lwarx r6,0,r7
> 398: 7c c6 40 78 andc r6,r6,r8
> 39c: 7c c0 39 2d stwcx. r6,0,r7
> 3a0: 40 a2 ff f4 bne 394 <interrupt_exit_kernel_prepare+0x4c>
> 3a4: 38 60 00 01 li r3,1
> 3a8: 4b ff ff c0 b 368 <interrupt_exit_kernel_prepare+0x20>
>
> Without this patch:
>
> 00000348 <interrupt_exit_kernel_prepare>:
> 348: 94 21 ff f0 stwu r1,-16(r1)
> 34c: 93 e1 00 0c stw r31,12(r1)
> 350: 7c 7f 1b 78 mr r31,r3
> 354: 81 23 00 84 lwz r9,132(r3)
> 358: 71 2a 00 02 andi. r10,r9,2
> 35c: 41 82 00 34 beq 390 <interrupt_exit_kernel_prepare+0x48>
> 360: 71 29 40 00 andi. r9,r9,16384
> 364: 40 82 00 28 bne 38c <interrupt_exit_kernel_prepare+0x44>
> 368: 80 62 00 70 lwz r3,112(r2)
> 36c: 74 63 00 01 andis. r3,r3,1
> 370: 40 82 00 3c bne 3ac <interrupt_exit_kernel_prepare+0x64>
> 374: 7d 20 00 a6 mfmsr r9
> 378: 7c 11 13 a6 mtspr 81,r0
> 37c: 7c 12 13 a6 mtspr 82,r0
> 380: 83 e1 00 0c lwz r31,12(r1)
> 384: 38 21 00 10 addi r1,r1,16
> 388: 4e 80 00 20 blr
> 38c: 48 00 00 00 b 38c <interrupt_exit_kernel_prepare+0x44>
> 390: 7c 08 02 a6 mflr r0
> 394: 90 01 00 14 stw r0,20(r1)
> 398: 48 00 00 01 bl 398 <interrupt_exit_kernel_prepare+0x50>
> 398: R_PPC_REL24 unrecoverable_exception
> 39c: 80 01 00 14 lwz r0,20(r1)
> 3a0: 81 3f 00 84 lwz r9,132(r31)
> 3a4: 7c 08 03 a6 mtlr r0
> 3a8: 4b ff ff b8 b 360 <interrupt_exit_kernel_prepare+0x18>
> 3ac: 39 02 00 70 addi r8,r2,112
> 3b0: 3d 40 00 01 lis r10,1
> 3b4: 7c e0 40 28 lwarx r7,0,r8
> 3b8: 7c e7 50 78 andc r7,r7,r10
> 3bc: 7c e0 41 2d stwcx. r7,0,r8
> 3c0: 40 a2 ff f4 bne 3b4 <interrupt_exit_kernel_prepare+0x6c>
> 3c4: 38 60 00 01 li r3,1
> 3c8: 4b ff ff ac b 374 <interrupt_exit_kernel_prepare+0x2c>
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/interrupt.h | 2 +-
> arch/powerpc/kernel/interrupt.c | 1 -
> arch/powerpc/kernel/traps.c | 2 ++
> 3 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index dcff30e3919b..fa8bfb91f8df 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -411,7 +411,7 @@ DECLARE_INTERRUPT_HANDLER(altivec_assist_exception);
> DECLARE_INTERRUPT_HANDLER(CacheLockingException);
> DECLARE_INTERRUPT_HANDLER(SPEFloatingPointException);
> DECLARE_INTERRUPT_HANDLER(SPEFloatingPointRoundException);
> -DECLARE_INTERRUPT_HANDLER(unrecoverable_exception);
> +DECLARE_INTERRUPT_HANDLER(unrecoverable_exception) __noreturn;
> DECLARE_INTERRUPT_HANDLER(WatchdogException);
> DECLARE_INTERRUPT_HANDLER(kernel_bad_stack);
>
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index eca3be36c18c..7e7106641ca9 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -440,7 +440,6 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
> return ret;
> }
>
> -void unrecoverable_exception(struct pt_regs *regs);
> void preempt_schedule_irq(void);
>
> notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr)
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 2afa05ad21c8..1ff776e9e8e3 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -2173,6 +2173,8 @@ DEFINE_INTERRUPT_HANDLER(unrecoverable_exception)
> pr_emerg("Unrecoverable exception %lx at %lx (msr=%lx)\n",
> regs->trap, regs->nip, regs->msr);
> die("Unrecoverable exception", regs, SIGABRT);
> + /* die() should not return */
> + BUG();
> }
> NOKPROBE_SYMBOL(unrecoverable_exception);
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/traps: Declare unrecoverable_exception() as __noreturn
2021-02-11 6:34 [PATCH] powerpc/traps: Declare unrecoverable_exception() as __noreturn Christophe Leroy
2021-02-11 7:17 ` Christophe Leroy
@ 2021-02-11 7:47 ` Gabriel Paubert
2021-02-11 9:02 ` Christophe Leroy
1 sibling, 1 reply; 4+ messages in thread
From: Gabriel Paubert @ 2021-02-11 7:47 UTC (permalink / raw)
To: Christophe Leroy
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
npiggin, linuxppc-dev, linux-kernel
On Thu, Feb 11, 2021 at 06:34:43AM +0000, Christophe Leroy wrote:
> unrecoverable_exception() is never expected to return, most callers
> have an infiniteloop in case it returns.
>
> Ensure it really never returns by terminating it with a BUG(), and
> declare it __no_return.
>
> It always GCC to really simplify functions calling it. In the exemple below,
s/always/allows ?
(Otherwise I can't parse it.)
> it avoids the stack frame in the likely fast path and avoids code duplication
> for the exit.
Indeed, nice code generation improvement.
>
> With this patch:
>
> 00000348 <interrupt_exit_kernel_prepare>:
> 348: 81 43 00 84 lwz r10,132(r3)
> 34c: 71 48 00 02 andi. r8,r10,2
> 350: 41 82 00 2c beq 37c <interrupt_exit_kernel_prepare+0x34>
> 354: 71 4a 40 00 andi. r10,r10,16384
> 358: 40 82 00 20 bne 378 <interrupt_exit_kernel_prepare+0x30>
> 35c: 80 62 00 70 lwz r3,112(r2)
> 360: 74 63 00 01 andis. r3,r3,1
> 364: 40 82 00 28 bne 38c <interrupt_exit_kernel_prepare+0x44>
> 368: 7d 40 00 a6 mfmsr r10
> 36c: 7c 11 13 a6 mtspr 81,r0
> 370: 7c 12 13 a6 mtspr 82,r0
> 374: 4e 80 00 20 blr
> 378: 48 00 00 00 b 378 <interrupt_exit_kernel_prepare+0x30>
Infinite loop (seems to be on test of MSR_PR)?
Gabriel
> 37c: 94 21 ff f0 stwu r1,-16(r1)
> 380: 7c 08 02 a6 mflr r0
> 384: 90 01 00 14 stw r0,20(r1)
> 388: 48 00 00 01 bl 388 <interrupt_exit_kernel_prepare+0x40>
> 388: R_PPC_REL24 unrecoverable_exception
> 38c: 38 e2 00 70 addi r7,r2,112
> 390: 3d 00 00 01 lis r8,1
> 394: 7c c0 38 28 lwarx r6,0,r7
> 398: 7c c6 40 78 andc r6,r6,r8
> 39c: 7c c0 39 2d stwcx. r6,0,r7
> 3a0: 40 a2 ff f4 bne 394 <interrupt_exit_kernel_prepare+0x4c>
> 3a4: 38 60 00 01 li r3,1
> 3a8: 4b ff ff c0 b 368 <interrupt_exit_kernel_prepare+0x20>
>
> Without this patch:
>
> 00000348 <interrupt_exit_kernel_prepare>:
> 348: 94 21 ff f0 stwu r1,-16(r1)
> 34c: 93 e1 00 0c stw r31,12(r1)
> 350: 7c 7f 1b 78 mr r31,r3
> 354: 81 23 00 84 lwz r9,132(r3)
> 358: 71 2a 00 02 andi. r10,r9,2
> 35c: 41 82 00 34 beq 390 <interrupt_exit_kernel_prepare+0x48>
> 360: 71 29 40 00 andi. r9,r9,16384
> 364: 40 82 00 28 bne 38c <interrupt_exit_kernel_prepare+0x44>
> 368: 80 62 00 70 lwz r3,112(r2)
> 36c: 74 63 00 01 andis. r3,r3,1
> 370: 40 82 00 3c bne 3ac <interrupt_exit_kernel_prepare+0x64>
> 374: 7d 20 00 a6 mfmsr r9
> 378: 7c 11 13 a6 mtspr 81,r0
> 37c: 7c 12 13 a6 mtspr 82,r0
> 380: 83 e1 00 0c lwz r31,12(r1)
> 384: 38 21 00 10 addi r1,r1,16
> 388: 4e 80 00 20 blr
> 38c: 48 00 00 00 b 38c <interrupt_exit_kernel_prepare+0x44>
> 390: 7c 08 02 a6 mflr r0
> 394: 90 01 00 14 stw r0,20(r1)
> 398: 48 00 00 01 bl 398 <interrupt_exit_kernel_prepare+0x50>
> 398: R_PPC_REL24 unrecoverable_exception
> 39c: 80 01 00 14 lwz r0,20(r1)
> 3a0: 81 3f 00 84 lwz r9,132(r31)
> 3a4: 7c 08 03 a6 mtlr r0
> 3a8: 4b ff ff b8 b 360 <interrupt_exit_kernel_prepare+0x18>
> 3ac: 39 02 00 70 addi r8,r2,112
> 3b0: 3d 40 00 01 lis r10,1
> 3b4: 7c e0 40 28 lwarx r7,0,r8
> 3b8: 7c e7 50 78 andc r7,r7,r10
> 3bc: 7c e0 41 2d stwcx. r7,0,r8
> 3c0: 40 a2 ff f4 bne 3b4 <interrupt_exit_kernel_prepare+0x6c>
> 3c4: 38 60 00 01 li r3,1
> 3c8: 4b ff ff ac b 374 <interrupt_exit_kernel_prepare+0x2c>
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/interrupt.h | 2 +-
> arch/powerpc/kernel/interrupt.c | 1 -
> arch/powerpc/kernel/traps.c | 2 ++
> 3 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
> index dcff30e3919b..fa8bfb91f8df 100644
> --- a/arch/powerpc/include/asm/interrupt.h
> +++ b/arch/powerpc/include/asm/interrupt.h
> @@ -411,7 +411,7 @@ DECLARE_INTERRUPT_HANDLER(altivec_assist_exception);
> DECLARE_INTERRUPT_HANDLER(CacheLockingException);
> DECLARE_INTERRUPT_HANDLER(SPEFloatingPointException);
> DECLARE_INTERRUPT_HANDLER(SPEFloatingPointRoundException);
> -DECLARE_INTERRUPT_HANDLER(unrecoverable_exception);
> +DECLARE_INTERRUPT_HANDLER(unrecoverable_exception) __noreturn;
> DECLARE_INTERRUPT_HANDLER(WatchdogException);
> DECLARE_INTERRUPT_HANDLER(kernel_bad_stack);
>
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index eca3be36c18c..7e7106641ca9 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -440,7 +440,6 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
> return ret;
> }
>
> -void unrecoverable_exception(struct pt_regs *regs);
> void preempt_schedule_irq(void);
>
> notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr)
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 2afa05ad21c8..1ff776e9e8e3 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -2173,6 +2173,8 @@ DEFINE_INTERRUPT_HANDLER(unrecoverable_exception)
> pr_emerg("Unrecoverable exception %lx at %lx (msr=%lx)\n",
> regs->trap, regs->nip, regs->msr);
> die("Unrecoverable exception", regs, SIGABRT);
> + /* die() should not return */
> + BUG();
> }
> NOKPROBE_SYMBOL(unrecoverable_exception);
>
> --
> 2.25.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] powerpc/traps: Declare unrecoverable_exception() as __noreturn
2021-02-11 7:47 ` Gabriel Paubert
@ 2021-02-11 9:02 ` Christophe Leroy
0 siblings, 0 replies; 4+ messages in thread
From: Christophe Leroy @ 2021-02-11 9:02 UTC (permalink / raw)
To: Gabriel Paubert
Cc: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman,
npiggin, linuxppc-dev, linux-kernel
Le 11/02/2021 à 08:47, Gabriel Paubert a écrit :
> On Thu, Feb 11, 2021 at 06:34:43AM +0000, Christophe Leroy wrote:
>> unrecoverable_exception() is never expected to return, most callers
>> have an infiniteloop in case it returns.
>>
>> Ensure it really never returns by terminating it with a BUG(), and
>> declare it __no_return.
>>
>> It always GCC to really simplify functions calling it. In the exemple below,
>
> s/always/allows ?
Yes
>
> (Otherwise I can't parse it.)
>
>> it avoids the stack frame in the likely fast path and avoids code duplication
>> for the exit.
>
> Indeed, nice code generation improvement.
>
>>
>> With this patch:
>>
>> 00000348 <interrupt_exit_kernel_prepare>:
>> 348: 81 43 00 84 lwz r10,132(r3)
>> 34c: 71 48 00 02 andi. r8,r10,2
>> 350: 41 82 00 2c beq 37c <interrupt_exit_kernel_prepare+0x34>
>> 354: 71 4a 40 00 andi. r10,r10,16384
>> 358: 40 82 00 20 bne 378 <interrupt_exit_kernel_prepare+0x30>
>> 35c: 80 62 00 70 lwz r3,112(r2)
>> 360: 74 63 00 01 andis. r3,r3,1
>> 364: 40 82 00 28 bne 38c <interrupt_exit_kernel_prepare+0x44>
>> 368: 7d 40 00 a6 mfmsr r10
>> 36c: 7c 11 13 a6 mtspr 81,r0
>> 370: 7c 12 13 a6 mtspr 82,r0
>> 374: 4e 80 00 20 blr
>> 378: 48 00 00 00 b 378 <interrupt_exit_kernel_prepare+0x30>
>
> Infinite loop (seems to be on test of MSR_PR)?
Yes, that's what you get when CONFIG_BUG is not selected.
/include/asm-generic/bug.h:
#ifndef HAVE_ARCH_BUG
#define BUG() do {} while (1)
#endif
>
> Gabriel
>
>> 37c: 94 21 ff f0 stwu r1,-16(r1)
>> 380: 7c 08 02 a6 mflr r0
>> 384: 90 01 00 14 stw r0,20(r1)
>> 388: 48 00 00 01 bl 388 <interrupt_exit_kernel_prepare+0x40>
>> 388: R_PPC_REL24 unrecoverable_exception
>> 38c: 38 e2 00 70 addi r7,r2,112
>> 390: 3d 00 00 01 lis r8,1
>> 394: 7c c0 38 28 lwarx r6,0,r7
>> 398: 7c c6 40 78 andc r6,r6,r8
>> 39c: 7c c0 39 2d stwcx. r6,0,r7
>> 3a0: 40 a2 ff f4 bne 394 <interrupt_exit_kernel_prepare+0x4c>
>> 3a4: 38 60 00 01 li r3,1
>> 3a8: 4b ff ff c0 b 368 <interrupt_exit_kernel_prepare+0x20>
>>
>> Without this patch:
>>
>> 00000348 <interrupt_exit_kernel_prepare>:
>> 348: 94 21 ff f0 stwu r1,-16(r1)
>> 34c: 93 e1 00 0c stw r31,12(r1)
>> 350: 7c 7f 1b 78 mr r31,r3
>> 354: 81 23 00 84 lwz r9,132(r3)
>> 358: 71 2a 00 02 andi. r10,r9,2
>> 35c: 41 82 00 34 beq 390 <interrupt_exit_kernel_prepare+0x48>
>> 360: 71 29 40 00 andi. r9,r9,16384
>> 364: 40 82 00 28 bne 38c <interrupt_exit_kernel_prepare+0x44>
>> 368: 80 62 00 70 lwz r3,112(r2)
>> 36c: 74 63 00 01 andis. r3,r3,1
>> 370: 40 82 00 3c bne 3ac <interrupt_exit_kernel_prepare+0x64>
>> 374: 7d 20 00 a6 mfmsr r9
>> 378: 7c 11 13 a6 mtspr 81,r0
>> 37c: 7c 12 13 a6 mtspr 82,r0
>> 380: 83 e1 00 0c lwz r31,12(r1)
>> 384: 38 21 00 10 addi r1,r1,16
>> 388: 4e 80 00 20 blr
>> 38c: 48 00 00 00 b 38c <interrupt_exit_kernel_prepare+0x44>
>> 390: 7c 08 02 a6 mflr r0
>> 394: 90 01 00 14 stw r0,20(r1)
>> 398: 48 00 00 01 bl 398 <interrupt_exit_kernel_prepare+0x50>
>> 398: R_PPC_REL24 unrecoverable_exception
>> 39c: 80 01 00 14 lwz r0,20(r1)
>> 3a0: 81 3f 00 84 lwz r9,132(r31)
>> 3a4: 7c 08 03 a6 mtlr r0
>> 3a8: 4b ff ff b8 b 360 <interrupt_exit_kernel_prepare+0x18>
>> 3ac: 39 02 00 70 addi r8,r2,112
>> 3b0: 3d 40 00 01 lis r10,1
>> 3b4: 7c e0 40 28 lwarx r7,0,r8
>> 3b8: 7c e7 50 78 andc r7,r7,r10
>> 3bc: 7c e0 41 2d stwcx. r7,0,r8
>> 3c0: 40 a2 ff f4 bne 3b4 <interrupt_exit_kernel_prepare+0x6c>
>> 3c4: 38 60 00 01 li r3,1
>> 3c8: 4b ff ff ac b 374 <interrupt_exit_kernel_prepare+0x2c>
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> arch/powerpc/include/asm/interrupt.h | 2 +-
>> arch/powerpc/kernel/interrupt.c | 1 -
>> arch/powerpc/kernel/traps.c | 2 ++
>> 3 files changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
>> index dcff30e3919b..fa8bfb91f8df 100644
>> --- a/arch/powerpc/include/asm/interrupt.h
>> +++ b/arch/powerpc/include/asm/interrupt.h
>> @@ -411,7 +411,7 @@ DECLARE_INTERRUPT_HANDLER(altivec_assist_exception);
>> DECLARE_INTERRUPT_HANDLER(CacheLockingException);
>> DECLARE_INTERRUPT_HANDLER(SPEFloatingPointException);
>> DECLARE_INTERRUPT_HANDLER(SPEFloatingPointRoundException);
>> -DECLARE_INTERRUPT_HANDLER(unrecoverable_exception);
>> +DECLARE_INTERRUPT_HANDLER(unrecoverable_exception) __noreturn;
>> DECLARE_INTERRUPT_HANDLER(WatchdogException);
>> DECLARE_INTERRUPT_HANDLER(kernel_bad_stack);
>>
>> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
>> index eca3be36c18c..7e7106641ca9 100644
>> --- a/arch/powerpc/kernel/interrupt.c
>> +++ b/arch/powerpc/kernel/interrupt.c
>> @@ -440,7 +440,6 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
>> return ret;
>> }
>>
>> -void unrecoverable_exception(struct pt_regs *regs);
>> void preempt_schedule_irq(void);
>>
>> notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsigned long msr)
>> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
>> index 2afa05ad21c8..1ff776e9e8e3 100644
>> --- a/arch/powerpc/kernel/traps.c
>> +++ b/arch/powerpc/kernel/traps.c
>> @@ -2173,6 +2173,8 @@ DEFINE_INTERRUPT_HANDLER(unrecoverable_exception)
>> pr_emerg("Unrecoverable exception %lx at %lx (msr=%lx)\n",
>> regs->trap, regs->nip, regs->msr);
>> die("Unrecoverable exception", regs, SIGABRT);
>> + /* die() should not return */
>> + BUG();
>> }
>> NOKPROBE_SYMBOL(unrecoverable_exception);
>>
>> --
>> 2.25.0
>>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-02-11 12:45 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-11 6:34 [PATCH] powerpc/traps: Declare unrecoverable_exception() as __noreturn Christophe Leroy
2021-02-11 7:17 ` Christophe Leroy
2021-02-11 7:47 ` Gabriel Paubert
2021-02-11 9:02 ` Christophe Leroy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).