Re: [PATCH v8 5/6] vfio: Enable cdev noiommu mode under iommufd

[Alex Williamson]

On Tue, 9 Jun 2026 11:50:58 -0700
Jacob Pan <jacob.pan@xxxxxxxxxxxxxxxxxxx> wrote:

> Hi Alex,
> 
> On Mon, 8 Jun 2026 17:19:56 -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>, Baolu Lu
> > <baolu.lu@xxxxxxxxxxxxxxx>, Saurabh Sengar
> > <ssengar@xxxxxxxxxxxxxxxxxxx>, skhawaja@xxxxxxxxxx,
> > pasha.tatashin@xxxxxxxxxx, Will Deacon <will@xxxxxxxxxx>,
> > alex@xxxxxxxxxxx Subject: Re: [PATCH v8 5/6] vfio: Enable cdev
> > noiommu mode under iommufd Date: Mon, 8 Jun 2026 17:19:56 -0600
> > X-Mailer: Claws Mail 4.4.0 (GTK 3.24.52; x86_64-pc-linux-gnu)
> > 
> > On Wed,  3 Jun 2026 15:02:10 -0700
> > Jacob Pan <jacob.pan@xxxxxxxxxxxxxxxxxxx> wrote:
> >   
> > > Now that devices under noiommu mode can bind with IOMMUFD and
> > > perform IOAS operations, lift restrictions on cdev from VFIO side.
> > > Use cases are documented in Documentation/driver-api/vfio.rst
> > > 
> > > Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> > > Signed-off-by: Jacob Pan <jacob.pan@xxxxxxxxxxxxxxxxxxx>
> > > ---
> > > v8:
> > >   - Fix warning message (Kevin)
> > > v7:
> > >   - Avoid treating emulated device as noiommu device (Sashiko)
> > >   - Keep platforms w/ GENERIC_ATOMIC64 to use VFIO group noiommu as
> > >     before (Sashiko)
> > >   - Restore order of group & cdev init for noiommu (Yi)
> > >   - Consolidate noiommu helper for cdev & group (Yi)
> > > v6:
> > >   - Revert back to unified VFIO_NOIOMMU Kconfig for both cdev and
> > > group. Use Kconfig dependency to restrict usages and avoid null
> > > group checks. (Alex & Yi)
> > >   - Add CAP_SYS_RAWIO checks for cdev open to maintain security
> > > parity with the group noiommu path. (Alex)
> > > v5:
> > >    - Add Kconfig VFIO_CDEV_NOIOMMU to select IOMMUFD_NOIOMMU
> > >      and its dependencies
> > >    - Add comment to explain vfio_noiommu conditional definition
> > > (Alex)
> > >    - Removed early return for group noiommu in bind/unbind
> > >    - Use consistent wording referring to VFIO noiommu mode (Kevin)
> > >    - Update unsafe_noiommu Kconfig help text (Kevin)
> > >    - Change dev_warn to dev_info for noiommu enabling msg (Kevin)
> > > v4:
> > >    - Remove early return in iommufd_bind for noiommu (Alex)
> > > v3:
> > >    - Consolidate into fewer patches
> > > v2:
> > >    - removed unnecessary device->noiommu set in
> > >      iommufd_vfio_compat_ioas_get_id()
> > > 
> > > ---
> > >  drivers/vfio/Kconfig       |  7 ++++---
> > >  drivers/vfio/device_cdev.c |  3 +++
> > >  drivers/vfio/iommufd.c     | 12 ++++++++----
> > >  drivers/vfio/vfio.h        | 23 +++++++++--------------
> > >  drivers/vfio/vfio_main.c   | 26 +++++++++++++++++++++++++-
> > >  include/linux/vfio.h       |  1 +
> > >  6 files changed, 50 insertions(+), 22 deletions(-)
> > > 
> > > diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
> > > index ceae52fd7586..b9d6e1c22aed 100644
> > > --- a/drivers/vfio/Kconfig
> > > +++ b/drivers/vfio/Kconfig
> > > @@ -22,8 +22,7 @@ config VFIO_DEVICE_CDEV
> > >  	  The VFIO device cdev is another way for userspace to get
> > > device access. Userspace gets device fd by opening device cdev under
> > >  	  /dev/vfio/devices/vfioX, and then bind the device fd
> > > with an iommufd
> > > -	  to set up secure DMA context for device access.  This
> > > interface does
> > > -	  not support noiommu.
> > > +	  to set up secure DMA context for device access.
> > >  
> > >  	  If you don't know what to do here, say N.
> > >  
> > > @@ -62,7 +61,9 @@ endif
> > >  
> > >  config VFIO_NOIOMMU
> > >  	bool "VFIO No-IOMMU support"
> > > -	depends on VFIO_GROUP
> > > +	depends on VFIO_GROUP || (VFIO_DEVICE_CDEV &&
> > > !GENERIC_ATOMIC64)
> > > +	depends on !VFIO_GROUP || VFIO_CONTAINER ||
> > > IOMMUFD_VFIO_CONTAINER
> > > +	select IOMMUFD_NOIOMMU if VFIO_DEVICE_CDEV &&
> > > !GENERIC_ATOMIC64    
> > 
> > Sashiko is warning about this and it seems real, if the config were
> > something like this:
> > 
> >   CONFIG_GENERIC_ATOMIC64=y
> >   CONFIG_VFIO=y
> >   CONFIG_VFIO_GROUP=y
> >   CONFIG_VFIO_CONTAINER=y
> >   CONFIG_VFIO_DEVICE_CDEV=y
> > 
> > The result is:
> > 
> >   # => CONFIG_VFIO_NOIOMMU=y
> >   # => CONFIG_IOMMUFD_NOIOMMU is not set
> > 
> > Which can result in:
> > 
> >   /dev/vfio/
> >   ├── devices/
> >   │   └── vfio0
> >   └── noiommu-0
> > 
> > The cdev exists without the noiommu- prefix.
> >   
> Indeed, I thought about this which is why I put this comment in the code
> "There cannot be a combination of a plain vfio%d cdev name and
> a no-IOMMU group because VFIO_NOIOMMU selects IOMMUFD_NOIOMMU."
> But I missed the select logic.
> 
> > Something like this might work
> > 
> >    config VFIO_NOIOMMU
> >    	bool "VFIO No-IOMMU support"
> >    	depends on VFIO_GROUP || (VFIO_DEVICE_CDEV &&
> > !GENERIC_ATOMIC64)
> >   +	depends on !VFIO_DEVICE_CDEV || !GENERIC_ATOMIC64
> >    	depends on !VFIO_GROUP || VFIO_CONTAINER ||
> > IOMMUFD_VFIO_CONTAINER
> >   -	select IOMMUFD_NOIOMMU if VFIO_DEVICE_CDEV &&
> > !GENERIC_ATOMIC64
> >   +	select IOMMUFD_NOIOMMU if VFIO_DEVICE_CDEV
> >    	help
> >    	  VFIO is built on the ability to isolate devices using
> > the IOMMU.
> >   
> 
> This will work, but it disables VFIO_NOIOMMU for configs with
> VFIO_DEVICE_CDEV=y and GENERIC_ATOMIC64=y, even though the legacy group
> noiommu path still works there. That can break existing distro configs
> which enable both VFIO_GROUP and VFIO_DEVICE_CDEV, right?
> 
> How about add code change to skip noiommu cdev registeration if
> IOMMUFD_NOIOMMU is not enabled? i.e.
> --- a/drivers/vfio/vfio.h
> +++ b/drivers/vfio/vfio.h
> @@ -359,13 +359,21 @@ void vfio_init_device_cdev(struct vfio_device
> *device);
> 
>  static inline int vfio_device_add(struct vfio_device *device)
>  {
> +       if (vfio_device_is_noiommu(device) &&
> +           !IS_ENABLED(CONFIG_IOMMUFD_NOIOMMU))
> +               return device_add(&device->device);
> +
>         vfio_init_device_cdev(device);
>         return cdev_device_add(&device->cdev, &device->device);
>  }
> 
>  static inline void vfio_device_del(struct vfio_device *device)
>  {
> -       cdev_device_del(&device->cdev, &device->device);
> +       if (vfio_device_is_noiommu(device) &&
> +           !IS_ENABLED(CONFIG_IOMMUFD_NOIOMMU))
> +               device_del(&device->device);
> +       else
> +               cdev_device_del(&device->cdev, &device->device);
>  }
> I will also update the documentation to state this behavior:
> 
> "The cdev noiommu path requires CONFIG_GENERIC_ATOMIC64=n.  When
> CONFIG_VFIO_GROUP=y, CONFIG_VFIO_DEVICE_CDEV=y, and
> CONFIG_GENERIC_ATOMIC64=y, CONFIG_VFIO_NOIOMMU remains selectable for
> the group path, but no noiommu device cdev is registered.  Cdev-only
> noiommu is not selectable on those platforms."

I suspect that the Venn diagram of the set of platforms that set
GENERIC_ATOMIC64 and the set of platforms we care about distro config
compatibility (or even the existence of a distro) is pretty nearly
disjoint.  That said, your solution is better.

One check though, it looks like cdev_device_{add,del} already degrade
to device_{add,del} when device->devt == 0, so we could maybe simplify
by making vfio_init_device_cdev() conditional and the rest falls out
automatically.  That also avoids the device->group traversal to check
noiommu on the del path.  Thanks,

Alex