[dpdk-dev] [PATCH v2 09/16] Enable VFIO device binding

[Thomas Monjalon]

2014-05-19 16:51, Anatoly Burakov:
> Add support for binding VFIO devices if RTE_PCI_DRV_NEED_IGB_UIO
> is set for this driver. Try VFIO first, if not mapped then try
> IGB_UIO too.

You have renamed RTE_PCI_DRV_NEED_IGB_UIO. Please update this log :)

> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -401,6 +401,7 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
> struct rte_pci_device *d {
>  	struct rte_pci_id *id_table;
>  	int ret = 0;
> +	int mapped = 0;
> 
>  	for (id_table = dr->id_table ; id_table->vendor_id != 0; id_table++) {
> 
> @@ -435,8 +436,17 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
> struct rte_pci_device *d }
> 
>  		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
> +			/* try mapping the NIC resources using VFIO if it exists */
> +#ifdef VFIO_PRESENT
> +			if (vfio_cfg.vfio_enabled) {
> +				if ((ret = pci_vfio_map_resource(dev)) == 0)
> +					mapped = 1;
> +				else if (ret < 0)
> +					return ret;
> +			}
> +#endif
>  			/* map resources for devices that use igb_uio */
> -			if ((ret = pci_uio_map_resource(dev)) != 0)
> +			if (!mapped && (ret = pci_uio_map_resource(dev)) != 0)
>  				return ret;

I think creating a function pci_map_resource() could be cleaner (you won't 
need variable mapped).

> +#ifdef VFIO_PRESENT
> +	memset(&vfio_cfg, 0, sizeof(vfio_cfg));
> +
> +	/* initialize group list */
> +	int i, ret;
> +
> +	for (i = 0; i < VFIO_MAX_GROUPS; i++) {
> +		vfio_cfg.vfio_groups[i].fd = -1;
> +		vfio_cfg.vfio_groups[i].group_no = -1;
> +	}
> +	vfio_cfg.vfio_container_fd = -1;
> +
> +	/* check if we have VFIO driver enabled */
> +	if (access(VFIO_DIR, F_OK) == 0) {
> +		static int socket_fd;
> +
> +		vfio_cfg.vfio_enabled = 1;
> +
> +		/* if we are primary process, create a thread to communicate with
> +		 * secondary processes. the thread will use a socket to wait for
> +		 * requests from secondary process to send open file descriptors,
> +		 * because VFIO does not allow multiple open descriptors on a group 
or
> +		 * VFIO container.
> +		 */
> +		if (internal_config.process_type == RTE_PROC_PRIMARY) {
> +			/* set up local socket */
> +			if ((socket_fd = pci_vfio_socket_setup()) < 0) {
> +				RTE_LOG(ERR, EAL, "Failed to set up local socket!\n");
> +				return -1;
> +			}
> +			ret = pthread_create(&socket_thread, NULL,
> +					pci_vfio_socket_thread, (void*) &socket_fd);
> +			if (ret) {
> +				RTE_LOG(ERR, EAL,
> +					"Failed to create thread for communication with secondary 
"
> +					"processes!\n");
> +				return -1;
> +			}
> +		}

Also here, it could help to have a dedicated function for vfio init.

> +	}
> +	else

checkpatch.pl reports an error: "else should follow close brace '}'"

> +		RTE_LOG(INFO, EAL, "VFIO driver not loaded or wrong permissions\n");
> +#endif

Thanks
-- 
Thomas