Re: [PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
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]
[PR] drivers/misc/optee.c: Add an SMC64 driver for arm64 [nuttx]
gpoulios opened a new pull request, #16309:
URL: https://github.com/apache/nuttx/pull/16309
## Summary
So far NuttX has supported OP-TEE interfacing over local and RPMsg sockets.
This PR introduces support for direct invocation of OP-TEE through arm64 SMCs.
The code is largely inspired by U-Boot's OP-TEE driver. SMC64 support is
enabled through `CONFIG_DEV_OPTEE_SMC64`.
The IOCTL interface remains the same except for the addition of
`TEE_IOC_SHM_REGISTER`, only for the newly introduced SMC driver. The added
ioctl enables the registration of dynamically allocated shared memory to the
driver (for automatic cleanup) and to OP-TEE (for use in invocations).
The PR introduces also ioctl argument memory checks to prevent the user from
specifying virtual addresses not belonging to the caller user task. The checks
are performed on all OP-TEE drivers implemented in NuttX, not just SMC.
## Impact
For users with no `CONFIG_DEV_OPTEE_*` configuration:
- No impact / no related changes
For users of `CONFIG_DEV_OPTEE_{LOCAL, RPMSG}`:
- No impact / same interface / with related changes
- Under the hood, the following changes (not affecting the interfacing):
- OP-TEE message arguments are now allocated on the kernel heap instead
of the stack (see `get_msg_arg()`) to the same effect.
- Memory used for IOCTL arguments provided by the user task is checked
for sanity. User tasks in builds with `CONFIG_ARCH_ADDRENV` are not allowed to
specify memory they don't own. Kernel thread invocations of the IOCTL interface
are not affected.
- Minor changes to the code that should not alter functionality in any
way. E.g.:
- Wrapping of the driver's socket reference into a new private data
struct (`optee_priv_data`).
- Wrapping of the call to `optee_send_recv()` into a new function
`optee_do_call_with_arg()`.
- `ret` codes checked against negative value instead of non-zero for
calls that are only expected to return negative error codes such as
`optee_send_recv()` and `optee_recv()`.
For users of `CONFIG_DEV_OPTEE_SMC64` on arm64 platforms running OP-TEE OS
Secure World:
- Ability to use native SMCs to directly invoke OP-TEE TAs.
## Testing
This has been tested on NXP i.MX93 EVK running:
- TF-A: version v2.10.0, tag `lf-6.6.52_2.2.0`
- OP-TEE OS: revision 4.4 (60beb308810f9561), tag `lf-6.6.52_2.2.0`
- NuttX configs `imx93-evk:nsh` and `imx93-evk:knsh` with the following
additional configuration:
- ```
CONFIG_ALLOW_BSD_COMPONENTS=y
CONFIG_ARCH_VMA_MAPPING=y (only on knsh)
CONFIG_ARCH_SHM_NPAGES=4096
CONFIG_ARCH_SHM_VBASE=0xFF00
CONFIG_DEV_OPTEE_SMC64=y
CONFIG_DEV_OPTEE_SMC64_TEST=y
CONFIG_FS_SHMFS=y
CONFIG_LIBC_MEMFD_SHMFS=y
CONFIG_EXAMPLES_OPTEE=y
```
- and this app: https://github.com/apache/nuttx-apps/pull/3069
Driver logs with OP-TEE debug build (NuttX side prefixed with `OP-TEE`):
```
OP-TEE: revision 4.4 (60beb308810f9561)
I/TC: Reserved shared memory is enabled
I/TC: Dynamic shared memory is enabled
I/TC: Normal World virtualization support is disabled
I/TC: Asynchronous notifications are disabled
OP-TEE: compatibility check complete
D/TC:0 0 core_mmu_xlat_table_alloc:528 xlat tables used 6 / 8
D/TC:? 0 tee_ta_init_pseudo_ta_session:303 Lookup pseudo TA
7011a688-ddde-4053-a5a9-7b3c4ddf13b8
D/TC:? 0 tee_ta_init_pseudo_ta_session:315 Open device.pta
D/TC:? 0 tee_ta_init_pseudo_ta_session:330 device.pta :
7011a688-ddde-4053-a5a9-7b3c4ddf13b8
F/TC:? 0 plat_prng_add_jitter_entropy:68 0x81D8
OP-TEE: optee_ioctl_open_session(pta_device): 0, session: 1
OP-TEE: optee_ioctl_invoke(PTA_CMD_GET_DEVICES, NULL): 0, size needed: 32
OP-TEE: optee_shm_alloc(32): 0
OP-TEE: optee_ioctl_invoke(PTA_CMD_GET_DEVICES, 0x80364878): 0, size: 32
OP-TEE: TA: d96a5b40-c3e5-21e3-8794-1002a5d5c61b
OP-TEE: TA: f04a0fe7-1f5d-4b9b-abf7-619b85b4ce8c
D/TC:? 0 tee_ta_close_session:460 csess 0x68a98f50 id 1
D/TC:? 0 tee_ta_close_session:479 Destroy session
OP-TEE: optee_ioctl_close_session(pta_device): 0
```
Example NuttX app (`CONFIG_EXAMPLES_OPTEE`) logs with OP-TEE debug build:
```
impl id: 1, impl caps: 1, gen caps: 9
D/TC:? 0 tee_ta_init_session_with_context:557 Re-open trusted service
7011a688-ddde-4053-a5a9-7b3c4ddf13b8
D/TC:? 0 tee_ta_close_session:460 csess 0x68a98da0 id 1
D/TC:? 0 tee_ta_close_session:479 Destroy session
Available devices:
d96a5b40-c3e5-21e3-8794-1002a5d5c61b
f04a0fe7-1f5d-4b9b-abf7-619b85b4ce8c
```
Unfortunately, I don't have a suitable device/setup to test back
`CONFIG_DEV_OPTEE_{LOCAL, RPMSG}` but I would be happy to make amendments if
someone can spot an issue. I have tested they both build fine and work up to
the call to `psock_connect(..)`. I have done my best to leave
