Re: [PATCH v14 4/7] PCI: endpoint: pci-ep-msi: Refactor doorbell allocation for new backends

[Max Boone]

On Tue, 14 Apr 2026 23:15:11 +0900, Koichiro Den wrote:
> Prepare pci-ep-msi for non-MSI doorbell backends.
> 
> Factor MSI doorbell allocation into a helper and extend struct
> pci_epf_doorbell_msg with:
> 
> - irq_flags: required IRQ request flags (e.g. IRQF_SHARED for some
> backends)
> - type: doorbell backend type
> - bar/offset: pre-exposed doorbell target location, if any
> 
> Initialize these fields for the existing MSI-backed doorbell
> implementation.
> 
> Also add PCI_EPF_DOORBELL_EMBEDDED type, which is to be implemented in a
> follow-up patch.
> 
> No functional changes.

I’m not very fond of keeping this implementation in the pci-ep-msi file,
as the platform MSI and this implementation are both iiuc specific to
the designware ep driver. Even more so because the MSI implementation
is enabled by config rather than through device tree.

Wouldn’t we want end-users to specify what kind of doorbell they want,
as it seems to be that a more specific doorbell BAR layout can be 
programmed with eDMA, allowing native support for nvmet’s doorbell
BAR for example.

Originally in a patchset by Frank Li the API that was proposed was more
generic, and the pci-epc-msi implementation was chosen because there
was only one implementation:
- https://lore.kernel.org/imx/20231019150441.GA7254@thinkpad/
- https://lore.kernel.org/imx/20231019172347.GC7254@thinkpad/

I’d personally prefer to see an abstraction that is weaved into pci-epc-core
and pci-epf-core that can be implemented by drivers as they wish. While 
still keeping the enum for different types.

That also gives room to pull a poll-mode doorbell into the pci-epc-core,
which deduplicates that code from the nvmet and vntb epfs, and allows
other functions to use RC->EP doorbells without needing to bother with
writing the polling mechanism.

P.S. I’ve been working on a vfio-user based epc for development purposes
personally, and the last hurdle before I want to send it in for comments is 
support for doorbells, and came across this patchset checking if there’s
any other activity in the space. Having an implementation-agnostic
doorbell API in the EPF/EPC core would be very helpful to me.

> 
> Reviewed-by: Frank Li <Frank.Li@xxxxxxx>
> Tested-by: Niklas Cassel <cassel@xxxxxxxxxx>
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> ---
> drivers/pci/endpoint/pci-ep-msi.c | 54 ++++++++++++++++++++++---------
> include/linux/pci-epf.h | 23 +++++++++++--
> 2 files changed, 60 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
> index 1395919571f8..85fe46103220 100644
> --- a/drivers/pci/endpoint/pci-ep-msi.c
> +++ b/drivers/pci/endpoint/pci-ep-msi.c
> @@ -8,6 +8,7 @@
> 
> #include <linux/device.h>
> #include <linux/export.h>
> +#include <linux/interrupt.h>
> #include <linux/irqdomain.h>
> #include <linux/module.h>
> #include <linux/msi.h>
> @@ -35,23 +36,13 @@ static void pci_epf_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
> pci_epc_put(epc);
> }
> 
> -int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
> +static int pci_epf_alloc_doorbell_msi(struct pci_epf *epf, u16 num_db)
> {
> - struct pci_epc *epc = epf->epc;
> + struct pci_epf_doorbell_msg *msg;
> struct device *dev = &epf->dev;
> + struct pci_epc *epc = epf->epc;
> struct irq_domain *domain;
> - void *msg;
> - int ret;
> - int i;
> -
> - /* TODO: Multi-EPF support */
> - if (list_first_entry_or_null(&epc->pci_epf, struct pci_epf, list) != epf) {
> - dev_err(dev, "MSI doorbell doesn't support multiple EPF\n");
> - return -EINVAL;
> - }
> -
> - if (epf->db_msg)
> - return -EBUSY;
> + int ret, i;
> 
> domain = of_msi_map_get_device_domain(epc->dev.parent, 0,
> DOMAIN_BUS_PLATFORM_MSI);
> @@ -74,6 +65,12 @@ int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
> if (!msg)
> return -ENOMEM;
> 
> + for (i = 0; i < num_db; i++)
> + msg[i] = (struct pci_epf_doorbell_msg) {
> + .type = PCI_EPF_DOORBELL_MSI,
> + .bar = NO_BAR,
> + };
> +
> epf->num_db = num_db;
> epf->db_msg = msg;
> 
> @@ -90,13 +87,40 @@ int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
> for (i = 0; i < num_db; i++)
> epf->db_msg[i].virq = msi_get_virq(epc->dev.parent, i);
> 
> + return 0;
> +}
> +
> +int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
> +{
> + struct pci_epc *epc = epf->epc;
> + struct device *dev = &epf->dev;
> + int ret;
> +
> + /* TODO: Multi-EPF support */
> + if (list_first_entry_or_null(&epc->pci_epf, struct pci_epf, list) != epf) {
> + dev_err(dev, "Doorbell doesn't support multiple EPF\n");
> + return -EINVAL;
> + }
> +
> + if (epf->db_msg)
> + return -EBUSY;
> +
> + ret = pci_epf_alloc_doorbell_msi(epf, num_db);
> + if (!ret)
> + return 0;
> +
> + dev_err(dev, "Failed to allocate doorbell: %d\n", ret);
> return ret;
> }
> EXPORT_SYMBOL_GPL(pci_epf_alloc_doorbell);
> 
> void pci_epf_free_doorbell(struct pci_epf *epf)
> {
> - platform_device_msi_free_irqs_all(epf->epc->dev.parent);
> + if (!epf->db_msg)
> + return;
> +
> + if (epf->db_msg[0].type == PCI_EPF_DOORBELL_MSI)
> + platform_device_msi_free_irqs_all(epf->epc->dev.parent);
> 
> kfree(epf->db_msg);
> epf->db_msg = NULL;
> diff --git a/include/linux/pci-epf.h b/include/linux/pci-epf.h
> index 7737a7c03260..cd747447a1ea 100644
> --- a/include/linux/pci-epf.h
> +++ b/include/linux/pci-epf.h
> @@ -152,14 +152,33 @@ struct pci_epf_bar {
> struct pci_epf_bar_submap *submap;
> };
> 
> +enum pci_epf_doorbell_type {
> + PCI_EPF_DOORBELL_MSI = 0,
> + PCI_EPF_DOORBELL_EMBEDDED,
> +};
> +
> /**
> * struct pci_epf_doorbell_msg - represents doorbell message
> - * @msg: MSI message
> - * @virq: IRQ number of this doorbell MSI message
> + * @msg: Doorbell address/data pair to be mapped into BAR space.
> + * For MSI-backed doorbells this is the MSI message, while for
> + * "embedded" doorbells this represents an MMIO write that asserts
> + * an interrupt on the EP side.
> + * @virq: IRQ number of this doorbell message
> + * @irq_flags: Required flags for request_irq()/request_threaded_irq().
> + * Callers may OR-in additional flags (e.g. IRQF_ONESHOT).
> + * @type: Doorbell type.
> + * @bar: BAR number where the doorbell target is already exposed to the RC
> + * (NO_BAR if not)
> + * @offset: offset within @bar for the doorbell target (valid iff
> + * @bar != NO_BAR)
> */
> struct pci_epf_doorbell_msg {
> struct msi_msg msg;
> int virq;
> + unsigned long irq_flags;
> + enum pci_epf_doorbell_type type;
> + enum pci_barno bar;
> + resource_size_t offset;
> };
> 
> /**
> -- 
> 2.51.0