linux-kernel - Re: [PATCH] MIPS: Enable interrupts in arch_cpu

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: <20130502145809.GA16236@linux-mips.org>
Date:	Thu, 2 May 2013 16:58:09 +0200
From:	Ralf Baechle <ralf@...ux-mips.org>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	Linus Torvalds <torvalds@...ux-foundation.org>,
	Jonas Gorski <jogo@...nwrt.org>, eunb.song@...sung.com,
	"linux-mips@...ux-mips.org" <linux-mips@...ux-mips.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] MIPS: Enable interrupts in arch_cpu_idle()

On Thu, May 02, 2013 at 04:33:52PM +0200, Thomas Gleixner wrote:

> commit cdbedc61c8 (mips: Use generic idle loop) broke MIPS as I did
> not realize that MIPS wants to invoke the wait instructions with
> interrupts enabled. Don't ask why that works correctly; Ralf suggested
> to get thoroughly drunk before even thinking about it. Looking sober
> at commit c65a5480 ([MIPS] Fix potential latency problem due to
> non-atomic cpu_wait) is not recommended.
> 
> Enable interrupts in arch_cpu_idle() on mips to repair the issue.
> 
> Reported-and-tested-by: Jonas Gorski <jogo@...nwrt.org>
> Reported-by: EunBong Song <eunb.song@...sung.com>
> Booze-recommended-by: Ralf Baechle <ralf@...ux-mips.org>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>

A very sobering commit message ;-)

The underlying issue is that the WAIT instruction on MIPS is architecturally
defined as "[...] It is implementation-dependent whether the pipeline
restarts when a non-enabled interrupt is requested.  [...]"

arch_local_irq_disable() disables interrupts by clearing the IE bit in
the CPU status register, thus disabling all interrupts.  If no a WAIT
instruction is executed it's legal for a CPU to stop the pipeline for
good.  Which obviously is pretty stupidtastik behaviour.

For a while we just used to live with the race condition resulting from
not disabling interrupts in the idle loop.  Then c65a5480 fixed this by
checking if we're returning to the WAIT instruction in the idle loop
when returning from an interrupt and iff so, rolling back the
program counter to point to the if (test_thread_flag(TIF_NEED_RESCHED))
test at the beginning of rollback_r4k_wait.

Linux knows which CPUs have the problematic behaviour and uses this special
variant of the idle loop only where needed.

  Ralf
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/