[PATCH v7 0/2] MIPS: Fix idle VS timer enqueue

[PATCH v7 0/2] MIPS: Fix idle VS timer enqueue

[Marco Crivellari]

This patch aims to fix idle routine while the CPU receive an interrupt,
because __r4k_wait() only checks if TIF_NEED_RESCHED is set before
going to sleep.
The same behavior has been changed in LoongArch [1].

Code (cross) compiled successfully and I manage to test it on a VM
emulating a malta board. I ran QEMU with:

qemu-system-mips64el -M malta -m 2G -kernel vmlinux -serial stdio -drive \
file=rootfs.ext2,format=raw -append "rootwait root=/dev/sda" -cpu 5Kc

rootfs generated using buildroot (malta default configuration).

- [1] https://github.com/chenhuacai/linux/commit/a8aa673ea46c03b3f62992ffa4ffe810ac84f6e3

---
Changes in v7:
 - patch 2 to handle __r4k_wait() moved to .cpuidle.text section
 - removed .noreorder section from __r4k_wait()
 - idle region of 32-bytes
 - removed C wrapper r4k_wait() and renamed __r4k_wait() to r4k_wait()
 - changes discussed in:
    https://lore.kernel.org/lkml/20250315194002.13778-1-marco.crivellari@suse.com/T/#m07f2568d9099101637063bd057a497ceb846be3d

Changes in v6:
 - typo in comment, capitalise sentences and full stops

Changes in v5:
 - comment: idle interrupt region, instead of rollback region

Changes in v4:
 - comments: 36 byte region

Changes in v3:
 - changed "daddiu k0, 1" with PTR_ADDIU k0, 5
 - replaced CONFIG_CPU_MICROMIPS with 3 _ssnop followed by _ehb
 - integrated the commit message with explanation about
   CONFIG_CPU_MICROMIPS replacement

Changes in v2:
 - Changes introduced by Huacai:
    https://lore.kernel.org/linux-mips/20250214105047.150835-1-marco.crivellari@suse.com/T/#m75d9c587829e15e0d7baec13078be4e65c936408

Marco Crivellari (2):
  MIPS: Fix idle VS timer enqueue
  MIPS: Move __r4k_wait() to .cpuidle.text section

 arch/mips/include/asm/idle.h |  3 +-
 arch/mips/kernel/genex.S     | 63 +++++++++++++++++++++---------------
 arch/mips/kernel/idle.c      |  7 ----
 3 files changed, 38 insertions(+), 35 deletions(-)

-- 
2.49.0

[PATCH v7 1/2] MIPS: Fix idle VS timer enqueue

[Marco Crivellari]

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.

CONFIG_CPU_MICROMIPS has been removed along with the nop instructions.
There, NOPs are 2 byte in size, so change the code with 3 _ssnop which are
always 4 byte and remove the ifdef. Added ehb to make sure the hazard
is always cleared.

Fixes: c65a5480ff29 ("[MIPS] Fix potential latency problem due to non-atomic cpu_wait.")
Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Acked-by: Frederic Weisbecker <frederic@kernel.org>
---
 arch/mips/include/asm/idle.h |  3 +-
 arch/mips/kernel/genex.S     | 62 +++++++++++++++++++++---------------
 arch/mips/kernel/idle.c      |  7 ----
 3 files changed, 37 insertions(+), 35 deletions(-)

diff --git a/arch/mips/include/asm/idle.h b/arch/mips/include/asm/idle.h
index 0992cad9c632..2bc3678455ed 100644
--- a/arch/mips/include/asm/idle.h
+++ b/arch/mips/include/asm/idle.h
@@ -6,8 +6,7 @@
 #include <linux/linkage.h>
 
 extern void (*cpu_wait)(void);
-extern void r4k_wait(void);
-extern asmlinkage void __r4k_wait(void);
+extern asmlinkage void r4k_wait(void);
 extern void r4k_wait_irqoff(void);
 
 static inline int using_rollback_handler(void)
diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S
index a572ce36a24f..46d975d00298 100644
--- a/arch/mips/kernel/genex.S
+++ b/arch/mips/kernel/genex.S
@@ -104,42 +104,52 @@ handle_vcei:
 
 	__FINIT
 
-	.align	5	/* 32 byte rollback region */
-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
-#ifdef CONFIG_CPU_MICROMIPS
-	nop
-	nop
-	nop
-	nop
-#endif
+	/* Align to 32 bytes for the maximum idle interrupt region size. */
+	.align	5
+LEAF(r4k_wait)
+	/* Keep the ISA bit clear for calculations on local labels here. */
+0:	.fill 	0
+	/* Start of idle interrupt region. */
+	local_irq_enable
+	/*
+	 * If an interrupt lands here, before 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.
+	 */
+1:	.fill	0
+	/* The R2 EI/EHB sequence takes 8 bytes, otherwise pad up.  */
+	.if		1b - 0b > 32
+	.error	"overlong idle interrupt region"
+	.elseif	1b - 0b > 8
+	.align	4
+	.endif
+2:	.fill	0
+	.equ	r4k_wait_idle_size, 2b - 0b
+	/* End of idle interrupt region; size has to be a power of 2. */
 	.set	MIPS_ISA_ARCH_LEVEL_RAW
+r4k_wait_insn:
 	wait
-	/* end of rollback region (the region size must be power of two) */
-1:
+r4k_wait_exit:
+	.set	mips0
+	local_irq_disable
 	jr	ra
-	 nop
-	.set	pop
-	END(__r4k_wait)
+	END(r4k_wait)
+	.previous
 
 	.macro	BUILD_ROLLBACK_PROLOGUE handler
 	FEXPORT(rollback_\handler)
 	.set	push
 	.set	noat
 	MFC0	k0, CP0_EPC
-	PTR_LA	k1, __r4k_wait
-	ori	k0, 0x1f	/* 32 byte rollback region */
-	xori	k0, 0x1f
+	/* Subtract/add 2 to let the ISA bit propagate through the mask.  */
+	PTR_LA	k1, r4k_wait_insn - 2
+	ori 	k0, r4k_wait_idle_size - 2
+	.set	noreorder
 	bne	k0, k1, \handler
+	PTR_ADDIU 	k0, r4k_wait_exit - r4k_wait_insn + 2
+	.set	reorder
 	MTC0	k0, CP0_EPC
 	.set pop
 	.endm
diff --git a/arch/mips/kernel/idle.c b/arch/mips/kernel/idle.c
index 5abc8b7340f8..80e8a04a642e 100644
--- a/arch/mips/kernel/idle.c
+++ b/arch/mips/kernel/idle.c
@@ -35,13 +35,6 @@ static void __cpuidle r3081_wait(void)
 	write_c0_conf(cfg | R30XX_CONF_HALT);
 }
 
-void __cpuidle r4k_wait(void)
-{
-	raw_local_irq_enable();
-	__r4k_wait();
-	raw_local_irq_disable();
-}
-
 /*
  * This variant is preferable as it allows testing need_resched and going to
  * sleep depending on the outcome atomically.  Unfortunately the "It is
-- 
2.49.0

[PATCH v7 2/2] MIPS: Move r4k_wait() to .cpuidle.text section

[Marco Crivellari]

Fix missing .cpuidle.text section assignment for r4k_wait() to correct
backtracing with nmi_backtrace().

Fixes: 97c8580e85cf ("MIPS: Annotate cpu_wait implementations with __cpuidle")
Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
Acked-by: Frederic Weisbecker <frederic@kernel.org>
---
 arch/mips/kernel/genex.S | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S
index 46d975d00298..2cf312d9a3b0 100644
--- a/arch/mips/kernel/genex.S
+++ b/arch/mips/kernel/genex.S
@@ -104,6 +104,7 @@ handle_vcei:
 
 	__FINIT
 
+	.section .cpuidle.text,"ax"
 	/* Align to 32 bytes for the maximum idle interrupt region size. */
 	.align	5
 LEAF(r4k_wait)
-- 
2.49.0

Re: [PATCH v7 1/2] MIPS: Fix idle VS timer enqueue

[Marco Crivellari]

Gentle Ping, also for "MIPS: rename rollback_handler 
with skipover_handler".

Thanks in advance.

Re: [PATCH v7 1/2] MIPS: Fix idle VS timer enqueue

[Marco Crivellari]

Gentle Ping.

Thanks.

Re: [PATCH v7 0/2] MIPS: Fix idle VS timer enqueue

[Thomas Bogendoerfer]

On Thu, Apr 03, 2025 at 06:11:41PM +0200, Marco Crivellari wrote:
> This patch aims to fix idle routine while the CPU receive an interrupt,
> because __r4k_wait() only checks if TIF_NEED_RESCHED is set before
> going to sleep.
> The same behavior has been changed in LoongArch [1].
> 
> Code (cross) compiled successfully and I manage to test it on a VM
> emulating a malta board. I ran QEMU with:
> 
> qemu-system-mips64el -M malta -m 2G -kernel vmlinux -serial stdio -drive \
> file=rootfs.ext2,format=raw -append "rootwait root=/dev/sda" -cpu 5Kc
> 
> rootfs generated using buildroot (malta default configuration).
> 
> - [1] https://github.com/chenhuacai/linux/commit/a8aa673ea46c03b3f62992ffa4ffe810ac84f6e3
> 
> ---
> Changes in v7:
>  - patch 2 to handle __r4k_wait() moved to .cpuidle.text section
>  - removed .noreorder section from __r4k_wait()
>  - idle region of 32-bytes
>  - removed C wrapper r4k_wait() and renamed __r4k_wait() to r4k_wait()
>  - changes discussed in:
>     https://lore.kernel.org/lkml/20250315194002.13778-1-marco.crivellari@suse.com/T/#m07f2568d9099101637063bd057a497ceb846be3d
> 
> Changes in v6:
>  - typo in comment, capitalise sentences and full stops
> 
> Changes in v5:
>  - comment: idle interrupt region, instead of rollback region
> 
> Changes in v4:
>  - comments: 36 byte region
> 
> Changes in v3:
>  - changed "daddiu k0, 1" with PTR_ADDIU k0, 5
>  - replaced CONFIG_CPU_MICROMIPS with 3 _ssnop followed by _ehb
>  - integrated the commit message with explanation about
>    CONFIG_CPU_MICROMIPS replacement
> 
> Changes in v2:
>  - Changes introduced by Huacai:
>     https://lore.kernel.org/linux-mips/20250214105047.150835-1-marco.crivellari@suse.com/T/#m75d9c587829e15e0d7baec13078be4e65c936408
> 
> Marco Crivellari (2):
>   MIPS: Fix idle VS timer enqueue
>   MIPS: Move __r4k_wait() to .cpuidle.text section
> 
>  arch/mips/include/asm/idle.h |  3 +-
>  arch/mips/kernel/genex.S     | 63 +++++++++++++++++++++---------------
>  arch/mips/kernel/idle.c      |  7 ----
>  3 files changed, 38 insertions(+), 35 deletions(-)

series applied to mips-fixes.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH v7 1/2] MIPS: Fix idle VS timer enqueue

[Huacai Chen]

Hi, Marco and Thomas,

On Fri, Apr 4, 2025 at 12:11 AM Marco Crivellari
<marco.crivellari@suse.com> wrote:
>
> 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.
>
> CONFIG_CPU_MICROMIPS has been removed along with the nop instructions.
> There, NOPs are 2 byte in size, so change the code with 3 _ssnop which are
> always 4 byte and remove the ifdef. Added ehb to make sure the hazard
> is always cleared.
>
> Fixes: c65a5480ff29 ("[MIPS] Fix potential latency problem due to non-atomic cpu_wait.")
> Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Acked-by: Frederic Weisbecker <frederic@kernel.org>
> ---
>  arch/mips/include/asm/idle.h |  3 +-
>  arch/mips/kernel/genex.S     | 62 +++++++++++++++++++++---------------
>  arch/mips/kernel/idle.c      |  7 ----
>  3 files changed, 37 insertions(+), 35 deletions(-)
>
> diff --git a/arch/mips/include/asm/idle.h b/arch/mips/include/asm/idle.h
> index 0992cad9c632..2bc3678455ed 100644
> --- a/arch/mips/include/asm/idle.h
> +++ b/arch/mips/include/asm/idle.h
> @@ -6,8 +6,7 @@
>  #include <linux/linkage.h>
>
>  extern void (*cpu_wait)(void);
> -extern void r4k_wait(void);
> -extern asmlinkage void __r4k_wait(void);
> +extern asmlinkage void r4k_wait(void);
>  extern void r4k_wait_irqoff(void);
>
>  static inline int using_rollback_handler(void)
> diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S
> index a572ce36a24f..46d975d00298 100644
> --- a/arch/mips/kernel/genex.S
> +++ b/arch/mips/kernel/genex.S
> @@ -104,42 +104,52 @@ handle_vcei:
>
>         __FINIT
>
> -       .align  5       /* 32 byte rollback region */
> -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
> -#ifdef CONFIG_CPU_MICROMIPS
> -       nop
> -       nop
> -       nop
> -       nop
> -#endif
> +       /* Align to 32 bytes for the maximum idle interrupt region size. */
> +       .align  5
> +LEAF(r4k_wait)
> +       /* Keep the ISA bit clear for calculations on local labels here. */
> +0:     .fill   0
> +       /* Start of idle interrupt region. */
> +       local_irq_enable
> +       /*
> +        * If an interrupt lands here, before 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.
> +        */
> +1:     .fill   0
> +       /* The R2 EI/EHB sequence takes 8 bytes, otherwise pad up.  */
> +       .if             1b - 0b > 32
> +       .error  "overlong idle interrupt region"
> +       .elseif 1b - 0b > 8
> +       .align  4
> +       .endif
> +2:     .fill   0
> +       .equ    r4k_wait_idle_size, 2b - 0b
> +       /* End of idle interrupt region; size has to be a power of 2. */
>         .set    MIPS_ISA_ARCH_LEVEL_RAW
> +r4k_wait_insn:
>         wait
> -       /* end of rollback region (the region size must be power of two) */
> -1:
> +r4k_wait_exit:
> +       .set    mips0
> +       local_irq_disable
>         jr      ra
> -        nop
> -       .set    pop
> -       END(__r4k_wait)
> +       END(r4k_wait)
> +       .previous
I'm very sorry for the late response, but I think ".previous" should
be moved to the second patch.

Huacai

>
>         .macro  BUILD_ROLLBACK_PROLOGUE handler
>         FEXPORT(rollback_\handler)
>         .set    push
>         .set    noat
>         MFC0    k0, CP0_EPC
> -       PTR_LA  k1, __r4k_wait
> -       ori     k0, 0x1f        /* 32 byte rollback region */
> -       xori    k0, 0x1f
> +       /* Subtract/add 2 to let the ISA bit propagate through the mask.  */
> +       PTR_LA  k1, r4k_wait_insn - 2
> +       ori     k0, r4k_wait_idle_size - 2
> +       .set    noreorder
>         bne     k0, k1, \handler
> +       PTR_ADDIU       k0, r4k_wait_exit - r4k_wait_insn + 2
> +       .set    reorder
>         MTC0    k0, CP0_EPC
>         .set pop
>         .endm
> diff --git a/arch/mips/kernel/idle.c b/arch/mips/kernel/idle.c
> index 5abc8b7340f8..80e8a04a642e 100644
> --- a/arch/mips/kernel/idle.c
> +++ b/arch/mips/kernel/idle.c
> @@ -35,13 +35,6 @@ static void __cpuidle r3081_wait(void)
>         write_c0_conf(cfg | R30XX_CONF_HALT);
>  }
>
> -void __cpuidle r4k_wait(void)
> -{
> -       raw_local_irq_enable();
> -       __r4k_wait();
> -       raw_local_irq_disable();
> -}
> -
>  /*
>   * This variant is preferable as it allows testing need_resched and going to
>   * sleep depending on the outcome atomically.  Unfortunately the "It is
> --
> 2.49.0
>

Re: [PATCH v7 1/2] MIPS: Fix idle VS timer enqueue

[Maciej W. Rozycki]

On Sun, 27 Apr 2025, Huacai Chen wrote:

> > +r4k_wait_exit:
> > +       .set    mips0
> > +       local_irq_disable
> >         jr      ra
> > -        nop
> > -       .set    pop
> > -       END(__r4k_wait)
> > +       END(r4k_wait)
> > +       .previous
> I'm very sorry for the late response, but I think ".previous" should
> be moved to the second patch.

 Indeed; does it even assemble?  Correctness aside I'd rather it didn't 
cause someone a problem with bisecting sometime.  NB I had no opportunity 
either to look at this version earlier.

  Maciej

Re: [PATCH v7 2/2] MIPS: Move r4k_wait() to .cpuidle.text section

[Huacai Chen]

Hi, Marco,

On Fri, Apr 4, 2025 at 12:12 AM Marco Crivellari
<marco.crivellari@suse.com> wrote:
>
> Fix missing .cpuidle.text section assignment for r4k_wait() to correct
> backtracing with nmi_backtrace().
>
> Fixes: 97c8580e85cf ("MIPS: Annotate cpu_wait implementations with __cpuidle")
> Signed-off-by: Marco Crivellari <marco.crivellari@suse.com>
> Signed-off-by: Maciej W. Rozycki <macro@orcam.me.uk>
> Acked-by: Frederic Weisbecker <frederic@kernel.org>
> ---
>  arch/mips/kernel/genex.S | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S
> index 46d975d00298..2cf312d9a3b0 100644
> --- a/arch/mips/kernel/genex.S
> +++ b/arch/mips/kernel/genex.S
> @@ -104,6 +104,7 @@ handle_vcei:
>
>         __FINIT
>
> +       .section .cpuidle.text,"ax"
If you submit a new version, adding a space before "ax" will be a little better.

Huacai

>         /* Align to 32 bytes for the maximum idle interrupt region size. */
>         .align  5
>  LEAF(r4k_wait)
> --
> 2.49.0
>

Re: [PATCH v7 2/2] MIPS: Move r4k_wait() to .cpuidle.text section

[Maciej W. Rozycki]

On Mon, 28 Apr 2025, Huacai Chen wrote:

> > diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S
> > index 46d975d00298..2cf312d9a3b0 100644
> > --- a/arch/mips/kernel/genex.S
> > +++ b/arch/mips/kernel/genex.S
> > @@ -104,6 +104,7 @@ handle_vcei:
> >
> >         __FINIT
> >
> > +       .section .cpuidle.text,"ax"
> If you submit a new version, adding a space before "ax" will be a little better.

 We use no space across our port though, which is why I haven't requested 
that before.

  Maciej

Re: [PATCH v7 2/2] MIPS: Move r4k_wait() to .cpuidle.text section

[Huacai Chen]

On Mon, Apr 28, 2025 at 11:28 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Mon, 28 Apr 2025, Huacai Chen wrote:
>
> > > diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S
> > > index 46d975d00298..2cf312d9a3b0 100644
> > > --- a/arch/mips/kernel/genex.S
> > > +++ b/arch/mips/kernel/genex.S
> > > @@ -104,6 +104,7 @@ handle_vcei:
> > >
> > >         __FINIT
> > >
> > > +       .section .cpuidle.text,"ax"
> > If you submit a new version, adding a space before "ax" will be a little better.
>
>  We use no space across our port though, which is why I haven't requested
> that before.

Current status:
arch/mips/include/asm/ftrace.h:         ".section .fixup, \"ax\"\n"
         \
arch/mips/include/asm/ftrace.h:         ".section .fixup, \"ax\"\n"     \
arch/mips/include/asm/futex.h:          "       .section .fixup,\"ax\"
                 \n"     \
arch/mips/include/asm/futex.h:          "       .section .fixup,\"ax\"
                 \n"     \
arch/mips/include/asm/futex.h:          "       .section .fixup,\"ax\"
                         \n"
arch/mips/include/asm/futex.h:          "       .section .fixup,\"ax\"
                         \n"
arch/mips/include/asm/paccess.h:        ".section\t.fixup,\"ax\"\n"
                                 \
arch/mips/include/asm/paccess.h:        ".section\t.fixup,\"ax\"\n"
                                 \

So there are a few files which have spaces.

Huacai


>
>   Maciej

Re: [PATCH v7 1/2] MIPS: Fix idle VS timer enqueue

[Marco Crivellari]

Hi everyone,

Sorry, I have tested the code only with both the patches applied
(when I also ran the tests with qemu) and I didn't notice my mistake.

I will submit a new version with the correction and the changes
suggested in the other patch, when we find the proper way to do it.

Thank you.


On Mon, Apr 28, 2025 at 3:32 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
>
> On Sun, 27 Apr 2025, Huacai Chen wrote:
>
> > > +r4k_wait_exit:
> > > +       .set    mips0
> > > +       local_irq_disable
> > >         jr      ra
> > > -        nop
> > > -       .set    pop
> > > -       END(__r4k_wait)
> > > +       END(r4k_wait)
> > > +       .previous
> > I'm very sorry for the late response, but I think ".previous" should
> > be moved to the second patch.
>
>  Indeed; does it even assemble?  Correctness aside I'd rather it didn't
> cause someone a problem with bisecting sometime.  NB I had no opportunity
> either to look at this version earlier.
>
>   Maciej



-- 

Marco Crivellari

L3 Support Engineer, Technology & Product




marco.crivellari@suse.com

Re: [PATCH v7 2/2] MIPS: Move r4k_wait() to .cpuidle.text section

[Marco Crivellari]

Hi,

If it sounds good also to Maciej, I will submit the new version with the
space before "ax" (and of course, the ".previous").

Thank you.

On Mon, Apr 28, 2025 at 6:11 AM Huacai Chen <chenhuacai@kernel.org> wrote:
>
> On Mon, Apr 28, 2025 at 11:28 AM Maciej W. Rozycki <macro@orcam.me.uk> wrote:
> >
> > On Mon, 28 Apr 2025, Huacai Chen wrote:
> >
> > > > diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S
> > > > index 46d975d00298..2cf312d9a3b0 100644
> > > > --- a/arch/mips/kernel/genex.S
> > > > +++ b/arch/mips/kernel/genex.S
> > > > @@ -104,6 +104,7 @@ handle_vcei:
> > > >
> > > >         __FINIT
> > > >
> > > > +       .section .cpuidle.text,"ax"
> > > If you submit a new version, adding a space before "ax" will be a little better.
> >
> >  We use no space across our port though, which is why I haven't requested
> > that before.
>
> Current status:
> arch/mips/include/asm/ftrace.h:         ".section .fixup, \"ax\"\n"
>          \
> arch/mips/include/asm/ftrace.h:         ".section .fixup, \"ax\"\n"     \
> arch/mips/include/asm/futex.h:          "       .section .fixup,\"ax\"
>                  \n"     \
> arch/mips/include/asm/futex.h:          "       .section .fixup,\"ax\"
>                  \n"     \
> arch/mips/include/asm/futex.h:          "       .section .fixup,\"ax\"
>                          \n"
> arch/mips/include/asm/futex.h:          "       .section .fixup,\"ax\"
>                          \n"
> arch/mips/include/asm/paccess.h:        ".section\t.fixup,\"ax\"\n"
>                                  \
> arch/mips/include/asm/paccess.h:        ".section\t.fixup,\"ax\"\n"
>                                  \
>
> So there are a few files which have spaces.
>
> Huacai
>
>
> >
> >   Maciej



-- 

Marco Crivellari

L3 Support Engineer, Technology & Product




marco.crivellari@suse.com

Re: [PATCH v7 2/2] MIPS: Move r4k_wait() to .cpuidle.text section

[Thomas Bogendoerfer]

On Mon, Apr 28, 2025 at 12:20:31PM +0200, Marco Crivellari wrote:
> Hi,
> 
> If it sounds good also to Maciej, I will submit the new version with the
> space before "ax" (and of course, the ".previous").

save your time, this is already applied and I won't rebase the branch
just because of this minor bisection problem.

Thomas.

-- 
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea.                                                [ RFC1925, 2.3 ]

Re: [PATCH v7 2/2] MIPS: Move r4k_wait() to .cpuidle.text section

[Maciej W. Rozycki]

On Mon, 28 Apr 2025, Thomas Bogendoerfer wrote:

> > If it sounds good also to Maciej, I will submit the new version with the
> > space before "ax" (and of course, the ".previous").
> 
> save your time, this is already applied and I won't rebase the branch
> just because of this minor bisection problem.

 For the record we actually have 121 cases without a space vs 4 with one, 
so formatting is a non-issue here in my view.

  Maciej

Re: [PATCH v7 2/2] MIPS: Move r4k_wait() to .cpuidle.text section

[Huacai Chen]

On Mon, Apr 28, 2025 at 6:24 PM Thomas Bogendoerfer
<tsbogend@alpha.franken.de> wrote:
>
> On Mon, Apr 28, 2025 at 12:20:31PM +0200, Marco Crivellari wrote:
> > Hi,
> >
> > If it sounds good also to Maciej, I will submit the new version with the
> > space before "ax" (and of course, the ".previous").
>
> save your time, this is already applied and I won't rebase the branch
> just because of this minor bisection problem.
But this series lacks a tested-by from Jiaxun Yang. The first patch is
so complicated because it should handle the pre MIPS III case, I have
no old MIPS hardware, Jiaxun said he has [1] but he hasn't tested it
by now.

[1] https://lore.kernel.org/linux-mips/20250315194002.13778-1-marco.crivellari@suse.com/T/#m632ac91077d4cbc750b8acaacef08cad8bfd29da

Huacai

>
> Thomas.
>
> --
> Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
> good idea.                                                [ RFC1925, 2.3 ]