linux-kernel - Re: [PATCH] genirq: Clarify that irq wake state is orthogonal to enable/disable

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: <5e3b279f.1c69fb81.383f9.1da3@mx.google.com>
Date:   Wed, 05 Feb 2020 12:37:51 -0800
From:   Stephen Boyd <swboyd@...omium.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     linux-kernel@...r.kernel.org, Marc Zyngier <maz@...nel.org>,
        Douglas Anderson <dianders@...omium.org>,
        Lina Iyer <ilina@...eaurora.org>,
        Maulik Shah <mkshah@...eaurora.org>
Subject: Re: [PATCH] genirq: Clarify that irq wake state is orthogonal to enable/disable

Quoting Thomas Gleixner (2020-02-05 04:27:06)
> Stephen Boyd <swboyd@...omium.org> writes:
> > There's some confusion around if an irq that's disabled with
> > disable_irq() can still wake the system from sleep states such as
> > "suspend to RAM". Let's clarify this in the kernel documentation for
> > irq_set_irq_wake() so that it's clear that an irq can be disabled and
> > still wake the system if it has been marked for wakeup.
> >
> > Cc: Marc Zyngier <maz@...nel.org>
> > Cc: Douglas Anderson <dianders@...omium.org>
> > Cc: Lina Iyer <ilina@...eaurora.org>
> > Cc: Maulik Shah <mkshah@...eaurora.org>
> > Signed-off-by: Stephen Boyd <swboyd@...omium.org>
> > ---
> >  kernel/irq/manage.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> >
> > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> > index 818b2802d3e7..fa8db98c8699 100644
> > --- a/kernel/irq/manage.c
> > +++ b/kernel/irq/manage.c
> > @@ -731,6 +731,11 @@ static int set_irq_wake_real(unsigned int irq, unsigned int on)
> >   *
> >   *   Wakeup mode lets this IRQ wake the system from sleep
> >   *   states like "suspend to RAM".
> > + *
> > + *   Note: irq enable/disable state is completely orthogonal
> > + *   to the enable/disable state of irq wake. An irq can be
> > + *   disabled with disable_irq() and still wake the system as
> > + *   long as the irq has wake enabled.
> 
> It clearly should say that this is really depending on the hardware
> implementation of the particual interrupt chip whether disabled + wake
> mode is supported.
> 

Ok. I'm having trouble parsing this. Is there a consistent wording that
can be put here?

The API seems fraught with peril if an implementation of an irqchip is
allowed to ignore wakeup on interrupts that are marked for wakeup while
the irq is disabled. Driver writers won't be able to write drivers that
work across implementations if the irq can't wake the system reliably.