Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1243492 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v4 3/6] iommu: add ARM short descriptor page table  allocator. Will Deacon <will.deacon@arm.com> - 2015-10-09 18:00 +0200
    Re: [PATCH v4 3/6] iommu: add ARM short descriptor page table  allocator. Robin Murphy <robin.murphy@arm.com> - 2015-10-09 19:50 +0200
      Re: [PATCH v4 3/6] iommu: add ARM short descriptor page table  allocator. Will Deacon <will.deacon@arm.com> - 2015-10-09 20:20 +0200

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

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?

Will
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1243555

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.

Robin.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1243579

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web