Re: [PATCH v11 7/7] PCI: endpoint: pci-ep-msi: Add embedded doorbell fallback
Koichiro Den <[email protected]> Fri, 27 Mar 2026 00:11:48 +0900
| Newsgroups | dev.linux.lists.ntb,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci |
|---|---|
| Message-ID | <r3vshsllq35op2dyajq2sr6el7ibxunmlu4r4hsvfs2v2237go@jcwp7xku6not> |
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 <[email protected]> > > Signed-off-by: Koichiro Den <[email protected]> > > --- > > 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 <[email protected]> > > */ > > > > +#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 > > -- > மணிவண்ணன் சதாசிவம்