Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH 0/2] PCI: endpoint: pci-epf-{v}ntb: A couple of fixes
@ 2026-04-07 12:44 Manivannan Sadhasivam
  2026-04-07 12:44 ` [PATCH 1/2] PCI: endpoint: pci-epf-vntb: Add check to detect 'db_count' value of 0 Manivannan Sadhasivam
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Manivannan Sadhasivam @ 2026-04-07 12:44 UTC (permalink / raw)
  To: mani, kwilczynski, kishon, bhelgaas
  Cc: jdmason, dave.jiang, allenbh, ntb, linux-pci, linux-kernel, den,
	Frank.li

Hi,

These two fixes are flagged by Sashiko during the review of doorbell series:
https://sashiko.dev/#/patchset/20260406155717.880246-1-den%40valinux.co.jp

Manivannan Sadhasivam (2):
  PCI: endpoint: pci-epf-vntb: Add check to detect 'db_count' value of 0
  PCI: endpoint: pci-epf-ntb: Add check to detect 'db_count' value of 0

 drivers/pci/endpoint/functions/pci-epf-ntb.c  | 15 +++++++++------
 drivers/pci/endpoint/functions/pci-epf-vntb.c |  9 +++------
 2 files changed, 12 insertions(+), 12 deletions(-)

-- 
2.51.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] PCI: endpoint: pci-epf-vntb: Add check to detect 'db_count' value of 0
  2026-04-07 12:44 [PATCH 0/2] PCI: endpoint: pci-epf-{v}ntb: A couple of fixes Manivannan Sadhasivam
@ 2026-04-07 12:44 ` Manivannan Sadhasivam
  2026-04-09  3:42   ` Frank Li
  2026-05-12  5:01   ` Koichiro Den
  2026-04-07 12:44 ` [PATCH 2/2] PCI: endpoint: pci-epf-ntb: " Manivannan Sadhasivam
  2026-05-12  5:34 ` [PATCH 0/2] PCI: endpoint: pci-epf-{v}ntb: A couple of fixes Krzysztof Wilczyński
  2 siblings, 2 replies; 7+ messages in thread
From: Manivannan Sadhasivam @ 2026-04-07 12:44 UTC (permalink / raw)
  To: mani, kwilczynski, kishon, bhelgaas
  Cc: jdmason, dave.jiang, allenbh, ntb, linux-pci, linux-kernel, den,
	Frank.li, Manivannan Sadhasivam

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

epf_ntb::db_count value should be within 1 to MAX_DB_COUNT. Current code
only checks for the upper bound, while the lower bound is unchecked. This
can cause a lot of issues in the driver if the user passes 'db_count' as 0.

So add a check for 0 also. While at it, remove the redundant 'db_count'
assignment.

Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/endpoint/functions/pci-epf-vntb.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index 2256c3062b1a..3d30aa4dbb84 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -483,7 +483,6 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
 {
 	const struct pci_epc_features *epc_features;
 	struct device *dev;
-	u32 db_count;
 	int ret;
 
 	dev = &ntb->epf->dev;
@@ -495,14 +494,12 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
 		return -EINVAL;
 	}
 
-	db_count = ntb->db_count;
-	if (db_count > MAX_DB_COUNT) {
-		dev_err(dev, "DB count cannot be more than %d\n", MAX_DB_COUNT);
+	if (!ntb->db_count || ntb->db_count > MAX_DB_COUNT) {
+		dev_err(dev, "DB count %d out of range (1 - %d)\n",
+			ntb->db_count, MAX_DB_COUNT);
 		return -EINVAL;
 	}
 
-	ntb->db_count = db_count;
-
 	if (epc_features->msi_capable) {
 		ret = pci_epc_set_msi(ntb->epf->epc,
 				      ntb->epf->func_no,
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/2] PCI: endpoint: pci-epf-ntb: Add check to detect 'db_count' value of 0
  2026-04-07 12:44 [PATCH 0/2] PCI: endpoint: pci-epf-{v}ntb: A couple of fixes Manivannan Sadhasivam
  2026-04-07 12:44 ` [PATCH 1/2] PCI: endpoint: pci-epf-vntb: Add check to detect 'db_count' value of 0 Manivannan Sadhasivam
@ 2026-04-07 12:44 ` Manivannan Sadhasivam
  2026-05-12  5:29   ` Krzysztof Wilczyński
  2026-05-12  5:34 ` [PATCH 0/2] PCI: endpoint: pci-epf-{v}ntb: A couple of fixes Krzysztof Wilczyński
  2 siblings, 1 reply; 7+ messages in thread
From: Manivannan Sadhasivam @ 2026-04-07 12:44 UTC (permalink / raw)
  To: mani, kwilczynski, kishon, bhelgaas
  Cc: jdmason, dave.jiang, allenbh, ntb, linux-pci, linux-kernel, den,
	Frank.li, Manivannan Sadhasivam

From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>

epf_ntb::db_count value should be within 1 to MAX_DB_COUNT. Current code
only checks for the upper bound, while the lower bound is unchecked. This
can cause a lot of issues in the driver if the user passes 'db_count' as 0.

So add a check for 0 also. While at it, remove the redundant 'db_count'
assignment.

Fixes: 8b821cf76150 ("PCI: endpoint: Add EP function driver to provide NTB functionality")
Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
---
 drivers/pci/endpoint/functions/pci-epf-ntb.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-ntb.c b/drivers/pci/endpoint/functions/pci-epf-ntb.c
index 2bdcc35b652c..7edd177f861f 100644
--- a/drivers/pci/endpoint/functions/pci-epf-ntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-ntb.c
@@ -559,12 +559,15 @@ static int epf_ntb_configure_db(struct epf_ntb *ntb,
 	struct pci_epc *epc;
 	int ret;
 
-	if (db_count > MAX_DB_COUNT)
-		return -EINVAL;
-
 	ntb_epc = ntb->epc[type];
 	epc = ntb_epc->epc;
 
+	if (!db_count || db_count > MAX_DB_COUNT) {
+		dev_err(&epc->dev, "DB count %d out of range (1 - %d)\n",
+			db_count, MAX_DB_COUNT);
+		return -EINVAL;
+	}
+
 	if (msix)
 		ret = epf_ntb_configure_msix(ntb, type, db_count);
 	else
@@ -1297,12 +1300,12 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb,
 	vfunc_no = ntb_epc->vfunc_no;
 
 	db_count = ntb->db_count;
-	if (db_count > MAX_DB_COUNT) {
-		dev_err(dev, "DB count cannot be more than %d\n", MAX_DB_COUNT);
+	if (!db_count || db_count > MAX_DB_COUNT) {
+		dev_err(dev, "DB count %d out of range (1 - %d)\n",
+			db_count, MAX_DB_COUNT);
 		return -EINVAL;
 	}
 
-	ntb->db_count = db_count;
 	epc = ntb_epc->epc;
 
 	if (msi_capable) {
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] PCI: endpoint: pci-epf-vntb: Add check to detect 'db_count' value of 0
  2026-04-07 12:44 ` [PATCH 1/2] PCI: endpoint: pci-epf-vntb: Add check to detect 'db_count' value of 0 Manivannan Sadhasivam
@ 2026-04-09  3:42   ` Frank Li
  2026-05-12  5:01   ` Koichiro Den
  1 sibling, 0 replies; 7+ messages in thread
From: Frank Li @ 2026-04-09  3:42 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: mani, kwilczynski, kishon, bhelgaas, jdmason, dave.jiang, allenbh,
	ntb, linux-pci, linux-kernel, den

On Tue, Apr 07, 2026 at 06:14:20PM +0530, Manivannan Sadhasivam wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
>
> epf_ntb::db_count value should be within 1 to MAX_DB_COUNT. Current code
> only checks for the upper bound, while the lower bound is unchecked. This
> can cause a lot of issues in the driver if the user passes 'db_count' as 0.
>
> So add a check for 0 also. While at it, remove the redundant 'db_count'
> assignment.
>
> Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>  drivers/pci/endpoint/functions/pci-epf-vntb.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index 2256c3062b1a..3d30aa4dbb84 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -483,7 +483,6 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
>  {
>  	const struct pci_epc_features *epc_features;
>  	struct device *dev;
> -	u32 db_count;
>  	int ret;
>
>  	dev = &ntb->epf->dev;
> @@ -495,14 +494,12 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
>  		return -EINVAL;
>  	}
>
> -	db_count = ntb->db_count;
> -	if (db_count > MAX_DB_COUNT) {
> -		dev_err(dev, "DB count cannot be more than %d\n", MAX_DB_COUNT);
> +	if (!ntb->db_count || ntb->db_count > MAX_DB_COUNT) {
> +		dev_err(dev, "DB count %d out of range (1 - %d)\n",
> +			ntb->db_count, MAX_DB_COUNT);
>  		return -EINVAL;
>  	}
>
> -	ntb->db_count = db_count;
> -
>  	if (epc_features->msi_capable) {
>  		ret = pci_epc_set_msi(ntb->epf->epc,
>  				      ntb->epf->func_no,
> --
> 2.51.0
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] PCI: endpoint: pci-epf-vntb: Add check to detect 'db_count' value of 0
  2026-04-07 12:44 ` [PATCH 1/2] PCI: endpoint: pci-epf-vntb: Add check to detect 'db_count' value of 0 Manivannan Sadhasivam
  2026-04-09  3:42   ` Frank Li
@ 2026-05-12  5:01   ` Koichiro Den
  1 sibling, 0 replies; 7+ messages in thread
From: Koichiro Den @ 2026-05-12  5:01 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: mani, kwilczynski, kishon, bhelgaas, jdmason, dave.jiang, allenbh,
	ntb, linux-pci, linux-kernel, Frank.li

On Tue, Apr 07, 2026 at 06:14:20PM +0530, Manivannan Sadhasivam wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> 
> epf_ntb::db_count value should be within 1 to MAX_DB_COUNT. Current code
> only checks for the upper bound, while the lower bound is unchecked. This
> can cause a lot of issues in the driver if the user passes 'db_count' as 0.
> 
> So add a check for 0 also. While at it, remove the redundant 'db_count'
> assignment.
> 
> Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@oss.qualcomm.com>
> ---

I noticed this one is still floating around. In case one more R-b helps:

Reviewed-by: Koichiro Den <den@valinux.co.jp>

P.S. For pci-epf-vntb, I think ntb->db_count needs to be at least 3 in practice
for doorbells to be useful, because of the link event slot (#0) and a
historically skipped slot (#1). Still, as a standalone fix, this patch looks
good to me.

>  drivers/pci/endpoint/functions/pci-epf-vntb.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index 2256c3062b1a..3d30aa4dbb84 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -483,7 +483,6 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
>  {
>  	const struct pci_epc_features *epc_features;
>  	struct device *dev;
> -	u32 db_count;
>  	int ret;
>  
>  	dev = &ntb->epf->dev;
> @@ -495,14 +494,12 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
>  		return -EINVAL;
>  	}
>  
> -	db_count = ntb->db_count;
> -	if (db_count > MAX_DB_COUNT) {
> -		dev_err(dev, "DB count cannot be more than %d\n", MAX_DB_COUNT);
> +	if (!ntb->db_count || ntb->db_count > MAX_DB_COUNT) {
> +		dev_err(dev, "DB count %d out of range (1 - %d)\n",
> +			ntb->db_count, MAX_DB_COUNT);
>  		return -EINVAL;
>  	}
>  
> -	ntb->db_count = db_count;
> -
>  	if (epc_features->msi_capable) {
>  		ret = pci_epc_set_msi(ntb->epf->epc,
>  				      ntb->epf->func_no,
> -- 
> 2.51.0
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] PCI: endpoint: pci-epf-ntb: Add check to detect 'db_count' value of 0
  2026-04-07 12:44 ` [PATCH 2/2] PCI: endpoint: pci-epf-ntb: " Manivannan Sadhasivam
@ 2026-05-12  5:29   ` Krzysztof Wilczyński
  0 siblings, 0 replies; 7+ messages in thread
From: Krzysztof Wilczyński @ 2026-05-12  5:29 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: mani, kishon, bhelgaas, jdmason, dave.jiang, allenbh, ntb,
	linux-pci, linux-kernel, den, Frank.li

Hello,

> @@ -1297,12 +1300,12 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb,
>  	vfunc_no = ntb_epc->vfunc_no;
>  
>  	db_count = ntb->db_count;
> -	if (db_count > MAX_DB_COUNT) {
> -		dev_err(dev, "DB count cannot be more than %d\n", MAX_DB_COUNT);
> +	if (!db_count || db_count > MAX_DB_COUNT) {
> +		dev_err(dev, "DB count %d out of range (1 - %d)\n",
> +			db_count, MAX_DB_COUNT);
>  		return -EINVAL;
>  	}

Something that I was wondering about here: would it make sense to also
remove this variable from here, too?  Even though it's referenced below
here (which is why I think you left it here).  Thoughts?

Thank you!

	Krzysztof

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/2] PCI: endpoint: pci-epf-{v}ntb: A couple of fixes
  2026-04-07 12:44 [PATCH 0/2] PCI: endpoint: pci-epf-{v}ntb: A couple of fixes Manivannan Sadhasivam
  2026-04-07 12:44 ` [PATCH 1/2] PCI: endpoint: pci-epf-vntb: Add check to detect 'db_count' value of 0 Manivannan Sadhasivam
  2026-04-07 12:44 ` [PATCH 2/2] PCI: endpoint: pci-epf-ntb: " Manivannan Sadhasivam
@ 2026-05-12  5:34 ` Krzysztof Wilczyński
  2 siblings, 0 replies; 7+ messages in thread
From: Krzysztof Wilczyński @ 2026-05-12  5:34 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: mani, kishon, bhelgaas, jdmason, dave.jiang, allenbh, ntb,
	linux-pci, linux-kernel, den, Frank.li

Hello,

> These two fixes are flagged by Sashiko during the review of doorbell series:
> https://sashiko.dev/#/patchset/20260406155717.880246-1-den%40valinux.co.jp
> 
> Manivannan Sadhasivam (2):
>   PCI: endpoint: pci-epf-vntb: Add check to detect 'db_count' value of 0
>   PCI: endpoint: pci-epf-ntb: Add check to detect 'db_count' value of 0
> 
>  drivers/pci/endpoint/functions/pci-epf-ntb.c  | 15 +++++++++------
>  drivers/pci/endpoint/functions/pci-epf-vntb.c |  9 +++------
>  2 files changed, 12 insertions(+), 12 deletions(-)

Might be out of scope, but since we removed one redundant "db_count"
variable already, perhaps we could also drop this one, too.  Just to add
a small clean-up since we are touching this code already...  Thoughts?

Looking at epf_ntb_db_mw_bar_init():

  (...)

  db_count = ntb->db_count;

  for (bar = BAR_DB_MW1, i = 0; i < num_mws; bar++, i++) {
        if (bar == BAR_DB_MW1) {
                align = align ? align : 4;
                size = db_count * align;
                size = ALIGN(size, ntb->mws_size[i]);
                ctrl = ntb_epc->reg;
                ctrl->mw1_offset = size;
                size += ntb->mws_size[i];
        } else {
                size = ntb->mws_size[i];
        }

        (...)
  }

Thank you!

	Krzysztof

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-05-12  5:34 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-07 12:44 [PATCH 0/2] PCI: endpoint: pci-epf-{v}ntb: A couple of fixes Manivannan Sadhasivam
2026-04-07 12:44 ` [PATCH 1/2] PCI: endpoint: pci-epf-vntb: Add check to detect 'db_count' value of 0 Manivannan Sadhasivam
2026-04-09  3:42   ` Frank Li
2026-05-12  5:01   ` Koichiro Den
2026-04-07 12:44 ` [PATCH 2/2] PCI: endpoint: pci-epf-ntb: " Manivannan Sadhasivam
2026-05-12  5:29   ` Krzysztof Wilczyński
2026-05-12  5:34 ` [PATCH 0/2] PCI: endpoint: pci-epf-{v}ntb: A couple of fixes Krzysztof Wilczyński

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox