Re: [PATCH v5 2/9] iommufd: Support a HWPT without an iommu driver for noiommu

[Jacob Pan]

Hi Baolu,

On Wed, 13 May 2026 14:58:40 +0800
Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx> wrote:

> On 5/12/26 02:41, Jacob Pan wrote:
> > From: Jason Gunthorpe<jgg@xxxxxxxxxx>
> > 
> > Create just a little part of a real iommu driver, enough to
> > slot in under the dev_iommu_ops() and allow iommufd to call
> > domain_alloc_paging_flags() and fail everything else.
> > 
> > This allows explicitly creating a HWPT under an IOAS.
> > 
> > A new Kconfig option IOMMUFD_NOIOMMU is introduced to differentiate
> > from the VFIO group/container based noiommu mode.
> > 
> > Signed-off-by: Jason Gunthorpe<jgg@xxxxxxxxxx>
> > Signed-off-by: Jacob Pan<jacob.pan@xxxxxxxxxxxxxxxxxxx>
> > ---
> > v5:
> >     - Use the new IOMMUFD_NOIOMMU Kconfig instead of VFIO_NOIOMMU
> >     - Use consistent wording referring to VFIO noiommu mode (Kevin)
> >     - Copyright date fix (Kevin)
> > v4:
> >     - Make iommufd_noiommu_ops const
> > v3:
> >     - Add comment to explain the design difference over the
> >       legacy noiommu VFIO code.
> > ---
> >   drivers/iommu/iommufd/Kconfig           |  13 +++
> >   drivers/iommu/iommufd/Makefile          |   1 +
> >   drivers/iommu/iommufd/hw_pagetable.c    |  15 +++-
> >   drivers/iommu/iommufd/hwpt_noiommu.c    | 102
> > ++++++++++++++++++++++++ drivers/iommu/iommufd/iommufd_private.h |
> >  2 + 5 files changed, 131 insertions(+), 2 deletions(-)
> >   create mode 100644 drivers/iommu/iommufd/hwpt_noiommu.c
> > 
> > diff --git a/drivers/iommu/iommufd/Kconfig
> > b/drivers/iommu/iommufd/Kconfig index 455bac0351f2..74d6ea5b5b3b
> > 100644 --- a/drivers/iommu/iommufd/Kconfig
> > +++ b/drivers/iommu/iommufd/Kconfig
> > @@ -16,6 +16,19 @@ config IOMMUFD
> >   	  If you don't know what to do here, say N.
> >   
> >   if IOMMUFD
> > +config IOMMUFD_NOIOMMU
> > +	bool
> > +	depends on !GENERIC_ATOMIC64 # IOMMU_PT_AMDV1 requires
> > cmpxchg64
> > +	select GENERIC_PT
> > +	select IOMMU_PT
> > +	select IOMMU_PT_AMDV1
> > +	help
> > +	  Provides a SW-only IO page table for devices without
> > hardware
> > +	  IOMMU backing. This uses the AMDV1 page table format for
> > +	  IOVA-to-PA lookups only, not for hardware DMA
> > translation. +
> > +	  Selected by VFIO_CDEV_NOIOMMU. Not intended to be
> > enabled directly. +
> >   config IOMMUFD_VFIO_CONTAINER
> >   	bool "IOMMUFD provides the VFIO container /dev/vfio/vfio"
> >   	depends on VFIO_GROUP && !VFIO_CONTAINER
> > diff --git a/drivers/iommu/iommufd/Makefile
> > b/drivers/iommu/iommufd/Makefile index 71d692c9a8f4..67207914bb6e
> > 100644 --- a/drivers/iommu/iommufd/Makefile
> > +++ b/drivers/iommu/iommufd/Makefile
> > @@ -10,6 +10,7 @@ iommufd-y := \
> >   	vfio_compat.o \
> >   	viommu.o
> >   
> > +iommufd-$(CONFIG_IOMMUFD_NOIOMMU) += hwpt_noiommu.o
> >   iommufd-$(CONFIG_IOMMUFD_TEST) += selftest.o
> >   
> >   obj-$(CONFIG_IOMMUFD) += iommufd.o
> > diff --git a/drivers/iommu/iommufd/hw_pagetable.c
> > b/drivers/iommu/iommufd/hw_pagetable.c index
> > fe789c2dc0c9..0ae14cd3fc72 100644 ---
> > a/drivers/iommu/iommufd/hw_pagetable.c +++
> > b/drivers/iommu/iommufd/hw_pagetable.c @@ -8,6 +8,15 @@
> >   #include "../iommu-priv.h"
> >   #include "iommufd_private.h"
> >   
> > +static const struct iommu_ops *get_iommu_ops(struct iommufd_device
> > *idev) +{
> > +	if (IS_ENABLED(CONFIG_IOMMUFD_NOIOMMU) &&
> > !idev->igroup->group)
> > +		return &iommufd_noiommu_ops;
> > +	if (WARN_ON_ONCE(!idev->dev->iommu))
> > +		return NULL;
> > +	return dev_iommu_ops(idev->dev);
> > +}
> > +
> >   static void __iommufd_hwpt_destroy(struct iommufd_hw_pagetable
> > *hwpt) {
> >   	if (hwpt->domain)
> > @@ -114,11 +123,13 @@ iommufd_hwpt_paging_alloc(struct iommufd_ctx
> > *ictx, struct iommufd_ioas *ioas, IOMMU_HWPT_ALLOC_DIRTY_TRACKING |
> >   				IOMMU_HWPT_FAULT_ID_VALID |
> >   				IOMMU_HWPT_ALLOC_PASID;
> > -	const struct iommu_ops *ops = dev_iommu_ops(idev->dev);
> > +	const struct iommu_ops *ops = get_iommu_ops(idev);
> >   	struct iommufd_hwpt_paging *hwpt_paging;
> >   	struct iommufd_hw_pagetable *hwpt;
> >   	int rc;
> >   
> > +	if (!ops)
> > +		return ERR_PTR(-ENODEV);  
> 
> Nit: it's unnecessary to add this check. get_iommu_ops() will WARN if
> ops is NULL. The previous code assumes that ops is always valid and
> does not return here. If you want to change this behavior, it’s
> better to explain the reasoning in the commit message.
> 
I am fine with removing it, just extra defensive to prevent null
pointer deref.

> Otherwise it looks good to me.
> 
> Reviewed-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> 
> Thanks,
> baolu

Thanks,

Jacob