| Message ID | 20230808105527.1707039-3-meenakshi.aggarwal@nxp.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: caam/jr - fix shared IRQ line handling | expand |
Reviewed-by: Gaurav Jain <gaurav.jain@nxp.com> > -----Original Message----- > From: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > Sent: Tuesday, August 8, 2023 4:25 PM > To: Horia Geanta <horia.geanta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>; > Pankaj Gupta <pankaj.gupta@nxp.com>; Gaurav Jain <gaurav.jain@nxp.com>; > herbert@gondor.apana.org.au; davem@davemloft.net; linux- > crypto@vger.kernel.org; linux-kernel@vger.kernel.org > Cc: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > Subject: [PATCH] crypto: caam/jr - fix shared IRQ line handling > > From: Horia Geantă <horia.geanta@nxp.com> > > There are cases when the interrupt status register (JRINTR) is non-zero, even > though: > 1. An interrupt was generated, but it was masked OR 2. There was no interrupt > generated at all for the corresponding job ring. > > 1. The case when interrupt is masked (JRCFGR_LS[IMSK]=1b'1) while other > events have happened and are being accounted for, e.g. > -JRINTR[HALT]=2b'10 - input job ring underwent a flush of all on-going jobs and > processing of still-existing jobs (sitting in the ring) has been halted > -JRINTR[HALT]=2b'01 - input job ring is currently undergoing a flush > -JRINTR[ENTER_FAIL]=1b'1 - SecMon / SNVS transitioned to FAIL MODE It > doesn't matter whether these events would assert the interrupt signal or not, > interrupt is anyhow masked. > > 2. The case when interrupt is not masked (JRCFGR_LS[IMSK]=1b'0), however the > events accounted for in JRINTR do not generate interrupts, e.g.: > -JRINTR[HALT]=2b'01 > -JRINTR[ENTER_FAIL]=1b'1 and JRCFGR_MS[FAIL_MODE]=1b'0 > > Currently in these cases, when the JR interrupt handler is invoked (as a > consequence of JR sharing the interrupt line with other devices - e.g. > the two JRs on i.MX7ULP) it continues execution instead of returning IRQ_NONE. > This could lead to situations like interrupt handler clearing JRINTR (and thus also > the JRINTR[HALT] field) while corresponding job ring is suspended and then that > job ring failing on resume path, due to expecting > JRINTR[HALT]=b'10 and reading instead JRINTR[HALT]=b'00. > > Fix this by checking status of JRINTR[JRI] in the JR interrupt handler. > If JRINTR[JRI]=1b'0, there was no interrupt generated for this JR and handler > must return IRQ_NONE. > > Signed-off-by: Horia Geantă <horia.geanta@nxp.com> > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > --- > drivers/crypto/caam/jr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c index > 5507d5d34a4c..07ec2f27cc68 100644 > --- a/drivers/crypto/caam/jr.c > +++ b/drivers/crypto/caam/jr.c > @@ -232,7 +232,7 @@ static irqreturn_t caam_jr_interrupt(int irq, void *st_dev) > * tasklet if jobs done. > */ > irqstate = rd_reg32(&jrp->rregs->jrintstatus); > - if (!irqstate) > + if (!(irqstate & JRINT_JR_INT)) > return IRQ_NONE; > > /* > -- > 2.25.1
On Tue, Aug 08, 2023 at 12:55:27PM +0200, meenakshi.aggarwal@nxp.com wrote: > From: Horia Geantă <horia.geanta@nxp.com> > > There are cases when the interrupt status register (JRINTR) is non-zero, > even though: > 1. An interrupt was generated, but it was masked OR > 2. There was no interrupt generated at all > for the corresponding job ring. > > 1. The case when interrupt is masked (JRCFGR_LS[IMSK]=1b'1) > while other events have happened and are being accounted for, e.g. > -JRINTR[HALT]=2b'10 - input job ring underwent a flush of all on-going > jobs and processing of still-existing jobs (sitting in the ring) has been > halted > -JRINTR[HALT]=2b'01 - input job ring is currently undergoing a flush > -JRINTR[ENTER_FAIL]=1b'1 - SecMon / SNVS transitioned to FAIL MODE > It doesn't matter whether these events would assert the interrupt signal > or not, interrupt is anyhow masked. > > 2. The case when interrupt is not masked (JRCFGR_LS[IMSK]=1b'0), however > the events accounted for in JRINTR do not generate interrupts, e.g.: > -JRINTR[HALT]=2b'01 > -JRINTR[ENTER_FAIL]=1b'1 and JRCFGR_MS[FAIL_MODE]=1b'0 > > Currently in these cases, when the JR interrupt handler is invoked (as a > consequence of JR sharing the interrupt line with other devices - e.g. > the two JRs on i.MX7ULP) it continues execution instead of returning > IRQ_NONE. > This could lead to situations like interrupt handler clearing JRINTR (and > thus also the JRINTR[HALT] field) while corresponding job ring is > suspended and then that job ring failing on resume path, due to expecting > JRINTR[HALT]=b'10 and reading instead JRINTR[HALT]=b'00. > > Fix this by checking status of JRINTR[JRI] in the JR interrupt handler. > If JRINTR[JRI]=1b'0, there was no interrupt generated for this JR and > handler must return IRQ_NONE. > > Signed-off-by: Horia Geantă <horia.geanta@nxp.com> > Signed-off-by: Meenakshi Aggarwal <meenakshi.aggarwal@nxp.com> > --- > drivers/crypto/caam/jr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Patch applied. Thanks.
diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c index 5507d5d34a4c..07ec2f27cc68 100644 --- a/drivers/crypto/caam/jr.c +++ b/drivers/crypto/caam/jr.c @@ -232,7 +232,7 @@ static irqreturn_t caam_jr_interrupt(int irq, void *st_dev) * tasklet if jobs done. */ irqstate = rd_reg32(&jrp->rregs->jrintstatus); - if (!irqstate) + if (!(irqstate & JRINT_JR_INT)) return IRQ_NONE; /*