Re: [PATCH v11 7/7] PCI: endpoint: pci-ep-msi: Add embedded doorbell fallback

[Manivannan Sadhasivam]

On Tue, Mar 24, 2026 at 05:37:28PM +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 DMA address returned by dma_map_resource() (which may be an IOVA
> when an IOMMU is enabled) 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.
> 
> Tested-by: Niklas Cassel <cassel@xxxxxxxxxx>
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>
> ---
>  drivers/pci/endpoint/pci-ep-msi.c | 139 +++++++++++++++++++++++++++++-
>  include/linux/pci-epf.h           |   8 ++
>  2 files changed, 144 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c
> index 85fe46103220..331d84a79193 100644
> --- a/drivers/pci/endpoint/pci-ep-msi.c
> +++ b/drivers/pci/endpoint/pci-ep-msi.c
> @@ -6,6 +6,8 @@
>   * Author: Frank Li <Frank.Li@xxxxxxx>
>   */
>  
> +#include <linux/align.h>
> +#include <linux/cleanup.h>
>  #include <linux/device.h>
>  #include <linux/export.h>
>  #include <linux/interrupt.h>
> @@ -36,6 +38,117 @@ 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;
> +	size_t map_size = 0, off = 0;
> +	dma_addr_t iova_base = 0;
> +	phys_addr_t phys_base;
> +	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;

This should be avoided...

resource_count API should return 0 for success and errno for failure with a
separate argument for resource count.

And resource_get API should return 0 for success and errno for failure.

> +
> +	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;

And here, you do the same :/

> +
> +	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");

I think technically it makes sense to have more than one doorbell resource per
endpoint. You may not warn here, but just ensure that you use the first unused
one. This also means, you need to track the allocated resource.

If you want to avoid the hassle, just assume that there will be only one
doorbell resource and add a TODO to extend it if needed.

- Mani

-- 
மணிவண்ணன் சதாசிவம்