Re: [Intel-gfx] [PATCH v12 20/24] vfio: Only check group->type for noiommu test

Liu, Yi L
2023-06-13T09:21:11Z

> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Tuesday, June 13, 2023 6:38 AM
> On Fri, 2 Jun 2023 05:16:49 -0700
> Yi Liu <yi.l.liu@intel.com> wrote:
>
> > group->type can be VFIO_NO_IOMMU only when vfio_noiommu option is true.
> > And vfio_noiommu option can only be true if CONFIG_VFIO_NOIOMMU is enabled.
> > So checking group->type is enough when testing noiommu.
> >
> > Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> > ---
> > drivers/vfio/group.c | 3 +--
> > drivers/vfio/vfio.h | 3 +--
> > 2 files changed, 2 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
> > index 41a09a2df690..653b62f93474 100644
> > --- a/drivers/vfio/group.c
> > +++ b/drivers/vfio/group.c
> > @@ -133,8 +133,7 @@ static int vfio_group_ioctl_set_container(struct vfio_group
> *group,
> >
> > iommufd = iommufd_ctx_from_file(f.file);
> > if (!IS_ERR(iommufd)) {
> > - if (IS_ENABLED(CONFIG_VFIO_NOIOMMU) &&
> > - group->type == VFIO_NO_IOMMU)
> > + if (group->type == VFIO_NO_IOMMU)
> > ret = iommufd_vfio_compat_set_no_iommu(iommufd);
> > else
> > ret = iommufd_vfio_compat_ioas_create(iommufd);
> > diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
> > index 5835c74e97ce..1b89e8bc8571 100644
> > --- a/drivers/vfio/vfio.h
> > +++ b/drivers/vfio/vfio.h
> > @@ -108,8 +108,7 @@ void vfio_group_cleanup(void);
> >
> > static inline bool vfio_device_is_noiommu(struct vfio_device *vdev)
> > {
> > - return IS_ENABLED(CONFIG_VFIO_NOIOMMU) &&
> > - vdev->group->type == VFIO_NO_IOMMU;
> > + return vdev->group->type == VFIO_NO_IOMMU;
> > }
> >
> > #if IS_ENABLED(CONFIG_VFIO_CONTAINER)
>
> This patch should be dropped. It's logically correct, but ignores that
> the config option can be determined at compile time and therefore the
> code can be better optimized based on that test. I think there was a
> specific case where I questioned it, but this drops an otherwise valid
> compiler optimization. Thanks,

Yes. in v11, you mentioned the compiler optimization and the fact that
vfio_noiommu can only be valid when VFIO_NOIOMMU is enabled. I'm
ok to drop this patch to keep the compiler optimization.

Regards,
Yi Liu