Support doorbell backends where the doorbell target is already exposed
via a platform-owned fixed BAR mapping and/or where the doorbell IRQ
must be requested with specific flags.
When pci_epf_alloc_doorbell() provides db_msg[].bar/offset, reuse the
pre-exposed BAR window and skip programming a new inbound mapping. Also
honor db_msg[].irq_flags when requesting the doorbell IRQ.
Multiple doorbells may share the same Linux IRQ. Avoid duplicate
request_irq() calls by requesting each unique virq once.
Make pci-epf-vntb work with platform-defined or embedded doorbell
backends without exposing backend-specific details to the consumer
layer.
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes since v8:
- Reword the last paragraph into imperative mood
- Rename s/epf_ntb_db_irq_is_first/epf_ntb_db_irq_is_duplicated/ and
invert the returned bool value
drivers/pci/endpoint/functions/pci-epf-vntb.c | 61 ++++++++++++++++++-
1 file changed, 58 insertions(+), 3 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index 52cf442ca1d9..7a27e9343394 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -134,6 +134,11 @@ struct epf_ntb {
u16 vntb_vid;
bool linkup;
+
+ /*
+ * True when doorbells are interrupt-driven (MSI or embedded), false
+ * when polled.
+ */
bool msi_doorbell;
u32 spad_size;
@@ -517,6 +522,17 @@ static int epf_ntb_configure_interrupt(struct epf_ntb *ntb)
return 0;
}
+static bool epf_ntb_db_irq_is_duplicated(const struct pci_epf *epf, unsigned int idx)
+{
+ unsigned int i;
+
+ for (i = 0; i < idx; i++)
+ if (epf->db_msg[i].virq == epf->db_msg[idx].virq)
+ return true;
+
+ return false;
+}
+
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,
@@ -533,9 +549,24 @@ static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
if (ret)
return ret;
+ /*
+ * The doorbell target may already be exposed by a platform-owned fixed
+ * BAR. In that case, we must reuse it and the requested db_bar must
+ * match.
+ */
+ if (epf->db_msg[0].bar != NO_BAR && epf->db_msg[0].bar != barno) {
+ ret = -EINVAL;
+ goto err_free_doorbell;
+ }
+
for (req = 0; req < ntb->db_count; req++) {
+ /* Avoid requesting duplicate handlers */
+ if (epf_ntb_db_irq_is_duplicated(epf, req))
+ continue;
+
ret = request_irq(epf->db_msg[req].virq, epf_ntb_doorbell_handler,
- 0, "pci_epf_vntb_db", ntb);
+ epf->db_msg[req].irq_flags, "pci_epf_vntb_db",
+ ntb);
if (ret) {
dev_err(&epf->dev,
@@ -545,6 +576,22 @@ static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
}
}
+ if (epf->db_msg[0].bar != NO_BAR) {
+ for (i = 0; i < ntb->db_count; i++) {
+ msg = &epf->db_msg[i].msg;
+
+ if (epf->db_msg[i].bar != barno) {
+ ret = -EINVAL;
+ goto err_free_irq;
+ }
+
+ ntb->reg->db_data[i] = msg->data;
+ ntb->reg->db_offset[i] = epf->db_msg[i].offset;
+ }
+ goto out;
+ }
+
+ /* Program inbound mapping for the doorbell */
msg = &epf->db_msg[0].msg;
high = 0;
@@ -591,6 +638,7 @@ static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
ntb->reg->db_offset[i] = offset;
}
+out:
ntb->reg->db_entry_size = 0;
ntb->msi_doorbell = true;
@@ -598,9 +646,13 @@ static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
return 0;
err_free_irq:
- for (req--; req >= 0; req--)
+ for (req--; req >= 0; req--) {
+ if (epf_ntb_db_irq_is_duplicated(epf, req))
+ continue;
free_irq(epf->db_msg[req].virq, ntb);
+ }
+err_free_doorbell:
pci_epf_free_doorbell(ntb->epf);
return ret;
}
@@ -666,8 +718,11 @@ static void epf_ntb_db_bar_clear(struct epf_ntb *ntb)
if (ntb->msi_doorbell) {
int i;
- for (i = 0; i < ntb->db_count; i++)
+ for (i = 0; i < ntb->db_count; i++) {
+ if (epf_ntb_db_irq_is_duplicated(ntb->epf, i))
+ continue;
free_irq(ntb->epf->db_msg[i].virq, ntb);
+ }
}
if (ntb->epf->db_msg)
--
2.51.0
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()?
> @@ -533,9 +549,24 @@ static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> if (ret)
> return ret;
>
> + /*
> + * The doorbell target may already be exposed by a platform-owned fixed
> + * BAR. In that case, we must reuse it and the requested db_bar must
> + * match.
> + */
> + if (epf->db_msg[0].bar != NO_BAR && epf->db_msg[0].bar != barno) {
> + ret = -EINVAL;
> + goto err_free_doorbell;
> + }
> +
> for (req = 0; req < ntb->db_count; req++) {
> + /* Avoid requesting duplicate handlers */
> + if (epf_ntb_db_irq_is_duplicated(epf, req))
> + continue;
> +
> ret = request_irq(epf->db_msg[req].virq, epf_ntb_doorbell_handler,
> - 0, "pci_epf_vntb_db", ntb);
> +
Thanks,
Alok
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.
That said, pci_epc_get_features() is documented to return NULL on failure, so
adding defensive checks would certainly imnprove robustness and align vNTB with
other EPF drivers.
Since this is independent of the doorbell rework in this series, I think it
would probably cleaner to address it in a separate patch.
If you are planning to send such a patch, I would be happy to test and/or review
it. Otherwise, I can prepare a small follow-up patch (with a Reported-by tag)
when I have a spare cycle. Given that this is pre-existing and does not seem to
cause observable issues today, I do not think it requires a Fixes: tag or stable
backporting.
Best regards,
Koichiro
>
> > @@ -533,9 +549,24 @@ static int epf_ntb_db_bar_init_msi_doorbell(struct epf_ntb *ntb,
> > if (ret)
> > return ret;
> > + /*
> > + * The doorbell target may already be exposed by a platform-owned fixed
> > + * BAR. In that case, we must reuse it and the requested db_bar must
> > + * match.
> > + */
> > + if (epf->db_msg[0].bar != NO_BAR && epf->db_msg[0].bar != barno) {
> > + ret = -EINVAL;
> > + goto err_free_doorbell;
> > + }
> > +
> > for (req = 0; req < ntb->db_count; req++) {
> > + /* Avoid requesting duplicate handlers */
> > + if (epf_ntb_db_irq_is_duplicated(epf, req))
> > + continue;
> > +
> > ret = request_irq(epf->db_msg[req].virq, epf_ntb_doorbell_handler,
> > - 0, "pci_epf_vntb_db", ntb);
> > +
>
>
> Thanks,
> Alok
On 2/20/2026 9:05 AM, Koichiro Den wrote: > Since this is independent of the doorbell rework in this series, I think it > would probably cleaner to address it in a separate patch. Yes, agreed this is independent of the doorbell rework. > > If you are planning to send such a patch, I would be happy to test and/or review > it. Otherwise, I can prepare a small follow-up patch (with a Reported-by tag) > when I have a spare cycle. Given that this is pre-existing and does not seem to > cause observable issues today, I do not think it requires a Fixes: tag or stable > backporting. I will send a separate patch for this. > > Best regards, > Koichiro Thanks, Alok
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
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
© 2016 - 2026 Red Hat, Inc.