From nobody Thu Dec 18 03:20:35 2025 Received: from abb.hmeau.com (abb.hmeau.com [144.6.53.87]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 436C22147F8; Thu, 8 May 2025 05:15:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=144.6.53.87 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1746681362; cv=none; b=Bl8Ic8Q1S4zED4l+nXK9PnybiZ3LmqDI8Me6YewaelUl8BBxi5VikoJLYoxjthkGh4F0+X5B9bwMzjvI/jXwSM87hDXX7jkfkT35R/0aWqAv9yi7d8FN5VBhe6pTdOJG0pbHQDL03Hyr1+2ZQwYO+c6HX7e81FhjfMLQ7FJsMn0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1746681362; c=relaxed/simple; bh=1v6DI/9WXyq4OQT7TwSDw43m/WmKKQeiOdRQ8aVrk7M=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=KvwpmxibgOpVKQIwZ3JLRy+t429y0vkReJvdr1jZ0cz1BixgKowUuqQWNydWtz6ST54MDKmGqFzjHcn0Vv9CNvi7giX7CAZrx8tuxuROxfc5FLOa+Gp4BGGERSxGBkTkJxLfjaXUpJ4O04xHYijUnPXh1w4J5j86CCe5v10w1O0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=gondor.apana.org.au; spf=pass smtp.mailfrom=gondor.apana.org.au; dkim=pass (2048-bit key) header.d=hmeau.com header.i=@hmeau.com header.b=V+d+49YL; arc=none smtp.client-ip=144.6.53.87 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=gondor.apana.org.au Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gondor.apana.org.au Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=hmeau.com header.i=@hmeau.com header.b="V+d+49YL" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=hmeau.com; s=formenos; h=In-Reply-To:Content-Type:MIME-Version:References:Message-ID: Subject:Cc:To:From:Date:Sender:Reply-To:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=wJ57py3kjq1HOOKAtKE+KQOWQ2pCiRGjulzU/iIvTiY=; b=V+d+49YL5VNC29vevrHNf17dby IeN4qNwndVtf8tPeLigopf47ZpMy9Hrrqqu99ix59YtqO3ijz+axrvMc/qUQEqzJAv8yJo4W4BdQS yuSVbpPpNOi4jZFU4OdwTKTM730fRsHqpPdp1rWNNO4NwIcvS8PLpC3w/lqi4Zv7udR/ltQCLmT/j OtH4VRxXOqmzrOEv5h5ZkPKhRdkZMiYGoYLU9cxsYQXtt9pv1/CnA4N18kXhiVMMVXk+KdJpfd6ya nH478bqMLWr3RjUQni2AmSV7Pd78ZqBQvEvZmAA7msMZ7rwqIrsnk1Wkm1z34TJyN0ma/DqlIbKSl NFabn2sA==; Received: from loth.rohan.me.apana.org.au ([192.168.167.2]) by formenos.hmeau.com with smtp (Exim 4.96 #2 (Debian)) id 1uCtbv-004RbY-0m; Thu, 08 May 2025 13:15:56 +0800 Received: by loth.rohan.me.apana.org.au (sSMTP sendmail emulation); Thu, 08 May 2025 13:15:55 +0800 Date: Thu, 8 May 2025 13:15:55 +0800 From: Herbert Xu To: Corentin Labbe Cc: Klaus Kudielka , regressions@lists.linux.dev, linux-kernel@vger.kernel.org, Linux Crypto Mailing List , Boris Brezillon , EBALARD Arnaud , Romain Perier Subject: [v2 PATCH] crypto: marvell/cesa - Do not chain submitted requests Message-ID: References: <7e38e34adddb14d0a23a13cf738b6b7cccbfce6f.camel@gmail.com> <15fadc356b73a1e8e24183f284b5c0a44a53e679.camel@gmail.com> <5db212655dc98945fa3f529925821879a03ff554.camel@gmail.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" On Wed, May 07, 2025 at 05:16:03PM +0200, Corentin Labbe wrote: > > I tested this patch and my armada-388-clearfog-pro panic with: Thanks for testing! I didn't realise that you had one of these. So just out of curiosity, does this driver actually pass the self-tests before this patch with CRYPTO_MANAGER_EXTRA_TESTS? Is your system SMP? How many CPUs? In any case, my patch screwed up the very first chaining and here is a fixed version: Reported-by: Klaus Kudielka ---8<--- This driver tries to chain requests together before submitting them to hardware in order to reduce completion interrupts. However, it even extends chains that have already been submitted to hardware. This is dangerous because there is no way of knowing whether the hardware has already read the DMA memory in question or not. Fix this by splitting the chain list into two. One for submitted requests and one for requests that have not yet been submitted. Only extend the latter. Reported-by: Klaus Kudielka Fixes: 85030c5168f1 ("crypto: marvell - Add support for chaining crypto req= uests in TDMA mode") Cc: Signed-off-by: Herbert Xu --- drivers/crypto/marvell/cesa/cesa.c | 2 +- drivers/crypto/marvell/cesa/cesa.h | 9 +++-- drivers/crypto/marvell/cesa/tdma.c | 55 ++++++++++++++++++------------ 3 files changed, 41 insertions(+), 25 deletions(-) diff --git a/drivers/crypto/marvell/cesa/cesa.c b/drivers/crypto/marvell/ce= sa/cesa.c index fa08f10e6f3f..9c21f5d835d2 100644 --- a/drivers/crypto/marvell/cesa/cesa.c +++ b/drivers/crypto/marvell/cesa/cesa.c @@ -94,7 +94,7 @@ static int mv_cesa_std_process(struct mv_cesa_engine *eng= ine, u32 status) =20 static int mv_cesa_int_process(struct mv_cesa_engine *engine, u32 status) { - if (engine->chain.first && engine->chain.last) + if (engine->chain_hw.first && engine->chain_hw.last) return mv_cesa_tdma_process(engine, status); =20 return mv_cesa_std_process(engine, status); diff --git a/drivers/crypto/marvell/cesa/cesa.h b/drivers/crypto/marvell/ce= sa/cesa.h index d215a6bed6bc..50ca1039fdaa 100644 --- a/drivers/crypto/marvell/cesa/cesa.h +++ b/drivers/crypto/marvell/cesa/cesa.h @@ -440,8 +440,10 @@ struct mv_cesa_dev { * SRAM * @queue: fifo of the pending crypto requests * @load: engine load counter, useful for load balancing - * @chain: list of the current tdma descriptors being processed - * by this engine. + * @chain_hw: list of the current tdma descriptors being processed + * by the hardware. + * @chain_sw: list of the current tdma descriptors that will be + * submitted to the hardware. * @complete_queue: fifo of the processed requests by the engine * * Structure storing CESA engine information. @@ -463,7 +465,8 @@ struct mv_cesa_engine { struct gen_pool *pool; struct crypto_queue queue; atomic_t load; - struct mv_cesa_tdma_chain chain; + struct mv_cesa_tdma_chain chain_hw; + struct mv_cesa_tdma_chain chain_sw; struct list_head complete_queue; int irq; }; diff --git a/drivers/crypto/marvell/cesa/tdma.c b/drivers/crypto/marvell/ce= sa/tdma.c index 388a06e180d6..02b609ef7043 100644 --- a/drivers/crypto/marvell/cesa/tdma.c +++ b/drivers/crypto/marvell/cesa/tdma.c @@ -38,6 +38,15 @@ void mv_cesa_dma_step(struct mv_cesa_req *dreq) { struct mv_cesa_engine *engine =3D dreq->engine; =20 + spin_lock_bh(&engine->lock); + if (engine->chain_sw.first =3D=3D dreq->chain.first) { + engine->chain_sw.first =3D NULL; + engine->chain_sw.last =3D NULL; + } + engine->chain_hw.first =3D dreq->chain.first; + engine->chain_hw.last =3D dreq->chain.last; + spin_unlock_bh(&engine->lock); + writel_relaxed(0, engine->regs + CESA_SA_CFG); =20 mv_cesa_set_int_mask(engine, CESA_SA_INT_ACC0_IDMA_DONE); @@ -96,25 +105,29 @@ void mv_cesa_dma_prepare(struct mv_cesa_req *dreq, void mv_cesa_tdma_chain(struct mv_cesa_engine *engine, struct mv_cesa_req *dreq) { - if (engine->chain.first =3D=3D NULL && engine->chain.last =3D=3D NULL) { - engine->chain.first =3D dreq->chain.first; - engine->chain.last =3D dreq->chain.last; + struct mv_cesa_tdma_desc *last =3D engine->chain_sw.last; + + /* + * Break the DMA chain if the request being queued needs the IV + * regs to be set before lauching the request. + */ + if (!last || dreq->chain.first->flags & CESA_TDMA_SET_STATE) { + engine->chain_sw.first =3D dreq->chain.first; + last =3D dreq->chain.last; + engine->chain_sw.last =3D last; } else { - struct mv_cesa_tdma_desc *last; - - last =3D engine->chain.last; last->next =3D dreq->chain.first; - engine->chain.last =3D dreq->chain.last; - - /* - * Break the DMA chain if the CESA_TDMA_BREAK_CHAIN is set on - * the last element of the current chain, or if the request - * being queued needs the IV regs to be set before lauching - * the request. - */ - if (!(last->flags & CESA_TDMA_BREAK_CHAIN) && - !(dreq->chain.first->flags & CESA_TDMA_SET_STATE)) - last->next_dma =3D cpu_to_le32(dreq->chain.first->cur_dma); + last->next_dma =3D cpu_to_le32(dreq->chain.first->cur_dma); + last =3D dreq->chain.last; + engine->chain_sw.last =3D last; + } + /* + * Break the DMA chain if the CESA_TDMA_BREAK_CHAIN is set on + * the last element of the current chain. + */ + if (last->flags & CESA_TDMA_BREAK_CHAIN) { + engine->chain_sw.first =3D NULL; + engine->chain_sw.last =3D NULL; } } =20 @@ -127,7 +140,7 @@ int mv_cesa_tdma_process(struct mv_cesa_engine *engine,= u32 status) =20 tdma_cur =3D readl(engine->regs + CESA_TDMA_CUR); =20 - for (tdma =3D engine->chain.first; tdma; tdma =3D next) { + for (tdma =3D engine->chain_hw.first; tdma; tdma =3D next) { spin_lock_bh(&engine->lock); next =3D tdma->next; spin_unlock_bh(&engine->lock); @@ -149,12 +162,12 @@ int mv_cesa_tdma_process(struct mv_cesa_engine *engin= e, u32 status) &backlog); =20 /* Re-chaining to the next request */ - engine->chain.first =3D tdma->next; + engine->chain_hw.first =3D tdma->next; tdma->next =3D NULL; =20 /* If this is the last request, clear the chain */ - if (engine->chain.first =3D=3D NULL) - engine->chain.last =3D NULL; + if (engine->chain_hw.first =3D=3D NULL) + engine->chain_hw.last =3D NULL; spin_unlock_bh(&engine->lock); =20 ctx =3D crypto_tfm_ctx(req->tfm); --=20 2.39.5 --=20 Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt