[PATCH] ARM: dma-mapping: Fix dma_get_sgtable() for regions without struct page

[PATCH] ARM: dma-mapping: Fix dma_get_sgtable() for regions without struct page

[Marek Szyprowski]

Some DMA regions, for example those created by dma_declare_coherent_memory,
might not be possible to be represented by struct page pointers. Getting
scatterlist for the buffer allocated from such region is not possible.
This patch adds a simple check in arm_dma_get_sgtable() function if the
exported struct page pointer is valid.

Fixes: dc2832e1e7db3 ("ARM: dma-mapping: add support for dma_get_sgtable()")
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 arch/arm/mm/dma-mapping.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index fc4a4eaa9934..4b2cf82fbca6 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -942,6 +942,10 @@ int arm_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
 	struct page *page = pfn_to_page(dma_to_pfn(dev, handle));
 	int ret;
 
+	/* Check if DMA handle has been properly translated to page. */
+	if (pfn_to_dma(dev, page_to_pfn(page)) != handle)
+		return -ENXIO;
+
 	ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
 	if (unlikely(ret))
 		return ret;
-- 
1.9.1

[PATCH] ARM: dma-mapping: Fix dma_get_sgtable() for regions without struct page

[Russell King - ARM Linux]

On Tue, Feb 21, 2017 at 02:16:38PM +0100, Marek Szyprowski wrote:
> Some DMA regions, for example those created by dma_declare_coherent_memory,
> might not be possible to be represented by struct page pointers. Getting
> scatterlist for the buffer allocated from such region is not possible.
> This patch adds a simple check in arm_dma_get_sgtable() function if the
> exported struct page pointer is valid.

I really don't like this - the "validation" here looks like total crap,
especially when you analyse what these macros expand to.

I also don't like the way dma_get_sgtable() appeared without any
documentation about (a) what it's for (b) what the expectations for its
arguments are and (c) what it's supposed to be doing.  There are no
comments in include/linux/dma-mapping.h describing the interface, nor
are there any comments about it in Documentation/DMA*.

So, I'm really not applying this patch until the purpose of this
function gets documented.

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: dma-mapping: Fix dma_get_sgtable() for regions without struct page

[Shuah Khan]

Hi Russel and Marek,

On Wed, Mar 22, 2017 at 5:10 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Tue, Feb 21, 2017 at 02:16:38PM +0100, Marek Szyprowski wrote:
>> Some DMA regions, for example those created by dma_declare_coherent_memory,
>> might not be possible to be represented by struct page pointers. Getting
>> scatterlist for the buffer allocated from such region is not possible.
>> This patch adds a simple check in arm_dma_get_sgtable() function if the
>> exported struct page pointer is valid.
>
> I really don't like this - the "validation" here looks like total crap,
> especially when you analyse what these macros expand to.
>
> I also don't like the way dma_get_sgtable() appeared without any
> documentation about (a) what it's for (b) what the expectations for its
> arguments are and (c) what it's supposed to be doing.  There are no
> comments in include/linux/dma-mapping.h describing the interface, nor
> are there any comments about it in Documentation/DMA*.

Soory for a long reply.

This check doesn't really do any kind of validation. I have been debugging
a pagefault when the sgtable created by arm_dma_get_sgtable() runs into
the following kernel error when vb2_dc_dmabuf_ops_map() tries to map
the exported buffer from another driver.

Unable to handle kernel paging request at virtual address f0004000

I tracked the buffer and page - dma_to_pfn() returns a page that is invalid
which gets into this sgtable and later on when another driver tries to map it,
it trips on this invalid sgtable entry.

[  160.717795] pgd = ece98000
[  160.720479] [f0004000] *pgd=00000000
[  160.724034] Internal error: Oops: 5 [#1] PREEMPT SMP ARM
[  160.729316] Modules linked in: cpufreq_userspace cpufreq_powersave
cpufreq_conservative exynos_gsc s5p_jpeg s5p_mfc v4l2_mem2mem
videobuf2_dma_contig v4l2_common videobuf2_memops videobuf2_v4l2
videobuf2_core videodev media
[  160.749077] CPU: 5 PID: 1979 Comm: v4l2video0dec0: Not tainted
4.10.0-rc5-00007-gfdc23c2-dirty #27
[  160.758000] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[  160.764066] task: eb852300 task.stack: eb958000
[  160.768578] PC is at page_address+0x4/0xd8
[  160.772651] LR is at vb2_dc_dmabuf_ops_map+0x140/0x220 [videobuf2_dma_contig]
[  160.779751] pc : [<c01c2bd0>]    lr : [<bf1bc7c8>]    psr: 60030013
               sp : eb959be8  ip : 00000007  fp : ed3456c0
[  160.791188] r10: 00000001  r9 : 00000001  r8 : ed3456c0
[  160.796388] r7 : c0c09a44  r6 : 00000001  r5 : 87654321  r4 : ed345ec0
[  160.802887] r3 : 00000000  r2 : 00000000  r1 : 00000001  r0 : f0004000
[  160.809387] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[  160.816492] Control: 10c5387d  Table: 6ce9806a  DAC: 00000051
[  160.822211] Process v4l2video0dec0: (pid: 1979, stack limit = 0xeb958210)
[  160.828970] Stack: (0xeb959be8 to 0xeb95a000)
[  160.833304] 9be0:                   ed345ec0 87654321 00000001
c0c09a44 ed3456c0 00000001
[  160.841449] 9c00: 00000001 bf1bc7c8 00000001 eb959c34 ed11f314
eeab0a10 c09bb5d4 ed345000
[  160.849595] 9c20: 00000001 00000000 ed345290 00000000 bf12aa8c
00000001 00000001 c044c0f0
[  160.857740] 9c40: ed345280 ed037600 00000000 bf1bc330 00000000
00000000 ed345290 000e6040
----
[  161.102113] [<c01c2bd0>] (page_address) from [<bf1bc7c8>]
(vb2_dc_dmabuf_ops_map+0x140/0x220 [videobuf2_dma_contig])
[  161.112602] [<bf1bc7c8>] (vb2_dc_dmabuf_ops_map
[videobuf2_dma_contig]) from [<c044c0f0>]
(dma_buf_map_attachment+0x44/0x68)
[  161.123774] [<c044c0f0>] (dma_buf_map_attachment) from [<bf1bc330>]
(vb2_dc_map_dmabuf+0x64/0x16c [videobuf2_dma_contig])
[  161.134697] [<bf1bc330>] (vb2_dc_map_dmabuf [videobuf2_dma_contig])
from [<bf1278c8>] (__qbuf_dmabuf+0x3c4/0x4c8 [videobuf2_core])
[  161.146396] [<bf1278c8>] (__qbuf_dmabuf [videobuf2_core]) from
[<bf127afc>] (__buf_prepare+0x130/0x16c [videobuf2_core])
[  161.157226] [<bf127afc>] (__buf_prepare [videobuf2_core]) from
[<bf127d3c>] (vb2_core_qbuf+0x128/0x204 [videobuf2_core])
[  161.168058] [<bf127d3c>] (vb2_core_qbuf [videobuf2_core]) from
[<bf1e4658>] (v4l2_m2m_qbuf+0x1c/0x34 [v4l2_mem2mem])
[  161.178564] [<bf1e4658>] (v4l2_m2m_qbuf [v4l2_mem2mem]) from
[<bf057914>] (__video_do_ioctl+0x290/0x2ec [videodev])
[  161.188972] [<bf057914>] (__video_do_ioctl [videodev]) from
[<bf057338>] (video_usercopy+0x1a0/0x4e0 [videodev])
[  161.199109] [<bf057338>] (video_usercopy [videodev]) from
[<bf0525a4>] (v4l2_ioctl+0xa0/0xd8 [videodev])
[  161.208539] [<bf0525a4>] (v4l2_ioctl [videodev]) from [<c01f991c>]
(do_vfs_ioctl+0x9c/0x8e0)
[  161.216923] [<c01f991c>] (do_vfs_ioctl) from [<c01fa194>]
(SyS_ioctl+0x34/0x5c)
[  161.224205] [<c01fa194>] (SyS_ioctl) from [<c0107740>]
(ret_fast_syscall+0x0/0x3c)
[  161.231741] Code: e0623623 e0800283 e12fff1e e92d47f0 (e5903000)
[  161.237807] ---[ end trace aa851942c99a7c54 ]---


Just an an experiment, I added this hack: in this case __dc_alloc()
returned values
is the mapped page. This is by mo means a fix - I am not sure if we can rely on
this page, since at DQBUF its get unmapped.

/* In this case cpu_addr is the page - just use it */
        if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) == 0)
                page = cpu_addr;

At this point I dumped some data:

[  154.627505] platform 11000000.codec:left:
debug_arm_dma_get_sgtable: bus_to_pfn 772608 dma_pfn_offset 0
PHYS_PFN_OFFSET 262144
[  154.627512] platform 11000000.codec:left:
debug_arm_dma_get_sgtable: dma_to_pfn 772608 virt_to_pfn 470272
virt_to_dma 1926234112 virt_to_page ef6ca000 page f0004000

cpu_addr is
[  154.627520] platform 11000000.codec:left:
debug_arm_dma_get_sgtable: cpu_addr f2d00000 handle bca00000 page =
f2d00000 size 946176 align size 946176
[  154.627527] platform 11000000.codec:left: after
dma_get_sgtable_attrs() vb2_dc_get_base_sgt: sgt ecdcec00 page
f2d00000 buf ecdced80 cookie f2d00000 vaddr f2d00000 dc_cookie
ecdced90 dma_addr bca00000 size 946176

cpu_addr is f2d00000 and the page you get from
pfn_to_page(dma_to_pfn(dev, handle)) is f0004000.

The page you get with struct page *page = pfn_to_page(dma_to_pfn(dev, handle));
is bogus. So this check Merek added doesn't find a bogus page, does a reverse
validation of the bogus page.

I also generated page tables and I do find this page f2d00000 it in
there. For some reason
dma_to_pfn(dev, handle) fails baldy at this stage which trips the kernel later.

I can help debug this further and find a fix - I have a use-case that
can reproduces on every single run. I am suing 4.10-rc5

With the hack to make page =  cpu_addr, it goes further as the page is
good and then runs into an error when an attempt is made to clean
D-cache.

[  154.687637] Unable to handle kernel paging request at virtual
address a4800000
[  154.693445] pgd = ed880000
[  154.696054] [a4800000] *pgd=00000000
[  154.699611] Internal error: Oops: 2805 [#1] PREEMPT SMP ARM
[  154.705153] Modules linked in: cpufreq_userspace cpufreq_powersave
cpufreq_conservative s5p_mfc s5p_jpeg exynos_gsc v4l2_mem2mem
videobuf2_dma_contig v4l2_common videobuf2_memops videobuf2_v4l2
videobuf2_core videodev media
[  154.724914] CPU: 3 PID: 1901 Comm: v4l2video6dec0: Not tainted
4.10.0-rc5-00007-gfdc23c2-dirty #28
[  154.733835] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[  154.739902] task: eda6cb00 task.stack: eb844000
[  154.744414] PC is at v7_dma_clean_range+0x1c/0x34
[  154.749094] LR is at __dma_page_cpu_to_dev+0x28/0x90
[  154.754027] pc : [<c0114070>]    lr : [<c010f624>]    psr: 20030013
               sp : eb845b90  ip : 00040000  fp : 00000001
[  154.765464] r10: 00000001  r9 : 00000000  r8 : c010f6d0
[  154.770664] r7 : 00000001  r6 : 000e7000  r5 : f2d00000  r4 : 00000000
[  154.777164] r3 : 0000003f  r2 : 00000040  r1 : a48e7000  r0 : a4800000
[  154.783663] Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
[  154.790767] Control: 10c5387d  Table: 6d88006a  DAC: 00000051
[  154.796486] Process v4l2video6dec0: (pid: 1901, stack limit = 0xeb844210)
[  154.803246] Stack: (0xeb845b90 to 0xeb846000)

---

[  155.092674] 5fe0: b6589200 b384393c b6556095 b6cf8ea6 20030030
0000001a 00000000 00000000
[  155.100828] [<c0114070>] (v7_dma_clean_range) from [<c010f624>]
(__dma_page_cpu_to_dev+0x28/0x90)
[  155.109661] [<c010f624>] (__dma_page_cpu_to_dev) from [<c010f734>]
(arm_dma_map_page+0x64/0x68)
[  155.118326] [<c010f734>] (arm_dma_map_page) from [<c0110760>]
(arm_dma_map_sg+0x90/0x1a4)
[  155.126486] [<c0110760>] (arm_dma_map_sg) from [<bf1c7814>]
(vb2_dc_dmabuf_ops_map+0x1a0/0x228 [videobuf2_dma_contig])
[  155.137143] [<bf1c7814>] (vb2_dc_dmabuf_ops_map
[videobuf2_dma_contig]) from [<c044c510>]
(dma_buf_map_attachment+0x44/0x68)
[  155.148314] [<c044c510>] (dma_buf_map_attachment) from [<bf1c7318>]
(vb2_dc_map_dmabuf+0x70/0x17c [videobuf2_dma_contig])
[  155.159254] [<bf1c7318>] (vb2_dc_map_dmabuf [videobuf2_dma_contig])
from [<bf1328c8>] (__qbuf_dmabuf+0x3c4/0x4c8 [videobuf2_core])
[  155.170940] [<bf1328c8>] (__qbuf_dmabuf [videobuf2_core]) from
[<bf132afc>] (__buf_prepare+0x130/0x16c [videobuf2_core])
[  155.181771] [<bf132afc>] (__buf_prepare [videobuf2_core]) from
[<bf132d3c>] (vb2_core_qbuf+0x128/0x204 [videobuf2_core])
[  155.192612] [<bf132d3c>] (vb2_core_qbuf [videobuf2_core]) from
[<bf1ef658>] (v4l2_m2m_qbuf+0x1c/0x34 [v4l2_mem2mem])
[  155.203195] [<bf1ef658>] (v4l2_m2m_qbuf [v4l2_mem2mem]) from
[<bf062914>] (__video_do_ioctl+0x290/0x2ec [videodev])
[  155.213542] [<bf062914>] (__video_do_ioctl [videodev]) from
[<bf062338>] (video_usercopy+0x1a0/0x4e0 [videodev])
[  155.223677] [<bf062338>] (video_usercopy [videodev]) from
[<bf05d5a4>] (v4l2_ioctl+0xa0/0xd8 [videodev])
[  155.233094] [<bf05d5a4>] (v4l2_ioctl [videodev]) from [<c01f9d3c>]
(do_vfs_ioctl+0x9c/0x8e0)
[  155.241461] [<c01f9d3c>] (do_vfs_ioctl) from [<c01fa5b4>]
(SyS_ioctl+0x34/0x5c)
[  155.248743] [<c01fa5b4>] (SyS_ioctl) from [<c0107740>]
(ret_fast_syscall+0x0/0x3c)
[  155.256278] Code: e3a02004 e1a02312 e2423001 e1c00003 (ee070f3a)
[  155.262444] ---[ end trace e7118e14619b5c07 ]---

thanks,
-- Shuah

>
> So, I'm really not applying this patch until the purpose of this
> function gets documented.
>
> Thanks.
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

[PATCH] ARM: dma-mapping: Fix dma_get_sgtable() for regions without struct page

[Russell King - ARM Linux]

On Wed, Mar 22, 2017 at 09:46:05AM -0600, Shuah Khan wrote:
> This check doesn't really do any kind of validation.

Exactly right.

> I have been debugging
> a pagefault when the sgtable created by arm_dma_get_sgtable() runs into
> the following kernel error when vb2_dc_dmabuf_ops_map() tries to map
> the exported buffer from another driver.

It is _completely_ incorrect to even try to pass memory allocated from
the coherent DMA allocator back into the _streaming_ DMA API.  That is
just something that the streaming DMA API does not support.

I am dismayed at dma_get_sgtable() ever having been proposed - I think
it's completely wrong.  Looking back to when it was created in 2012, it
appears that it never got any review from architecture people - there
certainly are no architecture people in the attributations for it.

There are two Samsung people and a DRM person in the sign-off, and IMHO
that's not good enough for a proposal which completely changes the way
DMA memory is dealt with - especially as _that_ is soo architecture
dependent.

Annoyingly, when I was looking at the dma_buf API while I was implementing
the armada DRM driver, I did point out the problems with the dma_buf API
wrt passing contiguous regions from one driver to another, and I suggested
separating the DMA mapping operation somewhat, but all that fell on deaf
ears.  I guess the results of that are now coming home to roost.

The fundamental point about coherent DMA memory, or memory that is
provided via the declaration mechanisms is that there is _no_ gurantee
that there will be a "struct page" backing it, so creating a scatterlist
with "struct page" pointers, which is then subsequently dma_map_sg()'d
elsewhere is always going to be totally broken.  In the case of some
coherent DMA allocators, there _may_ be a "struct page" backing the
memory, but that's not guaranteed, and since dma_map_sg() relies on
there being a valid "struct page", things will go wrong.

And by "wrong", I mean it could oops the kernel because of trying to
dereference the "struct page" pointer, or if that's successful, we
could end up invalidating the cache lines for some random region of
memory, which then goes on to cause a completely different failure, or
maybe filesystem corruption (eg, what if it hits a page cache page
containing an inode table, and invalidates all updates to that page?)
The consequences really don't bare thinking about.

> Just an an experiment, I added this hack: in this case __dc_alloc()
> returned values is the mapped page. This is by mo means a fix - I am
> not sure if we can rely on this page, since at DQBUF its get unmapped.
> 
> /* In this case cpu_addr is the page - just use it */
>         if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) == 0)
>                 page = cpu_addr;
> 
> At this point I dumped some data:
> 
> [  154.627505] platform 11000000.codec:left:
> debug_arm_dma_get_sgtable: bus_to_pfn 772608 dma_pfn_offset 0
> PHYS_PFN_OFFSET 262144
> [  154.627512] platform 11000000.codec:left:
> debug_arm_dma_get_sgtable: dma_to_pfn 772608 virt_to_pfn 470272
> virt_to_dma 1926234112 virt_to_page ef6ca000 page f0004000
> 
> cpu_addr is
> [  154.627520] platform 11000000.codec:left:
> debug_arm_dma_get_sgtable: cpu_addr f2d00000 handle bca00000 page =
> f2d00000 size 946176 align size 946176
> [  154.627527] platform 11000000.codec:left: after
> dma_get_sgtable_attrs() vb2_dc_get_base_sgt: sgt ecdcec00 page
> f2d00000 buf ecdced80 cookie f2d00000 vaddr f2d00000 dc_cookie
> ecdced90 dma_addr bca00000 size 946176
> 
> cpu_addr is f2d00000 and the page you get from
> pfn_to_page(dma_to_pfn(dev, handle)) is f0004000.

I'd like to see the diff that produced the above dumps to make better
sense of it...

> The page you get with struct page *page = pfn_to_page(dma_to_pfn(dev, handle));
> is bogus. So this check Merek added doesn't find a bogus page, does a reverse
> validation of the bogus page.

Right - dma_to_pfn() will correctly translate the "handle" to a PFN
number, so we can just multiply that with PAGE_SIZE to get the physical
address, which if I'm interpreting your information above correctly,
would be 0x72d00000.

However, what you're saying is that there is no valid "struct page"
associated with that address.

The correct way to validate this is:

	unsigned long pfn = dma_to_pfn(dev, handle);
	struct page *page;

	if (!pfn_valid(pfn))
		return -ENXIO;

	page = pfn_to_page(pfn);

However, I have a huge big problem with this - this can pass even for
memory obtained through the coherent allocator (because of the way CMA
works.)

This can end up with the DMA streaming API kmapping memory using
cacheable attributes while CMA has it mapped with uncacheable attributes,
where (as mentioned on IRC this morning):

	you enter the realm of "mismatched memory attributes",
	which don't blow up, but you can lose guarantees of both
	mappings wrt coherence and ordering

We really don't want to go there.  The problem, however, is that we
can't distingish a CMA page allocated via the DMA coherent allocator
(and so potentially mapped uncacheable) with a normal allocation of
the same page, which would be valid for streaming DMA API use.

I think the best we can do is as per the above for declared DMA memory
(iow, memory that isn't struct page backed.)  It should _never_ be
passed into the streaming DMA API.

I also think that the dma_get_sgtable() thing is a total trainwreck,
and we need to have a rethink about this.

This isn't a case of "something changed in the architecture that broke
dma_get_sgtable()" - this is a case that it's always been broken in
this way ever since dma_get_sgtable() was introduced, and dma_get_sgtable()
was never (properly?) reviewed with this kind of use-case in mind.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: dma-mapping: Fix dma_get_sgtable() for regions without struct page

[Shuah Khan]

On Wed, Mar 22, 2017 at 10:32 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Wed, Mar 22, 2017 at 09:46:05AM -0600, Shuah Khan wrote:
>> This check doesn't really do any kind of validation.
>
> Exactly right.
>
>> I have been debugging
>> a pagefault when the sgtable created by arm_dma_get_sgtable() runs into
>> the following kernel error when vb2_dc_dmabuf_ops_map() tries to map
>> the exported buffer from another driver.
>
> It is _completely_ incorrect to even try to pass memory allocated from
> the coherent DMA allocator back into the _streaming_ DMA API.  That is
> just something that the streaming DMA API does not support.
>
> I am dismayed at dma_get_sgtable() ever having been proposed - I think
> it's completely wrong.  Looking back to when it was created in 2012, it
> appears that it never got any review from architecture people - there
> certainly are no architecture people in the attributations for it.
>
> There are two Samsung people and a DRM person in the sign-off, and IMHO
> that's not good enough for a proposal which completely changes the way
> DMA memory is dealt with - especially as _that_ is soo architecture
> dependent.
>
> Annoyingly, when I was looking at the dma_buf API while I was implementing
> the armada DRM driver, I did point out the problems with the dma_buf API
> wrt passing contiguous regions from one driver to another, and I suggested
> separating the DMA mapping operation somewhat, but all that fell on deaf
> ears.  I guess the results of that are now coming home to roost.
>
> The fundamental point about coherent DMA memory, or memory that is
> provided via the declaration mechanisms is that there is _no_ gurantee
> that there will be a "struct page" backing it, so creating a scatterlist
> with "struct page" pointers, which is then subsequently dma_map_sg()'d
> elsewhere is always going to be totally broken.  In the case of some
> coherent DMA allocators, there _may_ be a "struct page" backing the
> memory, but that's not guaranteed, and since dma_map_sg() relies on
> there being a valid "struct page", things will go wrong.
>
> And by "wrong", I mean it could oops the kernel because of trying to
> dereference the "struct page" pointer, or if that's successful, we
> could end up invalidating the cache lines for some random region of
> memory, which then goes on to cause a completely different failure, or
> maybe filesystem corruption (eg, what if it hits a page cache page
> containing an inode table, and invalidates all updates to that page?)
> The consequences really don't bare thinking about.
>
>> Just an an experiment, I added this hack: in this case __dc_alloc()
>> returned values is the mapped page. This is by mo means a fix - I am
>> not sure if we can rely on this page, since at DQBUF its get unmapped.
>>
>> /* In this case cpu_addr is the page - just use it */
>>         if ((attrs & DMA_ATTR_NO_KERNEL_MAPPING) == 0)
>>                 page = cpu_addr;
>>
>> At this point I dumped some data:
>>
>> [  154.627505] platform 11000000.codec:left:
>> debug_arm_dma_get_sgtable: bus_to_pfn 772608 dma_pfn_offset 0
>> PHYS_PFN_OFFSET 262144
>> [  154.627512] platform 11000000.codec:left:
>> debug_arm_dma_get_sgtable: dma_to_pfn 772608 virt_to_pfn 470272
>> virt_to_dma 1926234112 virt_to_page ef6ca000 page f0004000
>>
>> cpu_addr is
>> [  154.627520] platform 11000000.codec:left:
>> debug_arm_dma_get_sgtable: cpu_addr f2d00000 handle bca00000 page =
>> f2d00000 size 946176 align size 946176
>> [  154.627527] platform 11000000.codec:left: after
>> dma_get_sgtable_attrs() vb2_dc_get_base_sgt: sgt ecdcec00 page
>> f2d00000 buf ecdced80 cookie f2d00000 vaddr f2d00000 dc_cookie
>> ecdced90 dma_addr bca00000 size 946176
>>
>> cpu_addr is f2d00000 and the page you get from
>> pfn_to_page(dma_to_pfn(dev, handle)) is f0004000.
>
> I'd like to see the diff that produced the above dumps to make better
> sense of it...

I added debug to track the buffer from allocation time to export buf
time and all
the way to the time map_sg fails.

Please see the attached diff. It is somewhat cluttered with lots of debug
messages. I also added a debug version (duplicate) arm_dma_get_sgtable()
so I can limit the messages to when it gets called from vb2_dc_get_base_sgt()
>
>> The page you get with struct page *page = pfn_to_page(dma_to_pfn(dev, handle));
>> is bogus. So this check Merek added doesn't find a bogus page, does a reverse
>> validation of the bogus page.
>
> Right - dma_to_pfn() will correctly translate the "handle" to a PFN
> number, so we can just multiply that with PAGE_SIZE to get the physical
> address, which if I'm interpreting your information above correctly,
> would be 0x72d00000.

Correct paddr is 0x72d00000. I forgot to add the allocation time debug dump:

[  154.619456] platform 11000000.codec:left: vb2_dc_alloc: buf
ecdced80 cookie f2d00000 vaddr f2d00000 paddr 72d00000 dma_addr
bca00000 size 946176 dc_cookie ecdced90 dma_to_pfn 772608

I added debug to track the buffer from allocation time to export buf
time and all the way to the time map_sg fails.

>
> However, what you're saying is that there is no valid "struct page"
> associated with that address.

>From what I can tell from the data I have, dma_to_pfn(dev, handle);
doesn't return
valid page at arm_dma_get_sgtable(), even though dma_to_pfn(dev,
handle); itself stays
the same from alloc time, as you can see in the above dump from
vb2_dc_alloc(): dma_to_pfn 772608

I am guessing page might not be valid, even at the alloc time. That is
next on debug list.
Which is somewhat puzzling, because dma_attrs don't have the
DMA_ATTR_NO_KERNEL_MAP set, __dma_alloc() passes in want_vaddr true.
__alloc_from_contiguous() will map the page and return a valid vaddr

__dma_alloc() then does
*handle = pfn_to_dma(dev, page_to_pfn(page));

to return the handle.

>
> The correct way to validate this is:
>
>         unsigned long pfn = dma_to_pfn(dev, handle);
>         struct page *page;
>
>         if (!pfn_valid(pfn))
>                 return -ENXIO;
>
>         page = pfn_to_page(pfn);
>
> However, I have a huge big problem with this - this can pass even for
> memory obtained through the coherent allocator (because of the way CMA
> works.)

Right. DMA_ATTR_NO_KERNEL_MAP is another variable, if set, there won't
be an kernel mapping.

>
> This can end up with the DMA streaming API kmapping memory using
> cacheable attributes while CMA has it mapped with uncacheable attributes,
> where (as mentioned on IRC this morning):

Which irc channel?? I can get on there for the discussion if it makes it easier.

>
>         you enter the realm of "mismatched memory attributes",
>         which don't blow up, but you can lose guarantees of both
>         mappings wrt coherence and ordering
>
> We really don't want to go there.  The problem, however, is that we
> can't distingish a CMA page allocated via the DMA coherent allocator
> (and so potentially mapped uncacheable) with a normal allocation of
> the same page, which would be valid for streaming DMA API use.
>
> I think the best we can do is as per the above for declared DMA memory
> (iow, memory that isn't struct page backed.)  It should _never_ be
> passed into the streaming DMA API.

We can see some of the evidence here in this case. One driver allocates
the buffer and another driber tries to import. After buffer is exported, there
is a window between DQBUF and QBUF, DQBUF unmaps the buffers. Then
the importer comes along referencing the sg_table and looks to map them.
Before mapping, looks like D-cache clean has to occur and that would need
vaddr if I understand correctly. So there a lot of places, it can trip all over.

>
> I also think that the dma_get_sgtable() thing is a total trainwreck,
> and we need to have a rethink about this.

I don't think it works for the case I am running into.

>
> This isn't a case of "something changed in the architecture that broke
> dma_get_sgtable()" - this is a case that it's always been broken in
> this way ever since dma_get_sgtable() was introduced, and dma_get_sgtable()
> was never (properly?) reviewed with this kind of use-case in mind.

I think the use-case I am debugging has always been broken. This is a fairly
involved gstreamer pipeline to my knowledge has never been tested. This
page fault is a known issue at least for a couple of releases- I started looking
into it a couple of releases ago towards the end of 4.9.

Maybe there are some simpler use-cases that will work. I am also curious if
this works well when DMA_ATTR_NO_KERNEL_MAPPING is set. It is not
in my use-case.

Thanks,
-- Shuah

>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: videobuf2-dma-contig.c_diff
Type: application/octet-stream
Size: 9116 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170322/21617ea4/attachment-0001.obj>

[PATCH] ARM: dma-mapping: Fix dma_get_sgtable() for regions without struct page

[Russell King - ARM Linux]

On Wed, Mar 22, 2017 at 02:29:09PM -0600, Shuah Khan wrote:
> On Wed, Mar 22, 2017 at 10:32 AM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > Right - dma_to_pfn() will correctly translate the "handle" to a PFN
> > number, so we can just multiply that with PAGE_SIZE to get the physical
> > address, which if I'm interpreting your information above correctly,
> > would be 0x72d00000.
> 
> Correct paddr is 0x72d00000. I forgot to add the allocation time debug dump:
> 
> [  154.619456] platform 11000000.codec:left: vb2_dc_alloc: buf
> ecdced80 cookie f2d00000 vaddr f2d00000 paddr 72d00000 dma_addr
> bca00000 size 946176 dc_cookie ecdced90 dma_to_pfn 772608
> 
> > However, what you're saying is that there is no valid "struct page"
> > associated with that address.
> 
> >From what I can tell from the data I have, dma_to_pfn(dev, handle);
> doesn't return valid page at arm_dma_get_sgtable(), even though
> dma_to_pfn(dev, handle); itself stays the same from alloc time, as you
> can see in the above dump from vb2_dc_alloc(): dma_to_pfn 772608

dma_to_pfn() returns the physical page frame number of the DMA address
in handle.  It's page number 772608, which multiplying by the page size
gives us a physical address of 0xbca00000.  So, it's correct in so far
as if the dma_addr (of 0xbca00000 above) is correct, then that
corresponds with a physical address of 0xbca00000.

I now think that 0xf2d00000 virtual is a remapped address, not part of
lowmem (it's within the vmalloc region) which will probably be confirmed
if you look in /proc/vmallocinfo.  Since this address is remapped,
"__pa(buf->vaddr)" is invalid, it does not reference the physical page
at 0x72d00000 at all.  __pa() is just doing a simple mathematical offset
translation, which is why it's undefined for remapped addresses.

So, I'll go with 0xbca00000 being the real physical address, not
0x72d00000.

> I am guessing page might not be valid, even at the alloc time. That is
> next on debug list.

If this is coming from dma_declare_coherent_memory() then it won't have
a struct page associated with it.

> Which is somewhat puzzling, because dma_attrs don't have the
> DMA_ATTR_NO_KERNEL_MAP set, __dma_alloc() passes in want_vaddr true.
> __alloc_from_contiguous() will map the page and return a valid vaddr
> 
> __dma_alloc() then does
> *handle = pfn_to_dma(dev, page_to_pfn(page));
> 
> to return the handle.

I think you're missing something, which is the code in
include/linux/dma-mapping.h - dma_alloc_attrs().

That calls dma_alloc_from_coherent() first, which is how memory provided
via dma_declare_coherent_memory() is allocated.  This bypasses the
architecture code in arch/arm/mm/dma-mapping.c.  This provides memory
which is _not_ backed by a valid "struct page".

> > The correct way to validate this is:
> >
> >         unsigned long pfn = dma_to_pfn(dev, handle);
> >         struct page *page;
> >
> >         if (!pfn_valid(pfn))
> >                 return -ENXIO;
> >
> >         page = pfn_to_page(pfn);
> >
> > However, I have a huge big problem with this - this can pass even for
> > memory obtained through the coherent allocator (because of the way CMA
> > works.)
> 
> Right. DMA_ATTR_NO_KERNEL_MAP is another variable, if set, there won't
> be an kernel mapping.

Correct - which is why the above is probably the most correct way on
ARM (provided there is no IOMMU in the path) to get the corresponding
valid struct page pointer.

> > This can end up with the DMA streaming API kmapping memory using
> > cacheable attributes while CMA has it mapped with uncacheable attributes,
> > where (as mentioned on IRC this morning):
> 
> Which irc channel?? I can get on there for the discussion if it makes
> it easier.

It was for an unrelated discussion.

> We can see some of the evidence here in this case. One driver allocates
> the buffer and another driber tries to import. After buffer is exported,
> there is a window between DQBUF and QBUF, DQBUF unmaps the buffers. Then
> the importer comes along referencing the sg_table and looks to map them.
> Before mapping, looks like D-cache clean has to occur and that would need
> vaddr if I understand correctly. So there a lot of places, it can trip
> all over.

This is wrong, and this is where I say that the dma_buf stuff is totally
broken.

DMA coherent memory must _never_ be passed to the streaming DMA API.  No
exceptions.

However, the dma_buf stuff does not differentiate between DMA coherent
memory and streaming DMA memory - the model is purely around the exporter
creating a scatterlist, and the importer calling dma_map_sg() on the
scatterlist to obtain its own set of DMA addresses.

(sorry for the caps, but it's an important point)  THIS DOES NOT WORK.

It is a violation of the DMA API.

> > I also think that the dma_get_sgtable() thing is a total trainwreck,
> > and we need to have a rethink about this.
> 
> I don't think it works for the case I am running into.

This is the root cause - the idea that you can create a valid
scatterlist from a DMA coherent area is _just_ completely broken.
It's not possible.  dma_get_sgtable() is _fundamentally_ broken in
its design.

> > This isn't a case of "something changed in the architecture that broke
> > dma_get_sgtable()" - this is a case that it's always been broken in
> > this way ever since dma_get_sgtable() was introduced, and dma_get_sgtable()
> > was never (properly?) reviewed with this kind of use-case in mind.
> 
> I think the use-case I am debugging has always been broken.

It's _always_ been invalid to pass memory allocated from
dma_alloc_coherent() into the streaming API functions (like
dma_map_sg().)  So, you are quite correct that this has _always_ been
broken.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: dma-mapping: Fix dma_get_sgtable() for regions without struct page

[Russell King - ARM Linux]

Okay, I'm about to merge the following patch for -rc, since refusing
to create a scattertable for non-page backed memory is the only valid
solution for that case.  I'm intending to queue this for -rc this
evening.

8<====
ARM: dma-mapping: disallow dma_get_sgtable() for non-kernel managed memory

dma_get_sgtable() tries to create a scatterlist table containing valid
struct page pointers for the coherent memory allocation passed in to it.

However, memory can be declared via dma_declare_coherent_memory(), or
via other reservation schemes which means that coherent memory is not
guaranteed to be backed by struct pages.  This means it is not possible
to create a valid scatterlist via this mechanism.

This patch adds detection of such memory, and refuses to create a
scatterlist table for such memory.

Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
 arch/arm/mm/dma-mapping.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
index 63eabb06f9f1..475811f5383a 100644
--- a/arch/arm/mm/dma-mapping.c
+++ b/arch/arm/mm/dma-mapping.c
@@ -935,13 +935,31 @@ static void arm_coherent_dma_free(struct device *dev, size_t size, void *cpu_add
 	__arm_dma_free(dev, size, cpu_addr, handle, attrs, true);
 }
 
+/*
+ * The whole dma_get_sgtable() idea is fundamentally unsafe - it seems
+ * that the intention is to allow exporting memory allocated via the
+ * coherent DMA APIs through the dma_buf API, which only accepts a
+ * scattertable.  This presents a couple of problems:
+ * 1. Not all memory allocated via the coherent DMA APIs is backed by
+ *    a struct page
+ * 2. Passing coherent DMA memory into the streaming APIs is not allowed
+ *    as we will try to flush the memory through a different alias to that
+ *    actually being used (and the flushes are redundant.)
+ */
 int arm_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
 		 void *cpu_addr, dma_addr_t handle, size_t size,
 		 unsigned long attrs)
 {
-	struct page *page = pfn_to_page(dma_to_pfn(dev, handle));
+	unsigned long pfn = dma_to_pfn(dev, handle);
+	struct page *page;
 	int ret;
 
+	/* If the PFN is not valid, we do not have a struct page */
+	if (!pfn_valid(pfn))
+		return -ENXIO;
+
+	page = pfn_to_page(pfn);
+
 	ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
 	if (unlikely(ret))
 		return ret;
-- 
2.7.4

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: dma-mapping: Fix dma_get_sgtable() for regions without struct page

[Shuah Khan]

Hi Russell,

On Wed, Mar 29, 2017 at 10:33 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> Okay, I'm about to merge the following patch for -rc, since refusing
> to create a scattertable for non-page backed memory is the only valid
> solution for that case.  I'm intending to queue this for -rc this
> evening.

I was bit sidetracked and getting back to this today. I am just about
to test this change on my system. I will test your patch and send you
results.

This might not help my use-case - I suspect it will trip this check.
That said I will send you results very soon.

>
> 8<====
> ARM: dma-mapping: disallow dma_get_sgtable() for non-kernel managed memory
>
> dma_get_sgtable() tries to create a scatterlist table containing valid
> struct page pointers for the coherent memory allocation passed in to it.
>
> However, memory can be declared via dma_declare_coherent_memory(), or
> via other reservation schemes which means that coherent memory is not
> guaranteed to be backed by struct pages.  This means it is not possible
> to create a valid scatterlist via this mechanism.

I have been thinking about this some. I would like to see if we can
provide a way forward to address the cases where coherent memory is
not guaranteed to be backed by struct pages. We know the memory isn't
backed at alloc time in dma_alloc_coherent(). Can we key off of that
maybe add a new attr flag to avoid page lookups. I am willing to work
on the fixing it.

Let me first send you the results after testing your patch. Maybe we
can explore ways to fix the problem.

thanks,
-- Shuah

>
> This patch adds detection of such memory, and refuses to create a
> scatterlist table for such memory.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
>  arch/arm/mm/dma-mapping.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
> index 63eabb06f9f1..475811f5383a 100644
> --- a/arch/arm/mm/dma-mapping.c
> +++ b/arch/arm/mm/dma-mapping.c
> @@ -935,13 +935,31 @@ static void arm_coherent_dma_free(struct device *dev, size_t size, void *cpu_add
>         __arm_dma_free(dev, size, cpu_addr, handle, attrs, true);
>  }
>
> +/*
> + * The whole dma_get_sgtable() idea is fundamentally unsafe - it seems
> + * that the intention is to allow exporting memory allocated via the
> + * coherent DMA APIs through the dma_buf API, which only accepts a
> + * scattertable.  This presents a couple of problems:
> + * 1. Not all memory allocated via the coherent DMA APIs is backed by
> + *    a struct page
> + * 2. Passing coherent DMA memory into the streaming APIs is not allowed
> + *    as we will try to flush the memory through a different alias to that
> + *    actually being used (and the flushes are redundant.)
> + */
>  int arm_dma_get_sgtable(struct device *dev, struct sg_table *sgt,
>                  void *cpu_addr, dma_addr_t handle, size_t size,
>                  unsigned long attrs)
>  {
> -       struct page *page = pfn_to_page(dma_to_pfn(dev, handle));
> +       unsigned long pfn = dma_to_pfn(dev, handle);
> +       struct page *page;
>         int ret;
>
> +       /* If the PFN is not valid, we do not have a struct page */
> +       if (!pfn_valid(pfn))
> +               return -ENXIO;
> +
> +       page = pfn_to_page(pfn);
> +
>         ret = sg_alloc_table(sgt, 1, GFP_KERNEL);
>         if (unlikely(ret))
>                 return ret;
> --
> 2.7.4
>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.

[PATCH] ARM: dma-mapping: Fix dma_get_sgtable() for regions without struct page

[Russell King - ARM Linux]

On Wed, Mar 29, 2017 at 10:51:05AM -0600, Shuah Khan wrote:
> Hi Russell,
> 
> On Wed, Mar 29, 2017 at 10:33 AM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > Okay, I'm about to merge the following patch for -rc, since refusing
> > to create a scattertable for non-page backed memory is the only valid
> > solution for that case.  I'm intending to queue this for -rc this
> > evening.
> 
> I was bit sidetracked and getting back to this today. I am just about
> to test this change on my system. I will test your patch and send you
> results.
> 
> This might not help my use-case - I suspect it will trip this check.
> That said I will send you results very soon.

I do hope that it _does_ trip this check - the result should be that you
get an error without the kernel oopsing.

> I have been thinking about this some. I would like to see if we can
> provide a way forward to address the cases where coherent memory is
> not guaranteed to be backed by struct pages. We know the memory isn't
> backed at alloc time in dma_alloc_coherent(). Can we key off of that
> maybe add a new attr flag to avoid page lookups. I am willing to work
> on the fixing it.

What we need is to revise the dma-buf API and DMA APIs to allow passing
coherent memory through it - there is no current provision in the DMA APIs
to have coherent memory mapped for two peripherals.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: dma-mapping: Fix dma_get_sgtable() for regions without struct page

[Shuah Khan]

Hi Russell,

On Wed, Mar 29, 2017 at 11:03 AM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Wed, Mar 29, 2017 at 10:51:05AM -0600, Shuah Khan wrote:
>> Hi Russell,
>>
>> On Wed, Mar 29, 2017 at 10:33 AM, Russell King - ARM Linux
>> <linux@armlinux.org.uk> wrote:
>> > Okay, I'm about to merge the following patch for -rc, since refusing
>> > to create a scattertable for non-page backed memory is the only valid
>> > solution for that case.  I'm intending to queue this for -rc this
>> > evening.
>>
>> I was bit sidetracked and getting back to this today. I am just about
>> to test this change on my system. I will test your patch and send you
>> results.
>>
>> This might not help my use-case - I suspect it will trip this check.
>> That said I will send you results very soon.
>
> I do hope that it _does_ trip this check - the result should be that you
> get an error without the kernel oopsing.

Agreed. I would rather have it fail gracefully earlier than limp along
and die later. I would make a suggestion based on these results.

Please consider adding a dev_err() to if (!pfn_valid(pfn)) condition
to indicate why get_sgtable failed including __func__

I worry about adding dev_err() to a core function, but in this case,
it would be helpful to do to make it very clear that the requested
use-case can't be supported.

Results:

[  235.706048] platform 11000000.codec:left: failed to get scatterlist
from DMA API

The following is a WARN_ON from vb2_dc_get_dmabuf() - probably should
go away. I will send a patch removing the WARN_ON to linux-media


[  235.711985] ------------[ cut here ]------------
[  235.712001] WARNING: CPU: 6 PID: 1987 at
drivers/media/v4l2-core/videobuf2-dma-contig.c:402
vb2_dc_get_dmabuf+0x130/0x168 [videobuf2_dma_contig]
[  235.712005] Modules linked in: cpufreq_userspace cpufreq_powersave
cpufreq_conservative s5p_jpeg s5p_mfc exynos_gsc v4l2_mem2mem
videobuf2_dma_contig v4l2_common videobuf2_memops videobuf2_v4l2
videobuf2_core videodev media
[  235.712063] CPU: 6 PID: 1987 Comm: qtdemux0:sink Not tainted
4.11.0-rc2-00001-g49214d0-dirty #11
[  235.712068] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[  235.712085] [<c010e244>] (unwind_backtrace) from [<c010aea0>]
(show_stack+0x10/0x14)
[  235.712096] [<c010aea0>] (show_stack) from [<c033ef50>]
(dump_stack+0x78/0x8c)
[  235.712106] [<c033ef50>] (dump_stack) from [<c011b3c8>] (__warn+0xe8/0x100)
[  235.712113] [<c011b3c8>] (__warn) from [<c011b490>]
(warn_slowpath_null+0x20/0x28)
[  235.712122] [<c011b490>] (warn_slowpath_null) from [<bf20d8b8>]
(vb2_dc_get_dmabuf+0x130/0x168 [videobuf2_dma_contig])
[  235.712141] [<bf20d8b8>] (vb2_dc_get_dmabuf [videobuf2_dma_contig])
from [<bf151ec4>] (vb2_core_expbuf+0x1d4/0x2d8 [videobuf2_core])
[  235.712159] [<bf151ec4>] (vb2_core_expbuf [videobuf2_core]) from
[<bf19c198>] (vb2_expbuf+0x2c/0x34 [videobuf2_v4l2])
[  235.712195] [<bf19c198>] (vb2_expbuf [videobuf2_v4l2]) from
[<bf04b834>] (__video_do_ioctl+0x204/0x2fc [videodev])
[  235.712241] [<bf04b834>] (__video_do_ioctl [videodev]) from
[<bf04b390>] (video_usercopy+0x24c/0x4e0 [videodev])
[  235.712285] [<bf04b390>] (video_usercopy [videodev]) from
[<bf0465a4>] (v4l2_ioctl+0xa0/0xd8 [videodev])
[  235.712316] [<bf0465a4>] (v4l2_ioctl [videodev]) from [<c01fe920>]
(do_vfs_ioctl+0x9c/0x8e0)
[  235.712325] [<c01fe920>] (do_vfs_ioctl) from [<c01ff198>]
(SyS_ioctl+0x34/0x5c)
[  235.712334] [<c01ff198>] (SyS_ioctl) from [<c0107740>]
(ret_fast_syscall+0x0/0x3c)
[  235.712339] ---[ end trace cdfda037de46497c ]---

>
>> I have been thinking about this some. I would like to see if we can
>> provide a way forward to address the cases where coherent memory is
>> not guaranteed to be backed by struct pages. We know the memory isn't
>> backed at alloc time in dma_alloc_coherent(). Can we key off of that
>> maybe add a new attr flag to avoid page lookups. I am willing to work
>> on the fixing it.
>
> What we need is to revise the dma-buf API and DMA APIs to allow passing
> coherent memory through it - there is no current provision in the DMA APIs
> to have coherent memory mapped for two peripherals.

I will explore adding the support. I do think there is value in being
able to use coherent memory
buffers for DMA.

thanks,
-- Shuah

>
> --
> RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.