Re: [PATCH v21 4/4] PCI/MSI: Enable memory decoding before restoring MSI-X messages

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Blindly restoring a stale PCI_COMMAND value introduces a TOCTOU race window that can erase concurrent modifications to the register.
--

commit ff043ecd9c4375492e235cd366b469432d15d4f3
Author: Farhan Ali <alifm@linux.ibm.com>

PCI/MSI: Enable memory decoding before restoring MSI-X messages

This commit modifies the MSI-X restoration path to temporarily enable the 
Command register Memory bit before writing MSI-X messages. This prevents 
Unsupported Request (UR) errors if the saved device state did not have 
memory decoding enabled.

> diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c
> index 209373c92e9e1..79c7e84d314b7 100644
> --- a/drivers/pci/msi/msi.c
> +++ b/drivers/pci/msi/msi.c
> @@ -870,6 +870,7 @@ void __pci_restore_msix_state(struct pci_dev *dev)
>  {
>  	struct msi_desc *entry;
>  	bool write_msg;
> +	u16 cmd;
>  
>  	if (!dev->msix_enabled)
>  		return;

[ ... ]

> @@ -879,6 +880,14 @@ void __pci_restore_msix_state(struct pci_dev *dev)
>  	pci_msix_clear_and_set_ctrl(dev, 0,
>  				PCI_MSIX_FLAGS_ENABLE | PCI_MSIX_FLAGS_MASKALL);
>  
> +	/*
> +	 * The restored device state may not have Memory decoding enabled
> +	 * in the Command register. Since the MSI-X was enabled for the
> +	 * device, enable Memory decoding before restoring MSI-X.
> +	 */
> +	pci_read_config_word(dev, PCI_COMMAND, &cmd);
> +	pci_write_config_word(dev, PCI_COMMAND, cmd | PCI_COMMAND_MEMORY);
> +
>  	write_msg = arch_restore_msi_irqs(dev);
>  
>  	scoped_guard (msi_descs_lock, &dev->dev) {
> @@ -889,6 +898,7 @@ void __pci_restore_msix_state(struct pci_dev *dev)
>  		}
>  	}
>  
> +	pci_write_config_word(dev, PCI_COMMAND, cmd);

[Severity: Medium]
Does unconditionally restoring the stale cmd value here introduce a race 
condition?

The function reads the PCI_COMMAND register into cmd, enables the memory bit,
and then enters a potentially sleeping section by acquiring msi_descs_lock.

If another thread or subsystem modifies PCI_COMMAND (such as changing Bus 
Mastering or INTx disable) while this lock is held, writing the stale cmd 
value back will silently overwrite and lose those concurrent modifications.

Additionally, if PCI_COMMAND_MEMORY was already enabled initially, does the 
code needlessly expose the device to this race instead of skipping the 
register modification entirely?

>  	pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260630164807.643-1-alifm@linux.ibm.com?part=4