mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] x86/apic/msi: Fix MSI interrupt affinity configuration
@ 2026-09-28 16:48 WeitaoWang-oc
  2026-09-29 16:29 ` Koichiro Den
  0 siblings, 1 reply; 7+ messages in thread
From: WeitaoWang-oc @ 2026-09-28 16:48 UTC (permalink / raw)
  To: tglx, mingo, bp, dave.hansen, x86, hpa, den, linux-kernel
  Cc: WeitaoWang, wwt8723, CobeChen, stable

commit b56ebe7c896d ("x86/apic/msi: Fix misconfigured non-maskable MSI
quirk") try to fix the issue of misconfigured non-maskable MSI quirk.
For non-maskable MSI interrupts, use the slow path to fix the MSI affinity
race. Maskable MSI interrupts use the direct path to update configuration
info. However, the current code logic is the opposite as only maskable
MSI interrupts will enable reservation mode.

Fixes: ef8dd01538ea ("genirq/msi: Make interrupt allocation less convoluted")
Cc: stable@vger.kernel.org
Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com>
---
 arch/x86/kernel/apic/msi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
index 66bc5d3e79db..da5f44a80313 100644
--- a/arch/x86/kernel/apic/msi.c
+++ b/arch/x86/kernel/apic/msi.c
@@ -62,7 +62,7 @@ msi_set_affinity(struct irq_data *irqd, const struct cpumask *mask, bool force)
 	 * - The interrupt is not yet started up
 	 * - The new destination CPU is the same as the old destination CPU
 	 */
-	if (!irqd_can_reserve(irqd) ||
+	if (irqd_can_reserve(irqd) ||
 	    cfg->vector == old_cfg.vector ||
 	    old_cfg.vector == MANAGED_IRQ_SHUTDOWN_VECTOR ||
 	    !irqd_is_started(irqd) ||
-- 
2.32.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] x86/apic/msi: Fix MSI interrupt affinity configuration
  2026-09-28 16:48 [PATCH] x86/apic/msi: Fix MSI interrupt affinity configuration WeitaoWang-oc
@ 2026-09-29 16:29 ` Koichiro Den
  2026-09-30  7:33   ` WeitaoWang-oc
  2026-10-15 22:12   ` Thomas Gleixner
  0 siblings, 2 replies; 7+ messages in thread
From: Koichiro Den @ 2026-09-29 16:29 UTC (permalink / raw)
  To: WeitaoWang-oc
  Cc: tglx, mingo, bp, dave.hansen, x86, hpa, linux-kernel, WeitaoWang,
	wwt8723, CobeChen, stable

On Tue, Sep 29, 2026 at 12:48:39AM +0800, WeitaoWang-oc wrote:
> commit b56ebe7c896d ("x86/apic/msi: Fix misconfigured non-maskable MSI
> quirk") try to fix the issue of misconfigured non-maskable MSI quirk.
> For non-maskable MSI interrupts, use the slow path to fix the MSI affinity
> race. Maskable MSI interrupts use the direct path to update configuration
> info. However, the current code logic is the opposite as only maskable
> MSI interrupts will enable reservation mode.

Hi Weitao, thanks for catching this.

> 
> Fixes: ef8dd01538ea ("genirq/msi: Make interrupt allocation less convoluted")

I'd rather (or also) add:

  Fixes: b56ebe7c896d ("x86/apic/msi: Fix misconfigured non-maskable MSI quirk")

As I recall, I started out trying to fix the issue, but then got too focused on
removing the flag. I missed that the check needed to be inverted. This
left the race unfixed and also made maskable interrupts possibly take the slow
path unnecessarily. Sorry for the trouble. Feel free to use this if it helps:

  Reviewed-by: Koichiro Den <den@valinux.co.jp>

Best regards,
Koichiro

> Cc: stable@vger.kernel.org
> Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com>
> ---
>  arch/x86/kernel/apic/msi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
> index 66bc5d3e79db..da5f44a80313 100644
> --- a/arch/x86/kernel/apic/msi.c
> +++ b/arch/x86/kernel/apic/msi.c
> @@ -62,7 +62,7 @@ msi_set_affinity(struct irq_data *irqd, const struct cpumask *mask, bool force)
>  	 * - The interrupt is not yet started up
>  	 * - The new destination CPU is the same as the old destination CPU
>  	 */
> -	if (!irqd_can_reserve(irqd) ||
> +	if (irqd_can_reserve(irqd) ||
>  	    cfg->vector == old_cfg.vector ||
>  	    old_cfg.vector == MANAGED_IRQ_SHUTDOWN_VECTOR ||
>  	    !irqd_is_started(irqd) ||
> -- 
> 2.32.0
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] x86/apic/msi: Fix MSI interrupt affinity configuration
  2026-09-29 16:29 ` Koichiro Den
@ 2026-09-30  7:33   ` WeitaoWang-oc
  2026-10-15 22:12   ` Thomas Gleixner
  1 sibling, 0 replies; 7+ messages in thread
From: WeitaoWang-oc @ 2026-09-30  7:33 UTC (permalink / raw)
  To: Koichiro Den
  Cc: tglx, mingo, bp, dave.hansen, x86, hpa, linux-kernel, WeitaoWang,
	wwt8723, CobeChen, stable

On 9/30/26 00:29, Koichiro Den wrote:
> On Tue, Sep 29, 2026 at 12:48:39AM +0800, WeitaoWang-oc wrote:
>> commit b56ebe7c896d ("x86/apic/msi: Fix misconfigured non-maskable MSI
>> quirk") try to fix the issue of misconfigured non-maskable MSI quirk.
>> For non-maskable MSI interrupts, use the slow path to fix the MSI affinity
>> race. Maskable MSI interrupts use the direct path to update configuration
>> info. However, the current code logic is the opposite as only maskable
>> MSI interrupts will enable reservation mode.
> 
> Hi Weitao, thanks for catching this.
> 
>>
>> Fixes: ef8dd01538ea ("genirq/msi: Make interrupt allocation less convoluted")
> 
> I'd rather (or also) add:
> 
>    Fixes: b56ebe7c896d ("x86/apic/msi: Fix misconfigured non-maskable MSI quirk")
> 
> As I recall, I started out trying to fix the issue, but then got too focused on
> removing the flag. I missed that the check needed to be inverted. This
> left the race unfixed and also made maskable interrupts possibly take the slow
> path unnecessarily. Sorry for the trouble. Feel free to use this if it helps:
> 
>    Reviewed-by: Koichiro Den <den@valinux.co.jp>
> 
> Best regards,
> Koichiro

Hi Koichiro,

Thank you four suggestion and review. I'll send a v2 patch.

Best Regards,
Weitao

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] x86/apic/msi: Fix MSI interrupt affinity configuration
  2026-09-29 16:29 ` Koichiro Den
  2026-09-30  7:33   ` WeitaoWang-oc
@ 2026-10-15 22:12   ` Thomas Gleixner
  2026-10-16  3:36     ` [此为外部邮件 谨防风险] " WeitaoWang-oc
  1 sibling, 1 reply; 7+ messages in thread
From: Thomas Gleixner @ 2026-10-15 22:12 UTC (permalink / raw)
  To: Koichiro Den, WeitaoWang-oc
  Cc: mingo, bp, dave.hansen, x86, hpa, linux-kernel, WeitaoWang,
	wwt8723, CobeChen, stable

On Wed, Sep 30 2026 at 01:29, Koichiro Den wrote:
> On Tue, Sep 29, 2026 at 12:48:39AM +0800, WeitaoWang-oc wrote:
>> commit b56ebe7c896d ("x86/apic/msi: Fix misconfigured non-maskable MSI
>> quirk") try to fix the issue of misconfigured non-maskable MSI quirk.
>> For non-maskable MSI interrupts, use the slow path to fix the MSI affinity
>> race. Maskable MSI interrupts use the direct path to update configuration
>> info. However, the current code logic is the opposite as only maskable
>> MSI interrupts will enable reservation mode.

Please read:

  https://docs.kernel.org/process/maintainer-tip.html#changelog

and restructure and reword the changelog accordingly. The above is
incomprehensible word salad.

>> Fixes: ef8dd01538ea ("genirq/msi: Make interrupt allocation less convoluted")
>
> I'd rather (or also) add:
>
>   Fixes: b56ebe7c896d ("x86/apic/msi: Fix misconfigured non-maskable MSI quirk")

That's the correct Fixes tag.

> As I recall, I started out trying to fix the issue, but then got too focused on
> removing the flag. I missed that the check needed to be inverted.

I missed it too :)

>> @@ -62,7 +62,7 @@ msi_set_affinity(struct irq_data *irqd, const struct cpumask *mask, bool force)
>>  	 * - The interrupt is not yet started up
>>  	 * - The new destination CPU is the same as the old destination CPU
>>  	 */
>> -	if (!irqd_can_reserve(irqd) ||
>> +	if (irqd_can_reserve(irqd) ||

Amazing that this went unnoticed so long ...

>>  	    cfg->vector == old_cfg.vector ||
>>  	    old_cfg.vector == MANAGED_IRQ_SHUTDOWN_VECTOR ||
>>  	    !irqd_is_started(irqd) ||
>> -- 
>> 2.32.0
>> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [此为外部邮件 谨防风险] Re: [PATCH] x86/apic/msi: Fix MSI interrupt affinity configuration
  2026-10-15 22:12   ` Thomas Gleixner
@ 2026-10-16  3:36     ` WeitaoWang-oc
  2026-10-16  9:30       ` Thomas Gleixner
  0 siblings, 1 reply; 7+ messages in thread
From: WeitaoWang-oc @ 2026-10-16  3:36 UTC (permalink / raw)
  To: Thomas Gleixner, Koichiro Den
  Cc: mingo, bp, dave.hansen, x86, hpa, linux-kernel, WeitaoWang,
	wwt8723, CobeChen, stable

On 10/16/26 06:12, Thomas Gleixner wrote:
> On Wed, Sep 30 2026 at 01:29, Koichiro Den wrote:
>> On Tue, Sep 29, 2026 at 12:48:39AM +0800, WeitaoWang-oc wrote:
>>> commit b56ebe7c896d ("x86/apic/msi: Fix misconfigured non-maskable MSI
>>> quirk") try to fix the issue of misconfigured non-maskable MSI quirk.
>>> For non-maskable MSI interrupts, use the slow path to fix the MSI affinity
>>> race. Maskable MSI interrupts use the direct path to update configuration
>>> info. However, the current code logic is the opposite as only maskable
>>> MSI interrupts will enable reservation mode.
> 
> Please read:
> 
>    https://docs.kernel.org/process/maintainer-tip.html#changelog
> 
> and restructure and reword the changelog accordingly. The above is
> incomprehensible word salad.

Hi Thomas,

Thank you for the suggestions. I realize the problem description in my
previous patch needed to be more precise. Before posting v3, I have
updated the change log as shown below. Could you please take a look?
If this looks good to you, I will send out the next version.

From: Weitao Wang <WeitaoWang-oc@zhaoxin.com>
Subject: [PATCH v3] x86/apic/msi: Fix MSI interrupt affinity configuration

The two step update sequence in msi_set_affinity() is required for MSI
interrupts which cannot be migrated atomically, i.e. devices which do not
provide MSI masking. The MSI message update writes address and data
registers separately, so such a device can raise an interrupt in between,
which results in a message built from half updated state: A stray
interrupt on a stale vector and a possibly lost device interrupt.

The update sequence is selected by a per interrupt quirk flag, which is
set for interrupts which cannot use reservation mode. Reservation mode is
only enabled for maskable interrupts, i.e. devices which provide MSI-X or
mask support, so the quirk flag is the inverse of the reservation mode
bit.

The removal of the quirk flag replaced the flag check with a reservation
mode check, but dropped the negation of the removed condition:

       - if (!irqd_msi_nomask_quirk(irqd) || ...)
       + if (!irqd_can_reserve(irqd) || ...)

As the quirk flag is the inverse of the reservation mode bit, the
replacement inverted the condition and selects the wrong update path for
every interrupt: Non-maskable interrupts take the direct update path
again, which reopens the race, while maskable interrupts needlessly go
through the two step update.

Restore the original condition so that the direct update path is selected
when reservation mode is enabled, which also matches the comment above the
check: Direct update is possible when the MSI is maskable, which is the
case when the reservation mode bit is set.

Fixes: b56ebe7c896d ("x86/apic/msi: Fix misconfigured non-maskable MSI quirk")
Signed-off-by: Weitao Wang <WeitaoWang-oc@zhaoxin.com>
Reviewed-by: Koichiro Den <den@valinux.co.jp>
Cc: <stable@vger.kernel.org> # v6.1+
---
  Changes in v3:
   - Rework the changelog as requested by Thomas:
     https://lore.kernel.org/all/87jynid352.ffs@fw13/

  Changes in v2:
   - Change the Fixes tag to b56ebe7c896d ("x86/apic/msi: Fix misconfigured
     Dnon-maskable MSI quirk")
   - Update commit messages for clarity

   - Link to v1: https://lore.kernel.org/all/20260928164839.6381-1-WeitaoWang-oc@zhaoxin.com/
   - Link to v2: https://lore.kernel.org/all/20260930155213.4380-1-WeitaoWang-oc@zhaoxin.com/

    arch/x86/kernel/apic/msi.c | 2 +-
    1 file changed, 1 insertion(+), 1 deletion(-)

   diff --git a/arch/x86/kernel/apic/msi.c b/arch/x86/kernel/apic/msi.c
   index 66bc5d3e79db..da5f44a80313 100644
   --- a/arch/x86/kernel/apic/msi.c
   +++ b/arch/x86/kernel/apic/msi.c
   @@ -62,7 +62,7 @@ msi_set_affinity(struct irq_data *irqd, const struct cpumask *mask, bool force)
          * - The interrupt is not yet started up
          * - The new destination CPU is the same as the old destination CPU
          */
   -     if (!irqd_can_reserve(irqd) ||
   +     if (irqd_can_reserve(irqd) ||
             cfg->vector == old_cfg.vector ||
             old_cfg.vector == MANAGED_IRQ_SHUTDOWN_VECTOR ||
             !irqd_is_started(irqd) ||
  --
  2.32.0

Thanks
weitao


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [此为外部邮件 谨防风险] Re: [PATCH] x86/apic/msi: Fix MSI interrupt affinity configuration
  2026-10-16  3:36     ` [此为外部邮件 谨防风险] " WeitaoWang-oc
@ 2026-10-16  9:30       ` Thomas Gleixner
  2026-10-16 11:58         ` [PATCH v3] " WeitaoWang-oc
  0 siblings, 1 reply; 7+ messages in thread
From: Thomas Gleixner @ 2026-10-16  9:30 UTC (permalink / raw)
  To: WeitaoWang-oc, Koichiro Den
  Cc: mingo, bp, dave.hansen, x86, hpa, linux-kernel, WeitaoWang,
	wwt8723, CobeChen, stable

On Fri, Oct 16 2026 at 11:36, WeitaoWang-oc wrote:
> On 10/16/26 06:12, Thomas Gleixner wrote:
>> On Wed, Sep 30 2026 at 01:29, Koichiro Den wrote:
>>> On Tue, Sep 29, 2026 at 12:48:39AM +0800, WeitaoWang-oc wrote:
>>>> commit b56ebe7c896d ("x86/apic/msi: Fix misconfigured non-maskable MSI
>>>> quirk") try to fix the issue of misconfigured non-maskable MSI quirk.
>>>> For non-maskable MSI interrupts, use the slow path to fix the MSI affinity
>>>> race. Maskable MSI interrupts use the direct path to update configuration
>>>> info. However, the current code logic is the opposite as only maskable
>>>> MSI interrupts will enable reservation mode.
> Thank you for the suggestions. I realize the problem description in my
> previous patch needed to be more precise. Before posting v3, I have
> updated the change log as shown below. Could you please take a look?
> If this looks good to you, I will send out the next version.

Looks good


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v3] x86/apic/msi: Fix MSI interrupt affinity configuration
  2026-10-16  9:30       ` Thomas Gleixner
@ 2026-10-16 11:58         ` WeitaoWang-oc
  0 siblings, 0 replies; 7+ messages in thread
From: WeitaoWang-oc @ 2026-10-16 11:58 UTC (permalink / raw)
  To: Thomas Gleixner, Koichiro Den
  Cc: mingo, bp, dave.hansen, x86, hpa, linux-kernel, WeitaoWang,
	wwt8723, CobeChen, stable

On 10/16/26 17:30, Thomas Gleixner wrote:
> On Fri, Oct 16 2026 at 11:36, WeitaoWang-oc wrote:
>> On 10/16/26 06:12, Thomas Gleixner wrote:
>>> On Wed, Sep 30 2026 at 01:29, Koichiro Den wrote:
>>>> On Tue, Sep 29, 2026 at 12:48:39AM +0800, WeitaoWang-oc wrote:
>>>>> commit b56ebe7c896d ("x86/apic/msi: Fix misconfigured non-maskable MSI
>>>>> quirk") try to fix the issue of misconfigured non-maskable MSI quirk.
>>>>> For non-maskable MSI interrupts, use the slow path to fix the MSI affinity
>>>>> race. Maskable MSI interrupts use the direct path to update configuration
>>>>> info. However, the current code logic is the opposite as only maskable
>>>>> MSI interrupts will enable reservation mode.
>> Thank you for the suggestions. I realize the problem description in my
>> previous patch needed to be more precise. Before posting v3, I have
>> updated the change log as shown below. Could you please take a look?
>> If this looks good to you, I will send out the next version.
> 
> Looks good
Hi Thomas,

I've reworked the changelog as you requested. The correct version is
the v3 sent at 18:10 UTC (Message-ID:
20261016181016.3850-1-WeitaoWang-oc@zhaoxin.com,
https://lore.kernel.org/20261016181016.3850-1-WeitaoWang-oc@zhaoxin.com/),
which includes the changelog and Cc to stable. Please ignore the
earlier v3 from 18:06 UTC. Sorry for the noise.

Thanks & Best Regards,
Weitao


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-10-16 11:58 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-28 16:48 [PATCH] x86/apic/msi: Fix MSI interrupt affinity configuration WeitaoWang-oc
2026-09-29 16:29 ` Koichiro Den
2026-09-30  7:33   ` WeitaoWang-oc
2026-10-15 22:12   ` Thomas Gleixner
2026-10-16  3:36     ` [此为外部邮件 谨防风险] " WeitaoWang-oc
2026-10-16  9:30       ` Thomas Gleixner
2026-10-16 11:58         ` [PATCH v3] " WeitaoWang-oc

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®