>>> On 23.08.18 at 11:47, <paul.durr...@citrix.com> wrote: > +static int iommuop_map(struct xen_iommu_op_map *op) > +{ > + struct domain *d, *currd = current->domain; > + struct domain_iommu *iommu = dom_iommu(currd); > + bool readonly = op->flags & XEN_IOMMUOP_map_readonly; > + bfn_t bfn = _bfn(op->bfn); > + struct page_info *page; > + unsigned int prot; > + int rc, ignore; > + > + if ( op->pad || > + (op->flags & ~(XEN_IOMMUOP_map_all | > + XEN_IOMMUOP_map_readonly)) ) > + return -EINVAL; > + > + if ( !iommu->iommu_op_ranges ) > + return -EOPNOTSUPP; > + > + /* Per-device mapping not yet supported */ > + if ( !(op->flags & XEN_IOMMUOP_map_all) ) > + return -EINVAL; > + > + /* Check whether the specified BFN falls in a reserved region */ > + if ( rangeset_contains_singleton(iommu->reserved_ranges, bfn_x(bfn)) ) > + return -EINVAL; > + > + d = rcu_lock_domain_by_any_id(op->domid); > + if ( !d ) > + return -ESRCH; > + > + rc = get_paged_gfn(d, _gfn(op->gfn), readonly, NULL, &page); > + if ( rc ) > + goto unlock; > + > + rc = -EINVAL; > + if ( !readonly && !get_page_type(page, PGT_writable_page) ) > + { > + put_page(page); > + goto unlock; > + } > + > + prot = IOMMUF_readable; > + if ( !readonly ) > + prot |= IOMMUF_writable; > + > + rc = -EIO; > + if ( iommu_map_page(currd, bfn, page_to_mfn(page), prot) ) > + goto release; > + > + rc = rangeset_add_singleton(iommu->iommu_op_ranges, bfn_x(bfn)); > + if ( rc ) > + goto unmap;
When a mapping is requested for the same BFN that a prior mapping was already established for, the page refs of that prior mapping get leaked here. I don't think you want to require an intermediate unmap, so checking the rangeset first is not an option. Hence I think you need to look up the translation anyway, which may mean that the rangeset's usefulness is quite limited (relevant with the additional context of my question regarding it perhaps requiring a pretty much unbounded amount of memory). In order to avoid shooting down all pre-existing RAM mappings - is there no way the page table entries could be marked to identify their origin? I also have another more general concern: Allowing the guest to manipulate its IOMMU page tables means that it can deliberately shatter large pages, growing the overall memory footprint of the domain. I'm hesitant to say this, but I'm afraid that resource tracking of such "behind the scenes" allocations might be a necessary prereq for the PV IOMMU work. Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel