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

[Koichiro Den]

On Thu, Mar 26, 2026 at 06:09:18PM +0530, Manivannan Sadhasivam wrote:
> 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 :/

Thanks for the review.

I'll update these to use the new pci_epc_count_aux_resources(), as suggested in
[PATCH v11 1/7].

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

I think I'll go with the latter approach for now.

The only provider added in this series (see [PATCH v11 3/7]) exposes a single
doorbell resource, so I think it is reasonable to keep that assumption for now
and extend it later if and when a real multi-doorbell provider appears.
This helps keep this series compact and reviewable too.

Best regards,
Koichiro

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