Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Nicolas Dufresne]

Le mardi 14 mars 2017 à 15:47 +0100, Benjamin Gaignard a écrit :
> Should we use /devi/ion/$heap instead of /dev/ion_$heap ?
> I think it would be easier for user to look into one directory rather
> then in whole /dev to find the heaps
> 
> > is that we don't have to worry about a limit of 32 possible
> > heaps per system (32-bit heap id allocation field). But dealing
> > with an ioctl seems easier than names. Userspace might be less
> > likely to hardcode random id numbers vs. names as well.
> 
> In the futur I think that heap type will be replaced by a "get caps"
> ioctl which will
> describe heap capabilities. At least that is my understanding of
> kernel part
> of "unix memory allocator" project

I think what we really need from userspace point of view, is the
ability to find a compatible heap for a set of drivers. And this
without specific knowledge of the drivers.

Nicolas

signature.asc
Description: This is a digitally signed message part

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Laura Abbott]

On 03/14/2017 07:47 AM, Benjamin Gaignard wrote:
> 2017-03-13 22:09 GMT+01:00 Laura Abbott :
>> On 03/12/2017 12:05 PM, Daniel Vetter wrote:
>>> On Sun, Mar 12, 2017 at 2:34 PM, Benjamin Gaignard
>>>  wrote:
 2017-03-09 18:38 GMT+01:00 Laura Abbott :
> On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
>> 2017-03-06 17:04 GMT+01:00 Daniel Vetter :
>>> On Mon, Mar 06, 2017 at 11:58:05AM +0100, Mark Brown wrote:
 On Mon, Mar 06, 2017 at 11:40:41AM +0100, Daniel Vetter wrote:

> No one gave a thing about android in upstream, so Greg KH just dumped 
> it
> all into staging/android/. We've discussed ION a bunch of times, 
> recorded
> anything we'd like to fix in staging/android/TODO, and Laura's patch
> series here addresses a big chunk of that.

> This is pretty much the same approach we (gpu folks) used to de-stage 
> the
> syncpt stuff.

 Well, there's also the fact that quite a few people have issues with 
 the
 design (like Laurent).  It seems like a lot of them have either got 
 more
 comfortable with it over time, or at least not managed to come up with
 any better ideas in the meantime.
>>>
>>> See the TODO, it has everything a really big group (look at the patch 
>>> for
>>> the full Cc: list) figured needs to be improved at LPC 2015. We don't 
>>> just
>>> merge stuff because merging stuff is fun :-)
>>>
>>> Laurent was even in that group ...
>>> -Daniel
>>
>> For me those patches are going in the right direction.
>>
>> I still have few questions:
>> - since alignment management has been remove from ion-core, should it
>> be also removed from ioctl structure ?
>
> Yes, I think I'm going to go with the suggestion to fixup the ABI
> so we don't need the compat layer and as part of that I'm also
> dropping the align argument.
>
>> - can you we ride off ion_handle (at least in userland) and only
>> export a dma-buf descriptor ?
>
> Yes, I think this is the right direction given we're breaking
> everything anyway. I was debating trying to keep the two but
> moving to only dma bufs is probably cleaner. The only reason
> I could see for keeping the handles is running out of file
> descriptors for dma-bufs but that seems unlikely.
>>
>> In the future how can we add new heaps ?
>> Some platforms have very specific memory allocation
>> requirements (just have a look in the number of gem custom allocator in 
>> drm)
>> Do you plan to add heap type/mask for each ?
>
> Yes, that was my thinking.

 My concern is about the policy to adding heaps, will you accept
 "customs" heap per
 platforms ? per devices ? or only generic ones ?
 If you are too strict, we will have lot of out-of-tree heaps and if
 you accept of of them
 it will be a nightmare to maintain
>>>
>>> I think ion should expose any heap that's also directly accessible to
>>> devices using dma_alloc(_coherent). That should leave very few things
>>> left, like your SMA heap.
>>>
 Another point is how can we put secure rules (like selinux policy) on
 heaps since all the allocations
 go to the same device (/dev/ion) ? For example, until now, in Android
 we have to give the same
 access rights to all the process that use ION.
 It will become problem when we will add secure heaps because we won't
 be able to distinguish secure
 processes to standard ones or set specific policy per heaps.
 Maybe I'm wrong here but I have never see selinux policy checking an
 ioctl field but if that
 exist it could be a solution.
>>>
>>> Hm, we might want to expose all the heaps as individual
>>> /dev/ion_$heapname nodes? Should we do this from the start, since
>>> we're massively revamping the uapi anyway (imo not needed, current
>>> state seems to work too)?
>>> -Daniel
>>>
>>
>> I thought about that. One advantage with separate /dev/ion_$heap
> 
> Should we use /devi/ion/$heap instead of /dev/ion_$heap ?
> I think it would be easier for user to look into one directory rather
> then in whole /dev to find the heaps
> 

If we decide to move away from /dev/ion we can consider it.

>> is that we don't have to worry about a limit of 32 possible
>> heaps per system (32-bit heap id allocation field). But dealing
>> with an ioctl seems easier than names. Userspace might be less
>> likely to hardcode random id numbers vs. names as well.
> 
> In the futur I think that heap type will be replaced by a "get caps"
> ioctl which will
> describe heap capabilities. At least that is my understanding of kernel part
> of "unix memory allocator" project
> 

I don't think it will be completely replaced. A heap can have multiple

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Benjamin Gaignard]

2017-03-13 22:09 GMT+01:00 Laura Abbott :
> On 03/12/2017 12:05 PM, Daniel Vetter wrote:
>> On Sun, Mar 12, 2017 at 2:34 PM, Benjamin Gaignard
>>  wrote:
>>> 2017-03-09 18:38 GMT+01:00 Laura Abbott :
 On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
> 2017-03-06 17:04 GMT+01:00 Daniel Vetter :
>> On Mon, Mar 06, 2017 at 11:58:05AM +0100, Mark Brown wrote:
>>> On Mon, Mar 06, 2017 at 11:40:41AM +0100, Daniel Vetter wrote:
>>>
 No one gave a thing about android in upstream, so Greg KH just dumped 
 it
 all into staging/android/. We've discussed ION a bunch of times, 
 recorded
 anything we'd like to fix in staging/android/TODO, and Laura's patch
 series here addresses a big chunk of that.
>>>
 This is pretty much the same approach we (gpu folks) used to de-stage 
 the
 syncpt stuff.
>>>
>>> Well, there's also the fact that quite a few people have issues with the
>>> design (like Laurent).  It seems like a lot of them have either got more
>>> comfortable with it over time, or at least not managed to come up with
>>> any better ideas in the meantime.
>>
>> See the TODO, it has everything a really big group (look at the patch for
>> the full Cc: list) figured needs to be improved at LPC 2015. We don't 
>> just
>> merge stuff because merging stuff is fun :-)
>>
>> Laurent was even in that group ...
>> -Daniel
>
> For me those patches are going in the right direction.
>
> I still have few questions:
> - since alignment management has been remove from ion-core, should it
> be also removed from ioctl structure ?

 Yes, I think I'm going to go with the suggestion to fixup the ABI
 so we don't need the compat layer and as part of that I'm also
 dropping the align argument.

> - can you we ride off ion_handle (at least in userland) and only
> export a dma-buf descriptor ?

 Yes, I think this is the right direction given we're breaking
 everything anyway. I was debating trying to keep the two but
 moving to only dma bufs is probably cleaner. The only reason
 I could see for keeping the handles is running out of file
 descriptors for dma-bufs but that seems unlikely.
>
> In the future how can we add new heaps ?
> Some platforms have very specific memory allocation
> requirements (just have a look in the number of gem custom allocator in 
> drm)
> Do you plan to add heap type/mask for each ?

 Yes, that was my thinking.
>>>
>>> My concern is about the policy to adding heaps, will you accept
>>> "customs" heap per
>>> platforms ? per devices ? or only generic ones ?
>>> If you are too strict, we will have lot of out-of-tree heaps and if
>>> you accept of of them
>>> it will be a nightmare to maintain
>>
>> I think ion should expose any heap that's also directly accessible to
>> devices using dma_alloc(_coherent). That should leave very few things
>> left, like your SMA heap.
>>
>>> Another point is how can we put secure rules (like selinux policy) on
>>> heaps since all the allocations
>>> go to the same device (/dev/ion) ? For example, until now, in Android
>>> we have to give the same
>>> access rights to all the process that use ION.
>>> It will become problem when we will add secure heaps because we won't
>>> be able to distinguish secure
>>> processes to standard ones or set specific policy per heaps.
>>> Maybe I'm wrong here but I have never see selinux policy checking an
>>> ioctl field but if that
>>> exist it could be a solution.
>>
>> Hm, we might want to expose all the heaps as individual
>> /dev/ion_$heapname nodes? Should we do this from the start, since
>> we're massively revamping the uapi anyway (imo not needed, current
>> state seems to work too)?
>> -Daniel
>>
>
> I thought about that. One advantage with separate /dev/ion_$heap

Should we use /devi/ion/$heap instead of /dev/ion_$heap ?
I think it would be easier for user to look into one directory rather
then in whole /dev to find the heaps

> is that we don't have to worry about a limit of 32 possible
> heaps per system (32-bit heap id allocation field). But dealing
> with an ioctl seems easier than names. Userspace might be less
> likely to hardcode random id numbers vs. names as well.

In the futur I think that heap type will be replaced by a "get caps"
ioctl which will
describe heap capabilities. At least that is my understanding of kernel part
of "unix memory allocator" project

>
> Thanks,
> Laura

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Laura Abbott]

On 03/13/2017 02:29 PM, Rob Clark wrote:
> On Mon, Mar 13, 2017 at 5:09 PM, Laura Abbott  wrote:
>>> Hm, we might want to expose all the heaps as individual
>>> /dev/ion_$heapname nodes? Should we do this from the start, since
>>> we're massively revamping the uapi anyway (imo not needed, current
>>> state seems to work too)?
>>> -Daniel
>>>
>>
>> I thought about that. One advantage with separate /dev/ion_$heap
>> is that we don't have to worry about a limit of 32 possible
>> heaps per system (32-bit heap id allocation field). But dealing
>> with an ioctl seems easier than names. Userspace might be less
>> likely to hardcode random id numbers vs. names as well.
> 
> 
> other advantage, I think, is selinux (brought up elsewhere on this
> thread).. heaps at known fixed PAs are useful for certain sorts of
> attacks so being able to restrict access more easily seems like a good
> thing
> 
> BR,
> -R
> 

Some other kind of filtering (BPF/LSM/???) might work as well
(http://kernsec.org/files/lss2015/vanderstoep.pdf ?)

The fixed PA issue is a larger problem. We're never going to
be able to get away from "this heap must exist at address X"
problems but the location of CMA in general should be
randomized. I haven't actually come up with a good proposal
to this though.

I'd like for Ion to be a framework for memory allocation and
not security exploits. Hopefully this isn't a pipe dream.

Thanks,
Laura

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Laura Abbott]

On 03/13/2017 06:21 AM, Mark Brown wrote:
> On Mon, Mar 13, 2017 at 10:54:33AM +, Brian Starkey wrote:
>> On Sun, Mar 12, 2017 at 02:34:14PM +0100, Benjamin Gaignard wrote:
> 
>>> Another point is how can we put secure rules (like selinux policy) on
>>> heaps since all the allocations
>>> go to the same device (/dev/ion) ? For example, until now, in Android
>>> we have to give the same
>>> access rights to all the process that use ION.
>>> It will become problem when we will add secure heaps because we won't
>>> be able to distinguish secure
>>> processes to standard ones or set specific policy per heaps.
>>> Maybe I'm wrong here but I have never see selinux policy checking an
>>> ioctl field but if that
>>> exist it could be a solution.
> 
>> I might be thinking of a different type of "secure", but...
> 
>> Should the security of secure heaps be enforced by OS-level
>> permissions? I don't know about other architectures, but at least on
>> arm/arm64 this is enforced in hardware; it doesn't matter who has
>> access to the ion heap, because only secure devices (or the CPU
>> running a secure process) is physically able to access the memory
>> backing the buffer.
> 3
>> In fact, in the use-cases I know of, the process asking for the ion
>> allocation is not a secure process, and so we wouldn't *want* to
>> restrict the secure heap to be allocated from only by secure
>> processes.
> 
> I think there's an orthogonal level of OS level security that can be
> applied here - it's reasonable for it to want to say things like "only
> processes that are supposed to be implementing functionality X should be
> able to try to allocate memory set aside for that functionality".  This
> mitigates against escallation attacks and so on, it's not really
> directly related to secure memory as such though.
> 

Ion also makes it pretty trivial to allocate large amounts of kernel
memory and possibly DoS the system. I'd like to have as little
policy in Ion as possible but more important would be a general
security review and people shouting "bad idea ahead".

Thanks,
Laura

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Rob Clark]

On Mon, Mar 13, 2017 at 5:09 PM, Laura Abbott  wrote:
>> Hm, we might want to expose all the heaps as individual
>> /dev/ion_$heapname nodes? Should we do this from the start, since
>> we're massively revamping the uapi anyway (imo not needed, current
>> state seems to work too)?
>> -Daniel
>>
>
> I thought about that. One advantage with separate /dev/ion_$heap
> is that we don't have to worry about a limit of 32 possible
> heaps per system (32-bit heap id allocation field). But dealing
> with an ioctl seems easier than names. Userspace might be less
> likely to hardcode random id numbers vs. names as well.


other advantage, I think, is selinux (brought up elsewhere on this
thread).. heaps at known fixed PAs are useful for certain sorts of
attacks so being able to restrict access more easily seems like a good
thing

BR,
-R

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Laura Abbott]

On 03/13/2017 03:54 AM, Brian Starkey wrote:
> On Sun, Mar 12, 2017 at 02:34:14PM +0100, Benjamin Gaignard wrote:
>> 2017-03-09 18:38 GMT+01:00 Laura Abbott :
>>> On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
 2017-03-06 17:04 GMT+01:00 Daniel Vetter :
> On Mon, Mar 06, 2017 at 11:58:05AM +0100, Mark Brown wrote:
>> On Mon, Mar 06, 2017 at 11:40:41AM +0100, Daniel Vetter wrote:
>>
>>> No one gave a thing about android in upstream, so Greg KH just dumped it
>>> all into staging/android/. We've discussed ION a bunch of times, 
>>> recorded
>>> anything we'd like to fix in staging/android/TODO, and Laura's patch
>>> series here addresses a big chunk of that.
>>
>>> This is pretty much the same approach we (gpu folks) used to de-stage 
>>> the
>>> syncpt stuff.
>>
>> Well, there's also the fact that quite a few people have issues with the
>> design (like Laurent).  It seems like a lot of them have either got more
>> comfortable with it over time, or at least not managed to come up with
>> any better ideas in the meantime.
>
> See the TODO, it has everything a really big group (look at the patch for
> the full Cc: list) figured needs to be improved at LPC 2015. We don't just
> merge stuff because merging stuff is fun :-)
>
> Laurent was even in that group ...
> -Daniel

 For me those patches are going in the right direction.

 I still have few questions:
 - since alignment management has been remove from ion-core, should it
 be also removed from ioctl structure ?
>>>
>>> Yes, I think I'm going to go with the suggestion to fixup the ABI
>>> so we don't need the compat layer and as part of that I'm also
>>> dropping the align argument.
>>>
 - can you we ride off ion_handle (at least in userland) and only
 export a dma-buf descriptor ?
>>>
>>> Yes, I think this is the right direction given we're breaking
>>> everything anyway. I was debating trying to keep the two but
>>> moving to only dma bufs is probably cleaner. The only reason
>>> I could see for keeping the handles is running out of file
>>> descriptors for dma-bufs but that seems unlikely.

 In the future how can we add new heaps ?
 Some platforms have very specific memory allocation
 requirements (just have a look in the number of gem custom allocator in 
 drm)
 Do you plan to add heap type/mask for each ?
>>>
>>> Yes, that was my thinking.
>>
>> My concern is about the policy to adding heaps, will you accept
>> "customs" heap per
>> platforms ? per devices ? or only generic ones ?
>> If you are too strict, we will have lot of out-of-tree heaps and if
>> you accept of of them
>> it will be a nightmare to maintain
>>
> 
> Are you concerned about actual heaps (e.g. a carveout at 0x8000 vs
> a carveout at 0x6000) or heap types?
> 
> For heap types, I think the policy can be strict - if it's generally
> useful then it should live in-tree in ion. Otherwise, it would be
> out-of-tree. I'd expect most "custom" heaps to be parameterisable to
> the point of being generally useful.
> 

I'm willing to be reasonably permissive in what lives in tree. A good
example would be something like a heap for the OMAP tiler which had
weird hardware requirements. The associated devices that go with the
heap should be well supported upstream though.

> For actual heap instances, I would expect them to be communicated via
> reserved-memory regions or something similar, and so the maintenance
> burden is pretty low.
> 

Yes. After the next round of review for this series I'm going to
start thinking about properties for chunk and carveout heaps if nobody
proposes something first.

> The existing query ioctl can allow heap IDs to get assigned
> dynamically at runtime, so there's no need to reserve "bit 6" for
> "CUSTOM_ACME_HEAP_1"
> 
>> Another point is how can we put secure rules (like selinux policy) on
>> heaps since all the allocations
>> go to the same device (/dev/ion) ? For example, until now, in Android
>> we have to give the same
>> access rights to all the process that use ION.
>> It will become problem when we will add secure heaps because we won't
>> be able to distinguish secure
>> processes to standard ones or set specific policy per heaps.
>> Maybe I'm wrong here but I have never see selinux policy checking an
>> ioctl field but if that
>> exist it could be a solution.
>>
> 
> I might be thinking of a different type of "secure", but...
> 
> Should the security of secure heaps be enforced by OS-level
> permissions? I don't know about other architectures, but at least on
> arm/arm64 this is enforced in hardware; it doesn't matter who has
> access to the ion heap, because only secure devices (or the CPU
> running a secure process) is physically able to access the memory
> backing the buffer.
> 
> In fact, in the use-cases I know of, the process asking for the ion
> 

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Laura Abbott]

On 03/12/2017 12:05 PM, Daniel Vetter wrote:
> On Sun, Mar 12, 2017 at 2:34 PM, Benjamin Gaignard
>  wrote:
>> 2017-03-09 18:38 GMT+01:00 Laura Abbott :
>>> On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
 2017-03-06 17:04 GMT+01:00 Daniel Vetter :
> On Mon, Mar 06, 2017 at 11:58:05AM +0100, Mark Brown wrote:
>> On Mon, Mar 06, 2017 at 11:40:41AM +0100, Daniel Vetter wrote:
>>
>>> No one gave a thing about android in upstream, so Greg KH just dumped it
>>> all into staging/android/. We've discussed ION a bunch of times, 
>>> recorded
>>> anything we'd like to fix in staging/android/TODO, and Laura's patch
>>> series here addresses a big chunk of that.
>>
>>> This is pretty much the same approach we (gpu folks) used to de-stage 
>>> the
>>> syncpt stuff.
>>
>> Well, there's also the fact that quite a few people have issues with the
>> design (like Laurent).  It seems like a lot of them have either got more
>> comfortable with it over time, or at least not managed to come up with
>> any better ideas in the meantime.
>
> See the TODO, it has everything a really big group (look at the patch for
> the full Cc: list) figured needs to be improved at LPC 2015. We don't just
> merge stuff because merging stuff is fun :-)
>
> Laurent was even in that group ...
> -Daniel

 For me those patches are going in the right direction.

 I still have few questions:
 - since alignment management has been remove from ion-core, should it
 be also removed from ioctl structure ?
>>>
>>> Yes, I think I'm going to go with the suggestion to fixup the ABI
>>> so we don't need the compat layer and as part of that I'm also
>>> dropping the align argument.
>>>
 - can you we ride off ion_handle (at least in userland) and only
 export a dma-buf descriptor ?
>>>
>>> Yes, I think this is the right direction given we're breaking
>>> everything anyway. I was debating trying to keep the two but
>>> moving to only dma bufs is probably cleaner. The only reason
>>> I could see for keeping the handles is running out of file
>>> descriptors for dma-bufs but that seems unlikely.

 In the future how can we add new heaps ?
 Some platforms have very specific memory allocation
 requirements (just have a look in the number of gem custom allocator in 
 drm)
 Do you plan to add heap type/mask for each ?
>>>
>>> Yes, that was my thinking.
>>
>> My concern is about the policy to adding heaps, will you accept
>> "customs" heap per
>> platforms ? per devices ? or only generic ones ?
>> If you are too strict, we will have lot of out-of-tree heaps and if
>> you accept of of them
>> it will be a nightmare to maintain
> 
> I think ion should expose any heap that's also directly accessible to
> devices using dma_alloc(_coherent). That should leave very few things
> left, like your SMA heap.
> 
>> Another point is how can we put secure rules (like selinux policy) on
>> heaps since all the allocations
>> go to the same device (/dev/ion) ? For example, until now, in Android
>> we have to give the same
>> access rights to all the process that use ION.
>> It will become problem when we will add secure heaps because we won't
>> be able to distinguish secure
>> processes to standard ones or set specific policy per heaps.
>> Maybe I'm wrong here but I have never see selinux policy checking an
>> ioctl field but if that
>> exist it could be a solution.
> 
> Hm, we might want to expose all the heaps as individual
> /dev/ion_$heapname nodes? Should we do this from the start, since
> we're massively revamping the uapi anyway (imo not needed, current
> state seems to work too)?
> -Daniel
> 

I thought about that. One advantage with separate /dev/ion_$heap
is that we don't have to worry about a limit of 32 possible
heaps per system (32-bit heap id allocation field). But dealing
with an ioctl seems easier than names. Userspace might be less
likely to hardcode random id numbers vs. names as well.

Thanks,
Laura

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Mark Brown]

On Mon, Mar 13, 2017 at 10:54:33AM +, Brian Starkey wrote:
> On Sun, Mar 12, 2017 at 02:34:14PM +0100, Benjamin Gaignard wrote:

> > Another point is how can we put secure rules (like selinux policy) on
> > heaps since all the allocations
> > go to the same device (/dev/ion) ? For example, until now, in Android
> > we have to give the same
> > access rights to all the process that use ION.
> > It will become problem when we will add secure heaps because we won't
> > be able to distinguish secure
> > processes to standard ones or set specific policy per heaps.
> > Maybe I'm wrong here but I have never see selinux policy checking an
> > ioctl field but if that
> > exist it could be a solution.

> I might be thinking of a different type of "secure", but...

> Should the security of secure heaps be enforced by OS-level
> permissions? I don't know about other architectures, but at least on
> arm/arm64 this is enforced in hardware; it doesn't matter who has
> access to the ion heap, because only secure devices (or the CPU
> running a secure process) is physically able to access the memory
> backing the buffer.

> In fact, in the use-cases I know of, the process asking for the ion
> allocation is not a secure process, and so we wouldn't *want* to
> restrict the secure heap to be allocated from only by secure
> processes.

I think there's an orthogonal level of OS level security that can be
applied here - it's reasonable for it to want to say things like "only
processes that are supposed to be implementing functionality X should be
able to try to allocate memory set aside for that functionality".  This
mitigates against escallation attacks and so on, it's not really
directly related to secure memory as such though.


signature.asc
Description: PGP signature

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Brian Starkey]

On Sun, Mar 12, 2017 at 02:34:14PM +0100, Benjamin Gaignard wrote:

2017-03-09 18:38 GMT+01:00 Laura Abbott :

On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:

2017-03-06 17:04 GMT+01:00 Daniel Vetter :

On Mon, Mar 06, 2017 at 11:58:05AM +0100, Mark Brown wrote:

On Mon, Mar 06, 2017 at 11:40:41AM +0100, Daniel Vetter wrote:


No one gave a thing about android in upstream, so Greg KH just dumped it
all into staging/android/. We've discussed ION a bunch of times, recorded
anything we'd like to fix in staging/android/TODO, and Laura's patch
series here addresses a big chunk of that.



This is pretty much the same approach we (gpu folks) used to de-stage the
syncpt stuff.


Well, there's also the fact that quite a few people have issues with the
design (like Laurent).  It seems like a lot of them have either got more
comfortable with it over time, or at least not managed to come up with
any better ideas in the meantime.


See the TODO, it has everything a really big group (look at the patch for
the full Cc: list) figured needs to be improved at LPC 2015. We don't just
merge stuff because merging stuff is fun :-)

Laurent was even in that group ...
-Daniel


For me those patches are going in the right direction.

I still have few questions:
- since alignment management has been remove from ion-core, should it
be also removed from ioctl structure ?


Yes, I think I'm going to go with the suggestion to fixup the ABI
so we don't need the compat layer and as part of that I'm also
dropping the align argument.


- can you we ride off ion_handle (at least in userland) and only
export a dma-buf descriptor ?


Yes, I think this is the right direction given we're breaking
everything anyway. I was debating trying to keep the two but
moving to only dma bufs is probably cleaner. The only reason
I could see for keeping the handles is running out of file
descriptors for dma-bufs but that seems unlikely.


In the future how can we add new heaps ?
Some platforms have very specific memory allocation
requirements (just have a look in the number of gem custom allocator in drm)
Do you plan to add heap type/mask for each ?


Yes, that was my thinking.


My concern is about the policy to adding heaps, will you accept
"customs" heap per
platforms ? per devices ? or only generic ones ?
If you are too strict, we will have lot of out-of-tree heaps and if
you accept of of them
it will be a nightmare to maintain



Are you concerned about actual heaps (e.g. a carveout at 0x8000 vs
a carveout at 0x6000) or heap types?

For heap types, I think the policy can be strict - if it's generally
useful then it should live in-tree in ion. Otherwise, it would be
out-of-tree. I'd expect most "custom" heaps to be parameterisable to
the point of being generally useful.

For actual heap instances, I would expect them to be communicated via
reserved-memory regions or something similar, and so the maintenance
burden is pretty low.

The existing query ioctl can allow heap IDs to get assigned
dynamically at runtime, so there's no need to reserve "bit 6" for
"CUSTOM_ACME_HEAP_1"


Another point is how can we put secure rules (like selinux policy) on
heaps since all the allocations
go to the same device (/dev/ion) ? For example, until now, in Android
we have to give the same
access rights to all the process that use ION.
It will become problem when we will add secure heaps because we won't
be able to distinguish secure
processes to standard ones or set specific policy per heaps.
Maybe I'm wrong here but I have never see selinux policy checking an
ioctl field but if that
exist it could be a solution.



I might be thinking of a different type of "secure", but...

Should the security of secure heaps be enforced by OS-level
permissions? I don't know about other architectures, but at least on
arm/arm64 this is enforced in hardware; it doesn't matter who has
access to the ion heap, because only secure devices (or the CPU
running a secure process) is physically able to access the memory
backing the buffer.

In fact, in the use-cases I know of, the process asking for the ion
allocation is not a secure process, and so we wouldn't *want* to
restrict the secure heap to be allocated from only by secure
processes.

-Brian





Benjamin



Thanks,
Laura

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Daniel Vetter]

On Sun, Mar 12, 2017 at 2:34 PM, Benjamin Gaignard
 wrote:
> 2017-03-09 18:38 GMT+01:00 Laura Abbott :
>> On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
>>> 2017-03-06 17:04 GMT+01:00 Daniel Vetter :
 On Mon, Mar 06, 2017 at 11:58:05AM +0100, Mark Brown wrote:
> On Mon, Mar 06, 2017 at 11:40:41AM +0100, Daniel Vetter wrote:
>
>> No one gave a thing about android in upstream, so Greg KH just dumped it
>> all into staging/android/. We've discussed ION a bunch of times, recorded
>> anything we'd like to fix in staging/android/TODO, and Laura's patch
>> series here addresses a big chunk of that.
>
>> This is pretty much the same approach we (gpu folks) used to de-stage the
>> syncpt stuff.
>
> Well, there's also the fact that quite a few people have issues with the
> design (like Laurent).  It seems like a lot of them have either got more
> comfortable with it over time, or at least not managed to come up with
> any better ideas in the meantime.

 See the TODO, it has everything a really big group (look at the patch for
 the full Cc: list) figured needs to be improved at LPC 2015. We don't just
 merge stuff because merging stuff is fun :-)

 Laurent was even in that group ...
 -Daniel
>>>
>>> For me those patches are going in the right direction.
>>>
>>> I still have few questions:
>>> - since alignment management has been remove from ion-core, should it
>>> be also removed from ioctl structure ?
>>
>> Yes, I think I'm going to go with the suggestion to fixup the ABI
>> so we don't need the compat layer and as part of that I'm also
>> dropping the align argument.
>>
>>> - can you we ride off ion_handle (at least in userland) and only
>>> export a dma-buf descriptor ?
>>
>> Yes, I think this is the right direction given we're breaking
>> everything anyway. I was debating trying to keep the two but
>> moving to only dma bufs is probably cleaner. The only reason
>> I could see for keeping the handles is running out of file
>> descriptors for dma-bufs but that seems unlikely.
>>>
>>> In the future how can we add new heaps ?
>>> Some platforms have very specific memory allocation
>>> requirements (just have a look in the number of gem custom allocator in drm)
>>> Do you plan to add heap type/mask for each ?
>>
>> Yes, that was my thinking.
>
> My concern is about the policy to adding heaps, will you accept
> "customs" heap per
> platforms ? per devices ? or only generic ones ?
> If you are too strict, we will have lot of out-of-tree heaps and if
> you accept of of them
> it will be a nightmare to maintain

I think ion should expose any heap that's also directly accessible to
devices using dma_alloc(_coherent). That should leave very few things
left, like your SMA heap.

> Another point is how can we put secure rules (like selinux policy) on
> heaps since all the allocations
> go to the same device (/dev/ion) ? For example, until now, in Android
> we have to give the same
> access rights to all the process that use ION.
> It will become problem when we will add secure heaps because we won't
> be able to distinguish secure
> processes to standard ones or set specific policy per heaps.
> Maybe I'm wrong here but I have never see selinux policy checking an
> ioctl field but if that
> exist it could be a solution.

Hm, we might want to expose all the heaps as individual
/dev/ion_$heapname nodes? Should we do this from the start, since
we're massively revamping the uapi anyway (imo not needed, current
state seems to work too)?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Benjamin Gaignard]

2017-03-09 18:38 GMT+01:00 Laura Abbott :
> On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
>> 2017-03-06 17:04 GMT+01:00 Daniel Vetter :
>>> On Mon, Mar 06, 2017 at 11:58:05AM +0100, Mark Brown wrote:
 On Mon, Mar 06, 2017 at 11:40:41AM +0100, Daniel Vetter wrote:

> No one gave a thing about android in upstream, so Greg KH just dumped it
> all into staging/android/. We've discussed ION a bunch of times, recorded
> anything we'd like to fix in staging/android/TODO, and Laura's patch
> series here addresses a big chunk of that.

> This is pretty much the same approach we (gpu folks) used to de-stage the
> syncpt stuff.

 Well, there's also the fact that quite a few people have issues with the
 design (like Laurent).  It seems like a lot of them have either got more
 comfortable with it over time, or at least not managed to come up with
 any better ideas in the meantime.
>>>
>>> See the TODO, it has everything a really big group (look at the patch for
>>> the full Cc: list) figured needs to be improved at LPC 2015. We don't just
>>> merge stuff because merging stuff is fun :-)
>>>
>>> Laurent was even in that group ...
>>> -Daniel
>>
>> For me those patches are going in the right direction.
>>
>> I still have few questions:
>> - since alignment management has been remove from ion-core, should it
>> be also removed from ioctl structure ?
>
> Yes, I think I'm going to go with the suggestion to fixup the ABI
> so we don't need the compat layer and as part of that I'm also
> dropping the align argument.
>
>> - can you we ride off ion_handle (at least in userland) and only
>> export a dma-buf descriptor ?
>
> Yes, I think this is the right direction given we're breaking
> everything anyway. I was debating trying to keep the two but
> moving to only dma bufs is probably cleaner. The only reason
> I could see for keeping the handles is running out of file
> descriptors for dma-bufs but that seems unlikely.
>>
>> In the future how can we add new heaps ?
>> Some platforms have very specific memory allocation
>> requirements (just have a look in the number of gem custom allocator in drm)
>> Do you plan to add heap type/mask for each ?
>
> Yes, that was my thinking.

My concern is about the policy to adding heaps, will you accept
"customs" heap per
platforms ? per devices ? or only generic ones ?
If you are too strict, we will have lot of out-of-tree heaps and if
you accept of of them
it will be a nightmare to maintain

Another point is how can we put secure rules (like selinux policy) on
heaps since all the allocations
go to the same device (/dev/ion) ? For example, until now, in Android
we have to give the same
access rights to all the process that use ION.
It will become problem when we will add secure heaps because we won't
be able to distinguish secure
processes to standard ones or set specific policy per heaps.
Maybe I'm wrong here but I have never see selinux policy checking an
ioctl field but if that
exist it could be a solution.

>
>>
>> Benjamin
>>
>
> Thanks,
> Laura
>

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Laura Abbott]

On 03/10/2017 06:27 AM, Brian Starkey wrote:
> On Fri, Mar 10, 2017 at 11:46:42AM +, Robin Murphy wrote:
>> On 10/03/17 10:31, Brian Starkey wrote:
>>> Hi,
>>>
>>> On Thu, Mar 09, 2017 at 09:38:49AM -0800, Laura Abbott wrote:
 On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
>>>
>>> [snip]
>>>
>
> For me those patches are going in the right direction.
>
> I still have few questions:
> - since alignment management has been remove from ion-core, should it
> be also removed from ioctl structure ?

 Yes, I think I'm going to go with the suggestion to fixup the ABI
 so we don't need the compat layer and as part of that I'm also
 dropping the align argument.

>>>
>>> Is the only motivation for removing the alignment parameter that
>>> no-one got around to using it for something useful yet?
>>> The original comment was true - different devices do have different
>>> alignment requirements.
>>>
>>> Better alignment can help SMMUs use larger blocks when mapping,
>>> reducing TLB pressure and the chance of a page table walk causing
>>> display underruns.
>>
>> For that use-case, though, alignment alone doesn't necessarily help -
>> you need the whole allocation granularity to match your block size (i.e.
>> given a 1MB block size, asking for 17KB and getting back 17KB starting
>> at a 1MB boundary doesn't help much - that whole 1MB needs to be
>> allocated and everyone needs to know it to ensure that the whole lot can
>> be mapped safely). Now, whether it's down to the callers or the heap
>> implementations to decide and enforce that granularity is another
>> question, but provided allocations are at least naturally aligned to
>> whatever the granularity is (which is a reasonable assumption to bake
>> in) then it's all good.
>>
>> Robin.
> 
> Agreed, alignment alone isn't enough. But lets assume that an app
> knows what a "good" granularity is, and always asks for allocation
> sizes which are suitably rounded to allow blocks to be used. Currently
> it looks like a "standard" ION_HEAP_TYPE_CARVEOUT heap would give me
> back just a PAGE_SIZE aligned buffer. So even *if* the caller knows
> its desired block size, there's no way for it to get guaranteed better
> alignment, which wouldn't be a bad feature to have.
> 
> Anyway as Daniel and Rob say, if the interface is designed properly
> this kind of extension would be possible later, or you can have a
> special heap with a larger granule.
> 
> I suppose it makes sense to remove it while there's no-one actually
> implementing it, in case an alternate method proves more usable.
> 
> -Brian

Part of the reason I want to remove it is to avoid confusion over
callers thinking it will do anything on most heaps. I agree being
able to specify a larger granularity would be beneficial but I
don't think a dedicated field in the ABI is the right approach.

Thanks,
Laura

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Brian Starkey]

On Fri, Mar 10, 2017 at 11:46:42AM +, Robin Murphy wrote:

On 10/03/17 10:31, Brian Starkey wrote:

Hi,

On Thu, Mar 09, 2017 at 09:38:49AM -0800, Laura Abbott wrote:

On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:


[snip]



For me those patches are going in the right direction.

I still have few questions:
- since alignment management has been remove from ion-core, should it
be also removed from ioctl structure ?


Yes, I think I'm going to go with the suggestion to fixup the ABI
so we don't need the compat layer and as part of that I'm also
dropping the align argument.



Is the only motivation for removing the alignment parameter that
no-one got around to using it for something useful yet?
The original comment was true - different devices do have different
alignment requirements.

Better alignment can help SMMUs use larger blocks when mapping,
reducing TLB pressure and the chance of a page table walk causing
display underruns.


For that use-case, though, alignment alone doesn't necessarily help -
you need the whole allocation granularity to match your block size (i.e.
given a 1MB block size, asking for 17KB and getting back 17KB starting
at a 1MB boundary doesn't help much - that whole 1MB needs to be
allocated and everyone needs to know it to ensure that the whole lot can
be mapped safely). Now, whether it's down to the callers or the heap
implementations to decide and enforce that granularity is another
question, but provided allocations are at least naturally aligned to
whatever the granularity is (which is a reasonable assumption to bake
in) then it's all good.

Robin.


Agreed, alignment alone isn't enough. But lets assume that an app
knows what a "good" granularity is, and always asks for allocation
sizes which are suitably rounded to allow blocks to be used. Currently
it looks like a "standard" ION_HEAP_TYPE_CARVEOUT heap would give me
back just a PAGE_SIZE aligned buffer. So even *if* the caller knows
its desired block size, there's no way for it to get guaranteed better
alignment, which wouldn't be a bad feature to have.

Anyway as Daniel and Rob say, if the interface is designed properly
this kind of extension would be possible later, or you can have a
special heap with a larger granule.

I suppose it makes sense to remove it while there's no-one actually
implementing it, in case an alternate method proves more usable.

-Brian





-Brian

___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Rob Clark]

On Fri, Mar 10, 2017 at 7:40 AM, Daniel Vetter  wrote:
> On Fri, Mar 10, 2017 at 10:31:13AM +, Brian Starkey wrote:
>> Hi,
>>
>> On Thu, Mar 09, 2017 at 09:38:49AM -0800, Laura Abbott wrote:
>> > On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
>>
>> [snip]
>>
>> > >
>> > > For me those patches are going in the right direction.
>> > >
>> > > I still have few questions:
>> > > - since alignment management has been remove from ion-core, should it
>> > > be also removed from ioctl structure ?
>> >
>> > Yes, I think I'm going to go with the suggestion to fixup the ABI
>> > so we don't need the compat layer and as part of that I'm also
>> > dropping the align argument.
>> >
>>
>> Is the only motivation for removing the alignment parameter that
>> no-one got around to using it for something useful yet?
>> The original comment was true - different devices do have different
>> alignment requirements.
>>
>> Better alignment can help SMMUs use larger blocks when mapping,
>> reducing TLB pressure and the chance of a page table walk causing
>> display underruns.
>
> Extending ioctl uapi is easy, trying to get rid of bad uapi is much
> harder. Given that right now we don't have an ion allocator that does
> alignment I think removing it makes sense. And if we go with lots of
> heaps, we might as well have an ion heap per alignment that your hw needs,
> so there's different ways to implement this in the future.

slight correction:  if you plan ahead (and do things like zero init if
userspace passes in a smaller ioctl struct like drm_ioctl does),
extending ioctl uapi is easy.. might be something worth fixing from
the get-go..

BR,
-R

> At least from the unix device memory allocator pov it's probably simpler
> to encode stuff like this into the heap name, instead of having to pass
> heap + list of additional properties/constraints.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Daniel Vetter]

On Fri, Mar 10, 2017 at 10:31:13AM +, Brian Starkey wrote:
> Hi,
> 
> On Thu, Mar 09, 2017 at 09:38:49AM -0800, Laura Abbott wrote:
> > On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
> 
> [snip]
> 
> > > 
> > > For me those patches are going in the right direction.
> > > 
> > > I still have few questions:
> > > - since alignment management has been remove from ion-core, should it
> > > be also removed from ioctl structure ?
> > 
> > Yes, I think I'm going to go with the suggestion to fixup the ABI
> > so we don't need the compat layer and as part of that I'm also
> > dropping the align argument.
> > 
> 
> Is the only motivation for removing the alignment parameter that
> no-one got around to using it for something useful yet?
> The original comment was true - different devices do have different
> alignment requirements.
> 
> Better alignment can help SMMUs use larger blocks when mapping,
> reducing TLB pressure and the chance of a page table walk causing
> display underruns.

Extending ioctl uapi is easy, trying to get rid of bad uapi is much
harder. Given that right now we don't have an ion allocator that does
alignment I think removing it makes sense. And if we go with lots of
heaps, we might as well have an ion heap per alignment that your hw needs,
so there's different ways to implement this in the future.

At least from the unix device memory allocator pov it's probably simpler
to encode stuff like this into the heap name, instead of having to pass
heap + list of additional properties/constraints.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Robin Murphy]

On 10/03/17 10:31, Brian Starkey wrote:
> Hi,
> 
> On Thu, Mar 09, 2017 at 09:38:49AM -0800, Laura Abbott wrote:
>> On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
> 
> [snip]
> 
>>>
>>> For me those patches are going in the right direction.
>>>
>>> I still have few questions:
>>> - since alignment management has been remove from ion-core, should it
>>> be also removed from ioctl structure ?
>>
>> Yes, I think I'm going to go with the suggestion to fixup the ABI
>> so we don't need the compat layer and as part of that I'm also
>> dropping the align argument.
>>
> 
> Is the only motivation for removing the alignment parameter that
> no-one got around to using it for something useful yet?
> The original comment was true - different devices do have different
> alignment requirements.
> 
> Better alignment can help SMMUs use larger blocks when mapping,
> reducing TLB pressure and the chance of a page table walk causing
> display underruns.

For that use-case, though, alignment alone doesn't necessarily help -
you need the whole allocation granularity to match your block size (i.e.
given a 1MB block size, asking for 17KB and getting back 17KB starting
at a 1MB boundary doesn't help much - that whole 1MB needs to be
allocated and everyone needs to know it to ensure that the whole lot can
be mapped safely). Now, whether it's down to the callers or the heap
implementations to decide and enforce that granularity is another
question, but provided allocations are at least naturally aligned to
whatever the granularity is (which is a reasonable assumption to bake
in) then it's all good.

Robin.

> 
> -Brian
> 
> ___
> linux-arm-kernel mailing list
> linux-arm-ker...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Brian Starkey]

Hi,

On Thu, Mar 09, 2017 at 09:38:49AM -0800, Laura Abbott wrote:

On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:


[snip]



For me those patches are going in the right direction.

I still have few questions:
- since alignment management has been remove from ion-core, should it
be also removed from ioctl structure ?


Yes, I think I'm going to go with the suggestion to fixup the ABI
so we don't need the compat layer and as part of that I'm also
dropping the align argument.



Is the only motivation for removing the alignment parameter that
no-one got around to using it for something useful yet?
The original comment was true - different devices do have different
alignment requirements.

Better alignment can help SMMUs use larger blocks when mapping,
reducing TLB pressure and the chance of a page table walk causing
display underruns.

-Brian

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Laura Abbott]

On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
> 2017-03-06 17:04 GMT+01:00 Daniel Vetter :
>> On Mon, Mar 06, 2017 at 11:58:05AM +0100, Mark Brown wrote:
>>> On Mon, Mar 06, 2017 at 11:40:41AM +0100, Daniel Vetter wrote:
>>>
 No one gave a thing about android in upstream, so Greg KH just dumped it
 all into staging/android/. We've discussed ION a bunch of times, recorded
 anything we'd like to fix in staging/android/TODO, and Laura's patch
 series here addresses a big chunk of that.
>>>
 This is pretty much the same approach we (gpu folks) used to de-stage the
 syncpt stuff.
>>>
>>> Well, there's also the fact that quite a few people have issues with the
>>> design (like Laurent).  It seems like a lot of them have either got more
>>> comfortable with it over time, or at least not managed to come up with
>>> any better ideas in the meantime.
>>
>> See the TODO, it has everything a really big group (look at the patch for
>> the full Cc: list) figured needs to be improved at LPC 2015. We don't just
>> merge stuff because merging stuff is fun :-)
>>
>> Laurent was even in that group ...
>> -Daniel
> 
> For me those patches are going in the right direction.
> 
> I still have few questions:
> - since alignment management has been remove from ion-core, should it
> be also removed from ioctl structure ?

Yes, I think I'm going to go with the suggestion to fixup the ABI
so we don't need the compat layer and as part of that I'm also
dropping the align argument.

> - can you we ride off ion_handle (at least in userland) and only
> export a dma-buf descriptor ?

Yes, I think this is the right direction given we're breaking
everything anyway. I was debating trying to keep the two but
moving to only dma bufs is probably cleaner. The only reason
I could see for keeping the handles is running out of file
descriptors for dma-bufs but that seems unlikely.
> 
> In the future how can we add new heaps ?
> Some platforms have very specific memory allocation
> requirements (just have a look in the number of gem custom allocator in drm)
> Do you plan to add heap type/mask for each ?

Yes, that was my thinking. 

> 
> Benjamin
> 

Thanks,
Laura

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Benjamin Gaignard]

2017-03-06 17:04 GMT+01:00 Daniel Vetter :
> On Mon, Mar 06, 2017 at 11:58:05AM +0100, Mark Brown wrote:
>> On Mon, Mar 06, 2017 at 11:40:41AM +0100, Daniel Vetter wrote:
>>
>> > No one gave a thing about android in upstream, so Greg KH just dumped it
>> > all into staging/android/. We've discussed ION a bunch of times, recorded
>> > anything we'd like to fix in staging/android/TODO, and Laura's patch
>> > series here addresses a big chunk of that.
>>
>> > This is pretty much the same approach we (gpu folks) used to de-stage the
>> > syncpt stuff.
>>
>> Well, there's also the fact that quite a few people have issues with the
>> design (like Laurent).  It seems like a lot of them have either got more
>> comfortable with it over time, or at least not managed to come up with
>> any better ideas in the meantime.
>
> See the TODO, it has everything a really big group (look at the patch for
> the full Cc: list) figured needs to be improved at LPC 2015. We don't just
> merge stuff because merging stuff is fun :-)
>
> Laurent was even in that group ...
> -Daniel

For me those patches are going in the right direction.

I still have few questions:
- since alignment management has been remove from ion-core, should it
be also removed from ioctl structure ?
- can you we ride off ion_handle (at least in userland) and only
export a dma-buf descriptor ?

In the future how can we add new heaps ?
Some platforms have very specific memory allocation
requirements (just have a look in the number of gem custom allocator in drm)
Do you plan to add heap type/mask for each ?

Benjamin

> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch


Follow Linaro: Facebook | Twitter | Blog

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Daniel Vetter]

On Mon, Mar 06, 2017 at 11:58:05AM +0100, Mark Brown wrote:
> On Mon, Mar 06, 2017 at 11:40:41AM +0100, Daniel Vetter wrote:
> 
> > No one gave a thing about android in upstream, so Greg KH just dumped it
> > all into staging/android/. We've discussed ION a bunch of times, recorded
> > anything we'd like to fix in staging/android/TODO, and Laura's patch
> > series here addresses a big chunk of that.
> 
> > This is pretty much the same approach we (gpu folks) used to de-stage the
> > syncpt stuff.
> 
> Well, there's also the fact that quite a few people have issues with the
> design (like Laurent).  It seems like a lot of them have either got more
> comfortable with it over time, or at least not managed to come up with
> any better ideas in the meantime.

See the TODO, it has everything a really big group (look at the patch for
the full Cc: list) figured needs to be improved at LPC 2015. We don't just
merge stuff because merging stuff is fun :-)

Laurent was even in that group ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Daniel Vetter]

On Mon, Mar 06, 2017 at 05:02:05PM +0200, Laurent Pinchart wrote:
> Hi Daniel,
> 
> On Monday 06 Mar 2017 11:38:20 Daniel Vetter wrote:
> > On Fri, Mar 03, 2017 at 06:45:40PM +0200, Laurent Pinchart wrote:
> > > - I haven't seen any proposal how a heap-based solution could be used in a
> > > generic distribution. This needs to be figured out before committing to
> > > any API/ABI.
> > 
> > Two replies from my side:
> > 
> > - Just because a patch doesn't solve world hunger isn't really a good
> >   reason to reject it.
> 
> As long as it goes in the right direction, sure :-) The points I mentioned 
> were to be interpreted that way, I want to make sure we're not going in a 
> dead-end (or worse, driving full speed into a wall).
> 
> > - Heap doesn't mean its not resizeable (but I'm not sure that's really
> >   your concern).
> 
> Not really, no. Heap is another word to mean pool here. It might not be the 
> best term in this context as it has a precise meaning in the context of 
> memory 
> allocation, but that's a detail.
> 
> > - Imo ION is very much part of the picture here to solve this for real. We
> >   need to bits:
> > 
> >   * Be able to allocate memory from specific pools, not going through a
> > specific driver. ION gives us that interface. This is e.g. also needed
> > for "special" memory, like SMA tries to expose.
> > 
> >   * Some way to figure out how to allocate the buffer object. This
> > is purely a userspace problem, and this is the part the unix memory
> > allocator tries to solve. There's no plans in there for big kernel
> > changes, instead userspace does a dance to reconcile all the
> > constraints, and one of the constraints might be "you have to allocate
> > this from this special ION heap". The only thing the kernel needs to
> > expose is which devices use which ION heaps (we kinda do that
> > already), and maybe some hints of how they can be generalized (but I
> > guess stuff like "minimal pagesize of x KB" is also fulfilled by any
> > CMA heap is knowledge userspace needs).
> 
> The constraint solver could live in userspace, I'm open to a solution that 
> would go in that direction, but it will require help from the kernel to fetch 
> the constraints from the devices that need to be involved in buffer sharing.
> 
> Given a userspace constraint resolver, the interface with the kernel 
> allocator 
> will likely be based on pools. I'm not opposed to that, as long as pool are 
> identified by opaque handles. I don't want userspace to know about the 
> meaning 
> of any particular ION heap. Application must not attempt to "allocate from 
> CMA" for instance, that would lock us to a crazy API that will grow 
> completely 
> out of hands as vendors will start adding all kind of custom heaps, and 
> applications will have to follow (or will be patched out-of-tree by vendors).
> 
> > Again I think waiting for this to be fully implemented before we merge any
> > part is going to just kill any upstreaming efforts. ION in itself, without
> > the full buffer negotiation dance seems clearly useful (also for stuff
> > like SMA), and having it merged will help with moving the buffer
> > allocation dance forward.
> 
> Again I'm not opposed to a kernel allocator based on pools/heaps, as long as
> 
> - pools/heaps stay internal to the kernel and are not directly exposed to 
> userspace

Agreed (and I think ION doesn't have fixed pools afaik, just kinda
conventions, at least after Laura's patches). But on a fixed board with a
fixed DT (for the cma regions) and fixed .config (for the generic heaps)
you can hardcode your heaps. You'll make your code non-portable, but hey
that's not our problem imo. E.g. board-specific code can also hard-code
how to wire connectors and which one is which in kms (and I've seen this).
I don't think the possibility of abusing the uabi should be a good reason
to prevent it from merging. Anything that provides something with indirect
connections can be abused by hardcoding the names or the indizes.

We do have a TODO entry that talks about exposing the device -> cma heap
link in sysfs or somewhere. I'm not versed enough to know whether Laura's
patches fixed that, this here mostly seems to tackle the fundamentals of
the dma api abuse first.

> - a reasonable way to size the different kinds of pools in a generic 
> distribution kernel can be found

So for the CMA heaps, you can't resize them at runtime, for obvious
reasons. For boot-time you can adjust them through DT, and I thought
everyone agreed that for different use-cases you might need to adjust your
reserved regions.

For all other heaps, they just use the normal allocator functions
(e.g. alloc_pages). There's not limit on those except OOM, so nothing to
adjust really.

I guess I'm still not entirely clear on your "memory pool" concern ... If
it's just the word, we have lots of auto-resizing heaps/pools all around.
And if it's just sizing, I think that's already solved as 

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Laurent Pinchart]

Hi Daniel,

On Monday 06 Mar 2017 11:38:20 Daniel Vetter wrote:
> On Fri, Mar 03, 2017 at 06:45:40PM +0200, Laurent Pinchart wrote:
> > - I haven't seen any proposal how a heap-based solution could be used in a
> > generic distribution. This needs to be figured out before committing to
> > any API/ABI.
> 
> Two replies from my side:
> 
> - Just because a patch doesn't solve world hunger isn't really a good
>   reason to reject it.

As long as it goes in the right direction, sure :-) The points I mentioned 
were to be interpreted that way, I want to make sure we're not going in a 
dead-end (or worse, driving full speed into a wall).

> - Heap doesn't mean its not resizeable (but I'm not sure that's really
>   your concern).

Not really, no. Heap is another word to mean pool here. It might not be the 
best term in this context as it has a precise meaning in the context of memory 
allocation, but that's a detail.

> - Imo ION is very much part of the picture here to solve this for real. We
>   need to bits:
> 
>   * Be able to allocate memory from specific pools, not going through a
> specific driver. ION gives us that interface. This is e.g. also needed
> for "special" memory, like SMA tries to expose.
> 
>   * Some way to figure out how to allocate the buffer object. This
> is purely a userspace problem, and this is the part the unix memory
> allocator tries to solve. There's no plans in there for big kernel
> changes, instead userspace does a dance to reconcile all the
> constraints, and one of the constraints might be "you have to allocate
> this from this special ION heap". The only thing the kernel needs to
> expose is which devices use which ION heaps (we kinda do that
> already), and maybe some hints of how they can be generalized (but I
> guess stuff like "minimal pagesize of x KB" is also fulfilled by any
> CMA heap is knowledge userspace needs).

The constraint solver could live in userspace, I'm open to a solution that 
would go in that direction, but it will require help from the kernel to fetch 
the constraints from the devices that need to be involved in buffer sharing.

Given a userspace constraint resolver, the interface with the kernel allocator 
will likely be based on pools. I'm not opposed to that, as long as pool are 
identified by opaque handles. I don't want userspace to know about the meaning 
of any particular ION heap. Application must not attempt to "allocate from 
CMA" for instance, that would lock us to a crazy API that will grow completely 
out of hands as vendors will start adding all kind of custom heaps, and 
applications will have to follow (or will be patched out-of-tree by vendors).

> Again I think waiting for this to be fully implemented before we merge any
> part is going to just kill any upstreaming efforts. ION in itself, without
> the full buffer negotiation dance seems clearly useful (also for stuff
> like SMA), and having it merged will help with moving the buffer
> allocation dance forward.

Again I'm not opposed to a kernel allocator based on pools/heaps, as long as

- pools/heaps stay internal to the kernel and are not directly exposed to 
userspace

- a reasonable way to size the different kinds of pools in a generic 
distribution kernel can be found

-- 
Regards,

Laurent Pinchart

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Michal Hocko]

On Mon 06-03-17 11:40:41, Daniel Vetter wrote:
> On Mon, Mar 06, 2017 at 08:42:59AM +0100, Michal Hocko wrote:
> > On Fri 03-03-17 09:37:55, Laura Abbott wrote:
> > > On 03/03/2017 05:29 AM, Michal Hocko wrote:
> > > > On Thu 02-03-17 13:44:32, Laura Abbott wrote:
> > > >> Hi,
> > > >>
> > > >> There's been some recent discussions[1] about Ion-like frameworks. 
> > > >> There's
> > > >> apparently interest in just keeping Ion since it works reasonablly 
> > > >> well.
> > > >> This series does what should be the final clean ups for it to possibly 
> > > >> be
> > > >> moved out of staging.
> > > >>
> > > >> This includes the following:
> > > >> - Some general clean up and removal of features that never got a lot 
> > > >> of use
> > > >>   as far as I can tell.
> > > >> - Fixing up the caching. This is the series I proposed back in 
> > > >> December[2]
> > > >>   but never heard any feedback on. It will certainly break existing
> > > >>   applications that rely on the implicit caching. I'd rather make an 
> > > >> effort
> > > >>   to move to a model that isn't going directly against the 
> > > >> establishement
> > > >>   though.
> > > >> - Fixing up the platform support. The devicetree approach was never 
> > > >> well
> > > >>   recieved by DT maintainers. The proposal here is to think of Ion 
> > > >> less as
> > > >>   specifying requirements and more of a framework for exposing memory 
> > > >> to
> > > >>   userspace.
> > > >> - CMA allocations now happen without the need of a dummy device 
> > > >> structure.
> > > >>   This fixes a bunch of the reasons why I attempted to add devicetree
> > > >>   support before.
> > > >>
> > > >> I've had problems getting feedback in the past so if I don't hear any 
> > > >> major
> > > >> objections I'm going to send out with the RFC dropped to be picked up.
> > > >> The only reason there isn't a patch to come out of staging is to 
> > > >> discuss any
> > > >> other changes to the ABI people might want. Once this comes out of 
> > > >> staging,
> > > >> I really don't want to mess with the ABI.
> > > > 
> > > > Could you recapitulate concerns preventing the code being merged
> > > > normally rather than through the staging tree and how they were
> > > > addressed?
> > > > 
> > > 
> > > Sorry, I'm really not understanding your question here, can you
> > > clarify?
> > 
> > There must have been a reason why this code ended up in the staging
> > tree, right? So my question is what those reasons were and how they were
> > handled in order to move the code from the staging subtree.
> 
> No one gave a thing about android in upstream, so Greg KH just dumped it
> all into staging/android/. We've discussed ION a bunch of times, recorded
> anything we'd like to fix in staging/android/TODO, and Laura's patch
> series here addresses a big chunk of that.

Thanks for the TODO reference. I was looking exactly at something like
that in drivers/staging/android/ion/. To bad I didn't look one directory
up.

Thanks for the clarification!

-- 
Michal Hocko
SUSE Labs

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Mark Brown]

On Mon, Mar 06, 2017 at 11:40:41AM +0100, Daniel Vetter wrote:

> No one gave a thing about android in upstream, so Greg KH just dumped it
> all into staging/android/. We've discussed ION a bunch of times, recorded
> anything we'd like to fix in staging/android/TODO, and Laura's patch
> series here addresses a big chunk of that.

> This is pretty much the same approach we (gpu folks) used to de-stage the
> syncpt stuff.

Well, there's also the fact that quite a few people have issues with the
design (like Laurent).  It seems like a lot of them have either got more
comfortable with it over time, or at least not managed to come up with
any better ideas in the meantime.


signature.asc
Description: PGP signature

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Daniel Vetter]

On Mon, Mar 06, 2017 at 08:42:59AM +0100, Michal Hocko wrote:
> On Fri 03-03-17 09:37:55, Laura Abbott wrote:
> > On 03/03/2017 05:29 AM, Michal Hocko wrote:
> > > On Thu 02-03-17 13:44:32, Laura Abbott wrote:
> > >> Hi,
> > >>
> > >> There's been some recent discussions[1] about Ion-like frameworks. 
> > >> There's
> > >> apparently interest in just keeping Ion since it works reasonablly well.
> > >> This series does what should be the final clean ups for it to possibly be
> > >> moved out of staging.
> > >>
> > >> This includes the following:
> > >> - Some general clean up and removal of features that never got a lot of 
> > >> use
> > >>   as far as I can tell.
> > >> - Fixing up the caching. This is the series I proposed back in 
> > >> December[2]
> > >>   but never heard any feedback on. It will certainly break existing
> > >>   applications that rely on the implicit caching. I'd rather make an 
> > >> effort
> > >>   to move to a model that isn't going directly against the establishement
> > >>   though.
> > >> - Fixing up the platform support. The devicetree approach was never well
> > >>   recieved by DT maintainers. The proposal here is to think of Ion less 
> > >> as
> > >>   specifying requirements and more of a framework for exposing memory to
> > >>   userspace.
> > >> - CMA allocations now happen without the need of a dummy device 
> > >> structure.
> > >>   This fixes a bunch of the reasons why I attempted to add devicetree
> > >>   support before.
> > >>
> > >> I've had problems getting feedback in the past so if I don't hear any 
> > >> major
> > >> objections I'm going to send out with the RFC dropped to be picked up.
> > >> The only reason there isn't a patch to come out of staging is to discuss 
> > >> any
> > >> other changes to the ABI people might want. Once this comes out of 
> > >> staging,
> > >> I really don't want to mess with the ABI.
> > > 
> > > Could you recapitulate concerns preventing the code being merged
> > > normally rather than through the staging tree and how they were
> > > addressed?
> > > 
> > 
> > Sorry, I'm really not understanding your question here, can you
> > clarify?
> 
> There must have been a reason why this code ended up in the staging
> tree, right? So my question is what those reasons were and how they were
> handled in order to move the code from the staging subtree.

No one gave a thing about android in upstream, so Greg KH just dumped it
all into staging/android/. We've discussed ION a bunch of times, recorded
anything we'd like to fix in staging/android/TODO, and Laura's patch
series here addresses a big chunk of that.

This is pretty much the same approach we (gpu folks) used to de-stage the
syncpt stuff.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Daniel Vetter]

On Fri, Mar 03, 2017 at 06:45:40PM +0200, Laurent Pinchart wrote:
> - I haven't seen any proposal how a heap-based solution could be used in a 
> generic distribution. This needs to be figured out before committing to any 
> API/ABI.

Two replies from my side:

- Just because a patch doesn't solve world hunger isn't really a good
  reason to reject it.

- Heap doesn't mean its not resizeable (but I'm not sure that's really
  your concern).

- Imo ION is very much part of the picture here to solve this for real. We
  need to bits:

  * Be able to allocate memory from specific pools, not going through a
specific driver. ION gives us that interface. This is e.g. also needed
for "special" memory, like SMA tries to expose.

  * Some way to figure out how to allocate the buffer object. This
is purely a userspace problem, and this is the part the unix memory
allocator tries to solve. There's no plans in there for big kernel
changes, instead userspace does a dance to reconcile all the
constraints, and one of the constraints might be "you have to allocate
this from this special ION heap". The only thing the kernel needs to
expose is which devices use which ION heaps (we kinda do that
already), and maybe some hints of how they can be generalized (but I
guess stuff like "minimal pagesize of x KB" is also fulfilled by any
CMA heap is knowledge userspace needs).

Again I think waiting for this to be fully implemented before we merge any
part is going to just kill any upstreaming efforts. ION in itself, without
the full buffer negotiation dance seems clearly useful (also for stuff
like SMA), and having it merged will help with moving the buffer
allocation dance forward.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Michal Hocko]

On Fri 03-03-17 09:37:55, Laura Abbott wrote:
> On 03/03/2017 05:29 AM, Michal Hocko wrote:
> > On Thu 02-03-17 13:44:32, Laura Abbott wrote:
> >> Hi,
> >>
> >> There's been some recent discussions[1] about Ion-like frameworks. There's
> >> apparently interest in just keeping Ion since it works reasonablly well.
> >> This series does what should be the final clean ups for it to possibly be
> >> moved out of staging.
> >>
> >> This includes the following:
> >> - Some general clean up and removal of features that never got a lot of use
> >>   as far as I can tell.
> >> - Fixing up the caching. This is the series I proposed back in December[2]
> >>   but never heard any feedback on. It will certainly break existing
> >>   applications that rely on the implicit caching. I'd rather make an effort
> >>   to move to a model that isn't going directly against the establishement
> >>   though.
> >> - Fixing up the platform support. The devicetree approach was never well
> >>   recieved by DT maintainers. The proposal here is to think of Ion less as
> >>   specifying requirements and more of a framework for exposing memory to
> >>   userspace.
> >> - CMA allocations now happen without the need of a dummy device structure.
> >>   This fixes a bunch of the reasons why I attempted to add devicetree
> >>   support before.
> >>
> >> I've had problems getting feedback in the past so if I don't hear any major
> >> objections I'm going to send out with the RFC dropped to be picked up.
> >> The only reason there isn't a patch to come out of staging is to discuss 
> >> any
> >> other changes to the ABI people might want. Once this comes out of staging,
> >> I really don't want to mess with the ABI.
> > 
> > Could you recapitulate concerns preventing the code being merged
> > normally rather than through the staging tree and how they were
> > addressed?
> > 
> 
> Sorry, I'm really not understanding your question here, can you
> clarify?

There must have been a reason why this code ended up in the staging
tree, right? So my question is what those reasons were and how they were
handled in order to move the code from the staging subtree.
-- 
Michal Hocko
SUSE Labs

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Laura Abbott]

On 03/03/2017 05:29 AM, Michal Hocko wrote:
> On Thu 02-03-17 13:44:32, Laura Abbott wrote:
>> Hi,
>>
>> There's been some recent discussions[1] about Ion-like frameworks. There's
>> apparently interest in just keeping Ion since it works reasonablly well.
>> This series does what should be the final clean ups for it to possibly be
>> moved out of staging.
>>
>> This includes the following:
>> - Some general clean up and removal of features that never got a lot of use
>>   as far as I can tell.
>> - Fixing up the caching. This is the series I proposed back in December[2]
>>   but never heard any feedback on. It will certainly break existing
>>   applications that rely on the implicit caching. I'd rather make an effort
>>   to move to a model that isn't going directly against the establishement
>>   though.
>> - Fixing up the platform support. The devicetree approach was never well
>>   recieved by DT maintainers. The proposal here is to think of Ion less as
>>   specifying requirements and more of a framework for exposing memory to
>>   userspace.
>> - CMA allocations now happen without the need of a dummy device structure.
>>   This fixes a bunch of the reasons why I attempted to add devicetree
>>   support before.
>>
>> I've had problems getting feedback in the past so if I don't hear any major
>> objections I'm going to send out with the RFC dropped to be picked up.
>> The only reason there isn't a patch to come out of staging is to discuss any
>> other changes to the ABI people might want. Once this comes out of staging,
>> I really don't want to mess with the ABI.
> 
> Could you recapitulate concerns preventing the code being merged
> normally rather than through the staging tree and how they were
> addressed?
> 

Sorry, I'm really not understanding your question here, can you
clarify?

Thanks,
Laura

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Laura Abbott]

On 03/03/2017 08:45 AM, Laurent Pinchart wrote:
> Hi Daniel,
> 
> On Friday 03 Mar 2017 11:04:33 Daniel Vetter wrote:
>> On Thu, Mar 02, 2017 at 01:44:32PM -0800, Laura Abbott wrote:
>>> Hi,
>>>
>>> There's been some recent discussions[1] about Ion-like frameworks. There's
>>> apparently interest in just keeping Ion since it works reasonablly well.
>>> This series does what should be the final clean ups for it to possibly be
>>> moved out of staging.
>>>
>>> This includes the following:
>>> - Some general clean up and removal of features that never got a lot of
>>>   use as far as I can tell.
>>>
>>> - Fixing up the caching. This is the series I proposed back in December[2]
>>>   but never heard any feedback on. It will certainly break existing
>>>   applications that rely on the implicit caching. I'd rather make an
>>>   effort to move to a model that isn't going directly against the
>>>   establishement though.
>>>
>>> - Fixing up the platform support. The devicetree approach was never well
>>>   recieved by DT maintainers. The proposal here is to think of Ion less as
>>>   specifying requirements and more of a framework for exposing memory to
>>>   userspace.
>>>
>>> - CMA allocations now happen without the need of a dummy device structure.
>>>   This fixes a bunch of the reasons why I attempted to add devicetree
>>>   support before.
>>>
>>> I've had problems getting feedback in the past so if I don't hear any
>>> major objections I'm going to send out with the RFC dropped to be picked
>>> up. The only reason there isn't a patch to come out of staging is to
>>> discuss any other changes to the ABI people might want. Once this comes
>>> out of staging, I really don't want to mess with the ABI.
>>>
>>> Feedback appreciated.
>>
>> Imo looks all good. And I just realized that cross-checking with the TODO,
>> the 2 items about _CUSTOM and _IMPORT ioctls I noted are already there.
>>
>> Otherwise I looked through the patches, looks all really reasonable.
> 
> Two more items that need to be addressed in my opinion :
> 
> - Let's not export the ion_client API, we don't want drivers to be ion-
> specific. Only the dma-buf interface should be visible to drivers.
> 

Yes, that's a good point. I never heard back from anyone about a need for
in kernel allocation via Ion.

Thanks,
Laura

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Laura Abbott]

On 03/03/2017 08:25 AM, Laurent Pinchart wrote:
> Hi Laura,
> 
> Thank you for the patches.
> 
> On Thursday 02 Mar 2017 13:44:32 Laura Abbott wrote:
>> Hi,
>>
>> There's been some recent discussions[1] about Ion-like frameworks. There's
>> apparently interest in just keeping Ion since it works reasonablly well.
>> This series does what should be the final clean ups for it to possibly be
>> moved out of staging.
>>
>> This includes the following:
>> - Some general clean up and removal of features that never got a lot of use
>>   as far as I can tell.
>> - Fixing up the caching. This is the series I proposed back in December[2]
>>   but never heard any feedback on. It will certainly break existing
>>   applications that rely on the implicit caching. I'd rather make an effort
>>   to move to a model that isn't going directly against the establishement
>>   though.
>> - Fixing up the platform support. The devicetree approach was never well
>>   recieved by DT maintainers. The proposal here is to think of Ion less as
>>   specifying requirements and more of a framework for exposing memory to
>>   userspace.
> 
> That's where most of my concerns with ion are. I still strongly believe that 
> the heap-based approach is inherently flawed, as it would need to be 
> configured for each device according to product-specific use cases. That's 
> not 
> something that could be easily shipped with a generic distribution. We should 
> replace that with a constraint-based system.
> 

I don't think of constraints and heaps as being mutually exclusive. Some general
heaps (e.g. system heaps) can be available always. Others might just be
exposed if there is a particular memory region available. The constraint solving
is responsible for querying and figuring out what's the best choice.

Thanks,
Laura

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Laurent Pinchart]

Hi Daniel,

On Friday 03 Mar 2017 11:04:33 Daniel Vetter wrote:
> On Thu, Mar 02, 2017 at 01:44:32PM -0800, Laura Abbott wrote:
> > Hi,
> > 
> > There's been some recent discussions[1] about Ion-like frameworks. There's
> > apparently interest in just keeping Ion since it works reasonablly well.
> > This series does what should be the final clean ups for it to possibly be
> > moved out of staging.
> > 
> > This includes the following:
> > - Some general clean up and removal of features that never got a lot of
> >   use as far as I can tell.
> > 
> > - Fixing up the caching. This is the series I proposed back in December[2]
> >   but never heard any feedback on. It will certainly break existing
> >   applications that rely on the implicit caching. I'd rather make an
> >   effort to move to a model that isn't going directly against the
> >   establishement though.
> > 
> > - Fixing up the platform support. The devicetree approach was never well
> >   recieved by DT maintainers. The proposal here is to think of Ion less as
> >   specifying requirements and more of a framework for exposing memory to
> >   userspace.
> > 
> > - CMA allocations now happen without the need of a dummy device structure.
> >   This fixes a bunch of the reasons why I attempted to add devicetree
> >   support before.
> > 
> > I've had problems getting feedback in the past so if I don't hear any
> > major objections I'm going to send out with the RFC dropped to be picked
> > up. The only reason there isn't a patch to come out of staging is to
> > discuss any other changes to the ABI people might want. Once this comes
> > out of staging, I really don't want to mess with the ABI.
> > 
> > Feedback appreciated.
> 
> Imo looks all good. And I just realized that cross-checking with the TODO,
> the 2 items about _CUSTOM and _IMPORT ioctls I noted are already there.
> 
> Otherwise I looked through the patches, looks all really reasonable.

Two more items that need to be addressed in my opinion :

- Let's not export the ion_client API, we don't want drivers to be ion-
specific. Only the dma-buf interface should be visible to drivers.

- I haven't seen any proposal how a heap-based solution could be used in a 
generic distribution. This needs to be figured out before committing to any 
API/ABI.

> Wrt merging, my experience from destaging the android syncpt stuff was
> that merging the patches through the staging tree lead to lots of
> cross-tree issues with the gpu folks wanting to use that. Ion will
> probably run into similar things, so I'd propose we pull these cleanup
> patches and the eventual de-staging in throught drm. Yes that defacto
> means I'm also volunteering myself a bit :-)
> 
> In the end we could put it all into drivers/gpu/ion or something like
> that.
> 
> Thoughts? Greg?
> -Daniel
> 
> > Thanks,
> > Laura
> > 
> > [1] https://marc.info/?l=linux-kernel=148699712602105=2
> > [2] https://marc.info/?l=linaro-mm-sig=148176050802908=2
> > 
> > Laura Abbott (12):
> >   staging: android: ion: Remove dmap_cnt
> >   staging: android: ion: Remove alignment from allocation field
> >   staging: android: ion: Duplicate sg_table
> >   staging: android: ion: Call dma_map_sg for syncing and mapping
> >   staging: android: ion: Remove page faulting support
> >   staging: android: ion: Remove crufty cache support
> >   staging: android: ion: Remove old platform support
> >   cma: Store a name in the cma structure
> >   cma: Introduce cma_for_each_area
> >   staging: android: ion: Use CMA APIs directly
> >   staging: android: ion: Make Ion heaps selectable
> >   staging; android: ion: Enumerate all available heaps
> >  
> >  drivers/base/dma-contiguous.c  |   5 +-
> >  drivers/staging/android/ion/Kconfig|  51 ++--
> >  drivers/staging/android/ion/Makefile   |  14 +-
> >  drivers/staging/android/ion/hisilicon/Kconfig  |   5 -
> >  drivers/staging/android/ion/hisilicon/Makefile |   1 -
> >  drivers/staging/android/ion/hisilicon/hi6220_ion.c | 113 -
> >  drivers/staging/android/ion/ion-ioctl.c|   6 -
> >  drivers/staging/android/ion/ion.c  | 282
> >  ++--- drivers/staging/android/ion/ion.h 
> >  |   5 +-
> >  drivers/staging/android/ion/ion_carveout_heap.c|  16 +-
> >  drivers/staging/android/ion/ion_chunk_heap.c   |  15 +-
> >  drivers/staging/android/ion/ion_cma_heap.c | 102 ++--
> >  drivers/staging/android/ion/ion_dummy_driver.c | 156 
> >  drivers/staging/android/ion/ion_enumerate.c|  89 +++
> >  drivers/staging/android/ion/ion_of.c   | 184 --
> >  drivers/staging/android/ion/ion_of.h   |  37 ---
> >  drivers/staging/android/ion/ion_page_pool.c|   3 -
> >  drivers/staging/android/ion/ion_priv.h |  57 -
> >  drivers/staging/android/ion/ion_system_heap.c  |  14 +-
> >  drivers/staging/android/ion/tegra/Makefile 

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Laurent Pinchart]

Hi Laura,

Thank you for the patches.

On Thursday 02 Mar 2017 13:44:32 Laura Abbott wrote:
> Hi,
> 
> There's been some recent discussions[1] about Ion-like frameworks. There's
> apparently interest in just keeping Ion since it works reasonablly well.
> This series does what should be the final clean ups for it to possibly be
> moved out of staging.
> 
> This includes the following:
> - Some general clean up and removal of features that never got a lot of use
>   as far as I can tell.
> - Fixing up the caching. This is the series I proposed back in December[2]
>   but never heard any feedback on. It will certainly break existing
>   applications that rely on the implicit caching. I'd rather make an effort
>   to move to a model that isn't going directly against the establishement
>   though.
> - Fixing up the platform support. The devicetree approach was never well
>   recieved by DT maintainers. The proposal here is to think of Ion less as
>   specifying requirements and more of a framework for exposing memory to
>   userspace.

That's where most of my concerns with ion are. I still strongly believe that 
the heap-based approach is inherently flawed, as it would need to be 
configured for each device according to product-specific use cases. That's not 
something that could be easily shipped with a generic distribution. We should 
replace that with a constraint-based system.

> - CMA allocations now happen without the need of a dummy device structure.
>   This fixes a bunch of the reasons why I attempted to add devicetree
>   support before.
> 
> I've had problems getting feedback in the past so if I don't hear any major
> objections I'm going to send out with the RFC dropped to be picked up.
> The only reason there isn't a patch to come out of staging is to discuss any
> other changes to the ABI people might want. Once this comes out of staging,
> I really don't want to mess with the ABI.
> 
> Feedback appreciated.
> 
> Thanks,
> Laura
> 
> [1] https://marc.info/?l=linux-kernel=148699712602105=2
> [2] https://marc.info/?l=linaro-mm-sig=148176050802908=2
> 
> Laura Abbott (12):
>   staging: android: ion: Remove dmap_cnt
>   staging: android: ion: Remove alignment from allocation field
>   staging: android: ion: Duplicate sg_table
>   staging: android: ion: Call dma_map_sg for syncing and mapping
>   staging: android: ion: Remove page faulting support
>   staging: android: ion: Remove crufty cache support
>   staging: android: ion: Remove old platform support
>   cma: Store a name in the cma structure
>   cma: Introduce cma_for_each_area
>   staging: android: ion: Use CMA APIs directly
>   staging: android: ion: Make Ion heaps selectable
>   staging; android: ion: Enumerate all available heaps
> 
>  drivers/base/dma-contiguous.c  |   5 +-
>  drivers/staging/android/ion/Kconfig|  51 ++--
>  drivers/staging/android/ion/Makefile   |  14 +-
>  drivers/staging/android/ion/hisilicon/Kconfig  |   5 -
>  drivers/staging/android/ion/hisilicon/Makefile |   1 -
>  drivers/staging/android/ion/hisilicon/hi6220_ion.c | 113 -
>  drivers/staging/android/ion/ion-ioctl.c|   6 -
>  drivers/staging/android/ion/ion.c  | 282 +-
>  drivers/staging/android/ion/ion.h  |   5 +-
>  drivers/staging/android/ion/ion_carveout_heap.c|  16 +-
>  drivers/staging/android/ion/ion_chunk_heap.c   |  15 +-
>  drivers/staging/android/ion/ion_cma_heap.c | 102 ++--
>  drivers/staging/android/ion/ion_dummy_driver.c | 156 
>  drivers/staging/android/ion/ion_enumerate.c|  89 +++
>  drivers/staging/android/ion/ion_of.c   | 184 --
>  drivers/staging/android/ion/ion_of.h   |  37 ---
>  drivers/staging/android/ion/ion_page_pool.c|   3 -
>  drivers/staging/android/ion/ion_priv.h |  57 -
>  drivers/staging/android/ion/ion_system_heap.c  |  14 +-
>  drivers/staging/android/ion/tegra/Makefile |   1 -
>  drivers/staging/android/ion/tegra/tegra_ion.c  |  80 --
>  include/linux/cma.h|   6 +-
>  mm/cma.c   |  25 +-
>  mm/cma.h   |   1 +
>  mm/cma_debug.c |   2 +-
>  25 files changed, 312 insertions(+), 958 deletions(-)
>  delete mode 100644 drivers/staging/android/ion/hisilicon/Kconfig
>  delete mode 100644 drivers/staging/android/ion/hisilicon/Makefile
>  delete mode 100644 drivers/staging/android/ion/hisilicon/hi6220_ion.c
>  delete mode 100644 drivers/staging/android/ion/ion_dummy_driver.c
>  create mode 100644 drivers/staging/android/ion/ion_enumerate.c
>  delete mode 100644 drivers/staging/android/ion/ion_of.c
>  delete mode 100644 drivers/staging/android/ion/ion_of.h
>  delete mode 100644 drivers/staging/android/ion/tegra/Makefile
>  delete mode 100644 

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Benjamin Gaignard]

2017-03-03 11:27 GMT+01:00 Daniel Vetter :
> On Fri, Mar 03, 2017 at 11:04:33AM +0100, Daniel Vetter wrote:
>> On Thu, Mar 02, 2017 at 01:44:32PM -0800, Laura Abbott wrote:
>> > Hi,
>> >
>> > There's been some recent discussions[1] about Ion-like frameworks. There's
>> > apparently interest in just keeping Ion since it works reasonablly well.
>> > This series does what should be the final clean ups for it to possibly be
>> > moved out of staging.
>> >
>> > This includes the following:
>> > - Some general clean up and removal of features that never got a lot of use
>> >   as far as I can tell.
>> > - Fixing up the caching. This is the series I proposed back in December[2]
>> >   but never heard any feedback on. It will certainly break existing
>> >   applications that rely on the implicit caching. I'd rather make an effort
>> >   to move to a model that isn't going directly against the establishement
>> >   though.
>> > - Fixing up the platform support. The devicetree approach was never well
>> >   recieved by DT maintainers. The proposal here is to think of Ion less as
>> >   specifying requirements and more of a framework for exposing memory to
>> >   userspace.
>> > - CMA allocations now happen without the need of a dummy device structure.
>> >   This fixes a bunch of the reasons why I attempted to add devicetree
>> >   support before.
>> >
>> > I've had problems getting feedback in the past so if I don't hear any major
>> > objections I'm going to send out with the RFC dropped to be picked up.
>> > The only reason there isn't a patch to come out of staging is to discuss 
>> > any
>> > other changes to the ABI people might want. Once this comes out of staging,
>> > I really don't want to mess with the ABI.
>> >
>> > Feedback appreciated.
>>
>> Imo looks all good. And I just realized that cross-checking with the TODO,
>> the 2 items about _CUSTOM and _IMPORT ioctls I noted are already there.
>
> One more for the todo: Add rst/sphinx documentation for ION. That's also
> always a good excuse to review the internal interfaces and exported
> symbols. But we can do that after destaging ...
> -Daniel

Removing alignment looks good for me but why not also remove it from
struct ion_allocation_data since the field become useless ?

Also does someone use ion_user_handle_t handle ? Can we directly export
a dma-buf file descriptor ?

Benjamin

>
>>
>> Otherwise I looked through the patches, looks all really reasonable.
>>
>> Wrt merging, my experience from destaging the android syncpt stuff was
>> that merging the patches through the staging tree lead to lots of
>> cross-tree issues with the gpu folks wanting to use that. Ion will
>> probably run into similar things, so I'd propose we pull these cleanup
>> patches and the eventual de-staging in throught drm. Yes that defacto
>> means I'm also volunteering myself a bit :-)
>>
>> In the end we could put it all into drivers/gpu/ion or something like
>> that.
>>
>> Thoughts? Greg?
>> -Daniel
>>
>>
>> >
>> > Thanks,
>> > Laura
>> >
>> > [1] https://marc.info/?l=linux-kernel=148699712602105=2
>> > [2] https://marc.info/?l=linaro-mm-sig=148176050802908=2
>> >
>> > Laura Abbott (12):
>> >   staging: android: ion: Remove dmap_cnt
>> >   staging: android: ion: Remove alignment from allocation field
>> >   staging: android: ion: Duplicate sg_table
>> >   staging: android: ion: Call dma_map_sg for syncing and mapping
>> >   staging: android: ion: Remove page faulting support
>> >   staging: android: ion: Remove crufty cache support
>> >   staging: android: ion: Remove old platform support
>> >   cma: Store a name in the cma structure
>> >   cma: Introduce cma_for_each_area
>> >   staging: android: ion: Use CMA APIs directly
>> >   staging: android: ion: Make Ion heaps selectable
>> >   staging; android: ion: Enumerate all available heaps
>> >
>> >  drivers/base/dma-contiguous.c  |   5 +-
>> >  drivers/staging/android/ion/Kconfig|  51 ++--
>> >  drivers/staging/android/ion/Makefile   |  14 +-
>> >  drivers/staging/android/ion/hisilicon/Kconfig  |   5 -
>> >  drivers/staging/android/ion/hisilicon/Makefile |   1 -
>> >  drivers/staging/android/ion/hisilicon/hi6220_ion.c | 113 -
>> >  drivers/staging/android/ion/ion-ioctl.c|   6 -
>> >  drivers/staging/android/ion/ion.c  | 282 
>> > ++---
>> >  drivers/staging/android/ion/ion.h  |   5 +-
>> >  drivers/staging/android/ion/ion_carveout_heap.c|  16 +-
>> >  drivers/staging/android/ion/ion_chunk_heap.c   |  15 +-
>> >  drivers/staging/android/ion/ion_cma_heap.c | 102 ++--
>> >  drivers/staging/android/ion/ion_dummy_driver.c | 156 
>> >  drivers/staging/android/ion/ion_enumerate.c|  89 +++
>> >  drivers/staging/android/ion/ion_of.c   | 184 --
>> >  drivers/staging/android/ion/ion_of.h   |  37 ---
>> >  

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Michal Hocko]

On Thu 02-03-17 13:44:32, Laura Abbott wrote:
> Hi,
> 
> There's been some recent discussions[1] about Ion-like frameworks. There's
> apparently interest in just keeping Ion since it works reasonablly well.
> This series does what should be the final clean ups for it to possibly be
> moved out of staging.
> 
> This includes the following:
> - Some general clean up and removal of features that never got a lot of use
>   as far as I can tell.
> - Fixing up the caching. This is the series I proposed back in December[2]
>   but never heard any feedback on. It will certainly break existing
>   applications that rely on the implicit caching. I'd rather make an effort
>   to move to a model that isn't going directly against the establishement
>   though.
> - Fixing up the platform support. The devicetree approach was never well
>   recieved by DT maintainers. The proposal here is to think of Ion less as
>   specifying requirements and more of a framework for exposing memory to
>   userspace.
> - CMA allocations now happen without the need of a dummy device structure.
>   This fixes a bunch of the reasons why I attempted to add devicetree
>   support before.
> 
> I've had problems getting feedback in the past so if I don't hear any major
> objections I'm going to send out with the RFC dropped to be picked up.
> The only reason there isn't a patch to come out of staging is to discuss any
> other changes to the ABI people might want. Once this comes out of staging,
> I really don't want to mess with the ABI.

Could you recapitulate concerns preventing the code being merged
normally rather than through the staging tree and how they were
addressed?
-- 
Michal Hocko
SUSE Labs

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Daniel Vetter]

On Thu, Mar 02, 2017 at 01:44:32PM -0800, Laura Abbott wrote:
> Hi,
> 
> There's been some recent discussions[1] about Ion-like frameworks. There's
> apparently interest in just keeping Ion since it works reasonablly well.
> This series does what should be the final clean ups for it to possibly be
> moved out of staging.
> 
> This includes the following:
> - Some general clean up and removal of features that never got a lot of use
>   as far as I can tell.
> - Fixing up the caching. This is the series I proposed back in December[2]
>   but never heard any feedback on. It will certainly break existing
>   applications that rely on the implicit caching. I'd rather make an effort
>   to move to a model that isn't going directly against the establishement
>   though.
> - Fixing up the platform support. The devicetree approach was never well
>   recieved by DT maintainers. The proposal here is to think of Ion less as
>   specifying requirements and more of a framework for exposing memory to
>   userspace.
> - CMA allocations now happen without the need of a dummy device structure.
>   This fixes a bunch of the reasons why I attempted to add devicetree
>   support before.
> 
> I've had problems getting feedback in the past so if I don't hear any major
> objections I'm going to send out with the RFC dropped to be picked up.
> The only reason there isn't a patch to come out of staging is to discuss any
> other changes to the ABI people might want. Once this comes out of staging,
> I really don't want to mess with the ABI.
> 
> Feedback appreciated.

Imo looks all good. And I just realized that cross-checking with the TODO,
the 2 items about _CUSTOM and _IMPORT ioctls I noted are already there.

Otherwise I looked through the patches, looks all really reasonable.

Wrt merging, my experience from destaging the android syncpt stuff was
that merging the patches through the staging tree lead to lots of
cross-tree issues with the gpu folks wanting to use that. Ion will
probably run into similar things, so I'd propose we pull these cleanup
patches and the eventual de-staging in throught drm. Yes that defacto
means I'm also volunteering myself a bit :-)

In the end we could put it all into drivers/gpu/ion or something like
that.

Thoughts? Greg?
-Daniel


> 
> Thanks,
> Laura
> 
> [1] https://marc.info/?l=linux-kernel=148699712602105=2
> [2] https://marc.info/?l=linaro-mm-sig=148176050802908=2
> 
> Laura Abbott (12):
>   staging: android: ion: Remove dmap_cnt
>   staging: android: ion: Remove alignment from allocation field
>   staging: android: ion: Duplicate sg_table
>   staging: android: ion: Call dma_map_sg for syncing and mapping
>   staging: android: ion: Remove page faulting support
>   staging: android: ion: Remove crufty cache support
>   staging: android: ion: Remove old platform support
>   cma: Store a name in the cma structure
>   cma: Introduce cma_for_each_area
>   staging: android: ion: Use CMA APIs directly
>   staging: android: ion: Make Ion heaps selectable
>   staging; android: ion: Enumerate all available heaps
> 
>  drivers/base/dma-contiguous.c  |   5 +-
>  drivers/staging/android/ion/Kconfig|  51 ++--
>  drivers/staging/android/ion/Makefile   |  14 +-
>  drivers/staging/android/ion/hisilicon/Kconfig  |   5 -
>  drivers/staging/android/ion/hisilicon/Makefile |   1 -
>  drivers/staging/android/ion/hisilicon/hi6220_ion.c | 113 -
>  drivers/staging/android/ion/ion-ioctl.c|   6 -
>  drivers/staging/android/ion/ion.c  | 282 
> ++---
>  drivers/staging/android/ion/ion.h  |   5 +-
>  drivers/staging/android/ion/ion_carveout_heap.c|  16 +-
>  drivers/staging/android/ion/ion_chunk_heap.c   |  15 +-
>  drivers/staging/android/ion/ion_cma_heap.c | 102 ++--
>  drivers/staging/android/ion/ion_dummy_driver.c | 156 
>  drivers/staging/android/ion/ion_enumerate.c|  89 +++
>  drivers/staging/android/ion/ion_of.c   | 184 --
>  drivers/staging/android/ion/ion_of.h   |  37 ---
>  drivers/staging/android/ion/ion_page_pool.c|   3 -
>  drivers/staging/android/ion/ion_priv.h |  57 -
>  drivers/staging/android/ion/ion_system_heap.c  |  14 +-
>  drivers/staging/android/ion/tegra/Makefile |   1 -
>  drivers/staging/android/ion/tegra/tegra_ion.c  |  80 --
>  include/linux/cma.h|   6 +-
>  mm/cma.c   |  25 +-
>  mm/cma.h   |   1 +
>  mm/cma_debug.c |   2 +-
>  25 files changed, 312 insertions(+), 958 deletions(-)
>  delete mode 100644 drivers/staging/android/ion/hisilicon/Kconfig
>  delete mode 100644 drivers/staging/android/ion/hisilicon/Makefile
>  delete mode 100644 drivers/staging/android/ion/hisilicon/hi6220_ion.c
>  delete mode 

Re: [RFC PATCH 00/12] Ion cleanup in preparation for moving out of staging

[Daniel Vetter]

On Fri, Mar 03, 2017 at 11:04:33AM +0100, Daniel Vetter wrote:
> On Thu, Mar 02, 2017 at 01:44:32PM -0800, Laura Abbott wrote:
> > Hi,
> > 
> > There's been some recent discussions[1] about Ion-like frameworks. There's
> > apparently interest in just keeping Ion since it works reasonablly well.
> > This series does what should be the final clean ups for it to possibly be
> > moved out of staging.
> > 
> > This includes the following:
> > - Some general clean up and removal of features that never got a lot of use
> >   as far as I can tell.
> > - Fixing up the caching. This is the series I proposed back in December[2]
> >   but never heard any feedback on. It will certainly break existing
> >   applications that rely on the implicit caching. I'd rather make an effort
> >   to move to a model that isn't going directly against the establishement
> >   though.
> > - Fixing up the platform support. The devicetree approach was never well
> >   recieved by DT maintainers. The proposal here is to think of Ion less as
> >   specifying requirements and more of a framework for exposing memory to
> >   userspace.
> > - CMA allocations now happen without the need of a dummy device structure.
> >   This fixes a bunch of the reasons why I attempted to add devicetree
> >   support before.
> > 
> > I've had problems getting feedback in the past so if I don't hear any major
> > objections I'm going to send out with the RFC dropped to be picked up.
> > The only reason there isn't a patch to come out of staging is to discuss any
> > other changes to the ABI people might want. Once this comes out of staging,
> > I really don't want to mess with the ABI.
> > 
> > Feedback appreciated.
> 
> Imo looks all good. And I just realized that cross-checking with the TODO,
> the 2 items about _CUSTOM and _IMPORT ioctls I noted are already there.

One more for the todo: Add rst/sphinx documentation for ION. That's also
always a good excuse to review the internal interfaces and exported
symbols. But we can do that after destaging ...
-Daniel

> 
> Otherwise I looked through the patches, looks all really reasonable.
> 
> Wrt merging, my experience from destaging the android syncpt stuff was
> that merging the patches through the staging tree lead to lots of
> cross-tree issues with the gpu folks wanting to use that. Ion will
> probably run into similar things, so I'd propose we pull these cleanup
> patches and the eventual de-staging in throught drm. Yes that defacto
> means I'm also volunteering myself a bit :-)
> 
> In the end we could put it all into drivers/gpu/ion or something like
> that.
> 
> Thoughts? Greg?
> -Daniel
> 
> 
> > 
> > Thanks,
> > Laura
> > 
> > [1] https://marc.info/?l=linux-kernel=148699712602105=2
> > [2] https://marc.info/?l=linaro-mm-sig=148176050802908=2
> > 
> > Laura Abbott (12):
> >   staging: android: ion: Remove dmap_cnt
> >   staging: android: ion: Remove alignment from allocation field
> >   staging: android: ion: Duplicate sg_table
> >   staging: android: ion: Call dma_map_sg for syncing and mapping
> >   staging: android: ion: Remove page faulting support
> >   staging: android: ion: Remove crufty cache support
> >   staging: android: ion: Remove old platform support
> >   cma: Store a name in the cma structure
> >   cma: Introduce cma_for_each_area
> >   staging: android: ion: Use CMA APIs directly
> >   staging: android: ion: Make Ion heaps selectable
> >   staging; android: ion: Enumerate all available heaps
> > 
> >  drivers/base/dma-contiguous.c  |   5 +-
> >  drivers/staging/android/ion/Kconfig|  51 ++--
> >  drivers/staging/android/ion/Makefile   |  14 +-
> >  drivers/staging/android/ion/hisilicon/Kconfig  |   5 -
> >  drivers/staging/android/ion/hisilicon/Makefile |   1 -
> >  drivers/staging/android/ion/hisilicon/hi6220_ion.c | 113 -
> >  drivers/staging/android/ion/ion-ioctl.c|   6 -
> >  drivers/staging/android/ion/ion.c  | 282 
> > ++---
> >  drivers/staging/android/ion/ion.h  |   5 +-
> >  drivers/staging/android/ion/ion_carveout_heap.c|  16 +-
> >  drivers/staging/android/ion/ion_chunk_heap.c   |  15 +-
> >  drivers/staging/android/ion/ion_cma_heap.c | 102 ++--
> >  drivers/staging/android/ion/ion_dummy_driver.c | 156 
> >  drivers/staging/android/ion/ion_enumerate.c|  89 +++
> >  drivers/staging/android/ion/ion_of.c   | 184 --
> >  drivers/staging/android/ion/ion_of.h   |  37 ---
> >  drivers/staging/android/ion/ion_page_pool.c|   3 -
> >  drivers/staging/android/ion/ion_priv.h |  57 -
> >  drivers/staging/android/ion/ion_system_heap.c  |  14 +-
> >  drivers/staging/android/ion/tegra/Makefile |   1 -
> >  drivers/staging/android/ion/tegra/tegra_ion.c  |  80 --
> >  include/linux/cma.h|   6 +-
> >  mm/cma.c