Re: [RFC v2 4/9] vfio: remove public wrappers

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 9/3/2025 5:17 PM, David Marchand wrote:
> The public header defines a number of wrappers that can be removed or
> hidden internally.
> 
> Either, those concern old Linux kernel versions that are not supported by
> dpdk anymore (DPDK now requires Linux v5.4 at least), like:
> - the request notifier feature, present since Linux v4.0,
> - the noiommu mode, present since Linux v4.5,
> - the capability support, present since Linux v4.6,
> - the msix mapping feature, present since Linux v4.16,
> 
> Or, those wrappers can be made private as only consumed internally.
> - VFIO_GET_REGION_IDX() has no equivalent in the Linux uapi, but
>    is only used by the Linux PCI bus code,
> - VFIO_DEVICE_FEATURE is only used by the CDX bus code,
> - the various macros around /dev/vfio/ or the noiommu kmod parameter are
>    only used by eal_vfio.c,
> 
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---

<snip>

>   	{
> -		.type_id = RTE_VFIO_NOIOMMU,
> +		.type_id = VFIO_NOIOMMU_IOMMU,
>   		.name = "No-IOMMU",
>   		.partial_unmap = true,
>   		.dma_map_func = &vfio_noiommu_dma_map,
> @@ -363,8 +363,7 @@ vfio_open_group_fd(int iommu_group_num)
>   	/* if primary, try to open the group */
>   	if (internal_conf->process_type == RTE_PROC_PRIMARY) {
>   		/* try regular group format */
> -		snprintf(filename, sizeof(filename),
> -				 VFIO_GROUP_FMT, iommu_group_num);
> +		snprintf(filename, sizeof(filename), "/dev/vfio/%u", iommu_group_num);
>   		vfio_group_fd = open(filename, O_RDWR);
>   		if (vfio_group_fd < 0) {
>   			/* if file not found, it's not an error */
> @@ -375,9 +374,8 @@ vfio_open_group_fd(int iommu_group_num)
>   			}
>   
>   			/* special case: try no-IOMMU path as well */
> -			snprintf(filename, sizeof(filename),
> -					VFIO_NOIOMMU_GROUP_FMT,
> -					iommu_group_num);
> +			snprintf(filename, sizeof(filename), "/dev/vfio/noiommu-%u",
> +				iommu_group_num);
>   			vfio_group_fd = open(filename, O_RDWR);
>   			if (vfio_group_fd < 0) {
>   				if (errno != ENOENT) {
> @@ -1128,7 +1126,7 @@ rte_vfio_enable(const char *modname)
>   	}
>   
>   	/* VFIO directory might not exist (e.g., unprivileged containers) */
> -	dir = opendir(VFIO_DIR);
> +	dir = opendir("/dev/vfio");

This and similar places: I would rather prefer if we used defines rather 
than repeating the same literals over and over in multiple places.

Other than that,

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly