[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto device
From: |
Michael S. Tsirkin |
Subject: |
Re: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto device specification |
Date: |
Thu, 10 Nov 2016 15:15:11 +0200 |
On Thu, Nov 10, 2016 at 09:37:40AM +0000, Gonglei (Arei) wrote:
> Hi,
>
> I attach a diff for next version in order to review more convenient, with
>
> - Drop the all gap stuff;
> - Drop all structures undefined in virtio_crypto.h
> - re-describe per request for per crypto service avoid confusion
>
> Pls review, thanks!
>
>
> diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> index 448296e..ab17e7b 100644
> --- a/virtio-crypto.tex
> +++ b/virtio-crypto.tex
> @@ -69,13 +69,13 @@ The following services are defined:
>
> \begin{lstlisting}
> /* CIPHER service */
> -#define VIRTIO_CRYPTO_SERVICE_CIPHER (0)
> +#define VIRTIO_CRYPTO_SERVICE_CIPHER 0
> /* HASH service */
> -#define VIRTIO_CRYPTO_SERVICE_HASH (1)
> +#define VIRTIO_CRYPTO_SERVICE_HASH 1
> /* MAC (Message Authentication Codes) service */
> -#define VIRTIO_CRYPTO_SERVICE_MAC (2)
> +#define VIRTIO_CRYPTO_SERVICE_MAC 2
> /* AEAD (Authenticated Encryption with Associated Data) service */
> -#define VIRTIO_CRYPTO_SERVICE_AEAD (3)
> +#define VIRTIO_CRYPTO_SERVICE_AEAD 3
> \end{lstlisting}
>
> The last driver-read-only fields specify detailed algorithms masks
> @@ -210,7 +210,7 @@ data that should be utilized in operations, and input
> data is equal to
> The general header for controlq is as follows:
>
> \begin{lstlisting}
> -#define VIRTIO_CRYPTO_OPCODE(service, op) ((service << 8) | (op))
> +#define VIRTIO_CRYPTO_OPCODE(service, op) (((service) << 8) | (op))
>
> struct virtio_crypto_ctrl_header {
> #define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION \
> @@ -380,20 +380,18 @@ struct virtio_crypto_mac_session_para {
> le32 auth_key_len;
> le32 padding;
> };
> -struct virtio_crypto_mac_session_output {
> - le64 auth_key_addr; /* guest key physical address */
> -};
>
> struct virtio_crypto_mac_create_session_req {
> /* Device-readable part */
> struct virtio_crypto_mac_session_para para;
> - struct virtio_crypto_mac_session_output out;
> + /* The authenticated key buffer */
> + /* output data here */
> +
> /* Device-writable part */
> struct virtio_crypto_session_input input;
> };
> \end{lstlisting}
>
> -The output data are
> \subparagraph{Session operation: Symmetric algorithms
> session}\label{sec:Device Types / Crypto Device / Device
> Operation / Control Virtqueue / Session operation / Session operation:
> Symmetric algorithms session}
>
> @@ -413,13 +411,13 @@ struct virtio_crypto_cipher_session_para {
> le32 padding;
> };
>
> -struct virtio_crypto_cipher_session_output {
> - le64 key_addr; /* guest key physical address */
> -};
> -
> struct virtio_crypto_cipher_session_req {
> + /* Device-readable part */
> struct virtio_crypto_cipher_session_para para;
> - struct virtio_crypto_cipher_session_output out;
> + /* The cipher key buffer */
> + /* Output data here */
> +
> + /* Device-writable part */
> struct virtio_crypto_session_input input;
> };
> \end{lstlisting}
> @@ -448,18 +446,20 @@ struct virtio_crypto_alg_chain_session_para {
> le32 padding;
> };
>
> -struct virtio_crypto_alg_chain_session_output {
> - struct virtio_crypto_cipher_session_output cipher;
> - struct virtio_crypto_mac_session_output mac;
> -};
> -
> struct virtio_crypto_alg_chain_session_req {
> + /* Device-readable part */
> struct virtio_crypto_alg_chain_session_para para;
> - struct virtio_crypto_alg_chain_session_output out;
> + /* The cipher key buffer */
> + /* The authenticated key buffer */
> + /* Output data here */
> +
> + /* Device-writable part */
> struct virtio_crypto_session_input input;
> };
> \end{lstlisting}
>
> +The output data includs both cipher key buffer and authenticated key buffer.
.. includes both the cipher key buffer and the uthenticated key buffer.
> +
> The packet for symmetric algorithm is as follows:
>
> \begin{lstlisting}
> @@ -503,13 +503,13 @@ struct virtio_crypto_aead_session_para {
> le32 padding;
> };
>
> -struct virtio_crypto_aead_session_output {
> - le64 key_addr; /* guest key physical address */
> -};
> -
> struct virtio_crypto_aead_create_session_req {
> + /* Device-readable part */
> struct virtio_crypto_aead_session_para para;
> - struct virtio_crypto_aead_session_output out;
> + /* The cipher key buffer */
> + /* Output data here */
> +
> + /* Device-writeable part */
> struct virtio_crypto_session_input input;
> };
> \end{lstlisting}
> @@ -568,19 +568,13 @@ struct virtio_crypto_op_data_req {
> The header is the general header and the union is of the algorithm-specific
> type,
> which is set by the driver. All properties in the union are shown as follows.
>
> -There is a unified idata structure for all symmetric algorithms, including
> CIPHER, HASH, MAC, and AEAD.
> +There is a unified idata structure for all service, including CIPHER, HASH,
> MAC, and AEAD.
for all services
>
> The structure is defined as follows:
>
> \begin{lstlisting}
> -struct virtio_crypto_sym_input {
> - /* Destination data guest address, it's useless for plain HASH and MAC */
> - le64 dst_data_addr;
> - /* Digest result guest address, it's useless for plain cipher algos */
guest address -> physical address?
it's useless -> unused?
> - le64 digest_result_addr;
> -
> - le32 status;
> - le32 padding;
> +struct virtio_crypto_inhdr {
> + u8 status;
> };
>
> \end{lstlisting}
> @@ -595,39 +589,29 @@ struct virtio_crypto_hash_para {
> le32 hash_result_len;
> };
>
> -struct virtio_crypto_hash_input {
> - struct virtio_crypto_sym_input input;
> -};
> -
> -struct virtio_crypto_hash_output {
> - /* source data guest address */
guest -> physical?
> - le64 src_data_addr;
> -};
> -
> struct virtio_crypto_hash_data_req {
> /* Device-readable part */
> struct virtio_crypto_hash_para para;
> - struct virtio_crypto_hash_output odata;
> + /* Source buffer */
> + /* Output data here */
> +
> /* Device-writable part */
> - struct virtio_crypto_hash_input idata;
> + /* Digest result buffer */
> + /* Input data here */
> + struct virtio_crypto_inhdr inhdr;
> };
> \end{lstlisting}
>
> Each data request uses virtio_crypto_hash_data_req structure to store
> information
> -used to run the HASH operations. The request only occupies one entry
> -in the Vring Descriptor Table in the virtio crypto device's dataq, which
> improves
> -the throughput of data transmitted for the HASH service, so that the virtio
> crypto
> -device can be better accelerated.
> +used to run the HASH operations.
>
> -The information includes the source data guest physical address stored by
> \field{odata}.\field{src_data_addr},
> -length of source data stored by \field{para}.\field{src_data_len}, and the
> digest result guest physical address
> -stored by \field{digest_result_addr} used to save the results of the HASH
> operations.
> -The address and length can determine exclusive content in the guest memory.
> +The information includes the hash paramenters stored by \field{para}, output
> data and input data.
> +The output data here includs the source buffer and the input data includes
> the digest result buffer used to save the results of the HASH operations.
includs -> includes
> +\field{inhdr} store the executing status of HASH operations.
>
> \begin{note}
> -The guest memory is always guaranteed to be allocated and
> physically-contiguous
> -pointed by \field{digest_result_addr} in struct virtio_crypto_hash_input and
> -\field{src_data_addr} in struct virtio_crypto_hash_output.
> +The request should by preference occupies one entry in the Vring Descriptor
> Table in the virtio crypto device's dataq, which improves
Don't use should outside confirmance statements.
occupies -> occupy
> +the throughput of data transmitted for the HASH service, so that the virtio
> crypto device can be better accelerated.
I'd just drop this note - I don't see why is crypto special here.
> \end{note}
>
> \drivernormative{\paragraph}{HASH Service Operation}{Device Types / Crypto
> Device / Device Operation / HASH Service Operation}
> @@ -641,8 +625,8 @@ pointed by \field{digest_result_addr} in struct
> virtio_crypto_hash_input and
> \devicenormative{\paragraph}{HASH Service Operation}{Device Types / Crypto
> Device / Device Operation / HASH Service Operation}
>
> \begin{itemize*}
> -\item The device MUST copy the results of HASH operations to the guest
> memory recorded by \field{digest_result_addr} field in struct
> virtio_crypto_hash_input.
> -\item The device MUST set \field{status} in strut virtio_crypto_hash_input:
> VIRTIO_CRYPTO_OK: success; VIRTIO_CRYPTO_ERR: creation failed or device
> error; VIRTIO_CRYPTO_NOTSUPP: not support.
> +\item The device MUST copy the results of HASH operations to the guest
> memory recorded by digest result buffer if HASH operations success.
> +\item The device MUST set \field{status} in strut virtio_crypto_inhdr:
> VIRTIO_CRYPTO_OK: success; VIRTIO_CRYPTO_ERR: creation failed or device
> error; VIRTIO_CRYPTO_NOTSUPP: not support; VIRTIO_CRYPTO_INVSESS: invalid
> session ID when the crypto operation is implemented.
strut -> struct?
add "to one of the values"? Also, just list the enum name here in case
we add more values?
not support - not supported?
> \end{itemize*}
>
> \subsubsection{MAC Service Operation}\label{sec:Device Types / Crypto Device
> / Device Operation / MAC Service Operation}
> @@ -652,39 +636,29 @@ struct virtio_crypto_mac_para {
> struct virtio_crypto_hash_para hash;
> };
>
> -struct virtio_crypto_mac_input {
> - struct virtio_crypto_sym_input input;
> -};
> -
> -struct virtio_crypto_mac_output {
> - struct virtio_crypto_hash_output hash_output;
> -};
> -
> struct virtio_crypto_mac_data_req {
> /* Device-readable part */
> struct virtio_crypto_mac_para para;
> - struct virtio_crypto_mac_output odata;
> + /* Source buffer */
> + /* Output data here */
> +
> /* Device-writable part */
> - struct virtio_crypto_mac_input idata;
> + /* Digest result buffer */
> + /* Input data here */
> + struct virtio_crypto_inhdr inhdr;
> };
> \end{lstlisting}
>
> Each data request uses virtio_crypto_mac_data_req structure to store
> information
> -used to run the MAC operations. The request only occupies one entry
> -in the Vring Descriptor Table in the virtio crypto device's dataq, which
> improves
> -the throughput of data transmitted for the MAC service, so that the virtio
> crypto
> -device can get the better result of acceleration.
> -
> -The information includes the source data guest physical address stored by
> \field{hash_output}.\field{src_data}.\field{addr},
> -the length of source data stored by
> \field{hash_output}.\field{src_data}.\field{len}, and the digest result guest
> physical address
> -stored by \field{digest_result_addr} used to save the results of the MAC
> operations.
> +used to run the MAC operations.
>
> -The address and length can determine exclusive content in the guest memory.
> +The information includes the hash paramenters stored by \field{para}, output
> data and input data.
> +The output data here includs the source buffer and the input data includes
> the digest result buffer used to save the results of the MAC operations.
includes
> +\field{inhdr} store the executing status of MAC operations.
stores
the executing status -> status of executing the MAC operations?
>
> \begin{note}
> -The guest memory is always guaranteed to be allocated and
> physically-contiguous
> -pointed by \field{digest_result_addr} in struct virtio_crypto_sym_input and
> -\field{hash_output}.\field{src_data_addr} in struct virtio_crypto_mac_output.
> +The request should by preference occupies one entry in the Vring Descriptor
> Table in the virtio crypto device's dataq, which improves
> +the throughput of data transmitted for the MAC service, so that the virtio
> crypto device can be better accelerated.
Again, let's just drop.
> \end{note}
>
> \drivernormative{\paragraph}{MAC Service Operation}{Device Types / Crypto
> Device / Device Operation / MAC Service Operation}
> @@ -698,8 +672,8 @@ pointed by \field{digest_result_addr} in struct
> virtio_crypto_sym_input and
> \devicenormative{\paragraph}{MAC Service Operation}{Device Types / Crypto
> Device / Device Operation / MAC Service Operation}
>
> \begin{itemize*}
> -\item The device MUST copy the results of MAC operations to the guest memory
> recorded by \field{digest_result_addr} field in struct
> virtio_crypto_mac_input.
> -\item The device MUST set \field{status} in strut virtio_crypto_mac_input:
> VIRTIO_CRYPTO_OK: success; VIRTIO_CRYPTO_ERR: creation failed or device
> error; VIRTIO_CRYPTO_NOTSUPP: not support.
> +\item The device MUST copy the results of MAC operations to the guest memory
> recorded by digest result buffer if HASH operations success.
> +\item The device MUST set \field{status} in strut virtio_crypto_inhdr:
> VIRTIO_CRYPTO_OK: success; VIRTIO_CRYPTO_ERR: creation failed or device
> error; VIRTIO_CRYPTO_NOTSUPP: not support; VIRTIO_CRYPTO_INVSESS: invalid
> session ID when the crypto operation is implemented.
same as above. I guess same issues repeat below, did not review.
> \end{itemize*}
>
> \subsubsection{Symmetric algorithms Operation}\label{sec:Device Types /
> Crypto Device / Device Operation / Symmetric algorithms Operation}
> @@ -709,12 +683,12 @@ The packet of plain CIPHER service is as follows:
> \begin{lstlisting}
> struct virtio_crypto_cipher_para {
> /*
> - * Byte Length of valid IV data pointed to by the below iv_addr.
> + * Byte Length of valid IV/Counter data pointed to by the below iv
> buffer.
> *
> - * - For block ciphers in CBC or F8 mode, or for Kasumi in F8 mode, or
> for
> + * For block ciphers in CBC or F8 mode, or for Kasumi in F8 mode, or for
> * SNOW3G in UEA2 mode, this is the length of the IV (which
> * must be the same as the block length of the cipher).
> - * - For block ciphers in CTR mode, this is the length of the counter
> + * For block ciphers in CTR mode, this is the length of the counter
> * (which must be the same as the block length of the cipher).
> */
> le32 iv_len;
> @@ -725,34 +699,28 @@ struct virtio_crypto_cipher_para {
> le32 padding;
> };
>
> -struct virtio_crypto_cipher_input {
> - struct virtio_crypto_sym_input input;
> -};
> -
> -struct virtio_crypto_cipher_output {
> - /*
> - * Initialization Vector or Counter Guest Address.
> +struct virtio_crypto_cipher_data_req {
> + /* Device-readable part */
> + struct virtio_crypto_cipher_para para;
> + /*
> + * Initialization Vector or Counter buffer.
> *
> - * - For block ciphers in CBC or F8 mode, or for Kasumi in F8 mode, or
> for
> + * For block ciphers in CBC or F8 mode, or for Kasumi in F8 mode, or for
> * SNOW3G in UEA2 mode, this is the Initialization Vector (IV)
> * value.
> - * - For block ciphers in CTR mode, this is the counter.
> - * - For AES-XTS, this is the 128bit tweak, i, from IEEE Std 1619-2007.
> + * For block ciphers in CTR mode, this is the counter.
> + * For AES-XTS, this is the 128bit tweak, i, from IEEE Std 1619-2007.
> *
> * The IV/Counter will be updated after every partial cryptographic
> * operation.
> */
> - le64 iv_addr;
> - /* source data guest address */
> - le64 src_data_addr;
> -};
> + /* Source buffer */
> + /* Output data here */
>
> -struct virtio_crypto_cipher_data_req {
> - /* Device-readable part */
> - struct virtio_crypto_cipher_para para;
> - struct virtio_crypto_cipher_output odata;
> /* Device-writable part */
> - struct virtio_crypto_cipher_input idata;
> + /* Destination data buffer */
> + /* Input data here */
> + struct virtio_crypto_inhdr inhdr;
> };
> \end{lstlisting}
>
> @@ -780,23 +748,19 @@ struct virtio_crypto_alg_chain_data_para {
> __virtio32 reserved;
> };
>
> -struct virtio_crypto_alg_chain_data_output {
> - /* Device-readable part */
> - struct virtio_crypto_cipher_output cipher;
> - /* Additional auth data guest address */
> - le64 aad_data_addr;
> -};
> -
> -struct virtio_crypto_alg_chain_data_input {
> - struct virtio_crypto_sym_input input;
> -};
> -
> struct virtio_crypto_alg_chain_data_req {
> /* Device-readable part */
> struct virtio_crypto_alg_chain_data_para para;
> - struct virtio_crypto_alg_chain_data_output odata;
> + /* Initialization Vector or Counter buffer */
> + /* Source buffer */
> + /* Additional authenticated buffer if exists */
> + /* Output data here */
> +
> /* Device-writable part */
> - struct virtio_crypto_alg_chain_data_input idata;
> + /* Destination data buffer */
> + /* Digest result buffer */
> + /* Input data here */
> + struct virtio_crypto_inhdr inhdr;
> };
> \end{lstlisting}
>
> @@ -817,29 +781,22 @@ struct virtio_crypto_sym_data_req {
> };
> \end{lstlisting}
>
> -Each data request uses the virtio_crypto_cipher_data_req structure to store
> information
> -used to run the CIPHER operations. The request only occupies one entry
> -in the Vring Descriptor Table in the virtio crypto device's dataq, which
> improves
> -the throughput of data transmitted for the CIPHER service, so that the
> virtio crypto
> -device can get the better result of acceleration.
> +Each data request uses virtio_crypto_sym_data_req structure to store
> information
> +used to run the CIPHER operations.
>
> +The information includes the hash paramenters stored by \field{para}, output
> data and input data.
> In the first virtio_crypto_cipher_para structure, \field{iv_len} specifies
> the length of the initialization vector or counter,
> \field{src_data_len} specifies the length of the source data, and
> \field{dst_data_len} specifies the
> -length of the destination data.
> +length of the destination data.
> +For plain CIPHER operations, the output data here includs the IV/Counter
> buffer and source buffer, and the input data includes the destination buffer
> used to save the results of the CIPHER operations.
>
> -In the following virtio_crypto_cipher_input structure, \field{dst_data_addr}
> specifies the destination
> -data guest physical address used to store the results of the CIPHER
> operations, and \field{status} specifies
> -the CIPHER operation status.
> -
> -In the virtio_crypto_cipher_output structure, \field{iv_addr} specifies the
> guest physical address of initialization vector,
> -\field{src_data_addr} specifies the source data guest physical address.
> -
> -The addresses and length can determine exclusive content in the guest memory.
> +For algorithms chain, the output data here includs the IV/Counter buffer,
> source buffer and additional authenticated data buffer if exists.
> +The input data includes both destionation buffer and digest result buffer
> used to store the results of the HASH/MAC operations.
> +\field{inhdr} store the executing status of crypto operations.
>
> \begin{note}
> -The guest memory is always guaranteed to be allocated and
> physically-contiguous
> -pointed by \field{dst_data_addr} in struct virtio_crypto_cipher_input,
> -\field{iv_addr} and \field{src_data_addr} in struct
> virtio_crypto_cipher_output.
> +The request should by preference occupies one entry in the Vring Descriptor
> Table in the virtio crypto device's dataq, which improves
> +the throughput of data transmitted for the crypto service, so that the
> virtio crypto device can be better accelerated.
> \end{note}
>
> \drivernormative{\paragraph}{Symmetric algorithms Operation}{Device Types /
> Crypto Device / Device Operation / Symmetric algorithms Operation}
> @@ -860,10 +817,10 @@ pointed by \field{dst_data_addr} in struct
> virtio_crypto_cipher_input,
> \item The device SHOULD only parse fields of struct
> virtio_crypto_cipher_data_req in struct virtio_crypto_sym_data_req if the
> created session is VIRTIO_CRYPTO_SYM_OP_CIPHER type.
> \item The device MUST parse fields of both struct
> virtio_crypto_cipher_data_req and struct virtio_crypto_mac_data_req in struct
> virtio_crypto_sym_data_req if the created
> session is of the VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING operation type and
> in the VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH mode.
> -\item The device MUST copy the result of cryptographic operation to the
> guest memory recorded by \field{dst_data_addr} field in struct
> virtio_crypto_cipher_input in plain CIPHER mode.
> -\item The device MUST copy the result of HASH/MAC operation to the guest
> memory recorded by \field{digest_result_addr} field in struct
> virtio_crypto_alg_chain_data_input is of the
> VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING type.
> -\item The device MUST set the \field{status} field in strut
> virtio_crypto_cipher_input or virtio_crypto_alg_chain_data_input structure:
> -VIRTIO_CRYPTO_OK: success; VIRTIO_CRYPTO_ERR: creation failed or device
> error; VIRTIO_CRYPTO_NOTSUPP: not supported.
> +\item The device MUST copy the result of cryptographic operation to the
> guest memory recorded by destination buffer in plain CIPHER mode.
> +\item The device MUST check the \field{para}.\field{add_len} is bigger than
> 0 before parse the additional authenticated buffer in plain algorithms chain
> mode.
> +\item The device MUST copy the result of HASH/MAC operation to the guest
> memory recorded by digest result buffer is of the
> VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING type.
> +\item The device MUST set the \field{status} field in strut
> virtio_crypto_inhdr: VIRTIO_CRYPTO_OK: success; VIRTIO_CRYPTO_ERR: creation
> failed or device error; VIRTIO_CRYPTO_NOTSUPP: not supported;
> VIRTIO_CRYPTO_INVSESS: invalid session ID when the crypto operation is
> implemented.
> \end{itemize*}
>
> \paragraph{Steps of Operation}\label{sec:Device Types / Crypto Device /
> Device Operation / Symmetric algorithms Operation / Steps of Operation}
> @@ -894,17 +851,15 @@ VIRTIO_CRYPTO_OK: success; VIRTIO_CRYPTO_ERR: creation
> failed or device error; V
> \item The device invokes the cryptographic APIs of the backend crypto
> accelerator;
> \item The backend crypto accelerator executes the cryptographic operation
> implicitly;
> \item The device receives the cryptographic results from the backend crypto
> accelerator (synchronous or asynchronous);
> -\item The device sets the \field{status} in struct
> virtio_crypto_cipher_input;
> +\item The device sets the \field{status} in struct virtio_crypto_inhdr;
> \item The device updates and flushes the Used Ring to return the
> cryptographic results to the driver;
> \item The device notifies the driver (Or the driver actively polls the
> dataq's Used Ring);
> \item The driver saves the cryptographic result.
> \end{enumerate}
>
> \begin{note}
> -\begin{itemize*}
> -\item For better performance, the device should by preference use vhost
> scheme (user space or kernel space)
> - as the backend crypto accelerator in the real production environment.
> -\end{itemize*}
> +For better performance, the device should by preference use vhost scheme
> (user space or kernel space)
> +as the backend crypto accelerator in the real production environment.
> \end{note}
>
> \subsubsection{AEAD Service Operation}\label{sec:Device Types / Crypto
> Device / Device Operation / AEAD Service Operation}
> @@ -912,11 +867,11 @@ VIRTIO_CRYPTO_OK: success; VIRTIO_CRYPTO_ERR: creation
> failed or device error; V
> \begin{lstlisting}
> struct virtio_crypto_aead_para {
> /*
> - * Byte Length of valid IV data pointed to by the below iv_addr
> parameter.
> + * Byte Length of valid IV data.
> *
> - * - For GCM mode, this is either 12 (for 96-bit IVs) or 16, in which
> - * case iv_addr points to J0.
> - * - For CCM mode, this is the length of the nonce, which can be in the
> + * For GCM mode, this is either 12 (for 96-bit IVs) or 16, in which
> + * case iv points to J0.
> + * For CCM mode, this is the length of the nonce, which can be in the
> * range 7 to 13 inclusive.
> */
> le32 iv_len;
> @@ -928,66 +883,51 @@ struct virtio_crypto_aead_para {
> le32 dst_data_len;
> };
>
> -struct virtio_crypto_aead_input {
> - struct virtio_crypto_sym_input input;
> -};
> -
> -struct virtio_crypto_aead_output {
> +struct virtio_crypto_aead_data_req {
> + /* Device-readable part */
> + struct virtio_crypto_aead_para para;
> /*
> - * Initialization Vector Guest Address.
> + * Initialization Vector buffer.
> *
> - * - For GCM mode, this is either the IV (if the length is 96 bits) or J0
> + * For GCM mode, this is either the IV (if the length is 96 bits) or J0
> * (for other sizes), where J0 is as defined by NIST SP800-38D.
> * Regardless of the IV length, a full 16 bytes needs to be allocated.
> - * - For CCM mode, the first byte is reserved, and the nonce should be
> - * written starting at &iv_addr[1] (to allow space for the
> implementation
> + * For CCM mode, the first byte is reserved, and the nonce should be
> + * written starting at &iv_buffer[1] (to allow space for the
> implementation
> * to write in the flags in the first byte). Note that a full 16 bytes
> * should be allocated, even though the iv_len field will have
> * a value less than this.
> *
> * The IV will be updated after every partial cryptographic operation.
> */
> - le64 iv_addr;
> - /* source data guest address */
> - le64 src_data_addr;
> - /* additional auth data guest address */
> - le64 add_data_addr;
> -};
> + /* Source buffer */
> + /* Additional authenticated buffer if exists */
> + /* Output data here */
>
> -struct virtio_crypto_aead_data_req {
> - /* Device-readable part */
> - struct virtio_crypto_aead_para para;
> - struct virtio_crypto_aead_output odata;
> /* Device-writable part */
> - struct virtio_crypto_aead_input idata;
> + /* Destination data buffer */
> + /* Digest result buffer */
> + /* Input data here */
> + struct virtio_crypto_inhdr inhdr;
> };
> \end{lstlisting}
>
> Each data request uses virtio_crypto_aead_data_req structure to store
> information
> -used to implement the CIPHER operations. The request only occupies one entry
> -in the Vring Descriptor Table in the virtio crypto device's dataq, which
> improves
> -the throughput of data transmitted for the AEAD service, so that the virtio
> crypto
> -device can be better accelerated.
> +used to run the AEAD operations.
>
> -In the first virtio_crypto_aead_para structure, \field{iv_len} specifies the
> length of the initialization vector;
> +The information includes the hash paramenters stored by \field{para}, output
> data and input data.
> +In the first virtio_crypto_aead_para structure, \field{iv_len} specifies the
> length of the initialization vector.
> \field{aad_len} specifies the length of additional authentication data,
> \field{src_data_len} specifies the
> length of the source data; \field{dst_data_len} specifies the length of the
> destination data.
> +The output data here includs the IV buffer and source buffer, and the input
> data includes the destination buffer used to save the results of the CIPHER
> operations.
>
> -In the following virtio_crypto_aead_input structure, \field{dst_data_addr}
> specifies destination
> -data guest physical address used to store the results of the CIPHER
> operations; \field{digest_result_addr} specifies
> -the digest result guest physical address used to store the results of the
> HASH/MAC operations; \field{status} specifies
> -the status of AEAD operation.
> -
> -In the virtio_crypto_aead_output structure, \field{iv_addr} specifies the
> guest physical address of initialization vector,
> -\field{src_data_addr} specifies the source data guest physical address,
> \field{add_data_addr} specifies the guest physical address
> -of additional authentication data.
> -
> -The addresses and length can determine exclusive content in the guest memory.
> +The output data here includs the IV/Counter buffer, source buffer and
> additional authenticated data buffer if exists.
> +The input data includes both destionation buffer used to save the results of
> the AEAD operations.
> +\field{inhdr} store the executing status of AEAD operations.
>
> \begin{note}
> -The guest memory MUST be guaranteed to be allocated and physically-contiguous
> -pointed by \field{dst_data_addr} and \field{digest_result_addr} in struct
> virtio_crypto_aead_input,
> -\field{iv_addr}, \field{src_data_addr} and \field{add_data_addr} in struct
> virtio_crypto_aead_output.
> +The request should by preference occupies one entry in the Vring Descriptor
> Table in the virtio crypto device's dataq, which improves
> +the throughput of data transmitted for the AEAD service, so that the virtio
> crypto device can be better accelerated.
> \end{note}
>
> \drivernormative{\paragraph}{AEAD Service Operation}{Device Types / Crypto
> Device / Device Operation / AEAD Service Operation}
> @@ -1002,8 +942,8 @@ pointed by \field{dst_data_addr} and
> \field{digest_result_addr} in struct virtio
>
> \begin{itemize*}
> \item The device MUST parse the virtio_crypto_aead_data_req based on the
> \field{opcode} in general header.
> -\item The device MUST copy the result of cryptographic operation to the
> guest memory recorded by \field{dst_data_addr} field in struct
> virtio_crypto_aead_input.
> -\item The device MUST copy the digest result to the guest memory recorded by
> \field{digest_result_addr} field in struct virtio_crypto_aead_input.
> -\item The device MUST set the \field{status} field in strut
> virtio_crypto_aead_input: VIRTIO_CRYPTO_OK: success; VIRTIO_CRYPTO_ERR:
> creation failed or device error; VIRTIO_CRYPTO_NOTSUPP: not supported.
> +\item The device MUST copy the result of cryptographic operation to the
> guest memory recorded by destination buffer.
> +\item The device MUST copy the digest result to the guest memory recorded by
> digest result buffer.
> +\item The device MUST set the \field{status} field in strut
> virtio_crypto_inhdr: VIRTIO_CRYPTO_OK: success; VIRTIO_CRYPTO_ERR: creation
> failed or device error; VIRTIO_CRYPTO_NOTSUPP: not supported;
> VIRTIO_CRYPTO_INVSESS: invalid session ID when the crypto operation is
> implemented.
> \item When the \field{opcode} is VIRTIO_CRYPTO_AEAD_DECRYPT, the device MUST
> verify and return the verification result to the driver, and if the
> verification result is incorrect, VIRTIO_CRYPTO_BADMSG (bad message) MUST be
> returned to the driver.
> \end{itemize*}
> \ No newline at end of file
>
>
> Regards,
> -Gonglei
>
>
>
> > -----Original Message-----
> > From: Gonglei (Arei)
> > Sent: Thursday, November 10, 2016 10:33 AM
> > To: 'Cornelia Huck'
> > Cc: Halil Pasic; address@hidden; address@hidden;
> > Huangweidong (C); address@hidden; address@hidden; Shiqing Fan;
> > Zhoujian (jay, Euler); address@hidden; address@hidden;
> > address@hidden; Hanweidong (Randy); address@hidden;
> > address@hidden; address@hidden; address@hidden;
> > Luonengjun; address@hidden; Huangpeng (Peter); address@hidden;
> > address@hidden; Jani Kokkonen; address@hidden; Claudio
> > Fontana; address@hidden; Wubin (H)
> > Subject: RE: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto
> > device specification
> >
> > > From: Cornelia Huck [mailto:address@hidden
> > > Sent: Wednesday, November 09, 2016 11:25 PM
> > > Subject: Re: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto
> > > device specification
> > >
> > > On Wed, 9 Nov 2016 01:11:20 +0000
> > > "Gonglei (Arei)" <address@hidden> wrote:
> > >
> > > > Nope, Actually I kept those description here is because I wanted to
> > represent
> > > each packet
> > > > Intuitionally, otherwise I don't know how to explain them only occupy
> > > > one
> > > entry in desc table
> > > > by indirect table method. So I changed the code completely as Stefan's
> > > suggestion and
> > > > revised the spec a little.
> > >
> > > I think you need to revise the spec a bit more :) IIUC, you should
> > > simply state that you use the indirect table method and not mention any
> > > guest physical addresses.
> > >
> > Yes, will remove the gpa description stuff. Using indirect table is a
> > method for
> > high performance
> > and throughput, I'll mention it as a note. Pls see my another reply to
> > Michael.
> >
> > > > This just is a representative of the realization so that the people can
> > > > easily
> > > understand what
> > > > the virtio crypto request's components. It isn't completely same with
> > > > the
> > > code.
> > > > For virtio-scsi device, the struct virtio_scsi_req_cmd also used this
> > > > way
> > IIUR.
> > >
> > > It seemes to have caused at least some confusion - perhaps you
> > > simplyfied too much?
> >
> > Yes, I will.
> >
> > Thanks,
> > -Gonglei
- Re: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto device specification, Halil Pasic, 2016/11/08
- Re: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto device specification, Gonglei (Arei), 2016/11/08
- Re: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto device specification, Cornelia Huck, 2016/11/09
- Re: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto device specification, Gonglei (Arei), 2016/11/09
- Re: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto device specification, Gonglei (Arei), 2016/11/10
- Re: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto device specification,
Michael S. Tsirkin <=
- Re: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto device specification, Halil Pasic, 2016/11/10
- Re: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto device specification, Michael S. Tsirkin, 2016/11/10
- Re: [Qemu-devel] [virtio-dev] Re: [PATCH v13 1/2] virtio-crypto: Add virtio crypto device specification, Gonglei (Arei), 2016/11/10
- Re: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto device specification, Gonglei (Arei), 2016/11/10
- Re: [Qemu-devel] [virtio-dev] Re: [PATCH v13 1/2] virtio-crypto: Add virtio crypto device specification, Gonglei (Arei), 2016/11/10
Re: [Qemu-devel] [PATCH v13 1/2] virtio-crypto: Add virtio crypto device specification, Michael S. Tsirkin, 2016/11/09