RE: [PATCH v4 09/17] iommufd: Data structure to provide IOVA to PFN mapping

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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?




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux