linux-kernel - Re: [PATCH] irq: fasteoi handler re-runs on concurrent invoke

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: <001d516c1bb6f0b6d2344f1ae160e796d003c24c.camel@amazon.com>
Date:   Fri, 17 Mar 2023 11:49:28 +0000
From:   "Gowans, James" <jgowans@...zon.com>
To:     "zouyipeng@...wei.com" <zouyipeng@...wei.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>
CC:     "maz@...nel.org" <maz@...nel.org>,
        "Raslan, KarimAllah" <karahmed@...zon.com>,
        "Woodhouse, David" <dwmw@...zon.co.uk>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] irq: fasteoi handler re-runs on concurrent invoke

On Fri, 2023-03-17 at 18:12 +0800, Yipeng Zou wrote:
> It seems that we have the same solution.

That's a good sign! :D

> (I introduced a new flow handler).

I considered this, but IMO a new handler isn't the way to go: we already have a
bit of handler proliferation going on here. As mentioned in my commit message
there this is starting to get closer to handle_edge_eoi_irq, and adding a new
generic handler which is a mix of the two existing seems to just add more
confusion: which one should a driver owner use? I think it'll be great if we can
enhance the existing generic handlers to cater for the various edge cases and
perhaps even merge these generic handlers in future.

What are your thoughts on this approach compared to your proposal?

There is also the "delay the affinity change of LPI until the next interrupt
acknowledge" option described in the previous thread [0]. I also considered that
but seeing as the handle_edge_irq does the approach implemented here of setting
the PENDING flag and then re-running it, it seemed like good prior art to draw
on. Is that option of enabling CONFIG_GENERIC_PENDING_IRQ a viable? IMO the
generic handlers should be resilient to this so I would prefer this fix than
depending on the user to know to set this config option.

JG

[0] https://lore.kernel.org/all/b0f2623b-ec70-d57e-b744-26c62b1ce523@huawei.com/