Received-SPF: pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   Iuliana Prodan <iuliana.prodan@nxp.com>
To:     Corentin Labbe <clabbe.montjoie@gmail.com>,
        "alexandre.torgue@st.com" <alexandre.torgue@st.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "herbert@gondor.apana.org.au" <herbert@gondor.apana.org.au>,
        "mcoquelin.stm32@gmail.com" <mcoquelin.stm32@gmail.com>,
        "mripard@kernel.org" <mripard@kernel.org>,
        "wens@csie.org" <wens@csie.org>,
        Horia Geanta <horia.geanta@nxp.com>,
        Aymen Sghaier <aymen.sghaier@nxp.com>
CC:     "linux-arm-kernel@lists.infradead.org" 
        <linux-arm-kernel@lists.infradead.org>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-stm32@st-md-mailman.stormreply.com" 
        <linux-stm32@st-md-mailman.stormreply.com>,
        "linux-sunxi@googlegroups.com" <linux-sunxi@googlegroups.com>
Subject: Re: [PATCH RFC 05/10] crypto: engine: transform cur_req in an array
Thread-Topic: [PATCH RFC 05/10] crypto: engine: transform cur_req in an array
Thread-Index: AQHVyuLoLn4V3kbUsEW/6unpDUGQYQ==
Date:   Thu, 16 Jan 2020 11:34:09 +0000
Message-ID: <VI1PR04MB444537D76D295AF3A337039D8C360@VI1PR04MB4445.eurprd04.prod.outlook.com>
References: <20200114135936.32422-1-clabbe.montjoie@gmail.com>
 <20200114135936.32422-6-clabbe.montjoie@gmail.com>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: nxp.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 03975f72-c4e3-49d8-e225-08d79a780105
X-MS-Exchange-CrossTenant-originalarrivaltime: 16 Jan 2020 11:34:09.9130
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: y1C+gGUxK5gPeiss0FxOexpPN9tFL4C2x6P/JIw333VVeD7lG736LpKGR+LH6mcd35FzJihwaCRizUL5db9Qcg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB7184
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

On 1/14/2020 3:59 PM, Corentin Labbe wrote:=0A=
> For having the ability of doing a batch of request in one do_one_request(=
), we=0A=
> should be able to store them in an array. (for unpreparing them later).=
=0A=
> This patch converts cur_req in an array of request, but for the moment=0A=
> hardcode the maximum to 1.=0A=
> =0A=
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>=0A=
> ---=0A=
>   crypto/crypto_engine.c  | 32 ++++++++++++++++++--------------=0A=
>   include/crypto/engine.h | 19 +++++++++++++++----=0A=
>   2 files changed, 33 insertions(+), 18 deletions(-)=0A=
> =0A=
> diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c=0A=
> index eb029ff1e05a..b72873550587 100644=0A=
> --- a/crypto/crypto_engine.c=0A=
> +++ b/crypto/crypto_engine.c=0A=
> @@ -30,26 +30,27 @@ static void crypto_finalize_request(struct crypto_eng=
ine *engine,=0A=
>   	struct crypto_engine_ctx *enginectx;=0A=
>   =0A=
>   	spin_lock_irqsave(&engine->queue_lock, flags);=0A=
> -	if (engine->cur_req =3D=3D req)=0A=
> +	if (engine->cur_reqs[0].req =3D=3D req)=0A=
>   		finalize_cur_req =3D true;=0A=
>   	spin_unlock_irqrestore(&engine->queue_lock, flags);=0A=
>   =0A=
>   	if (finalize_cur_req) {=0A=
> -		enginectx =3D crypto_tfm_ctx(req->tfm);=0A=
> -		if (engine->cur_req_prepared &&=0A=
> +		enginectx =3D crypto_tfm_ctx(engine->cur_reqs[0].req->tfm);=0A=
> +		if (engine->cur_reqs[0].prepared &&=0A=
>   		    enginectx->op.unprepare_request) {=0A=
> -			ret =3D enginectx->op.unprepare_request(engine, req);=0A=
> +			ret =3D enginectx->op.unprepare_request(engine, engine->cur_reqs[0].r=
eq);=0A=
>   			if (ret)=0A=
>   				dev_err(engine->dev, "failed to unprepare request\n");=0A=
>   		}=0A=
> +		engine->cur_reqs[0].req->complete(engine->cur_reqs[0].req, err);=0A=
>   		spin_lock_irqsave(&engine->queue_lock, flags);=0A=
> -		engine->cur_req =3D NULL;=0A=
> -		engine->cur_req_prepared =3D false;=0A=
> +		engine->cur_reqs[0].prepared =3D false;=0A=
> +		engine->cur_reqs[0].req =3D NULL;=0A=
>   		spin_unlock_irqrestore(&engine->queue_lock, flags);=0A=
> +	} else {=0A=
> +		req->complete(req, err);=0A=
>   	}=0A=
>   =0A=
> -	req->complete(req, err);=0A=
> -=0A=
>   	kthread_queue_work(engine->kworker, &engine->pump_requests);=0A=
>   }=0A=
>   =0A=
> @@ -74,7 +75,7 @@ static void crypto_pump_requests(struct crypto_engine *=
engine,=0A=
>   	spin_lock_irqsave(&engine->queue_lock, flags);=0A=
>   =0A=
>   	/* Make sure we are not already running a request */=0A=
> -	if (engine->cur_req)=0A=
> +	if (engine->cur_reqs[0].req)=0A=
>   		goto out;=0A=
>   =0A=
>   	/* If another context is idling then defer */=0A=
> @@ -114,7 +115,7 @@ static void crypto_pump_requests(struct crypto_engine=
 *engine,=0A=
>   	if (!async_req)=0A=
>   		goto out;=0A=
>   =0A=
> -	engine->cur_req =3D async_req;=0A=
> +	engine->cur_reqs[0].req =3D async_req;=0A=
>   	if (backlog)=0A=
>   		backlog->complete(backlog, -EINPROGRESS);=0A=
>   =0A=
> @@ -143,14 +144,14 @@ static void crypto_pump_requests(struct crypto_engi=
ne *engine,=0A=
>   				ret);=0A=
>   			goto req_err;=0A=
>   		}=0A=
> -		engine->cur_req_prepared =3D true;=0A=
> +		engine->cur_reqs[0].prepared =3D true;=0A=
>   	}=0A=
>   	if (!enginectx->op.do_one_request) {=0A=
>   		dev_err(engine->dev, "failed to do request\n");=0A=
>   		ret =3D -EINVAL;=0A=
>   		goto req_err;=0A=
>   	}=0A=
> -	ret =3D enginectx->op.do_one_request(engine, async_req);=0A=
> +	ret =3D enginectx->op.do_one_request(engine, engine->cur_reqs[0].req);=
=0A=
>   	if (ret) {=0A=
>   		dev_err(engine->dev, "Failed to do one request from queue: %d\n", ret=
);=0A=
>   		goto req_err;=0A=
> @@ -158,7 +159,7 @@ static void crypto_pump_requests(struct crypto_engine=
 *engine,=0A=
>   	return;=0A=
>   =0A=
>   req_err:=0A=
> -	crypto_finalize_request(engine, async_req, ret);=0A=
> +	crypto_finalize_request(engine, engine->cur_reqs[0].req, ret);=0A=
>   	return;=0A=
>   =0A=
>   out:=0A=
> @@ -411,10 +412,13 @@ struct crypto_engine *crypto_engine_alloc_init(stru=
ct device *dev, bool rt)=0A=
>   	engine->running =3D false;=0A=
>   	engine->busy =3D false;=0A=
>   	engine->idling =3D false;=0A=
> -	engine->cur_req_prepared =3D false;=0A=
>   	engine->priv_data =3D dev;=0A=
>   	snprintf(engine->name, sizeof(engine->name),=0A=
>   		 "%s-engine", dev_name(dev));=0A=
> +	engine->rmax =3D 1;=0A=
> +	engine->cur_reqs =3D devm_kzalloc(dev, sizeof(struct cur_req) * engine-=
>rmax, GFP_KERNEL);=0A=
> +	if (!engine->cur_reqs)=0A=
> +		return NULL;=0A=
>   =0A=
>   	crypto_init_queue(&engine->queue, CRYPTO_ENGINE_MAX_QLEN);=0A=
>   	spin_lock_init(&engine->queue_lock);=0A=
> diff --git a/include/crypto/engine.h b/include/crypto/engine.h=0A=
> index e29cd67f93c7..362134e226f4 100644=0A=
> --- a/include/crypto/engine.h=0A=
> +++ b/include/crypto/engine.h=0A=
> @@ -18,13 +18,23 @@=0A=
>   #include <crypto/skcipher.h>=0A=
>   =0A=
>   #define ENGINE_NAME_LEN	30=0A=
> +=0A=
> +/*=0A=
> + * struct cur_req - Represent a request to be processed=0A=
> + * @prepared:	Does the request was prepared=0A=
> + * @req:	The request to be processed=0A=
> + */=0A=
> +struct cur_req {=0A=
> +	bool				prepared;=0A=
> +	struct crypto_async_request	*req;=0A=
> +};=0A=
> +=0A=
>   /*=0A=
>    * struct crypto_engine - crypto hardware engine=0A=
>    * @name: the engine name=0A=
>    * @idling: the engine is entering idle state=0A=
>    * @busy: request pump is busy=0A=
>    * @running: the engine is on working=0A=
> - * @cur_req_prepared: current request is prepared=0A=
>    * @list: link with the global crypto engine list=0A=
>    * @queue_lock: spinlock to syncronise access to request queue=0A=
>    * @queue: the crypto queue of the engine=0A=
> @@ -38,14 +48,14 @@=0A=
>    * @kworker: kthread worker struct for request pump=0A=
>    * @pump_requests: work struct for scheduling work to the request pump=
=0A=
>    * @priv_data: the engine private data=0A=
> - * @cur_req: the current request which is on processing=0A=
> + * @rmax:	The number of request which can be processed in one batch=0A=
> + * @cur_reqs: 	A list for requests to be processed=0A=
>    */=0A=
>   struct crypto_engine {=0A=
>   	char			name[ENGINE_NAME_LEN];=0A=
>   	bool			idling;=0A=
>   	bool			busy;=0A=
>   	bool			running;=0A=
> -	bool			cur_req_prepared;=0A=
>   =0A=
>   	struct list_head	list;=0A=
>   	spinlock_t		queue_lock;=0A=
> @@ -61,7 +71,8 @@ struct crypto_engine {=0A=
>   	struct kthread_work             pump_requests;=0A=
>   =0A=
>   	void				*priv_data;=0A=
> -	struct crypto_async_request	*cur_req;=0A=
> +	int 				rmax;=0A=
> +	struct cur_req 			*cur_reqs;=0A=
>   };=0A=
=0A=
To keep requests independent IMO it would be best to have a list of =0A=
requests like: struct requests run_queue, where=0A=
=0A=
struct requests {=0A=
	unsigned int max_no_reqs;=0A=
	unsigned int current_no_reqs;=0A=
	struct cur_req *cur_reqs; //list of the requests=0A=
} run_queue;=0A=