diff mbox series

riscv: Avoid interrupts being erroneously enabled in handle_exception()

Message ID 1568623661-16779-1-git-send-email-vincent.chen@sifive.com (mailing list archive)
State New, archived
Headers show
Series riscv: Avoid interrupts being erroneously enabled in handle_exception() | expand

Commit Message

Vincent Chen Sept. 16, 2019, 8:47 a.m. UTC 
When the handle_exception function addresses an exception, the interrupts
will be unconditionally enabled after finishing the context save. However,
It may erroneously enable the interrupts if the interrupts are disabled
before entering the handle_exception.

For example, one of the WARN_ON() condition is satisfied in the scheduling
where the interrupt is disabled and rq.lock is locked. The WARN_ON will
trigger a break exception and the handle_exception function will enable the
interrupts before entering do_trap_break function. During the procedure, if
a timer interrupt is pending, it will be taken when interrupts are enabled.
In this case, it may cause a deadlock problem if the rq.lock is locked
again in the timer ISR.

Hence, the handle_exception() can only enable interrupts when the state of
sstatus.SPIE is 1.

This patch is tested on HiFive Unleashed board.

Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
Reviewed-by: Palmer Dabbelt <palmer@sifive.com>

---
 arch/riscv/kernel/entry.S | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Paul Walmsley Sept. 20, 2019, 3:41 p.m. UTC | #1 
On Mon, 16 Sep 2019, Vincent Chen wrote:

> When the handle_exception function addresses an exception, the interrupts
> will be unconditionally enabled after finishing the context save. However,
> It may erroneously enable the interrupts if the interrupts are disabled
> before entering the handle_exception.
> 
> For example, one of the WARN_ON() condition is satisfied in the scheduling
> where the interrupt is disabled and rq.lock is locked. The WARN_ON will
> trigger a break exception and the handle_exception function will enable the
> interrupts before entering do_trap_break function. During the procedure, if
> a timer interrupt is pending, it will be taken when interrupts are enabled.
> In this case, it may cause a deadlock problem if the rq.lock is locked
> again in the timer ISR.
> 
> Hence, the handle_exception() can only enable interrupts when the state of
> sstatus.SPIE is 1.
> 
> This patch is tested on HiFive Unleashed board.
> 
> Signed-off-by: Vincent Chen <vincent.chen@sifive.com>
> Reviewed-by: Palmer Dabbelt <palmer@sifive.com>

Thanks very much.  Queued for v5.4-rc.


- Paul
diff mbox series

Patch

diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index bc7a56e1ca6f..80444f1e57bb 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -166,9 +166,12 @@  ENTRY(handle_exception)
 	move a0, sp /* pt_regs */
 	tail do_IRQ
 1:
-	/* Exceptions run with interrupts enabled */
+	/* Exceptions run with interrupts enabled or disabled
+	   depending on the state of sstatus.SR_SPIE */
+	andi t0, s1, SR_SPIE
+	beqz t0, 1f
 	csrs sstatus, SR_SIE
-
+1:
 	/* Handle syscalls */
 	li t0, EXC_SYSCALL
 	beq s4, t0, handle_syscall