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:     Herbert Xu <herbert@gondor.apana.org.au>,
        Baolin Wang <baolin.wang@linaro.org>,
        Corentin Labbe <clabbe.montjoie@gmail.com>
CC:     Ard Biesheuvel <ard.biesheuvel@linaro.org>,
        Horia Geanta <horia.geanta@nxp.com>,
        Maxime Coquelin <mcoquelin.stm32@gmail.com>,
        Alexandre Torgue <alexandre.torgue@st.com>,
        Maxime Ripard <mripard@kernel.org>,
        Aymen Sghaier <aymen.sghaier@nxp.com>,
        "David S. Miller" <davem@davemloft.net>,
        Silvano Di Ninno <silvano.dininno@nxp.com>,
        Franck Lenormand <franck.lenormand@nxp.com>,
        "linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        dl-linux-imx <linux-imx@nxp.com>
Subject: Re: [RFC PATCH] Crypto-engine support for parallel requests
Thread-Topic: [RFC PATCH] Crypto-engine support for parallel requests
Thread-Index: AQHVz+nyH19jpegvq0OzoQlSx1aIPw==
Date:   Wed, 22 Jan 2020 12:29:22 +0000
Message-ID: <VI1PR04MB44455343230CBA7400D21C998C0C0@VI1PR04MB4445.eurprd04.prod.outlook.com>
References: <1579563149-3678-1-git-send-email-iuliana.prodan@nxp.com>
 <20200121100053.GA14095@Red>
 <VI1PR04MB44454A0468073981FDA603B38C0D0@VI1PR04MB4445.eurprd04.prod.outlook.com>
 <20200122104134.GA13173@Red>
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: de00cc46-5d81-463b-7384-08d79f36b601
X-MS-Exchange-CrossTenant-originalarrivaltime: 22 Jan 2020 12:29:22.5865
 (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: TthnQpUSexdizyn6g0zDVs5cUD7Y2QOHi935Df+e0Nqu/sLj6r8RBUN5kZoDcy/bJBZvizH/xKjcOI1UUkbTRw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB4829
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:41 PM, Corentin Labbe wrote:=0A=
> On Tue, Jan 21, 2020 at 02:20:27PM +0000, Iuliana Prodan wrote:=0A=
>> On 1/21/2020 12:00 PM, Corentin Labbe wrote:=0A=
>>> On Tue, Jan 21, 2020 at 01:32:29AM +0200, Iuliana Prodan wrote:=0A=
>>>> Added support for executing multiple requests, in parallel,=0A=
>>>> for crypto engine.=0A=
>>>> A no_reqs is initialized and set in the new=0A=
>>>> crypto_engine_alloc_init_and_set function.=0A=
>>>> =0A=
>>>>=0A=
>>>=0A=
>>> Hello=0A=
>>>=0A=
>>> In your model, who is running finalize_request() ?=0A=
>> finalize_request() in CAAM, and in other drivers, is called on the _done=
=0A=
>> callback (stm32, virtio and omap).=0A=
>>=0A=
>>> In caam it seems that you have a taskqueue dedicated for that but you c=
annot assume that all drivers will have this.=0A=
>>> I think the crypto_engine should be sufficient by itself and does not n=
eed external thread/taskqueue.=0A=
>>>=0A=
>>> But in your case, it seems that you dont have the choice, since do_one_=
request does not "do" but simply enqueue the request in the "jobring".=0A=
>>>=0A=
>> But, do_one_request it shouldn't, necessary,  execute the request. Is ok=
=0A=
>> to enqueue it, since we have asynchronous requests. do_one_request is=0A=
>> not blocking.=0A=
>>=0A=
>>> What about adding along prepare/do_one_request/unprepare a new enqueue(=
)/can_do_more() function ?=0A=
>>>=0A=
>>> The stream will be:=0A=
>>> retry:=0A=
>>> optionnal prepare=0A=
>>> optionnal enqueue=0A=
>>> optionnal can_do_more() (goto retry)=0A=
>>> optionnal do_one_request=0A=
>>>=0A=
>>> then=0A=
>>> finalize()=0A=
>>> optionnal unprepare=0A=
>>>=0A=
>>=0A=
>> I'm planning to improve crypto-engine incrementally, so I'm taking one=
=0A=
>> step at a time :)=0A=
>> But I'm not sure if adding an enqueue operation is a good idea, since,=
=0A=
>> my understanding, is that do_one_request is a non-blocking operation and=
=0A=
>> it shouldn't execute the request.=0A=
> =0A=
> do_one_request is a blocking operation on amlogic/sun8i-ce/sun8i-ss and t=
he "documentation" is clear "@do_one_request: do encryption for current req=
uest".=0A=
> But I agree that is a bit small for a documentation.=0A=
> =0A=
=0A=
Herbert, Baolin,=0A=
=0A=
What do you think about do_one_requet operation: is blocking or not?=0A=
=0A=
There are several drivers (stm32, omap, virtio, caam) that include =0A=
crypto-engine, and uses do_one_request as non-blocking, only the ones =0A=
mentioned and implemented by Corentin use do_one_request as blocking.=0A=
=0A=
>>=0A=
>> IMO, the crypto-engine flow should be kept simple:=0A=
>> 1. a request comes to hw -> this is doing transfer_request_to_engine;=0A=
>> 2. CE enqueue the requests=0A=
>> 3. on pump_requests:=0A=
>> 	3. a) optional prepare operation=0A=
>> 	3. b) sends the reqs to hw, by do_one_request operation. To wait for=0A=
>> completion here it contradicts the asynchronous crypto API.=0A=
> =0A=
> There are no contradiction, the call is asynchronous for the user of the =
API.=0A=
> =0A=
>> do_one_request operation has a crypto_async_request type as argument.=0A=
>> Note: Step 3. b) can be done several times, depending on size of hw queu=
e.=0A=
>> 4. in driver, when req is done:=0A=
>> 	4. a) optional unprepare operation=0A=
>> 	4. b) crypto_finalize_request is called=0A=
>> 	=0A=
> =0A=
> Since Herbert say the same thing than me:=0A=
> "Instead, we should just let the driver tell us when it is ready to accep=
t more requests."=0A=
> Let me insist on my proposal, I have updated my serie, and it should hand=
le your case and mine.=0A=
> I will send it within minutes.=0A=
> =0A=
=0A=
Corentin,=0A=
=0A=
In your new proposal, a few patches include my modifications. The others =
=0A=
include a solution that fits your drivers very well, but implies =0A=
modifications in all the other 4 drivers. It's not backwards compatible.=0A=
I believe it can be done better, so we won't need to modify, _at all_, =0A=
the other drivers.=0A=
=0A=
I'm working on a new version for my RFC, that has the can_enqueue_more, =0A=
as Herbert suggested, but I would really want to know how =0A=
crypto-engine's do_one_request was thought: blocking or non-blocking?=0A=
=0A=
Just your driver(s) use it as blocking, the other examples use it to =0A=
enqueue (don't block in waiting for request to finish).=0A=
=0A=
Thanks,=0A=
Iulia=0A=
=0A=
=0A=
=0A=
=0A=
=0A=