The vNTB endpoint function (pci-epf-vntb) can be configured and reconfigured through configfs (link/unlink functions, start/stop the controller, update parameters). In practice, several pitfalls present: double-unmapping when two windows share a BAR, wrong parameter order in .drop_link leading to wrong object lookups, duplicate EPC teardown that leads to oopses, a work item running after resources were torn down, and inability to re-link/restart fundamentally because ntb_dev was embedded and the vPCI bus teardown was incomplete.
This series addresses those issues and hardens resource management across NTB EPF and PCI EP core:
- Avoid double iounmap when PEER_SPAD and CONFIG share the same BAR. - Fix configfs .drop_link parameter order so the correct groups are used during unlink. - Remove duplicate EPC resource teardown in both pci-epf-vntb and pci-epf-ntb, avoiding crashes on .allow_link failures and during .drop_link. - Stop the delayed cmd_handler work before clearing BARs/doorbells. - Manage ntb_dev as a devm-managed allocation and implement .remove() in the vNTB PCI driver. Switch to pci_scan_root_bus().
With these changes, the controller can now be stopped, a function unlinked, configfs settings updated, and the controller re-linked and restarted without rebooting the endpoint, as long as the underlying pci_epc_ops .stop() is non-destructive and .start() restores normal operation.
Patches 1-5 carry Fixes tags and are candidates for stable. Patch 6 is a preparatory one for Patch 7. Patch 7 is a behavioral improvement that completes lifetime management for relink/restart scenarios.
Apologies for the delay between v2 and v3, and thank you for the review.
v2->v3 changes: - Added Reviewed-by tag for [PATCH v2 4/6] - Split [PATCH v2 6/6] into two, based on the feedback from Frank. (No code changes overall.) v1->v2 changes: - Incorporated feedback from Frank. - Added Reviewed-by tags (except for patches #4 and #6). - Fixed a typo in patch #5 title. (No code changes overall.)
v2: https://lore.kernel.org/all/20251029080321.807943-1-den@valinux.co.jp/ v1: https://lore.kernel.org/all/20251023071757.901181-1-den@valinux.co.jp/
Koichiro Den (7): NTB: epf: Avoid pci_iounmap() with offset when PEER_SPAD and CONFIG share BAR PCI: endpoint: Fix parameter order for .drop_link PCI: endpoint: pci-epf-vntb: Remove duplicate resource teardown PCI: endpoint: pci-epf-ntb: Remove duplicate resource teardown NTB: epf: vntb: Stop cmd_handler work in epf_ntb_epc_cleanup PCI: endpoint: pci-epf-vntb: Switch vpci_scan_bus() to use pci_scan_root_bus() PCI: endpoint: pci-epf-vntb: Manage ntb_dev lifetime and fix vpci bus teardown
drivers/ntb/hw/epf/ntb_hw_epf.c | 3 +- drivers/pci/endpoint/functions/pci-epf-ntb.c | 56 +----------- drivers/pci/endpoint/functions/pci-epf-vntb.c | 86 ++++++++++++------- drivers/pci/endpoint/pci-ep-cfs.c | 8 +- 4 files changed, 62 insertions(+), 91 deletions(-)
When BAR_PEER_SPAD and BAR_CONFIG share one PCI BAR, the module teardown path ends up calling pci_iounmap() on the same iomem with some offset, which is unnecessary and triggers a kernel warning like the following:
Trying to vunmap() nonexistent vm area (0000000069a5ffe8) WARNING: mm/vmalloc.c:3470 at vunmap+0x58/0x68, CPU#5: modprobe/2937 [...] Call trace: vunmap+0x58/0x68 (P) iounmap+0x34/0x48 pci_iounmap+0x2c/0x40 ntb_epf_pci_remove+0x44/0x80 [ntb_hw_epf] pci_device_remove+0x48/0xf8 device_remove+0x50/0x88 device_release_driver_internal+0x1c8/0x228 driver_detach+0x50/0xb0 bus_remove_driver+0x74/0x100 driver_unregister+0x34/0x68 pci_unregister_driver+0x34/0xa0 ntb_epf_pci_driver_exit+0x14/0xfe0 [ntb_hw_epf] [...]
Fix it by unmapping only when PEER_SPAD and CONFIG use difference bars.
Cc: stable@vger.kernel.org Fixes: e75d5ae8ab88 ("NTB: epf: Allow more flexibility in the memory BAR map method") Reviewed-by: Frank Li Frank.Li@nxp.com Signed-off-by: Koichiro Den den@valinux.co.jp --- drivers/ntb/hw/epf/ntb_hw_epf.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/ntb/hw/epf/ntb_hw_epf.c b/drivers/ntb/hw/epf/ntb_hw_epf.c index d3ecf25a5162..9935da48a52e 100644 --- a/drivers/ntb/hw/epf/ntb_hw_epf.c +++ b/drivers/ntb/hw/epf/ntb_hw_epf.c @@ -646,7 +646,8 @@ static void ntb_epf_deinit_pci(struct ntb_epf_dev *ndev) struct pci_dev *pdev = ndev->ntb.pdev;
pci_iounmap(pdev, ndev->ctrl_reg); - pci_iounmap(pdev, ndev->peer_spad_reg); + if (ndev->barno_map[BAR_PEER_SPAD] != ndev->barno_map[BAR_CONFIG]) + pci_iounmap(pdev, ndev->peer_spad_reg); pci_iounmap(pdev, ndev->db_reg);
pci_release_regions(pdev);
The unlink callbacks passed the parameters in the wrong order that led to looking up the wrong group objects. Swap the arguments so that the first parameter is the epf item and the second is the epc item.
Cc: stable@vger.kernel.org Fixes: e85a2d783762 ("PCI: endpoint: Add support in configfs to associate two EPCs with EPF") Reviewed-by: Frank Li Frank.Li@nxp.com Signed-off-by: Koichiro Den den@valinux.co.jp --- drivers/pci/endpoint/pci-ep-cfs.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/pci/endpoint/pci-ep-cfs.c b/drivers/pci/endpoint/pci-ep-cfs.c index ef50c82e647f..c7cf6c76d116 100644 --- a/drivers/pci/endpoint/pci-ep-cfs.c +++ b/drivers/pci/endpoint/pci-ep-cfs.c @@ -69,8 +69,8 @@ static int pci_secondary_epc_epf_link(struct config_item *epf_item, return 0; }
-static void pci_secondary_epc_epf_unlink(struct config_item *epc_item, - struct config_item *epf_item) +static void pci_secondary_epc_epf_unlink(struct config_item *epf_item, + struct config_item *epc_item) { struct pci_epf_group *epf_group = to_pci_epf_group(epf_item->ci_parent); struct pci_epc_group *epc_group = to_pci_epc_group(epc_item); @@ -133,8 +133,8 @@ static int pci_primary_epc_epf_link(struct config_item *epf_item, return 0; }
-static void pci_primary_epc_epf_unlink(struct config_item *epc_item, - struct config_item *epf_item) +static void pci_primary_epc_epf_unlink(struct config_item *epf_item, + struct config_item *epc_item) { struct pci_epf_group *epf_group = to_pci_epf_group(epf_item->ci_parent); struct pci_epc_group *epc_group = to_pci_epc_group(epc_item);
epf_ntb_epc_destroy() duplicates the teardown that the caller is supposed to perform later. This leads to an oops when .allow_link fails or when .drop_link is performed. The following is an example oops of the former case:
Unable to handle kernel paging request at virtual address dead000000000108 [...] [dead000000000108] address between user and kernel address ranges Internal error: Oops: 0000000096000044 [#1] SMP [...] Call trace: pci_epc_remove_epf+0x78/0xe0 (P) pci_primary_epc_epf_link+0x88/0xa8 configfs_symlink+0x1f4/0x5a0 vfs_symlink+0x134/0x1d8 do_symlinkat+0x88/0x138 __arm64_sys_symlinkat+0x74/0xe0 [...]
Remove the helper, and drop pci_epc_put(). EPC device refcounting is tied to the configfs EPC group lifetime, and pci_epc_put() in the .drop_link path is sufficient.
Cc: stable@vger.kernel.org Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP") Reviewed-by: Frank Li Frank.Li@nxp.com Signed-off-by: Koichiro Den den@valinux.co.jp --- drivers/pci/endpoint/functions/pci-epf-vntb.c | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c index 83e9ab10f9c4..49ce5d4b0ee5 100644 --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c @@ -644,19 +644,6 @@ static void epf_ntb_mw_bar_clear(struct epf_ntb *ntb, int num_mws) } }
-/** - * epf_ntb_epc_destroy() - Cleanup NTB EPC interface - * @ntb: NTB device that facilitates communication between HOST and VHOST - * - * Wrapper for epf_ntb_epc_destroy_interface() to cleanup all the NTB interfaces - */ -static void epf_ntb_epc_destroy(struct epf_ntb *ntb) -{ - pci_epc_remove_epf(ntb->epf->epc, ntb->epf, 0); - pci_epc_put(ntb->epf->epc); -} - - /** * epf_ntb_is_bar_used() - Check if a bar is used in the ntb configuration * @ntb: NTB device that facilitates communication between HOST and VHOST @@ -1406,7 +1393,7 @@ static int epf_ntb_bind(struct pci_epf *epf) ret = epf_ntb_init_epc_bar(ntb); if (ret) { dev_err(dev, "Failed to create NTB EPC\n"); - goto err_bar_init; + return ret; }
ret = epf_ntb_config_spad_bar_alloc(ntb); @@ -1446,9 +1433,6 @@ static int epf_ntb_bind(struct pci_epf *epf) err_bar_alloc: epf_ntb_config_spad_bar_free(ntb);
-err_bar_init: - epf_ntb_epc_destroy(ntb); - return ret; }
@@ -1464,7 +1448,6 @@ static void epf_ntb_unbind(struct pci_epf *epf)
epf_ntb_epc_cleanup(ntb); epf_ntb_config_spad_bar_free(ntb); - epf_ntb_epc_destroy(ntb);
pci_unregister_driver(&vntb_pci_driver); }
epf_ntb_epc_destroy() duplicates the teardown that the caller is supposed to do later. This leads to an oops when .allow_link fails or when .drop_link is performed. Remove the helper.
Also drop pci_epc_put(). EPC device refcounting is tied to configfs EPC group lifetime, and pci_epc_put() in the .drop_link path is sufficient.
Cc: stable@vger.kernel.org Fixes: 8b821cf76150 ("PCI: endpoint: Add EP function driver to provide NTB functionality") Reviewed-by: Frank Li Frank.Li@nxp.com Signed-off-by: Koichiro Den den@valinux.co.jp --- drivers/pci/endpoint/functions/pci-epf-ntb.c | 56 +------------------- 1 file changed, 2 insertions(+), 54 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-ntb.c b/drivers/pci/endpoint/functions/pci-epf-ntb.c index e01a98e74d21..7702ebb81d99 100644 --- a/drivers/pci/endpoint/functions/pci-epf-ntb.c +++ b/drivers/pci/endpoint/functions/pci-epf-ntb.c @@ -1494,47 +1494,6 @@ static int epf_ntb_db_mw_bar_init(struct epf_ntb *ntb, return ret; }
-/** - * epf_ntb_epc_destroy_interface() - Cleanup NTB EPC interface - * @ntb: NTB device that facilitates communication between HOST1 and HOST2 - * @type: PRIMARY interface or SECONDARY interface - * - * Unbind NTB function device from EPC and relinquish reference to pci_epc - * for each of the interface. - */ -static void epf_ntb_epc_destroy_interface(struct epf_ntb *ntb, - enum pci_epc_interface_type type) -{ - struct epf_ntb_epc *ntb_epc; - struct pci_epc *epc; - struct pci_epf *epf; - - if (type < 0) - return; - - epf = ntb->epf; - ntb_epc = ntb->epc[type]; - if (!ntb_epc) - return; - epc = ntb_epc->epc; - pci_epc_remove_epf(epc, epf, type); - pci_epc_put(epc); -} - -/** - * epf_ntb_epc_destroy() - Cleanup NTB EPC interface - * @ntb: NTB device that facilitates communication between HOST1 and HOST2 - * - * Wrapper for epf_ntb_epc_destroy_interface() to cleanup all the NTB interfaces - */ -static void epf_ntb_epc_destroy(struct epf_ntb *ntb) -{ - enum pci_epc_interface_type type; - - for (type = PRIMARY_INTERFACE; type <= SECONDARY_INTERFACE; type++) - epf_ntb_epc_destroy_interface(ntb, type); -} - /** * epf_ntb_epc_create_interface() - Create and initialize NTB EPC interface * @ntb: NTB device that facilitates communication between HOST1 and HOST2 @@ -1614,15 +1573,8 @@ static int epf_ntb_epc_create(struct epf_ntb *ntb)
ret = epf_ntb_epc_create_interface(ntb, epf->sec_epc, SECONDARY_INTERFACE); - if (ret) { + if (ret) dev_err(dev, "SECONDARY intf: Fail to create NTB EPC\n"); - goto err_epc_create; - } - - return 0; - -err_epc_create: - epf_ntb_epc_destroy_interface(ntb, PRIMARY_INTERFACE);
return ret; } @@ -1887,7 +1839,7 @@ static int epf_ntb_bind(struct pci_epf *epf) ret = epf_ntb_init_epc_bar(ntb); if (ret) { dev_err(dev, "Failed to create NTB EPC\n"); - goto err_bar_init; + return ret; }
ret = epf_ntb_config_spad_bar_alloc_interface(ntb); @@ -1909,9 +1861,6 @@ static int epf_ntb_bind(struct pci_epf *epf) err_bar_alloc: epf_ntb_config_spad_bar_free(ntb);
-err_bar_init: - epf_ntb_epc_destroy(ntb); - return ret; }
@@ -1927,7 +1876,6 @@ static void epf_ntb_unbind(struct pci_epf *epf)
epf_ntb_epc_cleanup(ntb); epf_ntb_config_spad_bar_free(ntb); - epf_ntb_epc_destroy(ntb); }
#define EPF_NTB_R(_name) \
Disable the delayed work before clearing BAR mappings and doorbells to avoid running the handler after resources have been torn down.
Unable to handle kernel paging request at virtual address ffff800083f46004 [...] Internal error: Oops: 0000000096000007 [#1] SMP [...] Call trace: epf_ntb_cmd_handler+0x54/0x200 [pci_epf_vntb] (P) process_one_work+0x154/0x3b0 worker_thread+0x2c8/0x400 kthread+0x148/0x210 ret_from_fork+0x10/0x20
Cc: stable@vger.kernel.org Fixes: e35f56bb0330 ("PCI: endpoint: Support NTB transfer between RC and EP") Reviewed-by: Frank Li Frank.Li@nxp.com Signed-off-by: Koichiro Den den@valinux.co.jp --- drivers/pci/endpoint/functions/pci-epf-vntb.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c index 49ce5d4b0ee5..750a246f79c9 100644 --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c @@ -823,6 +823,7 @@ static int epf_ntb_epc_init(struct epf_ntb *ntb) */ static void epf_ntb_epc_cleanup(struct epf_ntb *ntb) { + disable_delayed_work_sync(&ntb->cmd_handler); epf_ntb_mw_bar_clear(ntb, ntb->num_mws); epf_ntb_db_bar_clear(ntb); epf_ntb_config_sspad_bar_clear(ntb);
vpci_scan_bus() currently uses pci_scan_bus(), which creates a root bus without a parent struct device. In a subsequent change we want to tear down the virtual PCI root bus using pci_remove_root_bus(). For that to work correctly, the root bus must be associated with a parent device, similar to what the removed pci_scan_bus_parented() helper used to do.
Switch vpci_scan_bus() to use pci_scan_root_bus() and pass &ndev->epf->epc->dev as the parent. Build the resource list in the same way as pci_scan_bus(), so the behavior is unchanged except that the virtual root bus now has a proper parent device. This avoids crashes in the pci_epf_unbind() -> epf_ntb_unbind() -> pci_remove_root_bus() -> pci_bus_release_domain_nr() path once we start removing the root bus in a follow-up patch.
Signed-off-by: Koichiro Den den@valinux.co.jp --- drivers/pci/endpoint/functions/pci-epf-vntb.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c index 750a246f79c9..af0651c03b20 100644 --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c @@ -1098,7 +1098,19 @@ static int vpci_scan_bus(void *sysdata) struct pci_bus *vpci_bus; struct epf_ntb *ndev = sysdata;
- vpci_bus = pci_scan_bus(ndev->vbus_number, &vpci_ops, sysdata); + LIST_HEAD(resources); + static struct resource busn_res = { + .start = 0, + .end = 255, + .flags = IORESOURCE_BUS, + }; + + pci_add_resource(&resources, &ioport_resource); + pci_add_resource(&resources, &iomem_resource); + pci_add_resource(&resources, &busn_res); + + vpci_bus = pci_scan_root_bus(&ndev->epf->epc->dev, ndev->vbus_number, + &vpci_ops, sysdata, &resources); if (!vpci_bus) { pr_err("create pci bus failed\n"); return -EINVAL;
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [PATCH v3 6/7] PCI: endpoint: pci-epf-vntb: Switch vpci_scan_bus() to use pci_scan_root_bus() Link: https://lore.kernel.org/stable/20251130151100.2591822-7-den%40valinux.co.jp
Currently ntb_dev is embedded in epf_ntb, while configfs allows starting or stopping controller and linking or unlinking functions as you want. In fact, re-linking and re-starting is not possible with the embedded design and leads to oopses.
Allocate ntb_dev with devm and add a .remove callback to the pci driver that calls ntb_unregister_device(). This allows a fresh device to be created on the next .bind call.
With these changes, the controller can now be stopped, a function unlinked, configfs settings updated, and the controller re-linked and restarted without rebooting the endpoint, as long as the underlying pci_epc_ops .stop() operation is non-destructive, and .start() can restore normal operations.
Signed-off-by: Koichiro Den den@valinux.co.jp --- drivers/pci/endpoint/functions/pci-epf-vntb.c | 52 ++++++++++++++----- 1 file changed, 39 insertions(+), 13 deletions(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c index af0651c03b20..3059ed85a955 100644 --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c @@ -118,7 +118,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;
@@ -144,10 +144,16 @@ 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 inline struct epf_ntb *ntb_ndev(struct ntb_dev *ntb) +{ + return (struct epf_ntb *)ntb->pdev->sysdata; +}
static struct pci_epf_header epf_ntb_header = { .vendorid = PCI_ANY_ID, @@ -173,7 +179,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; }
@@ -261,7 +267,7 @@ static void epf_ntb_cmd_handler(struct work_struct *work) for (i = 1; i < ntb->db_count; i++) { if (ntb->epf_db[i]) { ntb->db |= 1 << (i - 1); - ntb_db_event(&ntb->ntb, i); + ntb_db_event(ntb->ntb, i); ntb->epf_db[i] = 0; } } @@ -1097,7 +1103,6 @@ static int vpci_scan_bus(void *sysdata) { struct pci_bus *vpci_bus; struct epf_ntb *ndev = sysdata; - LIST_HEAD(resources); static struct resource busn_res = { .start = 0, @@ -1115,6 +1120,7 @@ static int vpci_scan_bus(void *sysdata) pr_err("create pci bus failed\n"); return -EINVAL; } + ndev->vpci_bus = vpci_bus;
pci_bus_add_devices(vpci_bus);
@@ -1159,7 +1165,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 = &ntb->ntb->dev; barno = ntb->epf_ntb_bar[BAR_MW1 + idx]; epf_bar = &ntb->epf->bar[barno]; epf_bar->phys_addr = addr; @@ -1259,7 +1265,7 @@ static int vntb_epf_peer_db_set(struct ntb_dev *ndev, u64 db_bits) ret = pci_epc_raise_irq(ntb->epf->epc, func_no, vfunc_no, PCI_IRQ_MSI, interrupt_num + 1); if (ret) - dev_err(&ntb->ntb.dev, "Failed to raise IRQ\n"); + dev_err(&ntb->ntb->dev, "Failed to raise IRQ\n");
return ret; } @@ -1346,9 +1352,12 @@ static int pci_vntb_probe(struct pci_dev *pdev, const struct pci_device_id *id) struct epf_ntb *ndev = (struct epf_ntb *)pdev->sysdata; struct device *dev = &pdev->dev;
- ndev->ntb.pdev = pdev; - ndev->ntb.topo = NTB_TOPO_NONE; - ndev->ntb.ops = &vntb_epf_ops; + ndev->ntb = devm_kzalloc(dev, sizeof(*ndev->ntb), GFP_KERNEL); + if (!ndev->ntb) + return -ENOMEM; + ndev->ntb->pdev = pdev; + ndev->ntb->topo = NTB_TOPO_NONE; + ndev->ntb->ops = &vntb_epf_ops;
ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); if (ret) { @@ -1356,7 +1365,7 @@ 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(ndev->ntb); if (ret) { dev_err(dev, "Failed to register NTB device\n"); return ret; @@ -1366,6 +1375,17 @@ static int pci_vntb_probe(struct pci_dev *pdev, const struct pci_device_id *id) return 0; }
+static void pci_vntb_remove(struct pci_dev *pdev) +{ + struct epf_ntb *ndev = (struct epf_ntb *)pdev->sysdata; + + if (!ndev || !ndev->ntb) + return; + + ntb_unregister_device(ndev->ntb); + ndev->ntb = NULL; +} + static struct pci_device_id pci_vntb_table[] = { { PCI_DEVICE(0xffff, 0xffff), @@ -1377,6 +1397,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 ====================*/ @@ -1459,10 +1480,15 @@ static void epf_ntb_unbind(struct pci_epf *epf) { struct epf_ntb *ntb = epf_get_drvdata(epf);
+ pci_unregister_driver(&vntb_pci_driver); + + pci_lock_rescan_remove(); + pci_stop_root_bus(ntb->vpci_bus); + pci_remove_root_bus(ntb->vpci_bus); + pci_unlock_rescan_remove(); + epf_ntb_epc_cleanup(ntb); epf_ntb_config_spad_bar_free(ntb); - - pci_unregister_driver(&vntb_pci_driver); }
// EPF driver probe
On Mon, Dec 01, 2025 at 12:11:00AM +0900, Koichiro Den wrote:
Currently ntb_dev is embedded in epf_ntb, while configfs allows starting or stopping controller and linking or unlinking functions as you want. In fact, re-linking and re-starting is not possible with the embedded design and leads to oopses.
Allocate ntb_dev with devm and add a .remove callback to the pci driver that calls ntb_unregister_device(). This allows a fresh device to be created on the next .bind call.
With these changes, the controller can now be stopped, a function unlinked, configfs settings updated, and the controller re-linked and restarted without rebooting the endpoint, as long as the underlying pci_epc_ops .stop() operation is non-destructive, and .start() can restore normal operations.
Signed-off-by: Koichiro Den den@valinux.co.jp
drivers/pci/endpoint/functions/pci-epf-vntb.c | 52 ++++++++++++++----- 1 file changed, 39 insertions(+), 13 deletions(-)
...
@@ -1097,7 +1103,6 @@ static int vpci_scan_bus(void *sysdata) { struct pci_bus *vpci_bus; struct epf_ntb *ndev = sysdata;
Unneccsary change here.
Frank Li
LIST_HEAD(resources); static struct resource busn_res = { .start = 0, @@ -1115,6 +1120,7 @@ static int vpci_scan_bus(void *sysdata) pr_err("create pci bus failed\n"); return -EINVAL; }
ndev->vpci_bus = vpci_bus;
pci_bus_add_devices(vpci_bus);
@@ -1159,7 +1165,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 = &ntb->ntb->dev; barno = ntb->epf_ntb_bar[BAR_MW1 + idx]; epf_bar = &ntb->epf->bar[barno]; epf_bar->phys_addr = addr;
@@ -1259,7 +1265,7 @@ static int vntb_epf_peer_db_set(struct ntb_dev *ndev, u64 db_bits) ret = pci_epc_raise_irq(ntb->epf->epc, func_no, vfunc_no, PCI_IRQ_MSI, interrupt_num + 1); if (ret)
dev_err(&ntb->ntb.dev, "Failed to raise IRQ\n");
dev_err(&ntb->ntb->dev, "Failed to raise IRQ\n");return ret;
} @@ -1346,9 +1352,12 @@ static int pci_vntb_probe(struct pci_dev *pdev, const struct pci_device_id *id) struct epf_ntb *ndev = (struct epf_ntb *)pdev->sysdata; struct device *dev = &pdev->dev;
- ndev->ntb.pdev = pdev;
- ndev->ntb.topo = NTB_TOPO_NONE;
- ndev->ntb.ops = &vntb_epf_ops;
ndev->ntb = devm_kzalloc(dev, sizeof(*ndev->ntb), GFP_KERNEL);
if (!ndev->ntb)
return -ENOMEM;ndev->ntb->pdev = pdev;
ndev->ntb->topo = NTB_TOPO_NONE;
ndev->ntb->ops = &vntb_epf_ops;
ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(32)); if (ret) {
@@ -1356,7 +1365,7 @@ 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(ndev->ntb); if (ret) { dev_err(dev, "Failed to register NTB device\n"); return ret;
@@ -1366,6 +1375,17 @@ static int pci_vntb_probe(struct pci_dev *pdev, const struct pci_device_id *id) return 0; }
+static void pci_vntb_remove(struct pci_dev *pdev) +{
- struct epf_ntb *ndev = (struct epf_ntb *)pdev->sysdata;
- if (!ndev || !ndev->ntb)
return;- ntb_unregister_device(ndev->ntb);
- ndev->ntb = NULL;
+}
static struct pci_device_id pci_vntb_table[] = { { PCI_DEVICE(0xffff, 0xffff), @@ -1377,6 +1397,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 ====================*/ @@ -1459,10 +1480,15 @@ static void epf_ntb_unbind(struct pci_epf *epf) { struct epf_ntb *ntb = epf_get_drvdata(epf);
- pci_unregister_driver(&vntb_pci_driver);
- pci_lock_rescan_remove();
- pci_stop_root_bus(ntb->vpci_bus);
- pci_remove_root_bus(ntb->vpci_bus);
- pci_unlock_rescan_remove();
- epf_ntb_epc_cleanup(ntb); epf_ntb_config_spad_bar_free(ntb);
- pci_unregister_driver(&vntb_pci_driver);
}
// EPF driver probe
2.48.1
On Mon, Dec 01, 2025 at 12:10:59AM +0900, Koichiro Den wrote:
vpci_scan_bus() currently uses pci_scan_bus(), which creates a root bus without a parent struct device. In a subsequent change we want to tear down the virtual PCI root bus using pci_remove_root_bus(). For that to work correctly, the root bus must be associated with a parent device, similar to what the removed pci_scan_bus_parented() helper used to do.
Switch vpci_scan_bus() to use pci_scan_root_bus() and pass &ndev->epf->epc->dev as the parent. Build the resource list in the same way as pci_scan_bus(), so the behavior is unchanged except that the virtual root bus now has a proper parent device. This avoids crashes in the pci_epf_unbind() -> epf_ntb_unbind() -> pci_remove_root_bus() -> pci_bus_release_domain_nr() path once we start removing the root bus in a follow-up patch.
Signed-off-by: Koichiro Den den@valinux.co.jp
drivers/pci/endpoint/functions/pci-epf-vntb.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c index 750a246f79c9..af0651c03b20 100644 --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c @@ -1098,7 +1098,19 @@ static int vpci_scan_bus(void *sysdata) struct pci_bus *vpci_bus; struct epf_ntb *ndev = sysdata;
next patch's remove empty line should be in this patch.
Reviewed-by: Frank Li Frank.Li@nxp.com
Frank
- vpci_bus = pci_scan_bus(ndev->vbus_number, &vpci_ops, sysdata);
- LIST_HEAD(resources);
- static struct resource busn_res = {
.start = 0,.end = 255,.flags = IORESOURCE_BUS,- };
- pci_add_resource(&resources, &ioport_resource);
- pci_add_resource(&resources, &iomem_resource);
- pci_add_resource(&resources, &busn_res);
- vpci_bus = pci_scan_root_bus(&ndev->epf->epc->dev, ndev->vbus_number,
&vpci_ops, sysdata, &resources);-- 2.48.1
On Mon, Dec 01, 2025 at 01:51:49PM -0500, Frank Li wrote:
On Mon, Dec 01, 2025 at 12:10:59AM +0900, Koichiro Den wrote:
vpci_scan_bus() currently uses pci_scan_bus(), which creates a root bus without a parent struct device. In a subsequent change we want to tear down the virtual PCI root bus using pci_remove_root_bus(). For that to work correctly, the root bus must be associated with a parent device, similar to what the removed pci_scan_bus_parented() helper used to do.
Switch vpci_scan_bus() to use pci_scan_root_bus() and pass &ndev->epf->epc->dev as the parent. Build the resource list in the same way as pci_scan_bus(), so the behavior is unchanged except that the virtual root bus now has a proper parent device. This avoids crashes in the pci_epf_unbind() -> epf_ntb_unbind() -> pci_remove_root_bus() -> pci_bus_release_domain_nr() path once we start removing the root bus in a follow-up patch.
Signed-off-by: Koichiro Den den@valinux.co.jp
drivers/pci/endpoint/functions/pci-epf-vntb.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/endpoint/functions/pci-epf-vntb.c b/drivers/pci/endpoint/functions/pci-epf-vntb.c index 750a246f79c9..af0651c03b20 100644 --- a/drivers/pci/endpoint/functions/pci-epf-vntb.c +++ b/drivers/pci/endpoint/functions/pci-epf-vntb.c @@ -1098,7 +1098,19 @@ static int vpci_scan_bus(void *sysdata) struct pci_bus *vpci_bus; struct epf_ntb *ndev = sysdata;
next patch's remove empty line should be in this patch.
Sorry for the noise. Please let me send v4. Thank you for the careful review.
-Koichiro
Reviewed-by: Frank Li Frank.Li@nxp.com
Frank
- vpci_bus = pci_scan_bus(ndev->vbus_number, &vpci_ops, sysdata);
- LIST_HEAD(resources);
- static struct resource busn_res = {
.start = 0,.end = 255,.flags = IORESOURCE_BUS,- };
- pci_add_resource(&resources, &ioport_resource);
- pci_add_resource(&resources, &iomem_resource);
- pci_add_resource(&resources, &busn_res);
- vpci_bus = pci_scan_root_bus(&ndev->epf->epc->dev, ndev->vbus_number,
&vpci_ops, sysdata, &resources);-- 2.48.1
linux-stable-mirror@lists.linaro.org
-
Frank Li -
kernel test robot -
Koichiro Den