Re: [Qemu-devel] [RFC v2] virtio-crypto specification

[Cornelia Huck]

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.

> 
> 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?

> VIRTIO_CRYPTO_F_MQ (1)
>       Device supports multiqueue to encrypt and decrypt.

As commented by Stefan, just drop this.

> 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.

> 
> 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.

> On read-only bit (for the driver) is currently defined for the status field: 
> VIRTIO_CRYPTO_S_HW_READY.

s/On/One/

> #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?

> 
> 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".

Do you want to provide indications in this field beyond sym vs. asym,
e.g. which strength is available?

As said before, I don't think this should be negotiable. Just provide
this information at all time.

> 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?

> 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.

> 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.

> •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/ ?

> 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?

> 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?