RE: [virtio-dev] Re: [Qemu-devel] [PATCH v7 1/1] crypto: add virtio-crypto driver
Hi Michael, > > > > > > > > Subject: RE: [Qemu-devel] [PATCH v7 1/1] crypto: add virtio-crypto driver > > > > > > On Thursday, December 15, 2016 8:45 AM, Gonglei (Arei) Wrote: > > > < > > diff --git a/drivers/crypto/virtio/virtio_crypto_core.c > > > < > b/drivers/crypto/virtio/virtio_crypto_core.c > > > < > > new file mode 100644 > > > < > > index 000..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. > > > < > > > > < Currently yes, both the backend device (cryptodev-backend-builtin) > > > < and the frontend driver use one dataqueue. > > > < > > > > > > I think it makes sense to use per virtqueue lock here though it only uses > > > one > > > queue so far, > > > but in the spec we already have multi queues support. > > > > > Yes, I agree. Will do that in V8 soon. > > Hope to catch up with Michael's pull request for 4.10. > > > > Regards, > > -Gonglei > > I merged v7, this change will have to wait. Sorry. > That's OK. Thanks! I can post a separate patch after this pull request. Regards, -Gonglei -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v7 1/1] crypto: add virtio-crypto driver
On Thu, Dec 15, 2016 at 01:08:51AM +, Gonglei (Arei) wrote: > > > > > Regards, > -Gonglei > > > > -Original Message- > > From: Zeng, Xin [mailto:xin.z...@intel.com] > > Sent: Thursday, December 15, 2016 8:59 AM > > To: Gonglei (Arei); Halil Pasic; linux-ker...@vger.kernel.org; > > qemu-de...@nongnu.org; virtio-...@lists.oasis-open.org; > > virtualizat...@lists.linux-foundation.org; linux-crypto@vger.kernel.org > > Cc: Huangweidong (C); Claudio Fontana; m...@redhat.com; Luonengjun; > > Hanweidong (Randy); Xuquan (Quan Xu); Wanzongshun (Vincent); > > stefa...@redhat.com; Zhoujian (jay, Euler); cornelia.h...@de.ibm.com; > > longpeng; arei.gong...@hotmail.com; da...@davemloft.net; Wubin (H); > > herb...@gondor.apana.org.au > > Subject: RE: [Qemu-devel] [PATCH v7 1/1] crypto: add virtio-crypto driver > > > > On Thursday, December 15, 2016 8:45 AM, Gonglei (Arei) Wrote: > > < > > diff --git a/drivers/crypto/virtio/virtio_crypto_core.c > > < > b/drivers/crypto/virtio/virtio_crypto_core.c > > < > > new file mode 100644 > > < > > index 000..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. > > < > > > < Currently yes, both the backend device (cryptodev-backend-builtin) > > < and the frontend driver use one dataqueue. > > < > > > > I think it makes sense to use per virtqueue lock here though it only uses > > one > > queue so far, > > but in the spec we already have multi queues support. > > > Yes, I agree. Will do that in V8 soon. > Hope to catch up with Michael's pull request for 4.10. > > Regards, > -Gonglei I merged v7, this change will have to wait. Sorry. -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Qemu-devel] [PATCH v7 1/1] crypto: add virtio-crypto driver
Regards, -Gonglei > -Original Message- > From: Zeng, Xin [mailto:xin.z...@intel.com] > Sent: Thursday, December 15, 2016 8:59 AM > To: Gonglei (Arei); Halil Pasic; linux-ker...@vger.kernel.org; > qemu-de...@nongnu.org; virtio-...@lists.oasis-open.org; > virtualizat...@lists.linux-foundation.org; linux-crypto@vger.kernel.org > Cc: Huangweidong (C); Claudio Fontana; m...@redhat.com; Luonengjun; > Hanweidong (Randy); Xuquan (Quan Xu); Wanzongshun (Vincent); > stefa...@redhat.com; Zhoujian (jay, Euler); cornelia.h...@de.ibm.com; > longpeng; arei.gong...@hotmail.com; da...@davemloft.net; Wubin (H); > herb...@gondor.apana.org.au > Subject: RE: [Qemu-devel] [PATCH v7 1/1] crypto: add virtio-crypto driver > > On Thursday, December 15, 2016 8:45 AM, Gonglei (Arei) Wrote: > < > > diff --git a/drivers/crypto/virtio/virtio_crypto_core.c > < > b/drivers/crypto/virtio/virtio_crypto_core.c > < > > new file mode 100644 > < > > index 000..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. > < > > < Currently yes, both the backend device (cryptodev-backend-builtin) > < and the frontend driver use one dataqueue. > < > > I think it makes sense to use per virtqueue lock here though it only uses one > queue so far, > but in the spec we already have multi queues support. > Yes, I agree. Will do that in V8 soon. Hope to catch up with Michael's pull request for 4.10. Regards, -Gonglei -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Qemu-devel] [PATCH v7 1/1] crypto: add virtio-crypto driver
On Thursday, December 15, 2016 8:45 AM, Gonglei (Arei) Wrote: < > > diff --git a/drivers/crypto/virtio/virtio_crypto_core.c < > b/drivers/crypto/virtio/virtio_crypto_core.c < > > new file mode 100644 < > > index 000..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. < > < Currently yes, both the backend device (cryptodev-backend-builtin) < and the frontend driver use one dataqueue. < I think it makes sense to use per virtqueue lock here though it only uses one queue so far, but in the spec we already have multi queues support. < Regards, < -Gonglei -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [Qemu-devel] [PATCH v7 1/1] crypto: add virtio-crypto driver
> > > 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 000..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. > Currently yes, both the backend device (cryptodev-backend-builtin) and the frontend driver use one dataqueue. Regards, -Gonglei -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Qemu-devel] [PATCH v7 1/1] crypto: add virtio-crypto driver
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 000..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); > +} -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html