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 5629FC5AD49 for ; Mon, 26 May 2025 12:47:46 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:MIME-Version: Content-Transfer-Encoding:Content-ID:Content-Type:In-Reply-To:References: Message-ID:Date:Subject:CC:To:From:Reply-To:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=it4Q54ohHGXFGzLtFb5KAX8FUUrPR6nRKUGfTDC1QME=; b=KY5zqywLV794UFw09ScC0ga198 UKihfopA+XgOQg6jAk09NRkCBnKUn27IJjGPS9bgWm1b9r2hCoAcg/mZTKySt7XISkYcd0dldsRwA El4wjDsr/Nlf3BIYx24XYM0bGy6yylheY+058QNKCXy14oKvTD+gcJ5Gr54tFVxtfzl6WqySe1IrF VfUIoXHOxL9i7qQ87oAl3qO7nZiHGC1lWQ3bulPyj1pS9Qchf5nQdgN8J3MuGp2YEbv1/43aQrN9a VqUb+zaBbcij6ZpfgO+skutGZfi08pmFsXbiKGSUQGxCS+0nvmPffoNMs0p7q8rx1fAvCBV4l+uVS ztgnPm/w==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.98.2 #2 (Red Hat Linux)) id 1uJXEu-00000008okJ-0MJ4; Mon, 26 May 2025 12:47:36 +0000 Received: from mail.actia.se ([212.181.117.226]) by bombadil.infradead.org with esmtps (Exim 4.98.2 #2 (Red Hat Linux)) id 1uJXCk-00000008oPX-37EZ for linux-arm-kernel@lists.infradead.org; Mon, 26 May 2025 12:45:24 +0000 Received: from S036ANL.actianordic.se (10.12.31.117) by S035ANL.actianordic.se (10.12.31.116) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.39; Mon, 26 May 2025 14:45:14 +0200 Received: from S036ANL.actianordic.se ([fe80::e13e:1feb:4ea6:ec69]) by S036ANL.actianordic.se ([fe80::e13e:1feb:4ea6:ec69%3]) with mapi id 15.01.2507.039; Mon, 26 May 2025 14:45:14 +0200 From: John Ernberg To: Frank Li CC: =?iso-8859-2?Q?Horia_Geant=E3?= , Pankaj Gupta , Gaurav Jain , Herbert Xu , "David S . Miller" , "Rob Herring" , Krzysztof Kozlowski , "Conor Dooley" , Shawn Guo , Sascha Hauer , Pengutronix Kernel Team , Fabio Estevam , Thomas Richard , "linux-crypto@vger.kernel.org" , "devicetree@vger.kernel.org" , "linux-kernel@vger.kernel.org" , "imx@lists.linux.dev" , "linux-arm-kernel@lists.infradead.org" , "stable@kernel.org" Subject: Re: [PATCH 1/4] crypto: caam - Prevent crash on suspend with iMX8QM / iMX8ULP Thread-Topic: [PATCH 1/4] crypto: caam - Prevent crash on suspend with iMX8QM / iMX8ULP Thread-Index: AQHby+Uubq4/T0/2xEa3pJeQa3AymLPgGseAgAAHVgCAAAgpgIAElG2A Date: Mon, 26 May 2025 12:45:14 +0000 Message-ID: References: <20250523131814.1047662-1-john.ernberg@actia.se> <20250523131814.1047662-2-john.ernberg@actia.se> In-Reply-To: Accept-Language: en-US, sv-SE Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.12.12.63] x-esetresult: clean, is OK x-esetid: 37303A2956B1445360736A Content-Type: text/plain; charset="iso-8859-2" Content-ID: Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20250526_054522_788208_3915EE21 X-CRM114-Status: GOOD ( 33.20 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org Hi Frank, On Fri, May 23, 2025 at 10:48:51AM -0400, Frank Li wrote: > On Fri, May 23, 2025 at 02:19:38PM +0000, John Ernberg wrote: > > Hi Frank, > > > > On 5/23/25 3:53 PM, Frank Li wrote: > > > On Fri, May 23, 2025 at 01:18:32PM +0000, John Ernberg wrote: > > >> Since the CAAM on these SoCs is managed by another ARM core, called = the > > >> SECO (Security Controller) on iMX8QM and Secure Enclave on iMX8ULP, = which > > >> also reserves access to register page 0 suspend operations cannot to= uch > > >> this page. > > >> > > >> Introduce a variable to track this situation. Since this is synonymo= us > > >> with the optee case in suspend/resume the optee check is replaced wi= th > > >> this new check. > > >> > > >> Fixes the following splat at suspend: > > >> > > >> Internal error: synchronous external abort: 0000000096000010 [#= 1] SMP > > >> Hardware name: Freescale i.MX8QXP ACU6C (DT) > > >> pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=3D-= -) > > >> pc : readl+0x0/0x18 > > >> lr : rd_reg32+0x18/0x3c > > >> sp : ffffffc08192ba20 > > >> x29: ffffffc08192ba20 x28: ffffff8025190000 x27: 00000000000000= 00 > > >> x26: ffffffc0808ae808 x25: ffffffc080922338 x24: ffffff8020e890= 90 > > >> x23: 0000000000000000 x22: ffffffc080922000 x21: ffffff8020e890= 10 > > >> x20: ffffffc080387ef8 x19: ffffff8020e89010 x18: 000000005d8000= d5 > > >> x17: 0000000030f35963 x16: 000000008f785f3f x15: 000000003b8ef5= 7c > > >> x14: 00000000c418aef8 x13: 00000000f5fea526 x12: 00000000000000= 01 > > >> x11: 0000000000000002 x10: 0000000000000001 x9 : 00000000000000= 00 > > >> x8 : ffffff8025190870 x7 : ffffff8021726880 x6 : 00000000000000= 02 > > >> x5 : ffffff80217268f0 x4 : ffffff8021726880 x3 : ffffffc0812000= 00 > > >> x2 : 0000000000000001 x1 : ffffff8020e89010 x0 : ffffffc0812000= 04 > > >> Call trace: > > >> readl+0x0/0x18 > > >> caam_ctrl_suspend+0x30/0xdc > > >> dpm_run_callback.constprop.0+0x24/0x5c > > >> device_suspend+0x170/0x2e8 > > >> dpm_suspend+0xa0/0x104 > > >> dpm_suspend_start+0x48/0x50 > > >> suspend_devices_and_enter+0x7c/0x45c > > >> pm_suspend+0x148/0x160 > > >> state_store+0xb4/0xf8 > > >> kobj_attr_store+0x14/0x24 > > >> sysfs_kf_write+0x38/0x48 > > >> kernfs_fop_write_iter+0xb4/0x178 > > >> vfs_write+0x118/0x178 > > >> ksys_write+0x6c/0xd0 > > >> __arm64_sys_write+0x14/0x1c > > >> invoke_syscall.constprop.0+0x64/0xb0 > > >> do_el0_svc+0x90/0xb0 > > >> el0_svc+0x18/0x44 > > >> el0t_64_sync_handler+0x88/0x124 > > >> el0t_64_sync+0x150/0x154 > > >> Code: 88dffc21 88dffc21 5ac00800 d65f03c0 (b9400000) > > >> > > >> Fixes: d2835701d93c ("crypto: caam - i.MX8ULP donot have CAAM page0 = access") > > >> Fixes: 61bb8db6f682 ("crypto: caam - Add support for i.MX8QM") > > >> Cc: stable@kernel.org # v6.10+ > > >> Signed-off-by: John Ernberg > > >> > > >> --- > > >> > > >> I noticed this when enabling the iMX8QXP support (next patch), hence= the > > >> iMX8QXP backtrace, but the iMX8QM CAAM integration works exactly the= same > > >> and according to the NXP tree [1] the iMX8ULP suffers the same issue= . > > >> > > >> [1]: https://github.com/nxp-imx/linux-imx/commit/653712ffe52dd59f407= af1b781ce318f3d9e17bb > > >> --- > > >> drivers/crypto/caam/ctrl.c | 5 +++-- > > >> drivers/crypto/caam/intern.h | 1 + > > >> 2 files changed, 4 insertions(+), 2 deletions(-) > > >> > > >> diff --git a/drivers/crypto/caam/ctrl.c b/drivers/crypto/caam/ctrl.c > > >> index 38ff931059b4..766c447c9cfb 100644 > > >> --- a/drivers/crypto/caam/ctrl.c > > >> +++ b/drivers/crypto/caam/ctrl.c > > >> @@ -831,7 +831,7 @@ static int caam_ctrl_suspend(struct device *dev) > > >> { > > >> const struct caam_drv_private *ctrlpriv =3D dev_get_drvdata(d= ev); > > >> > > >> - if (ctrlpriv->caam_off_during_pm && !ctrlpriv->optee_en) > > >> + if (ctrlpriv->caam_off_during_pm && !ctrlpriv->no_page0) > > >> caam_state_save(dev); > > >> > > >> return 0; > > >> @@ -842,7 +842,7 @@ static int caam_ctrl_resume(struct device *dev) > > >> struct caam_drv_private *ctrlpriv =3D dev_get_drvdata(dev); > > >> int ret =3D 0; > > >> > > >> - if (ctrlpriv->caam_off_during_pm && !ctrlpriv->optee_en) { > > >> + if (ctrlpriv->caam_off_during_pm && !ctrlpriv->no_page0) { > > >> caam_state_restore(dev); > > >> > > >> /* HW and rng will be reset so deinstantiation can be= removed */ > > >> @@ -908,6 +908,7 @@ static int caam_probe(struct platform_device *pd= ev) > > >> > > >> imx_soc_data =3D imx_soc_match->data; > > >> reg_access =3D reg_access && imx_soc_data->page0_acce= ss; > > >> + ctrlpriv->no_page0 =3D !reg_access; > > > > > > If you want to use no_page0 to control if call caam_state_save(), you= 'd > > > better set ctrlpriv->no_page0 also after ctrlpriv->optee_en =3D !!np; > > > > > > Frank > > > > I'm not sure I understand, I cannot see a code path where no_page0 will > > be (un)set incorrectly. > > > > optee disables the page0 access, so reg_access is already the inverse o= f > > optee_en. reg_access =3D=3D false when optee_en =3D=3D true. > > > > Thus, if optee is loaded on a SoC that normally has page0_access the > > `reg_access =3D reg_access && imx_soc_data->page0_access;` statement on > > the line above setting no_page0 already takes care of it, so: > > reg_access =3D false && true -> false. > > > > Similarly if both reg_access =3D=3D false and page0_access =3D=3D false= , > > reg_access will still be false. >=20 > Okay, I check original code. You are right. You'd better to add descripto= n > in commit message about no_page0 is true when optee_en is true. >=20 > Frank Thanks for clarifying. I will update the commit message making this clearer= in V2, along with your other comments on 3/4 and 4/4. Thanks! // John Ernberg=