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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 481BFC05027 for ; Thu, 9 Feb 2023 09:21:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229606AbjBIJVH (ORCPT ); Thu, 9 Feb 2023 04:21:07 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53436 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230224AbjBIJUf (ORCPT ); Thu, 9 Feb 2023 04:20:35 -0500 Received: from mail-ej1-x62c.google.com (mail-ej1-x62c.google.com [IPv6:2a00:1450:4864:20::62c]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A2F1365ED6 for ; Thu, 9 Feb 2023 01:19:41 -0800 (PST) Received: by mail-ej1-x62c.google.com with SMTP id gr7so4433274ejb.5 for ; Thu, 09 Feb 2023 01:19:41 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=rPg4EdR7sFNNzQ3B/yZZxndPMDpt8LnVMvk1N2mtNRc=; b=SgcDEleUxhrO6ZMJCiY0GM6uME1n4qRmGiAk7KBD6rVjx2s4oRGYnK+zOzDMxOxIK5 NSlCNiGYIcEDdUgPMI05pklHQMxkwhIPNQnjR+QR3kWpdPFPozNnIoCl4jekSgBF7OTC r7KRVu3LFnGr6Sk4PBA1Z7s1ZORGwBW+E7R5SDgQ904LOP3Ue+u/7kyJTt8QVG4bW2F0 6wECXQoPNfw3hWpZ5bFA9RmFkQochZyFYtF7guodATaHpbLbGa2BE7rPbKzp5HGUJ1XW y7kp3biMfT/vpKX405SX3m7fVrcI/nMtiRRWk36ihZgfkbqjjzB84tXC0IUpJOvffNYD +4yQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=rPg4EdR7sFNNzQ3B/yZZxndPMDpt8LnVMvk1N2mtNRc=; b=0WVeXthNeoQc6VsYIvvVJnM6EH/pq5Klz8Qw5xhh9mzVDgpCUAgx5q00cHEkyvodvl 7d4X09j20FYiWWrbGfFB9PZPEot/yZaSKNy9EVsQxadoCp3wTH8nU9qzSMPOkNhfb8IU bN477gS8zj/PeQOILDA85Xg9aL4Im0RGBjr9DyarI4TOQqKQuGA4IyX502opsqC3e+Qi RdqC19lvWFsbZdofstu9sYj1Rarjxm1cVfErS32oJmeAdEe3sHmqJi81Z7eB+iKyq9CS j25HIEixrlWr79OrRrr+z91xuu4kvkeh4sMngowUrf84gULuCKfTVMzzeclxeNC69FLg FSJg== X-Gm-Message-State: AO0yUKVHVATA3UP2xIxUK85gjEr9Ifq7t8Ocv1D+5xTkcMP66B6PCniR MbSXBpcHzNNcm12snnVAjg8+UA== X-Google-Smtp-Source: AK7set+rE77rXdjt5nnew52wEY0H3Z9XKAK5R/SvPZF7NeM3sNTetogGiPPmE0Y+ouMUuKL16VYElQ== X-Received: by 2002:a17:906:57d6:b0:86f:fbcf:f30a with SMTP id u22-20020a17090657d600b0086ffbcff30amr12700322ejr.58.1675934379659; Thu, 09 Feb 2023 01:19:39 -0800 (PST) Received: from jade (h-46-59-78-111.A175.priv.bahnhof.se. [46.59.78.111]) by smtp.gmail.com with ESMTPSA id i10-20020a1709063c4a00b008af21450420sm629984ejg.85.2023.02.09.01.19.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Feb 2023 01:19:39 -0800 (PST) Date: Thu, 9 Feb 2023 10:19:37 +0100 From: Jens Wiklander To: Etienne Carriere Cc: Sumit Garg , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, Sudeep Holla , Cristian Marussi Subject: Re: [PATCH 1/2] tee: system invocation Message-ID: References: <20230130094157.1082712-1-etienne.carriere@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, On Thu, Feb 09, 2023 at 09:11:53AM +0100, Etienne Carriere wrote: > Hi Jens, > > > On Thu, 9 Feb 2023 at 08:14, Jens Wiklander wrote: > > > > Hi Etienne, > > > > On Wed, Feb 08, 2023 at 06:09:17PM +0100, Etienne Carriere wrote: > > > Hello Sumit, Jens, > > > > > [snip] > > > > > > > > > > > > > > > > if (rpc_arg && tee_shm_is_dynamic(shm)) { > > > > > > > > - param.a0 = OPTEE_SMC_CALL_WITH_REGD_ARG; > > > > > > > > + if (ctx->sys_service && > > > > > > > > + (optee->smc.sec_caps & OPTEE_SMC_SEC_CAP_SYSTEM_THREAD)) > > > > > > > > + param.a0 = OPTEE_SMC_CALL_SYSTEM_WITH_REGD_ARG; > > > > > > > > + else > > > > > > > > + param.a0 = OPTEE_SMC_CALL_WITH_REGD_ARG; > > > > > > > > > > > > > > This system thread flag should also be applicable to platforms without > > > > > > > registered arguments support. IOW, we need similar equivalents for > > > > > > > OPTEE_SMC_FUNCID_CALL_WITH_ARG and OPTEE_SMC_FUNCID_CALL_WITH_RPC_ARG > > > > > > > too. So I would rather suggest that we add following flag to all 3 > > > > > > > call types: > > > > > > > > > > > > > > #define OPTEE_SMC_CALL_SYSTEM_THREAD_FLAG 0x8000 > > > > > > > > > > > > The main reason platforms don't support registered arguments is that > > > > > > they haven't been updated since this was introduced. So if a platform > > > > > > needs system threads it could update to use registered arguments too. > > > > > > > > > > Are we hinting at deprecating reserved shared memory support? If yes, > > > > > wouldn't it be better to be explicit about it with a boot time warning > > > > > message about its deprecation? > > > > > > > > > > Otherwise it will be difficult to debug for the end user to find out > > > > > why system thread support isn't activated. > > > > > > > > > > > The Linux kernel already supports registered arguments. An advantage > > > > > > with the current approach is that the ABI is easier to implement > > > > > > since we have distinct SMC IDs for each function. > > > > > > > > > > I see your point but my initial thought was that we don't end up > > > > > making that list too large that it becomes cumbersome to maintain, > > > > > involving all the combinatorial. > > > > > > > > You have a point. Etienne, do you think we could give it a try at > > > > https://github.com/OP-TEE/optee_os/pull/5789 to better see how this > > > > would play out? > > > > > > > > > > Indeed I miss that... > > > With the patch proposed here, indeed if OP-TEE does not support > > > dynamic shared memory then Linux will never use the provisioned TEE > > > thread. This is weird as in such a case OP-TEE provisions resources > > > that will never be used, which is the exact opposite goal of this > > > feature. Verified on our qemu-arm setup. > > > > > > For simplicity, I think this system invocation should require OP-TEE > > > supports dyn shm. > > > > It's not obvious to me that this will easier to implement and maintain. > > Looking at the code in optee_os it looks like using a flag bit as > > proposed by Sumit would be quite easy to handle. > > OP-TEE could auto disable thread provis when dyn shm is disabled, right. > Will it be sufficient? We will still face cases where an OP-TEE > provisions thread but Linux kernel is not aware (older vanilla kernel > used with a recent OP-TEE OS). Not much platforms are really affected > I guess but those executing with pager in small RAMs where a 4kB > thread context costs. When you add exceptions you make it more complicated. Now we must remember to always use dyn shm if we are to succeed in configuring with system threads. What if both dyn shm and static shm is configured in OP-TEE, but the kernel only uses static shm? > > > If OP-TEE could know when Linux does not support TEE system > > > invocation, then OP-TEE could let any invocation use these provisioned > > > resources so that they are not wasted. > > > I think a good way would be Linux to expose if it supports this > > > capability, during capabilities exchange. > > > Would you agree with this approach? > > > > No, I'm not so keen on adding that side effect to > > OPTEE_SMC_EXCHANGE_CAPABILITIES. > > It is a capability REE would exchanges with TEE. > What kind of side effects do you fear? I was hoping to keep it stateless. One thing less to keep track of when handing over from a boot stage to the kernel. > > The way you're describing the problem it sounds like it's a normal world > > problem to know how many system threads are needed. How about adding a > > fast call where normal world can request how many system threads should > > be reserved? If none are requested, none will be reserved. > > Well, could be. With caps exchange, we have an SMC funcID to REE to > say to TEE: "reserved the default configured number of sys thread". I > think it is simpler. Until you realize the that the default number of system threads doesn't match what you need. > > With REE calling TEE to provision thread, we would need another call > to release the reservation. Whe caps exchange, we have a single SMC to > reconfig the negotiated caps. A single SMC with growing complexity in its arguments. Cheers, Jens