Re: [此为外部邮件 谨防风险] Re: [PATCH] x86/apic/msi: Fix MSI interrupt affinity configuration

[WeitaoWang-oc <weitaowang-oc@zhaoxin.com>]

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