On Thu, Sep 28, 2023 at 1:30 AM Benjamin Gaignard <benjamin.gaignard@xxxxxxxxxxxxx> wrote: > > > Le 27/09/2023 à 20:56, Jeffrey Kardatzke a écrit : > > On Wed, Sep 27, 2023 at 8:18 AM Benjamin Gaignard > > <benjamin.gaignard@xxxxxxxxxxxxx> wrote: > >> > >> Le 27/09/2023 à 15:46, Joakim Bech a écrit : > >>> On Mon, Sep 25, 2023 at 12:49:50PM +0000, Yong Wu (吴勇) wrote: > >>>> On Tue, 2023-09-12 at 11:32 +0200, AngeloGioacchino Del Regno wrote: > >>>>> Il 12/09/23 08:17, Yong Wu (吴勇) ha scritto: > >>>>>> On Mon, 2023-09-11 at 11:29 +0200, AngeloGioacchino Del Regno > >>>>>> wrote: > >>>>>>> Il 11/09/23 04:30, Yong Wu ha scritto: > >>>>>>>> The TEE probe later than dma-buf heap, and PROBE_DEDER doesn't > >>>>>>>> work > >>>>>>>> here since this is not a platform driver, therefore initialise > >>>>>>>> the > >>>>>>>> TEE > >>>>>>>> context/session while we allocate the first secure buffer. > >>>>>>>> > >>>>>>>> Signed-off-by: Yong Wu <yong.wu@xxxxxxxxxxxx> > >>>>>>>> --- > >>>>>>>> drivers/dma-buf/heaps/mtk_secure_heap.c | 61 > >>>>>>>> +++++++++++++++++++++++++ > >>>>>>>> 1 file changed, 61 insertions(+) > >>>>>>>> > >>>>>>>> diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c > >>>>>>>> b/drivers/dma- > >>>>>>>> buf/heaps/mtk_secure_heap.c > >>>>>>>> index bbf1c8dce23e..e3da33a3d083 100644 > >>>>>>>> --- a/drivers/dma-buf/heaps/mtk_secure_heap.c > >>>>>>>> +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c > >>>>>>>> @@ -10,6 +10,12 @@ > >>>>>>>> #include <linux/err.h> > >>>>>>>> #include <linux/module.h> > >>>>>>>> #include <linux/slab.h> > >>>>>>>> +#include <linux/tee_drv.h> > >>>>>>>> +#include <linux/uuid.h> > >>>>>>>> + > >>>>>>>> +#define TZ_TA_MEM_UUID "4477588a-8476-11e2-ad15- > >>>>>>>> e41f1390d676" > >>>>>>>> + > >>>>>>> Is this UUID the same for all SoCs and all TZ versions? > >>>>>> Yes. It is the same for all SoCs and all TZ versions currently. > >>>>>> > >>>>> That's good news! > >>>>> > >>>>> Is this UUID used in any userspace component? (example: Android > >>>>> HALs?) > >>>> No. Userspace never use it. If userspace would like to allocate this > >>>> secure buffer, it can achieve through the existing dmabuf IOCTL via > >>>> /dev/dma_heap/mtk_svp node. > >>>> > >>> In general I think as mentioned elsewhere in comments, that there isn't > >>> that much here that seems to be unique for MediaTek in this patch > >>> series, so I think it worth to see whether this whole patch set can be > >>> made more generic. Having said that, the UUID is always unique for a > >>> certain Trusted Application. So, it's not entirely true saying that the > >>> UUID is the same for all SoCs and all TrustZone versions. It might be > >>> true for a family of MediaTek devices and the TEE in use, but not > >>> generically. > >>> > >>> So, if we need to differentiate between different TA implementations, > >>> then we need different UUIDs. If it would be possible to make this patch > >>> set generic, then it sounds like a single UUID would be sufficient, but > >>> that would imply that all TA's supporting such a generic UUID would be > >>> implemented the same from an API point of view. Which also means that > >>> for example Trusted Application function ID's needs to be the same etc. > >>> Not impossible to achieve, but still not easy (different TEE follows > >>> different specifications) and it's not typically something we've done in > >>> the past. > >>> > >>> Unfortunately there is no standardized database of TA's describing what > >>> they implement and support. > >>> > >>> As an alternative, we could implement a query call in the TEE answering, > >>> "What UUID does your TA have that implements secure unmapped heap?". > >>> I.e., something that reminds of a lookup table. Then we wouldn't have to > >>> carry this in UAPI, DT or anywhere else. > >> Joakim does a TA could offer a generic API and hide the hardware specific > >> details (like kernel uAPI does for drivers) ? > > It would have to go through another layer (like the tee driver) to be > > a generic API. The main issue with TAs is that they have UUIDs you > > need to connect to and specific codes for each function; so we should > > abstract at a layer above where those exist in the dma-heap code. > >> Aside that question I wonder what are the needs to perform a 'secure' playback. > >> I have in mind 2 requirements: > >> - secure memory regions, which means configure the hardware to ensure that only > >> dedicated hardware blocks and read or write into it. > >> - set hardware blocks in secure modes so they access to secure memory. > >> Do you see something else ? > > This is more or less what is required, but this is out of scope for > > the Linux kernel since it can't be trusted to do these things...this > > is all done in firmware or the TEE itself. > > Yes kernel can't be trusted to do these things but know what we need could help > to define a API for a generic TA. > > Just to brainstorm on mailing list: > What about a TA API like > TA_secure_memory_region() and TA_unsecure_memory_region() with parameters like: > - device identifier (an ID or compatible string maybe) > - memory region (physical address, size, offset) > - requested access rights (read, write) > > and on kernel side a IOMMU driver because it basically have all this information already > (device attachment, kernel map/unmap). > > In my mind it sound like a solution to limit the impact (new controls, new memory type) > inside v4l2. Probably we won't need new heap either. > All hardware dedicated implementations could live inside the TA which can offer a generic > API. The main problem with that type of design is the limitations of TrustZone memory protection. Usually there is a limit to the number of regions you can define for memory protection (and there is on Mediatek). So you can't pass an arbitrary memory region and mark it protected/unprotected at a given time. You need to establish these regions in the firmware instead and then configure those regions for protection in the firmware or the TEE. > > >> Regards, > >> Benjamin > >>
