diff mbox series

crypto: caam: disable pkc for non-E SoCs

Message ID 20210915220307.3079917-1-michael@walle.cc
State Accepted
Commit f20311cc9c58052e0b215013046cbf390937910c
Headers show
Series crypto: caam: disable pkc for non-E SoCs | expand

Commit Message

Michael Walle Sept. 15, 2021, 10:03 p.m. UTC
On newer CAAM versions, not all accelerators are disabled if the SoC is
a non-E variant. While the driver checks most of the modules for
availability, there is one - PKHA - which sticks out. On non-E variants
it is still reported as available, that is the number of instances is
non-zero, but it has limited functionality. In particular it doesn't
support encryption and decryption, but just signing and verifying. This
is indicated by a bit in the PKHA_MISC field. Take this bit into account
if we are checking for availablitly.

This will the following error:
[    8.167817] caam_jr 8020000.jr: 20000b0f: CCB: desc idx 11: : Invalid CHA selected.

Tested on an NXP LS1028A (non-E) SoC.

Fixes: d239b10d4ceb ("crypto: caam - add register map changes cf. Era 10")
Signed-off-by: Michael Walle <michael@walle.cc>
---
 drivers/crypto/caam/caampkc.c | 19 +++++++++++++++----
 drivers/crypto/caam/regs.h    |  3 +++
 2 files changed, 18 insertions(+), 4 deletions(-)

Comments

Horia Geanta Sept. 22, 2021, 3:51 p.m. UTC | #1
On 9/16/2021 1:03 AM, Michael Walle wrote:
> On newer CAAM versions, not all accelerators are disabled if the SoC is

> a non-E variant. While the driver checks most of the modules for

> availability, there is one - PKHA - which sticks out. On non-E variants

Currently there's no dedicated support for "partially disabled" non-E
(export-controlled) parts in Linux kernel caam driver(s).

Up until recently firmware (U-boot) was deleting the "crypto" DT node
for all non-E parts [1].
Modifying the f/w indeed triggers changes across the s/w stack.

Since you are modifying only the caam code handling PKHA, is it correct
to assume that everything else is working fine?
For example: is the number of AES accelerators (AESA_VERSION[AESA_NUM])
being reported as 0 on non-E parts?

> it is still reported as available, that is the number of instances is

> non-zero, but it has limited functionality. In particular it doesn't

> support encryption and decryption, but just signing and verifying. This

> is indicated by a bit in the PKHA_MISC field. Take this bit into account

> if we are checking for availablitly.

typo:			 ^ availability
> 

> This will the following error:

> [    8.167817] caam_jr 8020000.jr: 20000b0f: CCB: desc idx 11: : Invalid CHA selected.

> 

> Tested on an NXP LS1028A (non-E) SoC.

Thanks.
Unfortunately I don't have a non-E part to test on.

Horia

[1] https://lore.kernel.org/u-boot/ff146322-e8c7-2418-ceb1-a3c0d4cee1a1@nxp.com
4eecc6f1a104 ("armv8: layerscape: don't remove crypto node if just partially disabled")
Michael Walle Sept. 22, 2021, 5:55 p.m. UTC | #2
Am 22. September 2021 17:51:23 MESZ schrieb "Horia Geantă" <horia.geanta@nxp.com>:
>On 9/16/2021 1:03 AM, Michael Walle wrote:

>> On newer CAAM versions, not all accelerators are disabled if the SoC is

>> a non-E variant. While the driver checks most of the modules for

>> availability, there is one - PKHA - which sticks out. On non-E variants

>Currently there's no dedicated support for "partially disabled" non-E

>(export-controlled) parts in Linux kernel caam driver(s).

>

>Up until recently firmware (U-boot) was deleting the "crypto" DT node

>for all non-E parts [1].

>Modifying the f/w indeed triggers changes across the s/w stack.


Well, it does check if a module is available or not. And this seem to be the only module which have "something in between". I.e. the number of available modules is not zero but it also doesn't work as expected. 

>Since you are modifying only the caam code handling PKHA, is it correct

>to assume that everything else is working fine?


Everything else is skipped because the "number of instances" is zero. 

>For example: is the number of AES accelerators (AESA_VERSION[AESA_NUM])

>being reported as 0 on non-E parts? 


yes, see above. 

>> it is still reported as available, that is the number of instances is

>> non-zero, but it has limited functionality. In particular it doesn't

>> support encryption and decryption, but just signing and verifying. This

>> is indicated by a bit in the PKHA_MISC field. Take this bit into account

>> if we are checking for availablitly.

>typo:			 ^ availability


If there is nothing else wrong, could this be fixed while applying?

>> 

>> This will the following error:

>> [    8.167817] caam_jr 8020000.jr: 20000b0f: CCB: desc idx 11: : Invalid CHA selected.

>> 

>> Tested on an NXP LS1028A (non-E) SoC.

>Thanks.

>Unfortunately I don't have a non-E part to test on.


You can take a look at the ls1028a RM where this bit is. described to verify what the patch is doing ;) 

Thanks for taking a look. 

-michael 

>

>Horia

>

>[1] https://lore.kernel.org/u-boot/ff146322-e8c7-2418-ceb1-a3c0d4cee1a1@nxp.com

>4eecc6f1a104 ("armv8: layerscape: don't remove crypto node if just partially disabled")
Horia Geanta Sept. 23, 2021, 2:26 p.m. UTC | #3
On 9/22/2021 8:55 PM, Michael Walle wrote:
> Am 22. September 2021 17:51:23 MESZ schrieb "Horia Geantă" <horia.geanta@nxp.com>:

>> On 9/16/2021 1:03 AM, Michael Walle wrote:

>>> On newer CAAM versions, not all accelerators are disabled if the SoC is

>>> a non-E variant. While the driver checks most of the modules for

>>> availability, there is one - PKHA - which sticks out. On non-E variants

>> Currently there's no dedicated support for "partially disabled" non-E

>> (export-controlled) parts in Linux kernel caam driver(s).

>>

>> Up until recently firmware (U-boot) was deleting the "crypto" DT node

>> for all non-E parts [1].

>> Modifying the f/w indeed triggers changes across the s/w stack.

> 

> Well, it does check if a module is available or not. And this seem to be the only module which have "something in between". I.e. the number of available modules is not zero but it also doesn't work as expected. 

> 

>> Since you are modifying only the caam code handling PKHA, is it correct

>> to assume that everything else is working fine?

> 

> Everything else is skipped because the "number of instances" is zero. 

> 

>> For example: is the number of AES accelerators (AESA_VERSION[AESA_NUM])

>> being reported as 0 on non-E parts? 

> 

> yes, see above. 

> 

Ok, thanks for confirming.
Documentation is not clear about what happens when the "NSEC" fuse is blown.

It looks like the reported number of accelerator instances is not reflecting
what's "built" into caam IP block, but what is available.
Blowing the "NSEC" fuse permanently disables some accelerators (e.g. AES),
while others are either not affected (e.g. MDHA, RNG) or partially disabled
(when they implement also functions that are not export-controlled) - only PKHA
seems to be in the last case.

>>> it is still reported as available, that is the number of instances is

>>> non-zero, but it has limited functionality. In particular it doesn't

>>> support encryption and decryption, but just signing and verifying. This

>>> is indicated by a bit in the PKHA_MISC field. Take this bit into account

>>> if we are checking for availablitly.

>> typo:			 ^ availability

> 

> If there is nothing else wrong, could this be fixed while applying?

> 

Fine by me, but Herbert will have to do this.

>>>

>>> This will the following error:

>>> [    8.167817] caam_jr 8020000.jr: 20000b0f: CCB: desc idx 11: : Invalid CHA selected.

>>>

>>> Tested on an NXP LS1028A (non-E) SoC.

>> Thanks.

>> Unfortunately I don't have a non-E part to test on.

> 

> You can take a look at the ls1028a RM where this bit is. described to verify what the patch is doing ;) 

> 

Yes, I've done this already.

Reviewed-by: Horia Geantă <horia.geanta@nxp.com>


Thanks,
Horia
Herbert Xu Sept. 24, 2021, 8:08 a.m. UTC | #4
On Thu, Sep 23, 2021 at 05:26:45PM +0300, Horia Geantă wrote:
>

> >>> it is still reported as available, that is the number of instances is

> >>> non-zero, but it has limited functionality. In particular it doesn't

> >>> support encryption and decryption, but just signing and verifying. This

> >>> is indicated by a bit in the PKHA_MISC field. Take this bit into account

> >>> if we are checking for availablitly.

> >> typo:			 ^ availability

> > 

> > If there is nothing else wrong, could this be fixed while applying?

> > 

> Fine by me, but Herbert will have to do this.


I fixed the typo while applying this.

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
diff mbox series

Patch

diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc.c
index e313233ec6de..bf6275ffc4aa 100644
--- a/drivers/crypto/caam/caampkc.c
+++ b/drivers/crypto/caam/caampkc.c
@@ -1153,16 +1153,27 @@  static struct caam_akcipher_alg caam_rsa = {
 int caam_pkc_init(struct device *ctrldev)
 {
 	struct caam_drv_private *priv = dev_get_drvdata(ctrldev);
-	u32 pk_inst;
+	u32 pk_inst, pkha;
 	int err;
 	init_done = false;
 
 	/* Determine public key hardware accelerator presence. */
-	if (priv->era < 10)
+	if (priv->era < 10) {
 		pk_inst = (rd_reg32(&priv->ctrl->perfmon.cha_num_ls) &
 			   CHA_ID_LS_PK_MASK) >> CHA_ID_LS_PK_SHIFT;
-	else
-		pk_inst = rd_reg32(&priv->ctrl->vreg.pkha) & CHA_VER_NUM_MASK;
+	} else {
+		pkha = rd_reg32(&priv->ctrl->vreg.pkha);
+		pk_inst = pkha & CHA_VER_NUM_MASK;
+
+		/*
+		 * Newer CAAMs support partially disabled functionality. If this is the
+		 * case, the number is non-zero, but this bit is set to indicate that
+		 * no encryption or decryption is supported. Only signing and verifying
+		 * is supported.
+		 */
+		if (pkha & CHA_VER_MISC_PKHA_NO_CRYPT)
+			pk_inst = 0;
+	}
 
 	/* Do not register algorithms if PKHA is not present. */
 	if (!pk_inst)
diff --git a/drivers/crypto/caam/regs.h b/drivers/crypto/caam/regs.h
index af61f3a2c0d4..3738625c0250 100644
--- a/drivers/crypto/caam/regs.h
+++ b/drivers/crypto/caam/regs.h
@@ -322,6 +322,9 @@  struct version_regs {
 /* CHA Miscellaneous Information - AESA_MISC specific */
 #define CHA_VER_MISC_AES_GCM	BIT(1 + CHA_VER_MISC_SHIFT)
 
+/* CHA Miscellaneous Information - PKHA_MISC specific */
+#define CHA_VER_MISC_PKHA_NO_CRYPT	BIT(7 + CHA_VER_MISC_SHIFT)
+
 /*
  * caam_perfmon - Performance Monitor/Secure Memory Status/
  *                CAAM Global Status/Component Version IDs