linux-kernel - Re: [PATCH] PCI: endpoint: Add custom notifier support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Tue, 15 Jun 2021 20:41:58 +0530
From:   Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To:     Kishon Vijay Abraham I <kishon@...com>
Cc:     lorenzo.pieralisi@....com, bhelgaas@...gle.com,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, hemantk@...eaurora.org,
        smohanad@...eaurora.org
Subject: Re: [PATCH] PCI: endpoint: Add custom notifier support

Hi Kishon,

On Tue, Jun 15, 2021 at 08:31:48PM +0530, Kishon Vijay Abraham I wrote:
> Hi Manivannan,
> 
> On 15/06/21 7:07 pm, Manivannan Sadhasivam wrote:
> > Add support for passing the custom notifications between the endpoint
> > controller and the function driver. This helps in passing the
> > notifications that are more specific to the controller and corresponding
> > function driver. The opaque `data` arugument in pci_epc_custom_notify()
> > function can be used to carry the event specific data that helps in
> > differentiating the events.
> > 
> > For instance, the Qcom EPC device generates specific events such as
> > MHI_A7, BME, DSTATE_CHANGE, PM_TURNOFF etc... These events needs to be
> > passed to the function driver for proper handling. Hence, this custom
> > notifier can be used to pass these events.
> 
> Bus master enable and PME events sounds generic events and not QCOM
> specific.

Yes, that's correct! I thought about adding the notifiers for them but not sure
about the convetion in EP stack. So went with an opaque notifier but I don't
have any issue in adding them.

> Also this patch should be sent along with how it's going to be
> used in function driver.
> 

The function driver which is going to use this notifier is under development but
I can share a snippet if that helps.

> In general my preference would be to add only well defined notifiers
> given that the endpoint function drivers are generic.
> 

Not all functions are generic ones. For example, on Qcom modems there is a MHI
function driver which transports the IP packets to the host and talks to the
MHI host stack[1].

So for sure we need to have a custom notifier for vendor specific events.

Thanks,
Mani

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/bus/mhi

> Thanks
> Kishon
> 
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> > ---
> >  drivers/pci/endpoint/pci-epc-core.c | 19 +++++++++++++++++++
> >  include/linux/pci-epc.h             |  1 +
> >  include/linux/pci-epf.h             |  1 +
> >  3 files changed, 21 insertions(+)
> > 
> > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c
> > index adec9bee72cf..86b6934c6297 100644
> > --- a/drivers/pci/endpoint/pci-epc-core.c
> > +++ b/drivers/pci/endpoint/pci-epc-core.c
> > @@ -658,6 +658,25 @@ void pci_epc_init_notify(struct pci_epc *epc)
> >  }
> >  EXPORT_SYMBOL_GPL(pci_epc_init_notify);
> >  
> > +/**
> > + * pci_epc_custom_notify() - Notify the EPF device about the custom events
> > + *			     in the EPC device
> > + * @epc: EPC device that generates the custom notification
> > + * @data: Data for the custom notifier
> > + *
> > + * Invoke to notify the EPF device about the custom events in the EPC device.
> > + * This notifier can be used to pass the EPC specific custom events that are
> > + * shared with the EPF device.
> > + */
> > +void pci_epc_custom_notify(struct pci_epc *epc, void *data)
> > +{
> > +	if (!epc || IS_ERR(epc))
> > +		return;
> > +
> > +	atomic_notifier_call_chain(&epc->notifier, CUSTOM, data);
> > +}
> > +EXPORT_SYMBOL_GPL(pci_epc_custom_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 b82c9b100e97..13140fdbcdf6 100644
> > --- a/include/linux/pci-epc.h
> > +++ b/include/linux/pci-epc.h
> > @@ -203,6 +203,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_custom_notify(struct pci_epc *epc, void *data);
> >  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,
> > diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> > index 6833e2160ef1..8d740c5cf0e3 100644
> > --- a/include/linux/pci-epf.h
> > +++ b/include/linux/pci-epf.h
> > @@ -20,6 +20,7 @@ enum pci_epc_interface_type;
> >  enum pci_notify_event {
> >  	CORE_INIT,
> >  	LINK_UP,
> > +	CUSTOM,
> >  };
> >  
> >  enum pci_barno {
> >