[v2,2/2] firmware: arm_scmi: optee: use optee system invocation

Message ID 20230214152047.1143106-2-etienne.carriere@linaro.org
State New
Headers
Series [v2,1/2] tee: system invocation |

Commit Message

Etienne Carriere Feb. 14, 2023, 3:20 p.m. UTC
  Changes SCMI optee transport to request an OP-TEE system invocation
context per SCMI channel. This prevents possible deadlock when many
Linux clients invoke OP-TEE are all suspended waiting for an OP-TEE
RPC request access an SCMI resource through the SCMI OP-TEE PTA service.

Signed-off-by: Etienne Carriere <etienne.carriere@linaro.org>
--
Changes since v1:
- Updated to use new tee API functions tee_client_request_system_context()
  and tee_client_release_system_context().
---
 drivers/firmware/arm_scmi/optee.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)
  

Comments

kernel test robot Feb. 15, 2023, 9:44 a.m. UTC | #1
Hi Etienne,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.2-rc8]
[cannot apply to soc/for-next next-20230215]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Etienne-Carriere/firmware-arm_scmi-optee-use-optee-system-invocation/20230214-233230
patch link:    https://lore.kernel.org/r/20230214152047.1143106-2-etienne.carriere%40linaro.org
patch subject: [PATCH v2 2/2] firmware: arm_scmi: optee: use optee system invocation
config: arm-allyesconfig
compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/7a2f7bc17be8c4a18d32c439458d2f0f99e18cd6
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Etienne-Carriere/firmware-arm_scmi-optee-use-optee-system-invocation/20230214-233230
        git checkout 7a2f7bc17be8c4a18d32c439458d2f0f99e18cd6
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202302151710.KAq1hph3-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/firmware/arm_scmi/optee.c: In function 'scmi_optee_chan_setup':
>> drivers/firmware/arm_scmi/optee.c:436:77: error: expected ';' before 'if'
     436 |         ret = tee_client_request_system_context(scmi_optee_private->tee_ctx)
         |                                                                             ^
         |                                                                             ;
     437 |         if (ret)
         |         ~~                                                                   
>> drivers/firmware/arm_scmi/optee.c:439:9: error: 'else' without a previous 'if'
     439 |         else
         |         ^~~~
>> drivers/firmware/arm_scmi/optee.c:470:79: error: expected ';' before 'return'
     470 |                 tee_client_release_system_context(scmi_optee_private->tee_ctx)
         |                                                                               ^
         |                                                                               ;
     471 | 
     472 |         return ret;
         |         ~~~~~~                                                                 
   drivers/firmware/arm_scmi/optee.c: In function 'scmi_optee_chan_free':
   drivers/firmware/arm_scmi/optee.c:497:79: error: expected ';' before 'return'
     497 |                 tee_client_release_system_context(scmi_optee_private->tee_ctx)
         |                                                                               ^
         |                                                                               ;
     498 | 
     499 |         return 0;
         |         ~~~~~~                                                                 
   drivers/firmware/arm_scmi/optee.c:500:1: error: no return statement in function returning non-void [-Werror=return-type]
     500 | }
         | ^
   drivers/firmware/arm_scmi/optee.c: In function 'scmi_optee_chan_setup':
   drivers/firmware/arm_scmi/optee.c:473:1: error: control reaches end of non-void function [-Werror=return-type]
     473 | }
         | ^
   cc1: some warnings being treated as errors


vim +436 drivers/firmware/arm_scmi/optee.c

   412	
   413	static int scmi_optee_chan_setup(struct scmi_chan_info *cinfo, struct device *dev, bool tx)
   414	{
   415		struct scmi_optee_channel *channel;
   416		uint32_t channel_id;
   417		int ret;
   418	
   419		if (!tx)
   420			return -ENODEV;
   421	
   422		channel = devm_kzalloc(dev, sizeof(*channel), GFP_KERNEL);
   423		if (!channel)
   424			return -ENOMEM;
   425	
   426		ret = of_property_read_u32_index(cinfo->dev->of_node, "linaro,optee-channel-id",
   427						 0, &channel_id);
   428		if (ret)
   429			return ret;
   430	
   431		cinfo->transport_info = channel;
   432		channel->cinfo = cinfo;
   433		channel->channel_id = channel_id;
   434		mutex_init(&channel->mu);
   435	
 > 436		ret = tee_client_request_system_context(scmi_optee_private->tee_ctx)
   437		if (ret)
   438			dev_warn(dev, "Couldn't provision an OP-TEE system context\n");
 > 439		else
   440			channel->sys_thread = true;
   441	
   442		ret = setup_shmem(dev, cinfo, channel);
   443		if (ret)
   444			goto err_release_sysctx;
   445	
   446		ret = open_session(scmi_optee_private, &channel->tee_session);
   447		if (ret)
   448			goto err_free_shm;
   449	
   450		ret = get_channel(channel);
   451		if (ret)
   452			goto err_close_sess;
   453	
   454		/* Enable polling */
   455		cinfo->no_completion_irq = true;
   456	
   457		mutex_lock(&scmi_optee_private->mu);
   458		list_add(&channel->link, &scmi_optee_private->channel_list);
   459		mutex_unlock(&scmi_optee_private->mu);
   460	
   461		return 0;
   462	
   463	err_close_sess:
   464		close_session(scmi_optee_private, channel->tee_session);
   465	err_free_shm:
   466		if (channel->tee_shm)
   467			tee_shm_free(channel->tee_shm);
   468	err_release_sysctx:
   469		if (channel->sys_thread)
 > 470			tee_client_release_system_context(scmi_optee_private->tee_ctx)
   471	
   472		return ret;
   473	}
   474
  

Patch

diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c
index 2a7aeab40e54..fe91e2de3f9c 100644
--- a/drivers/firmware/arm_scmi/optee.c
+++ b/drivers/firmware/arm_scmi/optee.c
@@ -119,6 +119,7 @@  struct scmi_optee_channel {
 	u32 tee_session;
 	u32 caps;
 	u32 rx_len;
+	bool sys_thread;
 	struct mutex mu;
 	struct scmi_chan_info *cinfo;
 	union {
@@ -432,9 +433,15 @@  static int scmi_optee_chan_setup(struct scmi_chan_info *cinfo, struct device *de
 	channel->channel_id = channel_id;
 	mutex_init(&channel->mu);
 
+	ret = tee_client_request_system_context(scmi_optee_private->tee_ctx)
+	if (ret)
+		dev_warn(dev, "Couldn't provision an OP-TEE system context\n");
+	else
+		channel->sys_thread = true;
+
 	ret = setup_shmem(dev, cinfo, channel);
 	if (ret)
-		return ret;
+		goto err_release_sysctx;
 
 	ret = open_session(scmi_optee_private, &channel->tee_session);
 	if (ret)
@@ -458,6 +465,9 @@  static int scmi_optee_chan_setup(struct scmi_chan_info *cinfo, struct device *de
 err_free_shm:
 	if (channel->tee_shm)
 		tee_shm_free(channel->tee_shm);
+err_release_sysctx:
+	if (channel->sys_thread)
+		tee_client_release_system_context(scmi_optee_private->tee_ctx)
 
 	return ret;
 }
@@ -483,6 +493,9 @@  static int scmi_optee_chan_free(int id, void *p, void *data)
 
 	scmi_free_channel(cinfo, data, id);
 
+	if (channel->sys_thread)
+		tee_client_release_system_context(scmi_optee_private->tee_ctx)
+
 	return 0;
 }