On 10/25/2022 8:21 PM, Manivannan Sadhasivam wrote:
> Instead of using the notifiers for passing the events from EPC to EPF,
> let's introduce a callback based mechanism where the EPF drivers can
> populate relevant callbacks for EPC events they want to subscribe.
>
> The use of notifiers in kernel is not recommended if there is a real link
> between the sender and receiver, like in this case. Also, the existing
> atomic notifier forces the notification functions to be in atomic context
> while the caller may be in non-atomic context. For instance, the two
> in-kernel users of the notifiers, pcie-qcom and pcie-tegra194, both are
> calling the notifier functions in non-atomic context (from threaded IRQ
> handlers). This creates a sleeping in atomic context issue with the
> existing EPF_TEST driver that calls the EPC APIs that may sleep.
>
> For all these reasons, let's get rid of the notifier chains and use the
> simple callback mechanism for signalling the events from EPC to EPF
> drivers. This preserves the context of the caller and avoids the latency
> of going through a separate interface for triggering the notifications.
>
> As a first step of the transition, the core_init() callback is introduced
> in this commit, that'll replace the existing CORE_INIT notifier used for
> signalling the init complete event from EPC.
>
> During the occurrence of the event, EPC will go over the list of EPF
> drivers attached to it and will call the core_init() callback if available.
>
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx>
Acked-by: Kishon Vijay Abraham I <kishon@xxxxxxxxxx>
> ---
> drivers/pci/endpoint/functions/pci-epf-test.c | 13 ++++++-------
> drivers/pci/endpoint/pci-epc-core.c | 11 ++++++++++-
> include/linux/pci-epf.h | 11 ++++++++++-
> 3 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index a6f906a96669..868de17e1ad2 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -826,20 +826,17 @@ static int pci_epf_test_core_init(struct pci_epf *epf)
> return 0;
> }
>
> +static const struct pci_epc_event_ops pci_epf_test_event_ops = {
> + .core_init = pci_epf_test_core_init,
> +};
> +
> static int pci_epf_test_notifier(struct notifier_block *nb, unsigned long val,
> void *data)
> {
> struct pci_epf *epf = container_of(nb, struct pci_epf, nb);
> struct pci_epf_test *epf_test = epf_get_drvdata(epf);
> - int ret;
>
> switch (val) {
> - case CORE_INIT:
> - ret = pci_epf_test_core_init(epf);
> - if (ret)
> - return NOTIFY_BAD;
> - break;
> -
> case LINK_UP:
> queue_delayed_work(kpcitest_workqueue, &epf_test->cmd_handler,
> msecs_to_jiffies(1));
> @@ -1010,6 +1007,8 @@ static int pci_epf_test_probe(struct pci_epf *epf, const struct pci_epf_device_i
>
> INIT_DELAYED_WORK(&epf_test->cmd_handler, pci_epf_test_cmd_handler);
>
> + epf->event_ops = &pci_epf_test_event_ops;
> +
> epf_set_drvdata(epf, epf_test);
> return 0;
> }
> diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> index 6cce430d431b..8bb60528f530 100644
> --- a/drivers/pci/endpoint/pci-epc-core.c
> +++ b/drivers/pci/endpoint/pci-epc-core.c
> @@ -707,10 +707,19 @@ EXPORT_SYMBOL_GPL(pci_epc_linkup);
> */
> void pci_epc_init_notify(struct pci_epc *epc)
> {
> + struct pci_epf *epf;
> +
> if (!epc || IS_ERR(epc))
> return;
>
> - atomic_notifier_call_chain(&epc->notifier, CORE_INIT, NULL);
> + mutex_lock(&epc->list_lock);
> + list_for_each_entry(epf, &epc->pci_epf, list) {
> + mutex_lock(&epf->lock);
> + if (epf->event_ops && epf->event_ops->core_init)
> + epf->event_ops->core_init(epf);
> + mutex_unlock(&epf->lock);
> + }
> + mutex_unlock(&epc->list_lock);
> }
> EXPORT_SYMBOL_GPL(pci_epc_init_notify);
>
> diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> index 0c94cc1513bc..a06f3b4c8bee 100644
> --- a/include/linux/pci-epf.h
> +++ b/include/linux/pci-epf.h
> @@ -18,7 +18,6 @@ struct pci_epf;
> enum pci_epc_interface_type;
>
> enum pci_notify_event {
> - CORE_INIT,
> LINK_UP,
> };
>
> @@ -72,6 +71,14 @@ struct pci_epf_ops {
> struct config_group *group);
> };
>
> +/**
> + * struct pci_epf_event_ops - Callbacks for capturing the EPC events
> + * @core_init: Callback for the EPC initialization complete event
> + */
> +struct pci_epc_event_ops {
> + int (*core_init)(struct pci_epf *epf);
> +};
> +
> /**
> * struct pci_epf_driver - represents the PCI EPF driver
> * @probe: ops to perform when a new EPF device has been bound to the EPF driver
> @@ -140,6 +147,7 @@ struct pci_epf_bar {
> * @is_vf: true - virtual function, false - physical function
> * @vfunction_num_map: bitmap to manage virtual function number
> * @pci_vepf: list of virtual endpoint functions associated with this function
> + * @event_ops: Callbacks for capturing the EPC events
> */
> struct pci_epf {
> struct device dev;
> @@ -170,6 +178,7 @@ struct pci_epf {
> unsigned int is_vf;
> unsigned long vfunction_num_map;
> struct list_head pci_vepf;
> + const struct pci_epc_event_ops *event_ops;
> };
>
> /**
