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()
>> The function is based on pci_probe_all_drivers. But it can not only
>> probe but also close drivers.
>> - Add pci_close_all_drivers()
>> The function tries to find a driver for the specified device, and
>> then close the driver.
>> - Add rte_eal_pci_probe_one() and rte_eal_pci_close_one()
>> The functions are used for probe and close a device.
>> First the function tries to find a device that has the specfied
>> PCI address. Then, probe or close the device.
>>
>> v5:
>> - Remove RTE_EAL_INVOKE_TYPE_UNKNOWN, because it's unused.
>> v4:
>> - Fix paramerter checking.
>> - Fix indent of 'if' statement.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa at igel.co.jp>
>> ---
>> lib/librte_eal/common/eal_common_pci.c | 90
>> +++++++++++++++++++++++++++++----
>> lib/librte_eal/common/eal_private.h | 24 +++++++++
>> lib/librte_eal/common/include/rte_pci.h | 33 ++++++++++++
>> lib/librte_eal/linuxapp/eal/eal_pci.c | 69 +++++++++++++++++++++++++
>> 4 files changed, 206 insertions(+), 10 deletions(-)
>>
>> diff --git a/lib/librte_eal/common/eal_common_pci.c
>> b/lib/librte_eal/common/eal_common_pci.c
>> index a89f5c3..7c9b8c5 100644
>> --- a/lib/librte_eal/common/eal_common_pci.c
>> +++ b/lib/librte_eal/common/eal_common_pci.c
>> @@ -99,19 +99,27 @@ static struct rte_devargs *pci_devargs_lookup(struct
>> rte_pci_device *dev)
>> return NULL;
>> }
>>
>> -/*
>> - * If vendor/device ID match, call the devinit() function of all
>> - * registered driver for the given device. Return -1 if initialization
>> - * failed, return 1 if no driver is found for this device.
>> - */
>> static int
>> -pci_probe_all_drivers(struct rte_pci_device *dev)
>> +pci_invoke_all_drivers(struct rte_pci_device *dev,
>> + enum rte_eal_invoke_type type)
>> {
>> struct rte_pci_driver *dr = NULL;
>> - int rc;
>> + int rc = 0;
>> +
>> + if ((dev == NULL) || (type >= RTE_EAL_INVOKE_TYPE_MAX))
>> + return -1;
>>
>> TAILQ_FOREACH(dr, &pci_driver_list, next) {
>> - rc = rte_eal_pci_probe_one_driver(dr, dev);
>> + switch (type) {
>> + case RTE_EAL_INVOKE_TYPE_PROBE:
>> + rc = rte_eal_pci_probe_one_driver(dr, dev);
>> + break;
>> + case RTE_EAL_INVOKE_TYPE_CLOSE:
>> + rc = rte_eal_pci_close_one_driver(dr, dev);
>> + break;
>> + default:
>> + return -1;
>> + }
>> if (rc < 0)
>> /* negative value is an error */
>> return -1;
>> @@ -123,6 +131,66 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
>> return 1;
>> }
>>
>> +#ifdef ENABLE_HOTPLUG
>> +static int
>> +rte_eal_pci_invoke_one(struct rte_pci_addr *addr,
>> + enum rte_eal_invoke_type type)
>> +{
>> + struct rte_pci_device *dev = NULL;
>> + int ret = 0;
>> +
>> + if ((addr == NULL) || (type >= RTE_EAL_INVOKE_TYPE_MAX))
>> + return -1;
>> +
>> + TAILQ_FOREACH(dev, &pci_device_list, next) {
>> + if (eal_compare_pci_addr(&dev->addr, addr))
>> + continue;
>> +
>> + ret = pci_invoke_all_drivers(dev, type);
>> + if (ret < 0)
>> + goto invoke_err_return;
>> +
>> + if (type == RTE_EAL_INVOKE_TYPE_CLOSE)
>> + goto remove_dev;
>> +
>> + return 0;
>> + }
>> +
>> + return -1;
>> +
>> +invoke_err_return:
>> + RTE_LOG(WARNING, EAL, "Requested device " PCI_PRI_FMT
>> + " cannot be used\n", dev->addr.domain, dev->addr.bus,
>> + dev->addr.devid, dev->addr.function);
>> + return -1;
>> +
>> +remove_dev:
>> + TAILQ_REMOVE(&pci_device_list, dev, next);
>> + return 0;
>> +}
>> +
>> +
>> +/*
>> + * Find the pci device specified by pci address, then invoke probe function
>> of
>> + * the driver of the devive.
>> + */
>> +int
>> +rte_eal_pci_probe_one(struct rte_pci_addr *addr)
>> +{
>> + return rte_eal_pci_invoke_one(addr, RTE_EAL_INVOKE_TYPE_PROBE);
>> +}
>> +
>> +/*
>> + * Find the pci device specified by pci address, then invoke close function
>> of
>> + * the driver of the devive.
>> + */
>> +int
>> +rte_eal_pci_close_one(struct rte_pci_addr *addr)
>> +{
>> + return rte_eal_pci_invoke_one(addr, RTE_EAL_INVOKE_TYPE_CLOSE);
>> +}
>> +#endif /* ENABLE_HOTPLUG */
>> +
>> /*
>> * Scan the content of the PCI bus, and call the devinit() function for
>> * all registered drivers that have a matching entry in its id_table
>> @@ -148,10 +216,12 @@ rte_eal_pci_probe(void)
>>
>> /* probe all or only whitelisted devices */
>> if (probe_all)
>> - ret = pci_probe_all_drivers(dev);
>> + ret = pci_invoke_all_drivers(dev,
>> + RTE_EAL_INVOKE_TYPE_PROBE);
>> else if (devargs != NULL &&
>> devargs->type == RTE_DEVTYPE_WHITELISTED_PCI)
>> - ret = pci_probe_all_drivers(dev);
>> + ret = pci_invoke_all_drivers(dev,
>> + RTE_EAL_INVOKE_TYPE_PROBE);
>> if (ret < 0)
>> rte_exit(EXIT_FAILURE, "Requested device " PCI_PRI_FMT
>> " cannot be used\n", dev->addr.domain,
>> dev->addr.bus,
>> diff --git a/lib/librte_eal/common/eal_private.h
>> b/lib/librte_eal/common/eal_private.h
>> index 159cd66..1a362ab 100644
>> --- a/lib/librte_eal/common/eal_private.h
>> +++ b/lib/librte_eal/common/eal_private.h
>> @@ -154,6 +154,15 @@ struct rte_pci_driver;
>> struct rte_pci_device;
>>
>> /**
>> + * The invoke type.
>> + */
>> +enum rte_eal_invoke_type {
>> + RTE_EAL_INVOKE_TYPE_PROBE, /**< invoke probe function */
>> + RTE_EAL_INVOKE_TYPE_CLOSE, /**< invoke close function */
>> + RTE_EAL_INVOKE_TYPE_MAX /**< max value of this enum */
>> +};
>> +
>> +/**
>> * Mmap memory for single PCI device
>> *
>> * This function is private to EAL.
>> @@ -165,6 +174,21 @@ int rte_eal_pci_probe_one_driver(struct rte_pci_driver
>> *dr,
>> struct rte_pci_device *dev);
>>
>> /**
>> + * Munmap memory for single PCI device
>> + *
>> + * This function is private to EAL.
>> + *
>> + * @param dr
>> + * The pointer to the pci driver structure
>> + * @param dev
>> + * The pointer to the pci device structure
>> + * @return
>> + * 0 on success, negative on error
>> + */
>> +int rte_eal_pci_close_one_driver(struct rte_pci_driver *dr,
>> + struct rte_pci_device *dev);
>> +
>> +/**
>> * Init tail queues for non-EAL library structures. This is to allow
>> * the rings, mempools, etc. lists to be shared among multiple processes
>> *
>> diff --git a/lib/librte_eal/common/include/rte_pci.h
>> b/lib/librte_eal/common/include/rte_pci.h
>> index 87ca4cf..a111066 100644
>> --- a/lib/librte_eal/common/include/rte_pci.h
>> +++ b/lib/librte_eal/common/include/rte_pci.h
>> @@ -82,6 +82,7 @@ extern "C" {
>> #include <inttypes.h>
>>
>> #include <rte_interrupts.h>
>> +#include <rte_dev_hotplug.h>
>>
>> TAILQ_HEAD(pci_device_list, rte_pci_device); /**< PCI devices in D-linked
>> Q. */
>> TAILQ_HEAD(pci_driver_list, rte_pci_driver); /**< PCI drivers in D-linked
>> Q. */
>> @@ -323,6 +324,38 @@ eal_compare_pci_addr(struct rte_pci_addr *addr, struct
>> rte_pci_addr *addr2)
>> */
>> int rte_eal_pci_probe(void);
>>
>> +#ifdef ENABLE_HOTPLUG
>> +/**
>> + * Probe the single PCI device.
>> + *
>> + * Scan the content of the PCI bus, and find the pci device specified by pci
>> + * address, then call the probe() function for registered driver that has a
>> + * matching entry in its id_table for discovered device.
>> + *
>> + * @param addr
>> + * The PCI Bus-Device-Function address to probe or close.
>> + * @return
>> + * - 0 on success.
>> + * - Negative on error.
>> + */
>> +int rte_eal_pci_probe_one(struct rte_pci_addr *addr);
>> +
>> +/**
>> + * Close the single PCI device.
>> + *
>> + * Scan the content of the PCI bus, and find the pci device specified by pci
>> + * address, then call the close() function for registered driver that has a
>> + * matching entry in its id_table for discovered device.
>> + *
>> + * @param addr
>> + * The PCI Bus-Device-Function address to probe or close.
>> + * @return
>> + * - 0 on success.
>> + * - Negative on error.
>> + */
>> +int rte_eal_pci_close_one(struct rte_pci_addr *addr);
>> +#endif /* ENABLE_HOTPLUG */
>> +
>> /**
>> * Dump the content of the PCI bus.
>> *
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c
>> b/lib/librte_eal/linuxapp/eal/eal_pci.c
>> index c3b7917..831422e 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
>> @@ -682,6 +682,75 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
>> struct rte_pci_device *d
>> return 1;
>> }
>>
>> +#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.
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)
