[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