[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v2] virtio-crypto specification
From: |
Gonglei (Arei) |
Subject: |
Re: [Qemu-devel] [RFC v2] virtio-crypto specification |
Date: |
Tue, 12 Apr 2016 09:03:33 +0000 |
Hi Cornelia,
Thank you so much for your review :)
> From: Cornelia Huck [mailto:address@hidden
> Sent: Tuesday, April 12, 2016 4:00 PM
> Subject: Re: [RFC v2] virtio-crypto specification
>
> On Tue, 5 Apr 2016 09:14:09 +0000
> "Gonglei (Arei)" <address@hidden> wrote:
>
>
> > Virtio-crypto device Spec
> >
> > 1 Crypto Device
> > The virtio crypto device is a virtual crypto device (ie. hardware crypto
> accelerator card). The encryption and decryption requests of are placed in the
> data queue, and handled by the real hardware crypto accelerators finally. A
> second queue is the control queue, which is used to create or destroy session
> for symmetric algorithms, and to control some advanced features.
> > 1.1 Device ID
> > 65535 (experimental)
>
> I think you should just go ahead and reserve a device ID for this.
>
OK, I want to reserve 20 as Crypto device's ID as 19 is reserved for Socket
device.
> >
> > 1.2 Virtqueues
> > 0 dataq
> > …
> > N-1 dataq
> > N controlq
> >
> > N=1 if VIRTIO_CRYPTO_F_MQ is not negotiated, otherwise N is set by
> max_virtqueues.
> >
> > 1.3 Feature bits
> > VIRTIO_CRYPTO_F_STATUS (0)
> > Configuration status field is available.
>
> I'm wondering if this really needs to be made optional?
>
I'm not sure. :(
> > VIRTIO_CRYPTO_F_MQ (1)
> > Device supports multiqueue to encrypt and decrypt.
>
> As commented by Stefan, just drop this.
>
Yep.
> > VIRTIO_CRYPTO_F_ALGS (2)
> > Configuration algorithms field is available.
>
> I'd also think that we always want this field, so this feature bit
> would be superfluous as well.
>
Yes, I agree. This feature is the same with VIRTIO_CRYPTO_F_MQ.
> >
> > 1.4 Device configuration layout
> > Three driver-read-only configuration fields are currently defined. The
> > status
> only exists if VIRTIO_CRYPTO_F_STATUS is set.
>
> I think the status field should just always be provided. It's nice if
> config layout doesn't change.
>
OK, I agree.
> > On read-only bit (for the driver) is currently defined for the status field:
> VIRTIO_CRYPTO_S_HW_READY.
>
> s/On/One/
>
Fixed.
> > #define VIRTIO_CRYPTO_S_HW_READY 1
>
> Do you have any plans for further status bits? E.g., do you want to be
> able to distinguish between hw not provided and hw in error?
>
TBH I haven't use other status bits, but I think we should identify the status
HW not provided.
As this situation of HW in error relies on the HW API, which is not necessary.
> >
> > The following driver-read-only field, max_virtqueues only exists if
> VIRTIO_CRYPTO_F_MQ is set. This field specifies the maximum number of data
> virtqueues (dataq1. . .dataqN) that can be configured once
> VIRTIO_CRYPTO_F_MQ is negotiated.
> > struct virtio_crypto_config {
> > le16 status;
> > le16 max_virtqueues;
> > le32 algorithms;
> > }
> >
> > The last driver-read-only field, algorithms only exists if
> VIRTIO_CRYPTO_F_ALGS is set. This field specifies the algorithms which the
> device offered once VIRTIO_CRYPTO_F_ALGS is negotiated. Two read-only bits
> (for the driver) are currently defined for the algorithms field:
> VIRTIO_CRYPTO_ALG_SYM and VIRTIO_CRYPTO_ALG_ASYM.
> > #define VIRTIO_CRYPTO_ALG_SYM (1 << 0)
> > #define VIRTIO_CRYPTO_ALG_SYM (1 << 1)
>
> I think the second one should be "ASYM".
>
Yes, it's a typo.
> Do you want to provide indications in this field beyond sym vs. asym,
> e.g. which strength is available?
>
Yes.
I focused on symmetric algorithms which have a relative
unified interface at present, but for asymmetric algorithms have different
interfaces,
such as DRBG/RSA/DH etc. I think this field should be extended since we add a
new
algorithm support.
> As said before, I don't think this should be negotiable. Just provide
> this information at all time.
>
Yes.
> > 1.4.1 Device Requirements: Device configuration layout
> > The device MUST set max_virtqueues to between 1 and 0x8000 inclusive, if it
> offers VIRTIO_CRYPTO_F_MQ.
>
> Where does "0x8000" come from?
>
It come from 5.1.4.1 (Network device). It seems that a wrong value, it should
be 65535=[(1 << 16) -1].
> > If the driver does not negotiate the VIRTIO_CRYPTO_F_STATUS feature, it
> SHOULD assume the hardware-backed implementation is ready, otherwise it
> SHOULD read the ready status from the bottom bit of status.
> > If the driver does not negotiate the VIRTIO_CRYPTO_F_ALGS feature, it
> SHOULD assume the
> > device support all algorithms.
>
> That would beg the question what "all algorithms" are :) If you want to
> be able to extend the list of available algorithms later on, this field
> needs to be mandatory.
>
OK.
> > 1.5 Device Initialization
> > A driver would perform a typical initialization routine like so:
> > 1. Identify and initialize data virtqueue, up to N if VIRTIO_CRYPTO_F_MQ
> feature bit is negotiated, N=max_virtqueues, otherwise identify N=1.
> > 2. Identify the control virtqueue.
> > 3. If the VIRTIO_CRYPTO_F_STATUS feature bit is negotiated, the ready
> status of hardware-backend comes from the bottom bit of status. Otherwise,
> the driver assumes it’s active.
> > 4. If the VIRTIO_CRYPTO_F_ALGS feature bit is negotiated, the driver can
> read the supported algorithms from bits of algorithms. Otherwise, the driver
> assumes all algorithms are supported.
> > 1.6 Device Operation
> > 1.6.1 Session operation
> > The symmetric algorithms have the concept of sessions. A session is a handle
> which describes the
> > cryptographic parameters to be applied to a number of buffers. The data
> within a session handle includes the following:
> > •1. The operation (cipher, hash or both, and if both, the order in which the
> algorithms should be applied).
>
> I think you want the driver to be able to discover what the device
> supports before it starts issuing requests.
>
Yes, you're right.
> > •2. The cipher setup data, including the cipher algorithm and mode, the key
> and its length, and the direction (encrypt or decrypt).
> > •3. The hash setup data, including the hash algorithm, mode (plain, nested
> > or
> authenticated), and digest result length (to allow for truncation).
> > Authenticated mode can refer to HMAC, which requires that the key and
> its length are also specified. It is also used for GCM and CCM authenticated
> encryption, in which case the AAD length is also specified.
> > For nested mode, the inner and outer prefix data and length are
> specified, as well as the outer hash algorithm.
>
> I'll defer looking at the actual interface...
>
> (...)
>
> > 1.6.2.1 Steps of encryption Operation
> > Both ctrlq and dataq virtqueue are bidirectional.
> > Step1: Create a session:
> > 1. The driver fill out the context message, include algorithm name, key,
> keylen etc;
> > 2. The driver send a context message to the backend device by controlq;
> > 3. The device create a session using the message transmitted by controlq;
> > 4. Return the session id to the driver.
> > Step 2: Execute the detail encryption operation:
> > 1. The driver fill out the encrypt requests;
> > 2. Put the requests into dataq and kick the virtqueue;
> > 3. The device execute the encryption operation according the requests’
> arguments;
> > 4. The device return the encryption result to the driver by dataq;
> > 5. The driver callback handle the result and over.
> >
> > Note: the driver CAN support both synchronous and asynchronous
> encryption.
>
> s/CAN/MAY/ ?
>
Ha-ha, maybe I should search the difference between CAN and MAY form the
dictionary ;)
> > Then the performance is poor in synchronous operation because frequent
> context switching and virtualization overhead.
>
> The difference would depend on the device implementation, I guess?
>
> > The driver SHOULD by preference use asynchronous encryption.
>
> So is the sync mode intended as a fallback?
>
Yes.
> > 1.6.3 Decryption Operation
> > The decryption process is the same with encryption, except that the device
> MUST verify and return the verify result to the driver. If the verify result
> is not
> correct, VIRTIO_CRYPTO_S_BADMSG (bad message) SHOULD be returned the
> driver.
>
> s/SHOULD/MUST/ ?
>
> I'd think it'd would be essential that the driver knows about a failed
> verification?
Yes, it's true.
Regards,
-Gonglei
- [Qemu-devel] [RFC v2] virtio-crypto specification, Gonglei (Arei), 2016/04/05
- Re: [Qemu-devel] [virtio-dev] [RFC v2] virtio-crypto specification, Stefan Hajnoczi, 2016/04/11
- Re: [Qemu-devel] [RFC v2] virtio-crypto specification, Cornelia Huck, 2016/04/12
- Re: [Qemu-devel] [RFC v2] virtio-crypto specification,
Gonglei (Arei) <=
- Re: [Qemu-devel] [RFC v2] virtio-crypto specification, Cornelia Huck, 2016/04/13
- Re: [Qemu-devel] [RFC v2] virtio-crypto specification, Gonglei (Arei), 2016/04/13
- Re: [Qemu-devel] [RFC v2] virtio-crypto specification, Cornelia Huck, 2016/04/14
- Re: [Qemu-devel] [RFC v2] virtio-crypto specification, Gonglei (Arei), 2016/04/14
- Re: [Qemu-devel] [RFC v2] virtio-crypto specification, Cornelia Huck, 2016/04/14
- Re: [Qemu-devel] [RFC v2] virtio-crypto specification, Michael S. Tsirkin, 2016/04/14
- Re: [Qemu-devel] [RFC v2] virtio-crypto specification, Gonglei (Arei), 2016/04/14
- Re: [Qemu-devel] [RFC v2] virtio-crypto specification, Stefan Hajnoczi, 2016/04/14
- Re: [Qemu-devel] [RFC v2] virtio-crypto specification, Gonglei (Arei), 2016/04/14
- Re: [Qemu-devel] [RFC v2] virtio-crypto specification, Cornelia Huck, 2016/04/14