The timer triggers when 500ms have gone by after triggering the engine
and no completion interrupt was received. The callback then tries to
sanitise things as well as possible.
Signed-off-by: Phil Sutter <[email protected]>
---
drivers/crypto/mv_cesa.c | 41 +++++++++++++++++++++++++++++++----------
1 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/drivers/crypto/mv_cesa.c b/drivers/crypto/mv_cesa.c
index e6ecc5f..8327bed 100644
--- a/drivers/crypto/mv_cesa.c
+++ b/drivers/crypto/mv_cesa.c
@@ -23,6 +23,7 @@
#define MV_CESA "MV-CESA:"
#define MAX_HW_HASH_SIZE 0xFFFF
+#define MV_CESA_EXPIRE 500 /* msec */
/*
* STM:
@@ -85,6 +86,7 @@ struct crypto_priv {
spinlock_t lock;
struct crypto_queue queue;
enum engine_status eng_st;
+ struct timer_list completion_timer;
struct crypto_async_request *cur_req;
struct req_progress p;
int max_req_size;
@@ -136,6 +138,29 @@ struct mv_req_hash_ctx {
int count_add;
};
+static void mv_completion_timer_callback(unsigned long unused)
+{
+ int active = readl(cpg->reg + SEC_ACCEL_CMD) & SEC_CMD_EN_SEC_ACCL0;
+
+ printk(KERN_ERR MV_CESA
+ "completion timer expired (CESA %sactive), cleaning up.\n",
+ active ? "" : "in");
+
+ del_timer(&cpg->completion_timer);
+ writel(SEC_CMD_DISABLE_SEC, cpg->reg + SEC_ACCEL_CMD);
+ while(readl(cpg->reg + SEC_ACCEL_CMD) & SEC_CMD_DISABLE_SEC)
+ printk(KERN_INFO MV_CESA "%s: waiting for engine finishing\n", __func__);
+ cpg->eng_st = ENGINE_W_DEQUEUE;
+ wake_up_process(cpg->queue_th);
+}
+
+static void mv_setup_timer(void)
+{
+ setup_timer(&cpg->completion_timer, &mv_completion_timer_callback, 0);
+ mod_timer(&cpg->completion_timer,
+ jiffies + msecs_to_jiffies(MV_CESA_EXPIRE));
+}
+
static void compute_aes_dec_key(struct mv_ctx *ctx)
{
struct crypto_aes_ctx gen_aes_key;
@@ -271,12 +296,8 @@ static void mv_process_current_q(int first_block)
sizeof(struct sec_accel_config));
/* GO */
+ mv_setup_timer();
writel(SEC_CMD_EN_SEC_ACCL0, cpg->reg + SEC_ACCEL_CMD);
-
- /*
- * XXX: add timer if the interrupt does not occur for some mystery
- * reason
- */
}
static void mv_crypto_algo_completion(void)
@@ -355,12 +376,8 @@ static void mv_process_hash_current(int first_block)
memcpy(cpg->sram + SRAM_CONFIG, &op, sizeof(struct sec_accel_config));
/* GO */
+ mv_setup_timer();
writel(SEC_CMD_EN_SEC_ACCL0, cpg->reg + SEC_ACCEL_CMD);
-
- /*
- * XXX: add timer if the interrupt does not occur for some mystery
- * reason
- */
}
static inline int mv_hash_import_sha1_ctx(const struct mv_req_hash_ctx *ctx,
@@ -886,6 +903,10 @@ irqreturn_t crypto_int(int irq, void *priv)
if (!(val & SEC_INT_ACCEL0_DONE))
return IRQ_NONE;
+ if (!del_timer(&cpg->completion_timer)) {
+ printk(KERN_WARNING MV_CESA
+ "got an interrupt but no pending timer?\n");
+ }
val &= ~SEC_INT_ACCEL0_DONE;
writel(val, cpg->reg + FPGA_INT_STATUS);
writel(val, cpg->reg + SEC_ACCEL_INT_STATUS);
--
1.7.3.4
Signed-off-by: Phil Sutter <[email protected]>
---
drivers/crypto/mv_cesa.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/crypto/mv_cesa.c b/drivers/crypto/mv_cesa.c
index 4a1f872..d4763fb 100644
--- a/drivers/crypto/mv_cesa.c
+++ b/drivers/crypto/mv_cesa.c
@@ -1073,6 +1073,7 @@ static int mv_probe(struct platform_device *pdev)
if (ret)
goto err_thread;
+ writel(0, cpg->reg + SEC_ACCEL_INT_STATUS);
writel(SEC_INT_ACCEL0_DONE, cpg->reg + SEC_ACCEL_INT_MASK);
writel(SEC_CFG_STOP_DIG_ERR, cpg->reg + SEC_ACCEL_CFG);
writel(SRAM_CONFIG, cpg->reg + SEC_ACCEL_DESC_P0);
--
1.7.3.4
Also drop the whole definition, since it's unused otherwise.
Signed-off-by: Phil Sutter <[email protected]>
---
drivers/crypto/mv_cesa.c | 1 -
drivers/crypto/mv_cesa.h | 7 -------
2 files changed, 0 insertions(+), 8 deletions(-)
diff --git a/drivers/crypto/mv_cesa.c b/drivers/crypto/mv_cesa.c
index 8327bed..4a1f872 100644
--- a/drivers/crypto/mv_cesa.c
+++ b/drivers/crypto/mv_cesa.c
@@ -908,7 +908,6 @@ irqreturn_t crypto_int(int irq, void *priv)
"got an interrupt but no pending timer?\n");
}
val &= ~SEC_INT_ACCEL0_DONE;
- writel(val, cpg->reg + FPGA_INT_STATUS);
writel(val, cpg->reg + SEC_ACCEL_INT_STATUS);
BUG_ON(cpg->eng_st != ENGINE_BUSY);
cpg->eng_st = ENGINE_W_DEQUEUE;
diff --git a/drivers/crypto/mv_cesa.h b/drivers/crypto/mv_cesa.h
index 08fcb11..81ce109 100644
--- a/drivers/crypto/mv_cesa.h
+++ b/drivers/crypto/mv_cesa.h
@@ -29,13 +29,6 @@
#define SEC_ST_ACT_0 (1 << 0)
#define SEC_ST_ACT_1 (1 << 1)
-/*
- * FPGA_INT_STATUS looks like a FPGA leftover and is documented only in Errata
- * 4.12. It looks like that it was part of an IRQ-controller in FPGA and
- * someone forgot to remove it while switching to the core and moving to
- * SEC_ACCEL_INT_STATUS.
- */
-#define FPGA_INT_STATUS 0xdd68
#define SEC_ACCEL_INT_STATUS 0xde20
#define SEC_INT_AUTH_DONE (1 << 0)
#define SEC_INT_DES_E_DONE (1 << 1)
--
1.7.3.4
Since mv_hash_final_fallback() uses ctx->state, read out the digest
state register before calling it.
Signed-off-by: Phil Sutter <[email protected]>
---
drivers/crypto/mv_cesa.c | 19 +++++++++++++------
1 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/crypto/mv_cesa.c b/drivers/crypto/mv_cesa.c
index d4763fb..3cc9237 100644
--- a/drivers/crypto/mv_cesa.c
+++ b/drivers/crypto/mv_cesa.c
@@ -421,6 +421,15 @@ out:
return rc;
}
+static void mv_save_digest_state(struct mv_req_hash_ctx *ctx)
+{
+ ctx->state[0] = readl(cpg->reg + DIGEST_INITIAL_VAL_A);
+ ctx->state[1] = readl(cpg->reg + DIGEST_INITIAL_VAL_B);
+ ctx->state[2] = readl(cpg->reg + DIGEST_INITIAL_VAL_C);
+ ctx->state[3] = readl(cpg->reg + DIGEST_INITIAL_VAL_D);
+ ctx->state[4] = readl(cpg->reg + DIGEST_INITIAL_VAL_E);
+}
+
static void mv_hash_algo_completion(void)
{
struct ahash_request *req = ahash_request_cast(cpg->cur_req);
@@ -435,14 +444,12 @@ static void mv_hash_algo_completion(void)
memcpy(req->result, cpg->sram + SRAM_DIGEST_BUF,
crypto_ahash_digestsize(crypto_ahash_reqtfm
(req)));
- } else
+ } else {
+ mv_save_digest_state(ctx);
mv_hash_final_fallback(req);
+ }
} else {
- ctx->state[0] = readl(cpg->reg + DIGEST_INITIAL_VAL_A);
- ctx->state[1] = readl(cpg->reg + DIGEST_INITIAL_VAL_B);
- ctx->state[2] = readl(cpg->reg + DIGEST_INITIAL_VAL_C);
- ctx->state[3] = readl(cpg->reg + DIGEST_INITIAL_VAL_D);
- ctx->state[4] = readl(cpg->reg + DIGEST_INITIAL_VAL_E);
+ mv_save_digest_state(ctx);
}
}
--
1.7.3.4
On 2012-5-25 21:54, Phil Sutter wrote:
> Also drop the whole definition, since it's unused otherwise.
>
> Signed-off-by: Phil Sutter<[email protected]>
> ---
> drivers/crypto/mv_cesa.c | 1 -
> drivers/crypto/mv_cesa.h | 7 -------
> 2 files changed, 0 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/crypto/mv_cesa.c b/drivers/crypto/mv_cesa.c
> index 8327bed..4a1f872 100644
> --- a/drivers/crypto/mv_cesa.c
> +++ b/drivers/crypto/mv_cesa.c
> @@ -908,7 +908,6 @@ irqreturn_t crypto_int(int irq, void *priv)
> "got an interrupt but no pending timer?\n");
> }
> val&= ~SEC_INT_ACCEL0_DONE;
> - writel(val, cpg->reg + FPGA_INT_STATUS);
> writel(val, cpg->reg + SEC_ACCEL_INT_STATUS);
> BUG_ON(cpg->eng_st != ENGINE_BUSY);
> cpg->eng_st = ENGINE_W_DEQUEUE;
> diff --git a/drivers/crypto/mv_cesa.h b/drivers/crypto/mv_cesa.h
> index 08fcb11..81ce109 100644
> --- a/drivers/crypto/mv_cesa.h
> +++ b/drivers/crypto/mv_cesa.h
> @@ -29,13 +29,6 @@
> #define SEC_ST_ACT_0 (1<< 0)
> #define SEC_ST_ACT_1 (1<< 1)
>
> -/*
> - * FPGA_INT_STATUS looks like a FPGA leftover and is documented only in Errata
> - * 4.12. It looks like that it was part of an IRQ-controller in FPGA and
> - * someone forgot to remove it while switching to the core and moving to
> - * SEC_ACCEL_INT_STATUS.
> - */
> -#define FPGA_INT_STATUS 0xdd68
> #define SEC_ACCEL_INT_STATUS 0xde20
> #define SEC_INT_AUTH_DONE (1<< 0)
> #define SEC_INT_DES_E_DONE (1<< 1)
According to the functional errata of 88F5182, the FPGA_INT_STATUS is
needed (at least for 88F5182-A1/A2). Here is the quote from that errata:
4.12 Clearing the Cryptographic Engines and Security Accelerator
Interrupt Cause Register
Type: Guideline
Ref#: GL-CESA-100
Relevant for: 88F5182-A1/A2
Description:
Writing 0 to bits[6:0] of the Crytographic Engines ... Interrupt Cause
register (offset 0x9DE20) has no effect.
Steps to be performed by the designer
Before writing 0 to any of the bits[6:0] of the Cryptographic Engines ..
Interrupt Cause register, the software must write 0 to the corresponding
bit of the internal register at offset 0x9DD68.
Writing to offset 0x9DD68 is not possible when any of the Security
Accelerators' sessions are active. Therefore, the software must verify
that no channel is active before clearing any of those interrupts.
Hi,
On Mon, May 28, 2012 at 09:58:32AM +0800, cloudy.linux wrote:
> On 2012-5-25 21:54, Phil Sutter wrote:
> > Also drop the whole definition, since it's unused otherwise.
> >
> > Signed-off-by: Phil Sutter<[email protected]>
> > ---
> > drivers/crypto/mv_cesa.c | 1 -
> > drivers/crypto/mv_cesa.h | 7 -------
> > 2 files changed, 0 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/crypto/mv_cesa.c b/drivers/crypto/mv_cesa.c
> > index 8327bed..4a1f872 100644
> > --- a/drivers/crypto/mv_cesa.c
> > +++ b/drivers/crypto/mv_cesa.c
> > @@ -908,7 +908,6 @@ irqreturn_t crypto_int(int irq, void *priv)
> > "got an interrupt but no pending timer?\n");
> > }
> > val&= ~SEC_INT_ACCEL0_DONE;
> > - writel(val, cpg->reg + FPGA_INT_STATUS);
> > writel(val, cpg->reg + SEC_ACCEL_INT_STATUS);
> > BUG_ON(cpg->eng_st != ENGINE_BUSY);
> > cpg->eng_st = ENGINE_W_DEQUEUE;
> > diff --git a/drivers/crypto/mv_cesa.h b/drivers/crypto/mv_cesa.h
> > index 08fcb11..81ce109 100644
> > --- a/drivers/crypto/mv_cesa.h
> > +++ b/drivers/crypto/mv_cesa.h
> > @@ -29,13 +29,6 @@
> > #define SEC_ST_ACT_0 (1<< 0)
> > #define SEC_ST_ACT_1 (1<< 1)
> >
> > -/*
> > - * FPGA_INT_STATUS looks like a FPGA leftover and is documented only in Errata
> > - * 4.12. It looks like that it was part of an IRQ-controller in FPGA and
> > - * someone forgot to remove it while switching to the core and moving to
> > - * SEC_ACCEL_INT_STATUS.
> > - */
> > -#define FPGA_INT_STATUS 0xdd68
> > #define SEC_ACCEL_INT_STATUS 0xde20
> > #define SEC_INT_AUTH_DONE (1<< 0)
> > #define SEC_INT_DES_E_DONE (1<< 1)
>
> According to the functional errata of 88F5182, the FPGA_INT_STATUS is
> needed (at least for 88F5182-A1/A2). Here is the quote from that errata:
>
> 4.12 Clearing the Cryptographic Engines and Security Accelerator
> Interrupt Cause Register
> Type: Guideline
> Ref#: GL-CESA-100
> Relevant for: 88F5182-A1/A2
>
> Description:
> Writing 0 to bits[6:0] of the Crytographic Engines ... Interrupt Cause
> register (offset 0x9DE20) has no effect.
>
> Steps to be performed by the designer
> Before writing 0 to any of the bits[6:0] of the Cryptographic Engines ..
> Interrupt Cause register, the software must write 0 to the corresponding
> bit of the internal register at offset 0x9DD68.
> Writing to offset 0x9DD68 is not possible when any of the Security
> Accelerators' sessions are active. Therefore, the software must verify
> that no channel is active before clearing any of those interrupts.
Oh, that explains why it's not needed on Kirkwood but still left there.
I could make it compile-time optional, depending on ARCH_ORION5X e.g. or
simply drop the patch and leave it alone since it really doesn't hurt
that much.
Anyway, thanks a lot for your kind review!
Greetings, Phil
Phil Sutter
Software Engineer
--
Viprinet GmbH
Mainzer Str. 43
55411 Bingen am Rhein
Germany
Phone/Zentrale: +49-6721-49030-0
Direct line/Durchwahl: +49-6721-49030-134
Fax: +49-6721-49030-209
[email protected]
http://www.viprinet.com
Registered office/Sitz der Gesellschaft: Bingen am Rhein
Commercial register/Handelsregister: Amtsgericht Mainz HRB40380
CEO/Geschäftsführer: Simon Kissel
On Tue, May 29, 2012 at 01:45:29PM +0200, Phil Sutter wrote:
>
> Oh, that explains why it's not needed on Kirkwood but still left there.
> I could make it compile-time optional, depending on ARCH_ORION5X e.g. or
> simply drop the patch and leave it alone since it really doesn't hurt
> that much.
OK I've skipped number two and applied the other three.
Thanks!
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt