From: =?iso-8859-2?Q?Horia_Geant=E3?= Subject: Re: [RESEND PATCH 0/6] Enable CAAM on i.MX7s fix TrustZone issues Date: Thu, 25 Jan 2018 17:50:31 +0000 Message-ID: References: <1516805435-15034-1-git-send-email-pure.logic@nexus-software.ie> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-2" Content-Transfer-Encoding: quoted-printable Cc: Fabio Estevam , Peng Fan , "herbert@gondor.apana.org.au" , "davem@davemloft.net" , "lukas.auer@aisec.fraunhofer.de" , "rui.silva@linaro.org" , "ryan.harkin@linaro.org" To: Bryan O'Donoghue , Aymen Sghaier , "linux-crypto@vger.kernel.org" , "linux-kernel@vger.kernel.org" Return-path: Received: from mail-he1eur01on0087.outbound.protection.outlook.com ([104.47.0.87]:29728 "EHLO EUR01-HE1-obe.outbound.protection.outlook.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751105AbeAYRug (ORCPT ); Thu, 25 Jan 2018 12:50:36 -0500 Content-Language: en-US Sender: linux-crypto-owner@vger.kernel.org List-ID: On 1/24/2018 4:50 PM, Bryan O'Donoghue wrote:=0A= > This patch-set enables CAAM on the i.MX7s and fixes a number of issues=0A= > identified with the CAAM driver and hardware when TrustZone mode is=0A= > enabled.=0A= > =0A= > The first block of patches are simple bug-fixes, followed by a second blo= ck=0A= > of patches which are simple enabling patches for the i.MX7Solo - note we= =0A= > aren't enabling for the i.MX7Dual since we don't have hardware to test th= at=0A= > out but it should be a 1:1 mapping for others to enable when appropriate.= =0A= > =0A= > The final block in this series implements a fix for using the CAAM when= =0A= > OPTEE/TrustZone is enabled. The various details are logged in these=0A= > threads.=0A= > =0A= > Link: https://github.com/OP-TEE/optee_os/issues/1408=0A= > Link: https://tinyurl.com/yam5gv9a=0A= > Link: https://patchwork.ozlabs.org/cover/865042=0A= > =0A= > In simple terms, when TrustZone is active the first page of the CAAM=0A= > becomes inaccessible to Linux as it has a special 'TZ bit' associated wit= h=0A= > it that software cannot toggle or even view AFAIK.=0A= If the first ("global") caam register page is not accessible, RNG init is n= ot=0A= the only problem. For e.g. device endianness detection won't work. A comple= te=0A= list could be generated by auditing all places where this page is r/w.=0A= =0A= IMHO the correct direction for solving such cases (i.e. Linux kernel is pro= vided=0A= only with access to a few job rings) is to split the driver in two independ= ent=0A= ones - controller driver and job ring driver - and have corresponding DT no= des=0A= for them. Controller DT node and one or more of the job ring DT nodes would= be=0A= deleted by the boot loader / trusted firmware if needed.=0A= Of course, the job ring DT node might need additional properties for the dr= iver=0A= to work.=0A= =0A= Thanks,=0A= Horia=0A= =0A= > =0A= > The patches here then=0A= > =0A= > 1. Detect when TrustZone is active=0A= > 2. Detect if u-boot (or OPTEE) has already initialized the RNG=0A= > =0A= > and loads the CAAM driver in a different way - skipping over the RNG=0A= > initialization that Linux now no-longer has permissions to carry out.=0A= > =0A= > Should #1 be true but #2 not be true, driver loading stops (and Rui's pat= ch=0A= > for the NULL pointer dereference fixes a cash on this path). If #2 is tru= e=0A= > but #1 is not then it's a NOP as Linux has full permission to rewrite the= =0A= > deco registers in the first page of CAAM registers.=0A= > =0A= > Finally then if #1 and #2 are true, the fixes here allow the CAAM to come= =0A= > up and for the RNG to be useable again.=0A= > =0A= > Bryan O'Donoghue (3):=0A= > crypto: caam: Fix endless loop when RNG is already initialized=0A= > crypto: caam: add logic to detect when running under TrustZone=0A= > crypto: caam: detect RNG init when TrustZone is active=0A= > =0A= > Rui Miguel Silva (3):=0A= > crypto: caam: Fix null dereference at error path=0A= > ARM: dts: imx7s: add CAAM device node=0A= > imx7d: add CAAM clocks=0A= > =0A= > arch/arm/boot/dts/imx7s.dtsi | 26 +++++++++++++++++++=0A= > drivers/clk/imx/clk-imx7d.c | 3 +++=0A= > drivers/crypto/caam/ctrl.c | 45 +++++++++++++++++++++++++++= +++---=0A= > drivers/crypto/caam/intern.h | 1 +=0A= > include/dt-bindings/clock/imx7d-clock.h | 5 +++-=0A= > 5 files changed, 76 insertions(+), 4 deletions(-)=0A= > =0A=