On 11/15/2021 12:07 PM, ZHIZHIKIN Andrey wrote: > Hello Michael, > >> -----Original Message----- >> From: Michael Walle <michael@xxxxxxxx> >> Sent: Friday, November 12, 2021 10:18 PM >> To: ZHIZHIKIN Andrey <andrey.zhizhikin@xxxxxxxxxxxxxxxxxxxx> >> Cc: horia.geanta@xxxxxxx; pankaj.gupta@xxxxxxx; >> herbert@xxxxxxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx; >> iuliana.prodan@xxxxxxx; linux-crypto@xxxxxxxxxxxxxxx; linux- >> kernel@xxxxxxxxxxxxxxx >> Subject: Re: [PATCH v2 2/2] crypto: caam - check jr permissions before probing >> >> >> Am 2021-11-11 17:46, schrieb Andrey Zhizhikin: >>> Job Rings can be set to be exclusively used by TrustZone which makes >>> the access to those rings only possible from Secure World. This access >>> separation is defined by setting bits in CAAM JRxDID_MS register. Once >>> reserved to be owned by TrustZone, this Job Ring becomes unavailable >>> for the Kernel. This reservation is performed early in the boot >>> process, even before the Kernel starts, which leads to unavailability >>> of the HW at the probing stage. Moreover, the reservation can be done >>> for any Job Ring and is not under control of the Kernel. >>> >>> Current implementation lists Job Rings as child nodes of CAAM driver, >>> and tries to perform probing on those regardless of whether JR HW is >>> accessible or not. >>> >>> This leads to the following error while probing: >>> [ 1.509894] caam 30900000.crypto: job rings = 3, qi = 0 >>> [ 1.525201] caam_jr 30901000.jr: failed to flush job ring 0 >>> [ 1.525214] caam_jr: probe of 30901000.jr failed with error -5 >>> >>> Implement a dynamic mechanism to identify which Job Ring is actually >>> marked as owned by TrustZone, and fail the probing of those child >>> nodes with -ENODEV. >> >> For other reviewers/maintainers: I'm still not sure this is the way to go. Instead >> one can let u-boot fix up the device tree and remove or disable the JR node if its >> not available. > > Just as further clarification: this patch is intended to accommodate for cases where > JR is claimed in S world at the boot and not available for Kernel. It does not account > for fully dynamic cases, where JRs can be reclaimed between S <-> NS Worlds > during runtime. It rather accounts for situation when any arbitrary JR can be reserved > by any software entity before Kernel starts without a need to disable nodes at > compile time. > I prefer f/w to fix the DT before passing it to the kernel, either by adding the "secure-status" property (set explicitly to "disabled") or by removing the job ring node(s) that are reserved. OP-TEE already uses the first option. We should probably pick this up. The reason I am supporting relying on DT and consequently avoiding registers is that accessing page 0 in the caam register space from Non-secure world should be avoided when caam is managed by Secure world (e.g. OP-TEE) or a Secure Enclave (e.g. SECO). Unfortunately support for HW-enforced access control for caam register space is not that great / fine-grained, with the exception of more recent parts like i.MX8MP and i.MX8ULP. Horia