Re: [PATCH v6 0/7] Add mediate-drm secure flow for SVP

[Jason-JH Lin (林睿祥)]

Hi Maxime,

On Thu, 2024-05-30 at 17:01 +0200, mripard@xxxxxxxxxx wrote:
> On Tue, May 28, 2024 at 07:15:34AM GMT, Jason-JH Lin (林睿祥) wrote:
> > Hi Maxime,
> > 
> > On Mon, 2024-05-27 at 16:06 +0200, Maxime Ripard wrote:
> > > Hi,
> > > 
> > > On Sun, May 26, 2024 at 07:29:21AM GMT, Jason-JH.Lin wrote:
> > > > From: Jason-jh Lin <
> > > > jason-jh.lin@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx>
> > > > 
> > > > Memory Definitions:
> > > > secure memory - Memory allocated in the TEE (Trusted Execution
> > > > Environment) which is inaccessible in the REE (Rich Execution
> > > > Environment, i.e. linux kernel/userspace).
> > > > secure handle - Integer value which acts as reference to
> > > > 'secure
> > > > memory'. Used in communication between TEE and REE to reference
> > > > 'secure memory'.
> > > > secure buffer - 'secure memory' that is used to store
> > > > decrypted,
> > > > compressed video or for other general purposes in the TEE.
> > > > secure surface - 'secure memory' that is used to store graphic
> > > > buffers.
> > > > 
> > > > Memory Usage in SVP:
> > > > The overall flow of SVP starts with encrypted video coming in
> > > > from
> > > > an
> > > > outside source into the REE. The REE will then allocate a
> > > > 'secure
> > > > buffer' and send the corresponding 'secure handle' along with
> > > > the
> > > > encrypted, compressed video data to the TEE. The TEE will then
> > > > decrypt
> > > > the video and store the result in the 'secure buffer'. The REE
> > > > will
> > > > then allocate a 'secure surface'. The REE will pass the 'secure
> > > > handles' for both the 'secure buffer' and 'secure surface' into
> > > > the
> > > > TEE for video decoding. The video decoder HW will then decode
> > > > the
> > > > contents of the 'secure buffer' and place the result in the
> > > > 'secure
> > > > surface'. The REE will then attach the 'secure surface' to the
> > > > overlay
> > > > plane for rendering of the video.
> > > > 
> > > > Everything relating to ensuring security of the actual contents
> > > > of
> > > > the
> > > > 'secure buffer' and 'secure surface' is out of scope for the
> > > > REE
> > > > and
> > > > is the responsibility of the TEE.
> > > > 
> > > > DRM driver handles allocation of gem objects that are backed by
> > > > a
> > > > 'secure
> > > > surface' and for displaying a 'secure surface' on the overlay
> > > > plane.
> > > > This introduces a new flag for object creation called
> > > > DRM_MTK_GEM_CREATE_RESTRICTED which indicates it should be a
> > > > 'secure
> > > > surface'. All changes here are in MediaTek specific code.
> > > > ---
> > > > TODO:
> > > > 1) Drop MTK_DRM_IOCTL_GEM_CREATE and use DMA_HEAP_IOCTL_ALLOC
> > > > in
> > > > userspace
> > > > 2) DRM driver use secure mailbox channel to handle normal and
> > > > secure flow
> > > > 3) Implement setting mmsys routing table in the secure world
> > > > series
> > > 
> > > I'm not sure what you mean here. Why are you trying to upstream
> > > something that still needs to be removed from your patch series?
> > > 
> > 
> > Because their is too much patches need to be fixed in this series,
> > so I
> > list down the remaining TODO items and send to review for the other
> > patches.
> > 
> > Sorry for the bothering, I'll drop this at the next version.
> 
> If you don't intend to use it, we just shouldn't add it. Removing the
> TODO item doesn't make sense, even more so if heaps should be the way
> you handle this.
> 
Sorry for this misunderstanding.

I mean I'll remove the DRM_IOCTL_GEM_CREATE patch and then change user
space calling DMA_HEAP_IOCTL_ALLOC to allocate buffer from secure heap.

> > > Also, I made some comments on the previous version that have been
> > > entirely ignored and still apply on this version:
> > > 
> > 
> > 
https://lore.kernel.org/dri-devel/20240415-guppy-of-perpetual-current-3a7974@houat/
> > > 
> > 
> > I lost that mail in my mailbox, so I didn't reply at that time.
> > I have imported that mail and replied to you. Hope you don't mind
> > :)
> 
> I haven't received that answer

I don't know why it doesn't show up at your link.

Could you see it here?

https://patchwork.kernel.org/project/linux-mediatek/patch/20240403102701.369-3-shawn.sung@xxxxxxxxxxxx/



Regards,
Jason-JH.Lin
> 
> Maxime