Thread (36 messages) 36 messages, 6 authors, 2022-01-08

RE: [PATCH] crypto: caam - check jr permissions before probing

From: ZHIZHIKIN Andrey <hidden>
Date: 2021-11-15 09:38:48
Also in: lkml

Hello Michael,
-----Original Message-----
From: Michael Walle <redacted>
Sent: Friday, November 12, 2021 7:56 PM
To: ZHIZHIKIN Andrey <redacted>
Cc: horia.geanta@nxp.com; pankaj.gupta@nxp.com;
herbert@gondor.apana.org.au; davem@davemloft.net;
iuliana.prodan@nxp.com; linux-crypto@vger.kernel.org; linux-
kernel@vger.kernel.org
Subject: Re: [PATCH] crypto: caam - check jr permissions before probing

Hi Andrey,

Am 2021-11-10 10:33, schrieb ZHIZHIKIN Andrey:
quoted
quoted
-----Original Message-----
From: Michael Walle <redacted>
quoted
quoted
First, thank you for taking the extra step here. Does "reserved
for HAB"
mean TF-A is using it or is the BootROM already using it. TF-A is
optional (so is HAB I guess?). So it might be possible to have jr0
in linux, right? If that is correct, the solution to disable the
jr0 at compile time is wrong.
From what I've seen in the U-Boot and ATF code, it seems that the
JR0 is reserved by BootROM. When the execution reaches the ATF
(after SPL) those bits are already set which concludes that the
reservation is done quite early. Since current U-Boot does not have
any support for CAAM integrated yet (patchset is under
review) -
it makes me believe that the reservation is done in BootROM.
Ok. I guess we have to wait for an answer from NXP. But it strikes as
odd that it there is no Secure World, you'll loose one job ring.
From HW perspective, the JR is not lost - it is just assigned to S
world.
I said its lost if there is no Secure World (which IMHO is still a valid case). I mean if
its already the BootROM which assign it
(unconditionally)
and there will be no secure world later in the boot process, then its lost.
Agree, in this case it can be considered as a total loss of JR for NS World.
quoted
This also provides an opportunity (at least for i.MX8M series) to
issue transactions and create Trusted descriptors in S world for NS world.
This is achieved by 2 sets of ICID/DID pairs, and this is where
USE_OUT bit is actually used. This however is missing on the LS series
(SRM does not state this is implemented), which leaves LS with only
one ICID/DID pair per ring.
But this would also be possible if the JR is only acquired later by TF-A (or optee),
no?
If I read the SRM correct, then the answer is rather no. There is a clear separation
that is done between 2 worlds, and they use the JR independently. The i.MX8M
series however adds support to issue transactions from S world on behalf of
both S and NS worlds by utilizing the second pair of ICID/DID, which LS family
does not have.
quoted
From OS perspective however, I would totally agree - the JR is indeed
lost, even if there is no software running that required S world.

The only description of process for control transfer from S to NS
world I was able to find is described in LS1028A SRM section 12.2.2.3,
which only details ring user re-assignment, but it does not detail
whether TZ_OWN can participate in this process (set or reset), and
this is also similar for i.MX8M family.
quoted
quoted
It is correct though: if the JR is not reserved - then it is
accessible in Linux, hence compile-time disabling does not looked
like a good solution to me.
Mh, I had a closer look at the IMX8M SRM (I don't have one for the
IMX8MM yet). It looks like secure world can reassign the Job Ring to
non-secure world though (unless LDID is set). If that is the case I
think, deciding at probe time if a job ring is available is not
correct; as it can be reassigned later.
That's exactly the culprit here: the LDID is not set on the JR
reserved.

This makes it possible for the code executing in S to transfer the JR
to NS.
Practically, I do not see that this would happen though, as even the
NXP proposed to disable the node at compile time, which gives me an
indication that the transfer was never planned. This is however a
dangerous assumption I have to admit, and in the general case - this
transfer can occur.

Moreover, from what I read in the SRM of both i.MX8MM and LS1028A -
there is no lock that is imposed on TZ_OWN bit by setting the LDID (or
LICID for LS family).
I've noticed that too, but then assumed that because TZ_PRIM=1 implies
TZ_OWN=1 and the lock bit will lock TZ_PRIM then TZ_OWN must also be 1.
But that's not the case for LS SoCs.
Correct, if PRIM_TZ is set and locked - then JR cannot be migrated to NS world
until next POR. This is an indirect lock implication I suppose.
quoted
Would it be possible for you to tell which section of SRM provides a
description of the JR transfer you mentioned above?
I don't have access to the IMX8M SEC right now. If memory serves correctly, I just
saw that on an overview. But I just had a look at the LS1028ASECRM, and there is
"SEC's Job Ring interface can be independently assigned (and re-assigned) to
different users." (ch 12.2).
I've seen that in both SRMs, and the description is pretty much the same. What is
not written in this section is the transitions between S <-> NS worlds, and this is 
the piece of information of the interest here... :( Perhaps, NXP can step up here to
comment on this transitions mechanism.
quoted
As for probing of the JR node, then I believe it is rather the
opposite:
deciding whether the JR is available during probing provides an
opportunity to bind the device later on when it would be released from
S to NS world (provided that this would ever occur). However, keeping
in mind that there is no HW mechanism to indicate that the JR appears
in NS world after the boot and the user transfer should be done
manually by some other SW entity - later bind can also be performed
there.
Sure, but it will also be the other way around, no? Like S world can "steal" the JR
from NS world. And thats what I'm worried about.
This is actually the key point, and I neglected this a bit completely unintentionally.

If the software entity running in S World would like to reclaim the JR from Kernel,
then it can do so at any given point of time. This for sure should be carefully crafted
according to "Ring user (re-)assignment procedure" described in SRM, but what this
would mean in practice for the Kernel is: any crypto operation running inside that JR
would either complete or be abrupted. Once S World entity would reclaim the JR,
the NS Word software entity should unbind the JR and stop using it while submitting
operations for CAAM Algos.

There is a conceptual problem here with this scenario, namely: S World should notify
the NS World that a particular resource (JR in our case) is reserved and should not be
used at all. AFAIK this mechanism is not present, and until it is not there - there
would be no chance to realize that the JR is gone.

Please note, that this patch (and consecutive V2 series) are not addressing above
problem and was never intended to. The scenario you're talking about is a part of
a bigger task, which is separate from what this patch covers.

Just to make it clear: this patch (and consecutive V2 series) tried to address the
functionality to dynamically identify which JR is not available for NS World at the
Kernel boot and mark them accordingly. This allows that different derivatives that
have CAAM HW IP to have any JR reserved, and would not require no changes in
DTB to have a node disabled.

There are several key components running in S World before Kernel (BootROM, SPL,
ATF, OP-TEE) and they all can have JR reserved. If any of those software instances
reserve the JR - then currently it should disable it in the DTB as well. This patch allows
them not to do so, and moving the identification logic into the Kernel to dynamically
figure out which resources are there to use.
quoted
The only difference to the current proposed solution would be to
examine the CAAM control register instead of the flag from JR while
probing, and this is what I'm currently testing on my end.
quoted
So maybe u-boot (or TF-A) should mark that node as disabled after
all.
If the U-Boot implementation would come up with similar dynamic
recognition - then it would not be necessary to disable the node there
as well.

This would also ensure that if in later derivatives or ATF code
updates another JR would be reserved as well - there would be no need
to change and align DTB to it, as it would be dynamically recognized.
To be clear, I don't talk about dynamic behavior here. Just try to determine
where the JR should be disabled/removed from the DT. And I'm assuming a static
partition of the JRs between S and NS world.
As I've written above, I believe it would be hard to rely on static partitioning
between S and NS Worlds, as we have several S World agents in the game
before Kernel starts. They either should have a clean contract to establish this
partitioning, or Kernel should be able to see which of those JRs are not available.
This patch addresses the later since we do not have any rules regarding the
partitioning contract.
To recap, NXP suggested to disabled it in the SoC dtsi in u-boot. This depends on
whether the BootROM actually assignes it to S worlds and there is no way for u-
boot to regain access (assuming that u-boot or u-boot SPL will be started in EL3).
If it is not possible to reassign it to NS world, then the JR should be disabled in
linux and u-boot DTs. If there is a chance to regain control and that there might
be no TF-A at all, then statically disable the JR in u-boot is wrong. Instead it should
be determined at runtime (again just static partitioning, no dynamic
reassignment).
Just to add: this proposal was done for i.MX8M Mini SoC only, which does not cover
all other derivatives implementing CAAM.

I assume that if we go with DTB approach - all other derivatives should be revised
and reserve nodes appropriate.
I had a fixup at runtime of the DT (both the active DT in u-boot as well as the DT
passed from u-boot to linux) in mind. Check the TZ_OWN bit and remove/disable
the JR.

There is also an ongoing discussion where and how the DT will be passed to u-
boot and linux. So I might be the case that TF-A will allocate one JR to itself and
just pass u-boot the DT where that JR is disabled or removed. Which might also fit
the "fixup" in u-boot.
Yes, but in this case - all derivatives should have this done, right? I'm not sure how
this can be enforced, and also not sure how to keep this up in the future...
quoted
quoted
If the BootROM is actually already assigning this to secure world
(and setting the lock bit LDID). Then it can also be removed from the
linux dtsi, because there is no way it can be assigned to linux
anyways.
As I've indicated above: the LDID bit is not set on this JR.
Ok, then u-boot should be able to reset the JR to its defaults if its started in EL3
(and there is no TF-A at all), right?
It can, if the CAAM driver finally lands in U-Boot and this functionality is
implemented in that driver. So far, both of those is not covered...

What I've just seen in V5 patch series for CAAM support in U-Boot [1],
there is a dynamic reservation provisioned in SPL for any arbitrary JR number.
Therefore, I believe this patch makes total sense to isolate Kernel and verify
which JR is available at boot.
quoted
quoted
quoted
quoted
quoted
quoted
quoted
diff --git a/drivers/crypto/caam/jr.c
b/drivers/crypto/caam/jr.c index
7f2b1101f567..a260981e0843 100644
--- a/drivers/crypto/caam/jr.c
+++ b/drivers/crypto/caam/jr.c
@@ -511,10 +511,27 @@ static int caam_jr_probe(struct
platform_device
*pdev)
      struct device_node *nprop;
      struct caam_job_ring __iomem *ctrl;
      struct caam_drv_private_jr *jrpriv;
+     struct caam_drv_private *caamctrlpriv;
      static int total_jobrs;
ugh.
Yes, I've seen that. At first, I even wanted to replace it with
the
ctrlpriv->total_jobrs,
but decided not to do it in order to keep this patch focused.
Having the total_jobrs (and using it for anything else than
messages) static will create an unnecessary dependency on the
correct probe order.
Yes, I've stumbled upon this logical problem myself as well.

I'd decided that this should go, and would replace it with the
option to use IRBAR_JRx as the indexing, since it has the address
aligned and can be used as a bit index.
- For LS1028A it would look like: IRBAR_JR[ring_id] >>  16
- For i.MX8M series it would be: IRBAR_JR[ring_id] >>  12

As those offsets are defined in the HW, they can be reliably used
as indexing parameter to interrogate the CAAM if the JR is reserved
for TZ or not.

In addition, in order not to access the caam_ctrl register set from
caam_jr driver, I'd still prefer to use a bitmask and compare the
bits set against that new indexing. This would allow the driver to
get rid of that static total_jobrs part at all.

I would appreciate the community opinion on the approach above
whether it is plausible and does not violate any kernel rules.
Will try to follow you here later.
I'm now working on a patchset that would supersede this one, and would
include the dynamic indexing based on the JR address instead of that
static variable used. This would also allow to re-order JR nodes
inside the DTS and do not rely on the order of appearance.
quoted
..
quoted
quoted
quoted
quoted
in general, does these marcros match with your reference manual?
Which one do you have?
I'm working on the imx8m mini, which has a v4.0 of CAAM, and
this bit is defined in JR[0:2]DID_MS registers.

The map looks like following:
LDID[31], USE_OUT[30], PRIM_ICID[29:19], LAMTD[17], AMTD[16],
TZ_OWN[15], SDID_MS[14:5], PRIM_TZ[4], PRIM_DID[3:0]

Perhaps, there is a deviation here between what is instantiated
in LS vs i.MX series.

At this point, I would also be interested to hear back from NXP
on this.
Probably, but that would also mean we'd have to distiguish between
these too.
You're checking PRIM_TZ which is SDID on the LS1028A (which is an
arbitrary number if I understand it correctly). So the check above
might actually trigger although it shouldn't.
It's maybe the opposite though: on the LS1028A if the TZ is set,
then NS would read SDID as all 0's. This presents the problem that
PRIM_TZ bit defined for i.MX8M series would read as 0 on LS series
and fail the reservation check.
I don't think you have to take the PRIM_TZ bit into account.
PRIM_TZ=1 implies OWN_TZ=1. (I'm not sure what PRIM_TZ=0 and
OWN_TZ=1
quoted
quoted
is good for though). But as mentioned above, I'm not convinced that
deciding at probe time is the solution here.
From what I read, PRIM_TZ bit is mixed into the SDID and also "locks"
JR
register interface to S world. Setting PRIM_TZ=0 and TZ_OWN=1 has
primarily an influence of SDID construction, this is outlined in
JRsDID_MS register description.
quoted
quoted
At this point I'd really like someone from NXP to comment on it,
specifically: is it enough
to just check the TZ bit for all families to recognize that JR is
reserved for usage in Secure world only?
yep.
I've compared both i.MX8M and LS family SRMs, and looks like the
OWN_TZ bit is the only unification point here that can be verified.

I 'm currently testing the implementation where only that bit is
checked and so far I have good results. I would post a V2 as a series
and supersede this patch, where only that check would be included.
quoted
quoted
quoted
That said, whats PRIM_TZ? When is it set?
It is set together with TZ_OWN early at the boot and is used for
several purposes, namely:
to derive SDID_MS (it is done dynamically), and also to indicate
that the access to that JR registers (config, interrupt, buffers,
etc.) are only possible from Secure World.
Thanks, I also read the SRM for this bit, right now.

-michael
--
-michael
Link: [1]: http://patchwork.ozlabs.org/project/uboot/patch/20211115070014.17586-2-gaurav.jain@nxp.com/

-- andrey
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help