| Message ID | 1466496520-28806-3-git-send-email-romain.perier@free-electrons.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
Romain Perier <romain.perier@free-electrons.com> wrote: > Add a BUG_ON() call when the driver tries to launch a crypto request > while the engine is still processing the previous one. This replaces > a silent system hang by a verbose kernel panic with the associated > backtrace to let the user know that something went wrong in the CESA > driver. Hmm, so how can this happen? If it is triggerable then we better try to recover from it more gracefully. If it is not triggerable then why bother? Thanks,
On Wed, Jun 22, 2016 at 01:23:39PM +0200, Romain Perier wrote: > Hello, > > Le 22/06/2016 12:33, Herbert Xu a écrit : > >Romain Perier <romain.perier@free-electrons.com> wrote: > >>Add a BUG_ON() call when the driver tries to launch a crypto request > >>while the engine is still processing the previous one. This replaces > >>a silent system hang by a verbose kernel panic with the associated > >>backtrace to let the user know that something went wrong in the CESA > >>driver. > > > >Hmm, so how can this happen? > If it is triggerable then we better > >try to recover from it more gracefully. If it is not triggerable > >then why bother? > > > > Well, It does not happen with the current driver (in mainline). This > is bug I had when I added support to chain requests. Take a look at > the patch 008/010, it changes the way the requests are "prepared". > If you really enable a request while the engine is running, that's > very hard to debug. This is more useful to have a backtrace to let > the user know that something is wrong instead of having a silent > system hang. That's easier to debug and you can detect regressions. OK. All applied. Thanks.
diff --git a/drivers/crypto/marvell/cipher.c b/drivers/crypto/marvell/cipher.c index dcf1fce..8c1432e 100644 --- a/drivers/crypto/marvell/cipher.c +++ b/drivers/crypto/marvell/cipher.c @@ -106,6 +106,8 @@ static void mv_cesa_ablkcipher_std_step(struct ablkcipher_request *req) mv_cesa_set_int_mask(engine, CESA_SA_INT_ACCEL0_DONE); writel_relaxed(CESA_SA_CFG_PARA_DIS, engine->regs + CESA_SA_CFG); + BUG_ON(readl(engine->regs + CESA_SA_CMD) & + CESA_SA_CMD_EN_CESA_SA_ACCL0); writel(CESA_SA_CMD_EN_CESA_SA_ACCL0, engine->regs + CESA_SA_CMD); } diff --git a/drivers/crypto/marvell/hash.c b/drivers/crypto/marvell/hash.c index 7ca2e0f..80bddd7 100644 --- a/drivers/crypto/marvell/hash.c +++ b/drivers/crypto/marvell/hash.c @@ -237,6 +237,8 @@ static void mv_cesa_ahash_std_step(struct ahash_request *req) mv_cesa_set_int_mask(engine, CESA_SA_INT_ACCEL0_DONE); writel_relaxed(CESA_SA_CFG_PARA_DIS, engine->regs + CESA_SA_CFG); + BUG_ON(readl(engine->regs + CESA_SA_CMD) & + CESA_SA_CMD_EN_CESA_SA_ACCL0); writel(CESA_SA_CMD_EN_CESA_SA_ACCL0, engine->regs + CESA_SA_CMD); } diff --git a/drivers/crypto/marvell/tdma.c b/drivers/crypto/marvell/tdma.c index 7642798..8c86bb6 100644 --- a/drivers/crypto/marvell/tdma.c +++ b/drivers/crypto/marvell/tdma.c @@ -53,6 +53,8 @@ void mv_cesa_dma_step(struct mv_cesa_tdma_req *dreq) engine->regs + CESA_SA_CFG); writel_relaxed(dreq->chain.first->cur_dma, engine->regs + CESA_TDMA_NEXT_ADDR); + BUG_ON(readl(engine->regs + CESA_SA_CMD) & + CESA_SA_CMD_EN_CESA_SA_ACCL0); writel(CESA_SA_CMD_EN_CESA_SA_ACCL0, engine->regs + CESA_SA_CMD); }