Re: [RFC v1 2/2] crypto/virtio-crypto: Register an algo only if it's supported

From: Farhan Ali <alifm@linux.ibm.com>
To: "Gonglei (Arei)" <arei.gonglei@huawei.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>
Cc: "mst@redhat.com" <mst@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	longpeng <longpeng2@huawei.com>,
	"pasic@linux.ibm.com" <pasic@linux.ibm.com>,
	"frankja@linux.ibm.com" <frankja@linux.ibm.com>,
	"borntraeger@de.ibm.com" <borntraeger@de.ibm.com>
Subject: Re: [RFC v1 2/2] crypto/virtio-crypto: Register an algo only if it's supported
Date: Tue, 12 Jun 2018 13:07:41 -0400	[thread overview]
Message-ID: <7957a36b-6881-1967-fa60-eaeaa38256c0@linux.ibm.com> (raw)
In-Reply-To: <33183CC9F5247A488A2544077AF19020DB015ABF@dggeml511-mbx.china.huawei.com>

On 06/11/2018 04:48 AM, Gonglei (Arei) wrote:
> 
> 
>> -----Original Message-----
>> From: Farhan Ali [mailto:alifm@linux.ibm.com]
>> Sent: Saturday, June 09, 2018 3:09 AM
>> To: linux-kernel@vger.kernel.org; kvm@vger.kernel.org
>> Cc: mst@redhat.com; qemu-devel@nongnu.org; Gonglei (Arei)
>> <arei.gonglei@huawei.com>; longpeng <longpeng2@huawei.com>;
>> pasic@linux.ibm.com; frankja@linux.ibm.com; borntraeger@de.ibm.com;
>> alifm@linux.ibm.com
>> Subject: [RFC v1 2/2] crypto/virtio-crypto: Register an algo only if it's supported
>>
>> From: Farhan Ali <alifm@linux.vnet.ibm.com>
>>
>> Register a crypto algo with the Linux crypto layer only if
>> the algorithm is supported by the backend virtio-crypto
>> device.
>>
>> Also route crypto requests to a virtio-crypto
>> device, only if it can support the requested service and
>> algorithm.
>>
>> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
>> ---
>>   drivers/crypto/virtio/virtio_crypto_algs.c   | 110
>> ++++++++++++++++++---------
>>   drivers/crypto/virtio/virtio_crypto_common.h |  11 ++-
>>   drivers/crypto/virtio/virtio_crypto_mgr.c    |  81 ++++++++++++++++++--
>>   3 files changed, 158 insertions(+), 44 deletions(-)
>>
>> diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c
>> b/drivers/crypto/virtio/virtio_crypto_algs.c
>> index ba190cf..fef112a 100644
>> --- a/drivers/crypto/virtio/virtio_crypto_algs.c
>> +++ b/drivers/crypto/virtio/virtio_crypto_algs.c
>> @@ -49,12 +49,18 @@ struct virtio_crypto_sym_request {
>>   	bool encrypt;
>>   };
>>
>> +struct virtio_crypto_algo {
>> +	uint32_t algonum;
>> +	uint32_t service;
>> +	unsigned int active_devs;
>> +	struct crypto_alg algo;
>> +};
>> +
>>   /*
>>    * The algs_lock protects the below global virtio_crypto_active_devs
>>    * and crypto algorithms registion.
>>    */
>>   static DEFINE_MUTEX(algs_lock);
>> -static unsigned int virtio_crypto_active_devs;
>>   static void virtio_crypto_ablkcipher_finalize_req(
>>   	struct virtio_crypto_sym_request *vc_sym_req,
>>   	struct ablkcipher_request *req,
>> @@ -312,13 +318,19 @@ static int virtio_crypto_ablkcipher_setkey(struct
>> crypto_ablkcipher *tfm,
>>   					 unsigned int keylen)
>>   {
>>   	struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(tfm);
>> +	uint32_t alg;
>>   	int ret;
>>
>> +	ret = virtio_crypto_alg_validate_key(keylen, &alg);
>> +	if (ret)
>> +		return ret;
>> +
>>   	if (!ctx->vcrypto) {
>>   		/* New key */
>>   		int node = virtio_crypto_get_current_node();
>>   		struct virtio_crypto *vcrypto =
>> -				      virtcrypto_get_dev_node(node);
>> +				      virtcrypto_get_dev_node(node,
>> +				      VIRTIO_CRYPTO_SERVICE_CIPHER, alg);
>>   		if (!vcrypto) {
>>   			pr_err("virtio_crypto: Could not find a virtio device in the
>> system\n");
> 
> We'd better change the above error message now. What about:
>   " virtio_crypto: Could not find a virtio device in the system or unsupported algo" ?
> 
> Regards,
> -Gonglei

Sure, I will update the error message. But other than that does the rest 
of the code looks good to you?

Thanks
Farhan

> 
> 
> 
> 