[PATCH V2 0/4] Add DeInit support in the PCIe Endpoint framework

[PATCH V2 0/4] Add DeInit support in the PCIe Endpoint framework

[Vidya Sagar]

Endpoint function driver should cleanup its resources which accesses the
hardware during endpoint controller deinitialization. The patches in this
series address this requirement by first adding the callback notification
in the endpoint code driver. This notification is invoked by the controller
driver which is propagated further to the function driver.

This patch series depends on Manivanna's following series.
https://patchwork.ozlabs.org/project/linux-pci/list/?series=321660

V2:
* Reworded commit messages
* Added a new patch for deinit of the function driver

Vidya Sagar (4):
  PCI: endpoint: Add core_deinit() callback support
  PCI: dwc: Add a DWC wrapper to pci_epc_deinit_notify()
  PCI: endpoint: Delete list entry before freeing
  PCI: endpoint: Add deinit in epf test driver

 .../pci/controller/dwc/pcie-designware-ep.c   |  8 +++++
 drivers/pci/controller/dwc/pcie-designware.h  |  5 ++++
 drivers/pci/endpoint/functions/pci-epf-test.c | 30 +++++++++++++++++++
 drivers/pci/endpoint/pci-epc-core.c           | 26 ++++++++++++++++
 drivers/pci/endpoint/pci-epf-core.c           |  5 ++--
 include/linux/pci-epc.h                       |  1 +
 include/linux/pci-epf.h                       |  2 ++
 7 files changed, 75 insertions(+), 2 deletions(-)

-- 
2.17.1

[PATCH V2 1/4] PCI: endpoint: Add core_deinit() callback support

[Vidya Sagar]

The endpoint function driver should undo the things done in core_init()
and stop hardware access before deinitializing the controller. Add
core_deinit() callback support for function driver to do this cleanup.
This core_deinit() callback should be invoked by the controller driver
before deinitializing the controller.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
V2:
* Reworded the commit message

 drivers/pci/endpoint/pci-epc-core.c | 26 ++++++++++++++++++++++++++
 include/linux/pci-epc.h             |  1 +
 include/linux/pci-epf.h             |  2 ++
 3 files changed, 29 insertions(+)

diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
index 5dac1496cf16..689450f01f75 100644
--- a/drivers/pci/endpoint/pci-epc-core.c
+++ b/drivers/pci/endpoint/pci-epc-core.c
@@ -732,6 +732,32 @@ void pci_epc_init_notify(struct pci_epc *epc)
 }
 EXPORT_SYMBOL_GPL(pci_epc_init_notify);
 
+/**
+ * pci_epc_deinit_notify() - Notify the EPF device that EPC device's core
+ *			     deinitialization is scheduled.
+ * @epc: the EPC device whose core deinitialization is scheduled
+ *
+ * Invoke to Notify the EPF device that the EPC device's deinitialization
+ * is scheduled.
+ */
+void pci_epc_deinit_notify(struct pci_epc *epc)
+{
+	struct pci_epf *epf;
+
+	if (!epc || IS_ERR(epc))
+		return;
+
+	mutex_lock(&epc->list_lock);
+	list_for_each_entry(epf, &epc->pci_epf, list) {
+		mutex_lock(&epf->lock);
+		if (epf->event_ops->core_deinit)
+			epf->event_ops->core_deinit(epf);
+		mutex_unlock(&epf->lock);
+	}
+	mutex_unlock(&epc->list_lock);
+}
+EXPORT_SYMBOL_GPL(pci_epc_deinit_notify);
+
 /**
  * pci_epc_destroy() - destroy the EPC device
  * @epc: the EPC device that has to be destroyed
diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
index 301bb0e53707..b95dc4b3e302 100644
--- a/include/linux/pci-epc.h
+++ b/include/linux/pci-epc.h
@@ -204,6 +204,7 @@ int pci_epc_add_epf(struct pci_epc *epc, struct pci_epf *epf,
 		    enum pci_epc_interface_type type);
 void pci_epc_linkup(struct pci_epc *epc);
 void pci_epc_init_notify(struct pci_epc *epc);
+void pci_epc_deinit_notify(struct pci_epc *epc);
 void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf,
 			enum pci_epc_interface_type type);
 int pci_epc_write_header(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
index a215dc8ce693..fa51579951db 100644
--- a/include/linux/pci-epf.h
+++ b/include/linux/pci-epf.h
@@ -70,10 +70,12 @@ struct pci_epf_ops {
 /**
  * struct pci_epf_event_ops - Callbacks for capturing the EPC events
  * @core_init: Callback for the EPC initialization complete event
+ * @core_deinit: Callback for the EPC deinitialization schedule event
  * @link_up: Callback for the EPC link up event
  */
 struct pci_epc_event_ops {
 	int (*core_init)(struct pci_epf *epf);
+	int (*core_deinit)(struct pci_epf *epf);
 	int (*link_up)(struct pci_epf *epf);
 };
 
-- 
2.17.1

Re: [PATCH V2 1/4] PCI: endpoint: Add core_deinit() callback support

[Manivannan Sadhasivam]

On Thu, Oct 13, 2022 at 11:48:12PM +0530, Vidya Sagar wrote:
> The endpoint function driver should undo the things done in core_init()
> and stop hardware access before deinitializing the controller. Add
> core_deinit() callback support for function driver to do this cleanup.
> This core_deinit() callback should be invoked by the controller driver
> before deinitializing the controller.
> 
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> V2:
> * Reworded the commit message
> 
>  drivers/pci/endpoint/pci-epc-core.c | 26 ++++++++++++++++++++++++++
>  include/linux/pci-epc.h             |  1 +
>  include/linux/pci-epf.h             |  2 ++
>  3 files changed, 29 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 5dac1496cf16..689450f01f75 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -732,6 +732,32 @@ void pci_epc_init_notify(struct pci_epc *epc)
>  }
>  EXPORT_SYMBOL_GPL(pci_epc_init_notify);
>  
> +/**
> + * pci_epc_deinit_notify() - Notify the EPF device that EPC device's core
> + *			     deinitialization is scheduled.
> + * @epc: the EPC device whose core deinitialization is scheduled
> + *
> + * Invoke to Notify the EPF device that the EPC device's deinitialization
> + * is scheduled.
> + */
> +void pci_epc_deinit_notify(struct pci_epc *epc)
> +{
> +	struct pci_epf *epf;
> +
> +	if (!epc || IS_ERR(epc))
> +		return;
> +
> +	mutex_lock(&epc->list_lock);
> +	list_for_each_entry(epf, &epc->pci_epf, list) {
> +		mutex_lock(&epf->lock);
> +		if (epf->event_ops->core_deinit)

I've added a check for the existence of the "event_ops" in latest series.
Please rebase on top of that and add the check here too.

Thanks,
Mani

> +			epf->event_ops->core_deinit(epf);
> +		mutex_unlock(&epf->lock);
> +	}
> +	mutex_unlock(&epc->list_lock);
> +}
> +EXPORT_SYMBOL_GPL(pci_epc_deinit_notify);
> +
>  /**
>   * pci_epc_destroy() - destroy the EPC device
>   * @epc: the EPC device that has to be destroyed
> diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h
> index 301bb0e53707..b95dc4b3e302 100644
> --- a/include/linux/pci-epc.h
> +++ b/include/linux/pci-epc.h
> @@ -204,6 +204,7 @@ int pci_epc_add_epf(struct pci_epc *epc, struct pci_epf *epf,
>  		    enum pci_epc_interface_type type);
>  void pci_epc_linkup(struct pci_epc *epc);
>  void pci_epc_init_notify(struct pci_epc *epc);
> +void pci_epc_deinit_notify(struct pci_epc *epc);
>  void pci_epc_remove_epf(struct pci_epc *epc, struct pci_epf *epf,
>  			enum pci_epc_interface_type type);
>  int pci_epc_write_header(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
> diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> index a215dc8ce693..fa51579951db 100644
> --- a/include/linux/pci-epf.h
> +++ b/include/linux/pci-epf.h
> @@ -70,10 +70,12 @@ struct pci_epf_ops {
>  /**
>   * struct pci_epf_event_ops - Callbacks for capturing the EPC events
>   * @core_init: Callback for the EPC initialization complete event
> + * @core_deinit: Callback for the EPC deinitialization schedule event
>   * @link_up: Callback for the EPC link up event
>   */
>  struct pci_epc_event_ops {
>  	int (*core_init)(struct pci_epf *epf);
> +	int (*core_deinit)(struct pci_epf *epf);
>  	int (*link_up)(struct pci_epf *epf);
>  };
>  
> -- 
> 2.17.1
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH V2 2/4] PCI: dwc: Add a DWC wrapper to pci_epc_deinit_notify()

[Vidya Sagar]

Add a wrapper for the pci_epc_deinit_notify() at the DWC layer for all
the DesignWare based platform controller drivers to invoke during their
respective endpoint controllers deinitialization.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
V2:
* Reworded the commit message

 drivers/pci/controller/dwc/pcie-designware-ep.c | 8 ++++++++
 drivers/pci/controller/dwc/pcie-designware.h    | 5 +++++
 2 files changed, 13 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index f300ea2f7bf7..fd11c2e68fef 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -737,6 +737,14 @@ int dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
 }
 EXPORT_SYMBOL_GPL(dw_pcie_ep_init_notify);
 
+void dw_pcie_ep_deinit_notify(struct dw_pcie_ep *ep)
+{
+	struct pci_epc *epc = ep->epc;
+
+	pci_epc_deinit_notify(epc);
+}
+EXPORT_SYMBOL_GPL(dw_pcie_ep_deinit_notify);
+
 int dw_pcie_ep_init(struct dw_pcie_ep *ep)
 {
 	int ret;
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index 7252513956b7..0f3ab39c5281 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -469,6 +469,7 @@ static inline void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus,
 void dw_pcie_ep_linkup(struct dw_pcie_ep *ep);
 int dw_pcie_ep_init(struct dw_pcie_ep *ep);
 int dw_pcie_ep_init_notify(struct dw_pcie_ep *ep);
+void dw_pcie_ep_deinit_notify(struct dw_pcie_ep *ep);
 void dw_pcie_ep_exit(struct dw_pcie_ep *ep);
 int dw_pcie_ep_raise_legacy_irq(struct dw_pcie_ep *ep, u8 func_no);
 int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
@@ -495,6 +496,10 @@ static inline int dw_pcie_ep_init_notify(struct dw_pcie_ep *ep)
 	return 0;
 }
 
+static inline void dw_pcie_ep_deinit_notify(struct dw_pcie_ep *ep)
+{
+}
+
 static inline void dw_pcie_ep_exit(struct dw_pcie_ep *ep)
 {
 }
-- 
2.17.1

[PATCH V2 3/4] PCI: endpoint: Delete list entry before freeing

[Vidya Sagar]

Currently, epf_group list is traversed, and each group entry is freed and
epf_group list head is deleted in the end. Deleting the list head is
corrupting the data in the group entries that are already freed, leading to
random crashes. To fix this issue, delete each group entry and then free
it, and don't delete epf_group list head.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
V2:
* Reworded the commit message

 drivers/pci/endpoint/pci-epf-core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
index 9ed556936f48..a7f4ae33905d 100644
--- a/drivers/pci/endpoint/pci-epf-core.c
+++ b/drivers/pci/endpoint/pci-epf-core.c
@@ -340,9 +340,10 @@ static void pci_epf_remove_cfs(struct pci_epf_driver *driver)
 		return;
 
 	mutex_lock(&pci_epf_mutex);
-	list_for_each_entry_safe(group, tmp, &driver->epf_group, group_entry)
+	list_for_each_entry_safe(group, tmp, &driver->epf_group, group_entry) {
+		list_del(&group->group_entry);
 		pci_ep_cfs_remove_epf_group(group);
-	list_del(&driver->epf_group);
+	}
 	mutex_unlock(&pci_epf_mutex);
 }
 
-- 
2.17.1

Re: [PATCH V2 3/4] PCI: endpoint: Delete list entry before freeing

[Manivannan Sadhasivam]

On Thu, Oct 13, 2022 at 11:48:14PM +0530, Vidya Sagar wrote:
> Currently, epf_group list is traversed, and each group entry is freed and
> epf_group list head is deleted in the end. Deleting the list head is
> corrupting the data in the group entries that are already freed, leading to
> random crashes. To fix this issue, delete each group entry and then free
> it, and don't delete epf_group list head.
> 
> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>

Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>

Thanks,
Mani

> ---
> V2:
> * Reworded the commit message
> 
>  drivers/pci/endpoint/pci-epf-core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c
> index 9ed556936f48..a7f4ae33905d 100644
> --- a/drivers/pci/endpoint/pci-epf-core.c
> +++ b/drivers/pci/endpoint/pci-epf-core.c
> @@ -340,9 +340,10 @@ static void pci_epf_remove_cfs(struct pci_epf_driver *driver)
>  		return;
>  
>  	mutex_lock(&pci_epf_mutex);
> -	list_for_each_entry_safe(group, tmp, &driver->epf_group, group_entry)
> +	list_for_each_entry_safe(group, tmp, &driver->epf_group, group_entry) {
> +		list_del(&group->group_entry);
>  		pci_ep_cfs_remove_epf_group(group);
> -	list_del(&driver->epf_group);
> +	}
>  	mutex_unlock(&pci_epf_mutex);
>  }
>  
> -- 
> 2.17.1
> 

-- 
மணிவண்ணன் சதாசிவம்

[PATCH V2 4/4] PCI: endpoint: Add deinit in epf test driver

[Vidya Sagar]

Add support for clearing the BAR info during the deinitialization phase of
the epf test driver.

Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
---
V2:
* This is a new patch in this series

 drivers/pci/endpoint/functions/pci-epf-test.c | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
index 136470019a24..25ac3d161fac 100644
--- a/drivers/pci/endpoint/functions/pci-epf-test.c
+++ b/drivers/pci/endpoint/functions/pci-epf-test.c
@@ -826,6 +826,35 @@ static int pci_epf_test_core_init(struct pci_epf *epf)
 	return 0;
 }
 
+static int pci_epf_test_clear_bar(struct pci_epf *epf)
+{
+	int bar, add;
+	struct pci_epf_bar *epf_bar;
+	struct pci_epc *epc = epf->epc;
+	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
+	const struct pci_epc_features *epc_features;
+
+	epc_features = epf_test->epc_features;
+
+	for (bar = 0; bar < PCI_STD_NUM_BARS; bar += add) {
+		epf_bar = &epf->bar[bar];
+		add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1;
+
+		if (!!(epc_features->reserved_bar & (1 << bar)))
+			continue;
+
+		pci_epc_clear_bar(epc, epf->func_no, epf->vfunc_no, epf_bar);
+	}
+
+	return 0;
+}
+
+static int pci_epf_test_core_deinit(struct pci_epf *epf)
+{
+	pci_epf_test_clear_bar(epf);
+	return 0;
+}
+
 int pci_epf_test_link_up(struct pci_epf *epf)
 {
 	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
@@ -839,6 +868,7 @@ int pci_epf_test_link_up(struct pci_epf *epf)
 static const struct pci_epc_event_ops pci_epf_test_event_ops = {
 	.core_init = pci_epf_test_core_init,
 	.link_up = pci_epf_test_link_up,
+	.core_deinit = pci_epf_test_core_deinit,
 };
 
 static int pci_epf_test_alloc_space(struct pci_epf *epf)
-- 
2.17.1

Re: [PATCH V2 4/4] PCI: endpoint: Add deinit in epf test driver

[Manivannan Sadhasivam]

On Thu, Oct 13, 2022 at 11:48:15PM +0530, Vidya Sagar wrote:
> Add support for clearing the BAR info during the deinitialization phase of
> the epf test driver.
> 

BAR is currently cleared during unbind() time. With this deinit callback, it
has to be removed from unbind().

> Signed-off-by: Vidya Sagar <vidyas@nvidia.com>
> ---
> V2:
> * This is a new patch in this series
> 
>  drivers/pci/endpoint/functions/pci-epf-test.c | 30 +++++++++++++++++++
>  1 file changed, 30 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index 136470019a24..25ac3d161fac 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -826,6 +826,35 @@ static int pci_epf_test_core_init(struct pci_epf *epf)
>  	return 0;
>  }
>  
> +static int pci_epf_test_clear_bar(struct pci_epf *epf)
> +{
> +	int bar, add;

I don't think "add" is a good variable name. Maybe "bar_type"?

Thanks,
Mani

> +	struct pci_epf_bar *epf_bar;
> +	struct pci_epc *epc = epf->epc;
> +	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> +	const struct pci_epc_features *epc_features;
> +
> +	epc_features = epf_test->epc_features;
> +
> +	for (bar = 0; bar < PCI_STD_NUM_BARS; bar += add) {
> +		epf_bar = &epf->bar[bar];
> +		add = (epf_bar->flags & PCI_BASE_ADDRESS_MEM_TYPE_64) ? 2 : 1;
> +
> +		if (!!(epc_features->reserved_bar & (1 << bar)))
> +			continue;
> +
> +		pci_epc_clear_bar(epc, epf->func_no, epf->vfunc_no, epf_bar);
> +	}
> +
> +	return 0;
> +}
> +
> +static int pci_epf_test_core_deinit(struct pci_epf *epf)
> +{
> +	pci_epf_test_clear_bar(epf);
> +	return 0;
> +}
> +
>  int pci_epf_test_link_up(struct pci_epf *epf)
>  {
>  	struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> @@ -839,6 +868,7 @@ int pci_epf_test_link_up(struct pci_epf *epf)
>  static const struct pci_epc_event_ops pci_epf_test_event_ops = {
>  	.core_init = pci_epf_test_core_init,
>  	.link_up = pci_epf_test_link_up,
> +	.core_deinit = pci_epf_test_core_deinit,
>  };
>  
>  static int pci_epf_test_alloc_space(struct pci_epf *epf)
> -- 
> 2.17.1
> 

-- 
மணிவண்ணன் சதாசிவம்