2022-11-11 14:25:05

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 34/39] PCI/MSI: Reject multi-MSI early

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 <[email protected]>
---
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
+ */
+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



2022-11-16 17:06:03

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [patch 34/39] PCI/MSI: Reject multi-MSI early

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 <[email protected]>

Acked-by: Bjorn Helgaas <[email protected]>

> ---
> 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
>

2022-11-16 18:47:19

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [patch 34/39] PCI/MSI: Reject multi-MSI early

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 <[email protected]>
> ---
> 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 <[email protected]>

Jason

2022-11-17 09:18:33

by Thomas Gleixner

[permalink] [raw]
Subject: Re: [patch 34/39] PCI/MSI: Reject multi-MSI early

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

Subject: [tip: irq/core] PCI/MSI: Reject multi-MSI early

The following commit has been merged into the irq/core branch of tip:

Commit-ID: d2a463b297415ca6dd4d60bb1c867dd7c931587b
Gitweb: https://git.kernel.org/tip/d2a463b297415ca6dd4d60bb1c867dd7c931587b
Author: Thomas Gleixner <[email protected]>
AuthorDate: Fri, 11 Nov 2022 14:55:09 +01:00
Committer: Thomas Gleixner <[email protected]>
CommitterDate: Thu, 17 Nov 2022 15:15:22 +01:00

PCI/MSI: Reject multi-MSI early

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 <[email protected]>
Reviewed-by: Jason Gunthorpe <[email protected]>
Acked-by: Bjorn Helgaas <[email protected]>
Link: https://lore.kernel.org/r/[email protected]

---
drivers/pci/msi/irqdomain.c | 22 ++++++++++++++++++++++
drivers/pci/msi/msi.c | 4 ++++
drivers/pci/msi/msi.h | 9 +++++++++
3 files changed, 35 insertions(+)

diff --git a/drivers/pci/msi/irqdomain.c b/drivers/pci/msi/irqdomain.c
index edd0cc2..666ed21 100644
--- a/drivers/pci/msi/irqdomain.c
+++ b/drivers/pci/msi/irqdomain.c
@@ -187,6 +187,28 @@ struct irq_domain *pci_msi_create_irq_domain(struct fwnode_handle *fwnode,
}
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
diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c
index 794ec97..bc84647 100644
--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -347,6 +347,10 @@ static int msi_capability_init(struct pci_dev *dev, int nvec,
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.
diff --git a/drivers/pci/msi/msi.h b/drivers/pci/msi/msi.h
index 8170ef2..9d75b6f 100644
--- a/drivers/pci/msi/msi.h
+++ b/drivers/pci/msi/msi.h
@@ -97,6 +97,15 @@ int __pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries, int
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