| Message ID | 20210813122650.25764-1-21cnbao@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | PCI/MSI: Clarify the irq sysfs ABI for PCI devices | expand |
On Sat, Aug 14, 2021 at 12:26:50AM +1200, Barry Song wrote: > From: Barry Song <song.bao.hua@hisilicon.com> > > /sys/bus/pci/devices/.../irq has been there for many years but it has never > been documented. This patch is trying to document it. Plus, irq ABI is very > confusing at this moment especially for MSI and MSI-x cases. MSI sets irq > to the first number in the vector, but MSI-X does nothing for this though > it saves default_irq in msix_setup_entries(). Weird the saved default_irq > for MSI-X is never used in pci_msix_shutdown(), which is quite different > with pci_msi_shutdown(). Thus, this patch also moves to show the first IRQ > number which is from the first msi_entry for MSI-X. Hopefully, this can > make irq ABI more clear and more consistent. > > Signed-off-by: Barry Song <song.bao.hua@hisilicon.com> > --- > Documentation/ABI/testing/sysfs-bus-pci | 8 ++++++++ > drivers/pci/msi.c | 6 ++++++ > 2 files changed, 14 insertions(+) Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
On Sat, Aug 14, 2021 at 12:26:50AM +1200, Barry Song wrote: > From: Barry Song <song.bao.hua@hisilicon.com> > > /sys/bus/pci/devices/.../irq has been there for many years but it has never > been documented. This patch is trying to document it. Plus, irq ABI is very > confusing at this moment especially for MSI and MSI-x cases. MSI sets irq > to the first number in the vector, but MSI-X does nothing for this though > it saves default_irq in msix_setup_entries(). Weird the saved default_irq > for MSI-X is never used in pci_msix_shutdown(), which is quite different > with pci_msi_shutdown(). Thus, this patch also moves to show the first IRQ > number which is from the first msi_entry for MSI-X. Hopefully, this can > make irq ABI more clear and more consistent. s/MSI-x/MSI-X/ s/irq/IRQ/ This looks like it should be two patches: - Update documentation - Some sort of bug fix in msix_capability_init() and pci_msix_shutdown() Perhaps do the bug fix first if you want the doc to match the code. > Signed-off-by: Barry Song <song.bao.hua@hisilicon.com> > --- > Documentation/ABI/testing/sysfs-bus-pci | 8 ++++++++ > drivers/pci/msi.c | 6 ++++++ > 2 files changed, 14 insertions(+) > > diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci > index 793cbb7..8d42385 100644 > --- a/Documentation/ABI/testing/sysfs-bus-pci > +++ b/Documentation/ABI/testing/sysfs-bus-pci > @@ -96,6 +96,14 @@ Description: > This attribute indicates the mode that the irq vector named by > the file is in (msi vs. msix) > > +What: /sys/bus/pci/devices/.../irq > +Date: August 2021 > +Contact: Barry Song <song.bao.hua@hisilicon.com> > +Description: > + Historically this attribute represent the IRQ line which runs > + from the PCI device to the Interrupt controller. With MSI and > + MSI-X, this attribute is the first IRQ number of IRQ vectors. > + > What: /sys/bus/pci/devices/.../remove > Date: January 2009 > Contact: Linux PCI developers <linux-pci@vger.kernel.org> > diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c > index 9232255..6bbf81b 100644 > --- a/drivers/pci/msi.c > +++ b/drivers/pci/msi.c > @@ -771,6 +771,7 @@ static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries, > int ret; > u16 control; > void __iomem *base; > + struct msi_desc *desc; > > /* Ensure MSI-X is disabled while it is set up */ > pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0); > @@ -814,6 +815,10 @@ static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries, > pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0); > > pcibios_free_irq(dev); > + > + desc = first_pci_msi_entry(dev); > + dev->irq = desc->irq; > + > return 0; > > out_avail: > @@ -1024,6 +1029,7 @@ static void pci_msix_shutdown(struct pci_dev *dev) > pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0); > pci_intx_for_msi(dev, 1); > dev->msix_enabled = 0; > + dev->irq = entry->msi_attrib.default_irq; > pcibios_alloc_irq(dev); > } > > -- > 1.8.3.1 >
diff --git a/Documentation/ABI/testing/sysfs-bus-pci b/Documentation/ABI/testing/sysfs-bus-pci index 793cbb7..8d42385 100644 --- a/Documentation/ABI/testing/sysfs-bus-pci +++ b/Documentation/ABI/testing/sysfs-bus-pci @@ -96,6 +96,14 @@ Description: This attribute indicates the mode that the irq vector named by the file is in (msi vs. msix) +What: /sys/bus/pci/devices/.../irq +Date: August 2021 +Contact: Barry Song <song.bao.hua@hisilicon.com> +Description: + Historically this attribute represent the IRQ line which runs + from the PCI device to the Interrupt controller. With MSI and + MSI-X, this attribute is the first IRQ number of IRQ vectors. + What: /sys/bus/pci/devices/.../remove Date: January 2009 Contact: Linux PCI developers <linux-pci@vger.kernel.org> diff --git a/drivers/pci/msi.c b/drivers/pci/msi.c index 9232255..6bbf81b 100644 --- a/drivers/pci/msi.c +++ b/drivers/pci/msi.c @@ -771,6 +771,7 @@ static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries, int ret; u16 control; void __iomem *base; + struct msi_desc *desc; /* Ensure MSI-X is disabled while it is set up */ pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0); @@ -814,6 +815,10 @@ static int msix_capability_init(struct pci_dev *dev, struct msix_entry *entries, pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_MASKALL, 0); pcibios_free_irq(dev); + + desc = first_pci_msi_entry(dev); + dev->irq = desc->irq; + return 0; out_avail: @@ -1024,6 +1029,7 @@ static void pci_msix_shutdown(struct pci_dev *dev) pci_msix_clear_and_set_ctrl(dev, PCI_MSIX_FLAGS_ENABLE, 0); pci_intx_for_msi(dev, 1); dev->msix_enabled = 0; + dev->irq = entry->msi_attrib.default_irq; pcibios_alloc_irq(dev); }