On Wed, Mar 19, 2025 at 11:56:57AM +0100, Thomas Gleixner wrote:
> The PCI/TPH driver fiddles with the MSI-X control word of an active
> interrupt completely unserialized against concurrent operations issued
> from the interrupt core. It also brings the PCI/MSI-X internal cached
> control word out of sync.
> 
> Provide a function, which has the required serialization and keeps the
> control word cache in sync.
> 
> Unfortunately this requires to look up and lock the interrupt descriptor,
> which should be only done in the interrupt core code. But confining this
> particular oddity in the PCI/MSI core is the lesser of all evil. A
> interrupt core implementation would require a larger pile of infrastructure
> and indirections for dubious value.
> 
> Signed-off-by: Thomas Gleixner <t...@linutronix.de>
> Acked-by: Bjorn Helgaas <bhelg...@google.com>
> Cc: Wei Huang <wei.hua...@amd.com>
> Cc: linux-...@vger.kernel.org
> 
> 
> 
> ---
>  drivers/pci/msi/msi.c |   47 +++++++++++++++++++++++++++++++++++++++++++++++
>  drivers/pci/pci.h     |    9 +++++++++
>  2 files changed, 56 insertions(+)
> 
> --- a/drivers/pci/msi/msi.c
> +++ b/drivers/pci/msi/msi.c
> @@ -910,6 +910,53 @@ void pci_free_msi_irqs(struct pci_dev *d
>       }
>  }
>  
> +#ifdef CONFIG_PCIE_TPH
> +/**
> + * pci_msix_write_tph_tag - Update the TPH tag for a given MSI-X vector
> + * @pdev:    The PCIe device to update
> + * @index:   The MSI-X index to update
> + * @tag:     The tag to write
> + *
> + * Returns: 0 on success, error code on failure
> + */
> +int pci_msix_write_tph_tag(struct pci_dev *pdev, unsigned int index, u16 tag)
> +{
> +     struct msi_desc *msi_desc;
> +     struct irq_desc *irq_desc;
> +     unsigned int virq;
> +
> +     if (!pdev->msix_enabled)
> +             return -ENXIO;
> +
> +     guard(msi_descs_lock)(&pdev->dev);
> +     virq = msi_get_virq(&pdev->dev, index);
> +     if (!virq)
> +             return -ENXIO;
> +     /*
> +      * This is a horrible hack, but short of implementing a PCI
> +      * specific interrupt chip callback and a huge pile of
> +      * infrastructure, this is the minor nuissance. It provides the
> +      * protection against concurrent operations on this entry and keeps
> +      * the control word cache in sync.
> +      */
> +     irq_desc = irq_to_desc(virq);
> +     if (!irq_desc)
> +             return -ENXIO;
> +
> +     guard(raw_spinlock_irq)(&irq_desc->lock);
> +     msi_desc = irq_data_get_msi_desc(&irq_desc->irq_data);
> +     if (!msi_desc || msi_desc->pci.msi_attrib.is_virtual)
> +             return -ENXIO;
> +
> +     msi_desc->pci.msix_ctrl &= ~PCI_MSIX_ENTRY_CTRL_ST;
> +     msi_desc->pci.msix_ctrl |= FIELD_PREP(PCI_MSIX_ENTRY_CTRL_ST, tag);
> +     pci_msix_write_vector_ctrl(msi_desc, msi_desc->pci.msix_ctrl);
> +     /* Flush the write */
> +     readl(pci_msix_desc_addr(msi_desc));
> +     return 0;
> +}

Looks like this change might add this warning, which I don't claim to
understand:

  $ make C=2 drivers/pci/msi/msi.o
  drivers/pci/msi/msi.c:928:5: warning: context imbalance in 
'pci_msix_write_tph_tag' - wrong count at exit

This appeared in v6.16-rc1 as d5124a9957b2 ("PCI/MSI: Provide a sane
mechanism for TPH")

Reply via email to