From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 19159C433F5 for ; Fri, 22 Apr 2022 12:58:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=ZQcuI0qvF/nZvBWG5DCHkljBdBA70TsXvDsxtTrMSCA=; b=gpuN0rgrXN90I1 w2FciVDWIM5h2g6ttzwTTbqQRJHOFhVFVpLlg2espxIfanzNT6SSQ1ImmYXx4RtqAbxcSSa72j+4y 8ledS+Ld563ePywSY+VVM8tLHM3jedEqaSSl3FDaNYEvrb0VZ/rTI080fZwduopRCFyjZ1IQwaSPG /b+KLv1NFQBUnJy8+QaJR0/6IRAq4S09cURsfQ0J8+7pBrYsBpCgLblSwvbKNd3GS7FoZJTAAQRo/ kO1N45HNNAVBLNig1+hsIkn2kdJ+TwpSdNS9DkrI9avrckcsowf9gJ+4oPRDzVsGpurUPWNkVnH9x +oS9hlagjirQNCcfCXSw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nhsqW-000VK7-JF; Fri, 22 Apr 2022 12:57:12 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nhsqT-000VJ4-38 for linux-arm-kernel@lists.infradead.org; Fri, 22 Apr 2022 12:57:11 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 5FABA1595; Fri, 22 Apr 2022 05:57:02 -0700 (PDT) Received: from e120937-lin (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 91F6C3F5A1; Fri, 22 Apr 2022 05:57:01 -0700 (PDT) Date: Fri, 22 Apr 2022 13:56:55 +0100 From: Cristian Marussi To: Etienne Carriere Cc: linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Sudeep Holla , Vincent Guittot Subject: Re: [PATCH v2] firmware: arm_scmi: support optee shared memory in optee transport Message-ID: References: <20220421074935.7549-1-etienne.carriere@linaro.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220421074935.7549-1-etienne.carriere@linaro.org> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220422_055709_269909_D31DEF3E X-CRM114-Status: GOOD ( 32.28 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Thu, Apr 21, 2022 at 09:49:35AM +0200, Etienne Carriere wrote: > Adds support for tee shared memory in optee scmi transport. When using > tee shared memory, scmi optee transport manages SCMI messages using > msg protocol (from msg.c) in shared memory, whereas smt (from shmem.c) > protocol is used with static IOMEM shared buffers. > > Signed-off-by: Etienne Carriere > Hi Etienne, the bot complaints on linking failure are due to a missing dependency: diff --git a/drivers/firmware/arm_scmi/Kconfig b/drivers/firmware/arm_scmi/Kconfig index 7794bd41eaa0..1e7b7fec97d9 100644 --- a/drivers/firmware/arm_scmi/Kconfig +++ b/drivers/firmware/arm_scmi/Kconfig @@ -59,6 +59,7 @@ config ARM_SCMI_TRANSPORT_OPTEE depends on OPTEE=y || OPTEE=ARM_SCMI_PROTOCOL select ARM_SCMI_HAVE_TRANSPORT select ARM_SCMI_HAVE_SHMEM + select ARM_SCMI_HAVE_MSG default y help This enables the OP-TEE service based transport for SCMI. needed when you compile without any other transport using MSGs. One remark down below. > drivers/firmware/arm_scmi/optee.c | 128 ++++++++++++++++++++++++------ > 1 file changed, 104 insertions(+), 24 deletions(-) > > diff --git a/drivers/firmware/arm_scmi/optee.c b/drivers/firmware/arm_scmi/optee.c > index 8302a2b4aeeb..6676b17d2587 100644 > --- a/drivers/firmware/arm_scmi/optee.c > +++ b/drivers/firmware/arm_scmi/optee.c > @@ -64,6 +64,22 @@ enum scmi_optee_pta_cmd { > * [in] value[0].b: Requested capabilities mask (enum pta_scmi_caps) > */ > PTA_SCMI_CMD_GET_CHANNEL = 3, > + > + /* > + * PTA_SCMI_CMD_PROCESS_MSG_CHANNEL - Process SCMI message in a MSG > + * buffer pointed by memref parameters > + * > + * [in] value[0].a: Channel handle > + * [in] memref[1]: Message buffer (MSG and SCMI payload) > + * [out] memref[2]: Response buffer (MSG and SCMI payload) > + * > + * Shared memories used for SCMI message/response are MSG buffers > + * referenced by param[1] and param[2]. MSG transport protocol > + * uses a 32bit header to carry SCMI meta-data (protocol ID and > + * protocol message ID) followed by the effective SCMI message > + * payload. > + */ > + PTA_SCMI_CMD_PROCESS_MSG_CHANNEL = 4, > }; > > /* > @@ -72,9 +88,17 @@ enum scmi_optee_pta_cmd { > * PTA_SCMI_CAPS_SMT_HEADER > * When set, OP-TEE supports command using SMT header protocol (SCMI shmem) in > * shared memory buffers to carry SCMI protocol synchronisation information. > + * > + * PTA_SCMI_CAPS_MSG_HEADER > + * When set, OP-TEE supports command using MSG header protocol in an OP-TEE > + * shared memory to carry SCMI protocol synchronisation information and SCMI > + * message payload. > */ > #define PTA_SCMI_CAPS_NONE 0 > #define PTA_SCMI_CAPS_SMT_HEADER BIT(0) > +#define PTA_SCMI_CAPS_MSG_HEADER BIT(1) > +#define PTA_SCMI_CAPS_MASK (PTA_SCMI_CAPS_SMT_HEADER | \ > + PTA_SCMI_CAPS_MSG_HEADER) > > /** > * struct scmi_optee_channel - Description of an OP-TEE SCMI channel > @@ -85,7 +109,8 @@ enum scmi_optee_pta_cmd { > * @mu: Mutex protection on channel access > * @cinfo: SCMI channel information > * @shmem: Virtual base address of the shared memory > - * @tee_shm: Reference to TEE shared memory or NULL if using static shmem > + * @req: Shared memory protocol handle for SCMI request and synchronous response > + * @tee_shm: TEE shared memory handle @req or NULL if using IOMEM shmem > * @link: Reference in agent's channel list > */ > struct scmi_optee_channel { > @@ -94,7 +119,10 @@ struct scmi_optee_channel { > u32 caps; > struct mutex mu; > struct scmi_chan_info *cinfo; > - struct scmi_shared_mem __iomem *shmem; > + union { > + struct scmi_shared_mem __iomem *shmem; > + struct scmi_msg_payld *msg; > + } req; > struct tee_shm *tee_shm; > struct list_head link; > }; > @@ -178,8 +206,8 @@ static int get_capabilities(struct scmi_optee_agent *agent) > > caps = param[0].u.value.a; > > - if (!(caps & PTA_SCMI_CAPS_SMT_HEADER)) { > - dev_err(agent->dev, "OP-TEE SCMI PTA doesn't support SMT\n"); > + if (!(caps & (PTA_SCMI_CAPS_SMT_HEADER | PTA_SCMI_CAPS_MSG_HEADER))) { > + dev_err(agent->dev, "OP-TEE SCMI PTA doesn't support SMT and MSG\n"); > return -EOPNOTSUPP; > } > > @@ -193,9 +221,14 @@ static int get_channel(struct scmi_optee_channel *channel) > struct device *dev = scmi_optee_private->dev; > struct tee_ioctl_invoke_arg arg = { }; > struct tee_param param[1] = { }; > - unsigned int caps = PTA_SCMI_CAPS_SMT_HEADER; > + unsigned int caps = 0; > int ret; > > + if (channel->tee_shm) > + caps = PTA_SCMI_CAPS_MSG_HEADER; > + else > + caps = PTA_SCMI_CAPS_SMT_HEADER; > + > arg.func = PTA_SCMI_CMD_GET_CHANNEL; > arg.session = channel->tee_session; > arg.num_params = 1; > @@ -249,6 +282,37 @@ static int invoke_process_smt_channel(struct scmi_optee_channel *channel) > return 0; > } > Looking at the invoke_process_smt_channel() code, I see that it still refers to ->tee_shm in an if condition when you send a PTA_SCMI_CMD_PROCESS_SMT_CHANNEL_MESSAGE but that channel->tee_shm is always NULL when calling invoke_process_smt_channel() since tee_shm is only allocated by non-SMT transport via setup_dynamic_shmem(), so when this if-branch inside invoke_process_smt_channel() is supposed to be used ? or is it a leftover ? (or I am missing something :D) > +static int invoke_process_msg_channel(struct scmi_optee_channel *channel, size_t msg_size) > +{ > + struct tee_ioctl_invoke_arg arg = { > + .func = PTA_SCMI_CMD_PROCESS_MSG_CHANNEL, > + .session = channel->tee_session, > + .num_params = 3, > + }; > + struct tee_param param[3] = { }; > + int ret; > + > + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_VALUE_INPUT; > + param[0].u.value.a = channel->channel_id; > + > + param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT; > + param[1].u.memref.shm = channel->tee_shm; > + param[1].u.memref.size = msg_size; > + > + param[2].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT; > + param[2].u.memref.shm = channel->tee_shm; > + param[2].u.memref.size = SCMI_OPTEE_MAX_MSG_SIZE; > + > + ret = tee_client_invoke_func(scmi_optee_private->tee_ctx, &arg, param); > + if (ret < 0 || arg.ret) { > + dev_err(scmi_optee_private->dev, "Can't invoke channel %u: %d / %#x\n", > + channel->channel_id, ret, arg.ret); > + return -EIO; > + } > + > + return 0; > +} > + Thanks, Cristian _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel