Re: [PATCH v4 2/5] PCI: endpoint: Add RC-to-EP doorbell support using platform MSI controller

[Krishna Chaitanya Chundru <quic_krichai@quicinc.com>]

On 10/31/2024 9:57 PM, Frank Li wrote:
> Doorbell feature is implemented by mapping the EP's MSI interrupt
> controller message address to a dedicated BAR in the EPC core. It is the
> responsibility of the EPF driver to pass the actual message data to be
> written by the host to the doorbell BAR region through its own logic.
>
Hi Frank,

Currently you are using single doorbell callback for all MSI's
received from the host side.
Instead of that can we expose a API which will be used by EPF driver
to request for IRQ by passing there own irq handler. we can skip
request_irq in epc_alloc_doorbell expose a seperate API for the
EPF driver to request_IRQ or let the EPF driver do request_irq by
filling  epf->db_msg[i].virq

- Krishna Chaitanya.

> Signed-off-by: Frank Li <Frank.Li@nxp.com>
> ---
> Change from v3 to v4
> - msi change to use msi_get_virq() avoid use msi_for_each_desc().
> - add new struct for pci_epf_doorbell_msg to msi msg,virq and irq name.
> - move mutex lock to epc function
> - initialize variable at declear place.
> - passdown epf to epc*() function to simplify code.
> ---
>   drivers/pci/endpoint/Makefile     |   2 +-
>   drivers/pci/endpoint/pci-ep-msi.c | 128 ++++++++++++++++++++++++++++++++++++++
>   include/linux/pci-ep-msi.h        |  15 +++++
>   include/linux/pci-epf.h           |  19 ++++++
>   4 files changed, 163 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/endpoint/Makefile b/drivers/pci/endpoint/Makefile
> index 95b2fe47e3b06..a1ccce440c2c5 100644
> --- a/drivers/pci/endpoint/Makefile
> +++ b/drivers/pci/endpoint/Makefile
> @@ -5,4 +5,4 @@
>   
>   obj-$(CONFIG_PCI_ENDPOINT_CONFIGFS)	+= pci-ep-cfs.o
>   obj-$(CONFIG_PCI_ENDPOINT)		+= pci-epc-core.o pci-epf-core.o\
> -					   pci-epc-mem.o functions/
> +					   pci-epc-mem.o pci-ep-msi.o functions/
> diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
> new file mode 100644
> index 0000000000000..91207fb66db32
> --- /dev/null
> +++ b/drivers/pci/endpoint/pci-ep-msi.c
> @@ -0,0 +1,128 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * PCI Endpoint *Controller* (EPC) MSI library
> + *
> + * Copyright (C) 2024 NXP
> + * Author: Frank Li <Frank.Li@nxp.com>
> + */
> +
> +#include <linux/cleanup.h>
> +#include <linux/device.h>
> +#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>
> +#include <linux/pci-ep-msi.h>
> +
> +static irqreturn_t pci_epf_doorbell_handler(int irq, void *data)
> +{
> +	struct pci_epf *epf = data;
> +	struct msi_desc *desc = irq_get_msi_desc(irq);
> +
> +	if (desc && epf->event_ops && epf->event_ops->doorbell)
> +		epf->event_ops->doorbell(epf, desc->msi_index);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static bool pci_epc_match_parent(struct device *dev, void *param)
> +{
> +	return dev->parent == param;
> +}
> +
> +static void pci_epc_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
> +{
> +	struct pci_epc *epc __free(pci_epc_put) = NULL;
> +	struct pci_epf *epf;
> +
> +	epc = pci_epc_get_fn(pci_epc_match_parent, desc->dev);
> +	if (!epc)
> +		return;
> +
> +	/* Only support one EPF for doorbell */
> +	epf = list_first_entry_or_null(&epc->pci_epf, struct pci_epf, list);
> +
> +	if (epf && epf->db_msg && desc->msi_index < epf->num_db)
> +		memcpy(&epf->db_msg[desc->msi_index].msg, msg, sizeof(*msg));
> +}
> +
> +static void pci_epc_free_doorbell(struct pci_epc *epc, struct pci_epf *epf)
> +{
> +	int i;
> +
> +	guard(mutex)(&epc->lock);
> +
> +	for (i = 0; i < epf->num_db && epf->db_msg[i].virq; i++) {
> +		free_irq(epf->db_msg[i].virq, epf);
> +		epf->db_msg[i].virq = 0;
> +		kfree(epf->db_msg[i].name);
> +		epf->db_msg[i].name = NULL;
> +	}
> +
> +	platform_device_msi_free_irqs_all(epc->dev.parent);
> +}
> +
> +static int pci_epc_alloc_doorbell(struct pci_epc *epc, struct pci_epf *epf)
> +{
> +	struct device *dev = epc->dev.parent;
> +	u16 num_db = epf->num_db;
> +	int i = 0;
> +	int ret;
> +
> +	guard(mutex)(&epc->lock);
> +
> +	ret = platform_device_msi_init_and_alloc_irqs(dev, num_db, pci_epc_write_msi_msg);
> +	if (ret) {
> +		dev_err(dev, "Failed to allocate MSI\n");
> +		return -ENOMEM;
> +	}
> +
> +	for (i = 0; i < num_db; i++) {
> +		epf->db_msg[i].virq = msi_get_virq(dev, i);
> +		epf->db_msg[i].name = kasprintf(GFP_KERNEL, "pci-epc-doorbell%d", i);
> +		ret = request_irq(epf->db_msg[i].virq, pci_epf_doorbell_handler, 0,
> +				  epf->db_msg[i].name, epf);
> +		if (ret) {
> +			dev_err(dev, "Failed to request doorbell\n");
> +			pci_epc_free_doorbell(epc, epf);
> +			return ret;
> +		}
> +	}
> +
> +	return 0;
> +};
> +
> +int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
> +{
> +	struct pci_epc *epc = epf->epc;
> +	void *msg;
> +	int ret;
> +
> +	msg = kcalloc(num_db, sizeof(struct pci_epf_doorbell_msg), GFP_KERNEL);
> +	if (!msg)
> +		return -ENOMEM;
> +
> +	epf->num_db = num_db;
> +	epf->db_msg = msg;
> +
> +	ret = pci_epc_alloc_doorbell(epc, epf);
> +	if (ret)
> +		kfree(msg);
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(pci_epf_alloc_doorbell);
> +
> +void pci_epf_free_doorbell(struct pci_epf *epf)
> +{
> +	struct pci_epc *epc = epf->epc;
> +
> +	pci_epc_free_doorbell(epc, epf);
> +
> +	kfree(epf->db_msg);
> +	epf->db_msg = NULL;
> +	epf->num_db = 0;
> +}
> +EXPORT_SYMBOL_GPL(pci_epf_free_doorbell);
> diff --git a/include/linux/pci-ep-msi.h b/include/linux/pci-ep-msi.h
> new file mode 100644
> index 0000000000000..f0cfecf491199
> --- /dev/null
> +++ b/include/linux/pci-ep-msi.h
> @@ -0,0 +1,15 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * PCI Endpoint *Function* side MSI header file
> + *
> + * Copyright (C) 2024 NXP
> + * Author: Frank Li <Frank.Li@nxp.com>
> + */
> +
> +#ifndef __PCI_EP_MSI__
> +#define __PCI_EP_MSI__
> +
> +int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 nums);
> +void pci_epf_free_doorbell(struct pci_epf *epf);
> +
> +#endif /* __PCI_EP_MSI__ */
> diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> index 18a3aeb62ae4e..50c0f877f2efb 100644
> --- a/include/linux/pci-epf.h
> +++ b/include/linux/pci-epf.h
> @@ -12,6 +12,7 @@
>   #include <linux/configfs.h>
>   #include <linux/device.h>
>   #include <linux/mod_devicetable.h>
> +#include <linux/msi.h>
>   #include <linux/pci.h>
>   
>   struct pci_epf;
> @@ -75,6 +76,7 @@ struct pci_epf_ops {
>    * @link_up: Callback for the EPC link up event
>    * @link_down: Callback for the EPC link down event
>    * @bus_master_enable: Callback for the EPC Bus Master Enable event
> + * @doorbell: Callback for EPC receive MSI from RC side
>    */
>   struct pci_epc_event_ops {
>   	int (*epc_init)(struct pci_epf *epf);
> @@ -82,6 +84,7 @@ struct pci_epc_event_ops {
>   	int (*link_up)(struct pci_epf *epf);
>   	int (*link_down)(struct pci_epf *epf);
>   	int (*bus_master_enable)(struct pci_epf *epf);
> +	int (*doorbell)(struct pci_epf *epf, int index);
>   };
>   
>   /**
> @@ -125,6 +128,18 @@ struct pci_epf_bar {
>   	int		flags;
>   };
>   
> +/**
> + * struct pci_epf_doorbell_msg - represents doorbell message
> + * @msi_msg: MSI message
> + * @virq: irq number of this doorbell MSI message
> + * @name: irq name for doorbell interrupt
> + */
> +struct pci_epf_doorbell_msg {
> +	struct msi_msg msg;
> +	int virq;
> +	char *name;
> +};
> +
>   /**
>    * struct pci_epf - represents the PCI EPF device
>    * @dev: the PCI EPF device
> @@ -152,6 +167,8 @@ struct pci_epf_bar {
>    * @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
> + * @db_msg: data for MSI from RC side
> + * @num_db: number of doorbells
>    */
>   struct pci_epf {
>   	struct device		dev;
> @@ -182,6 +199,8 @@ struct pci_epf {
>   	unsigned long		vfunction_num_map;
>   	struct list_head	pci_vepf;
>   	const struct pci_epc_event_ops *event_ops;
> +	struct pci_epf_doorbell_msg *db_msg;
> +	u16 num_db;
>   };
>   
>   /**
>