Skip to contentSkip to conversation

RISC-V: fix taking the text_mutex twice during sifive errata patching

PATCH Applied Conor Dooley 4 participants 6 messages Mar 2, 2023 – Mar 7, 2023

Subsystems (2)
RISC-V SiFive drivers
lore
Share & tools
lore

Conor Dooley

author
#0 lore raw

PATCH 1/1 RISC-V: fix taking the text_mutex twice during sifive errata patching

From: Conor Dooley <[email protected]>

Chris pointed out that some bonehead, *cough* me *cough*, added two mutex_locks() to the SiFive errata patching. The second was meant to have been a mutex_unlock().

Reported-by Chris Hofstaedtler <[email protected]>
Fixes 9493e6f3ce02 ("RISC-V: take text_mutex during alternative patching")
Signed-off-by Conor Dooley <[email protected]>

---
Palmer, you want to take this one for PR#2, or is it too late?

---
 arch/riscv/errata/sifive/errata.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/riscv/errata/sifive/errata.c +1 -1
index da55cb247e89..31d2ebea4286 100644
@@ -111,7 +111,7 @@ void __init_or_module sifive_errata_patch_func(struct alt_entry *begin,
 			mutex_lock(&text_mutex);
 			patch_text_nosync(ALT_OLD_PTR(alt), ALT_ALT_PTR(alt),
 					  alt->alt_len);
-			mutex_lock(&text_mutex);
+			mutex_unlock(&text_mutex);
 			cpu_apply_errata |= tmp;
 		}
 	}
Signature · 7 lines
2.39.2


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Tested-by on v1 1/1
#1 lore raw

Hi Conor,

Quoted 9 lines · Conor Dooley

From: Conor Dooley <[email protected]>

Chris pointed out that some bonehead, *cough* me *cough*, added two mutex_locks() to the SiFive errata patching. The second was meant to have been a mutex_unlock().

Reported-by Chris Hofstaedtler <[email protected]>
Fixes 9493e6f3ce02 ("RISC-V: take text_mutex during alternative patching")
Signed-off-by Conor Dooley <[email protected]>

Thanks for your patch!

This fixes the crash I see (with "earlycon keep_bootcon") on StarLight, and which I've just bisected to aforementioned commit, so

Tested-by Geert Uytterhoeven <[email protected]>

You may want to include (a part of) the crash log below in your patch description, to make it easier for affected people to find the fix.

Unable to handle kernel NULL pointer dereference at virtual address
0000000000000030
Oops [#1]
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted
6.2.0-rc1-starlight-00079-g9493e6f3ce02 #229
Hardware name: BeagleV Starlight Beta (DT)
epc : __schedule+0x42/0x500
 ra : schedule+0x46/0xce

epc : ffffffff8065957c ra : ffffffff80659a80 sp : ffffffff81203c80
gp : ffffffff812d50a0 tp : ffffffff8120db40 t0 : ffffffff81203d68 t1 : 0000000000000001 t2 : 4c45203a76637369 s0 : ffffffff81203cf0 s1 : ffffffff8120db40 a0 : 0000000000000000 a1 : ffffffff81213958 a2 : ffffffff81213958 a3 : 0000000000000000 a4 : 0000000000000000 a5 : ffffffff80a1bd00 a6 : 0000000000000000 a7 : 0000000052464e43 s2 : ffffffff8120db41 s3 : ffffffff80a1ad00 s4 : 0000000000000000 s5 : 0000000000000002 s6 : ffffffff81213938 s7 : 0000000000000000 s8 : 0000000000000000 s9 : 0000000000000001 s10: ffffffff812d7204
s11: ffffffff80d3c920 t3 : 0000000000000001 t4 : ffffffff812e6dd7 t5 : ffffffff812e6dd8 t6 : ffffffff81203bb8
status: 0000000200000100 badaddr: 0000000000000030 cause: 000000000000000d

[<ffffffff80659a80>] schedule+0x46/0xce
[<ffffffff80659dce>] schedule_preempt_disabled+0x16/0x28
[<ffffffff8065ae0c>] __mutex_lock.constprop.0+0x3fe/0x652
[<ffffffff8065b138>] __mutex_lock_slowpath+0xe/0x16
[<ffffffff8065b182>] mutex_lock+0x42/0x4c
[<ffffffff8000ad94>] sifive_errata_patch_func+0xf6/0x18c
[<ffffffff80002b92>] _apply_alternatives+0x74/0x76
[<ffffffff80802ee8>] apply_boot_alternatives+0x3c/0xfa
[<ffffffff80803cb0>] setup_arch+0x60c/0x640
[<ffffffff80800926>] start_kernel+0x8e/0x99c
---[ end trace 0000000000000000 ]---

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

author
#2 lore raw
Quoted 15 lines · Geert Uytterhoeven

Hi Conor,

Quoted 9 lines · Conor Dooley

From: Conor Dooley <[email protected]>

Chris pointed out that some bonehead, *cough* me *cough*, added two mutex_locks() to the SiFive errata patching. The second was meant to have been a mutex_unlock().

Reported-by Chris Hofstaedtler <[email protected]>
Fixes 9493e6f3ce02 ("RISC-V: take text_mutex during alternative patching")
Signed-off-by Conor Dooley <[email protected]>

Thanks for your patch!

Quoted 7 lines

This fixes the crash I see (with "earlycon keep_bootcon") on StarLight, and which I've just bisected to aforementioned commit, so

Tested-by Geert Uytterhoeven <[email protected]>

You may want to include (a part of) the crash log below in your patch description, to make it easier for affected people to find the fix.

There was no crash log when Chris pointed it out on IRC & I never asked if they had hit it in a running system.
If there's a v2 for whatever reason, I'll incorporate the splat. Apologies for the inconvenience caused by my boneheadedness! I'd hoped that this would make Palmer's second PR, but it was not to be :(

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

maintainer
#3 lore raw
Quoted 24 lines · Conor Dooley
Quoted 15 lines · Geert Uytterhoeven

Hi Conor,

Quoted 9 lines · Conor Dooley

From: Conor Dooley <[email protected]>

Chris pointed out that some bonehead, *cough* me *cough*, added two mutex_locks() to the SiFive errata patching. The second was meant to have been a mutex_unlock().

Reported-by Chris Hofstaedtler <[email protected]>
Fixes 9493e6f3ce02 ("RISC-V: take text_mutex during alternative patching")
Signed-off-by Conor Dooley <[email protected]>

Thanks for your patch!

Quoted 7 lines

This fixes the crash I see (with "earlycon keep_bootcon") on StarLight, and which I've just bisected to aforementioned commit, so

Tested-by Geert Uytterhoeven <[email protected]>

You may want to include (a part of) the crash log below in your patch description, to make it easier for affected people to find the fix.

Quoted 6 lines

There was no crash log when Chris pointed it out on IRC & I never asked if they had hit it in a running system.
If there's a v2 for whatever reason, I'll incorporate the splat. Apologies for the inconvenience caused by my boneheadedness! I'd hoped that this would make Palmer's second PR, but it was not to be :(

Sorry about that, there was just too much going on in the second half of the merge window and I was trying to dig through stuff. I'd actually though you were talking about the other mutex patch (the text_mutex one) when you pinged it, I forgot there were two.

I just queued this up, it's testing but I doubt it'll trip anything. It should be in fixes soon.

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

maintainer
#4 lore raw
In reply to Geert Uytterhoeven
Quoted 15 lines · On Tue, 07 Mar 2023 12:13:01 PST (-0800), [email protected] wrote:

Hi Conor,

Quoted 9 lines · Conor Dooley

From: Conor Dooley <[email protected]>

Chris pointed out that some bonehead, *cough* me *cough*, added two mutex_locks() to the SiFive errata patching. The second was meant to have been a mutex_unlock().

Reported-by Chris Hofstaedtler <[email protected]>
Fixes 9493e6f3ce02 ("RISC-V: take text_mutex during alternative patching")
Signed-off-by Conor Dooley <[email protected]>

Thanks for your patch!

Quoted 7 lines

This fixes the crash I see (with "earlycon keep_bootcon") on StarLight, and which I've just bisected to aforementioned commit, so

Tested-by Geert Uytterhoeven <[email protected]>

You may want to include (a part of) the crash log below in your patch description, to make it easier for affected people to find the fix.

I added it to the commit, thanks. This is in fixes.

Quoted 34 lines
Unable to handle kernel NULL pointer dereference at virtual address
0000000000000030
Oops [#1]
Modules linked in:
CPU: 0 PID: 0 Comm: swapper Not tainted
6.2.0-rc1-starlight-00079-g9493e6f3ce02 #229
Hardware name: BeagleV Starlight Beta (DT)
epc : __schedule+0x42/0x500
 ra : schedule+0x46/0xce

epc : ffffffff8065957c ra : ffffffff80659a80 sp : ffffffff81203c80
gp : ffffffff812d50a0 tp : ffffffff8120db40 t0 : ffffffff81203d68 t1 : 0000000000000001 t2 : 4c45203a76637369 s0 : ffffffff81203cf0 s1 : ffffffff8120db40 a0 : 0000000000000000 a1 : ffffffff81213958 a2 : ffffffff81213958 a3 : 0000000000000000 a4 : 0000000000000000 a5 : ffffffff80a1bd00 a6 : 0000000000000000 a7 : 0000000052464e43 s2 : ffffffff8120db41 s3 : ffffffff80a1ad00 s4 : 0000000000000000 s5 : 0000000000000002 s6 : ffffffff81213938 s7 : 0000000000000000 s8 : 0000000000000000 s9 : 0000000000000001 s10: ffffffff812d7204
s11: ffffffff80d3c920 t3 : 0000000000000001 t4 : ffffffff812e6dd7 t5 : ffffffff812e6dd8 t6 : ffffffff81203bb8
status: 0000000200000100 badaddr: 0000000000000030 cause: 000000000000000d

[<ffffffff80659a80>] schedule+0x46/0xce
[<ffffffff80659dce>] schedule_preempt_disabled+0x16/0x28
[<ffffffff8065ae0c>] __mutex_lock.constprop.0+0x3fe/0x652
[<ffffffff8065b138>] __mutex_lock_slowpath+0xe/0x16
[<ffffffff8065b182>] mutex_lock+0x42/0x4c
[<ffffffff8000ad94>] sifive_errata_patch_func+0xf6/0x18c
[<ffffffff80002b92>] _apply_alternatives+0x74/0x76
[<ffffffff80802ee8>] apply_boot_alternatives+0x3c/0xfa
[<ffffffff80803cb0>] setup_arch+0x60c/0x640
[<ffffffff80800926>] start_kernel+0x8e/0x99c
---[ end trace 0000000000000000 ]---
Quoted 3 lines

Gr{oetje,eeting}s,

Geert

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

CI Applied
#5 lore raw
In reply to Conor Dooley
This patch was applied to riscv/linux.git (fixes) by Palmer Dabbelt <[email protected]>:

Hello:

This patch was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <[email protected]>:

Quoted 11 lines · On Thu, 2 Mar 2023 17:41:55 +0000 you wrote:

From: Conor Dooley <[email protected]>

Chris pointed out that some bonehead, *cough* me *cough*, added two mutex_locks() to the SiFive errata patching. The second was meant to have been a mutex_unlock().

Reported-by Chris Hofstaedtler <[email protected]>
Fixes 9493e6f3ce02 ("RISC-V: take text_mutex during alternative patching")
Signed-off-by Conor Dooley <[email protected]>

[...]

Here is the summary with links:
- RISC-V: fix taking the text_mutex twice during sifive errata patching

    https://git.kernel.org/riscv/c/bf89b7ee52af

You are awesome, thank you!

Signature · 9 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv