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

[Alex Williamson]

On Fri, 17 Apr 2026 10:06:09 -0700
Jacob Pan <jacob.pan@xxxxxxxxxxxxxxxxxxx> wrote:

> Hi Alex,
> 
> On Thu, 16 Apr 2026 14:06:01 -0600
> Alex Williamson <alex@xxxxxxxxxxx> wrote:
> 
> > From: Alex Williamson <alex@xxxxxxxxxxx>
> > To: Jacob Pan <jacob.pan@xxxxxxxxxxxxxxxxxxx>
> > Cc: linux-kernel@xxxxxxxxxxxxxxx, "iommu@xxxxxxxxxxxxxxx"
> > <iommu@xxxxxxxxxxxxxxx>, Jason Gunthorpe <jgg@xxxxxxxxxx>, Joerg
> > Roedel  <joro@xxxxxxxxxx>, Mostafa Saleh <smostafa@xxxxxxxxxx>, David
> > Matlack  <dmatlack@xxxxxxxxxx>, Robin Murphy <robin.murphy@xxxxxxx>,
> > Nicolin Chen  <nicolinc@xxxxxxxxxx>, "Tian, Kevin"
> > <kevin.tian@xxxxxxxxx>, Yi Liu  <yi.l.liu@xxxxxxxxx>,
> > skhawaja@xxxxxxxxxx, pasha.tatashin@xxxxxxxxxx, Will  Deacon
> > <will@xxxxxxxxxx>, Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>,
> > alex@xxxxxxxxxxx Subject: Re: [PATCH V4 05/10] vfio: Allow null group
> > for noiommu without  containers Date: Thu, 16 Apr 2026 14:06:01 -0600
> > X-Mailer: Claws Mail 4.3.1 (GTK 3.24.51; x86_64-pc-linux-gnu)
> > 
> > On Tue, 14 Apr 2026 14:14:07 -0700
> > Jacob Pan <jacob.pan@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxxx>
> > > 
> > > ---
> > > 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?  
> If we solve this in Kconfig, I think the dependency should be
> config VFIO_GROUP
>         bool "Support for the VFIO group /dev/vfio/$group_id"
> +       depends on !(VFIO_NOIOMMU && !(VFIO_CONTAINER ||
>   IOMMUFD_VFIO_CONTAINER))
> But this causes circular dependency in that
>        symbol VFIO_NOIOMMU depends on VFIO_GROUP
>        symbol VFIO_GROUP depends on VFIO_NOIOMMU
> 
> If we cannot force VFIO_GROUP=n when container is entirely disabled and
> NOIOMMU & cdev is enabled, then user is free to set VFIO_GROUP=y, which
> creates a VFIO_GROUP that cannot be used due to lack of container.
> There is no functional issue but less clean.
> i.e.
> #  tree /dev/vfio/    
> /dev/vfio/            
> |-- devices           
> |   `-- noiommu-vfio0 
> `-- noiommu-0         //not usable
> # ls /sys/class/vfio
> noiommu-0           
> 
> Maybe there is a way to force VFIO_GROUP=n w/o the circular dependency?

What I'm trying to point out is that vfio_null_group_allowed() is being
used in a scenario in group.c that shouldn't exist.  If all container
support is disabled, all group support should also be disabled,
regardless of no-iommu.  Otherwise we get into the scenario you show
above.  No-iommu is a feature, currently only a feature of the
group/container model, but that's what we're trying to address here.

I'm not sure what the Kconfig looks like to achieve that.

> > Also note that vfio_noiommu is S_IWUSR, so it is mutable at runtime.  
> Good point, maybe we can make it a one-way latch? i.e.
>  - echo 1 > .../enable_unsafe_noiommu_mode — works (n→y)
>  - echo 0 > .../enable_unsafe_noiommu_mode — returns -EPERM (y→n blocked)
>  - Boot param vfio.enable_unsafe_noiommu_mode=1 — works
>  - Writing 1 when already 1 — no-op, succeeds

It's a question of whether you'll break anyone.  IIRC, group-based
no-iommu works that you can enabled it, create a no-iommu group,
disabled it, and the no-iommu group continues to work.  Is it useful,
does anyone use it... I dunno.  Thanks,

Alex