Re: [PATCH v9 5/7] PCI: endpoint: pci-epf-vntb: Reuse pre-exposed doorbells and IRQ flags

[Koichiro Den]

On Fri, Feb 20, 2026 at 11:27:05AM +0100, Niklas Cassel wrote:
> On Fri, Feb 20, 2026 at 12:35:31PM +0900, Koichiro Den wrote:
> > On Thu, Feb 19, 2026 at 10:00:19PM +0530, ALOK TIWARI wrote:
> > > 
> > > 
> > > On 2/19/2026 1:43 PM, Koichiro Den wrote:
> > > >   static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> > > >   					    struct pci_epf_bar *db_bar,
> > > >   					    const struct pci_epc_features *epc_features,
> > > 
> > > The return value of pci_epc_get_features() seems to be used here
> > > without checking for NULL.
> > > 
> > > Since this function can return NULL, and other EPF drivers
> > > (pci-epf-test.c, pci-epf-ntb.c) handle that case,
> > > is VNTB assuming that epc_features is always non-NULL,
> > > or should a defensive NULL check be added for pci_epc_get_features()?
> > 
> > Thanks for the comment, good catch.
> > 
> > AFAICT, this is a pre-existing issue (at least since the initial vNTB merge,
> > commit e35f56bb0330), and the same pattern can be found in a few other paths in
> > epf-vntb, such as:
> > 
> >   - epf_ntb_config_spad_bar_alloc()
> >   - epf_ntb_configure_interrupt()
> >   - epf_ntb_db_bar_init() (the one you pointed out)
> > 
> > From the current mainline state, all in-tree pci_epc_ops implementations provide
> > a .get_features callback and return a non-NULL pointer, and the same holds for
> > the in-tree dw_pcie_ep_ops implementations. So in practice this does not appear
> > to be triggering a NULL-dereference issue today.
> 
> We should really clean this up somehow.
> 
> 
> The problems are:
> 
> 1) A long time ago, not all EPC driver had a get_features callback.
>    Now, EPC drivers do have such a callback.
>    Ideally, we should probably add a check that an EPC driver implements
>    epc->ops_get_features in __pci_epc_create(), and return failure if it
>    doesn't.
> 
> This way we can remove the if (!epc->ops_get_features) check in e.g.
> pci_epc_get_features().
> 
> 
> 2) DWC based glue drivers have their own get_features callback in
> struct dw_pcie_ep
> But here we should just have some check in dw_pcie_ep_init() that
> returns failure if the glue driver has not implemented
> (struct dw_pcie_ep *)->ops->get_features)
> 
> This way we can remove the
> if (!ep->ops->get_features) checks in pcie-designware-ep.c.
> 
> 
> 3) Even if the get_features callback is implemented, EPF drivers call
>    pci_epc_get_features(), which has this code:
> 
>         if (!pci_epc_function_is_valid(epc, func_no, vfunc_no))
>                 return NULL;
> 
> So, it will return NULL for invalid func_no / vfunc_no.
> 
> I think this currently makes it quite hard to remove the NULL checks on the
> return value from a epc->ops_get_features() call in the EPF drivers.
> 
> 
> 
> 
> How pci-epf-test has managed to "workaround" this the silliness of having
> 
> features = pci_epc_get_features(epc, func_no, vfunc_no);
> if (!features)
> 
> checks everywhere (problem 3): It calls pci_epc_get_features() once in .bind()
> and if it fails, it fails bind(), if it returns non-NULL, it caches the result:
> https://github.com/torvalds/linux/blob/v6.19/drivers/pci/endpoint/functions/pci-epf-test.c#L1112-L1123
> 
> That way, all other places in pci-epf-test.c does not need to NULL check
> pci_epc_get_features(). (Instead it uses the cached value in struct pci_epf_test *)
> 
> pci-epf-vntb.c should probably do something similar to avoid sprinkling
> NULL checks all over pci-epf-vntb.c.

Niklas, agreed, I had the same thought (ie. bind-time check could be
sufficient). Thanks for the follow-up.

Alok, thanks for picking it up.

Best regards,
Koichiro


> 
> 
> Kind regards,
> Niklas