* [PATCH 1/4] mv_cesa: add an expiry timer in case anything goes wrong
@ 2012-05-25 13:54 Phil Sutter
2012-05-25 13:54 ` [PATCH 2/4] mv_cesa: no need to write to that FPGA_INT_STATUS field Phil Sutter
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Phil Sutter @ 2012-05-25 13:54 UTC (permalink / raw)
To: linux-crypto; +Cc: Herbert Xu
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 <phil.sutter@viprinet.com>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH 2/4] mv_cesa: no need to write to that FPGA_INT_STATUS field
2012-05-25 13:54 [PATCH 1/4] mv_cesa: add an expiry timer in case anything goes wrong Phil Sutter
@ 2012-05-25 13:54 ` Phil Sutter
2012-05-28 1:58 ` cloudy.linux
2012-05-25 13:54 ` [PATCH 3/4] mv_cesa: initialise the interrupt status field to zero Phil Sutter
2012-05-25 13:54 ` [PATCH 4/4] mv_cesa: fix for hash finalisation with data Phil Sutter
2 siblings, 1 reply; 7+ messages in thread
From: Phil Sutter @ 2012-05-25 13:54 UTC (permalink / raw)
To: linux-crypto; +Cc: Herbert Xu
Also drop the whole definition, since it's unused otherwise.
Signed-off-by: Phil Sutter <phil.sutter@viprinet.com>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/4] mv_cesa: no need to write to that FPGA_INT_STATUS field
2012-05-25 13:54 ` [PATCH 2/4] mv_cesa: no need to write to that FPGA_INT_STATUS field Phil Sutter
@ 2012-05-28 1:58 ` cloudy.linux
2012-05-29 11:45 ` Phil Sutter
0 siblings, 1 reply; 7+ messages in thread
From: cloudy.linux @ 2012-05-28 1:58 UTC (permalink / raw)
To: Phil Sutter; +Cc: linux-crypto, Herbert Xu
On 2012-5-25 21:54, Phil Sutter wrote:
> Also drop the whole definition, since it's unused otherwise.
>
> Signed-off-by: Phil Sutter<phil.sutter@viprinet.com>
> ---
> 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/4] mv_cesa: no need to write to that FPGA_INT_STATUS field
2012-05-28 1:58 ` cloudy.linux
@ 2012-05-29 11:45 ` Phil Sutter
2012-06-12 10:03 ` Herbert Xu
0 siblings, 1 reply; 7+ messages in thread
From: Phil Sutter @ 2012-05-29 11:45 UTC (permalink / raw)
To: cloudy.linux; +Cc: linux-crypto, Herbert Xu
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<phil.sutter@viprinet.com>
> > ---
> > 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
phil.sutter@viprinet.com
http://www.viprinet.com
Registered office/Sitz der Gesellschaft: Bingen am Rhein
Commercial register/Handelsregister: Amtsgericht Mainz HRB40380
CEO/Geschäftsführer: Simon Kissel
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 3/4] mv_cesa: initialise the interrupt status field to zero
2012-05-25 13:54 [PATCH 1/4] mv_cesa: add an expiry timer in case anything goes wrong Phil Sutter
2012-05-25 13:54 ` [PATCH 2/4] mv_cesa: no need to write to that FPGA_INT_STATUS field Phil Sutter
@ 2012-05-25 13:54 ` Phil Sutter
2012-05-25 13:54 ` [PATCH 4/4] mv_cesa: fix for hash finalisation with data Phil Sutter
2 siblings, 0 replies; 7+ messages in thread
From: Phil Sutter @ 2012-05-25 13:54 UTC (permalink / raw)
To: linux-crypto; +Cc: Herbert Xu
Signed-off-by: Phil Sutter <phil.sutter@viprinet.com>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 4/4] mv_cesa: fix for hash finalisation with data
2012-05-25 13:54 [PATCH 1/4] mv_cesa: add an expiry timer in case anything goes wrong Phil Sutter
2012-05-25 13:54 ` [PATCH 2/4] mv_cesa: no need to write to that FPGA_INT_STATUS field Phil Sutter
2012-05-25 13:54 ` [PATCH 3/4] mv_cesa: initialise the interrupt status field to zero Phil Sutter
@ 2012-05-25 13:54 ` Phil Sutter
2 siblings, 0 replies; 7+ messages in thread
From: Phil Sutter @ 2012-05-25 13:54 UTC (permalink / raw)
To: linux-crypto; +Cc: Herbert Xu
Since mv_hash_final_fallback() uses ctx->state, read out the digest
state register before calling it.
Signed-off-by: Phil Sutter <phil.sutter@viprinet.com>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-06-12 10:04 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-25 13:54 [PATCH 1/4] mv_cesa: add an expiry timer in case anything goes wrong Phil Sutter
2012-05-25 13:54 ` [PATCH 2/4] mv_cesa: no need to write to that FPGA_INT_STATUS field Phil Sutter
2012-05-28 1:58 ` cloudy.linux
2012-05-29 11:45 ` Phil Sutter
2012-06-12 10:03 ` Herbert Xu
2012-05-25 13:54 ` [PATCH 3/4] mv_cesa: initialise the interrupt status field to zero Phil Sutter
2012-05-25 13:54 ` [PATCH 4/4] mv_cesa: fix for hash finalisation with data Phil Sutter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).