Re: [PATCH v7 9/9] PCI: endpoint: pci-ep-msi: Add embedded eDMA doorbell fallback

On Mon, Feb 16, 2026 at 01:38:47AM +0900, Koichiro Den wrote:
> Some endpoint platforms cannot use platform MSI / GIC ITS to implement
> EP-side doorbells. In those cases, EPF drivers cannot provide an
> interrupt-driven doorbell and often fall back to polling.
> 
> Add an "embedded" doorbell backend that uses a controller-integrated
> doorbell target (e.g. DesignWare integrated eDMA interrupt-emulation
> doorbell).
> 
> The backend locates the doorbell register and a corresponding Linux IRQ
> via the EPC aux-resource API. If the doorbell register is already
> exposed via a fixed BAR mapping, provide BAR+offset. Otherwise provide
> the physical address so EPF drivers can map it into BAR space.
> 
> When MSI doorbell allocation fails with -ENODEV,
> pci_epf_alloc_doorbell() falls back to this embedded backend.
> 
> Signed-off-by: Koichiro Den <[email protected]>
> ---
>  drivers/pci/endpoint/pci-ep-msi.c | 90 +++++++++++++++++++++++++++++++
>  1 file changed, 90 insertions(+)
> 
> diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
> index a42f69ad24ad..6e1524c2d891 100644
> --- a/drivers/pci/endpoint/pci-ep-msi.c
> +++ b/drivers/pci/endpoint/pci-ep-msi.c
> @@ -6,6 +6,7 @@
>   * Author: Frank Li <[email protected]>
>   */
>  
> +#include <linux/cleanup.h>
>  #include <linux/device.h>
>  #include <linux/export.h>
>  #include <linux/interrupt.h>
> @@ -36,6 +37,82 @@ static void pci_epf_write_msi_msg(struct msi_desc *desc, struct msi_msg *msg)
>  	pci_epc_put(epc);
>  }
>  
> +static int pci_epf_alloc_doorbell_embedded(struct pci_epf *epf, u16 num_db)
> +{
> +	const struct pci_epc_aux_resource *doorbell = NULL;
> +	struct pci_epf_doorbell_msg *msg;
> +	struct pci_epc *epc = epf->epc;
> +	struct device *dev = &epf->dev;
> +	int count, ret, i;
> +	u64 addr;
> +
> +	count = pci_epc_get_aux_resources(epc, epf->func_no, epf->vfunc_no,
> +					  NULL, 0);
> +	if (count == -EOPNOTSUPP || count == 0)
> +		return -ENODEV;
> +	if (count < 0)
> +		return count;
> +
> +	struct pci_epc_aux_resource *res __free(kfree) =
> +				kcalloc(count, sizeof(*res), GFP_KERNEL);
> +	if (!res)
> +		return -ENOMEM;
> +
> +	ret = pci_epc_get_aux_resources(epc, epf->func_no, epf->vfunc_no,
> +					res, count);
> +	if (ret == -EOPNOTSUPP || ret == 0)
> +		return -ENODEV;
> +	if (ret < 0)
> +		return ret;
> +
> +	count = ret;
> +
> +	for (i = 0; i < count; i++) {
> +		if (res[i].type == PCI_EPC_AUX_DOORBELL_MMIO) {
> +			if (doorbell) {
> +				dev_warn(dev,
> +					 "Duplicate DOORBELL_MMIO resource found\n");
> +				continue;
> +			}
> +			doorbell = &res[i];
> +		}
> +	}
> +	if (!doorbell)
> +		return -ENODEV;
> +
> +	msg = kcalloc(num_db, sizeof(*msg), GFP_KERNEL);
> +	if (!msg)
> +		return -ENOMEM;
> +
> +	addr = doorbell->phys_addr;
> +
> +	/*
> +	 * Embedded doorbell backends (e.g. DesignWare eDMA interrupt emulation)
> +	 * typically provide a single IRQ and do not offer per-doorbell
> +	 * distinguishable address/data pairs. The EPC aux resource therefore
> +	 * exposes one DOORBELL_MMIO entry (u.db_mmio.irq).
> +	 *
> +	 * Still, pci_epf_alloc_doorbell() allows requesting multiple doorbells.
> +	 * For such backends we replicate the same address/data for each entry
> +	 * and mark the IRQ as shared (IRQF_SHARED). Consumers must treat them
> +	 * as equivalent "kick" doorbells.
> +	 */
> +	for (i = 0; i < num_db; i++) {
> +		msg[i].msg.address_lo = (u32)addr;
> +		msg[i].msg.address_hi = (u32)(addr >> 32);
> +		msg[i].msg.data = 0;

Hi Frank,

Thanks for your feedback on [PATCH v7 6/9]:
https://lore.kernel.org/linux-pci/aZNA3lQCne2laAW6@lizhi-Precision-Tower-5810/
I realized that pci_epf_alloc_doorbell_embedded() assumes a "write-0-to-assert"
semantics, which appears to be specific to DesignWare eDMA interrupt emulation.

I'm wondering whether it would be better to extend pci_epc_aux_resource
(introduced in [PATCH v7 3/9]) from the beginning, instead of deferring the
change until another vendor wants to support a PCI_EPC_AUX_DOORBELL_MMIO
resource description with different semantics (i.e. not "write-0-to-assert"):

Personally, I'm inclined to extend it at this point, like this:

     struct pci_epc_aux_resource {
             enum pci_epc_aux_resource_type type;
             phys_addr_t phys_addr;
             resource_size_t size;
             enum pci_barno bar;
             resource_size_t bar_offset;
     
             union {
                     /* PCI_EPC_AUX_DOORBELL_MMIO */
                     struct {
                             int irq;
    +                        u32 data; /* write data to kick */
                     } db_mmio;
             } u;
     };

In that case, I suppose I would need to drop your Reviewed-by tag for that commit.

Best regards,
Koichiro

> +		msg[i].virq = doorbell->u.db_mmio.irq;
> +		msg[i].irq_flags = IRQF_SHARED;
> +		msg[i].type = PCI_EPF_DOORBELL_EMBEDDED;
> +		msg[i].bar = doorbell->bar;
> +		msg[i].offset = (doorbell->bar == NO_BAR) ? 0 : doorbell->bar_offset;
> +	}
> +
> +	epf->num_db = num_db;
> +	epf->db_msg = msg;
> +	return 0;
> +}
> +
>  static int pci_epf_alloc_doorbell_msi(struct pci_epf *epf, u16 num_db)
>  {
>  	struct pci_epf_doorbell_msg *msg;
> @@ -110,6 +187,19 @@ int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db)
>  	if (!ret)
>  		return 0;
>  
> +	/*
> +	 * Fall back to embedded doorbell only when platform MSI is unavailable
> +	 * for this EPC.
> +	 */
> +	if (ret != -ENODEV)
> +		return ret;
> +
> +	ret = pci_epf_alloc_doorbell_embedded(epf, num_db);
> +	if (!ret) {
> +		dev_info(dev, "Using embedded (DMA) doorbell fallback\n");
> +		return 0;
> +	}
> +
>  	dev_err(dev, "Failed to allocate doorbell: %d\n", ret);
>  	return ret;
>  }
> -- 
> 2.51.0
> 
>