[PATCH v4 3/6] iommu: add ARM short descriptor page table allocator.

[Will Deacon]

On Fri, Oct 09, 2015 at 06:41:51PM +0100, Robin Murphy wrote:
> On 09/10/15 16:57, Will Deacon wrote:
> >On Tue, Sep 22, 2015 at 03:12:47PM +0100, Yong Wu wrote:
> >>      I would like to show you a problem I met, The recursion here may
> >>lead to stack overflow while we test FHD video decode.
> >>
> >>     From the log, I get the internal variable in the error case: the
> >>"size" is 0x100000, the "iova" is 0xfea00000, but at that time the
> >>"blk_size" is 0x1000 as it was the map of small-page. so it enter the
> >>recursion here.
> >>
> >>     After check the unmap flow, there is only a iommu_unmap in
> >>__iommu_dma_unmap, and it won't check the physical address align in
> >>iommu_unmap.
> >
> >That sounds like a bug in __iommu_dma_unmap. Robin?
> 
> Isn't it just cf27ec930be9 again wearing different trousers? All I do is
> call iommu_unmap with the same total size that was mapped originally.

I don't think it's the same as that issue, which was to do with installing
block mappings over the top of an existing table entry. The problem here
seems to be that we don't walk the page table properly on unmap.

The long descriptor code has:

	/* If the size matches this level, we're in the right place */
	if (size == blk_size) {
		__arm_lpae_set_pte(ptep, 0, &data->iop.cfg);

		if (!iopte_leaf(pte, lvl)) {
			/* Also flush any partial walks */
			tlb->tlb_add_flush(iova, size, false, cookie);
			tlb->tlb_sync(cookie);
			ptep = iopte_deref(pte, data);
			__arm_lpae_free_pgtable(data, lvl + 1, ptep);
		} else {
			tlb->tlb_add_flush(iova, size, true, cookie);
		}

		return size;
	} else if (iopte_leaf(pte, lvl)) {
		/*
		 * Insert a table at the next level to map the old region,
		 * minus the part we want to unmap
		 */
		return arm_lpae_split_blk_unmap(data, iova, size,
						iopte_prot(pte), lvl, ptep,
						blk_size);
	}

why doesn't something similar work for short descriptors?

Will