[4/9] dma-buf: heaps: Initialise MediaTek secure heap

  Initialise a mtk_svp heap. Currently just add a null heap, Prepare for
the later patches.

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
 drivers/dma-buf/heaps/Kconfig           |  8 ++
 drivers/dma-buf/heaps/Makefile          |  1 +
 drivers/dma-buf/heaps/mtk_secure_heap.c | 99 +++++++++++++++++++++++++
 3 files changed, 108 insertions(+)
 create mode 100644 drivers/dma-buf/heaps/mtk_secure_heap.c
  

Hi Yong,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on robh/for-next linus/master v6.6-rc1 next-20230911]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Yong-Wu/dma-buf-heaps-Deduplicate-docs-and-adopt-common-format/20230911-103308
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20230911023038.30649-5-yong.wu%40mediatek.com
patch subject: [PATCH 4/9] dma-buf: heaps: Initialise MediaTek secure heap
config: openrisc-allmodconfig (https://download.01.org/0day-ci/archive/20230911/202309111534.u4wfJ4vk-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230911/202309111534.u4wfJ4vk-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202309111534.u4wfJ4vk-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/dma-buf/heaps/mtk_secure_heap.c:68:27: error: initialization of 'struct dma_buf * (*)(struct dma_heap *, long unsigned int,  long unsigned int,  long unsigned int)' from incompatible pointer type 'struct dma_buf * (*)(struct dma_heap *, size_t,  long unsigned int,  long unsigned int)' {aka 'struct dma_buf * (*)(struct dma_heap *, unsigned int,  long unsigned int,  long unsigned int)'} [-Werror=incompatible-pointer-types]
      68 |         .allocate       = mtk_sec_heap_allocate,
         |                           ^~~~~~~~~~~~~~~~~~~~~
   drivers/dma-buf/heaps/mtk_secure_heap.c:68:27: note: (near initialization for 'mtk_sec_heap_ops.allocate')
   cc1: some warnings being treated as errors


vim +68 drivers/dma-buf/heaps/mtk_secure_heap.c

    66	
    67	static const struct dma_heap_ops mtk_sec_heap_ops = {
  > 68		.allocate	= mtk_sec_heap_allocate,
    69	};
    70
  

On Mon, Sep 11, 2023 at 10:30:33AM +0800, Yong Wu wrote:
> Initialise a mtk_svp heap. Currently just add a null heap, Prepare for
> the later patches.
> 
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> ---
>  drivers/dma-buf/heaps/Kconfig           |  8 ++
>  drivers/dma-buf/heaps/Makefile          |  1 +
>  drivers/dma-buf/heaps/mtk_secure_heap.c | 99 +++++++++++++++++++++++++
>  3 files changed, 108 insertions(+)
>  create mode 100644 drivers/dma-buf/heaps/mtk_secure_heap.c
> 
> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> index a5eef06c4226..729c0cf3eb7c 100644
> --- a/drivers/dma-buf/heaps/Kconfig
> +++ b/drivers/dma-buf/heaps/Kconfig
> @@ -12,3 +12,11 @@ config DMABUF_HEAPS_CMA
>  	  Choose this option to enable dma-buf CMA heap. This heap is backed
>  	  by the Contiguous Memory Allocator (CMA). If your system has these
>  	  regions, you should say Y here.
> +
> +config DMABUF_HEAPS_MTK_SECURE
> +	bool "DMA-BUF MediaTek Secure Heap"
> +	depends on DMABUF_HEAPS && TEE
> +	help
> +	  Choose this option to enable dma-buf MediaTek secure heap for Secure
> +	  Video Path. This heap is backed by TEE client interfaces. If in
Although this is intended for SVP right now, this is something that very
well could work for other use cases. So, I think I'd not mention "Secure
Video Path" and just mention "secure heap".

> +	  doubt, say N.
> diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
> index 974467791032..df559dbe33fe 100644
> --- a/drivers/dma-buf/heaps/Makefile
> +++ b/drivers/dma-buf/heaps/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)	+= system_heap.o
>  obj-$(CONFIG_DMABUF_HEAPS_CMA)		+= cma_heap.o
> +obj-$(CONFIG_DMABUF_HEAPS_MTK_SECURE)	+= mtk_secure_heap.o
> diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c b/drivers/dma-buf/heaps/mtk_secure_heap.c
> new file mode 100644
> index 000000000000..bbf1c8dce23e
> --- /dev/null
> +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DMABUF mtk_secure_heap exporter
> + *
> + * Copyright (C) 2023 MediaTek Inc.
> + */
> +
> +#include <linux/dma-buf.h>
> +#include <linux/dma-heap.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +
> +/*
> + * MediaTek secure (chunk) memory type
> + *
> + * @KREE_MEM_SEC_CM_TZ: static chunk memory carved out for trustzone.
nit: s/trustzone/TrustZone/
  

On 9/11/2023 8:00 AM, Yong Wu wrote:
> Initialise a mtk_svp heap. Currently just add a null heap, Prepare for
> the later patches.
> 
> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> ---
>  drivers/dma-buf/heaps/Kconfig           |  8 ++
>  drivers/dma-buf/heaps/Makefile          |  1 +
>  drivers/dma-buf/heaps/mtk_secure_heap.c | 99 +++++++++++++++++++++++++
>  3 files changed, 108 insertions(+)
>  create mode 100644 drivers/dma-buf/heaps/mtk_secure_heap.c
> 
> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
> index a5eef06c4226..729c0cf3eb7c 100644
> --- a/drivers/dma-buf/heaps/Kconfig
> +++ b/drivers/dma-buf/heaps/Kconfig
> @@ -12,3 +12,11 @@ config DMABUF_HEAPS_CMA
>  	  Choose this option to enable dma-buf CMA heap. This heap is backed
>  	  by the Contiguous Memory Allocator (CMA). If your system has these
>  	  regions, you should say Y here.
> +
> +config DMABUF_HEAPS_MTK_SECURE
> +	bool "DMA-BUF MediaTek Secure Heap"
> +	depends on DMABUF_HEAPS && TEE
> +	help
> +	  Choose this option to enable dma-buf MediaTek secure heap for Secure
> +	  Video Path. 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 974467791032..df559dbe33fe 100644
> --- a/drivers/dma-buf/heaps/Makefile
> +++ b/drivers/dma-buf/heaps/Makefile
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0
>  obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)	+= system_heap.o
>  obj-$(CONFIG_DMABUF_HEAPS_CMA)		+= cma_heap.o
> +obj-$(CONFIG_DMABUF_HEAPS_MTK_SECURE)	+= mtk_secure_heap.o
> diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c b/drivers/dma-buf/heaps/mtk_secure_heap.c
> new file mode 100644
> index 000000000000..bbf1c8dce23e
> --- /dev/null
> +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c
> @@ -0,0 +1,99 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * DMABUF mtk_secure_heap exporter
> + *
> + * Copyright (C) 2023 MediaTek Inc.
> + */
> +
> +#include <linux/dma-buf.h>
> +#include <linux/dma-heap.h>
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +
> +/*
> + * MediaTek secure (chunk) memory type
> + *
> + * @KREE_MEM_SEC_CM_TZ: static chunk memory carved out for trustzone.
> + */
> +enum kree_mem_type {
> +	KREE_MEM_SEC_CM_TZ = 1,
> +};
> +
> +struct mtk_secure_heap_buffer {
> +	struct dma_heap		*heap;
> +	size_t			size;
> +};
> +
> +struct mtk_secure_heap {
> +	const char		*name;
> +	const enum kree_mem_type mem_type;
> +};
> +
> +static struct dma_buf *
> +mtk_sec_heap_allocate(struct dma_heap *heap, size_t size,
> +		      unsigned long fd_flags, unsigned long heap_flags)
> +{
> +	struct mtk_secure_heap_buffer *sec_buf;
> +	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> +	struct dma_buf *dmabuf;
> +	int ret;
> +
> +	sec_buf = kzalloc(sizeof(*sec_buf), GFP_KERNEL);

As we know, kzalloc can only allocate 4MB at max. So, secure heap has this limitation.
can we have a way to allocate more memory in secure heap ? maybe similar to how system heap does?

Thanks,
Vijay

> +	if (!sec_buf)
> +		return ERR_PTR(-ENOMEM);
> +
> +	sec_buf->size = size;
> +	sec_buf->heap = heap;
> +
> +	exp_info.exp_name = dma_heap_get_name(heap);
> +	exp_info.size = sec_buf->size;
> +	exp_info.flags = fd_flags;
> +	exp_info.priv = sec_buf;
> +
> +	dmabuf = dma_buf_export(&exp_info);
> +	if (IS_ERR(dmabuf)) {
> +		ret = PTR_ERR(dmabuf);
> +		goto err_free_buf;
> +	}
> +
> +	return dmabuf;
> +
> +err_free_buf:
> +	kfree(sec_buf);
> +	return ERR_PTR(ret);
> +}
> +
> +static const struct dma_heap_ops mtk_sec_heap_ops = {
> +	.allocate	= mtk_sec_heap_allocate,
> +};
> +
> +static struct mtk_secure_heap mtk_sec_heap[] = {
> +	{
> +		.name		= "mtk_svp",
> +		.mem_type	= KREE_MEM_SEC_CM_TZ,
> +	},
> +};
> +
> +static int mtk_sec_heap_init(void)
> +{
> +	struct mtk_secure_heap *sec_heap = mtk_sec_heap;
> +	struct dma_heap_export_info exp_info;
> +	struct dma_heap *heap;
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(mtk_sec_heap); i++, sec_heap++) {
> +		exp_info.name = sec_heap->name;
> +		exp_info.ops = &mtk_sec_heap_ops;
> +		exp_info.priv = (void *)sec_heap;
> +
> +		heap = dma_heap_add(&exp_info);
> +		if (IS_ERR(heap))
> +			return PTR_ERR(heap);
> +	}
> +	return 0;
> +}
> +
> +module_init(mtk_sec_heap_init);
> +MODULE_DESCRIPTION("MediaTek Secure Heap Driver");
> +MODULE_LICENSE("GPL");
  

On Thu, 2023-10-19 at 10:15 +0530, Vijayanand Jitta wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  
> 
> On 9/11/2023 8:00 AM, Yong Wu wrote:
> > Initialise a mtk_svp heap. Currently just add a null heap, Prepare
> for
> > the later patches.
> > 
> > Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> > ---
> >  drivers/dma-buf/heaps/Kconfig           |  8 ++
> >  drivers/dma-buf/heaps/Makefile          |  1 +
> >  drivers/dma-buf/heaps/mtk_secure_heap.c | 99
> +++++++++++++++++++++++++
> >  3 files changed, 108 insertions(+)
> >  create mode 100644 drivers/dma-buf/heaps/mtk_secure_heap.c
> > 
> > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-
> buf/heaps/Kconfig
> > index a5eef06c4226..729c0cf3eb7c 100644
> > --- a/drivers/dma-buf/heaps/Kconfig
> > +++ b/drivers/dma-buf/heaps/Kconfig
> > @@ -12,3 +12,11 @@ config DMABUF_HEAPS_CMA
> >    Choose this option to enable dma-buf CMA heap. This heap is
> backed
> >    by the Contiguous Memory Allocator (CMA). If your system has
> these
> >    regions, you should say Y here.
> > +
> > +config DMABUF_HEAPS_MTK_SECURE
> > +bool "DMA-BUF MediaTek Secure Heap"
> > +depends on DMABUF_HEAPS && TEE
> > +help
> > +  Choose this option to enable dma-buf MediaTek secure heap for
> Secure
> > +  Video Path. 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 974467791032..df559dbe33fe 100644
> > --- a/drivers/dma-buf/heaps/Makefile
> > +++ b/drivers/dma-buf/heaps/Makefile
> > @@ -1,3 +1,4 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)+= system_heap.o
> >  obj-$(CONFIG_DMABUF_HEAPS_CMA)+= cma_heap.o
> > +obj-$(CONFIG_DMABUF_HEAPS_MTK_SECURE)+= mtk_secure_heap.o
> > diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c b/drivers/dma-
> buf/heaps/mtk_secure_heap.c
> > new file mode 100644
> > index 000000000000..bbf1c8dce23e
> > --- /dev/null
> > +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c
> > @@ -0,0 +1,99 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * DMABUF mtk_secure_heap exporter
> > + *
> > + * Copyright (C) 2023 MediaTek Inc.
> > + */
> > +
> > +#include <linux/dma-buf.h>
> > +#include <linux/dma-heap.h>
> > +#include <linux/err.h>
> > +#include <linux/module.h>
> > +#include <linux/slab.h>
> > +
> > +/*
> > + * MediaTek secure (chunk) memory type
> > + *
> > + * @KREE_MEM_SEC_CM_TZ: static chunk memory carved out for
> trustzone.
> > + */
> > +enum kree_mem_type {
> > +KREE_MEM_SEC_CM_TZ = 1,
> > +};
> > +
> > +struct mtk_secure_heap_buffer {
> > +struct dma_heap*heap;
> > +size_tsize;
> > +};
> > +
> > +struct mtk_secure_heap {
> > +const char*name;
> > +const enum kree_mem_type mem_type;
> > +};
> > +
> > +static struct dma_buf *
> > +mtk_sec_heap_allocate(struct dma_heap *heap, size_t size,
> > +      unsigned long fd_flags, unsigned long heap_flags)
> > +{
> > +struct mtk_secure_heap_buffer *sec_buf;
> > +DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> > +struct dma_buf *dmabuf;
> > +int ret;
> > +
> > +sec_buf = kzalloc(sizeof(*sec_buf), GFP_KERNEL);
> 
> As we know, kzalloc can only allocate 4MB at max. So, secure heap has
> this limitation.
> can we have a way to allocate more memory in secure heap ? maybe
> similar to how system heap does?

This is just the size of a internal structure. I guess you mean the
secure memory size here. Regarding secure memory allocating flow, our
flow may be different with yours.

Let me explain our flow, we have two secure buffer types(heaps).
a) mtk_svp
b) mtk_svp_cma which requires the cma binding.

The memory management of both is inside the TEE. We only need to tell
the TEE which type and size of buffer we want, and then the TEE will
perform and return the memory handle to the kernel. The
kzalloc/alloc_pages is for the normal buffers.

Regarding the CMA buffer, we only call cma_alloc once, and its
management is also within the TEE.

> 
> Thanks,
> Vijay
>
  

On 10/20/2023 3:29 PM, Yong Wu (吴勇) wrote:
> On Thu, 2023-10-19 at 10:15 +0530, Vijayanand Jitta wrote:
>>  	 
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>  
>>
>> On 9/11/2023 8:00 AM, Yong Wu wrote:
>>> Initialise a mtk_svp heap. Currently just add a null heap, Prepare
>> for
>>> the later patches.
>>>
>>> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
>>> ---
>>>  drivers/dma-buf/heaps/Kconfig           |  8 ++
>>>  drivers/dma-buf/heaps/Makefile          |  1 +
>>>  drivers/dma-buf/heaps/mtk_secure_heap.c | 99
>> +++++++++++++++++++++++++
>>>  3 files changed, 108 insertions(+)
>>>  create mode 100644 drivers/dma-buf/heaps/mtk_secure_heap.c
>>>
>>> diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-
>> buf/heaps/Kconfig
>>> index a5eef06c4226..729c0cf3eb7c 100644
>>> --- a/drivers/dma-buf/heaps/Kconfig
>>> +++ b/drivers/dma-buf/heaps/Kconfig
>>> @@ -12,3 +12,11 @@ config DMABUF_HEAPS_CMA
>>>    Choose this option to enable dma-buf CMA heap. This heap is
>> backed
>>>    by the Contiguous Memory Allocator (CMA). If your system has
>> these
>>>    regions, you should say Y here.
>>> +
>>> +config DMABUF_HEAPS_MTK_SECURE
>>> +bool "DMA-BUF MediaTek Secure Heap"
>>> +depends on DMABUF_HEAPS && TEE
>>> +help
>>> +  Choose this option to enable dma-buf MediaTek secure heap for
>> Secure
>>> +  Video Path. 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 974467791032..df559dbe33fe 100644
>>> --- a/drivers/dma-buf/heaps/Makefile
>>> +++ b/drivers/dma-buf/heaps/Makefile
>>> @@ -1,3 +1,4 @@
>>>  # SPDX-License-Identifier: GPL-2.0
>>>  obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)+= system_heap.o
>>>  obj-$(CONFIG_DMABUF_HEAPS_CMA)+= cma_heap.o
>>> +obj-$(CONFIG_DMABUF_HEAPS_MTK_SECURE)+= mtk_secure_heap.o
>>> diff --git a/drivers/dma-buf/heaps/mtk_secure_heap.c b/drivers/dma-
>> buf/heaps/mtk_secure_heap.c
>>> new file mode 100644
>>> index 000000000000..bbf1c8dce23e
>>> --- /dev/null
>>> +++ b/drivers/dma-buf/heaps/mtk_secure_heap.c
>>> @@ -0,0 +1,99 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +/*
>>> + * DMABUF mtk_secure_heap exporter
>>> + *
>>> + * Copyright (C) 2023 MediaTek Inc.
>>> + */
>>> +
>>> +#include <linux/dma-buf.h>
>>> +#include <linux/dma-heap.h>
>>> +#include <linux/err.h>
>>> +#include <linux/module.h>
>>> +#include <linux/slab.h>
>>> +
>>> +/*
>>> + * MediaTek secure (chunk) memory type
>>> + *
>>> + * @KREE_MEM_SEC_CM_TZ: static chunk memory carved out for
>> trustzone.
>>> + */
>>> +enum kree_mem_type {
>>> +KREE_MEM_SEC_CM_TZ = 1,
>>> +};
>>> +
>>> +struct mtk_secure_heap_buffer {
>>> +struct dma_heap*heap;
>>> +size_tsize;
>>> +};
>>> +
>>> +struct mtk_secure_heap {
>>> +const char*name;
>>> +const enum kree_mem_type mem_type;
>>> +};
>>> +
>>> +static struct dma_buf *
>>> +mtk_sec_heap_allocate(struct dma_heap *heap, size_t size,
>>> +      unsigned long fd_flags, unsigned long heap_flags)
>>> +{
>>> +struct mtk_secure_heap_buffer *sec_buf;
>>> +DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
>>> +struct dma_buf *dmabuf;
>>> +int ret;
>>> +
>>> +sec_buf = kzalloc(sizeof(*sec_buf), GFP_KERNEL);
>>
>> As we know, kzalloc can only allocate 4MB at max. So, secure heap has
>> this limitation.
>> can we have a way to allocate more memory in secure heap ? maybe
>> similar to how system heap does?
> 
> This is just the size of a internal structure. I guess you mean the
> secure memory size here. Regarding secure memory allocating flow, our
> flow may be different with yours.
> 
> Let me explain our flow, we have two secure buffer types(heaps).
> a) mtk_svp
> b) mtk_svp_cma which requires the cma binding.
> 
> The memory management of both is inside the TEE. We only need to tell
> the TEE which type and size of buffer we want, and then the TEE will
> perform and return the memory handle to the kernel. The
> kzalloc/alloc_pages is for the normal buffers.
> 
> Regarding the CMA buffer, we only call cma_alloc once, and its
> management is also within the TEE.
> 

Thanks for the details.

I see for mvp_svp, allocation is also specific to TEE, as TEE takes
care of allocation as well. 

I was thinking if allocation path can also be made generic ? without having
dependency on TEE.
For eg : A case where we want to allocate from kernel and secure that memory,
the current secure heap design can't be used. 

Also i suppose TEE allocates contiguous memory for mtk_svp ? or does it support
scattered memory ?

>>
>> Thanks,
>> Vijay
>>
  

On Thu, 2023-10-26 at 10:18 +0530, Vijayanand Jitta wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  
> 
> On 10/20/2023 3:29 PM, Yong Wu (吴勇) wrote:
> > On Thu, 2023-10-19 at 10:15 +0530, Vijayanand Jitta wrote:
> >>   
> >> External email : Please do not click links or open attachments
> until
> >> you have verified the sender or the content.
> >>  
> >>
> >> On 9/11/2023 8:00 AM, Yong Wu wrote:
> >>> Initialise a mtk_svp heap. Currently just add a null heap,
> Prepare
> >> for
> >>> the later patches.
> >>>
> >>> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
> >>> ---
> >>>  drivers/dma-buf/heaps/Kconfig           |  8 ++
> >>>  drivers/dma-buf/heaps/Makefile          |  1 +
> >>>  drivers/dma-buf/heaps/mtk_secure_heap.c | 99
> >> +++++++++++++++++++++++++

[...]

> >>> +
> >>> +static struct dma_buf *
> >>> +mtk_sec_heap_allocate(struct dma_heap *heap, size_t size,
> >>> +      unsigned long fd_flags, unsigned long heap_flags)
> >>> +{
> >>> +struct mtk_secure_heap_buffer *sec_buf;
> >>> +DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
> >>> +struct dma_buf *dmabuf;
> >>> +int ret;
> >>> +
> >>> +sec_buf = kzalloc(sizeof(*sec_buf), GFP_KERNEL);
> >>
> >> As we know, kzalloc can only allocate 4MB at max. So, secure heap
> has
> >> this limitation.
> >> can we have a way to allocate more memory in secure heap ? maybe
> >> similar to how system heap does?
> > 
> > This is just the size of a internal structure. I guess you mean the
> > secure memory size here. Regarding secure memory allocating flow,
> our
> > flow may be different with yours.
> > 
> > Let me explain our flow, we have two secure buffer types(heaps).
> > a) mtk_svp
> > b) mtk_svp_cma which requires the cma binding.
> > 
> > The memory management of both is inside the TEE. We only need to
> tell
> > the TEE which type and size of buffer we want, and then the TEE
> will
> > perform and return the memory handle to the kernel. The
> > kzalloc/alloc_pages is for the normal buffers.
> > 
> > Regarding the CMA buffer, we only call cma_alloc once, and its
> > management is also within the TEE.
> > 
> 
> Thanks for the details.
> 
> I see for mvp_svp, allocation is also specific to TEE, as TEE takes
> care of allocation as well.

Yes. The allocation management of these two heaps is in the TEE.

> 
> I was thinking if allocation path can also be made generic ? without
> having
> dependency on TEE.
> For eg : A case where we want to allocate from kernel and secure that
> memory,
> the current secure heap design can't be used. 

Sorry, This may be because our HW is special. The HW could protect a
certain region, but it can only protect 32 regions. So we cannot
allocate them in the kernel arbitrarily and then enter TEE to protect
them.

> 
> Also i suppose TEE allocates contiguous memory for mtk_svp ? or does
> it support
> scattered memory ?

Yes. After the TEE runs for a period of time, the TEE memory will
become discontinuous, and a secure IOMMU exists in the TEE.

> >>
> >> Thanks,
> >> Vijay
> >>
  

On 10/27/2023 1:17 PM, Yong Wu (吴勇) wrote:
> On Thu, 2023-10-26 at 10:18 +0530, Vijayanand Jitta wrote:
>>  	 
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>  
>>
>> On 10/20/2023 3:29 PM, Yong Wu (吴勇) wrote:
>>> On Thu, 2023-10-19 at 10:15 +0530, Vijayanand Jitta wrote:
>>>>   
>>>> External email : Please do not click links or open attachments
>> until
>>>> you have verified the sender or the content.
>>>>  
>>>>
>>>> On 9/11/2023 8:00 AM, Yong Wu wrote:
>>>>> Initialise a mtk_svp heap. Currently just add a null heap,
>> Prepare
>>>> for
>>>>> the later patches.
>>>>>
>>>>> Signed-off-by: Yong Wu <yong.wu@mediatek.com>
>>>>> ---
>>>>>  drivers/dma-buf/heaps/Kconfig           |  8 ++
>>>>>  drivers/dma-buf/heaps/Makefile          |  1 +
>>>>>  drivers/dma-buf/heaps/mtk_secure_heap.c | 99
>>>> +++++++++++++++++++++++++
> 
> [...]
> 
>>>>> +
>>>>> +static struct dma_buf *
>>>>> +mtk_sec_heap_allocate(struct dma_heap *heap, size_t size,
>>>>> +      unsigned long fd_flags, unsigned long heap_flags)
>>>>> +{
>>>>> +struct mtk_secure_heap_buffer *sec_buf;
>>>>> +DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
>>>>> +struct dma_buf *dmabuf;
>>>>> +int ret;
>>>>> +
>>>>> +sec_buf = kzalloc(sizeof(*sec_buf), GFP_KERNEL);
>>>>
>>>> As we know, kzalloc can only allocate 4MB at max. So, secure heap
>> has
>>>> this limitation.
>>>> can we have a way to allocate more memory in secure heap ? maybe
>>>> similar to how system heap does?
>>>
>>> This is just the size of a internal structure. I guess you mean the
>>> secure memory size here. Regarding secure memory allocating flow,
>> our
>>> flow may be different with yours.
>>>
>>> Let me explain our flow, we have two secure buffer types(heaps).
>>> a) mtk_svp
>>> b) mtk_svp_cma which requires the cma binding.
>>>
>>> The memory management of both is inside the TEE. We only need to
>> tell
>>> the TEE which type and size of buffer we want, and then the TEE
>> will
>>> perform and return the memory handle to the kernel. The
>>> kzalloc/alloc_pages is for the normal buffers.
>>>
>>> Regarding the CMA buffer, we only call cma_alloc once, and its
>>> management is also within the TEE.
>>>
>>
>> Thanks for the details.
>>
>> I see for mvp_svp, allocation is also specific to TEE, as TEE takes
>> care of allocation as well.
> 
> Yes. The allocation management of these two heaps is in the TEE.
> 
>>
>> I was thinking if allocation path can also be made generic ? without
>> having
>> dependency on TEE.
>> For eg : A case where we want to allocate from kernel and secure that
>> memory,
>> the current secure heap design can't be used. 
> 
> Sorry, This may be because our HW is special. The HW could protect a
> certain region, but it can only protect 32 regions. So we cannot
> allocate them in the kernel arbitrarily and then enter TEE to protect
> them.
> 

Got your point , I see for your case allocation must happen in TEE.
I was just saying if we want to make secure heap generic and remove
hard dependency on TEE, we must have a way to allocate irrespective
of what hypervisor/TZ being used. As current design for secure heap
assumes OPTEE.

We have a case where allocation happens in kernel and we secure it
using qcom_scm_assign_mem , this wouldn't be possible with current
design.

Probably some ops to allocate, similar to ops you pointed out to secure ?
in you case these ops would just allocate the internal structure.

Thanks,
Vijay

>>
>> Also i suppose TEE allocates contiguous memory for mtk_svp ? or does
>> it support
>> scattered memory ?
> 
> Yes. After the TEE runs for a period of time, the TEE memory will
> become discontinuous, and a secure IOMMU exists in the TEE.
> 
>>>>
>>>> Thanks,
>>>> Vijay
>>>>