There are 2 problems on MSI/MSI-X mask states in suspend/resume.
[1]:
It is better to restore the mask states of MSI/MSI-X to initial states
(MSI is unmasked, MSI-X is masked) when we release the device.
The pci_msi_shutdown() does the restoration of mask states for MSI,
while the msi_free_irqs() does it for MSI-X. In other words, in the
"disable" path both of MSI and MSI-X are cared, but in the "shutdown"
path only MSI is cared.
MSI:
pci_disable_msi()
=> pci_msi_shutdown()
[ mask states for MSI restored ]
=> msi_set_enable(dev, pos, 0);
=> msi_free_irqs()
MSI-X:
pci_disable_msix()
=> pci_msix_shutdown()
=> msix_set_enable(dev, 0);
=> msix_free_all_irqs
=> msi_free_irqs()
[ mask states for MSI-X restored ]
This patch moves the masking for MSI-X from msi_free_irqs() to
pci_msix_shutdown().
This change has some positive side effects:
- It prevents OS from touching mask states before reading preserved
bits in the register, which can be happen if msi_free_irqs() is
called from error path in msix_capability_init().
- It also prevents touching the register after turning off MSI-X in
"disable" path, which can be a problem on some devices.
[2]:
We have cache of the mask state in msi_desc, which is automatically
updated when msi/msix_mask_irq() is called. This cached states are
used for the resume.
But since what need to be restored in the resume is the states before
the shutdown on the suspend, calling msi/msix_mask_irq() from
pci_msi/msix_shutdown() is not appropriate.
This patch introduces msi/msix_mask_irq_nocache() that do mask as same
as msi/msix_mask_irq() but does not update cached state, for use in
pci_msi/msix_shutdown().
Signed-off-by: Hidetoshi Seto <seto.hidetoshi@xxxxxxxxxxxxxx>
---
drivers/pci/msi.c | 45 +++++++++++++++++++++++++++++++++++++++------
1 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index d9f06fb..e176316 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -127,7 +127,7 @@ static inline __attribute_const__ u32 msi_enabled_mask(u16 control)
* reliably as devices without an INTx disable bit will then generate a
* level IRQ which will never be cleared.
*/
-static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
+static void __msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag, int save)
{
u32 mask_bits = desc->masked;
@@ -137,7 +137,19 @@ static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
mask_bits &= ~mask;
mask_bits |= flag;
pci_write_config_dword(desc->dev, desc->mask_pos, mask_bits);
- desc->masked = mask_bits;
+
+ if (save)
+ desc->masked = mask_bits;
+}
+
+static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
+{
+ __msi_mask_irq(desc, mask, flag, 1);
+}
+
+static void msi_mask_irq_nocache(struct msi_desc *desc, u32 mask, u32 flag)
+{
+ __msi_mask_irq(desc, mask, flag, 0);
}
/*
@@ -147,7 +159,7 @@ static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
* file. This saves a few milliseconds when initialising devices with lots
* of MSI-X interrupts.
*/
-static void msix_mask_irq(struct msi_desc *desc, u32 flag)
+static void __msix_mask_irq(struct msi_desc *desc, u32 flag, int save)
{
u32 mask_bits = desc->masked;
unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
@@ -155,7 +167,19 @@ static void msix_mask_irq(struct msi_desc *desc, u32 flag)
mask_bits &= ~1;
mask_bits |= flag;
writel(mask_bits, desc->mask_base + offset);
- desc->masked = mask_bits;
+
+ if (save)
+ desc->masked = mask_bits;
+}
+
+static void msix_mask_irq(struct msi_desc *desc, u32 flag)
+{
+ __msix_mask_irq(desc, flag, 1);
+}
+
+static void msix_mask_irq_nocache(struct msi_desc *desc, u32 flag)
+{
+ __msix_mask_irq(desc, flag, 0);
}
static void msi_set_mask_bit(unsigned irq, u32 flag)
@@ -611,9 +635,11 @@ void pci_msi_shutdown(struct pci_dev *dev)
pci_intx_for_msi(dev, 1);
dev->msi_enabled = 0;
+ /* Return the device with MSI unmasked as initial states */
pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &ctrl);
mask = msi_capable_mask(ctrl);
- msi_mask_irq(desc, mask, ~mask);
+ /* Keep cached state to be restored */
+ msi_mask_irq_nocache(desc, mask, ~mask);
/* Restore dev->irq to its default pin-assertion irq */
dev->irq = desc->msi_attrib.default_irq;
@@ -653,7 +679,6 @@ static int msi_free_irqs(struct pci_dev* dev)
list_for_each_entry_safe(entry, tmp, &dev->msi_list, list) {
if (entry->msi_attrib.is_msix) {
- msix_mask_irq(entry, 1);
if (list_is_last(&entry->list, &dev->msi_list))
iounmap(entry->mask_base);
}
@@ -741,9 +766,17 @@ static void msix_free_all_irqs(struct pci_dev *dev)
void pci_msix_shutdown(struct pci_dev* dev)
{
+ struct msi_desc *entry;
+
if (!pci_msi_enable || !dev || !dev->msix_enabled)
return;
+ /* Return the device with MSI-X masked as initial states */
+ list_for_each_entry(entry, &dev->msi_list, list) {
+ /* Keep cached state to be restored */
+ msix_mask_irq_nocache(entry, 1);
+ }
+
msix_set_enable(dev, 0);
pci_intx_for_msi(dev, 1);
dev->msix_enabled = 0;
--
1.6.3.3
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
