On Fri, Sep 29, 2023 at 03:00:16PM +0530, Kishon Vijay Abraham I wrote: > Hi Frank, > > On 9/12/2023 3:39 AM, Frank Li wrote: > > This commit introduces a common method for sending messages from the Root > > Complex (RC) to the Endpoint (EP) by utilizing the platform MSI interrupt > > controller, such as ARM GIC, as an EP doorbell. Maps the memory assigned > > for the BAR region by the PCI host to the message address of the platform > > MSI interrupt controller in the PCI EP. As a result, when the PCI RC writes > > to the BAR region, it triggers an IRQ at the EP. This implementation serves > > as a common method for all endpoint function drivers. > > This would help avoid the polling used in current EPF drivers. Thanks! > > > > However, it currently supports only one EP physical function due to > > limitations in ARM MSI/IMS readiness. > > Any such platform or architecture restrictions should not be handled in the > endpoint core layer. > > > > Signed-off-by: Frank Li <Frank.Li@xxxxxxx> > > --- > > drivers/pci/endpoint/pci-epc-core.c | 192 ++++++++++++++++++++++++++++ > > drivers/pci/endpoint/pci-epf-core.c | 44 +++++++ > > include/linux/pci-epc.h | 6 + > > include/linux/pci-epf.h | 7 + > > 4 files changed, 249 insertions(+) > > > > diff --git a/drivers/pci/endpoint/pci-epc-core.c b/drivers/pci/endpoint/pci-epc-core.c > > index 5a4a8b0be6262..d336a99c6a94f 100644 > > --- a/drivers/pci/endpoint/pci-epc-core.c > > +++ b/drivers/pci/endpoint/pci-epc-core.c > > @@ -10,6 +10,7 @@ > > #include <linux/slab.h> > > #include <linux/module.h> > > +#include <linux/msi.h> > > #include <linux/pci-epc.h> > > #include <linux/pci-epf.h> > > #include <linux/pci-ep-cfs.h> > > @@ -783,6 +784,197 @@ void pci_epc_bme_notify(struct pci_epc *epc) > > } > > EXPORT_SYMBOL_GPL(pci_epc_bme_notify); > > +/** > > + * pci_epc_alloc_doorbell() - alloc an address space to let RC trigger EP side IRQ by write data to > > + * the space. > > + * > > + * @epc: the EPC device that need doorbell address and data from RC. > > + * @func_no: the physical endpoint function number in the EPC device. > > + * @vfunc_no: the virtual endpoint function number in the physical function. > > + * @num_msgs: the total number of doorbell messages > > + * > > + * Return: 0 success, other is failure > > + */ > > +int pci_epc_alloc_doorbell(struct pci_epc *epc, u8 func_no, u8 vfunc_no, int num_msgs) > > +{ > > + int ret; > > + > > + if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > > + return -EINVAL; > > + > > + if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > > + return -EINVAL; > > + > > + if (!epc->ops->alloc_doorbell) > > + return 0; > > + > > + mutex_lock(&epc->lock); > > + ret = epc->ops->alloc_doorbell(epc, func_no, vfunc_no, num_msgs); > > + mutex_unlock(&epc->lock); > > + > > + return ret; > > +} > > +EXPORT_SYMBOL_GPL(pci_epc_alloc_doorbell); > > + > > +/** > > + * pci_epc_free_doorbell() - free resource allocated by pci_epc_alloc_doorbell() > > + * > > + * @epc: the EPC device that need doorbell address and data from RC. > > + * @func_no: the physical endpoint function number in the EPC device. > > + * @vfunc_no: the virtual endpoint function number in the physical function. > > + * > > + * Return: 0 success, other is failure > > + */ > > +void pci_epc_free_doorbell(struct pci_epc *epc, u8 func_no, u8 vfunc_no) > > +{ > > + if (IS_ERR_OR_NULL(epc) || func_no >= epc->max_functions) > > + return; > > + > > + if (vfunc_no > 0 && (!epc->max_vfs || vfunc_no > epc->max_vfs[func_no])) > > + return; > > + > > + if (!epc->ops->free_doorbell) > > + return; > > + > > + mutex_lock(&epc->lock); > > + epc->ops->free_doorbell(epc, func_no, vfunc_no); > > + mutex_unlock(&epc->lock); > > +} > > +EXPORT_SYMBOL_GPL(pci_epc_free_doorbell); > > + > > +static irqreturn_t pci_epf_generic_doorbell_handler(int irq, void *data) > > +{ > > + struct pci_epf *epf = data; > > + > > + if (epf->event_ops && epf->event_ops->doorbell) > > + epf->event_ops->doorbell(epf, irq - epf->virq_base); > > + > > + return IRQ_HANDLED; > > +} > > IMO the handler should be directly implemented in the EPF drivers. There > should be one API which returns the virq and the msi_msg to the EPF driver > and the EPF driver should do request_irq. > > + > > +static void pci_epc_generic_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg) > > +{ > > + struct pci_epc *epc = NULL; > > + struct class_dev_iter iter; > > + struct pci_epf *epf; > > + struct device *dev; > > + > > + class_dev_iter_init(&iter, pci_epc_class, NULL, NULL); > > + while ((dev = class_dev_iter_next(&iter))) { > > + if (dev->parent != desc->dev) > > + continue; > > Ideally the msi_desc should be associated directly with the EPF device. The key problem is platform_msi_domain_alloc_irqs only support put epc driver's dev into desc. IMS may resolve this problem, but ARM IMS progress is quite slow. > > + > > + epc = to_pci_epc(dev); > > + > > + class_dev_iter_exit(&iter); > > + break; > > + } > > + > > + if (!epc) > > + return; > > + > > + /* Only support one EPF for doorbell */ > > + epf = list_first_entry_or_null(&epc->pci_epf, struct pci_epf, list); > > + > > + if (!epf) > > + return; > > If this is a platform restriction, this should be moved elsewhere. where is good place? > > + > > + if (epf->msg && desc->msi_index < epf->num_msgs) > > + epf->msg[desc->msi_index] = *msg; > > +} > > + > > + > > +/** > > + * pci_epc_generic_alloc_doorbell() - Common help function. Allocate address space from MSI > > + * controller > > + * > > + * @epc: the EPC device that need doorbell address and data from RC. > > + * @func_no: the physical endpoint function number in the EPC device. > > + * @vfunc_no: the virtual endpoint function number in the physical function. > > + * @num_msgs: the total number of doorbell messages > > + * > > + * Remark: use this function only if EPC driver just register one EPC device. > > + * > > + * Return: 0 success, other is failure > > + */ > > +int pci_epc_generic_alloc_doorbell(struct pci_epc *epc, u8 func_no, u8 vfunc_no, int num_msgs) > > +{ > > + struct pci_epf *epf; > > + struct device *dev; > > + int virq, last; > > + int ret; > > + int i; > > + > > + if (IS_ERR_OR_NULL(epc)) > > + return -EINVAL; > > + > > + /* Currently only support one func and one vfunc for doorbell */ > > + if (func_no || vfunc_no) > > + return -EINVAL; > > + > > + epf = list_first_entry_or_null(&epc->pci_epf, struct pci_epf, list); > > + if (!epf) > > + return -EINVAL; > > + > > + dev = epc->dev.parent; > > + ret = platform_msi_domain_alloc_irqs(dev, num_msgs, pci_epc_generic_write_msi_msg); > > + if (ret) { > > + dev_err(dev, "Failed to allocate MSI\n"); > > + return -ENOMEM; > > + } > > The alloc_irqs should be for a EPF device IMO. The GIC ITS driver need of_node information to alloc msi for such devices. But EPF device have not of_node information. > > + > > + last = -1; > > + for (i = 0; i < num_msgs; i++) { > > + virq = msi_get_virq(dev, i); > > + if (i == 0) > > + epf->virq_base = virq; > > + > > + ret = request_irq(virq, pci_epf_generic_doorbell_handler, 0, > > + kasprintf(GFP_KERNEL, "pci-epc-doorbell%d", i), epf); > > + > > + if (ret) { > > + dev_err(dev, "Failed to request doorbell\n"); > > + goto err_free_irq; > > + } > > + last = i; > > + } > > + > > + return 0; > > + > > +err_free_irq: > > + for (i = 0; i < last; i++) > > + kfree(free_irq(epf->virq_base + i, epf)); > > + platform_msi_domain_free_irqs(dev); > > + > > + return -EINVAL; > > +} > > Thanks, > Kishon