From mboxrd@z Thu Jan  1 00:00:00 1970
From: Thomas Monjalon <thomas@monjalon.net>
Subject: Re: [PATCH] eal/vfio: export internal vfio functions
Date: Tue, 27 Mar 2018 17:53:44 +0200
Message-ID: <1882630.IHjtNmfOsM@xps>
References: <1521014434-3399-1-git-send-email-hemant.agrawal@nxp.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7Bit
Cc: dev@dpdk.org, bruce.richardson@intel.com, anatoly.burakov@intel.comm,
 xiao.w.wang@intel.com, junjie.j.chen@intel.com
To: Hemant Agrawal <hemant.agrawal@nxp.com>
Return-path: <dev-bounces@dpdk.org>
Received: from out5-smtp.messagingengine.com (out5-smtp.messagingengine.com
 [66.111.4.29]) by dpdk.org (Postfix) with ESMTP id AA6A42C5E
 for <dev@dpdk.org>; Tue, 27 Mar 2018 17:54:00 +0200 (CEST)
In-Reply-To: <1521014434-3399-1-git-send-email-hemant.agrawal@nxp.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

14/03/2018 09:00, Hemant Agrawal:
> This patch moves some of the internal vfio functions from
> eal_vfio.h to rte_vfio.h for common uses with "rte_" prefix.
> 
> This patch also change the FSLMC bus usages from the internal
> VFIO functions to external ones with "rte_" prefix
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
> --- a/lib/librte_eal/common/include/rte_vfio.h
> +++ b/lib/librte_eal/common/include/rte_vfio.h
> @@ -28,6 +28,12 @@
> +#if LINUX_VERSION_CODE < KERNEL_VERSION(4, 5, 0)
> +#define RTE_VFIO_NOIOMMU 8
> +#else
> +#define RTE_VFIO_NOIOMMU VFIO_NOIOMMU_IOMMU
> +#endif

I know this is just a move of an existing code,
but do you know why this check is against a version number (4.5.0),
instead of #ifdef VFIO_NOIOMMU_IOMMU which would be backport-safe?

> +/**
> + * Parse IOMMU group number for a device
> + *
> + * This function is only relevant to linux and will return
> + * an error on BSD.
> + *
> + * @return
> + *   1 on success
> + *   0 for non-existent group
> + *  <0 for errors
> + */
> +int __rte_experimental
> +rte_vfio_get_group_no(const char *sysfs_base,
> +		const char *dev_addr, int *iommu_group_no);
> +
> +/**
> + * Open VFIO container fd or get an existing one
> + *
> + * This function is only relevant to linux and will return
> + * an error on BSD.
> + *
> + * @return
> + *  > 0 container fd
> + *  < 0 for errors
> + */
> +int __rte_experimental
> +rte_vfio_get_container_fd(void);
> +
> +/**
> + * Open VFIO group fd or get an existing one
> + *
> + * This function is only relevant to linux and will return
> + * an error on BSD.
> + *
> + * @return
> + *  > 0 group fd
> + *  < 0 for errors
> + */
> +int __rte_experimental
> +rte_vfio_get_group_fd(int iommu_group_no);

All these new functions should have some @param documentation.

This file is not included in doxygen, probably because @file is missing.

About the naming, are you sure about "group_no" instead of "group_num"?

> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -254,5 +254,8 @@ EXPERIMENTAL {
>  	rte_service_set_runstate_mapped_check;
>  	rte_service_set_stats_enable;
>  	rte_service_start_with_defaults;
> +        rte_vfio_get_group_no;
> +        rte_vfio_get_container_fd;
> +        rte_vfio_get_group_fd;

Please indent with tabs.