Commit fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
introduced multi-MSI support with a broken allocation mechanism (it failed
to reserve the proper number of bits from the inner domain). Natural
alignment of the base vector number was also not guaranteed.
Fixes: fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
Reported-by: Pali Rohár <[email protected]>
Signed-off-by: Sandor Bodo-Merle <[email protected]>
---
drivers/pci/controller/pcie-iproc-msi.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
index eede4e8f3f75..557d93dcb3bc 100644
--- a/drivers/pci/controller/pcie-iproc-msi.c
+++ b/drivers/pci/controller/pcie-iproc-msi.c
@@ -252,18 +252,18 @@ static int iproc_msi_irq_domain_alloc(struct irq_domain *domain,
mutex_lock(&msi->bitmap_lock);
- /* Allocate 'nr_cpus' number of MSI vectors each time */
- hwirq = bitmap_find_next_zero_area(msi->bitmap, msi->nr_msi_vecs, 0,
- msi->nr_cpus, 0);
- if (hwirq < msi->nr_msi_vecs) {
- bitmap_set(msi->bitmap, hwirq, msi->nr_cpus);
- } else {
- mutex_unlock(&msi->bitmap_lock);
- return -ENOSPC;
- }
+ /*
+ * Allocate 'nr_irqs' multiplied by 'nr_cpus' number of MSI vectors
+ * each time
+ */
+ hwirq = bitmap_find_free_region(msi->bitmap, msi->nr_msi_vecs,
+ order_base_2(msi->nr_cpus * nr_irqs));
mutex_unlock(&msi->bitmap_lock);
+ if (hwirq < 0)
+ return -ENOSPC;
+
for (i = 0; i < nr_irqs; i++) {
irq_domain_set_info(domain, virq + i, hwirq + i,
&iproc_msi_bottom_irq_chip,
@@ -284,7 +284,8 @@ static void iproc_msi_irq_domain_free(struct irq_domain *domain,
mutex_lock(&msi->bitmap_lock);
hwirq = hwirq_to_canonical_hwirq(msi, data->hwirq);
- bitmap_clear(msi->bitmap, hwirq, msi->nr_cpus);
+ bitmap_release_region(msi->bitmap, hwirq,
+ order_base_2(msi->nr_cpus * nr_irqs));
mutex_unlock(&msi->bitmap_lock);
--
2.31.0
The interrupt affinity scheme used by this driver is incompatible with
multi-MSI as it implies moving the doorbell address to that of another MSI
group. This isn't possible for multi-MSI, as all the MSIs must have the
same doorbell address. As such it is restricted to systems with a single
CPU.
Fixes: fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
Reported-by: Marc Zyngier <[email protected]>
Signed-off-by: Sandor Bodo-Merle <[email protected]>
---
drivers/pci/controller/pcie-iproc-msi.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
index 557d93dcb3bc..81b4effeb130 100644
--- a/drivers/pci/controller/pcie-iproc-msi.c
+++ b/drivers/pci/controller/pcie-iproc-msi.c
@@ -171,7 +171,7 @@ static struct irq_chip iproc_msi_irq_chip = {
static struct msi_domain_info iproc_msi_domain_info = {
.flags = MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
- MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX,
+ MSI_FLAG_PCI_MSIX,
.chip = &iproc_msi_irq_chip,
};
@@ -250,6 +250,9 @@ static int iproc_msi_irq_domain_alloc(struct irq_domain *domain,
struct iproc_msi *msi = domain->host_data;
int hwirq, i;
+ if (msi->nr_cpus > 1 && nr_irqs > 1)
+ return -EINVAL;
+
mutex_lock(&msi->bitmap_lock);
/*
@@ -540,6 +543,9 @@ int iproc_msi_init(struct iproc_pcie *pcie, struct device_node *node)
mutex_init(&msi->bitmap_lock);
msi->nr_cpus = num_possible_cpus();
+ if (msi->nr_cpus == 1)
+ iproc_msi_domain_info.flags |= MSI_FLAG_MULTI_PCI_MSI;
+
msi->nr_irqs = of_irq_count(node);
if (!msi->nr_irqs) {
dev_err(pcie->dev, "found no MSI GIC interrupt\n");
--
2.31.0
On 2021-06-06 13:30, Sandor Bodo-Merle wrote:
> The interrupt affinity scheme used by this driver is incompatible with
> multi-MSI as it implies moving the doorbell address to that of another
> MSI
> group. This isn't possible for multi-MSI, as all the MSIs must have
> the
> same doorbell address. As such it is restricted to systems with a
> single
> CPU.
>
> Fixes: fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
> Reported-by: Marc Zyngier <[email protected]>
> Signed-off-by: Sandor Bodo-Merle <[email protected]>
Acked-by: Marc Zyngier <[email protected]>
M.
--
Jazz is not dead. It just smells funny...
On 2021-06-06 13:30, Sandor Bodo-Merle wrote:
> Commit fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
> introduced multi-MSI support with a broken allocation mechanism (it
> failed
> to reserve the proper number of bits from the inner domain). Natural
> alignment of the base vector number was also not guaranteed.
>
> Fixes: fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
> Reported-by: Pali Rohár <[email protected]>
> Signed-off-by: Sandor Bodo-Merle <[email protected]>
Acked-by: Marc Zyngier <[email protected]>
M.
--
Who you jivin' with that Cosmik Debris?
On Sunday 06 June 2021 14:30:43 Sandor Bodo-Merle wrote:
> Commit fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
> introduced multi-MSI support with a broken allocation mechanism (it failed
> to reserve the proper number of bits from the inner domain). Natural
> alignment of the base vector number was also not guaranteed.
>
> Fixes: fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
> Reported-by: Pali Rohár <[email protected]>
> Signed-off-by: Sandor Bodo-Merle <[email protected]>
Acked-by: Pali Rohár <[email protected]>
> ---
> drivers/pci/controller/pcie-iproc-msi.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
> index eede4e8f3f75..557d93dcb3bc 100644
> --- a/drivers/pci/controller/pcie-iproc-msi.c
> +++ b/drivers/pci/controller/pcie-iproc-msi.c
> @@ -252,18 +252,18 @@ static int iproc_msi_irq_domain_alloc(struct irq_domain *domain,
>
> mutex_lock(&msi->bitmap_lock);
>
> - /* Allocate 'nr_cpus' number of MSI vectors each time */
> - hwirq = bitmap_find_next_zero_area(msi->bitmap, msi->nr_msi_vecs, 0,
> - msi->nr_cpus, 0);
> - if (hwirq < msi->nr_msi_vecs) {
> - bitmap_set(msi->bitmap, hwirq, msi->nr_cpus);
> - } else {
> - mutex_unlock(&msi->bitmap_lock);
> - return -ENOSPC;
> - }
> + /*
> + * Allocate 'nr_irqs' multiplied by 'nr_cpus' number of MSI vectors
> + * each time
> + */
> + hwirq = bitmap_find_free_region(msi->bitmap, msi->nr_msi_vecs,
> + order_base_2(msi->nr_cpus * nr_irqs));
>
> mutex_unlock(&msi->bitmap_lock);
>
> + if (hwirq < 0)
> + return -ENOSPC;
> +
> for (i = 0; i < nr_irqs; i++) {
> irq_domain_set_info(domain, virq + i, hwirq + i,
> &iproc_msi_bottom_irq_chip,
> @@ -284,7 +284,8 @@ static void iproc_msi_irq_domain_free(struct irq_domain *domain,
> mutex_lock(&msi->bitmap_lock);
>
> hwirq = hwirq_to_canonical_hwirq(msi, data->hwirq);
> - bitmap_clear(msi->bitmap, hwirq, msi->nr_cpus);
> + bitmap_release_region(msi->bitmap, hwirq,
> + order_base_2(msi->nr_cpus * nr_irqs));
>
> mutex_unlock(&msi->bitmap_lock);
>
> --
> 2.31.0
>
On 6/6/2021 5:30 AM, Sandor Bodo-Merle wrote:
> The interrupt affinity scheme used by this driver is incompatible with
> multi-MSI as it implies moving the doorbell address to that of another MSI
> group. This isn't possible for multi-MSI, as all the MSIs must have the
> same doorbell address. As such it is restricted to systems with a single
> CPU.
>
> Fixes: fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
> Reported-by: Marc Zyngier <[email protected]>
> Signed-off-by: Sandor Bodo-Merle <[email protected]>
> ---
> drivers/pci/controller/pcie-iproc-msi.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
> index 557d93dcb3bc..81b4effeb130 100644
> --- a/drivers/pci/controller/pcie-iproc-msi.c
> +++ b/drivers/pci/controller/pcie-iproc-msi.c
> @@ -171,7 +171,7 @@ static struct irq_chip iproc_msi_irq_chip = {
>
> static struct msi_domain_info iproc_msi_domain_info = {
> .flags = MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> - MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX,
> + MSI_FLAG_PCI_MSIX,
> .chip = &iproc_msi_irq_chip,
> };
>
> @@ -250,6 +250,9 @@ static int iproc_msi_irq_domain_alloc(struct irq_domain *domain,
> struct iproc_msi *msi = domain->host_data;
> int hwirq, i;
>
> + if (msi->nr_cpus > 1 && nr_irqs > 1)
> + return -EINVAL;
> +
This should never happen since the framework would have guarded against
this. But I guess it does not hurt to have the check here.
> mutex_lock(&msi->bitmap_lock);
>
> /*
> @@ -540,6 +543,9 @@ int iproc_msi_init(struct iproc_pcie *pcie, struct device_node *node)
> mutex_init(&msi->bitmap_lock);
> msi->nr_cpus = num_possible_cpus();
>
> + if (msi->nr_cpus == 1)
> + iproc_msi_domain_info.flags |= MSI_FLAG_MULTI_PCI_MSI;
> +
> msi->nr_irqs = of_irq_count(node);
> if (!msi->nr_irqs) {
> dev_err(pcie->dev, "found no MSI GIC interrupt\n");
>
Looks fine to me. Thanks.
Acked-by: Ray Jui <[email protected]>
On 6/6/2021 5:30 AM, Sandor Bodo-Merle wrote:
> Commit fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
> introduced multi-MSI support with a broken allocation mechanism (it failed
> to reserve the proper number of bits from the inner domain). Natural
> alignment of the base vector number was also not guaranteed.
>
> Fixes: fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
> Reported-by: Pali Rohár <[email protected]>
> Signed-off-by: Sandor Bodo-Merle <[email protected]>
> ---
> drivers/pci/controller/pcie-iproc-msi.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
> index eede4e8f3f75..557d93dcb3bc 100644
> --- a/drivers/pci/controller/pcie-iproc-msi.c
> +++ b/drivers/pci/controller/pcie-iproc-msi.c
> @@ -252,18 +252,18 @@ static int iproc_msi_irq_domain_alloc(struct irq_domain *domain,
>
> mutex_lock(&msi->bitmap_lock);
>
> - /* Allocate 'nr_cpus' number of MSI vectors each time */
> - hwirq = bitmap_find_next_zero_area(msi->bitmap, msi->nr_msi_vecs, 0,
> - msi->nr_cpus, 0);
> - if (hwirq < msi->nr_msi_vecs) {
> - bitmap_set(msi->bitmap, hwirq, msi->nr_cpus);
> - } else {
> - mutex_unlock(&msi->bitmap_lock);
> - return -ENOSPC;
> - }
> + /*
> + * Allocate 'nr_irqs' multiplied by 'nr_cpus' number of MSI vectors
> + * each time
> + */
> + hwirq = bitmap_find_free_region(msi->bitmap, msi->nr_msi_vecs,
> + order_base_2(msi->nr_cpus * nr_irqs));
>
> mutex_unlock(&msi->bitmap_lock);
>
> + if (hwirq < 0)
> + return -ENOSPC;
> +
> for (i = 0; i < nr_irqs; i++) {
> irq_domain_set_info(domain, virq + i, hwirq + i,
> &iproc_msi_bottom_irq_chip,
> @@ -284,7 +284,8 @@ static void iproc_msi_irq_domain_free(struct irq_domain *domain,
> mutex_lock(&msi->bitmap_lock);
>
> hwirq = hwirq_to_canonical_hwirq(msi, data->hwirq);
> - bitmap_clear(msi->bitmap, hwirq, msi->nr_cpus);
> + bitmap_release_region(msi->bitmap, hwirq,
> + order_base_2(msi->nr_cpus * nr_irqs));
>
> mutex_unlock(&msi->bitmap_lock);
>
>
Looks good to me too. Thanks.
Acked-by: Ray Jui <[email protected]>
On Monday 07 June 2021 09:48:21 Ray Jui wrote:
> On 6/6/2021 5:30 AM, Sandor Bodo-Merle wrote:
> > The interrupt affinity scheme used by this driver is incompatible with
> > multi-MSI as it implies moving the doorbell address to that of another MSI
> > group. This isn't possible for multi-MSI, as all the MSIs must have the
> > same doorbell address. As such it is restricted to systems with a single
> > CPU.
> >
> > Fixes: fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
> > Reported-by: Marc Zyngier <[email protected]>
> > Signed-off-by: Sandor Bodo-Merle <[email protected]>
> > ---
> > drivers/pci/controller/pcie-iproc-msi.c | 8 +++++++-
> > 1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
> > index 557d93dcb3bc..81b4effeb130 100644
> > --- a/drivers/pci/controller/pcie-iproc-msi.c
> > +++ b/drivers/pci/controller/pcie-iproc-msi.c
> > @@ -171,7 +171,7 @@ static struct irq_chip iproc_msi_irq_chip = {
> >
> > static struct msi_domain_info iproc_msi_domain_info = {
> > .flags = MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> > - MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX,
> > + MSI_FLAG_PCI_MSIX,
> > .chip = &iproc_msi_irq_chip,
> > };
> >
> > @@ -250,6 +250,9 @@ static int iproc_msi_irq_domain_alloc(struct irq_domain *domain,
> > struct iproc_msi *msi = domain->host_data;
> > int hwirq, i;
> >
> > + if (msi->nr_cpus > 1 && nr_irqs > 1)
> > + return -EINVAL;
> > +
>
> This should never happen since the framework would have guarded against
> this. But I guess it does not hurt to have the check here.
Yes, this should not happen, but I suggested to add a comment or assert
or some other way to document this kind of constrain. Lot of times code
is copy+pasted to new drivers and because only this one driver has
.alloc function which is using nr_cpus for allocating msi bitmap, it
really makes sense to document this constrain also explicitly.
> > mutex_lock(&msi->bitmap_lock);
> >
> > /*
> > @@ -540,6 +543,9 @@ int iproc_msi_init(struct iproc_pcie *pcie, struct device_node *node)
> > mutex_init(&msi->bitmap_lock);
> > msi->nr_cpus = num_possible_cpus();
> >
> > + if (msi->nr_cpus == 1)
> > + iproc_msi_domain_info.flags |= MSI_FLAG_MULTI_PCI_MSI;
^^
Just a small note: there are two spaces instead of just one
Otherwise looks good to me:
Acked-by: Pali Rohár <[email protected]>
> > +
> > msi->nr_irqs = of_irq_count(node);
> > if (!msi->nr_irqs) {
> > dev_err(pcie->dev, "found no MSI GIC interrupt\n");
> >
>
> Looks fine to me. Thanks.
>
> Acked-by: Ray Jui <[email protected]>
On Sun, Jun 06, 2021 at 02:30:43PM +0200, Sandor Bodo-Merle wrote:
> Commit fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
> introduced multi-MSI support with a broken allocation mechanism (it failed
> to reserve the proper number of bits from the inner domain). Natural
> alignment of the base vector number was also not guaranteed.
>
> Fixes: fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
> Reported-by: Pali Roh?r <[email protected]>
> Signed-off-by: Sandor Bodo-Merle <[email protected]>
Looks good to me; thanks for splitting this. I think Lorenzo will
take care of this and maybe he'll also adjust the subject to match the
other patch, e.g.,
- PCI: iproc: fix the base vector number allocation for multi-MSI
+ PCI: iproc: Fix multi-MSI base vector number allocation
> ---
> drivers/pci/controller/pcie-iproc-msi.c | 21 +++++++++++----------
> 1 file changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
> index eede4e8f3f75..557d93dcb3bc 100644
> --- a/drivers/pci/controller/pcie-iproc-msi.c
> +++ b/drivers/pci/controller/pcie-iproc-msi.c
> @@ -252,18 +252,18 @@ static int iproc_msi_irq_domain_alloc(struct irq_domain *domain,
>
> mutex_lock(&msi->bitmap_lock);
>
> - /* Allocate 'nr_cpus' number of MSI vectors each time */
> - hwirq = bitmap_find_next_zero_area(msi->bitmap, msi->nr_msi_vecs, 0,
> - msi->nr_cpus, 0);
> - if (hwirq < msi->nr_msi_vecs) {
> - bitmap_set(msi->bitmap, hwirq, msi->nr_cpus);
> - } else {
> - mutex_unlock(&msi->bitmap_lock);
> - return -ENOSPC;
> - }
> + /*
> + * Allocate 'nr_irqs' multiplied by 'nr_cpus' number of MSI vectors
> + * each time
> + */
> + hwirq = bitmap_find_free_region(msi->bitmap, msi->nr_msi_vecs,
> + order_base_2(msi->nr_cpus * nr_irqs));
>
> mutex_unlock(&msi->bitmap_lock);
>
> + if (hwirq < 0)
> + return -ENOSPC;
> +
> for (i = 0; i < nr_irqs; i++) {
> irq_domain_set_info(domain, virq + i, hwirq + i,
> &iproc_msi_bottom_irq_chip,
> @@ -284,7 +284,8 @@ static void iproc_msi_irq_domain_free(struct irq_domain *domain,
> mutex_lock(&msi->bitmap_lock);
>
> hwirq = hwirq_to_canonical_hwirq(msi, data->hwirq);
> - bitmap_clear(msi->bitmap, hwirq, msi->nr_cpus);
> + bitmap_release_region(msi->bitmap, hwirq,
> + order_base_2(msi->nr_cpus * nr_irqs));
>
> mutex_unlock(&msi->bitmap_lock);
>
> --
> 2.31.0
>
On Sun, Jun 06, 2021 at 02:30:44PM +0200, Sandor Bodo-Merle wrote:
> The interrupt affinity scheme used by this driver is incompatible with
> multi-MSI as it implies moving the doorbell address to that of another MSI
> group. This isn't possible for multi-MSI, as all the MSIs must have the
> same doorbell address. As such it is restricted to systems with a single
> CPU.
>
> Fixes: fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
> Reported-by: Marc Zyngier <[email protected]>
> Signed-off-by: Sandor Bodo-Merle <[email protected]>
> ---
> drivers/pci/controller/pcie-iproc-msi.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
Can you just resend the series with the very minor changes requested
fixed please ?
Please carry/apply the review tags as well.
Thanks,
Lorenzo
> diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
> index 557d93dcb3bc..81b4effeb130 100644
> --- a/drivers/pci/controller/pcie-iproc-msi.c
> +++ b/drivers/pci/controller/pcie-iproc-msi.c
> @@ -171,7 +171,7 @@ static struct irq_chip iproc_msi_irq_chip = {
>
> static struct msi_domain_info iproc_msi_domain_info = {
> .flags = MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
> - MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX,
> + MSI_FLAG_PCI_MSIX,
> .chip = &iproc_msi_irq_chip,
> };
>
> @@ -250,6 +250,9 @@ static int iproc_msi_irq_domain_alloc(struct irq_domain *domain,
> struct iproc_msi *msi = domain->host_data;
> int hwirq, i;
>
> + if (msi->nr_cpus > 1 && nr_irqs > 1)
> + return -EINVAL;
> +
> mutex_lock(&msi->bitmap_lock);
>
> /*
> @@ -540,6 +543,9 @@ int iproc_msi_init(struct iproc_pcie *pcie, struct device_node *node)
> mutex_init(&msi->bitmap_lock);
> msi->nr_cpus = num_possible_cpus();
>
> + if (msi->nr_cpus == 1)
> + iproc_msi_domain_info.flags |= MSI_FLAG_MULTI_PCI_MSI;
> +
> msi->nr_irqs = of_irq_count(node);
> if (!msi->nr_irqs) {
> dev_err(pcie->dev, "found no MSI GIC interrupt\n");
> --
> 2.31.0
>
The interrupt affinity scheme used by this driver is incompatible with
multi-MSI as it implies moving the doorbell address to that of another MSI
group. This isn't possible for multi-MSI, as all the MSIs must have the
same doorbell address. As such it is restricted to systems with a single
CPU.
Fixes: fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
Reported-by: Marc Zyngier <[email protected]>
Signed-off-by: Sandor Bodo-Merle <[email protected]>
Acked-by: Marc Zyngier <[email protected]>
Acked-by: Pali Rohár <[email protected]>
Acked-by: Ray Jui <[email protected]>
---
drivers/pci/controller/pcie-iproc-msi.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
index 557d93dcb3bc..81b4effeb130 100644
--- a/drivers/pci/controller/pcie-iproc-msi.c
+++ b/drivers/pci/controller/pcie-iproc-msi.c
@@ -171,7 +171,7 @@ static struct irq_chip iproc_msi_irq_chip = {
static struct msi_domain_info iproc_msi_domain_info = {
.flags = MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
- MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX,
+ MSI_FLAG_PCI_MSIX,
.chip = &iproc_msi_irq_chip,
};
@@ -250,6 +250,9 @@ static int iproc_msi_irq_domain_alloc(struct irq_domain *domain,
struct iproc_msi *msi = domain->host_data;
int hwirq, i;
+ if (msi->nr_cpus > 1 && nr_irqs > 1)
+ return -EINVAL;
+
mutex_lock(&msi->bitmap_lock);
/*
@@ -540,6 +543,9 @@ int iproc_msi_init(struct iproc_pcie *pcie, struct device_node *node)
mutex_init(&msi->bitmap_lock);
msi->nr_cpus = num_possible_cpus();
+ if (msi->nr_cpus == 1)
+ iproc_msi_domain_info.flags |= MSI_FLAG_MULTI_PCI_MSI;
+
msi->nr_irqs = of_irq_count(node);
if (!msi->nr_irqs) {
dev_err(pcie->dev, "found no MSI GIC interrupt\n");
--
2.31.0
Commit fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
introduced multi-MSI support with a broken allocation mechanism (it failed
to reserve the proper number of bits from the inner domain). Natural
alignment of the base vector number was also not guaranteed.
Fixes: fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
Reported-by: Pali Rohár <[email protected]>
Signed-off-by: Sandor Bodo-Merle <[email protected]>
Acked-by: Marc Zyngier <[email protected]>
Acked-by: Pali Rohár <[email protected]>
Acked-by: Ray Jui <[email protected]>
---
drivers/pci/controller/pcie-iproc-msi.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/drivers/pci/controller/pcie-iproc-msi.c b/drivers/pci/controller/pcie-iproc-msi.c
index eede4e8f3f75..557d93dcb3bc 100644
--- a/drivers/pci/controller/pcie-iproc-msi.c
+++ b/drivers/pci/controller/pcie-iproc-msi.c
@@ -252,18 +252,18 @@ static int iproc_msi_irq_domain_alloc(struct irq_domain *domain,
mutex_lock(&msi->bitmap_lock);
- /* Allocate 'nr_cpus' number of MSI vectors each time */
- hwirq = bitmap_find_next_zero_area(msi->bitmap, msi->nr_msi_vecs, 0,
- msi->nr_cpus, 0);
- if (hwirq < msi->nr_msi_vecs) {
- bitmap_set(msi->bitmap, hwirq, msi->nr_cpus);
- } else {
- mutex_unlock(&msi->bitmap_lock);
- return -ENOSPC;
- }
+ /*
+ * Allocate 'nr_irqs' multiplied by 'nr_cpus' number of MSI vectors
+ * each time
+ */
+ hwirq = bitmap_find_free_region(msi->bitmap, msi->nr_msi_vecs,
+ order_base_2(msi->nr_cpus * nr_irqs));
mutex_unlock(&msi->bitmap_lock);
+ if (hwirq < 0)
+ return -ENOSPC;
+
for (i = 0; i < nr_irqs; i++) {
irq_domain_set_info(domain, virq + i, hwirq + i,
&iproc_msi_bottom_irq_chip,
@@ -284,7 +284,8 @@ static void iproc_msi_irq_domain_free(struct irq_domain *domain,
mutex_lock(&msi->bitmap_lock);
hwirq = hwirq_to_canonical_hwirq(msi, data->hwirq);
- bitmap_clear(msi->bitmap, hwirq, msi->nr_cpus);
+ bitmap_release_region(msi->bitmap, hwirq,
+ order_base_2(msi->nr_cpus * nr_irqs));
mutex_unlock(&msi->bitmap_lock);
--
2.31.0
On Tue, 22 Jun 2021 17:26:29 +0200, Sandor Bodo-Merle wrote:
> Commit fc54bae28818 ("PCI: iproc: Allow allocation of multiple MSIs")
> introduced multi-MSI support with a broken allocation mechanism (it failed
> to reserve the proper number of bits from the inner domain). Natural
> alignment of the base vector number was also not guaranteed.
Applied to pci/iproc, thanks!
[1/2] PCI: iproc: Fix multi-MSI base vector number allocation
https://git.kernel.org/lpieralisi/pci/c/e673d697b9
[2/2] PCI: iproc: Support multi-MSI only on uniprocessor kernel
https://git.kernel.org/lpieralisi/pci/c/2dc0a201d0
Thanks,
Lorenzo