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>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "herbert@gondor.apana.org.au" <herbert@gondor.apana.org.au>,
        "mripard@kernel.org" <mripard@kernel.org>,
        "wens@csie.org" <wens@csie.org>
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-sunxi@googlegroups.com" <linux-sunxi@googlegroups.com>
Subject: Re: [PATCH 5/9] crypto: engine: add enqueue_request/can_do_more
Thread-Topic: [PATCH 5/9] crypto: engine: add enqueue_request/can_do_more
Thread-Index: AQHV0REbcTp6Ox1gSU+lAn+fB1GkDg==
Date:   Mon, 27 Jan 2020 22:58:36 +0000
Message-ID: <AM0PR04MB717155300E3575C07D31E1D08C0B0@AM0PR04MB7171.eurprd04.prod.outlook.com>
References: <20200122104528.30084-1-clabbe.montjoie@gmail.com>
 <20200122104528.30084-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="Windows-1252"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 1b139fd7-ab62-49cc-fe68-08d7a37c7129
X-MS-Exchange-CrossTenant-originalarrivaltime: 27 Jan 2020 22:58:36.5088
 (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: F+CnJMpdU+pAJl/uOX4TpKCZ31WgrO9p/bnUtW+3nByPp91grqJ9pXZQo2slD8fOONFXV7gy/+efyu35LQe+kQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR04MB5107
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/22/2020 12:45 PM, Corentin Labbe wrote:=0A=
> This patchs adds two new function wrapper in crypto_engine.=0A=
> - enqueue_request() for drivers enqueuing request to hardware.=0A=
> - can_queue_more() for letting drivers to tell if they can=0A=
> enqueue/prepare more.=0A=
> =0A=
> Since some drivers (like caam) only enqueue request without "doing"=0A=
> them, do_one_request() is now optional.=0A=
> =0A=
> Signed-off-by: Corentin Labbe <clabbe.montjoie@gmail.com>=0A=
> ---=0A=
>   crypto/crypto_engine.c  | 25 ++++++++++++++++++++++---=0A=
>   include/crypto/engine.h | 14 ++++++++------=0A=
>   2 files changed, 30 insertions(+), 9 deletions(-)=0A=
> =0A=
> diff --git a/crypto/crypto_engine.c b/crypto/crypto_engine.c=0A=
> index 5bcb1e740fd9..4a28548c49aa 100644=0A=
> --- a/crypto/crypto_engine.c=0A=
> +++ b/crypto/crypto_engine.c=0A=
> @@ -83,6 +83,7 @@ static void crypto_pump_requests(struct crypto_engine *=
engine,=0A=
>   		goto out;=0A=
>   	}=0A=
>   =0A=
> +retry:=0A=
>   	/* Get the fist request from the engine queue to handle */=0A=
>   	backlog =3D crypto_get_backlog(&engine->queue);=0A=
>   	async_req =3D crypto_dequeue_request(&engine->queue);=0A=
> @@ -118,10 +119,28 @@ static void crypto_pump_requests(struct crypto_engi=
ne *engine,=0A=
>   			goto req_err2;=0A=
>   		}=0A=
>   	}=0A=
> +=0A=
> +	if (enginectx->op.enqueue_request) {=0A=
> +		ret =3D enginectx->op.enqueue_request(engine, async_req);=0A=
> +		if (ret) {=0A=
> +			dev_err(engine->dev, "failed to enqueue request: %d\n",=0A=
> +				ret);=0A=
> +			goto req_err;=0A=
> +		}=0A=
> +	}=0A=
> +	if (enginectx->op.can_queue_more && engine->queue.qlen > 0) {=0A=
> +		ret =3D enginectx->op.can_queue_more(engine, async_req);=0A=
> +		if (ret > 0) {=0A=
> +			spin_lock_irqsave(&engine->queue_lock, flags);=0A=
> +			goto retry;=0A=
> +		}=0A=
> +		if (ret < 0) {=0A=
> +			dev_err(engine->dev, "failed to call can_queue_more\n");=0A=
> +			/* TODO */=0A=
> +		}=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=
> +		return;=0A=
>   	}=0A=
>   	ret =3D enginectx->op.do_one_request(engine, async_req);=0A=
>   	if (ret) {=0A=
> diff --git a/include/crypto/engine.h b/include/crypto/engine.h=0A=
> index 03d9f9ec1cea..8ab9d26e30fe 100644=0A=
> --- a/include/crypto/engine.h=0A=
> +++ b/include/crypto/engine.h=0A=
> @@ -63,14 +63,16 @@ struct crypto_engine {=0A=
>    * @prepare__request: do some prepare if need before handle the current=
 request=0A=
>    * @unprepare_request: undo any work done by prepare_request()=0A=
>    * @do_one_request: do encryption for current request=0A=
> + * @enqueue_request:	Enqueue the request in the hardware=0A=
> + * @can_queue_more:	if this function return > 0, it will tell the crypto=
=0A=
> + * 	engine that more space are availlable for prepare/enqueue request=0A=
>    */=0A=
>   struct crypto_engine_op {=0A=
> -	int (*prepare_request)(struct crypto_engine *engine,=0A=
> -			       void *areq);=0A=
> -	int (*unprepare_request)(struct crypto_engine *engine,=0A=
> -				 void *areq);=0A=
> -	int (*do_one_request)(struct crypto_engine *engine,=0A=
> -			      void *areq);=0A=
> +	int (*prepare_request)(struct crypto_engine *engine, void *areq);=0A=
> +	int (*unprepare_request)(struct crypto_engine *engine, void *areq);=0A=
> +	int (*do_one_request)(struct crypto_engine *engine, void *areq);=0A=
> +	int (*enqueue_request)(struct crypto_engine *engine, void *areq);=0A=
> +	int (*can_queue_more)(struct crypto_engine *engine, void *areq);=0A=
>   };=0A=
=0A=
As I mentioned in another thread [1], these crypto-engine patches (#1 - =0A=
#5) imply modifications in all the drivers that use crypto-engine.=0A=
It's not backwards compatible.=0A=
Your changes imply that do_one_request executes the request & waits for =0A=
completion and enqueue_request sends it to hardware. That means that all =
=0A=
the other drivers need to be modify, to implement enqueue_request, =0A=
instead of do_one_request. They need to be compliant with the new =0A=
changes, new API. Otherwise, they are not using crypto-engine right, =0A=
don't you think?=0A=
=0A=
Also, do_one_request it shouldn=92t be blocking. We got this confirmation =
=0A=
from Herbert [2].=0A=
=0A=
[1] =0A=
https://lore.kernel.org/lkml/VI1PR04MB44455343230CBA7400D21C998C0C0@VI1PR04=
MB4445.eurprd04.prod.outlook.com/=0A=
[2] =0A=
https://lore.kernel.org/lkml/20200122144134.axqpwx65j7xysyy3@gondor.apana.o=
rg.au/=0A=