[dpdk-dev] [PATCH v6 10/13] eal/pci: Cleanup pci driver initialization code

[Tetsuya Mukawa]

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)
>>