Re: [PATCH V4 04/10] iommufd: Add an ioctl IOMMU_IOAS_GET_PA to query PA from IOVA

Jacob Pan
2026-05-04T23:03:49Z

Hi Kevin,

On Thu, 16 Apr 2026 08:02:33 +0000
"Tian, Kevin" <kevin.tian@intel.com> wrote:

> > From: Jacob Pan <jacob.pan@linux.microsoft.com>
> > Sent: Wednesday, April 15, 2026 5:14 AM
> >
> > To support no-IOMMU mode where userspace drivers perform unsafe DMA
> > using physical addresses, introduce a new API to retrieve the
> > physical address of a user-allocated DMA buffer that has been
> > mapped to an IOVA via IOAS. The mapping is backed by mock I/O page
> > tables maintained
> > by generic IOMMUPT framework.
> >
> > Suggested-by: Jason Gunthorpe <jgg@nvidia.com>
> > Signed-off-by: Jacob Pan <jacob.pan@linux.microsoft.com>
> > Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
>
> Your s-o-b should be the last
will do.

>
> > + /*
> > + * Scan the domain for the contiguous physical address
> > length so that
> > + * userspace search can be optimized for fewer ioctls.
> > + */
> > + while (iova < iopt_area_last_iova(area)) {
> > + unsigned long next_iova;
> > + u64 next_paddr;
> > +
> > + if (check_add_overflow(iova, PAGE_SIZE,
> > &next_iova))
> > + break;
> > +
> > + next_paddr =
> > iommu_iova_to_phys(area->storage_domain, next_iova);
> > +
> > + if (!next_paddr || next_paddr != tmp_paddr +
> > PAGE_SIZE)
> > + break;
>
> the 2nd condition should be WARN_ON?
I don't think it should be a WARN_ON since PA non-contiguity is one of
the normal exit conditions.

>
> > @@ -432,6 +432,7 @@ union ucmd_buffer {
> > struct iommu_veventq_alloc veventq;
> > struct iommu_vfio_ioas vfio_ioas;
> > struct iommu_viommu_alloc viommu;
> > + struct iommu_ioas_get_pa get_pa;
>
> alphabetic order
>
will do.

> > #ifdef CONFIG_IOMMUFD_TEST
> > struct iommu_test_cmd test;
> > #endif
> > @@ -484,6 +485,8 @@ static const struct iommufd_ioctl_op
> > iommufd_ioctl_ops[] = {
> > struct iommu_ioas_map_file, iova),
> > IOCTL_OP(IOMMU_IOAS_UNMAP, iommufd_ioas_unmap, struct
> > iommu_ioas_unmap,
> > length),
> > + IOCTL_OP(IOMMU_IOAS_GET_PA, iommufd_ioas_get_pa, struct
> > iommu_ioas_get_pa,
> > + out_phys),
>
> ditto
will do

> >
> > +/**
> > + * struct iommu_ioas_get_pa - ioctl(IOMMU_IOAS_GET_PA)
> > + * @size: sizeof(struct iommu_ioas_get_pa)
> > + * @flags: Reserved, must be 0 for now
> > + * @ioas_id: IOAS ID to query IOVA to PA mapping from
> > + * @__reserved: Must be 0
> > + * @iova: IOVA to query
> > + * @out_length: Number of bytes contiguous physical address
> > starting from phys
> > + * @out_phys: Output physical address the IOVA maps to
> > + *
> > + * Query the physical address backing an IOVA range. The entire
> > range must be
> > + * mapped already. For noiommu devices doing unsafe DMA only.
> > + */
> > +struct iommu_ioas_get_pa {
> > + __u32 size;
> > + __u32 flags;
> > + __u32 ioas_id;
> > + __u32 __reserved;
> > + __aligned_u64 iova;
> > + __aligned_u64 out_length;
> > + __aligned_u64 out_phys;
> > +};
> > +#define IOMMU_IOAS_GET_PA _IO(IOMMUFD_TYPE,
> > IOMMUFD_CMD_IOAS_GET_PA)
> > +
>
> Based on its purpose let's make the restriction in the name, e.g.
>
> IOMMUFD_CMD_IOAS_NOIOMMU_GET_PA

yeah, make sense. will rename all the function names as well.