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 8C52CC48BF8 for ; Thu, 22 Feb 2024 19:03:02 +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=pE783gazfRF9l6HB6s/nmATICcax9MJ4H1g+HU3RvoY=; b=yakLYx69EH0Oaw x+bMtAFOVu1AEzXGNqr6RB5j+ZG2n+HVc+1kds2XSySm6i+Fgy/eH0AWMyRC+JBegz2Vua2lmW8rS zWEHuacHyOW9rXxs9lR4e+b9dYehfWKpQbS97nAKWqvwROdsTf25CY+M/UUVirrNpkPqlUMMRXiWV wYcJQpztmkwy+T/AMYOID9/wfCO4x+93j29inYVfPq1Tr4reE8uEIgWeFWD7H8hsMDhwiMZj/DUrG y5PLmxhIFCWkkvK8LJzPmi7pC7Aki2tJbPxCL2A/WcJ7bU1XO6i2zJUHRGeXAgoi7Nf5AXa/DbeCc Wsb02Eoyr8Bmq3xBetQw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rdELI-00000006EnM-1qvJ; Thu, 22 Feb 2024 19:02:48 +0000 Received: from mail-pg1-x52f.google.com ([2607:f8b0:4864:20::52f]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rdELF-00000006Emg-1687 for linux-arm-kernel@lists.infradead.org; Thu, 22 Feb 2024 19:02:46 +0000 Received: by mail-pg1-x52f.google.com with SMTP id 41be03b00d2f7-5dbd519bde6so22642a12.1 for ; Thu, 22 Feb 2024 11:02:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1708628563; x=1709233363; darn=lists.infradead.org; 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=vTYNxcj2neo6HPPvD99c1Q/mh/wbkm0pvCmLFsw0RNE=; b=CikPUpTGpRNS+lO7XOi/cM9+wmSBQtsztiMQjV0GaUxxt757DdofmIoYhrsg+LB0dV TuHwNV8IAfkr4XvqAimmHj2aDAm+bqpXCHA6R4aLUIWXEgnTRTQGXy0UHyDjeKPK9D4B VcNZbZo7vef7TnZiY5k9f6hdxYiP05o98oalGBtWYEHRWH+GOIWaphA9vM0DYQSTddge Ppi442OI6Kl/ZUSVWRzm8hJpPaSe4aWjeLCDExV1H36MKFXZgFxEgYGZRyUOYs5EA+YL KJLMYxLbxoFfxp/GSjLSl/rOixyoF58UkYzgf7ZybdXNHLy7kJedhsFQ+jaffsoaILnB DlDw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708628563; x=1709233363; 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=vTYNxcj2neo6HPPvD99c1Q/mh/wbkm0pvCmLFsw0RNE=; b=KUlNzi/fUaiVCy70sjq/2L3yc+UBLFbclasn/9CbMi2A5Ja1R/SQuKD8Jj5AAHKqF3 MwQwnjtyOalfycnE5+Hk1LHtV05SC6RFMCjwfiuiISQuEn5vnf+Ws8G7WGOu5PT+83r7 N9iMijTy4ibQ0gtLESfmrdBnAzFU4dlaXKxkCKJ5MQhTgjIdgo0eIDNDMVzIM2n4CcHq nIth4IcNb+tzy2hTbFIZ8ZotBTTJVi+Rj1lN1NYXSfBNvSxgMVNuHffvY/dz4kRxd3y4 HMmIJdSZbr0/DanH5hUtAbZdhhE2tSEj//jYr6HsgDUI5k0kiSuqcw+asJY5n5AvZO2N PQ9w== X-Forwarded-Encrypted: i=1; AJvYcCXhPsBFr74paEfy7vRA/NodYMATWjiY5qfNMHF2ncByi3EwdEHz1/Ccsc+ymgoRqO1Bqh09g5QriTEjBqKcR/XAyAGZUpPnCToZi5qdjGZBe97q/uE= X-Gm-Message-State: AOJu0Ywnn1xFFPltk8mTW9tYcFqCi+Bj+Y8pZDBQaPktN/ZCeq5S305A QsgZjf2n9TbW7JwZCjPgqJiRqpzSRMuDW9pdf+FGszw687DkUMVllT57PyGzeck= X-Google-Smtp-Source: AGHT+IGHpe6qaxfqw4bUvBWsv+UIbIpFUkh4ZbooGLlTNoQO8V6VqfMPRcSemGOLTfiSJOpq7TrZDw== X-Received: by 2002:a05:6a21:9101:b0:19e:ccb2:fd80 with SMTP id tn1-20020a056a21910100b0019eccb2fd80mr20697588pzb.8.1708628563292; Thu, 22 Feb 2024 11:02:43 -0800 (PST) Received: from p14s ([2604:3d09:148c:c800:1fd:7619:a246:fc72]) by smtp.gmail.com with ESMTPSA id u10-20020a056a00124a00b006e4de94c111sm10598pfi.65.2024.02.22.11.02.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 22 Feb 2024 11:02:42 -0800 (PST) Date: Thu, 22 Feb 2024 12:02:40 -0700 From: Mathieu Poirier To: Arnaud Pouliquen Cc: Bjorn Andersson , Jens Wiklander , Rob Herring , Krzysztof Kozlowski , Conor Dooley , linux-stm32@st-md-mailman.stormreply.com, linux-arm-kernel@lists.infradead.org, linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org, op-tee@lists.trustedfirmware.org, devicetree@vger.kernel.org Subject: Re: [PATCH v3 7/7] remoteproc: stm32: Add support of an OP-TEE TA to load the firmware Message-ID: References: <20240214172127.1022199-1-arnaud.pouliquen@foss.st.com> <20240214172127.1022199-8-arnaud.pouliquen@foss.st.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240214172127.1022199-8-arnaud.pouliquen@foss.st.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240222_110245_489003_310BCB1D X-CRM114-Status: GOOD ( 37.59 ) 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 Hi, On Wed, Feb 14, 2024 at 06:21:27PM +0100, Arnaud Pouliquen wrote: > The new TEE remoteproc device is used to manage remote firmware in a > secure, trusted context. The 'st,stm32mp1-m4-tee' compatibility is > introduced to delegate the loading of the firmware to the trusted > execution context. In such cases, the firmware should be signed and > adhere to the image format defined by the TEE. > > A new "to_attach" field is introduced to differentiate the use cases > "firmware loaded by the boot stage" and "firmware loaded by the TEE". > > Signed-off-by: Arnaud Pouliquen > --- > V2 to V3 update: > - remove stm32_rproc_tee_elf_sanity_check(), stm32_rproc_tee_elf_load() > stm32_rproc_tee_elf_find_loaded_rsc_table() and stm32_rproc_tee_start() that are bnow unused > - use new rproc::alt_boot field to sepcify that the alternate fboot method is used > - use stm32_rproc::to_attach field to differenciate attch mode from remoteproc tee boot mode. > - remove the used of stm32_rproc::fw_loaded > --- > drivers/remoteproc/stm32_rproc.c | 85 +++++++++++++++++++++++++++++--- > 1 file changed, 79 insertions(+), 6 deletions(-) > > diff --git a/drivers/remoteproc/stm32_rproc.c b/drivers/remoteproc/stm32_rproc.c > index fcc0001e2657..9cfcf66462e0 100644 > --- a/drivers/remoteproc/stm32_rproc.c > +++ b/drivers/remoteproc/stm32_rproc.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > #include > > #include "remoteproc_internal.h" > @@ -49,6 +50,9 @@ > #define M4_STATE_STANDBY 4 > #define M4_STATE_CRASH 5 > > +/* Remote processor unique identifier aligned with the Trusted Execution Environment definitions */ > +#define STM32_MP1_M4_PROC_ID 0 > + > struct stm32_syscon { > struct regmap *map; > u32 reg; > @@ -90,6 +94,8 @@ struct stm32_rproc { > struct stm32_mbox mb[MBOX_NB_MBX]; > struct workqueue_struct *workqueue; > bool hold_boot_smc; > + bool to_attach; > + struct tee_rproc *trproc; > void __iomem *rsc_va; > }; > > @@ -253,10 +259,30 @@ static int stm32_rproc_release(struct rproc *rproc) > return err; > } > } > + ddata->to_attach = false; > > return err; > } > > +static int stm32_rproc_tee_attach(struct rproc *rproc) > +{ > + /* Nothing to do, remote proc already started by the secured context. */ > + return 0; > +} > + > +static int stm32_rproc_tee_stop(struct rproc *rproc) > +{ > + int err; > + > + stm32_rproc_request_shutdown(rproc); > + > + err = tee_rproc_stop(rproc); > + if (err) > + return err; > + > + return stm32_rproc_release(rproc); > +} > + > static int stm32_rproc_prepare(struct rproc *rproc) > { > struct device *dev = rproc->dev.parent; > @@ -637,10 +663,14 @@ stm32_rproc_get_loaded_rsc_table(struct rproc *rproc, size_t *table_sz) > { > struct stm32_rproc *ddata = rproc->priv; > struct device *dev = rproc->dev.parent; > + struct tee_rproc *trproc = ddata->trproc; > phys_addr_t rsc_pa; > u32 rsc_da; > int err; > > + if (trproc && !ddata->to_attach) > + return tee_rproc_get_loaded_rsc_table(rproc, table_sz); > + Why do we need a flag at all? Why can't st_rproc_tee_ops::get_loaded_rsc_table be set to tee_rproc_get_loaded_rsc_table()? > /* The resource table has already been mapped, nothing to do */ > if (ddata->rsc_va) > goto done; > @@ -693,8 +723,20 @@ static const struct rproc_ops st_rproc_ops = { > .get_boot_addr = rproc_elf_get_boot_addr, > }; > > +static const struct rproc_ops st_rproc_tee_ops = { > + .prepare = stm32_rproc_prepare, > + .start = tee_rproc_start, > + .stop = stm32_rproc_tee_stop, > + .attach = stm32_rproc_tee_attach, > + .kick = stm32_rproc_kick, > + .get_loaded_rsc_table = stm32_rproc_get_loaded_rsc_table, > + .find_loaded_rsc_table = tee_rproc_find_loaded_rsc_table, > + .load = tee_rproc_load_fw, > +}; > + > static const struct of_device_id stm32_rproc_match[] = { > - { .compatible = "st,stm32mp1-m4" }, > + {.compatible = "st,stm32mp1-m4",}, > + {.compatible = "st,stm32mp1-m4-tee",}, > {}, > }; > MODULE_DEVICE_TABLE(of, stm32_rproc_match); > @@ -853,6 +895,7 @@ static int stm32_rproc_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct stm32_rproc *ddata; > struct device_node *np = dev->of_node; > + struct tee_rproc *trproc = NULL; > struct rproc *rproc; > unsigned int state; > int ret; > @@ -861,12 +904,33 @@ static int stm32_rproc_probe(struct platform_device *pdev) > if (ret) > return ret; > > - rproc = rproc_alloc(dev, np->name, &st_rproc_ops, NULL, sizeof(*ddata)); > - if (!rproc) > - return -ENOMEM; This patch doesn't apply to rproc-next - please rebase. > + if (of_device_is_compatible(np, "st,stm32mp1-m4-tee")) { > + /* > + * Delegate the firmware management to the secure context. > + * The firmware loaded has to be signed. > + */ > + trproc = tee_rproc_register(dev, STM32_MP1_M4_PROC_ID); > + if (IS_ERR(trproc)) { > + dev_err_probe(dev, PTR_ERR(trproc), > + "signed firmware not supported by TEE\n"); > + return PTR_ERR(trproc); > + } > + } > > - ddata = rproc->priv; > + rproc = rproc_alloc(dev, np->name, > + trproc ? &st_rproc_tee_ops : &st_rproc_ops, > + NULL, sizeof(*ddata)); > + if (!rproc) { > + ret = -ENOMEM; > + goto free_tee; > + } > > + ddata = rproc->priv; > + ddata->trproc = trproc; My opinion hasn't changed from the previous patchet, i.e tee_rproc should be folded in struct rproc as rproc::tee_interface. More comments to come shortly... > + if (trproc) { > + rproc->alt_boot = true; > + trproc->rproc = rproc; > + } > rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE); > > ret = stm32_rproc_parse_dt(pdev, ddata, &rproc->auto_boot); > @@ -881,8 +945,10 @@ static int stm32_rproc_probe(struct platform_device *pdev) > if (ret) > goto free_rproc; > > - if (state == M4_STATE_CRUN) > + if (state == M4_STATE_CRUN) { > rproc->state = RPROC_DETACHED; > + ddata->to_attach = true; > + } > > rproc->has_iommu = false; > ddata->workqueue = create_workqueue(dev_name(dev)); > @@ -916,6 +982,10 @@ static int stm32_rproc_probe(struct platform_device *pdev) > device_init_wakeup(dev, false); > } > rproc_free(rproc); > +free_tee: > + if (trproc) > + tee_rproc_unregister(trproc); > + > return ret; > } > > @@ -923,6 +993,7 @@ static void stm32_rproc_remove(struct platform_device *pdev) > { > struct rproc *rproc = platform_get_drvdata(pdev); > struct stm32_rproc *ddata = rproc->priv; > + struct tee_rproc *trproc = ddata->trproc; > struct device *dev = &pdev->dev; > > if (atomic_read(&rproc->power) > 0) > @@ -937,6 +1008,8 @@ static void stm32_rproc_remove(struct platform_device *pdev) > device_init_wakeup(dev, false); > } > rproc_free(rproc); > + if (trproc) > + tee_rproc_unregister(trproc); > } > > static int stm32_rproc_suspend(struct device *dev) > -- > 2.25.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel