Hi Halil, On 2017/9/15 23:59, Halil Pasic wrote:
> > > On 09/11/2017 03:12 AM, Longpeng(Mike) wrote: >> From: Gonglei <arei.gong...@huawei.com> >> >> The virtio crypto device is a virtual crypto device (ie. hardware >> crypto accelerator card). Currently, the virtio crypto device provides >> the following crypto services: CIPHER, MAC, HASH, and AEAD. >> >> In this patch, CIPHER, MAC, HASH, AEAD services are introduced. >> >> VIRTIO-153 >> >> Signed-off-by: Gonglei <arei.gong...@huawei.com> >> Signed-off-by: Longpeng(Mike) <longpe...@huawei.com> >> --- > [..] > > >> +\subsubsection{Control Virtqueue}\label{sec:Device Types / Crypto Device / >> Device Operation / Control Virtqueue} >> + >> +The driver uses the control virtqueue to send control commands to the >> +device, such as session operations (See \ref{sec:Device Types / Crypto >> Device / Device Operation / Control Virtqueue / Session operation}). >> + >> +The header for controlq is of the following form: >> +\begin{lstlisting} >> +#define VIRTIO_CRYPTO_OPCODE(service, op) (((service) << 8) | (op)) >> + >> +struct virtio_crypto_ctrl_header { >> +#define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION \ >> + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x02) >> +#define VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION \ >> + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x03) >> +#define VIRTIO_CRYPTO_HASH_CREATE_SESSION \ >> + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x02) >> +#define VIRTIO_CRYPTO_HASH_DESTROY_SESSION \ >> + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x03) >> +#define VIRTIO_CRYPTO_MAC_CREATE_SESSION \ >> + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x02) >> +#define VIRTIO_CRYPTO_MAC_DESTROY_SESSION \ >> + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x03) >> +#define VIRTIO_CRYPTO_AEAD_CREATE_SESSION \ >> + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02) >> +#define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \ >> + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03) >> + le32 opcode; >> + /* algo should be service-specific algorithms */ >> + le32 algo; >> + le32 flag; >> + /* data virtqueue id */ >> + le32 queue_id; >> +}; >> +\end{lstlisting} >> + >> +The format of the controlq request depends on the VIRTIO_CRYPTO_F_MUX_MODE >> feature bit: >> + >> +\begin{itemize*} >> +\item If the VIRTIO_CRYPTO_F_MUX_MODE feature bit is NOT negotiated the >> controlq request is >> + a fixed-size structure of form: >> +\begin{lstlisting} >> +struct virtio_crypto_op_ctrl_req { >> + struct virtio_crypto_ctrl_header header; >> + >> + union { >> + struct virtio_crypto_sym_create_session_req sym_create_session; >> + struct virtio_crypto_hash_create_session_req hash_create_session; >> + struct virtio_crypto_mac_create_session_req mac_create_session; >> + struct virtio_crypto_aead_create_session_req aead_create_session; >> + struct virtio_crypto_destroy_session_req destroy_session; >> + } u; >> +}; >> +\end{lstlisting} >> +The header is the general header, and the union is of the >> algorithm-specific type or the >> +virtio_crypto_destroy_session_req structure, which is set by the driver. >> All the properties >> +in the union are shown as follows. >> + >> +\item If the VIRTIO_CRYPTO_F_MUX_MODE feature bit is negotiated the >> controlq request is composed >> + of two parts, the additional paramenters are preceded by the general >> header. >> + >> +\begin{lstlisting} >> +struct virtio_crypto_op_ctrl_req_mux { >> + struct virtio_crypto_ctrl_header header; >> + >> + /* additional paramenter */ >> + u8 additional_para[addl_para_len]; >> +}; >> +\end{lstlisting} >> + >> +The additional paramenters are stored in a >> virtio_crypto_destroy_session_req structure or >> +in a algorithm-specific structure: >> +\begin{itemize*} >> +\item struct virtio_crypto_sym_create_session_req >> +\item struct virtio_crypto_hash_create_session_req >> +\item struct virtio_crypto_mac_create_session_req >> +\item struct virtio_crypto_aead_create_session_req >> +\end{itemize*} >> +All of the structures above are shown as follows. >> +\end{itemize*} >> + >> +\paragraph{Session operation}\label{sec:Device Types / Crypto Device / >> Device Operation / Control Virtqueue / Session operation} >> + >> +The session is a >> +handle which describes the cryptographic parameters to be applied to >> +a number of buffers. >> + >> +The following structure stores the result of session creation set by the >> device: >> + >> +\begin{lstlisting} >> +struct virtio_crypto_session_input { >> + /* Device-writable part */ >> + le64 session_id; >> + le32 status; >> + le32 padding; >> +}; >> +\end{lstlisting} >> + >> +A request to destroy a session includes the following information: >> + >> +\begin{lstlisting} >> +struct virtio_crypto_destroy_session_req { >> + /* Device-readable part */ >> + le64 session_id; >> + /* Device-writable part */ >> + le32 status; >> + le32 padding; >> +}; >> +\end{lstlisting} >> + >> +\subparagraph{Session operation: HASH session}\label{sec:Device Types / >> Crypto Device / Device >> +Operation / Control Virtqueue / Session operation / Session operation: HASH >> session} >> + >> +HASH session requests are as follows: >> + >> +\begin{lstlisting} >> +struct virtio_crypto_hash_session_para { >> + /* See VIRTIO_CRYPTO_HASH_* above */ >> + le32 algo; >> + /* hash result length */ >> + le32 hash_result_len; >> +}; >> +struct virtio_crypto_hash_create_session_req { >> + /* Device-readable part */ >> + struct virtio_crypto_hash_session_para para; >> + /* Device-writable part */ >> + struct virtio_crypto_session_input input; >> +}; >> +\end{lstlisting} >> + >> +The information required by HASH session creation is stored in the >> virtio_crypto_hash_create_session_req >> +structure, including the hash parameters stored in \field{para}. >> \field{input} stores the result of this operation. >> + >> +\subparagraph{Session operation: MAC session}\label{sec:Device Types / >> Crypto Device / Device >> +Operation / Control Virtqueue / Session operation / Session operation: MAC >> session} >> + >> +MAC session requests are as follows: >> + >> +\begin{lstlisting} >> +struct virtio_crypto_mac_session_para { >> + /* See VIRTIO_CRYPTO_MAC_* above */ >> + le32 algo; >> + /* hash result length */ >> + le32 hash_result_len; >> + /* length of authenticated key */ >> + le32 auth_key_len; >> + le32 padding; >> +}; >> + >> +struct virtio_crypto_mac_create_session_req { >> + /* Device-readable part */ >> + struct virtio_crypto_mac_session_para para; >> + /* The authenticated key */ >> + u8 auth_key[auth_key_len]; >> + >> + /* Device-writable part */ >> + struct virtio_crypto_session_input input; >> +}; >> +\end{lstlisting} >> + >> +The information required by MAC session creation is stored in the >> virtio_crypto_mac_create_session_req >> +structure, including the mac parameters stored in \field{para} and the >> authenticated key in \field{auth_key}. >> +\field{input} stores the result of this operation. >> + >> +\subparagraph{Session operation: Symmetric algorithms >> session}\label{sec:Device Types / Crypto Device / Device >> +Operation / Control Virtqueue / Session operation / Session operation: >> Symmetric algorithms session} >> + >> +The request of symmetric session includes two parts, CIPHER algorithms and >> chain >> +algorithms (chaining CIPHER and HASH/MAC). >> + >> +CIPHER session requests are as follows: >> + >> +\begin{lstlisting} >> +struct virtio_crypto_cipher_session_para { >> + /* See VIRTIO_CRYPTO_CIPHER* above */ >> + le32 algo; >> + /* length of key */ >> + le32 keylen; >> +#define VIRTIO_CRYPTO_OP_ENCRYPT 1 >> +#define VIRTIO_CRYPTO_OP_DECRYPT 2 >> + /* encryption or decryption */ >> + le32 op; >> + le32 padding; >> +}; >> + >> +struct virtio_crypto_cipher_session_req { >> + /* Device-readable part */ >> + struct virtio_crypto_cipher_session_para para; >> + /* The cipher key */ >> + u8 cipher_key[keylen]; >> + >> + /* Device-writable part */ >> + struct virtio_crypto_session_input input;that >> +}; >> +\end{lstlisting} >> + >> +Algorithm chaining requests are as follows: >> + >> +\begin{lstlisting} >> +struct virtio_crypto_alg_chain_session_para { >> +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER 1 >> +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH 2 >> + le32 alg_chain_order; >> +/* Plain hash */ >> +#define VIRTIO_CRYPTO_SYM_HASH_MODE_PLAIN 1 >> +/* Authenticated hash (mac) */ >> +#define VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH 2 >> +/* Nested hash */ >> +#define VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED 3 >> + le32 hash_mode; >> + struct virtio_crypto_cipher_session_para cipher_param; >> + union { >> + struct virtio_crypto_hash_session_para hash_param; >> + struct virtio_crypto_mac_session_para mac_param; >> + } u; >> + /* length of the additional authenticated data (AAD) in bytes */ >> + le32 aad_len; >> + le32 padding; >> +}; >> + >> +struct virtio_crypto_alg_chain_session_req { >> + /* Device-readable part */ >> + struct virtio_crypto_alg_chain_session_para para; >> + /* The cipher key */ >> + u8 cipher_key[keylen]; >> + /* The authenticated key */ >> + u8 auth_key[auth_key_len]; >> + >> + /* Device-writable part */ >> + struct virtio_crypto_session_input input; >> +}; >> +\end{lstlisting} >> + >> +Symmetric algorithm requests are as follows: >> + >> +\begin{lstlisting} >> +struct virtio_crypto_sym_create_session_req { >> + union { >> + struct virtio_crypto_cipher_session_req cipher; >> + struct virtio_crypto_alg_chain_session_req chain; >> + } u; >> + >> + /* Device-readable part */ >> + >> +/* No operation */ >> +#define VIRTIO_CRYPTO_SYM_OP_NONE 0 >> +/* Cipher only operation on the data */ >> +#define VIRTIO_CRYPTO_SYM_OP_CIPHER 1 >> +/* Chain any cipher with any hash or mac operation. The order >> + depends on the value of alg_chain_order param */ >> +#define VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING 2 >> + le32 op_type; >> + le32 padding; >> +}; >> +\end{lstlisting} >> + >> +The information required by symmetric algorithms session creation is stored >> in the >> +virtio_crypto_sym_create_session_req structure, including the symmetric >> operation >> +type in \field{op_type} and the cipher parameters stored in \field{cipher} >> or the >> +algorithm chaining paramenters in \field{chain}. >> + >> +The driver can set the \field{op_type} field in struct >> virtio_crypto_sym_create_session_req >> +as follows: VIRTIO_CRYPTO_SYM_OP_NONE: no operation; >> VIRTIO_CRYPTO_SYM_OP_CIPHER: Cipher only >> +operation on the data; VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING: Chain any >> cipher with any hash >> +or mac operation. >> + >> +\subparagraph{Session operation: AEAD session}\label{sec:Device Types / >> Crypto Device / Device >> +Operation / Control Virtqueue / Session operation / Session operation: AEAD >> session} >> + >> +AEAD session requests are as follows: >> + >> +\begin{lstlisting} >> +struct virtio_crypto_aead_session_para { >> + /* See VIRTIO_CRYPTO_AEAD_* above */ >> + le32 algo; >> + /* length of key */ >> + le32 key_len; >> + /* Authentication tag length */ >> + le32 tag_len; >> + /* The length of the additional authenticated data (AAD) in bytes */ >> + le32 aad_len; >> + /* encryption or decryption, See above VIRTIO_CRYPTO_OP_* */ >> + le32 op; >> + le32 padding; >> +}; >> + >> +struct virtio_crypto_aead_create_session_req { >> + /* Device-readable part */ >> + struct virtio_crypto_aead_session_para para; >> + u8 key[key_len]; >> + >> + /* Device-writeable part */ >> + struct virtio_crypto_session_input input; >> +}; >> +\end{lstlisting} >> + >> +The information required by AEAD session creation is stored in the >> virtio_crypto_aead_create_session_req >> +structure, including the aead parameters stored in \field{para} and the >> cipher key in \field{key}. >> +\field{input} stores the result of this operation. >> + >> +\drivernormative{\subparagraph}{Session operation: create session}{Device >> Types / Crypto Device / Device Operation / Control Virtqueue / Session >> operation / Session operation: create session} >> + >> +\begin{itemize*} >> +\item The driver MUST set the control general header and the corresponding >> algorithm-specific structure. >> + See \ref{sec:Device Types / Crypto Device / Device Operation / Control >> Virtqueue}. >> +\item The driver MUST set the \field{opcode} field based on service type: >> CIPHER, HASH, MAC, or AEAD. >> +\item The driver MUST set the \field{queue_id} field to show used dataq. > > I've failed to figure out the semantic behind queue_id. This could mean, > that sessions and session_id's are queue scoped. But then we should be > (IMHO) more explicit on this -- my guess is that session_id's aren't > dataqueue scoped. > >> +\end{itemize*} >> + >> +\devicenormative{\subparagraph}{Session operation: create session}{Device >> Types / Crypto Device / Device >> +Operation / Control Virtqueue / Session operation / Session operation: >> create session} >> + >> +\begin{itemize*} >> +\item The device MUST use the corresponding algorithm-specific structure >> according to the >> + \field{opcode} in the control general header. >> +\item The device MUST set the \field{status} field to one of the following >> values of enum >> + VIRTIO_CRYPTO_STATUS after finish a session creation: >> +\begin{itemize*} >> +\item VIRTIO_CRYPTO_OK if a session is created successfully. >> +\item VIRTIO_CRYPTO_NOTSUPP if the requested algorithm or operation is >> unsupported. >> +\item VIRTIO_CRYPTO_NOSPC if no free session ID (only when the >> VIRTIO_CRYPTO_F_MUX_MODE >> + feature bit is negotiated). >> +\item VIRTIO_CRYPTO_ERR if failure not mentioned above occurs. > > I guess an invalid queue_id would be among these. > >> +\end{itemize*} >> +\item The device MUST set the \field{session_id} field to a unique session >> identifieronly > s/identifieronly/identifier only >> + if the status is set to VIRTIO_CRYPTO_OK. >> +\end{itemize*} >> + >> +\drivernormative{\subparagraph}{Session operation: destroy session}{Device >> Types / Crypto Device / Device >> +Operation / Control Virtqueue / Session operation / Session operation: >> destroy session} >> + >> +\begin{itemize*} >> +\item The driver MUST set the \field{opcode} field based on service type: >> CIPHER, HASH, MAC, or AEAD. >> +\item The driver MUST set the \field{session_id} to a valid value assigned >> by the device >> + when the session was created. > > Destroy does not need to specify queue_id. That means session_id's aren't > queue scoped from namespace perspective. The question remains what is > queue_id good for, and whether a session type op request should be > rejected if the the session id originates from a session creation > request specifying a different dataqueue (not the dataqueue containing > the given request)? > Gonglei and you have reached consensus, so we will make it reserved and must be zero in the next version. >> +\end{itemize*} >> + >> +\devicenormative{\subparagraph}{Session operation: destroy session}{Device >> Types / Crypto Device / Device >> +Operation / Control Virtqueue / Session operation / Session operation: >> destroy session} >> + >> +\begin{itemize*} >> +\item The device MUST set the \field{status} field to one of the following >> values of enum VIRTIO_CRYPTO_STATUS. >> +\begin{itemize*} >> +\item VIRTIO_CRYPTO_OK if a session is created successfully. >> +\item VIRTIO_CRYPTO_ERR if any failure occurs. >> +\end{itemize*} >> +\end{itemize*} >> + >> +\subsubsection{Data Virtqueue}\label{sec:Device Types / Crypto Device / >> Device Operation / Data Virtqueue} >> > > [..] > > I've run through the rest. It some stuff seems very repetitive. I wonder if > we can do better. > > I also dislike this 5.7.7.5.3 Steps of Operation part. I also don't > understand why is symmetric special in this respect (HASH, MAC and > AEAD don't have a 'Steps of Operation' section. > I have a offline discussion with Gonglei about it, we prefer to drop the 5.7.7.5.3 Steps of Operation part. > I would like to try some things out with the reference implementation. > Depending on how that goes I may or may not end up providing a detailed > review for the rest before discussing what I've already addressed. > > Regards, > Halil > > > . > -- Regards, Longpeng(Mike)