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:xin.z...@intel.com] > > Sent: Friday, July 22, 2016 8:48 AM > > To: Gonglei (Arei); virtio-...@lists.oasis-open.org; qemu- > de...@nongnu.org > > Cc: Hanweidong (Randy); Stefan Hajnoczi; Cornelia Huck; m...@redhat.com; > > Lingli Deng; Jani Kokkonen; Luonengjun; Huangpeng (Peter); Zhoujian (jay, > > Euler); chenshanxi 00222737; 'Ola liljed...@arm.com'; 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 <arei.gong...@huawei.com> > > > > > > 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 > > > > > > > > >