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: ccc21213f013 ("iommu: Add calls for IOMMU_DEBUG_PAGEALLOC")
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Pranjal Shrivastava <praan@google.com>
Reviewed-by: Samiullah Khawaja <skhawaja@google.com>
Reviewed-by: Mostafa Saleh <smostafa@google.com>
Tested-by: Josua Mayer <josua@solid-run.com>
Signed-off-by: Joerg Roedel <joerg.roedel@amd.com>

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index c21d1e3c4876bfd4f9a89698265ca1557044109c..d1a9e713d3a05f659a577ec2e9e5105086efc1b6 100644 (file)
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -2615,12 +2615,11 @@ out_set_count:
 
 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;
-       }
+                       return ret;
 
-       /* 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);
-               if (ret) {
-                       iommu_unmap(domain, iova, mapped);
-                       return ret;
-               }
-       } else {
+       else
                ret = __iommu_map_domain_pgtbl(domain, iova, paddr, size, prot,
-                                              gfp);
-               if (ret)
-                       return ret;
-       }
+                                              gfp, &mapped);
 
-       trace_map(iova, paddr, size);
-       iommu_debug_map(domain, paddr, size);
+       trace_map(iova, paddr, mapped);
+       iommu_debug_map(domain, paddr, mapped);
+       if (ret) {
+               iommu_unmap(domain, iova, mapped);
+               return ret;
+       }
        return 0;
 }