[PATCHv3 0/4] crypto: inside-secure - broaden driver scope

[PATCHv3 0/4] crypto: inside-secure - broaden driver scope

[Pascal van Leeuwen]

This is a first baby step towards making the inside-secure crypto driver
more broadly useful. The current driver only works for Marvell Armada HW
and requires proprietary firmware, only available under NDA from Marvell,
to be installed. This patch set allows the driver to be used with other
hardware and removes the dependence on that proprietary firmware.

changes since v1:
- changed dev_info's into dev_dbg to reduce normal verbosity
- terminate all message strings with \n
- use priv->version field strictly to enumerate device context
- fixed some code & comment style issues
- removed EIP97/197 references from messages
- use #if(IS_ENABLED(CONFIG_PCI)) to remove all PCI related code
- use #if(IS_ENABLED(CONFIG_OF)) to remove all device tree related code
- do not inline the minifw but read it from /lib/firmware instead

changes since v2:
- split off removal of alg to engine mapping code into separate patch
- replaced some constants with nice defines
- added missing \n to some error messages
- removed some redundant parenthesis
- aligned some #if's properly
- added some comments to clarify code
- report error on FW load for unknown HW instead of loading EIP197B FW
- use readl_relaxed() instead of readl() + cpu_relax() in polling loop
- merged patch "fix null ptr dereference on rmmod for macchiatobin" here
- merged patch "removed unused struct entry"

Pascal van Leeuwen (4):
  crypto: inside-secure - make driver selectable for non-Marvell
    hardware
  crypto: inside-secure - Remove redundant algo to engine mapping code
  crypto: inside-secure - add support for PCI based FPGA development
    board
  crypto: inside-secure - add support for using the EIP197 without
    vendor firmware

 drivers/crypto/Kconfig                         |  12 +-
 drivers/crypto/inside-secure/safexcel.c        | 761 +++++++++++++++++--------
 drivers/crypto/inside-secure/safexcel.h        |  43 +-
 drivers/crypto/inside-secure/safexcel_cipher.c |  11 -
 drivers/crypto/inside-secure/safexcel_hash.c   |  12 -
 drivers/crypto/inside-secure/safexcel_ring.c   |   3 +-
 6 files changed, 586 insertions(+), 256 deletions(-)

--
1.8.3.1

[PATCHv3 1/4] crypto: inside-secure - make driver selectable for non-Marvell hardware

[Pascal van Leeuwen]

While being a generic EIP97/EIP197 driver, the driver was only selectable
for Marvell Armada hardware. This fix makes the driver selectable for any
Device Tree supporting kernel configuration, allowing it to be used for
other compatible hardware by just adding the correct device tree entry.

It also allows the driver to be selected for PCI(E) supporting kernel con-
figurations, to be able to use it with PCIE based FPGA development boards
for pre-silicon driver development by both Inside Secure and its IP custo-
mers.

Signed-off-by: Pascal van Leeuwen <pvanleeuwen@verimatrix.com>
---
 drivers/crypto/Kconfig | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 67af688..0d9f67d 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -716,8 +716,7 @@ source "drivers/crypto/stm32/Kconfig"

 config CRYPTO_DEV_SAFEXCEL
 	tristate "Inside Secure's SafeXcel cryptographic engine driver"
-	depends on OF
-	depends on (ARM64 && ARCH_MVEBU) || (COMPILE_TEST && 64BIT)
+	depends on OF || PCI || COMPILE_TEST
 	select CRYPTO_AES
 	select CRYPTO_AUTHENC
 	select CRYPTO_BLKCIPHER
@@ -729,10 +728,11 @@ config CRYPTO_DEV_SAFEXCEL
 	select CRYPTO_SHA256
 	select CRYPTO_SHA512
 	help
-	  This driver interfaces with the SafeXcel EIP-197 cryptographic engine
-	  designed by Inside Secure. Select this if you want to use CBC/ECB
-	  chain mode, AES cipher mode and SHA1/SHA224/SHA256/SHA512 hash
-	  algorithms.
+	  This driver interfaces with the SafeXcel EIP-97 and EIP-197 cryptographic
+	  engines designed by Inside Secure. It currently accelerates DES, 3DES and
+	  AES block ciphers in ECB and CBC mode, as well as SHA1, SHA224, SHA256,
+	  SHA384 and SHA512 hash algorithms for both basic hash and HMAC.
+	  Additionally, it accelerates combined AES-CBC/HMAC-SHA AEAD operations.

 config CRYPTO_DEV_ARTPEC6
 	tristate "Support for Axis ARTPEC-6/7 hardware crypto acceleration."
--
1.8.3.1

Re: [PATCHv3 1/4] crypto: inside-secure - make driver selectable for non-Marvell hardware

[Antoine Tenart]

Hi Pascal,

On Wed, Jul 31, 2019 at 05:29:16PM +0200, Pascal van Leeuwen wrote:
> While being a generic EIP97/EIP197 driver, the driver was only selectable
> for Marvell Armada hardware. This fix makes the driver selectable for any
> Device Tree supporting kernel configuration, allowing it to be used for
> other compatible hardware by just adding the correct device tree entry.
> 
> It also allows the driver to be selected for PCI(E) supporting kernel con-
> figurations, to be able to use it with PCIE based FPGA development boards
> for pre-silicon driver development by both Inside Secure and its IP custo-
> mers.
> 
> Signed-off-by: Pascal van Leeuwen <pvanleeuwen@verimatrix.com>

Acked-by: Antoine Tenart <antoine.tenart@bootlin.com>

Thanks!
Antoine

> ---
>  drivers/crypto/Kconfig | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index 67af688..0d9f67d 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -716,8 +716,7 @@ source "drivers/crypto/stm32/Kconfig"
> 
>  config CRYPTO_DEV_SAFEXCEL
>  	tristate "Inside Secure's SafeXcel cryptographic engine driver"
> -	depends on OF
> -	depends on (ARM64 && ARCH_MVEBU) || (COMPILE_TEST && 64BIT)
> +	depends on OF || PCI || COMPILE_TEST
>  	select CRYPTO_AES
>  	select CRYPTO_AUTHENC
>  	select CRYPTO_BLKCIPHER
> @@ -729,10 +728,11 @@ config CRYPTO_DEV_SAFEXCEL
>  	select CRYPTO_SHA256
>  	select CRYPTO_SHA512
>  	help
> -	  This driver interfaces with the SafeXcel EIP-197 cryptographic engine
> -	  designed by Inside Secure. Select this if you want to use CBC/ECB
> -	  chain mode, AES cipher mode and SHA1/SHA224/SHA256/SHA512 hash
> -	  algorithms.
> +	  This driver interfaces with the SafeXcel EIP-97 and EIP-197 cryptographic
> +	  engines designed by Inside Secure. It currently accelerates DES, 3DES and
> +	  AES block ciphers in ECB and CBC mode, as well as SHA1, SHA224, SHA256,
> +	  SHA384 and SHA512 hash algorithms for both basic hash and HMAC.
> +	  Additionally, it accelerates combined AES-CBC/HMAC-SHA AEAD operations.
> 
>  config CRYPTO_DEV_ARTPEC6
>  	tristate "Support for Axis ARTPEC-6/7 hardware crypto acceleration."
> --
> 1.8.3.1

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCHv3 2/4] crypto: inside-secure - Remove redundant algo to engine mapping code

[Pascal van Leeuwen]

This removes some code determine which engine has which algorithms which
was effectively redundant (may have been forward-looking?) due to always
enabling all algorithms for all currently supported engines.
A future patch will use a different, more scalable approach to achieve
this. This is removed now because otherwise the next patch will add new
hardware which would otherwise have to be added to all algorithms, so
now is a convenient time to just get rid of this.

Signed-off-by: Pascal van Leeuwen <pvanleeuwen@verimatrix.com>
---
 drivers/crypto/inside-secure/safexcel.c        |  9 ---------
 drivers/crypto/inside-secure/safexcel.h        |  1 -
 drivers/crypto/inside-secure/safexcel_cipher.c | 11 -----------
 drivers/crypto/inside-secure/safexcel_hash.c   | 12 ------------
 4 files changed, 33 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index df43a2c..a066152 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -869,9 +869,6 @@ static int safexcel_register_algorithms(struct safexcel_crypto_priv *priv)
 	for (i = 0; i < ARRAY_SIZE(safexcel_algs); i++) {
 		safexcel_algs[i]->priv = priv;

-		if (!(safexcel_algs[i]->engines & priv->version))
-			continue;
-
 		if (safexcel_algs[i]->type == SAFEXCEL_ALG_TYPE_SKCIPHER)
 			ret = crypto_register_skcipher(&safexcel_algs[i]->alg.skcipher);
 		else if (safexcel_algs[i]->type == SAFEXCEL_ALG_TYPE_AEAD)
@@ -887,9 +884,6 @@ static int safexcel_register_algorithms(struct safexcel_crypto_priv *priv)

 fail:
 	for (j = 0; j < i; j++) {
-		if (!(safexcel_algs[j]->engines & priv->version))
-			continue;
-
 		if (safexcel_algs[j]->type == SAFEXCEL_ALG_TYPE_SKCIPHER)
 			crypto_unregister_skcipher(&safexcel_algs[j]->alg.skcipher);
 		else if (safexcel_algs[j]->type == SAFEXCEL_ALG_TYPE_AEAD)
@@ -906,9 +900,6 @@ static void safexcel_unregister_algorithms(struct safexcel_crypto_priv *priv)
 	int i;

 	for (i = 0; i < ARRAY_SIZE(safexcel_algs); i++) {
-		if (!(safexcel_algs[i]->engines & priv->version))
-			continue;
-
 		if (safexcel_algs[i]->type == SAFEXCEL_ALG_TYPE_SKCIPHER)
 			crypto_unregister_skcipher(&safexcel_algs[i]->alg.skcipher);
 		else if (safexcel_algs[i]->type == SAFEXCEL_ALG_TYPE_AEAD)
diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
index e0c202f..e53c232 100644
--- a/drivers/crypto/inside-secure/safexcel.h
+++ b/drivers/crypto/inside-secure/safexcel.h
@@ -655,7 +655,6 @@ struct safexcel_ahash_export_state {
 struct safexcel_alg_template {
 	struct safexcel_crypto_priv *priv;
 	enum safexcel_alg_type type;
-	u32 engines;
 	union {
 		struct skcipher_alg skcipher;
 		struct aead_alg aead;
diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
index 8cdbdbe..ee8a0c3 100644
--- a/drivers/crypto/inside-secure/safexcel_cipher.c
+++ b/drivers/crypto/inside-secure/safexcel_cipher.c
@@ -881,7 +881,6 @@ static void safexcel_aead_cra_exit(struct crypto_tfm *tfm)

 struct safexcel_alg_template safexcel_alg_ecb_aes = {
 	.type = SAFEXCEL_ALG_TYPE_SKCIPHER,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.skcipher = {
 		.setkey = safexcel_skcipher_aes_setkey,
 		.encrypt = safexcel_ecb_aes_encrypt,
@@ -920,7 +919,6 @@ static int safexcel_cbc_aes_decrypt(struct skcipher_request *req)

 struct safexcel_alg_template safexcel_alg_cbc_aes = {
 	.type = SAFEXCEL_ALG_TYPE_SKCIPHER,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.skcipher = {
 		.setkey = safexcel_skcipher_aes_setkey,
 		.encrypt = safexcel_cbc_aes_encrypt,
@@ -990,7 +988,6 @@ static int safexcel_des_setkey(struct crypto_skcipher *ctfm, const u8 *key,

 struct safexcel_alg_template safexcel_alg_cbc_des = {
 	.type = SAFEXCEL_ALG_TYPE_SKCIPHER,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.skcipher = {
 		.setkey = safexcel_des_setkey,
 		.encrypt = safexcel_cbc_des_encrypt,
@@ -1030,7 +1027,6 @@ static int safexcel_ecb_des_decrypt(struct skcipher_request *req)

 struct safexcel_alg_template safexcel_alg_ecb_des = {
 	.type = SAFEXCEL_ALG_TYPE_SKCIPHER,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.skcipher = {
 		.setkey = safexcel_des_setkey,
 		.encrypt = safexcel_ecb_des_encrypt,
@@ -1093,7 +1089,6 @@ static int safexcel_des3_ede_setkey(struct crypto_skcipher *ctfm,

 struct safexcel_alg_template safexcel_alg_cbc_des3_ede = {
 	.type = SAFEXCEL_ALG_TYPE_SKCIPHER,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.skcipher = {
 		.setkey = safexcel_des3_ede_setkey,
 		.encrypt = safexcel_cbc_des3_ede_encrypt,
@@ -1133,7 +1128,6 @@ static int safexcel_ecb_des3_ede_decrypt(struct skcipher_request *req)

 struct safexcel_alg_template safexcel_alg_ecb_des3_ede = {
 	.type = SAFEXCEL_ALG_TYPE_SKCIPHER,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.skcipher = {
 		.setkey = safexcel_des3_ede_setkey,
 		.encrypt = safexcel_ecb_des3_ede_encrypt,
@@ -1203,7 +1197,6 @@ static int safexcel_aead_sha1_cra_init(struct crypto_tfm *tfm)

 struct safexcel_alg_template safexcel_alg_authenc_hmac_sha1_cbc_aes = {
 	.type = SAFEXCEL_ALG_TYPE_AEAD,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.aead = {
 		.setkey = safexcel_aead_aes_setkey,
 		.encrypt = safexcel_aead_encrypt,
@@ -1238,7 +1231,6 @@ static int safexcel_aead_sha256_cra_init(struct crypto_tfm *tfm)

 struct safexcel_alg_template safexcel_alg_authenc_hmac_sha256_cbc_aes = {
 	.type = SAFEXCEL_ALG_TYPE_AEAD,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.aead = {
 		.setkey = safexcel_aead_aes_setkey,
 		.encrypt = safexcel_aead_encrypt,
@@ -1273,7 +1265,6 @@ static int safexcel_aead_sha224_cra_init(struct crypto_tfm *tfm)

 struct safexcel_alg_template safexcel_alg_authenc_hmac_sha224_cbc_aes = {
 	.type = SAFEXCEL_ALG_TYPE_AEAD,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.aead = {
 		.setkey = safexcel_aead_aes_setkey,
 		.encrypt = safexcel_aead_encrypt,
@@ -1308,7 +1299,6 @@ static int safexcel_aead_sha512_cra_init(struct crypto_tfm *tfm)

 struct safexcel_alg_template safexcel_alg_authenc_hmac_sha512_cbc_aes = {
 	.type = SAFEXCEL_ALG_TYPE_AEAD,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.aead = {
 		.setkey = safexcel_aead_aes_setkey,
 		.encrypt = safexcel_aead_encrypt,
@@ -1343,7 +1333,6 @@ static int safexcel_aead_sha384_cra_init(struct crypto_tfm *tfm)

 struct safexcel_alg_template safexcel_alg_authenc_hmac_sha384_cbc_aes = {
 	.type = SAFEXCEL_ALG_TYPE_AEAD,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.aead = {
 		.setkey = safexcel_aead_aes_setkey,
 		.encrypt = safexcel_aead_encrypt,
diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c
index a80a5e7..2c31536 100644
--- a/drivers/crypto/inside-secure/safexcel_hash.c
+++ b/drivers/crypto/inside-secure/safexcel_hash.c
@@ -802,7 +802,6 @@ static void safexcel_ahash_cra_exit(struct crypto_tfm *tfm)

 struct safexcel_alg_template safexcel_alg_sha1 = {
 	.type = SAFEXCEL_ALG_TYPE_AHASH,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.ahash = {
 		.init = safexcel_sha1_init,
 		.update = safexcel_ahash_update,
@@ -1035,7 +1034,6 @@ static int safexcel_hmac_sha1_setkey(struct crypto_ahash *tfm, const u8 *key,

 struct safexcel_alg_template safexcel_alg_hmac_sha1 = {
 	.type = SAFEXCEL_ALG_TYPE_AHASH,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.ahash = {
 		.init = safexcel_hmac_sha1_init,
 		.update = safexcel_ahash_update,
@@ -1099,7 +1097,6 @@ static int safexcel_sha256_digest(struct ahash_request *areq)

 struct safexcel_alg_template safexcel_alg_sha256 = {
 	.type = SAFEXCEL_ALG_TYPE_AHASH,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.ahash = {
 		.init = safexcel_sha256_init,
 		.update = safexcel_ahash_update,
@@ -1162,7 +1159,6 @@ static int safexcel_sha224_digest(struct ahash_request *areq)

 struct safexcel_alg_template safexcel_alg_sha224 = {
 	.type = SAFEXCEL_ALG_TYPE_AHASH,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.ahash = {
 		.init = safexcel_sha224_init,
 		.update = safexcel_ahash_update,
@@ -1218,7 +1214,6 @@ static int safexcel_hmac_sha224_digest(struct ahash_request *areq)

 struct safexcel_alg_template safexcel_alg_hmac_sha224 = {
 	.type = SAFEXCEL_ALG_TYPE_AHASH,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.ahash = {
 		.init = safexcel_hmac_sha224_init,
 		.update = safexcel_ahash_update,
@@ -1275,7 +1270,6 @@ static int safexcel_hmac_sha256_digest(struct ahash_request *areq)

 struct safexcel_alg_template safexcel_alg_hmac_sha256 = {
 	.type = SAFEXCEL_ALG_TYPE_AHASH,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.ahash = {
 		.init = safexcel_hmac_sha256_init,
 		.update = safexcel_ahash_update,
@@ -1347,7 +1341,6 @@ static int safexcel_sha512_digest(struct ahash_request *areq)

 struct safexcel_alg_template safexcel_alg_sha512 = {
 	.type = SAFEXCEL_ALG_TYPE_AHASH,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.ahash = {
 		.init = safexcel_sha512_init,
 		.update = safexcel_ahash_update,
@@ -1418,7 +1411,6 @@ static int safexcel_sha384_digest(struct ahash_request *areq)

 struct safexcel_alg_template safexcel_alg_sha384 = {
 	.type = SAFEXCEL_ALG_TYPE_AHASH,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.ahash = {
 		.init = safexcel_sha384_init,
 		.update = safexcel_ahash_update,
@@ -1474,7 +1466,6 @@ static int safexcel_hmac_sha512_digest(struct ahash_request *areq)

 struct safexcel_alg_template safexcel_alg_hmac_sha512 = {
 	.type = SAFEXCEL_ALG_TYPE_AHASH,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.ahash = {
 		.init = safexcel_hmac_sha512_init,
 		.update = safexcel_ahash_update,
@@ -1531,7 +1522,6 @@ static int safexcel_hmac_sha384_digest(struct ahash_request *areq)

 struct safexcel_alg_template safexcel_alg_hmac_sha384 = {
 	.type = SAFEXCEL_ALG_TYPE_AHASH,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.ahash = {
 		.init = safexcel_hmac_sha384_init,
 		.update = safexcel_ahash_update,
@@ -1591,7 +1581,6 @@ static int safexcel_md5_digest(struct ahash_request *areq)

 struct safexcel_alg_template safexcel_alg_md5 = {
 	.type = SAFEXCEL_ALG_TYPE_AHASH,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.ahash = {
 		.init = safexcel_md5_init,
 		.update = safexcel_ahash_update,
@@ -1647,7 +1636,6 @@ static int safexcel_hmac_md5_digest(struct ahash_request *areq)

 struct safexcel_alg_template safexcel_alg_hmac_md5 = {
 	.type = SAFEXCEL_ALG_TYPE_AHASH,
-	.engines = EIP97IES | EIP197B | EIP197D,
 	.alg.ahash = {
 		.init = safexcel_hmac_md5_init,
 		.update = safexcel_ahash_update,
--
1.8.3.1

Re: [PATCHv3 2/4] crypto: inside-secure - Remove redundant algo to engine mapping code

[Antoine Tenart]

Hi Pascal,

On Wed, Jul 31, 2019 at 05:29:17PM +0200, Pascal van Leeuwen wrote:
> This removes some code determine which engine has which algorithms which
> was effectively redundant (may have been forward-looking?) due to always
> enabling all algorithms for all currently supported engines.
> A future patch will use a different, more scalable approach to achieve
> this. This is removed now because otherwise the next patch will add new
> hardware which would otherwise have to be added to all algorithms, so
> now is a convenient time to just get rid of this.
> 
> Signed-off-by: Pascal van Leeuwen <pvanleeuwen@verimatrix.com>

Acked-by: Antoine Tenart <antoine.tenart@bootlin.com>

Thanks!
Antoine

> ---
>  drivers/crypto/inside-secure/safexcel.c        |  9 ---------
>  drivers/crypto/inside-secure/safexcel.h        |  1 -
>  drivers/crypto/inside-secure/safexcel_cipher.c | 11 -----------
>  drivers/crypto/inside-secure/safexcel_hash.c   | 12 ------------
>  4 files changed, 33 deletions(-)
> 
> diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
> index df43a2c..a066152 100644
> --- a/drivers/crypto/inside-secure/safexcel.c
> +++ b/drivers/crypto/inside-secure/safexcel.c
> @@ -869,9 +869,6 @@ static int safexcel_register_algorithms(struct safexcel_crypto_priv *priv)
>  	for (i = 0; i < ARRAY_SIZE(safexcel_algs); i++) {
>  		safexcel_algs[i]->priv = priv;
> 
> -		if (!(safexcel_algs[i]->engines & priv->version))
> -			continue;
> -
>  		if (safexcel_algs[i]->type == SAFEXCEL_ALG_TYPE_SKCIPHER)
>  			ret = crypto_register_skcipher(&safexcel_algs[i]->alg.skcipher);
>  		else if (safexcel_algs[i]->type == SAFEXCEL_ALG_TYPE_AEAD)
> @@ -887,9 +884,6 @@ static int safexcel_register_algorithms(struct safexcel_crypto_priv *priv)
> 
>  fail:
>  	for (j = 0; j < i; j++) {
> -		if (!(safexcel_algs[j]->engines & priv->version))
> -			continue;
> -
>  		if (safexcel_algs[j]->type == SAFEXCEL_ALG_TYPE_SKCIPHER)
>  			crypto_unregister_skcipher(&safexcel_algs[j]->alg.skcipher);
>  		else if (safexcel_algs[j]->type == SAFEXCEL_ALG_TYPE_AEAD)
> @@ -906,9 +900,6 @@ static void safexcel_unregister_algorithms(struct safexcel_crypto_priv *priv)
>  	int i;
> 
>  	for (i = 0; i < ARRAY_SIZE(safexcel_algs); i++) {
> -		if (!(safexcel_algs[i]->engines & priv->version))
> -			continue;
> -
>  		if (safexcel_algs[i]->type == SAFEXCEL_ALG_TYPE_SKCIPHER)
>  			crypto_unregister_skcipher(&safexcel_algs[i]->alg.skcipher);
>  		else if (safexcel_algs[i]->type == SAFEXCEL_ALG_TYPE_AEAD)
> diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
> index e0c202f..e53c232 100644
> --- a/drivers/crypto/inside-secure/safexcel.h
> +++ b/drivers/crypto/inside-secure/safexcel.h
> @@ -655,7 +655,6 @@ struct safexcel_ahash_export_state {
>  struct safexcel_alg_template {
>  	struct safexcel_crypto_priv *priv;
>  	enum safexcel_alg_type type;
> -	u32 engines;
>  	union {
>  		struct skcipher_alg skcipher;
>  		struct aead_alg aead;
> diff --git a/drivers/crypto/inside-secure/safexcel_cipher.c b/drivers/crypto/inside-secure/safexcel_cipher.c
> index 8cdbdbe..ee8a0c3 100644
> --- a/drivers/crypto/inside-secure/safexcel_cipher.c
> +++ b/drivers/crypto/inside-secure/safexcel_cipher.c
> @@ -881,7 +881,6 @@ static void safexcel_aead_cra_exit(struct crypto_tfm *tfm)
> 
>  struct safexcel_alg_template safexcel_alg_ecb_aes = {
>  	.type = SAFEXCEL_ALG_TYPE_SKCIPHER,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.skcipher = {
>  		.setkey = safexcel_skcipher_aes_setkey,
>  		.encrypt = safexcel_ecb_aes_encrypt,
> @@ -920,7 +919,6 @@ static int safexcel_cbc_aes_decrypt(struct skcipher_request *req)
> 
>  struct safexcel_alg_template safexcel_alg_cbc_aes = {
>  	.type = SAFEXCEL_ALG_TYPE_SKCIPHER,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.skcipher = {
>  		.setkey = safexcel_skcipher_aes_setkey,
>  		.encrypt = safexcel_cbc_aes_encrypt,
> @@ -990,7 +988,6 @@ static int safexcel_des_setkey(struct crypto_skcipher *ctfm, const u8 *key,
> 
>  struct safexcel_alg_template safexcel_alg_cbc_des = {
>  	.type = SAFEXCEL_ALG_TYPE_SKCIPHER,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.skcipher = {
>  		.setkey = safexcel_des_setkey,
>  		.encrypt = safexcel_cbc_des_encrypt,
> @@ -1030,7 +1027,6 @@ static int safexcel_ecb_des_decrypt(struct skcipher_request *req)
> 
>  struct safexcel_alg_template safexcel_alg_ecb_des = {
>  	.type = SAFEXCEL_ALG_TYPE_SKCIPHER,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.skcipher = {
>  		.setkey = safexcel_des_setkey,
>  		.encrypt = safexcel_ecb_des_encrypt,
> @@ -1093,7 +1089,6 @@ static int safexcel_des3_ede_setkey(struct crypto_skcipher *ctfm,
> 
>  struct safexcel_alg_template safexcel_alg_cbc_des3_ede = {
>  	.type = SAFEXCEL_ALG_TYPE_SKCIPHER,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.skcipher = {
>  		.setkey = safexcel_des3_ede_setkey,
>  		.encrypt = safexcel_cbc_des3_ede_encrypt,
> @@ -1133,7 +1128,6 @@ static int safexcel_ecb_des3_ede_decrypt(struct skcipher_request *req)
> 
>  struct safexcel_alg_template safexcel_alg_ecb_des3_ede = {
>  	.type = SAFEXCEL_ALG_TYPE_SKCIPHER,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.skcipher = {
>  		.setkey = safexcel_des3_ede_setkey,
>  		.encrypt = safexcel_ecb_des3_ede_encrypt,
> @@ -1203,7 +1197,6 @@ static int safexcel_aead_sha1_cra_init(struct crypto_tfm *tfm)
> 
>  struct safexcel_alg_template safexcel_alg_authenc_hmac_sha1_cbc_aes = {
>  	.type = SAFEXCEL_ALG_TYPE_AEAD,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.aead = {
>  		.setkey = safexcel_aead_aes_setkey,
>  		.encrypt = safexcel_aead_encrypt,
> @@ -1238,7 +1231,6 @@ static int safexcel_aead_sha256_cra_init(struct crypto_tfm *tfm)
> 
>  struct safexcel_alg_template safexcel_alg_authenc_hmac_sha256_cbc_aes = {
>  	.type = SAFEXCEL_ALG_TYPE_AEAD,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.aead = {
>  		.setkey = safexcel_aead_aes_setkey,
>  		.encrypt = safexcel_aead_encrypt,
> @@ -1273,7 +1265,6 @@ static int safexcel_aead_sha224_cra_init(struct crypto_tfm *tfm)
> 
>  struct safexcel_alg_template safexcel_alg_authenc_hmac_sha224_cbc_aes = {
>  	.type = SAFEXCEL_ALG_TYPE_AEAD,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.aead = {
>  		.setkey = safexcel_aead_aes_setkey,
>  		.encrypt = safexcel_aead_encrypt,
> @@ -1308,7 +1299,6 @@ static int safexcel_aead_sha512_cra_init(struct crypto_tfm *tfm)
> 
>  struct safexcel_alg_template safexcel_alg_authenc_hmac_sha512_cbc_aes = {
>  	.type = SAFEXCEL_ALG_TYPE_AEAD,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.aead = {
>  		.setkey = safexcel_aead_aes_setkey,
>  		.encrypt = safexcel_aead_encrypt,
> @@ -1343,7 +1333,6 @@ static int safexcel_aead_sha384_cra_init(struct crypto_tfm *tfm)
> 
>  struct safexcel_alg_template safexcel_alg_authenc_hmac_sha384_cbc_aes = {
>  	.type = SAFEXCEL_ALG_TYPE_AEAD,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.aead = {
>  		.setkey = safexcel_aead_aes_setkey,
>  		.encrypt = safexcel_aead_encrypt,
> diff --git a/drivers/crypto/inside-secure/safexcel_hash.c b/drivers/crypto/inside-secure/safexcel_hash.c
> index a80a5e7..2c31536 100644
> --- a/drivers/crypto/inside-secure/safexcel_hash.c
> +++ b/drivers/crypto/inside-secure/safexcel_hash.c
> @@ -802,7 +802,6 @@ static void safexcel_ahash_cra_exit(struct crypto_tfm *tfm)
> 
>  struct safexcel_alg_template safexcel_alg_sha1 = {
>  	.type = SAFEXCEL_ALG_TYPE_AHASH,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.ahash = {
>  		.init = safexcel_sha1_init,
>  		.update = safexcel_ahash_update,
> @@ -1035,7 +1034,6 @@ static int safexcel_hmac_sha1_setkey(struct crypto_ahash *tfm, const u8 *key,
> 
>  struct safexcel_alg_template safexcel_alg_hmac_sha1 = {
>  	.type = SAFEXCEL_ALG_TYPE_AHASH,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.ahash = {
>  		.init = safexcel_hmac_sha1_init,
>  		.update = safexcel_ahash_update,
> @@ -1099,7 +1097,6 @@ static int safexcel_sha256_digest(struct ahash_request *areq)
> 
>  struct safexcel_alg_template safexcel_alg_sha256 = {
>  	.type = SAFEXCEL_ALG_TYPE_AHASH,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.ahash = {
>  		.init = safexcel_sha256_init,
>  		.update = safexcel_ahash_update,
> @@ -1162,7 +1159,6 @@ static int safexcel_sha224_digest(struct ahash_request *areq)
> 
>  struct safexcel_alg_template safexcel_alg_sha224 = {
>  	.type = SAFEXCEL_ALG_TYPE_AHASH,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.ahash = {
>  		.init = safexcel_sha224_init,
>  		.update = safexcel_ahash_update,
> @@ -1218,7 +1214,6 @@ static int safexcel_hmac_sha224_digest(struct ahash_request *areq)
> 
>  struct safexcel_alg_template safexcel_alg_hmac_sha224 = {
>  	.type = SAFEXCEL_ALG_TYPE_AHASH,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.ahash = {
>  		.init = safexcel_hmac_sha224_init,
>  		.update = safexcel_ahash_update,
> @@ -1275,7 +1270,6 @@ static int safexcel_hmac_sha256_digest(struct ahash_request *areq)
> 
>  struct safexcel_alg_template safexcel_alg_hmac_sha256 = {
>  	.type = SAFEXCEL_ALG_TYPE_AHASH,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.ahash = {
>  		.init = safexcel_hmac_sha256_init,
>  		.update = safexcel_ahash_update,
> @@ -1347,7 +1341,6 @@ static int safexcel_sha512_digest(struct ahash_request *areq)
> 
>  struct safexcel_alg_template safexcel_alg_sha512 = {
>  	.type = SAFEXCEL_ALG_TYPE_AHASH,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.ahash = {
>  		.init = safexcel_sha512_init,
>  		.update = safexcel_ahash_update,
> @@ -1418,7 +1411,6 @@ static int safexcel_sha384_digest(struct ahash_request *areq)
> 
>  struct safexcel_alg_template safexcel_alg_sha384 = {
>  	.type = SAFEXCEL_ALG_TYPE_AHASH,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.ahash = {
>  		.init = safexcel_sha384_init,
>  		.update = safexcel_ahash_update,
> @@ -1474,7 +1466,6 @@ static int safexcel_hmac_sha512_digest(struct ahash_request *areq)
> 
>  struct safexcel_alg_template safexcel_alg_hmac_sha512 = {
>  	.type = SAFEXCEL_ALG_TYPE_AHASH,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.ahash = {
>  		.init = safexcel_hmac_sha512_init,
>  		.update = safexcel_ahash_update,
> @@ -1531,7 +1522,6 @@ static int safexcel_hmac_sha384_digest(struct ahash_request *areq)
> 
>  struct safexcel_alg_template safexcel_alg_hmac_sha384 = {
>  	.type = SAFEXCEL_ALG_TYPE_AHASH,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.ahash = {
>  		.init = safexcel_hmac_sha384_init,
>  		.update = safexcel_ahash_update,
> @@ -1591,7 +1581,6 @@ static int safexcel_md5_digest(struct ahash_request *areq)
> 
>  struct safexcel_alg_template safexcel_alg_md5 = {
>  	.type = SAFEXCEL_ALG_TYPE_AHASH,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.ahash = {
>  		.init = safexcel_md5_init,
>  		.update = safexcel_ahash_update,
> @@ -1647,7 +1636,6 @@ static int safexcel_hmac_md5_digest(struct ahash_request *areq)
> 
>  struct safexcel_alg_template safexcel_alg_hmac_md5 = {
>  	.type = SAFEXCEL_ALG_TYPE_AHASH,
> -	.engines = EIP97IES | EIP197B | EIP197D,
>  	.alg.ahash = {
>  		.init = safexcel_hmac_md5_init,
>  		.update = safexcel_ahash_update,
> --
> 1.8.3.1

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[PATCHv3 3/4] crypto: inside-secure - add support for PCI based FPGA development board

[Pascal van Leeuwen]

This patch adds support for a PCIE development board with FPGA from Xilinx,
to facilitate pre-silicon driver development by both Inside Secure and its
IP customers. Since Inside Secure neither produces nor has access to actual
silicon, this is required functionality to allow us to contribute.

Signed-off-by: Pascal van Leeuwen <pvanleeuwen@verimatrix.com>
---
 drivers/crypto/inside-secure/safexcel.c      | 560 +++++++++++++++++++--------
 drivers/crypto/inside-secure/safexcel.h      |  30 +-
 drivers/crypto/inside-secure/safexcel_ring.c |   3 +-
 3 files changed, 420 insertions(+), 173 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index a066152..01e03ca 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/of_platform.h>
 #include <linux/of_irq.h>
+#include <linux/pci.h>
 #include <linux/platform_device.h>
 #include <linux/workqueue.h>

@@ -32,16 +33,17 @@ static void eip197_trc_cache_init(struct safexcel_crypto_priv *priv)
 	u32 val, htable_offset;
 	int i, cs_rc_max, cs_ht_wc, cs_trc_rec_wc, cs_trc_lg_rec_wc;

-	if (priv->version == EIP197B) {
-		cs_rc_max = EIP197B_CS_RC_MAX;
-		cs_ht_wc = EIP197B_CS_HT_WC;
-		cs_trc_rec_wc = EIP197B_CS_TRC_REC_WC;
-		cs_trc_lg_rec_wc = EIP197B_CS_TRC_LG_REC_WC;
-	} else {
+	if (priv->version == EIP197D_MRVL) {
 		cs_rc_max = EIP197D_CS_RC_MAX;
 		cs_ht_wc = EIP197D_CS_HT_WC;
 		cs_trc_rec_wc = EIP197D_CS_TRC_REC_WC;
 		cs_trc_lg_rec_wc = EIP197D_CS_TRC_LG_REC_WC;
+	} else {
+		/* Default to minimum "safe" settings */
+		cs_rc_max = EIP197B_CS_RC_MAX;
+		cs_ht_wc = EIP197B_CS_HT_WC;
+		cs_trc_rec_wc = EIP197B_CS_TRC_REC_WC;
+		cs_trc_lg_rec_wc = EIP197B_CS_TRC_LG_REC_WC;
 	}

 	/* Enable the record cache memory access */
@@ -145,23 +147,19 @@ static int eip197_load_firmwares(struct safexcel_crypto_priv *priv)
 	int i, j, ret = 0, pe;
 	u32 val;

-	switch (priv->version) {
-	case EIP197B:
-		dir = "eip197b";
-		break;
-	case EIP197D:
+	if (priv->version == EIP197D_MRVL)
 		dir = "eip197d";
-		break;
-	default:
-		/* No firmware is required */
-		return 0;
-	}
+	else if (priv->version == EIP197D_MRVL ||
+		 priv->version == EIP197_DEVBRD)
+		dir = "eip197b";
+	else
+		return -ENODEV;

 	for (i = 0; i < FW_NB; i++) {
 		snprintf(fw_path, 31, "inside-secure/%s/%s", dir, fw_name[i]);
 		ret = request_firmware(&fw[i], fw_path, priv->dev);
 		if (ret) {
-			if (priv->version != EIP197B)
+			if (priv->version != EIP197B_MRVL)
 				goto release_fw;

 			/* Fallback to the old firmware location for the
@@ -294,6 +292,9 @@ static int safexcel_hw_init(struct safexcel_crypto_priv *priv)
 	u32 version, val;
 	int i, ret, pe;

+	dev_dbg(priv->dev, "HW init: using %d pipe(s) and %d ring(s)\n",
+		priv->config.pes, priv->config.rings);
+
 	/* Determine endianess and configure byte swap */
 	version = readl(EIP197_HIA_AIC(priv) + EIP197_HIA_VERSION);
 	val = readl(EIP197_HIA_AIC(priv) + EIP197_HIA_MST_CTRL);
@@ -303,8 +304,11 @@ static int safexcel_hw_init(struct safexcel_crypto_priv *priv)
 	else if (((version >> 16) & 0xffff) == EIP197_HIA_VERSION_LE)
 		val |= (EIP197_MST_CTRL_NO_BYTE_SWAP >> 24);

-	/* For EIP197 set maximum number of TX commands to 2^5 = 32 */
-	if (priv->version == EIP197B || priv->version == EIP197D)
+	/*
+	 * For EIP197's only set maximum number of TX commands to 2^5 = 32
+	 * Skip for the EIP97 as it does not have this field.
+	 */
+	if (priv->version != EIP97IES_MRVL)
 		val |= EIP197_MST_CTRL_TX_MAX_CMD(5);

 	writel(val, EIP197_HIA_AIC(priv) + EIP197_HIA_MST_CTRL);
@@ -330,11 +334,10 @@ static int safexcel_hw_init(struct safexcel_crypto_priv *priv)
 		writel(EIP197_DxE_THR_CTRL_RESET_PE,
 		       EIP197_HIA_DFE_THR(priv) + EIP197_HIA_DFE_THR_CTRL(pe));

-		if (priv->version == EIP197B || priv->version == EIP197D) {
-			/* Reset HIA input interface arbiter */
+		if (priv->version != EIP97IES_MRVL)
+			/* Reset HIA input interface arbiter (EIP197 only) */
 			writel(EIP197_HIA_RA_PE_CTRL_RESET,
 			       EIP197_HIA_AIC(priv) + EIP197_HIA_RA_PE_CTRL(pe));
-		}

 		/* DMA transfer size to use */
 		val = EIP197_HIA_DFE_CFG_DIS_DEBUG;
@@ -357,12 +360,11 @@ static int safexcel_hw_init(struct safexcel_crypto_priv *priv)
 		       EIP197_PE_IN_xBUF_THRES_MAX(7),
 		       EIP197_PE(priv) + EIP197_PE_IN_TBUF_THRES(pe));

-		if (priv->version == EIP197B || priv->version == EIP197D) {
+		if (priv->version != EIP97IES_MRVL)
 			/* enable HIA input interface arbiter and rings */
 			writel(EIP197_HIA_RA_PE_CTRL_EN |
 			       GENMASK(priv->config.rings - 1, 0),
 			       EIP197_HIA_AIC(priv) + EIP197_HIA_RA_PE_CTRL(pe));
-		}

 		/* Data Store Engine configuration */

@@ -381,10 +383,11 @@ static int safexcel_hw_init(struct safexcel_crypto_priv *priv)
 		       EIP197_HIA_DxE_CFG_MAX_DATA_SIZE(8);
 		val |= EIP197_HIA_DxE_CFG_DATA_CACHE_CTRL(WR_CACHE_3BITS);
 		val |= EIP197_HIA_DSE_CFG_ALWAYS_BUFFERABLE;
-		/* FIXME: instability issues can occur for EIP97 but disabling it impact
-		 * performances.
+		/*
+		 * FIXME: instability issues can occur for EIP97 but disabling
+		 * it impacts performance.
 		 */
-		if (priv->version == EIP197B || priv->version == EIP197D)
+		if (priv->version != EIP97IES_MRVL)
 			val |= EIP197_HIA_DSE_CFG_EN_SINGLE_WR;
 		writel(val, EIP197_HIA_DSE(priv) + EIP197_HIA_DSE_CFG(pe));

@@ -479,7 +482,7 @@ static int safexcel_hw_init(struct safexcel_crypto_priv *priv)
 	/* Clear any HIA interrupt */
 	writel(GENMASK(30, 20), EIP197_HIA_AIC_G(priv) + EIP197_HIA_AIC_G_ACK);

-	if (priv->version == EIP197B || priv->version == EIP197D) {
+	if (priv->version != EIP97IES_MRVL) {
 		eip197_trc_cache_init(priv);

 		ret = eip197_load_firmwares(priv);
@@ -514,7 +517,8 @@ void safexcel_dequeue(struct safexcel_crypto_priv *priv, int ring)
 	struct safexcel_context *ctx;
 	int ret, nreq = 0, cdesc = 0, rdesc = 0, commands, results;

-	/* If a request wasn't properly dequeued because of a lack of resources,
+	/*
+	 * If a request wasn't properly dequeued because of a lack of resources,
 	 * proceeded it first,
 	 */
 	req = priv->ring[ring].req;
@@ -543,7 +547,8 @@ void safexcel_dequeue(struct safexcel_crypto_priv *priv, int ring)
 		if (backlog)
 			backlog->complete(backlog, -EINPROGRESS);

-		/* In case the send() helper did not issue any command to push
+		/*
+		 * In case the send() helper did not issue any command to push
 		 * to the engine because the input data was cached, continue to
 		 * dequeue other requests as this is valid and not an error.
 		 */
@@ -556,7 +561,8 @@ void safexcel_dequeue(struct safexcel_crypto_priv *priv, int ring)
 	}

 request_failed:
-	/* Not enough resources to handle all the requests. Bail out and save
+	/*
+	 * Not enough resources to handle all the requests. Bail out and save
 	 * the request and the backlog for the next dequeue call (per-ring).
 	 */
 	priv->ring[ring].req = req;
@@ -711,7 +717,8 @@ static inline void safexcel_handle_result_descriptor(struct safexcel_crypto_priv
 		ndesc = ctx->handle_result(priv, ring, req,
 					   &should_complete, &ret);
 		if (ndesc < 0) {
-			dev_err(priv->dev, "failed to handle result (%d)", ndesc);
+			dev_err(priv->dev, "failed to handle result (%d)\n",
+				ndesc);
 			goto acknowledge;
 		}

@@ -731,7 +738,8 @@ static inline void safexcel_handle_result_descriptor(struct safexcel_crypto_priv
 		       EIP197_xDR_PROC_xD_COUNT(tot_descs * priv->config.rd_offset),
 		       EIP197_HIA_RDR(priv, ring) + EIP197_HIA_xDR_PROC_COUNT);

-	/* If the number of requests overflowed the counter, try to proceed more
+	/*
+	 * If the number of requests overflowed the counter, try to proceed more
 	 * requests.
 	 */
 	if (nreq == EIP197_xDR_PROC_xD_PKT_MASK)
@@ -783,7 +791,7 @@ static irqreturn_t safexcel_irq_ring(int irq, void *data)
 			 * reinitialized. This should not happen under
 			 * normal circumstances.
 			 */
-			dev_err(priv->dev, "RDR: fatal error.");
+			dev_err(priv->dev, "RDR: fatal error.\n");
 		} else if (likely(stat & EIP197_xDR_THRESH)) {
 			rc = IRQ_WAKE_THREAD;
 		}
@@ -813,23 +821,45 @@ static irqreturn_t safexcel_irq_ring_thread(int irq, void *data)
 	return IRQ_HANDLED;
 }

-static int safexcel_request_ring_irq(struct platform_device *pdev, const char *name,
+static int safexcel_request_ring_irq(void *pdev, int irqid,
+				     int is_pci_dev,
 				     irq_handler_t handler,
 				     irq_handler_t threaded_handler,
 				     struct safexcel_ring_irq_data *ring_irq_priv)
 {
-	int ret, irq = platform_get_irq_byname(pdev, name);
+	int ret, irq;
+	struct device *dev;
+
+	if (IS_ENABLED(CONFIG_PCI) && is_pci_dev) {
+		struct pci_dev *pci_pdev = pdev;
+
+		dev = &pci_pdev->dev;
+		irq = pci_irq_vector(pci_pdev, irqid);
+		if (irq < 0) {
+			dev_err(dev, "unable to get device MSI IRQ %d (err %d)\n",
+				irqid, irq);
+			return irq;
+		}
+	} else if (IS_ENABLED(CONFIG_OF)) {
+		struct platform_device *plf_pdev = pdev;
+		char irq_name[6] = {0}; /* "ringX\0" */

-	if (irq < 0) {
-		dev_err(&pdev->dev, "unable to get IRQ '%s'\n", name);
-		return irq;
+		snprintf(irq_name, 6, "ring%d", irqid);
+		dev = &plf_pdev->dev;
+		irq = platform_get_irq_byname(plf_pdev, irq_name);
+
+		if (irq < 0) {
+			dev_err(dev, "unable to get IRQ '%s' (err %d)\n",
+				irq_name, irq);
+			return irq;
+		}
 	}

-	ret = devm_request_threaded_irq(&pdev->dev, irq, handler,
+	ret = devm_request_threaded_irq(dev, irq, handler,
 					threaded_handler, IRQF_ONESHOT,
-					dev_name(&pdev->dev), ring_irq_priv);
+					dev_name(dev), ring_irq_priv);
 	if (ret) {
-		dev_err(&pdev->dev, "unable to request IRQ %d\n", irq);
+		dev_err(dev, "unable to request IRQ %d\n", irq);
 		return ret;
 	}

@@ -916,22 +946,20 @@ static void safexcel_configure(struct safexcel_crypto_priv *priv)
 	val = readl(EIP197_HIA_AIC_G(priv) + EIP197_HIA_OPTIONS);

 	/* Read number of PEs from the engine */
-	switch (priv->version) {
-	case EIP197B:
-	case EIP197D:
-		mask = EIP197_N_PES_MASK;
-		break;
-	default:
+	if (priv->version == EIP97IES_MRVL)
+		/* Narrow field width for EIP97 type engine */
 		mask = EIP97_N_PES_MASK;
-	}
+	else
+		/* Wider field width for all EIP197 type engines */
+		mask = EIP197_N_PES_MASK;
+
 	priv->config.pes = (val >> EIP197_N_PES_OFFSET) & mask;

+	priv->config.rings = min_t(u32, val & GENMASK(3, 0), max_rings);
+
 	val = (val & GENMASK(27, 25)) >> 25;
 	mask = BIT(val) - 1;

-	val = readl(EIP197_HIA_AIC_G(priv) + EIP197_HIA_OPTIONS);
-	priv->config.rings = min_t(u32, val & GENMASK(3, 0), max_rings);
-
 	priv->config.cd_size = (sizeof(struct safexcel_command_desc) / sizeof(u32));
 	priv->config.cd_offset = (priv->config.cd_size + mask) & ~mask;

@@ -943,21 +971,7 @@ static void safexcel_init_register_offsets(struct safexcel_crypto_priv *priv)
 {
 	struct safexcel_register_offsets *offsets = &priv->offsets;

-	switch (priv->version) {
-	case EIP197B:
-	case EIP197D:
-		offsets->hia_aic	= EIP197_HIA_AIC_BASE;
-		offsets->hia_aic_g	= EIP197_HIA_AIC_G_BASE;
-		offsets->hia_aic_r	= EIP197_HIA_AIC_R_BASE;
-		offsets->hia_aic_xdr	= EIP197_HIA_AIC_xDR_BASE;
-		offsets->hia_dfe	= EIP197_HIA_DFE_BASE;
-		offsets->hia_dfe_thr	= EIP197_HIA_DFE_THR_BASE;
-		offsets->hia_dse	= EIP197_HIA_DSE_BASE;
-		offsets->hia_dse_thr	= EIP197_HIA_DSE_THR_BASE;
-		offsets->hia_gen_cfg	= EIP197_HIA_GEN_CFG_BASE;
-		offsets->pe		= EIP197_PE_BASE;
-		break;
-	case EIP97IES:
+	if (priv->version == EIP97IES_MRVL) {
 		offsets->hia_aic	= EIP97_HIA_AIC_BASE;
 		offsets->hia_aic_g	= EIP97_HIA_AIC_G_BASE;
 		offsets->hia_aic_r	= EIP97_HIA_AIC_R_BASE;
@@ -968,135 +982,119 @@ static void safexcel_init_register_offsets(struct safexcel_crypto_priv *priv)
 		offsets->hia_dse_thr	= EIP97_HIA_DSE_THR_BASE;
 		offsets->hia_gen_cfg	= EIP97_HIA_GEN_CFG_BASE;
 		offsets->pe		= EIP97_PE_BASE;
-		break;
+	} else {
+		offsets->hia_aic	= EIP197_HIA_AIC_BASE;
+		offsets->hia_aic_g	= EIP197_HIA_AIC_G_BASE;
+		offsets->hia_aic_r	= EIP197_HIA_AIC_R_BASE;
+		offsets->hia_aic_xdr	= EIP197_HIA_AIC_xDR_BASE;
+		offsets->hia_dfe	= EIP197_HIA_DFE_BASE;
+		offsets->hia_dfe_thr	= EIP197_HIA_DFE_THR_BASE;
+		offsets->hia_dse	= EIP197_HIA_DSE_BASE;
+		offsets->hia_dse_thr	= EIP197_HIA_DSE_THR_BASE;
+		offsets->hia_gen_cfg	= EIP197_HIA_GEN_CFG_BASE;
+		offsets->pe		= EIP197_PE_BASE;
 	}
 }

-static int safexcel_probe(struct platform_device *pdev)
+/*
+ * Generic part of probe routine, shared by platform and PCI driver
+ *
+ * Assumes IO resources have been mapped, private data mem has been allocated,
+ * clocks have been enabled, device pointer has been assigned etc.
+ *
+ */
+static int safexcel_probe_generic(void *pdev,
+				  struct safexcel_crypto_priv *priv,
+				  int is_pci_dev)
 {
-	struct device *dev = &pdev->dev;
-	struct resource *res;
-	struct safexcel_crypto_priv *priv;
+	struct device *dev = priv->dev;
 	int i, ret;

-	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
-	if (!priv)
+	priv->context_pool = dmam_pool_create("safexcel-context", dev,
+					      sizeof(struct safexcel_context_record),
+					      1, 0);
+	if (!priv->context_pool)
 		return -ENOMEM;

-	priv->dev = dev;
-	priv->version = (enum safexcel_eip_version)of_device_get_match_data(dev);
-
-	if (priv->version == EIP197B || priv->version == EIP197D)
-		priv->flags |= EIP197_TRC_CACHE;
-
 	safexcel_init_register_offsets(priv);

-	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
-	priv->base = devm_ioremap_resource(dev, res);
-	if (IS_ERR(priv->base)) {
-		dev_err(dev, "failed to get resource\n");
-		return PTR_ERR(priv->base);
-	}
+	if (priv->version != EIP97IES_MRVL)
+		priv->flags |= EIP197_TRC_CACHE;

-	priv->clk = devm_clk_get(&pdev->dev, NULL);
-	ret = PTR_ERR_OR_ZERO(priv->clk);
-	/* The clock isn't mandatory */
-	if  (ret != -ENOENT) {
-		if (ret)
-			return ret;
+	safexcel_configure(priv);

-		ret = clk_prepare_enable(priv->clk);
-		if (ret) {
-			dev_err(dev, "unable to enable clk (%d)\n", ret);
+	if (IS_ENABLED(CONFIG_PCI) && priv->version == EIP197_DEVBRD) {
+		/*
+		 * Request MSI vectors for global + 1 per ring -
+		 * or just 1 for older dev images
+		 */
+		struct pci_dev *pci_pdev = pdev;
+
+		ret = pci_alloc_irq_vectors(pci_pdev,
+					    priv->config.rings + 1,
+					    priv->config.rings + 1,
+					    PCI_IRQ_MSI | PCI_IRQ_MSIX);
+		if (ret < 0) {
+			dev_err(dev, "Failed to allocate PCI MSI interrupts\n");
 			return ret;
 		}
 	}

-	priv->reg_clk = devm_clk_get(&pdev->dev, "reg");
-	ret = PTR_ERR_OR_ZERO(priv->reg_clk);
-	/* The clock isn't mandatory */
-	if  (ret != -ENOENT) {
-		if (ret)
-			goto err_core_clk;
-
-		ret = clk_prepare_enable(priv->reg_clk);
-		if (ret) {
-			dev_err(dev, "unable to enable reg clk (%d)\n", ret);
-			goto err_core_clk;
-		}
-	}
-
-	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
-	if (ret)
-		goto err_reg_clk;
-
-	priv->context_pool = dmam_pool_create("safexcel-context", dev,
-					      sizeof(struct safexcel_context_record),
-					      1, 0);
-	if (!priv->context_pool) {
-		ret = -ENOMEM;
-		goto err_reg_clk;
-	}
-
-	safexcel_configure(priv);
-
+	/* Register the ring IRQ handlers and configure the rings */
 	priv->ring = devm_kcalloc(dev, priv->config.rings,
 				  sizeof(*priv->ring),
 				  GFP_KERNEL);
-	if (!priv->ring) {
-		ret = -ENOMEM;
-		goto err_reg_clk;
-	}
+	if (!priv->ring)
+		return -ENOMEM;

 	for (i = 0; i < priv->config.rings; i++) {
-		char irq_name[6] = {0}; /* "ringX\0" */
-		char wq_name[9] = {0}; /* "wq_ringX\0" */
+		char wq_name[9] = {0};
 		int irq;
 		struct safexcel_ring_irq_data *ring_irq;

 		ret = safexcel_init_ring_descriptors(priv,
 						     &priv->ring[i].cdr,
 						     &priv->ring[i].rdr);
-		if (ret)
-			goto err_reg_clk;
+		if (ret) {
+			dev_err(dev, "Failed to initialize rings\n");
+			return ret;
+		}

 		priv->ring[i].rdr_req = devm_kcalloc(dev,
 			EIP197_DEFAULT_RING_SIZE,
 			sizeof(priv->ring[i].rdr_req),
 			GFP_KERNEL);
-		if (!priv->ring[i].rdr_req) {
-			ret = -ENOMEM;
-			goto err_reg_clk;
-		}
+		if (!priv->ring[i].rdr_req)
+			return -ENOMEM;

 		ring_irq = devm_kzalloc(dev, sizeof(*ring_irq), GFP_KERNEL);
-		if (!ring_irq) {
-			ret = -ENOMEM;
-			goto err_reg_clk;
-		}
+		if (!ring_irq)
+			return -ENOMEM;

 		ring_irq->priv = priv;
 		ring_irq->ring = i;

-		snprintf(irq_name, 6, "ring%d", i);
-		irq = safexcel_request_ring_irq(pdev, irq_name, safexcel_irq_ring,
+		irq = safexcel_request_ring_irq(pdev,
+						EIP197_IRQ_NUMBER(i, is_pci_dev),
+						is_pci_dev,
+						safexcel_irq_ring,
 						safexcel_irq_ring_thread,
 						ring_irq);
 		if (irq < 0) {
-			ret = irq;
-			goto err_reg_clk;
+			dev_err(dev, "Failed to get IRQ ID for ring %d\n", i);
+			return irq;
 		}

 		priv->ring[i].work_data.priv = priv;
 		priv->ring[i].work_data.ring = i;
-		INIT_WORK(&priv->ring[i].work_data.work, safexcel_dequeue_work);
+		INIT_WORK(&priv->ring[i].work_data.work,
+			  safexcel_dequeue_work);

 		snprintf(wq_name, 9, "wq_ring%d", i);
-		priv->ring[i].workqueue = create_singlethread_workqueue(wq_name);
-		if (!priv->ring[i].workqueue) {
-			ret = -ENOMEM;
-			goto err_reg_clk;
-		}
+		priv->ring[i].workqueue =
+			create_singlethread_workqueue(wq_name);
+		if (!priv->ring[i].workqueue)
+			return -ENOMEM;

 		priv->ring[i].requests = 0;
 		priv->ring[i].busy = false;
@@ -1108,28 +1106,21 @@ static int safexcel_probe(struct platform_device *pdev)
 		spin_lock_init(&priv->ring[i].queue_lock);
 	}

-	platform_set_drvdata(pdev, priv);
 	atomic_set(&priv->ring_used, 0);

 	ret = safexcel_hw_init(priv);
 	if (ret) {
-		dev_err(dev, "EIP h/w init failed (%d)\n", ret);
-		goto err_reg_clk;
+		dev_err(dev, "HW init failed (%d)\n", ret);
+		return ret;
 	}

 	ret = safexcel_register_algorithms(priv);
 	if (ret) {
 		dev_err(dev, "Failed to register algorithms (%d)\n", ret);
-		goto err_reg_clk;
+		return ret;
 	}

 	return 0;
-
-err_reg_clk:
-	clk_disable_unprepare(priv->reg_clk);
-err_core_clk:
-	clk_disable_unprepare(priv->clk);
-	return ret;
 }

 static void safexcel_hw_reset_rings(struct safexcel_crypto_priv *priv)
@@ -1151,6 +1142,80 @@ static void safexcel_hw_reset_rings(struct safexcel_crypto_priv *priv)
 	}
 }

+#if IS_ENABLED(CONFIG_OF)
+/*
+ * for Device Tree platform driver
+ */
+
+static int safexcel_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct resource *res;
+	struct safexcel_crypto_priv *priv;
+	int ret;
+
+	priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	priv->version = (enum safexcel_eip_version)of_device_get_match_data(dev);
+
+	platform_set_drvdata(pdev, priv);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	priv->base = devm_ioremap_resource(dev, res);
+	if (IS_ERR(priv->base)) {
+		dev_err(dev, "failed to get resource\n");
+		return PTR_ERR(priv->base);
+	}
+
+	priv->clk = devm_clk_get(&pdev->dev, NULL);
+	ret = PTR_ERR_OR_ZERO(priv->clk);
+	/* The clock isn't mandatory */
+	if  (ret != -ENOENT) {
+		if (ret)
+			return ret;
+
+		ret = clk_prepare_enable(priv->clk);
+		if (ret) {
+			dev_err(dev, "unable to enable clk (%d)\n", ret);
+			return ret;
+		}
+	}
+
+	priv->reg_clk = devm_clk_get(&pdev->dev, "reg");
+	ret = PTR_ERR_OR_ZERO(priv->reg_clk);
+	/* The clock isn't mandatory */
+	if  (ret != -ENOENT) {
+		if (ret)
+			goto err_core_clk;
+
+		ret = clk_prepare_enable(priv->reg_clk);
+		if (ret) {
+			dev_err(dev, "unable to enable reg clk (%d)\n", ret);
+			goto err_core_clk;
+		}
+	}
+
+	ret = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+	if (ret)
+		goto err_reg_clk;
+
+	/* Generic EIP97/EIP197 device probing */
+	ret = safexcel_probe_generic(pdev, priv, 0);
+	if (ret)
+		goto err_reg_clk;
+
+	return 0;
+
+err_reg_clk:
+	clk_disable_unprepare(priv->reg_clk);
+err_core_clk:
+	clk_disable_unprepare(priv->clk);
+	return ret;
+}
+
 static int safexcel_remove(struct platform_device *pdev)
 {
 	struct safexcel_crypto_priv *priv = platform_get_drvdata(pdev);
@@ -1170,30 +1235,28 @@ static int safexcel_remove(struct platform_device *pdev)
 static const struct of_device_id safexcel_of_match_table[] = {
 	{
 		.compatible = "inside-secure,safexcel-eip97ies",
-		.data = (void *)EIP97IES,
+		.data = (void *)EIP97IES_MRVL,
 	},
 	{
 		.compatible = "inside-secure,safexcel-eip197b",
-		.data = (void *)EIP197B,
+		.data = (void *)EIP197B_MRVL,
 	},
 	{
 		.compatible = "inside-secure,safexcel-eip197d",
-		.data = (void *)EIP197D,
+		.data = (void *)EIP197D_MRVL,
 	},
+	/* For backward compatibility and intended for generic use */
 	{
-		/* Deprecated. Kept for backward compatibility. */
 		.compatible = "inside-secure,safexcel-eip97",
-		.data = (void *)EIP97IES,
+		.data = (void *)EIP97IES_MRVL,
 	},
 	{
-		/* Deprecated. Kept for backward compatibility. */
 		.compatible = "inside-secure,safexcel-eip197",
-		.data = (void *)EIP197B,
+		.data = (void *)EIP197B_MRVL,
 	},
 	{},
 };

-
 static struct platform_driver  crypto_safexcel = {
 	.probe		= safexcel_probe,
 	.remove		= safexcel_remove,
@@ -1202,10 +1265,169 @@ static int safexcel_remove(struct platform_device *pdev)
 		.of_match_table = safexcel_of_match_table,
 	},
 };
-module_platform_driver(crypto_safexcel);
+#endif
+
+#if IS_ENABLED(CONFIG_PCI)
+/*
+ * PCIE devices - i.e. Inside Secure development boards
+ */
+
+static int crypto_is_pci_probe(struct pci_dev *pdev,
+			       const struct pci_device_id *ent)
+{
+	struct device *dev = &pdev->dev;
+	struct safexcel_crypto_priv *priv;
+	void __iomem *pciebase;
+	int rc;
+	u32 val;
+
+	dev_dbg(dev, "Probing PCIE device: vendor %04x, device %04x, subv %04x, subdev %04x, ctxt %lx\n",
+		ent->vendor, ent->device, ent->subvendor,
+		ent->subdevice, ent->driver_data);
+
+	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = dev;
+	priv->version = (enum safexcel_eip_version)ent->driver_data;
+
+	pci_set_drvdata(pdev, priv);
+
+	/* enable the device */
+	rc = pcim_enable_device(pdev);
+	if (rc) {
+		dev_err(dev, "Failed to enable PCI device\n");
+		return rc;
+	}
+
+	/* take ownership of PCI BAR0 */
+	rc = pcim_iomap_regions(pdev, 1, "crypto_safexcel");
+	if (rc) {
+		dev_err(dev, "Failed to map IO region for BAR0\n");
+		return rc;
+	}
+	priv->base = pcim_iomap_table(pdev)[0];
+
+	if (priv->version == EIP197_DEVBRD) {
+		dev_dbg(dev, "Device identified as FPGA based development board - applying HW reset\n");
+
+		rc = pcim_iomap_regions(pdev, 4, "crypto_safexcel");
+		if (rc) {
+			dev_err(dev, "Failed to map IO region for BAR4\n");
+			return rc;
+		}
+
+		pciebase = pcim_iomap_table(pdev)[2];
+		val = readl(pciebase + EIP197_XLX_IRQ_BLOCK_ID_ADDR);
+		if ((val >> 16) == EIP197_XLX_IRQ_BLOCK_ID_VALUE) {
+			dev_dbg(dev, "Detected Xilinx PCIE IRQ block version %d, multiple MSI support enabled\n",
+				(val & 0xff));
+
+			/* Setup MSI identity map mapping */
+			writel(EIP197_XLX_USER_VECT_LUT0_IDENT,
+			       pciebase + EIP197_XLX_USER_VECT_LUT0_ADDR);
+			writel(EIP197_XLX_USER_VECT_LUT1_IDENT,
+			       pciebase + EIP197_XLX_USER_VECT_LUT1_ADDR);
+			writel(EIP197_XLX_USER_VECT_LUT2_IDENT,
+			       pciebase + EIP197_XLX_USER_VECT_LUT2_ADDR);
+			writel(EIP197_XLX_USER_VECT_LUT3_IDENT,
+			       pciebase + EIP197_XLX_USER_VECT_LUT3_ADDR);
+
+			/* Enable all device interrupts */
+			writel(GENMASK(31, 0),
+			       pciebase + EIP197_XLX_USER_INT_ENB_MSK);
+		} else {
+			dev_err(dev, "Unrecognised IRQ block identifier %x\n",
+				val);
+			return -ENODEV;
+		}
+
+		/* HW reset FPGA dev board */
+		/* assert reset */
+		writel(1, priv->base + EIP197_XLX_GPIO_BASE);
+		wmb(); /* maintain strict ordering for accesses here */
+		/* deassert reset */
+		writel(0, priv->base + EIP197_XLX_GPIO_BASE);
+		wmb(); /* maintain strict ordering for accesses here */
+	}
+
+	/* enable bus mastering */
+	pci_set_master(pdev);
+
+	/* Generic EIP97/EIP197 device probing */
+	rc = safexcel_probe_generic(pdev, priv, 1);
+	return rc;
+}
+
+void crypto_is_pci_remove(struct pci_dev *pdev)
+{
+	struct safexcel_crypto_priv *priv = pci_get_drvdata(pdev);
+	int i;
+
+	safexcel_unregister_algorithms(priv);
+
+	for (i = 0; i < priv->config.rings; i++)
+		destroy_workqueue(priv->ring[i].workqueue);
+
+	safexcel_hw_reset_rings(priv);
+}
+
+static const struct pci_device_id crypto_is_pci_ids[] = {
+	{
+		PCI_DEVICE_SUB(PCI_VENDOR_ID_XILINX, 0x9038,
+			       0x16ae, 0xc522),
+		/* assume EIP197B for now */
+		.driver_data = EIP197_DEVBRD,
+	},
+	{},
+};
+
+MODULE_DEVICE_TABLE(pci, crypto_is_pci_ids);
+
+static struct pci_driver crypto_is_pci_driver = {
+	.name          = "crypto-safexcel",
+	.id_table      = crypto_is_pci_ids,
+	.probe         = crypto_is_pci_probe,
+	.remove        = crypto_is_pci_remove,
+};
+#endif
+
+static int __init crypto_is_init(void)
+{
+	int rc;
+
+#if IS_ENABLED(CONFIG_OF)
+		/* Register platform driver */
+		platform_driver_register(&crypto_safexcel);
+#endif
+
+#if IS_ENABLED(CONFIG_PCI)
+		/* Register PCI driver */
+		rc = pci_register_driver(&crypto_is_pci_driver);
+#endif
+
+	return 0;
+}
+
+static void __exit crypto_is_exit(void)
+{
+#if IS_ENABLED(CONFIG_OF)
+		/* Unregister platform driver */
+		platform_driver_unregister(&crypto_safexcel);
+#endif
+
+#if IS_ENABLED(CONFIG_PCI)
+		/* Unregister PCI driver if successfully registered before */
+		pci_unregister_driver(&crypto_is_pci_driver);
+#endif
+}
+
+module_init(crypto_is_init);
+module_exit(crypto_is_exit);

 MODULE_AUTHOR("Antoine Tenart <antoine.tenart@free-electrons.com>");
 MODULE_AUTHOR("Ofer Heifetz <oferh@marvell.com>");
 MODULE_AUTHOR("Igal Liberman <igall@marvell.com>");
-MODULE_DESCRIPTION("Support for SafeXcel cryptographic engine EIP197");
+MODULE_DESCRIPTION("Support for SafeXcel cryptographic engines: EIP97 & EIP197");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
index e53c232..a9a239b 100644
--- a/drivers/crypto/inside-secure/safexcel.h
+++ b/drivers/crypto/inside-secure/safexcel.h
@@ -38,6 +38,27 @@
 	char __##name##_desc[size] CRYPTO_MINALIGN_ATTR; \
 	struct type##_request *name = (void *)__##name##_desc

+/* Xilinx dev board base offsets */
+#define EIP197_XLX_GPIO_BASE		0x200000
+#define EIP197_XLX_IRQ_BLOCK_ID_ADDR	0x2000
+#define EIP197_XLX_IRQ_BLOCK_ID_VALUE	0x1fc2
+#define EIP197_XLX_USER_INT_ENB_MSK	0x2004
+#define EIP197_XLX_USER_INT_ENB_SET	0x2008
+#define EIP197_XLX_USER_INT_ENB_CLEAR	0x200c
+#define EIP197_XLX_USER_INT_BLOCK	0x2040
+#define EIP197_XLX_USER_INT_PEND	0x2048
+#define EIP197_XLX_USER_VECT_LUT0_ADDR	0x2080
+#define EIP197_XLX_USER_VECT_LUT0_IDENT	0x03020100
+#define EIP197_XLX_USER_VECT_LUT1_ADDR	0x2084
+#define EIP197_XLX_USER_VECT_LUT1_IDENT	0x07060504
+#define EIP197_XLX_USER_VECT_LUT2_ADDR	0x2088
+#define EIP197_XLX_USER_VECT_LUT2_IDENT	0x0b0a0908
+#define EIP197_XLX_USER_VECT_LUT3_ADDR	0x208c
+#define EIP197_XLX_USER_VECT_LUT3_IDENT	0x0f0e0d0c
+
+/* Helper defines for probe function */
+#define EIP197_IRQ_NUMBER(i, is_pci)	(i + is_pci)
+
 /* Register base offsets */
 #define EIP197_HIA_AIC(priv)		((priv)->base + (priv)->offsets.hia_aic)
 #define EIP197_HIA_AIC_G(priv)		((priv)->base + (priv)->offsets.hia_aic_g)
@@ -581,10 +602,13 @@ struct safexcel_ring {
 	struct crypto_async_request *backlog;
 };

+/* EIP integration context flags */
 enum safexcel_eip_version {
-	EIP97IES = BIT(0),
-	EIP197B  = BIT(1),
-	EIP197D  = BIT(2),
+	/* Platform (EIP integration context) specifier */
+	EIP97IES_MRVL,
+	EIP197B_MRVL,
+	EIP197D_MRVL,
+	EIP197_DEVBRD
 };

 struct safexcel_register_offsets {
diff --git a/drivers/crypto/inside-secure/safexcel_ring.c b/drivers/crypto/inside-secure/safexcel_ring.c
index 142bc3f..2402a62 100644
--- a/drivers/crypto/inside-secure/safexcel_ring.c
+++ b/drivers/crypto/inside-secure/safexcel_ring.c
@@ -145,7 +145,8 @@ struct safexcel_command_desc *safexcel_add_cdesc(struct safexcel_crypto_priv *pr
 			(lower_32_bits(context) & GENMASK(31, 2)) >> 2;
 		cdesc->control_data.context_hi = upper_32_bits(context);

-		if (priv->version == EIP197B || priv->version == EIP197D)
+		if (priv->version == EIP197B_MRVL ||
+		    priv->version == EIP197D_MRVL)
 			cdesc->control_data.options |= EIP197_OPTION_RC_AUTO;

 		/* TODO: large xform HMAC with SHA-384/512 uses refresh = 3 */
--
1.8.3.1

Re: [PATCHv3 3/4] crypto: inside-secure - add support for PCI based FPGA development board

[Antoine Tenart]

Hello Pascal,

The patch looks mostly good, just a few comments below.

On Wed, Jul 31, 2019 at 05:29:18PM +0200, Pascal van Leeuwen wrote:
> @@ -381,10 +383,11 @@ static int safexcel_hw_init(struct safexcel_crypto_priv *priv)
>  		       EIP197_HIA_DxE_CFG_MAX_DATA_SIZE(8);
>  		val |= EIP197_HIA_DxE_CFG_DATA_CACHE_CTRL(WR_CACHE_3BITS);
>  		val |= EIP197_HIA_DSE_CFG_ALWAYS_BUFFERABLE;
> -		/* FIXME: instability issues can occur for EIP97 but disabling it impact
> -		 * performances.
> +		/*
> +		 * FIXME: instability issues can occur for EIP97 but disabling
> +		 * it impacts performance.
>  		 */

No need to change the comment style here.

> @@ -514,7 +517,8 @@ void safexcel_dequeue(struct safexcel_crypto_priv *priv, int ring)
>  	struct safexcel_context *ctx;
>  	int ret, nreq = 0, cdesc = 0, rdesc = 0, commands, results;
> 
> -	/* If a request wasn't properly dequeued because of a lack of resources,
> +	/*
> +	 * If a request wasn't properly dequeued because of a lack of resources,
>  	 * proceeded it first,
>  	 */

Ditto.

> @@ -543,7 +547,8 @@ void safexcel_dequeue(struct safexcel_crypto_priv *priv, int ring)
> 
> -		/* In case the send() helper did not issue any command to push
> +		/*
> +		 * In case the send() helper did not issue any command to push

Ditto.

> -	/* Not enough resources to handle all the requests. Bail out and save
> +	/*
> +	 * Not enough resources to handle all the requests. Bail out and save

Ditto.

> @@ -731,7 +738,8 @@ static inline void safexcel_handle_result_descriptor(struct safexcel_crypto_priv
>  		       EIP197_xDR_PROC_xD_COUNT(tot_descs * priv->config.rd_offset),
>  		       EIP197_HIA_RDR(priv, ring) + EIP197_HIA_xDR_PROC_COUNT);
> 
> -	/* If the number of requests overflowed the counter, try to proceed more
> +	/*
> +	 * If the number of requests overflowed the counter, try to proceed more

Ditto.

> +#if IS_ENABLED(CONFIG_OF)
> +/*
> + * for Device Tree platform driver
> + */

Single line comment should be:

/* comment */

> +#if IS_ENABLED(CONFIG_PCI)
> +/*
> + * PCIE devices - i.e. Inside Secure development boards
> + */

Here as well.

> +static int crypto_is_pci_probe(struct pci_dev *pdev,
> +			       const struct pci_device_id *ent)

The whole driver uses the "safexcel_" prefix for functions. You should
use it here as well for consistency.

> +void crypto_is_pci_remove(struct pci_dev *pdev)

Here as well.

> +static const struct pci_device_id crypto_is_pci_ids[] = {

Here as well.

> +static struct pci_driver crypto_is_pci_driver = {

Here as well.

> +static int __init crypto_is_init(void)

Here as well.

> +static void __exit crypto_is_exit(void)

Here as well.

Thanks,
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

RE: [PATCHv3 3/4] crypto: inside-secure - add support for PCI based FPGA development board

[Pascal Van Leeuwen]

Hi Antoine,

Thanks for the review and I agree with all of your comments below.
So I'm willing to fix those but I'm a bit unclear of the procedure now,
since you acked part of the patch set already.

Should I resend just the subpatches that need fixes or should I resend 
the whole patchset? Please advise :-)

> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of
> Antoine Tenart
> Sent: Monday, August 5, 2019 10:36 AM
> To: Pascal van Leeuwen <pascalvanl@gmail.com>
> Cc: linux-crypto@vger.kernel.org; antoine.tenart@bootlin.com; herbert@gondor.apana.org.au;
> davem@davemloft.net; Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Subject: Re: [PATCHv3 3/4] crypto: inside-secure - add support for PCI based FPGA
> development board
> 
> Hello Pascal,
> 
> The patch looks mostly good, just a few comments below.
> 
> On Wed, Jul 31, 2019 at 05:29:18PM +0200, Pascal van Leeuwen wrote:
> > @@ -381,10 +383,11 @@ static int safexcel_hw_init(struct safexcel_crypto_priv *priv)
> >  		       EIP197_HIA_DxE_CFG_MAX_DATA_SIZE(8);
> >  		val |= EIP197_HIA_DxE_CFG_DATA_CACHE_CTRL(WR_CACHE_3BITS);
> >  		val |= EIP197_HIA_DSE_CFG_ALWAYS_BUFFERABLE;
> > -		/* FIXME: instability issues can occur for EIP97 but disabling it impact
> > -		 * performances.
> > +		/*
> > +		 * FIXME: instability issues can occur for EIP97 but disabling
> > +		 * it impacts performance.
> >  		 */
> 
> No need to change the comment style here.
> 
> > @@ -514,7 +517,8 @@ void safexcel_dequeue(struct safexcel_crypto_priv *priv, int ring)
> >  	struct safexcel_context *ctx;
> >  	int ret, nreq = 0, cdesc = 0, rdesc = 0, commands, results;
> >
> > -	/* If a request wasn't properly dequeued because of a lack of resources,
> > +	/*
> > +	 * If a request wasn't properly dequeued because of a lack of resources,
> >  	 * proceeded it first,
> >  	 */
> 
> Ditto.
> 
> > @@ -543,7 +547,8 @@ void safexcel_dequeue(struct safexcel_crypto_priv *priv, int ring)
> >
> > -		/* In case the send() helper did not issue any command to push
> > +		/*
> > +		 * In case the send() helper did not issue any command to push
> 
> Ditto.
> 
> > -	/* Not enough resources to handle all the requests. Bail out and save
> > +	/*
> > +	 * Not enough resources to handle all the requests. Bail out and save
> 
> Ditto.
> 
> > @@ -731,7 +738,8 @@ static inline void safexcel_handle_result_descriptor(struct
> safexcel_crypto_priv
> >  		       EIP197_xDR_PROC_xD_COUNT(tot_descs * priv->config.rd_offset),
> >  		       EIP197_HIA_RDR(priv, ring) + EIP197_HIA_xDR_PROC_COUNT);
> >
> > -	/* If the number of requests overflowed the counter, try to proceed more
> > +	/*
> > +	 * If the number of requests overflowed the counter, try to proceed more
> 
> Ditto.
> 
> > +#if IS_ENABLED(CONFIG_OF)
> > +/*
> > + * for Device Tree platform driver
> > + */
> 
> Single line comment should be:
> 
> /* comment */
> 
> > +#if IS_ENABLED(CONFIG_PCI)
> > +/*
> > + * PCIE devices - i.e. Inside Secure development boards
> > + */
> 
> Here as well.
> 
> > +static int crypto_is_pci_probe(struct pci_dev *pdev,
> > +			       const struct pci_device_id *ent)
> 
> The whole driver uses the "safexcel_" prefix for functions. You should
> use it here as well for consistency.
> 
> > +void crypto_is_pci_remove(struct pci_dev *pdev)
> 
> Here as well.
> 
> > +static const struct pci_device_id crypto_is_pci_ids[] = {
> 
> Here as well.
> 
> > +static struct pci_driver crypto_is_pci_driver = {
> 
> Here as well.
> 
> > +static int __init crypto_is_init(void)
> 
> Here as well.
> 
> > +static void __exit crypto_is_exit(void)
> 
> Here as well.
> 
> Thanks,
> Antoine
> 
> --
> Antoine Ténart, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com

Re: [PATCHv3 3/4] crypto: inside-secure - add support for PCI based FPGA development board

[Antoine Tenart]

Hi Pascal,

On Mon, Aug 05, 2019 at 08:47:42AM +0000, Pascal Van Leeuwen wrote:
> 
> Thanks for the review and I agree with all of your comments below.
> So I'm willing to fix those but I'm a bit unclear of the procedure now,
> since you acked part of the patch set already.
> 
> Should I resend just the subpatches that need fixes or should I resend 
> the whole patchset? Please advise :-)

You should add my Acked-by tag on the first two patches (below your SoB
tag), and then yes please resubmit the whole series.

Thanks!
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

RE: [PATCHv3 3/4] crypto: inside-secure - add support for PCI based FPGA development board

[Pascal Van Leeuwen]

> -----Original Message-----
> From: Antoine Tenart <antoine.tenart@bootlin.com>
> Sent: Monday, August 5, 2019 11:10 AM
> To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Cc: Antoine Tenart <antoine.tenart@bootlin.com>; Pascal van Leeuwen
> <pascalvanl@gmail.com>; linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au;
> davem@davemloft.net
> Subject: Re: [PATCHv3 3/4] crypto: inside-secure - add support for PCI based FPGA
> development board
> 
> Hi Pascal,
> 
> On Mon, Aug 05, 2019 at 08:47:42AM +0000, Pascal Van Leeuwen wrote:
> >
> > Thanks for the review and I agree with all of your comments below.
> > So I'm willing to fix those but I'm a bit unclear of the procedure now,
> > since you acked part of the patch set already.
> >
> > Should I resend just the subpatches that need fixes or should I resend
> > the whole patchset? Please advise :-)
> 
> You should add my Acked-by tag on the first two patches (below your SoB
> tag), and then yes please resubmit the whole series.
> 
Ok, will do, thanks!

> Thanks!
> Antoine
> 
> --
> Antoine Ténart, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com

[PATCHv3 4/4] crypto: inside-secure - add support for using the EIP197 without vendor firmware

[Pascal van Leeuwen]

Until now, the inside-secure driver required a set of firmware images
supplied by the silicon vendor, typically under NDA, to be present in
/lib/firmware/inside-secure in order to be able to function.
This patch removes the dependence on this official vendor firmware by
falling back to generic "mini" FW - developed specifically for this
driver - that can be provided under GPL 2.0 through linux-firmwares.

Signed-off-by: Pascal van Leeuwen <pvanleeuwen@verimatrix.com>
---
 drivers/crypto/inside-secure/safexcel.c | 194 ++++++++++++++++++++++++--------
 drivers/crypto/inside-secure/safexcel.h |  12 ++
 2 files changed, 161 insertions(+), 45 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index 01e03ca..e25beef 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -108,44 +108,143 @@ static void eip197_trc_cache_init(struct safexcel_crypto_priv *priv)
 	writel(val, priv->base + EIP197_TRC_PARAMS);
 }

-static void eip197_write_firmware(struct safexcel_crypto_priv *priv,
-				  const struct firmware *fw, int pe, u32 ctrl,
-				  u32 prog_en)
+static void eip197_init_firmware(struct safexcel_crypto_priv *priv)
 {
-	const u32 *data = (const u32 *)fw->data;
+	int pe, i;
 	u32 val;
-	int i;

-	/* Reset the engine to make its program memory accessible */
-	writel(EIP197_PE_ICE_x_CTRL_SW_RESET |
-	       EIP197_PE_ICE_x_CTRL_CLR_ECC_CORR |
-	       EIP197_PE_ICE_x_CTRL_CLR_ECC_NON_CORR,
-	       EIP197_PE(priv) + ctrl);
+	for (pe = 0; pe < priv->config.pes; pe++) {
+		/* Configure the token FIFO's */
+		writel(3, EIP197_PE(priv) + EIP197_PE_ICE_PUTF_CTRL(pe));
+		writel(0, EIP197_PE(priv) + EIP197_PE_ICE_PPTF_CTRL(pe));
+
+		/* Clear the ICE scratchpad memory */
+		val = readl(EIP197_PE(priv) + EIP197_PE_ICE_SCRATCH_CTRL(pe));
+		val |= EIP197_PE_ICE_SCRATCH_CTRL_CHANGE_TIMER |
+		       EIP197_PE_ICE_SCRATCH_CTRL_TIMER_EN |
+		       EIP197_PE_ICE_SCRATCH_CTRL_SCRATCH_ACCESS |
+		       EIP197_PE_ICE_SCRATCH_CTRL_CHANGE_ACCESS;
+		writel(val, EIP197_PE(priv) + EIP197_PE_ICE_SCRATCH_CTRL(pe));
+
+		/* clear the scratchpad RAM using 32 bit writes only */
+		for (i = 0; i < EIP197_NUM_OF_SCRATCH_BLOCKS; i++)
+			writel(0, EIP197_PE(priv) +
+				  EIP197_PE_ICE_SCRATCH_RAM(pe) + (i<<2));
+
+		/* Reset the IFPP engine to make its program mem accessible */
+		writel(EIP197_PE_ICE_x_CTRL_SW_RESET |
+		       EIP197_PE_ICE_x_CTRL_CLR_ECC_CORR |
+		       EIP197_PE_ICE_x_CTRL_CLR_ECC_NON_CORR,
+		       EIP197_PE(priv) + EIP197_PE_ICE_FPP_CTRL(pe));
+
+		/* Reset the IPUE engine to make its program mem accessible */
+		writel(EIP197_PE_ICE_x_CTRL_SW_RESET |
+		       EIP197_PE_ICE_x_CTRL_CLR_ECC_CORR |
+		       EIP197_PE_ICE_x_CTRL_CLR_ECC_NON_CORR,
+		       EIP197_PE(priv) + EIP197_PE_ICE_PUE_CTRL(pe));
+
+		/* Enable access to all IFPP program memories */
+		writel(EIP197_PE_ICE_RAM_CTRL_FPP_PROG_EN,
+		       EIP197_PE(priv) + EIP197_PE_ICE_RAM_CTRL(pe));
+	}
+
+}

-	/* Enable access to the program memory */
-	writel(prog_en, EIP197_PE(priv) + EIP197_PE_ICE_RAM_CTRL(pe));
+static int eip197_write_firmware(struct safexcel_crypto_priv *priv,
+				  const struct firmware *fw)
+{
+	const u32 *data = (const u32 *)fw->data;
+	int i;

 	/* Write the firmware */
 	for (i = 0; i < fw->size / sizeof(u32); i++)
 		writel(be32_to_cpu(data[i]),
 		       priv->base + EIP197_CLASSIFICATION_RAMS + i * sizeof(u32));

-	/* Disable access to the program memory */
-	writel(0, EIP197_PE(priv) + EIP197_PE_ICE_RAM_CTRL(pe));
+	/* Exclude final 2 NOPs from size */
+	return i - EIP197_FW_TERMINAL_NOPS;
+}
+
+/*
+ * If FW is actual production firmware, then poll for its initialization
+ * to complete and check if it is good for the HW, otherwise just return OK.
+ */
+static bool poll_fw_ready(struct safexcel_crypto_priv *priv, int fpp)
+{
+	int pe, pollcnt;
+	u32 base, pollofs;
+
+	if (fpp)
+		pollofs  = EIP197_FW_FPP_READY;
+	else
+		pollofs  = EIP197_FW_PUE_READY;

-	/* Release engine from reset */
-	val = readl(EIP197_PE(priv) + ctrl);
-	val &= ~EIP197_PE_ICE_x_CTRL_SW_RESET;
-	writel(val, EIP197_PE(priv) + ctrl);
+	for (pe = 0; pe < priv->config.pes; pe++) {
+		base = EIP197_PE_ICE_SCRATCH_RAM(pe);
+		pollcnt = EIP197_FW_START_POLLCNT;
+		while (pollcnt &&
+		       (readl_relaxed(EIP197_PE(priv) + base +
+			      pollofs) != 1)) {
+			pollcnt--;
+		}
+		if (!pollcnt) {
+			dev_err(priv->dev, "FW(%d) for PE %d failed to start\n",
+				fpp, pe);
+			return false;
+		}
+	}
+	return true;
+}
+
+static bool eip197_start_firmware(struct safexcel_crypto_priv *priv,
+				  int ipuesz, int ifppsz, int minifw)
+{
+	int pe;
+	u32 val;
+
+	for (pe = 0; pe < priv->config.pes; pe++) {
+		/* Disable access to all program memory */
+		writel(0, EIP197_PE(priv) + EIP197_PE_ICE_RAM_CTRL(pe));
+
+		/* Start IFPP microengines */
+		if (minifw)
+			val = 0;
+		else
+			val = EIP197_PE_ICE_UENG_START_OFFSET((ifppsz - 1) &
+					EIP197_PE_ICE_UENG_INIT_ALIGN_MASK) |
+				EIP197_PE_ICE_UENG_DEBUG_RESET;
+		writel(val, EIP197_PE(priv) + EIP197_PE_ICE_FPP_CTRL(pe));
+
+		/* Start IPUE microengines */
+		if (minifw)
+			val = 0;
+		else
+			val = EIP197_PE_ICE_UENG_START_OFFSET((ipuesz - 1) &
+					EIP197_PE_ICE_UENG_INIT_ALIGN_MASK) |
+				EIP197_PE_ICE_UENG_DEBUG_RESET;
+		writel(val, EIP197_PE(priv) + EIP197_PE_ICE_PUE_CTRL(pe));
+	}
+
+	/* For miniFW startup, there is no initialization, so always succeed */
+	if (minifw)
+		return true;
+
+	/* Wait until all the firmwares have properly started up */
+	if (!poll_fw_ready(priv, 1))
+		return false;
+	if (!poll_fw_ready(priv, 0))
+		return false;
+
+	return true;
 }

 static int eip197_load_firmwares(struct safexcel_crypto_priv *priv)
 {
 	const char *fw_name[] = {"ifpp.bin", "ipue.bin"};
 	const struct firmware *fw[FW_NB];
-	char fw_path[31], *dir = NULL;
+	char fw_path[37], *dir = NULL;
 	int i, j, ret = 0, pe;
-	u32 val;
+	int ipuesz, ifppsz, minifw = 0;

 	if (priv->version == EIP197D_MRVL)
 		dir = "eip197d";
@@ -155,51 +254,56 @@ static int eip197_load_firmwares(struct safexcel_crypto_priv *priv)
 	else
 		return -ENODEV;

+retry_fw:
 	for (i = 0; i < FW_NB; i++) {
-		snprintf(fw_path, 31, "inside-secure/%s/%s", dir, fw_name[i]);
-		ret = request_firmware(&fw[i], fw_path, priv->dev);
+		snprintf(fw_path, 37, "inside-secure/%s/%s", dir, fw_name[i]);
+		ret = firmware_request_nowarn(&fw[i], fw_path, priv->dev);
 		if (ret) {
-			if (priv->version != EIP197B_MRVL)
+			if (minifw || priv->version != EIP197B_MRVL)
 				goto release_fw;

 			/* Fallback to the old firmware location for the
 			 * EIP197b.
 			 */
-			ret = request_firmware(&fw[i], fw_name[i], priv->dev);
-			if (ret) {
-				dev_err(priv->dev,
-					"Failed to request firmware %s (%d)\n",
-					fw_name[i], ret);
+			ret = firmware_request_nowarn(&fw[i], fw_name[i],
+						      priv->dev);
+			if (ret)
 				goto release_fw;
-			}
 		}
 	}

-	for (pe = 0; pe < priv->config.pes; pe++) {
-		/* Clear the scratchpad memory */
-		val = readl(EIP197_PE(priv) + EIP197_PE_ICE_SCRATCH_CTRL(pe));
-		val |= EIP197_PE_ICE_SCRATCH_CTRL_CHANGE_TIMER |
-		       EIP197_PE_ICE_SCRATCH_CTRL_TIMER_EN |
-		       EIP197_PE_ICE_SCRATCH_CTRL_SCRATCH_ACCESS |
-		       EIP197_PE_ICE_SCRATCH_CTRL_CHANGE_ACCESS;
-		writel(val, EIP197_PE(priv) + EIP197_PE_ICE_SCRATCH_CTRL(pe));
+	eip197_init_firmware(priv);

-		memset_io(EIP197_PE(priv) + EIP197_PE_ICE_SCRATCH_RAM(pe), 0,
-			  EIP197_NUM_OF_SCRATCH_BLOCKS * sizeof(u32));
+	ifppsz = eip197_write_firmware(priv, fw[FW_IFPP]);

-		eip197_write_firmware(priv, fw[FW_IFPP], pe,
-				      EIP197_PE_ICE_FPP_CTRL(pe),
-				      EIP197_PE_ICE_RAM_CTRL_FPP_PROG_EN);
+	/* Enable access to IPUE program memories */
+	for (pe = 0; pe < priv->config.pes; pe++)
+		writel(EIP197_PE_ICE_RAM_CTRL_PUE_PROG_EN,
+		       EIP197_PE(priv) + EIP197_PE_ICE_RAM_CTRL(pe));

-		eip197_write_firmware(priv, fw[FW_IPUE], pe,
-				      EIP197_PE_ICE_PUE_CTRL(pe),
-				      EIP197_PE_ICE_RAM_CTRL_PUE_PROG_EN);
+	ipuesz = eip197_write_firmware(priv, fw[FW_IPUE]);
+
+	if (eip197_start_firmware(priv, ipuesz, ifppsz, minifw)) {
+		dev_dbg(priv->dev, "Firmware loaded successfully");
+		return 0;
 	}

+	ret = -ENODEV;
+
 release_fw:
 	for (j = 0; j < i; j++)
 		release_firmware(fw[j]);

+	if (!minifw) {
+		/* Retry with minifw path */
+		dev_dbg(priv->dev, "Firmware set not (fully) present or init failed, falling back to BCLA mode\n");
+		dir = "eip197_minifw";
+		minifw = 1;
+		goto retry_fw;
+	}
+
+	dev_dbg(priv->dev, "Firmware load failed.\n");
+
 	return ret;
 }

diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
index a9a239b..ffb1c66 100644
--- a/drivers/crypto/inside-secure/safexcel.h
+++ b/drivers/crypto/inside-secure/safexcel.h
@@ -136,8 +136,10 @@
 #define EIP197_PE_IN_TBUF_THRES(n)		(0x0100 + (0x2000 * (n)))
 #define EIP197_PE_ICE_SCRATCH_RAM(n)		(0x0800 + (0x2000 * (n)))
 #define EIP197_PE_ICE_PUE_CTRL(n)		(0x0c80 + (0x2000 * (n)))
+#define EIP197_PE_ICE_PUTF_CTRL(n)		(0x0d00 + (0x2000 * (n)))
 #define EIP197_PE_ICE_SCRATCH_CTRL(n)		(0x0d04 + (0x2000 * (n)))
 #define EIP197_PE_ICE_FPP_CTRL(n)		(0x0d80 + (0x2000 * (n)))
+#define EIP197_PE_ICE_PPTF_CTRL(n)		(0x0e00 + (0x2000 * (n)))
 #define EIP197_PE_ICE_RAM_CTRL(n)		(0x0ff0 + (0x2000 * (n)))
 #define EIP197_PE_EIP96_TOKEN_CTRL(n)		(0x1000 + (0x2000 * (n)))
 #define EIP197_PE_EIP96_FUNCTION_EN(n)		(0x1004 + (0x2000 * (n)))
@@ -228,6 +230,11 @@
 #define EIP197_DxE_THR_CTRL_EN			BIT(30)
 #define EIP197_DxE_THR_CTRL_RESET_PE		BIT(31)

+/* EIP197_PE_ICE_PUE/FPP_CTRL */
+#define EIP197_PE_ICE_UENG_START_OFFSET(n)	((n) << 16)
+#define EIP197_PE_ICE_UENG_INIT_ALIGN_MASK	0x7ff0
+#define EIP197_PE_ICE_UENG_DEBUG_RESET		BIT(3)
+
 /* EIP197_HIA_AIC_G_ENABLED_STAT */
 #define EIP197_G_IRQ_DFE(n)			BIT((n) << 1)
 #define EIP197_G_IRQ_DSE(n)			BIT(((n) << 1) + 1)
@@ -530,6 +537,11 @@ struct safexcel_command_desc {
  * Internal structures & functions
  */

+#define EIP197_FW_TERMINAL_NOPS		2
+#define EIP197_FW_START_POLLCNT		16
+#define EIP197_FW_PUE_READY		0x14
+#define EIP197_FW_FPP_READY		0x18
+
 enum eip197_fw {
 	FW_IFPP = 0,
 	FW_IPUE,
--
1.8.3.1

Re: [PATCHv3 4/4] crypto: inside-secure - add support for using the EIP197 without vendor firmware

[Antoine Tenart]

Hi Pascal,

Just a small comment below,

On Wed, Jul 31, 2019 at 05:29:19PM +0200, Pascal van Leeuwen wrote:
> 
> -	/* Release engine from reset */
> -	val = readl(EIP197_PE(priv) + ctrl);
> -	val &= ~EIP197_PE_ICE_x_CTRL_SW_RESET;
> -	writel(val, EIP197_PE(priv) + ctrl);
> +	for (pe = 0; pe < priv->config.pes; pe++) {
> +		base = EIP197_PE_ICE_SCRATCH_RAM(pe);
> +		pollcnt = EIP197_FW_START_POLLCNT;
> +		while (pollcnt &&
> +		       (readl_relaxed(EIP197_PE(priv) + base +
> +			      pollofs) != 1)) {
> +			pollcnt--;

You might want to use readl_relaxed_poll_timeout() here, instead of a
busy polling.

Thanks,
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

RE: [PATCHv3 4/4] crypto: inside-secure - add support for using the EIP197 without vendor firmware

[Pascal Van Leeuwen]

> -----Original Message-----
> From: linux-crypto-owner@vger.kernel.org <linux-crypto-owner@vger.kernel.org> On Behalf Of
> Antoine Tenart
> Sent: Monday, August 5, 2019 11:07 AM
> To: Pascal van Leeuwen <pascalvanl@gmail.com>
> Cc: linux-crypto@vger.kernel.org; antoine.tenart@bootlin.com; herbert@gondor.apana.org.au;
> davem@davemloft.net; Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Subject: Re: [PATCHv3 4/4] crypto: inside-secure - add support for using the EIP197
> without vendor firmware
> 
> Hi Pascal,
> 
> Just a small comment below,
> 
> On Wed, Jul 31, 2019 at 05:29:19PM +0200, Pascal van Leeuwen wrote:
> >
> > -	/* Release engine from reset */
> > -	val = readl(EIP197_PE(priv) + ctrl);
> > -	val &= ~EIP197_PE_ICE_x_CTRL_SW_RESET;
> > -	writel(val, EIP197_PE(priv) + ctrl);
> > +	for (pe = 0; pe < priv->config.pes; pe++) {
> > +		base = EIP197_PE_ICE_SCRATCH_RAM(pe);
> > +		pollcnt = EIP197_FW_START_POLLCNT;
> > +		while (pollcnt &&
> > +		       (readl_relaxed(EIP197_PE(priv) + base +
> > +			      pollofs) != 1)) {
> > +			pollcnt--;
> 
> You might want to use readl_relaxed_poll_timeout() here, instead of a
> busy polling.
>
Didn't know such a thing existed, but I also wonder how appropriate it
is in this case, condering it measures in whole microseconds, while the 
response time I'm expecting here is in the order of a few dozen nano-
seconds internally ... i.e. 1 microsecond is already a *huge* overkill.

The current implementation runs that loop for only 16 iterations which
should be both more than sufficient (it probably could be reduced 
further, I picked 16 rather arbitrarily) and at the same time take so
few cycles on the CPU that I doubt it is worthwhile to reschedule/
preempt/whatever?

> 
> Thanks,
> Antoine
> 
> --
> Antoine Ténart, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com

Re: [PATCHv3 4/4] crypto: inside-secure - add support for using the EIP197 without vendor firmware

[Antoine Tenart]

On Mon, Aug 05, 2019 at 09:48:13AM +0000, Pascal Van Leeuwen wrote:
> > On Wed, Jul 31, 2019 at 05:29:19PM +0200, Pascal van Leeuwen wrote:
> > >
> > > -	/* Release engine from reset */
> > > -	val = readl(EIP197_PE(priv) + ctrl);
> > > -	val &= ~EIP197_PE_ICE_x_CTRL_SW_RESET;
> > > -	writel(val, EIP197_PE(priv) + ctrl);
> > > +	for (pe = 0; pe < priv->config.pes; pe++) {
> > > +		base = EIP197_PE_ICE_SCRATCH_RAM(pe);
> > > +		pollcnt = EIP197_FW_START_POLLCNT;
> > > +		while (pollcnt &&
> > > +		       (readl_relaxed(EIP197_PE(priv) + base +
> > > +			      pollofs) != 1)) {
> > > +			pollcnt--;
> > 
> > You might want to use readl_relaxed_poll_timeout() here, instead of a
> > busy polling.
> >
> Didn't know such a thing existed, but I also wonder how appropriate it
> is in this case, condering it measures in whole microseconds, while the 
> response time I'm expecting here is in the order of a few dozen nano-
> seconds internally ... i.e. 1 microsecond is already a *huge* overkill.
> 
> The current implementation runs that loop for only 16 iterations which
> should be both more than sufficient (it probably could be reduced 
> further, I picked 16 rather arbitrarily) and at the same time take so
> few cycles on the CPU that I doubt it is worthwhile to reschedule/
> preempt/whatever?

Your choice, I was just making a suggestion :)

Thanks,
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

RE: [PATCHv3 4/4] crypto: inside-secure - add support for using the EIP197 without vendor firmware

[Pascal Van Leeuwen]

> -----Original Message-----
> From: Antoine Tenart <antoine.tenart@bootlin.com>
> Sent: Monday, August 5, 2019 11:54 AM
> To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Cc: Antoine Tenart <antoine.tenart@bootlin.com>; Pascal van Leeuwen
> <pascalvanl@gmail.com>; linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au;
> davem@davemloft.net
> Subject: Re: [PATCHv3 4/4] crypto: inside-secure - add support for using the EIP197
> without vendor firmware
> 
> On Mon, Aug 05, 2019 at 09:48:13AM +0000, Pascal Van Leeuwen wrote:
> > > On Wed, Jul 31, 2019 at 05:29:19PM +0200, Pascal van Leeuwen wrote:
> > > >
> > > > -	/* Release engine from reset */
> > > > -	val = readl(EIP197_PE(priv) + ctrl);
> > > > -	val &= ~EIP197_PE_ICE_x_CTRL_SW_RESET;
> > > > -	writel(val, EIP197_PE(priv) + ctrl);
> > > > +	for (pe = 0; pe < priv->config.pes; pe++) {
> > > > +		base = EIP197_PE_ICE_SCRATCH_RAM(pe);
> > > > +		pollcnt = EIP197_FW_START_POLLCNT;
> > > > +		while (pollcnt &&
> > > > +		       (readl_relaxed(EIP197_PE(priv) + base +
> > > > +			      pollofs) != 1)) {
> > > > +			pollcnt--;
> > >
> > > You might want to use readl_relaxed_poll_timeout() here, instead of a
> > > busy polling.
> > >
> > Didn't know such a thing existed, but I also wonder how appropriate it
> > is in this case, condering it measures in whole microseconds, while the
> > response time I'm expecting here is in the order of a few dozen nano-
> > seconds internally ... i.e. 1 microsecond is already a *huge* overkill.
> >
> > The current implementation runs that loop for only 16 iterations which
> > should be both more than sufficient (it probably could be reduced
> > further, I picked 16 rather arbitrarily) and at the same time take so
> > few cycles on the CPU that I doubt it is worthwhile to reschedule/
> > preempt/whatever?
> 
> Your choice, I was just making a suggestion :)
> 
And I appreciate that, as I now know about this 
readl_relaxed_poll_timeout() which I'm sure I will need for something
someday. I was just asking for comments on this particular situation.

After reading the implementation code, I think using it with a 
sleep_us value of 0 and a time-out value of 1 may be reasonable here,
as time-out means some serious error that shouldn't normally happen.
I just wouldn't want any microsecond delays for the "normal" case.

> Thanks,
> Antoine
> 
> --
> Antoine Ténart, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com

Re: [PATCHv3 4/4] crypto: inside-secure - add support for using the EIP197 without vendor firmware

[Antoine Tenart]

On Mon, Aug 05, 2019 at 10:12:07AM +0000, Pascal Van Leeuwen wrote:
> > From: Antoine Tenart <antoine.tenart@bootlin.com>
> > On Mon, Aug 05, 2019 at 09:48:13AM +0000, Pascal Van Leeuwen wrote:
> > > > On Wed, Jul 31, 2019 at 05:29:19PM +0200, Pascal van Leeuwen wrote:
> > > > >
> > > > > -	/* Release engine from reset */
> > > > > -	val = readl(EIP197_PE(priv) + ctrl);
> > > > > -	val &= ~EIP197_PE_ICE_x_CTRL_SW_RESET;
> > > > > -	writel(val, EIP197_PE(priv) + ctrl);
> > > > > +	for (pe = 0; pe < priv->config.pes; pe++) {
> > > > > +		base = EIP197_PE_ICE_SCRATCH_RAM(pe);
> > > > > +		pollcnt = EIP197_FW_START_POLLCNT;
> > > > > +		while (pollcnt &&
> > > > > +		       (readl_relaxed(EIP197_PE(priv) + base +
> > > > > +			      pollofs) != 1)) {
> > > > > +			pollcnt--;
> > > >
> > > > You might want to use readl_relaxed_poll_timeout() here, instead of a
> > > > busy polling.
> > > >
> > > Didn't know such a thing existed, but I also wonder how appropriate it
> > > is in this case, condering it measures in whole microseconds, while the
> > > response time I'm expecting here is in the order of a few dozen nano-
> > > seconds internally ... i.e. 1 microsecond is already a *huge* overkill.
> > >
> > > The current implementation runs that loop for only 16 iterations which
> > > should be both more than sufficient (it probably could be reduced
> > > further, I picked 16 rather arbitrarily) and at the same time take so
> > > few cycles on the CPU that I doubt it is worthwhile to reschedule/
> > > preempt/whatever?
> > 
> > Your choice, I was just making a suggestion :)
> > 
> After reading the implementation code, I think using it with a 
> sleep_us value of 0 and a time-out value of 1 may be reasonable here,
> as time-out means some serious error that shouldn't normally happen.
> I just wouldn't want any microsecond delays for the "normal" case.

Using this function that is designed to sleep with a delay of 0 and
designed to timeout with a value of 1 does not seem to follow what the
function is designed for :) It could work but I would suggest to keep
the polling as it is in the patch in this case.

Thanks,
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

RE: [PATCHv3 4/4] crypto: inside-secure - add support for using the EIP197 without vendor firmware

[Pascal Van Leeuwen]

> -----Original Message-----
> From: Antoine Tenart <antoine.tenart@bootlin.com>
> Sent: Monday, August 5, 2019 2:43 PM
> To: Pascal Van Leeuwen <pvanleeuwen@verimatrix.com>
> Cc: Antoine Tenart <antoine.tenart@bootlin.com>; Pascal van Leeuwen
> <pascalvanl@gmail.com>; linux-crypto@vger.kernel.org; herbert@gondor.apana.org.au;
> davem@davemloft.net
> Subject: Re: [PATCHv3 4/4] crypto: inside-secure - add support for using the EIP197
> without vendor firmware
> 
> On Mon, Aug 05, 2019 at 10:12:07AM +0000, Pascal Van Leeuwen wrote:
> > > From: Antoine Tenart <antoine.tenart@bootlin.com>
> > > On Mon, Aug 05, 2019 at 09:48:13AM +0000, Pascal Van Leeuwen wrote:
> > > > > On Wed, Jul 31, 2019 at 05:29:19PM +0200, Pascal van Leeuwen wrote:
> > > > > >
> > > > > > -	/* Release engine from reset */
> > > > > > -	val = readl(EIP197_PE(priv) + ctrl);
> > > > > > -	val &= ~EIP197_PE_ICE_x_CTRL_SW_RESET;
> > > > > > -	writel(val, EIP197_PE(priv) + ctrl);
> > > > > > +	for (pe = 0; pe < priv->config.pes; pe++) {
> > > > > > +		base = EIP197_PE_ICE_SCRATCH_RAM(pe);
> > > > > > +		pollcnt = EIP197_FW_START_POLLCNT;
> > > > > > +		while (pollcnt &&
> > > > > > +		       (readl_relaxed(EIP197_PE(priv) + base +
> > > > > > +			      pollofs) != 1)) {
> > > > > > +			pollcnt--;
> > > > >
> > > > > You might want to use readl_relaxed_poll_timeout() here, instead of a
> > > > > busy polling.
> > > > >
> > > > Didn't know such a thing existed, but I also wonder how appropriate it
> > > > is in this case, condering it measures in whole microseconds, while the
> > > > response time I'm expecting here is in the order of a few dozen nano-
> > > > seconds internally ... i.e. 1 microsecond is already a *huge* overkill.
> > > >
> > > > The current implementation runs that loop for only 16 iterations which
> > > > should be both more than sufficient (it probably could be reduced
> > > > further, I picked 16 rather arbitrarily) and at the same time take so
> > > > few cycles on the CPU that I doubt it is worthwhile to reschedule/
> > > > preempt/whatever?
> > >
> > > Your choice, I was just making a suggestion :)
> > >
> > After reading the implementation code, I think using it with a
> > sleep_us value of 0 and a time-out value of 1 may be reasonable here,
> > as time-out means some serious error that shouldn't normally happen.
> > I just wouldn't want any microsecond delays for the "normal" case.
> 
> Using this function that is designed to sleep with a delay of 0 and
> designed to timeout with a value of 1 does not seem to follow what the
> function is designed for :) It could work but I would suggest to keep
> the polling as it is in the patch in this case.
> 
Ok, will do. Does that mean that patch 4/4 is good as is?
i.e. can I add an "Acked-by: Antoine Tenart" line when I resubmit?

> Thanks,
> Antoine
> 
> --
> Antoine Ténart, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com

Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com

Re: [PATCHv3 4/4] crypto: inside-secure - add support for using the EIP197 without vendor firmware

[Antoine Tenart]

On Mon, Aug 05, 2019 at 01:02:08PM +0000, Pascal Van Leeuwen wrote:
> > From: Antoine Tenart <antoine.tenart@bootlin.com>
> > 
> > Using this function that is designed to sleep with a delay of 0 and
> > designed to timeout with a value of 1 does not seem to follow what the
> > function is designed for :) It could work but I would suggest to keep
> > the polling as it is in the patch in this case.
> > 
> Ok, will do. Does that mean that patch 4/4 is good as is?
> i.e. can I add an "Acked-by: Antoine Tenart" line when I resubmit?

Sure, the rest of the patch looked good and you can add:

Acked-by: Antoine Tenart <antoine.tenart@bootlin.com>

Thanks!
Antoine

-- 
Antoine Ténart, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com