Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


xiaoxiang781216 commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2073694997


##
drivers/misc/Kconfig:
##
@@ -43,31 +43,66 @@ choice
prompt "Select OP-TEE dev implementation"
default DEV_OPTEE_NONE
---help---
-   There are two implementations of optee server,
+   There are three implementations of optee server,
one is soft tee server which does not distinguish
between secure and non-secure states and uses socket 
communication,
-   and the other is teeos which runs in the secure state of
-   the cpu and uses rpmsg cross-core communication.
+   and the other two is teeos which runs in the secure state of
+   the cpu and uses either rpmsg cross-core communication or ARM64 
SMCs.
 
 config DEV_OPTEE_LOCAL
bool "OPTEE Local Socket Support"
depends on NET_LOCAL
+   depends on LIBC_MEMFD_SHMFS
depends on ALLOW_BSD_COMPONENTS
 
 config DEV_OPTEE_RPMSG
bool "OP-TEE RPMSG Socket Support"
depends on NET_RPMSG
+   depends on LIBC_MEMFD_SHMFS
+   depends on ALLOW_BSD_COMPONENTS
+
+config DEV_OPTEE_SMC64

Review Comment:
   Documentation  and Kconfig



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


xiaoxiang781216 commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2073693225


##
drivers/misc/optee.c:
##
@@ -653,5 +1849,47 @@ static int optee_ioctl(FAR struct file *filep, int cmd, 
unsigned long arg)
 
 int optee_register(void)
 {
+#ifdef CONFIG_DEV_OPTEE_SMC64

Review Comment:
   it could be possible, since nuttx support kernel module too.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


gpoulios commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2073681063


##
drivers/misc/optee.c:
##
@@ -653,5 +1849,47 @@ static int optee_ioctl(FAR struct file *filep, int cmd, 
unsigned long arg)
 
 int optee_register(void)
 {
+#ifdef CONFIG_DEV_OPTEE_SMC64

Review Comment:
   Removed. But like I said, this kind of test _cannot_ be performed through 
nuttx-apps.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


xiaoxiang781216 commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2073675102


##
drivers/misc/CMakeLists.txt:
##
@@ -80,6 +80,7 @@ endif()
 
 if(NOT CONFIG_DEV_OPTEE_NONE)
   list(APPEND SRCS optee.c)
+  list(APPEND SRCS optee_rpmsg.c)

Review Comment:
   let's call the backend optee_socket.c? since it supports both rpmsg and 
local socket.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


xiaoxiang781216 commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2073671566


##
drivers/misc/optee.c:
##
@@ -653,5 +1849,47 @@ static int optee_ioctl(FAR struct file *filep, int cmd, 
unsigned long arg)
 
 int optee_register(void)
 {
+#ifdef CONFIG_DEV_OPTEE_SMC64

Review Comment:
   it's always good to add more test, but we should put test code to 
apps/testing folder.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


gpoulios commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2073142076


##
drivers/misc/Kconfig:
##
@@ -43,31 +43,66 @@ choice
prompt "Select OP-TEE dev implementation"
default DEV_OPTEE_NONE
---help---
-   There are two implementations of optee server,
+   There are three implementations of optee server,
one is soft tee server which does not distinguish
between secure and non-secure states and uses socket 
communication,
-   and the other is teeos which runs in the secure state of
-   the cpu and uses rpmsg cross-core communication.
+   and the other two is teeos which runs in the secure state of
+   the cpu and uses either rpmsg cross-core communication or ARM64 
SMCs.
 
 config DEV_OPTEE_LOCAL
bool "OPTEE Local Socket Support"
depends on NET_LOCAL
+   depends on LIBC_MEMFD_SHMFS
depends on ALLOW_BSD_COMPONENTS
 
 config DEV_OPTEE_RPMSG
bool "OP-TEE RPMSG Socket Support"
depends on NET_RPMSG
+   depends on LIBC_MEMFD_SHMFS
+   depends on ALLOW_BSD_COMPONENTS
+
+config DEV_OPTEE_SMC64

Review Comment:
   Ok, as for the warning, I'm thinking of using all of the following:
   1. `#warning "..."` that will break build when warnings are treated as errors
   2. Kconfig help text warning
   3. Documentation
   
   Do you agree with (1) ?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


gpoulios commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2073142076


##
drivers/misc/Kconfig:
##
@@ -43,31 +43,66 @@ choice
prompt "Select OP-TEE dev implementation"
default DEV_OPTEE_NONE
---help---
-   There are two implementations of optee server,
+   There are three implementations of optee server,
one is soft tee server which does not distinguish
between secure and non-secure states and uses socket 
communication,
-   and the other is teeos which runs in the secure state of
-   the cpu and uses rpmsg cross-core communication.
+   and the other two is teeos which runs in the secure state of
+   the cpu and uses either rpmsg cross-core communication or ARM64 
SMCs.
 
 config DEV_OPTEE_LOCAL
bool "OPTEE Local Socket Support"
depends on NET_LOCAL
+   depends on LIBC_MEMFD_SHMFS
depends on ALLOW_BSD_COMPONENTS
 
 config DEV_OPTEE_RPMSG
bool "OP-TEE RPMSG Socket Support"
depends on NET_RPMSG
+   depends on LIBC_MEMFD_SHMFS
+   depends on ALLOW_BSD_COMPONENTS
+
+config DEV_OPTEE_SMC64

Review Comment:
   Ok, as for the warning, I'm thinking of:
   1. `#warning "..."` that will break build when warnings are treated as errors
   2. Kconfig help text warning
   3. Documentation
   
   Do you agree with (1) ?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


gpoulios commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2073066955


##
drivers/misc/optee.c:
##
@@ -653,5 +1849,47 @@ static int optee_ioctl(FAR struct file *filep, int cmd, 
unsigned long arg)
 
 int optee_register(void)
 {
+#ifdef CONFIG_DEV_OPTEE_SMC64

Review Comment:
   I take it you mean to remove `CONFIG_DEV_OPTEE_SMC64_TEST` entirely. No 
problem (I just thought since I wrote it, why not make it available for anyone 
else, but I will remove it).
   
   Though I must admit, I didn't get why it "isn't good". Code quality or just 
the fact that test code exists there?



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


xiaoxiang781216 commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2073050014


##
drivers/misc/Kconfig:
##
@@ -43,31 +43,66 @@ choice
prompt "Select OP-TEE dev implementation"
default DEV_OPTEE_NONE
---help---
-   There are two implementations of optee server,
+   There are three implementations of optee server,
one is soft tee server which does not distinguish
between secure and non-secure states and uses socket 
communication,
-   and the other is teeos which runs in the secure state of
-   the cpu and uses rpmsg cross-core communication.
+   and the other two is teeos which runs in the secure state of
+   the cpu and uses either rpmsg cross-core communication or ARM64 
SMCs.
 
 config DEV_OPTEE_LOCAL
bool "OPTEE Local Socket Support"
depends on NET_LOCAL
+   depends on LIBC_MEMFD_SHMFS
depends on ALLOW_BSD_COMPONENTS
 
 config DEV_OPTEE_RPMSG
bool "OP-TEE RPMSG Socket Support"
depends on NET_RPMSG
+   depends on LIBC_MEMFD_SHMFS
+   depends on ALLOW_BSD_COMPONENTS
+
+config DEV_OPTEE_SMC64

Review Comment:
   the similar code exist on arm32 arch, so it's better to not limit the change 
just for arm64. Maybe, you can warn the user arm32 isn't tested.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


xiaoxiang781216 commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2073048953


##
drivers/misc/optee.c:
##
@@ -653,5 +1849,47 @@ static int optee_ioctl(FAR struct file *filep, int cmd, 
unsigned long arg)
 
 int optee_register(void)
 {
+#ifdef CONFIG_DEV_OPTEE_SMC64

Review Comment:
   The basic check(version and capability) could be done in driver, but 
other(e.g. call TA) isn't good.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


xiaoxiang781216 commented on PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#issuecomment-2850275500

   > > it's better to split the change into the small and independent patch
   > 
   > Sure, will do that. I assume you mean multiple commits on the same PR, as 
opposed to multiple PRs (some of which might be non-functional for the SMC 
case).
   > 
   
   Yes, multiple commits in one PR.
   
   > > BTW, could you design an abstract interface for rpmsg/local socket and 
smc call
   > 
   > I could certainly split the backend into different files, but I doubt the 
interface can become any more abstract than:
   > 
   > ```
   > optee_do_call_with_arg(struct optee_priv_data *, struct optee_msg_arg *)
   > ```
   > 
   > Will do that.
   
   Thanks.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


xiaoxiang781216 commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2073045632


##
drivers/misc/optee.c:
##
@@ -36,165 +36,977 @@
 #include 
 #include 
 #include 
+#include 
+
+#ifdef CONFIG_ARCH_ADDRENV
+#  include 
+#  include 
+#  include 
+#endif
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+#  include 
+#  include 
+#  include 
+#  include 
+#  include 
+#  include "optee_smc.h"
+#endif
 
 #include "optee_msg.h"
 
-/
- *   The driver's main purpose is to support the porting of the open source
- * component optee_client (https://github.com/OP-TEE/optee_client) to NuttX.
- *   The basic function of the driver module is to convert
- * the REE application layer data and send it to the TEE through rpmsg.
- * TEE implementation is optee_os(https://github.com/OP-TEE/optee_os).
- /
+/
+ *   The driver's main purpose is to support the porting of the open source
+ * component optee_client (https://github.com/OP-TEE/optee_client) to NuttX.
+ *   The basic function of the driver module is to convert
+ * the REE application layer data and send it to the TEE through rpmsg.
+ * TEE implementation is optee_os(https://github.com/OP-TEE/optee_os).
+ /
+
+/
+ * Pre-processor Definitions
+ /
+
+/* Some GlobalPlatform error codes used in this driver */
+
+#define TEE_SUCCESS0x
+#define TEE_ERROR_BAD_PARAMETERS   0x0006
+#define TEE_ERROR_NOT_SUPPORTED0x000A
+#define TEE_ERROR_COMMUNICATION0x000E
+#define TEE_ERROR_OUT_OF_MEMORY0x000C
+#define TEE_ERROR_BUSY 0x000D
+#define TEE_ERROR_SHORT_BUFFER 0x0010
+
+#define TEE_ORIGIN_COMMS   0x0002
+
+#define OPTEE_MAX_IOVEC_NUM7
+#define OPTEE_MAX_PARAM_NUM6
+
+#define __round_mask(x, y) ((__typeof__(x))((y)-1))
+#define round_up(x, y) x)-1) | __round_mask(x, y))+1)
+#define round_down(x, y)   ((x) & ~__round_mask(x, y))
+#define DIV_ROUND_UP(n,d)  (((n) + (d) - 1) / (d))
+
+#define PAGELIST_ENTRIES_PER_PAGE \
+((OPTEE_MSG_NONCONTIG_PAGE_SIZE / sizeof(uint64_t)) - 1)
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+
+/* is there a nice way to include arm64_arch.h instead? */
+#  define SCTLR_C_BIT  BIT(2)
+
+#  ifdef CONFIG_DEV_OPTEE_SMC64_TEST
+#define PTA_DEVICE_ENUM{ 0x7011a688, 0xddde, 0x4053, \
+ 0xa5, 0xa9, \
+ { 0x7b, 0x3c, 0x4d, 0xdf, \
+   0x13, 0xb8 } }
+#define PTA_CMD_GET_DEVICES0x0
+#  endif
+#endif
+
+/
+ * Private Types
+ /
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+typedef void (optee_invoke_fn)(unsigned long, unsigned long, unsigned long,
+   unsigned long, unsigned long, unsigned long,
+   unsigned long, unsigned long,
+   struct arm64_smccc_res *);
+
+struct rpc_param
+{
+  uint32_t a0;
+  uint32_t a1;
+  uint32_t a2;
+  uint32_t a3;
+  uint32_t a4;
+  uint32_t a5;
+  uint32_t a6;
+  uint32_t a7;
+};
+
+union os_revision
+{
+  struct arm64_smccc_res smccc;
+  struct optee_smc_call_get_os_revision_result result;
+};
+#endif
+
+struct optee_priv_data;
+
+struct optee_shm_entry
+{
+  struct optee_priv_data *priv;
+  struct tee_ioctl_shm_register_data shm;
+  SLIST_ENTRY(optee_shm_entry) link;
+};
+
+SLIST_HEAD(optee_shm_head, optee_shm_entry);
+
+struct optee_priv_data
+{
+#ifdef CONFIG_DEV_OPTEE_SMC64
+  optee_invoke_fn *invoke_fn;
+#else
+  FAR struct socket *psock;
+#endif
+  struct optee_shm_head list_shm;
+};
+
+struct page_data
+{
+  uint64_t pages_list[PAGELIST_ENTRIES_PER_PAGE];
+  uint64_t next_page_data;
+};
+
+/
+ * Private Function Prototypes
+ /
+
+/* The file operation functions */
+
+static int optee_open(FAR struct file *filep);
+static int optee_close(FAR struct file *filep);
+static int optee_ioctl(FAR struct file *filep, int cmd,
+   unsigned long arg);
+
+/* Other forward-declared functions */
+
+static int optee_shm_add(FAR struct optee_priv_data *priv, uintptr_t align,
+ FAR void *addr

Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


xiaoxiang781216 commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2073044653


##
drivers/misc/optee.c:
##
@@ -36,165 +36,977 @@
 #include 
 #include 
 #include 
+#include 
+
+#ifdef CONFIG_ARCH_ADDRENV
+#  include 
+#  include 
+#  include 
+#endif
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+#  include 
+#  include 
+#  include 
+#  include 
+#  include 
+#  include "optee_smc.h"
+#endif
 
 #include "optee_msg.h"
 
-/
- *   The driver's main purpose is to support the porting of the open source
- * component optee_client (https://github.com/OP-TEE/optee_client) to NuttX.
- *   The basic function of the driver module is to convert
- * the REE application layer data and send it to the TEE through rpmsg.
- * TEE implementation is optee_os(https://github.com/OP-TEE/optee_os).
- /
+/
+ *   The driver's main purpose is to support the porting of the open source
+ * component optee_client (https://github.com/OP-TEE/optee_client) to NuttX.
+ *   The basic function of the driver module is to convert
+ * the REE application layer data and send it to the TEE through rpmsg.
+ * TEE implementation is optee_os(https://github.com/OP-TEE/optee_os).
+ /
+
+/
+ * Pre-processor Definitions
+ /
+
+/* Some GlobalPlatform error codes used in this driver */
+
+#define TEE_SUCCESS0x
+#define TEE_ERROR_BAD_PARAMETERS   0x0006
+#define TEE_ERROR_NOT_SUPPORTED0x000A
+#define TEE_ERROR_COMMUNICATION0x000E
+#define TEE_ERROR_OUT_OF_MEMORY0x000C
+#define TEE_ERROR_BUSY 0x000D
+#define TEE_ERROR_SHORT_BUFFER 0x0010
+
+#define TEE_ORIGIN_COMMS   0x0002
+
+#define OPTEE_MAX_IOVEC_NUM7
+#define OPTEE_MAX_PARAM_NUM6
+
+#define __round_mask(x, y) ((__typeof__(x))((y)-1))
+#define round_up(x, y) x)-1) | __round_mask(x, y))+1)
+#define round_down(x, y)   ((x) & ~__round_mask(x, y))
+#define DIV_ROUND_UP(n,d)  (((n) + (d) - 1) / (d))
+
+#define PAGELIST_ENTRIES_PER_PAGE \
+((OPTEE_MSG_NONCONTIG_PAGE_SIZE / sizeof(uint64_t)) - 1)
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+
+/* is there a nice way to include arm64_arch.h instead? */
+#  define SCTLR_C_BIT  BIT(2)
+
+#  ifdef CONFIG_DEV_OPTEE_SMC64_TEST
+#define PTA_DEVICE_ENUM{ 0x7011a688, 0xddde, 0x4053, \
+ 0xa5, 0xa9, \
+ { 0x7b, 0x3c, 0x4d, 0xdf, \
+   0x13, 0xb8 } }
+#define PTA_CMD_GET_DEVICES0x0
+#  endif
+#endif
+
+/
+ * Private Types
+ /
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+typedef void (optee_invoke_fn)(unsigned long, unsigned long, unsigned long,
+   unsigned long, unsigned long, unsigned long,
+   unsigned long, unsigned long,
+   struct arm64_smccc_res *);
+
+struct rpc_param
+{
+  uint32_t a0;
+  uint32_t a1;
+  uint32_t a2;
+  uint32_t a3;
+  uint32_t a4;
+  uint32_t a5;
+  uint32_t a6;
+  uint32_t a7;
+};
+
+union os_revision
+{
+  struct arm64_smccc_res smccc;
+  struct optee_smc_call_get_os_revision_result result;
+};
+#endif
+
+struct optee_priv_data;
+
+struct optee_shm_entry
+{
+  struct optee_priv_data *priv;
+  struct tee_ioctl_shm_register_data shm;
+  SLIST_ENTRY(optee_shm_entry) link;
+};
+
+SLIST_HEAD(optee_shm_head, optee_shm_entry);
+
+struct optee_priv_data
+{
+#ifdef CONFIG_DEV_OPTEE_SMC64
+  optee_invoke_fn *invoke_fn;
+#else
+  FAR struct socket *psock;
+#endif
+  struct optee_shm_head list_shm;
+};
+
+struct page_data
+{
+  uint64_t pages_list[PAGELIST_ENTRIES_PER_PAGE];
+  uint64_t next_page_data;
+};
+
+/
+ * Private Function Prototypes
+ /
+
+/* The file operation functions */
+
+static int optee_open(FAR struct file *filep);
+static int optee_close(FAR struct file *filep);
+static int optee_ioctl(FAR struct file *filep, int cmd,
+   unsigned long arg);
+
+/* Other forward-declared functions */
+
+static int optee_shm_add(FAR struct optee_priv_data *priv, uintptr_t align,
+ FAR void *addr

Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


gpoulios commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2072941504


##
drivers/misc/Kconfig:
##
@@ -43,31 +43,66 @@ choice
prompt "Select OP-TEE dev implementation"
default DEV_OPTEE_NONE
---help---
-   There are two implementations of optee server,
+   There are three implementations of optee server,
one is soft tee server which does not distinguish
between secure and non-secure states and uses socket 
communication,
-   and the other is teeos which runs in the secure state of
-   the cpu and uses rpmsg cross-core communication.
+   and the other two is teeos which runs in the secure state of
+   the cpu and uses either rpmsg cross-core communication or ARM64 
SMCs.
 
 config DEV_OPTEE_LOCAL
bool "OPTEE Local Socket Support"
depends on NET_LOCAL
+   depends on LIBC_MEMFD_SHMFS
depends on ALLOW_BSD_COMPONENTS
 
 config DEV_OPTEE_RPMSG
bool "OP-TEE RPMSG Socket Support"
depends on NET_RPMSG
+   depends on LIBC_MEMFD_SHMFS
+   depends on ALLOW_BSD_COMPONENTS
+
+config DEV_OPTEE_SMC64

Review Comment:
   I don't know and I have a dependency on arch/arm64 for `arm64_smccc_res` and 
other parts of the code, but the biggest reason I did it this way, is that I 
can't test arm32 (don't have a board/setup). Hence I opted for `_SMC64` and 
dependency on `ARCH_ARM64` to make things clear for everyone.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


gpoulios commented on PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#issuecomment-2850164875

   > it's better to split the change into the small and independent patch
   
   Sure, will do that. I assume you mean multiple commits on the same PR, as 
opposed to multiple PRs (some of which might be non-functional for the SMC 
case).
   
   > BTW, could you design an abstract interface for rpmsg/local socket and smc 
call
   
   I could certainly split the backend into different files, but I doubt the 
interface can become any more abstract than:
   
   ```
   optee_do_call_with_arg(struct optee_priv_data *, struct optee_msg_arg *)
   ```
   Will do that.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


gpoulios commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2072953422


##
drivers/misc/optee.c:
##
@@ -36,165 +36,977 @@
 #include 
 #include 
 #include 
+#include 
+
+#ifdef CONFIG_ARCH_ADDRENV
+#  include 
+#  include 
+#  include 
+#endif
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+#  include 
+#  include 
+#  include 
+#  include 
+#  include 
+#  include "optee_smc.h"
+#endif
 
 #include "optee_msg.h"
 
-/
- *   The driver's main purpose is to support the porting of the open source
- * component optee_client (https://github.com/OP-TEE/optee_client) to NuttX.
- *   The basic function of the driver module is to convert
- * the REE application layer data and send it to the TEE through rpmsg.
- * TEE implementation is optee_os(https://github.com/OP-TEE/optee_os).
- /
+/
+ *   The driver's main purpose is to support the porting of the open source
+ * component optee_client (https://github.com/OP-TEE/optee_client) to NuttX.
+ *   The basic function of the driver module is to convert
+ * the REE application layer data and send it to the TEE through rpmsg.
+ * TEE implementation is optee_os(https://github.com/OP-TEE/optee_os).
+ /
+
+/
+ * Pre-processor Definitions
+ /
+
+/* Some GlobalPlatform error codes used in this driver */
+
+#define TEE_SUCCESS0x
+#define TEE_ERROR_BAD_PARAMETERS   0x0006
+#define TEE_ERROR_NOT_SUPPORTED0x000A
+#define TEE_ERROR_COMMUNICATION0x000E
+#define TEE_ERROR_OUT_OF_MEMORY0x000C
+#define TEE_ERROR_BUSY 0x000D
+#define TEE_ERROR_SHORT_BUFFER 0x0010
+
+#define TEE_ORIGIN_COMMS   0x0002
+
+#define OPTEE_MAX_IOVEC_NUM7
+#define OPTEE_MAX_PARAM_NUM6
+
+#define __round_mask(x, y) ((__typeof__(x))((y)-1))
+#define round_up(x, y) x)-1) | __round_mask(x, y))+1)
+#define round_down(x, y)   ((x) & ~__round_mask(x, y))
+#define DIV_ROUND_UP(n,d)  (((n) + (d) - 1) / (d))
+
+#define PAGELIST_ENTRIES_PER_PAGE \
+((OPTEE_MSG_NONCONTIG_PAGE_SIZE / sizeof(uint64_t)) - 1)
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+
+/* is there a nice way to include arm64_arch.h instead? */
+#  define SCTLR_C_BIT  BIT(2)
+
+#  ifdef CONFIG_DEV_OPTEE_SMC64_TEST
+#define PTA_DEVICE_ENUM{ 0x7011a688, 0xddde, 0x4053, \
+ 0xa5, 0xa9, \
+ { 0x7b, 0x3c, 0x4d, 0xdf, \
+   0x13, 0xb8 } }
+#define PTA_CMD_GET_DEVICES0x0
+#  endif
+#endif
+
+/
+ * Private Types
+ /
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+typedef void (optee_invoke_fn)(unsigned long, unsigned long, unsigned long,
+   unsigned long, unsigned long, unsigned long,
+   unsigned long, unsigned long,
+   struct arm64_smccc_res *);
+
+struct rpc_param
+{
+  uint32_t a0;
+  uint32_t a1;
+  uint32_t a2;
+  uint32_t a3;
+  uint32_t a4;
+  uint32_t a5;
+  uint32_t a6;
+  uint32_t a7;
+};
+
+union os_revision
+{
+  struct arm64_smccc_res smccc;
+  struct optee_smc_call_get_os_revision_result result;
+};
+#endif
+
+struct optee_priv_data;
+
+struct optee_shm_entry
+{
+  struct optee_priv_data *priv;
+  struct tee_ioctl_shm_register_data shm;
+  SLIST_ENTRY(optee_shm_entry) link;
+};
+
+SLIST_HEAD(optee_shm_head, optee_shm_entry);
+
+struct optee_priv_data
+{
+#ifdef CONFIG_DEV_OPTEE_SMC64
+  optee_invoke_fn *invoke_fn;
+#else
+  FAR struct socket *psock;
+#endif
+  struct optee_shm_head list_shm;
+};
+
+struct page_data
+{
+  uint64_t pages_list[PAGELIST_ENTRIES_PER_PAGE];
+  uint64_t next_page_data;
+};
+
+/
+ * Private Function Prototypes
+ /
+
+/* The file operation functions */
+
+static int optee_open(FAR struct file *filep);
+static int optee_close(FAR struct file *filep);
+static int optee_ioctl(FAR struct file *filep, int cmd,
+   unsigned long arg);
+
+/* Other forward-declared functions */
+
+static int optee_shm_add(FAR struct optee_priv_data *priv, uintptr_t align,
+ FAR void *addr, size_

Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


gpoulios commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2072953422


##
drivers/misc/optee.c:
##
@@ -36,165 +36,977 @@
 #include 
 #include 
 #include 
+#include 
+
+#ifdef CONFIG_ARCH_ADDRENV
+#  include 
+#  include 
+#  include 
+#endif
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+#  include 
+#  include 
+#  include 
+#  include 
+#  include 
+#  include "optee_smc.h"
+#endif
 
 #include "optee_msg.h"
 
-/
- *   The driver's main purpose is to support the porting of the open source
- * component optee_client (https://github.com/OP-TEE/optee_client) to NuttX.
- *   The basic function of the driver module is to convert
- * the REE application layer data and send it to the TEE through rpmsg.
- * TEE implementation is optee_os(https://github.com/OP-TEE/optee_os).
- /
+/
+ *   The driver's main purpose is to support the porting of the open source
+ * component optee_client (https://github.com/OP-TEE/optee_client) to NuttX.
+ *   The basic function of the driver module is to convert
+ * the REE application layer data and send it to the TEE through rpmsg.
+ * TEE implementation is optee_os(https://github.com/OP-TEE/optee_os).
+ /
+
+/
+ * Pre-processor Definitions
+ /
+
+/* Some GlobalPlatform error codes used in this driver */
+
+#define TEE_SUCCESS0x
+#define TEE_ERROR_BAD_PARAMETERS   0x0006
+#define TEE_ERROR_NOT_SUPPORTED0x000A
+#define TEE_ERROR_COMMUNICATION0x000E
+#define TEE_ERROR_OUT_OF_MEMORY0x000C
+#define TEE_ERROR_BUSY 0x000D
+#define TEE_ERROR_SHORT_BUFFER 0x0010
+
+#define TEE_ORIGIN_COMMS   0x0002
+
+#define OPTEE_MAX_IOVEC_NUM7
+#define OPTEE_MAX_PARAM_NUM6
+
+#define __round_mask(x, y) ((__typeof__(x))((y)-1))
+#define round_up(x, y) x)-1) | __round_mask(x, y))+1)
+#define round_down(x, y)   ((x) & ~__round_mask(x, y))
+#define DIV_ROUND_UP(n,d)  (((n) + (d) - 1) / (d))
+
+#define PAGELIST_ENTRIES_PER_PAGE \
+((OPTEE_MSG_NONCONTIG_PAGE_SIZE / sizeof(uint64_t)) - 1)
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+
+/* is there a nice way to include arm64_arch.h instead? */
+#  define SCTLR_C_BIT  BIT(2)
+
+#  ifdef CONFIG_DEV_OPTEE_SMC64_TEST
+#define PTA_DEVICE_ENUM{ 0x7011a688, 0xddde, 0x4053, \
+ 0xa5, 0xa9, \
+ { 0x7b, 0x3c, 0x4d, 0xdf, \
+   0x13, 0xb8 } }
+#define PTA_CMD_GET_DEVICES0x0
+#  endif
+#endif
+
+/
+ * Private Types
+ /
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+typedef void (optee_invoke_fn)(unsigned long, unsigned long, unsigned long,
+   unsigned long, unsigned long, unsigned long,
+   unsigned long, unsigned long,
+   struct arm64_smccc_res *);
+
+struct rpc_param
+{
+  uint32_t a0;
+  uint32_t a1;
+  uint32_t a2;
+  uint32_t a3;
+  uint32_t a4;
+  uint32_t a5;
+  uint32_t a6;
+  uint32_t a7;
+};
+
+union os_revision
+{
+  struct arm64_smccc_res smccc;
+  struct optee_smc_call_get_os_revision_result result;
+};
+#endif
+
+struct optee_priv_data;
+
+struct optee_shm_entry
+{
+  struct optee_priv_data *priv;
+  struct tee_ioctl_shm_register_data shm;
+  SLIST_ENTRY(optee_shm_entry) link;
+};
+
+SLIST_HEAD(optee_shm_head, optee_shm_entry);
+
+struct optee_priv_data
+{
+#ifdef CONFIG_DEV_OPTEE_SMC64
+  optee_invoke_fn *invoke_fn;
+#else
+  FAR struct socket *psock;
+#endif
+  struct optee_shm_head list_shm;
+};
+
+struct page_data
+{
+  uint64_t pages_list[PAGELIST_ENTRIES_PER_PAGE];
+  uint64_t next_page_data;
+};
+
+/
+ * Private Function Prototypes
+ /
+
+/* The file operation functions */
+
+static int optee_open(FAR struct file *filep);
+static int optee_close(FAR struct file *filep);
+static int optee_ioctl(FAR struct file *filep, int cmd,
+   unsigned long arg);
+
+/* Other forward-declared functions */
+
+static int optee_shm_add(FAR struct optee_priv_data *priv, uintptr_t align,
+ FAR void *addr, size_

Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


gpoulios commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2072957899


##
drivers/misc/optee.c:
##
@@ -653,5 +1849,47 @@ static int optee_ioctl(FAR struct file *filep, int cmd, 
unsigned long arg)
 
 int optee_register(void)
 {
+#ifdef CONFIG_DEV_OPTEE_SMC64

Review Comment:
   Not really, the point of this was/is to test through kernel thread context, 
or otherwise without bringing up any apps/userspace.
   
   Btw I see you marked the block containing also the compatibility checks. 
Perhaps by accident? The compatibility checks I think are even more so needed 
there to avoid registering the driver altogether.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


gpoulios commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2072953422


##
drivers/misc/optee.c:
##
@@ -36,165 +36,977 @@
 #include 
 #include 
 #include 
+#include 
+
+#ifdef CONFIG_ARCH_ADDRENV
+#  include 
+#  include 
+#  include 
+#endif
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+#  include 
+#  include 
+#  include 
+#  include 
+#  include 
+#  include "optee_smc.h"
+#endif
 
 #include "optee_msg.h"
 
-/
- *   The driver's main purpose is to support the porting of the open source
- * component optee_client (https://github.com/OP-TEE/optee_client) to NuttX.
- *   The basic function of the driver module is to convert
- * the REE application layer data and send it to the TEE through rpmsg.
- * TEE implementation is optee_os(https://github.com/OP-TEE/optee_os).
- /
+/
+ *   The driver's main purpose is to support the porting of the open source
+ * component optee_client (https://github.com/OP-TEE/optee_client) to NuttX.
+ *   The basic function of the driver module is to convert
+ * the REE application layer data and send it to the TEE through rpmsg.
+ * TEE implementation is optee_os(https://github.com/OP-TEE/optee_os).
+ /
+
+/
+ * Pre-processor Definitions
+ /
+
+/* Some GlobalPlatform error codes used in this driver */
+
+#define TEE_SUCCESS0x
+#define TEE_ERROR_BAD_PARAMETERS   0x0006
+#define TEE_ERROR_NOT_SUPPORTED0x000A
+#define TEE_ERROR_COMMUNICATION0x000E
+#define TEE_ERROR_OUT_OF_MEMORY0x000C
+#define TEE_ERROR_BUSY 0x000D
+#define TEE_ERROR_SHORT_BUFFER 0x0010
+
+#define TEE_ORIGIN_COMMS   0x0002
+
+#define OPTEE_MAX_IOVEC_NUM7
+#define OPTEE_MAX_PARAM_NUM6
+
+#define __round_mask(x, y) ((__typeof__(x))((y)-1))
+#define round_up(x, y) x)-1) | __round_mask(x, y))+1)
+#define round_down(x, y)   ((x) & ~__round_mask(x, y))
+#define DIV_ROUND_UP(n,d)  (((n) + (d) - 1) / (d))
+
+#define PAGELIST_ENTRIES_PER_PAGE \
+((OPTEE_MSG_NONCONTIG_PAGE_SIZE / sizeof(uint64_t)) - 1)
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+
+/* is there a nice way to include arm64_arch.h instead? */
+#  define SCTLR_C_BIT  BIT(2)
+
+#  ifdef CONFIG_DEV_OPTEE_SMC64_TEST
+#define PTA_DEVICE_ENUM{ 0x7011a688, 0xddde, 0x4053, \
+ 0xa5, 0xa9, \
+ { 0x7b, 0x3c, 0x4d, 0xdf, \
+   0x13, 0xb8 } }
+#define PTA_CMD_GET_DEVICES0x0
+#  endif
+#endif
+
+/
+ * Private Types
+ /
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+typedef void (optee_invoke_fn)(unsigned long, unsigned long, unsigned long,
+   unsigned long, unsigned long, unsigned long,
+   unsigned long, unsigned long,
+   struct arm64_smccc_res *);
+
+struct rpc_param
+{
+  uint32_t a0;
+  uint32_t a1;
+  uint32_t a2;
+  uint32_t a3;
+  uint32_t a4;
+  uint32_t a5;
+  uint32_t a6;
+  uint32_t a7;
+};
+
+union os_revision
+{
+  struct arm64_smccc_res smccc;
+  struct optee_smc_call_get_os_revision_result result;
+};
+#endif
+
+struct optee_priv_data;
+
+struct optee_shm_entry
+{
+  struct optee_priv_data *priv;
+  struct tee_ioctl_shm_register_data shm;
+  SLIST_ENTRY(optee_shm_entry) link;
+};
+
+SLIST_HEAD(optee_shm_head, optee_shm_entry);
+
+struct optee_priv_data
+{
+#ifdef CONFIG_DEV_OPTEE_SMC64
+  optee_invoke_fn *invoke_fn;
+#else
+  FAR struct socket *psock;
+#endif
+  struct optee_shm_head list_shm;
+};
+
+struct page_data
+{
+  uint64_t pages_list[PAGELIST_ENTRIES_PER_PAGE];
+  uint64_t next_page_data;
+};
+
+/
+ * Private Function Prototypes
+ /
+
+/* The file operation functions */
+
+static int optee_open(FAR struct file *filep);
+static int optee_close(FAR struct file *filep);
+static int optee_ioctl(FAR struct file *filep, int cmd,
+   unsigned long arg);
+
+/* Other forward-declared functions */
+
+static int optee_shm_add(FAR struct optee_priv_data *priv, uintptr_t align,
+ FAR void *addr, size_

Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


gpoulios commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2072948032


##
drivers/misc/optee.c:
##
@@ -36,165 +36,977 @@
 #include 
 #include 
 #include 
+#include 
+
+#ifdef CONFIG_ARCH_ADDRENV
+#  include 
+#  include 
+#  include 
+#endif
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+#  include 
+#  include 
+#  include 
+#  include 
+#  include 
+#  include "optee_smc.h"
+#endif
 
 #include "optee_msg.h"
 
-/
- *   The driver's main purpose is to support the porting of the open source
- * component optee_client (https://github.com/OP-TEE/optee_client) to NuttX.
- *   The basic function of the driver module is to convert
- * the REE application layer data and send it to the TEE through rpmsg.
- * TEE implementation is optee_os(https://github.com/OP-TEE/optee_os).
- /
+/
+ *   The driver's main purpose is to support the porting of the open source
+ * component optee_client (https://github.com/OP-TEE/optee_client) to NuttX.
+ *   The basic function of the driver module is to convert
+ * the REE application layer data and send it to the TEE through rpmsg.
+ * TEE implementation is optee_os(https://github.com/OP-TEE/optee_os).
+ /
+
+/
+ * Pre-processor Definitions
+ /
+
+/* Some GlobalPlatform error codes used in this driver */
+
+#define TEE_SUCCESS0x
+#define TEE_ERROR_BAD_PARAMETERS   0x0006
+#define TEE_ERROR_NOT_SUPPORTED0x000A
+#define TEE_ERROR_COMMUNICATION0x000E
+#define TEE_ERROR_OUT_OF_MEMORY0x000C
+#define TEE_ERROR_BUSY 0x000D
+#define TEE_ERROR_SHORT_BUFFER 0x0010
+
+#define TEE_ORIGIN_COMMS   0x0002
+
+#define OPTEE_MAX_IOVEC_NUM7
+#define OPTEE_MAX_PARAM_NUM6
+
+#define __round_mask(x, y) ((__typeof__(x))((y)-1))
+#define round_up(x, y) x)-1) | __round_mask(x, y))+1)
+#define round_down(x, y)   ((x) & ~__round_mask(x, y))
+#define DIV_ROUND_UP(n,d)  (((n) + (d) - 1) / (d))
+
+#define PAGELIST_ENTRIES_PER_PAGE \
+((OPTEE_MSG_NONCONTIG_PAGE_SIZE / sizeof(uint64_t)) - 1)
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+
+/* is there a nice way to include arm64_arch.h instead? */
+#  define SCTLR_C_BIT  BIT(2)
+
+#  ifdef CONFIG_DEV_OPTEE_SMC64_TEST
+#define PTA_DEVICE_ENUM{ 0x7011a688, 0xddde, 0x4053, \
+ 0xa5, 0xa9, \
+ { 0x7b, 0x3c, 0x4d, 0xdf, \
+   0x13, 0xb8 } }
+#define PTA_CMD_GET_DEVICES0x0
+#  endif
+#endif
+
+/
+ * Private Types
+ /
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+typedef void (optee_invoke_fn)(unsigned long, unsigned long, unsigned long,
+   unsigned long, unsigned long, unsigned long,
+   unsigned long, unsigned long,
+   struct arm64_smccc_res *);
+
+struct rpc_param
+{
+  uint32_t a0;
+  uint32_t a1;
+  uint32_t a2;
+  uint32_t a3;
+  uint32_t a4;
+  uint32_t a5;
+  uint32_t a6;
+  uint32_t a7;
+};
+
+union os_revision
+{
+  struct arm64_smccc_res smccc;
+  struct optee_smc_call_get_os_revision_result result;
+};
+#endif
+
+struct optee_priv_data;
+
+struct optee_shm_entry
+{
+  struct optee_priv_data *priv;
+  struct tee_ioctl_shm_register_data shm;
+  SLIST_ENTRY(optee_shm_entry) link;
+};
+
+SLIST_HEAD(optee_shm_head, optee_shm_entry);
+
+struct optee_priv_data
+{
+#ifdef CONFIG_DEV_OPTEE_SMC64
+  optee_invoke_fn *invoke_fn;
+#else
+  FAR struct socket *psock;
+#endif
+  struct optee_shm_head list_shm;
+};
+
+struct page_data
+{
+  uint64_t pages_list[PAGELIST_ENTRIES_PER_PAGE];
+  uint64_t next_page_data;
+};
+
+/
+ * Private Function Prototypes
+ /
+
+/* The file operation functions */
+
+static int optee_open(FAR struct file *filep);
+static int optee_close(FAR struct file *filep);
+static int optee_ioctl(FAR struct file *filep, int cmd,
+   unsigned long arg);
+
+/* Other forward-declared functions */
+
+static int optee_shm_add(FAR struct optee_priv_data *priv, uintptr_t align,
+ FAR void *addr, size_

Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


gpoulios commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2072941504


##
drivers/misc/Kconfig:
##
@@ -43,31 +43,66 @@ choice
prompt "Select OP-TEE dev implementation"
default DEV_OPTEE_NONE
---help---
-   There are two implementations of optee server,
+   There are three implementations of optee server,
one is soft tee server which does not distinguish
between secure and non-secure states and uses socket 
communication,
-   and the other is teeos which runs in the secure state of
-   the cpu and uses rpmsg cross-core communication.
+   and the other two is teeos which runs in the secure state of
+   the cpu and uses either rpmsg cross-core communication or ARM64 
SMCs.
 
 config DEV_OPTEE_LOCAL
bool "OPTEE Local Socket Support"
depends on NET_LOCAL
+   depends on LIBC_MEMFD_SHMFS
depends on ALLOW_BSD_COMPONENTS
 
 config DEV_OPTEE_RPMSG
bool "OP-TEE RPMSG Socket Support"
depends on NET_RPMSG
+   depends on LIBC_MEMFD_SHMFS
+   depends on ALLOW_BSD_COMPONENTS
+
+config DEV_OPTEE_SMC64

Review Comment:
   I don't know and I have a dependencies on arch/arm64 for `arm64_smccc_res` 
and other parts of the code, but the biggest reason I did it this way, is that 
I can't test arm32 (don't have a board/setup). Hence I opted for `_SMC64` and 
dependency on `ARCH_ARM64` to make things clear for everyone.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-05 Thread via GitHub


xiaoxiang781216 commented on PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#issuecomment-2850099225

   > Apologies in advance for the messed up diff of `optee.c`, by the time I 
realized that git loses track, it was already too late to start breaking it up 
in shorter commits. If that's an issue, I could try breaking it up now.
   
   yes, it's better to split the change into the small and independent patch, 
which will help the maintainer to review the change.
   BTW, could you design an abstract interface for rpmsg/local socket and smc 
call?


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-04 Thread via GitHub


xiaoxiang781216 commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2072846960


##
drivers/misc/Kconfig:
##
@@ -43,31 +43,66 @@ choice
prompt "Select OP-TEE dev implementation"
default DEV_OPTEE_NONE
---help---
-   There are two implementations of optee server,
+   There are three implementations of optee server,
one is soft tee server which does not distinguish
between secure and non-secure states and uses socket 
communication,
-   and the other is teeos which runs in the secure state of
-   the cpu and uses rpmsg cross-core communication.
+   and the other two is teeos which runs in the secure state of
+   the cpu and uses either rpmsg cross-core communication or ARM64 
SMCs.
 
 config DEV_OPTEE_LOCAL
bool "OPTEE Local Socket Support"
depends on NET_LOCAL
+   depends on LIBC_MEMFD_SHMFS
depends on ALLOW_BSD_COMPONENTS
 
 config DEV_OPTEE_RPMSG
bool "OP-TEE RPMSG Socket Support"
depends on NET_RPMSG
+   depends on LIBC_MEMFD_SHMFS
+   depends on ALLOW_BSD_COMPONENTS
+
+config DEV_OPTEE_SMC64

Review Comment:
   since SMC also support for arm32, could we remove 64 or add both 32 and 64?



##
drivers/misc/optee.c:
##
@@ -36,165 +36,977 @@
 #include 
 #include 
 #include 
+#include 
+
+#ifdef CONFIG_ARCH_ADDRENV
+#  include 
+#  include 
+#  include 
+#endif
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+#  include 
+#  include 
+#  include 
+#  include 
+#  include 
+#  include "optee_smc.h"
+#endif
 
 #include "optee_msg.h"
 
-/
- *   The driver's main purpose is to support the porting of the open source
- * component optee_client (https://github.com/OP-TEE/optee_client) to NuttX.
- *   The basic function of the driver module is to convert
- * the REE application layer data and send it to the TEE through rpmsg.
- * TEE implementation is optee_os(https://github.com/OP-TEE/optee_os).
- /
+/
+ *   The driver's main purpose is to support the porting of the open source
+ * component optee_client (https://github.com/OP-TEE/optee_client) to NuttX.
+ *   The basic function of the driver module is to convert
+ * the REE application layer data and send it to the TEE through rpmsg.
+ * TEE implementation is optee_os(https://github.com/OP-TEE/optee_os).
+ /
+
+/
+ * Pre-processor Definitions
+ /
+
+/* Some GlobalPlatform error codes used in this driver */
+
+#define TEE_SUCCESS0x
+#define TEE_ERROR_BAD_PARAMETERS   0x0006
+#define TEE_ERROR_NOT_SUPPORTED0x000A
+#define TEE_ERROR_COMMUNICATION0x000E
+#define TEE_ERROR_OUT_OF_MEMORY0x000C
+#define TEE_ERROR_BUSY 0x000D
+#define TEE_ERROR_SHORT_BUFFER 0x0010
+
+#define TEE_ORIGIN_COMMS   0x0002
+
+#define OPTEE_MAX_IOVEC_NUM7
+#define OPTEE_MAX_PARAM_NUM6
+
+#define __round_mask(x, y) ((__typeof__(x))((y)-1))
+#define round_up(x, y) x)-1) | __round_mask(x, y))+1)
+#define round_down(x, y)   ((x) & ~__round_mask(x, y))
+#define DIV_ROUND_UP(n,d)  (((n) + (d) - 1) / (d))
+
+#define PAGELIST_ENTRIES_PER_PAGE \
+((OPTEE_MSG_NONCONTIG_PAGE_SIZE / sizeof(uint64_t)) - 1)
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+
+/* is there a nice way to include arm64_arch.h instead? */
+#  define SCTLR_C_BIT  BIT(2)
+
+#  ifdef CONFIG_DEV_OPTEE_SMC64_TEST
+#define PTA_DEVICE_ENUM{ 0x7011a688, 0xddde, 0x4053, \
+ 0xa5, 0xa9, \
+ { 0x7b, 0x3c, 0x4d, 0xdf, \
+   0x13, 0xb8 } }
+#define PTA_CMD_GET_DEVICES0x0
+#  endif
+#endif
+
+/
+ * Private Types
+ /
+
+#ifdef CONFIG_DEV_OPTEE_SMC64
+typedef void (optee_invoke_fn)(unsigned long, unsigned long, unsigned long,
+   unsigned long, unsigned long, unsigned long,
+   unsigned long, unsigned long,
+   struct arm64_smccc_res *);
+
+struct rpc_param
+{
+  uint32_t a0;
+  uint32_t a1;
+  uint32_t a2;
+  uint32_t a3;
+  uint32_t a4;
+  uint32_t a5;
+  uint32_t a6;
+  uint32_t a7;

Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-04 Thread via GitHub


gpoulios commented on PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#issuecomment-2849109650

   Okay, I will re-write those from scratch. If you don't mind keeping this 
open, I'll force-push here, otherwise let's close it and I'll make a new PR.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-04 Thread via GitHub


raiden00pl commented on PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#issuecomment-2849106636

   there are many pieces of code that are directly copied from 
https://github.com/u-boot/u-boot/blob/master/drivers/tee/optee/core.c which is: 
   ```
   SPDX-License-Identifier: GPL-2.0+
   ```
   
   This PR cannot be merged. We recently rejected this PR due to similar 
licence issues: https://github.com/apache/nuttx-apps/pull/2970
   When porting things to NuttX we can't relay on GPL code


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-04 Thread via GitHub


gpoulios commented on PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#issuecomment-2849087176

   > If you can, please add an initial Documentation/ about optee on NuttX, 
that is something that people will need to use it.
   
   Indeed, pushed some doco as well.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-04 Thread via GitHub


gpoulios commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2072557212


##
arch/arm64/include/syscall.h:
##
@@ -110,6 +110,50 @@
 #define ARM_SMCC_RES_A6   (6)
 #define ARM_SMCC_RES_A7   (7)
 
+#define ARM_SMCCC_STD_CALL0UL
+#define ARM_SMCCC_FAST_CALL   1UL
+#define ARM_SMCCC_TYPE_SHIFT  31
+
+#define ARM_SMCCC_SMC_32  0
+#define ARM_SMCCC_SMC_64  1
+#define ARM_SMCCC_CALL_CONV_SHIFT 30
+
+#define ARM_SMCCC_OWNER_MASK  0x3F
+#define ARM_SMCCC_OWNER_SHIFT 24
+
+#define ARM_SMCCC_FUNC_MASK   0x
+
+#define ARM_SMCCC_IS_FAST_CALL(smc_val) \
+  ((smc_val) & (ARM_SMCCC_FAST_CALL << ARM_SMCCC_TYPE_SHIFT))
+#define ARM_SMCCC_IS_64(smc_val) \
+  ((smc_val) & (ARM_SMCCC_SMC_64 << ARM_SMCCC_CALL_CONV_SHIFT))
+#define ARM_SMCCC_FUNC_NUM(smc_val)  ((smc_val) & ARM_SMCCC_FUNC_MASK)
+#define ARM_SMCCC_OWNER_NUM(smc_val) \
+  (((smc_val) >> ARM_SMCCC_OWNER_SHIFT) & ARM_SMCCC_OWNER_MASK)
+
+#define ARM_SMCCC_CALL_VAL(type, calling_convention, owner, func_num) \
+  (((type) << ARM_SMCCC_TYPE_SHIFT) | \
+  ((calling_convention) << ARM_SMCCC_CALL_CONV_SHIFT) | \
+  (((owner) & ARM_SMCCC_OWNER_MASK) << ARM_SMCCC_OWNER_SHIFT) | \
+  ((func_num) & ARM_SMCCC_FUNC_MASK))
+
+#define ARM_SMCCC_OWNER_ARCH   0
+#define ARM_SMCCC_OWNER_CPU1
+#define ARM_SMCCC_OWNER_SIP2
+#define ARM_SMCCC_OWNER_OEM3
+#define ARM_SMCCC_OWNER_STANDARD 4
+#define ARM_SMCCC_OWNER_TRUSTED_APP 48
+#define ARM_SMCCC_OWNER_TRUSTED_APP_END 49
+#define ARM_SMCCC_OWNER_TRUSTED_OS 50
+#define ARM_SMCCC_OWNER_TRUSTED_OS_END 63

Review Comment:
   Done



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-03 Thread via GitHub


acassis commented on PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#issuecomment-2848754528

   Nice work @gpoulios ! If you can, please add an initial Documentation/ about 
optee on NuttX, that is something that people will need to use it.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-03 Thread via GitHub


acassis commented on code in PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#discussion_r2072445096


##
arch/arm64/include/syscall.h:
##
@@ -110,6 +110,50 @@
 #define ARM_SMCC_RES_A6   (6)
 #define ARM_SMCC_RES_A7   (7)
 
+#define ARM_SMCCC_STD_CALL0UL
+#define ARM_SMCCC_FAST_CALL   1UL
+#define ARM_SMCCC_TYPE_SHIFT  31
+
+#define ARM_SMCCC_SMC_32  0
+#define ARM_SMCCC_SMC_64  1
+#define ARM_SMCCC_CALL_CONV_SHIFT 30
+
+#define ARM_SMCCC_OWNER_MASK  0x3F
+#define ARM_SMCCC_OWNER_SHIFT 24
+
+#define ARM_SMCCC_FUNC_MASK   0x
+
+#define ARM_SMCCC_IS_FAST_CALL(smc_val) \
+  ((smc_val) & (ARM_SMCCC_FAST_CALL << ARM_SMCCC_TYPE_SHIFT))
+#define ARM_SMCCC_IS_64(smc_val) \
+  ((smc_val) & (ARM_SMCCC_SMC_64 << ARM_SMCCC_CALL_CONV_SHIFT))
+#define ARM_SMCCC_FUNC_NUM(smc_val)  ((smc_val) & ARM_SMCCC_FUNC_MASK)
+#define ARM_SMCCC_OWNER_NUM(smc_val) \
+  (((smc_val) >> ARM_SMCCC_OWNER_SHIFT) & ARM_SMCCC_OWNER_MASK)
+
+#define ARM_SMCCC_CALL_VAL(type, calling_convention, owner, func_num) \
+  (((type) << ARM_SMCCC_TYPE_SHIFT) | \
+  ((calling_convention) << ARM_SMCCC_CALL_CONV_SHIFT) | \
+  (((owner) & ARM_SMCCC_OWNER_MASK) << ARM_SMCCC_OWNER_SHIFT) | \
+  ((func_num) & ARM_SMCCC_FUNC_MASK))
+
+#define ARM_SMCCC_OWNER_ARCH   0
+#define ARM_SMCCC_OWNER_CPU1
+#define ARM_SMCCC_OWNER_SIP2
+#define ARM_SMCCC_OWNER_OEM3
+#define ARM_SMCCC_OWNER_STANDARD 4
+#define ARM_SMCCC_OWNER_TRUSTED_APP 48
+#define ARM_SMCCC_OWNER_TRUSTED_APP_END 49
+#define ARM_SMCCC_OWNER_TRUSTED_OS 50
+#define ARM_SMCCC_OWNER_TRUSTED_OS_END 63

Review Comment:
   Please align the numbers, they all should be at same column aligned to "49"



##
arch/arm64/include/syscall.h:
##
@@ -110,6 +110,50 @@
 #define ARM_SMCC_RES_A6   (6)
 #define ARM_SMCC_RES_A7   (7)
 
+#define ARM_SMCCC_STD_CALL0UL
+#define ARM_SMCCC_FAST_CALL   1UL
+#define ARM_SMCCC_TYPE_SHIFT  31
+
+#define ARM_SMCCC_SMC_32  0
+#define ARM_SMCCC_SMC_64  1
+#define ARM_SMCCC_CALL_CONV_SHIFT 30
+
+#define ARM_SMCCC_OWNER_MASK  0x3F
+#define ARM_SMCCC_OWNER_SHIFT 24
+
+#define ARM_SMCCC_FUNC_MASK   0x
+
+#define ARM_SMCCC_IS_FAST_CALL(smc_val) \
+  ((smc_val) & (ARM_SMCCC_FAST_CALL << ARM_SMCCC_TYPE_SHIFT))
+#define ARM_SMCCC_IS_64(smc_val) \
+  ((smc_val) & (ARM_SMCCC_SMC_64 << ARM_SMCCC_CALL_CONV_SHIFT))
+#define ARM_SMCCC_FUNC_NUM(smc_val)  ((smc_val) & ARM_SMCCC_FUNC_MASK)
+#define ARM_SMCCC_OWNER_NUM(smc_val) \
+  (((smc_val) >> ARM_SMCCC_OWNER_SHIFT) & ARM_SMCCC_OWNER_MASK)
+
+#define ARM_SMCCC_CALL_VAL(type, calling_convention, owner, func_num) \
+  (((type) << ARM_SMCCC_TYPE_SHIFT) | \
+  ((calling_convention) << ARM_SMCCC_CALL_CONV_SHIFT) | \
+  (((owner) & ARM_SMCCC_OWNER_MASK) << ARM_SMCCC_OWNER_SHIFT) | \
+  ((func_num) & ARM_SMCCC_FUNC_MASK))
+
+#define ARM_SMCCC_OWNER_ARCH   0
+#define ARM_SMCCC_OWNER_CPU1
+#define ARM_SMCCC_OWNER_SIP2
+#define ARM_SMCCC_OWNER_OEM3
+#define ARM_SMCCC_OWNER_STANDARD 4
+#define ARM_SMCCC_OWNER_TRUSTED_APP 48
+#define ARM_SMCCC_OWNER_TRUSTED_APP_END 49
+#define ARM_SMCCC_OWNER_TRUSTED_OS 50
+#define ARM_SMCCC_OWNER_TRUSTED_OS_END 63
+
+#define ARM_SMCCC_QUIRK_NONE   0
+#define ARM_SMCCC_QUIRK_QCOM_A6 1 /* Save/restore register a6 */

Review Comment:
   align



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]

2025-05-03 Thread via GitHub


gpoulios commented on PR #16309:
URL: https://github.com/apache/nuttx/pull/16309#issuecomment-2848734568

   Apologies in advance for the messed up diff of `optee.c`, by the time I 
realized that git loses track, it was already too late to start breaking it up 
in shorter commits. If that's an issue, I could try breaking it up.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]