Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] vdpa: octeon_ep: MODULE_DEVICE_TABLE() conflicts with octeon_ep and Kconfig --
commit 2202a6588d5755d6dbbded2b962a2580a97810a5 Author: Pengpeng Hou <[email protected]> vdpa: octeon_ep: add missing MODULE_DEVICE_TABLE() This commit adds a MODULE_DEVICE_TABLE(pci, octep_pci_vdpa_map) entry to drivers/vdpa/octeon_ep/octep_vdpa_main.c to generate PCI module aliases for automatic module loading. > diff --git a/drivers/vdpa/octeon_ep/octep_vdpa_main.c > b/drivers/vdpa/octeon_ep/octep_vdpa_main.c > index 9e8d07078606f..3c7245ff32507 100644 > --- a/drivers/vdpa/octeon_ep/octep_vdpa_main.c > +++ b/drivers/vdpa/octeon_ep/octep_vdpa_main.c > @@ -883,6 +883,7 @@ static struct pci_device_id octep_pci_vdpa_map[] = { > { PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_VDPA_DEVID_CN103K_VF) }, > { 0 }, > }; > +MODULE_DEVICE_TABLE(pci, octep_pci_vdpa_map); [Severity: High] Was MODULE_DEVICE_TABLE(pci, octep_pci_vdpa_map) intentionally omitted here? In drivers/vdpa/Kconfig, config OCTEONEP_VDPA specifies: config OCTEONEP_VDPA tristate "vDPA driver for Octeon DPU devices" depends on m depends on PCI_MSI help This is a vDPA driver designed for Marvell's Octeon DPU devices. This driver enables the offloading of the VirtIO dataplane to an Octeon DPU device. Please note that this driver must be built as a module and it cannot be loaded until the Octeon emulation software is running. Exporting octep_pci_vdpa_map via MODULE_DEVICE_TABLE() causes udev/kmod to autoload octep_vdpa during boot or PCI enumeration before the Octeon emulation software is running. If OCTEP_EPF_RINFO(0) is 0 because the emulation software has not initialized the rings yet, octep_vdpa_probe() -> octep_vdpa_probe_pf() -> octep_vdpa_pf_setup() fails with -EINVAL: drivers/vdpa/octeon_ep/octep_vdpa_main.c:octep_vdpa_pf_setup() { ... addr = octpf->base[OCTEP_HW_MBOX_BAR]; val = readq(addr + OCTEP_EPF_RINFO(0)); if (val == 0) { dev_err(&pdev->dev, "Invalid device configuration\n"); return -EINVAL; } ... } In addition, all six PCI device IDs in octep_pci_vdpa_map (0x177d:0xb900, 0xb903, 0xba00, 0xba03, 0xbd00, and 0xbd03 with PCI_ANY_ID for subvendor, subdevice, and class) are identical to the PCI device IDs already exported via MODULE_DEVICE_TABLE() by the in-tree Ethernet PF and VF drivers: drivers/net/ethernet/marvell/octeon_ep/octep_main.c: static const struct pci_device_id octep_pci_id_tbl[] = { ... {PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CN10KA_PF)}, {PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CNF10KA_PF)}, {PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CNF10KB_PF)}, {PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CN10KB_PF)}, {0, }, }; MODULE_DEVICE_TABLE(pci, octep_pci_id_tbl); drivers/net/ethernet/marvell/octeon_ep_vf/octep_vf_main.c: static const struct pci_device_id octep_vf_pci_id_tbl[] = { ... {PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CN10KA_VF)}, {PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CNF10KA_VF)}, {PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CNF10KB_VF)}, {PCI_DEVICE(PCI_VENDOR_ID_CAVIUM, OCTEP_PCI_DEVICE_ID_CN10KB_VF)}, {0, }, }; MODULE_DEVICE_TABLE(pci, octep_vf_pci_id_tbl); Neither octep_vdpa_probe_pf() nor octep_vdpa_probe_vf() checks whether the device is running vDPA firmware rather than standard NIC firmware before claiming the PCI device. In octep_vdpa_pf_setup(), OCTEP_EPF_RINFO(0) is also non-zero in standard NIC mode, causing it to shrink BAR 4 via octep_vdpa_pf_bar_shrink() and succeed, while octep_vdpa_probe_vf() queues octep_vdpa_setup_task and returns 0 unconditionally: drivers/vdpa/octeon_ep/octep_vdpa_main.c:octep_vdpa_probe_vf() { ... INIT_WORK(&mgmt_dev->setup_task, octep_vdpa_setup_task); schedule_work(&mgmt_dev->setup_task); dev_info(&pdev->dev, "octep vdpa mgmt device setup task is queued\n"); return 0; } Could publishing MODULE_DEVICE_TABLE(pci, octep_pci_vdpa_map) cause octep_vdpa to autoload prematurely and race with octeon_ep and octeon_ep_vf to claim standard NIC PF and VF devices? > > static struct pci_driver octep_pci_vdpa = { > .name = OCTEP_VDPA_DRIVER_NAME, -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
