lkml.org 
[lkml]   [2021]   [Jul]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 2/8] PCI/MSI: Mask all unused MSI-X entries
    Hi Thomas

    On Wed, Jul 21, 2021 at 09:11:28PM +0200, Thomas Gleixner wrote:

    [snip]

    > --- a/drivers/pci/msi.c
    > +++ b/drivers/pci/msi.c
    > @@ -691,6 +691,7 @@ static int msix_setup_entries(struct pci
    > {
    > struct irq_affinity_desc *curmsk, *masks = NULL;
    > struct msi_desc *entry;
    > + void __iomem *addr;
    > int ret, i;
    > int vec_count = pci_msix_vec_count(dev);
    >
    > @@ -711,6 +712,7 @@ static int msix_setup_entries(struct pci
    >
    > entry->msi_attrib.is_msix = 1;
    > entry->msi_attrib.is_64 = 1;
    > +
    > if (entries)
    > entry->msi_attrib.entry_nr = entries[i].entry;
    > else
    > @@ -722,6 +724,10 @@ static int msix_setup_entries(struct pci
    > entry->msi_attrib.default_irq = dev->irq;
    > entry->mask_base = base;
    >
    > + addr = pci_msix_desc_addr(entry);
    > + if (addr)
    > + entry->masked = readl(addr + PCI_MSIX_ENTRY_VECTOR_CTRL);

    Silly question:
    Do we have to read what the HW has to set this entry->masked? Shouldn't
    this be all masked before we start the setup?

    > +
    > list_add_tail(&entry->list, dev_to_msi_list(&dev->dev));
    > if (masks)
    > curmsk++;
    > @@ -732,26 +738,25 @@ static int msix_setup_entries(struct pci
    > return ret;
    > }
    >
    > -static void msix_program_entries(struct pci_dev *dev,
    > - struct msix_entry *entries)
    > +static void msix_update_entries(struct pci_dev *dev, struct msix_entry *entries)
    > {
    > struct msi_desc *entry;
    > - int i = 0;
    > - void __iomem *desc_addr;
    >
    > for_each_pci_msi_entry(entry, dev) {
    > - if (entries)
    > - entries[i++].vector = entry->irq;
    > + if (entries) {
    > + entries->vector = entry->irq;
    > + entries++;
    > + }
    > + }
    > +}
    >
    > - desc_addr = pci_msix_desc_addr(entry);
    > - if (desc_addr)
    > - entry->masked = readl(desc_addr +
    > - PCI_MSIX_ENTRY_VECTOR_CTRL);
    > - else
    > - entry->masked = 0;
    > +static void msix_mask_all(void __iomem *base, int tsize)
    > +{
    > + u32 ctrl = PCI_MSIX_ENTRY_CTRL_MASKBIT;
    > + int i;
    >
    > - msix_mask_irq(entry, 1);
    > - }
    > + for (i = 0; i < tsize; i++, base += PCI_MSIX_ENTRY_SIZE)
    > + writel(ctrl, base + PCI_MSIX_ENTRY_VECTOR_CTRL);

    shouldn't we initialize entry->masked here?

    > }
    >
    > /**
    > @@ -768,9 +773,9 @@ static void msix_program_entries(struct
    > static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries,
    > int nvec, struct irq_affinity *affd)
    > {
    > - int ret;
    > - u16 control;
    > void __iomem *base;
    > + int ret, tsize;
    > + u16 control;
    >
    > /*
    > * Some devices require MSI-X to be enabled before the MSI-X
    > @@ -782,12 +787,16 @@ static int msix_capability_init(struct p
    >
    > pci_read_config_word(dev, dev->msix_cap + PCI_MSIX_FLAGS, &control);
    > /* Request & Map MSI-X table region */
    > - base = msix_map_region(dev, msix_table_size(control));
    > + tsize = msix_table_size(control);
    > + base = msix_map_region(dev, tsize);
    > if (!base) {
    > ret = -ENOMEM;
    > goto out_disable;
    > }
    >
    > + /* Ensure that all table entries are masked. */
    > + msix_mask_all(base, tsize);
    > +
    > ret = msix_setup_entries(dev, base, entries, nvec, affd);
    > if (ret)
    > goto out_disable;
    > @@ -801,7 +810,7 @@ static int msix_capability_init(struct p
    > if (ret)
    > goto out_free;
    >
    > - msix_program_entries(dev, entries);
    > + msix_update_entries(dev, entries);
    >
    > ret = populate_msi_sysfs(dev);
    > if (ret)
    >

    --
    Cheers,
    Ashok

    [Forgiveness is the attribute of the STRONG - Gandhi]

    \
     
     \ /
      Last update: 2021-07-22 00:24    [W:4.862 / U:0.300 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site