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

[David Marchand <david.marchand@redhat.com>]

Hello Anatoly,

On Tue, 9 Sept 2025 at 17:38, Burakov, Anatoly
<anatoly.burakov@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.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.

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.burakov@intel.com>

But ok, we can keep those macros.
They'll need to be prefixed, polluting the VFIO_ namespace is dangerous.


-- 
David Marchand