linux-kernel - Re: CONFIG_DEBUG

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: <CAAEAJfAPVgxhRnfYJYzebFJDRJ4Ooam6n+0B5HJ-+uLJ0exTLA@mail.gmail.com>
Date:	Wed, 26 Aug 2015 16:53:27 -0300
From:	Ezequiel Garcia <ezequiel@...guardiasur.com.ar>
To:	Felipe Balbi <balbi@...com>
Cc:	Ingo Molnar <mingo@...e.hu>, Tony Lindgren <tony@...mide.com>,
	Linux OMAP Mailing List <linux-omap@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Linux ARM Kernel Mailing List 
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: CONFIG_DEBUG_SHIRQ and PM

On 26 August 2015 at 16:38, Felipe Balbi <balbi@...com> wrote:
> Hi,
>
> On Wed, Aug 26, 2015 at 04:29:52PM -0300, Ezequiel Garcia wrote:
>> Felipe,
>>
>> On 25 August 2015 at 16:58, Felipe Balbi <balbi@...com> wrote:
>> > Hi Ingo,
>> >
>> > I'm facing an issue with CONFIG_DEBUG_SHIRQ and pm_runtime when using
>> > devm_request_*irq().
>> >
>>
>> I may be jumping on the gun here, but I believe here's your problem.
>> Using devm_request_irq with shared IRQs is not a good idea.
>>
>> Or at least, it forces you to handle interrupts after your device
>> is _completely_ removed (e.g. your IRQ cookie could be bogus).
>>
>> AFAICS, the CONFIG_DEBUG_SHIRQ option is just triggering a couple
>> spurious interrupts, that are expected to happen anyway.
>>
>> Your IRQ is shared, and so you may get any IRQ at any time,
>> coming from another device (not yours).
>>
>> So, if I'm right, my suggestion is simple: use request_irq/free_irq
>> and free your IRQ before you disable your clocks, remove your device,
>> etc.
>
> yeah, that's just a workaround though. Specially with IRQ flags coming
> from DT, driver might have no knowledge that its IRQ is shared to start
> with.
>

Really? Is there any way the DT can set the IRQF_SHARED flag?
The caller of devm_request_irq / request_irq needs to pass the irqflags,
so I'd say the driver is perfectly aware of the IRQ being shared or not.

Maybe you can clarify what I'm missing here.

> Besides, removing devm_* is just a workaround to the real problem. It
> seems, to me at least, that drivers shouldn't be calling
> pm_runtime_put_sync(); pm_runtime_disable() from their ->remove(),
> rather the bus driver should be responsible for doing so; much
> usb_bus_type and pci_bus_type do. Of course, this has the side effect of
> requiring buses to make sure that by the time ->probe() is called, that
> device's clocks are stable and running and the device is active.
>
> However, that's not done today for the platform_bus_type and, frankly,
> that would be somewhat of a complex problem to solve, considering that
> different SoCs integrate IPs the way they want.
>
> Personally, I think removing devm_* is but a workaround to the real
> thing we're dealing with.
>

I don't see any problems here: if your interrupt is shared, then you must
be prepared to handle it any time, coming from any sources (not only
your device). And CONFIG_DEBUG_SHIRQ does exactly that, in order to
make sure all the drivers passing IRQF_SHARED comply with that rule.

So you either avoid using devm_request_irq, or you prepare your handler
accordingly to be ready to handle an interrupt _any time_.

-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
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/