From: Niklas Cassel <cassel@kernel.org>
To: Koichiro Den <den@valinux.co.jp>
Cc: ALOK TIWARI <alok.a.tiwari@oracle.com>,
jingoohan1@gmail.com, mani@kernel.org, lpieralisi@kernel.org,
kwilczynski@kernel.org, robh@kernel.org, bhelgaas@google.com,
kishon@kernel.org, jdmason@kudzu.us, dave.jiang@intel.com,
allenbh@gmail.com, Frank.Li@nxp.com, shinichiro.kawasaki@wdc.com,
christian.bruel@foss.st.com, mmaddireddy@nvidia.com,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
ntb@lists.linux.dev
Subject: Re: [PATCH v9 5/7] PCI: endpoint: pci-epf-vntb: Reuse pre-exposed doorbells and IRQ flags
Date: Fri, 20 Feb 2026 11:27:05 +0100 [thread overview]
Message-ID: <aZg28ligp2ovwuiT@ryzen> (raw)
In-Reply-To: <4hn7xbjltx6z37j5foj4mikuz5t5wntf4pr4hxiy577ebuw24w@efke5jakhhjh>
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.
Kind regards,
Niklas
next prev parent reply other threads:[~2026-02-20 10:27 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-19 8:13 [PATCH v9 0/7] PCI: endpoint: pci-ep-msi: Add embedded doorbell fallback Koichiro Den
2026-02-19 8:13 ` [PATCH v9 1/7] PCI: endpoint: Add auxiliary resource query API Koichiro Den
2026-02-19 8:13 ` [PATCH v9 2/7] PCI: dwc: Record integrated eDMA register window Koichiro Den
2026-02-19 8:13 ` [PATCH v9 3/7] PCI: dwc: ep: Expose integrated eDMA resources via EPC aux-resource API Koichiro Den
2026-02-19 8:13 ` [PATCH v9 4/7] PCI: endpoint: pci-ep-msi: Refactor doorbell allocation for new backends Koichiro Den
2026-02-19 8:13 ` [PATCH v9 5/7] PCI: endpoint: pci-epf-vntb: Reuse pre-exposed doorbells and IRQ flags Koichiro Den
2026-02-19 16:30 ` ALOK TIWARI
2026-02-20 3:35 ` Koichiro Den
2026-02-20 10:27 ` Niklas Cassel [this message]
2026-02-20 17:53 ` Koichiro Den
2026-02-20 15:14 ` ALOK TIWARI
2026-02-19 8:13 ` [PATCH v9 6/7] PCI: endpoint: pci-epf-test: Reuse pre-exposed doorbell targets Koichiro Den
2026-02-19 8:13 ` [PATCH v9 7/7] PCI: endpoint: pci-ep-msi: Add embedded doorbell fallback Koichiro Den
2026-02-20 17:42 ` Koichiro Den
2026-02-20 17:59 ` Koichiro Den
2026-02-27 15:34 ` Niklas Cassel
2026-02-28 16:33 ` Koichiro Den
2026-02-27 15:06 ` [PATCH v9 0/7] " Niklas Cassel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aZg28ligp2ovwuiT@ryzen \
--to=cassel@kernel.org \
--cc=Frank.Li@nxp.com \
--cc=allenbh@gmail.com \
--cc=alok.a.tiwari@oracle.com \
--cc=bhelgaas@google.com \
--cc=christian.bruel@foss.st.com \
--cc=dave.jiang@intel.com \
--cc=den@valinux.co.jp \
--cc=jdmason@kudzu.us \
--cc=jingoohan1@gmail.com \
--cc=kishon@kernel.org \
--cc=kwilczynski@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=lpieralisi@kernel.org \
--cc=mani@kernel.org \
--cc=mmaddireddy@nvidia.com \
--cc=ntb@lists.linux.dev \
--cc=robh@kernel.org \
--cc=shinichiro.kawasaki@wdc.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.