On Mon, Nov 28, 2022 at 06:53:12PM +0100, Eric Auger wrote:
> > +static int iommufd_vfio_map_dma(struct iommufd_ctx *ictx, unsigned int cmd,
> > + void __user *arg)
> > +{
> > + u32 supported_flags = VFIO_DMA_MAP_FLAG_READ | VFIO_DMA_MAP_FLAG_WRITE;
> > + size_t minsz = offsetofend(struct vfio_iommu_type1_dma_map, size);
> > + struct vfio_iommu_type1_dma_map map;
> > + int iommu_prot = IOMMU_CACHE;
> > + struct iommufd_ioas *ioas;
> > + unsigned long iova;
> > + int rc;
> > +
> > + if (copy_from_user(&map, arg, minsz))
> > + return -EFAULT;
> > +
> > + if (map.argsz < minsz || map.flags & ~supported_flags)
> > + return -EINVAL;
> > +
> > + if (map.flags & VFIO_DMA_MAP_FLAG_READ)
> > + iommu_prot |= IOMMU_READ;
> > + if (map.flags & VFIO_DMA_MAP_FLAG_WRITE)
> > + iommu_prot |= IOMMU_WRITE;
> > +
> > + ioas = get_compat_ioas(ictx);
> > + if (IS_ERR(ioas))
> > + return PTR_ERR(ioas);
> > +
> > + /*
> > + * Maps created through the legacy interface always use VFIO compatible
> > + * rlimit accounting. If the user wishes to use the faster user based
> > + * rlimit accounting then they must use the new interface.
> s/they/he
"they" has become a common neutral singular pronoun in English.
> > +static int iommufd_vfio_unmap_dma(struct iommufd_ctx *ictx, unsigned int cmd,
> > + void __user *arg)
> > +{
> > + size_t minsz = offsetofend(struct vfio_iommu_type1_dma_unmap, size);
> > + /*
> > + * VFIO_DMA_UNMAP_FLAG_GET_DIRTY_BITMAP is obsoleted by the new
> > + * dirty tracking direction:
> > + * https://lore.kernel.org/kvm/20220731125503.142683-1-yishaih@xxxxxxxxxx/
> > + * https://lore.kernel.org/kvm/20220428210933.3583-1-joao.m.martins@xxxxxxxxxx/
> > + */
> > + u32 supported_flags = VFIO_DMA_UNMAP_FLAG_ALL;
> > + struct vfio_iommu_type1_dma_unmap unmap;
> > + unsigned long unmapped = 0;
> > + struct iommufd_ioas *ioas;
> > + int rc;
> > +
> > + if (copy_from_user(&unmap, arg, minsz))
> > + return -EFAULT;
> > +
> > + if (unmap.argsz < minsz || unmap.flags & ~supported_flags)
> > + return -EINVAL;
> > +
> > + ioas = get_compat_ioas(ictx);
> > + if (IS_ERR(ioas))
> > + return PTR_ERR(ioas);
> > +
> > + if (unmap.flags & VFIO_DMA_UNMAP_FLAG_ALL) {
> > + if (unmap.iova != 0 || unmap.size != 0) {
> > + rc = -EINVAL;
> > + goto err_put;
> > + }
> > + rc = iopt_unmap_all(&ioas->iopt, &unmapped);
> > + } else {
> > + if (READ_ONCE(ioas->iopt.disable_large_pages)) {
> > + unsigned long iovas[] = { unmap.iova + unmap.size - 1,
> > + unmap.iova - 1 };
> > +
> > + rc = iopt_cut_iova(&ioas->iopt, iovas,
> > + unmap.iova ? 2 : 1);
> please can you add a comment to explain what this is supposed to do?
iova -1 when iova == 0 will underflow and becomes garbage
/*
* Create cuts at the start and last of the requested
* range. If the start IOVA is 0 then it doesn't need to
* be cut.
*/
> > +static int iommufd_vfio_set_iommu(struct iommufd_ctx *ictx, unsigned long type)
> > +{
> > + struct iommufd_ioas *ioas = NULL;
> > + int rc = 0;
> > +
> > + if (type != VFIO_TYPE1_IOMMU && type != VFIO_TYPE1v2_IOMMU)
> > + return -EINVAL;
> > +
> > + /* VFIO fails the set_iommu if there is no group */
> > + ioas = get_compat_ioas(ictx);
> > + if (IS_ERR(ioas))
> > + return PTR_ERR(ioas);
> > + if (type == VFIO_TYPE1_IOMMU)
> > + rc = iopt_disable_large_pages(&ioas->iopt);
> please can you document/explain this setting?
/*
* The difference between TYPE1 and TYPE1v2 is the ability to unmap in
* the middle of mapped ranges. This is complicated by huge page support
* which creates single large IOPTEs that cannot be split by the iommu
* driver. TYPE1 is very old at this point and likely nothing uses it,
* however it is simple enough to emulate by simply disabling the
* problematic large IOPTEs. Then we can safely unmap within any range.
*/
Thanks,
Jason
