linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PCI/MSI: Clarify the irq sysfs ABI for PCI devices
@ 2021-08-13 12:26 Barry Song
  2021-08-14  8:24 ` Greg KH
  2021-08-20 21:43 ` Bjorn Helgaas
  0 siblings, 2 replies; 3+ messages in thread
From: Barry Song @ 2021-08-13 12:26 UTC (permalink / raw)
  To: bhelgaas, corbet, linux-kernel, linux-pci
  Cc: mchehab+huawei, gregkh, Jonathan.Cameron, leon, schnelle, bilbao,
	luzmaximilian, linuxarm, Barry Song

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(+)

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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] PCI/MSI: Clarify the irq sysfs ABI for PCI devices
  2021-08-13 12:26 [PATCH] PCI/MSI: Clarify the irq sysfs ABI for PCI devices Barry Song
@ 2021-08-14  8:24 ` Greg KH
  2021-08-20 21:43 ` Bjorn Helgaas
  1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2021-08-14  8:24 UTC (permalink / raw)
  To: Barry Song
  Cc: bhelgaas, corbet, linux-kernel, linux-pci, mchehab+huawei,
	Jonathan.Cameron, leon, schnelle, bilbao, luzmaximilian,
	linuxarm, Barry Song

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>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] PCI/MSI: Clarify the irq sysfs ABI for PCI devices
  2021-08-13 12:26 [PATCH] PCI/MSI: Clarify the irq sysfs ABI for PCI devices Barry Song
  2021-08-14  8:24 ` Greg KH
@ 2021-08-20 21:43 ` Bjorn Helgaas
  1 sibling, 0 replies; 3+ messages in thread
From: Bjorn Helgaas @ 2021-08-20 21:43 UTC (permalink / raw)
  To: Barry Song
  Cc: bhelgaas, corbet, linux-kernel, linux-pci, mchehab+huawei,
	gregkh, Jonathan.Cameron, leon, schnelle, bilbao, luzmaximilian,
	linuxarm, Barry Song

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
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-08-20 21:43 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-13 12:26 [PATCH] PCI/MSI: Clarify the irq sysfs ABI for PCI devices Barry Song
2021-08-14  8:24 ` Greg KH
2021-08-20 21:43 ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).