RE: [EXT] Re: [PATCH v2 1/1] tee: new ioctl to a register tee_shm from a dmabuf file descriptor

[Cyrille Fleury]

-----Original Message-----
From: Jerome Forissier <jerome.forissier@xxxxxxxxxx>
Sent: Friday, February 3, 2023 1:32 PM
To: Etienne Carriere <etienne.carriere@xxxxxxxxxx>; Olivier Masse <olivier.masse@xxxxxxx>
Cc: sumit.garg@xxxxxxxxxx; linux-media@xxxxxxxxxxxxxxx; fredgc@xxxxxxxxxx; linaro-mm-sig@xxxxxxxxxxxxxxxx; afd@xxxxxx; op-tee@xxxxxxxxxxxxxxxxxxxxxxxxx; jens.wiklander@xxxxxxxxxx; joakim.bech@xxxxxxxxxx; sumit.semwal@xxxxxxxxxx; Cyrille Fleury <cyrille.fleury@xxxxxxx>; Peter Griffin <peter.griffin@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx; dri-devel@xxxxxxxxxxxxxxxxxxxxx; Clément Faure <clement.faure@xxxxxxx>; christian.koenig@xxxxxxx
Subject: Re: [EXT] Re: [PATCH v2 1/1] tee: new ioctl to a register tee_shm from a dmabuf file descriptor

On 2/3/23 15:12, Cyrille Fleury wrote:
Hi all,

>On 2/3/23 12:37, Etienne Carriere wrote:
>> Hell all,
>>
>> +jerome f.
>>
>> On Fri, 3 Feb 2023 at 12:01, Olivier Masse <olivier.masse@xxxxxxx> wrote:
>>>
>>> On jeu., 2023-02-02 at 10:58 +0100, Etienne Carriere wrote:
>>>> Caution: EXT Email
>>>>
>>>> On Thu, 2 Feb 2023 at 09:35, Sumit Garg <sumit.garg@xxxxxxxxxx>
>>>> wrote:
>>>>> Hi Cyrille,
>>>>>
>>>>> Please don't top post as it makes it harder to follow-up.
>>>>>
>>>>> On Thu, 2 Feb 2023 at 13:26, Cyrille Fleury <cyrille.fleury@xxxxxxx
>>>>>> wrote:
>>>>>> Hi Sumit, all
>>>>>>
>>>>>> Upstream OP-TEE should support registering a dmabuf since a while, 
>>>>>> given how widely dmabuf is used in Linux for passing buffers 
>>>>>> around between devices.
>>>>>>
>>>>>> Purpose of the new register_tee_shm ioctl is to allow OPTEE to use 
>>>>>> memory allocated from the exiting linux dma buffer. We don't need 
>>>>>> to have secure dma-heap up streamed.
>>>>>>
>>>>>> You mentioned secure dma-buffer, but secure dma-buffer is a dma- 
>>>>>> buffer, so the work to be done for secure or "regular" dma buffers 
>>>>>> by the register_tee_shm ioctl is 100% the same.
>>>>>>
>>>>>> The scope of this ioctl is limited to what existing upstream dma- 
>>>>>> buffers are:
>>>>>>         -> sharing buffers for hardware (DMA) access across 
>>>>>> multiple device drivers and subsystems, and for synchronizing 
>>>>>> asynchronous hardware access.
>>>>>>        -> It means continuous memory only.
>>>>>>
>>>>>> So if we reduce the scope of register tee_shm to exiting dma- 
>>>>>> buffer area, the current patch does the job.
>>>>>
>>>>> Do you have a corresponding real world use-case supported by 
>>>>> upstream OP-TEE? AFAIK, the Secure Data Path (SDP) use-case is the 
>>>>> one supported in OP-TEE upstream but without secure dmabuf heap [1] 
>>>>> available, the new ioctl can't be exercised.
>>>>>
>>>>> [1] 
>>>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fg
>>>>> ithub.com%2FOP-TEE%2Foptee_test%2Fblob%2Fmaster%2Fhost%2Fxtest%2Fsd
>>>>> p_basic.h%23L15&data=05%7C01%7Ccyrille.fleury%40nxp.com%7C9ff962fb5
>>>>> 8f6401c597808db05e2a64b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%
>>>>> 7C638110243232457377%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLC
>>>>> JQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=
>>>>> UNB88rvmhQ5qRoIGN%2FpS4cQTES5joM8AjoyAAYzPKl0%3D&reserved=0
>>>>
>>>> OP-TEE has some SDP test taht can exercice SDP: 'xtest 
>>>> regression_1014'.
>>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
>>>> thub.com%2FOP-TEE%2Foptee_test%2Fblob%2F3.20.0%2Fhost%2Fxtest%2Fregr
>>>> ession_1000.c%23L1256&data=05%7C01%7Ccyrille.fleury%40nxp.com%7C9ff9
>>>> 62fb58f6401c597808db05e2a64b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%
>>>> 7C0%7C638110243232457377%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDA
>>>> iLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdat
>>>> a=e%2B40rwWvtvVFG8aWZNeu%2FgjMXXvZ3pRhJfHLkdurovs%3D&reserved=0
>>>>
>>>> The test relies on old staged ION + local secure dmabuf heaps no 
>>>> more maintained, so this test is currently not functional.
>>>> If we upgrade the test to mainline dmabuf alloc means, and apply the 
>>>> change discussed here, we should be able to regularly test SDP in 
>>>> OP-TEE project CI.
>>>> The part to update is the userland allocation of the dmabuf:
>>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
>>>> thub.com%2FOP-TEE%2Foptee_test%2Fblob%2F3.20.0%2Fhost%2Fxtest%2Fsdp_
>>>> basic.c%23L91&data=05%7C01%7Ccyrille.fleury%40nxp.com%7C9ff962fb58f6
>>>> 401c597808db05e2a64b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63
>>>> 8110243232457377%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjo
>>>> iV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=5rPV1j
>>>> qzqjVh2N5pdUW41YwF6EkgIDwfhyfYkgmtdZI%3D&reserved=0
>>>>
>>>>
>>>
>>> the test was already updated to support secure dma heap with Kernel 
>>> version 5.11 and higher. the userland allocation could be find here:
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
>>> hub.com%2FOP-TEE%2Foptee_test%2Fblob%2F3.20.0%2Fhost%2Fxtest%2Fsdp_ba
>>> sic.c%23L153&data=05%7C01%7Ccyrille.fleury%40nxp.com%7C9ff962fb58f640
>>> 1c597808db05e2a64b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63811
>>> 0243232457377%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2l
>>> uMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=01H96n47K6R
>>> mBKZQhRdcqX3nE5VBHOXNfGuMmmkVSvc%3D&reserved=0
>>>
>>
>> Oh, right. So fine, optee_test is ready for the new flavor of secure 
>> buffer fd's.
>>
>>
>>> This upgrade need a Linux dma-buf patch:
>>> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flor
>>> e.kernel.org%2Fall%2F20220805154139.2qkqxwklufjpsfdx%40000377403353%2
>>> FT%2F&data=05%7C01%7Ccyrille.fleury%40nxp.com%7C9ff962fb58f6401c59780
>>> 8db05e2a64b%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638110243232
>>> 457377%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLC
>>> JBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=yCS%2BDcuGp%2BafAL
>>> tpw74O1bI0K%2Fwnt%2FOw5ob1ngfDA0E%3D&reserved=0
>>
>> @Jens, @Jerome, do we want to pick the 2 necessary Linux patches in 
>> our Linux kernel fork (github.com/linaro-swg/linux.git) to exercise 
>> SDP in our CI and be ready if dma-buf secure heaps (ref right above) 
>> is accepted and merged in mainline kernel?.
>
>How would that help? I mean, when the kernel patches are merged and if things break we can make the necessary adjustments in the optee_test app or whatever, but in the meantime I don't see much point. I suppose the people who are actively developing the patches do make sure it works with OP-TEE ;-)
>
>Regards,
>--
>Jerome

As mentioned in the cover letter, this IOCTL got tested by Jens Wiklander <jens.wiklander@xxxxxxxxxx>, using Linaro reference board from Hikey 6620:
https://lists.trustedfirmware.org/archives/list/op-tee@xxxxxxxxxxxxxxxxxxxxxxxxx/thread/I3TZN4TBDOUVE567VMMN2TAXGWZNY7S3/
It also works on i.MX8M EVK boards.

My understanding today is we are good to upstream this patch, knowing:
     - Upstream OPTEE driver should support registering a dmabuf since a while, given how widely dmabuf is used in Linux for passing buffers around between devices.
     - review is OK 
     - test environment is already available in optee-test
     - it has been tested on 2 different platforms
     - the scope of the new ioctl is limited to existing feature in dma-buffer
     
What is missing from this list preventing to upstream ? 
Who do we still need to convince ?

Regards.