Re: [PATCH V4 05/10] vfio: Allow null group for noiommu without containers

[Alex Williamson <alex@shazbot.org>]

On Tue, 14 Apr 2026 14:14:07 -0700
Jacob Pan <jacob.pan@linux.microsoft.com> wrote:

> In case of noiommu mode is enabled for VFIO cdev without VFIO container
> nor IOMMUFD provided compatibility container, there is no need to
> create a dummy group. Update the group operations to tolerate null group
> pointer.
> 
> Signed-off-by: Jacob Pan <jacob.pan@linux.microsoft.com>
> 
> ---
> v4: (Jason)
>    - Avoid null pointer deref in error unwind
>    - Add null group check in vfio_device_group_unregister
>    - repartition to include vfio_device_has_group() in this patch
> ---
>  drivers/vfio/group.c     | 20 ++++++++++++++++++++
>  drivers/vfio/vfio.h      | 17 +++++++++++++++++
>  drivers/vfio/vfio_main.c | 14 ++++++++++++++
>  include/linux/vfio.h     |  9 +++++++++
>  4 files changed, 60 insertions(+)
> 
> diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c
> index 0fa9761b13d3..451e49d851f8 100644
> --- a/drivers/vfio/group.c
> +++ b/drivers/vfio/group.c
> @@ -390,6 +390,9 @@ int vfio_device_block_group(struct vfio_device *device)
>  	struct vfio_group *group = device->group;
>  	int ret = 0;
>  
> +	if (vfio_null_group_allowed() && !group)
> +		return 0;

I think this comes down to the fact that at the end of this series,
VFIO_NOIOMMU still depends on VFIO_GROUP.  vfio_null_group_allowed()
can only return true if CONTAINER support is entirely disabled.  Why do
we still select VFIO_GROUP for VFIO_NOIOMMU and build group.s when
there's no container support to use it?

Also note that vfio_noiommu is S_IWUSR, so it is mutable at runtime.
Thanks,

Alex

> +
>  	mutex_lock(&group->group_lock);
>  	if (group->opened_file) {
>  		ret = -EBUSY;
> @@ -407,6 +410,9 @@ void vfio_device_unblock_group(struct vfio_device *device)
>  {
>  	struct vfio_group *group = device->group;
>  
> +	if (vfio_null_group_allowed() && !group)
> +		return;
> +
>  	mutex_lock(&group->group_lock);
>  	group->cdev_device_open_cnt--;
>  	mutex_unlock(&group->group_lock);
> @@ -598,6 +604,14 @@ static struct vfio_group *vfio_noiommu_group_alloc(struct device *dev,
>  	struct vfio_group *group;
>  	int ret;
>  
> +	/*
> +	 * With noiommu enabled under cdev interface only, there is no need to
> +	 * create a vfio_group if the group based containers are not enabled.
> +	 * The cdev interface is exclusively used for iommufd.
> +	 */
> +	if (vfio_null_group_allowed())
> +		return NULL;
> +
>  	iommu_group = iommu_group_alloc();
>  	if (IS_ERR(iommu_group))
>  		return ERR_CAST(iommu_group);
> @@ -705,6 +719,9 @@ void vfio_device_remove_group(struct vfio_device *device)
>  	struct vfio_group *group = device->group;
>  	struct iommu_group *iommu_group;
>  
> +	if (!group)
> +		return;
> +
>  	if (group->type == VFIO_NO_IOMMU || group->type == VFIO_EMULATED_IOMMU)
>  		iommu_group_remove_device(device->dev);
>  
> @@ -756,6 +773,9 @@ void vfio_device_group_register(struct vfio_device *device)
>  
>  void vfio_device_group_unregister(struct vfio_device *device)
>  {
> +	if (!device->group)
> +		return;
> +
>  	mutex_lock(&device->group->device_lock);
>  	list_del(&device->group_next);
>  	mutex_unlock(&device->group->device_lock);
> diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
> index 8fcc98cf9577..db1530bb1716 100644
> --- a/drivers/vfio/vfio.h
> +++ b/drivers/vfio/vfio.h
> @@ -114,6 +114,18 @@ bool vfio_device_has_container(struct vfio_device *device);
>  int __init vfio_group_init(void);
>  void vfio_group_cleanup(void);
>  
> +/*
> + * With noiommu enabled and no containers are supported, allow devices that
> + * don't have a dummy group.
> + */
> +static inline bool vfio_null_group_allowed(void)
> +{
> +	if (vfio_noiommu && (!IS_ENABLED(CONFIG_VFIO_CONTAINER) && !IS_ENABLED(CONFIG_IOMMUFD_VFIO_CONTAINER)))
> +		return true;
> +
> +	return false;
> +}
> +
>  static inline bool vfio_device_is_noiommu(struct vfio_device *vdev)
>  {
>  	return IS_ENABLED(CONFIG_VFIO_NOIOMMU) &&
> @@ -190,6 +202,11 @@ static inline void vfio_group_cleanup(void)
>  {
>  }
>  
> +static inline bool vfio_null_group_allowed(void)
> +{
> +	return false;
> +}
> +
>  static inline bool vfio_device_is_noiommu(struct vfio_device *vdev)
>  {
>  	return false;
> diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> index e5886235cad4..5d7c2d014689 100644
> --- a/drivers/vfio/vfio_main.c
> +++ b/drivers/vfio/vfio_main.c
> @@ -358,6 +358,10 @@ static int __vfio_register_dev(struct vfio_device *device,
>  	/* Refcounting can't start until the driver calls register */
>  	refcount_set(&device->refcount, 1);
>  
> +	/* noiommu device w/o container may have NULL group */
> +	if (!vfio_device_has_group(device))
> +		return 0;
> +
>  	vfio_device_group_register(device);
>  	vfio_device_debugfs_init(device);
>  
> @@ -392,6 +396,16 @@ void vfio_unregister_group_dev(struct vfio_device *device)
>  	bool interrupted = false;
>  	long rc;
>  
> +	/*
> +	 * For noiommu devices without a container, thus no dummy group,
> +	 * simply delete and unregister to balance refcount.
> +	 */
> +	if (!vfio_device_has_group(device)) {
> +		vfio_device_del(device);
> +		vfio_device_put_registration(device);
> +		return;
> +	}
> +
>  	/*
>  	 * Prevent new device opened by userspace via the
>  	 * VFIO_GROUP_GET_DEVICE_FD in the group path.
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 7384965d15d7..ceb5034c3a2e 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -328,6 +328,10 @@ struct iommu_group *vfio_file_iommu_group(struct file *file);
>  #if IS_ENABLED(CONFIG_VFIO_GROUP)
>  bool vfio_file_is_group(struct file *file);
>  bool vfio_file_has_dev(struct file *file, struct vfio_device *device);
> +static inline bool vfio_device_has_group(struct vfio_device *device)
> +{
> +	return device->group;
> +}
>  #else
>  static inline bool vfio_file_is_group(struct file *file)
>  {
> @@ -338,6 +342,11 @@ static inline bool vfio_file_has_dev(struct file *file, struct vfio_device *devi
>  {
>  	return false;
>  }
> +
> +static inline bool vfio_device_has_group(struct vfio_device *device)
> +{
> +	return false;
> +}
>  #endif
>  bool vfio_file_is_valid(struct file *file);
>  bool vfio_file_enforced_coherent(struct file *file);