Hi,
Here's a group of patches to the tee subsystem with cleanups and not so
urgent fixes related to tee shared memory.
- Unused parts of the shared memory object (struct tee_shm) are removed.
- Shared memory ids usable from user space are not assigned to driver
private shared memory objects
- The TEE_SHM_USER_MAPPED is used instead of TEE_SHM_REGISTER to accurately
tell if a shared memory object originates from another user space mapping.
Only unused "features" should be removed with this patch set, there should
be no change in behaviour or breakage with other code.
Thanks,
Jens
Jens Wiklander (5):
tee: remove linked list of struct tee_shm
tee: remove unused tee_shm_priv_alloc()
tee: don't assign shm id for private shms
tee: remove redundant teedev in struct tee_shm
tee: tee_shm_op_mmap(): use TEE_SHM_USER_MAPPED
drivers/tee/tee_core.c | 1 -
drivers/tee/tee_private.h | 3 +-
drivers/tee/tee_shm.c | 85 +++++++++++----------------------------
include/linux/tee_drv.h | 19 +--------
4 files changed, 27 insertions(+), 81 deletions(-)
--
2.17.1
Removes list_shm from struct tee_context since the linked list isn't used
any longer.
Signed-off-by: Jens Wiklander <[email protected]>
---
drivers/tee/tee_core.c | 1 -
drivers/tee/tee_shm.c | 12 +-----------
include/linux/tee_drv.h | 3 ---
3 files changed, 1 insertion(+), 15 deletions(-)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c
index 37d22e39fd8d..6aec502c495c 100644
--- a/drivers/tee/tee_core.c
+++ b/drivers/tee/tee_core.c
@@ -44,7 +44,6 @@ static struct tee_context *teedev_open(struct tee_device *teedev)
kref_init(&ctx->refcount);
ctx->teedev = teedev;
- INIT_LIST_HEAD(&ctx->list_shm);
rc = teedev->desc->ops->open(ctx);
if (rc)
goto err;
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 09ddcd06c715..6cabb834db7d 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -17,8 +17,6 @@ static void tee_shm_release(struct tee_shm *shm)
mutex_lock(&teedev->mutex);
idr_remove(&teedev->idr, shm->id);
- if (shm->ctx)
- list_del(&shm->link);
mutex_unlock(&teedev->mutex);
if (shm->flags & TEE_SHM_POOL) {
@@ -174,12 +172,8 @@ static struct tee_shm *__tee_shm_alloc(struct tee_context *ctx,
}
}
- if (ctx) {
+ if (ctx)
teedev_ctx_get(ctx);
- mutex_lock(&teedev->mutex);
- list_add_tail(&shm->link, &ctx->list_shm);
- mutex_unlock(&teedev->mutex);
- }
return shm;
err_rem:
@@ -307,10 +301,6 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr,
}
}
- mutex_lock(&teedev->mutex);
- list_add_tail(&shm->link, &ctx->list_shm);
- mutex_unlock(&teedev->mutex);
-
return shm;
err:
if (shm) {
diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
index 7a03f68fb982..cbddb883a7f8 100644
--- a/include/linux/tee_drv.h
+++ b/include/linux/tee_drv.h
@@ -49,7 +49,6 @@ struct tee_shm_pool;
*/
struct tee_context {
struct tee_device *teedev;
- struct list_head list_shm;
void *data;
struct kref refcount;
bool releasing;
@@ -170,7 +169,6 @@ void tee_device_unregister(struct tee_device *teedev);
* struct tee_shm - shared memory object
* @teedev: device used to allocate the object
* @ctx: context using the object, if NULL the context is gone
- * @link link element
* @paddr: physical address of the shared memory
* @kaddr: virtual address of the shared memory
* @size: size of shared memory
@@ -187,7 +185,6 @@ void tee_device_unregister(struct tee_device *teedev);
struct tee_shm {
struct tee_device *teedev;
struct tee_context *ctx;
- struct list_head link;
phys_addr_t paddr;
void *kaddr;
size_t size;
--
2.17.1
The ctx element in struct tee_shm is always valid. So remove the now
redundant teedev element.
Signed-off-by: Jens Wiklander <[email protected]>
---
drivers/tee/tee_shm.c | 7 ++-----
include/linux/tee_drv.h | 4 +---
2 files changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index e636cf82acdb..c64ec5c5e464 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -13,7 +13,7 @@
static void tee_shm_release(struct tee_shm *shm)
{
- struct tee_device *teedev = shm->teedev;
+ struct tee_device *teedev = shm->ctx->teedev;
if (shm->flags & TEE_SHM_DMA_BUF) {
mutex_lock(&teedev->mutex);
@@ -44,8 +44,7 @@ static void tee_shm_release(struct tee_shm *shm)
kfree(shm->pages);
}
- if (shm->ctx)
- teedev_ctx_put(shm->ctx);
+ teedev_ctx_put(shm->ctx);
kfree(shm);
@@ -132,7 +131,6 @@ struct tee_shm *tee_shm_alloc(struct tee_context *ctx, size_t size, u32 flags)
}
shm->flags = flags | TEE_SHM_POOL;
- shm->teedev = teedev;
shm->ctx = ctx;
if (flags & TEE_SHM_DMA_BUF)
poolm = teedev->pool->dma_buf_mgr;
@@ -221,7 +219,6 @@ struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr,
}
shm->flags = flags | TEE_SHM_REGISTER;
- shm->teedev = teedev;
shm->ctx = ctx;
shm->id = -1;
addr = untagged_addr(addr);
diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
index 42687f6c546d..1412e9cc79ce 100644
--- a/include/linux/tee_drv.h
+++ b/include/linux/tee_drv.h
@@ -167,8 +167,7 @@ void tee_device_unregister(struct tee_device *teedev);
/**
* struct tee_shm - shared memory object
- * @teedev: device used to allocate the object
- * @ctx: context using the object, if NULL the context is gone
+ * @ctx: context using the object
* @paddr: physical address of the shared memory
* @kaddr: virtual address of the shared memory
* @size: size of shared memory
@@ -183,7 +182,6 @@ void tee_device_unregister(struct tee_device *teedev);
* subsystem and from drivers that implements their own shm pool manager.
*/
struct tee_shm {
- struct tee_device *teedev;
struct tee_context *ctx;
phys_addr_t paddr;
void *kaddr;
--
2.17.1
Private shared memory object must not be referenced from user space. To
guarantee that, don't assign an id to shared memory objects which are
driver private.
Signed-off-by: Jens Wiklander <[email protected]>
---
drivers/tee/tee_private.h | 3 ++-
drivers/tee/tee_shm.c | 31 ++++++++++++++++++-------------
2 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/drivers/tee/tee_private.h b/drivers/tee/tee_private.h
index f797171f0434..e55204df31ce 100644
--- a/drivers/tee/tee_private.h
+++ b/drivers/tee/tee_private.h
@@ -37,7 +37,8 @@ struct tee_shm_pool {
* @num_users: number of active users of this device
* @c_no_user: completion used when unregistering the device
* @mutex: mutex protecting @num_users and @idr
- * @idr: register of shared memory object allocated on this device
+ * @idr: register of user space shared memory objects allocated or
+ * registered on this device
* @pool: shared memory pool
*/
struct tee_device {
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 8afe08b23242..e636cf82acdb 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -15,9 +15,11 @@ static void tee_shm_release(struct tee_shm *shm)
{
struct tee_device *teedev = shm->teedev;
- mutex_lock(&teedev->mutex);
- idr_remove(&teedev->idr, shm->id);
- mutex_unlock(&teedev->mutex);
+ if (shm->flags & TEE_SHM_DMA_BUF) {
+ mutex_lock(&teedev->mutex);
+ idr_remove(&teedev->idr, shm->id);
+ mutex_unlock(&teedev->mutex);
+ }
if (shm->flags & TEE_SHM_POOL) {
struct tee_shm_pool_mgr *poolm;
@@ -143,17 +145,18 @@ struct tee_shm *tee_shm_alloc(struct tee_context *ctx, size_t size, u32 flags)
goto err_kfree;
}
- mutex_lock(&teedev->mutex);
- shm->id = idr_alloc(&teedev->idr, shm, 1, 0, GFP_KERNEL);
- mutex_unlock(&teedev->mutex);
- if (shm->id < 0) {
- ret = ERR_PTR(shm->id);
- goto err_pool_free;
- }
if (flags & TEE_SHM_DMA_BUF) {
DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
+ mutex_lock(&teedev->mutex);
+ shm->id = idr_alloc(&teedev->idr, shm, 1, 0, GFP_KERNEL);
+ mutex_unlock(&teedev->mutex);
+ if (shm->id < 0) {
+ ret = ERR_PTR(shm->id);
+ goto err_pool_free;
+ }
+
exp_info.ops = &tee_shm_dma_buf_ops;
exp_info.size = shm->size;
exp_info.flags = O_RDWR;
@@ -171,9 +174,11 @@ struct tee_shm *tee_shm_alloc(struct tee_context *ctx, size_t size, u32 flags)
return shm;
err_rem:
- mutex_lock(&teedev->mutex);
- idr_remove(&teedev->idr, shm->id);
- mutex_unlock(&teedev->mutex);
+ if (flags & TEE_SHM_DMA_BUF) {
+ mutex_lock(&teedev->mutex);
+ idr_remove(&teedev->idr, shm->id);
+ mutex_unlock(&teedev->mutex);
+ }
err_pool_free:
poolm->ops->free(poolm, shm);
err_kfree:
--
2.17.1
tee_shm_priv_alloc() isn't useful in the current state and it's also not
not used so remove it.
Signed-off-by: Jens Wiklander <[email protected]>
---
drivers/tee/tee_shm.c | 33 ++-------------------------------
include/linux/tee_drv.h | 12 ------------
2 files changed, 2 insertions(+), 43 deletions(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index 6cabb834db7d..8afe08b23242 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -95,20 +95,14 @@ static const struct dma_buf_ops tee_shm_dma_buf_ops = {
.mmap = tee_shm_op_mmap,
};
-static struct tee_shm *__tee_shm_alloc(struct tee_context *ctx,
- struct tee_device *teedev,
- size_t size, u32 flags)
+struct tee_shm *tee_shm_alloc(struct tee_context *ctx, size_t size, u32 flags)
{
+ struct tee_device *teedev = ctx->teedev;
struct tee_shm_pool_mgr *poolm = NULL;
struct tee_shm *shm;
void *ret;
int rc;
- if (ctx && ctx->teedev != teedev) {
- dev_err(teedev->dev.parent, "ctx and teedev mismatch\n");
- return ERR_PTR(-EINVAL);
- }
-
if (!(flags & TEE_SHM_MAPPED)) {
dev_err(teedev->dev.parent,
"only mapped allocations supported\n");
@@ -188,31 +182,8 @@ static struct tee_shm *__tee_shm_alloc(struct tee_context *ctx,
tee_device_put(teedev);
return ret;
}
-
-/**
- * tee_shm_alloc() - Allocate shared memory
- * @ctx: Context that allocates the shared memory
- * @size: Requested size of shared memory
- * @flags: Flags setting properties for the requested shared memory.
- *
- * Memory allocated as global shared memory is automatically freed when the
- * TEE file pointer is closed. The @flags field uses the bits defined by
- * TEE_SHM_* in <linux/tee_drv.h>. TEE_SHM_MAPPED must currently always be
- * set. If TEE_SHM_DMA_BUF global shared memory will be allocated and
- * associated with a dma-buf handle, else driver private memory.
- */
-struct tee_shm *tee_shm_alloc(struct tee_context *ctx, size_t size, u32 flags)
-{
- return __tee_shm_alloc(ctx, ctx->teedev, size, flags);
-}
EXPORT_SYMBOL_GPL(tee_shm_alloc);
-struct tee_shm *tee_shm_priv_alloc(struct tee_device *teedev, size_t size)
-{
- return __tee_shm_alloc(NULL, teedev, size, TEE_SHM_MAPPED);
-}
-EXPORT_SYMBOL_GPL(tee_shm_priv_alloc);
-
struct tee_shm *tee_shm_register(struct tee_context *ctx, unsigned long addr,
size_t length, u32 flags)
{
diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h
index cbddb883a7f8..42687f6c546d 100644
--- a/include/linux/tee_drv.h
+++ b/include/linux/tee_drv.h
@@ -315,18 +315,6 @@ void *tee_get_drvdata(struct tee_device *teedev);
*/
struct tee_shm *tee_shm_alloc(struct tee_context *ctx, size_t size, u32 flags);
-/**
- * tee_shm_priv_alloc() - Allocate shared memory privately
- * @dev: Device that allocates the shared memory
- * @size: Requested size of shared memory
- *
- * Allocates shared memory buffer that is not associated with any client
- * context. Such buffers are owned by TEE driver and used for internal calls.
- *
- * @returns a pointer to 'struct tee_shm'
- */
-struct tee_shm *tee_shm_priv_alloc(struct tee_device *teedev, size_t size);
-
/**
* tee_shm_register() - Register shared memory buffer
* @ctx: Context that registers the shared memory
--
2.17.1
tee_shm_op_mmap() uses the TEE_SHM_USER_MAPPED flag instead of the
TEE_SHM_REGISTER flag to tell if a shared memory object is originating
from registered user space memory.
Signed-off-by: Jens Wiklander <[email protected]>
---
drivers/tee/tee_shm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c
index c64ec5c5e464..deb22f877881 100644
--- a/drivers/tee/tee_shm.c
+++ b/drivers/tee/tee_shm.c
@@ -81,7 +81,7 @@ static int tee_shm_op_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
size_t size = vma->vm_end - vma->vm_start;
/* Refuse sharing shared memory provided by application */
- if (shm->flags & TEE_SHM_REGISTER)
+ if (shm->flags & TEE_SHM_USER_MAPPED)
return -EINVAL;
return remap_pfn_range(vma, vma->vm_start, shm->paddr >> PAGE_SHIFT,
--
2.17.1
Hi,
On Thu, Jan 9, 2020 at 1:37 PM Jens Wiklander <[email protected]> wrote:
>
> Hi,
>
> Here's a group of patches to the tee subsystem with cleanups and not so
> urgent fixes related to tee shared memory.
>
> - Unused parts of the shared memory object (struct tee_shm) are removed.
> - Shared memory ids usable from user space are not assigned to driver
> private shared memory objects
> - The TEE_SHM_USER_MAPPED is used instead of TEE_SHM_REGISTER to accurately
> tell if a shared memory object originates from another user space mapping.
>
> Only unused "features" should be removed with this patch set, there should
> be no change in behaviour or breakage with other code.
I'll pick up these as they are in a few days if there's no further comments.
Thanks,
Jens
>
> Thanks,
> Jens
>
> Jens Wiklander (5):
> tee: remove linked list of struct tee_shm
> tee: remove unused tee_shm_priv_alloc()
> tee: don't assign shm id for private shms
> tee: remove redundant teedev in struct tee_shm
> tee: tee_shm_op_mmap(): use TEE_SHM_USER_MAPPED
>
> drivers/tee/tee_core.c | 1 -
> drivers/tee/tee_private.h | 3 +-
> drivers/tee/tee_shm.c | 85 +++++++++++----------------------------
> include/linux/tee_drv.h | 19 +--------
> 4 files changed, 27 insertions(+), 81 deletions(-)
>
> --
> 2.17.1
>