[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v4] virtio-crypto specification
From: |
Zeng, Xin |
Subject: |
Re: [Qemu-devel] [RFC v4] virtio-crypto specification |
Date: |
Fri, 22 Jul 2016 05:31:29 +0000 |
On Friday, July 22, 2016 10:53 AM Gonglei (Arei) wrote:
>
> Hi Xin,
>
> Thank you so much for your great comments.
> I agree with you almostly except some trivial detals.
> Please see my below replies.
>
> And I'll submit V5 next week, and you can finish the asym algos parts if you
> like.
> Let's co-work to finish the virtio-crypto spec, shall we?
>
That's great.
> Regards,
> -Gonglei
>
>
> > -----Original Message-----
> > From: Zeng, Xin [mailto:address@hidden
> > Sent: Friday, July 22, 2016 8:48 AM
> > To: Gonglei (Arei); address@hidden; qemu-
> address@hidden
> > Cc: Hanweidong (Randy); Stefan Hajnoczi; Cornelia Huck; address@hidden;
> > Lingli Deng; Jani Kokkonen; Luonengjun; Huangpeng (Peter); Zhoujian (jay,
> > Euler); chenshanxi 00222737; 'Ola address@hidden'; Varun Sethi
> > Subject: RE: [RFC v4] virtio-crypto specification
> >
> > On Sunday, June 26, 2016 5:35 PM, Gonglei (Arei) Wrote:
> > > Hi all,
> > >
> > > This is the specification (version 4) about a new virtio crypto device.
> > >
> >
> > In general, our comments around this proposal are listed below:
> > 1. Suggest to introduce crypto services into virtio crypto device. The
> services
> > currently defined are CIPHER, MAC, HASH, AEAD, KDF, ASYM, PRIMITIVE.
>
> Yes, I agree, whether DRBG/NDRBG are included in PRIMITIVE service or
> not?
> If not, we'd better add another separate service.
Yes, I think we can add these two into PRIMITIVE services.
>
> > 2. Suggest to define a unified crypto request format that is consisted of
> > general header + service specific request, Where 'general header' is for
> > all
> > crypto request, 'service specific request' is composed of
> > operation parameter + input data + output data in generally.
> > operation parameter is algorithm-specific parameters,
> > input data is the data should be operated ,
> > output data is the "operation result + result buffer".
> >
> It makes sense. Good.
>
> > #define VIRTIO_CRYPTO_OPCODE(service, op) (((service)<<8) | (op))
> > struct virtio_crypto_op_header {
> > #define VIRTIO_CRYPTO_CIPHER_ENCRYPT
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_CIPHER, 0x00)
> > #define VIRTIO_CRYPTO_CIPHER_DECRYPT
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_CIPHER, 0x01)
> > #define VIRTIO_CRYPTO_HASH
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_HASH, 0x00)
> > #define VIRTIO_CRYPTO_MAC
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_MAC, 0x00)
> > #define VIRTIO_CRYPTO_KDF
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_KDF, 0x00)
> > #define VIRTIO_CRYPTO_ASYM_KEY_GEN
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_ASYM, 0x00)
> > #define VIRTIO_CRYPTO_ASYM_KEY_EXCHG
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_ASYM, 0x01)
> > #define VIRTIO_CRYPTO_ASYM_SIGN
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_ASYM, 0x02)
> > #define VIRTIO_CRYPTO_ASYM_VERIFY
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_ASYM, 0x03)
> > #define VIRTIO_CRYPTO_ASYM_ENCRYPT
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_ASYM, 0x04)
> > #define VIRTIO_CRYPTO_ASYM_DECRYPT
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_ASYM, 0x05)
> > #define VIRTIO_CRYPTO_AEAD_ENCRYPT
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_AEAD, 0x00)
> > #define VIRTIO_CRYPTO_AEAD_DECRYPT
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_AEAD, 0x01)
> > #define VIRTIO_CRYPTO_PRIMITIVE
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_PRIMITIVE, 0x00)
> > u32 opcode;
> > u8 algo; /*service-specific algorithms*/
> > u8 flag; /*control flag*/
>
> We'd better add a U64 session_id property here for service-specific
> algorithms.
>
Can we put session_id into parameter filed inside service-specific request?
For ASYM service, it doesn't need session_id.
And for HASH service, it might not need a session_id as well.
> > };
> >
> > Take rsa_sign_request as example,
> > A rsa sign service specific request is defined as:
> > struct virtio_crypto_asym_rsa_sign_req{
> > struct virtio_crypto_rsa_sign_para parameter;
> > struct virtio_crypto_rsa_sign_input idata;
> > struct virtio_crypto_rsa_sign_output odata;
> > };
> >
> > A complete crypto service request is defined as:
> > struct virtio_crypto_op_data_req {
> > struct virtio_crypto_op_header header;
> > union {
> > struct virtio_crypto_asym_rsa_sign_req
> > rsa_sign_req;
> > /*other service request*/
> > }u;
> > };
> >
> I wanted to do this in fact. ;)
>
> > More detailed comments are embedded below:
> >
> > > Changes from v3:
> > > - Don't use enum is the spec but macros in specific structures. [Michael
> > > &
> > > Stefan]
> > > - Add two complete structures for session creation and closing, so that
> > > the spec is clear on how to lay out the request. [Stefan]
> > > - Definite the crypto operation request with assigned structure, in this
> way,
> > > each data request only occupies *one entry* of the Vring descriptor
> table,
> > > which *improves* the *throughput* of data transferring.
> > >
> > > Changes from v2:
> > > - Reserve virtio device ID 20 for crypto device. [Cornelia]
> > > - Drop all feature bits, those capabilities are offered by the device
> > > all the
> > > time. [Stefan & Cornelia]
> > > - Add a new section 1.4.2 for driver requirements. [Stefan]
> > > - Use definite type definition instead of enum type in some structure.
> > > [Stefan]
> > > - Add virtio_crypto_cipher_alg definition. [Stefan]
> > > - Add a "Device requirements" section as using MUST. [Stefan]
> > > - Some grammar nits fixes and typo fixes. [Stefan & Cornelia]
> > > - Add one VIRTIO_CRYPTO_S_STARTED status for the driver as the flag of
> > > virtio-crypto device started and can work now.
> > >
> > > Great thanks for Stefan and Cornelia!
> > >
> > > Changes from v1:
> > > - Drop the feature bit definition for each algorithm, and using config
> space
> > > instead [Cornelia]
> > > - Add multiqueue support and add corresponding feature bit
> > > - Update Encryption process and header definition
> > > - Add session operation process and add corresponding header
> description
> > > - Other better description in order to fit for virtio spec [Michael]
> > > - Some other trivial fixes.
> > >
> > > If you have any comments, please let me know, thanks :)
> > >
> > >
> > > Virtio-crypto device Spec
> > > Signed-off-by:
> > > Gonglei <address@hidden>
> > >
> > > 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.
> > > The second queue is the control queue, which is used to create or
> destroy
> > > session for symmetric algorithms, and to control some advanced features
> in
> > > the future.
> > > 1.1 Device ID
> > > 20
> > > 1.2 Virtqueues
> > > 0 dataq
> > > …
> > > N-1 dataq
> > > N controlq
> > > N is set by max_virtqueues (max_virtqueues >= 1).
> > > 1.3 Feature bits
> > > There are no feature bits (yet).
> > > 1.4 Device configuration layout
> > > Three driver-read-only configuration fields are currently defined. One
> read-
> > > only bit (for the device) is currently defined for the status field:
> > > VIRTIO_CRYPTO_S_HW_READY. One read-only bit (for the driver) is
> currently
> > > defined for the status field: VIRTIO_CRYPTO_S_STARTED.
> > > #define VIRTIO_CRYPTO_S_HW_READY (1 << 0) #define
> > > VIRTIO_CRYPTO_S_STARTED (1 << 1)
> > >
> > > The following driver-read-only field, max_virtqueues specifies the
> maximum
> > > number of data virtqueues (dataq1. . .dataqN) .
> > > struct virtio_crypto_config {
> > > le16 status;
> > > le16 max_virtqueues;
> > > le32 algorithms;
> > > };
> > >
> > > The last driver-read-only field, algorithms specifies the algorithms which
> the
> > > device offered. 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_ASYM (1 << 1)
> >
> > Suggest to change the virtio_crypto_config structure to following structure
> to
> > define detail algorithms that the device supports in device configuration
> field.
> > struct virtio_crypto_config {
> > le32 version;
> > le16 status;
> > le16 max_dataqueues;
> > #define VIRTIO_CRYPTO_S_CIPHER 0 /*cipher services*/
> > #define VIRTIO_CRYPTO_S_HASH 1 /*hash service*/
> > #define VIRTIO_CRYPTO_S_MAC 2 /*MAC(Message Authentication Codes)
> > service*/
> > #define VIRTIO_CRYPTO_S_AEAD 3 /* AEAD(Authenticated Encryption
> with
> > Associated Data) service*/
> > #define VIRTIO_CRYPTO_S_KDF 4 /*KDF(Key Derivation Functions)
> service*/
> > #define VIRTIO_CRYPTO_S_ASYM 5 /*asymmetric service*/
> > #define VIRTIO_CRYPTO_S_PRIMITIVE 6 /*Essential mathematics
> computing
> > service*/
>
> I'd like to use s/ VIRTIO_CRYPTO_S_/ VIRTIO_CRYPTO_SERIVCE_/g, avoiding
> to
> conflict with VIRTIO_CRYPTO_SATAUS_ which all virtio spec always used.
That's OK.
>
> > le32 crypto_servcies; /*overall services mask*/
> > /*detailed algorithms mask*/
> > le32 cipher_algo_l;
> > le32 cipher_algo_h;
> > le32 hash_algo;
> > le32 mac_algo_l;
> > le32 mac_algo_h;
> > le32 asym_algo;
> > le32 kdf_algo;
> > le32 aead_algo;
> > le32 primitive_algo;
> > };
> >
> > 15 bits are defined for cipher algorithms currently.
> > #define VIRTIO_CRYPTO_CIPHER_ARC4 0
> > #define VIRTIO_CRYPTO_CIPHER_AES_ECB 1
> > #define VIRTIO_CRYPTO_CIPHER_AES_CBC 2
> > #define VIRTIO_CRYPTO_CIPHER_AES_CTR 3
> > #define VIRTIO_CRYPTO_CIPHER_DES_ECB 6
> > #define VIRTIO_CRYPTO_CIPHER_DES_CBC 7
> > #define VIRTIO_CRYPTO_CIPHER_3DES_ECB 8
> > #define VIRTIO_CRYPTO_CIPHER_3DES_CBC 9
> > #define VIRTIO_CRYPTO_CIPHER_3DES_CTR 9
> > #define VIRTIO_CRYPTO_CIPHER_KASUMI_F8 10
> > #define VIRTIO_CRYPTO_CIPHER_SNOW3G_UEA2 11
> > #define VIRTIO_CRYPTO_CIPHER_AES_F8 12
> > #define VIRTIO_CRYPTO_CIPHER_AES_XTS 13
> > #define VIRTIO_CRYPTO_CIPHER_ZUC_EEA3 14
> >
> > 12 bits are defined for Hash algorithms currently.
> > #define VIRTIO_CRYPTO_HASH_MD5 0
> > #define VIRTIO_CRYPTO_HASH_SHA1 1
> > #define VIRTIO_CRYPTO_HASH_SHA_224 2
> > #define VIRTIO_CRYPTO_HASH_SHA_256 3
> > #define VIRTIO_CRYPTO_HASH_SHA_384 4
> > #define VIRTIO_CRYPTO_HASH_SHA_512 5
> > #define VIRTIO_CRYPTO_HASH_SHA3_224 6
> > #define VIRTIO_CRYPTO_HASH_SHA3_256 7
> > #define VIRTIO_CRYPTO_HASH_SHA3_384 8
> > #define VIRTIO_CRYPTO_HASH_SHA3_512 9
> > #define VIRTIO_CRYPTO_HASH_SHA3_SHAKE128 10
> > #define VIRTIO_CRYPTO_HASH_SHA3_SHAKE256 11
> >
> > 15 bits are defined for MAC algorithms currently
> > #define VIRTIO_CRYPTO_MAC_HMAC_MD5 0
> > #define VIRTIO_CRYPTO_MAC_HMAC_SHA1 1
> > #define VIRTIO_CRYPTO_MAC_HMAC_SHA_224 2
> > #define VIRTIO_CRYPTO_MAC_HMAC_SHA_256 3
> > #define VIRTIO_CRYPTO_MAC_HMAC_SHA_384 4
> > #define VIRTIO_CRYPTO_MAC_HMAC_SHA_512 5
> > #define VIRTIO_CRYPTO_MAC_CMAC_3DES 24
>
> Why not 6 here? I'd like to keep increasing steadily, we can extend
> the value when some other algorithms are supported.
The intension is to keep the same kind of algorithms into continuous bits
as possible as.
>
> > #define VIRTIO_CRYPTO_MAC_CMAC_AES 25
> > #define VIRTIO_CRYPTO_MAC_CMAC_KASUMI_F9 26
> > #define VIRTIO_CRYPTO_MAC_CMAC_ SNOW3G_UIA2 27
> > #define VIRTIO_CRYPTO_MAC_GMAC_AES 40
> > #define VIRTIO_CRYPTO_MAC_GMAC_TWOFISH 41
> > #define VIRTIO_CRYPTO_MAC_CBCMAC_AES 48
>
> Dose this duplicate with VIRTIO_CRYPTO_MAC_CMAC_AES ?
CMAC_AES is different with CBCMAC_AES, there is a history
about these.
>
> > #define VIRTIO_CRYPTO_MAC_CBCMAC_KASUMI_F9 49
>
> Duplicates with VIRTIO_CRYPTO_MAC_CMAC_KASUMI_F9 ?
CMAC_KASUMI_F9 is also different with CBCMAC_KASUMI_F9.
>
> > #define VIRTIO_CRYPTO_MAC_XCBC_AES 52
> >
> > 5 bits are defined for asymmetric algorithms currently.
> > #define VIRTIO_CRYPTO_ASYM_RSA 0
> > #define VIRTIO_CRYPTO_ASYM_DSA 1
> > #define VIRTIO_CRYPTO_ASYM_DH 2
> > #define VIRTIO_CRYPTO_ASYM_ECDSA 3
> > #define VIRTIO_CRYPTO_ASYM_ECDH 4
> >
> > 7 bits are defined for KDF algorithms currently.
> > #define VIRTIO_CRYPTO_KDF_NONE 0
> > #define VIRTIO_CRYPTO_KDF_ANSI-X9.42 1
> > #define VIRTIO_CRYPTO_KDF_ANSI-X9.62 2
> > #define VIRTIO_CRYPTO_KDF_IKEv2 3
> > #define VIRTIO_CRYPTO_KDF_TLS_1.0 4
> > #define VIRTIO_CRYPTO_KDF_TLS_1.2 5
> > #define VIRTIO_CRYPTO_KDF_NIST-800-56-Concatenation 6
> >
> > 2 bits are defined for AEAD algorithms currently.
> > #define VIRTIO_CRYPTO_AEAD_GCM 0
> > #define VIRTIO_CRYPTO_AEAD_CCM 1
> >
> > 4 bits are defined for PRIMITIVE algorithms currently
> > #define VIRTIO_CRYPTO_PRIMITIVE_LNMOD_EXP 0
> > #define VIRTIO_CRYPTO_PRIMITIVE_LNMOD_INV 1
> > #define VIRTIO_CRYPTO_PRIMITIVE_ ECPOINT_MULTIPLY 2
> > #define VIRTIO_CRYPTO_PRIMITIVE_ ECPOINT_VERIFY 3
> >
> > > 1.4.1 Device Requirements: Device configuration layout
> >
> > Add "The device MUST set the version in version filed."
> >
> For compatibility? Okay.
>
> > > The device MUST set max_virtqueues to between 1 and 65535 inclusive.
> > >
> > > The device SHOULD set status according to the status of the hardware-
> > > backed implementation.
> > >
> > > The device MUST set algorithms according to the algorithms which the
> device
> > > offered.
> > > 1.4.2 Driver Requirements: Device configuration layout
> > > The driver MUST read the ready status from the bottom bit of status to
> check
> > > whether the hardware-backed implementation is ready or not.
> >
> > Add "The driver MAY read max_dataqueues field to discover how many
> data
> > queues
> > the device supports."
> >
> OK.
>
> > > The driver MUST read the algorithms to discover the algorithms which the
> > > device supports.
> >
> > Because of the presence of overall service field and detail algorithms
> > field,
> > suggest
> > To change this to "
> > The driver MUST read crypto_services field to discover which services the
> > device is
> > able to offer.
> > The driver MUST read detail algorithms field according to crypto_services
> field.
> > "
> >
> Yes, makes sense.
>
> > > 1.5 Device Initialization
> > > A driver would perform a typical initialization routine like so:
> > > 1. Identify and initialize data virtqueue, up to max_virtqueues.
> > > 2. Identify the control virtqueue.
> > > 3. Identify the ready status of hardware-backend comes from the bottom
> bit
> > > of status.
> > > 4. Read the supported algorithms from bits of algorithms.
> > > 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).
> > > •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.
> > >
> > > The controlq virtqueue is used to control session operations, including
> > > creation or close. The request is preceded by a header:
> > > struct virtio_crypto_sym_ctlhdr {
> > > /* control type */
> > > u8 type;
> > > };
> > > Two bits are currently defined for the control header type:
> > > #define VIRTIO_CRYPTO_CTRL_CREATE_SESSION 1 #define
> > > VIRTIO_CRYPTO_CTRL_CLOSE_SESSION 2
> > >
> >
> > Suggest to change virtio_crypto_sym_ctlhdr structure to the a general
> > header below, and define a unified control request structure to keep
> > consistent with crypto service request format.
> > struct virtio_crypto_ctrl_header{
> > #define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_CIPHER, 0x02)
> > #define VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_CIPHER, 0x03)
> > #define VIRTIO_CRYPTO_MAC_CREATE_SESSION
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_MAC, 0x02)
> > #define VIRTIO_CRYPTO_MAC_DESTROY_SESSION
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_S_MAC, 0x03)
> > u32 opcode;
> > u8 algo;
> > u8 flag;
> > };
> >
> We MUST add a queue_id property in header, which shows which data
> virtqueue
> we will used for multiqueue support.
>
Ok.
> > > 1.6.1.1 Session creation operation
> > > A request of creating a session including the following information:
> > > struct virtio_crypto_sym_session_creation {
> > > struct virtio_crypto_sym_ctlhdr ctlhdr; /* OUT */
> > > struct virtio_crypto_sym_session_op session_op; /* OUT */
> > > struct virtio_crypto_sym_session_op_inhdr inhdr; /* IN */ }; The
> > > details of specific structure, including struct
> virtio_crypto_sym_session_op
> > > and struct virtio_crypto_sym_session_op_inhdr are defined by the
> following
> > > sections.
> > > 1.6.1.1.1 Driver Requirements: Session creation operation The driver
> MUST
> > > set the control type with VIRTIO_CRYPTO_CTRL_CREATE_SESSION before
> > > the request is preceded by an operation header when executing session
> > > creation:
> > > typedef struct virtio_crypto_sym_session_op { /**< No operation */
> > > #define VIRTIO_CRYPTO_SYM_OP_NONE 0
> > > /**< Cipher only operation on the data */
> > > #define VIRTIO_CRYPTO_SYM_OP_CIPHER 1
> > > /**< Hash only operation on the data */
> > > #define VIRTIO_CRYPTO_SYM_OP_HASH 2
> > > /**< Chain any cipher with any hash operation */
> > > #define VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING 3
> > > u8 op_type; /* Operation type */
> > > virtio_crypto_sym_cipher_t cipher_setup_data;
> > > virtio_crypto_sym_hash_t hash_setup_data;
> > > /* Perform the hash operation followed by the cipher operation */
> > > #define
> VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER
> > > 1
> > > /* Perform the cipher operation followed by the hash operation */
> > > #define
> VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH
> > > 2
> > > u8 alg_chain_order;
> > > } virtio_crypto_sym_session_op_t;
> > > And the structures definition details are:
> > > typedef struct virtio_crypto_sym_hash_auth_mode {
> > > /* length of authenticated key */
> > > le32 auth_key_len;
> > > /* The length of the additional authenticated data (AAD) in bytes */
> > > le32 aad_len;
> > > } virtio_crypto_sym_hash_auth_mode_t;
> > >
> > > typedef struct virtio_crypto_sym_cipher {
> > > /* Option to not do any cryptography */
> > > #define VIRTIO_CRYPTO_NO_CIPHER 0
> > > #define VIRTIO_CRYPTO_CIPHER_DES 1
> > > #define VIRTIO_CRYPTO_CIPHER_DES_CBC 2
> > > #define VIRTIO_CRYPTO_CIPHER_DES3 3
> > > #define VIRTIO_CRYPTO_CIPHER_DES3_CBC 4
> > > #define VIRTIO_CRYPTO_CIPHER_AES 5
> > > #define VIRTIO_CRYPTO_CIPHER_AES_CBC 6
> > > #define VIRTIO_CRYPTO_CIPHER_KASUMI_F8 7
> > > le32 alg; /* cipher algorithm type (ie. aes-cbc ) */
> > > /* length of key */
> > > le32 keylen;
> > > #define VIRTIO_CRYPTO_DECRYPT 0
> > > #define VIRTIO_CRYPTO_ENCRYPT 1
> > > u8 op; /* encrypt or decrypt */
> > > } virtio_crypto_sym_cipher_t;
> > >
> > > typedef struct virtio_crypto_sym_hash {
> > > /* Option to not do any hash */
> > > #define VIRTIO_CRYPTO_NO_HASH 0
> > > #define VIRTIO_CRYPTO_HASH_MD5 1
> > > #define VIRTIO_CRYPTO_HASH_SHA1 2
> > > #define VIRTIO_CRYPTO_HASH_SHA1_96 3
> > > #define VIRTIO_CRYPTO_HASH_SHA224 4
> > > #define VIRTIO_CRYPTO_HASH_SHA256 5
> > > #define VIRTIO_CRYPTO_HASH_SHA384 6
> > > #define VIRTIO_CRYPTO_HASH_SHA512 7
> > > #define VIRTIO_CRYPTO_HASH_AES_XCBC 8
> > > #define VIRTIO_CRYPTO_HASH_AES_XCBC_96 9
> > > #define VIRTIO_CRYPTO_HASH_KASUMI_F9 10
> > > le32 hash_alg; /* hash algorithm type */
> > > #define VIRTIO_CRYPTO_SYM_HASH_MODE_PLAIN 1
> > > #define VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH 2
> > > #define VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED 3
> > > u8 hash_mode; /* mode of hash operation, including
> > > authenticated/plain/nested hash */
> > > /* hash result length */
> > > le32 hash_result_len;
> > > virtio_crypto_sym_hash_auth_mode_t auth_mode_setup_data; }
> > > virtio_crypto_sym_hash_t; The driver MUST set the control type with
> > > VIRTIO_CRYPTO_CTRL_CLOSE_SESSION and pass the session_id to the
> > > device when executing session close.
> >
> > This structure mixed HASH and MAC, suggest to define separated
> > services structure, cipher service structure can reference that.
> >
> OK, and we can still name it 'sym', include plain cipher and chain-algorithms.
>
> > > 1.6.1.1.2 Device Requirements: Session creation operation The device
> MUST
> > > return a session identifier to the driver when the device finishes the
> > > processing of session close. The session creation request MUST end by a
> > > tailer:
> > > typedef struct virtio_crypto_sym_session_op_inhdr {
> > > u8 status;
> > > le64 session_id;
> > > } virtio_crypto_sym_session_op_inhdr_t;
> > > Both status and session_id are written by the device: either
> > > VIRTIO_CRYPTO_CTRL_OK for success, VIRTIO_CRYPTO_CTRL_ERR for
> > > creation failed or device error.
> > > #define VIRTIO_CRYPTO_CTRL_OK 0
> > > #define VIRTIO_CRYPTO_CTRL_ERR 1
> > > 1.6.1.2 Session closing operation
> > > A request of closing a session including the following information:
> > > struct virtio_crypto_sym_session_creation {
> > > struct virtio_crypto_sym_ctlhdr ctlhdr; /* OUT */
> > > le64 session_id; /* OUT */
> > > u8 status; /* IN */
> > > };
> > > 1.6.1.2.1 Driver Requirements: Session closing operation The driver MUST
> set
> > > the control type with VIRTIO_CRYPTO_CTRL_CLOSE_SESSION, and the
> > > session_id MUST be a valid value which assigned by the device when a
> > > session was created.
> > > 1.6.1.2.2 Device Requirements: Session closing operation Status is written
> by
> > > the device: either VIRTIO_CRYPTO_CTRL_OK for success,
> > > VIRTIO_CRYPTO_CTRL_ERR for creation failed or device error.
> > > #define VIRTIO_CRYPTO_CTRL_OK 0
> > > #define VIRTIO_CRYPTO_CTRL_ERR 1
> >
> > Need two sets of error code be defined(see 1.6.2.2 )? The unified error
> code for
> > all
> > virtio crypto device should be better. We suggest the error codes below for
> > virtio
> > crypto devices:
> > #define VIRTIO_CRYPTO_OP_OK 0
> > #define VIRTIO_CRYPTO_OP_ERR 1
> > #define VIRTIO_CRYPTO_OP_BADMSG 2
> > #define VIRTIO_CRYPTO_OP_NOTSUPP 3
> >
> Makes sense.
>
> > > 1.6.2 Encryption operation
> > > 1.6.2.1 Driver Requirements: Encryption operation The encryption and
> > > decryption requests and the corresponding results are transmitted by
> placing
> > > them in dataq. The symmetric algorithms requests are preceded by a
> header:
> > > struct virtio_crypto_sym_op_hdr {
> > > /* the backend returned session identifier */
> > > le64 session_id;
> > > /* length of initial vector */
> > > le32 iv_len;
> > > /* iv offset in the whole crypto data memory */
> > > le32 iv_offset;
> > > /* length of additional auth data */
> > > le32 auth_len;
> > > /* additional auth data offset in the whole crypto data memory */
> > > le32 additional_auth_offset;
> > > /* cipher start source offest */
> > > le32 cipher_start_src_offset;
> > > le32 len_to_cipher;
> > > /* hash start source offest */
> > > le32 hash_start_src_offset;
> > > le32 len_to_hash;
> > > /* length of source data */
> > > le32 source_len;
> > > } ;
> > > The encryption request MUST end by a tailer:
> > > typedef struct virtio_crypto_sym_op_inhdr {
> > > u8 status;
> > > } virtio_crypto_sym _op_inhdr_t;
> > > The specific content of symmetric algorithms requests SHOULD be same
> as
> > > below:
> > > struct virtio_crypto_sym_op_data {
> > > struct virtio_crypto_sym_op_hdr hdr_info;
> > > le64 iv_addr; /* iv guest address */
> > > le64 auth_data_addr; /* associated data guest address */
> > > le64 src_data_addr; /* source data guest address */
> > > le64 dst_data_addr; /* destination data guest address */
> > > le64 digest_result_addr; /* digest result guest address */
> > > le64 inhdr_addr; /* in-header guest address */ }; In this way, each
> > > data request only occupies one entry of the Vring descriptor table, which
> > > improves the throughput of data transferring.
> >
> > Suggest to change virtio_crypto_sym_op_data to unified format listed
> above.
> > All these can also be fitted into one vring descriptor.
> >
> Sure.
>
> > > 1.6.2.2 Device Requirements: Encryption operation The struct
> > > virtio_crypto_sym_op_inhdr’s status byte is written by the device: either
> > > VIRTIO_CRYPTO_S_OK for success, VIRTIO_CRYPTO_S_ERR for device or
> > > driver error, VIRTIO_CRYPTO_S_BADMSG for verification failed when
> > > decrypt AEAD algorithms:
> > > #define VIRTIO_CRYPTO_S_OK 0
> > > #define VIRTIO_CRYPTO_S_ERR 1
> > > #define VIRTIO_CRYPTO_S_BADMSG 2
> >
> > Same comments for error code above.
> >
> > >
> > > 1.6.2.3 Steps of encryption Operation
> > > Step1: Create a session:
> > > 1. The driver fills out the context message, including algorithm
> > > name,
> > > key, keylen etc;
> > > 2. The driver sends a context message to the backend device by
> > > controlq;
> > > 3. The device creates 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 fills out the encrypt requests;
> > > 2. Put the requests into dataq and kick the virtqueue;
> > > 3. The device executes the encryption operation according the
> > > requests' arguments;
> > > 4. The device returns the encryption result to the driver by dataq;
> > > 5. The driver callback handle the result and over.
> > >
> > > Note: the driver MAY support both synchronous and asynchronous
> > > encryption. Then the performance is poor in synchronous operation
> because
> > > frequent context switching and virtualization overhead. The driver
> SHOULD
> > > by preference use asynchronous encryption.
> > > 1.6.3 Decryption Operation
> > > The decryption process is the same with encryption.
> > > 1.6.3.1 Device Requirements: Decryption operation 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) MUST be returned the
> driver.
> > >
> > > Regards,
> > > -Gonglei
> > >
> > >
> > >