diff mbox series

[34/39] PCI/MSI: Reject multi-MSI early

Message ID 20221111122015.574339988@linutronix.de
State New
Headers
Series genirq, PCI/MSI: Support for per device MSI and PCI/IMS - Part 1 cleanups |

Commit Message

Thomas Gleixner Nov. 11, 2022, 1:55 p.m. UTC 
  When hierarchical MSI interrupt domains are enabled then there is no point
to do tons of work and detect the missing support for multi-MSI late in the
allocation path.

Just query the domain feature flags right away. The query function is going
to be used for other purposes later and has a mode argument which influences
the result:

  ALLOW_LEGACY returns true when:
     - there is no irq domain attached (legacy support)
     - there is a irq domain attached which has the feature flag set

  DENY_LEGACY returns only true when:
     - there is a irq domain attached which has the feature flag set

This allows to use the function universally without ifdeffery in the
calling code.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
 drivers/pci/msi/irqdomain.c |   22 ++++++++++++++++++++++
 drivers/pci/msi/msi.c       |    4 ++++
 drivers/pci/msi/msi.h       |    9 +++++++++
 3 files changed, 35 insertions(+)

Comments

Bjorn Helgaas Nov. 16, 2022, 4:31 p.m. UTC | #1 
On Fri, Nov 11, 2022 at 02:55:09PM +0100, Thomas Gleixner wrote:
> When hierarchical MSI interrupt domains are enabled then there is no point
> to do tons of work and detect the missing support for multi-MSI late in the
> allocation path.
> 
> Just query the domain feature flags right away. The query function is going
> to be used for other purposes later and has a mode argument which influences
> the result:
> 
>   ALLOW_LEGACY returns true when:
>      - there is no irq domain attached (legacy support)
>      - there is a irq domain attached which has the feature flag set
> 
>   DENY_LEGACY returns only true when:
>      - there is a irq domain attached which has the feature flag set
> 
> This allows to use the function universally without ifdeffery in the
> calling code.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Acked-by: Bjorn Helgaas <bhelgaas@google.com>

> ---
>  drivers/pci/msi/irqdomain.c |   22 ++++++++++++++++++++++
>  drivers/pci/msi/msi.c       |    4 ++++
>  drivers/pci/msi/msi.h       |    9 +++++++++
>  3 files changed, 35 insertions(+)
> 
> --- a/drivers/pci/msi/irqdomain.c
> +++ b/drivers/pci/msi/irqdomain.c
> @@ -187,6 +187,28 @@ struct irq_domain *pci_msi_create_irq_do
>  }
>  EXPORT_SYMBOL_GPL(pci_msi_create_irq_domain);
>  
> +/**
> + * pci_msi_domain_supports - Check for support of a particular feature flag
> + * @pdev:		The PCI device to operate on
> + * @feature_mask:	The feature mask to check for (full match)
> + * @mode:		If ALLOW_LEGACY this grants the feature when there is no irq domain
> + *			associated to the device. If DENY_LEGACY the lack of an irq domain
> + *			makes the feature unsupported

Looks like some of these might be wider than 80 columns, which I think
was the typical width of this file.

> + */
> +bool pci_msi_domain_supports(struct pci_dev *pdev, unsigned int feature_mask,
> +			     enum support_mode mode)
> +{
> +	struct msi_domain_info *info;
> +	struct irq_domain *domain;
> +
> +	domain = dev_get_msi_domain(&pdev->dev);
> +
> +	if (!domain || !irq_domain_is_hierarchy(domain))
> +		return mode == ALLOW_LEGACY;
> +	info = domain->host_data;
> +	return (info->flags & feature_mask) == feature_mask;
> +}
> +
>  /*
>   * Users of the generic MSI infrastructure expect a device to have a single ID,
>   * so with DMA aliases we have to pick the least-worst compromise. Devices with
> --- a/drivers/pci/msi/msi.c
> +++ b/drivers/pci/msi/msi.c
> @@ -347,6 +347,10 @@ static int msi_capability_init(struct pc
>  	struct msi_desc *entry;
>  	int ret;
>  
> +	/* Reject multi-MSI early on irq domain enabled architectures */
> +	if (nvec > 1 && !pci_msi_domain_supports(dev, MSI_FLAG_MULTI_PCI_MSI, ALLOW_LEGACY))
> +		return 1;
> +
>  	/*
>  	 * Disable MSI during setup in the hardware, but mark it enabled
>  	 * so that setup code can evaluate it.
> --- a/drivers/pci/msi/msi.h
> +++ b/drivers/pci/msi/msi.h
> @@ -97,6 +97,15 @@ int __pci_enable_msix_range(struct pci_d
>  void __pci_restore_msi_state(struct pci_dev *dev);
>  void __pci_restore_msix_state(struct pci_dev *dev);
>  
> +/* irq_domain related functionality */
> +
> +enum support_mode {
> +	ALLOW_LEGACY,
> +	DENY_LEGACY,
> +};
> +
> +bool pci_msi_domain_supports(struct pci_dev *dev, unsigned int feature_mask, enum support_mode mode);
> +
>  /* Legacy (!IRQDOMAIN) fallbacks */
>  
>  #ifdef CONFIG_PCI_MSI_ARCH_FALLBACKS
>
Jason Gunthorpe Nov. 16, 2022, 5:59 p.m. UTC | #2 
On Fri, Nov 11, 2022 at 02:55:09PM +0100, Thomas Gleixner wrote:
> When hierarchical MSI interrupt domains are enabled then there is no point
> to do tons of work and detect the missing support for multi-MSI late in the
> allocation path.
> 
> Just query the domain feature flags right away. The query function is going
> to be used for other purposes later and has a mode argument which influences
> the result:
> 
>   ALLOW_LEGACY returns true when:
>      - there is no irq domain attached (legacy support)
>      - there is a irq domain attached which has the feature flag set
> 
>   DENY_LEGACY returns only true when:
>      - there is a irq domain attached which has the feature flag set
> 
> This allows to use the function universally without ifdeffery in the
> calling code.
> 
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
>  drivers/pci/msi/irqdomain.c |   22 ++++++++++++++++++++++
>  drivers/pci/msi/msi.c       |    4 ++++
>  drivers/pci/msi/msi.h       |    9 +++++++++
>  3 files changed, 35 insertions(+)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason
Thomas Gleixner Nov. 17, 2022, 8:22 a.m. UTC | #3 
On Wed, Nov 16 2022 at 10:31, Bjorn Helgaas wrote:
> On Fri, Nov 11, 2022 at 02:55:09PM +0100, Thomas Gleixner wrote:
>>  
>> +/**
>> + * pci_msi_domain_supports - Check for support of a particular feature flag
>> + * @pdev:		The PCI device to operate on
>> + * @feature_mask:	The feature mask to check for (full match)
>> + * @mode:		If ALLOW_LEGACY this grants the feature when there is no irq domain
>> + *			associated to the device. If DENY_LEGACY the lack of an irq domain
>> + *			makes the feature unsupported
>
> Looks like some of these might be wider than 80 columns, which I think
> was the typical width of this file.

I accommodated to the general sentiment that 80 columns is yesterday. My
new cutoff is 100.

Thanks,

        tglx
diff mbox series

Patch

--- a/drivers/pci/msi/irqdomain.c
+++ b/drivers/pci/msi/irqdomain.c
@@ -187,6 +187,28 @@  struct irq_domain *pci_msi_create_irq_do
 }
 EXPORT_SYMBOL_GPL(pci_msi_create_irq_domain);
 
+/**
+ * pci_msi_domain_supports - Check for support of a particular feature flag
+ * @pdev:		The PCI device to operate on
+ * @feature_mask:	The feature mask to check for (full match)
+ * @mode:		If ALLOW_LEGACY this grants the feature when there is no irq domain
+ *			associated to the device. If DENY_LEGACY the lack of an irq domain
+ *			makes the feature unsupported
+ */
+bool pci_msi_domain_supports(struct pci_dev *pdev, unsigned int feature_mask,
+			     enum support_mode mode)
+{
+	struct msi_domain_info *info;
+	struct irq_domain *domain;
+
+	domain = dev_get_msi_domain(&pdev->dev);
+
+	if (!domain || !irq_domain_is_hierarchy(domain))
+		return mode == ALLOW_LEGACY;
+	info = domain->host_data;
+	return (info->flags & feature_mask) == feature_mask;
+}
+
 /*
  * Users of the generic MSI infrastructure expect a device to have a single ID,
  * so with DMA aliases we have to pick the least-worst compromise. Devices with
--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -347,6 +347,10 @@  static int msi_capability_init(struct pc
 	struct msi_desc *entry;
 	int ret;
 
+	/* Reject multi-MSI early on irq domain enabled architectures */
+	if (nvec > 1 && !pci_msi_domain_supports(dev, MSI_FLAG_MULTI_PCI_MSI, ALLOW_LEGACY))
+		return 1;
+
 	/*
 	 * Disable MSI during setup in the hardware, but mark it enabled
 	 * so that setup code can evaluate it.
--- a/drivers/pci/msi/msi.h
+++ b/drivers/pci/msi/msi.h
@@ -97,6 +97,15 @@  int __pci_enable_msix_range(struct pci_d
 void __pci_restore_msi_state(struct pci_dev *dev);
 void __pci_restore_msix_state(struct pci_dev *dev);
 
+/* irq_domain related functionality */
+
+enum support_mode {
+	ALLOW_LEGACY,
+	DENY_LEGACY,
+};
+
+bool pci_msi_domain_supports(struct pci_dev *dev, unsigned int feature_mask, enum support_mode mode);
+
 /* Legacy (!IRQDOMAIN) fallbacks */
 
 #ifdef CONFIG_PCI_MSI_ARCH_FALLBACKS