diff mbox series

[v2] PCI: xilinx-nwl: Fix Multi MSI data programming

Message ID 1552304759-5394-1-git-send-email-bharat.kumar.gogada@xilinx.com (mailing list archive)
State Superseded, archived
Headers show
Series [v2] PCI: xilinx-nwl: Fix Multi MSI data programming | expand

Commit Message

Bharat Kumar Gogada March 11, 2019, 11:45 a.m. UTC
The current Multi MSI data programming fails if multiple end points
requesting MSI and multi MSI are connected with switch, i.e the current
multi MSI data being given is not considering the number of vectors
being requested in case of multi MSI.
Due to this if multiple end points are connected and requesting MSI
and multi MSI combination, the multi MSI data is ending up using
wrong MSI data, which might be used by different device.

Fix Multi MSI data programming with required alignment by
using number of vectors being requested.

Fixes: ab597d35ef11 ("PCI: xilinx-nwl: Add support for Xilinx NWL PCIe
Host Controller")
Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
---
V2:
 - Added more description of fix
---
 drivers/pci/controller/pcie-xilinx-nwl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Bharat Kumar Gogada April 1, 2019, 5 p.m. UTC | #1
Hi All,

Please let me know if anyone has any inputs on this.

Regards,
Bharat
> 
> The current Multi MSI data programming fails if multiple end points
> requesting MSI and multi MSI are connected with switch, i.e the current
> multi MSI data being given is not considering the number of vectors being
> requested in case of multi MSI.
> Due to this if multiple end points are connected and requesting MSI and
> multi MSI combination, the multi MSI data is ending up using wrong MSI
> data, which might be used by different device.
> 
> Fix Multi MSI data programming with required alignment by using number
> of vectors being requested.
> 
> Fixes: ab597d35ef11 ("PCI: xilinx-nwl: Add support for Xilinx NWL PCIe Host
> Controller")
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> ---
> V2:
>  - Added more description of fix
> ---
>  drivers/pci/controller/pcie-xilinx-nwl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c
> b/drivers/pci/controller/pcie-xilinx-nwl.c
> index 81538d7..36669c5 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -484,7 +484,7 @@ static int nwl_irq_domain_alloc(struct irq_domain
> *domain, unsigned int virq,
> 
>  	mutex_lock(&msi->lock);
>  	bit = bitmap_find_next_zero_area(msi->bitmap, INT_PCI_MSI_NR, 0,
> -					 nr_irqs, 0);
> +					 nr_irqs, nr_irqs - 1);
>  	if (bit >= INT_PCI_MSI_NR) {
>  		mutex_unlock(&msi->lock);
>  		return -ENOSPC;
> --
> 2.7.4
Lorenzo Pieralisi April 2, 2019, 4:08 p.m. UTC | #2
On Mon, Apr 01, 2019 at 05:00:40PM +0000, Bharat Kumar Gogada wrote:
> Hi All,
> 
> Please let me know if anyone has any inputs on this.
> 
> Regards,
> Bharat
> > 
> > The current Multi MSI data programming fails if multiple end points
> > requesting MSI and multi MSI are connected with switch, i.e the current
> > multi MSI data being given is not considering the number of vectors being
> > requested in case of multi MSI.
> > Due to this if multiple end points are connected and requesting MSI and
> > multi MSI combination, the multi MSI data is ending up using wrong MSI
> > data, which might be used by different device.
> > 
> > Fix Multi MSI data programming with required alignment by using number
> > of vectors being requested.

I still do not understand what you mean I am sorry. An example is worth
two thousand words, I would start with that as it stands this commit
log does not provide any information on what you are actually fixing.

Lorenzo

> > Fixes: ab597d35ef11 ("PCI: xilinx-nwl: Add support for Xilinx NWL PCIe Host
> > Controller")
> > Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> > ---
> > V2:
> >  - Added more description of fix
> > ---
> >  drivers/pci/controller/pcie-xilinx-nwl.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c
> > b/drivers/pci/controller/pcie-xilinx-nwl.c
> > index 81538d7..36669c5 100644
> > --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> > +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> > @@ -484,7 +484,7 @@ static int nwl_irq_domain_alloc(struct irq_domain
> > *domain, unsigned int virq,
> > 
> >  	mutex_lock(&msi->lock);
> >  	bit = bitmap_find_next_zero_area(msi->bitmap, INT_PCI_MSI_NR, 0,
> > -					 nr_irqs, 0);
> > +					 nr_irqs, nr_irqs - 1);
> >  	if (bit >= INT_PCI_MSI_NR) {
> >  		mutex_unlock(&msi->lock);
> >  		return -ENOSPC;
> > --
> > 2.7.4
>
Ard Biesheuvel April 2, 2019, 4:23 p.m. UTC | #3
On Mon, 11 Mar 2019 at 12:46, Bharat Kumar Gogada
<bharat.kumar.gogada@xilinx.com> wrote:
>
> The current Multi MSI data programming fails if multiple end points
> requesting MSI and multi MSI are connected with switch, i.e the current
> multi MSI data being given is not considering the number of vectors
> being requested in case of multi MSI.
> Due to this if multiple end points are connected and requesting MSI
> and multi MSI combination, the multi MSI data is ending up using
> wrong MSI data, which might be used by different device.
>
> Fix Multi MSI data programming with required alignment by
> using number of vectors being requested.
>
> Fixes: ab597d35ef11 ("PCI: xilinx-nwl: Add support for Xilinx NWL PCIe
> Host Controller")
> Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> ---
> V2:
>  - Added more description of fix
> ---
>  drivers/pci/controller/pcie-xilinx-nwl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
> index 81538d7..36669c5 100644
> --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> @@ -484,7 +484,7 @@ static int nwl_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
>
>         mutex_lock(&msi->lock);
>         bit = bitmap_find_next_zero_area(msi->bitmap, INT_PCI_MSI_NR, 0,
> -                                        nr_irqs, 0);
> +                                        nr_irqs, nr_irqs - 1);

Are you sure nr_irqs is guaranteed to be a power of 2?

>         if (bit >= INT_PCI_MSI_NR) {
>                 mutex_unlock(&msi->lock);
>                 return -ENOSPC;
> --
> 2.7.4
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Bharat Kumar Gogada April 4, 2019, 11:48 a.m. UTC | #4
> On Mon, Apr 01, 2019 at 05:00:40PM +0000, Bharat Kumar Gogada wrote:
> > Hi All,
> >
> > Please let me know if anyone has any inputs on this.
> >
> > Regards,
> > Bharat
> > >
> > > The current Multi MSI data programming fails if multiple end points
> > > requesting MSI and multi MSI are connected with switch, i.e the
> > > current multi MSI data being given is not considering the number of
> > > vectors being requested in case of multi MSI.
> > > Due to this if multiple end points are connected and requesting MSI
> > > and multi MSI combination, the multi MSI data is ending up using
> > > wrong MSI data, which might be used by different device.
> > >
> > > Fix Multi MSI data programming with required alignment by using
> > > number of vectors being requested.
> 
> I still do not understand what you mean I am sorry. An example is worth
> two thousand words, I would start with that as it stands this commit log
> does not provide any information on what you are actually fixing.
>
Hi Lorenzo, Thanks for your time. 
Yes, will add an example to describe the problem.
 
Regards,
Bharat
> > > Fixes: ab597d35ef11 ("PCI: xilinx-nwl: Add support for Xilinx NWL
> > > PCIe Host
> > > Controller")
> > > Signed-off-by: Bharat Kumar Gogada
> <bharat.kumar.gogada@xilinx.com>
> > > ---
> > > V2:
> > >  - Added more description of fix
> > > ---
> > >  drivers/pci/controller/pcie-xilinx-nwl.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c
> > > b/drivers/pci/controller/pcie-xilinx-nwl.c
> > > index 81538d7..36669c5 100644
> > > --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> > > +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> > > @@ -484,7 +484,7 @@ static int nwl_irq_domain_alloc(struct
> > > irq_domain *domain, unsigned int virq,
> > >
> > >  	mutex_lock(&msi->lock);
> > >  	bit = bitmap_find_next_zero_area(msi->bitmap, INT_PCI_MSI_NR, 0,
> > > -					 nr_irqs, 0);
> > > +					 nr_irqs, nr_irqs - 1);
> > >  	if (bit >= INT_PCI_MSI_NR) {
> > >  		mutex_unlock(&msi->lock);
> > >  		return -ENOSPC;
> > > --
> > > 2.7.4
> >
Bharat Kumar Gogada April 4, 2019, 11:51 a.m. UTC | #5
> Subject: Re: [PATCH v2] PCI: xilinx-nwl: Fix Multi MSI data programming
> 
> On Mon, 11 Mar 2019 at 12:46, Bharat Kumar Gogada
> <bharat.kumar.gogada@xilinx.com> wrote:
> >
> > The current Multi MSI data programming fails if multiple end points
> > requesting MSI and multi MSI are connected with switch, i.e the
> > current multi MSI data being given is not considering the number of
> > vectors being requested in case of multi MSI.
> > Due to this if multiple end points are connected and requesting MSI
> > and multi MSI combination, the multi MSI data is ending up using wrong
> > MSI data, which might be used by different device.
> >
> > Fix Multi MSI data programming with required alignment by using number
> > of vectors being requested.
> >
> > Fixes: ab597d35ef11 ("PCI: xilinx-nwl: Add support for Xilinx NWL PCIe
> > Host Controller")
> > Signed-off-by: Bharat Kumar Gogada <bharat.kumar.gogada@xilinx.com>
> > ---
> > V2:
> >  - Added more description of fix
> > ---
> >  drivers/pci/controller/pcie-xilinx-nwl.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c
> > b/drivers/pci/controller/pcie-xilinx-nwl.c
> > index 81538d7..36669c5 100644
> > --- a/drivers/pci/controller/pcie-xilinx-nwl.c
> > +++ b/drivers/pci/controller/pcie-xilinx-nwl.c
> > @@ -484,7 +484,7 @@ static int nwl_irq_domain_alloc(struct irq_domain
> > *domain, unsigned int virq,
> >
> >         mutex_lock(&msi->lock);
> >         bit = bitmap_find_next_zero_area(msi->bitmap, INT_PCI_MSI_NR, 0,
> > -                                        nr_irqs, 0);
> > +                                        nr_irqs, nr_irqs - 1);
> 
> Are you sure nr_irqs is guaranteed to be a power of 2?
Hi Ard, Thanks for your time.
After checking other source codes its not guaranteed, will add extra check and resend patch.

Regards,
Bharat
> 
> >         if (bit >= INT_PCI_MSI_NR) {
> >                 mutex_unlock(&msi->lock);
> >                 return -ENOSPC;
> > --
> > 2.7.4
> >
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
diff mbox series

Patch

diff --git a/drivers/pci/controller/pcie-xilinx-nwl.c b/drivers/pci/controller/pcie-xilinx-nwl.c
index 81538d7..36669c5 100644
--- a/drivers/pci/controller/pcie-xilinx-nwl.c
+++ b/drivers/pci/controller/pcie-xilinx-nwl.c
@@ -484,7 +484,7 @@  static int nwl_irq_domain_alloc(struct irq_domain *domain, unsigned int virq,
 
 	mutex_lock(&msi->lock);
 	bit = bitmap_find_next_zero_area(msi->bitmap, INT_PCI_MSI_NR, 0,
-					 nr_irqs, 0);
+					 nr_irqs, nr_irqs - 1);
 	if (bit >= INT_PCI_MSI_NR) {
 		mutex_unlock(&msi->lock);
 		return -ENOSPC;