Re: [Qemu-devel] [PATCH v7 1/1] crypto: add virtio-crypto driver

From: Halil Pasic
Date: Wed Dec 14 2016 - 10:39:47 EST




On 12/14/2016 12:50 PM, Gonglei wrote:
> diff --git a/drivers/crypto/virtio/virtio_crypto_core.c b/drivers/crypto/virtio/virtio_crypto_core.c
> new file mode 100644
> index 0000000..c0854a1
> --- /dev/null
> +++ b/drivers/crypto/virtio/virtio_crypto_core.c
> @@ -0,0 +1,474 @@
[..]
> +
> +static void virtcrypto_dataq_callback(struct virtqueue *vq)
> +{
> + struct virtio_crypto *vcrypto = vq->vdev->priv;
> + struct virtio_crypto_request *vc_req;
> + unsigned long flags;
> + unsigned int len;
> + struct ablkcipher_request *ablk_req;
> + int error;
> +
> + spin_lock_irqsave(&vcrypto->lock, flags);

Would it make sense to use a per virtqueue lock
like in virtio_blk for example instead of locking on the whole
device? OK, it seems you use only one dataqueue, so it
may not be that relevant.

> + do {
> + virtqueue_disable_cb(vq);
> + while ((vc_req = virtqueue_get_buf(vq, &len)) != NULL) {
> + if (vc_req->type == VIRTIO_CRYPTO_SYM_OP_CIPHER) {
> + switch (vc_req->status) {
> + case VIRTIO_CRYPTO_OK:
> + error = 0;
> + break;
> + case VIRTIO_CRYPTO_INVSESS:
> + case VIRTIO_CRYPTO_ERR:
> + error = -EINVAL;
> + break;
> + case VIRTIO_CRYPTO_BADMSG:
> + error = -EBADMSG;
> + break;
> + default:
> + error = -EIO;
> + break;
> + }
> + ablk_req = vc_req->ablkcipher_req;
> + virtcrypto_clear_request(vc_req);
> +
> + spin_unlock_irqrestore(&vcrypto->lock, flags);
> + /* Finish the encrypt or decrypt process */
> + ablk_req->base.complete(&ablk_req->base, error);
> + spin_lock_irqsave(&vcrypto->lock, flags);
> + }
> + }
> + } while (!virtqueue_enable_cb(vq));
> + spin_unlock_irqrestore(&vcrypto->lock, flags);
> +}