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

[Jacob Pan <jacob.pan@linux.microsoft.com>]

Hi Alex,

On Tue, 9 Jun 2026 14:07:57 -0600
Alex Williamson <alex@shazbot.org> wrote:

> On Tue, 9 Jun 2026 11:50:58 -0700
> Jacob Pan <jacob.pan@linux.microsoft.com> wrote:
> 
> > Hi Alex,
> > 
> > On Mon, 8 Jun 2026 17:19:56 -0600
> > Alex Williamson <alex@shazbot.org> wrote:
> >   
> > > From: Alex Williamson <alex@shazbot.org>
> > > To: Jacob Pan <jacob.pan@linux.microsoft.com>
> > > Cc: linux-kernel@vger.kernel.org, "iommu@lists.linux.dev"
> > > <iommu@lists.linux.dev>, Jason Gunthorpe <jgg@nvidia.com>, Joerg
> > > Roedel  <joro@8bytes.org>, Mostafa Saleh <smostafa@google.com>,
> > > David Matlack  <dmatlack@google.com>, Robin Murphy
> > > <robin.murphy@arm.com>, Nicolin Chen  <nicolinc@nvidia.com>,
> > > "Tian, Kevin" <kevin.tian@intel.com>, Yi Liu
> > > <yi.l.liu@intel.com>, Baolu Lu <baolu.lu@linux.intel.com>,
> > > Saurabh Sengar <ssengar@linux.microsoft.com>, skhawaja@google.com,
> > > pasha.tatashin@soleen.com, Will Deacon <will@kernel.org>,
> > > alex@shazbot.org 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@linux.microsoft.com> 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@intel.com>
> > > > Signed-off-by: Jacob Pan <jacob.pan@linux.microsoft.com>
> > > > ---
> > > > 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,
> 
Indeed, this is much simpler. Will do below as you suggested, Thanks.
--- a/drivers/vfio/device_cdev.c
+++ b/drivers/vfio/device_cdev.c
@@ -11,6 +11,10 @@ static dev_t device_devt;

 void vfio_init_device_cdev(struct vfio_device *device)
 {
+       if (vfio_device_is_noiommu(device) &&
+           !IS_ENABLED(CONFIG_IOMMUFD_NOIOMMU))
+               return;
+