* [PATCH 1/2] PCI: endpoint: Clean up error handling in vpci_scan_bus()
2024-06-10 9:32 [PATCH 0/2] PCI: endpoint: fix a couple error handling bugs Dan Carpenter
@ 2024-06-10 9:33 ` Dan Carpenter
2024-06-14 12:57 ` Ilpo Järvinen
2024-06-10 9:33 ` [PATCH 2/2] PCI: endpoint: Fix epf_ntb_epc_cleanup() a bit Dan Carpenter
2024-07-01 20:34 ` [PATCH 0/2] PCI: endpoint: fix a couple error handling bugs Krzysztof Wilczyński
2 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2024-06-10 9:33 UTC (permalink / raw)
To: Frank Li
Cc: Jon Mason, Dave Jiang, Allen Hubbe, Manivannan Sadhasivam,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
ntb, linux-pci, linux-kernel, kernel-janitors
Smatch complains about inconsistent NULL checking in vpci_scan_bus():
drivers/pci/endpoint/functions/pci-epf-vntb.c:1024 vpci_scan_bus()
error: we previously assumed 'vpci_bus' could be null (see line 1021)
Instead of printing an error message and then crashing we should return
an error code and clean up. Also the NULL check is reversed so it
prints an error for success instead of failure.
Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
drivers/pci/endpoint/functions/pci-epf-vntb.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index 8e779eecd62d..7f05a44e9a9f 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -1018,8 +1018,10 @@ static int vpci_scan_bus(void *sysdata)
struct epf_ntb *ndev = sysdata;
vpci_bus = pci_scan_bus(ndev->vbus_number, &vpci_ops, sysdata);
- if (vpci_bus)
- pr_err("create pci bus\n");
+ if (!vpci_bus) {
+ pr_err("create pci bus failed\n");
+ return -EINVAL;
+ }
pci_bus_add_devices(vpci_bus);
@@ -1338,10 +1340,14 @@ static int epf_ntb_bind(struct pci_epf *epf)
goto err_bar_alloc;
}
- vpci_scan_bus(ntb);
+ ret = vpci_scan_bus(ntb);
+ if (ret)
+ goto err_unregister;
return 0;
+err_unregister:
+ pci_unregister_driver(&vntb_pci_driver);
err_bar_alloc:
epf_ntb_config_spad_bar_free(ntb);
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] PCI: endpoint: Clean up error handling in vpci_scan_bus()
2024-06-10 9:33 ` [PATCH 1/2] PCI: endpoint: Clean up error handling in vpci_scan_bus() Dan Carpenter
@ 2024-06-14 12:57 ` Ilpo Järvinen
0 siblings, 0 replies; 6+ messages in thread
From: Ilpo Järvinen @ 2024-06-14 12:57 UTC (permalink / raw)
To: Dan Carpenter
Cc: Frank Li, Jon Mason, Dave Jiang, Allen Hubbe,
Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, ntb, linux-pci, LKML,
kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1812 bytes --]
On Mon, 10 Jun 2024, Dan Carpenter wrote:
> Smatch complains about inconsistent NULL checking in vpci_scan_bus():
>
> drivers/pci/endpoint/functions/pci-epf-vntb.c:1024 vpci_scan_bus()
> error: we previously assumed 'vpci_bus' could be null (see line 1021)
>
> Instead of printing an error message and then crashing we should return
> an error code and clean up. Also the NULL check is reversed so it
> prints an error for success instead of failure.
>
> Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> drivers/pci/endpoint/functions/pci-epf-vntb.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index 8e779eecd62d..7f05a44e9a9f 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -1018,8 +1018,10 @@ static int vpci_scan_bus(void *sysdata)
> struct epf_ntb *ndev = sysdata;
>
> vpci_bus = pci_scan_bus(ndev->vbus_number, &vpci_ops, sysdata);
> - if (vpci_bus)
> - pr_err("create pci bus\n");
> + if (!vpci_bus) {
> + pr_err("create pci bus failed\n");
> + return -EINVAL;
> + }
>
> pci_bus_add_devices(vpci_bus);
>
> @@ -1338,10 +1340,14 @@ static int epf_ntb_bind(struct pci_epf *epf)
> goto err_bar_alloc;
> }
>
> - vpci_scan_bus(ntb);
> + ret = vpci_scan_bus(ntb);
> + if (ret)
> + goto err_unregister;
>
> return 0;
>
> +err_unregister:
> + pci_unregister_driver(&vntb_pci_driver);
> err_bar_alloc:
> epf_ntb_config_spad_bar_free(ntb);
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] PCI: endpoint: Fix epf_ntb_epc_cleanup() a bit
2024-06-10 9:32 [PATCH 0/2] PCI: endpoint: fix a couple error handling bugs Dan Carpenter
2024-06-10 9:33 ` [PATCH 1/2] PCI: endpoint: Clean up error handling in vpci_scan_bus() Dan Carpenter
@ 2024-06-10 9:33 ` Dan Carpenter
2024-06-14 13:01 ` Ilpo Järvinen
2024-07-01 20:34 ` [PATCH 0/2] PCI: endpoint: fix a couple error handling bugs Krzysztof Wilczyński
2 siblings, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2024-06-10 9:33 UTC (permalink / raw)
To: Frank Li
Cc: Jon Mason, Dave Jiang, Allen Hubbe, Manivannan Sadhasivam,
Krzysztof Wilczyński, Kishon Vijay Abraham I, Bjorn Helgaas,
ntb, linux-pci, linux-kernel, kernel-janitors
There are two issues related to epf_ntb_epc_cleanup().
1) It should call epf_ntb_config_sspad_bar_clear().
2) The epf_ntb_bind() function should call epf_ntb_epc_cleanup()
to cleanup.
I also changed the ordering a bit. Unwinding should be done in the
mirror order from how they are allocated.
Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
---
drivers/pci/endpoint/functions/pci-epf-vntb.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index 7f05a44e9a9f..874cb097b093 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -799,8 +799,9 @@ static int epf_ntb_epc_init(struct epf_ntb *ntb)
*/
static void epf_ntb_epc_cleanup(struct epf_ntb *ntb)
{
- epf_ntb_db_bar_clear(ntb);
epf_ntb_mw_bar_clear(ntb, ntb->num_mws);
+ epf_ntb_db_bar_clear(ntb);
+ epf_ntb_config_sspad_bar_clear(ntb);
}
#define EPF_NTB_R(_name) \
@@ -1337,7 +1338,7 @@ static int epf_ntb_bind(struct pci_epf *epf)
ret = pci_register_driver(&vntb_pci_driver);
if (ret) {
dev_err(dev, "failure register vntb pci driver\n");
- goto err_bar_alloc;
+ goto err_epc_cleanup;
}
ret = vpci_scan_bus(ntb);
@@ -1348,6 +1349,8 @@ static int epf_ntb_bind(struct pci_epf *epf)
err_unregister:
pci_unregister_driver(&vntb_pci_driver);
+err_epc_cleanup:
+ epf_ntb_epc_cleanup(ntb);
err_bar_alloc:
epf_ntb_config_spad_bar_free(ntb);
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/2] PCI: endpoint: Fix epf_ntb_epc_cleanup() a bit
2024-06-10 9:33 ` [PATCH 2/2] PCI: endpoint: Fix epf_ntb_epc_cleanup() a bit Dan Carpenter
@ 2024-06-14 13:01 ` Ilpo Järvinen
0 siblings, 0 replies; 6+ messages in thread
From: Ilpo Järvinen @ 2024-06-14 13:01 UTC (permalink / raw)
To: Dan Carpenter
Cc: Frank Li, Jon Mason, Dave Jiang, Allen Hubbe,
Manivannan Sadhasivam, Krzysztof Wilczyński,
Kishon Vijay Abraham I, Bjorn Helgaas, ntb, linux-pci, LKML,
kernel-janitors
[-- Attachment #1: Type: text/plain, Size: 1855 bytes --]
On Mon, 10 Jun 2024, Dan Carpenter wrote:
> There are two issues related to epf_ntb_epc_cleanup().
> 1) It should call epf_ntb_config_sspad_bar_clear().
> 2) The epf_ntb_bind() function should call epf_ntb_epc_cleanup()
> to cleanup.
>
> I also changed the ordering a bit. Unwinding should be done in the
> mirror order from how they are allocated.
>
> Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
> Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org>
> ---
> drivers/pci/endpoint/functions/pci-epf-vntb.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index 7f05a44e9a9f..874cb097b093 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -799,8 +799,9 @@ static int epf_ntb_epc_init(struct epf_ntb *ntb)
> */
> static void epf_ntb_epc_cleanup(struct epf_ntb *ntb)
> {
> - epf_ntb_db_bar_clear(ntb);
> epf_ntb_mw_bar_clear(ntb, ntb->num_mws);
> + epf_ntb_db_bar_clear(ntb);
> + epf_ntb_config_sspad_bar_clear(ntb);
> }
>
> #define EPF_NTB_R(_name) \
> @@ -1337,7 +1338,7 @@ static int epf_ntb_bind(struct pci_epf *epf)
> ret = pci_register_driver(&vntb_pci_driver);
> if (ret) {
> dev_err(dev, "failure register vntb pci driver\n");
> - goto err_bar_alloc;
> + goto err_epc_cleanup;
> }
>
> ret = vpci_scan_bus(ntb);
> @@ -1348,6 +1349,8 @@ static int epf_ntb_bind(struct pci_epf *epf)
>
> err_unregister:
> pci_unregister_driver(&vntb_pci_driver);
> +err_epc_cleanup:
> + epf_ntb_epc_cleanup(ntb);
> err_bar_alloc:
> epf_ntb_config_spad_bar_free(ntb);
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
--
i.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] PCI: endpoint: fix a couple error handling bugs
2024-06-10 9:32 [PATCH 0/2] PCI: endpoint: fix a couple error handling bugs Dan Carpenter
2024-06-10 9:33 ` [PATCH 1/2] PCI: endpoint: Clean up error handling in vpci_scan_bus() Dan Carpenter
2024-06-10 9:33 ` [PATCH 2/2] PCI: endpoint: Fix epf_ntb_epc_cleanup() a bit Dan Carpenter
@ 2024-07-01 20:34 ` Krzysztof Wilczyński
2 siblings, 0 replies; 6+ messages in thread
From: Krzysztof Wilczyński @ 2024-07-01 20:34 UTC (permalink / raw)
To: Dan Carpenter
Cc: Frank Li, Jon Mason, Dave Jiang, Allen Hubbe,
Manivannan Sadhasivam, Kishon Vijay Abraham I, Bjorn Helgaas, ntb,
linux-pci, linux-kernel, kernel-janitors
Hello,
> Two small error error handling and cleanup patches. The first one fixes
> an incorrect error message printed on success. The other one fixes some
> cleanup. Which is probably not required because PCI code is generally
> required for a functioning system...
Applied to endpoint, thank you!
[01/02] PCI: endpoint: Clean up error handling in vpci_scan_bus()
https://git.kernel.org/pci/pci/c/72705e5b5957
[02/02] PCI: endpoint: Fix error handling in epf_ntb_epc_cleanup()
https://git.kernel.org/pci/pci/c/05214340e133
Krzysztof
^ permalink raw reply [flat|nested] 6+ messages in thread