We should not need to index into SHMs based on absolute VA/PA. These functions are not used and this kind of usage should not be encouraged anyway. Remove these functions.
Signed-off-by: Andrew Davis afd@ti.com --- drivers/tee/tee_shm.c | 50 ----------------------------------------- include/linux/tee_drv.h | 18 --------------- 2 files changed, 68 deletions(-)
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index f31e29e8f1cac..b0c6d553d3a70 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -414,56 +414,6 @@ void tee_shm_free(struct tee_shm *shm) } EXPORT_SYMBOL_GPL(tee_shm_free);
-/** - * tee_shm_va2pa() - Get physical address of a virtual address - * @shm: Shared memory handle - * @va: Virtual address to tranlsate - * @pa: Returned physical address - * @returns 0 on success and < 0 on failure - */ -int tee_shm_va2pa(struct tee_shm *shm, void *va, phys_addr_t *pa) -{ - if (!shm->kaddr) - return -EINVAL; - /* Check that we're in the range of the shm */ - if ((char *)va < (char *)shm->kaddr) - return -EINVAL; - if ((char *)va >= ((char *)shm->kaddr + shm->size)) - return -EINVAL; - - return tee_shm_get_pa( - shm, (unsigned long)va - (unsigned long)shm->kaddr, pa); -} -EXPORT_SYMBOL_GPL(tee_shm_va2pa); - -/** - * tee_shm_pa2va() - Get virtual address of a physical address - * @shm: Shared memory handle - * @pa: Physical address to tranlsate - * @va: Returned virtual address - * @returns 0 on success and < 0 on failure - */ -int tee_shm_pa2va(struct tee_shm *shm, phys_addr_t pa, void **va) -{ - if (!shm->kaddr) - return -EINVAL; - /* Check that we're in the range of the shm */ - if (pa < shm->paddr) - return -EINVAL; - if (pa >= (shm->paddr + shm->size)) - return -EINVAL; - - if (va) { - void *v = tee_shm_get_va(shm, pa - shm->paddr); - - if (IS_ERR(v)) - return PTR_ERR(v); - *va = v; - } - return 0; -} -EXPORT_SYMBOL_GPL(tee_shm_pa2va); - /** * tee_shm_get_va() - Get virtual address of a shared memory plus an offset * @shm: Shared memory handle diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h index 911cad324acc7..17eb1c5205d34 100644 --- a/include/linux/tee_drv.h +++ b/include/linux/tee_drv.h @@ -298,24 +298,6 @@ void tee_shm_free(struct tee_shm *shm); */ void tee_shm_put(struct tee_shm *shm);
-/** - * tee_shm_va2pa() - Get physical address of a virtual address - * @shm: Shared memory handle - * @va: Virtual address to tranlsate - * @pa: Returned physical address - * @returns 0 on success and < 0 on failure - */ -int tee_shm_va2pa(struct tee_shm *shm, void *va, phys_addr_t *pa); - -/** - * tee_shm_pa2va() - Get virtual address of a physical address - * @shm: Shared memory handle - * @pa: Physical address to tranlsate - * @va: Returned virtual address - * @returns 0 on success and < 0 on failure - */ -int tee_shm_pa2va(struct tee_shm *shm, phys_addr_t pa, void **va); - /** * tee_shm_get_va() - Get virtual address of a shared memory plus an offset * @shm: Shared memory handle
These look to be leftover from an early edition of this driver. Userspace does not need this information. Checking all users of this that I have access to I have verified no one is using them.
They leak internal use flags out to userspace. Even more they are not correct anymore after a45ea4efa358. Lets drop these flags before someone does try to use them for something and they become ABI.
Signed-off-by: Andrew Davis afd@ti.com --- drivers/tee/tee_core.c | 1 - include/uapi/linux/tee.h | 4 ---- 2 files changed, 5 deletions(-)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index 8aa1a4836b92f..650dd87a38e77 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -339,7 +339,6 @@ tee_ioctl_shm_register(struct tee_context *ctx, return PTR_ERR(shm);
data.id = shm->id; - data.flags = shm->flags; data.length = shm->size;
if (copy_to_user(udata, &data, sizeof(data))) diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h index 25a6c534beb1b..23e57164693c4 100644 --- a/include/uapi/linux/tee.h +++ b/include/uapi/linux/tee.h @@ -42,10 +42,6 @@ #define TEE_IOC_MAGIC 0xa4 #define TEE_IOC_BASE 0
-/* Flags relating to shared memory */ -#define TEE_IOCTL_SHM_MAPPED 0x1 /* memory mapped in normal world */ -#define TEE_IOCTL_SHM_DMA_BUF 0x2 /* dma-buf handle on shared memory */ - #define TEE_MAX_ARG_SIZE 1024
#define TEE_GEN_CAP_GP (1 << 0)/* GlobalPlatform compliant TEE */
On Fri, 22 Apr 2022 at 23:31, Andrew Davis afd@ti.com wrote:
These look to be leftover from an early edition of this driver. Userspace does not need this information. Checking all users of this that I have access to I have verified no one is using them.
They leak internal use flags out to userspace. Even more they are not correct anymore after a45ea4efa358. Lets drop these flags before someone does try to use them for something and they become ABI.
Sounds reasonable to me.
Signed-off-by: Andrew Davis afd@ti.com
drivers/tee/tee_core.c | 1 - include/uapi/linux/tee.h | 4 ---- 2 files changed, 5 deletions(-)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index 8aa1a4836b92f..650dd87a38e77 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -339,7 +339,6 @@ tee_ioctl_shm_register(struct tee_context *ctx, return PTR_ERR(shm);
data.id = shm->id;
data.flags = shm->flags; data.length = shm->size;
This change is required for tee_ioctl_shm_alloc() as well.
-Sumit
if (copy_to_user(udata, &data, sizeof(data)))diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h index 25a6c534beb1b..23e57164693c4 100644 --- a/include/uapi/linux/tee.h +++ b/include/uapi/linux/tee.h @@ -42,10 +42,6 @@ #define TEE_IOC_MAGIC 0xa4 #define TEE_IOC_BASE 0
-/* Flags relating to shared memory */ -#define TEE_IOCTL_SHM_MAPPED 0x1 /* memory mapped in normal world */ -#define TEE_IOCTL_SHM_DMA_BUF 0x2 /* dma-buf handle on shared memory */
#define TEE_MAX_ARG_SIZE 1024
#define TEE_GEN_CAP_GP (1 << 0)/* GlobalPlatform compliant TEE */
2.17.1
On 4/25/22 1:46 AM, Sumit Garg wrote:
On Fri, 22 Apr 2022 at 23:31, Andrew Davis afd@ti.com wrote:
These look to be leftover from an early edition of this driver. Userspace does not need this information. Checking all users of this that I have access to I have verified no one is using them.
They leak internal use flags out to userspace. Even more they are not correct anymore after a45ea4efa358. Lets drop these flags before someone does try to use them for something and they become ABI.
Sounds reasonable to me.
Signed-off-by: Andrew Davis afd@ti.com
drivers/tee/tee_core.c | 1 - include/uapi/linux/tee.h | 4 ---- 2 files changed, 5 deletions(-)
diff --git a/drivers/tee/tee_core.c b/drivers/tee/tee_core.c index 8aa1a4836b92f..650dd87a38e77 100644 --- a/drivers/tee/tee_core.c +++ b/drivers/tee/tee_core.c @@ -339,7 +339,6 @@ tee_ioctl_shm_register(struct tee_context *ctx, return PTR_ERR(shm);
data.id = shm->id;
data.flags = shm->flags; data.length = shm->size;This change is required for tee_ioctl_shm_alloc() as well.
Indeed it is, adding for v2.
Thanks, Andrew
-Sumit
if (copy_to_user(udata, &data, sizeof(data)))diff --git a/include/uapi/linux/tee.h b/include/uapi/linux/tee.h index 25a6c534beb1b..23e57164693c4 100644 --- a/include/uapi/linux/tee.h +++ b/include/uapi/linux/tee.h @@ -42,10 +42,6 @@ #define TEE_IOC_MAGIC 0xa4 #define TEE_IOC_BASE 0
-/* Flags relating to shared memory */ -#define TEE_IOCTL_SHM_MAPPED 0x1 /* memory mapped in normal world */ -#define TEE_IOCTL_SHM_DMA_BUF 0x2 /* dma-buf handle on shared memory */
#define TEE_MAX_ARG_SIZE 1024
#define TEE_GEN_CAP_GP (1 << 0)/* GlobalPlatform compliant TEE */
-- 2.17.1
On Fri, 22 Apr 2022 at 23:31, Andrew Davis afd@ti.com wrote:
We should not need to index into SHMs based on absolute VA/PA. These functions are not used and this kind of usage should not be encouraged anyway. Remove these functions.
Signed-off-by: Andrew Davis afd@ti.com
drivers/tee/tee_shm.c | 50 ----------------------------------------- include/linux/tee_drv.h | 18 --------------- 2 files changed, 68 deletions(-)
Sounds good to me as there are tee_shm_get_{va/pa}() which are well suited/used for index based VA/PA. FWIW:
Reviewed-by: Sumit Garg sumit.garg@linaro.org
-Sumit
diff --git a/drivers/tee/tee_shm.c b/drivers/tee/tee_shm.c index f31e29e8f1cac..b0c6d553d3a70 100644 --- a/drivers/tee/tee_shm.c +++ b/drivers/tee/tee_shm.c @@ -414,56 +414,6 @@ void tee_shm_free(struct tee_shm *shm) } EXPORT_SYMBOL_GPL(tee_shm_free);
-/**
- tee_shm_va2pa() - Get physical address of a virtual address
- @shm: Shared memory handle
- @va: Virtual address to tranlsate
- @pa: Returned physical address
- @returns 0 on success and < 0 on failure
- */
-int tee_shm_va2pa(struct tee_shm *shm, void *va, phys_addr_t *pa) -{
if (!shm->kaddr)return -EINVAL;/* Check that we're in the range of the shm */if ((char *)va < (char *)shm->kaddr)return -EINVAL;if ((char *)va >= ((char *)shm->kaddr + shm->size))return -EINVAL;return tee_shm_get_pa(shm, (unsigned long)va - (unsigned long)shm->kaddr, pa);-} -EXPORT_SYMBOL_GPL(tee_shm_va2pa);
-/**
- tee_shm_pa2va() - Get virtual address of a physical address
- @shm: Shared memory handle
- @pa: Physical address to tranlsate
- @va: Returned virtual address
- @returns 0 on success and < 0 on failure
- */
-int tee_shm_pa2va(struct tee_shm *shm, phys_addr_t pa, void **va) -{
if (!shm->kaddr)return -EINVAL;/* Check that we're in the range of the shm */if (pa < shm->paddr)return -EINVAL;if (pa >= (shm->paddr + shm->size))return -EINVAL;if (va) {void *v = tee_shm_get_va(shm, pa - shm->paddr);if (IS_ERR(v))return PTR_ERR(v);*va = v;}return 0;-} -EXPORT_SYMBOL_GPL(tee_shm_pa2va);
/**
- tee_shm_get_va() - Get virtual address of a shared memory plus an offset
- @shm: Shared memory handle
diff --git a/include/linux/tee_drv.h b/include/linux/tee_drv.h index 911cad324acc7..17eb1c5205d34 100644 --- a/include/linux/tee_drv.h +++ b/include/linux/tee_drv.h @@ -298,24 +298,6 @@ void tee_shm_free(struct tee_shm *shm); */ void tee_shm_put(struct tee_shm *shm);
-/**
- tee_shm_va2pa() - Get physical address of a virtual address
- @shm: Shared memory handle
- @va: Virtual address to tranlsate
- @pa: Returned physical address
- @returns 0 on success and < 0 on failure
- */
-int tee_shm_va2pa(struct tee_shm *shm, void *va, phys_addr_t *pa);
-/**
- tee_shm_pa2va() - Get virtual address of a physical address
- @shm: Shared memory handle
- @pa: Physical address to tranlsate
- @va: Returned virtual address
- @returns 0 on success and < 0 on failure
- */
-int tee_shm_pa2va(struct tee_shm *shm, phys_addr_t pa, void **va);
/**
- tee_shm_get_va() - Get virtual address of a shared memory plus an offset
- @shm: Shared memory handle
-- 2.17.1
op-tee@lists.trustedfirmware.org
-
Andrew Davis -
Sumit Garg