linux-kernel - Re: [PATCH v5 1/1] MIPS: Fix idle VS timer enqueue

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.21.2503020042100.12637@angie.orcam.me.uk>
Date: Sun, 2 Mar 2025 00:54:23 +0000 (GMT)
From: "Maciej W. Rozycki" <macro@...am.me.uk>
To: Marco Crivellari <marco.crivellari@...e.com>
cc: linux-mips@...r.kernel.org, linux-kernel@...r.kernel.org, 
    Thomas Bogendoerfer <tsbogend@...ha.franken.de>, 
    Frederic Weisbecker <frederic@...nel.org>, 
    Anna-Maria Behnsen <anna-maria@...utronix.de>, 
    Thomas Gleixner <tglx@...utronix.de>, 
    Peter Zijlstra <peterz@...radead.org>, Huacai Chen <chenhuacai@...nel.org>
Subject: Re: [PATCH v5 1/1] MIPS: Fix idle VS timer enqueue

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.

> +	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