2023-07-15 13:59:50

by Guo Ren

[permalink] [raw]
Subject: [PATCH 0/2] riscv: stack: Fixup independent softirq/irq stack for CONFIG_FRAME_POINTER=n

From: Guo Ren <[email protected]>

The independent softirq/irq stack uses s0 to save & restore sp, but s0
would be corrupted when CONFIG_FRAME_POINTER=n. So add s0 in the clobber
list to fix the problem.

<+0>: addi sp,sp,-32
<+2>: sd s0,16(sp)
<+4>: sd s1,8(sp)
<+6>: sd ra,24(sp)
<+8>: sd s2,0(sp)
<+10>: mv s0,a0 --> compiler allocate s0 for a0 when CONFIG_FRAME_POINTER=n
<+12>: jal ra,0xffffffff800bc0ce <irqentry_enter>
<+16>: ld a5,56(tp) # 0x38
<+20>: lui a4,0x4
<+22>: mv s1,a0
<+24>: xor a5,a5,sp
<+28>: bgeu a5,a4,0xffffffff800bc092 <do_irq+88>
<+32>: auipc s2,0x5d
<+36>: ld s2,1118(s2) # 0xffffffff801194b8 <irq_stack_ptr>
<+40>: add s2,s2,a4
<+42>: addi sp,sp,-8
<+44>: sd ra,0(sp)
<+46>: addi sp,sp,-8
<+48>: sd s0,0(sp)
<+50>: addi s0,sp,16 --> our code clobber the s0
<+52>: mv sp,s2
<+54>: mv a0,s0 --> a0 got wrong value for handle_riscv_irq
<+56>: jal ra,0xffffffff800bbb3a <handle_riscv_irq>

Guo Ren (2):
riscv: stack: Fixup independent irq stack for CONFIG_FRAME_POINTER=n
riscv: stack: Fixup independent softirq stack for
CONFIG_FRAME_POINTER=n

arch/riscv/kernel/irq.c | 2 +-
arch/riscv/kernel/traps.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)

--
2.36.1



2023-07-15 14:30:31

by Guo Ren

[permalink] [raw]
Subject: [PATCH 2/2] riscv: stack: Fixup independent softirq stack for CONFIG_FRAME_POINTER=n

From: Guo Ren <[email protected]>

The independent softirq stack uses s0 to save & restore sp, but s0 would
be corrupted when CONFIG_FRAME_POINTER=n. So add s0 in the clobber list
to fix the problem.

Fixes: dd69d07a5a6c ("riscv: stack: Support HAVE_SOFTIRQ_ON_OWN_STACK")
Reported-by: Zhangjin Wu <[email protected]>
Signed-off-by: Guo Ren <[email protected]>
Signed-off-by: Guo Ren <[email protected]>
---
arch/riscv/kernel/irq.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index d0577cc6a081..0b58720109db 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -83,7 +83,7 @@ void do_softirq_own_stack(void)
:
: [sp] "r" (sp)
: "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7",
- "t0", "t1", "t2", "t3", "t4", "t5", "t6",
+ "t0", "t1", "t2", "t3", "t4", "t5", "t6", "s0",
"memory");
} else
#endif
--
2.36.1


2023-07-15 14:44:41

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/2] riscv: stack: Fixup independent softirq stack for CONFIG_FRAME_POINTER=n

On Sat, Jul 15, 2023 at 09:45:52AM -0400, [email protected] wrote:
> From: Guo Ren <[email protected]>
>
> The independent softirq stack uses s0 to save & restore sp, but s0 would
> be corrupted when CONFIG_FRAME_POINTER=n. So add s0 in the clobber list
> to fix the problem.
>
> Fixes: dd69d07a5a6c ("riscv: stack: Support HAVE_SOFTIRQ_ON_OWN_STACK")
> Reported-by: Zhangjin Wu <[email protected]>
> Signed-off-by: Guo Ren <[email protected]>
> Signed-off-by: Guo Ren <[email protected]>
> ---
> arch/riscv/kernel/irq.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
> index d0577cc6a081..0b58720109db 100644
> --- a/arch/riscv/kernel/irq.c
> +++ b/arch/riscv/kernel/irq.c
> @@ -83,7 +83,7 @@ void do_softirq_own_stack(void)
> :
> : [sp] "r" (sp)
> : "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7",
> - "t0", "t1", "t2", "t3", "t4", "t5", "t6",
> + "t0", "t1", "t2", "t3", "t4", "t5", "t6", "s0",
> "memory");
> } else
> #endif
> --
> 2.36.1
>

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

2023-07-16 01:09:55

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH 0/2] riscv: stack: Fixup independent softirq/irq stack for CONFIG_FRAME_POINTER=n

I'm sorry, this patch would break CONFIG_FRAME_POINTER=y, so please abandon it.

I've updated a v2 with the [email protected] tag fixed:
https://lore.kernel.org/linux-riscv/[email protected]/


On Sat, Jul 15, 2023 at 9:46 PM <[email protected]> wrote:
>
> From: Guo Ren <[email protected]>
>
> The independent softirq/irq stack uses s0 to save & restore sp, but s0
> would be corrupted when CONFIG_FRAME_POINTER=n. So add s0 in the clobber
> list to fix the problem.
>
> <+0>: addi sp,sp,-32
> <+2>: sd s0,16(sp)
> <+4>: sd s1,8(sp)
> <+6>: sd ra,24(sp)
> <+8>: sd s2,0(sp)
> <+10>: mv s0,a0 --> compiler allocate s0 for a0 when CONFIG_FRAME_POINTER=n
> <+12>: jal ra,0xffffffff800bc0ce <irqentry_enter>
> <+16>: ld a5,56(tp) # 0x38
> <+20>: lui a4,0x4
> <+22>: mv s1,a0
> <+24>: xor a5,a5,sp
> <+28>: bgeu a5,a4,0xffffffff800bc092 <do_irq+88>
> <+32>: auipc s2,0x5d
> <+36>: ld s2,1118(s2) # 0xffffffff801194b8 <irq_stack_ptr>
> <+40>: add s2,s2,a4
> <+42>: addi sp,sp,-8
> <+44>: sd ra,0(sp)
> <+46>: addi sp,sp,-8
> <+48>: sd s0,0(sp)
> <+50>: addi s0,sp,16 --> our code clobber the s0
> <+52>: mv sp,s2
> <+54>: mv a0,s0 --> a0 got wrong value for handle_riscv_irq
> <+56>: jal ra,0xffffffff800bbb3a <handle_riscv_irq>
>
> Guo Ren (2):
> riscv: stack: Fixup independent irq stack for CONFIG_FRAME_POINTER=n
> riscv: stack: Fixup independent softirq stack for
> CONFIG_FRAME_POINTER=n
>
> arch/riscv/kernel/irq.c | 2 +-
> arch/riscv/kernel/traps.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> --
> 2.36.1
>


--
Best Regards

Guo Ren