[PATCH v3 0/3] PCI: endpoint: fix bug for 2 APIs and simplify 1 API

[PATCH v3 0/3] PCI: endpoint: fix bug for 2 APIs and simplify 1 API

[Zijun Hu]

This patch series is to fix bug for APIs
- devm_pci_epc_destroy().
- pci_epf_remove_vepf().

and simplify APIs below:
- pci_epc_get().

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
Changes in v3:
- Remove stable tag of patch 1/3
- Add one more patch 3/3
- Link to v2: https://lore.kernel.org/all/20241102-pci-epc-core_fix-v2-0-0785f8435be5@quicinc.com

Changes in v2:
- Correct tile and commit message for patch 1/2.
- Add one more patch 2/2 to simplify API pci_epc_get().
- Link to v1: https://lore.kernel.org/r/20241020-pci-epc-core_fix-v1-1-3899705e3537@quicinc.com

---
Zijun Hu (3):
      PCI: endpoint: Fix that API devm_pci_epc_destroy() fails to destroy the EPC device
      PCI: endpoint: Simplify API pci_epc_get() implementation
      PCI: endpoint: Fix API pci_epf_add_vepf() returning -EBUSY error

 drivers/pci/endpoint/pci-epc-core.c | 23 +++++++----------------
 drivers/pci/endpoint/pci-epf-core.c |  1 +
 2 files changed, 8 insertions(+), 16 deletions(-)
---
base-commit: 11066801dd4b7c4d75fce65c812723a80c1481ae
change-id: 20241020-pci-epc-core_fix-a92512fa9d19

Best regards,
-- 
Zijun Hu <quic_zijuhu@quicinc.com>

[PATCH v3 1/3] PCI: endpoint: Fix that API devm_pci_epc_destroy() fails to destroy the EPC device

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

For devm_pci_epc_destroy(), its comment says it needs to destroy the EPC
device, but it will not actually do that since devres_destroy() does not
call devm_pci_epc_release(), and it also can not fully undo what the API
devm_pci_epc_create() does, so it is faulty.

Fortunately, the faulty API has not been used by current kernel tree.
Fixed by using devres_release() instead of devres_destroy() within the API.

Fixes: 5e8cb4033807 ("PCI: endpoint: Add EP core layer to enable EP controller and EP functions")
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/pci/endpoint/pci-epc-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 17f00710925508e60fbd21116af5b424abdcd3e7..71b6d100056e54438d0554f7ee82aaa64e0debb5 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -857,7 +857,7 @@ void devm_pci_epc_destroy(struct device *dev, struct pci_epc *epc)
 {
 	int r;
 
-	r = devres_destroy(dev, devm_pci_epc_release, devm_pci_epc_match,
+	r = devres_release(dev, devm_pci_epc_release, devm_pci_epc_match,
 			   epc);
 	dev_WARN_ONCE(dev, r, "couldn't find PCI EPC resource\n");
 }

-- 
2.34.1

[PATCH v3 2/3] PCI: endpoint: Simplify API pci_epc_get() implementation

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

Simplify pci_epc_get() implementation by API class_find_device_by_name().

Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/pci/endpoint/pci-epc-core.c | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 71b6d100056e54438d0554f7ee82aaa64e0debb5..eb02d477bc7ca43a91b8a4a13c6ce96dab0b02fd 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -60,26 +60,17 @@ struct pci_epc *pci_epc_get(const char *epc_name)
 	int ret = -EINVAL;
 	struct pci_epc *epc;
 	struct device *dev;
-	struct class_dev_iter iter;
 
-	class_dev_iter_init(&iter, &pci_epc_class, NULL, NULL);
-	while ((dev = class_dev_iter_next(&iter))) {
-		if (strcmp(epc_name, dev_name(dev)))
-			continue;
+	dev = class_find_device_by_name(&pci_epc_class, epc_name);
+	if (!dev)
+		goto err;
 
-		epc = to_pci_epc(dev);
-		if (!try_module_get(epc->ops->owner)) {
-			ret = -EINVAL;
-			goto err;
-		}
-
-		class_dev_iter_exit(&iter);
-		get_device(&epc->dev);
+	epc = to_pci_epc(dev);
+	if (try_module_get(epc->ops->owner))
 		return epc;
-	}
 
 err:
-	class_dev_iter_exit(&iter);
+	put_device(dev);
 	return ERR_PTR(ret);
 }
 EXPORT_SYMBOL_GPL(pci_epc_get);

-- 
2.34.1

[PATCH v3 3/3] PCI: endpoint: Fix API pci_epf_add_vepf() returning -EBUSY error

[Zijun Hu]

From: Zijun Hu <quic_zijuhu@quicinc.com>

pci_epf_add_vepf() will suffer -EBUSY error by steps below:

pci_epf_add_vepf(@epf_pf, @epf_vf)       // add
pci_epf_remove_vepf(@epf_pf, @epf_vf)   // remove
pci_epf_add_vepf(@epf_pf, @epf_vf)     // add again, -EBUSY error.

Fix by clearing @epf_vf->epf_pf in pci_epf_remove_vepf().

Fixes: 1cf362e907f3 ("PCI: endpoint: Add support to add virtual function in endpoint core")
Cc: stable@vger.kernel.org
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/pci/endpoint/pci-epf-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index 8fa2797d4169a9f21136bbf73daa818da6c4ac49..50bc2892a36c54aa82c819ac5a9c99e9155d92c1 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -202,6 +202,7 @@ void pci_epf_remove_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf)
 
 	mutex_lock(&epf_pf->lock);
 	clear_bit(epf_vf->vfunc_no, &epf_pf->vfunction_num_map);
+	epf_vf->epf_pf = NULL;
 	list_del(&epf_vf->list);
 	mutex_unlock(&epf_pf->lock);
 }

-- 
2.34.1

Re: [PATCH v3 0/3] PCI: endpoint: fix bug for 2 APIs and simplify 1 API

[Dan Carpenter]

On Tue, Dec 10, 2024 at 10:00:17PM +0800, Zijun Hu wrote:
> This patch series is to fix bug for APIs
> - devm_pci_epc_destroy().
> - pci_epf_remove_vepf().
> 
> and simplify APIs below:
> - pci_epc_get().
> 
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---

This is very good.  This is Config FS.  Is there a kself test for configfs
or did you create your own test?

regards,
dan carpenter

Re: [PATCH v3 0/3] PCI: endpoint: fix bug for 2 APIs and simplify 1 API

[Zijun Hu]

On 2024/12/12 17:35, Dan Carpenter wrote:
> On Tue, Dec 10, 2024 at 10:00:17PM +0800, Zijun Hu wrote:
>> This patch series is to fix bug for APIs
>> - devm_pci_epc_destroy().
>> - pci_epf_remove_vepf().
>>
>> and simplify APIs below:
>> - pci_epc_get().
>>
>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
>> ---
> 
> This is very good.  This is Config FS.  Is there a kself test for configfs
> or did you create your own test?
> 

no.

In order to investigate devres_release() usage for patch 1/3, i read
2 PCI source files and then find a few obvious bugs. (^^)

> regards,
> dan carpenter
> 
> 

Re: [PATCH v3 2/3] PCI: endpoint: Simplify API pci_epc_get() implementation

[Frank Li]

On Tue, Dec 10, 2024 at 10:00:19PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
>
> Simplify pci_epc_get() implementation by API class_find_device_by_name().
>
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>

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

> ---
>  drivers/pci/endpoint/pci-epc-core.c | 21 ++++++---------------
>  1 file changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 71b6d100056e54438d0554f7ee82aaa64e0debb5..eb02d477bc7ca43a91b8a4a13c6ce96dab0b02fd 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -60,26 +60,17 @@ struct pci_epc *pci_epc_get(const char *epc_name)
>  	int ret = -EINVAL;
>  	struct pci_epc *epc;
>  	struct device *dev;
> -	struct class_dev_iter iter;
>
> -	class_dev_iter_init(&iter, &pci_epc_class, NULL, NULL);
> -	while ((dev = class_dev_iter_next(&iter))) {
> -		if (strcmp(epc_name, dev_name(dev)))
> -			continue;
> +	dev = class_find_device_by_name(&pci_epc_class, epc_name);
> +	if (!dev)
> +		goto err;
>
> -		epc = to_pci_epc(dev);
> -		if (!try_module_get(epc->ops->owner)) {
> -			ret = -EINVAL;
> -			goto err;
> -		}
> -
> -		class_dev_iter_exit(&iter);
> -		get_device(&epc->dev);
> +	epc = to_pci_epc(dev);
> +	if (try_module_get(epc->ops->owner))
>  		return epc;
> -	}
>
>  err:
> -	class_dev_iter_exit(&iter);
> +	put_device(dev);
>  	return ERR_PTR(ret);
>  }
>  EXPORT_SYMBOL_GPL(pci_epc_get);
>
> --
> 2.34.1
>

Re: [PATCH v3 3/3] PCI: endpoint: Fix API pci_epf_add_vepf() returning -EBUSY error

[Frank Li]

On Tue, Dec 10, 2024 at 10:00:20PM +0800, Zijun Hu wrote:
> From: Zijun Hu <quic_zijuhu@quicinc.com>
>
> pci_epf_add_vepf() will suffer -EBUSY error by steps below:
>
> pci_epf_add_vepf(@epf_pf, @epf_vf)       // add
> pci_epf_remove_vepf(@epf_pf, @epf_vf)   // remove
> pci_epf_add_vepf(@epf_pf, @epf_vf)     // add again, -EBUSY error.

nit: can you align comments to the same column?

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

>
> Fix by clearing @epf_vf->epf_pf in pci_epf_remove_vepf().
>
> Fixes: 1cf362e907f3 ("PCI: endpoint: Add support to add virtual function in endpoint core")
> Cc: stable@vger.kernel.org
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> ---
>  drivers/pci/endpoint/pci-epf-core.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> index 8fa2797d4169a9f21136bbf73daa818da6c4ac49..50bc2892a36c54aa82c819ac5a9c99e9155d92c1 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -202,6 +202,7 @@ void pci_epf_remove_vepf(struct pci_epf *epf_pf, struct pci_epf *epf_vf)
>
>  	mutex_lock(&epf_pf->lock);
>  	clear_bit(epf_vf->vfunc_no, &epf_pf->vfunction_num_map);
> +	epf_vf->epf_pf = NULL;
>  	list_del(&epf_vf->list);
>  	mutex_unlock(&epf_pf->lock);
>  }
>
> --
> 2.34.1
>

Re: [PATCH v3 0/3] PCI: endpoint: fix bug for 2 APIs and simplify 1 API

[Bjorn Helgaas]

On Tue, Dec 10, 2024 at 10:00:17PM +0800, Zijun Hu wrote:
> This patch series is to fix bug for APIs
> - devm_pci_epc_destroy().
> - pci_epf_remove_vepf().
> 
> and simplify APIs below:
> - pci_epc_get().
> 
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>

Applied to pci/endpoint for v6.14, thanks!

> ---
> Changes in v3:
> - Remove stable tag of patch 1/3
> - Add one more patch 3/3
> - Link to v2: https://lore.kernel.org/all/20241102-pci-epc-core_fix-v2-0-0785f8435be5@quicinc.com
> 
> Changes in v2:
> - Correct tile and commit message for patch 1/2.
> - Add one more patch 2/2 to simplify API pci_epc_get().
> - Link to v1: https://lore.kernel.org/r/20241020-pci-epc-core_fix-v1-1-3899705e3537@quicinc.com
> 
> ---
> Zijun Hu (3):
>       PCI: endpoint: Fix that API devm_pci_epc_destroy() fails to destroy the EPC device
>       PCI: endpoint: Simplify API pci_epc_get() implementation
>       PCI: endpoint: Fix API pci_epf_add_vepf() returning -EBUSY error
> 
>  drivers/pci/endpoint/pci-epc-core.c | 23 +++++++----------------
>  drivers/pci/endpoint/pci-epf-core.c |  1 +
>  2 files changed, 8 insertions(+), 16 deletions(-)
> ---
> base-commit: 11066801dd4b7c4d75fce65c812723a80c1481ae
> change-id: 20241020-pci-epc-core_fix-a92512fa9d19
> 
> Best regards,
> -- 
> Zijun Hu <quic_zijuhu@quicinc.com>
>