Could you address the findings from Sashiko?

https://sashiko.dev/#/patchset/20260521221155.1375144-1-jacob.pan%40linux.microsoft.com

> From: Jacob Pan <jacob.pan@linux.microsoft.com>
> Sent: Friday, May 22, 2026 6:12 AM
> 
> VFIO's unsafe_noiommu_mode has long provided a way for userspace
> drivers
> to operate on platforms lacking a hardware IOMMU. Today, IOMMUFD also
> supports No-IOMMU mode for group-based devices under vfio_compat
> mode.
> However, IOMMUFD's native character device (cdev) does not yet support
> No-IOMMU mode, which is the purpose of this patch.
> 
> In summary, we have:
> 
> |-------------------------+------+---------------|
> | Device access mode      | VFIO | IOMMUFD       |
> |-------------------------+------+---------------|
> | group /dev/vfio/$GROUP  | Yes  | Yes           |
> |-------------------------+------+---------------|
> | cdev /dev/vfio/devices/ | No   | This patch    |
> |-------------------------+------+---------------|
> 
> Beyond enabling cdev for IOMMUFD, this patch also addresses the following
> deficiencies in the current No-IOMMU mode suggested by Jason[1]:
> - Devices operating under No-IOMMU mode are limited to device-level UAPI
>   access, without container or IOAS-level capabilities. Consequently,
>   user-space drivers lack structured mechanisms for page pinning and often
>   resort to mlock(), which is less robust than pin_user_pages() used for
>   devices backed by a physical IOMMU. For example, mlock() does not
> prevent
>   page migration.
> - There is no architectural mechanism for obtaining physical addresses for
>   DMA. As a workaround, user-space drivers frequently rely on
> /proc/pagemap
>   tricks or hardcoded values.
> 
> By allowing noiommu device access to IOMMUFD IOAS and HWPT objects,
> this
> patch brings No-IOMMU mode closer to full citizenship within the IOMMU
> subsystem. In addition to addressing the two deficiencies mentioned above,
> the expectation is that it will also enable No-IOMMU devices to seamlessly
> participate in live update sessions via KHO [2].
> 
> Furthermore, these devices will use the IOMMUFD-based ownership
> checking model for
> VFIO_DEVICE_PCI_HOT_RESET, eliminating the need for an iommufd_access
> object
> as required in a previous attempt [3].
> 
> ChangeLog:
> V6:
>   - Delete rename VFIO_IOMMU patch
>   - 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)
>   - Updated documentation with Kconfig usage matrix
>   - Added max length limit to get_pa ioctl (Baolu & Jason)
> V5:
>   - Split CONFIG_VFIO_NOIOMMU into CONFIG_VFIO_GROUP_NOIOMMU
> and
>     CONFIG_VFIO_CDEV_NOIOMMU so cdev noiommu is independent of
>     VFIO_GROUP (Alex)
>   - Add CAP_SYS_RAWIO check for cdev open and bind under noiommu,
>     security parity with group noiommu (Alex)
>   - Add IS_ENABLED(CONFIG_IOMMUFD_NOIOMMU) guard in
>     iommufd_device_is_noiommu() to prevent noiommu bind when feature
>     is disabled
>   - Add prep patch to tolerate NULL group for cdev noiommu devices
>     when CONFIG_VFIO_GROUP_NOIOMMU is not set [7/9]
>   - Rename IOCTL to IOMMUFD_CMD_IOAS_NOIOMMU_GET_PA to be more
>     specific (Kevin)
>   - Simplify iommufd_device_is_noiommu, use iommufd_bind_noiommu
>     helper (Kevin, Yi)
>   - Move IOMMU cap check under iommufd_bind_iommu() (Yi)
>   - Fix next_iova exceeding iopt_area_last_iova in GET_PA (Alex)
>   - Fix const hwpt, copyright date, typo in moved comment (Kevin)
>   - Add Reviewed-by tags
>   - Squash noiommu cdev selftest fix into selftest patch
>   - Drop DSA selftest patch
>   - Details in each patch changelog.
> 
> V4:
>   - Fix various corner cases pointed out by (Sashiko)
>     Details in each patch changelog.
> 
> V3:
>   - Improve error handling [3/10] (Mostafa)
>   - Simplify vfio_device_is_noiommu logic and merged in [6/10] (Mostafa)
>   - Add comment to explain the design difference over the legacy noiommu
>     VFIO code.[1/10]
> 
> V2:
>   - Fix build dependency by adding IOMMU_SUPPORT in [8/11]
>   - Add an optimization to scan beyond the first page for a contiguous
>     physical address range and return its length instead of a single
>     page.[4/11]
> 
> Since RFC[4]:
>   - Abandoned dummy iommu driver approach as patch 1-3 absorbed the
>     changes into iommufd.
> 
> [1] https://lore.kernel.org/linux-
> iommu/20250603175403.GA407344@nvidia.com/
> [2] https://lore.kernel.org/linux-
> pci/20251027134430.00007e46@linux.microsoft.com/
> [3] https://lore.kernel.org/kvm/20230522115751.326947-1-
> yi.l.liu@intel.com/
> [4] https://lore.kernel.org/linux-iommu/20251201173012.18371-1-
> jacob.pan@linux.microsoft.com/
> 
> Future cleanup: consolidate all CONFIG_IOMMUFD_NOIOMMU code
> (iopt_get_phys, iommufd_ioas_noiommu_get_pa, iommufd_noiommu_ops)
> into
> hwpt_noiommu.c to eliminate #ifdef guards from ioas.c and io_pagetable.c.
> 
> Signed-off-by: Jacob Pan <jacob.pan@linux.microsoft.com>
> 
> 
> Jacob Pan (4):
>   iommufd: Add an ioctl to query PA from IOVA for noiommu mode
>   vfio: Enable cdev noiommu mode under iommufd
>   selftests/vfio: Add iommufd noiommu mode selftest for cdev
>   Documentation: Update VFIO NOIOMMU mode
> 
> Jason Gunthorpe (3):
>   iommufd: Support a HWPT without an iommu driver for noiommu
>   iommufd: Move igroup allocation to a function
>   iommufd: Allow binding to a noiommu device
> 
>  Documentation/driver-api/vfio.rst             |  83 ++-
>  drivers/iommu/iommufd/Kconfig                 |  12 +
>  drivers/iommu/iommufd/Makefile                |   1 +
>  drivers/iommu/iommufd/device.c                | 192 +++--
>  drivers/iommu/iommufd/hw_pagetable.c          |  15 +-
>  drivers/iommu/iommufd/hwpt_noiommu.c          |  97 +++
>  drivers/iommu/iommufd/io_pagetable.c          |  72 ++
>  drivers/iommu/iommufd/ioas.c                  |  30 +
>  drivers/iommu/iommufd/iommufd_private.h       |  20 +
>  drivers/iommu/iommufd/main.c                  |   3 +
>  drivers/vfio/Kconfig                          |   8 +-
>  drivers/vfio/device_cdev.c                    |   3 +
>  drivers/vfio/iommufd.c                        |   6 +-
>  drivers/vfio/vfio.h                           |  20 +-
>  drivers/vfio/vfio_main.c                      |  23 +-
>  include/linux/vfio.h                          |   1 +
>  include/uapi/linux/iommufd.h                  |  27 +
>  tools/testing/selftests/vfio/Makefile         |   1 +
>  .../lib/include/libvfio/vfio_pci_device.h     |  16 +
>  .../selftests/vfio/lib/vfio_pci_device.c      |   5 +-
>  .../vfio/vfio_iommufd_noiommu_test.c          | 664 ++++++++++++++++++
>  21 files changed, 1221 insertions(+), 78 deletions(-)
>  create mode 100644 drivers/iommu/iommufd/hwpt_noiommu.c
>  create mode 100644
> tools/testing/selftests/vfio/vfio_iommufd_noiommu_test.c
> 
> --
> 2.43.0

Hi Kevin,

On Mon, 25 May 2026 08:30:12 +0000
"Tian, Kevin" <kevin.tian@intel.com> wrote:

> Could you address the findings from Sashiko?
> 
> https://sashiko.dev/#/patchset/20260521221155.1375144-1-jacob.pan%40linux.microsoft.com
> 
I have go over my Sashiko review setup, but there are lots of
false positives, like this one below we already discussed in earlier
version. Is there a specific concern?

e.g.
> +static bool iommufd_device_is_noiommu(struct iommufd_device *idev)
> +{
> +	return IS_ENABLED(CONFIG_IOMMUFD_NOIOMMU) &&
> !idev->dev->iommu; +}
Does dynamically evaluating dev->iommu here allow the noiommu state to
flip during the device's lifetime?


> > From: Jacob Pan <jacob.pan@linux.microsoft.com>
> > Sent: Friday, May 22, 2026 6:12 AM
> > 
> > VFIO's unsafe_noiommu_mode has long provided a way for userspace
> > drivers
> > to operate on platforms lacking a hardware IOMMU. Today, IOMMUFD
> > also supports No-IOMMU mode for group-based devices under
> > vfio_compat mode.
> > However, IOMMUFD's native character device (cdev) does not yet
> > support No-IOMMU mode, which is the purpose of this patch.
> > 
> > In summary, we have:
> > 
> > |-------------------------+------+---------------|
> > | Device access mode      | VFIO | IOMMUFD       |
> > |-------------------------+------+---------------|
> > | group /dev/vfio/$GROUP  | Yes  | Yes           |
> > |-------------------------+------+---------------|
> > | cdev /dev/vfio/devices/ | No   | This patch    |
> > |-------------------------+------+---------------|
> > 
> > Beyond enabling cdev for IOMMUFD, this patch also addresses the
> > following deficiencies in the current No-IOMMU mode suggested by
> > Jason[1]:
> > - Devices operating under No-IOMMU mode are limited to device-level
> > UAPI access, without container or IOAS-level capabilities.
> > Consequently, user-space drivers lack structured mechanisms for
> > page pinning and often resort to mlock(), which is less robust than
> > pin_user_pages() used for devices backed by a physical IOMMU. For
> > example, mlock() does not prevent
> >   page migration.
> > - There is no architectural mechanism for obtaining physical
> > addresses for DMA. As a workaround, user-space drivers frequently
> > rely on /proc/pagemap
> >   tricks or hardcoded values.
> > 
> > By allowing noiommu device access to IOMMUFD IOAS and HWPT objects,
> > this
> > patch brings No-IOMMU mode closer to full citizenship within the
> > IOMMU subsystem. In addition to addressing the two deficiencies
> > mentioned above, the expectation is that it will also enable
> > No-IOMMU devices to seamlessly participate in live update sessions
> > via KHO [2].
> > 
> > Furthermore, these devices will use the IOMMUFD-based ownership
> > checking model for
> > VFIO_DEVICE_PCI_HOT_RESET, eliminating the need for an
> > iommufd_access object
> > as required in a previous attempt [3].
> > 
> > ChangeLog:
> > V6:
> >   - Delete rename VFIO_IOMMU patch
> >   - 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)
> >   - Updated documentation with Kconfig usage matrix
> >   - Added max length limit to get_pa ioctl (Baolu & Jason)
> > V5:
> >   - Split CONFIG_VFIO_NOIOMMU into CONFIG_VFIO_GROUP_NOIOMMU
> > and
> >     CONFIG_VFIO_CDEV_NOIOMMU so cdev noiommu is independent of
> >     VFIO_GROUP (Alex)
> >   - Add CAP_SYS_RAWIO check for cdev open and bind under noiommu,
> >     security parity with group noiommu (Alex)
> >   - Add IS_ENABLED(CONFIG_IOMMUFD_NOIOMMU) guard in
> >     iommufd_device_is_noiommu() to prevent noiommu bind when feature
> >     is disabled
> >   - Add prep patch to tolerate NULL group for cdev noiommu devices
> >     when CONFIG_VFIO_GROUP_NOIOMMU is not set [7/9]
> >   - Rename IOCTL to IOMMUFD_CMD_IOAS_NOIOMMU_GET_PA to be more
> >     specific (Kevin)
> >   - Simplify iommufd_device_is_noiommu, use iommufd_bind_noiommu
> >     helper (Kevin, Yi)
> >   - Move IOMMU cap check under iommufd_bind_iommu() (Yi)
> >   - Fix next_iova exceeding iopt_area_last_iova in GET_PA (Alex)
> >   - Fix const hwpt, copyright date, typo in moved comment (Kevin)
> >   - Add Reviewed-by tags
> >   - Squash noiommu cdev selftest fix into selftest patch
> >   - Drop DSA selftest patch
> >   - Details in each patch changelog.
> > 
> > V4:
> >   - Fix various corner cases pointed out by (Sashiko)
> >     Details in each patch changelog.
> > 
> > V3:
> >   - Improve error handling [3/10] (Mostafa)
> >   - Simplify vfio_device_is_noiommu logic and merged in [6/10]
> > (Mostafa)
> >   - Add comment to explain the design difference over the legacy
> > noiommu VFIO code.[1/10]
> > 
> > V2:
> >   - Fix build dependency by adding IOMMU_SUPPORT in [8/11]
> >   - Add an optimization to scan beyond the first page for a
> > contiguous physical address range and return its length instead of
> > a single page.[4/11]
> > 
> > Since RFC[4]:
> >   - Abandoned dummy iommu driver approach as patch 1-3 absorbed the
> >     changes into iommufd.
> > 
> > [1] https://lore.kernel.org/linux-
> > iommu/20250603175403.GA407344@nvidia.com/
> > [2] https://lore.kernel.org/linux-
> > pci/20251027134430.00007e46@linux.microsoft.com/
> > [3] https://lore.kernel.org/kvm/20230522115751.326947-1-
> > yi.l.liu@intel.com/
> > [4] https://lore.kernel.org/linux-iommu/20251201173012.18371-1-
> > jacob.pan@linux.microsoft.com/
> > 
> > Future cleanup: consolidate all CONFIG_IOMMUFD_NOIOMMU code
> > (iopt_get_phys, iommufd_ioas_noiommu_get_pa, iommufd_noiommu_ops)
> > into
> > hwpt_noiommu.c to eliminate #ifdef guards from ioas.c and
> > io_pagetable.c.
> > 
> > Signed-off-by: Jacob Pan <jacob.pan@linux.microsoft.com>
> > 
> > 
> > Jacob Pan (4):
> >   iommufd: Add an ioctl to query PA from IOVA for noiommu mode
> >   vfio: Enable cdev noiommu mode under iommufd
> >   selftests/vfio: Add iommufd noiommu mode selftest for cdev
> >   Documentation: Update VFIO NOIOMMU mode
> > 
> > Jason Gunthorpe (3):
> >   iommufd: Support a HWPT without an iommu driver for noiommu
> >   iommufd: Move igroup allocation to a function
> >   iommufd: Allow binding to a noiommu device
> > 
> >  Documentation/driver-api/vfio.rst             |  83 ++-
> >  drivers/iommu/iommufd/Kconfig                 |  12 +
> >  drivers/iommu/iommufd/Makefile                |   1 +
> >  drivers/iommu/iommufd/device.c                | 192 +++--
> >  drivers/iommu/iommufd/hw_pagetable.c          |  15 +-
> >  drivers/iommu/iommufd/hwpt_noiommu.c          |  97 +++
> >  drivers/iommu/iommufd/io_pagetable.c          |  72 ++
> >  drivers/iommu/iommufd/ioas.c                  |  30 +
> >  drivers/iommu/iommufd/iommufd_private.h       |  20 +
> >  drivers/iommu/iommufd/main.c                  |   3 +
> >  drivers/vfio/Kconfig                          |   8 +-
> >  drivers/vfio/device_cdev.c                    |   3 +
> >  drivers/vfio/iommufd.c                        |   6 +-
> >  drivers/vfio/vfio.h                           |  20 +-
> >  drivers/vfio/vfio_main.c                      |  23 +-
> >  include/linux/vfio.h                          |   1 +
> >  include/uapi/linux/iommufd.h                  |  27 +
> >  tools/testing/selftests/vfio/Makefile         |   1 +
> >  .../lib/include/libvfio/vfio_pci_device.h     |  16 +
> >  .../selftests/vfio/lib/vfio_pci_device.c      |   5 +-
> >  .../vfio/vfio_iommufd_noiommu_test.c          | 664
> > ++++++++++++++++++ 21 files changed, 1221 insertions(+), 78
> > deletions(-) create mode 100644 drivers/iommu/iommufd/hwpt_noiommu.c
> >  create mode 100644
> > tools/testing/selftests/vfio/vfio_iommufd_noiommu_test.c
> > 
> > --
> > 2.43.0

On Tue, 26 May 2026 08:32:37 -0700
Jacob Pan <jacob.pan@linux.microsoft.com> wrote:

> Hi Kevin,
> 
> On Mon, 25 May 2026 08:30:12 +0000
> "Tian, Kevin" <kevin.tian@intel.com> wrote:
> 
> > Could you address the findings from Sashiko?
> > 
> > https://sashiko.dev/#/patchset/20260521221155.1375144-1-jacob.pan%40linux.microsoft.com
> >   
> I have go over my Sashiko review setup, but there are lots of
> false positives, like this one below we already discussed in earlier
> version. Is there a specific concern?
> 
> e.g.
> > +static bool iommufd_device_is_noiommu(struct iommufd_device *idev)
> > +{
> > +	return IS_ENABLED(CONFIG_IOMMUFD_NOIOMMU) &&
> > !idev->dev->iommu; +}  
> Does dynamically evaluating dev->iommu here allow the noiommu state to
> flip during the device's lifetime?

Yes, that one is at best a theoretical issue, but the next two NULL
pointer dereference if user passes noiommu device fd through an
unexpected iommufd interface appear quite real.

We're also still struggling with the Kconfig in patch 5, this Sashiko
comment is valid:

>> @@ -62,7 +61,10 @@ endif
>>  
>>  config VFIO_NOIOMMU
>>  	bool "VFIO No-IOMMU support"
>> -	depends on VFIO_GROUP
>> +	depends on VFIO_GROUP || VFIO_DEVICE_CDEV
>> +	depends on !VFIO_GROUP || VFIO_CONTAINER || IOMMUFD_VFIO_CONTAINER
>> +	depends on !VFIO_DEVICE_CDEV || !GENERIC_ATOMIC64
>
> Does this disable VFIO_NOIOMMU completely for legacy group users on
> architectures using generic atomic64 implementations?
>
> On architectures like 32-bit ARM or x86, !GENERIC_ATOMIC64 evaluates
> to false.  If a distribution enables VFIO_DEVICE_CDEV, this dependency
> resolves to false, silently breaking backwards compatibility and
> depriving legacy group-based users of noiommu support.

That's true, the question is do we care (I'd prefer to) and if so,
should we block the relevant interfaces from working though iommufd
rather than disallowing the Kconfig option.

The next issue regarding classifying emulated IOMMU devices as no-iommu
also appears valid, mdev devices like kvmgt for example.

The next issue raises a valid concern whether the dev_warn() should be
a dev_warn_once().

The sysfs naming comment is invalid, we intentionally name noiommu
devices uniquely to force userspace opt-in.

In patch 6, adding dead code is a valid comment, the unchecked asprintf
does seem to be an outlier in selftest code, the unmap comment may be a
false positive, as is the hugepage size, but the masked return comments
could arguably be skips or asserts.  There are potentially a couple
remaining nits and style issues noted.

Overall, more signal than noise afaict.  Thanks,

Alex

Hi Alex,

On Tue, 26 May 2026 11:57:09 -0600
Alex Williamson <alex@shazbot.org> wrote:

> On Tue, 26 May 2026 08:32:37 -0700
> Jacob Pan <jacob.pan@linux.microsoft.com> wrote:
> 
> > Hi Kevin,
> > 
> > On Mon, 25 May 2026 08:30:12 +0000
> > "Tian, Kevin" <kevin.tian@intel.com> wrote:
> >   
> > > Could you address the findings from Sashiko?
> > > 
> > > https://sashiko.dev/#/patchset/20260521221155.1375144-1-jacob.pan%40linux.microsoft.com
> > >     
> > I have go over my Sashiko review setup, but there are lots of
> > false positives, like this one below we already discussed in earlier
> > version. Is there a specific concern?
> > 
> > e.g.  
> > > +static bool iommufd_device_is_noiommu(struct iommufd_device
> > > *idev) +{
> > > +	return IS_ENABLED(CONFIG_IOMMUFD_NOIOMMU) &&
> > > !idev->dev->iommu; +}    
> > Does dynamically evaluating dev->iommu here allow the noiommu state
> > to flip during the device's lifetime?  
> 
> Yes, that one is at best a theoretical issue, but the next two NULL
> pointer dereference if user passes noiommu device fd through an
> unexpected iommufd interface appear quite real.
> 
> We're also still struggling with the Kconfig in patch 5, this Sashiko
> comment is valid:
> 
> >> @@ -62,7 +61,10 @@ endif
> >>  
> >>  config VFIO_NOIOMMU
> >>  	bool "VFIO No-IOMMU support"
> >> -	depends on VFIO_GROUP
> >> +	depends on VFIO_GROUP || VFIO_DEVICE_CDEV
> >> +	depends on !VFIO_GROUP || VFIO_CONTAINER ||
> >> IOMMUFD_VFIO_CONTAINER
> >> +	depends on !VFIO_DEVICE_CDEV || !GENERIC_ATOMIC64  
> >
> > Does this disable VFIO_NOIOMMU completely for legacy group users on
> > architectures using generic atomic64 implementations?
> >
> > On architectures like 32-bit ARM or x86, !GENERIC_ATOMIC64 evaluates
> > to false.  If a distribution enables VFIO_DEVICE_CDEV, this
> > dependency resolves to false, silently breaking backwards
> > compatibility and depriving legacy group-based users of noiommu
> > support.  
> 
> That's true, the question is do we care (I'd prefer to) and if so,
> should we block the relevant interfaces from working though iommufd
> rather than disallowing the Kconfig option.
> 
Yes, I think we should preserve the legacy VFIO_GROUP/VFIO_CONTAINER
noiommu path and only disable the IOMMUFD/cdev noiommu support on
GENERIC_ATOMIC64 platforms.
How about:
diff --git a/drivers/vfio/Kconfig b/drivers/vfio/Kconfig
index d3d8fef2855c..b9d6e1c22aed 100644
--- a/drivers/vfio/Kconfig
+++ b/drivers/vfio/Kconfig
@@ -61,10 +61,9 @@ endif
 
 config VFIO_NOIOMMU
        bool "VFIO No-IOMMU support"
-       depends on VFIO_GROUP || VFIO_DEVICE_CDEV
+       depends on VFIO_GROUP || (VFIO_DEVICE_CDEV && !GENERIC_ATOMIC64)
        depends on !VFIO_GROUP || VFIO_CONTAINER || IOMMUFD_VFIO_CONTAINER
-       depends on !VFIO_DEVICE_CDEV || !GENERIC_ATOMIC64
-       select IOMMUFD_NOIOMMU if VFIO_DEVICE_CDEV
+       select IOMMUFD_NOIOMMU if VFIO_DEVICE_CDEV && !GENERIC_ATOMIC64

With this, VFIO_NOIOMMU remains selectable for legacy group/container users
on GENERIC_ATOMIC64 architectures, e.g. on ARM I can have
CONFIG_VFIO_DEVICE_CDEV=y
CONFIG_VFIO_GROUP=y
CONFIG_VFIO_CONTAINER=y
CONFIG_VFIO_IOMMU_TYPE1=y
CONFIG_VFIO_NOIOMMU=y
CONFIG_GENERIC_ATOMIC64=y

Also, gate this on iommufd:
 static int vfio_device_set_noiommu_and_name(struct vfio_device *device, enum vfio_group_type type)
 {
-       if (IS_ENABLED(CONFIG_VFIO_DEVICE_CDEV) && vfio_noiommu &&
+       if (IS_ENABLED(CONFIG_IOMMUFD_NOIOMMU) && vfio_noiommu &&

> The next issue regarding classifying emulated IOMMU devices as
> no-iommu also appears valid, mdev devices like kvmgt for example.
> 
I will fix this by adding vfio_group_type check.
-static int vfio_device_set_noiommu_and_name(struct vfio_device *device)
+static int vfio_device_set_noiommu_and_name(struct vfio_device *device, enum vfio_group_type type)
 {
-       if (IS_ENABLED(CONFIG_VFIO_DEVICE_CDEV) && vfio_noiommu && !device->dev->iommu) {
+       if (IS_ENABLED(CONFIG_VFIO_DEVICE_CDEV) && vfio_noiommu &&
+           !device->dev->iommu && type == VFIO_IOMMU) {

> The next issue raises a valid concern whether the dev_warn() should be
> a dev_warn_once().
> 
I think dev_warn() is appropriate here, matching the existing group
path. The warning is per device per path. Using dev_warn_once() would
suppress warnings for later devices at the same callsite, which would
hide which devices were exposed through the unsafe noiommu path.
 
For example, with both group and cdev enabled today we get:
 
vfio-pci 0000:01:00.0: Adding kernel taint for vfio-noiommu group on
device
vfio-pci 0000:01:00.0: Adding kernel taint for vfio-noiommu cdev
on device

> The sysfs naming comment is invalid, we intentionally name noiommu
> devices uniquely to force userspace opt-in.
> 
> In patch 6, adding dead code is a valid comment, the unchecked
> asprintf does seem to be an outlier in selftest code, the unmap
> comment may be a false positive, as is the hugepage size, but the
> masked return comments could arguably be skips or asserts.  There are
> potentially a couple remaining nits and style issues noted.
> 
yeah, I will fix the test code, bsed on David's feedback also.

> Overall, more signal than noise afaict.  Thanks,
> 
> Alex