2019-09-17 10:59:51

by Pascal van Leeuwen

[permalink] [raw]
Subject: [PATCH 0/2] crypto: inside-secure: [URGENT] Fix stability issue

This patchset fixes some issues with the TRC RAM probing that caused
instability (random failures) on the Macchiatobin board and incorrect
configuration of the TRC for some other corner case RAM configuration.

The patchset has been tested with the eip197c_iewxkbc configuration with
163840 bytes of TRC data RAM and 4096 words of TRC admin RAM on the
Xilinx VCU118 development board as well as on the Macchiatobin board
(Marvell A8K: EIP197b-ieswx w/ 15350 bytes data RAM & 80 words admin RAM),
including the testmgr extra tests.

Pascal van Leeuwen (2):
crypto: inside-secure: [URGENT] Fix stability issue with Macchiatobin
crypto: inside-secure - Fixed corner case TRC admin RAM probing issue

drivers/crypto/inside-secure/safexcel.c | 52 +++++++++++++++++++++------------
drivers/crypto/inside-secure/safexcel.h | 2 ++
2 files changed, 36 insertions(+), 18 deletions(-)

--
1.8.3.1


2019-09-17 11:00:03

by Pascal van Leeuwen

[permalink] [raw]
Subject: [PATCH 1/2] crypto: inside-secure: [URGENT] Fix stability issue with Macchiatobin

This patch corrects an error in the Transform Record Cache initialization
code that was causing intermittent stability problems on the Macchiatobin
board.

Unfortunately, due to HW platform specifics, the problem could not happen
on the main development platform, being the VCU118 Xilinx development
board. And since it was a problem with hash table access, it was very
dependent on the actual physical context record DMA buffers being used,
i.e. with some (bad) luck it could seemingly work quit stable for a while.

Signed-off-by: Pascal van Leeuwen <[email protected]>
---
drivers/crypto/inside-secure/safexcel.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index dc04112..ac3e1ed 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -221,9 +221,9 @@ static void eip197_trc_cache_init(struct safexcel_crypto_priv *priv)
/* Step #3: Determine log2 of hash table size */
cs_ht_sz = __fls(asize - cs_rc_max) - 2;
/* Step #4: determine current size of hash table in dwords */
- cs_ht_wc = 16<<cs_ht_sz; /* dwords, not admin words */
+ cs_ht_wc = 16 << cs_ht_sz; /* dwords, not admin words */
/* Step #5: add back excess words and see if we can fit more records */
- cs_rc_max = min_t(uint, cs_rc_abs_max, asize - (cs_ht_wc >> 4));
+ cs_rc_max = min_t(uint, cs_rc_abs_max, asize - (cs_ht_wc >> 2));

/* Clear the cache RAMs */
eip197_trc_cache_clear(priv, cs_rc_max, cs_ht_wc);
--
1.8.3.1

2019-09-17 11:00:37

by Pascal van Leeuwen

[permalink] [raw]
Subject: [PATCH 2/2] crypto: inside-secure - Fixed corner case TRC admin RAM probing issue

This patch fixed a corner case admin RAM probing issue witnessed on the
Xilinx VCU118 FPGA development board with an EIP197 configuration with
4096 words of admin RAM, of which only 2050 were recognised.

Signed-off-by: Pascal van Leeuwen <[email protected]>
---
drivers/crypto/inside-secure/safexcel.c | 48 ++++++++++++++++++++++-----------
drivers/crypto/inside-secure/safexcel.h | 2 ++
2 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/drivers/crypto/inside-secure/safexcel.c b/drivers/crypto/inside-secure/safexcel.c
index ac3e1ed..ed34118 100644
--- a/drivers/crypto/inside-secure/safexcel.c
+++ b/drivers/crypto/inside-secure/safexcel.c
@@ -75,9 +75,9 @@ static void eip197_trc_cache_banksel(struct safexcel_crypto_priv *priv,
}

static u32 eip197_trc_cache_probe(struct safexcel_crypto_priv *priv,
- int maxbanks, u32 probemask)
+ int maxbanks, u32 probemask, u32 stride)
{
- u32 val, addrhi, addrlo, addrmid;
+ u32 val, addrhi, addrlo, addrmid, addralias, delta, marker;
int actbank;

/*
@@ -87,32 +87,37 @@ static u32 eip197_trc_cache_probe(struct safexcel_crypto_priv *priv,
addrhi = 1 << (16 + maxbanks);
addrlo = 0;
actbank = min(maxbanks - 1, 0);
- while ((addrhi - addrlo) > 32) {
+ while ((addrhi - addrlo) > stride) {
/* write marker to lowest address in top half */
addrmid = (addrhi + addrlo) >> 1;
+ marker = (addrmid ^ 0xabadbabe) & probemask; /* Unique */
eip197_trc_cache_banksel(priv, addrmid, &actbank);
- writel((addrmid | (addrlo << 16)) & probemask,
+ writel(marker,
priv->base + EIP197_CLASSIFICATION_RAMS +
(addrmid & 0xffff));

- /* write marker to lowest address in bottom half */
- eip197_trc_cache_banksel(priv, addrlo, &actbank);
- writel((addrlo | (addrhi << 16)) & probemask,
- priv->base + EIP197_CLASSIFICATION_RAMS +
- (addrlo & 0xffff));
+ /* write invalid markers to possible aliases */
+ delta = 1 << __fls(addrmid);
+ while (delta >= stride) {
+ addralias = addrmid - delta;
+ eip197_trc_cache_banksel(priv, addralias, &actbank);
+ writel(~marker,
+ priv->base + EIP197_CLASSIFICATION_RAMS +
+ (addralias & 0xffff));
+ delta >>= 1;
+ }

/* read back marker from top half */
eip197_trc_cache_banksel(priv, addrmid, &actbank);
val = readl(priv->base + EIP197_CLASSIFICATION_RAMS +
(addrmid & 0xffff));

- if (val == ((addrmid | (addrlo << 16)) & probemask)) {
+ if ((val & probemask) == marker)
/* read back correct, continue with top half */
addrlo = addrmid;
- } else {
+ else
/* not read back correct, continue with bottom half */
addrhi = addrmid;
- }
}
return addrhi;
}
@@ -150,7 +155,7 @@ static void eip197_trc_cache_clear(struct safexcel_crypto_priv *priv,
htable_offset + i * sizeof(u32));
}

-static void eip197_trc_cache_init(struct safexcel_crypto_priv *priv)
+static int eip197_trc_cache_init(struct safexcel_crypto_priv *priv)
{
u32 val, dsize, asize;
int cs_rc_max, cs_ht_wc, cs_trc_rec_wc, cs_trc_lg_rec_wc;
@@ -183,7 +188,7 @@ static void eip197_trc_cache_init(struct safexcel_crypto_priv *priv)
writel(val, priv->base + EIP197_TRC_PARAMS);

/* Probed data RAM size in bytes */
- dsize = eip197_trc_cache_probe(priv, maxbanks, 0xffffffff);
+ dsize = eip197_trc_cache_probe(priv, maxbanks, 0xffffffff, 32);

/*
* Now probe the administration RAM size pretty much the same way
@@ -196,11 +201,18 @@ static void eip197_trc_cache_init(struct safexcel_crypto_priv *priv)
writel(val, priv->base + EIP197_TRC_PARAMS);

/* Probed admin RAM size in admin words */
- asize = eip197_trc_cache_probe(priv, 0, 0xbfffffff) >> 4;
+ asize = eip197_trc_cache_probe(priv, 0, 0x3fffffff, 16) >> 4;

/* Clear any ECC errors detected while probing! */
writel(0, priv->base + EIP197_TRC_ECCCTRL);

+ /* Sanity check probing results */
+ if (dsize < EIP197_MIN_DSIZE || asize < EIP197_MIN_ASIZE) {
+ dev_err(priv->dev, "Record cache probing failed (%d,%d).",
+ dsize, asize);
+ return -ENODEV;
+ }
+
/*
* Determine optimal configuration from RAM sizes
* Note that we assume that the physical RAM configuration is sane
@@ -251,6 +263,7 @@ static void eip197_trc_cache_init(struct safexcel_crypto_priv *priv)

dev_info(priv->dev, "TRC init: %dd,%da (%dr,%dh)\n",
dsize, asize, cs_rc_max, cs_ht_wc + cs_ht_wc);
+ return 0;
}

static void eip197_init_firmware(struct safexcel_crypto_priv *priv)
@@ -737,7 +750,10 @@ static int safexcel_hw_init(struct safexcel_crypto_priv *priv)
writel(GENMASK(30, 20), EIP197_HIA_AIC_G(priv) + EIP197_HIA_AIC_G_ACK);

if (priv->flags & SAFEXCEL_HW_EIP197) {
- eip197_trc_cache_init(priv);
+ ret = eip197_trc_cache_init(priv);
+ if (ret)
+ return ret;
+
priv->flags |= EIP197_TRC_CACHE;

ret = eip197_load_firmwares(priv);
diff --git a/drivers/crypto/inside-secure/safexcel.h b/drivers/crypto/inside-secure/safexcel.h
index e473dab..0b95389 100644
--- a/drivers/crypto/inside-secure/safexcel.h
+++ b/drivers/crypto/inside-secure/safexcel.h
@@ -442,6 +442,8 @@ struct safexcel_context_record {
#define EIP197_TRC_PARAMS2_RC_SZ_SMALL(n) ((n) << 18)

/* Cache helpers */
+#define EIP197_MIN_DSIZE 1024
+#define EIP197_MIN_ASIZE 8
#define EIP197_CS_TRC_REC_WC 64
#define EIP197_CS_RC_SIZE (4 * sizeof(u32))
#define EIP197_CS_RC_NEXT(x) (x)
--
1.8.3.1

2019-10-04 15:43:06

by Herbert Xu

[permalink] [raw]
Subject: Re: [PATCH 0/2] crypto: inside-secure: [URGENT] Fix stability issue

On Tue, Sep 17, 2019 at 11:55:17AM +0200, Pascal van Leeuwen wrote:
> This patchset fixes some issues with the TRC RAM probing that caused
> instability (random failures) on the Macchiatobin board and incorrect
> configuration of the TRC for some other corner case RAM configuration.
>
> The patchset has been tested with the eip197c_iewxkbc configuration with
> 163840 bytes of TRC data RAM and 4096 words of TRC admin RAM on the
> Xilinx VCU118 development board as well as on the Macchiatobin board
> (Marvell A8K: EIP197b-ieswx w/ 15350 bytes data RAM & 80 words admin RAM),
> including the testmgr extra tests.
>
> Pascal van Leeuwen (2):
> crypto: inside-secure: [URGENT] Fix stability issue with Macchiatobin
> crypto: inside-secure - Fixed corner case TRC admin RAM probing issue
>
> drivers/crypto/inside-secure/safexcel.c | 52 +++++++++++++++++++++------------
> drivers/crypto/inside-secure/safexcel.h | 2 ++
> 2 files changed, 36 insertions(+), 18 deletions(-)

All applied. Thanks.
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt