linux-kernel - Re: [PATCH] genirq / PM: Add flag for shared NO

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: <20150227083859.GD21418@twins.programming.kicks-ass.net>
Date:	Fri, 27 Feb 2015 09:38:59 +0100
From:	Peter Zijlstra <peterz@...radead.org>
To:	"Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:	Boris Brezillon <boris.brezillon@...e-electrons.com>,
	Mark Rutland <mark.rutland@....com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Jason Cooper <jason@...edaemon.net>,
	linux-kernel@...r.kernel.org,
	Nicolas Ferre <nicolas.ferre@...el.com>,
	Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>,
	Alexandre Belloni <alexandre.belloni@...e-electrons.com>,
	linux-arm-kernel@...ts.infradead.org,
	Linux PM list <linux-pm@...r.kernel.org>
Subject: Re: [PATCH] genirq / PM: Add flag for shared NO_SUSPEND interrupt
 lines

On Fri, Feb 27, 2015 at 12:07:55AM +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> 
> It currently is required that all users of NO_SUSPEND interrupt
> lines pass the IRQF_NO_SUSPEND flag when requesting the IRQ or the
> WARN_ON_ONCE() in irq_pm_install_action() will trigger.  That is
> done to warn about situations in which unprepared interrupt handlers
> may be run unnecessarily for suspended devices and may attempt to
> access those devices by mistake.  However, it may cause drivers
> that have no technical reasons for using IRQF_NO_SUSPEND to set
> that flag just because they happen to share the interrupt line
> with something like a timer.
> 
> Moreover, the generic handling of wakeup interrupts introduced by
> commit 9ce7a25849e8 (genirq: Simplify wakeup mechanism) only works
> for IRQs without any NO_SUSPEND users, so the drivers of wakeup
> devices needing to use shared NO_SUSPEND interrupt lines for
> signaling system wakeup generally have to detect wakeup in their
> interrupt handlers.  Thus if they happen to share an interrupt line
> with a NO_SUSPEND user, they also need to request that their
> interrupt handlers be run after suspend_device_irqs().
> 
> In both cases the reason for using IRQF_NO_SUSPEND is not because
> the driver in question has a genuine need to run its interrupt
> handler after suspend_device_irqs(), but because it happens to
> share the line with some other NO_SUSPEND user.  Otherwise, the
> driver would do without IRQF_NO_SUSPEND just fine.
> 
> To make it possible to specify that condition explicitly, introduce
> a new IRQ action handler flag for shared IRQs, IRQF_COND_SUSPEND,
> that, when set, will indicate to the IRQ core that the interrupt
> user is generally fine with suspending the IRQ, but it also can
> tolerate handler invocations after suspend_device_irqs() and, in
> particular, it is capable of detecting system wakeup and triggering
> it as appropriate from its interrupt handler.
> 
> That will allow us to work around a problem with a shared timer
> interrupt line on at91 platforms.
> 
> Link: http://marc.info/?l=linux-kernel&m=142252777602084&w=2
> Link: http://marc.info/?t=142252775300011&r=1&w=2
> Linx: https://lkml.org/lkml/2014/12/15/552
> Reported-by: Boris Brezillon <boris.brezillon@...e-electrons.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>

Seems good to me. Should I take this through tip/irq ?

Also, should we warn if people use enable_irq_wake() where there is only
a single descriptor with NO_SUSPEND?
--
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/