Re: [dpdk-dev] [PATCH v2 11/15] eal: Consolidate rte_eal_pci_probe/close_one_driver() of linuxapp and bsdapp

["Iremonger, Bernard" <bernard.iremonger@intel.com>]

> -----Original Message-----
> From: Tetsuya Mukawa [mailto:mukawa@igel.co.jp]
> Sent: Thursday, March 12, 2015 10:18 AM
> To: dev@dpdk.org
> Cc: Iremonger, Bernard; Richardson, Bruce; Tetsuya Mukawa
> Subject: [PATCH v2 11/15] eal: Consolidate rte_eal_pci_probe/close_one_driver() of linuxapp and
> bsdapp
> 
> This patch consolidates below functions, and implements these in common eal code.
>  - rte_eal_pci_probe_one_driver()
>  - rte_eal_pci_close_one_driver()
> 
> Because pci_map_device() is only implemented in linuxapp, the patch implements it in bsdapp too.
> This implemented function will be merged to linuxapp one with later patch.
> 
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  lib/librte_eal/bsdapp/eal/eal_pci.c    |  79 ++---------------
>  lib/librte_eal/common/eal_common_pci.c | 138 +++++++++++++++++++++++++++++
>  lib/librte_eal/common/eal_private.h    |  21 ++---
>  lib/librte_eal/linuxapp/eal/eal_pci.c  | 153 ++-------------------------------
>  4 files changed, 162 insertions(+), 229 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
> index 67ad88f..3778763 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
> @@ -88,7 +88,7 @@ static struct rte_tailq_elem rte_uio_tailq = {
>  EAL_REGISTER_TAILQ(rte_uio_tailq)
> 
>  /* unbind kernel driver for this device */ -static int
> +int
>  pci_unbind_kernel_driver(struct rte_pci_device *dev __rte_unused)  {
>  	RTE_LOG(ERR, EAL, "RTE_PCI_DRV_FORCE_UNBIND flag is not implemented "
> @@ -429,6 +429,13 @@ skipdev:
>  	return 0;
>  }
> 
> +/* Map pci device */
> +int
> +pci_map_device(struct rte_pci_device *dev) {
> +	return pci_uio_map_resource(dev);
> +}
> +
>  /*
>   * Scan the content of the PCI bus, and add the devices in the devices
>   * list. Call pci_scan_one() for each pci entry found.
> @@ -479,76 +486,6 @@ error:
>  	return -1;
>  }
> 
> -/*
> - * If vendor/device ID match, call the devinit() function of the
> - * driver.
> - */
> -int
> -rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev) -{
> -	struct rte_pci_id *id_table;
> -	int ret;
> -
> -	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,
> -			"  probe driver: %x:%x %s\n", dev->id.vendor_id,
> -			dev->id.device_id, dr->name);
> -
> -		/* no initialization when blacklisted, return without error */
> -		if (dev->devargs != NULL &&
> -			dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
> -
> -			RTE_LOG(DEBUG, EAL,
> -				"  Device is blacklisted, not initializing\n");
> -			return 0;
> -		}
> -
> -		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
> -			/* map resources for devices that use igb_uio */
> -			ret = pci_uio_map_resource(dev);
> -			if (ret != 0)
> -				return ret;
> -		} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
> -		           rte_eal_process_type() == RTE_PROC_PRIMARY) {
> -			/* unbind current driver */
> -			if (pci_unbind_kernel_driver(dev) < 0)
> -				return -1;
> -		}
> -
> -		/* reference driver structure */
> -		dev->driver = dr;
> -
> -		/* call the driver devinit() function */
> -		return dr->devinit(dr, dev);
> -	}
> -	/* return positive value if driver is not found */
> -	return 1;
> -}
> -
>  /* Init the PCI EAL subsystem */
>  int
>  rte_eal_pci_init(void)
> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
> index 808b87b..538b2d9 100644
> --- a/lib/librte_eal/common/eal_common_pci.c
> +++ b/lib/librte_eal/common/eal_common_pci.c
> @@ -99,6 +99,144 @@ static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev)  }
> 
>  /*
> + * If vendor/device ID match, call the devinit() function of the
> + * driver.
> + */
> +static int
> +rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
> +		struct rte_pci_device *dev)
> +{
> +	int ret;
> +	struct rte_pci_id *id_table;
> +
> +	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,
> +			"  probe driver: %x:%x %s\n", dev->id.vendor_id,
> +			dev->id.device_id, dr->name);
> +
> +		/* no initialization when blacklisted, return without error */
> +		if (dev->devargs != NULL &&
> +			dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
> +			RTE_LOG(DEBUG, EAL,
> +				"  Device is blacklisted, not initializing\n");
> +			return 1;
> +		}
> +
> +		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) { #ifdef RTE_PCI_CONFIG
> +			/*
> +			 * Set PCIe config space for high performance.
> +			 * Return value can be ignored.
> +			 */
> +			pci_config_space_set(dev);
> +#endif
> +			/* map resources for devices that use igb_uio */
> +			ret = pci_map_device(dev);
> +			if (ret != 0)
> +				return ret;
> +		} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
> +				rte_eal_process_type() == RTE_PROC_PRIMARY) {
> +			/* unbind current driver */
> +			if (pci_unbind_kernel_driver(dev) < 0)
> +				return -1;
> +		}
> +
> +		/* reference driver structure */
> +		dev->driver = dr;
> +
> +		/* call the driver devinit() function */
> +		return dr->devinit(dr, dev);
> +	}
> +	/* return positive value if driver is not found */
> +	return 1;
> +}
> +
> +#ifdef RTE_LIBRTE_EAL_HOTPLUG

Hi Tetsuya,

Can  #ifdef RTE_LIBRTE_EAL_HOTPLUG be removed now as file is common ?

Regards,

Bernard.


> +/*
> + * If vendor/device ID match, call the devuninit() function of the
> + * driver.
> + */
> +static 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(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);
> +
> +		return 0;
> +	}
> +	/* return positive value if driver is not found */
> +	return 1;
> +}
> +#endif /* RTE_LIBRTE_EAL_HOTPLUG */

Hi Tetsuya,

Can the #endif be removed too?

Regards,

Bernard.

 

> +
> +/*
>   * 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.
> diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
> index 4acf5a0..fe2c596 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -154,30 +154,31 @@ struct rte_pci_driver;  struct rte_pci_device;
> 
>  /**
> - * Mmap memory for single PCI device
> + * Unbind kernel driver for this device
>   *
>   * This function is private to EAL.
>   *
>   * @return
>   *   0 on success, negative on error
>   */
> -int rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
> -		struct rte_pci_device *dev);
> +int pci_unbind_kernel_driver(struct rte_pci_device *dev);
> 
>  /**
> - * Munmap memory for single PCI device
> + * Map this 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);
> +int pci_map_device(struct rte_pci_device *dev);
> +
> +/**
> + * Unmap this device
> + *
> + * This function is private to EAL.
> + */
> +void pci_unmap_device(struct rte_pci_device *dev);
> 
>  /**
>   * Init tail queues for non-EAL library structures. This is to allow diff --git
> a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
> index 353b0b8..dbc2548 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -57,7 +57,7 @@
>   */
> 
>  /* unbind kernel driver for this device */ -static int
> +int
>  pci_unbind_kernel_driver(struct rte_pci_device *dev)  {
>  	int n;
> @@ -565,7 +565,8 @@ pci_config_space_set(struct rte_pci_device *dev)  }  #endif
> 
> -static int
> +/* Map pci device */
> +int
>  pci_map_device(struct rte_pci_device *dev)  {
>  	int ret = -1;
> @@ -594,7 +595,8 @@ pci_map_device(struct rte_pci_device *dev)  }
> 
>  #ifdef RTE_LIBRTE_EAL_HOTPLUG
> -static void
> +/* Unmap pci device */
> +void
>  pci_unmap_device(struct rte_pci_device *dev)  {
>  	if (dev == NULL)
> @@ -618,151 +620,6 @@ pci_unmap_device(struct rte_pci_device *dev)  }  #endif /*
> RTE_LIBRTE_EAL_HOTPLUG */
> 
> -/*
> - * If vendor/device ID match, call the devinit() function of the
> - * driver.
> - */
> -int
> -rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev) -{
> -	int ret;
> -	struct rte_pci_id *id_table;
> -
> -	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,
> -			"  probe driver: %x:%x %s\n", dev->id.vendor_id,
> -			dev->id.device_id, dr->name);
> -
> -		/* no initialization when blacklisted, return without error */
> -		if (dev->devargs != NULL &&
> -			dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
> -			RTE_LOG(DEBUG, EAL,
> -				"  Device is blacklisted, not initializing\n");
> -			return 1;
> -		}
> -
> -		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
> -#ifdef RTE_PCI_CONFIG
> -			/*
> -			 * Set PCIe config space for high performance.
> -			 * Return value can be ignored.
> -			 */
> -			pci_config_space_set(dev);
> -#endif
> -			/* map resources for devices that use igb_uio */
> -			ret = pci_map_device(dev);
> -			if (ret != 0)
> -				return ret;
> -		} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
> -		           rte_eal_process_type() == RTE_PROC_PRIMARY) {
> -			/* unbind current driver */
> -			if (pci_unbind_kernel_driver(dev) < 0)
> -				return -1;
> -		}
> -
> -		/* reference driver structure */
> -		dev->driver = dr;
> -
> -		/* call the driver devinit() function */
> -		return dr->devinit(dr, dev);
> -	}
> -	/* return positive value if driver is not found */
> -	return 1;
> -}
> -
> -#ifdef RTE_LIBRTE_EAL_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(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);
> -
> -		return 0;
> -	}
> -	/* return positive value if driver is not found */
> -	return 1;
> -}
> -#else /* RTE_LIBRTE_EAL_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 /* RTE_LIBRTE_EAL_HOTPLUG */
> -
>  /* Init the PCI EAL subsystem */
>  int
>  rte_eal_pci_init(void)
> --
> 1.9.1