On Mon, Jun 16, 2025 at 11:13:19AM -0300, Jason Gunthorpe wrote:
On Sat, Jun 14, 2025 at 12:14:38AM -0700, Nicolin Chen wrote:
+/* Entry for iommufd_ctx::mt_mmap */ +struct iommufd_mmap {
- struct iommufd_object *owner;
- /* Allocated start position in mt_mmap tree */
- unsigned long startp;
pgoff_t, looks like this is already in PAGE_SIZE units.
- /* Physical range for io_remap_pfn_range() */
- unsigned long mmio_pfn;
physaddr_t and maybe don't use pfn?
- unsigned long num_pfns;
size_t
I made the following changes, though I kept "unsigned long" for "vm_pgoff" aligning with vma->vm_pgoff:
diff --git a/drivers/iommu/iommufd/driver.c b/drivers/iommu/iommufd/driver.c index d797b77e878c6..73fcf3a89bf82 100644 --- a/drivers/iommu/iommufd/driver.c +++ b/drivers/iommu/iommufd/driver.c @@ -53,19 +53,19 @@ int _iommufd_alloc_mmap(struct iommufd_ctx *ictx, struct iommufd_object *owner, if (!immap) return -ENOMEM; immap->owner = owner; - immap->num_pfns = length >> PAGE_SHIFT; - immap->mmio_pfn = mmio_addr >> PAGE_SHIFT; + immap->length = length; + immap->mmio_addr = mmio_addr;
- rc = mtree_alloc_range(&ictx->mt_mmap, &startp, immap, immap->num_pfns, - 0, U32_MAX >> PAGE_SHIFT, GFP_KERNEL); + rc = mtree_alloc_range(&ictx->mt_mmap, &startp, immap, immap->length, + 0, PHYS_ADDR_MAX, GFP_KERNEL); if (rc < 0) { kfree(immap); return rc; }
- immap->startp = startp; - /* mmap() syscall will right-shift the offset in vma->vm_pgoff */ - *offset = startp << PAGE_SHIFT; + /* mmap() syscall will right-shift the offset in vma->vm_pgoff too */ + immap->vm_pgoff = startp >> PAGE_SHIFT; + *offset = startp; return 0; } EXPORT_SYMBOL_NS_GPL(_iommufd_alloc_mmap, "IOMMUFD"); diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index cc9088ba5c812..ebac6a4b3538c 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -61,12 +61,12 @@ struct iommufd_ctx { struct iommufd_mmap { struct iommufd_object *owner;
- /* Allocated start position in mt_mmap tree */ - unsigned long startp; + /* Page-shifted start position in mt_mmap to validate vma->vm_pgoff */ + unsigned long vm_pgoff;
/* Physical range for io_remap_pfn_range() */ - unsigned long mmio_pfn; - unsigned long num_pfns; + phys_addr_t mmio_addr; + size_t length; };
/* diff --git a/drivers/iommu/iommufd/main.c b/drivers/iommu/iommufd/main.c index 339a269ebbc81..0fb81a905cb13 100644 --- a/drivers/iommu/iommufd/main.c +++ b/drivers/iommu/iommufd/main.c @@ -514,17 +514,15 @@ static int iommufd_fops_mmap(struct file *filp, struct vm_area_struct *vma) if (vma->vm_flags & VM_EXEC) return -EPERM;
- /* vma->vm_pgoff carries an index to an mtree entry (immap) */ - immap = mtree_load(&ictx->mt_mmap, vma->vm_pgoff); + /* vma->vm_pgoff carries a page-shifted start position to an immap */ + immap = mtree_load(&ictx->mt_mmap, vma->vm_pgoff << PAGE_SHIFT); if (!immap) return -ENXIO; /* - * mtree_load() returns the immap for any contained pgoff, only allow - * the immap thing to be mapped + * mtree_load() returns the immap for any contained mmio_addr, so only + * allow the exact immap thing to be mapped */ - if (vma->vm_pgoff != immap->startp) - return -ENXIO; - if (length != immap->num_pfns << PAGE_SHIFT) + if (vma->vm_pgoff != immap->vm_pgoff || length != immap->length) return -ENXIO;
vma->vm_pgoff = 0; @@ -532,7 +530,8 @@ static int iommufd_fops_mmap(struct file *filp, struct vm_area_struct *vma) vma->vm_ops = &iommufd_vma_ops; vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
- rc = io_remap_pfn_range(vma, vma->vm_start, immap->mmio_pfn, length, + rc = io_remap_pfn_range(vma, vma->vm_start, + immap->mmio_addr >> PAGE_SHIFT, length, vma->vm_page_prot); if (rc) return rc;