Commit 0735c548 authored by Jason Gunthorpe's avatar Jason Gunthorpe Committed by Joerg Roedel
Browse files

iommu: Handle unmap error when iommu_debug is enabled 



Sashiko noticed a latent bug where the map error flow called iommu_unmap()
which calls iommu_debug_unmap_begin()/iommu_debug_unmap_end() however
since this is an error path the map flow never actually established the
original iommu_debug_map() it will malfunction.

Lift the unmap error handling into iommu_map_nosync() and reorder it so
the trace_map()/iommu_debug_map() records the partial mapping and then
immediately unmaps it. This avoid creating the unbalanced tracking and
provides saner tracing instead of a unmap unmatched to any map.

Fixes: ccc21213 ("iommu: Add calls for IOMMU_DEBUG_PAGEALLOC")
Signed-off-by: default avatarJason Gunthorpe <jgg@nvidia.com>
Reviewed-by: default avatarPranjal Shrivastava <praan@google.com>
Reviewed-by: default avatarSamiullah Khawaja <skhawaja@google.com>
Reviewed-by: default avatarMostafa Saleh <smostafa@google.com>
Tested-by: default avatarJosua Mayer <josua@solid-run.com>
Signed-off-by: default avatarJoerg Roedel <joerg.roedel@amd.com>
parent b948a872

drivers/iommu/iommu.c

+19 −30
Original line number Diff line number Diff line
@@ -2615,12 +2615,11 @@ static size_t iommu_pgsize(struct iommu_domain *domain, unsigned long iova, 


static int __iommu_map_domain_pgtbl(struct iommu_domain *domain,

				    unsigned long iova, phys_addr_t paddr,

				    size_t size, int prot, gfp_t gfp)

				    size_t size, int prot, gfp_t gfp,

				    size_t *mapped)

{

	const struct iommu_domain_ops *ops = domain->ops;

	unsigned long orig_iova = iova;

	unsigned int min_pagesz;

	size_t orig_size = size;

	int ret = 0;



	if (WARN_ON(!ops->map_pages))
@@ -2643,31 +2642,25 @@ static int __iommu_map_domain_pgtbl(struct iommu_domain *domain, 
	pr_debug("map: iova 0x%lx pa %pa size 0x%zx\n", iova, &paddr, size);



	while (size) {

		size_t pgsize, count, mapped = 0;

		size_t pgsize, count, op_mapped = 0;



		pgsize = iommu_pgsize(domain, iova, paddr, size, &count);



		pr_debug("mapping: iova 0x%lx pa %pa pgsize 0x%zx count %zu\n",

			 iova, &paddr, pgsize, count);

		ret = ops->map_pages(domain, iova, paddr, pgsize, count, prot,

				     gfp, &mapped);

				     gfp, &op_mapped);

		/*

		 * Some pages may have been mapped, even if an error occurred,

		 * so we should account for those so they can be unmapped.

		 */

		size -= mapped;



		*mapped += op_mapped;

		if (ret)

			break;



		iova += mapped;

		paddr += mapped;

	}



	/* unroll mapping in case something went wrong */

	if (ret) {

		iommu_unmap(domain, orig_iova, orig_size - size);

			return ret;



		size -= op_mapped;

		iova += op_mapped;

		paddr += op_mapped;

	}

	return 0;

}
@@ -2685,6 +2678,7 @@ int iommu_map_nosync(struct iommu_domain *domain, unsigned long iova, 
		phys_addr_t paddr, size_t size, int prot, gfp_t gfp)

{

	struct pt_iommu *pt = iommupt_from_domain(domain);

	size_t mapped = 0;

	int ret;



	might_sleep_if(gfpflags_allow_blocking(gfp));
@@ -2696,24 +2690,19 @@ int iommu_map_nosync(struct iommu_domain *domain, unsigned long iova, 
				 __GFP_HIGHMEM))))

		return -EINVAL;



	if (pt) {

		size_t mapped = 0;



	if (pt)

		ret = pt->ops->map_range(pt, iova, paddr, size, prot, gfp,

					 &mapped);

	else

		ret = __iommu_map_domain_pgtbl(domain, iova, paddr, size, prot,

					       gfp, &mapped);



	trace_map(iova, paddr, mapped);

	iommu_debug_map(domain, paddr, mapped);

	if (ret) {

		iommu_unmap(domain, iova, mapped);

		return ret;

	}

	} else {

		ret = __iommu_map_domain_pgtbl(domain, iova, paddr, size, prot,

					       gfp);

		if (ret)

			return ret;

	}



	trace_map(iova, paddr, size);

	iommu_debug_map(domain, paddr, size);

	return 0;

}