linux-kernel - Re: [PATCH v6 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: <Z-PmTcGvmHgww6KP@alpha.franken.de>
Date: Wed, 26 Mar 2025 12:34:37 +0100
From: Thomas Bogendoerfer <tsbogend@...ha.franken.de>
To: Marco Crivellari <marco.crivellari@...e.com>
Cc: Huacai Chen <chenhuacai@...nel.org>,
	"Maciej W. Rozycki" <macro@...am.me.uk>, linux-mips@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Frederic Weisbecker <frederic@...nel.org>,
	Anna-Maria Behnsen <anna-maria@...utronix.de>,
	Thomas Gleixner <tglx@...utronix.de>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH v6 1/1] MIPS: Fix idle VS timer enqueue

On Wed, Mar 26, 2025 at 10:46:08AM +0100, Marco Crivellari wrote:
> I'm mostly thinking about future changes in this part of the code.
> But if it is ok what we have now, and future changes are not a
> problem, let's keep this version.
> 
> Would this be ok with you @Maciej?
> 
> If so, the region is now 40 bytes. This is what I did yesterday:
> 
> @@ -110,6 +110,7 @@ LEAF(__r4k_wait)
>        .set    noreorder
>        /* Start of idle interrupt region. */
>        MFC0    t0, CP0_STATUS
> +       nop
>        /* Enable interrupt. */
>        ori     t0, 0x1f
>        xori    t0, 0x1e
> @@ -128,7 +129,11 @@ LEAF(__r4k_wait)
>         */
>        wait
>        /* End of idle interrupt region. */
> -1:
> +__r4k_wait_exit:
> +       /* Check idle interrupt region size. */
> +       .if ((__r4k_wait_exit - __r4k_wait) != 40)
> +       .error  "Idle interrupt region size mismatch: expected 40 bytes."
> +       .endif
>        jr      ra
>         nop
>        .set    pop
> @@ -139,10 +144,10 @@ LEAF(__r4k_wait)
>        .set    push
>        .set    noat
>        MFC0    k0, CP0_EPC
> -       PTR_LA  k1, 1b
> -       /* 36 byte idle interrupt region. */
> +       PTR_LA  k1, __r4k_wait_exit
> +       /* 40 byte idle interrupt region. */

IMHO, we can't extend this above 36 bytes, because the interrupt could
hit at the instruction before the wait, which is then in the
next 32byte block.

I was thinking about aligning __r4k_wait 16 byte earlier and place
the wait at the end of the 

Something like:

	.align 5
	.align 4
LEAF(__r4k_wait)
	// irq enable sequence
	wait // align to end of the 32byte block
_r4k_wait_exit:

>        ori     k0, 0x1f
> -       PTR_ADDIU       k0, 5
> +       PTR_ADDIU       k0, 9

this needs to
	PTR_ADDIU       k0, 1

then. and __r4k_wait_exit - __r4k_wait is 48

But there might better ways...

Thomas.

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