2022-11-11 14:33:48

by Thomas Gleixner

[permalink] [raw]
Subject: [patch 36/39] PCI/MSI: Validate MSIX contiguous restriction early

With interrupt domains the sanity check for MSI-X vector validation can be
done _before_ any allocation happens. The sanity check only applies to the
allocation functions which have an 'entries' array argument. The entries
array is filled by the caller with the requested MSI-X indicies. Some drivers
have gaps in the index space which is not supported on all architectures.

The PCI/MSI irqdomain has a 'feature' bit to enforce this validation late
during the allocation phase.

Just do it right away before doing any other work along with the other
sanity checks on that array.

Signed-off-by: Thomas Gleixner <[email protected]>
---
drivers/pci/msi/msi.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -725,13 +725,17 @@ static int msix_capability_init(struct p
return ret;
}

-static bool pci_msix_validate_entries(struct msix_entry *entries, int nvec, int hwsize)
+static bool pci_msix_validate_entries(struct pci_dev *dev, struct msix_entry *entries,
+ int nvec, int hwsize)
{
+ bool nogap;
int i, j;

if (!entries)
return true;

+ nogap = pci_msi_domain_supports(dev, MSI_FLAG_MSIX_CONTIGUOUS, DENY_LEGACY);
+
for (i = 0; i < nvec; i++) {
/* Entry within hardware limit? */
if (entries[i].entry >= hwsize)
@@ -742,6 +746,9 @@ static bool pci_msix_validate_entries(st
if (entries[i].entry == entries[j].entry)
return false;
}
+ /* Check for unsupported gaps */
+ if (nogap && entries[i].entry != i)
+ return false;
}
return true;
}
@@ -773,7 +780,7 @@ int __pci_enable_msix_range(struct pci_d
if (hwsize < 0)
return hwsize;

- if (!pci_msix_validate_entries(entries, nvec, hwsize))
+ if (!pci_msix_validate_entries(dev, entries, nvec, hwsize))
return -EINVAL;

/* PCI_IRQ_VIRTUAL is a horrible hack! */



2022-11-16 16:55:42

by Bjorn Helgaas

[permalink] [raw]
Subject: Re: [patch 36/39] PCI/MSI: Validate MSIX contiguous restriction early

On Fri, Nov 11, 2022 at 02:55:12PM +0100, Thomas Gleixner wrote:
> With interrupt domains the sanity check for MSI-X vector validation can be
> done _before_ any allocation happens. The sanity check only applies to the
> allocation functions which have an 'entries' array argument. The entries
> array is filled by the caller with the requested MSI-X indicies. Some drivers
> have gaps in the index space which is not supported on all architectures.
>
> The PCI/MSI irqdomain has a 'feature' bit to enforce this validation late
> during the allocation phase.
>
> Just do it right away before doing any other work along with the other
> sanity checks on that array.
>
> Signed-off-by: Thomas Gleixner <[email protected]>

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

s/indicies/indices/ (commit log)
s/irqdomain/irq domain/? IIRC previous logs used "irq domain"
s/MSIX/MSI-X/ (subject line)

> ---
> drivers/pci/msi/msi.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> --- a/drivers/pci/msi/msi.c
> +++ b/drivers/pci/msi/msi.c
> @@ -725,13 +725,17 @@ static int msix_capability_init(struct p
> return ret;
> }
>
> -static bool pci_msix_validate_entries(struct msix_entry *entries, int nvec, int hwsize)
> +static bool pci_msix_validate_entries(struct pci_dev *dev, struct msix_entry *entries,
> + int nvec, int hwsize)
> {
> + bool nogap;
> int i, j;
>
> if (!entries)
> return true;
>
> + nogap = pci_msi_domain_supports(dev, MSI_FLAG_MSIX_CONTIGUOUS, DENY_LEGACY);
> +
> for (i = 0; i < nvec; i++) {
> /* Entry within hardware limit? */
> if (entries[i].entry >= hwsize)
> @@ -742,6 +746,9 @@ static bool pci_msix_validate_entries(st
> if (entries[i].entry == entries[j].entry)
> return false;
> }
> + /* Check for unsupported gaps */
> + if (nogap && entries[i].entry != i)
> + return false;
> }
> return true;
> }
> @@ -773,7 +780,7 @@ int __pci_enable_msix_range(struct pci_d
> if (hwsize < 0)
> return hwsize;
>
> - if (!pci_msix_validate_entries(entries, nvec, hwsize))
> + if (!pci_msix_validate_entries(dev, entries, nvec, hwsize))
> return -EINVAL;
>
> /* PCI_IRQ_VIRTUAL is a horrible hack! */
>

2022-11-16 18:13:24

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [patch 36/39] PCI/MSI: Validate MSIX contiguous restriction early

On Fri, Nov 11, 2022 at 02:55:12PM +0100, Thomas Gleixner wrote:
> With interrupt domains the sanity check for MSI-X vector validation can be
> done _before_ any allocation happens. The sanity check only applies to the
> allocation functions which have an 'entries' array argument. The entries
> array is filled by the caller with the requested MSI-X indicies. Some drivers
> have gaps in the index space which is not supported on all architectures.
>
> The PCI/MSI irqdomain has a 'feature' bit to enforce this validation late
> during the allocation phase.
>
> Just do it right away before doing any other work along with the other
> sanity checks on that array.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> ---
> drivers/pci/msi/msi.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)

Reviewed-by: Jason Gunthorpe <[email protected]>

Jason

Subject: [tip: irq/core] PCI/MSI: Validate MSI-X contiguous restriction early

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

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

PCI/MSI: Validate MSI-X contiguous restriction early

With interrupt domains the sanity check for MSI-X vector validation can be
done _before_ any allocation happens. The sanity check only applies to the
allocation functions which have an 'entries' array argument. The entries
array is filled by the caller with the requested MSI-X indices. Some drivers
have gaps in the index space which is not supported on all architectures.

The PCI/MSI irq domain has a 'feature' bit to enforce this validation late
during the allocation phase.

Just do it right away before doing any other work along with the other
sanity checks on that array.

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/msi.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/msi/msi.c b/drivers/pci/msi/msi.c
index 0740acd..b94f6da 100644
--- a/drivers/pci/msi/msi.c
+++ b/drivers/pci/msi/msi.c
@@ -725,13 +725,17 @@ out_disable:
return ret;
}

-static bool pci_msix_validate_entries(struct msix_entry *entries, int nvec, int hwsize)
+static bool pci_msix_validate_entries(struct pci_dev *dev, struct msix_entry *entries,
+ int nvec, int hwsize)
{
+ bool nogap;
int i, j;

if (!entries)
return true;

+ nogap = pci_msi_domain_supports(dev, MSI_FLAG_MSIX_CONTIGUOUS, DENY_LEGACY);
+
for (i = 0; i < nvec; i++) {
/* Entry within hardware limit? */
if (entries[i].entry >= hwsize)
@@ -742,6 +746,9 @@ static bool pci_msix_validate_entries(struct msix_entry *entries, int nvec, int
if (entries[i].entry == entries[j].entry)
return false;
}
+ /* Check for unsupported gaps */
+ if (nogap && entries[i].entry != i)
+ return false;
}
return true;
}
@@ -773,7 +780,7 @@ int __pci_enable_msix_range(struct pci_dev *dev, struct msix_entry *entries, int
if (hwsize < 0)
return hwsize;

- if (!pci_msix_validate_entries(entries, nvec, hwsize))
+ if (!pci_msix_validate_entries(dev, entries, nvec, hwsize))
return -EINVAL;

/* PCI_IRQ_VIRTUAL is a horrible hack! */