All of lore.kernel.org
 help / color / mirror / Atom feed
From: Marco Crivellari <marco.crivellari@suse.com>
To: linux-mips@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
	Marco Crivellari <marco.crivellari@suse.com>,
	Frederic Weisbecker <frederic@kernel.org>
Subject: [PATCH] MIPS: Fix idle VS timer enqueue
Date: Fri, 14 Feb 2025 11:50:47 +0100	[thread overview]
Message-ID: <20250214105047.150835-2-marco.crivellari@suse.com> (raw)
In-Reply-To: <20250214105047.150835-1-marco.crivellari@suse.com>

MIPS re-enables interrupts on its idle routine and performs
a TIF_NEED_RESCHED check afterwards before putting the CPU to sleep.

The IRQs firing between the check and the 'wait' instruction may set the
TIF_NEED_RESCHED flag. In order to deal with this possible race, IRQs
interrupting __r4k_wait() rollback their return address to the
beginning of __r4k_wait() so that TIF_NEED_RESCHED is checked
again before going back to sleep.

However idle IRQs can also queue timers that may require a tick
reprogramming through a new generic idle loop iteration but those timers
would go unnoticed here because __r4k_wait() only checks
TIF_NEED_RESCHED. It doesn't check for pending timers.

Fix this with fast-forwarding idle IRQs return address to the end of the
idle routine instead of the beginning, so that the generic idle loop
handles both TIF_NEED_RESCHED and pending timers.

Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
---
 arch/mips/kernel/genex.S | 36 ++++++++++++++++++++----------------
 arch/mips/kernel/idle.c  |  1 -
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S
index a572ce36a24f..a78d5132c940 100644
--- a/arch/mips/kernel/genex.S
+++ b/arch/mips/kernel/genex.S
@@ -104,18 +104,16 @@ handle_vcei:
 
 	__FINIT
 
-	.align	5	/* 32 byte rollback region */
+	.align	5
 LEAF(__r4k_wait)
 	.set	push
 	.set	noreorder
-	/* start of rollback region */
-	LONG_L	t0, TI_FLAGS($28)
-	nop
-	andi	t0, _TIF_NEED_RESCHED
-	bnez	t0, 1f
-	 nop
-	nop
-	nop
+	/* start of idle interrupt region */
+	MFC0	k0, CP0_STATUS
+	/* Enable Interrupt */
+	ori 	k0, 0x1f
+	xori	k0, 0x1e
+	MTC0	k0, CP0_STATUS
 #ifdef CONFIG_CPU_MICROMIPS
 	nop
 	nop
@@ -123,11 +121,17 @@ LEAF(__r4k_wait)
 	nop
 #endif
 	.set	MIPS_ISA_ARCH_LEVEL_RAW
+	/*
+	 * If an interrupt lands here, between enabling interrupts above and
+	 * going idle on the next instruction, we must *NOT* go idle since the
+	 * interrupt could have set TIF_NEED_RESCHED or caused a timer to need
+	 * resched. Fall through -- see rollback_handler below -- and have
+	 * the idle loop take care of things.
+	 */
 	wait
-	/* end of rollback region (the region size must be power of two) */
-1:
+	/* end of idle interrupt region (the region size must be power of two) */
+SYM_INNER_LABEL(__r4k_wait_exit, SYM_L_LOCAL)
 	jr	ra
-	 nop
 	.set	pop
 	END(__r4k_wait)
 
@@ -136,10 +140,10 @@ LEAF(__r4k_wait)
 	.set	push
 	.set	noat
 	MFC0	k0, CP0_EPC
-	PTR_LA	k1, __r4k_wait
-	ori	k0, 0x1f	/* 32 byte rollback region */
-	xori	k0, 0x1f
-	bne	k0, k1, \handler
+	PTR_LA	k1, __r4k_wait_exit
+	/* 3 instructions rollback region */
+	ori 	k0, k0, 0x0c
+	bne 	k0, k1, \handler
 	MTC0	k0, CP0_EPC
 	.set pop
 	.endm
diff --git a/arch/mips/kernel/idle.c b/arch/mips/kernel/idle.c
index 5abc8b7340f8..1f74c0589f7e 100644
--- a/arch/mips/kernel/idle.c
+++ b/arch/mips/kernel/idle.c
@@ -37,7 +37,6 @@ static void __cpuidle r3081_wait(void)
 
 void __cpuidle r4k_wait(void)
 {
-	raw_local_irq_enable();
 	__r4k_wait();
 	raw_local_irq_disable();
 }
-- 
2.48.1


  reply	other threads:[~2025-02-14 10:51 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-14 10:50 [PATCH 0/1] MIPS: Fix idle VS timer enqueue Marco Crivellari
2025-02-14 10:50 ` Marco Crivellari [this message]
2025-02-14 12:32   ` [PATCH] " Frederic Weisbecker
2025-02-14 14:46   ` Huacai Chen
2025-02-14 15:57     ` Marco Crivellari
2025-02-15 14:41       ` Huacai Chen
2025-02-15  5:05   ` Maciej W. Rozycki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250214105047.150835-2-marco.crivellari@suse.com \
    --to=marco.crivellari@suse.com \
    --cc=frederic@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mips@vger.kernel.org \
    --cc=tsbogend@alpha.franken.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.