On Wed, 28 Feb 2024 at 14:11, Jens Wiklander jens.wiklander@linaro.org wrote:
On Wed, Feb 28, 2024 at 6:58 AM Sumit Garg sumit.garg@linaro.org wrote:
On Tue, 27 Feb 2024 at 21:20, Jens Wiklander jens.wiklander@linaro.org wrote:
On Tue, Feb 27, 2024 at 7:06 AM Sumit Garg sumit.garg@linaro.org wrote:
[snip]
--- a/include/linux/tee_drv.h +++ b/include/linux/tee_drv.h @@ -275,6 +275,17 @@ void *tee_get_drvdata(struct tee_device *teedev); struct tee_shm *tee_shm_alloc_priv_buf(struct tee_context *ctx, size_t size); struct tee_shm *tee_shm_alloc_kernel_buf(struct tee_context *ctx, size_t size);
+int tee_shm_pool_op_alloc_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
size_t size, size_t align,int (*shm_register)(struct tee_context *ctx,struct tee_shm *shm,struct page **pages,size_t num_pages,unsigned long start));+void tee_shm_pool_op_free_helper(struct tee_shm_pool *pool, struct tee_shm *shm,
int (*shm_unregister)(struct tee_context *ctx,struct tee_shm *shm));These rather belong to drivers/tee/tee_private.h as we shouldn't expose them to other kernel client drivers.
This is the right place, this .h file is for TEE drivers too.
But this is shared with other kernel TEE client drivers and we shouldn't expose internal APIs which aren't meant for them with a side effect of API abuse too. Any particular reason to not use drivers/tee/tee_private.h?
drivers/tee/tee_private.h is supposed to be used internally by only the files in drivers/tee.
How about "struct tee_device" being in drivers/tee/tee_private.h?
If you look in include/linux/tee_drv.h you'll find a few functions and other definitions that a kernel TEE client driver should not use, for instance, tee_device_register() and tee_device_unregister(). This kernel TEE client interface was introduced with commit 25559c22cef8 ("tee: add kernel internal client interface"). include/linux/tee_drv.h existed before we even had any kernel TEE client interface.
Anyhow, it looks like there is a chance for refactoring here. How about splitting this header further in something like include/linux/tee_core.h which will contain all the pieces relevant to TEE drivers?
BTW, this patch series can keep using include/linux/tee_drv.h for the time being.
-Sumit
Cheers, Jens