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
<benjamin.gaignard@xxxxxxxxxx> wrote:
> 2017-03-09 18:38 GMT+01:00 Laura Abbott <labbott@xxxxxxxxxx>:
>> On 03/09/2017 02:00 AM, Benjamin Gaignard wrote:
>>> 2017-03-06 17:04 GMT+01:00 Daniel Vetter <daniel@xxxxxxxx>:
>>>> 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