On Wed, May 15, 2024 at 07:23:06PM GMT, Yong Wu wrote:
> Add a MediaTek restricted heap which uses TEE service call to restrict
> buffer. Currently this restricted heap is NULL, Prepare for the later
> patch. Mainly there are two changes:
> a) Add a heap_init ops since TEE probe late than restricted heap, thus
>    initialize the heap when we require the buffer the first time.
> b) Add a priv_data for each heap, like the special data used by MTK
>    (such as "TEE session") can be placed in priv_data.
> 
> Currently our heap depends on CMA which could only be bool, thus
> depend on "TEE=y".
> 
> Signed-off-by: Yong Wu <yong...@mediatek.com>
> ---
>  drivers/dma-buf/heaps/Kconfig               |   7 ++
>  drivers/dma-buf/heaps/Makefile              |   1 +
>  drivers/dma-buf/heaps/restricted_heap.c     |  11 ++
>  drivers/dma-buf/heaps/restricted_heap.h     |   2 +
>  drivers/dma-buf/heaps/restricted_heap_mtk.c | 115 ++++++++++++++++++++
>  5 files changed, 136 insertions(+)
>  create mode 100644 drivers/dma-buf/heaps/restricted_heap_mtk.c
> 
> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> index e54506f480ea..84f748fb2856 100644
> --- a/drivers/dma-buf/heaps/Kconfig
> +++ b/drivers/dma-buf/heaps/Kconfig
> @@ -21,3 +21,10 @@ config DMABUF_HEAPS_RESTRICTED
>         heap is to manage buffers that are inaccessible to the kernel and 
> user space.
>         There may be several ways to restrict it, for example it may be 
> encrypted or
>         protected by a TEE or hypervisor. If in doubt, say N.
> +
> +config DMABUF_HEAPS_RESTRICTED_MTK
> +     bool "MediaTek DMA-BUF Restricted Heap"
> +     depends on DMABUF_HEAPS_RESTRICTED && TEE=y
> +     help
> +       Enable restricted dma-buf heaps for MediaTek platform. This heap is 
> backed by
> +       TEE client interfaces. If in doubt, say N.
> diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
> index a2437c1817e2..0028aa9d875f 100644
> --- a/drivers/dma-buf/heaps/Makefile
> +++ b/drivers/dma-buf/heaps/Makefile
> @@ -1,4 +1,5 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_DMABUF_HEAPS_CMA)               += cma_heap.o
>  obj-$(CONFIG_DMABUF_HEAPS_RESTRICTED)        += restricted_heap.o
> +obj-$(CONFIG_DMABUF_HEAPS_RESTRICTED_MTK)    += restricted_heap_mtk.o
>  obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)    += system_heap.o
> diff --git a/drivers/dma-buf/heaps/restricted_heap.c 
> b/drivers/dma-buf/heaps/restricted_heap.c
> index 4e45d46a6467..8bc8a5e3f969 100644
> --- a/drivers/dma-buf/heaps/restricted_heap.c
> +++ b/drivers/dma-buf/heaps/restricted_heap.c
> @@ -151,11 +151,22 @@ restricted_heap_allocate(struct dma_heap *heap, 
> unsigned long size,
>                        unsigned long fd_flags, unsigned long heap_flags)
>  {
>       struct restricted_heap *rheap = dma_heap_get_drvdata(heap);
> +     const struct restricted_heap_ops *ops = rheap->ops;
>       struct restricted_buffer *restricted_buf;
>       DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
>       struct dma_buf *dmabuf;
>       int ret;
>  
> +     /*
> +      * In some implements, TEE is required to protect buffer. However TEE 
> probe
> +      * may be late, Thus heap_init is performed when the first buffer is 
> requested.
> +      */
> +     if (ops->heap_init) {
> +             ret = ops->heap_init(rheap);
> +             if (ret)
> +                     return ERR_PTR(ret);
> +     }

I wonder if we should make this parameterized rather than the default.
Perhaps we can add a "init_on_demand" (or whatever other name) flag to
struct restricted_heap_ops and then call this from heap initialization
if possible and defer initialization depending on the restricted heap
provider?

> +
>       restricted_buf = kzalloc(sizeof(*restricted_buf), GFP_KERNEL);
>       if (!restricted_buf)
>               return ERR_PTR(-ENOMEM);
> diff --git a/drivers/dma-buf/heaps/restricted_heap.h 
> b/drivers/dma-buf/heaps/restricted_heap.h
> index 6d9599a4a34e..2a33a1c7a48b 100644
> --- a/drivers/dma-buf/heaps/restricted_heap.h
> +++ b/drivers/dma-buf/heaps/restricted_heap.h
> @@ -19,6 +19,8 @@ struct restricted_heap {
>       const char              *name;
>  
>       const struct restricted_heap_ops *ops;
> +
> +     void                    *priv_data;

Honestly, I would just get rid of any of this extra padding/indentation
in these structures. There's really no benefit to this, except maybe if
you *really* like things to be aligned, in which case the above is now
probably worse than if you didn't try to align in the first place.

Thierry

Attachment: signature.asc
Description: PGP signature

Reply via email to