Hello Anatoly,
On Tue, 9 Sept 2025 at 17:38, Burakov, Anatoly
<anatoly.bura...@intel.com> wrote:
>
> 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.march...@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.
VFIO_DIR is a bad example, since it is only used here.
Code using such macros never changes.
>
> Other than that,
>
> Acked-by: Anatoly Burakov <anatoly.bura...@intel.com>
But ok, we can keep those macros.
They'll need to be prefixed, polluting the VFIO_ namespace is dangerous.
--
David Marchand
