Re: [PATCH v3 5/7] tee: Support shm registration without dma-buf backing
From: Allen Pais <hidden>
Date: 2021-06-10 07:40:45
Also in:
linux-mips, lkml, op-tee
quoted
AFAIK, its due the the inherent nature of tee_shm_alloc() and tee_shm_register() where tee_shm_alloc() doesn't need to know whether its a kernel or user-space memory since it is the one that allocates whereas tee_shm_register() need to know that since it has to register pre-allocated client memory.quoted
- Why does tee_shm_register() unconditionally use non-contiguous allocations without ever taking into account whether or not OPTEE_SMC_SEC_CAP_DYNAMIC_SHM was set? It sounds like that's required from my reading of https://optee.readthedocs.io/en/latest/architecture/core.html#noncontiguous-shared-buffers.Yeah, but do we have platforms in OP-TEE that don't support dynamic shared memory? I guess it has become the sane default which is a mandatory requirement when it comes to OP-TEE driver in u-boot.quoted
- Why is TEE_SHM_REGISTER implemented at the TEE driver level when it is specific to OP-TEE? How to better abstract that away?I would like you to go through Section "3.2.4. Shared Memory" in TEE Client API Specification. There are two standard ways for shared memory approach with TEE: 1. A Shared Memory block can either be existing Client Application memory (kernel driver in our case) which is subsequently registered with the TEE Client API (using tee_shm_register() in our case). 2. Or memory which is allocated on behalf of the Client Application using the TEE Client API (using tee_shm_alloc() in our case).quoted
Let me know if you agree with the more minimal approach that I took for these bug fix series or still feel like tee_shm_register() should be fixed up so that it is usable. Thanks!From drivers perspective I think the change should be: tee_shm_alloc() to kcalloc() tee_shm_register()I've just posted "[PATCH 0/7] tee: shared memory updates", https://lore.kernel.org/lkml/20210609102324.2222332-1-jens.wiklander@linaro.org/ (local) Where tee_shm_alloc() is replaced by among other functions tee_shm_alloc_kernel_buf(). tee_shm_alloc_kernel_buf() takes care of the problem with TEE_SHM_DMA_BUF.
Thanks Jens. The series looks fine. Tested too. - Allen