On Wed, Jul 24, 2013 at 11:08:10AM +0800, Zhenzhong Duan wrote:If removed, msi entry couldn't be restored, such as pci_reset_function who will reset pci registers.xen_initdom_restore_msi_irqs trigger a hypercall to restore addr/data/maskWhy not remove the hypercall then?
in dom0. It's better to do the same for default_restore_msi_irqs in baremetal.
Move restore of mask in default_restore_msi_irqs, this could avoid mask
restored twice in dom0, once in hypercall, the other in kernel.
Or alter the function to detectThen we need to add the check for dom0 only.
whether the restore of the mask has occurred?
pci_write_config_dword(~msi_capable_mask(_control) | entry->masked) restore per vector
Without that, qlcnic driver calling pci_reset_function will lost interruptBefore this patch we had:
in dom0.
Tested-by: Sucheta Chakraborty <sucheta.chakraborty@xxxxxxxxxx>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@xxxxxxxxxx>
---
drivers/pci/msi.c | 17 ++++++++++++++---
1 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c
index 87223ae..922fb49 100644
--- a/drivers/pci/msi.c
+++ b/drivers/pci/msi.c
@@ -216,6 +216,8 @@ void unmask_msi_irq(struct irq_data *data)
#ifdef HAVE_DEFAULT_MSI_RESTORE_IRQS
void default_restore_msi_irqs(struct pci_dev *dev, int irq)
{
+ int pos;
+ u16 control;
struct msi_desc *entry;
entry = NULL;
@@ -228,8 +230,19 @@ void default_restore_msi_irqs(struct pci_dev *dev, int irq)
entry = irq_get_msi_desc(irq);
}
- if (entry)
+ if (entry) {
write_msi_msg(irq, &entry->msg);
+ if (dev->msix_enabled) {
+ msix_mask_irq(entry, entry->masked);
+ readl(entry->mask_base);
+ } else {
+ pos = entry->msi_attrib.pos;
+ pci_read_config_word(dev, pos + PCI_MSI_FLAGS,
+ &control);
+ msi_mask_irq(entry, msi_capable_mask(control),
+ entry->masked);
+ }
+ }
}
#endif
@@ -406,7 +419,6 @@ static void __pci_restore_msi_state(struct pci_dev *dev)
arch_restore_msi_irqs(dev, dev->irq);
pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &control);
- msi_mask_irq(entry, msi_capable_mask(control), entry->masked);
write_msi_msg(..)
pci_read_config_work(PCI_MSI_FLAGS, &control)
pci_write_config_dword(~msi_capable_mask(control) | entry->masked)
control &= ~_PCI_MSI_FLAGS_QSIZE;
control |= ...
pci_write_config_dword(PCI_MSI_FLAGS, control)
while with this you have now:
write_msi_msg(..)
pci_read_config_work(PCI_MSI_FLAGS, &_control)
pci_write_config_dword(~msi_capable_mask(_control) | entry->masked)
--> pci_read_config_work(PCI_MSI_FLAGS, &control)
control &= ~_PCI_MSI_FLAGS_QSIZE;
control |= ...
pci_write_config_dword(PCI_MSI_FLAGS, control)
see the problem? The 'control' value in __pci_restore_msi_state reads the
value _after_ it has been masked (which is now done in default_restore_msi_irqs).
Wouldn't that cause problems?