03/04/2018 08:27, Hemant Agrawal: > 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 <hemant.agra...@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? > Agreed. please check it in v3.
Yes, in v2. > >> +/** > >> + * 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? I think yes. It is an exported header of EAL. The @file is missing to make it visible in doxygen. > > 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. I don't see any other function using "_no". What about naming the function "rte_vfio_get_group_no" as "rte_vfio_get_group_num"?
