xiaoxiang781216 commented on PR #16309: URL: https://github.com/apache/nuttx/pull/16309#issuecomment-2862951631
> > @gpoulios why mark the patch to draft? > > 3 reasons: > > * I don't see this getting @raiden00pl 's approval so I thought I'd try re-writing some parts of the code but it turns out I can't. On the contrary, I'm thinking of putting back `reg_pair_to_64` and co. as they are [actually licensed under BSD](https://github.com/tiiuae/imx-optee-os/blob/8dd180b6d149c1e1314b5869697179f665bd9ca3/lib/libutils/ext/include/util.h#L169). > > * I need to guard all list accesses around a mutex. Btw, @xiaoxiang781216 would you rather see this as a new commit or an amendment to the existing one? > both are fine, if the defect in the unmerged code, it's better to fix the problem direclty. > * I noticed the IDs in `tee_ioctl_shm_{alloc,register,register_fd}_data` are _all_ output parameters (whereas I was using `rdata.id` as input): > > > https://github.com/apache/nuttx/blob/468c9eacd8fd30817cd683a07a10ead8d2652a0a/include/nuttx/tee.h#L113 > > https://github.com/apache/nuttx/blob/468c9eacd8fd30817cd683a07a10ead8d2652a0a/include/nuttx/tee.h#L145 > > https://github.com/apache/nuttx/blob/468c9eacd8fd30817cd683a07a10ead8d2652a0a/include/nuttx/tee.h#L393 > > @xiaoxiang781216 Do we really need those? I'm having a hard time generating unique IDs for shared memory registrations. Thinking of the following options: > > 1. casting the actual buffer address to int32 > > * not guaranteed to be unique with 64-bit addresses > > 2. traversing the list to find a free integer ID starting from `INT32_MIN` > > * needs one full list traversal at best, multiple if we've done `UIN32_MAX` allocations-deallocations; Without deallocations we don't expect IDs to overflow as we will be out of memory much sooner > > 3. using list head's ID + 1 > > * we need to stick to adding new entries at the head of the list and might overflow sooner, but is efficient > > 4. using a `shm_nextid` field in `struct optee_priv_data` > > * need to guard with mutex (which I'm also doing for list accesses right now, so not such a big deal) > > > Leaning towards 3, but would appreciate your thoughts on this. > Yes, method 3 is the simple and good enough. > On the contrary, points for removing the IDs entirely: > > * we can also remove this (correct me if I'm wrong) https://github.com/apache/nuttx/blob/1a8fba827a73bb842a60f86b610470c2f38ad4f4/drivers/misc/optee.c#L586 > > * it's not like we can prevent duplicate list entries anyways. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
