Commit 12abb8ba authored by Hidetoshi Seto's avatar Hidetoshi Seto Committed by Jesse Barnes

PCI MSI: Fix restoration of MSI/MSI-X mask states in suspend/resume

There are 2 problems on 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 handled, but in the "shutdown"
path only MSI is handled.

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() that do mask as same
as msi/msix_mask_irq() but does not update cached state, for use
in pci_msi/msix_shutdown().

[updated: get rid of msi/msix_mask_irq_nocache() (proposed by Matthew Wilcox)]
Reviewed-by: default avatarMatthew Wilcox <willy@linux.intel.com>
Signed-off-by: default avatarHidetoshi Seto <seto.hidetoshi@jp.fujitsu.com>
Signed-off-by: default avatarJesse Barnes <jbarnes@virtuousgeek.org>
parent 7ba1930d
...@@ -127,17 +127,23 @@ static inline __attribute_const__ u32 msi_enabled_mask(u16 control) ...@@ -127,17 +127,23 @@ static inline __attribute_const__ u32 msi_enabled_mask(u16 control)
* reliably as devices without an INTx disable bit will then generate a * reliably as devices without an INTx disable bit will then generate a
* level IRQ which will never be cleared. * level IRQ which will never be cleared.
*/ */
static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag) static u32 __msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
{ {
u32 mask_bits = desc->masked; u32 mask_bits = desc->masked;
if (!desc->msi_attrib.maskbit) if (!desc->msi_attrib.maskbit)
return; return 0;
mask_bits &= ~mask; mask_bits &= ~mask;
mask_bits |= flag; mask_bits |= flag;
pci_write_config_dword(desc->dev, desc->mask_pos, mask_bits); pci_write_config_dword(desc->dev, desc->mask_pos, mask_bits);
desc->masked = mask_bits;
return mask_bits;
}
static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag)
{
desc->masked = __msi_mask_irq(desc, mask, flag);
} }
/* /*
...@@ -147,7 +153,7 @@ static void msi_mask_irq(struct msi_desc *desc, u32 mask, u32 flag) ...@@ -147,7 +153,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 * file. This saves a few milliseconds when initialising devices with lots
* of MSI-X interrupts. * of MSI-X interrupts.
*/ */
static void msix_mask_irq(struct msi_desc *desc, u32 flag) static u32 __msix_mask_irq(struct msi_desc *desc, u32 flag)
{ {
u32 mask_bits = desc->masked; u32 mask_bits = desc->masked;
unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE + unsigned offset = desc->msi_attrib.entry_nr * PCI_MSIX_ENTRY_SIZE +
...@@ -155,7 +161,13 @@ static void msix_mask_irq(struct msi_desc *desc, u32 flag) ...@@ -155,7 +161,13 @@ static void msix_mask_irq(struct msi_desc *desc, u32 flag)
mask_bits &= ~1; mask_bits &= ~1;
mask_bits |= flag; mask_bits |= flag;
writel(mask_bits, desc->mask_base + offset); writel(mask_bits, desc->mask_base + offset);
desc->masked = mask_bits;
return mask_bits;
}
static void msix_mask_irq(struct msi_desc *desc, u32 flag)
{
desc->masked = __msix_mask_irq(desc, flag);
} }
static void msi_set_mask_bit(unsigned irq, u32 flag) static void msi_set_mask_bit(unsigned irq, u32 flag)
...@@ -616,9 +628,11 @@ void pci_msi_shutdown(struct pci_dev *dev) ...@@ -616,9 +628,11 @@ void pci_msi_shutdown(struct pci_dev *dev)
pci_intx_for_msi(dev, 1); pci_intx_for_msi(dev, 1);
dev->msi_enabled = 0; dev->msi_enabled = 0;
/* Return the device with MSI unmasked as initial states */
pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &ctrl); pci_read_config_word(dev, pos + PCI_MSI_FLAGS, &ctrl);
mask = msi_capable_mask(ctrl); mask = msi_capable_mask(ctrl);
msi_mask_irq(desc, mask, ~mask); /* Keep cached state to be restored */
__msi_mask_irq(desc, mask, ~mask);
/* Restore dev->irq to its default pin-assertion irq */ /* Restore dev->irq to its default pin-assertion irq */
dev->irq = desc->msi_attrib.default_irq; dev->irq = desc->msi_attrib.default_irq;
...@@ -658,7 +672,6 @@ static int msi_free_irqs(struct pci_dev* dev) ...@@ -658,7 +672,6 @@ static int msi_free_irqs(struct pci_dev* dev)
list_for_each_entry_safe(entry, tmp, &dev->msi_list, list) { list_for_each_entry_safe(entry, tmp, &dev->msi_list, list) {
if (entry->msi_attrib.is_msix) { if (entry->msi_attrib.is_msix) {
msix_mask_irq(entry, 1);
if (list_is_last(&entry->list, &dev->msi_list)) if (list_is_last(&entry->list, &dev->msi_list))
iounmap(entry->mask_base); iounmap(entry->mask_base);
} }
...@@ -746,9 +759,17 @@ static void msix_free_all_irqs(struct pci_dev *dev) ...@@ -746,9 +759,17 @@ static void msix_free_all_irqs(struct pci_dev *dev)
void pci_msix_shutdown(struct pci_dev* dev) void pci_msix_shutdown(struct pci_dev* dev)
{ {
struct msi_desc *entry;
if (!pci_msi_enable || !dev || !dev->msix_enabled) if (!pci_msi_enable || !dev || !dev->msix_enabled)
return; return;
/* Return the device with MSI-X masked as initial states */
list_for_each_entry(entry, &dev->msi_list, list) {
/* Keep cached states to be restored */
__msix_mask_irq(entry, 1);
}
msix_set_enable(dev, 0); msix_set_enable(dev, 0);
pci_intx_for_msi(dev, 1); pci_intx_for_msi(dev, 1);
dev->msix_enabled = 0; dev->msix_enabled = 0;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment