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

[Gonglei (Arei)]

> -----Original Message-----
> From: Farhan Ali [mailto:alifm@xxxxxxxxxxxxx]
> Sent: Wednesday, June 13, 2018 1:08 AM
> To: Gonglei (Arei) <arei.gonglei@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx;
> kvm@xxxxxxxxxxxxxxx
> Cc: mst@xxxxxxxxxx; qemu-devel@xxxxxxxxxx; longpeng
> <longpeng2@xxxxxxxxxx>; pasic@xxxxxxxxxxxxx; frankja@xxxxxxxxxxxxx;
> borntraeger@xxxxxxxxxx
> Subject: Re: [RFC v1 2/2] crypto/virtio-crypto: Register an algo only if it's
> supported
> 
> 
> 
> On 06/11/2018 04:48 AM, Gonglei (Arei) wrote:
> >
> >
> >> -----Original Message-----
> >> From: Farhan Ali [mailto:alifm@xxxxxxxxxxxxx]
> >> Sent: Saturday, June 09, 2018 3:09 AM
> >> To: linux-kernel@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx
> >> Cc: mst@xxxxxxxxxx; qemu-devel@xxxxxxxxxx; Gonglei (Arei)
> >> <arei.gonglei@xxxxxxxxxx>; longpeng <longpeng2@xxxxxxxxxx>;
> >> pasic@xxxxxxxxxxxxx; frankja@xxxxxxxxxxxxx; borntraeger@xxxxxxxxxx;
> >> alifm@xxxxxxxxxxxxx
> >> Subject: [RFC v1 2/2] crypto/virtio-crypto: Register an algo only if it's
> supported
> >>
> >> From: Farhan Ali <alifm@xxxxxxxxxxxxxxxxxx>
> >>
> >> 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@xxxxxxxxxxxxx>
> >> ---
> >>   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?
> 
Yes, good work. You can add my ack in v2:

Acked-by: Gonglei <arei.gonglei@xxxxxxxxxx>

Regards,
-Gonglei