Re: [RFC 4/4] crypto: caam - use job ring for RNG instantiation instead of DECO

[Horia Geanta <horia.geanta@nxp.com>]

On 2/25/2019 4:37 PM, Chris Spencer wrote:
> On Mon, 25 Feb 2019 at 14:22, Horia Geanta <horia.geanta@nxp.com> wrote:
>>
>> On 2/22/2019 12:07 PM, spencercw@gmail.com wrote:
>>> From: Chris Spencer <christopher.spencer@sea.co.uk>
>>>
>>> This is required to support the i.MX8.
>>>
>> Why exactly is this required?
>> You should provide more details.
> 
> I don't know. :) These changes were made in [1], although the change
> is actually described in [2] where it says: "Use a Job ring for RNG
> instantiation rather than DECO, even for i.MX6/7 families." This does
> indeed seem to be necessary to get it to work, but I can only
> speculate why.
> 
The commit messages you refer are indeed rather poor, however you need to
understand the context: the history reflects NXP internal development.

It's expected for the changes to be clearly explained, justified when reaching
mainline kernel.

> The original commit which added the run_descriptor_deco0 function [3]
> states "[...] and another function that performs the running of the
> said descriptor using the DECO debug mechanism." My best guess is that
> the DECO debug mechanism is no longer available so it has to go via
> the job ring.
> 
This change is actually orthogonal to i.MX8 MQ support and is related to
SECO-based i.MX8 SoCs (QXP, QM) and/or OP-TEE support.

Access to CAAM global registers (CCSR page 0) won't be allowed in cases when
Linux runs in Normal World.
From this perspective, it makes sense to eliminate accesses to these registers.
Some of the accesses are done for RNG initialization via the DECO debug
mechanism; to remove them, RNG is initialized via the Job Ring interface.
However, I still see some illegal accesses - for e.g. reading RDSTA register to
check which virtual RNGs ("state handles" in RM terms) are already initialized.

It looks like cutting access to CAAM global registers must come with some
assumptions (since it implies some checks can no longer be performed, see RDSTA
above).
For e.g. in case OP-TEE is detected, we could assume that RNG was already
initialized. Similar for other cases like SECO f/w (i.MX8 QM and QXP).
If we make this assumption, it actually means RNG initialization is no longer
needed:

if (!ctrlpriv->mc_en && rng_vid >= 4 && !normal_world) {
	... // rng init as before - using DECO debug mechanism
}

>> Strictly related to the implementation, why is the code moved to a new file?
>> Please do not shuffle the code without providing a good reason.
> 
> This was done in an attempt to keep it reasonably close to the changes
> in the downstream driver. The file was moved in [1] with the
> description "Move the code related to RNG instanciation to another
> file to ease comprehension."
> 
> The RNG initialisation code is not really needed in ctrl.c anymore
> since it is only used in jr.c, so moving it out seems sensible.
> 
> Thanks,
> Chris
> 
> [1] https://source.codeaurora.org/external/imx/linux-imx/commit/drivers/crypto/caam/inst_rng.c?h=imx_4.14.78_1.0.0_ga&id=5fd8fe866172c38e8cd07741d98d79b995f05bc9
> [2] https://source.codeaurora.org/external/imx/linux-imx/commit/drivers/crypto/caam/ctrl.c?h=imx_4.14.78_1.0.0_ga&id=afa53c2c96a54b4432a26372e3b5e7dbccbb9671
> [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/crypto/caam/ctrl.c?id=04cddbfe6b5f334aa337a1a9797eb8914822f2f8
>