Hi Andrey,
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.
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.
So maybe u-boot (or TF-A) should mark that node as disabled after
all.
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.
..
>> > 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.
..
>> 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 is good
for though). But as mentioned above, I'm not convinced that deciding
at probe time is the solution here.
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.
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
