From mboxrd@z Thu Jan 1 00:00:00 1970 From: Hemant Agrawal Subject: Re: [PATCH] eal/vfio: export internal vfio functions Date: Tue, 3 Apr 2018 11:57:40 +0530 Message-ID: <61905017-5c93-87f7-032d-e0b1907f56c0@nxp.com> References: <1521014434-3399-1-git-send-email-hemant.agrawal@nxp.com> <1882630.IHjtNmfOsM@xps> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org, bruce.richardson@intel.com, anatoly.burakov@intel.comm, xiao.w.wang@intel.com, junjie.j.chen@intel.com To: Thomas Monjalon , Hemant Agrawal Return-path: Received: from EUR01-HE1-obe.outbound.protection.outlook.com (mail-he1eur01on0041.outbound.protection.outlook.com [104.47.0.41]) by dpdk.org (Postfix) with ESMTP id C1FBB1B659 for ; Tue, 3 Apr 2018 08:27:55 +0200 (CEST) In-Reply-To: <1882630.IHjtNmfOsM@xps> Content-Language: en-US List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Thomas, On 3/27/2018 9:23 PM, Thomas Monjalon wrote: > 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 >> --- >> --- 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? Agreed. please check it in v3. >> +/** >> + * 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. added the @param > This file is not included in doxygen, probably because @file is missing. most of these functions are internal functions. do you think we should add it in doxygen as well? > > About the naming, are you sure about "group_no" instead of "group_num"? Agree, but this is already in many places.  I feel this change will be unnecessary. > >> --- 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. > > done in v3