[PATCH] drm/etnaviv: remove BUG_ON in MMU unmap path

Lucas Stach l.stach at pengutronix.de
Fri Apr 29 08:37:49 UTC 2016


Am Donnerstag, den 28.04.2016, 15:37 +0100 schrieb Russell King - ARM
Linux:
> On Thu, Apr 28, 2016 at 04:04:58PM +0200, Lucas Stach wrote:
> > The observation was that the common code in iommu_map() rightfully
> > rejected to map things, as mapping something unaligned to the page size
> > is totally bogus.
> 
> Shouldn't iommu_map() detect this?
> 
>         /*
>          * both the virtual address and the physical one, as well as
>          * the size of the mapping, must be aligned (at least) to the
>          * size of the smallest page supported by the hardware
>          */
>         if (!IS_ALIGNED(iova | paddr | size, min_pagesz)) {
>                 pr_err("unaligned: iova 0x%lx pa %pa size 0x%zx min_pagesz 0x%x\n",
>                        iova, &paddr, size, min_pagesz);
>                 return -EINVAL;
>         }
> 
> where min_pagesz is derived from:
> 
>                 .pgsize_bitmap = SZ_4K,
> 
> and will be 4096.  So, iommu_map() should reject this, and
> etnaviv_iommu_map() will tear down the partially created mapping, and
> propagate the error code to its caller, that being etnaviv_iommu_map_gem().
> 
> etnaviv_iommu_map_gem() will remove the drm_mm node, propagating the
> failure up to etnaviv_gem_mapping_get(), which will free the
> etnaviv_vram_mapping structure.
> 
> I fail to see how we could get into etnaviv_iommu_unmap() with a bad
> mapping, other than if there's memory corruption, and if there is
> memory corruption, hanging the kernel with a BUG_ON() is totally the
> right thing to do.  Better that than a corrupted filesystem.
> 
Right, if something goes wrong in the unmap() path, something is screwed
with kernel internal state. I'll drop the patch.

Thanks,
Lucas



More information about the dri-devel mailing list