Re: [PATCH 1/3] PCI: endpoint: pci-epf-vntb: Pass PF/VF number when BAR programming

[Bjorn Helgaas <helgaas@kernel.org>]

On Wed, Sep 09, 2026 at 01:53:53PM +0900, Koichiro Den wrote:
> On Tue, Sep 08, 2026 at 09:23:11PM -0500, Bjorn Helgaas wrote:
> > On Wed, Sep 09, 2026 at 11:01:48AM +0900, Koichiro Den wrote:
> > > On Tue, Sep 08, 2026 at 11:53:15AM -0500, Bjorn Helgaas wrote:
> > > > On Wed, Jul 29, 2026 at 02:23:04AM +0900, Koichiro Den wrote:
> > > > > vntb_epf_mw_set_trans() programs the memory-window BAR through
> > > > > pci_epc_set_bar() with hardcoded function numbers (0, 0), so the BAR
> > > > > lands on the wrong function whenever the vNTB EPF is bound to anything
> > > > > but PF0. The other BAR programming sites in the vNTB driver already pass
> > > > > the EPF's own numbers.
> > > > > 
> > > > > Pass the EPF's own func_no/vfunc_no here as well.
> > > > 
> > > > We're referring to these as "PF" and "VF" in the subject and "physical
> > > > endpoint function" and "virtual endpoint function" in the
> > > > pci_epc_set_bar() kernel-doc, but I don't think these have anything to
> > > > do with the SR-IOV PF and VF concepts, do they?
> > > 
> > > Not in the specific case that motivated this series. But AFAICT, the EPC API
> > > uses the same (func_no, vfunc_no) pair to cover both ordinary functions and
> > > SR-IOV PFs/VFs scenarios. With vfunc_no == 0, func_no can identify either an
> > > ordinary function or an SR-IOV PF. A non-zero vfunc_no identifies a VF
> > > associated with the PF selected by func_no.
> > 
> > Now I'm even more confused :)
> > 
> > Are you saying that a non-zero vfunc_no always identifies an SR-IOV
> > VF?  And there's some dependency on that?  I don't any mention of
> > "iov" in drivers/pci/endpoint/.
> 
> Yes, that is my understanding of the current in-tree implementation. You're
> right that drivers/pci/endpoint/ itself contains no explicit reference to
> SR-IOV. E.g. pci_epf_add_vepf() just calls it a "virtual EP function".
> So my saying was kind of assumptive, but I still think the same because:
> 
> - The support was introduced for SR-IOV:
>   https://lore.kernel.org/r/20210819123343.1951-1-kishon@ti.com/
> 
> - The core rejects a non-zero vfunc_no unless the EPC provides max_vfs, and
>   Cadence is the only in-tree EPC driver I found that does so. For example,
>   cdns_pcie_ep_set_bar() calls cdns_pcie_get_fn_from_vfn(), which uses the
>   SR-IOV First VF Offset and VF Stride for a non-zero vfn.

Thanks, that's helpful.  I still have to work hard to change my point
of view from host-side drivers to endpoint drivers operating on the
other end of the link.  The fact that there are several interfaces
that need (func_no, vfunc_no) suggests that callers really do need to
understand what's going on, and maybe we should try to connect the
kernel-doc and abbreviations more closely with PCIe spec terms.

E.g., if "physical EP function" and "virtual EP function" refer to
SR-IOV PF and VF, maybe we should word them as "endpoint PF" or
"endpoint VF" (or "EP PF", "EP VF" for short).  If
"pci_epf_add_vepf()" adds an SR-IOV VF, maybe "pci_epf_add_vf()" would
be descriptive enough.  We already know we're on the endpoint because
of "epf", so we probably don't need another hint in "vepf", which
includes a "pf" that doesn't mean SR-IOV PF.

> > > > I don't have a better naming suggestion, but this is slightly
> > > > confusing.
> > > 
> > > Perhaps a better subject might be:
> > > 
> > >   PCI: endpoint: pci-epf-vntb: Pass (func_no, vfunc_no) when programming BARs
> > > 
> > > Best regards,
> > > Koichiro
> > > 
> > > > 
> > > > > Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
> > > > > Signed-off-by: Koichiro Den <den@valinux.co.jp>
> > > > > ---
> > > > >  drivers/pci/endpoint/functions/pci-epf-vntb.c | 3 ++-
> > > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > > > index c3caec927d74..fba65abfb6b2 100644
> > > > > --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > > > +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> > > > > @@ -1427,7 +1427,8 @@ static int vntb_epf_mw_set_trans(struct ntb_dev *ndev, int pidx, int idx,
> > > > >  	epf_bar->barno = barno;
> > > > >  	epf_bar->size = size;
> > > > >  
> > > > > -	ret = pci_epc_set_bar(ntb->epf->epc, 0, 0, epf_bar);
> > > > > +	ret = pci_epc_set_bar(ntb->epf->epc, ntb->epf->func_no,
> > > > > +			      ntb->epf->vfunc_no, epf_bar);
> > > > >  	if (ret) {
> > > > >  		dev_err(dev, "failure set mw trans\n");
> > > > >  		return ret;
> > > > > -- 
> > > > > 2.51.0
> > > > >