linux-kernel - Re: [v4.1.10-rt10][PATCH 1/2] genirq: introduce new generic_handle_irq_rt

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: <alpine.DEB.2.11.1511032014580.4032@nanos>
Date:	Tue, 3 Nov 2015 20:51:03 +0100 (CET)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Grygorii Strashko <grygorii.strashko@...com>
cc:	bigeasy@...utronix.de, linux-rt-users@...r.kernel.org,
	linux-kernel@...r.kernel.org, Sekhar Nori <nsekhar@...com>
Subject: Re: [v4.1.10-rt10][PATCH 1/2] genirq: introduce new generic_handle_irq_rt_wa()
 api

On Tue, 3 Nov 2015, Grygorii Strashko wrote:
> On 11/02/2015 09:38 PM, Thomas Gleixner wrote:
> > 
> > Why aren't you simply marking these demultiplex handlers with IRQ_NO_THREAD?
> > 
> In general, it's possible. But, in this case, worst scenario will look like:

> dra7xx_pcie_msi_irq_handler()
> -> dw_handle_msi_irq()
>    [code simplified]
>    -> for (i = 0; i < MAX_MSI_IRQS; i++) {
> 	...
> 	generic_handle_irq(Y(i));
> 	...
>    }
> where MAX_MSI_IRQS = 32 now, but potentially can be increased up to 256.

And you really oversimplified the code above. The reality is:

    for (i = 0; i < MAX_MSI_CTRLS: i++) {
    	u32 status = read_msi_ctrl(i);

	for_each_bit(status)
		handle_irq();
    }

So sure, the worst case here is MAX_MSI_CTRLS * 32, but if all
possible 256 MSI interrupts are pending at the same time, you have
other problems than that.

In the current configuration (32 interrupts), which cannot change
because it's hardwired in silicon, this is a single status read and
assuming that only a few (most of the time it will be exactly ONE) of
those interrupts are pending at the same time is pretty much a sane
assumption. If it wouldn't be then all users of chained interrupt
handlers which usually demultiplex 32 interrupts would suffer from
that problem already.

Aside of that, you would prevent that any of these PCIe interrupts can
be utilized as a "fast" non threaded interrupt on RT. And that I would
consider a real bad limitation for no value. 

MSI has been invented to overcome the issues of wired interrupts
(demultiplexing and sharing), so I don't know why the involved
hardware designers came to the conclusion that demultiplexing MSI
interrupts in software is a sane approach. But then I really gave up
trying to understand hardware designers long ago.

The only sane way to deal with that is to actually mark those handlers
NOTRHEAD and document the limitations of your hardware, so your
customers won't trip over it. If they insist on having 32 MSI
producers on that PCIe bus and make them fire all at the same time,
then you still can provide them your "solution".

Just face it, it's a bad hardware design decision and adding a half
baken hackery which actually hurts sane use cases is not making it any
better.

Thanks,

	tglx

--
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/