Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [RFC][PATCH 0/5 v2] DMA-BUF Heaps (destaging ION)
To:     Benjamin Gaignard <benjamin.gaignard@linaro.org>,
        John Stultz <john.stultz@linaro.org>
CC:     Rob Clark <robdclark@gmail.com>,
        Alistair Strachan <astrachan@google.com>,
        Vincent Donnefort <Vincent.Donnefort@arm.com>,
        Greg KH <gregkh@linuxfoundation.org>,
        Chenbo Feng <fengc@google.com>,
        lkml <linux-kernel@vger.kernel.org>,
        Liam Mark <lmark@codeaurora.org>,
        Marissa Wall <marissaw@google.com>,
        dri-devel <dri-devel@lists.freedesktop.org>
References: <1551819273-640-1-git-send-email-john.stultz@linaro.org>
 <alpine.DEB.2.10.1903131309260.5023@lmark-linux.qualcomm.com>
 <CALAqxLUkGME+K=0g5QSja4CvN0w9z0WV60djy+5rRC+Dg-jCGg@mail.gmail.com>
 <CA+M3ks7q8QnNirk_WkaE5CmFWkEPn4uu5Hfb2kP+4d5qapHvrA@mail.gmail.com>
 <c51be371-086a-bd5b-b70a-5579f5b95320@ti.com>
 <CAF6AEGss21WmpF7sHGTxOEjuadSNpM6M_Z-WweK2W68FEHOFdA@mail.gmail.com>
 <CALAqxLVbdpj+T4u3jZ_5Uqda5La1Py-ZGpXSep_CCoLzWyap8A@mail.gmail.com>
 <CA+M3ks61BGPkPjNd01g32LV7YZfDpy8t100KxQhScOFgGO4KjA@mail.gmail.com>
From:   "Andrew F. Davis" <afd@ti.com>
Message-ID: <d6f8092d-9f90-d5ff-2ab3-b1867f8f5700@ti.com>
Date:   Wed, 20 Mar 2019 09:44:23 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101
 Thunderbird/60.4.0
MIME-Version: 1.0
In-Reply-To: <CA+M3ks61BGPkPjNd01g32LV7YZfDpy8t100KxQhScOFgGO4KjA@mail.gmail.com>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 3/20/19 4:16 AM, Benjamin Gaignard wrote:
> Le mar. 19 mars 2019 à 23:36, John Stultz <john.stultz@linaro.org> a écrit :
>>
>> On Tue, Mar 19, 2019 at 2:58 PM Rob Clark <robdclark@gmail.com> wrote:
>>>
>>> On Tue, Mar 19, 2019 at 1:00 PM Andrew F. Davis <afd@ti.com> wrote:
>>>>
>>>> On 3/19/19 11:54 AM, Benjamin Gaignard wrote:
>>>>> Le mer. 13 mars 2019 à 23:31, John Stultz <john.stultz@linaro.org> a écrit :
>>>>>>
>>>>>> On Wed, Mar 13, 2019 at 1:11 PM Liam Mark <lmark@codeaurora.org> wrote:
>>>>>>> On Tue, 5 Mar 2019, John Stultz wrote:
>>>>>>>>
>>>>>>>> Eventual TODOS:
>>>>>>>> * Reimplement page-pool for system heap (working on this)
>>>>>>>> * Add stats accounting to system/cma heaps
>>>>>>>> * Make the kselftest actually useful
>>>>>>>> * Add other heaps folks see as useful (would love to get
>>>>>>>>   some help from actual carveout/chunk users)!
>>>>>>>
>>>>>>> We use a modified carveout heap for certain secure use cases.
>>>>>>
>>>>>> Cool! It would be great to see if you have any concerns about adding
>>>>>> such a secure-carveout heap to this framework. I suspect it would be
>>>>>> fairly similar to how its integrated into ION, but particularly I'd be
>>>>>> interested in issues around the lack of private flags and other
>>>>>> allocation arguments like alignment.
>>>>>>
>>>>>>> Although there would probably be some benefit in discssing how the dma-buf
>>>>>>> heap framework may want to support
>>>>>>> secure heaps in the future it is a large topic which I assume you don't
>>>>>>> want to tackle now.
>>>>>>
>>>>>> So I suspect others (Benjamin?) would have a more informed opinion on
>>>>>> the details, but the intent is to allow secure heap implementations.
>>>>>> I'm not sure what areas of concern you have for this allocation
>>>>>> framework in particular?
>>>>>
>>>>> yes I would be great to understand how you provide the information to
>>>>> tell that a dmabuf
>>>>> is secure (or not) since we can't add flag in dmabuf structure itself.
>>>>> An option is manage
>>>>> the access rights when a device attach itself to the dmabuf but in
>>>>> this case you need define
>>>>> a list of allowed devices per heap...
>>>>> If you have a good solution for secure heaps you are welcome :-)
>>>>>
>>>>
>>>> Do we really need any of that? A secure buffer is secured by the
>>>> hardware firewalls that keep out certain IP (including often the
>>>> processor running Linux). So the only thing we need to track internally
>>>> is that we should not allow mmap/kmap on the buffer. That can be done in
>>>> the per-heap layer, everything else stays the same as a standard
>>>> carveout heap.
>>>
>>> For at least some hw the importing driver needs to configure things
>>> differently for secure buffers :-/
>>
>> Does the import ioctl need/use a flag for that then? Userland already
>> has to keep meta-data about dmabufs around.
> 
> To secure a buffer you need to know who is allowed to write/read it and
> hardware block involved in the dataflow may need to know that the buffer
> is secure to configure themself.
> As example for a video decoding you allow hw video decoder to read in
> a buffer and display to read it. You can also allow cpu to write on the buffer
> to add subtitles. For that we need to be able to mmap/kmap the buffer.
> Using a carveout heap for secure buffer mean that you reserved a large
> memory region only for this purpose, that isn't possible on embedded device
> where we are always limited in memory so we use CMA.
> In the past I have used dmabuf's attach function to know who write into
> the buffer and then configure who will be able to read it. It was working well
> but the issue was how to in generic way this behavior.
> 

Okay, I think I see what you are saying now.

The way we handle secure playback is to firewall everything upfront and
it is up to the application to inform the hardware about what it can and
cannot do to the buffer, or simply not ask anything not allowed (E.g.
writeback the decrypted stream) else it will get a firewall exception.
The buffer itself doesn't have to carry any information.

It sounds like you want the hardware driver to be able to detect the
use-case based on the buffer itself and configure itself accordingly? Or
the exporter at attach time to check access permissions?

The first would need a change to DMA-BUF framework, maybe an added flag.
The second would just need a heap exporter with the system wide smarts,
but as you say that is not very generic..

Andrew

>>
>> thanks
>> -john