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:     Bastian Krause <bst@pengutronix.de>,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Horia Geanta <horia.geanta@nxp.com>,
        Aymen Sghaier <aymen.sghaier@nxp.com>
CC:     "David S. Miller" <davem@davemloft.net>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        Gary Hook <gary.hook@amd.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>,
        "kernel@pengutronix.de" <kernel@pengutronix.de>
Subject: Re: [PATCH 06/12] crypto: caam - change return code in
 caam_jr_enqueue function
Thread-Topic: [PATCH 06/12] crypto: caam - change return code in
 caam_jr_enqueue function
Thread-Index: AQHVnZazAqjqlL3De02YIoYBT7QMrw==
Date:   Tue, 10 Dec 2019 12:28:29 +0000
Message-ID: <VI1PR04MB4445826D1424BF72A4C019B58C5B0@VI1PR04MB4445.eurprd04.prod.outlook.com>
References: <1574029845-22796-1-git-send-email-iuliana.prodan@nxp.com>
 <1574029845-22796-7-git-send-email-iuliana.prodan@nxp.com>
 <705afee0-155b-2b6f-3b57-615a07fb7e4f@pengutronix.de>
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="iso-8859-2"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 66ec7c5d-d1fe-46f8-938e-08d77d6c76c9
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 Dec 2019 12:28:29.7807
 (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: m4ysh7dC5Niv2Rw4p5RSVvRrXxJfK3kcaCt6Wy0C42SAXmuPNdDb9QbjSJq9dP0aOBVt1Kc0ch8DOV1r1klCTQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB4975
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 12/10/2019 1:56 PM, Bastian Krause wrote:=0A=
> Hi,=0A=
> =0A=
> On 11/17/19 11:30 PM, Iuliana Prodan wrote:=0A=
>> Change the return code of caam_jr_enqueue function to -EINPROGRESS, in=
=0A=
>> case of success, -ENOSPC in case the CAAM is busy (has no space left=0A=
>> in job ring queue), -EIO if it cannot map the caller's descriptor.=0A=
>>=0A=
>> Update, also, the cases for resource-freeing for each algorithm type.=0A=
>>=0A=
>> Signed-off-by: Iuliana Prodan <iuliana.prodan@nxp.com>=0A=
>> Reviewed-by: Horia Geant=E3 <horia.geanta@nxp.com>=0A=
>> ---=0A=
>>   drivers/crypto/caam/caamalg.c  | 16 ++++------------=0A=
>>   drivers/crypto/caam/caamhash.c | 34 +++++++++++-----------------------=
=0A=
>>   drivers/crypto/caam/caampkc.c  | 16 ++++++++--------=0A=
>>   drivers/crypto/caam/caamrng.c  |  4 ++--=0A=
>>   drivers/crypto/caam/jr.c       |  8 ++++----=0A=
>>   drivers/crypto/caam/key_gen.c  |  2 +-=0A=
>>   6 files changed, 30 insertions(+), 50 deletions(-)=0A=
>>=0A=
>> diff --git a/drivers/crypto/caam/caamalg.c b/drivers/crypto/caam/caamalg=
.c=0A=
>> index 6e021692..21b6172 100644=0A=
>> --- a/drivers/crypto/caam/caamalg.c=0A=
>> +++ b/drivers/crypto/caam/caamalg.c=0A=
>> @@ -1417,9 +1417,7 @@ static inline int chachapoly_crypt(struct aead_req=
uest *req, bool encrypt)=0A=
>>   			     1);=0A=
>>   =0A=
>>   	ret =3D caam_jr_enqueue(jrdev, desc, aead_crypt_done, req);=0A=
>> -	if (!ret) {=0A=
>> -		ret =3D -EINPROGRESS;=0A=
>> -	} else {=0A=
>> +	if (ret !=3D -EINPROGRESS) {=0A=
>>   		aead_unmap(jrdev, edesc, req);=0A=
>>   		kfree(edesc);=0A=
>>   	}=0A=
>> @@ -1462,9 +1460,7 @@ static inline int aead_crypt(struct aead_request *=
req, bool encrypt)=0A=
>>   =0A=
>>   	desc =3D edesc->hw_desc;=0A=
>>   	ret =3D caam_jr_enqueue(jrdev, desc, aead_crypt_done, req);=0A=
>> -	if (!ret) {=0A=
>> -		ret =3D -EINPROGRESS;=0A=
>> -	} else {=0A=
>> +	if (ret !=3D -EINPROGRESS) {=0A=
>>   		aead_unmap(jrdev, edesc, req);=0A=
>>   		kfree(edesc);=0A=
>>   	}=0A=
>> @@ -1507,9 +1503,7 @@ static inline int gcm_crypt(struct aead_request *r=
eq, bool encrypt)=0A=
>>   =0A=
>>   	desc =3D edesc->hw_desc;=0A=
>>   	ret =3D caam_jr_enqueue(jrdev, desc, aead_crypt_done, req);=0A=
>> -	if (!ret) {=0A=
>> -		ret =3D -EINPROGRESS;=0A=
>> -	} else {=0A=
>> +	if (ret !=3D -EINPROGRESS) {=0A=
>>   		aead_unmap(jrdev, edesc, req);=0A=
>>   		kfree(edesc);=0A=
>>   	}=0A=
>> @@ -1725,9 +1719,7 @@ static inline int skcipher_crypt(struct skcipher_r=
equest *req, bool encrypt)=0A=
>>   	desc =3D edesc->hw_desc;=0A=
>>   	ret =3D caam_jr_enqueue(jrdev, desc, skcipher_crypt_done, req);=0A=
>>   =0A=
>> -	if (!ret) {=0A=
>> -		ret =3D -EINPROGRESS;=0A=
>> -	} else {=0A=
>> +	if (ret !=3D -EINPROGRESS) {=0A=
>>   		skcipher_unmap(jrdev, edesc, req);=0A=
>>   		kfree(edesc);=0A=
>>   	}=0A=
>> diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamha=
sh.c=0A=
>> index 5f9f16c..baf4ab1 100644=0A=
>> --- a/drivers/crypto/caam/caamhash.c=0A=
>> +++ b/drivers/crypto/caam/caamhash.c=0A=
>> @@ -422,7 +422,7 @@ static int hash_digest_key(struct caam_hash_ctx *ctx=
, u32 *keylen, u8 *key,=0A=
>>   	init_completion(&result.completion);=0A=
>>   =0A=
>>   	ret =3D caam_jr_enqueue(jrdev, desc, split_key_done, &result);=0A=
>> -	if (!ret) {=0A=
>> +	if (ret =3D=3D -EINPROGRESS) {=0A=
>>   		/* in progress */=0A=
>>   		wait_for_completion(&result.completion);=0A=
>>   		ret =3D result.err;=0A=
>> @@ -858,10 +858,8 @@ static int ahash_update_ctx(struct ahash_request *r=
eq)=0A=
>>   				     desc_bytes(desc), 1);=0A=
>>   =0A=
>>   		ret =3D caam_jr_enqueue(jrdev, desc, ahash_done_bi, req);=0A=
>> -		if (ret)=0A=
>> +		if (ret !=3D -EINPROGRESS)=0A=
>>   			goto unmap_ctx;=0A=
>> -=0A=
>> -		ret =3D -EINPROGRESS;=0A=
>>   	} else if (*next_buflen) {=0A=
>>   		scatterwalk_map_and_copy(buf + *buflen, req->src, 0,=0A=
>>   					 req->nbytes, 0);=0A=
>> @@ -936,10 +934,9 @@ static int ahash_final_ctx(struct ahash_request *re=
q)=0A=
>>   			     1);=0A=
>>   =0A=
>>   	ret =3D caam_jr_enqueue(jrdev, desc, ahash_done_ctx_src, req);=0A=
>> -	if (ret)=0A=
>> -		goto unmap_ctx;=0A=
>> +	if (ret =3D=3D -EINPROGRESS)=0A=
>> +		return ret;=0A=
>>   =0A=
>> -	return -EINPROGRESS;=0A=
>>    unmap_ctx:=0A=
>>   	ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_BIDIRECTIONAL);=0A=
>>   	kfree(edesc);=0A=
>> @@ -1013,10 +1010,9 @@ static int ahash_finup_ctx(struct ahash_request *=
req)=0A=
>>   			     1);=0A=
>>   =0A=
>>   	ret =3D caam_jr_enqueue(jrdev, desc, ahash_done_ctx_src, req);=0A=
>> -	if (ret)=0A=
>> -		goto unmap_ctx;=0A=
>> +	if (ret =3D=3D -EINPROGRESS)=0A=
>> +		return ret;=0A=
>>   =0A=
>> -	return -EINPROGRESS;=0A=
>>    unmap_ctx:=0A=
>>   	ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_BIDIRECTIONAL);=0A=
>>   	kfree(edesc);=0A=
>> @@ -1086,9 +1082,7 @@ static int ahash_digest(struct ahash_request *req)=
=0A=
>>   			     1);=0A=
>>   =0A=
>>   	ret =3D caam_jr_enqueue(jrdev, desc, ahash_done, req);=0A=
>> -	if (!ret) {=0A=
>> -		ret =3D -EINPROGRESS;=0A=
>> -	} else {=0A=
>> +	if (ret !=3D -EINPROGRESS) {=0A=
>>   		ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);=0A=
>>   		kfree(edesc);=0A=
>>   	}=0A=
>> @@ -1138,9 +1132,7 @@ static int ahash_final_no_ctx(struct ahash_request=
 *req)=0A=
>>   			     1);=0A=
>>   =0A=
>>   	ret =3D caam_jr_enqueue(jrdev, desc, ahash_done, req);=0A=
>> -	if (!ret) {=0A=
>> -		ret =3D -EINPROGRESS;=0A=
>> -	} else {=0A=
>> +	if (ret !=3D -EINPROGRESS) {=0A=
>>   		ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);=0A=
>>   		kfree(edesc);=0A=
>>   	}=0A=
>> @@ -1258,10 +1250,9 @@ static int ahash_update_no_ctx(struct ahash_reque=
st *req)=0A=
>>   				     desc_bytes(desc), 1);=0A=
>>   =0A=
>>   		ret =3D caam_jr_enqueue(jrdev, desc, ahash_done_ctx_dst, req);=0A=
>> -		if (ret)=0A=
>> +		if (ret !=3D -EINPROGRESS)=0A=
>>   			goto unmap_ctx;=0A=
>>   =0A=
>> -		ret =3D -EINPROGRESS;=0A=
>>   		state->update =3D ahash_update_ctx;=0A=
>>   		state->finup =3D ahash_finup_ctx;=0A=
>>   		state->final =3D ahash_final_ctx;=0A=
>> @@ -1353,9 +1344,7 @@ static int ahash_finup_no_ctx(struct ahash_request=
 *req)=0A=
>>   			     1);=0A=
>>   =0A=
>>   	ret =3D caam_jr_enqueue(jrdev, desc, ahash_done, req);=0A=
>> -	if (!ret) {=0A=
>> -		ret =3D -EINPROGRESS;=0A=
>> -	} else {=0A=
>> +	if (ret !=3D -EINPROGRESS) {=0A=
>>   		ahash_unmap_ctx(jrdev, edesc, req, digestsize, DMA_FROM_DEVICE);=0A=
>>   		kfree(edesc);=0A=
>>   	}=0A=
>> @@ -1452,10 +1441,9 @@ static int ahash_update_first(struct ahash_reques=
t *req)=0A=
>>   				     desc_bytes(desc), 1);=0A=
>>   =0A=
>>   		ret =3D caam_jr_enqueue(jrdev, desc, ahash_done_ctx_dst, req);=0A=
>> -		if (ret)=0A=
>> +		if (ret !=3D -EINPROGRESS)=0A=
>>   			goto unmap_ctx;=0A=
>>   =0A=
>> -		ret =3D -EINPROGRESS;=0A=
>>   		state->update =3D ahash_update_ctx;=0A=
>>   		state->finup =3D ahash_finup_ctx;=0A=
>>   		state->final =3D ahash_final_ctx;=0A=
>> diff --git a/drivers/crypto/caam/caampkc.c b/drivers/crypto/caam/caampkc=
.c=0A=
>> index ebf1677..7f7ea32 100644=0A=
>> --- a/drivers/crypto/caam/caampkc.c=0A=
>> +++ b/drivers/crypto/caam/caampkc.c=0A=
>> @@ -634,8 +634,8 @@ static int caam_rsa_enc(struct akcipher_request *req=
)=0A=
>>   	init_rsa_pub_desc(edesc->hw_desc, &edesc->pdb.pub);=0A=
>>   =0A=
>>   	ret =3D caam_jr_enqueue(jrdev, edesc->hw_desc, rsa_pub_done, req);=0A=
>> -	if (!ret)=0A=
>> -		return -EINPROGRESS;=0A=
>> +	if (ret =3D=3D -EINPROGRESS)=0A=
>> +		return ret;=0A=
>>   =0A=
>>   	rsa_pub_unmap(jrdev, edesc, req);=0A=
>>   =0A=
>> @@ -667,8 +667,8 @@ static int caam_rsa_dec_priv_f1(struct akcipher_requ=
est *req)=0A=
>>   	init_rsa_priv_f1_desc(edesc->hw_desc, &edesc->pdb.priv_f1);=0A=
>>   =0A=
>>   	ret =3D caam_jr_enqueue(jrdev, edesc->hw_desc, rsa_priv_f_done, req);=
=0A=
>> -	if (!ret)=0A=
>> -		return -EINPROGRESS;=0A=
>> +	if (ret =3D=3D -EINPROGRESS)=0A=
>> +		return ret;=0A=
>>   =0A=
>>   	rsa_priv_f1_unmap(jrdev, edesc, req);=0A=
>>   =0A=
>> @@ -700,8 +700,8 @@ static int caam_rsa_dec_priv_f2(struct akcipher_requ=
est *req)=0A=
>>   	init_rsa_priv_f2_desc(edesc->hw_desc, &edesc->pdb.priv_f2);=0A=
>>   =0A=
>>   	ret =3D caam_jr_enqueue(jrdev, edesc->hw_desc, rsa_priv_f_done, req);=
=0A=
>> -	if (!ret)=0A=
>> -		return -EINPROGRESS;=0A=
>> +	if (ret =3D=3D -EINPROGRESS)=0A=
>> +		return ret;=0A=
>>   =0A=
>>   	rsa_priv_f2_unmap(jrdev, edesc, req);=0A=
>>   =0A=
>> @@ -733,8 +733,8 @@ static int caam_rsa_dec_priv_f3(struct akcipher_requ=
est *req)=0A=
>>   	init_rsa_priv_f3_desc(edesc->hw_desc, &edesc->pdb.priv_f3);=0A=
>>   =0A=
>>   	ret =3D caam_jr_enqueue(jrdev, edesc->hw_desc, rsa_priv_f_done, req);=
=0A=
>> -	if (!ret)=0A=
>> -		return -EINPROGRESS;=0A=
>> +	if (ret =3D=3D -EINPROGRESS)=0A=
>> +		return ret;=0A=
>>   =0A=
>>   	rsa_priv_f3_unmap(jrdev, edesc, req);=0A=
>>   =0A=
>> diff --git a/drivers/crypto/caam/caamrng.c b/drivers/crypto/caam/caamrng=
.c=0A=
>> index e8baaca..e3e4bf2 100644=0A=
>> --- a/drivers/crypto/caam/caamrng.c=0A=
>> +++ b/drivers/crypto/caam/caamrng.c=0A=
>> @@ -133,7 +133,7 @@ static inline int submit_job(struct caam_rng_ctx *ct=
x, int to_current)=0A=
>>   	dev_dbg(jrdev, "submitting job %d\n", !(to_current ^ ctx->current_buf=
));=0A=
>>   	init_completion(&bd->filled);=0A=
>>   	err =3D caam_jr_enqueue(jrdev, desc, rng_done, ctx);=0A=
>> -	if (err)=0A=
>> +	if (err !=3D EINPROGRESS)=0A=
> =0A=
> Shouldn't this be -EINPROGRESS ?=0A=
> =0A=
Yes, it should be -EINPROGRESS.=0A=
I'm working on a v2 and will update this, also.=0A=
=0A=
Thanks,=0A=
Iulia=0A=
=0A=
>>   		complete(&bd->filled); /* don't wait on failed job*/=0A=
>>   	else=0A=
>>   		atomic_inc(&bd->empty); /* note if pending */=0A=
>> @@ -153,7 +153,7 @@ static int caam_read(struct hwrng *rng, void *data, =
size_t max, bool wait)=0A=
>>   		if (atomic_read(&bd->empty) =3D=3D BUF_EMPTY) {=0A=
>>   			err =3D submit_job(ctx, 1);=0A=
>>   			/* if can't submit job, can't even wait */=0A=
>> -			if (err)=0A=
>> +			if (err !=3D EINPROGRESS)=0A=
> =0A=
> And here the same?=0A=
> =0A=
> Regards,=0A=
> Bastian=0A=
> =0A=
>>   				return 0;=0A=
>>   		}=0A=
>>   		/* no immediate data, so exit if not waiting */=0A=
>> diff --git a/drivers/crypto/caam/jr.c b/drivers/crypto/caam/jr.c=0A=
>> index fc97cde..df2a050 100644=0A=
>> --- a/drivers/crypto/caam/jr.c=0A=
>> +++ b/drivers/crypto/caam/jr.c=0A=
>> @@ -324,8 +324,8 @@ void caam_jr_free(struct device *rdev)=0A=
>>   EXPORT_SYMBOL(caam_jr_free);=0A=
>>   =0A=
>>   /**=0A=
>> - * caam_jr_enqueue() - Enqueue a job descriptor head. Returns 0 if OK,=
=0A=
>> - * -EBUSY if the queue is full, -EIO if it cannot map the caller's=0A=
>> + * caam_jr_enqueue() - Enqueue a job descriptor head. Returns -EINPROGR=
ESS=0A=
>> + * if OK, -ENOSPC if the queue is full, -EIO if it cannot map the calle=
r's=0A=
>>    * descriptor.=0A=
>>    * @dev:  device of the job ring to be used. This device should have=
=0A=
>>    *        been assigned prior by caam_jr_register().=0A=
>> @@ -377,7 +377,7 @@ int caam_jr_enqueue(struct device *dev, u32 *desc,=
=0A=
>>   	    CIRC_SPACE(head, tail, JOBR_DEPTH) <=3D 0) {=0A=
>>   		spin_unlock_bh(&jrp->inplock);=0A=
>>   		dma_unmap_single(dev, desc_dma, desc_size, DMA_TO_DEVICE);=0A=
>> -		return -EBUSY;=0A=
>> +		return -ENOSPC;=0A=
>>   	}=0A=
>>   =0A=
>>   	head_entry =3D &jrp->entinfo[head];=0A=
>> @@ -414,7 +414,7 @@ int caam_jr_enqueue(struct device *dev, u32 *desc,=
=0A=
>>   =0A=
>>   	spin_unlock_bh(&jrp->inplock);=0A=
>>   =0A=
>> -	return 0;=0A=
>> +	return -EINPROGRESS;=0A=
>>   }=0A=
>>   EXPORT_SYMBOL(caam_jr_enqueue);=0A=
>>   =0A=
>> diff --git a/drivers/crypto/caam/key_gen.c b/drivers/crypto/caam/key_gen=
.c=0A=
>> index 5a851dd..b0e8a49 100644=0A=
>> --- a/drivers/crypto/caam/key_gen.c=0A=
>> +++ b/drivers/crypto/caam/key_gen.c=0A=
>> @@ -108,7 +108,7 @@ int gen_split_key(struct device *jrdev, u8 *key_out,=
=0A=
>>   	init_completion(&result.completion);=0A=
>>   =0A=
>>   	ret =3D caam_jr_enqueue(jrdev, desc, split_key_done, &result);=0A=
>> -	if (!ret) {=0A=
>> +	if (ret =3D=3D -EINPROGRESS) {=0A=
>>   		/* in progress */=0A=
>>   		wait_for_completion(&result.completion);=0A=
>>   		ret =3D result.err;=0A=
>>=0A=
> =0A=
> =0A=
=0A=