Re: [PATCH v3 00/11] Add mediate-drm secure flow for SVP

["Jason-JH Lin (林睿祥)" <Jason-JH.Lin@mediatek.com>]

Hi CK,

Thanks for the reviews.

On Thu, 2023-12-28 at 06:27 +0000, CK Hu (胡俊光) wrote:
> Hi, Jason:
> 
> On Sun, 2023-12-24 at 02:29 +0800, Jason-JH.Lin wrote:
> > From: Jason-jh Lin <jason-jh.lin@mediatek.corp-partner.google.com>
> > 
> > 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_ENCRYPTED which indicates it should be a 'secure
> > surface'. All changes here are in MediaTek specific code.
> 
> I would like to decouple secure display and secure decode. One reason
> is that I would like secure display could be tested itself without
> secure decode. Another reason is that if someone has draw an image
> and
> want to display securely, this is not related to decode.
> 
> To achieve this, mediatek drm driver should provide render function
> on
> secure surface. The most simple function is to bitblt a normal
> surface
> onto secure surface. User could allocate both normal surface and
> secure
> surface, draw on normal surface and bitblt normal surface onto secure
> surface. We could have limitation that normal surface and secure
> surface have the same width, height, pitch, pixel format, and the
> bitblt is the full image bitblt. So mediatek drm driver just need a
> TEE
> function that do memory copy from normal surface to secure surface.
> 
> This is not a must-be function, but it has some benefit for secure
> display.
> 
> Regards,
> CK
> 

OK, I'll also add this to TODO.

Regards,
Jason-JH.Lin

> > ---
> > TODO:
> > 1) Remove get sec larb port interface in ddp_comp, ovl and
> > ovl_adaptor.
> > 2) Verify instruction for enabling/disabling dapc and larb port in
> > TEE
> >    drop the sec_engine flags in normal world and.
> > 3) Move DISP_REG_OVL_SECURE setting to secure world for
> > mtk_disp_ovl.c.
> > 4) Change the parameter register address in mtk_ddp_sec_write()
> >    from "u32 addr" to "struct cmdq_client_reg *cmdq_reg".
> > 5) Implement setting mmsys routing table in the secure world
> > series.
> > ---
> > Based on 5 series and 1 patch:
> > [1] v3 dma-buf: heaps: Add MediaTek secure heap
> > - 
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/list/?series=809023__;!!CTRNKA9wMg0ARbw!lYGWfjjIBlxJvwBXWyxHTyc2vew5YagqT_qJZrYONTH20h95qxC3PH9V91vjplYU3S0ayseyHpxRQFSqATHgDnU$
> >  
> > [2] v3 add driver to support secure video decoder
> > - 
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/list/?series=807308__;!!CTRNKA9wMg0ARbw!lYGWfjjIBlxJvwBXWyxHTyc2vew5YagqT_qJZrYONTH20h95qxC3PH9V91vjplYU3S0ayseyHpxRQFSq9TXMSIQ$
> >  
> > [3] v4 soc: mediatek: Add register definitions for GCE
> > - 
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/patch/20231212121957.19231-2-shawn.sung@mediatek.com/__;!!CTRNKA9wMg0ARbw!lYGWfjjIBlxJvwBXWyxHTyc2vew5YagqT_qJZrYONTH20h95qxC3PH9V91vjplYU3S0ayseyHpxRQFSqkO4_0ac$
> >  
> > [4] v2 Add CMDQ driver support for mt8188
> > - 
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/list/?series=810302__;!!CTRNKA9wMg0ARbw!lYGWfjjIBlxJvwBXWyxHTyc2vew5YagqT_qJZrYONTH20h95qxC3PH9V91vjplYU3S0ayseyHpxRQFSqcXdKnXU$
> >  
> > [5] Add mediatek,gce-events definition to mediatek,gce-mailbox
> > bindings
> > - 
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/list/?series=810938__;!!CTRNKA9wMg0ARbw!lYGWfjjIBlxJvwBXWyxHTyc2vew5YagqT_qJZrYONTH20h95qxC3PH9V91vjplYU3S0ayseyHpxRQFSqqGM08aE$
> >  
> > [6] v3 Add CMDQ secure driver for SVP
> > - 
> > 
https://urldefense.com/v3/__https://patchwork.kernel.org/project/linux-mediatek/list/?series=812379__;!!CTRNKA9wMg0ARbw!lYGWfjjIBlxJvwBXWyxHTyc2vew5YagqT_qJZrYONTH20h95qxC3PH9V91vjplYU3S0ayseyHpxRQFSq_YXTH9A$
> >  
> > ---
> > Change in v3:
> > 1. fix kerneldoc problems
> > 2. fix typo in title and commit message
> > 3. adjust naming for secure variable
> > 4. add the missing part for is_suecure plane implementation
> > 5. use BIT_ULL macro to replace bit shifting
> > 6. move modification of ovl_adaptor part to the correct patch
> > 7. add TODO list in commit message
> > 8. add commit message for using share memory to store execute count
> > 
> > Change in v2:
> > 
> > 1. remove the DRIVER_RDNDER flag for mtk_drm_ioctl
> > 2. move cmdq_insert_backup_cookie into client driver
> > 3. move secure gce node define from mt8195-cherry.dtsi to
> > mt8195.dtsi
> > ---
> > CK Hu (1):
> >   drm/mediatek: Add interface to allocate MediaTek GEM buffer.
> > 
> > Jason-JH.Lin (10):
> >   drm/mediatek/uapi: Add DRM_MTK_GEM_CREATE_ENCRYPTED flag
> >   drm/mediatek: Add secure buffer control flow to mtk_drm_gem
> >   drm/mediatek: Add secure identify flag and funcution to
> > mtk_drm_plane
> >   drm/mediatek: Add mtk_ddp_sec_write to config secure buffer info
> >   drm/mediatek: Add get_sec_port interface to mtk_ddp_comp
> >   drm/mediatek: Add secure layer config support for ovl
> >   drm/mediatek: Add secure layer config support for ovl_adaptor
> >   drm/mediatek: Add secure flow support to mediatek-drm
> >   drm/mediatek: Add cmdq_insert_backup_cookie before secure pkt
> > finalize
> >   arm64: dts: mt8195: Add secure mbox settings for vdosys
> > 
> >  arch/arm64/boot/dts/mediatek/mt8195.dtsi      |   6 +-
> >  drivers/gpu/drm/mediatek/mtk_disp_drv.h       |   3 +
> >  drivers/gpu/drm/mediatek/mtk_disp_ovl.c       |  31 +-
> >  .../gpu/drm/mediatek/mtk_disp_ovl_adaptor.c   |  15 +
> >  drivers/gpu/drm/mediatek/mtk_drm_crtc.c       | 274
> > +++++++++++++++++-
> >  drivers/gpu/drm/mediatek/mtk_drm_crtc.h       |   1 +
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.c   |  30 ++
> >  drivers/gpu/drm/mediatek/mtk_drm_ddp_comp.h   |  14 +
> >  drivers/gpu/drm/mediatek/mtk_drm_drv.c        |  13 +
> >  drivers/gpu/drm/mediatek/mtk_drm_gem.c        | 122 ++++++++
> >  drivers/gpu/drm/mediatek/mtk_drm_gem.h        |  16 +
> >  drivers/gpu/drm/mediatek/mtk_drm_plane.c      |  26 ++
> >  drivers/gpu/drm/mediatek/mtk_drm_plane.h      |   2 +
> >  drivers/gpu/drm/mediatek/mtk_mdp_rdma.c       |  11 +-
> >  drivers/gpu/drm/mediatek/mtk_mdp_rdma.h       |   2 +
> >  include/uapi/drm/mediatek_drm.h               |  59 ++++
> >  16 files changed, 607 insertions(+), 18 deletions(-)
> >  create mode 100644 include/uapi/drm/mediatek_drm.h
> >