On 2015/02/03 14:05, Qiu, Michael wrote: > 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, I've got your point. Yes, you are right. I will fix it. Tetsuya > 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) >>