[PATCH v6 0/2] PCI: endpoint: pci-epf-*ntb: Harden vNTB resource management

[PATCH v6 0/2] PCI: endpoint: pci-epf-*ntb: Harden vNTB resource management

[Koichiro Den]

Hi,

This is a follow-up to v5:

  https://lore.kernel.org/r/20260226084142.2226875-1-den@valinux.co.jp/

Patches 1-3 from v5 were applied. This v6 addresses the remaining
lifetime issue. Mani, sorry for the delay.

Before this series:

  bind:   EPC resources -> virtual PCI bus -> ntb_dev -> NTB clients
  unbind: EPC resources freed
          virtual PCI hierarchy, ntb_dev, and NTB clients left behind

	  (The bus returned by pci_scan_bus() was not retained, and
	   pci-vntb had no .remove callback.)

After this series:

  bind:   EPC resources -> virtual PCI root bus -> ntb_dev -> NTB clients
  unbind: NTB clients -> ntb_dev -> virtual PCI root bus -> EPC resources


Testing
=======

I ran the following script on R-Car S4. It only checks removal and
re-creation of the local virtual PCI and NTB devices. No peer or PCIe
link is needed.

  #!/bin/bash
  set -eu

  modprobe ntb_transport
  modprobe pci_epf_vntb

  cd /sys/kernel/config/pci_ep
  F=functions/pci_epf_vntb/func0
  V=$F/pci_epf_vntb.0
  C=e65d0000.pcie-ep

  mkdir "$F"
  echo 0x1957   > "$F/vendorid"
  echo 0x0809   > "$F/deviceid"
  echo 16       > "$V/db_count"
  echo 128      > "$V/spad_count"
  echo 1        > "$V/num_mws"
  echo 0x100000 > "$V/mw1"
  echo 0x1957   > "$V/vntb_vid"
  echo 0x080a   > "$V/vntb_pid"
  echo 0x10     > "$V/vbus_number"
  echo 0        > "$V/ctrl_bar"
  echo 2        > "$V/mw1_bar"

  ln -s "controllers/$C" "$F/primary/"
  modprobe ntb_netdev

  unlink "$F/primary/$C"
  rmmod ntb_netdev
  rmmod ntb_transport

  test ! -e /sys/bus/pci/devices/0001:10:00.0
  test ! -e /sys/class/pci_bus/0001:10
  test ! -e /sys/bus/ntb/devices/0001:10:00.0

  ln -s "controllers/$C" "$F/primary/"
  modprobe ntb_transport
  modprobe ntb_netdev

  test -e /sys/bus/pci/devices/0001:10:00.0
  test -e /sys/class/pci_bus/0001:10
  test -e /sys/bus/ntb/devices/0001:10:00.0

  echo PASSED

Before this series, removing ntb_transport after the EPF unlink
panicked:

  Unable to handle kernel NULL pointer dereference at virtual address 0000000000000040
  pc : vntb_epf_get_dma_dev+0x10/0x20 [pci_epf_vntb]
  lr : ntb_free_mw+0x38/0xd0 [ntb_transport]
  ...
  ntb_transport_free+0x2c/0x1c8 [ntb_transport]
  ntb_remove+0x2c/0x48 [ntb]
  ...
  ntb_unregister_client+0x14/0x28 [ntb]
  ntb_transport_exit+0x18/0xb70 [ntb_transport]
  ...
  Kernel panic - not syncing: Oops: Fatal exception

Unlinking the EPF freed endpoint resources and cleared epf->epc, but
left the NTB device registered, Removing ntb_transport later
dereferenced the NULL pointer.

After this series, unlinking the EPF removed both the virtual PCI bus
and the NTB device. The same EPF could then be linked again, and the
script printed:

  PASSED

Best regards,
Koichiro
---
Changes in v6:
  - Add the missing PCI rescan/remove lock noted during review of v5
    patch 4.
  - Rework v5 patch 5.
  - Drop v5 patch 4, as f7245901de89 fixed the NULL-parent bug it worked
    around.

v5: https://lore.kernel.org/r/20260226084142.2226875-1-den@valinux.co.jp/


Koichiro Den (2):
  PCI: endpoint: pci-epf-vntb: Serialize virtual PCI bus scan
  PCI: endpoint: pci-epf-vntb: Manage virtual NTB and PCI bus lifetime

 drivers/pci/endpoint/functions/pci-epf-vntb.c | 90 +++++++++++++++----
 1 file changed, 75 insertions(+), 15 deletions(-)


base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
prerequisite-patch-id: 0000000000000000000000000000000000000000
-- 
2.51.0

[PATCH v6 1/2] PCI: endpoint: pci-epf-vntb: Serialize virtual PCI bus scan

[Koichiro Den]

The virtual PCI bus is scanned without holding the PCI rescan/remove
lock. This can race with other PCI topology changes.

Hold the lock across the bus scan and device addition.

Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
Suggested-by: Manivannan Sadhasivam <mani@kernel.org>
Link: https://lore.kernel.org/r/3tgrcel5fl55ulf3zjya4fkz4t2ms7iwwjif5rnzn2rafsydd6@i4e5etm7uqvz/
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v6:
  - New patch based on Mani's review of v5 patch 4.

 drivers/pci/endpoint/functions/pci-epf-vntb.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index c3caec927d74..412e8cc6fb1d 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -1332,14 +1332,19 @@ static int vpci_scan_bus(void *sysdata)
 	struct pci_bus *vpci_bus;
 	struct epf_ntb *ndev = sysdata;
 
+	pci_lock_rescan_remove();
+
 	vpci_bus = pci_scan_bus(ndev->vbus_number, &vpci_ops, sysdata);
 	if (!vpci_bus) {
 		pr_err("create pci bus failed\n");
+		pci_unlock_rescan_remove();
 		return -EINVAL;
 	}
 
 	pci_bus_add_devices(vpci_bus);
 
+	pci_unlock_rescan_remove();
+
 	return 0;
 }
 
-- 
2.51.0

Re: [PATCH v6 1/2] PCI: endpoint: pci-epf-vntb: Serialize virtual PCI bus scan

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Global static PCI driver and ID table are mutated and registered inside a per-device bind callback, preventing multi-instance support and corrupting global state.
- [High] Improper mixing of C struct accesses and I/O accessors (readl/writel) on a DMA coherent RAM pointer.
--

commit 52fb844432dfeb3fe13eef7dc35765e5cdc04cfa
Author: Koichiro Den <den@valinux.co.jp>

PCI: endpoint: pci-epf-vntb: Serialize virtual PCI bus scan

The virtual PCI bus is scanned without holding the PCI rescan/remove
lock. This can race with other PCI topology changes.

Hold the lock across the bus scan and device addition.

Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
Link: https://lore.kernel.org/r/3tgrcel5fl55ulf3zjya4fkz4t2ms7iwwjif5rnzn2rafsydd6@i4e5etm7uqvz/

> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index c3caec927d748..412e8cc6fb1d2 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c

[ ... ]

While reviewing this file, I noticed a couple of pre-existing issues.

drivers/pci/endpoint/functions/pci-epf-vntb.c:epf_ntb_bind() {
    ...
	pci_space[0] = (ntb->vntb_pid << 16) | ntb->vntb_vid;
	pci_vntb_table[0].vendor = ntb->vntb_vid;
	pci_vntb_table[0].device = ntb->vntb_pid;

	ret = pci_register_driver(&vntb_pci_driver);
    ...
}

[Severity: High]
This isn't a bug introduced by this patch, but does this code corrupt global
state if multiple instances of the endpoint function are bound?

If multiple endpoint functions are bound via configfs, a second call to
epf_ntb_bind() would overwrite the shared pci_vntb_table[0] and pci_space[0]
without synchronization, corrupting the first instance's state.

Also, won't calling pci_register_driver(&vntb_pci_driver) a second time fail
with -EEXIST, preventing the bind? Should this dynamic registration of a
global PCI driver be moved to the module initialization sequence instead?


A second issue occurs when allocating and accessing spad memory:

drivers/pci/endpoint/functions/pci-epf-vntb.c:epf_ntb_config_spad_bar_alloc() {
    ...
	base = pci_epf_alloc_space(epf, ctrl_size + spad_size,
				   barno, epc_features, 0);
    ...
	ntb->reg = base;

	ctrl = ntb->reg;
	ctrl->spad_offset = ctrl_size;
    ...
}

The exact same RAM pointer is then accessed via I/O accessors:

drivers/pci/endpoint/functions/pci-epf-vntb.c:vntb_epf_spad_read() {
    ...
	void __iomem *base = (void __iomem *)ntb->reg;

	val = readl(base + off + ct + idx * sizeof(u32));
    ...
}

[Severity: High]
This is a pre-existing issue, but does this code cause undefined behavior by
mixing C struct accesses and I/O accessors on a standard RAM pointer?

The driver allocates memory via pci_epf_alloc_space() (which uses
dma_alloc_coherent()) and assigns the returned standard virtual memory
pointer to ntb->reg.

Later, in vntb_epf_spad_read(), this standard RAM pointer is forcefully cast
to void __iomem * and accessed via readl().

On architectures with strictly separated I/O memory or strictly typed memory
domains, using MMIO instructions on standard cacheable memory can trigger
exceptions or alignment faults. Is it possible to avoid using MMIO accessors
on memory that isn't mapped as I/O memory?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901063238.631166-1-den@valinux.co.jp?part=1

Re: [PATCH v6 1/2] PCI: endpoint: pci-epf-vntb: Serialize virtual PCI bus scan

[Frank Li]

On Tue, Sep 01, 2026 at 03:32:37PM +0900, Koichiro Den wrote:
> The virtual PCI bus is scanned without holding the PCI rescan/remove
> lock. This can race with other PCI topology changes.
>
> Hold the lock across the bus scan and device addition.
>
> Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
> Suggested-by: Manivannan Sadhasivam <mani@kernel.org>
> Link: https://lore.kernel.org/r/3tgrcel5fl55ulf3zjya4fkz4t2ms7iwwjif5rnzn2rafsydd6@i4e5etm7uqvz/
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---

It is okay for now, I suggest define cleanup by use guard() in future.

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v6:
>   - New patch based on Mani's review of v5 patch 4.
>
>  drivers/pci/endpoint/functions/pci-epf-vntb.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index c3caec927d74..412e8cc6fb1d 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -1332,14 +1332,19 @@ static int vpci_scan_bus(void *sysdata)
>  	struct pci_bus *vpci_bus;
>  	struct epf_ntb *ndev = sysdata;
>
> +	pci_lock_rescan_remove();
> +
>  	vpci_bus = pci_scan_bus(ndev->vbus_number, &vpci_ops, sysdata);
>  	if (!vpci_bus) {
>  		pr_err("create pci bus failed\n");
> +		pci_unlock_rescan_remove();
>  		return -EINVAL;
>  	}
>
>  	pci_bus_add_devices(vpci_bus);
>
> +	pci_unlock_rescan_remove();
> +
>  	return 0;
>  }
>
> --
> 2.51.0
>

[PATCH v6 2/2] PCI: endpoint: pci-epf-vntb: Manage virtual NTB and PCI bus lifetime

[Koichiro Den]

The virtual PCI driver registers an ntb_dev but has no remove callback.
Unbinding the endpoint function can therefore free BARs while the NTB
device and its client still use them. It also leaves the virtual PCI
devices and root bus allocated.

Allocate an ntb_dev for each virtual PCI probe and unregister it from the
matching remove callback. Start command processing only after registration.
Publish the device for doorbell IRQs at the same point. During remove, stop
the command work and drain IRQ handlers before unregistering the device.

Retain the root bus returned by pci_scan_bus() so it can be removed on
unbind. Unregister the virtual PCI driver before releasing endpoint
resources. Stop and remove the root bus under the PCI rescan/remove lock,
then release its host bridge.

Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
Cc: stable@vger.kernel.org # 6.0+
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
Changes in v6:
  - Rework v5 patch 5 on v7.3-rc1.
  - Publish ntb_dev only after registration, and tie command work and
    doorbell delivery to its lifetime.
  - Retain and remove the virtual root bus, then release its host bridge.
  - Drop v5 patch 4; f7245901de89 ("PCI: Check parent for NULL in
    of_pci_bus_release_domain_nr()") fixed the bug it worked around.
    v5: https://lore.kernel.org/r/20260226084142.2226875-6-den@valinux.co.jp/

@Frank, the code changed substantially since v5, so I did not carry your
R-b tag. I would appreciate another look.

 drivers/pci/endpoint/functions/pci-epf-vntb.c | 85 +++++++++++++++----
 1 file changed, 70 insertions(+), 15 deletions(-)

diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
index 412e8cc6fb1d..992f5e7f8d4a 100644
--- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
+++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
@@ -132,7 +132,7 @@ struct epf_ntb_ctrl {
 } __packed;
 
 struct epf_ntb {
-	struct ntb_dev ntb;
+	struct ntb_dev *ntb;
 	struct pci_epf *epf;
 	struct config_group group;
 
@@ -166,10 +166,15 @@ struct epf_ntb {
 	void __iomem *vpci_mw_addr[MAX_MW];
 
 	struct delayed_work cmd_handler;
+	struct pci_bus *vpci_bus;
 };
 
 #define to_epf_ntb(epf_group) container_of((epf_group), struct epf_ntb, group)
-#define ntb_ndev(__ntb) container_of(__ntb, struct epf_ntb, ntb)
+
+static struct epf_ntb *ntb_ndev(struct ntb_dev *ntb)
+{
+	return ntb->pdev->sysdata;
+}
 
 static struct pci_epf_header epf_ntb_header = {
 	.vendorid	= PCI_ANY_ID,
@@ -195,7 +200,7 @@ static int epf_ntb_link_up(struct epf_ntb *ntb, bool link_up)
 	else
 		ntb->reg->link_status &= ~LINK_STATUS_UP;
 
-	ntb_link_event(&ntb->ntb);
+	ntb_link_event(ntb->ntb);
 	return 0;
 }
 
@@ -284,7 +289,7 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
 	     i++) {
 		if (ntb->epf_db[i]) {
 			atomic64_or(1 << (i - EPF_IRQ_DB_START), &ntb->db);
-			ntb_db_event(&ntb->ntb, i - EPF_IRQ_DB_START);
+			ntb_db_event(ntb->ntb, i - EPF_IRQ_DB_START);
 			ntb->epf_db[i] = 0;
 		}
 	}
@@ -348,12 +353,18 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
 static irqreturn_t epf_ntb_doorbell_handler(int irq, void *data)
 {
 	struct epf_ntb *ntb = data;
+	struct ntb_dev *ndev;
 	int i;
 
+	/* Pair with smp_store_release() in pci_vntb_probe(). */
+	ndev = smp_load_acquire(&ntb->ntb);
+	if (!ndev)
+		return IRQ_HANDLED;
+
 	for (i = EPF_IRQ_DB_START; i < ntb->db_count; i++)
 		if (irq == ntb->epf->db_msg[i].virq) {
 			atomic64_or(1 << (i - EPF_IRQ_DB_START), &ntb->db);
-			ntb_db_event(&ntb->ntb, i - EPF_IRQ_DB_START);
+			ntb_db_event(ndev, i - EPF_IRQ_DB_START);
 		}
 
 	return IRQ_HANDLED;
@@ -985,7 +996,6 @@ static int epf_ntb_epc_init(struct epf_ntb *ntb)
 	}
 
 	INIT_DELAYED_WORK(&ntb->cmd_handler, epf_ntb_cmd_handler);
-	queue_work(kpcintb_workqueue, &ntb->cmd_handler.work);
 
 	atomic64_set(&ntb->peer_db_pending, 0);
 	enable_work(&ntb->peer_db_work);
@@ -1340,6 +1350,7 @@ static int vpci_scan_bus(void *sysdata)
 		pci_unlock_rescan_remove();
 		return -EINVAL;
 	}
+	ndev->vpci_bus = vpci_bus;
 
 	pci_bus_add_devices(vpci_bus);
 
@@ -1425,7 +1436,7 @@ static int vntb_epf_mw_set_trans(struct ntb_dev *ndev, int pidx, int idx,
 	int ret;
 	struct device *dev;
 
-	dev = &ntb->ntb.dev;
+	dev = &ndev->dev;
 	barno = ntb->epf_ntb_bar[BAR_MW1 + idx];
 	epf_bar = &ntb->epf->bar[barno];
 	epf_bar->phys_addr = addr;
@@ -1563,7 +1574,7 @@ static void vntb_epf_peer_db_work(struct work_struct *work)
 			ret = pci_epc_raise_irq(epf->epc, func_no, vfunc_no,
 						PCI_IRQ_MSI, interrupt_num);
 			if (ret)
-				dev_err(&ntb->ntb.dev,
+				dev_err(&epf->dev,
 					"Failed to raise IRQ for interrupt_num %u: %d\n",
 					interrupt_num, ret);
 		}
@@ -1681,13 +1692,18 @@ static const struct ntb_dev_ops vntb_epf_ops = {
 
 static int pci_vntb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 {
-	int ret;
 	struct epf_ntb *ndev = (struct epf_ntb *)pdev->sysdata;
 	struct device *dev = &pdev->dev;
+	struct ntb_dev *ntb;
+	int ret;
+
+	ntb = devm_kzalloc(dev, sizeof(*ntb), GFP_KERNEL);
+	if (!ntb)
+		return -ENOMEM;
 
-	ndev->ntb.pdev = pdev;
-	ndev->ntb.topo = NTB_TOPO_NONE;
-	ndev->ntb.ops =  &vntb_epf_ops;
+	ntb->pdev = pdev;
+	ntb->topo = NTB_TOPO_NONE;
+	ntb->ops = &vntb_epf_ops;
 
 	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
 	if (ret) {
@@ -1695,16 +1711,41 @@ static int pci_vntb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 		return ret;
 	}
 
-	ret = ntb_register_device(&ndev->ntb);
+	ret = ntb_register_device(ntb);
 	if (ret) {
 		dev_err(dev, "Failed to register NTB device\n");
 		return ret;
 	}
 
+	/* Publish after ntb_register_device() succeeds. */
+	smp_store_release(&ndev->ntb, ntb);
+	queue_delayed_work(kpcintb_workqueue, &ndev->cmd_handler, 0);
+
 	dev_dbg(dev, "PCI Virtual NTB driver loaded\n");
 	return 0;
 }
 
+static void pci_vntb_remove(struct pci_dev *pdev)
+{
+	struct epf_ntb *ndev = pdev->sysdata;
+	struct ntb_dev *ntb;
+	unsigned int i;
+
+	/* Stop the work reader, then close and drain the IRQ gate. */
+	cancel_delayed_work_sync(&ndev->cmd_handler);
+
+	ntb = xchg(&ndev->ntb, NULL);
+
+	if (ndev->msi_doorbell)
+		for (i = 0; i < ndev->db_count; i++) {
+			if (epf_ntb_db_irq_is_duplicated(ndev->epf, i))
+				continue;
+			synchronize_irq(ndev->epf->db_msg[i].virq);
+		}
+
+	ntb_unregister_device(ntb);
+}
+
 static struct pci_device_id pci_vntb_table[] = {
 	{
 		PCI_DEVICE(0xffff, 0xffff),
@@ -1716,6 +1757,7 @@ static struct pci_driver vntb_pci_driver = {
 	.name           = "pci-vntb",
 	.id_table       = pci_vntb_table,
 	.probe          = pci_vntb_probe,
+	.remove         = pci_vntb_remove,
 };
 
 /* ============ PCIe EPF Driver Bind ====================*/
@@ -1796,12 +1838,25 @@ static int epf_ntb_bind(struct pci_epf *epf)
  */
 static void epf_ntb_unbind(struct pci_epf *epf)
 {
+	struct pci_host_bridge *bridge;
 	struct epf_ntb *ntb = epf_get_drvdata(epf);
 
+	pci_unregister_driver(&vntb_pci_driver);
+
+	if (ntb->vpci_bus) {
+		bridge = to_pci_host_bridge(ntb->vpci_bus->bridge);
+
+		pci_lock_rescan_remove();
+		pci_stop_root_bus(ntb->vpci_bus);
+		pci_remove_root_bus(ntb->vpci_bus);
+		ntb->vpci_bus = NULL;
+		pci_unlock_rescan_remove();
+
+		pci_free_host_bridge(bridge);
+	}
+
 	epf_ntb_epc_cleanup(ntb);
 	epf_ntb_config_spad_bar_free(ntb);
-
-	pci_unregister_driver(&vntb_pci_driver);
 }
 
 // EPF driver probe
-- 
2.51.0

Re: [PATCH v6 2/2] PCI: endpoint: pci-epf-vntb: Manage virtual NTB and PCI bus lifetime

[sashiko-bot]

> The virtual PCI driver registers an ntb_dev but has no remove callback.
> Unbinding the endpoint function can therefore free BARs while the NTB
> device and its client still use them. It also leaves the virtual PCI
> devices and root bus allocated.
> 
> Allocate an ntb_dev for each virtual PCI probe and unregister it from the
> matching remove callback. Start command processing only after registration.
> Publish the device for doorbell IRQs at the same point. During remove, stop
> the command work and drain IRQ handlers before unregistering the device.
> 
> Retain the root bus returned by pci_scan_bus() so it can be removed on
> unbind. Unregister the virtual PCI driver before releasing endpoint
> resources. Stop and remove the root bus under the PCI rescan/remove lock,
> then release its host bridge.
> 
> Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
> Cc: stable@vger.kernel.org # 6.0+
> Signed-off-by: Koichiro Den <den@valinux.co.jp>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901063238.631166-1-den@valinux.co.jp?part=2

Re: [PATCH v6 2/2] PCI: endpoint: pci-epf-vntb: Manage virtual NTB and PCI bus lifetime

[Frank Li]

On Tue, Sep 01, 2026 at 03:32:38PM +0900, Koichiro Den wrote:
> The virtual PCI driver registers an ntb_dev but has no remove callback.
> Unbinding the endpoint function can therefore free BARs while the NTB
> device and its client still use them. It also leaves the virtual PCI
> devices and root bus allocated.
>
> Allocate an ntb_dev for each virtual PCI probe and unregister it from the
> matching remove callback. Start command processing only after registration.
> Publish the device for doorbell IRQs at the same point. During remove, stop
> the command work and drain IRQ handlers before unregistering the device.
>
> Retain the root bus returned by pci_scan_bus() so it can be removed on
> unbind. Unregister the virtual PCI driver before releasing endpoint
> resources. Stop and remove the root bus under the PCI rescan/remove lock,
> then release its host bridge.
>
> Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP")
> Cc: stable@vger.kernel.org # 6.0+
> Signed-off-by: Koichiro Den <den@valinux.co.jp>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> Changes in v6:
>   - Rework v5 patch 5 on v7.3-rc1.
>   - Publish ntb_dev only after registration, and tie command work and
>     doorbell delivery to its lifetime.
>   - Retain and remove the virtual root bus, then release its host bridge.
>   - Drop v5 patch 4; f7245901de89 ("PCI: Check parent for NULL in
>     of_pci_bus_release_domain_nr()") fixed the bug it worked around.
>     v5: https://lore.kernel.org/r/20260226084142.2226875-6-den@valinux.co.jp/
>
> @Frank, the code changed substantially since v5, so I did not carry your
> R-b tag. I would appreciate another look.
>
>  drivers/pci/endpoint/functions/pci-epf-vntb.c | 85 +++++++++++++++----
>  1 file changed, 70 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> index 412e8cc6fb1d..992f5e7f8d4a 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c
> @@ -132,7 +132,7 @@ struct epf_ntb_ctrl {
>  } __packed;
>
>  struct epf_ntb {
> -	struct ntb_dev ntb;
> +	struct ntb_dev *ntb;
>  	struct pci_epf *epf;
>  	struct config_group group;
>
> @@ -166,10 +166,15 @@ struct epf_ntb {
>  	void __iomem *vpci_mw_addr[MAX_MW];
>
>  	struct delayed_work cmd_handler;
> +	struct pci_bus *vpci_bus;
>  };
>
>  #define to_epf_ntb(epf_group) container_of((epf_group), struct epf_ntb, group)
> -#define ntb_ndev(__ntb) container_of(__ntb, struct epf_ntb, ntb)
> +
> +static struct epf_ntb *ntb_ndev(struct ntb_dev *ntb)
> +{
> +	return ntb->pdev->sysdata;
> +}
>
>  static struct pci_epf_header epf_ntb_header = {
>  	.vendorid	= PCI_ANY_ID,
> @@ -195,7 +200,7 @@ static int epf_ntb_link_up(struct epf_ntb *ntb, bool link_up)
>  	else
>  		ntb->reg->link_status &= ~LINK_STATUS_UP;
>
> -	ntb_link_event(&ntb->ntb);
> +	ntb_link_event(ntb->ntb);
>  	return 0;
>  }
>
> @@ -284,7 +289,7 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
>  	     i++) {
>  		if (ntb->epf_db[i]) {
>  			atomic64_or(1 << (i - EPF_IRQ_DB_START), &ntb->db);
> -			ntb_db_event(&ntb->ntb, i - EPF_IRQ_DB_START);
> +			ntb_db_event(ntb->ntb, i - EPF_IRQ_DB_START);
>  			ntb->epf_db[i] = 0;
>  		}
>  	}
> @@ -348,12 +353,18 @@ static void epf_ntb_cmd_handler(struct work_struct *work)
>  static irqreturn_t epf_ntb_doorbell_handler(int irq, void *data)
>  {
>  	struct epf_ntb *ntb = data;
> +	struct ntb_dev *ndev;
>  	int i;
>
> +	/* Pair with smp_store_release() in pci_vntb_probe(). */
> +	ndev = smp_load_acquire(&ntb->ntb);
> +	if (!ndev)
> +		return IRQ_HANDLED;
> +
>  	for (i = EPF_IRQ_DB_START; i < ntb->db_count; i++)
>  		if (irq == ntb->epf->db_msg[i].virq) {
>  			atomic64_or(1 << (i - EPF_IRQ_DB_START), &ntb->db);
> -			ntb_db_event(&ntb->ntb, i - EPF_IRQ_DB_START);
> +			ntb_db_event(ndev, i - EPF_IRQ_DB_START);
>  		}
>
>  	return IRQ_HANDLED;
> @@ -985,7 +996,6 @@ static int epf_ntb_epc_init(struct epf_ntb *ntb)
>  	}
>
>  	INIT_DELAYED_WORK(&ntb->cmd_handler, epf_ntb_cmd_handler);
> -	queue_work(kpcintb_workqueue, &ntb->cmd_handler.work);
>
>  	atomic64_set(&ntb->peer_db_pending, 0);
>  	enable_work(&ntb->peer_db_work);
> @@ -1340,6 +1350,7 @@ static int vpci_scan_bus(void *sysdata)
>  		pci_unlock_rescan_remove();
>  		return -EINVAL;
>  	}
> +	ndev->vpci_bus = vpci_bus;
>
>  	pci_bus_add_devices(vpci_bus);
>
> @@ -1425,7 +1436,7 @@ static int vntb_epf_mw_set_trans(struct ntb_dev *ndev, int pidx, int idx,
>  	int ret;
>  	struct device *dev;
>
> -	dev = &ntb->ntb.dev;
> +	dev = &ndev->dev;
>  	barno = ntb->epf_ntb_bar[BAR_MW1 + idx];
>  	epf_bar = &ntb->epf->bar[barno];
>  	epf_bar->phys_addr = addr;
> @@ -1563,7 +1574,7 @@ static void vntb_epf_peer_db_work(struct work_struct *work)
>  			ret = pci_epc_raise_irq(epf->epc, func_no, vfunc_no,
>  						PCI_IRQ_MSI, interrupt_num);
>  			if (ret)
> -				dev_err(&ntb->ntb.dev,
> +				dev_err(&epf->dev,
>  					"Failed to raise IRQ for interrupt_num %u: %d\n",
>  					interrupt_num, ret);
>  		}
> @@ -1681,13 +1692,18 @@ static const struct ntb_dev_ops vntb_epf_ops = {
>
>  static int pci_vntb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  {
> -	int ret;
>  	struct epf_ntb *ndev = (struct epf_ntb *)pdev->sysdata;
>  	struct device *dev = &pdev->dev;
> +	struct ntb_dev *ntb;
> +	int ret;
> +
> +	ntb = devm_kzalloc(dev, sizeof(*ntb), GFP_KERNEL);
> +	if (!ntb)
> +		return -ENOMEM;
>
> -	ndev->ntb.pdev = pdev;
> -	ndev->ntb.topo = NTB_TOPO_NONE;
> -	ndev->ntb.ops =  &vntb_epf_ops;
> +	ntb->pdev = pdev;
> +	ntb->topo = NTB_TOPO_NONE;
> +	ntb->ops = &vntb_epf_ops;
>
>  	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32));
>  	if (ret) {
> @@ -1695,16 +1711,41 @@ static int pci_vntb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  		return ret;
>  	}
>
> -	ret = ntb_register_device(&ndev->ntb);
> +	ret = ntb_register_device(ntb);
>  	if (ret) {
>  		dev_err(dev, "Failed to register NTB device\n");
>  		return ret;
>  	}
>
> +	/* Publish after ntb_register_device() succeeds. */
> +	smp_store_release(&ndev->ntb, ntb);
> +	queue_delayed_work(kpcintb_workqueue, &ndev->cmd_handler, 0);
> +
>  	dev_dbg(dev, "PCI Virtual NTB driver loaded\n");
>  	return 0;
>  }
>
> +static void pci_vntb_remove(struct pci_dev *pdev)
> +{
> +	struct epf_ntb *ndev = pdev->sysdata;
> +	struct ntb_dev *ntb;
> +	unsigned int i;
> +
> +	/* Stop the work reader, then close and drain the IRQ gate. */
> +	cancel_delayed_work_sync(&ndev->cmd_handler);
> +
> +	ntb = xchg(&ndev->ntb, NULL);
> +
> +	if (ndev->msi_doorbell)
> +		for (i = 0; i < ndev->db_count; i++) {
> +			if (epf_ntb_db_irq_is_duplicated(ndev->epf, i))
> +				continue;
> +			synchronize_irq(ndev->epf->db_msg[i].virq);
> +		}
> +
> +	ntb_unregister_device(ntb);
> +}
> +
>  static struct pci_device_id pci_vntb_table[] = {
>  	{
>  		PCI_DEVICE(0xffff, 0xffff),
> @@ -1716,6 +1757,7 @@ static struct pci_driver vntb_pci_driver = {
>  	.name           = "pci-vntb",
>  	.id_table       = pci_vntb_table,
>  	.probe          = pci_vntb_probe,
> +	.remove         = pci_vntb_remove,
>  };
>
>  /* ============ PCIe EPF Driver Bind ====================*/
> @@ -1796,12 +1838,25 @@ static int epf_ntb_bind(struct pci_epf *epf)
>   */
>  static void epf_ntb_unbind(struct pci_epf *epf)
>  {
> +	struct pci_host_bridge *bridge;
>  	struct epf_ntb *ntb = epf_get_drvdata(epf);
>
> +	pci_unregister_driver(&vntb_pci_driver);
> +
> +	if (ntb->vpci_bus) {
> +		bridge = to_pci_host_bridge(ntb->vpci_bus->bridge);
> +
> +		pci_lock_rescan_remove();
> +		pci_stop_root_bus(ntb->vpci_bus);
> +		pci_remove_root_bus(ntb->vpci_bus);
> +		ntb->vpci_bus = NULL;
> +		pci_unlock_rescan_remove();
> +
> +		pci_free_host_bridge(bridge);
> +	}
> +
>  	epf_ntb_epc_cleanup(ntb);
>  	epf_ntb_config_spad_bar_free(ntb);
> -
> -	pci_unregister_driver(&vntb_pci_driver);
>  }
>
>  // EPF driver probe
> --
> 2.51.0
>