Re: [PATCH v6 4/5] dma-buf: heaps: Add CMA heap to dmabuf heaps

[Andrew F. Davis]

On 7/25/19 10:30 AM, Christoph Hellwig wrote:
> On Thu, Jul 25, 2019 at 10:25:50AM -0400, Andrew F. Davis wrote:
>> On 7/25/19 10:11 AM, Christoph Hellwig wrote:
>>> On Thu, Jul 25, 2019 at 10:10:08AM -0400, Andrew F. Davis wrote:
>>>> Pages yes, but not "normal" pages from the kernel managed area.
>>>> page_to_pfn() will return bad values on the pages returned by this
>>>> allocator and so will any of the kernel sync/map functions. Therefor
>>>> those operations cannot be common and need special per-heap handling.
>>>
>>> Well, that means this thing is buggy and abuses the scatterlist API
>>> and we can't merge it anyway, so it is irrelevant.
>>>
>>
>> Since when do scatterlists need to only have kernel virtual backed
>> memory pages? Device memory is stored in scatterlists and
>> dma_sync_sg_for_* would fail just the same when the cache ops were
>> attempted.
> 
> I'm not sure what you mean with virtual backed memory pages, as we
> don't really have that concept.
> 
> But a page in the scatterlist needs to be able to be used everywhere
> we'd normally use a page, e.g. page_to_phys, page_to_pfn, kmap,
> page_address (if !highmem) as consumers including the dma mapping
> interface do all that.
> 
> If you want to dma map memory that does not have page backing you
> need to use dma_map_resource.
> 

I probably should have worded that better.

It does have page backing, what I meant by "page_to_pfn() will return
bad values" is not that it won't give you the correct pfn, it will, but
that then that pfn is not part of the normal memory space
(lowmem/highmem) it's device memory, so cache ops won't work. But you
should not be doing that on device memory anyway.

That is a problem with Ion I want to avoid, it assumed all buffers were
in DDR and so would do cache operations on them unconditionally, too
many assumptions were made as too much was moved into the common core
code and not enough was left for the heaps themselves to decide.