Re: [EXT] Re: [PATCH v2 1/1] tee: new ioctl to a register tee_shm from a dmabuf file descriptor
On jeu., 2023-02-02 at 10:58 +0100, Etienne Carriere wrote: > Caution: EXT Email > > On Thu, 2 Feb 2023 at 09:35, Sumit Garg > wrote: > > Hi Cyrille, > > > > Please don't top post as it makes it harder to follow-up. > > > > On Thu, 2 Feb 2023 at 13:26, Cyrille Fleury > > wrote: > > > Hi Sumit, all > > > > > > Upstream OP-TEE should support registering a dmabuf since a > > > while, given how widely dmabuf is used in Linux for passing > > > buffers around between devices. > > > > > > Purpose of the new register_tee_shm ioctl is to allow OPTEE to > > > use memory allocated from the exiting linux dma buffer. We don't > > > need to have secure dma-heap up streamed. > > > > > > You mentioned secure dma-buffer, but secure dma-buffer is a dma- > > > buffer, so the work to be done for secure or "regular" dma > > > buffers by the register_tee_shm ioctl is 100% the same. > > > > > > The scope of this ioctl is limited to what existing upstream dma- > > > buffers are: > > > -> sharing buffers for hardware (DMA) access across > > > multiple device drivers and subsystems, and for synchronizing > > > asynchronous hardware access. > > >-> It means continuous memory only. > > > > > > So if we reduce the scope of register tee_shm to exiting dma- > > > buffer area, the current patch does the job. > > > > Do you have a corresponding real world use-case supported by > > upstream > > OP-TEE? AFAIK, the Secure Data Path (SDP) use-case is the one > > supported in OP-TEE upstream but without secure dmabuf heap [1] > > available, the new ioctl can't be exercised. > > > > [1] > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FOP-TEE%2Foptee_test%2Fblob%2Fmaster%2Fhost%2Fxtest%2Fsdp_basic.h%23L15=05%7C01%7Colivier.masse%40nxp.com%7Ca27f690d9d7244c2bcff08db05040f11%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638109287211847995%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=eNUbc0uaKjfmxau8L7ZB8u%2BtYdUxT4pIS%2Fht29uwRKg%3D=0 > > OP-TEE has some SDP test taht can exercice SDP: 'xtest > regression_1014'. > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FOP-TEE%2Foptee_test%2Fblob%2F3.20.0%2Fhost%2Fxtest%2Fregression_1000.c%23L1256=05%7C01%7Colivier.masse%40nxp.com%7Ca27f690d9d7244c2bcff08db05040f11%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638109287211847995%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=%2BOtAlVOq7%2Fi6SloSTZuwa5VtlC5RqtcJ4fGtio0YI8A%3D=0 > > The test relies on old staged ION + local secure dmabuf heaps no more > maintained, so this test is currently not functional. > If we upgrade the test to mainline dmabuf alloc means, and apply the > change discussed here, we should be able to regularly test SDP in > OP-TEE project CI. > The part to update is the userland allocation of the dmabuf: > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2FOP-TEE%2Foptee_test%2Fblob%2F3.20.0%2Fhost%2Fxtest%2Fsdp_basic.c%23L91=05%7C01%7Colivier.masse%40nxp.com%7Ca27f690d9d7244c2bcff08db05040f11%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638109287211847995%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C=K2NB2Bj7V3CXNsM9fZy95OEjF3EzqU4mgmM1PTY3J1Y%3D=0 > > the test was already updated to support secure dma heap with Kernel version 5.11 and higher. the userland allocation could be find here: https://github.com/OP-TEE/optee_test/blob/3.20.0/host/xtest/sdp_basic.c#L153 This upgrade need a Linux dma-buf patch: https://lore.kernel.org/all/20220805154139.2qkqxwklufjpsfdx@000377403353/T/ > br, > etienne > > > > -Sumit > > > > > Regards. > > > > > > -Original Message- > > > From: Sumit Garg > > > Sent: Wednesday, February 1, 2023 6:34 AM > > > To: Olivier Masse > > > Cc: fre...@google.com; linux-me...@vger.kernel.org; > > > linaro-mm-...@lists.linaro.org; a...@ti.com; > > > op-...@lists.trustedfirmware.org; jens.wiklan...@linaro.org; > > > joakim.b...@linaro.org; sumit.sem...@linaro.org; Peter Griffin < > > > peter.grif...@linaro.org>; linux-ker...@vger.kernel.org; > > > etienne.carri...@linaro.org; dri-devel@lists.freedesktop.org; > > > christian.koe...@amd.com; Clément Faure ; > > > Cyrille Fleury > > > Subject: [EXT] Re: [PATCH v2 1/1] tee: new ioctl to a register > > > tee_shm from a
Re: [PATCH v2 1/1] tee: new ioctl to a register tee_shm from a dmabuf file descriptor
Hi Joakim,
Hi Etienne,
Let me bring back this pull request for OPTEE Linux driver.
Last feedback was from Christian König and Sumit Garg.
>From Christian:
> Just two comments:
>
> 1. Dmitry is working on a change which renames some functions and
> makes
> it mandatory to call them with the dma_resv lock held.
>
> Depending on how you want to upstream this change you will certainly
> run
> into conflicts with that.
Is there any update on these changes ?
>
> 2. Would it be possible to do this dynamically? In other words does
> the
> tee driver has a concept of buffers moving around?
We do not support dynamic secure memory heap.
>From Sumit:
> What limits you to extend this feature to non-contiguous memory
> buffers? I believe that should be possible with OP-TEE dynamic shared
> memory which gives you the granularity to register a list of pages.
Our solution use a fixed protected reserved memory region and do not
rely on a dynamic protection managed in secure.
The scope of this implementation rely on a static memory region handled
by a specific DMA Heap type.
Best regards,
Olivier MASSE
On ven., 2022-08-12 at 16:30 +0200, Olivier Masse wrote:
> From: Etienne Carriere
>
> This change allows userland to create a tee_shm object that refers
> to a dmabuf reference.
>
> Userland provides a dmabuf file descriptor as buffer reference.
> The created tee_shm object exported as a brand new dmabuf reference
> used to provide a clean fd to userland. Userland shall closed this
> new
> fd to release the tee_shm object resources. The initial dmabuf
> resources
> are tracked independently through original dmabuf file descriptor.
>
> Once the buffer is registered and until it is released, TEE driver
> keeps a refcount on the registered dmabuf structure.
>
> This change only support dmabuf references that relates to physically
> contiguous memory buffers.
>
> New tee_shm flag to identify tee_shm objects built from a registered
> dmabuf: TEE_SHM_EXT_DMA_BUF. Such tee_shm structures are flagged with
> TEE_SHM_EXT_DMA_BUF.
>
> Co-Developed-by: Etienne Carriere
> Signed-off-by: Olivier Masse
> Reported-by: kernel test robot
> From: https://github.com/linaro-swg/linux.git
> (cherry picked from commit 41e21e5c405530590dc2dd10b2a8dbe64589840f)
> ---
> drivers/tee/tee_core.c | 38 +++
> drivers/tee/tee_shm.c| 99
> +++-
> include/linux/tee_drv.h | 11 +
> include/uapi/linux/tee.h | 29
> 4 files changed, 175 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> index 8aa1a4836b92..7c45cbf85eb9 100644
> --- a/drivers/tee/tee_core.c
> +++ b/drivers/tee/tee_core.c
> @@ -355,6 +355,42 @@ tee_ioctl_shm_register(struct tee_context *ctx,
> return ret;
> }
>
> +static int tee_ioctl_shm_register_fd(struct tee_context *ctx,
> + struct
> tee_ioctl_shm_register_fd_data __user *udata)
> +{
> + struct tee_ioctl_shm_register_fd_data data;
> + struct tee_shm *shm;
> + long ret;
> +
> + if (copy_from_user(, udata, sizeof(data)))
> + return -EFAULT;
> +
> + /* Currently no input flags are supported */
> + if (data.flags)
> + return -EINVAL;
> +
> + shm = tee_shm_register_fd(ctx, data.fd);
> + if (IS_ERR(shm))
> + return -EINVAL;
> +
> + data.id = shm->id;
> + data.flags = shm->flags;
> + data.size = shm->size;
> +
> + if (copy_to_user(udata, , sizeof(data)))
> + ret = -EFAULT;
> + else
> + ret = tee_shm_get_fd(shm);
> +
> + /*
> + * When user space closes the file descriptor the shared memory
> + * should be freed or if tee_shm_get_fd() failed then it will
> + * be freed immediately.
> + */
> + tee_shm_put(shm);
> + return ret;
> +}
> +
> static int params_from_user(struct tee_context *ctx, struct
> tee_param *params,
> size_t num_params,
> struct tee_ioctl_param __user *uparams)
> @@ -829,6 +865,8 @@ static long tee_ioctl(struct file *filp, unsigned
> int cmd, unsigned long arg)
> return tee_ioctl_shm_alloc(ctx, uarg);
> case TEE_IOC_SHM_REGISTER:
> return tee_ioctl_shm_register(ctx, uarg);
> + case TEE_IOC_SHM_REGISTER_FD:
> + return tee_ioctl_shm_register_fd(ctx, uarg);
> case TEE_IOC_OPEN_SESSION:
> return tee_ioctl_open_session(ctx, uarg);
> case TEE_IOC_INVOKE:
> diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
> index 836872467dc6..55a
Re: [EXT] Re: [PATCH v2 0/1] tee: Add tee_shm_register_fd
Hi Sumit On ven., 2022-08-19 at 13:54 +0530, Sumit Garg wrote: > Caution: EXT Email > > Hi Olivier, > > On Fri, 12 Aug 2022 at 20:01, Olivier Masse > wrote: > > > > Add a new ioctl called TEE_IOC_SHM_REGISTER_FD to register a > > shared memory from a dmabuf file descriptor. > > This new ioctl will allow the Linux Kernel to register a buffer > > to be used by the Secure Data Path OPTEE OS feature. > > > > Please find more information here: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstatic.linaro.org%2Fconnect%2Fsan19%2Fpresentations%2Fsan19-107.pdfdata=05%7C01%7Colivier.masse%40nxp.com%7C05071ff1c28044ab740908da81bc44e2%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637964942860947359%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=nYLb2iMoJExdKEg4CL4eW5%2FQ%2Bqqj4Iw1TxFsd1UqWW0%3Dreserved=0 > > > > Patch tested on Hikey 6220. > > > > AFAIU, for the OP-TEE SDP feature to work you need to have a DMA-BUF > heap driver for allocating secure buffers through exposed chardev: > "/dev/dma_heap/sdp". Have you tested it with some out-of-tree driver > as I can't find it upstream? Also, do you plan to push that upstream > as well? It has been tested with linaro,secure-heap reserved dma heap memory which is also in review for upstream. > > BTW, please add a changelog while sending newer patch-set versions. > > -Sumit > > > Etienne Carriere (1): > > tee: new ioctl to a register tee_shm from a dmabuf file > > descriptor > > > > drivers/tee/tee_core.c | 38 +++ > > drivers/tee/tee_shm.c| 99 > > +++- > > include/linux/tee_drv.h | 11 + > > include/uapi/linux/tee.h | 29 > > 4 files changed, 175 insertions(+), 2 deletions(-) > > > > -- > > 2.25.0 > >
Re: [EXT] Re: [PATCH 1/3] dma-buf: heaps: add Linaro secure dmabuf heap support
+Cyrille Hi Nicolas, On mer., 2022-08-17 at 10:29 -0400, Nicolas Dufresne wrote: > Caution: EXT Email > > Hi Folks, > > Le mardi 16 août 2022 à 11:20 +, Olivier Masse a écrit : > > Hi Brian, > > > > > > On ven., 2022-08-12 at 17:39 +0100, Brian Starkey wrote: > > > Caution: EXT Ema > > > > > [...] > > > > > > > Interesting, that's not how the devices I've worked on operated. > > > > > > Are you saying that you have to have a display controller driver > > > running in the TEE to display one of these buffers? > > > > In fact the display controller is managing 3 plans : UI, PiP and > > video. The video plan is protected in secure as you can see on > > slide > > 11: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstatic.linaro.org%2Fconnect%2Fsan19%2Fpresentations%2Fsan19-107.pdfdata=05%7C01%7Colivier.masse%40nxp.com%7Ce0e00be789a54dff8e5208da805ce2f6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637963433695707516%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=GHjEfbgqRkfHK16oyNaYJob4LRVqvoffRElKR%2F7Rtes%3Dreserved=0 > > > > just wanted to highlight that all the WPE/GStreamer bit in this > presentation is > based on NXP Vendor Media CODEC design, which rely on their own i.MX > VPU API. I > don't see any effort to extend this to a wider audience. It is not > explaining > how this can work with a mainline kernel with v4l2 stateful or > stateless drivers > and generic GStreamer/FFMPEG/Chromium support. Maybe Cyrille can explain what it is currently done at NXP level regarding the integration of v4l2 with NXP VPU. > > I'm raising this, since I'm worried that no one cares of solving that > high level > problem from a generic point of view. In that context, any additions > to the > mainline Linux kernel can only be flawed and will only serves > specific vendors > and not the larger audience. > > Another aspect, is that this design might be bound to a specific (NXP > ?) > security design. I've learn recently that newer HW is going to use > multiple > level of MMU (like virtual machines do) to protect the memory rather > then > marking pages. Will all this work for that too ? our fire-walling hardware is protecting memory behind the MMU and so rely on physical memory layout. this work is only relying on a reserved physical memory. Regards, Olivier > > regards, > Nicolas
Re: [EXT] Re: [PATCH 3/5] dma-buf: heaps: add Linaro secure dmabuf heap support
Hi Nicolas, Thanks for your comment, indeed these PR is linked to OPTEE OS. This one is using the same bindings to define the Secure Data Path reserved memory: https://github.com/OP-TEE/optee_os/commit/eb108a04369fbfaf60c03c0e00bbe9489a761c69 However, I'm not aware of another shared heap that could match our need. For information. it was previously done using ION heap: https://www.slideshare.net/linaroorg/bud17400-secure-data-path-with-optee Best regards, Olivier On mar., 2022-08-16 at 09:31 -0400, Nicolas Dufresne wrote: > Caution: EXT Email > > Hi, > > Le mardi 02 août 2022 à 11:58 +0200, Olivier Masse a écrit : > > add Linaro secure heap bindings: linaro,secure-heap > > Just a curiosity, how is this specific to Linaro OPTEE OS ? Shouldn't > it be "de- > linaro-ified" somehow ? > > regards, > Nicolas > > > use genalloc to allocate/free buffer from buffer pool. > > buffer pool info is from dts. > > use sg_table instore the allocated memory info, the length of > > sg_table is 1. > > implement secure_heap_buf_ops to implement buffer share in > > difference device: > > 1. Userspace passes this fd to all drivers it wants this buffer > > to share with: First the filedescriptor is converted to a _buf > > using > > dma_buf_get(). Then the buffer is attached to the device using > > dma_buf_attach(). > > 2. Once the buffer is attached to all devices userspace can > > initiate DMA > > access to the shared buffer. In the kernel this is done by calling > > dma_buf_map_attachment() > > 3. get sg_table with dma_buf_map_attachment in difference device. > > > > Signed-off-by: Olivier Masse > > --- > > drivers/dma-buf/heaps/Kconfig | 21 +- > > drivers/dma-buf/heaps/Makefile | 1 + > > drivers/dma-buf/heaps/secure_heap.c | 588 > > > > 3 files changed, 606 insertions(+), 4 deletions(-) > > create mode 100644 drivers/dma-buf/heaps/secure_heap.c > > > > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma- > > buf/heaps/Kconfig > > index 6a33193a7b3e..b2406932192e 100644 > > --- a/drivers/dma-buf/heaps/Kconfig > > +++ b/drivers/dma-buf/heaps/Kconfig > > @@ -1,8 +1,12 @@ > > -config DMABUF_HEAPS_DEFERRED_FREE > > - tristate > > +menuconfig DMABUF_HEAPS_DEFERRED_FREE > > + bool "DMA-BUF heaps deferred-free library" > > + help > > + Choose this option to enable the DMA-BUF heaps deferred- > > free library. > > > > -config DMABUF_HEAPS_PAGE_POOL > > - tristate > > +menuconfig DMABUF_HEAPS_PAGE_POOL > > + bool "DMA-BUF heaps page-pool library" > > + help > > + Choose this option to enable the DMA-BUF heaps page-pool > > library. > > > > config DMABUF_HEAPS_SYSTEM > > bool "DMA-BUF System Heap" > > @@ -26,3 +30,12 @@ config DMABUF_HEAPS_DSP > >Choose this option to enable the dsp dmabuf heap. The > > dsp heap > >is allocated by gen allocater. it's allocated according > > the dts. > >If in doubt, say Y. > > + > > +config DMABUF_HEAPS_SECURE > > + tristate "DMA-BUF Secure Heap" > > + depends on DMABUF_HEAPS && DMABUF_HEAPS_DEFERRED_FREE > > + help > > + Choose this option to enable the secure dmabuf heap. The > > secure heap > > + pools are defined according to the DT. Heaps are allocated > > + in the pools using gen allocater. > > + If in doubt, say Y. > > diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma- > > buf/heaps/Makefile > > index e70722ea615e..08f6aa5919d1 100644 > > --- a/drivers/dma-buf/heaps/Makefile > > +++ b/drivers/dma-buf/heaps/Makefile > > @@ -4,3 +4,4 @@ obj-$(CONFIG_DMABUF_HEAPS_PAGE_POOL) += > > page_pool.o > > obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)+= system_heap.o > > obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o > > obj-$(CONFIG_DMABUF_HEAPS_DSP) += dsp_heap.o > > +obj-$(CONFIG_DMABUF_HEAPS_SECURE)+= secure_heap.o > > diff --git a/drivers/dma-buf/heaps/secure_heap.c b/drivers/dma- > > buf/heaps/secure_heap.c > > new file mode 100644 > > index ..31aac5d050b4 > > --- /dev/null > > +++ b/drivers/dma-buf/heaps/secure_heap.c > > @@ -0,0 +1,588 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * DMABUF secure heap exporter > > + * > > + * Copyright 2021 NXP. > > + */ > > + > > +#include > > +#include > > +#include
Re: [EXT] Re: [PATCH 1/3] dma-buf: heaps: add Linaro secure dmabuf heap support
Hi Brian,
On ven., 2022-08-12 at 17:39 +0100, Brian Starkey wrote:
> Caution: EXT Email
>
> Hi,
>
> On Mon, Aug 08, 2022 at 02:39:53PM +, Olivier Masse wrote:
> > Hi Brian,
> >
> > On ven., 2022-08-05 at 16:41 +0100, Brian Starkey wrote:
> > > Caution: EXT Email
> > >
> > > Hi Olivier,
> > >
> > > Thanks, I think this is looking much better.
> > >
> > > I'd like to know how others feel about landing this heap; there's
> > > been
> > > push-back in the past about heaps in device-tree and discussions
> > > around how "custom" heaps should be treated (though IMO this is
> > > quite
> > > a generic one).
> > >
> > > On Fri, Aug 05, 2022 at 03:53:28PM +0200, Olivier Masse wrote:
> > > > add Linaro secure heap bindings: linaro,secure-heap
> > > > use genalloc to allocate/free buffer from buffer pool.
> > > > buffer pool info is from dts.
> > > > use sg_table instore the allocated memory info, the length of
> > > > sg_table is 1.
> > > > implement secure_heap_buf_ops to implement buffer share in
> > > > difference device:
> > > > 1. Userspace passes this fd to all drivers it wants this buffer
> > > > to share with: First the filedescriptor is converted to a
> > > > _buf
> > > > using
> > > > dma_buf_get(). Then the buffer is attached to the device using
> > > > dma_buf_attach().
> > > > 2. Once the buffer is attached to all devices userspace can
> > > > initiate DMA
> > > > access to the shared buffer. In the kernel this is done by
> > > > calling
> > > > dma_buf_map_attachment()
> > > > 3. get sg_table with dma_buf_map_attachment in difference
> > > > device.
> > > >
> > >
> > > I think this commit message could use a little rework. A few
> > > thoughts:
> > >
> > > * The bindings are in a separate commit, so seems strange to
> > > mention
> > > here.
> >
> > what about:
> > "add Linaro secure heap compatible reserved memory: linaro,secure-
> > heap"
> >
>
> I'd say something like:
>
> Add a dma-buf heap to allocate secure buffers from a reserved-memory
> region.
>
> ..snip
ok right.
>
> > > > +
> > > > +static struct sg_table *secure_heap_map_dma_buf(struct
> > > > dma_buf_attachment *attachment,
> > > > + enum
> > > > dma_data_direction direction)
> > > > +{
> > > > + struct secure_heap_attachment *a = attachment->priv;
> > > > +
> > > > + return a->table;
> > >
> > > I think you still need to implement mapping and unmapping using
> > > the
> > > DMA APIs. For example devices might be behind IOMMUs and the
> > > buffer
> > > will need mapping into the IOMMU.
> >
> > Devices that will need access to the buffer must be in secure.
> > The tee driver will only need the scatter-list table to get dma
> > address
> > and len. Mapping will be done in the TEE.
> > Please find tee_shm_register_fd in the following commit
> >
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Flinaro-swg%2Flinux%2Fcommit%2F41e21e5c405530590dc2dd10b2a8dbe64589840fdata=05%7C01%7Colivier.masse%40nxp.com%7C6b3d47f1e15c41a8cf7108da7c813ef6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637959191795668899%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=OKZhaNevD5dj7Wjm6zbZlij0mPA9XYyio1NAN3VjTVM%3Dreserved=0
> >
> > This patch need to be up-streamed as well.
> >
>
> Interesting, that's not how the devices I've worked on operated.
>
> Are you saying that you have to have a display controller driver
> running in the TEE to display one of these buffers?
In fact the display controller is managing 3 plans : UI, PiP and
video. The video plan is protected in secure as you can see on slide
11:
https://static.linaro.org/connect/san19/presentations/san19-107.pdf
The DCSS (display controller) is able to read from the protected secure
heap and composition result is send directly to the HDMI/HDCP port.
> If everything
> needs to be in the TEE, then why even have these buffers allocated
> by non-secure Linux at all?
The TEE is only doing decryption using the HW Crypto Accelerator
(CAAM).
The CAAM will read from a non protected encrypted buffer to write clear
content to a secure buffer all
Re: [EXT] Re: [PATCH v2 0/1] tee: Add tee_shm_register_fd
Hi Jens, On mar., 2022-08-16 at 10:17 +0200, Jens Wiklander wrote: > Caution: EXT Email > > Hi Olivier, > > On Fri, Aug 12, 2022 at 4:31 PM Olivier Masse > wrote: > > > > Add a new ioctl called TEE_IOC_SHM_REGISTER_FD to register a > > shared memory from a dmabuf file descriptor. > > This new ioctl will allow the Linux Kernel to register a buffer > > to be used by the Secure Data Path OPTEE OS feature. > > > > Please find more information here: > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstatic.linaro.org%2Fconnect%2Fsan19%2Fpresentations%2Fsan19-107.pdfdata=05%7C01%7Colivier.masse%40nxp.com%7C20ddb873be8f4cd89b5408da7f5fda26%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C1%7C637962346897373445%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=cMbuihC0Hat4DEVORzcGhzwjO%2FxclAW43AIcvR8yReE%3Dreserved=0 > > > > Patch tested on Hikey 6220. > > What's new in this V2? Just updated the cover letter and minor change to fix a build error with gcc-11 for x86 architecture: >> ./usr/include/linux/tee.h:136:13: error: expected declaration specifiers or '...' before numeric constant 136 | } __aligned(8); | ^ Best regards, Olivier > > Thanks, > Jens > > > > > Etienne Carriere (1): > > tee: new ioctl to a register tee_shm from a dmabuf file > > descriptor > > > > drivers/tee/tee_core.c | 38 +++ > > drivers/tee/tee_shm.c| 99 > > +++- > > include/linux/tee_drv.h | 11 + > > include/uapi/linux/tee.h | 29 > > 4 files changed, 175 insertions(+), 2 deletions(-) > > > > -- > > 2.25.0 > >
[PATCH 1/1] tee: new ioctl to a register tee_shm from a dmabuf file descriptor
From: Etienne Carriere
This change allows userland to create a tee_shm object that refers
to a dmabuf reference.
Userland provides a dmabuf file descriptor as buffer reference.
The created tee_shm object exported as a brand new dmabuf reference
used to provide a clean fd to userland. Userland shall closed this new
fd to release the tee_shm object resources. The initial dmabuf resources
are tracked independently through original dmabuf file descriptor.
Once the buffer is registered and until it is released, TEE driver
keeps a refcount on the registered dmabuf structure.
This change only support dmabuf references that relates to physically
contiguous memory buffers.
New tee_shm flag to identify tee_shm objects built from a registered
dmabuf: TEE_SHM_EXT_DMA_BUF. Such tee_shm structures are flagged with
TEE_SHM_EXT_DMA_BUF.
Co-Developed-by: Etienne Carriere
Signed-off-by: Olivier Masse
Reported-by: kernel test robot
From: https://github.com/linaro-swg/linux.git
(cherry picked from commit 41e21e5c405530590dc2dd10b2a8dbe64589840f)
---
drivers/tee/tee_core.c | 38 +++
drivers/tee/tee_shm.c| 99 +++-
include/linux/tee_drv.h | 11 +
include/uapi/linux/tee.h | 29
4 files changed, 175 insertions(+), 2 deletions(-)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 8aa1a4836b92..7c45cbf85eb9 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -355,6 +355,42 @@ tee_ioctl_shm_register(struct tee_context *ctx,
return ret;
}
+static int tee_ioctl_shm_register_fd(struct tee_context *ctx,
+struct tee_ioctl_shm_register_fd_data
__user *udata)
+{
+ struct tee_ioctl_shm_register_fd_data data;
+ struct tee_shm *shm;
+ long ret;
+
+ if (copy_from_user(, udata, sizeof(data)))
+ return -EFAULT;
+
+ /* Currently no input flags are supported */
+ if (data.flags)
+ return -EINVAL;
+
+ shm = tee_shm_register_fd(ctx, data.fd);
+ if (IS_ERR(shm))
+ return -EINVAL;
+
+ data.id = shm->id;
+ data.flags = shm->flags;
+ data.size = shm->size;
+
+ if (copy_to_user(udata, , sizeof(data)))
+ ret = -EFAULT;
+ else
+ ret = tee_shm_get_fd(shm);
+
+ /*
+* When user space closes the file descriptor the shared memory
+* should be freed or if tee_shm_get_fd() failed then it will
+* be freed immediately.
+*/
+ tee_shm_put(shm);
+ return ret;
+}
+
static int params_from_user(struct tee_context *ctx, struct tee_param *params,
size_t num_params,
struct tee_ioctl_param __user *uparams)
@@ -829,6 +865,8 @@ static long tee_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
return tee_ioctl_shm_alloc(ctx, uarg);
case TEE_IOC_SHM_REGISTER:
return tee_ioctl_shm_register(ctx, uarg);
+ case TEE_IOC_SHM_REGISTER_FD:
+ return tee_ioctl_shm_register_fd(ctx, uarg);
case TEE_IOC_OPEN_SESSION:
return tee_ioctl_open_session(ctx, uarg);
case TEE_IOC_INVOKE:
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 836872467dc6..55a3fbbb022e 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -4,6 +4,7 @@
*/
#include
#include
+#include
#include
#include
#include
@@ -12,6 +13,14 @@
#include
#include "tee_private.h"
+/* extra references appended to shm object for registered shared memory */
+struct tee_shm_dmabuf_ref {
+ struct tee_shm shm;
+ struct dma_buf *dmabuf;
+ struct dma_buf_attachment *attach;
+ struct sg_table *sgt;
+};
+
static void shm_put_kernel_pages(struct page **pages, size_t page_count)
{
size_t n;
@@ -71,7 +80,16 @@ static void release_registered_pages(struct tee_shm *shm)
static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm)
{
- if (shm->flags & TEE_SHM_POOL) {
+ if (shm->flags & TEE_SHM_EXT_DMA_BUF) {
+ struct tee_shm_dmabuf_ref *ref;
+
+ ref = container_of(shm, struct tee_shm_dmabuf_ref, shm);
+ dma_buf_unmap_attachment(ref->attach, ref->sgt,
+DMA_BIDIRECTIONAL);
+
+ dma_buf_detach(ref->dmabuf, ref->attach);
+ dma_buf_put(ref->dmabuf);
+ } else if (shm->flags & TEE_SHM_POOL) {
teedev->pool->ops->free(teedev->pool, shm);
} else if (shm->flags & TEE_SHM_DYNAMIC) {
int rc = teedev->desc->ops->shm_unregister(shm->ctx, shm);
@@ -195,7 +213,7 @@ struct tee_shm *tee_shm_alloc_user_buf(struct tee_context
*ctx, size_t size)
* tee_client_invoke_func(). The memory allocated is later freed with a
* call to
[PATCH v2 0/1] tee: Add tee_shm_register_fd
Add a new ioctl called TEE_IOC_SHM_REGISTER_FD to register a shared memory from a dmabuf file descriptor. This new ioctl will allow the Linux Kernel to register a buffer to be used by the Secure Data Path OPTEE OS feature. Please find more information here: https://static.linaro.org/connect/san19/presentations/san19-107.pdf Patch tested on Hikey 6220. Etienne Carriere (1): tee: new ioctl to a register tee_shm from a dmabuf file descriptor drivers/tee/tee_core.c | 38 +++ drivers/tee/tee_shm.c| 99 +++- include/linux/tee_drv.h | 11 + include/uapi/linux/tee.h | 29 4 files changed, 175 insertions(+), 2 deletions(-) -- 2.25.0
Re: [EXT] Re: [PATCH 1/1] tee: new ioctl to a register tee_shm from a dmabuf file descriptor
Hi Sumit,
Thanks for your comments, please find my answer below.
On ven., 2022-08-12 at 19:36 +0530, Sumit Garg wrote:
> Caution: EXT Email
>
> Hi Olivier,
>
> On Thu, 11 Aug 2022 at 19:26, Olivier Masse
> wrote:
> >
> > From: Etienne Carriere
> >
> > This change allows userland to create a tee_shm object that refers
> > to a dmabuf reference.
> >
> > Userland provides a dmabuf file descriptor as buffer reference.
> > The created tee_shm object exported as a brand new dmabuf reference
> > used to provide a clean fd to userland. Userland shall closed this
> > new
> > fd to release the tee_shm object resources. The initial dmabuf
> > resources
> > are tracked independently through original dmabuf file descriptor.
> >
> > Once the buffer is registered and until it is released, TEE driver
> > keeps a refcount on the registered dmabuf structure.
> >
> > This change only support dmabuf references that relates to
> > physically
> > contiguous memory buffers.
>
> What limits you to extend this feature to non-contiguous memory
> buffers? I believe that should be possible with OP-TEE dynamic shared
> memory which gives you the granularity to register a list of pages.
But it will need more logic in OPTEE OS to verify that all pages are in
the Secure Data Path protected area.
Our solution use a fixed protected reserved memory region and do not
rely on a dynamic protection set in secure.
Best regards,
Olivier
>
> -Sumit
>
> >
> > New tee_shm flag to identify tee_shm objects built from a
> > registered
> > dmabuf: TEE_SHM_EXT_DMA_BUF. Such tee_shm structures are flagged
> > with
> > TEE_SHM_EXT_DMA_BUF.
> >
> > Co-Developed-by: Etienne Carriere
> > Signed-off-by: Olivier Masse
> > From:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Flinaro-swg%2Flinux.gitdata=05%7C01%7Colivier.masse%40nxp.com%7C204e0821d1c84355ed4208da7c6be5c6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637959100100176447%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=MRfnQNbwAIj%2Bnd9tSjSb5Nzrv3sQVpVMIdOxxRfu6U4%3Dreserved=0
> > (cherry picked from commit
> > 41e21e5c405530590dc2dd10b2a8dbe64589840f)
> > ---
> > drivers/tee/tee_core.c | 38 +++
> > drivers/tee/tee_shm.c| 99
> > +++-
> > include/linux/tee_drv.h | 11 +
> > include/uapi/linux/tee.h | 29
> > 4 files changed, 175 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> > index 8aa1a4836b92..7c45cbf85eb9 100644
> > --- a/drivers/tee/tee_core.c
> > +++ b/drivers/tee/tee_core.c
> > @@ -355,6 +355,42 @@ tee_ioctl_shm_register(struct tee_context
> > *ctx,
> > return ret;
> > }
> >
> > +static int tee_ioctl_shm_register_fd(struct tee_context *ctx,
> > +struct
> > tee_ioctl_shm_register_fd_data __user *udata)
> > +{
> > + struct tee_ioctl_shm_register_fd_data data;
> > + struct tee_shm *shm;
> > + long ret;
> > +
> > + if (copy_from_user(, udata, sizeof(data)))
> > + return -EFAULT;
> > +
> > + /* Currently no input flags are supported */
> > + if (data.flags)
> > + return -EINVAL;
> > +
> > + shm = tee_shm_register_fd(ctx, data.fd);
> > + if (IS_ERR(shm))
> > + return -EINVAL;
> > +
> > + data.id = shm->id;
> > + data.flags = shm->flags;
> > + data.size = shm->size;
> > +
> > + if (copy_to_user(udata, , sizeof(data)))
> > + ret = -EFAULT;
> > + else
> > + ret = tee_shm_get_fd(shm);
> > +
> > + /*
> > +* When user space closes the file descriptor the shared
> > memory
> > +* should be freed or if tee_shm_get_fd() failed then it
> > will
> > +* be freed immediately.
> > +*/
> > + tee_shm_put(shm);
> > + return ret;
> > +}
> > +
> > static int params_from_user(struct tee_context *ctx, struct
> > tee_param *params,
> > size_t num_params,
> > struct tee_ioctl_param __user *uparams)
> > @@ -829,6 +865,8 @@ static long tee_ioctl(struct file *filp,
> > unsigned int cmd, unsigned long arg)
> > return tee_ioctl_shm_alloc(ctx, uarg);
> &
Re: [EXT] Re: [PATCH 1/1] tee: new ioctl to a register tee_shm from a dmabuf file descriptor
Hi Christian,
On ven., 2022-08-12 at 12:18 +0200, Christian König wrote:
> Caution: EXT Email
>
> Hi Etienne,
>
> at least I don't see anything obvious broken.
>
> Just two comments:
>
> 1. Dmitry is working on a change which renames some functions and
> makes
> it mandatory to call them with the dma_resv lock held.
>
> Depending on how you want to upstream this change you will certainly
> run
> into conflicts with that.
ok could you send me some link to his changes ?
>
> 2. Would it be possible to do this dynamically? In other words does
> the
> tee driver has a concept of buffers moving around?
I'm not sure to understand what you mean by a moving buffer ?
For information the TEE need to map the buffer in secure which is done
when calling the invoke operation function.
>
> Am 11.08.22 um 15:56 schrieb Olivier Masse:
> > From: Etienne Carriere
> >
> > This change allows userland to create a tee_shm object that refers
> > to a dmabuf reference.
> >
> > Userland provides a dmabuf file descriptor as buffer reference.
> > The created tee_shm object exported as a brand new dmabuf reference
> > used to provide a clean fd to userland. Userland shall closed this
> > new
> > fd to release the tee_shm object resources. The initial dmabuf
> > resources
> > are tracked independently through original dmabuf file descriptor.
> >
> > Once the buffer is registered and until it is released, TEE driver
> > keeps a refcount on the registered dmabuf structure.
> >
> > This change only support dmabuf references that relates to
> > physically
> > contiguous memory buffers.
>
> That sounds like a pretty hard restriction, but I obviously don't see
> how to avoid it either.
>
> Regards,
> Christian.
>
> >
> > New tee_shm flag to identify tee_shm objects built from a
> > registered
> > dmabuf: TEE_SHM_EXT_DMA_BUF. Such tee_shm structures are flagged
> > with
> > TEE_SHM_EXT_DMA_BUF.
> >
> > Co-Developed-by: Etienne Carriere
> > Signed-off-by: Olivier Masse
> > From:
> > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Flinaro-swg%2Flinux.gitdata=05%7C01%7Colivier.masse%40nxp.com%7C03c2d3cc95b8429693c108da7c4bf7a7%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637958962989857231%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=vz8Y7ZwKXteePpWQVO8EhmZ1oyIX0xOCRd%2BGH7xHjII%3Dreserved=0
> > (cherry picked from commit
> > 41e21e5c405530590dc2dd10b2a8dbe64589840f)
> > ---
> > drivers/tee/tee_core.c | 38 +++
> > drivers/tee/tee_shm.c| 99
> > +++-
> > include/linux/tee_drv.h | 11 +
> > include/uapi/linux/tee.h | 29
> > 4 files changed, 175 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
> > index 8aa1a4836b92..7c45cbf85eb9 100644
> > --- a/drivers/tee/tee_core.c
> > +++ b/drivers/tee/tee_core.c
> > @@ -355,6 +355,42 @@ tee_ioctl_shm_register(struct tee_context
> > *ctx,
> > return ret;
> > }
> >
> > +static int tee_ioctl_shm_register_fd(struct tee_context *ctx,
> > + struct
> > tee_ioctl_shm_register_fd_data __user *udata)
> > +{
> > + struct tee_ioctl_shm_register_fd_data data;
> > + struct tee_shm *shm;
> > + long ret;
> > +
> > + if (copy_from_user(, udata, sizeof(data)))
> > + return -EFAULT;
> > +
> > + /* Currently no input flags are supported */
> > + if (data.flags)
> > + return -EINVAL;
> > +
> > + shm = tee_shm_register_fd(ctx, data.fd);
> > + if (IS_ERR(shm))
> > + return -EINVAL;
> > +
> > + data.id = shm->id;
> > + data.flags = shm->flags;
> > + data.size = shm->size;
> > +
> > + if (copy_to_user(udata, , sizeof(data)))
> > + ret = -EFAULT;
> > + else
> > + ret = tee_shm_get_fd(shm);
> > +
> > + /*
> > + * When user space closes the file descriptor the shared
> > memory
> > + * should be freed or if tee_shm_get_fd() failed then it will
> > + * be freed immediately.
> > + */
> > + tee_shm_put(shm);
> > + return ret;
> > +}
> > +
> > static int params_from_user(struct tee_context *ctx, struct
> > tee_param *params,
> > size_t num_params,
> >
[PATCH 1/1] tee: new ioctl to a register tee_shm from a dmabuf file descriptor
From: Etienne Carriere
This change allows userland to create a tee_shm object that refers
to a dmabuf reference.
Userland provides a dmabuf file descriptor as buffer reference.
The created tee_shm object exported as a brand new dmabuf reference
used to provide a clean fd to userland. Userland shall closed this new
fd to release the tee_shm object resources. The initial dmabuf resources
are tracked independently through original dmabuf file descriptor.
Once the buffer is registered and until it is released, TEE driver
keeps a refcount on the registered dmabuf structure.
This change only support dmabuf references that relates to physically
contiguous memory buffers.
New tee_shm flag to identify tee_shm objects built from a registered
dmabuf: TEE_SHM_EXT_DMA_BUF. Such tee_shm structures are flagged with
TEE_SHM_EXT_DMA_BUF.
Co-Developed-by: Etienne Carriere
Signed-off-by: Olivier Masse
From: https://github.com/linaro-swg/linux.git
(cherry picked from commit 41e21e5c405530590dc2dd10b2a8dbe64589840f)
---
drivers/tee/tee_core.c | 38 +++
drivers/tee/tee_shm.c| 99 +++-
include/linux/tee_drv.h | 11 +
include/uapi/linux/tee.h | 29
4 files changed, 175 insertions(+), 2 deletions(-)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 8aa1a4836b92..7c45cbf85eb9 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -355,6 +355,42 @@ tee_ioctl_shm_register(struct tee_context *ctx,
return ret;
}
+static int tee_ioctl_shm_register_fd(struct tee_context *ctx,
+struct tee_ioctl_shm_register_fd_data
__user *udata)
+{
+ struct tee_ioctl_shm_register_fd_data data;
+ struct tee_shm *shm;
+ long ret;
+
+ if (copy_from_user(, udata, sizeof(data)))
+ return -EFAULT;
+
+ /* Currently no input flags are supported */
+ if (data.flags)
+ return -EINVAL;
+
+ shm = tee_shm_register_fd(ctx, data.fd);
+ if (IS_ERR(shm))
+ return -EINVAL;
+
+ data.id = shm->id;
+ data.flags = shm->flags;
+ data.size = shm->size;
+
+ if (copy_to_user(udata, , sizeof(data)))
+ ret = -EFAULT;
+ else
+ ret = tee_shm_get_fd(shm);
+
+ /*
+* When user space closes the file descriptor the shared memory
+* should be freed or if tee_shm_get_fd() failed then it will
+* be freed immediately.
+*/
+ tee_shm_put(shm);
+ return ret;
+}
+
static int params_from_user(struct tee_context *ctx, struct tee_param *params,
size_t num_params,
struct tee_ioctl_param __user *uparams)
@@ -829,6 +865,8 @@ static long tee_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg)
return tee_ioctl_shm_alloc(ctx, uarg);
case TEE_IOC_SHM_REGISTER:
return tee_ioctl_shm_register(ctx, uarg);
+ case TEE_IOC_SHM_REGISTER_FD:
+ return tee_ioctl_shm_register_fd(ctx, uarg);
case TEE_IOC_OPEN_SESSION:
return tee_ioctl_open_session(ctx, uarg);
case TEE_IOC_INVOKE:
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 836872467dc6..55a3fbbb022e 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -4,6 +4,7 @@
*/
#include
#include
+#include
#include
#include
#include
@@ -12,6 +13,14 @@
#include
#include "tee_private.h"
+/* extra references appended to shm object for registered shared memory */
+struct tee_shm_dmabuf_ref {
+ struct tee_shm shm;
+ struct dma_buf *dmabuf;
+ struct dma_buf_attachment *attach;
+ struct sg_table *sgt;
+};
+
static void shm_put_kernel_pages(struct page **pages, size_t page_count)
{
size_t n;
@@ -71,7 +80,16 @@ static void release_registered_pages(struct tee_shm *shm)
static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm)
{
- if (shm->flags & TEE_SHM_POOL) {
+ if (shm->flags & TEE_SHM_EXT_DMA_BUF) {
+ struct tee_shm_dmabuf_ref *ref;
+
+ ref = container_of(shm, struct tee_shm_dmabuf_ref, shm);
+ dma_buf_unmap_attachment(ref->attach, ref->sgt,
+DMA_BIDIRECTIONAL);
+
+ dma_buf_detach(ref->dmabuf, ref->attach);
+ dma_buf_put(ref->dmabuf);
+ } else if (shm->flags & TEE_SHM_POOL) {
teedev->pool->ops->free(teedev->pool, shm);
} else if (shm->flags & TEE_SHM_DYNAMIC) {
int rc = teedev->desc->ops->shm_unregister(shm->ctx, shm);
@@ -195,7 +213,7 @@ struct tee_shm *tee_shm_alloc_user_buf(struct tee_context
*ctx, size_t size)
* tee_client_invoke_func(). The memory allocated is later freed with a
* call to tee_shm_free().
*
- * @retur
[PATCH 0/1] tee: Add tee_shm_register_fd
Add a new ioctl called TEE_IOC_SHM_REGISTER_FD to register a shared memory from a dmabuf file descriptor. Etienne Carriere (1): tee: new ioctl to a register tee_shm from a dmabuf file descriptor drivers/tee/tee_core.c | 38 +++ drivers/tee/tee_shm.c| 99 +++- include/linux/tee_drv.h | 11 + include/uapi/linux/tee.h | 29 4 files changed, 175 insertions(+), 2 deletions(-) -- 2.25.0
Re: [EXT] Re: [PATCH 1/3] dma-buf: heaps: add Linaro secure dmabuf heap support
Hi Christian,
Thanks for your comments, please find my answer below.
On mer., 2022-08-10 at 11:43 +0200, Christian König wrote:
> Caution: EXT Email
>
> Hi guys,
>
> Am 05.08.22 um 15:53 schrieb Olivier Masse:
> > add Linaro secure heap bindings: linaro,secure-heap
> > use genalloc to allocate/free buffer from buffer pool.
> > buffer pool info is from dts.
> > use sg_table instore the allocated memory info, the length of
> > sg_table is 1.
> > implement secure_heap_buf_ops to implement buffer share in
> > difference device:
> > 1. Userspace passes this fd to all drivers it wants this buffer
> > to share with: First the filedescriptor is converted to a _buf
> > using
> > dma_buf_get(). Then the buffer is attached to the device using
> > dma_buf_attach().
> > 2. Once the buffer is attached to all devices userspace can
> > initiate DMA
> > access to the shared buffer. In the kernel this is done by calling
> > dma_buf_map_attachment()
> > 3. get sg_table with dma_buf_map_attachment in difference device.
>
> I'm not sure Christoph will approve that you are messing with the sg
> table internals so much here.
>
> Why are you not using the DMA API directly for this?
Do you mean for secure_heap_map_dma_buf and secure_heap_unmap_dma_buf ?
If yes, maybe something like the following could be more appropriate:
static struct sg_table *secure_heap_map_dma_buf(struct dma_buf_attachment
*attachment,
enum dma_data_direction
direction)
{
struct secure_heap_attachment *a = attachment->priv;
struct sg_table *sgt;
sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
if (!sgt) {
dev_err(a->dev, "failed to alloc sg table\n");
return NULL;
}
ret = dma_get_sgtable(a->dev, sgt, NULL, sg_dma_address(a->table->sgl),
sg_dma_len(a->table->sgl));
if (ret < 0) {
dev_err(a->dev, "failed to get scatterlist from DMA API\n");
kfree(sgt);
return NULL;
}
return sgt;
}
Regards,
Olivier
>
> Regards,
> Christian.
>
> >
> > Signed-off-by: Olivier Masse
> > ---
> > drivers/dma-buf/heaps/Kconfig | 9 +
> > drivers/dma-buf/heaps/Makefile | 1 +
> > drivers/dma-buf/heaps/secure_heap.c | 357
> >
> > 3 files changed, 367 insertions(+)
> > create mode 100644 drivers/dma-buf/heaps/secure_heap.c
> >
> > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-
> > buf/heaps/Kconfig
> > index 3782eeeb91c0..c9070c728b9a 100644
> > --- a/drivers/dma-buf/heaps/Kconfig
> > +++ b/drivers/dma-buf/heaps/Kconfig
> > @@ -20,3 +20,12 @@ config DMABUF_HEAPS_DSP
> > Choose this option to enable the dsp dmabuf heap. The
> > dsp heap
> > is allocated by gen allocater. it's allocated according
> > the dts.
> > If in doubt, say Y.
> > +
> > +config DMABUF_HEAPS_SECURE
> > + tristate "DMA-BUF Secure Heap"
> > + depends on DMABUF_HEAPS
> > + help
> > + Choose this option to enable the secure dmabuf heap. The
> > secure heap
> > + pools are defined according to the DT. Heaps are allocated
> > + in the pools using gen allocater.
> > + If in doubt, say Y.
> > diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-
> > buf/heaps/Makefile
> > index 29733f84c354..863ef10056a3 100644
> > --- a/drivers/dma-buf/heaps/Makefile
> > +++ b/drivers/dma-buf/heaps/Makefile
> > @@ -2,3 +2,4 @@
> > obj-$(CONFIG_DMABUF_HEAPS_SYSTEM) += system_heap.o
> > obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o
> > obj-$(CONFIG_DMABUF_HEAPS_DSP) += dsp_heap.o
> > +obj-$(CONFIG_DMABUF_HEAPS_SECURE)+= secure_heap.o
> > diff --git a/drivers/dma-buf/heaps/secure_heap.c b/drivers/dma-
> > buf/heaps/secure_heap.c
> > new file mode 100644
> > index ..25b3629613f3
> > --- /dev/null
> > +++ b/drivers/dma-buf/heaps/secure_heap.c
> > @@ -0,0 +1,357 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * DMABUF secure heap exporter
> > + *
> > + * Copyright 2021 NXP.
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#defin
Re: [EXT] Re: [PATCH 1/3] dma-buf: heaps: add Linaro secure dmabuf heap support
Hi Brian,
> > +
> > + rmem->ops = _dma_ops;
> > + pr_info("Reserved memory: DMA buf secure pool %s at
> > %pa, size %ld MiB\n",
> > + secure_data[secure_data_count].name,
> > + >base, (unsigned long)rmem->size /
> > SZ_1M);
>
> nit: What if rmem->size < SZ_1M, or not 1M-aligned
>
Let's assume that size is 1K-aligned, maybe something like that could
be better ?
unsigned long mb = rmem->size >> 20;
unsigned long kb = (rmem->size & (SZ_1M - 1)) >> 10;
pr_info("Reserved memory: DMA buf secure pool %s at %pa, size
%ld MiB and %ld KiB",
secure_data[secure_data_count].name,
>base, mb, kb);
Cheers,
Olivier
Re: [EXT] Re: [PATCH 2/3] dt-bindings: reserved-memory: add linaro,secure-heap
Hi Brian,
It was part of a discussion during a Devicetree evolution meeting with
Bill Mills from Linaro.
I've done some modification to OPTEE OS and OPTEE TEST to support dma
buf:
OPTEE OS
https://github.com/OP-TEE/optee_os/commit/eb108a04369fbfaf60c03c0e00bbe9489a761c69
https://github.com/OP-TEE/optee_os/commit/513b0748d46e7eefa17dadb204289e49dc17f854
OPTEE TEST
https://github.com/OP-TEE/optee_test/commit/da5282a011b40621a2cf7a296c11a35c833ed91b
BR / Olivier
On ven., 2022-08-05 at 16:46 +0100, Brian Starkey wrote:
> Caution: EXT Email
>
> +Rob and devicetree list.
>
> I don't know if this should be "linaro" or something more generic,
> and also where previous discussions got to about DMA heaps in DT.
>
> Cheers,
> -Brian
>
> On Fri, Aug 05, 2022 at 03:53:29PM +0200, Olivier Masse wrote:
> > DMABUF Reserved memory definition for OP-TEE SDP feaure.
> >
> > Signed-off-by: Olivier Masse
> > ---
> > .../reserved-memory/linaro,secure-heap.yaml | 56
> > +++
> > 1 file changed, 56 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/reserved-
> > memory/linaro,secure-heap.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/reserved-
> > memory/linaro,secure-heap.yaml
> > b/Documentation/devicetree/bindings/reserved-memory/linaro,secure-
> > heap.yaml
> > new file mode 100644
> > index ..80522a4e2989
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/reserved-
> > memory/linaro,secure-heap.yaml
> > @@ -0,0 +1,56 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id:
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Freserved-memory%2Flinaro%2Csecure-heap.yaml%23data=05%7C01%7Colivier.masse%40nxp.com%7C0a9e67bbd65446aa05e408da76f9b82a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637953112157450452%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=lb9U8Fnt1Y43UgObcgakAC%2FZx4je%2BCoNX5vhkFvgbdQ%3Dreserved=0
> > +$schema:
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23data=05%7C01%7Colivier.masse%40nxp.com%7C0a9e67bbd65446aa05e408da76f9b82a%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637953112157450452%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=UVuVI%2FmUFj7jX7c6DY0rRi9lkZW7kqTJyQurQxvNvz8%3Dreserved=0
> > +
> > +title: Linaro Secure DMABUF Heap
> > +
> > +maintainers:
> > + - Olivier Masse
> > +
> > +description:
> > + Linaro OP-TEE firmware needs a reserved memory for the
> > + Secure Data Path feature (aka SDP).
> > + The purpose is to provide a secure memory heap which allow
> > + non-secure OS to allocate/free secure buffers.
> > + The TEE is reponsible for protecting the SDP memory buffers.
> > + TEE Trusted Application can access secure memory references
> > + provided as parameters (DMABUF file descriptor).
> > +
> > +allOf:
> > + - $ref: "reserved-memory.yaml"
> > +
> > +properties:
> > + compatible:
> > +const: linaro,secure-heap
> > +
> > + reg:
> > +description:
> > + Region of memory reserved for OP-TEE SDP feature
> > +
> > + no-map:
> > +$ref: /schemas/types.yaml#/definitions/flag
> > +description:
> > + Avoid creating a virtual mapping of the region as part of
> > the OS'
> > + standard mapping of system memory.
> > +
> > +unevaluatedProperties: false
> > +
> > +required:
> > + - compatible
> > + - reg
> > + - no-map
> > +
> > +examples:
> > + - |
> > + reserved-memory {
> > +#address-cells = <2>;
> > +#size-cells = <2>;
> > +
> > +sdp@3e80 {
> > + compatible = "linaro,secure-heap";
> > + no-map;
> > + reg = <0 0x3E80 0 0x0040>;
> > +};
> > + };
> > --
> > 2.25.0
> >
[PATCH 1/3] dma-buf: heaps: add Linaro secure dmabuf heap support
add Linaro secure heap compatible reserved mem: linaro,secure-heap
use genalloc to allocate/free buffer from buffer pool.
buffer pool info is defined from a dts reserved memory.
Signed-off-by: Olivier Masse
---
drivers/dma-buf/heaps/Kconfig | 9 +
drivers/dma-buf/heaps/Makefile | 1 +
drivers/dma-buf/heaps/secure_heap.c | 339
3 files changed, 349 insertions(+)
create mode 100644 drivers/dma-buf/heaps/secure_heap.c
diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
index 3782eeeb91c0..c9070c728b9a 100644
--- a/drivers/dma-buf/heaps/Kconfig
+++ b/drivers/dma-buf/heaps/Kconfig
@@ -20,3 +20,12 @@ config DMABUF_HEAPS_DSP
Choose this option to enable the dsp dmabuf heap. The dsp heap
is allocated by gen allocater. it's allocated according the dts.
If in doubt, say Y.
+
+config DMABUF_HEAPS_SECURE
+ tristate "DMA-BUF Secure Heap"
+ depends on DMABUF_HEAPS
+ help
+ Choose this option to enable the secure dmabuf heap. The secure heap
+ pools are defined according to the DT. Heaps are allocated
+ in the pools using gen allocater.
+ If in doubt, say Y.
diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
index 29733f84c354..863ef10056a3 100644
--- a/drivers/dma-buf/heaps/Makefile
+++ b/drivers/dma-buf/heaps/Makefile
@@ -2,3 +2,4 @@
obj-$(CONFIG_DMABUF_HEAPS_SYSTEM) += system_heap.o
obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o
obj-$(CONFIG_DMABUF_HEAPS_DSP) += dsp_heap.o
+obj-$(CONFIG_DMABUF_HEAPS_SECURE) += secure_heap.o
diff --git a/drivers/dma-buf/heaps/secure_heap.c
b/drivers/dma-buf/heaps/secure_heap.c
new file mode 100644
index ..a3023bf8d457
--- /dev/null
+++ b/drivers/dma-buf/heaps/secure_heap.c
@@ -0,0 +1,339 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DMABUF secure heap exporter
+ *
+ * Copyright 2022 NXP.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_SECURE_HEAP 2
+#define MAX_HEAP_NAME_LEN 32
+
+struct secure_heap_buffer {
+ struct dma_heap *heap;
+ struct list_head attachments;
+ struct mutex lock;
+ unsigned long len;
+ struct sg_table sg_table;
+ int vmap_cnt;
+ void *vaddr;
+};
+
+struct secure_heap_attachment {
+ struct device *dev;
+ struct sg_table *table;
+ struct list_head list;
+};
+
+struct secure_heap_info {
+ struct gen_pool *pool;
+};
+
+struct rmem_secure {
+ phys_addr_t base;
+ phys_addr_t size;
+
+ char name[MAX_HEAP_NAME_LEN];
+};
+
+static struct rmem_secure secure_data[MAX_SECURE_HEAP] = {0};
+static unsigned int secure_data_count;
+
+static struct sg_table *dup_sg_table(struct sg_table *table)
+{
+ struct sg_table *new_table;
+ int ret, i;
+ struct scatterlist *sg, *new_sg;
+
+ new_table = kzalloc(sizeof(*new_table), GFP_KERNEL);
+ if (!new_table)
+ return ERR_PTR(-ENOMEM);
+
+ ret = sg_alloc_table(new_table, table->orig_nents, GFP_KERNEL);
+ if (ret) {
+ kfree(new_table);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ new_sg = new_table->sgl;
+ for_each_sgtable_sg(table, sg, i) {
+ sg_set_page(new_sg, sg_page(sg), sg->length, sg->offset);
+ new_sg->dma_address = sg->dma_address;
+#ifdef CONFIG_NEED_SG_DMA_LENGTH
+ new_sg->dma_length = sg->dma_length;
+#endif
+ new_sg = sg_next(new_sg);
+ }
+
+ return new_table;
+}
+
+static int secure_heap_attach(struct dma_buf *dmabuf,
+ struct dma_buf_attachment *attachment)
+{
+ struct secure_heap_buffer *buffer = dmabuf->priv;
+ struct secure_heap_attachment *a;
+ struct sg_table *table;
+
+ a = kzalloc(sizeof(*a), GFP_KERNEL);
+ if (!a)
+ return -ENOMEM;
+
+ table = dup_sg_table(>sg_table);
+ if (IS_ERR(table)) {
+ kfree(a);
+ return PTR_ERR(table);
+ }
+
+ a->table = table;
+ a->dev = attachment->dev;
+ INIT_LIST_HEAD(>list);
+ attachment->priv = a;
+
+ mutex_lock(>lock);
+ list_add(>list, >attachments);
+ mutex_unlock(>lock);
+
+ return 0;
+}
+
+static void secure_heap_detach(struct dma_buf *dmabuf,
+ struct dma_buf_attachment *attachment)
+{
+ struct secure_heap_buffer *buffer = dmabuf->priv;
+ struct secure_heap_attachment *a = attachment->priv;
+
+ mutex_lock(>lock);
+ list_del(>list);
+ mutex_unlock(>lock);
+
+ sg_free_table(a->table);
+ kfree(a->table);
+ kfree(a);
+}
+
+static struct sg_table *secure_heap_map_dma_buf(stru
Re: [EXT] Re: [PATCH 1/3] dma-buf: heaps: add Linaro secure dmabuf heap support
Hi Brian,
On ven., 2022-08-05 at 16:41 +0100, Brian Starkey wrote:
> Caution: EXT Email
>
> Hi Olivier,
>
> Thanks, I think this is looking much better.
>
> I'd like to know how others feel about landing this heap; there's
> been
> push-back in the past about heaps in device-tree and discussions
> around how "custom" heaps should be treated (though IMO this is quite
> a generic one).
>
> On Fri, Aug 05, 2022 at 03:53:28PM +0200, Olivier Masse wrote:
> > add Linaro secure heap bindings: linaro,secure-heap
> > use genalloc to allocate/free buffer from buffer pool.
> > buffer pool info is from dts.
> > use sg_table instore the allocated memory info, the length of
> > sg_table is 1.
> > implement secure_heap_buf_ops to implement buffer share in
> > difference device:
> > 1. Userspace passes this fd to all drivers it wants this buffer
> > to share with: First the filedescriptor is converted to a _buf
> > using
> > dma_buf_get(). Then the buffer is attached to the device using
> > dma_buf_attach().
> > 2. Once the buffer is attached to all devices userspace can
> > initiate DMA
> > access to the shared buffer. In the kernel this is done by calling
> > dma_buf_map_attachment()
> > 3. get sg_table with dma_buf_map_attachment in difference device.
> >
>
> I think this commit message could use a little rework. A few
> thoughts:
>
> * The bindings are in a separate commit, so seems strange to mention
> here.
what about:
"add Linaro secure heap compatible reserved memory: linaro,secure-heap"
> * "buffer pool info is from dts" --> I think you should mention that
> this uses a reserved-memory region.
ok
> * sg_table nents and genalloc seem like low-level implementation
> details, so probably not needed in the commit message
> * The usage steps 1, 2, 3 aren't specific to this heap - that's how
> all dma-buf sharing works.
ok, let's cleanup and removed this.
>
> > Signed-off-by: Olivier Masse
> > ---
> > drivers/dma-buf/heaps/Kconfig | 9 +
> > drivers/dma-buf/heaps/Makefile | 1 +
> > drivers/dma-buf/heaps/secure_heap.c | 357
> >
> > 3 files changed, 367 insertions(+)
> > create mode 100644 drivers/dma-buf/heaps/secure_heap.c
> >
> > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-
> > buf/heaps/Kconfig
> > index 3782eeeb91c0..c9070c728b9a 100644
> > --- a/drivers/dma-buf/heaps/Kconfig
> > +++ b/drivers/dma-buf/heaps/Kconfig
> > @@ -20,3 +20,12 @@ config DMABUF_HEAPS_DSP
> >Choose this option to enable the dsp dmabuf heap. The
> > dsp heap
> >is allocated by gen allocater. it's allocated according
> > the dts.
> >If in doubt, say Y.
> > +
> > +config DMABUF_HEAPS_SECURE
> > + tristate "DMA-BUF Secure Heap"
> > + depends on DMABUF_HEAPS
> > + help
> > + Choose this option to enable the secure dmabuf heap. The
> > secure heap
> > + pools are defined according to the DT. Heaps are allocated
> > + in the pools using gen allocater.
> > + If in doubt, say Y.
> > diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-
> > buf/heaps/Makefile
> > index 29733f84c354..863ef10056a3 100644
> > --- a/drivers/dma-buf/heaps/Makefile
> > +++ b/drivers/dma-buf/heaps/Makefile
> > @@ -2,3 +2,4 @@
> > obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)+= system_heap.o
> > obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o
> > obj-$(CONFIG_DMABUF_HEAPS_DSP) += dsp_heap.o
> > +obj-$(CONFIG_DMABUF_HEAPS_SECURE)+= secure_heap.o
> > diff --git a/drivers/dma-buf/heaps/secure_heap.c b/drivers/dma-
> > buf/heaps/secure_heap.c
> > new file mode 100644
> > index ..25b3629613f3
> > --- /dev/null
> > +++ b/drivers/dma-buf/heaps/secure_heap.c
> > @@ -0,0 +1,357 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * DMABUF secure heap exporter
> > + *
> > + * Copyright 2021 NXP.
>
> It's 2022 :-)
>
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define MAX_SECURE_HEAP 2
> > +#define MAX_HEAP_NAME_LEN 32
> > +
> > +struct secure_heap_buffer {
> > + struct dma_heap *heap;
> > + struct list_head attachments;
Re: [EXT] Re: [PATCH 3/5] dma-buf: heaps: add Linaro secure dmabuf heap support
Hi Brian, Sorry for pushing all the patches again, but I've done some cleanup regarding your comments. secure-heap are now protected by default and no mapping could be done for the CPU. deferred-free is no more needed. useless page pool code removed too. Best regards, Olivier On mer., 2022-08-03 at 13:37 +0100, Brian Starkey wrote: > Caution: EXT Email > > Hi, > > On Wed, Aug 03, 2022 at 11:07:54AM +, Olivier Masse wrote: > > Hi Brian, > > > > Thanks for your comments, please find my reply below. > > > > On mar., 2022-08-02 at 15:39 +0100, Brian Starkey wrote: > > > Caution: EXT Email > > > > > > Hi Olivier, > > > > > > Some comments below as I mentioned off-list. > > > > > > One additional point: some devices need to know if a buffer is > > > protected, so that they can configure registers appropriately to > > > make > > > use of that protected buffer. There was previously a discussion > > > about > > > adding a flag to a dma_buf to indicate that it is allocated from > > > protected memory[1]. > > > > > > [1] > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Farchives%2Fdri-devel%2F2019-September%2F238059.htmldata=05%7C01%7Colivier.masse%40nxp.com%7Cafa24901ad92491c9a6a08da754d00b5%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637951270862339002%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=LoQfoWYr0yHvTWa2tth9XOm2PACji4ATHnx3BRNKnJM%3Dreserved=0 > > > > > > > > > > Interesting, could we introduce is_protected 1-bit flag into struct > > dma_buf ? > > That was the earlier proposal, yeah. > > > struct dma_buf_ops.map_dma_buf and struct dma_buf_ops.unmap_dma_buf > > could become optional for such buffer ? > > > > map_dma_buf and unmap_dma_buf are needed to give devices access to > the > dma-buf, I don't think they should become optional. > > Mapping to the CPU maybe should be optional/disallowed for protected > buffers. > > > > On Tue, Aug 02, 2022 at 11:58:41AM +0200, Olivier Masse wrote: > > > > add Linaro secure heap bindings: linaro,secure-heap > > > > use genalloc to allocate/free buffer from buffer pool. > > > > buffer pool info is from dts. > > > > use sg_table instore the allocated memory info, the length of > > > > sg_table is 1. > > > > implement secure_heap_buf_ops to implement buffer share in > > > > difference device: > > > > 1. Userspace passes this fd to all drivers it wants this buffer > > > > to share with: First the filedescriptor is converted to a > > > > _buf > > > > using > > > > dma_buf_get(). Then the buffer is attached to the device using > > > > dma_buf_attach(). > > > > 2. Once the buffer is attached to all devices userspace can > > > > initiate DMA > > > > access to the shared buffer. In the kernel this is done by > > > > calling > > > > dma_buf_map_attachment() > > > > 3. get sg_table with dma_buf_map_attachment in difference > > > > device. > > > > > > > > Signed-off-by: Olivier Masse > > > > --- > > > > drivers/dma-buf/heaps/Kconfig | 21 +- > > > > drivers/dma-buf/heaps/Makefile | 1 + > > > > drivers/dma-buf/heaps/secure_heap.c | 588 > > > > > > > > 3 files changed, 606 insertions(+), 4 deletions(-) > > > > create mode 100644 drivers/dma-buf/heaps/secure_heap.c > > > > > > > > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma- > > > > buf/heaps/Kconfig > > > > index 6a33193a7b3e..b2406932192e 100644 > > > > --- a/drivers/dma-buf/heaps/Kconfig > > > > +++ b/drivers/dma-buf/heaps/Kconfig > > > > @@ -1,8 +1,12 @@ > > > > -config DMABUF_HEAPS_DEFERRED_FREE > > > > - tristate > > > > +menuconfig DMABUF_HEAPS_DEFERRED_FREE > > > > + bool "DMA-BUF heaps deferred-free library" > > > > + help > > > > + Choose this option to enable the DMA-BUF heaps > > > > deferred- > > > > free library. > > > > > > > > -config DMABUF_HEAPS_PAGE_POOL > > > > - tristate > > > > +menuconfig DMABUF_HEAPS_PAGE_POOL > > > > + bool "DMA-BUF heaps page-pool library" > > > > + help > > >
[PATCH 3/3] plat-hikey: Add linaro,secure-heap compatible
Add DMABUF_HEAPS_SECURE in defconfig
Signed-off-by: Olivier Masse
---
arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 11 +++
arch/arm64/configs/defconfig | 2 ++
2 files changed, 13 insertions(+)
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index 3df2afb2f637..e4af0d914279 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -258,6 +258,17 @@ optee {
};
};
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ sdp@3e80 {
+ compatible = "linaro,secure-heap";
+ no-map;
+ reg = <0 0x3E80 0 0x0040>;
+ };
+ };
+
sound_card {
compatible = "audio-graph-card";
dais = <_port0>;
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index c09b07c22d57..ffdc45acef94 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1465,6 +1465,8 @@ CONFIG_CRYPTO_DEV_HISI_SEC2=m
CONFIG_CRYPTO_DEV_HISI_ZIP=m
CONFIG_CRYPTO_DEV_HISI_HPRE=m
CONFIG_CRYPTO_DEV_HISI_TRNG=m
+CONFIG_DMABUF_HEAPS=y
+CONFIG_DMABUF_HEAPS_SECURE=y
CONFIG_CMA_SIZE_MBYTES=32
CONFIG_PRINTK_TIME=y
CONFIG_DEBUG_KERNEL=y
--
2.25.0
[PATCH 1/3] dma-buf: heaps: add Linaro secure dmabuf heap support
add Linaro secure heap bindings: linaro,secure-heap
use genalloc to allocate/free buffer from buffer pool.
buffer pool info is from dts.
use sg_table instore the allocated memory info, the length of sg_table is 1.
implement secure_heap_buf_ops to implement buffer share in difference device:
1. Userspace passes this fd to all drivers it wants this buffer
to share with: First the filedescriptor is converted to a _buf using
dma_buf_get(). Then the buffer is attached to the device using dma_buf_attach().
2. Once the buffer is attached to all devices userspace can initiate DMA
access to the shared buffer. In the kernel this is done by calling
dma_buf_map_attachment()
3. get sg_table with dma_buf_map_attachment in difference device.
Signed-off-by: Olivier Masse
---
drivers/dma-buf/heaps/Kconfig | 9 +
drivers/dma-buf/heaps/Makefile | 1 +
drivers/dma-buf/heaps/secure_heap.c | 357
3 files changed, 367 insertions(+)
create mode 100644 drivers/dma-buf/heaps/secure_heap.c
diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
index 3782eeeb91c0..c9070c728b9a 100644
--- a/drivers/dma-buf/heaps/Kconfig
+++ b/drivers/dma-buf/heaps/Kconfig
@@ -20,3 +20,12 @@ config DMABUF_HEAPS_DSP
Choose this option to enable the dsp dmabuf heap. The dsp heap
is allocated by gen allocater. it's allocated according the dts.
If in doubt, say Y.
+
+config DMABUF_HEAPS_SECURE
+ tristate "DMA-BUF Secure Heap"
+ depends on DMABUF_HEAPS
+ help
+ Choose this option to enable the secure dmabuf heap. The secure heap
+ pools are defined according to the DT. Heaps are allocated
+ in the pools using gen allocater.
+ If in doubt, say Y.
diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
index 29733f84c354..863ef10056a3 100644
--- a/drivers/dma-buf/heaps/Makefile
+++ b/drivers/dma-buf/heaps/Makefile
@@ -2,3 +2,4 @@
obj-$(CONFIG_DMABUF_HEAPS_SYSTEM) += system_heap.o
obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o
obj-$(CONFIG_DMABUF_HEAPS_DSP) += dsp_heap.o
+obj-$(CONFIG_DMABUF_HEAPS_SECURE) += secure_heap.o
diff --git a/drivers/dma-buf/heaps/secure_heap.c
b/drivers/dma-buf/heaps/secure_heap.c
new file mode 100644
index ..25b3629613f3
--- /dev/null
+++ b/drivers/dma-buf/heaps/secure_heap.c
@@ -0,0 +1,357 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DMABUF secure heap exporter
+ *
+ * Copyright 2021 NXP.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MAX_SECURE_HEAP 2
+#define MAX_HEAP_NAME_LEN 32
+
+struct secure_heap_buffer {
+ struct dma_heap *heap;
+ struct list_head attachments;
+ struct mutex lock;
+ unsigned long len;
+ struct sg_table sg_table;
+ int vmap_cnt;
+ void *vaddr;
+};
+
+struct secure_heap_attachment {
+ struct device *dev;
+ struct sg_table *table;
+ struct list_head list;
+};
+
+struct secure_heap_info {
+ struct gen_pool *pool;
+};
+
+struct rmem_secure {
+ phys_addr_t base;
+ phys_addr_t size;
+
+ char name[MAX_HEAP_NAME_LEN];
+};
+
+static struct rmem_secure secure_data[MAX_SECURE_HEAP] = {0};
+static unsigned int secure_data_count;
+
+static struct sg_table *dup_sg_table(struct sg_table *table)
+{
+ struct sg_table *new_table;
+ int ret, i;
+ struct scatterlist *sg, *new_sg;
+
+ new_table = kzalloc(sizeof(*new_table), GFP_KERNEL);
+ if (!new_table)
+ return ERR_PTR(-ENOMEM);
+
+ ret = sg_alloc_table(new_table, table->orig_nents, GFP_KERNEL);
+ if (ret) {
+ kfree(new_table);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ new_sg = new_table->sgl;
+ for_each_sgtable_sg(table, sg, i) {
+ sg_set_page(new_sg, sg_page(sg), sg->length, sg->offset);
+ new_sg->dma_address = sg->dma_address;
+#ifdef CONFIG_NEED_SG_DMA_LENGTH
+ new_sg->dma_length = sg->dma_length;
+#endif
+ new_sg = sg_next(new_sg);
+ }
+
+ return new_table;
+}
+
+static int secure_heap_attach(struct dma_buf *dmabuf,
+ struct dma_buf_attachment *attachment)
+{
+ struct secure_heap_buffer *buffer = dmabuf->priv;
+ struct secure_heap_attachment *a;
+ struct sg_table *table;
+
+ a = kzalloc(sizeof(*a), GFP_KERNEL);
+ if (!a)
+ return -ENOMEM;
+
+ table = dup_sg_table(>sg_table);
+ if (IS_ERR(table)) {
+ kfree(a);
+ return -ENOMEM;
+ }
+
+ a->table = table;
+ a->dev = attachment->dev;
+ INIT_LIST_HEAD(>list);
+ attachment->priv = a;
+
+ mutex_lock(>lock);
+ list_add(>list, &
[PATCH 2/3] dt-bindings: reserved-memory: add linaro,secure-heap
DMABUF Reserved memory definition for OP-TEE SDP feaure.
Signed-off-by: Olivier Masse
---
.../reserved-memory/linaro,secure-heap.yaml | 56 +++
1 file changed, 56 insertions(+)
create mode 100644
Documentation/devicetree/bindings/reserved-memory/linaro,secure-heap.yaml
diff --git
a/Documentation/devicetree/bindings/reserved-memory/linaro,secure-heap.yaml
b/Documentation/devicetree/bindings/reserved-memory/linaro,secure-heap.yaml
new file mode 100644
index ..80522a4e2989
--- /dev/null
+++ b/Documentation/devicetree/bindings/reserved-memory/linaro,secure-heap.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/linaro,secure-heap.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Linaro Secure DMABUF Heap
+
+maintainers:
+ - Olivier Masse
+
+description:
+ Linaro OP-TEE firmware needs a reserved memory for the
+ Secure Data Path feature (aka SDP).
+ The purpose is to provide a secure memory heap which allow
+ non-secure OS to allocate/free secure buffers.
+ The TEE is reponsible for protecting the SDP memory buffers.
+ TEE Trusted Application can access secure memory references
+ provided as parameters (DMABUF file descriptor).
+
+allOf:
+ - $ref: "reserved-memory.yaml"
+
+properties:
+ compatible:
+const: linaro,secure-heap
+
+ reg:
+description:
+ Region of memory reserved for OP-TEE SDP feature
+
+ no-map:
+$ref: /schemas/types.yaml#/definitions/flag
+description:
+ Avoid creating a virtual mapping of the region as part of the OS'
+ standard mapping of system memory.
+
+unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - no-map
+
+examples:
+ - |
+ reserved-memory {
+#address-cells = <2>;
+#size-cells = <2>;
+
+sdp@3e80 {
+ compatible = "linaro,secure-heap";
+ no-map;
+ reg = <0 0x3E80 0 0x0040>;
+};
+ };
--
2.25.0
[PATCH 0/3] Add dma-buf secure-heap
Purpose of these patches is to add a new dma-buf heap: linaro,secure-heap Linaro OPTEE OS Secure Data Path feature is relying on a reserved memory defined at Linux Kernel level and OPTEE OS level. >From Linux Kernel side, heap management is using dma-buf heaps interface. Olivier Masse (3): dma-buf: heaps: add Linaro secure dmabuf heap support dt-bindings: reserved-memory: add linaro,secure-heap plat-hikey: Add linaro,secure-heap compatible .../reserved-memory/linaro,secure-heap.yaml | 56 +++ .../arm64/boot/dts/hisilicon/hi6220-hikey.dts | 11 + arch/arm64/configs/defconfig | 2 + drivers/dma-buf/heaps/Kconfig | 9 + drivers/dma-buf/heaps/Makefile| 1 + drivers/dma-buf/heaps/secure_heap.c | 357 ++ 6 files changed, 436 insertions(+) create mode 100644 Documentation/devicetree/bindings/reserved-memory/linaro,secure-heap.yaml create mode 100644 drivers/dma-buf/heaps/secure_heap.c -- 2.25.0
Re: [EXT] Re: [PATCH 3/5] dma-buf: heaps: add Linaro secure dmabuf heap support
Hi Brian, Thanks for your comments, please find my reply below. On mar., 2022-08-02 at 15:39 +0100, Brian Starkey wrote: > Caution: EXT Email > > Hi Olivier, > > Some comments below as I mentioned off-list. > > One additional point: some devices need to know if a buffer is > protected, so that they can configure registers appropriately to make > use of that protected buffer. There was previously a discussion about > adding a flag to a dma_buf to indicate that it is allocated from > protected memory[1]. > > [1] > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Farchives%2Fdri-devel%2F2019-September%2F238059.htmldata=05%7C01%7Colivier.masse%40nxp.com%7C64e0ce1952ac4e926a8208da7494d0bb%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637950479760002497%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=bmlw9uLeGtn%2F7JliZ07nSm6XDEzEqdwn4mBQHIVnma0%3Dreserved=0 > > Interesting, could we introduce is_protected 1-bit flag into struct dma_buf ? struct dma_buf_ops.map_dma_buf and struct dma_buf_ops.unmap_dma_buf could become optional for such buffer ? > On Tue, Aug 02, 2022 at 11:58:41AM +0200, Olivier Masse wrote: > > add Linaro secure heap bindings: linaro,secure-heap > > use genalloc to allocate/free buffer from buffer pool. > > buffer pool info is from dts. > > use sg_table instore the allocated memory info, the length of > > sg_table is 1. > > implement secure_heap_buf_ops to implement buffer share in > > difference device: > > 1. Userspace passes this fd to all drivers it wants this buffer > > to share with: First the filedescriptor is converted to a _buf > > using > > dma_buf_get(). Then the buffer is attached to the device using > > dma_buf_attach(). > > 2. Once the buffer is attached to all devices userspace can > > initiate DMA > > access to the shared buffer. In the kernel this is done by calling > > dma_buf_map_attachment() > > 3. get sg_table with dma_buf_map_attachment in difference device. > > > > Signed-off-by: Olivier Masse > > --- > > drivers/dma-buf/heaps/Kconfig | 21 +- > > drivers/dma-buf/heaps/Makefile | 1 + > > drivers/dma-buf/heaps/secure_heap.c | 588 > > > > 3 files changed, 606 insertions(+), 4 deletions(-) > > create mode 100644 drivers/dma-buf/heaps/secure_heap.c > > > > diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma- > > buf/heaps/Kconfig > > index 6a33193a7b3e..b2406932192e 100644 > > --- a/drivers/dma-buf/heaps/Kconfig > > +++ b/drivers/dma-buf/heaps/Kconfig > > @@ -1,8 +1,12 @@ > > -config DMABUF_HEAPS_DEFERRED_FREE > > - tristate > > +menuconfig DMABUF_HEAPS_DEFERRED_FREE > > + bool "DMA-BUF heaps deferred-free library" > > + help > > + Choose this option to enable the DMA-BUF heaps deferred- > > free library. > > > > -config DMABUF_HEAPS_PAGE_POOL > > - tristate > > +menuconfig DMABUF_HEAPS_PAGE_POOL > > + bool "DMA-BUF heaps page-pool library" > > + help > > + Choose this option to enable the DMA-BUF heaps page-pool > > library. > > > > config DMABUF_HEAPS_SYSTEM > > bool "DMA-BUF System Heap" > > @@ -26,3 +30,12 @@ config DMABUF_HEAPS_DSP > >Choose this option to enable the dsp dmabuf heap. The > > dsp heap > >is allocated by gen allocater. it's allocated according > > the dts. > >If in doubt, say Y. > > + > > +config DMABUF_HEAPS_SECURE > > + tristate "DMA-BUF Secure Heap" > > + depends on DMABUF_HEAPS && DMABUF_HEAPS_DEFERRED_FREE > > + help > > + Choose this option to enable the secure dmabuf heap. The > > secure heap > > + pools are defined according to the DT. Heaps are allocated > > + in the pools using gen allocater. > > + If in doubt, say Y. > > diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma- > > buf/heaps/Makefile > > index e70722ea615e..08f6aa5919d1 100644 > > --- a/drivers/dma-buf/heaps/Makefile > > +++ b/drivers/dma-buf/heaps/Makefile > > @@ -4,3 +4,4 @@ obj-$(CONFIG_DMABUF_HEAPS_PAGE_POOL) += > > page_pool.o > > obj-$(CONFIG_DMABUF_HEAPS_SYSTEM)+= system_heap.o > > obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o > > obj-$(CONFIG_DMABUF_HEAPS_DSP) += dsp_heap.o > > +obj-$(CONFIG_DMABUF_HEAPS_SECURE)+= secure_heap.o > > diff --git a/drivers/dma-buf/heaps/secure_heap.c b/drive
[PATCH 4/5] dt-bindings: reserved-memory: add linaro,secure-heap
DMABUF Reserved memory definition for OP-TEE SDP feaure.
Signed-off-by: Olivier Masse
---
.../reserved-memory/linaro,secure-heap.yaml | 56 +++
1 file changed, 56 insertions(+)
create mode 100644
Documentation/devicetree/bindings/reserved-memory/linaro,secure-heap.yaml
diff --git
a/Documentation/devicetree/bindings/reserved-memory/linaro,secure-heap.yaml
b/Documentation/devicetree/bindings/reserved-memory/linaro,secure-heap.yaml
new file mode 100644
index ..80522a4e2989
--- /dev/null
+++ b/Documentation/devicetree/bindings/reserved-memory/linaro,secure-heap.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/linaro,secure-heap.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Linaro Secure DMABUF Heap
+
+maintainers:
+ - Olivier Masse
+
+description:
+ Linaro OP-TEE firmware needs a reserved memory for the
+ Secure Data Path feature (aka SDP).
+ The purpose is to provide a secure memory heap which allow
+ non-secure OS to allocate/free secure buffers.
+ The TEE is reponsible for protecting the SDP memory buffers.
+ TEE Trusted Application can access secure memory references
+ provided as parameters (DMABUF file descriptor).
+
+allOf:
+ - $ref: "reserved-memory.yaml"
+
+properties:
+ compatible:
+const: linaro,secure-heap
+
+ reg:
+description:
+ Region of memory reserved for OP-TEE SDP feature
+
+ no-map:
+$ref: /schemas/types.yaml#/definitions/flag
+description:
+ Avoid creating a virtual mapping of the region as part of the OS'
+ standard mapping of system memory.
+
+unevaluatedProperties: false
+
+required:
+ - compatible
+ - reg
+ - no-map
+
+examples:
+ - |
+ reserved-memory {
+#address-cells = <2>;
+#size-cells = <2>;
+
+sdp@3e80 {
+ compatible = "linaro,secure-heap";
+ no-map;
+ reg = <0 0x3E80 0 0x0040>;
+};
+ };
--
2.25.0
[PATCH 0/5] Add dma-buf secure-heap
Purpose of these patches is to add a new dma-buf heap: linaro,secure-heap Linaro OPTEE OS Secure Data Path feature is relying on a reserved memory defined at Linux Kernel level and OPTEE OS level. >From Linux Kernel side, heap management is using dma-buf heaps interface. John Stultz (2): ANDROID: dma-buf: heaps: Add deferred-free-helper library code ANDROID: dma-buf: heaps: Add a shrinker controlled page pool Olivier Masse (3): dma-buf: heaps: add Linaro secure dmabuf heap support dt-bindings: reserved-memory: add linaro,secure-heap plat-hikey: Add linaro,secure-heap compatible .../reserved-memory/linaro,secure-heap.yaml | 56 ++ .../arm64/boot/dts/hisilicon/hi6220-hikey.dts | 11 + arch/arm64/configs/defconfig | 4 + drivers/dma-buf/heaps/Kconfig | 19 + drivers/dma-buf/heaps/Makefile| 3 + drivers/dma-buf/heaps/deferred-free-helper.c | 136 drivers/dma-buf/heaps/deferred-free-helper.h | 63 ++ drivers/dma-buf/heaps/page_pool.c | 246 drivers/dma-buf/heaps/page_pool.h | 55 ++ drivers/dma-buf/heaps/secure_heap.c | 588 ++ 10 files changed, 1181 insertions(+) create mode 100644 Documentation/devicetree/bindings/reserved-memory/linaro,secure-heap.yaml create mode 100644 drivers/dma-buf/heaps/deferred-free-helper.c create mode 100644 drivers/dma-buf/heaps/deferred-free-helper.h create mode 100644 drivers/dma-buf/heaps/page_pool.c create mode 100644 drivers/dma-buf/heaps/page_pool.h create mode 100644 drivers/dma-buf/heaps/secure_heap.c -- 2.25.0
[PATCH 5/5] plat-hikey: Add linaro,secure-heap compatible
Add DMABUF_HEAPS_SECURE in defconfig
Signed-off-by: Olivier Masse
---
arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts | 11 +++
arch/arm64/configs/defconfig | 4
2 files changed, 15 insertions(+)
diff --git a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
index 3df2afb2f637..e4af0d914279 100644
--- a/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
+++ b/arch/arm64/boot/dts/hisilicon/hi6220-hikey.dts
@@ -258,6 +258,17 @@ optee {
};
};
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ sdp@3e80 {
+ compatible = "linaro,secure-heap";
+ no-map;
+ reg = <0 0x3E80 0 0x0040>;
+ };
+ };
+
sound_card {
compatible = "audio-graph-card";
dais = <_port0>;
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index c09b07c22d57..4b625043313d 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -1465,6 +1465,10 @@ CONFIG_CRYPTO_DEV_HISI_SEC2=m
CONFIG_CRYPTO_DEV_HISI_ZIP=m
CONFIG_CRYPTO_DEV_HISI_HPRE=m
CONFIG_CRYPTO_DEV_HISI_TRNG=m
+CONFIG_DMABUF_HEAPS=y
+CONFIG_DMABUF_HEAPS_DEFERRED_FREE=y
+CONFIG_DMABUF_HEAPS_PAGE_POOL=y
+CONFIG_DMABUF_HEAPS_SECURE=y
CONFIG_CMA_SIZE_MBYTES=32
CONFIG_PRINTK_TIME=y
CONFIG_DEBUG_KERNEL=y
--
2.25.0
[PATCH 1/5] ANDROID: dma-buf: heaps: Add deferred-free-helper library code
From: John Stultz
This patch provides infrastructure for deferring buffer frees.
This is a feature ION provided which when used with some form
of a page pool, provides a nice performance boost in an
allocation microbenchmark. The reason it helps is it allows the
page-zeroing to be done out of the normal allocation/free path,
and pushed off to a kthread.
As not all heaps will find this useful, its implemented as
a optional helper library that heaps can utilize.
Cc: Daniel Vetter
Cc: Sumit Semwal
Cc: Liam Mark
Cc: Chris Goldsworthy
Cc: Laura Abbott
Cc: Brian Starkey
Cc: Hridya Valsaraju
Cc: Suren Baghdasaryan
Cc: Sandeep Patil
Cc: Daniel Mentz
Cc: Ørjan Eide
Cc: Robin Murphy
Cc: Ezequiel Garcia
Cc: Simon Ser
Cc: James Jones
Cc: linux-me...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz
Bug: 168742043
---
drivers/dma-buf/heaps/Kconfig| 3 +
drivers/dma-buf/heaps/Makefile | 1 +
drivers/dma-buf/heaps/deferred-free-helper.c | 136 +++
drivers/dma-buf/heaps/deferred-free-helper.h | 63 +
4 files changed, 203 insertions(+)
create mode 100644 drivers/dma-buf/heaps/deferred-free-helper.c
create mode 100644 drivers/dma-buf/heaps/deferred-free-helper.h
diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
index 3782eeeb91c0..8ee64277a5d2 100644
--- a/drivers/dma-buf/heaps/Kconfig
+++ b/drivers/dma-buf/heaps/Kconfig
@@ -1,3 +1,6 @@
+config DMABUF_HEAPS_DEFERRED_FREE
+ tristate
+
config DMABUF_HEAPS_SYSTEM
bool "DMA-BUF System Heap"
depends on DMABUF_HEAPS
diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
index 29733f84c354..5de95b77e169 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_DEFERRED_FREE) += deferred-free-helper.o
obj-$(CONFIG_DMABUF_HEAPS_SYSTEM) += system_heap.o
obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o
obj-$(CONFIG_DMABUF_HEAPS_DSP) += dsp_heap.o
diff --git a/drivers/dma-buf/heaps/deferred-free-helper.c
b/drivers/dma-buf/heaps/deferred-free-helper.c
new file mode 100644
index ..478faa319908
--- /dev/null
+++ b/drivers/dma-buf/heaps/deferred-free-helper.c
@@ -0,0 +1,136 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Deferred dmabuf freeing helper
+ *
+ * Copyright (C) 2020 Linaro, Ltd.
+ *
+ * Based on the ION page pool code
+ * Copyright (C) 2011 Google, Inc.
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#include "deferred-free-helper.h"
+
+static LIST_HEAD(free_list);
+static size_t list_nr_pages;
+static wait_queue_head_t freelist_waitqueue;
+static struct task_struct *freelist_task;
+static DEFINE_SPINLOCK(free_list_lock);
+
+void deferred_free(struct deferred_freelist_item *item,
+ void (*free)(struct deferred_freelist_item*,
+ enum df_reason),
+ size_t nr_pages)
+{
+ unsigned long flags;
+
+ if (!nr_pages)
+ return;
+
+ INIT_LIST_HEAD(>list);
+ item->nr_pages = nr_pages;
+ item->free = free;
+
+ spin_lock_irqsave(_list_lock, flags);
+ list_add(>list, _list);
+ list_nr_pages += nr_pages;
+ spin_unlock_irqrestore(_list_lock, flags);
+ wake_up(_waitqueue);
+}
+EXPORT_SYMBOL_GPL(deferred_free);
+
+static size_t free_one_item(enum df_reason reason)
+{
+ unsigned long flags;
+ size_t nr_pages;
+ struct deferred_freelist_item *item;
+
+ spin_lock_irqsave(_list_lock, flags);
+ if (list_empty(_list)) {
+ spin_unlock_irqrestore(_list_lock, flags);
+ return 0;
+ }
+ item = list_first_entry(_list, struct deferred_freelist_item,
list);
+ list_del(>list);
+ nr_pages = item->nr_pages;
+ list_nr_pages -= nr_pages;
+ spin_unlock_irqrestore(_list_lock, flags);
+
+ item->free(item, reason);
+ return nr_pages;
+}
+
+static unsigned long get_freelist_nr_pages(void)
+{
+ unsigned long nr_pages;
+ unsigned long flags;
+
+ spin_lock_irqsave(_list_lock, flags);
+ nr_pages = list_nr_pages;
+ spin_unlock_irqrestore(_list_lock, flags);
+ return nr_pages;
+}
+
+static unsigned long freelist_shrink_count(struct shrinker *shrinker,
+ struct shrink_control *sc)
+{
+ return get_freelist_nr_pages();
+}
+
+static unsigned long freelist_shrink_scan(struct shrinker *shrinker,
+ struct shrink_control *sc)
+{
+ unsigned long total_freed = 0;
+
+ while (total_freed < sc->nr_to_scan) {
+ size_t pages_freed = free_one_item(DF_UNDER_PRESSURE);
+
+ if (!pages_freed)
+ break;
+
+ total_freed += pages_freed;
+ }
+
+ return total_freed;
+}
+
[PATCH 3/5] dma-buf: heaps: add Linaro secure dmabuf heap support
add Linaro secure heap bindings: linaro,secure-heap
use genalloc to allocate/free buffer from buffer pool.
buffer pool info is from dts.
use sg_table instore the allocated memory info, the length of sg_table is 1.
implement secure_heap_buf_ops to implement buffer share in difference device:
1. Userspace passes this fd to all drivers it wants this buffer
to share with: First the filedescriptor is converted to a _buf using
dma_buf_get(). Then the buffer is attached to the device using dma_buf_attach().
2. Once the buffer is attached to all devices userspace can initiate DMA
access to the shared buffer. In the kernel this is done by calling
dma_buf_map_attachment()
3. get sg_table with dma_buf_map_attachment in difference device.
Signed-off-by: Olivier Masse
---
drivers/dma-buf/heaps/Kconfig | 21 +-
drivers/dma-buf/heaps/Makefile | 1 +
drivers/dma-buf/heaps/secure_heap.c | 588
3 files changed, 606 insertions(+), 4 deletions(-)
create mode 100644 drivers/dma-buf/heaps/secure_heap.c
diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
index 6a33193a7b3e..b2406932192e 100644
--- a/drivers/dma-buf/heaps/Kconfig
+++ b/drivers/dma-buf/heaps/Kconfig
@@ -1,8 +1,12 @@
-config DMABUF_HEAPS_DEFERRED_FREE
- tristate
+menuconfig DMABUF_HEAPS_DEFERRED_FREE
+ bool "DMA-BUF heaps deferred-free library"
+ help
+ Choose this option to enable the DMA-BUF heaps deferred-free library.
-config DMABUF_HEAPS_PAGE_POOL
- tristate
+menuconfig DMABUF_HEAPS_PAGE_POOL
+ bool "DMA-BUF heaps page-pool library"
+ help
+ Choose this option to enable the DMA-BUF heaps page-pool library.
config DMABUF_HEAPS_SYSTEM
bool "DMA-BUF System Heap"
@@ -26,3 +30,12 @@ config DMABUF_HEAPS_DSP
Choose this option to enable the dsp dmabuf heap. The dsp heap
is allocated by gen allocater. it's allocated according the dts.
If in doubt, say Y.
+
+config DMABUF_HEAPS_SECURE
+ tristate "DMA-BUF Secure Heap"
+ depends on DMABUF_HEAPS && DMABUF_HEAPS_DEFERRED_FREE
+ help
+ Choose this option to enable the secure dmabuf heap. The secure heap
+ pools are defined according to the DT. Heaps are allocated
+ in the pools using gen allocater.
+ If in doubt, say Y.
diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
index e70722ea615e..08f6aa5919d1 100644
--- a/drivers/dma-buf/heaps/Makefile
+++ b/drivers/dma-buf/heaps/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_DMABUF_HEAPS_PAGE_POOL)+= page_pool.o
obj-$(CONFIG_DMABUF_HEAPS_SYSTEM) += system_heap.o
obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o
obj-$(CONFIG_DMABUF_HEAPS_DSP) += dsp_heap.o
+obj-$(CONFIG_DMABUF_HEAPS_SECURE) += secure_heap.o
diff --git a/drivers/dma-buf/heaps/secure_heap.c
b/drivers/dma-buf/heaps/secure_heap.c
new file mode 100644
index ..31aac5d050b4
--- /dev/null
+++ b/drivers/dma-buf/heaps/secure_heap.c
@@ -0,0 +1,588 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DMABUF secure heap exporter
+ *
+ * Copyright 2021 NXP.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "deferred-free-helper.h"
+#include "page_pool.h"
+
+#define MAX_SECURE_HEAP 2
+#define MAX_HEAP_NAME_LEN 32
+
+struct secure_heap_buffer {
+ struct dma_heap *heap;
+ struct list_head attachments;
+ struct mutex lock;
+ unsigned long len;
+ struct sg_table sg_table;
+ int vmap_cnt;
+ struct deferred_freelist_item deferred_free;
+ void *vaddr;
+ bool uncached;
+};
+
+struct dma_heap_attachment {
+ struct device *dev;
+ struct sg_table *table;
+ struct list_head list;
+ bool no_map;
+ bool mapped;
+ bool uncached;
+};
+
+struct secure_heap_info {
+ struct gen_pool *pool;
+
+ bool no_map;
+};
+
+struct rmem_secure {
+ phys_addr_t base;
+ phys_addr_t size;
+
+ char name[MAX_HEAP_NAME_LEN];
+
+ bool no_map;
+};
+
+static struct rmem_secure secure_data[MAX_SECURE_HEAP] = {0};
+static unsigned int secure_data_count;
+
+static struct sg_table *dup_sg_table(struct sg_table *table)
+{
+ struct sg_table *new_table;
+ int ret, i;
+ struct scatterlist *sg, *new_sg;
+
+ new_table = kzalloc(sizeof(*new_table), GFP_KERNEL);
+ if (!new_table)
+ return ERR_PTR(-ENOMEM);
+
+ ret = sg_alloc_table(new_table, table->orig_nents, GFP_KERNEL);
+ if (ret) {
+ kfree(new_table);
+ return ERR_PTR(-ENOMEM);
+ }
+
+ new_sg = new_table->sgl;
+ for_each_sgtable_sg(table, sg, i) {
+ sg_set_page(new_sg, sg_page(sg), sg->length, sg->offset);
+
[PATCH 2/5] ANDROID: dma-buf: heaps: Add a shrinker controlled page pool
From: John Stultz
This patch adds a simple shrinker controlled page pool to the
dmabuf heaps subsystem.
This replaces the use of the networking page_pool, over concerns
that the lack of a shrinker for that implementation may cause
additional low-memory kills
TODO: Take another pass at trying to unify this w/ the ttm pool
Thoughts and feedback would be greatly appreciated!
Cc: Sumit Semwal
Cc: Liam Mark
Cc: Laura Abbott
Cc: Brian Starkey
Cc: Hridya Valsaraju
Cc: Suren Baghdasaryan
Cc: Sandeep Patil
Cc: Daniel Mentz
Cc: Chris Goldsworthy
Cc: Ørjan Eide
Cc: Robin Murphy
Cc: Ezequiel Garcia
Cc: Simon Ser
Cc: James Jones
Cc: Andrew Morton
Cc: Dave Hansen
Cc: linux...@kvack.org
Cc: linux-me...@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: John Stultz
Signed-off-by: Olivier Masse
Bug: 168742043
---
drivers/dma-buf/heaps/Kconfig | 3 +
drivers/dma-buf/heaps/Makefile| 1 +
drivers/dma-buf/heaps/page_pool.c | 246 ++
drivers/dma-buf/heaps/page_pool.h | 55 +++
4 files changed, 305 insertions(+)
create mode 100644 drivers/dma-buf/heaps/page_pool.c
create mode 100644 drivers/dma-buf/heaps/page_pool.h
diff --git a/drivers/dma-buf/heaps/Kconfig b/drivers/dma-buf/heaps/Kconfig
index 8ee64277a5d2..6a33193a7b3e 100644
--- a/drivers/dma-buf/heaps/Kconfig
+++ b/drivers/dma-buf/heaps/Kconfig
@@ -1,6 +1,9 @@
config DMABUF_HEAPS_DEFERRED_FREE
tristate
+config DMABUF_HEAPS_PAGE_POOL
+ tristate
+
config DMABUF_HEAPS_SYSTEM
bool "DMA-BUF System Heap"
depends on DMABUF_HEAPS
diff --git a/drivers/dma-buf/heaps/Makefile b/drivers/dma-buf/heaps/Makefile
index 5de95b77e169..e70722ea615e 100644
--- a/drivers/dma-buf/heaps/Makefile
+++ b/drivers/dma-buf/heaps/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
obj-$(CONFIG_DMABUF_HEAPS_DEFERRED_FREE) += deferred-free-helper.o
+obj-$(CONFIG_DMABUF_HEAPS_PAGE_POOL) += page_pool.o
obj-$(CONFIG_DMABUF_HEAPS_SYSTEM) += system_heap.o
obj-$(CONFIG_DMABUF_HEAPS_CMA) += cma_heap.o
obj-$(CONFIG_DMABUF_HEAPS_DSP) += dsp_heap.o
diff --git a/drivers/dma-buf/heaps/page_pool.c
b/drivers/dma-buf/heaps/page_pool.c
new file mode 100644
index ..3dd4c3862dca
--- /dev/null
+++ b/drivers/dma-buf/heaps/page_pool.c
@@ -0,0 +1,246 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * DMA BUF page pool system
+ *
+ * Copyright (C) 2020 Linaro Ltd.
+ *
+ * Based on the ION page pool code
+ * Copyright (C) 2011 Google, Inc.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include "page_pool.h"
+
+static LIST_HEAD(pool_list);
+static DEFINE_MUTEX(pool_list_lock);
+
+static struct page *dmabuf_page_pool_alloc_pages(struct dmabuf_page_pool *pool)
+{
+ if (fatal_signal_pending(current))
+ return NULL;
+ return alloc_pages(pool->gfp_mask, pool->order);
+}
+
+static void dmabuf_page_pool_free_pages(struct dmabuf_page_pool *pool,
+ struct page *page)
+{
+ __free_pages(page, pool->order);
+}
+
+static void dmabuf_page_pool_add(struct dmabuf_page_pool *pool, struct page
*page)
+{
+ int index;
+
+ if (PageHighMem(page))
+ index = POOL_HIGHPAGE;
+ else
+ index = POOL_LOWPAGE;
+
+ mutex_lock(>mutex);
+ list_add_tail(>lru, >items[index]);
+ pool->count[index]++;
+ mod_node_page_state(page_pgdat(page), NR_KERNEL_MISC_RECLAIMABLE,
+ 1 << pool->order);
+ mutex_unlock(>mutex);
+}
+
+static struct page *dmabuf_page_pool_remove(struct dmabuf_page_pool *pool, int
index)
+{
+ struct page *page;
+
+ mutex_lock(>mutex);
+ page = list_first_entry_or_null(>items[index], struct page, lru);
+ if (page) {
+ pool->count[index]--;
+ list_del(>lru);
+ mod_node_page_state(page_pgdat(page),
NR_KERNEL_MISC_RECLAIMABLE,
+ -(1 << pool->order));
+ }
+ mutex_unlock(>mutex);
+
+ return page;
+}
+
+static struct page *dmabuf_page_pool_fetch(struct dmabuf_page_pool *pool)
+{
+ struct page *page = NULL;
+
+ page = dmabuf_page_pool_remove(pool, POOL_HIGHPAGE);
+ if (!page)
+ page = dmabuf_page_pool_remove(pool, POOL_LOWPAGE);
+
+ return page;
+}
+
+struct page *dmabuf_page_pool_alloc(struct dmabuf_page_pool *pool)
+{
+ struct page *page = NULL;
+
+ if (WARN_ON(!pool))
+ return NULL;
+
+ page = dmabuf_page_pool_fetch(pool);
+ if (!page)
+ page = dmabuf_page_pool_alloc_pages(pool);
+
+ return page;
+}
+EXPORT_SYMBOL_GPL(dmabuf_page_pool_alloc);
+
+void dmabuf_page_pool_free(struct dmabuf_page_pool *pool, struct page *page)
+{
+ if (WARN_ON(pool->order != compound_order(page)))
+
