Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

15 participants
2903 discussions

[PATCH v3] drm/syncobj: Extend EXPORT_SYNC_FILE for timeline syncobjs

by Rob Clark

From: Rob Clark <robdclark(a)chromium.org> Add support for exporting a dma_fence fd for a specific point on a timeline. This is needed for vtest/vpipe[1][2] to implement timeline syncobj support, as it needs a way to turn a point on a timeline back into a dma_fence fd. It also closes an odd omission from the syncobj UAPI. [1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33433 [2] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/805 v2: Add DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE v3: Add unstaged uabi header hunk Signed-off-by: Rob Clark <robdclark(a)chromium.org> --- drivers/gpu/drm/drm_syncobj.c | 18 +++++++++++++----- include/uapi/drm/drm.h | 3 +++ 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 4f2ab8a7b50f..bc57d6f1a22e 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -762,7 +762,7 @@ static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private, } static int drm_syncobj_export_sync_file(struct drm_file *file_private, - int handle, int *p_fd) + int handle, u64 point, int *p_fd) { int ret; struct dma_fence *fence; @@ -772,7 +772,7 @@ static int drm_syncobj_export_sync_file(struct drm_file *file_private, if (fd < 0) return fd; - ret = drm_syncobj_find_fence(file_private, handle, 0, 0, &fence); + ret = drm_syncobj_find_fence(file_private, handle, point, 0, &fence); if (ret) goto err_put_fd; @@ -869,6 +869,9 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private) { struct drm_syncobj_handle *args = data; + unsigned valid_flags = DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE | + DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE; + u64 point = 0; if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) return -EOPNOTSUPP; @@ -876,13 +879,18 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data, if (args->pad) return -EINVAL; - if (args->flags != 0 && - args->flags != DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE) + if (args->flags != 0 && (args->flags & ~valid_flags)) return -EINVAL; + if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE) + point = args->point; + if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE) return drm_syncobj_export_sync_file(file_private, args->handle, - &args->fd); + point, &args->fd); + + if (args->point) + return -EINVAL; return drm_syncobj_handle_to_fd(file_private, args->handle, &args->fd); diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 7fba37b94401..dd0fd13eadbd 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -906,12 +906,15 @@ struct drm_syncobj_destroy { #define DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE (1 << 0) #define DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE (1 << 0) +#define DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE (1 << 1) struct drm_syncobj_handle { __u32 handle; __u32 flags; __s32 fd; __u32 pad; + + __u64 point; }; struct drm_syncobj_transfer { -- 2.49.0

3 months

[PATCH v2] drm/syncobj: Extend EXPORT_SYNC_FILE for timeline syncobjs

by Rob Clark

From: Rob Clark <robdclark(a)chromium.org> Add support for exporting a dma_fence fd for a specific point on a timeline. This is needed for vtest/vpipe[1][2] to implement timeline syncobj support, as it needs a way to turn a point on a timeline back into a dma_fence fd. It also closes an odd omission from the syncobj UAPI. [1] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/33433 [2] https://gitlab.freedesktop.org/virgl/virglrenderer/-/merge_requests/805 v2: Add DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE Signed-off-by: Rob Clark <robdclark(a)chromium.org> --- drivers/gpu/drm/drm_syncobj.c | 18 +++++++++++++----- include/uapi/drm/drm.h | 2 ++ 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 4f2ab8a7b50f..bc57d6f1a22e 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -762,7 +762,7 @@ static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private, } static int drm_syncobj_export_sync_file(struct drm_file *file_private, - int handle, int *p_fd) + int handle, u64 point, int *p_fd) { int ret; struct dma_fence *fence; @@ -772,7 +772,7 @@ static int drm_syncobj_export_sync_file(struct drm_file *file_private, if (fd < 0) return fd; - ret = drm_syncobj_find_fence(file_private, handle, 0, 0, &fence); + ret = drm_syncobj_find_fence(file_private, handle, point, 0, &fence); if (ret) goto err_put_fd; @@ -869,6 +869,9 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data, struct drm_file *file_private) { struct drm_syncobj_handle *args = data; + unsigned valid_flags = DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE | + DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE; + u64 point = 0; if (!drm_core_check_feature(dev, DRIVER_SYNCOBJ)) return -EOPNOTSUPP; @@ -876,13 +879,18 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data, if (args->pad) return -EINVAL; - if (args->flags != 0 && - args->flags != DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE) + if (args->flags != 0 && (args->flags & ~valid_flags)) return -EINVAL; + if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_TIMELINE) + point = args->point; + if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE) return drm_syncobj_export_sync_file(file_private, args->handle, - &args->fd); + point, &args->fd); + + if (args->point) + return -EINVAL; return drm_syncobj_handle_to_fd(file_private, args->handle, &args->fd); diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 7fba37b94401..c71a8f4439f2 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -912,6 +912,8 @@ struct drm_syncobj_handle { __s32 fd; __u32 pad; + + __u64 point; }; struct drm_syncobj_transfer { -- 2.49.0

3 months

Re: [PATCH] drm/nouveau: Prevent signalled fences in pending list

by Christian König

Am 27.03.25 um 09:42 schrieb Philipp Stanner: > Nouveau currently relies on the assumption that dma_fences will only > ever get signalled through nouveau_fence_signal(), which takes care of > removing a signalled fence from the list nouveau_fence_chan.pending. > > This self-imposed rule is violated in nouveau_fence_done(), where > dma_fence_is_signaled() can signal the fence without removing it from > the list. This enables accesses to already signalled fences through the > list, which is a bug. > > Furthermore, it must always be possible to use standard dma_fence > methods an a dma_fence and observe valid behavior. The canonical way of > ensuring that signalling a fence has additional effects is to add those > effects to a callback and register it on the fence. Good catch. > > Move the code from nouveau_fence_signal() into a dma_fence callback. > Register that callback when creating the fence. But that's a really ugly approach. Either nouveau shouldn't implement the signaled callback or make sure that when returning true from the signaled callback the fence is also removed from the pending list. > > Cc: <stable(a)vger.kernel.org> # 4.10+ > Fixes: f54d1867005c ("dma-buf: Rename struct fence to dma_fence") > Signed-off-by: Philipp Stanner <phasta(a)kernel.org> > --- > I'm not entirely sure what Fixes-Tag is appropriate. The last time the > line causing the signalled fence in the list was touched is the commit > listed above. Yeah, that's most likely not correct. My educated guess is that the bug was always there just never discovered. Regards, Christian. > --- > drivers/gpu/drm/nouveau/nouveau_fence.c | 41 ++++++++++++++++--------- > drivers/gpu/drm/nouveau/nouveau_fence.h | 1 + > 2 files changed, 27 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.c b/drivers/gpu/drm/nouveau/nouveau_fence.c > index 7cc84472cece..b2c2241a8803 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_fence.c > +++ b/drivers/gpu/drm/nouveau/nouveau_fence.c > @@ -50,24 +50,22 @@ nouveau_fctx(struct nouveau_fence *fence) > return container_of(fence->base.lock, struct nouveau_fence_chan, lock); > } > > -static int > -nouveau_fence_signal(struct nouveau_fence *fence) > +static void > +nouveau_fence_cleanup_cb(struct dma_fence *dfence, struct dma_fence_cb *cb) > { > - int drop = 0; > + struct nouveau_fence_chan *fctx; > + struct nouveau_fence *fence; > + > + fence = container_of(dfence, struct nouveau_fence, base); > + fctx = nouveau_fctx(fence); > > - dma_fence_signal_locked(&fence->base); > list_del(&fence->head); > rcu_assign_pointer(fence->channel, NULL); > > - if (test_bit(DMA_FENCE_FLAG_USER_BITS, &fence->base.flags)) { > - struct nouveau_fence_chan *fctx = nouveau_fctx(fence); > - > - if (!--fctx->notify_ref) > - drop = 1; > - } > + if (test_bit(DMA_FENCE_FLAG_USER_BITS, &fence->base.flags)) > + --fctx->notify_ref; > > dma_fence_put(&fence->base); > - return drop; > } > > static struct nouveau_fence * > @@ -93,7 +91,8 @@ nouveau_fence_context_kill(struct nouveau_fence_chan *fctx, int error) > if (error) > dma_fence_set_error(&fence->base, error); > > - if (nouveau_fence_signal(fence)) > + dma_fence_signal_locked(&fence->base); > + if (fctx->notify_ref == 0) > nvif_event_block(&fctx->event); > } > fctx->killed = 1; > @@ -131,7 +130,6 @@ static int > nouveau_fence_update(struct nouveau_channel *chan, struct nouveau_fence_chan *fctx) > { > struct nouveau_fence *fence; > - int drop = 0; > u32 seq = fctx->read(chan); > > while (!list_empty(&fctx->pending)) { > @@ -140,10 +138,10 @@ nouveau_fence_update(struct nouveau_channel *chan, struct nouveau_fence_chan *fc > if ((int)(seq - fence->base.seqno) < 0) > break; > > - drop |= nouveau_fence_signal(fence); > + dma_fence_signal_locked(&fence->base); > } > > - return drop; > + return fctx->notify_ref == 0 ? 1 : 0; > } > > static void > @@ -235,6 +233,19 @@ nouveau_fence_emit(struct nouveau_fence *fence) > &fctx->lock, fctx->context, ++fctx->sequence); > kref_get(&fctx->fence_ref); > > + fence->cb.func = nouveau_fence_cleanup_cb; > + /* Adding a callback runs into __dma_fence_enable_signaling(), which will > + * ultimately run into nouveau_fence_no_signaling(), where a WARN_ON > + * would fire because the refcount can be dropped there. > + * > + * Increment the refcount here temporarily to work around that. > + */ > + dma_fence_get(&fence->base); > + ret = dma_fence_add_callback(&fence->base, &fence->cb, nouveau_fence_cleanup_cb); > + dma_fence_put(&fence->base); > + if (ret) > + return ret; > + > ret = fctx->emit(fence); > if (!ret) { > dma_fence_get(&fence->base); > diff --git a/drivers/gpu/drm/nouveau/nouveau_fence.h b/drivers/gpu/drm/nouveau/nouveau_fence.h > index 8bc065acfe35..e6b2df7fdc42 100644 > --- a/drivers/gpu/drm/nouveau/nouveau_fence.h > +++ b/drivers/gpu/drm/nouveau/nouveau_fence.h > @@ -10,6 +10,7 @@ struct nouveau_bo; > > struct nouveau_fence { > struct dma_fence base; > + struct dma_fence_cb cb; > > struct list_head head; >

3 months

[PATCH v6 00/10] TEE subsystem for restricted dma-buf allocations

by Jens Wiklander

Hi, This patch set allocates the restricted DMA-bufs from a DMA-heap instantiated from the TEE subsystem. The TEE subsystem handles the DMA-buf allocations since it is the TEE (OP-TEE, AMD-TEE, TS-TEE, or perhaps a future QTEE) which sets up the restrictions for the memory used for the DMA-bufs. The DMA-heap uses a restricted memory pool provided by the backend TEE driver, allowing it to choose how to allocate the restricted physical memory. The allocated DMA-bufs must be imported with a new TEE_IOC_SHM_REGISTER_FD before they can be passed as arguments when requesting services from the secure world. Three use-cases (Secure Video Playback, Trusted UI, and Secure Video Recording) has been identified so far to serve as examples of what can be expected. The use-cases has predefined DMA-heap names, "restricted,secure-video", "restricted,trusted-ui", and "restricted,secure-video-record". The backend driver registers restricted memory pools for the use-cases it supports. Each use-case has it's own restricted memory pool since different use-cases requires isolation from different parts of the system. A restricted memory pool can be based on a static carveout instantiated while probing the TEE backend driver, or dynamically allocated from CMA and made restricted as needed by the TEE. This can be tested on a RockPi 4B+ with the following steps: repo init -u https://github.com/jenswi-linaro/manifest.git -m rockpi4.xml \ -b prototype/sdp-v6 repo sync -j8 cd build make toolchains -j$(nproc) make all -j$(nproc) # Copy ../out/rockpi4.img to an SD card and boot the RockPi from that # Connect a monitor to the RockPi # login and at the prompt: gst-launch-1.0 videotestsrc ! \ aesenc key=1f9423681beb9a79215820f6bda73d0f \ iv=e9aa8e834d8d70b7e0d254ff670dd718 serialize-iv=true ! \ aesdec key=1f9423681beb9a79215820f6bda73d0f ! \ kmssink The aesdec module has been hacked to use an OP-TEE TA to decrypt the stream into restricted DMA-bufs which are consumed by the kmssink. The primitive QEMU tests from previous patch set can be tested on RockPi in the same way with: xtest --sdp-basic The primitive test are tested on QEMU with the following steps: repo init -u https://github.com/jenswi-linaro/manifest.git -m qemu_v8.xml \ -b prototype/sdp-v6 repo sync -j8 cd build make toolchains -j$(nproc) make SPMC_AT_EL=1 all -j$(nproc) make SPMC_AT_EL=1 run-only # login and at the prompt: xtest --sdp-basic The SPMC_AT_EL=1 parameter configures the build with FF-A and an SPMC at S-EL1 inside OP-TEE. The parameter can be changed into SPMC_AT_EL=n to test without FF-A using the original SMC ABI instead. Please remember to do %rm -rf ../trusted-firmware-a/build/qemu for TF-A to be rebuilt properly using the new configuration. https://optee.readthedocs.io/en/latest/building/prerequisites.html list dependencies needed to build the above. The tests are pretty basic, mostly checking that a Trusted Application in the secure world can access and manipulate the memory. There are also some negative tests for out of bounds buffers etc. Thanks, Jens Changes since V5: * Removing "tee: add restricted memory allocation" and "tee: add TEE_IOC_RSTMEM_FD_INFO" * Adding "tee: implement restricted DMA-heap", "tee: new ioctl to a register tee_shm from a dmabuf file descriptor", "tee: add tee_shm_alloc_cma_phys_mem()", "optee: pass parent device to tee_device_alloc()", and "tee: tee_device_alloc(): copy dma_mask from parent device" * The two TEE driver OPs "rstmem_alloc()" and "rstmem_free()" are replaced with a struct tee_rstmem_pool abstraction. * Replaced the the TEE_IOC_RSTMEM_ALLOC user space API with the DMA-heap API Changes since V4: * Adding the patch "tee: add TEE_IOC_RSTMEM_FD_INFO" needed by the GStreamer demo * Removing the dummy CPU access and mmap functions from the dma_buf_ops * Fixing a compile error in "optee: FF-A: dynamic restricted memory allocation" reported by kernel test robot <lkp(a)intel.com> Changes since V3: * Make the use_case and flags field in struct tee_shm u32's instead of u16's * Add more description for TEE_IOC_RSTMEM_ALLOC in the header file * Import namespace DMA_BUF in module tee, reported by lkp(a)intel.com * Added a note in the commit message for "optee: account for direction while converting parameters" why it's needed * Factor out dynamic restricted memory allocation from "optee: support restricted memory allocation" into two new commits "optee: FF-A: dynamic restricted memory allocation" and "optee: smc abi: dynamic restricted memory allocation" * Guard CMA usage with #ifdef CONFIG_CMA, effectively disabling dynamic restricted memory allocate if CMA isn't configured Changes since the V2 RFC: * Based on v6.12 * Replaced the flags for SVP and Trusted UID memory with a u32 field with unique id for each use case * Added dynamic allocation of restricted memory pools * Added OP-TEE ABI both with and without FF-A for dynamic restricted memory * Added support for FF-A with FFA_LEND Changes since the V1 RFC: * Based on v6.11 * Complete rewrite, replacing the restricted heap with TEE_IOC_RSTMEM_ALLOC Changes since Olivier's post [2]: * Based on Yong Wu's post [1] where much of dma-buf handling is done in the generic restricted heap * Simplifications and cleanup * New commit message for "dma-buf: heaps: add Linaro restricted dmabuf heap support" * Replaced the word "secure" with "restricted" where applicable Etienne Carriere (1): tee: new ioctl to a register tee_shm from a dmabuf file descriptor Jens Wiklander (9): tee: tee_device_alloc(): copy dma_mask from parent device optee: pass parent device to tee_device_alloc() optee: account for direction while converting parameters optee: sync secure world ABI headers tee: implement restricted DMA-heap tee: add tee_shm_alloc_cma_phys_mem() optee: support restricted memory allocation optee: FF-A: dynamic restricted memory allocation optee: smc abi: dynamic restricted memory allocation drivers/tee/Makefile | 1 + drivers/tee/optee/Makefile | 1 + drivers/tee/optee/call.c | 10 +- drivers/tee/optee/core.c | 1 + drivers/tee/optee/ffa_abi.c | 194 +++++++++++- drivers/tee/optee/optee_ffa.h | 27 +- drivers/tee/optee/optee_msg.h | 65 ++++- drivers/tee/optee/optee_private.h | 55 +++- drivers/tee/optee/optee_smc.h | 71 ++++- drivers/tee/optee/rpc.c | 31 +- drivers/tee/optee/rstmem.c | 329 +++++++++++++++++++++ drivers/tee/optee/smc_abi.c | 190 ++++++++++-- drivers/tee/tee_core.c | 147 +++++++--- drivers/tee/tee_heap.c | 470 ++++++++++++++++++++++++++++++ drivers/tee/tee_private.h | 7 + drivers/tee/tee_shm.c | 199 ++++++++++++- include/linux/tee_core.h | 67 +++++ include/linux/tee_drv.h | 10 + include/uapi/linux/tee.h | 29 ++ 19 files changed, 1781 insertions(+), 123 deletions(-) create mode 100644 drivers/tee/optee/rstmem.c create mode 100644 drivers/tee/tee_heap.c base-commit: 7eb172143d5508b4da468ed59ee857c6e5e01da6 -- 2.43.0

3 months

[PATCH] drm/syncobj: Extend EXPORT_SYNC_FILE for timeline syncobjs

by Rob Clark

From: Rob Clark <robdclark(a)chromium.org> Add support for exporting a dma_fence fd for a specific point on a timeline. Signed-off-by: Rob Clark <robdclark(a)chromium.org> --- drivers/gpu/drm/drm_syncobj.c | 8 ++++++-- include/uapi/drm/drm.h | 2 ++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/drm_syncobj.c b/drivers/gpu/drm/drm_syncobj.c index 4f2ab8a7b50f..eb7a2dd2e261 100644 --- a/drivers/gpu/drm/drm_syncobj.c +++ b/drivers/gpu/drm/drm_syncobj.c @@ -762,7 +762,7 @@ static int drm_syncobj_import_sync_file_fence(struct drm_file *file_private, } static int drm_syncobj_export_sync_file(struct drm_file *file_private, - int handle, int *p_fd) + int handle, u64 point, int *p_fd) { int ret; struct dma_fence *fence; @@ -772,7 +772,7 @@ static int drm_syncobj_export_sync_file(struct drm_file *file_private, if (fd < 0) return fd; - ret = drm_syncobj_find_fence(file_private, handle, 0, 0, &fence); + ret = drm_syncobj_find_fence(file_private, handle, point, 0, &fence); if (ret) goto err_put_fd; @@ -882,8 +882,12 @@ drm_syncobj_handle_to_fd_ioctl(struct drm_device *dev, void *data, if (args->flags & DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE) return drm_syncobj_export_sync_file(file_private, args->handle, + args->point, &args->fd); + if (args->point) + return -EINVAL; + return drm_syncobj_handle_to_fd(file_private, args->handle, &args->fd); } diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 7fba37b94401..c71a8f4439f2 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -912,6 +912,8 @@ struct drm_syncobj_handle { __s32 fd; __u32 pad; + + __u64 point; }; struct drm_syncobj_transfer { -- 2.49.0

3 months

Re: [PATCH v6 09/10] optee: FF-A: dynamic restricted memory allocation

by Jens Wiklander

Hi Sumit, On Tue, Mar 25, 2025 at 8:42 AM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > On Wed, Mar 05, 2025 at 02:04:15PM +0100, Jens Wiklander wrote: > > Add support in the OP-TEE backend driver dynamic restricted memory > > allocation with FF-A. > > > > The restricted memory pools for dynamically allocated restrict memory > > are instantiated when requested by user-space. This instantiation can > > fail if OP-TEE doesn't support the requested use-case of restricted > > memory. > > > > Restricted memory pools based on a static carveout or dynamic allocation > > can coexist for different use-cases. We use only dynamic allocation with > > FF-A. > > > > Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org> > > --- > > drivers/tee/optee/Makefile | 1 + > > drivers/tee/optee/ffa_abi.c | 143 ++++++++++++- > > drivers/tee/optee/optee_private.h | 13 +- > > drivers/tee/optee/rstmem.c | 329 ++++++++++++++++++++++++++++++ > > 4 files changed, 483 insertions(+), 3 deletions(-) > > create mode 100644 drivers/tee/optee/rstmem.c > > > > diff --git a/drivers/tee/optee/Makefile b/drivers/tee/optee/Makefile > > index a6eff388d300..498969fb8e40 100644 > > --- a/drivers/tee/optee/Makefile > > +++ b/drivers/tee/optee/Makefile > > @@ -4,6 +4,7 @@ optee-objs += core.o > > optee-objs += call.o > > optee-objs += notif.o > > optee-objs += rpc.o > > +optee-objs += rstmem.o > > optee-objs += supp.o > > optee-objs += device.o > > optee-objs += smc_abi.o > > diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c > > index e4b08cd195f3..6a55114232ef 100644 > > --- a/drivers/tee/optee/ffa_abi.c > > +++ b/drivers/tee/optee/ffa_abi.c > > @@ -672,6 +672,123 @@ static int optee_ffa_do_call_with_arg(struct tee_context *ctx, > > return optee_ffa_yielding_call(ctx, &data, rpc_arg, system_thread); > > } > > > > +static int do_call_lend_rstmem(struct optee *optee, u64 cookie, u32 use_case) > > +{ > > + struct optee_shm_arg_entry *entry; > > + struct optee_msg_arg *msg_arg; > > + struct tee_shm *shm; > > + u_int offs; > > + int rc; > > + > > + msg_arg = optee_get_msg_arg(optee->ctx, 1, &entry, &shm, &offs); > > + if (IS_ERR(msg_arg)) > > + return PTR_ERR(msg_arg); > > + > > + msg_arg->cmd = OPTEE_MSG_CMD_ASSIGN_RSTMEM; > > + msg_arg->params[0].attr = OPTEE_MSG_ATTR_TYPE_VALUE_INPUT; > > + msg_arg->params[0].u.value.a = cookie; > > + msg_arg->params[0].u.value.b = use_case; > > + > > + rc = optee->ops->do_call_with_arg(optee->ctx, shm, offs, false); > > + if (rc) > > + goto out; > > + if (msg_arg->ret != TEEC_SUCCESS) { > > + rc = -EINVAL; > > + goto out; > > + } > > + > > +out: > > + optee_free_msg_arg(optee->ctx, entry, offs); > > + return rc; > > +} > > + > > +static int optee_ffa_lend_rstmem(struct optee *optee, struct tee_shm *rstmem, > > + u16 *end_points, unsigned int ep_count, > > + u32 use_case) > > +{ > > + struct ffa_device *ffa_dev = optee->ffa.ffa_dev; > > + const struct ffa_mem_ops *mem_ops = ffa_dev->ops->mem_ops; > > + const struct ffa_msg_ops *msg_ops = ffa_dev->ops->msg_ops; > > + struct ffa_send_direct_data data; > > + struct ffa_mem_region_attributes *mem_attr; > > + struct ffa_mem_ops_args args = { > > + .use_txbuf = true, > > + .tag = use_case, > > + }; > > + struct page *page; > > + struct scatterlist sgl; > > + unsigned int n; > > + int rc; > > + > > + mem_attr = kcalloc(ep_count, sizeof(*mem_attr), GFP_KERNEL); > > + for (n = 0; n < ep_count; n++) { > > + mem_attr[n].receiver = end_points[n]; > > + mem_attr[n].attrs = FFA_MEM_RW; > > + } > > + args.attrs = mem_attr; > > + args.nattrs = ep_count; > > + > > + page = phys_to_page(rstmem->paddr); > > + sg_init_table(&sgl, 1); > > + sg_set_page(&sgl, page, rstmem->size, 0); > > + > > + args.sg = &sgl; > > + rc = mem_ops->memory_lend(&args); > > + kfree(mem_attr); > > + if (rc) > > + return rc; > > + > > + rc = do_call_lend_rstmem(optee, args.g_handle, use_case); > > + if (rc) > > + goto err_reclaim; > > + > > + rc = optee_shm_add_ffa_handle(optee, rstmem, args.g_handle); > > + if (rc) > > + goto err_unreg; > > + > > + rstmem->sec_world_id = args.g_handle; > > + > > + return 0; > > + > > +err_unreg: > > + data = (struct ffa_send_direct_data){ > > + .data0 = OPTEE_FFA_RELEASE_RSTMEM, > > + .data1 = (u32)args.g_handle, > > + .data2 = (u32)(args.g_handle >> 32), > > + }; > > + msg_ops->sync_send_receive(ffa_dev, &data); > > +err_reclaim: > > + mem_ops->memory_reclaim(args.g_handle, 0); > > + return rc; > > +} > > + > > +static int optee_ffa_reclaim_rstmem(struct optee *optee, struct tee_shm *rstmem) > > +{ > > + struct ffa_device *ffa_dev = optee->ffa.ffa_dev; > > + const struct ffa_msg_ops *msg_ops = ffa_dev->ops->msg_ops; > > + const struct ffa_mem_ops *mem_ops = ffa_dev->ops->mem_ops; > > + u64 global_handle = rstmem->sec_world_id; > > + struct ffa_send_direct_data data = { > > + .data0 = OPTEE_FFA_RELEASE_RSTMEM, > > + .data1 = (u32)global_handle, > > + .data2 = (u32)(global_handle >> 32) > > + }; > > + int rc; > > + > > + optee_shm_rem_ffa_handle(optee, global_handle); > > + rstmem->sec_world_id = 0; > > + > > + rc = msg_ops->sync_send_receive(ffa_dev, &data); > > + if (rc) > > + pr_err("Release SHM id 0x%llx rc %d\n", global_handle, rc); > > + > > + rc = mem_ops->memory_reclaim(global_handle, 0); > > + if (rc) > > + pr_err("mem_reclaim: 0x%llx %d", global_handle, rc); > > + > > + return rc; > > +} > > + > > /* > > * 6. Driver initialization > > * > > @@ -833,6 +950,8 @@ static const struct optee_ops optee_ffa_ops = { > > .do_call_with_arg = optee_ffa_do_call_with_arg, > > .to_msg_param = optee_ffa_to_msg_param, > > .from_msg_param = optee_ffa_from_msg_param, > > + .lend_rstmem = optee_ffa_lend_rstmem, > > + .reclaim_rstmem = optee_ffa_reclaim_rstmem, > > }; > > > > static void optee_ffa_remove(struct ffa_device *ffa_dev) > > @@ -941,7 +1060,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) > > optee->pool, optee); > > if (IS_ERR(teedev)) { > > rc = PTR_ERR(teedev); > > - goto err_free_pool; > > + goto err_free_shm_pool; > > } > > optee->teedev = teedev; > > > > @@ -988,6 +1107,24 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) > > rc); > > } > > > > + if (IS_ENABLED(CONFIG_CMA) && !IS_MODULE(CONFIG_OPTEE) && > > The CMA dependency should be managed via Kconfig. Yes, I'll fix it. > > > + (sec_caps & OPTEE_FFA_SEC_CAP_RSTMEM)) { > > + enum tee_dma_heap_id id = TEE_DMA_HEAP_SECURE_VIDEO_PLAY; > > + struct tee_rstmem_pool *pool; > > + > > + pool = optee_rstmem_alloc_cma_pool(optee, id); > > + if (IS_ERR(pool)) { > > + rc = PTR_ERR(pool); > > + goto err_notif_uninit; > > + } > > + > > + rc = tee_device_register_dma_heap(optee->teedev, id, pool); > > + if (rc) { > > + pool->ops->destroy_pool(pool); > > + goto err_notif_uninit; > > + } > > + } > > + > > rc = optee_enumerate_devices(PTA_CMD_GET_DEVICES); > > if (rc) > > goto err_unregister_devices; > > @@ -1001,6 +1138,8 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) > > > > err_unregister_devices: > > optee_unregister_devices(); > > + tee_device_unregister_all_dma_heaps(optee->teedev); > > +err_notif_uninit: > > if (optee->ffa.bottom_half_value != U32_MAX) > > notif_ops->notify_relinquish(ffa_dev, > > optee->ffa.bottom_half_value); > > @@ -1018,7 +1157,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev) > > tee_device_unregister(optee->supp_teedev); > > err_unreg_teedev: > > tee_device_unregister(optee->teedev); > > -err_free_pool: > > +err_free_shm_pool: > > tee_shm_pool_free(pool); > > err_free_optee: > > kfree(optee); > > diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h > > index 20eda508dbac..faab31ad7c52 100644 > > --- a/drivers/tee/optee/optee_private.h > > +++ b/drivers/tee/optee/optee_private.h > > @@ -174,9 +174,14 @@ struct optee; > > * @do_call_with_arg: enters OP-TEE in secure world > > * @to_msg_param: converts from struct tee_param to OPTEE_MSG parameters > > * @from_msg_param: converts from OPTEE_MSG parameters to struct tee_param > > + * @lend_rstmem: lends physically contiguous memory as restricted > > + * memory, inaccessible by the kernel > > + * @reclaim_rstmem: reclaims restricted memory previously lent with > > + * @lend_rstmem() and makes it accessible by the > > + * kernel again > > * > > * These OPs are only supposed to be used internally in the OP-TEE driver > > - * as a way of abstracting the different methogs of entering OP-TEE in > > + * as a way of abstracting the different methods of entering OP-TEE in > > * secure world. > > */ > > struct optee_ops { > > @@ -191,6 +196,10 @@ struct optee_ops { > > size_t num_params, > > const struct optee_msg_param *msg_params, > > bool update_out); > > + int (*lend_rstmem)(struct optee *optee, struct tee_shm *rstmem, > > + u16 *end_points, unsigned int ep_count, > > + u32 use_case); > > + int (*reclaim_rstmem)(struct optee *optee, struct tee_shm *rstmem); > > }; > > > > /** > > @@ -285,6 +294,8 @@ u32 optee_supp_thrd_req(struct tee_context *ctx, u32 func, size_t num_params, > > void optee_supp_init(struct optee_supp *supp); > > void optee_supp_uninit(struct optee_supp *supp); > > void optee_supp_release(struct optee_supp *supp); > > +struct tee_rstmem_pool *optee_rstmem_alloc_cma_pool(struct optee *optee, > > + enum tee_dma_heap_id id); > > > > int optee_supp_recv(struct tee_context *ctx, u32 *func, u32 *num_params, > > struct tee_param *param); > > diff --git a/drivers/tee/optee/rstmem.c b/drivers/tee/optee/rstmem.c > > new file mode 100644 > > index 000000000000..ea27769934d4 > > --- /dev/null > > +++ b/drivers/tee/optee/rstmem.c > > @@ -0,0 +1,329 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (c) 2025, Linaro Limited > > + */ > > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > + > > +#include <linux/errno.h> > > +#include <linux/genalloc.h> > > +#include <linux/slab.h> > > +#include <linux/string.h> > > +#include <linux/tee_core.h> > > +#include <linux/types.h> > > +#include "optee_private.h" > > + > > +struct optee_rstmem_cma_pool { > > + struct tee_rstmem_pool pool; > > + struct gen_pool *gen_pool; > > + struct optee *optee; > > + size_t page_count; > > + u16 *end_points; > > + u_int end_point_count; > > + u_int align; > > + refcount_t refcount; > > + u32 use_case; > > + struct tee_shm *rstmem; > > + /* Protects when initializing and tearing down this struct */ > > + struct mutex mutex; > > +}; > > + > > +static struct optee_rstmem_cma_pool * > > +to_rstmem_cma_pool(struct tee_rstmem_pool *pool) > > +{ > > + return container_of(pool, struct optee_rstmem_cma_pool, pool); > > +} > > + > > +static int init_cma_rstmem(struct optee_rstmem_cma_pool *rp) > > +{ > > + int rc; > > + > > + rp->rstmem = tee_shm_alloc_cma_phys_mem(rp->optee->ctx, rp->page_count, > > + rp->align); > > + if (IS_ERR(rp->rstmem)) { > > + rc = PTR_ERR(rp->rstmem); > > + goto err_null_rstmem; > > + } > > + > > + /* > > + * TODO unmap the memory range since the physical memory will > > + * become inaccesible after the lend_rstmem() call. > > + */ > > What's your plan for this TODO? I think we need a CMA allocator here > which can allocate un-mapped memory such that any cache speculation > won't lead to CPU hangs once the memory restriction comes into picture. What happens is platform-specific. For some platforms, it might be enough to avoid explicit access. Yes, a CMA allocator with unmapped memory or where memory can be unmapped is one option. > > > + rc = rp->optee->ops->lend_rstmem(rp->optee, rp->rstmem, rp->end_points, > > + rp->end_point_count, rp->use_case); > > + if (rc) > > + goto err_put_shm; > > + rp->rstmem->flags |= TEE_SHM_DYNAMIC; > > + > > + rp->gen_pool = gen_pool_create(PAGE_SHIFT, -1); > > + if (!rp->gen_pool) { > > + rc = -ENOMEM; > > + goto err_reclaim; > > + } > > + > > + rc = gen_pool_add(rp->gen_pool, rp->rstmem->paddr, > > + rp->rstmem->size, -1); > > + if (rc) > > + goto err_free_pool; > > + > > + refcount_set(&rp->refcount, 1); > > + return 0; > > + > > +err_free_pool: > > + gen_pool_destroy(rp->gen_pool); > > + rp->gen_pool = NULL; > > +err_reclaim: > > + rp->optee->ops->reclaim_rstmem(rp->optee, rp->rstmem); > > +err_put_shm: > > + tee_shm_put(rp->rstmem); > > +err_null_rstmem: > > + rp->rstmem = NULL; > > + return rc; > > +} > > + > > +static int get_cma_rstmem(struct optee_rstmem_cma_pool *rp) > > +{ > > + int rc = 0; > > + > > + if (!refcount_inc_not_zero(&rp->refcount)) { > > + mutex_lock(&rp->mutex); > > + if (rp->gen_pool) { > > + /* > > + * Another thread has already initialized the pool > > + * before us, or the pool was just about to be torn > > + * down. Either way we only need to increase the > > + * refcount and we're done. > > + */ > > + refcount_inc(&rp->refcount); > > + } else { > > + rc = init_cma_rstmem(rp); > > + } > > + mutex_unlock(&rp->mutex); > > + } > > + > > + return rc; > > +} > > + > > +static void release_cma_rstmem(struct optee_rstmem_cma_pool *rp) > > +{ > > + gen_pool_destroy(rp->gen_pool); > > + rp->gen_pool = NULL; > > + > > + rp->optee->ops->reclaim_rstmem(rp->optee, rp->rstmem); > > + rp->rstmem->flags &= ~TEE_SHM_DYNAMIC; > > + > > + WARN(refcount_read(&rp->rstmem->refcount) != 1, "Unexpected refcount"); > > + tee_shm_put(rp->rstmem); > > + rp->rstmem = NULL; > > +} > > + > > +static void put_cma_rstmem(struct optee_rstmem_cma_pool *rp) > > +{ > > + if (refcount_dec_and_test(&rp->refcount)) { > > + mutex_lock(&rp->mutex); > > + if (rp->gen_pool) > > + release_cma_rstmem(rp); > > + mutex_unlock(&rp->mutex); > > + } > > +} > > + > > +static int rstmem_pool_op_cma_alloc(struct tee_rstmem_pool *pool, > > + struct sg_table *sgt, size_t size, > > + size_t *offs) > > +{ > > + struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool); > > + size_t sz = ALIGN(size, PAGE_SIZE); > > + phys_addr_t pa; > > + int rc; > > + > > + rc = get_cma_rstmem(rp); > > + if (rc) > > + return rc; > > + > > + pa = gen_pool_alloc(rp->gen_pool, sz); > > + if (!pa) { > > + rc = -ENOMEM; > > + goto err_put; > > + } > > + > > + rc = sg_alloc_table(sgt, 1, GFP_KERNEL); > > + if (rc) > > + goto err_free; > > + > > + sg_set_page(sgt->sgl, phys_to_page(pa), size, 0); > > + *offs = pa - rp->rstmem->paddr; > > + > > + return 0; > > +err_free: > > + gen_pool_free(rp->gen_pool, pa, size); > > +err_put: > > + put_cma_rstmem(rp); > > + > > + return rc; > > +} > > + > > +static void rstmem_pool_op_cma_free(struct tee_rstmem_pool *pool, > > + struct sg_table *sgt) > > +{ > > + struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool); > > + struct scatterlist *sg; > > + int i; > > + > > + for_each_sgtable_sg(sgt, sg, i) > > + gen_pool_free(rp->gen_pool, sg_phys(sg), sg->length); > > + sg_free_table(sgt); > > + put_cma_rstmem(rp); > > +} > > + > > +static int rstmem_pool_op_cma_update_shm(struct tee_rstmem_pool *pool, > > + struct sg_table *sgt, size_t offs, > > + struct tee_shm *shm, > > + struct tee_shm **parent_shm) > > +{ > > + struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool); > > + > > + *parent_shm = rp->rstmem; > > + > > + return 0; > > +} > > + > > +static void pool_op_cma_destroy_pool(struct tee_rstmem_pool *pool) > > +{ > > + struct optee_rstmem_cma_pool *rp = to_rstmem_cma_pool(pool); > > + > > + mutex_destroy(&rp->mutex); > > + kfree(rp); > > +} > > + > > +static struct tee_rstmem_pool_ops rstmem_pool_ops_cma = { > > + .alloc = rstmem_pool_op_cma_alloc, > > + .free = rstmem_pool_op_cma_free, > > + .update_shm = rstmem_pool_op_cma_update_shm, > > + .destroy_pool = pool_op_cma_destroy_pool, > > +}; > > + > > +static int get_rstmem_config(struct optee *optee, u32 use_case, > > + size_t *min_size, u_int *min_align, > > + u16 *end_points, u_int *ep_count) > > I guess this end points terminology is specific to FF-A ABI. Is there > any relevance for this in the common APIs? Yes, endpoints are specific to FF-A ABI. The list of end-points must be presented to FFA_MEM_LEND. We're relying on the secure world to know which endpoints are needed for a specific use case. Cheers, Jens > > -Sumit > > > +{ > > + struct tee_param params[2] = { > > + [0] = { > > + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INOUT, > > + .u.value.a = use_case, > > + }, > > + [1] = { > > + .attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT, > > + }, > > + }; > > + struct optee_shm_arg_entry *entry; > > + struct tee_shm *shm_param = NULL; > > + struct optee_msg_arg *msg_arg; > > + struct tee_shm *shm; > > + u_int offs; > > + int rc; > > + > > + if (end_points && *ep_count) { > > + params[1].u.memref.size = *ep_count * sizeof(*end_points); > > + shm_param = tee_shm_alloc_priv_buf(optee->ctx, > > + params[1].u.memref.size); > > + if (IS_ERR(shm_param)) > > + return PTR_ERR(shm_param); > > + params[1].u.memref.shm = shm_param; > > + } > > + > > + msg_arg = optee_get_msg_arg(optee->ctx, ARRAY_SIZE(params), &entry, > > + &shm, &offs); > > + if (IS_ERR(msg_arg)) { > > + rc = PTR_ERR(msg_arg); > > + goto out_free_shm; > > + } > > + msg_arg->cmd = OPTEE_MSG_CMD_GET_RSTMEM_CONFIG; > > + > > + rc = optee->ops->to_msg_param(optee, msg_arg->params, > > + ARRAY_SIZE(params), params, > > + false /*!update_out*/); > > + if (rc) > > + goto out_free_msg; > > + > > + rc = optee->ops->do_call_with_arg(optee->ctx, shm, offs, false); > > + if (rc) > > + goto out_free_msg; > > + if (msg_arg->ret && msg_arg->ret != TEEC_ERROR_SHORT_BUFFER) { > > + rc = -EINVAL; > > + goto out_free_msg; > > + } > > + > > + rc = optee->ops->from_msg_param(optee, params, ARRAY_SIZE(params), > > + msg_arg->params, true /*update_out*/); > > + if (rc) > > + goto out_free_msg; > > + > > + if (!msg_arg->ret && end_points && > > + *ep_count < params[1].u.memref.size / sizeof(u16)) { > > + rc = -EINVAL; > > + goto out_free_msg; > > + } > > + > > + *min_size = params[0].u.value.a; > > + *min_align = params[0].u.value.b; > > + *ep_count = params[1].u.memref.size / sizeof(u16); > > + > > + if (msg_arg->ret == TEEC_ERROR_SHORT_BUFFER) { > > + rc = -ENOSPC; > > + goto out_free_msg; > > + } > > + > > + if (end_points) > > + memcpy(end_points, tee_shm_get_va(shm_param, 0), > > + params[1].u.memref.size); > > + > > +out_free_msg: > > + optee_free_msg_arg(optee->ctx, entry, offs); > > +out_free_shm: > > + if (shm_param) > > + tee_shm_free(shm_param); > > + return rc; > > +} > > + > > +struct tee_rstmem_pool *optee_rstmem_alloc_cma_pool(struct optee *optee, > > + enum tee_dma_heap_id id) > > +{ > > + struct optee_rstmem_cma_pool *rp; > > + u32 use_case = id; > > + size_t min_size; > > + int rc; > > + > > + rp = kzalloc(sizeof(*rp), GFP_KERNEL); > > + if (!rp) > > + return ERR_PTR(-ENOMEM); > > + rp->use_case = use_case; > > + > > + rc = get_rstmem_config(optee, use_case, &min_size, &rp->align, NULL, > > + &rp->end_point_count); > > + if (rc) { > > + if (rc != -ENOSPC) > > + goto err; > > + rp->end_points = kcalloc(rp->end_point_count, > > + sizeof(*rp->end_points), GFP_KERNEL); > > + if (!rp->end_points) { > > + rc = -ENOMEM; > > + goto err; > > + } > > + rc = get_rstmem_config(optee, use_case, &min_size, &rp->align, > > + rp->end_points, &rp->end_point_count); > > + if (rc) > > + goto err_kfree_eps; > > + } > > + > > + rp->pool.ops = &rstmem_pool_ops_cma; > > + rp->optee = optee; > > + rp->page_count = min_size / PAGE_SIZE; > > + mutex_init(&rp->mutex); > > + > > + return &rp->pool; > > + > > +err_kfree_eps: > > + kfree(rp->end_points); > > +err: > > + kfree(rp); > > + return ERR_PTR(rc); > > +} > > -- > > 2.43.0 > >

3 months

Re: [PATCH v6 04/10] optee: sync secure world ABI headers

by Jens Wiklander

Hi Sumit, On Tue, Mar 25, 2025 at 7:20 AM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > Hi Jens, > > It has taken a bit of time for me to review this patch-set as I am > settling in my new role. > > On Wed, Mar 05, 2025 at 02:04:10PM +0100, Jens Wiklander wrote: > > Update the header files describing the secure world ABI, both with and > > without FF-A. The ABI is extended to deal with restricted memory, but as > > usual backward compatible. > > > > Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org> > > --- > > drivers/tee/optee/optee_ffa.h | 27 ++++++++++--- > > drivers/tee/optee/optee_msg.h | 65 ++++++++++++++++++++++++++++++-- > > drivers/tee/optee/optee_smc.h | 71 ++++++++++++++++++++++++++++++++++- > > 3 files changed, 154 insertions(+), 9 deletions(-) > > > > diff --git a/drivers/tee/optee/optee_ffa.h b/drivers/tee/optee/optee_ffa.h > > index 257735ae5b56..7bd037200343 100644 > > --- a/drivers/tee/optee/optee_ffa.h > > +++ b/drivers/tee/optee/optee_ffa.h > > @@ -81,7 +81,7 @@ > > * as the second MSG arg struct for > > * OPTEE_FFA_YIELDING_CALL_WITH_ARG. > > * Bit[31:8]: Reserved (MBZ) > > - * w5: Bitfield of secure world capabilities OPTEE_FFA_SEC_CAP_* below, > > + * w5: Bitfield of OP-TEE capabilities OPTEE_FFA_SEC_CAP_* > > * w6: The maximum secure world notification number > > * w7: Not used (MBZ) > > */ > > @@ -94,6 +94,8 @@ > > #define OPTEE_FFA_SEC_CAP_ASYNC_NOTIF BIT(1) > > /* OP-TEE supports probing for RPMB device if needed */ > > #define OPTEE_FFA_SEC_CAP_RPMB_PROBE BIT(2) > > +/* OP-TEE supports Restricted Memory for secure data path */ > > +#define OPTEE_FFA_SEC_CAP_RSTMEM BIT(3) > > > > #define OPTEE_FFA_EXCHANGE_CAPABILITIES OPTEE_FFA_BLOCKING_CALL(2) > > > > @@ -108,7 +110,7 @@ > > * > > * Return register usage: > > * w3: Error code, 0 on success > > - * w4-w7: Note used (MBZ) > > + * w4-w7: Not used (MBZ) > > */ > > #define OPTEE_FFA_UNREGISTER_SHM OPTEE_FFA_BLOCKING_CALL(3) > > > > @@ -119,16 +121,31 @@ > > * Call register usage: > > * w3: Service ID, OPTEE_FFA_ENABLE_ASYNC_NOTIF > > * w4: Notification value to request bottom half processing, should be > > - * less than OPTEE_FFA_MAX_ASYNC_NOTIF_VALUE. > > + * less than OPTEE_FFA_MAX_ASYNC_NOTIF_VALUE > > * w5-w7: Not used (MBZ) > > * > > * Return register usage: > > * w3: Error code, 0 on success > > - * w4-w7: Note used (MBZ) > > + * w4-w7: Not used (MBZ) > > */ > > #define OPTEE_FFA_ENABLE_ASYNC_NOTIF OPTEE_FFA_BLOCKING_CALL(5) > > > > -#define OPTEE_FFA_MAX_ASYNC_NOTIF_VALUE 64 > > +#define OPTEE_FFA_MAX_ASYNC_NOTIF_VALUE 64 > > + > > +/* > > + * Release Restricted memory > > + * > > + * Call register usage: > > + * w3: Service ID, OPTEE_FFA_RECLAIM_RSTMEM > > + * w4: Shared memory handle, lower bits > > + * w5: Shared memory handle, higher bits > > + * w6-w7: Not used (MBZ) > > + * > > + * Return register usage: > > + * w3: Error code, 0 on success > > + * w4-w7: Note used (MBZ) > > + */ > > +#define OPTEE_FFA_RELEASE_RSTMEM OPTEE_FFA_BLOCKING_CALL(8) > > > > /* > > * Call with struct optee_msg_arg as argument in the supplied shared memory > > diff --git a/drivers/tee/optee/optee_msg.h b/drivers/tee/optee/optee_msg.h > > index e8840a82b983..1b558526e7d9 100644 > > --- a/drivers/tee/optee/optee_msg.h > > +++ b/drivers/tee/optee/optee_msg.h > > @@ -133,13 +133,13 @@ struct optee_msg_param_rmem { > > }; > > > > /** > > - * struct optee_msg_param_fmem - ffa memory reference parameter > > + * struct optee_msg_param_fmem - FF-A memory reference parameter > > * @offs_lower: Lower bits of offset into shared memory reference > > * @offs_upper: Upper bits of offset into shared memory reference > > * @internal_offs: Internal offset into the first page of shared memory > > * reference > > * @size: Size of the buffer > > - * @global_id: Global identifier of Shared memory > > + * @global_id: Global identifier of the shared memory > > */ > > struct optee_msg_param_fmem { > > u32 offs_low; > > @@ -165,7 +165,7 @@ struct optee_msg_param_value { > > * @attr: attributes > > * @tmem: parameter by temporary memory reference > > * @rmem: parameter by registered memory reference > > - * @fmem: parameter by ffa registered memory reference > > + * @fmem: parameter by FF-A registered memory reference > > * @value: parameter by opaque value > > * @octets: parameter by octet string > > * > > @@ -296,6 +296,18 @@ struct optee_msg_arg { > > */ > > #define OPTEE_MSG_FUNCID_GET_OS_REVISION 0x0001 > > > > +/* > > + * Values used in OPTEE_MSG_CMD_LEND_RSTMEM below > > + * OPTEE_MSG_RSTMEM_RESERVED Reserved > > + * OPTEE_MSG_RSTMEM_SECURE_VIDEO_PLAY Secure Video Playback > > + * OPTEE_MSG_RSTMEM_TRUSTED_UI Trused UI > > + * OPTEE_MSG_RSTMEM_SECURE_VIDEO_RECORD Secure Video Recording > > + */ > > +#define OPTEE_MSG_RSTMEM_RESERVED 0 > > +#define OPTEE_MSG_RSTMEM_SECURE_VIDEO_PLAY 1 > > +#define OPTEE_MSG_RSTMEM_TRUSTED_UI 2 > > +#define OPTEE_MSG_RSTMEM_SECURE_VIDEO_RECORD 3 > > + > > /* > > * Do a secure call with struct optee_msg_arg as argument > > * The OPTEE_MSG_CMD_* below defines what goes in struct optee_msg_arg::cmd > > @@ -337,6 +349,49 @@ struct optee_msg_arg { > > * OPTEE_MSG_CMD_STOP_ASYNC_NOTIF informs secure world that from now is > > * normal world unable to process asynchronous notifications. Typically > > * used when the driver is shut down. > > + * > > + * OPTEE_MSG_CMD_LEND_RSTMEM lends restricted memory. The passed normal > > + * physical memory is restricted from normal world access. The memory > > + * should be unmapped prior to this call since it becomes inaccessible > > + * during the request. > > + * Parameters are passed as: > > + * [in] param[0].attr OPTEE_MSG_ATTR_TYPE_VALUE_INPUT > > + * [in] param[0].u.value.a OPTEE_MSG_RSTMEM_* defined above > > + * [in] param[1].attr OPTEE_MSG_ATTR_TYPE_TMEM_INPUT > > + * [in] param[1].u.tmem.buf_ptr physical address > > + * [in] param[1].u.tmem.size size > > + * [in] param[1].u.tmem.shm_ref holds restricted memory reference > > + * > > + * OPTEE_MSG_CMD_RECLAIM_RSTMEM reclaims a previously lent restricted > > + * memory reference. The physical memory is accessible by the normal world > > + * after this function has return and can be mapped again. The information > > + * is passed as: > > + * [in] param[0].attr OPTEE_MSG_ATTR_TYPE_VALUE_INPUT > > + * [in] param[0].u.value.a holds restricted memory cookie > > + * > > + * OPTEE_MSG_CMD_GET_RSTMEM_CONFIG get configuration for a specific > > + * restricted memory use case. Parameters are passed as: > > + * [in] param[0].attr OPTEE_MSG_ATTR_TYPE_VALUE_INOUT > > + * [in] param[0].value.a OPTEE_MSG_RSTMEM_* > > + * [in] param[1].attr OPTEE_MSG_ATTR_TYPE_{R,F}MEM_OUTPUT > > + * [in] param[1].u.{r,f}mem Buffer or NULL > > + * [in] param[1].u.{r,f}mem.size Provided size of buffer or 0 for query > > + * output for the restricted use case: > > + * [out] param[0].value.a Minimal size of SDP memory > > + * [out] param[0].value.b Required alignment of size and start of > > + * restricted memory > > + * [out] param[1].{r,f}mem.size Size of output data > > + * [out] param[1].{r,f}mem If non-NULL, contains an array of > > + * uint16_t holding endpoints that > > + * must be included when lending > > + * memory for this use case > > + * > > + * OPTEE_MSG_CMD_ASSIGN_RSTMEM assigns use-case to restricted memory > > + * previously lent using the FFA_LEND framework ABI. Parameters are passed > > + * as: > > + * [in] param[0].attr OPTEE_MSG_ATTR_TYPE_VALUE_INPUT > > + * [in] param[0].u.value.a holds restricted memory cookie > > + * [in] param[0].u.value.b OPTEE_MSG_RSTMEM_* defined above > > */ > > #define OPTEE_MSG_CMD_OPEN_SESSION 0 > > #define OPTEE_MSG_CMD_INVOKE_COMMAND 1 > > @@ -346,6 +401,10 @@ struct optee_msg_arg { > > #define OPTEE_MSG_CMD_UNREGISTER_SHM 5 > > #define OPTEE_MSG_CMD_DO_BOTTOM_HALF 6 > > #define OPTEE_MSG_CMD_STOP_ASYNC_NOTIF 7 > > +#define OPTEE_MSG_CMD_LEND_RSTMEM 8 > > +#define OPTEE_MSG_CMD_RECLAIM_RSTMEM 9 > > +#define OPTEE_MSG_CMD_GET_RSTMEM_CONFIG 10 > > +#define OPTEE_MSG_CMD_ASSIGN_RSTMEM 11 > > #define OPTEE_MSG_FUNCID_CALL_WITH_ARG 0x0004 > > > > #endif /* _OPTEE_MSG_H */ > > diff --git a/drivers/tee/optee/optee_smc.h b/drivers/tee/optee/optee_smc.h > > index 879426300821..abc379ce190c 100644 > > --- a/drivers/tee/optee/optee_smc.h > > +++ b/drivers/tee/optee/optee_smc.h > > @@ -264,7 +264,6 @@ struct optee_smc_get_shm_config_result { > > #define OPTEE_SMC_SEC_CAP_HAVE_RESERVED_SHM BIT(0) > > /* Secure world can communicate via previously unregistered shared memory */ > > #define OPTEE_SMC_SEC_CAP_UNREGISTERED_SHM BIT(1) > > - > > /* > > * Secure world supports commands "register/unregister shared memory", > > * secure world accepts command buffers located in any parts of non-secure RAM > > @@ -280,6 +279,10 @@ struct optee_smc_get_shm_config_result { > > #define OPTEE_SMC_SEC_CAP_RPC_ARG BIT(6) > > /* Secure world supports probing for RPMB device if needed */ > > #define OPTEE_SMC_SEC_CAP_RPMB_PROBE BIT(7) > > +/* Secure world supports Secure Data Path */ > > +#define OPTEE_SMC_SEC_CAP_SDP BIT(8) > > +/* Secure world supports dynamic restricted memory */ > > +#define OPTEE_SMC_SEC_CAP_DYNAMIC_RSTMEM BIT(9) > > > > #define OPTEE_SMC_FUNCID_EXCHANGE_CAPABILITIES 9 > > #define OPTEE_SMC_EXCHANGE_CAPABILITIES \ > > @@ -451,6 +454,72 @@ struct optee_smc_disable_shm_cache_result { > > > > /* See OPTEE_SMC_CALL_WITH_REGD_ARG above */ > > #define OPTEE_SMC_FUNCID_CALL_WITH_REGD_ARG 19 > > +/* > > + * Get Secure Data Path memory config > > + * > > + * Returns the Secure Data Path memory config. > > + * > > + * Call register usage: > > + * a0 SMC Function ID, OPTEE_SMC_GET_SDP_CONFIG > > + * a2-6 Not used, must be zero > > + * a7 Hypervisor Client ID register > > + * > > + * Have config return register usage: > > + * a0 OPTEE_SMC_RETURN_OK > > + * a1 Physical address of start of SDP memory > > + * a2 Size of SDP memory > > + * a3 Not used > > + * a4-7 Preserved > > + * > > + * Not available register usage: > > + * a0 OPTEE_SMC_RETURN_ENOTAVAIL > > + * a1-3 Not used > > + * a4-7 Preserved > > + */ > > +#define OPTEE_SMC_FUNCID_GET_SDP_CONFIG 20 > > Let's have a consistent ABI naming here. I think RSTMEM is more generic > rather than SDP, so let's use same naming convention as we use for FF-A > ABI. Yes, I'll fix it. Cheers, Jens > > -Sumit > > > +#define OPTEE_SMC_GET_SDP_CONFIG \ > > + OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_GET_SDP_CONFIG) > > + > > +struct optee_smc_get_sdp_config_result { > > + unsigned long status; > > + unsigned long start; > > + unsigned long size; > > + unsigned long flags; > > +}; > > + > > +/* > > + * Get Secure Data Path dynamic memory config > > + * > > + * Returns the Secure Data Path dynamic memory config. > > + * > > + * Call register usage: > > + * a0 SMC Function ID, OPTEE_SMC_GET_DYN_SHM_CONFIG > > + * a2-6 Not used, must be zero > > + * a7 Hypervisor Client ID register > > + * > > + * Have config return register usage: > > + * a0 OPTEE_SMC_RETURN_OK > > + * a1 Minamal size of SDP memory > > + * a2 Required alignment of size and start of registered SDP memory > > + * a3 Not used > > + * a4-7 Preserved > > + * > > + * Not available register usage: > > + * a0 OPTEE_SMC_RETURN_ENOTAVAIL > > + * a1-3 Not used > > + * a4-7 Preserved > > + */ > > + > > +#define OPTEE_SMC_FUNCID_GET_DYN_SDP_CONFIG 21 > > +#define OPTEE_SMC_GET_DYN_SDP_CONFIG \ > > + OPTEE_SMC_FAST_CALL_VAL(OPTEE_SMC_FUNCID_GET_DYN_SDP_CONFIG) > > + > > +struct optee_smc_get_dyn_sdp_config_result { > > + unsigned long status; > > + unsigned long size; > > + unsigned long align; > > + unsigned long flags; > > +}; > > > > /* > > * Resume from RPC (for example after processing a foreign interrupt) > > -- > > 2.43.0 > >

3 months

Re: [PATCH] udmabuf: fix a buf size overflow issue during udmabuf creation

by Christian König

Am 26.03.25 um 03:59 schrieb Kasireddy, Vivek: > Hi Christian, > >> Subject: Re: [PATCH] udmabuf: fix a buf size overflow issue during udmabuf >> creation >> >> Am 25.03.25 um 07:23 schrieb Kasireddy, Vivek: >>> Hi Christian, >>> >>>> Am 21.03.25 um 17:41 schrieb Xiaogang.Chen: >>>>> From: Xiaogang Chen <xiaogang.chen(a)amd.com> >>>>> >>>>> by casting size_limit_mb to u64 when calculate pglimit. >>>>> >>>>> Signed-off-by: Xiaogang Chen<Xiaogang.Chen(a)amd.com> >>>> Reviewed-by: Christian König <christian.koenig(a)amd.com> >>>> >>>> If nobody objects I'm going to push that to drm-misc-fixes. >>> No objection but I wish the author would have added more details in the >> commit >>> message particularly the value they have used to trigger the overflow. I >> guess >>> Xiaogang can still comment here and briefly describe the exact use- >> case/test-case >>> they are running where they encountered this issue. >> Isn't that obvious? At least it was for me. >> >> As soon as you have a value larger than 4095 the 32bit multiplication >> overflows, resulting in incorrectly limiting the buffer size. > Right, that part makes sense. I was mostly curious about why or how they > were using such a large buffer (use-case details). Well I suggested that we use udmabuf to implement shareable dma-bufs which can be allocated from a specific NUMA node and are also accounted in memcg. But to be honest I have absolutely no idea what's the use case for a buffer larger than 4GiB. Regards, Christian. > > > Thanks, > Vivek > >> Regards, >> Christian. >> >>> Thanks, >>> Vivek >>> >>>> Regards, >>>> Christian. >>>> >>>>> --- >>>>> drivers/dma-buf/udmabuf.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/drivers/dma-buf/udmabuf.c b/drivers/dma-buf/udmabuf.c >>>>> index 8ce1f074c2d3..e99e3a65a470 100644 >>>>> --- a/drivers/dma-buf/udmabuf.c >>>>> +++ b/drivers/dma-buf/udmabuf.c >>>>> @@ -398,7 +398,7 @@ static long udmabuf_create(struct miscdevice >>>> *device, >>>>> if (!ubuf) >>>>> return -ENOMEM; >>>>> >>>>> - pglimit = (size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; >>>>> + pglimit = ((u64)size_limit_mb * 1024 * 1024) >> PAGE_SHIFT; >>>>> for (i = 0; i < head->count; i++) { >>>>> pgoff_t subpgcnt; >>>>>

3 months

Re: [PATCH v6 08/10] optee: support restricted memory allocation

by Jens Wiklander

On Tue, Mar 25, 2025 at 8:07 AM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > On Wed, Mar 05, 2025 at 02:04:14PM +0100, Jens Wiklander wrote: > > Add support in the OP-TEE backend driver for restricted memory > > allocation. The support is limited to only the SMC ABI and for secure > > video buffers. > > > > OP-TEE is probed for the range of restricted physical memory and a > > memory pool allocator is initialized if OP-TEE have support for such > > memory. > > > > Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org> > > --- > > drivers/tee/optee/core.c | 1 + > > drivers/tee/optee/smc_abi.c | 44 +++++++++++++++++++++++++++++++++++-- > > 2 files changed, 43 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c > > index c75fddc83576..c7fd8040480e 100644 > > --- a/drivers/tee/optee/core.c > > +++ b/drivers/tee/optee/core.c > > @@ -181,6 +181,7 @@ void optee_remove_common(struct optee *optee) > > tee_device_unregister(optee->supp_teedev); > > tee_device_unregister(optee->teedev); > > > > + tee_device_unregister_all_dma_heaps(optee->teedev); > > tee_shm_pool_free(optee->pool); > > optee_supp_uninit(&optee->supp); > > mutex_destroy(&optee->call_queue.mutex); > > diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c > > index cfdae266548b..a14ff0b7d3b3 100644 > > --- a/drivers/tee/optee/smc_abi.c > > +++ b/drivers/tee/optee/smc_abi.c > > @@ -1620,6 +1620,41 @@ static inline int optee_load_fw(struct platform_device *pdev, > > } > > #endif > > > > +static int optee_sdp_pool_init(struct optee *optee) > > +{ > > + enum tee_dma_heap_id heap_id = TEE_DMA_HEAP_SECURE_VIDEO_PLAY; > > + struct tee_rstmem_pool *pool; > > + int rc; > > + > > + if (optee->smc.sec_caps & OPTEE_SMC_SEC_CAP_SDP) { > > Is this SDP capability an ABI yet since we haven't supported it in > upstream kernel? If no then can we rename it as > OPTEE_SMC_SEC_CAP_RSTMEM? No problem. We can rename it. > > > + union { > > + struct arm_smccc_res smccc; > > + struct optee_smc_get_sdp_config_result result; > > + } res; > > + > > + optee->smc.invoke_fn(OPTEE_SMC_GET_SDP_CONFIG, 0, 0, 0, 0, 0, 0, > > + 0, &res.smccc); > > + if (res.result.status != OPTEE_SMC_RETURN_OK) { > > + pr_err("Secure Data Path service not available\n"); > > + return 0; > > + } > > + > > + pool = tee_rstmem_static_pool_alloc(res.result.start, > > + res.result.size); > > + if (IS_ERR(pool)) > > + return PTR_ERR(pool); > > + > > + rc = tee_device_register_dma_heap(optee->teedev, heap_id, pool); > > + if (rc) > > + goto err; > > + } > > + > > + return 0; > > +err: > > + pool->ops->destroy_pool(pool); > > + return rc; > > +} > > + > > static int optee_probe(struct platform_device *pdev) > > { > > optee_invoke_fn *invoke_fn; > > @@ -1715,7 +1750,7 @@ static int optee_probe(struct platform_device *pdev) > > optee = kzalloc(sizeof(*optee), GFP_KERNEL); > > if (!optee) { > > rc = -ENOMEM; > > - goto err_free_pool; > > + goto err_free_shm_pool; > > } > > > > optee->ops = &optee_ops; > > @@ -1788,6 +1823,10 @@ static int optee_probe(struct platform_device *pdev) > > pr_info("Asynchronous notifications enabled\n"); > > } > > > > + rc = optee_sdp_pool_init(optee); > > s/optee_sdp_pool_init/optee_rstmem_pool_init/ OK Cheers, Jens > > -Sumit > > > + if (rc) > > + goto err_notif_uninit; > > + > > /* > > * Ensure that there are no pre-existing shm objects before enabling > > * the shm cache so that there's no chance of receiving an invalid > > @@ -1823,6 +1862,7 @@ static int optee_probe(struct platform_device *pdev) > > optee_disable_shm_cache(optee); > > optee_smc_notif_uninit_irq(optee); > > optee_unregister_devices(); > > + tee_device_unregister_all_dma_heaps(optee->teedev); > > err_notif_uninit: > > optee_notif_uninit(optee); > > err_close_ctx: > > @@ -1839,7 +1879,7 @@ static int optee_probe(struct platform_device *pdev) > > tee_device_unregister(optee->teedev); > > err_free_optee: > > kfree(optee); > > -err_free_pool: > > +err_free_shm_pool: > > tee_shm_pool_free(pool); > > if (memremaped_shm) > > memunmap(memremaped_shm); > > -- > > 2.43.0 > >

3 months

Re: [PATCH v6 06/10] tee: new ioctl to a register tee_shm from a dmabuf file descriptor

by Jens Wiklander

Hi Sumit, On Tue, Mar 25, 2025 at 7:50 AM Sumit Garg <sumit.garg(a)kernel.org> wrote: > > Hi Jens, > > On Wed, Mar 05, 2025 at 02:04:12PM +0100, Jens Wiklander wrote: > > From: Etienne Carriere <etienne.carriere(a)linaro.org> > > > > Enable userspace to create a tee_shm object that refers to a dmabuf > > reference. > > > > Userspace registers the dmabuf file descriptor as in a tee_shm object. > > The registration is completed with a tee_shm file descriptor returned to > > userspace. > > > > Userspace is free to close the dmabuf file descriptor now since all the > > resources are now held via the tee_shm object. > > > > Closing the tee_shm file descriptor will release all resources used by the > > tee_shm object. > > > > 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_DMA_BUF. > > > > Signed-off-by: Etienne Carriere <etienne.carriere(a)linaro.org> > > Signed-off-by: Olivier Masse <olivier.masse(a)nxp.com> > > Signed-off-by: Jens Wiklander <jens.wiklander(a)linaro.org> > > --- > > drivers/tee/tee_core.c | 145 ++++++++++++++++++++++++++----------- > > drivers/tee/tee_private.h | 1 + > > drivers/tee/tee_shm.c | 146 ++++++++++++++++++++++++++++++++++++-- > > include/linux/tee_core.h | 1 + > > include/linux/tee_drv.h | 10 +++ > > include/uapi/linux/tee.h | 29 ++++++++ > > 6 files changed, 288 insertions(+), 44 deletions(-) > > > > I am still trying to find if we really need a separate IOCTL to register > DMA heap with TEE subsystem. Can't we initialize tee_shm as a member of > struct tee_heap_buffer in tee_dma_heap_alloc() where the allocation > happens? No, that's not possible since we don't have a tee_context availble so we can't assign an ID that userspace can use for this shm object. We could add new attribute types TEE_IOCTL_PARAM_ATTR_TYPE_MEMFD_*, but it's a bit more complicated than that since we'd also need to update for the life cycle. > We can always find a reference back to tee_shm object from DMA > buffer. Yes Cheers, Jens > > -Sumit > > > diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c > > index 685afcaa3ea1..3a71643766d5 100644 > > --- a/drivers/tee/tee_core.c > > +++ b/drivers/tee/tee_core.c > > @@ -353,6 +353,103 @@ 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(&data, 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, &data, 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 param_from_user_memref(struct tee_context *ctx, > > + struct tee_param_memref *memref, > > + struct tee_ioctl_param *ip) > > +{ > > + struct tee_shm *shm; > > + size_t offs = 0; > > + > > + /* > > + * If a NULL pointer is passed to a TA in the TEE, > > + * the ip.c IOCTL parameters is set to TEE_MEMREF_NULL > > + * indicating a NULL memory reference. > > + */ > > + if (ip->c != TEE_MEMREF_NULL) { > > + /* > > + * If we fail to get a pointer to a shared > > + * memory object (and increase the ref count) > > + * from an identifier we return an error. All > > + * pointers that has been added in params have > > + * an increased ref count. It's the callers > > + * responibility to do tee_shm_put() on all > > + * resolved pointers. > > + */ > > + shm = tee_shm_get_from_id(ctx, ip->c); > > + if (IS_ERR(shm)) > > + return PTR_ERR(shm); > > + > > + /* > > + * Ensure offset + size does not overflow > > + * offset and does not overflow the size of > > + * the referred shared memory object. > > + */ > > + if ((ip->a + ip->b) < ip->a || > > + (ip->a + ip->b) > shm->size) { > > + tee_shm_put(shm); > > + return -EINVAL; > > + } > > + > > + if (shm->flags & TEE_SHM_DMA_BUF) { > > + struct tee_shm *parent_shm; > > + > > + parent_shm = tee_shm_get_parent_shm(shm, &offs); > > + if (parent_shm) { > > + tee_shm_put(shm); > > + shm = parent_shm; > > + } > > + } > > + } else if (ctx->cap_memref_null) { > > + /* Pass NULL pointer to OP-TEE */ > > + shm = NULL; > > + } else { > > + return -EINVAL; > > + } > > + > > + memref->shm_offs = ip->a + offs; > > + memref->size = ip->b; > > + memref->shm = shm; > > + > > + return 0; > > +} > > + > > static int params_from_user(struct tee_context *ctx, struct tee_param *params, > > size_t num_params, > > struct tee_ioctl_param __user *uparams) > > @@ -360,8 +457,8 @@ static int params_from_user(struct tee_context *ctx, struct tee_param *params, > > size_t n; > > > > for (n = 0; n < num_params; n++) { > > - struct tee_shm *shm; > > struct tee_ioctl_param ip; > > + int rc; > > > > if (copy_from_user(&ip, uparams + n, sizeof(ip))) > > return -EFAULT; > > @@ -384,45 +481,10 @@ static int params_from_user(struct tee_context *ctx, struct tee_param *params, > > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT: > > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT: > > case TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INOUT: > > - /* > > - * If a NULL pointer is passed to a TA in the TEE, > > - * the ip.c IOCTL parameters is set to TEE_MEMREF_NULL > > - * indicating a NULL memory reference. > > - */ > > - if (ip.c != TEE_MEMREF_NULL) { > > - /* > > - * If we fail to get a pointer to a shared > > - * memory object (and increase the ref count) > > - * from an identifier we return an error. All > > - * pointers that has been added in params have > > - * an increased ref count. It's the callers > > - * responibility to do tee_shm_put() on all > > - * resolved pointers. > > - */ > > - shm = tee_shm_get_from_id(ctx, ip.c); > > - if (IS_ERR(shm)) > > - return PTR_ERR(shm); > > - > > - /* > > - * Ensure offset + size does not overflow > > - * offset and does not overflow the size of > > - * the referred shared memory object. > > - */ > > - if ((ip.a + ip.b) < ip.a || > > - (ip.a + ip.b) > shm->size) { > > - tee_shm_put(shm); > > - return -EINVAL; > > - } > > - } else if (ctx->cap_memref_null) { > > - /* Pass NULL pointer to OP-TEE */ > > - shm = NULL; > > - } else { > > - return -EINVAL; > > - } > > - > > - params[n].u.memref.shm_offs = ip.a; > > - params[n].u.memref.size = ip.b; > > - params[n].u.memref.shm = shm; > > + rc = param_from_user_memref(ctx, &params[n].u.memref, > > + &ip); > > + if (rc) > > + return rc; > > break; > > default: > > /* Unknown attribute */ > > @@ -827,6 +889,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: > > @@ -1288,3 +1352,4 @@ MODULE_AUTHOR("Linaro"); > > MODULE_DESCRIPTION("TEE Driver"); > > MODULE_VERSION("1.0"); > > MODULE_LICENSE("GPL v2"); > > +MODULE_IMPORT_NS("DMA_BUF"); > > diff --git a/drivers/tee/tee_private.h b/drivers/tee/tee_private.h > > index 6c6ff5d5eed2..aad7f6c7e0f0 100644 > > --- a/drivers/tee/tee_private.h > > +++ b/drivers/tee/tee_private.h > > @@ -24,6 +24,7 @@ void teedev_ctx_put(struct tee_context *ctx); > > struct tee_shm *tee_shm_alloc_user_buf(struct tee_context *ctx, size_t size); > > struct tee_shm *tee_shm_register_user_buf(struct tee_context *ctx, > > unsigned long addr, size_t length); > > +struct tee_shm *tee_shm_get_parent_shm(struct tee_shm *shm, size_t *offs); > > > > int tee_heap_update_from_dma_buf(struct tee_device *teedev, > > struct dma_buf *dmabuf, size_t *offset, > > diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c > > index daf6e5cfd59a..8b79918468b5 100644 > > --- a/drivers/tee/tee_shm.c > > +++ b/drivers/tee/tee_shm.c > > @@ -4,6 +4,7 @@ > > */ > > #include <linux/anon_inodes.h> > > #include <linux/device.h> > > +#include <linux/dma-buf.h> > > #include <linux/idr.h> > > #include <linux/io.h> > > #include <linux/mm.h> > > @@ -15,6 +16,16 @@ > > #include <linux/highmem.h> > > #include "tee_private.h" > > > > +/* extra references appended to shm object for registered shared memory */ > > +struct tee_shm_dmabuf_ref { > > + struct tee_shm shm; > > + size_t offset; > > + struct dma_buf *dmabuf; > > + struct dma_buf_attachment *attach; > > + struct sg_table *sgt; > > + struct tee_shm *parent_shm; > > +}; > > + > > static void shm_put_kernel_pages(struct page **pages, size_t page_count) > > { > > size_t n; > > @@ -45,7 +56,23 @@ 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) { > > + struct tee_shm *parent_shm = NULL; > > + void *p = shm; > > + > > + if (shm->flags & TEE_SHM_DMA_BUF) { > > + struct tee_shm_dmabuf_ref *ref; > > + > > + ref = container_of(shm, struct tee_shm_dmabuf_ref, shm); > > + parent_shm = ref->parent_shm; > > + p = ref; > > + if (ref->attach) { > > + 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); > > @@ -57,9 +84,10 @@ static void tee_shm_release(struct tee_device *teedev, struct tee_shm *shm) > > release_registered_pages(shm); > > } > > > > - teedev_ctx_put(shm->ctx); > > + if (shm->ctx) > > + teedev_ctx_put(shm->ctx); > > > > - kfree(shm); > > + kfree(p); > > > > tee_device_put(teedev); > > } > > @@ -169,7 +197,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(). > > * > > - * @returns a pointer to 'struct tee_shm' > > + * @returns a pointer to 'struct tee_shm' on success, and ERR_PTR on failure > > */ > > struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size) > > { > > @@ -179,6 +207,116 @@ struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size) > > } > > EXPORT_SYMBOL_GPL(tee_shm_alloc_kernel_buf); > > > > +struct tee_shm *tee_shm_register_fd(struct tee_context *ctx, int fd) > > +{ > > + struct tee_shm_dmabuf_ref *ref; > > + int rc; > > + > > + if (!tee_device_get(ctx->teedev)) > > + return ERR_PTR(-EINVAL); > > + > > + teedev_ctx_get(ctx); > > + > > + ref = kzalloc(sizeof(*ref), GFP_KERNEL); > > + if (!ref) { > > + rc = -ENOMEM; > > + goto err_put_tee; > > + } > > + > > + refcount_set(&ref->shm.refcount, 1); > > + ref->shm.ctx = ctx; > > + ref->shm.id = -1; > > + ref->shm.flags = TEE_SHM_DMA_BUF; > > + > > + ref->dmabuf = dma_buf_get(fd); > > + if (IS_ERR(ref->dmabuf)) { > > + rc = PTR_ERR(ref->dmabuf); > > + goto err_kfree_ref; > > + } > > + > > + rc = tee_heap_update_from_dma_buf(ctx->teedev, ref->dmabuf, > > + &ref->offset, &ref->shm, > > + &ref->parent_shm); > > + if (!rc) > > + goto out; > > + if (rc != -EINVAL) > > + goto err_put_dmabuf; > > + > > + ref->attach = dma_buf_attach(ref->dmabuf, &ctx->teedev->dev); > > + if (IS_ERR(ref->attach)) { > > + rc = PTR_ERR(ref->attach); > > + goto err_put_dmabuf; > > + } > > + > > + ref->sgt = dma_buf_map_attachment(ref->attach, DMA_BIDIRECTIONAL); > > + if (IS_ERR(ref->sgt)) { > > + rc = PTR_ERR(ref->sgt); > > + goto err_detach; > > + } > > + > > + if (sg_nents(ref->sgt->sgl) != 1) { > > + rc = PTR_ERR(ref->sgt->sgl); > > + goto err_unmap_attachement; > > + } > > + > > + ref->shm.paddr = page_to_phys(sg_page(ref->sgt->sgl)); > > + ref->shm.size = ref->sgt->sgl->length; > > + > > +out: > > + mutex_lock(&ref->shm.ctx->teedev->mutex); > > + ref->shm.id = idr_alloc(&ref->shm.ctx->teedev->idr, &ref->shm, > > + 1, 0, GFP_KERNEL); > > + mutex_unlock(&ref->shm.ctx->teedev->mutex); > > + if (ref->shm.id < 0) { > > + rc = ref->shm.id; > > + if (ref->attach) > > + goto err_unmap_attachement; > > + goto err_put_dmabuf; > > + } > > + > > + return &ref->shm; > > + > > +err_unmap_attachement: > > + dma_buf_unmap_attachment(ref->attach, ref->sgt, DMA_BIDIRECTIONAL); > > +err_detach: > > + dma_buf_detach(ref->dmabuf, ref->attach); > > +err_put_dmabuf: > > + dma_buf_put(ref->dmabuf); > > +err_kfree_ref: > > + kfree(ref); > > +err_put_tee: > > + teedev_ctx_put(ctx); > > + tee_device_put(ctx->teedev); > > + > > + return ERR_PTR(rc); > > +} > > +EXPORT_SYMBOL_GPL(tee_shm_register_fd); > > + > > +struct tee_shm *tee_shm_get_parent_shm(struct tee_shm *shm, size_t *offs) > > +{ > > + struct tee_shm *parent_shm = NULL; > > + > > + if (shm->flags & TEE_SHM_DMA_BUF) { > > + struct tee_shm_dmabuf_ref *ref; > > + > > + ref = container_of(shm, struct tee_shm_dmabuf_ref, shm); > > + if (ref->parent_shm) { > > + /* > > + * the shm already has one reference to > > + * ref->parent_shm so we should be clear of 0. > > + * We're getting another reference since the caller > > + * of this function expects to put the returned > > + * parent_shm when it's done with it. > > + */ > > + parent_shm = ref->parent_shm; > > + refcount_inc(&parent_shm->refcount); > > + *offs = ref->offset; > > + } > > + } > > + > > + return parent_shm; > > +} > > + > > /** > > * tee_shm_alloc_priv_buf() - Allocate shared memory for a privately shared > > * kernel buffer > > diff --git a/include/linux/tee_core.h b/include/linux/tee_core.h > > index 16ef078247ae..6bd833b6d0e1 100644 > > --- a/include/linux/tee_core.h > > +++ b/include/linux/tee_core.h > > @@ -28,6 +28,7 @@ > > #define TEE_SHM_USER_MAPPED BIT(1) /* Memory mapped in user space */ > > #define TEE_SHM_POOL BIT(2) /* Memory allocated from pool */ > > #define TEE_SHM_PRIV BIT(3) /* Memory private to TEE driver */ > > +#define TEE_SHM_DMA_BUF BIT(4) /* Memory with dma-buf handle */ > > > > #define TEE_DEVICE_FLAG_REGISTERED 0x1 > > #define TEE_MAX_DEV_NAME_LEN 32 > > diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h > > index a54c203000ed..824f1251de60 100644 > > --- a/include/linux/tee_drv.h > > +++ b/include/linux/tee_drv.h > > @@ -116,6 +116,16 @@ struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size); > > struct tee_shm *tee_shm_register_kernel_buf(struct tee_context *ctx, > > void *addr, size_t length); > > > > +/** > > + * tee_shm_register_fd() - Register shared memory from file descriptor > > + * > > + * @ctx: Context that allocates the shared memory > > + * @fd: Shared memory file descriptor reference > > + * > > + * @returns a pointer to 'struct tee_shm' on success, and ERR_PTR on failure > > + */ > > +struct tee_shm *tee_shm_register_fd(struct tee_context *ctx, int fd); > > + > > /** > > * tee_shm_free() - Free shared memory > > * @shm: Handle to shared memory to free > > diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h > > index d0430bee8292..1f9a4ac2b211 100644 > > --- a/include/uapi/linux/tee.h > > +++ b/include/uapi/linux/tee.h > > @@ -118,6 +118,35 @@ struct tee_ioctl_shm_alloc_data { > > #define TEE_IOC_SHM_ALLOC _IOWR(TEE_IOC_MAGIC, TEE_IOC_BASE + 1, \ > > struct tee_ioctl_shm_alloc_data) > > > > +/** > > + * struct tee_ioctl_shm_register_fd_data - Shared memory registering argument > > + * @fd: [in] File descriptor identifying the shared memory > > + * @size: [out] Size of shared memory to allocate > > + * @flags: [in] Flags to/from allocation. > > + * @id: [out] Identifier of the shared memory > > + * > > + * The flags field should currently be zero as input. Updated by the call > > + * with actual flags as defined by TEE_IOCTL_SHM_* above. > > + * This structure is used as argument for TEE_IOC_SHM_REGISTER_FD below. > > + */ > > +struct tee_ioctl_shm_register_fd_data { > > + __s64 fd; > > + __u64 size; > > + __u32 flags; > > + __s32 id; > > +}; > > + > > +/** > > + * TEE_IOC_SHM_REGISTER_FD - register a shared memory from a file descriptor > > + * > > + * Returns a file descriptor on success or < 0 on failure > > + * > > + * The returned file descriptor refers to the shared memory object in kernel > > + * land. The shared memory is freed when the descriptor is closed. > > + */ > > +#define TEE_IOC_SHM_REGISTER_FD _IOWR(TEE_IOC_MAGIC, TEE_IOC_BASE + 8, \ > > + struct tee_ioctl_shm_register_fd_data) > > + > > /** > > * struct tee_ioctl_buf_data - Variable sized buffer > > * @buf_ptr: [in] A __user pointer to a buffer > > -- > > 2.43.0 > >

3 months

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig