Hi, Maciej, On Sun, Mar 2, 2025 at 8:54 AM Maciej W. Rozycki <macro@xxxxxxxxxxx> wrote: > > On Fri, 28 Feb 2025, Marco Crivellari wrote: > > > diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S > > index a572ce36a24f..474738d9124e 100644 > > --- a/arch/mips/kernel/genex.S > > +++ b/arch/mips/kernel/genex.S > > @@ -104,27 +104,30 @@ 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 > > -#ifdef CONFIG_CPU_MICROMIPS > > - nop > > - nop > > - nop > > - nop > > -#endif > > + /* start of idle interrupt region */ > > + MFC0 t0, CP0_STATUS > > + /* Enable Interrput */ > ^^ > Typo here; also please capitalise sentences and use full stops. > > > + ori t0, 0x1f > > No time for a thorough review here as I'm heading for a holiday right > away, but I can see you still have a coprocessor move hazard here with > MIPS III hardware. The incoming value of $t0 to ORI is not what MFC0 has > obtained. It's the value from before MFC0. If this is a problem, then the current local_irq_enable() is also wrong for all MIPS III hardware, because this patch uses the same instruction sequence of local_irq_enable(). Huacai > > > + xori t0, 0x1e > > And then it's only this XORI that sees the output from MFC0 (though > there's actually a race between the two competing writes to $t0), so > effectively you clobber the CP0.Status register... > > > + MTC0 t0, CP0_STATUS > > ... here. You need to schedule your instructions differently. But do > you need `.set noreorder' in the first place? Can you maybe find a way to > avoid it, making all the hazard avoidance stuff much easier? > > Maciej
