Skip to content

Commit aa8092c

Browse files
KAGA-KOKOgregkh
authored andcommitted
PCI/MSI: Mask all unused MSI-X entries
commit 7d5ec3d upstream. 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] Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent 7e90e81 commit aa8092c

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
@@ -698,6 +698,7 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
698698
{
699699
struct irq_affinity_desc *curmsk, *masks = NULL;
700700
struct msi_desc *entry;
701+
void __iomem *addr;
701702
int ret, i;
702703
int vec_count = pci_msix_vec_count(dev);
703704

@@ -718,6 +719,7 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
718719

719720
entry->msi_attrib.is_msix = 1;
720721
entry->msi_attrib.is_64 = 1;
722+
721723
if (entries)
722724
entry->msi_attrib.entry_nr = entries[i].entry;
723725
else
@@ -729,6 +731,10 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
729731
entry->msi_attrib.default_irq = dev->irq;
730732
entry->mask_base = base;
731733

734+
addr = pci_msix_desc_addr(entry);
735+
if (addr)
736+
entry->masked = readl(addr + PCI_MSIX_ENTRY_VECTOR_CTRL);
737+
732738
list_add_tail(&entry->list, dev_to_msi_list(&dev->dev));
733739
if (masks)
734740
curmsk++;
@@ -739,26 +745,25 @@ static int msix_setup_entries(struct pci_dev *dev, void __iomem *base,
739745
return ret;
740746
}
741747

742-
static void msix_program_entries(struct pci_dev *dev,
743-
struct msix_entry *entries)
748+
static void msix_update_entries(struct pci_dev *dev, struct msix_entry *entries)
744749
{
745750
struct msi_desc *entry;
746-
int i = 0;
747-
void __iomem *desc_addr;
748751

749752
for_each_pci_msi_entry(entry, dev) {
750-
if (entries)
751-
entries[i++].vector = entry->irq;
753+
if (entries) {
754+
entries->vector = entry->irq;
755+
entries++;
756+
}
757+
}
758+
}
752759

753-
desc_addr = pci_msix_desc_addr(entry);
754-
if (desc_addr)
755-
entry->masked = readl(desc_addr +
756-
PCI_MSIX_ENTRY_VECTOR_CTRL);
757-
else
758-
entry->masked = 0;
760+
static void msix_mask_all(void __iomem *base, int tsize)
761+
{
762+
u32 ctrl = PCI_MSIX_ENTRY_CTRL_MASKBIT;
763+
int i;
759764

760-
msix_mask_irq(entry, 1);
761-
}
765+
for (i = 0; i < tsize; i++, base += PCI_MSIX_ENTRY_SIZE)
766+
writel(ctrl, base + PCI_MSIX_ENTRY_VECTOR_CTRL);
762767
}
763768

764769
/**
@@ -775,9 +780,9 @@ static void msix_program_entries(struct pci_dev *dev,
775780
static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries,
776781
int nvec, struct irq_affinity *affd)
777782
{
778-
int ret;
779-
u16 control;
780783
void __iomem *base;
784+
int ret, tsize;
785+
u16 control;
781786

782787
/*
783788
* Some devices require MSI-X to be enabled before the MSI-X
@@ -789,12 +794,16 @@ static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries,
789794

790795
pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &control);
791796
/* Request & Map MSI-X table region */
792-
base = msix_map_region(dev, msix_table_size(control));
797+
tsize = msix_table_size(control);
798+
base = msix_map_region(dev, tsize);
793799
if (!base) {
794800
ret = -ENOMEM;
795801
goto out_disable;
796802
}
797803

804+
/* Ensure that all table entries are masked. */
805+
msix_mask_all(base, tsize);
806+
798807
ret = msix_setup_entries(dev, base, entries, nvec, affd);
799808
if (ret)
800809
goto out_disable;
@@ -808,7 +817,7 @@ static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries,
808817
if (ret)
809818
goto out_free;
810819

811-
msix_program_entries(dev, entries);
820+
msix_update_entries(dev, entries);
812821

813822
ret = populate_msi_sysfs(dev);
814823
if (ret)

0 commit comments

Comments
 (0)