Messages in this thread | | | From | Sumit Garg <> | Date | Thu, 14 May 2020 11:08:43 +0530 | Subject | Re: [PATCH] optee: don't fail on unsuccessful device enumeration |
| |
Hi Volodymyr,
On Thu, 14 May 2020 at 06:48, Volodymyr Babchuk <vlad.babchuk@gmail.com> wrote: > > Hi Sumit, > > On Wed, 13 May 2020 at 11:24, Sumit Garg <sumit.garg@linaro.org> wrote: > > > > Hi Volodymyr, > > > > On Wed, 13 May 2020 at 13:30, Jens Wiklander <jens.wiklander@linaro.org> wrote: > > > > > > Hi Volodymyr, > > > > > > On Wed, May 13, 2020 at 2:36 AM Volodymyr Babchuk > > > <vlad.babchuk@gmail.com> wrote: > > > > > > > > optee_enumerate_devices() can fail for multiple of reasons. For > > > > example, I encountered issue when Xen OP-TEE mediator NACKed > > > > PTA_CMD_GET_DEVICES call. > > > > Could you share a detailed description of the issue which you are > > facing? optee_enumerate_devices() is a simple invocation of pseudo TA > > and cases where OP-TEE doesn't provide corresponding pseudo TA are > > handled very well. > > Yes, I did some research and looks like issue is broader, than I > expected. It is my fault, that I wasn't paying attention to the tee > client support in the kernel. Basically, it is incompatible with the > virtualization feature. You see, the main issue with virtual machines > is the second stage MMU. Intermediate physical address, that appear to > be contiguous for the kernel may be not contiguous in the real > physical memory due to 2nd stage MMU mappings. This is the reason I > introduced OPTEE_MSG_ATTR_NONCONTIG in the kernel driver. > > But, looks like kernel-side optee client does not use this feature. It > tries to provide SHM buffer as a simple contiguous span of memory. Xen > blocks calls with OPTEE_MSG_ATTR_TYPE_TMEM_* but without > OPTEE_MSG_ATTR_NONCONTIG , because it can't translate IPAs to PAs for > such buffers. This is why call to PTA_CMD_GET_DEVICES fails. > > Valid fix would be to use OPTEE_MSG_ATTR_NONCONTIG whenever possible. >
Thanks for the detailed analysis. It looks like you are missing the following fix patch in your tree which basically fixed broken tee_shm_alloc() in case dynamic shared memory is enabled (IIRC virtualization only supports dynamic shared memory).
commit a249dd200d03791cab23e47571f3e13d9c72af6c Author: Sumit Garg <sumit.garg@linaro.org> Date: Fri Nov 8 16:57:14 2019 +0530
tee: optee: Fix dynamic shm pool allocations
In case of dynamic shared memory pool, kernel memory allocated using dmabuf_mgr pool needs to be registered with OP-TEE prior to its usage during optee_open_session() or optee_invoke_func().
So fix dmabuf_mgr pool allocations via an additional call to optee_shm_register().
Also, allow kernel pages to be registered as shared memory with OP-TEE.
Fixes: 9733b072a12a ("optee: allow to work without static shared memory") Signed-off-by: Sumit Garg <sumit.garg@linaro.org> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
After this fix, your issue should be resolved as it uses OPTEE_MSG_ATTR_NONCONTIG attribute for kernel memory allocated via tee_shm_alloc().
> > > > > > This should not result in driver > > > > initialization error because this is an optional feature. > > > > I wouldn't call it an optional feature as there might be real kernel > > drivers dependent on this enumeration. Also, it is a simple example to > > self test OP-TEE functionality too. So I am not sure how much > > functional OP-TEE would be if this basic TA invocation fails. > > Well, it fixed case when Xen is involved. I think, this is valid > combination, when platform have the newest OP-TEE, but slightly older > kernel. So, imagine that OP-TEE provides PTA_CMD_GET_DEVICES, but > kernel can't use because it uses plain TMEM arguments,which are not > supported in virtualized environment. > > If there are kernel drivers, that depend on this PTA, they would not > work in any case. But at least userspace clients still be able to use > OP-TEE. This is why I call this feature "optional".
As you can see above, tee_shm_alloc() being broken in your case was detected via this simple pseudo TA invocation. So IMO, it would be better to keep the existing behaviour as it provides a kind of basic OP-TEE driver runtime self test too. Also, I think it would be a better user experience to have every OP-TEE interface working rather than a partially broken interface.
-Sumit
> > -- > WBR Volodymyr Babchuk aka lorc [+380976646013] > mailto: vlad.babchuk@gmail.com
| |