Re: [PATCH 0/9] dma-buf: heaps: Add MediaTek secure heap

[Vijayanand Jitta]

On 9/11/2023 8:00 AM, Yong Wu wrote:
> This patchset consists of two parts, the first is from John and TJ.
> It adds some heap interfaces, then our kernel users could allocate buffer
> from special heap. The second part is adding MTK secure heap for SVP
> (Secure Video Path). A total of two heaps are added, one is mtk_svp and
> the other is mtk_svp_cma. The mtk_svp buffer is reserved for the secure
> world after bootup and it is used for ES/working buffer, while the
> mtk_svp_cma buffer is dynamically reserved for the secure world and will
> be get ready when we start playing secure videos, this heap is used for the
> frame buffer. Once the security video playing is complete, the CMA will be
> released.
> 
> For easier viewing, I've split the new heap file into several patches.
> 
> The consumers of new heap and new interfaces are our codec and drm which
> will send upstream soon, probably this week.
> 
> Base on v6.6-rc1.
> 
> John Stultz (2):
>   dma-heap: Add proper kref handling on dma-buf heaps
>   dma-heap: Provide accessors so that in-kernel drivers can allocate
>     dmabufs from specific heaps
> 
> T.J. Mercier (1):
>   dma-buf: heaps: Deduplicate docs and adopt common format
> 
> Yong Wu (6):
>   dma-buf: heaps: Initialise MediaTek secure heap
>   dma-buf: heaps: mtk_sec_heap: Initialise tee session
>   dma-buf: heaps: mtk_sec_heap: Add tee service call for buffer
>     allocating/freeing
>   dma-buf: heaps: mtk_sec_heap: Add dma_ops
>   dt-bindings: reserved-memory: MediaTek: Add reserved memory for SVP
>   dma_buf: heaps: mtk_sec_heap: Add a new CMA heap
> 
>  .../mediatek,secure_cma_chunkmem.yaml         |  42 ++
>  drivers/dma-buf/dma-heap.c                    | 127 +++--
>  drivers/dma-buf/heaps/Kconfig                 |   8 +
>  drivers/dma-buf/heaps/Makefile                |   1 +
>  drivers/dma-buf/heaps/mtk_secure_heap.c       | 458 ++++++++++++++++++
>  include/linux/dma-heap.h                      |  42 +-
>  6 files changed, 630 insertions(+), 48 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/reserved-memory/mediatek,secure_cma_chunkmem.yaml
>  create mode 100644 drivers/dma-buf/heaps/mtk_secure_heap.c
> 

Thanks for this patch series.

In Qualcomm as well we have similar usecases which need secure heap. We are working on
posting them upstream, would share more details on usecases soon.

Have few comments on the current implementation.

1) I see most the implementation here is mtk specific, even file names ,heap names etc.
   But secure heap is a common requirement, can we keep naming as well generic may be secure_heap ?

2) secure heap has two parts, one is allocation and other one is securing the memory.
   Have few comments on making these interfaces generic, would post those on corresponding 
   patches.

Thanks,
Vijay