linux-kernel - Re: [PATCH 0/5] Fix regression introduced by set_irq

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] [day] [month] [year] [list]

Date:   Fri, 24 Feb 2017 17:56:50 +0100
From:   Thomas Petazzoni <thomas.petazzoni@...e-electrons.com>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Lior Amsalem <alior@...vell.com>, Andrew Lunn <andrew@...n.ch>,
        Jason Cooper <jason@...edaemon.net>,
        Tawfik Bayouk <tawfik@...vell.com>,
        Marc Zyngier <marc.zyngier@....com>,
        linux-kernel@...r.kernel.org, Nadav Haklai <nadavh@...vell.com>,
        Gregory Clement <gregory.clement@...e-electrons.com>,
        linux-arm-kernel@...ts.infradead.org,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>
Subject: Re: [PATCH 0/5] Fix regression introduced by set_irq_flags()
 removal

Thomas,

On Tue, 8 Dec 2015 11:54:11 +0100 (CET), Thomas Gleixner wrote:
> On Tue, 8 Dec 2015, Thomas Petazzoni wrote:
> > When a device driver uses a normal (non per-CPU) interrupt, then it
> > doesn't have to take care of disabling the interrupt on suspend and
> > re-enabling the interrupt on resume at the interrupt controller level.
> > This is all transparently handled by the irqchip driver.
> > 
> > Why should the handling of per-CPU interrupts be different and require
> > explicit handling from each device driver rather than being
> > transparently handled by the irqchip driver ?  
> 
> Fair enough. Did not think about the boot cpu part.

I am reviving this *very* old thread, as I wanted to push the remaining
patches. But it seems like the issue no longer exists.

Summary of the story:

 - Between 4.2 and 4.3-rc, commit d17cab4451df1 broke suspend/resume on
   Armada XP. The issue was that the IRQ_NOAUTOEN was no longer
   cleared, and therefore irqd_irq_disabled() no longer indicated that
   our per-CPU interrupts were enabled, and consequently our local
   timer per-CPU interrupt was not re-enabled upon resume.

 - After some discussion, we merged a very simple workaround for 4.3,
   which consists in clearing IRQ_NOAUTOEN:

    irq_clear_status_flags(virq, IRQ_NOAUTOEN);

   But the idea was to do a better implementation later on, which my
   patch series at the same time was trying.

 - Now, fast forward to 4.10, if I remove the:

    irq_clear_status_flags(virq, IRQ_NOAUTOEN);

   then I can no longer reproduce the original issue from 4.3-rc, which
   clearing IRQ_NOAUTOEN was fixing. My system resumes fine.

Has there been some changes in this area that could explain that
irqd_irq_disable() now indicates that my per-CPU interrupt is enabled,
while it didn't say so back in 4.3-rc ?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com