Re: [PATCH v2 2/2] crypto: caam - check jr permissions before probing

[Horia Geantă]

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