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

[Farhan Ali <alifm@linux.ibm.com>]

On 6/22/2026 11:54 AM, Niklas Schnelle wrote:
> On Mon, 2026-06-22 at 10:18 -0700, Farhan Ali wrote:
>> The current MSI-X restoration path assumes the Command register Memory bit
>> is enabled when writing MSI-X messages. But its possible the last saved and
> SpellingL "it's"
>
>> restored state of device may not have the Memory bit enabled, even if a
> Missing "a" in "of a device"

Ah I missed correcting these.


>
>> device driver later enables Memory bit and MSI-X. Attempting to access
>> Memory space without Memory bit enabled can lead to Unsupported Request
>> (UR) from the device. Fix this by enabling Memory bit and restore
>> it afterwards.
> The official spec term is "Memory Space Enable" and I think "can lead
> to" is actually underselling it, as the PCIe spec says that accesses
> are "caused to be handled as Unsupported Requests". Also as I
> understand you did see this in practice. Maybe we even want a Fixes
> tag?

This behavior has been present since 41017f0cac92 ("[PATCH] PCI: MSI(X) 
save/restore for suspend/resume"), so I am not sure a fixes tag makes sense?


>> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
>> ---
>>   drivers/pci/msi/msi.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c
>> index 81d24a270a79..46a0d9f68a57 100644
>> --- a/drivers/pci/msi/msi.c
>> +++ b/drivers/pci/msi/msi.c
>> @@ -874,6 +874,7 @@ void __pci_restore_msix_state(struct pci_dev *dev)
>>   {
>>   	struct msi_desc *entry;
>>   	bool write_msg;
>> +	u16 cmd;
>>   
>>   	if (!dev->msix_enabled)
>>   		return;
>> @@ -882,6 +883,8 @@ void __pci_restore_msix_state(struct pci_dev *dev)
>>   	pci_intx_for_msi(dev, 0);
>>   	pci_msix_clear_and_set_ctrl(dev, 0,
>>   				PCI_MSIX_FLAGS_ENABLE | PCI_MSIX_FLAGS_MASKALL);
>> +	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);
>>   
>> @@ -893,6 +896,7 @@ void __pci_restore_msix_state(struct pci_dev *dev)
>>   		}
>>   	}
>>   
>> +	pci_write_config_word(dev, PCI_COMMAND, cmd);
>>   	pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0);
>>   }
>>   
> Code wise this all looks good to me so feel free to add.
>
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
>
> Thanks,
> Niklas

Thanks for reviewing!

Thanks

Farhan