linux-kernel - Re: [PATCH v1] irq: Introduce IRQF_COND

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]

Date: Mon, 25 Mar 2024 14:31:45 +0100
From: Linus Walleij <linus.walleij@...aro.org>
To: "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc: Thomas Gleixner <tglx@...utronix.de>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	LKML <linux-kernel@...r.kernel.org>, Hans de Goede <hdegoede@...hat.com>, 
	Linux ACPI <linux-acpi@...r.kernel.org>, 
	Francisco Ayala Le Brun <francisco@...eowindow.eu>, Mario Limonciello <mario.limonciello@....com>, 
	Adrian Hunter <adrian.hunter@...el.com>
Subject: Re: [PATCH v1] irq: Introduce IRQF_COND_ONESHOT and use it in pinctrl-amd

On Mon, Mar 25, 2024 at 1:58 PM Rafael J. Wysocki <rjw@...ysocki.net> wrote:

> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
>
> There is a problem when a driver requests a shared IRQ line to run
> a threaded handler on it without IRQF_ONESHOT set if that flag has
> been set already for the IRQ in question by somebody else.  Namely,
> the request fails which usually leads to a probe failure even though
> the driver might have worked just fine with IRQF_ONESHOT, but it does
> not want to use it by default.  Currently, the only way to handle this
> is to try to request the IRQ without IRQF_ONESHOT, but with
> IRQF_PROBE_SHARED set and if this fails, try again with IRQF_ONESHOT
> set.  However, this is a bit cumbersome and not very clean.
>
> When commit 7a36b901a6eb ("ACPI: OSL: Use a threaded interrupt handler
> for SCI") switched over the ACPI subsystem to using a threaded interrupt
> handler for the SCI, it had to use IRQF_ONESHOT for it because that's
> required due to the way the SCI handler works (it needs to walk all of
> the enabled GPEs before IRQ line can be unmasked).  The SCI IRQ line is
> not shared with other users very often due to the SCI handling overhead,
> but on sone systems it is shared and when the other user of it attempts
> to install a threaded handler, a flags mismatch related to IRQF_ONESHOT
> may occur.  As it turned out, that happened to the pinctrl-amd driver
> and so commit 4451e8e8415e ("pinctrl: amd: Add IRQF_ONESHOT to the
> interrupt request") attempted to address the issue by adding
> IRQF_ONESHOT to the interrupt flags in that driver, but this is now
> causing an IRQF_ONESHOT-related mismatch to occur on another system
> which cannot boot as a result of it.
>
> Clearly, pinctrl-amd can work with IRQF_ONESHOT if need be, but it
> should not set that flag by default, so it needs a way to indicate that
> to the IRQ subsystem.
>
> To that end, introdcuce a new interrupt flag, IRQF_COND_ONESHOT, which
> will only have effect when the IRQ line is shared and IRQF_ONESHOT has
> been set for it already, in which case it will be promoted to the
> latter.
>
> This is sufficient for drivers sharing the IRQ line with the SCI as it
> is requested by the ACPI subsystem before any drivers are probed, so
> they will always see IRQF_ONESHOT set for the IRQ in question.
>
> Closes: https://lore.kernel.org/lkml/CAN-StX1HqWqi+YW=t+V52-38Mfp5fAz7YHx4aH-CQjgyNiKx3g@mail.gmail.com/
> Fixes: 4451e8e8415e ("pinctrl: amd: Add IRQF_ONESHOT to the interrupt request")
> Cc: 6.8+ <stable@...r.kernel.org> # 6.8+
> Reported-by: Francisco Ayala Le Brun <francisco@...eowindow.eu>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>

> -#if !defined(CONFIG_GENERIC_IRQ_PROBE)
> +#if !defined(CONFIG_GENERIC_IRQ_PROBE)

Is that some whitespace fix? Not that it matters to me, so:

Reviewed-by: Linus Walleij <linus.walleij@...aro.org>

I expect that Thomas want to apply this one.

Yours,
Linus Walleij