On 2/3/2015 12:07 PM, Tetsuya Mukawa wrote: > On 2015/02/03 11:35, Qiu, Michael wrote: >> On 2/1/2015 12:02 PM, Tetsuya Mukawa wrote: >>> - Add rte_eal_pci_close_one_dirver() >>> The function is used for closing the specified driver and device. >>> - Add pci_invoke_all_drivers()
[...] >>> >>> +#ifdef ENABLE_HOTPLUG >>> +/* >>> + * If vendor/device ID match, call the devuninit() function of the >>> + * driver. >>> + */ >>> +int >>> +rte_eal_pci_close_one_driver(struct rte_pci_driver *dr, >>> + struct rte_pci_device *dev) >>> +{ >>> + struct rte_pci_id *id_table; >>> + >>> + if ((dr == NULL) || (dev == NULL)) >>> + return -EINVAL; >>> + >>> + for (id_table = dr->id_table ; id_table->vendor_id != 0; id_table++) { >>> + >>> + /* check if device's identifiers match the driver's ones */ >>> + if (id_table->vendor_id != dev->id.vendor_id && >>> + id_table->vendor_id != PCI_ANY_ID) >>> + continue; >>> + if (id_table->device_id != dev->id.device_id && >>> + id_table->device_id != PCI_ANY_ID) >>> + continue; >>> + if (id_table->subsystem_vendor_id != >>> + dev->id.subsystem_vendor_id && >>> + id_table->subsystem_vendor_id != PCI_ANY_ID) >>> + continue; >>> + if (id_table->subsystem_device_id != >>> + dev->id.subsystem_device_id && >>> + id_table->subsystem_device_id != PCI_ANY_ID) >>> + continue; >>> + >>> + struct rte_pci_addr *loc = &dev->addr; >>> + >>> + RTE_LOG(DEBUG, EAL, >>> + "PCI device "PCI_PRI_FMT" on NUMA socket %i\n", >>> + loc->domain, loc->bus, loc->devid, >>> + loc->function, dev->numa_node); >>> + >>> + RTE_LOG(DEBUG, EAL, " remove driver: %x:%x %s\n", >>> + dev->id.vendor_id, dev->id.device_id, >>> + dr->name); >>> + >>> + /* call the driver devuninit() function */ >>> + if (dr->devuninit && (dr->devuninit(dr, dev) < 0)) >>> + return -1; /* negative value is an error */ >>> + >>> + /* clear driver structure */ >>> + dev->driver = NULL; >>> + >>> + if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) >>> + /* unmap resources for devices that use igb_uio */ >>> + pci_unmap_device(dev); >> Hi, Tetsuya >> >> I have one question, as the code shows, in pci_unmap_device(), will >> check pt_driver. >> >> But assume that, we are now try to detach a vfio device, after print out >> a error message of unsupported, the does this port workable? >> >> I think this port will unworkable, am I right? >> >> But actually, we should keep it workable. >> >> My suggestion is to add a check in rte_eth_dev_check_detachable() for >> pci_device port. > Hi Michael, > > I appreciate your comment. > In the function called "rte_eal_dev_detach_pdev()", > "rte_eth_dev_check_detachable()" has been already checked. What I mean is check the pt_driver for pci_dev in rte_eth_dev_check_detachable(), so that hotplug framework will not affect vfio devices, just as I reply in another mail. Current logic will affect vfio devices if try to detach( Not do the really test, just the logic shows), am I right? Thanks, Michael > But in the future, someone may want to reuse > "rte_eal_pci_close_one_driver()". > So I will add the checking like your suggestion. > > Thanks, > Tetsuya > >> Thanks >> Michael >> >>> + >>> + return 0; >>> + } >>> + /* return positive value if driver is not found */ >>> + return 1; >>> +} >>> +#else /* ENABLE_HOTPLUG */ >>> +int >>> +rte_eal_pci_close_one_driver(struct rte_pci_driver *dr __rte_unused, >>> + struct rte_pci_device *dev __rte_unused) >>> +{ >>> + RTE_LOG(ERR, EAL, "Hotplug support isn't enabled\n"); >>> + return -1; >>> +} >>> +#endif /* ENABLE_HOTPLUG */ >>> + >>> /* Init the PCI EAL subsystem */ >>> int >>> rte_eal_pci_init(void) > >