From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41861) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aptFR-00066E-0j for qemu-devel@nongnu.org; Tue, 12 Apr 2016 04:00:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aptFN-0006n4-PF for qemu-devel@nongnu.org; Tue, 12 Apr 2016 04:00:32 -0400 Received: from e06smtp17.uk.ibm.com ([195.75.94.113]:57527) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aptFN-0006mr-Cv for qemu-devel@nongnu.org; Tue, 12 Apr 2016 04:00:29 -0400 Received: from localhost by e06smtp17.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Tue, 12 Apr 2016 09:00:26 +0100 Received: from b06cxnps4075.portsmouth.uk.ibm.com (d06relay12.portsmouth.uk.ibm.com [9.149.109.197]) by d06dlp02.portsmouth.uk.ibm.com (Postfix) with ESMTP id 0AAF2219007E for ; Tue, 12 Apr 2016 09:00:04 +0100 (BST) Received: from d06av07.portsmouth.uk.ibm.com (d06av07.portsmouth.uk.ibm.com [9.149.37.248]) by b06cxnps4075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id u3C80OsQ4522468 for ; Tue, 12 Apr 2016 08:00:25 GMT Received: from d06av07.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av07.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id u3C80MUv014101 for ; Tue, 12 Apr 2016 04:00:24 -0400 Date: Tue, 12 Apr 2016 10:00:17 +0200 From: Cornelia Huck Message-ID: <20160412100017.2c522f94.cornelia.huck@de.ibm.com> In-Reply-To: <33183CC9F5247A488A2544077AF19020B030EDAE@SZXEMA503-MBS.china.huawei.com> References: <33183CC9F5247A488A2544077AF19020B030EDAE@SZXEMA503-MBS.china.huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RFC v2] virtio-crypto specification List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Gonglei (Arei)" Cc: "virtio-dev@lists.oasis-open.org" , "qemu-devel@nongnu.org" , "mst@redhat.com" , "Huangpeng (Peter)" , "Hanweidong (Randy)" , "Zhoujian (jay, Euler)" , Jani Kokkonen , Mihai Claudiu Caraman , Lingli Deng , "'Karlsson, Magnus'" , "'Ola Liljedahl@arm.com'" , Crasta Denis-B22176 , Varun Sethi , =?UTF-8?B?J0ZyYW7Dp29pcy1GcsOp?= =?UTF-8?B?ZMOpcmlj?= Ozog' , "'Venkatesan, Venky'" , Catalin Vasile On Tue, 5 Apr 2016 09:14:09 +0000 "Gonglei (Arei)" wrote: > Virtio-crypto device Spec >=20 > 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 finall= y. 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. >=20 > 1.2 Virtqueues > 0 dataq > =E2=80=A6 > N-1 dataq > N controlq >=20 > N=3D1 if VIRTIO_CRYPTO_F_MQ is not negotiated, otherwise N is set by max_= virtqueues. >=20 > 1.3 Feature bits > VIRTIO_CRYPTO_F_STATUS (0)=09 > Configuration status field is available.=20 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. >=20 > 1.4 Device configuration layout > Three driver-read-only configuration fields are currently defined. The st= atus 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 fie= ld: 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? >=20 > The following driver-read-only field, max_virtqueues only exists if VIRTI= O_CRYPTO_F_MQ is set. This field specifies the maximum number of data virtq= ueues (dataq1. . .dataqN) that can be configured once VIRTIO_CRYPTO_F_MQ is= negotiated. > struct virtio_crypto_config { > le16 status; > le16 max_virtqueues; > le32 algorithms; > } >=20 > 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 drive= r) are currently defined for the algorithms field: VIRTIO_CRYPTO_ALG_SYM an= d 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 S= HOULD assume the hardware-backed implementation is ready, otherwise it SHOU= LD read the ready status from the bottom bit of status. > If the driver does not negotiate the VIRTIO_CRYPTO_F_ALGS feature, it SHO= ULD 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=3Dmax_virtqueues, otherwise identify N=3D1. > 2. Identify the control virtqueue. > 3. If the VIRTIO_CRYPTO_F_STATUS feature bit is negotiated, the ready sta= tus of hardware-backend comes from the bottom bit of status. Otherwise, the= driver assumes it=E2=80=99s 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 drive= r 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 han= dle which describes the > cryptographic parameters to be applied to a number of buffers. The data w= ithin a session handle includes the following: > =E2=80=A21. The operation (cipher, hash or both, and if both, the order i= n 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. > =E2=80=A22. The cipher setup data, including the cipher algorithm and mod= e, the key and its length, and the direction (encrypt or decrypt). > =E2=80=A23. The hash setup data, including the hash algorithm, mode (plai= n, nested or authenticated), and digest result length (to allow for truncat= ion). > =EF=81=AC =EF=80=AD 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 specifi= ed. > =EF=81=AC =EF=80=AD 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.=20 > 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=E2= =80=99 arguments; > 4. The device return the encryption result to the driver by dataq; > 5. The driver callback handle the result and over. >=20 > Note: the driver CAN support both synchronous and asynchronous encryption= .=20 s/CAN/MAY/ ? > Then the performance is poor in synchronous operation because frequent co= ntext 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 devic= e MUST verify and return the verify result to the driver. If the verify res= ult 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?