Skip to content

Commit 7d5ec3d

Browse files
committed
PCI/MSI: Mask all unused MSI-X entries
When MSI-X is enabled the ordering of calls is: msix_map_region(); msix_setup_entries(); pci_msi_setup_msi_irqs(); msix_program_entries(); This has a few interesting issues: 1) msix_setup_entries() allocates the MSI descriptors and initializes them except for the msi_desc:masked member which is left zero initialized. 2) pci_msi_setup_msi_irqs() allocates the interrupt descriptors and sets up the MSI interrupts which ends up in pci_write_msi_msg() unless the interrupt chip provides its own irq_write_msi_msg() function. 3) msix_program_entries() does not do what the name suggests. It solely updates the entries array (if not NULL) and initializes the masked member for each MSI descriptor by reading the hardware state and then masks the entry. Obviously this has some issues: 1) The uninitialized masked member of msi_desc prevents the enforcement of masking the entry in pci_write_msi_msg() depending on the cached masked bit. Aside of that half initialized data is a NONO in general 2) msix_program_entries() only ensures that the actually allocated entries are masked. This is wrong as experimentation with crash testing and crash kernel kexec has shown. This limited testing unearthed that when the production kernel had more entries in use and unmasked when it crashed and the crash kernel allocated a smaller amount of entries, then a full scan of all entries found unmasked entries which were in use in the production kernel. This is obviously a device or emulation issue as the device reset should mask all MSI-X table entries, but obviously that's just part of the paper specification. Cure this by: 1) Masking all table entries in hardware 2) Initializing msi_desc::masked in msix_setup_entries() 3) Removing the mask dance in msix_program_entries() 4) Renaming msix_program_entries() to msix_update_entries() to reflect the purpose of that function. As the masking of unused entries has never been done the Fixes tag refers to a commit in: git://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git Fixes: f036d4e ("[PATCH] ia32 Message Signalled Interrupt support") Signed-off-by: Thomas Gleixner <[email protected]> Tested-by: Marc Zyngier <[email protected]> Reviewed-by: Marc Zyngier <[email protected]> Acked-by: Bjorn Helgaas <[email protected]> Cc: [email protected] Link: https://lore.kernel.org/r/[email protected]
1 parent 4385539 commit 7d5ec3d

File tree

1 file changed

+27
-18
lines changed

1 file changed

+27
-18
lines changed

drivers/pci/msi.c

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -691,6 +691,7 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
691691
{
692692
struct irq_affinity_desc *curmsk, *masks = NULL;
693693
struct msi_desc *entry;
694+
void __iomem *addr;
694695
int ret, i;
695696
int vec_count = pci_msix_vec_count(dev);
696697

@@ -711,6 +712,7 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
711712

712713
entry->msi_attrib.is_msix = 1;
713714
entry->msi_attrib.is_64 = 1;
715+
714716
if (entries)
715717
entry->msi_attrib.entry_nr = entries[i].entry;
716718
else
@@ -722,6 +724,10 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
722724
entry->msi_attrib.default_irq = dev->irq;
723725
entry->mask_base = base;
724726

727+
addr = pci_msix_desc_addr(entry);
728+
if (addr)
729+
entry->masked = readl(addr + PCI_MSIX_ENTRY_VECTOR_CTRL);
730+
725731
list_add_tail(&entry->list, dev_to_msi_list(&dev->dev));
726732
if (masks)
727733
curmsk++;
@@ -732,26 +738,25 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
732738
return ret;
733739
}
734740

735-
static void msix_program_entries(struct pci_dev *dev,
736-
struct msix_entry *entries)
741+
static void msix_update_entries(struct pci_dev *dev, struct msix_entry *entries)
737742
{
738743
struct msi_desc *entry;
739-
int i = 0;
740-
void __iomem *desc_addr;
741744

742745
for_each_pci_msi_entry(entry, dev) {
743-
if (entries)
744-
entries[i++].vector = entry->irq;
746+
if (entries) {
747+
entries->vector = entry->irq;
748+
entries++;
749+
}
750+
}
751+
}
745752

746-
desc_addr = pci_msix_desc_addr(entry);
747-
if (desc_addr)
748-
entry->masked = readl(desc_addr +
749-
PCI_MSIX_ENTRY_VECTOR_CTRL);
750-
else
751-
entry->masked = 0;
753+
static void msix_mask_all(void __iomem *base, int tsize)
754+
{
755+
u32 ctrl = PCI_MSIX_ENTRY_CTRL_MASKBIT;
756+
int i;
752757

753-
msix_mask_irq(entry, 1);
754-
}
758+
for (i = 0; i < tsize; i++, base += PCI_MSIX_ENTRY_SIZE)
759+
writel(ctrl, base + PCI_MSIX_ENTRY_VECTOR_CTRL);
755760
}
756761

757762
/**
@@ -768,9 +773,9 @@ static void msix_program_entries(struct pci_dev *dev,
768773
static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries,
769774
int nvec, struct irq_affinity *affd)
770775
{
771-
int ret;
772-
u16 control;
773776
void __iomem *base;
777+
int ret, tsize;
778+
u16 control;
774779

775780
/*
776781
* Some devices require MSI-X to be enabled before the MSI-X
@@ -782,12 +787,16 @@ static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries,
782787

783788
pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &control);
784789
/* Request & Map MSI-X table region */
785-
base = msix_map_region(dev, msix_table_size(control));
790+
tsize = msix_table_size(control);
791+
base = msix_map_region(dev, tsize);
786792
if (!base) {
787793
ret = -ENOMEM;
788794
goto out_disable;
789795
}
790796

797+
/* Ensure that all table entries are masked. */
798+
msix_mask_all(base, tsize);
799+
791800
ret = msix_setup_entries(dev, base, entries, nvec, affd);
792801
if (ret)
793802
goto out_disable;
@@ -801,7 +810,7 @@ static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries,
801810
if (ret)
802811
goto out_free;
803812

804-
msix_program_entries(dev, entries);
813+
msix_update_entries(dev, entries);
805814

806815
ret = populate_msi_sysfs(dev);
807816
if (ret)

0 commit comments

Comments
 (0)