> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Tuesday, November 8, 2022 8:49 AM
>
> +
> +/*
> + * Automatically find a block of IOVA that is not being used and not reserved.
> + * Does not return a 0 IOVA even if it is valid.
what is the problem with 0? should this be documented in uAPI?
> + interval_tree_for_each_span(&allowed_span, &iopt->allowed_itree,
> + PAGE_SIZE, ULONG_MAX - PAGE_SIZE) {
> + if (RB_EMPTY_ROOT(&iopt->allowed_itree.rb_root)) {
> + allowed_span.start_used = PAGE_SIZE;
> + allowed_span.last_used = ULONG_MAX - PAGE_SIZE;
> + allowed_span.is_hole = false;
> + }
statically initialize it when iopt is created?
> +
> + if (!__alloc_iova_check_used(&allowed_span, length,
> + iova_alignment, page_offset))
> + continue;
> +
> + interval_tree_for_each_span(&area_span, &iopt->area_itree,
> + allowed_span.start_used,
> + allowed_span.last_used) {
> + if (!__alloc_iova_check_hole(&area_span, length,
> + iova_alignment,
> + page_offset))
> + continue;
> +
> + interval_tree_for_each_span(&reserved_span,
> + &iopt->reserved_itree,
> + area_span.start_used,
> + area_span.last_used) {
> + if (!__alloc_iova_check_hole(
> + &reserved_span, length,
> + iova_alignment, page_offset))
> + continue;
this could be simplified by double span.
> +static int iopt_check_iova(struct io_pagetable *iopt, unsigned long iova,
> + unsigned long length)
> +{
> + unsigned long last;
> +
> + lockdep_assert_held(&iopt->iova_rwsem);
> +
> + if ((iova & (iopt->iova_alignment - 1)))
> + return -EINVAL;
> +
> + if (check_add_overflow(iova, length - 1, &last))
> + return -EOVERFLOW;
> +
> + /* No reserved IOVA intersects the range */
> + if (iopt_reserved_iter_first(iopt, iova, last))
> + return -ENOENT;
vfio type1 returns -EINVAL
> +
> + /* Check that there is not already a mapping in the range */
> + if (iopt_area_iter_first(iopt, iova, last))
> + return -EADDRINUSE;
vfio type1 returns -EEXIST
> +static int iopt_unmap_iova_range(struct io_pagetable *iopt, unsigned long
> start,
> + unsigned long end, unsigned long
s/end/last/
> +int iopt_unmap_iova(struct io_pagetable *iopt, unsigned long iova,
> + unsigned long length, unsigned long *unmapped)
> +{
> + unsigned long iova_end;
s/iova_end/iova_last/
> +static int iopt_calculate_iova_alignment(struct io_pagetable *iopt)
> +{
> + unsigned long new_iova_alignment;
> + struct iommufd_access *access;
> + struct iommu_domain *domain;
> + unsigned long index;
> +
> + lockdep_assert_held_write(&iopt->iova_rwsem);
> + lockdep_assert_held(&iopt->domains_rwsem);
> +
> + if (iopt->disable_large_pages)
> + new_iova_alignment = PAGE_SIZE;
> + else
> + new_iova_alignment = 1;
I didn't understand why we start searching alignment from a
smaller value when large pages is enabled. what is the
connection here?
> + interval_tree_remove(&area->node, &iopt->area_itree);
> + rc = iopt_insert_area(iopt, lhs, area->pages, start_iova,
> + iopt_area_start_byte(area, start_iova),
> + (new_start - 1) - start_iova + 1,
> + area->iommu_prot);
> + if (WARN_ON(rc))
> + goto err_insert;
> +
> + rc = iopt_insert_area(iopt, rhs, area->pages, new_start,
> + iopt_area_start_byte(area, new_start),
> + last_iova - new_start + 1, area->iommu_prot);
> + if (WARN_ON(rc))
> + goto err_remove_lhs;
> +
> + lhs->storage_domain = area->storage_domain;
> + lhs->num_accesses = area->num_accesses;
> + lhs->pages = area->pages;
> + rhs->storage_domain = area->storage_domain;
> + rhs->num_accesses = area->num_accesses;
if an access only spans one side, is it correct to have both split sides
keep the access number?
