[Qemu-devel] [PATCH v6 0/2] virtio-crypto: virtio crypto device specification

[Qemu-devel] [PATCH v6 0/2] virtio-crypto: virtio crypto device specification

[Gonglei]

This is the specification (version 6) about a new virtio crypto device.
After a big reconstruction, the spec (symmetric algos) is near to stabilize.
This version fix some problems of formating and return value, etc.

If you have any comments, please let me know, thanks :)

CC: Michael S. Tsirkin <mst@redhat.com>
CC: Cornelia Huck <cornelia.huck@de.ibm.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Lingli Deng <denglingli@chinamobile.com>
CC: Jani Kokkonen <Jani.Kokkonen@huawei.com>
CC: Ola Liljedahl <Ola.Liljedahl@arm.com>
CC: Varun Sethi <Varun.Sethi@freescale.com>
CC: Zeng Xin <xin.zeng@intel.com>
CC: Keating Brian <brian.a.keating@intel.com>
CC: Ma Liang J <liang.j.ma@intel.com>
CC: Griffin John <john.griffin@intel.com>
CC: Hanweidong <hanweidong@huawei.com>

Changes from v6:
 - add conformance clauses for virtio crypto device. [Michael]
 - drop VIRTIO_CRYPTO_S_STARTED. [Michael]
 - fix some characters problems. [Stefan]
 - add a MAC algorithm, named VIRTIO_CRYPTO_MAC_ZUC_EIA3. [Zeng Xin]
 - add the fourth return code, named VIRTIO_CRYPTO_OP_INVSESS used
   for invalid session id when executing crypto operations.
 - drop some gpu stuff forgot to delete. [Michael]
 - convert tab to space all over the content.

Changes from v4:
 - introduce crypto services into virtio crypto device. The services 
   currently defined are CIPHER, MAC, HASH, AEAD, KDF, ASYM, PRIMITIVE.
 - 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".
 - redefine the algorithms and structure based on above crypto services.
 - rearrange the title and subtitle
 - Only support CIPHER, MAC, HASH and AEAD crypto services, and Xin will
   focus KDF, ASYM and PRIMITIVE services.
 - Some other corresponding fixes.
 - Make a formal patch using tex type.

This version is a big reconstruction based on Zeng, Xin' comments, thanks a lot.

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.


Gonglei (2):
  virtio-crypto: Add virtio crypto device specification
  virtio-crypto: Add conformance clauses

 conformance.tex   |  30 +++
 content.tex       |   2 +
 virtio-crypto.tex | 793 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 825 insertions(+)
 create mode 100644 virtio-crypto.tex

-- 
1.7.12.4

[Qemu-devel] [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification

[Gonglei]

The virtio crypto device is a virtual crypto device (ie. hardware
crypto accelerator card). The virtio crypto device can provide
five crypto services: CIPHER, MAC, HASH, AEAD, KDF, ASYM, PRIMITIVE.

In this patch, CIPHER, MAC, HASH, AEAD services are introduced.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
CC: Michael S. Tsirkin <mst@redhat.com>
CC: Cornelia Huck <cornelia.huck@de.ibm.com>
CC: Stefan Hajnoczi <stefanha@redhat.com>
CC: Lingli Deng <denglingli@chinamobile.com>
CC: Jani Kokkonen <Jani.Kokkonen@huawei.com>
CC: Ola Liljedahl <Ola.Liljedahl@arm.com>
CC: Varun Sethi <Varun.Sethi@freescale.com>
CC: Zeng Xin <xin.zeng@intel.com>
CC: Keating Brian <brian.a.keating@intel.com>
CC: Ma Liang J <liang.j.ma@intel.com>
CC: Griffin John <john.griffin@intel.com>
CC: Hanweidong <hanweidong@huawei.com>
---
 content.tex       |   2 +
 virtio-crypto.tex | 793 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 795 insertions(+)
 create mode 100644 virtio-crypto.tex

diff --git a/content.tex b/content.tex
index 4b45678..ab75f78 100644
--- a/content.tex
+++ b/content.tex
@@ -5750,6 +5750,8 @@ descriptor for the \field{sense_len}, \field{residual},
 \field{status_qualifier}, \field{status}, \field{response} and
 \field{sense} fields.
 
+\input{virtio-crypto.tex}
+
 \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
 
 Currently there are three device-independent feature bits defined:
diff --git a/virtio-crypto.tex b/virtio-crypto.tex
new file mode 100644
index 0000000..9465de3
--- /dev/null
+++ b/virtio-crypto.tex
@@ -0,0 +1,793 @@
+\section{Crypto Device}\label{sec:Device Types / 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. The virtio crypto
+device can provide seven crypto services: CIPHER, MAC, HASH, AEAD,
+KDF, ASYM, PRIMITIVE.
+
+\subsection{Device ID}\label{sec:Device Types / Crypto Device / Device ID}
+
+20
+
+\subsection{Virtqueues}\label{sec:Device Types / Crypto Device / Virtqueues}
+
+\begin{description}
+\item[0] dataq1
+\item[\ldots]
+\item[N-1] dataqN
+\item[N] controlq
+\end{description}
+
+N is set by \field{max_dataqueues} (\field{max_dataqueues} >= 1).
+
+\subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature bits}
+  None currently defined
+
+\subsection{Device configuration layout}\label{sec:Device Types / Crypto Device / Device configuration layout}
+
+Thirteen driver-read-only configuration fields are currently defined.
+
+\begin{lstlisting}
+struct virtio_crypto_config {
+    le32  version;
+    le16  status;
+    le16  max_dataqueues;
+    le32  crypto_services;
+    /* 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;
+};
+\end{lstlisting}
+
+The first driver-read-only field, \field{version} specifies the virtio crypto's
+version, which is reserved for back-compatibility in future.It's currently
+defined for the version field:
+
+\begin{lstlisting}
+#define VIRTIO_CRYPTO_VERSION_1  (1)
+\end{lstlisting}
+
+One read-only bit (for the device) is currently defined for the \field{status}
+field: VIRTIO_CRYPTO_S_HW_READY.
+
+\begin{lstlisting}
+#define VIRTIO_CRYPTO_S_HW_READY  (1 << 0)
+\end{lstlisting}
+
+The following driver-read-only field, \field{max_dataqueuess} specifies the
+maximum number of data virtqueues (dataq1\ldots dataqN). The crypto_services
+shows the crypto services the virtio crypto supports. The service currently
+defined are:
+
+\begin{lstlisting}
+#define VIRTIO_CRYPTO_NO_SERVICE (0) /* cipher services */
+#define VIRTIO_CRYPTO_SERVICE_CIPHER (1) /* cipher services */
+#define VIRTIO_CRYPTO_SERVICE_HASH  (2) /* hash service */
+#define VIRTIO_CRYPTO_SERVICE_MAC   (3) /* MAC (Message Authentication Codes) service */
+#define VIRTIO_CRYPTO_SERVICE_AEAD  (4) /* AEAD(Authenticated Encryption with Associated Data) service */
+\end{lstlisting}
+
+The last driver-read-only fields specify detailed algorithms mask which
+the device offered for corresponding service. The below CIPHER algorithms
+are defined currently:
+
+\begin{lstlisting}
+#define VIRTIO_CRYPTO_NO_CIPHER                 0
+#define VIRTIO_CRYPTO_CIPHER_ARC4               1
+#define VIRTIO_CRYPTO_CIPHER_AES_ECB            2
+#define VIRTIO_CRYPTO_CIPHER_AES_CBC            3
+#define VIRTIO_CRYPTO_CIPHER_AES_CTR            4
+#define VIRTIO_CRYPTO_CIPHER_DES_ECB            5
+#define VIRTIO_CRYPTO_CIPHER_DES_CBC            6
+#define VIRTIO_CRYPTO_CIPHER_3DES_ECB           7
+#define VIRTIO_CRYPTO_CIPHER_3DES_CBC           8
+#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
+\end{lstlisting}
+
+The below HASH algorithms are defined currently:
+
+\begin{lstlisting}
+#define VIRTIO_CRYPTO_NO_HASH            0
+#define VIRTIO_CRYPTO_HASH_MD5           1
+#define VIRTIO_CRYPTO_HASH_SHA1          2
+#define VIRTIO_CRYPTO_HASH_SHA_224       3
+#define VIRTIO_CRYPTO_HASH_SHA_256       4
+#define VIRTIO_CRYPTO_HASH_SHA_384       5
+#define VIRTIO_CRYPTO_HASH_SHA_512       6
+#define VIRTIO_CRYPTO_HASH_SHA3_224      7
+#define VIRTIO_CRYPTO_HASH_SHA3_256      8
+#define VIRTIO_CRYPTO_HASH_SHA3_384      9
+#define VIRTIO_CRYPTO_HASH_SHA3_512      10
+#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE128      11
+#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE256      12
+\end{lstlisting}
+
+The below MAC algorithms are defined currently:
+
+\begin{lstlisting}
+#define VIRTIO_CRYPTO_NO_MAC                       0
+#define VIRTIO_CRYPTO_MAC_HMAC_MD5                 1
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA1                2
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA_224             3
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA_256             4
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA_384             5
+#define VIRTIO_CRYPTO_MAC_HMAC_SHA_512             6
+#define VIRTIO_CRYPTO_MAC_CMAC_3DES                25
+#define VIRTIO_CRYPTO_MAC_CMAC_AES                 26
+#define VIRTIO_CRYPTO_MAC_CMAC_KASUMI_F9           27
+#define VIRTIO_CRYPTO_MAC_CMAC_SNOW3G_UIA2         28
+#define VIRTIO_CRYPTO_MAC_GMAC_AES                 41
+#define VIRTIO_CRYPTO_MAC_GMAC_TWOFISH             42
+#define VIRTIO_CRYPTO_MAC_CBCMAC_AES               49
+#define VIRTIO_CRYPTO_MAC_CBCMAC_KASUMI_F9         50
+#define VIRTIO_CRYPTO_MAC_XCBC_AES                 53
+#define VIRTIO_CRYPTO_MAC_ZUC_EIA3                 54
+\end{lstlisting}
+
+The below AEAD algorithms are defined currently:
+
+\begin{lstlisting}
+#define VIRTIO_CRYPTO_NO_AEAD     0
+#define VIRTIO_CRYPTO_AEAD_GCM    1
+#define VIRTIO_CRYPTO_AEAD_CCM    2
+#define VIRTIO_CRYPTO_AEAD_CHACHA20_POLY1305  3
+\end{lstlisting}
+
+\subsubsection{Device Requirements: Device configuration layout}\label{sec:Device Types / Crypto Device / Device configuration layout / Device Requirements: Device configuration layout}
+
+\begin{itemize*}
+\item The device MUST set \field{version} in version filed.
+\item The device MUST set \field{max_dataqueues} to between 1 and 65535 inclusive.
+\item The device SHOULD set \field{status} according to the status of the hardware-backed implementation.
+\item The device MUST set \field{crypto_services} according to the crypto services which the device offered.
+\item The device MUST set detailed algorithms mask according to crypto_services field.
+\end{itemize*}
+
+\subsubsection{Driver Requirements: Device configuration layout}\label{sec:Device Types / Crypto Device / Device configuration layout / Driver Requirements: Device configuration layout}
+
+\begin{itemize*}
+\item The driver MUST read the ready \field{status} from the bottom bit of status to
+      check whether the hardware-backed implementation is ready or not.
+\item The driver MAY read \field{max_dataqueues} field to discover how many data queues the device supports.
+\item The driver MUST read \field{crypto_services} field to discover which services the device is able to offer.
+\item The driver MUST read detail algorithms field according to crypto_services field.
+\end{itemize*}
+
+\subsection{Device Initialization}{Device Types / Crypto Device / Device Initialization}
+
+The driver SHOULD perform a typical initialization routine like so:
+
+\begin{enumerate}
+\item Identify and initialize data virtqueue, up to \field{max_dataqueues}.
+\item Identify the control virtqueue.
+\item Identify the ready status of hardware-backend comes from the \field{status}.
+\item Read the supported crypto services from bits of \field{crypto_servies}.
+\item Read the supported algorithms according to \field{crypto_services} field.
+\end{enumerate}
+
+\subsection{Device Operation}\label{sec:Device Types / Crypto Device / Device Operation}
+
+Packets can be transmitted by placing them in both the controlq and dataq.
+The packet are consisted of general header and services 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 parameters are algorithm-specific parameters, input data is the
+data should be operated , output data is the "operation result + result buffer".
+The general header of controlq:
+
+\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;
+    /* control flag to control the request */
+    u8 flag;
+    /* data virtqeueu id */
+    le16 queue_id;
+};
+\end{lstlisting}
+
+The general header of dataq:
+
+\begin{lstlisting}
+struct virtio_crypto_op_header {
+#define VIRTIO_CRYPTO_CIPHER_ENCRYPT    VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x00)
+#define VIRTIO_CRYPTO_CIPHER_DECRYPT    VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x01)
+#define VIRTIO_CRYPTO_HASH              VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x00)
+#define VIRTIO_CRYPTO_MAC               VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x00)
+#define VIRTIO_CRYPTO_AEAD_ENCRYPT      VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x00)
+#define VIRTIO_CRYPTO_AEAD_DECRYPT      VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x01)
+    le32 opcode;
+    /* algo should be service-specific algorithms */
+    le32 algo;
+    /* control flag to control the request */
+    u8 flag;
+    /* data virtqeueu id */
+    le16 queue_id;
+    /* session_id should be service-specific algorithms */
+    le64 session_id;
+};
+\end{lstlisting}
+
+\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 which finish the non-data plane operations, such as session
+operations (see \ref{sec:Device Types / Crypto Device / Device Operation / Session operation}).
+The packet of controlq:
+
+\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      sym_destroy_session;
+    } u;
+};
+\end{lstlisting}
+
+The header is the general header, the union is algorithm-specific type,
+which is set by the driver. All the properties in the union will be shown as follow.
+
+\subsubsection{Session operation}\label{sec:Device Types / Crypto Device / Device Operation / 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:
+
+\begin{enumerate}
+\item The operation (cipher, hash/mac or both, and if both, the order in
+      which the algorithms should be applied).
+\item The cipher setup data, including the cipher algorithm and mode,
+      the key and its length, and the direction (encrypt or decrypt).
+\item The hash/mac setup data, including the hash algorithm or mac algorithm,
+      and digest result length (to allow for truncation).
+\begin{itemize*}
+\item Authenticated mode can refer to MAC, which requires that the key and
+      its length are also specified.
+\item For nested mode, the inner and outer prefix data and length are specified,
+      as well as the outer hash algorithm.
+\end{itemize*}
+\end{enumerate}
+
+\paragraph{Session operation: HASH session}\label{sec:Device Types / Crypto Device / Device Operation / Session operation / Session operation: HASH session}
+
+The packet of HASH session is:
+
+\begin{lstlisting}
+struct virtio_crypto_sym_session_input {
+    u8     status;
+    le64    session_id;
+};
+
+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 {
+    struct virtio_crypto_hash_session_para parameter;
+    /*
+     * in header guest physical address, See struct virtio_crypto_sym_session_input
+     * above
+     */
+    le64 inhdr_addr;
+};
+\end{lstlisting}
+
+\paragraph{Session operation: MAC session}\label{sec:Device Types / Crypto Device / Device
+Operation / Session operation / Session operation: MAC session}
+
+The packet of MAC session is:
+
+\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;
+};
+struct virtio_crypto_mac_create_session_req {
+    struct virtio_crypto_mac_session_para  parameter;
+    /*
+     * in header guest physical address, See struct virtio_crypto_sym_session_input
+     * above
+     */
+    le64 inhdr_addr;
+};
+\end{lstlisting}
+
+\paragraph{Session operation: Symmetric algorithms session}\label{sec:Device Types / Crypto Device / Device
+Operation / Session operation / Session operation: Symmetric algorithms session}
+
+The symmetric session include both ciphers (CIPHER service) and chain
+algorithms (chain cipher and hash/mac). The packet of symmetric session is:
+
+\begin{lstlisting}
+struct virtio_crypto_cipher_session_para {
+/* See VIRTIO_CRYPTO_CIPHER* above */
+    le32 algo;
+    /* length of key */
+    le32 keylen;
+    /* encrypt or decrypt */
+    u8 op;
+};
+struct virtio_crypto_sym_create_session_req {
+/* 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
+    u8 op_type;
+
+    union {
+        struct virtio_crypto_cipher_session_para cipher_param;
+        struct virtio_crypto_alg_chain_param {
+#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER  1
+#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH  2
+            u8 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
+            u8 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;
+        } chain;
+    } sym;
+
+    /*
+     * in header guest physical address, See struct virtio_crypto_sym_session_input
+     * above
+     */
+    le64 inhdr_addr;
+};
+\end{lstlisting}
+
+\paragraph{Session operation: AEAD session}\label{sec:Device Types / Crypto Device / Device
+Operation / Session operation / Session operation: AEAD session}
+
+The packet of AEAD session is:
+
+\begin{lstlisting}
+struct virtio_crypto_aead_session_para {
+    /* See VIRTIO_CRYPTO_AEAD_* above*/
+    u8 algo;
+    /* length of key */
+    le32 key_len;
+    /* digest result length */
+    le32 digest_result_len;
+    /* The length of the additional authenticated data (AAD) in bytes */
+    le32 aad_len;
+    /* encrypt or decrypt */
+    u8 op;
+};
+struct virtio_crypto_aead_create_session_req {
+    struct virtio_crypto_aead_session_para parameter;
+    /*
+     * in header guest physical address, See struct virtio_crypto_sym_session_input
+     * above
+     */
+    le64 inhdr_addr;
+};
+\end{lstlisting}
+
+\paragraph{Session operation: create session}\label{sec:Device Types / Crypto Device / Device
+Operation / Session operation / Session operation: create session}
+
+A request of creating a session including the following information:
+
+\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      sym_destroy_session;
+    } u;
+};
+\end{lstlisting}
+
+\subparagraph{Driver Requirements: Session operation: create session}\label{sec:Device Types / Crypto Device / Device
+Operation / Session operation / Session operation: create session / Driver Requirements: Session operation: create session}
+
+The driver MUST set the control general header and corresponding property
+of union in structure virtio_crypto_op_ctrl_req. See \ref{sec:Device Types / Crypto Device / Device Operation}.
+Take the example of MAC service, the driver MUST set VIRTIO_CRYPTO_MAC_CREATE_SESSION
+for \field{opcode} field in struct virtio_crypto_op_ctrl_req, and set the \field{queue_id}
+to show dataq used.
+
+\subparagraph{Device Requirements: Session operation: create session}\label{sec:Device Types / Crypto Device / Device
+Operation / Session operation / Session operation: create session / Device Requirements: Session operation: create session}
+
+The device MUST return a session identifier to the driver when the device
+finishes the processing of session creation. The session creation request
+MUST end by a GPA of tailer: \field{inhdr_addr} field in struct virtio_crypto_*_create_session_req
+of each crypto service. The \field{inhdr_addr} field point to the below structure:
+
+\begin{lstlisting}
+struct virtio_crypto_sym_session_input {
+    u8     status;
+    le64    session_id;
+};
+\end{lstlisting}
+
+Both status and session_id are written by the device: either VIRTIO_CRYPTO_OP_OK for success,
+VIRTIO_CRYPTO_OP_ERR for creation failed or device error, VIRTIO_CRYPTO_OP_NOTSUPP for not support,
+VIRTIO_CRYPTO_OP_INVSESS for invalid session id when executing crypto operations.
+
+\begin{lstlisting}
+#define VIRTIO_CRYPTO_OP_OK        0
+#define VIRTIO_CRYPTO_OP_ERR       1
+#define VIRTIO_CRYPTO_OP_BADMSG    2
+#define VIRTIO_CRYPTO_OP_NOTSUPP   3
+#define VIRTIO_CRYPTO_OP_INVSESS   4
+\end{lstlisting}
+
+\paragraph{Session operation: destroy session}\label{sec:Device Types / Crypto Device / Device
+Operation / Session operation / Session operation: destroy session}
+
+A request which destroy a session including the following information:
+
+\begin{lstlisting}
+struct virtio_crypto_destroy_session_req {
+    le64    session_id; /* OUT parameter */
+    u8     status; /* IN parameter */
+};
+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      sym_destroy_session;
+    } u;
+};
+\end{lstlisting}
+
+\subparagraph{Driver Requirements: Session operation: destroy session}\label{sec:Device Types / Crypto Device / Device
+Operation / Session operation / Session operation: destroy session / Driver Requirements: Session operation: destroy session}
+
+The driver MUST set the control general header and corresponding property
+of union in struct virtio_crypto_op_ctrl_req. See \ref{sec:Device Types / Crypto Device / Device Operation}.
+Take the example of MAC service, the driver MUST set VIRTIO_CRYPTO_MAC_DESTROY_SESSION
+for \field{opcode} field in struct virtio_crypto_op_ctrl_req, and set the \field{queue_id} shows dataq used.
+The driver MUST set the \field{session_id} which MUST be a valid value which assigned by the
+device when a session was created.
+
+\subparagraph{Device Requirements: Session operation: destroy session}\label{sec:Device Types / Crypto Device / Device
+Operation / Session operation / Session operation: destroy session / Device Requirements: Session operation: destroy session}
+
+Status is written by the device: either VIRTIO_CRYPTO_OP_OK for success, VIRTIO_CRYPTO_OP_ERR for failure or device error.
+
+\subsubsection{Crypto operation}\label{sec:Device Types / Crypto Device / Device Operation / Crypto operation}
+
+The driver uses the dataq to finish the data plane operations (such as crypto operation).
+The packet of dataq:
+
+\begin{lstlisting}
+struct virtio_crypto_op_data_req {
+    struct virtio_crypto_op_header header;
+
+    union {
+        struct virtio_crypto_sym_data_req   sym_req;
+        struct virtio_crypto_hash_data_req  hash_req;
+        struct virtio_crypto_mac_data_req   mac_req;
+        struct virtio_crypto_aead_data_req  aead_req;
+    } u;
+};
+\end{lstlisting}
+
+The header is the general header, the union is algorithm-specific type,
+which are set by the driver. All the properties in the union will be shown as follow.
+
+\paragraph{Crypto operation: HASH operation}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: HASH operation}
+
+\begin{lstlisting}
+struct virtio_crypto_sym_crypt_op_inhdr {
+    u8    status;
+};
+
+struct virtio_crypto_hash_para {
+    /* length of source data */
+    le32 src_data_len;
+};
+
+struct virtio_crypto_hash_input {
+    le64 digest_result_addr; /* digest result guest physical address */
+    /*
+     * in header guest physical address, See struct virtio_crypto_sym_crypt_op_inhdr
+     * above
+     */
+    le64 inhdr_addr;
+};
+
+struct virtio_crypto_hash_output {
+    le64 src_data_addr; /* source data guest physical address */
+};
+
+struct virtio_crypto_hash_data_req {
+    struct virtio_crypto_hash_para parameter;
+    struct virtio_crypto_hash_input idata;
+    struct virtio_crypto_hash_output odata;
+};
+\end{lstlisting}
+
+Both input and output parameters store the guest physical address (GPA) only so
+that each data operation request only occupies one entry of the Vring descriptor
+table, which improves the throughput of data transferring for HASH crypto service.
+The struct virtio_crypto_hash_para store the corresponding parameters, such as
+\field{src_data_len}. The length and GPA can determine the specific content in
+the guest memory.
+
+\subparagraph{Driver Requirements: Crypto operation: HASH operation}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: HASH operation / Driver Requirements: Crypto operation: HASH operation}
+
+The driver MUST set the \field{opcode}, \field{session_id} in struct virtio_crypto_op_header.
+The \field{opcode} is set to VIRTIO_CRYPTO_HASH, \field{session_id} MUST be a valid value
+which assigned by the device when a session was created.
+The driver SHOULD set the \field{queue_id} field to show dataq used in struct virtio_crypto_op_header.
+The driver MUST fill out all fields in struct virtio_crypto_hash_data_req, including \field{parameter},
+\field{idata} and \field{odata} sub structures.
+
+Note: \field{inhdr_addr} is a GPA pointed the struct virtio_crypto_sym_crypt_op_inhdr
+which allocated by the driver.
+
+\subparagraph{Device Requirements: Crypto operation: HASH operation}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: HASH operation / Device Requirements: Crypto operation: HASH operation}
+
+The device MUST copy the result of hash operation recorded by \field{digest_result_addr}
+filed in struct virtio_crypto_hash_input.
+The device MUST set the status recorded by \field{inhdr_addr field} in strut virtio_crypto_hash_input
+which point struct virtio_crypto_sym_crypt_op_inhdr: either VIRTIO_CRYPTO_OP_OK for success,
+VIRTIO_CRYPTO_OP_ERR for creation failed or device error, VIRTIO_CRYPTO_OP_NOTSUPP for not support.
+
+\paragraph{Crypto operation: MAC operation}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: MAC operation}
+
+\begin{lstlisting}
+struct virtio_crypto_mac_para {
+    struct virtio_crypto_hash_para hash_para;
+};
+
+struct virtio_crypto_mac_input {
+    struct virtio_crypto_hash_input hash_input;
+};
+
+struct virtio_crypto_mac_output {
+    struct virtio_crypto_hash_output hash_output;
+};
+
+struct virtio_crypto_mac_data_req {
+    struct virtio_crypto_mac_para parameter;
+    struct virtio_crypto_mac_input idata;
+    struct virtio_crypto_mac_output odata;
+};
+\end{lstlisting}
+
+Both input and output parameters store the guest physical address (GPA) only so
+that each data operation request only occupies one entry of the Vring descriptor
+table, which improves the throughput of data transferring for MAC crypto service.
+The struct virtio_crypto_mac_para store the corresponding parameters, such as
+\field{src_data_len}. The length and GPA can determine the specific content in the guest memory.
+
+\subparagraph{Driver Requirements: Crypto operation: MAC operation}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: MAC operation / Driver Requirements: Crypto operation: MAC operation}
+
+Refer to the hash operation.
+The driver MUST set the \field{opcode} field to VIRTIO_CRYPTO_MAC.
+
+\subparagraph{Device Requirements: Crypto operation: MAC operation}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: MAC operation / Device Requirements: Crypto operation: MAC operation}
+
+Refer to the hash operation.
+
+\paragraph{Crypto operation: Symmetric algorithms operation}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: Symmetric algorithms operation}
+
+\begin{lstlisting}
+struct virtio_crypto_cipher_para {
+    le32 iv_len;
+    /* length of source data */
+    le32 src_data_len;
+    /* length of dst data */
+    le32 dst_data_len;
+};
+struct virtio_crypto_cipher_input {
+    le64 dst_data_addr; /* destination data guest physical address */
+    /*
+     * in header guest physical address, See struct virtio_crypto_sym_crypt_op_inhdr
+     * above
+     */
+    le64 inhdr_addr;
+};
+
+struct virtio_crypto_cipher_output {
+    le64 iv_addr; /* iv guest physical address */
+    le64 src_data_addr; /* source data guest physical address */
+};
+struct virtio_crypto_cipher_data_req {
+    struct virtio_crypto_cipher_para parameter;
+    struct virtio_crypto_cipher_input idata;
+    struct virtio_crypto_cipher_output odata;
+};
+struct virtio_crypto_sym_data_req {
+    struct virtio_crypto_cipher_data_req cipher_req;
+    union {
+        struct virtio_crypto_hash_data_req hash_req;
+        struct virtio_crypto_mac_data_req mac_req;
+    } u;
+};
+\end{lstlisting}
+
+Both input and output parameters store the guest physical address (GPA) only so that
+each data operation request only occupies one entry of the Vring descriptor table,
+which improves the throughput of data transferring for symmetric algorithms.
+The struct virtio_crypto_cipher_para store the corresponding parameters,
+such as \field{src_data_len}. The length and GPA can determine the specific content in the guest memory.
+
+\subparagraph{Driver Requirements: Crypto operation: Symmetric algorithms encryption}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: Symmetric algorithms operation / Driver Requirements: Crypto operation: Symmetric algorithms encryption}
+
+Refer to the hash operation.
+The driver MUST set the opcode field to VIRTIO_CRYPTO_CIPHER_ENCRYPT.
+The driver MUST fill out the fields in struct virtio_crypto_sym_data_req according to
+the operation type of the session. For example, if the created session is based
+on VIRTIO_CRYPTO_SYM_OP_CIPHER, that means the driver just SHOULD fill out fields
+of struct virtio_crypto_cipher_data_req in struct virtio_crypto_sym_data_req.
+If the created session is VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING type and
+VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH mode, that means the driver SHOULD fill out
+fields of both struct virtio_crypto_cipher_data_req and struct
+virtio_crypto_mac_data_req mac_req in struct virtio_crypto_sym_data_req.
+
+\subparagraph{Device Requirements: Crypto operation: Symmetric algorithms encryption}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: Symmetric algorithms operation / Device Requirements: Crypto operation: Symmetric algorithms encryption}
+
+The device MUST parse the virtio_crypto_sym_data_req according to the session_id in general header.
+For example, The device SHOULD only parsed 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.
+The driver SHOULD parse fields of both struct virtio_crypto_cipher_data_req and struct
+virtio_crypto_mac_data_req mac_req in struct virtio_crypto_sym_data_req if the created
+session is VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING operation type and VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH mode.
+
+The device MUST copy the result of encryption operation recorded by \filed{dst_data_addr} filed in struct virtio_crypto_cipher_input in plain cipher mode.
+The device MUST copy the result of hash operation recorded by \filed{digest_result_addr} filed in struct virtio_crypto_hash_input in chain hash/mac mode.
+The device MUST set the status recorded by \filed{inhdr_addr} field in strut virtio_crypto_cipher_input which point
+struct virtio_crypto_sym_crypt_op_inhdr: either VIRTIO_CRYPTO_OP_OK for success, VIRTIO_CRYPTO_OP_ERR for creation
+failed or device error, VIRTIO_CRYPTO_OP_NOTSUPP for not support.
+
+\subparagraph{Device Requirements: Crypto operation: Steps of encryption}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: Symmetric algorithms operation / Device Requirements: Crypto operation: Steps of encryption}
+
+Step1: Create a session:
+\begin{enumerate}
+\item The driver fills out the context message, including algorithm name, key, keylen etc;
+\item The driver sends a context message to the backend device by controlq;
+\item The device creates a session using the message transmitted by controlq;
+\item Return the session id to the driver.
+\end{enumerate}
+
+Step2: Execute the detail encryption operation:
+\begin{enumerate}
+\item The driver fills out the encrypt requests;
+\item Put the requests into dataq and kick the virtqueue;
+\item The device executes the encryption operation according the requests' arguments;
+\item The device returns the encryption result to the driver by dataq;
+\item The driver callback handle the result and over.
+\end{enumerate}
+
+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.
+
+\paragraph{Crypto operation: AEAD operation}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: AEAD operation}
+
+\begin{lstlisting}
+struct virtio_crypto_aead_para {
+    le32 iv_len;
+    /* length of additional auth data */
+    le32 aad_len;
+    /* length of source data */
+    le32 src_data_len;
+    /* length of dst data */
+    le32 dst_data_len;
+};
+
+struct virtio_crypto_aead_input {
+    le64 dst_data_addr; /* destination data guest physical address */
+    le64 digest_result_addr; /* digest result guest physical address */
+    /*
+     * in header guest physical address, See struct virtio_crypto_sym_crypt_op_inhdr
+     * above
+     */
+    le64 inhdr_addr;
+};
+
+struct virtio_crypto_aead_output {
+    le64 iv_addr; /* iv guest physical address */
+    le64 src_data_addr; /* source data guest physical address */
+    le64 add_data_addr; /* additional auth data guest physical address */
+};
+struct virtio_crypto_aead_data_req {
+    struct virtio_crypto_aead_para parameter;
+    struct virtio_crypto_aead_input idata;
+    struct virtio_crypto_aead_output odata;
+};
+\end{lstlisting}
+
+Both input and output parameters store the guest physical address (GPA) only so that
+each data operation request only occupies one entry of the Vring descriptor table,
+which improves the throughput of data transferring for AEAD crypto service. The
+struct virtio_crypto_aead_para store the corresponding parameters, such as \field{src_data_len}.
+The length and GPA can determine the specific content in the guest memory.
+
+\subparagraph{Driver Requirements: Crypto operation: AEAD encryption}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: AEAD operation / Driver Requirements: Crypto operation: AEAD encryption}
+
+Refer to the symmetric algorithms encryption operation.
+The driver MUST set the \field{opcode} field to VIRTIO_CRYPTO_AEAD_ENCRYPT.
+
+\subparagraph{Device Requirements: Crypto operation: AEAD encryption}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: AEAD operation / Device Requirements: Crypto operation: AEAD encryption}
+
+Refer to the symmetric algorithms encryption operation.
+
+\subparagraph{Driver Requirements: Crypto operation: AEAD decryption}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: AEAD operation / Driver Requirements: Crypto operation: AEAD decryption}
+
+Refer to the symmetric algorithms encryption operation.
+The driver MUST set the \field{opcode} field to VIRTIO_CRYPTO_AEAD_DECRYPT.
+
+\subparagraph{Device Requirements: Crypto operation: AEAD decryption}\label{sec:Device Types / Crypto Device / Device
+Operation / Crypto operation / Crypto operation: AEAD operation / Device Requirements: Crypto operation: AEAD decryption}
+
+The device MUST verify and return the verify result to the driver.
+If the verify result is not correct, VIRTIO_CRYPTO_OP_BADMSG (bad message)
+MUST be returned the driver.
-- 
1.7.12.4

[Qemu-devel] [PATCH v6 2/2] virtio-crypto: add conformance clauses

[Gonglei]

Add the conformance targets and clauses for
virtio-crypto device.

Signed-off-by: Gonglei <arei.gonglei@huawei.com>
---
 conformance.tex | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/conformance.tex b/conformance.tex
index f59e360..915a9f0 100644
--- a/conformance.tex
+++ b/conformance.tex
@@ -146,6 +146,21 @@ An SCSI host driver MUST conform to the following normative statements:
 \item \ref{drivernormative:Device Types / SCSI Host Device / Device Operation / Device Operation: eventq}
 \end{itemize}
 
+\subsection{Crypto Driver Conformance}\label{sec:Conformance / Driver Conformance / Crypto Driver Conformance}
+
+An Crypto driver MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{drivernormative:Device Types / Crypto Device / Device configuration layout / Driver Requirements: Device configuration layout}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Session operation / Session operation: create session / Driver Requirements: Session operation: create session}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Session operation / Session operation: destroy session / Driver Requirements: Session operation: destroy session}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Crypto operation / Crypto operation: HASH operation / Driver Requirements: Crypto operation: HASH operation}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Crypto operation / Crypto operation: MAC operation / Driver Requirements: Crypto operation: MAC operation}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Crypto operation / Crypto operation: Symmetric algorithms operation / Driver Requirements: Crypto operation: Symmetric algorithms encryption}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Crypto operation / Crypto operation: AEAD operation / Driver Requirements: Crypto operation: AEAD encryption}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Crypto operation / Crypto operation: AEAD operation / Driver Requirements: Crypto operation: AEAD decryption}
+\end{itemize}
+
 \section{Device Conformance}\label{sec:Conformance / Device Conformance}
 
 A device MUST conform to the following normative statements:
@@ -267,6 +282,21 @@ An SCSI host device MUST conform to the following normative statements:
 \item \ref{devicenormative:Device Types / SCSI Host Device / Device Operation / Device Operation: eventq}
 \end{itemize}
 
+\subsection{Crypto Device Conformance}\label{sec:Conformance / Device Conformance / Crypto Device Conformance}
+
+An Crypto device MUST conform to the following normative statements:
+
+\begin{itemize}
+\item \ref{drivernormative:Device Types / Crypto Device / Device configuration layout / Device Requirements: Device configuration layout}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Session operation / Session operation: create session / Device Requirements: Session operation: create session}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Session operation / Session operation: destroy session / Device Requirements: Session operation: destroy session}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Crypto operation / Crypto operation: HASH operation / Device Requirements: Crypto operation: HASH operation}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Crypto operation / Crypto operation: MAC operation / Device Requirements: Crypto operation: MAC operation}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Crypto operation / Crypto operation: Symmetric algorithms operation / Device Requirements: Crypto operation: Symmetric algorithms encryption}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Crypto operation / Crypto operation: AEAD operation / Device Requirements: Crypto operation: AEAD encryption}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Crypto operation / Crypto operation: AEAD operation / Device Requirements: Crypto operation: AEAD decryption}
+\end{itemize}
+
 \section{Legacy Interface: Transitional Device and
 Transitional Driver Conformance}\label{sec:Conformance / Legacy
 Interface: Transitional Device and 
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH v6 0/2] virtio-crypto: virtio crypto device specification

[Michael S. Tsirkin]

On Mon, Aug 01, 2016 at 05:20:19PM +0800, Gonglei wrote:
> This is the specification (version 6) about a new virtio crypto device.
> After a big reconstruction, the spec (symmetric algos) is near to stabilize.
> This version fix some problems of formating and return value, etc.
> 
> If you have any comments, please let me know, thanks :)

You might want to open a jira tracker in oasis jira to add this.


> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Cornelia Huck <cornelia.huck@de.ibm.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Lingli Deng <denglingli@chinamobile.com>
> CC: Jani Kokkonen <Jani.Kokkonen@huawei.com>
> CC: Ola Liljedahl <Ola.Liljedahl@arm.com>
> CC: Varun Sethi <Varun.Sethi@freescale.com>
> CC: Zeng Xin <xin.zeng@intel.com>
> CC: Keating Brian <brian.a.keating@intel.com>
> CC: Ma Liang J <liang.j.ma@intel.com>
> CC: Griffin John <john.griffin@intel.com>
> CC: Hanweidong <hanweidong@huawei.com>
> 
> Changes from v6:
>  - add conformance clauses for virtio crypto device. [Michael]
>  - drop VIRTIO_CRYPTO_S_STARTED. [Michael]
>  - fix some characters problems. [Stefan]
>  - add a MAC algorithm, named VIRTIO_CRYPTO_MAC_ZUC_EIA3. [Zeng Xin]
>  - add the fourth return code, named VIRTIO_CRYPTO_OP_INVSESS used
>    for invalid session id when executing crypto operations.
>  - drop some gpu stuff forgot to delete. [Michael]
>  - convert tab to space all over the content.
> 
> Changes from v4:
>  - introduce crypto services into virtio crypto device. The services 
>    currently defined are CIPHER, MAC, HASH, AEAD, KDF, ASYM, PRIMITIVE.
>  - 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".
>  - redefine the algorithms and structure based on above crypto services.
>  - rearrange the title and subtitle
>  - Only support CIPHER, MAC, HASH and AEAD crypto services, and Xin will
>    focus KDF, ASYM and PRIMITIVE services.
>  - Some other corresponding fixes.
>  - Make a formal patch using tex type.
> 
> This version is a big reconstruction based on Zeng, Xin' comments, thanks a lot.
> 
> 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.
> 
> 
> Gonglei (2):
>   virtio-crypto: Add virtio crypto device specification
>   virtio-crypto: Add conformance clauses
> 
>  conformance.tex   |  30 +++
>  content.tex       |   2 +
>  virtio-crypto.tex | 793 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 825 insertions(+)
>  create mode 100644 virtio-crypto.tex
> 
> -- 
> 1.7.12.4
> 

Re: [Qemu-devel] [PATCH v6 0/2] virtio-crypto: virtio crypto device specification

[Cornelia Huck]

On Wed, 3 Aug 2016 17:30:22 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Mon, Aug 01, 2016 at 05:20:19PM +0800, Gonglei wrote:
> > This is the specification (version 6) about a new virtio crypto device.
> > After a big reconstruction, the spec (symmetric algos) is near to stabilize.
> > This version fix some problems of formating and return value, etc.
> > 
> > If you have any comments, please let me know, thanks :)
> 
> You might want to open a jira tracker in oasis jira to add this.

It may make sense to open one/many jira trackers to reserve the device
ids (for the various device types that have accumulated) at least.

Re: [Qemu-devel] [PATCH v6 0/2] virtio-crypto: virtio crypto device specification

[Michael S. Tsirkin]

On Wed, Aug 03, 2016 at 04:33:28PM +0200, Cornelia Huck wrote:
> On Wed, 3 Aug 2016 17:30:22 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Mon, Aug 01, 2016 at 05:20:19PM +0800, Gonglei wrote:
> > > This is the specification (version 6) about a new virtio crypto device.
> > > After a big reconstruction, the spec (symmetric algos) is near to stabilize.
> > > This version fix some problems of formating and return value, etc.
> > > 
> > > If you have any comments, please let me know, thanks :)
> > 
> > You might want to open a jira tracker in oasis jira to add this.
> 
> It may make sense to open one/many jira trackers to reserve the device
> ids (for the various device types that have accumulated) at least.

Yes! Can you do this please?
As a reminder for new people, the process works like this:

- when you want patches to be considered
  for spec inclusion, make sure there's a
  jira issue (if you don't have jira access, let me know pls),
  and include the issue number in patch submitted to mailing list, on a line
  by itself e.g.

VIRTIO-123

- summarize change in proposal field
  add link to patch in archives (gmane or oasis
  archives)

- in Fix Version/s, please include future versions which
  should include the fix.

- in environment, pls list who reported this, and if
  the comment was submitted on virtio-comments or
  the oasis web form, pls mention this.

- when you want people to vote,
  move issue to open state, and send me a reminder.


I have a tool that, given the issue number, will:

- copy title and number to ballot title.
- fill in versions to include change in question field.
- fill proposal is description field.

and will then start a 7-day ballot to approve inclusion
in relevant spec version(s).

-- 
MST

Re: [Qemu-devel] [PATCH v6 0/2] virtio-crypto: virtio crypto device specification

[Cornelia Huck]

On Wed, 3 Aug 2016 17:40:07 +0300
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Wed, Aug 03, 2016 at 04:33:28PM +0200, Cornelia Huck wrote:
> > On Wed, 3 Aug 2016 17:30:22 +0300
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > 
> > > On Mon, Aug 01, 2016 at 05:20:19PM +0800, Gonglei wrote:
> > > > This is the specification (version 6) about a new virtio crypto device.
> > > > After a big reconstruction, the spec (symmetric algos) is near to stabilize.
> > > > This version fix some problems of formating and return value, etc.
> > > > 
> > > > If you have any comments, please let me know, thanks :)
> > > 
> > > You might want to open a jira tracker in oasis jira to add this.
> > 
> > It may make sense to open one/many jira trackers to reserve the device
> > ids (for the various device types that have accumulated) at least.
> 
> Yes! Can you do this please?

Sure, will do.

Re: [Qemu-devel] [PATCH v6 0/2] virtio-crypto: virtio crypto device specification

[Zeng, Xin]

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Wednesday, August 3, 2016 10:30 PM
> To: Gonglei <arei.gonglei@huawei.com>
> Cc: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org;
> peter.huangpeng@huawei.com; luonengjun@huawei.com;
> cornelia.huck@de.ibm.com; stefanha@redhat.com;
> denglingli@chinamobile.com; Jani.Kokkonen@huawei.com;
> Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Zeng, Xin
> <xin.zeng@intel.com>; Keating, Brian A <brian.a.keating@intel.com>; Ma,
> Liang J <liang.j.ma@intel.com>; Griffin, John <john.griffin@intel.com>;
> hanweidong@huawei.com; weidong.huang@huawei.com
> Subject: Re: [PATCH v6 0/2] virtio-crypto: virtio crypto device specification
> 
> On Mon, Aug 01, 2016 at 05:20:19PM +0800, Gonglei wrote:
> > This is the specification (version 6) about a new virtio crypto device.
> > After a big reconstruction, the spec (symmetric algos) is near to stabilize.
> > This version fix some problems of formating and return value, etc.
> >
> > If you have any comments, please let me know, thanks :)
> 
> You might want to open a jira tracker in oasis jira to add this.
> 

Could we hold on for a few days? I am still preparing patchset
based on this one.

> 
> > CC: Michael S. Tsirkin <mst@redhat.com>
> > CC: Cornelia Huck <cornelia.huck@de.ibm.com>
> > CC: Stefan Hajnoczi <stefanha@redhat.com>
> > CC: Lingli Deng <denglingli@chinamobile.com>
> > CC: Jani Kokkonen <Jani.Kokkonen@huawei.com>
> > CC: Ola Liljedahl <Ola.Liljedahl@arm.com>
> > CC: Varun Sethi <Varun.Sethi@freescale.com>
> > CC: Zeng Xin <xin.zeng@intel.com>
> > CC: Keating Brian <brian.a.keating@intel.com>
> > CC: Ma Liang J <liang.j.ma@intel.com>
> > CC: Griffin John <john.griffin@intel.com>
> > CC: Hanweidong <hanweidong@huawei.com>
> >
> > Changes from v6:
> >  - add conformance clauses for virtio crypto device. [Michael]
> >  - drop VIRTIO_CRYPTO_S_STARTED. [Michael]
> >  - fix some characters problems. [Stefan]
> >  - add a MAC algorithm, named VIRTIO_CRYPTO_MAC_ZUC_EIA3. [Zeng Xin]
> >  - add the fourth return code, named VIRTIO_CRYPTO_OP_INVSESS used
> >    for invalid session id when executing crypto operations.
> >  - drop some gpu stuff forgot to delete. [Michael]
> >  - convert tab to space all over the content.
> >
> > Changes from v4:
> >  - introduce crypto services into virtio crypto device. The services
> >    currently defined are CIPHER, MAC, HASH, AEAD, KDF, ASYM, PRIMITIVE.
> >  - 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".
> >  - redefine the algorithms and structure based on above crypto services.
> >  - rearrange the title and subtitle
> >  - Only support CIPHER, MAC, HASH and AEAD crypto services, and Xin will
> >    focus KDF, ASYM and PRIMITIVE services.
> >  - Some other corresponding fixes.
> >  - Make a formal patch using tex type.
> >
> > This version is a big reconstruction based on Zeng, Xin' comments, thanks a
> lot.
> >
> > 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.
> >
> >
> > Gonglei (2):
> >   virtio-crypto: Add virtio crypto device specification
> >   virtio-crypto: Add conformance clauses
> >
> >  conformance.tex   |  30 +++
> >  content.tex       |   2 +
> >  virtio-crypto.tex | 793
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 825 insertions(+)
> >  create mode 100644 virtio-crypto.tex
> >
> > --
> > 1.7.12.4
> >

Re: [Qemu-devel] [PATCH v6 0/2] virtio-crypto: virtio crypto device specification

[Michael S. Tsirkin]

On Wed, Aug 03, 2016 at 03:17:06PM +0000, Zeng, Xin wrote:
> 
> 
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Wednesday, August 3, 2016 10:30 PM
> > To: Gonglei <arei.gonglei@huawei.com>
> > Cc: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org;
> > peter.huangpeng@huawei.com; luonengjun@huawei.com;
> > cornelia.huck@de.ibm.com; stefanha@redhat.com;
> > denglingli@chinamobile.com; Jani.Kokkonen@huawei.com;
> > Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Zeng, Xin
> > <xin.zeng@intel.com>; Keating, Brian A <brian.a.keating@intel.com>; Ma,
> > Liang J <liang.j.ma@intel.com>; Griffin, John <john.griffin@intel.com>;
> > hanweidong@huawei.com; weidong.huang@huawei.com
> > Subject: Re: [PATCH v6 0/2] virtio-crypto: virtio crypto device specification
> > 
> > On Mon, Aug 01, 2016 at 05:20:19PM +0800, Gonglei wrote:
> > > This is the specification (version 6) about a new virtio crypto device.
> > > After a big reconstruction, the spec (symmetric algos) is near to stabilize.
> > > This version fix some problems of formating and return value, etc.
> > >
> > > If you have any comments, please let me know, thanks :)
> > 
> > You might want to open a jira tracker in oasis jira to add this.
> > 
> 
> Could we hold on for a few days? I am still preparing patchset
> based on this one.

Of course. Please see

	process for voting and issue tracking for virtio spec

basically ideally the final proposal includes the # of the
issue it addesses, but as long as it's just material for
discussion, there's no rush to track it.

> > 
> > > CC: Michael S. Tsirkin <mst@redhat.com>
> > > CC: Cornelia Huck <cornelia.huck@de.ibm.com>
> > > CC: Stefan Hajnoczi <stefanha@redhat.com>
> > > CC: Lingli Deng <denglingli@chinamobile.com>
> > > CC: Jani Kokkonen <Jani.Kokkonen@huawei.com>
> > > CC: Ola Liljedahl <Ola.Liljedahl@arm.com>
> > > CC: Varun Sethi <Varun.Sethi@freescale.com>
> > > CC: Zeng Xin <xin.zeng@intel.com>
> > > CC: Keating Brian <brian.a.keating@intel.com>
> > > CC: Ma Liang J <liang.j.ma@intel.com>
> > > CC: Griffin John <john.griffin@intel.com>
> > > CC: Hanweidong <hanweidong@huawei.com>
> > >
> > > Changes from v6:
> > >  - add conformance clauses for virtio crypto device. [Michael]
> > >  - drop VIRTIO_CRYPTO_S_STARTED. [Michael]
> > >  - fix some characters problems. [Stefan]
> > >  - add a MAC algorithm, named VIRTIO_CRYPTO_MAC_ZUC_EIA3. [Zeng Xin]
> > >  - add the fourth return code, named VIRTIO_CRYPTO_OP_INVSESS used
> > >    for invalid session id when executing crypto operations.
> > >  - drop some gpu stuff forgot to delete. [Michael]
> > >  - convert tab to space all over the content.
> > >
> > > Changes from v4:
> > >  - introduce crypto services into virtio crypto device. The services
> > >    currently defined are CIPHER, MAC, HASH, AEAD, KDF, ASYM, PRIMITIVE.
> > >  - 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".
> > >  - redefine the algorithms and structure based on above crypto services.
> > >  - rearrange the title and subtitle
> > >  - Only support CIPHER, MAC, HASH and AEAD crypto services, and Xin will
> > >    focus KDF, ASYM and PRIMITIVE services.
> > >  - Some other corresponding fixes.
> > >  - Make a formal patch using tex type.
> > >
> > > This version is a big reconstruction based on Zeng, Xin' comments, thanks a
> > lot.
> > >
> > > 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.
> > >
> > >
> > > Gonglei (2):
> > >   virtio-crypto: Add virtio crypto device specification
> > >   virtio-crypto: Add conformance clauses
> > >
> > >  conformance.tex   |  30 +++
> > >  content.tex       |   2 +
> > >  virtio-crypto.tex | 793
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 825 insertions(+)
> > >  create mode 100644 virtio-crypto.tex
> > >
> > > --
> > > 1.7.12.4
> > >

Re: [Qemu-devel] [PATCH v6 0/2] virtio-crypto: virtio crypto device specification

[Cornelia Huck]

On Wed, 3 Aug 2016 16:53:36 +0200
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:

> On Wed, 3 Aug 2016 17:40:07 +0300
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
> > On Wed, Aug 03, 2016 at 04:33:28PM +0200, Cornelia Huck wrote:
> > > On Wed, 3 Aug 2016 17:30:22 +0300
> > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > 
> > > > On Mon, Aug 01, 2016 at 05:20:19PM +0800, Gonglei wrote:
> > > > > This is the specification (version 6) about a new virtio crypto device.
> > > > > After a big reconstruction, the spec (symmetric algos) is near to stabilize.
> > > > > This version fix some problems of formating and return value, etc.
> > > > > 
> > > > > If you have any comments, please let me know, thanks :)
> > > > 
> > > > You might want to open a jira tracker in oasis jira to add this.
> > > 
> > > It may make sense to open one/many jira trackers to reserve the device
> > > ids (for the various device types that have accumulated) at least.
> > 
> > Yes! Can you do this please?
> 
> Sure, will do.

Done (issues 147-150).

I'm just unsure what to put as target version :/

Re: [Qemu-devel] [PATCH v6 0/2] virtio-crypto: virtio crypto device specification

[Michael S. Tsirkin]

On Wed, Aug 03, 2016 at 07:23:36PM +0200, Cornelia Huck wrote:
> On Wed, 3 Aug 2016 16:53:36 +0200
> Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> 
> > On Wed, 3 Aug 2016 17:40:07 +0300
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > 
> > > On Wed, Aug 03, 2016 at 04:33:28PM +0200, Cornelia Huck wrote:
> > > > On Wed, 3 Aug 2016 17:30:22 +0300
> > > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > 
> > > > > On Mon, Aug 01, 2016 at 05:20:19PM +0800, Gonglei wrote:
> > > > > > This is the specification (version 6) about a new virtio crypto device.
> > > > > > After a big reconstruction, the spec (symmetric algos) is near to stabilize.
> > > > > > This version fix some problems of formating and return value, etc.
> > > > > > 
> > > > > > If you have any comments, please let me know, thanks :)
> > > > > 
> > > > > You might want to open a jira tracker in oasis jira to add this.
> > > > 
> > > > It may make sense to open one/many jira trackers to reserve the device
> > > > ids (for the various device types that have accumulated) at least.
> > > 
> > > Yes! Can you do this please?
> > 
> > Sure, will do.
> 
> Done (issues 147-150).
> 
> I'm just unsure what to put as target version :/

OK, let's discuss that. I'll send a summary of the
versioning and we can decide.

-- 
MST

Re: [Qemu-devel] [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification

[Zeng, Xin]

On Monday, August 01, 2016 5:20 PM, Gonglei Wrote:
> -----Original Message-----
> From: Gonglei [mailto:arei.gonglei@huawei.com]
> Sent: Monday, August 1, 2016 5:20 PM
> To: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org
> Cc: peter.huangpeng@huawei.com; luonengjun@huawei.com;
> mst@redhat.com; cornelia.huck@de.ibm.com; stefanha@redhat.com;
> denglingli@chinamobile.com; Jani.Kokkonen@huawei.com;
> Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Zeng, Xin
> <xin.zeng@intel.com>; Keating, Brian A <brian.a.keating@intel.com>; Ma,
> Liang J <liang.j.ma@intel.com>; Griffin, John <john.griffin@intel.com>;
> hanweidong@huawei.com; weidong.huang@huawei.com; Gonglei
> <arei.gonglei@huawei.com>
> Subject: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification
> 
> The virtio crypto device is a virtual crypto device (ie. hardware
> crypto accelerator card). The virtio crypto device can provide
> five crypto services: CIPHER, MAC, HASH, AEAD, KDF, ASYM, PRIMITIVE.
> 
> In this patch, CIPHER, MAC, HASH, AEAD services are introduced.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Cornelia Huck <cornelia.huck@de.ibm.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Lingli Deng <denglingli@chinamobile.com>
> CC: Jani Kokkonen <Jani.Kokkonen@huawei.com>
> CC: Ola Liljedahl <Ola.Liljedahl@arm.com>
> CC: Varun Sethi <Varun.Sethi@freescale.com>
> CC: Zeng Xin <xin.zeng@intel.com>
> CC: Keating Brian <brian.a.keating@intel.com>
> CC: Ma Liang J <liang.j.ma@intel.com>
> CC: Griffin John <john.griffin@intel.com>
> CC: Hanweidong <hanweidong@huawei.com>
> ---
>  content.tex       |   2 +
>  virtio-crypto.tex | 793
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 795 insertions(+)
>  create mode 100644 virtio-crypto.tex
> 
> diff --git a/content.tex b/content.tex
> index 4b45678..ab75f78 100644
> --- a/content.tex
> +++ b/content.tex
> @@ -5750,6 +5750,8 @@ descriptor for the \field{sense_len}, \field{residual},
>  \field{status_qualifier}, \field{status}, \field{response} and
>  \field{sense} fields.
> 
> +\input{virtio-crypto.tex}
> +
>  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> 
>  Currently there are three device-independent feature bits defined:
> diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> new file mode 100644
> index 0000000..9465de3
> --- /dev/null
> +++ b/virtio-crypto.tex
> @@ -0,0 +1,793 @@
> +\section{Crypto Device}\label{sec:Device Types / 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. The virtio crypto
> +device can provide seven crypto services: CIPHER, MAC, HASH, AEAD,
> +KDF, ASYM, PRIMITIVE.
> +
> +\subsection{Device ID}\label{sec:Device Types / Crypto Device / Device ID}
> +
> +20
> +
> +\subsection{Virtqueues}\label{sec:Device Types / Crypto Device /
> Virtqueues}
> +
> +\begin{description}
> +\item[0] dataq1
> +\item[\ldots]
> +\item[N-1] dataqN
> +\item[N] controlq
> +\end{description}
> +
> +N is set by \field{max_dataqueues} (\field{max_dataqueues} >= 1).
> +
> +\subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature
> bits}
> +  None currently defined
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / Crypto
> Device / Device configuration layout}
> +
> +Thirteen driver-read-only configuration fields are currently defined.
> +
> +\begin{lstlisting}
> +struct virtio_crypto_config {
> +    le32  version;
> +    le16  status;
> +    le16  max_dataqueues;
> +    le32  crypto_services;
> +    /* 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;
> +};
> +\end{lstlisting}
> +
> +The first driver-read-only field, \field{version} specifies the virtio crypto's
> +version, which is reserved for back-compatibility in future.It's currently
> +defined for the version field:
> +
> +\begin{lstlisting}
> +#define VIRTIO_CRYPTO_VERSION_1  (1)

Suggest to remove this macro, 
Do you think a version which is composed of major version and 
minor version is better?

> +\end{lstlisting}
> +
> +One read-only bit (for the device) is currently defined for the \field{status}
> +field: VIRTIO_CRYPTO_S_HW_READY.
> +
> +\begin{lstlisting}
> +#define VIRTIO_CRYPTO_S_HW_READY  (1 << 0)
> +\end{lstlisting}
> +
> +The following driver-read-only field, \field{max_dataqueuess} specifies the
> +maximum number of data virtqueues (dataq1\ldots dataqN). The
> crypto_services
> +shows the crypto services the virtio crypto supports. The service currently
> +defined are:
> +
> +\begin{lstlisting}
> +#define VIRTIO_CRYPTO_NO_SERVICE (0) /* cipher services */

Why is this bit needed?

> +#define VIRTIO_CRYPTO_SERVICE_CIPHER (1) /* cipher services */
> +#define VIRTIO_CRYPTO_SERVICE_HASH  (2) /* hash service */
> +#define VIRTIO_CRYPTO_SERVICE_MAC   (3) /* MAC (Message
> Authentication Codes) service */
> +#define VIRTIO_CRYPTO_SERVICE_AEAD  (4) /* AEAD(Authenticated
> Encryption with Associated Data) service */
> +\end{lstlisting}
> +
> +The last driver-read-only fields specify detailed algorithms mask which
> +the device offered for corresponding service. The below CIPHER algorithms
> +are defined currently:
> +
> +\begin{lstlisting}
> +#define VIRTIO_CRYPTO_NO_CIPHER                 0
> +#define VIRTIO_CRYPTO_CIPHER_ARC4               1
> +#define VIRTIO_CRYPTO_CIPHER_AES_ECB            2
> +#define VIRTIO_CRYPTO_CIPHER_AES_CBC            3
> +#define VIRTIO_CRYPTO_CIPHER_AES_CTR            4
> +#define VIRTIO_CRYPTO_CIPHER_DES_ECB            5
> +#define VIRTIO_CRYPTO_CIPHER_DES_CBC            6
> +#define VIRTIO_CRYPTO_CIPHER_3DES_ECB           7
> +#define VIRTIO_CRYPTO_CIPHER_3DES_CBC           8
> +#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
> +\end{lstlisting}
> +
> +The below HASH algorithms are defined currently:
> +
> +\begin{lstlisting}
> +#define VIRTIO_CRYPTO_NO_HASH            0
> +#define VIRTIO_CRYPTO_HASH_MD5           1
> +#define VIRTIO_CRYPTO_HASH_SHA1          2
> +#define VIRTIO_CRYPTO_HASH_SHA_224       3
> +#define VIRTIO_CRYPTO_HASH_SHA_256       4
> +#define VIRTIO_CRYPTO_HASH_SHA_384       5
> +#define VIRTIO_CRYPTO_HASH_SHA_512       6
> +#define VIRTIO_CRYPTO_HASH_SHA3_224      7
> +#define VIRTIO_CRYPTO_HASH_SHA3_256      8
> +#define VIRTIO_CRYPTO_HASH_SHA3_384      9
> +#define VIRTIO_CRYPTO_HASH_SHA3_512      10
> +#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE128      11
> +#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE256      12
> +\end{lstlisting}
> +
> +The below MAC algorithms are defined currently:
> +
> +\begin{lstlisting}
> +#define VIRTIO_CRYPTO_NO_MAC                       0
> +#define VIRTIO_CRYPTO_MAC_HMAC_MD5                 1
> +#define VIRTIO_CRYPTO_MAC_HMAC_SHA1                2
> +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_224             3
> +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_256             4
> +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_384             5
> +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_512             6
> +#define VIRTIO_CRYPTO_MAC_CMAC_3DES                25
> +#define VIRTIO_CRYPTO_MAC_CMAC_AES                 26
> +#define VIRTIO_CRYPTO_MAC_CMAC_KASUMI_F9           27
> +#define VIRTIO_CRYPTO_MAC_CMAC_SNOW3G_UIA2         28
> +#define VIRTIO_CRYPTO_MAC_GMAC_AES                 41
> +#define VIRTIO_CRYPTO_MAC_GMAC_TWOFISH             42
> +#define VIRTIO_CRYPTO_MAC_CBCMAC_AES               49
> +#define VIRTIO_CRYPTO_MAC_CBCMAC_KASUMI_F9         50
> +#define VIRTIO_CRYPTO_MAC_XCBC_AES                 53
> +#define VIRTIO_CRYPTO_MAC_ZUC_EIA3                 54
> +\end{lstlisting}
> +
> +The below AEAD algorithms are defined currently:
> +
> +\begin{lstlisting}
> +#define VIRTIO_CRYPTO_NO_AEAD     0
> +#define VIRTIO_CRYPTO_AEAD_GCM    1
> +#define VIRTIO_CRYPTO_AEAD_CCM    2
> +#define VIRTIO_CRYPTO_AEAD_CHACHA20_POLY1305  3
> +\end{lstlisting}
> +
> +\subsubsection{Device Requirements: Device configuration
> layout}\label{sec:Device Types / Crypto Device / Device configuration layout
> / Device Requirements: Device configuration layout}
> +
> +\begin{itemize*}
> +\item The device MUST set \field{version} in version filed.
> +\item The device MUST set \field{max_dataqueues} to between 1 and
> 65535 inclusive.
> +\item The device SHOULD set \field{status} according to the status of the
> hardware-backed implementation.
> +\item The device MUST set \field{crypto_services} according to the crypto
> services which the device offered.
> +\item The device MUST set detailed algorithms mask according to
> crypto_services field.
> +\end{itemize*}
> +
> +\subsubsection{Driver Requirements: Device configuration
> layout}\label{sec:Device Types / Crypto Device / Device configuration layout
> / Driver Requirements: Device configuration layout}
> +
> +\begin{itemize*}
> +\item The driver MUST read the ready \field{status} from the bottom bit of
> status to
> +      check whether the hardware-backed implementation is ready or not.
> +\item The driver MAY read \field{max_dataqueues} field to discover how
> many data queues the device supports.
> +\item The driver MUST read \field{crypto_services} field to discover which
> services the device is able to offer.
> +\item The driver MUST read detail algorithms field according to
> crypto_services field.
> +\end{itemize*}
> +
> +\subsection{Device Initialization}{Device Types / Crypto Device / Device
> Initialization}
> +
> +The driver SHOULD perform a typical initialization routine like so:
> +
> +\begin{enumerate}
> +\item Identify and initialize data virtqueue, up to \field{max_dataqueues}.
> +\item Identify the control virtqueue.
> +\item Identify the ready status of hardware-backend comes from the
> \field{status}.
> +\item Read the supported crypto services from bits of
> \field{crypto_servies}.
> +\item Read the supported algorithms according to \field{crypto_services}
> field.
> +\end{enumerate}
> +
> +\subsection{Device Operation}\label{sec:Device Types / Crypto Device /
> Device Operation}
> +
> +Packets can be transmitted by placing them in both the controlq and dataq.
> +The packet are consisted of general header and services 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 parameters are algorithm-specific parameters, input data is the
> +data should be operated , output data is the "operation result + result
> buffer".

According to following sections, input are device-writable.
output are device-readable. So need to update the description 
according to following sections.

> +The general header of controlq:
> +
> +\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;
> +    /* control flag to control the request */
> +    u8 flag;
> +    /* data virtqeueu id */
> +    le16 queue_id;
> +};
> +\end{lstlisting}
> +
> +The general header of dataq:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_op_header {
> +#define VIRTIO_CRYPTO_CIPHER_ENCRYPT
> VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x00)
> +#define VIRTIO_CRYPTO_CIPHER_DECRYPT
> VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x01)
> +#define VIRTIO_CRYPTO_HASH
> VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x00)
> +#define VIRTIO_CRYPTO_MAC
> VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x00)
> +#define VIRTIO_CRYPTO_AEAD_ENCRYPT
> VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x00)
> +#define VIRTIO_CRYPTO_AEAD_DECRYPT
> VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x01)
> +    le32 opcode;
> +    /* algo should be service-specific algorithms */
> +    le32 algo;
> +    /* control flag to control the request */
> +    u8 flag;

Here we could define flag u16 to make structure alignment.

> +    /* data virtqeueu id */
> +    le16 queue_id;
> +    /* session_id should be service-specific algorithms */
> +    le64 session_id;
> +};
> +\end{lstlisting}
> +
> +\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 which finish the non-data plane operations, such as session
> +operations (see \ref{sec:Device Types / Crypto Device / Device Operation /
> Session operation}).
> +The packet of controlq:
> +
> +\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      sym_destroy_session;
> +    } u;
> +};
> +\end{lstlisting}
> +
> +The header is the general header, the union is algorithm-specific type,
> +which is set by the driver. All the properties in the union will be shown as
> follow.
> +
> +\subsubsection{Session operation}\label{sec:Device Types / Crypto Device
> / Device Operation / 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:
> +
> +\begin{enumerate}
> +\item The operation (cipher, hash/mac or both, and if both, the order in
> +      which the algorithms should be applied).
> +\item The cipher setup data, including the cipher algorithm and mode,
> +      the key and its length, and the direction (encrypt or decrypt).
> +\item The hash/mac setup data, including the hash algorithm or mac
> algorithm,
> +      and digest result length (to allow for truncation).
> +\begin{itemize*}
> +\item Authenticated mode can refer to MAC, which requires that the key
> and
> +      its length are also specified.
> +\item For nested mode, the inner and outer prefix data and length are
> specified,
> +      as well as the outer hash algorithm.
> +\end{itemize*}
> +\end{enumerate}
> +
> +\paragraph{Session operation: HASH session}\label{sec:Device Types /
> Crypto Device / Device Operation / Session operation / Session operation:
> HASH session}
> +
> +The packet of HASH session is:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_sym_session_input {
> +    u8     status;
> +    le64    session_id;
> +};
> +
> +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 {
> +    struct virtio_crypto_hash_session_para parameter;
> +    /*
> +     * in header guest physical address, See struct
> virtio_crypto_sym_session_input
> +     * above
> +     */
> +    le64 inhdr_addr;
> +};
> +\end{lstlisting}
> +

Does the inhdr_addr point to a virtio_crypto_sym_session_input structure?
Why not put virtio_crypto_sym_session_input into virtio_crypto_hash_session_para
directly? 
If inhdr_addr point to a physical address, if the address is not physically
contiguous, how does the backend device handle?
Same comments to MAC/SYM/AEAD session creation.

> +\paragraph{Session operation: MAC session}\label{sec:Device Types /
> Crypto Device / Device
> +Operation / Session operation / Session operation: MAC session}
> +
> +The packet of MAC session is:
> +
> +\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;
> +};
> +struct virtio_crypto_mac_create_session_req {
> +    struct virtio_crypto_mac_session_para  parameter;
> +    /*
> +     * in header guest physical address, See struct
> virtio_crypto_sym_session_input
> +     * above
> +     */
> +    le64 inhdr_addr;
> +};
> +\end{lstlisting}
> +
> +\paragraph{Session operation: Symmetric algorithms
> session}\label{sec:Device Types / Crypto Device / Device
> +Operation / Session operation / Session operation: Symmetric algorithms
> session}
> +
> +The symmetric session include both ciphers (CIPHER service) and chain
> +algorithms (chain cipher and hash/mac). The packet of symmetric session is:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_cipher_session_para {
> +/* See VIRTIO_CRYPTO_CIPHER* above */
> +    le32 algo;
> +    /* length of key */
> +    le32 keylen;
> +    /* encrypt or decrypt */
> +    u8 op;
> +};
> +struct virtio_crypto_sym_create_session_req {
> +/* 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
> +    u8 op_type;
> +
> +    union {
> +        struct virtio_crypto_cipher_session_para cipher_param;
> +        struct virtio_crypto_alg_chain_param {
> +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER
> 1
> +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH
> 2
> +            u8 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
> +            u8 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;
> +        } chain;
> +    } sym;
> +
> +    /*
> +     * in header guest physical address, See struct
> virtio_crypto_sym_session_input
> +     * above
> +     */
> +    le64 inhdr_addr;
> +};
> +\end{lstlisting}
> +
> +\paragraph{Session operation: AEAD session}\label{sec:Device Types /
> Crypto Device / Device
> +Operation / Session operation / Session operation: AEAD session}
> +
> +The packet of AEAD session is:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_aead_session_para {
> +    /* See VIRTIO_CRYPTO_AEAD_* above*/
> +    u8 algo;
> +    /* length of key */
> +    le32 key_len;
> +    /* digest result length */
> +    le32 digest_result_len;
> +    /* The length of the additional authenticated data (AAD) in bytes */
> +    le32 aad_len;
> +    /* encrypt or decrypt */
> +    u8 op;
> +};
> +struct virtio_crypto_aead_create_session_req {
> +    struct virtio_crypto_aead_session_para parameter;
> +    /*
> +     * in header guest physical address, See struct
> virtio_crypto_sym_session_input
> +     * above
> +     */
> +    le64 inhdr_addr;
> +};
> +\end{lstlisting}
> +
> +\paragraph{Session operation: create session}\label{sec:Device Types /
> Crypto Device / Device
> +Operation / Session operation / Session operation: create session}
> +
> +A request of creating a session including the following information:
> +
> +\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      sym_destroy_session;
> +    } u;
> +};
> +\end{lstlisting}
> +
> +\subparagraph{Driver Requirements: Session operation: create
> session}\label{sec:Device Types / Crypto Device / Device
> +Operation / Session operation / Session operation: create session / Driver
> Requirements: Session operation: create session}
> +
> +The driver MUST set the control general header and corresponding
> property
> +of union in structure virtio_crypto_op_ctrl_req. See \ref{sec:Device Types /
> Crypto Device / Device Operation}.
> +Take the example of MAC service, the driver MUST set
> VIRTIO_CRYPTO_MAC_CREATE_SESSION
> +for \field{opcode} field in struct virtio_crypto_op_ctrl_req, and set the
> \field{queue_id}
> +to show dataq used.
> +
> +\subparagraph{Device Requirements: Session operation: create
> session}\label{sec:Device Types / Crypto Device / Device
> +Operation / Session operation / Session operation: create session / Device
> Requirements: Session operation: create session}
> +
> +The device MUST return a session identifier to the driver when the device
> +finishes the processing of session creation. The session creation request
> +MUST end by a GPA of tailer: \field{inhdr_addr} field in struct
> virtio_crypto_*_create_session_req
> +of each crypto service. The \field{inhdr_addr} field point to the below
> structure:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_sym_session_input {
> +    u8     status;
> +    le64    session_id;
> +};
> +\end{lstlisting}
> +
> +Both status and session_id are written by the device: either
> VIRTIO_CRYPTO_OP_OK for success,
> +VIRTIO_CRYPTO_OP_ERR for creation failed or device error,
> VIRTIO_CRYPTO_OP_NOTSUPP for not support,
> +VIRTIO_CRYPTO_OP_INVSESS for invalid session id when executing crypto
> operations.
> +
> +\begin{lstlisting}
> +#define VIRTIO_CRYPTO_OP_OK        0
> +#define VIRTIO_CRYPTO_OP_ERR       1
> +#define VIRTIO_CRYPTO_OP_BADMSG    2
> +#define VIRTIO_CRYPTO_OP_NOTSUPP   3
> +#define VIRTIO_CRYPTO_OP_INVSESS   4
> +\end{lstlisting}
> +
> +\paragraph{Session operation: destroy session}\label{sec:Device Types /
> Crypto Device / Device
> +Operation / Session operation / Session operation: destroy session}
> +
> +A request which destroy a session including the following information:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_destroy_session_req {
> +    le64    session_id; /* OUT parameter */
> +    u8     status; /* IN parameter */
> +};
> +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      sym_destroy_session;
> +    } u;
> +};
> +\end{lstlisting}
> +
> +\subparagraph{Driver Requirements: Session operation: destroy
> session}\label{sec:Device Types / Crypto Device / Device
> +Operation / Session operation / Session operation: destroy session / Driver
> Requirements: Session operation: destroy session}
> +
> +The driver MUST set the control general header and corresponding
> property
> +of union in struct virtio_crypto_op_ctrl_req. See \ref{sec:Device Types /
> Crypto Device / Device Operation}.
> +Take the example of MAC service, the driver MUST set
> VIRTIO_CRYPTO_MAC_DESTROY_SESSION
> +for \field{opcode} field in struct virtio_crypto_op_ctrl_req, and set the
> \field{queue_id} shows dataq used.
> +The driver MUST set the \field{session_id} which MUST be a valid value
> which assigned by the
> +device when a session was created.
> +
> +\subparagraph{Device Requirements: Session operation: destroy
> session}\label{sec:Device Types / Crypto Device / Device
> +Operation / Session operation / Session operation: destroy session / Device
> Requirements: Session operation: destroy session}
> +
> +Status is written by the device: either VIRTIO_CRYPTO_OP_OK for success,
> VIRTIO_CRYPTO_OP_ERR for failure or device error.
> +
> +\subsubsection{Crypto operation}\label{sec:Device Types / Crypto Device /
> Device Operation / Crypto operation}
> +
> +The driver uses the dataq to finish the data plane operations (such as crypto
> operation).
> +The packet of dataq:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_op_data_req {
> +    struct virtio_crypto_op_header header;
> +
> +    union {
> +        struct virtio_crypto_sym_data_req   sym_req;
> +        struct virtio_crypto_hash_data_req  hash_req;
> +        struct virtio_crypto_mac_data_req   mac_req;
> +        struct virtio_crypto_aead_data_req  aead_req;
> +    } u;
> +};
> +\end{lstlisting}
> +
> +The header is the general header, the union is algorithm-specific type,
> +which are set by the driver. All the properties in the union will be shown as
> follow.
> +
> +\paragraph{Crypto operation: HASH operation}\label{sec:Device Types /
> Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: HASH operation}
> +
> +\begin{lstlisting}
> +struct virtio_crypto_sym_crypt_op_inhdr {
> +    u8    status;
> +};
> +
> +struct virtio_crypto_hash_para {
> +    /* length of source data */
> +    le32 src_data_len;
> +};
> +

src_data_len can be put in virtio_crypto_hash_input.
If a request doesn't need a parameter, we can just
ignore it.

> +struct virtio_crypto_hash_input {
> +    le64 digest_result_addr; /* digest result guest physical address */
> +    /*
> +     * in header guest physical address, See struct
> virtio_crypto_sym_crypt_op_inhdr
> +     * above
> +     */
> +    le64 inhdr_addr;
> +};
> +
> +struct virtio_crypto_hash_output {
> +    le64 src_data_addr; /* source data guest physical address */
> +};
> +
> +struct virtio_crypto_hash_data_req {
> +    struct virtio_crypto_hash_para parameter;
> +    struct virtio_crypto_hash_input idata;
> +    struct virtio_crypto_hash_output odata;
> +};
> +\end{lstlisting}
> +
> +Both input and output parameters store the guest physical address (GPA)
> only so
> +that each data operation request only occupies one entry of the Vring
> descriptor
> +table, which improves the throughput of data transferring for HASH crypto
> service.
> +The struct virtio_crypto_hash_para store the corresponding parameters,
> such as
> +\field{src_data_len}. The length and GPA can determine the specific
> content in
> +the guest memory.
> +
> +\subparagraph{Driver Requirements: Crypto operation: HASH
> operation}\label{sec:Device Types / Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: HASH operation / Driver
> Requirements: Crypto operation: HASH operation}
> +
> +The driver MUST set the \field{opcode}, \field{session_id} in struct
> virtio_crypto_op_header.
> +The \field{opcode} is set to VIRTIO_CRYPTO_HASH, \field{session_id} MUST
> be a valid value
> +which assigned by the device when a session was created.
> +The driver SHOULD set the \field{queue_id} field to show dataq used in
> struct virtio_crypto_op_header.
> +The driver MUST fill out all fields in struct virtio_crypto_hash_data_req,
> including \field{parameter},
> +\field{idata} and \field{odata} sub structures.
> +
> +Note: \field{inhdr_addr} is a GPA pointed the struct
> virtio_crypto_sym_crypt_op_inhdr
> +which allocated by the driver.
> +
> +\subparagraph{Device Requirements: Crypto operation: HASH
> operation}\label{sec:Device Types / Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: HASH operation / Device
> Requirements: Crypto operation: HASH operation}
> +
> +The device MUST copy the result of hash operation recorded by
> \field{digest_result_addr}
> +filed in struct virtio_crypto_hash_input.
> +The device MUST set the status recorded by \field{inhdr_addr field} in strut
> virtio_crypto_hash_input
> +which point struct virtio_crypto_sym_crypt_op_inhdr: either
> VIRTIO_CRYPTO_OP_OK for success,
> +VIRTIO_CRYPTO_OP_ERR for creation failed or device error,
> VIRTIO_CRYPTO_OP_NOTSUPP for not support.
> +
> +\paragraph{Crypto operation: MAC operation}\label{sec:Device Types /
> Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: MAC operation}
> +
> +\begin{lstlisting}
> +struct virtio_crypto_mac_para {
> +    struct virtio_crypto_hash_para hash_para;
> +};
> +
> +struct virtio_crypto_mac_input {
> +    struct virtio_crypto_hash_input hash_input;
> +};
> +
> +struct virtio_crypto_mac_output {
> +    struct virtio_crypto_hash_output hash_output;
> +};
> +
> +struct virtio_crypto_mac_data_req {
> +    struct virtio_crypto_mac_para parameter;
> +    struct virtio_crypto_mac_input idata;
> +    struct virtio_crypto_mac_output odata;
> +};
> +\end{lstlisting}
> +
> +Both input and output parameters store the guest physical address (GPA)
> only so
> +that each data operation request only occupies one entry of the Vring
> descriptor
> +table, which improves the throughput of data transferring for MAC crypto
> service.
> +The struct virtio_crypto_mac_para store the corresponding parameters,
> such as
> +\field{src_data_len}. The length and GPA can determine the specific
> content in the guest memory.
> +
> +\subparagraph{Driver Requirements: Crypto operation: MAC
> operation}\label{sec:Device Types / Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: MAC operation / Driver
> Requirements: Crypto operation: MAC operation}
> +
> +Refer to the hash operation.
> +The driver MUST set the \field{opcode} field to VIRTIO_CRYPTO_MAC.
> +
> +\subparagraph{Device Requirements: Crypto operation: MAC
> operation}\label{sec:Device Types / Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: MAC operation / Device
> Requirements: Crypto operation: MAC operation}
> +
> +Refer to the hash operation.
> +
> +\paragraph{Crypto operation: Symmetric algorithms
> operation}\label{sec:Device Types / Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: Symmetric algorithms
> operation}
> +
> +\begin{lstlisting}
> +struct virtio_crypto_cipher_para {
> +    le32 iv_len;
> +    /* length of source data */
> +    le32 src_data_len;
> +    /* length of dst data */
> +    le32 dst_data_len;
> +};
> +struct virtio_crypto_cipher_input {
> +    le64 dst_data_addr; /* destination data guest physical address */
> +    /*
> +     * in header guest physical address, See struct
> virtio_crypto_sym_crypt_op_inhdr
> +     * above
> +     */
> +    le64 inhdr_addr;
> +};
> +
> +struct virtio_crypto_cipher_output {
> +    le64 iv_addr; /* iv guest physical address */
> +    le64 src_data_addr; /* source data guest physical address */
> +};
> +struct virtio_crypto_cipher_data_req {
> +    struct virtio_crypto_cipher_para parameter;
> +    struct virtio_crypto_cipher_input idata;
> +    struct virtio_crypto_cipher_output odata;
> +};
> +struct virtio_crypto_sym_data_req {
> +    struct virtio_crypto_cipher_data_req cipher_req;
> +    union {
> +        struct virtio_crypto_hash_data_req hash_req;
> +        struct virtio_crypto_mac_data_req mac_req;
> +    } u;
> +};
> +\end{lstlisting}
> +
> +Both input and output parameters store the guest physical address (GPA)
> only so that
> +each data operation request only occupies one entry of the Vring descriptor
> table,
> +which improves the throughput of data transferring for symmetric
> algorithms.
> +The struct virtio_crypto_cipher_para store the corresponding parameters,
> +such as \field{src_data_len}. The length and GPA can determine the specific
> content in the guest memory.
> +
> +\subparagraph{Driver Requirements: Crypto operation: Symmetric
> algorithms encryption}\label{sec:Device Types / Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: Symmetric algorithms
> operation / Driver Requirements: Crypto operation: Symmetric algorithms
> encryption}
> +
> +Refer to the hash operation.
> +The driver MUST set the opcode field to VIRTIO_CRYPTO_CIPHER_ENCRYPT.
> +The driver MUST fill out the fields in struct virtio_crypto_sym_data_req
> according to
> +the operation type of the session. For example, if the created session is
> based
> +on VIRTIO_CRYPTO_SYM_OP_CIPHER, that means the driver just SHOULD
> fill out fields
> +of struct virtio_crypto_cipher_data_req in struct
> virtio_crypto_sym_data_req.
> +If the created session is VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING
> type and
> +VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH mode, that means the driver
> SHOULD fill out
> +fields of both struct virtio_crypto_cipher_data_req and struct
> +virtio_crypto_mac_data_req mac_req in struct virtio_crypto_sym_data_req.
> +
> +\subparagraph{Device Requirements: Crypto operation: Symmetric
> algorithms encryption}\label{sec:Device Types / Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: Symmetric algorithms
> operation / Device Requirements: Crypto operation: Symmetric algorithms
> encryption}
> +
> +The device MUST parse the virtio_crypto_sym_data_req according to the
> session_id in general header.
> +For example, The device SHOULD only parsed 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.
> +The driver SHOULD parse fields of both struct
> virtio_crypto_cipher_data_req and struct
> +virtio_crypto_mac_data_req mac_req in struct virtio_crypto_sym_data_req
> if the created
> +session is VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING operation
> type and VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH mode.
> +
> +The device MUST copy the result of encryption operation recorded by
> \filed{dst_data_addr} filed in struct virtio_crypto_cipher_input in plain cipher
> mode.
> +The device MUST copy the result of hash operation recorded by
> \filed{digest_result_addr} filed in struct virtio_crypto_hash_input in chain
> hash/mac mode.
> +The device MUST set the status recorded by \filed{inhdr_addr} field in strut
> virtio_crypto_cipher_input which point
> +struct virtio_crypto_sym_crypt_op_inhdr: either VIRTIO_CRYPTO_OP_OK
> for success, VIRTIO_CRYPTO_OP_ERR for creation
> +failed or device error, VIRTIO_CRYPTO_OP_NOTSUPP for not support.
> +
> +\subparagraph{Device Requirements: Crypto operation: Steps of
> encryption}\label{sec:Device Types / Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: Symmetric algorithms
> operation / Device Requirements: Crypto operation: Steps of encryption}
> +
> +Step1: Create a session:
> +\begin{enumerate}
> +\item The driver fills out the context message, including algorithm name,
> key, keylen etc;
> +\item The driver sends a context message to the backend device by
> controlq;
> +\item The device creates a session using the message transmitted by
> controlq;
> +\item Return the session id to the driver.
> +\end{enumerate}
> +
> +Step2: Execute the detail encryption operation:
> +\begin{enumerate}
> +\item The driver fills out the encrypt requests;
> +\item Put the requests into dataq and kick the virtqueue;
> +\item The device executes the encryption operation according the
> requests' arguments;
> +\item The device returns the encryption result to the driver by dataq;
> +\item The driver callback handle the result and over.
> +\end{enumerate}
> +
> +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.
> +
> +\paragraph{Crypto operation: AEAD operation}\label{sec:Device Types /
> Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: AEAD operation}
> +
> +\begin{lstlisting}
> +struct virtio_crypto_aead_para {
> +    le32 iv_len;
> +    /* length of additional auth data */
> +    le32 aad_len;
> +    /* length of source data */
> +    le32 src_data_len;
> +    /* length of dst data */
> +    le32 dst_data_len;
> +};
> +
> +struct virtio_crypto_aead_input {
> +    le64 dst_data_addr; /* destination data guest physical address */
> +    le64 digest_result_addr; /* digest result guest physical address */
> +    /*
> +     * in header guest physical address, See struct
> virtio_crypto_sym_crypt_op_inhdr
> +     * above
> +     */
> +    le64 inhdr_addr;
> +};
> +
> +struct virtio_crypto_aead_output {
> +    le64 iv_addr; /* iv guest physical address */
> +    le64 src_data_addr; /* source data guest physical address */
> +    le64 add_data_addr; /* additional auth data guest physical address */
> +};
> +struct virtio_crypto_aead_data_req {
> +    struct virtio_crypto_aead_para parameter;
> +    struct virtio_crypto_aead_input idata;
> +    struct virtio_crypto_aead_output odata;
> +};
> +\end{lstlisting}
> +
> +Both input and output parameters store the guest physical address (GPA)
> only so that
> +each data operation request only occupies one entry of the Vring descriptor
> table,
> +which improves the throughput of data transferring for AEAD crypto service.
> The
> +struct virtio_crypto_aead_para store the corresponding parameters, such
> as \field{src_data_len}.
> +The length and GPA can determine the specific content in the guest
> memory.
> +
> +\subparagraph{Driver Requirements: Crypto operation: AEAD
> encryption}\label{sec:Device Types / Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: AEAD operation / Driver
> Requirements: Crypto operation: AEAD encryption}
> +
> +Refer to the symmetric algorithms encryption operation.
> +The driver MUST set the \field{opcode} field to
> VIRTIO_CRYPTO_AEAD_ENCRYPT.
> +
> +\subparagraph{Device Requirements: Crypto operation: AEAD
> encryption}\label{sec:Device Types / Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: AEAD operation / Device
> Requirements: Crypto operation: AEAD encryption}
> +
> +Refer to the symmetric algorithms encryption operation.
> +
> +\subparagraph{Driver Requirements: Crypto operation: AEAD
> decryption}\label{sec:Device Types / Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: AEAD operation / Driver
> Requirements: Crypto operation: AEAD decryption}
> +
> +Refer to the symmetric algorithms encryption operation.
> +The driver MUST set the \field{opcode} field to
> VIRTIO_CRYPTO_AEAD_DECRYPT.
> +
> +\subparagraph{Device Requirements: Crypto operation: AEAD
> decryption}\label{sec:Device Types / Crypto Device / Device
> +Operation / Crypto operation / Crypto operation: AEAD operation / Device
> Requirements: Crypto operation: AEAD decryption}
> +
> +The device MUST verify and return the verify result to the driver.
> +If the verify result is not correct, VIRTIO_CRYPTO_OP_BADMSG (bad
> message)
> +MUST be returned the driver.
> --
> 1.7.12.4
> 

Re: [Qemu-devel] [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification

[Gonglei (Arei)]

Hi Xin,

Thanks for your feedback! Please see my embedded reply.

Regards,
-Gonglei


> -----Original Message-----
> From: Zeng, Xin [mailto:xin.zeng@intel.com]
> Sent: Thursday, August 04, 2016 3:48 PM
> To: Gonglei (Arei); qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org
> Cc: Huangpeng (Peter); Luonengjun; mst@redhat.com;
> cornelia.huck@de.ibm.com; stefanha@redhat.com;
> denglingli@chinamobile.com; Jani Kokkonen; Ola.Liljedahl@arm.com;
> Varun.Sethi@freescale.com; Keating, Brian A; Ma, Liang J; Griffin, John;
> Hanweidong (Randy); Huangweidong (C)
> Subject: RE: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification
> 
> On Monday, August 01, 2016 5:20 PM, Gonglei Wrote:
> > -----Original Message-----
> > From: Gonglei [mailto:arei.gonglei@huawei.com]
> > Sent: Monday, August 1, 2016 5:20 PM
> > To: qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org
> > Cc: peter.huangpeng@huawei.com; luonengjun@huawei.com;
> > mst@redhat.com; cornelia.huck@de.ibm.com; stefanha@redhat.com;
> > denglingli@chinamobile.com; Jani.Kokkonen@huawei.com;
> > Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Zeng, Xin
> > <xin.zeng@intel.com>; Keating, Brian A <brian.a.keating@intel.com>; Ma,
> > Liang J <liang.j.ma@intel.com>; Griffin, John <john.griffin@intel.com>;
> > hanweidong@huawei.com; weidong.huang@huawei.com; Gonglei
> > <arei.gonglei@huawei.com>
> > Subject: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification
> >
> > The virtio crypto device is a virtual crypto device (ie. hardware
> > crypto accelerator card). The virtio crypto device can provide
> > five crypto services: CIPHER, MAC, HASH, AEAD, KDF, ASYM, PRIMITIVE.
> >
> > In this patch, CIPHER, MAC, HASH, AEAD services are introduced.
> >
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > CC: Michael S. Tsirkin <mst@redhat.com>
> > CC: Cornelia Huck <cornelia.huck@de.ibm.com>
> > CC: Stefan Hajnoczi <stefanha@redhat.com>
> > CC: Lingli Deng <denglingli@chinamobile.com>
> > CC: Jani Kokkonen <Jani.Kokkonen@huawei.com>
> > CC: Ola Liljedahl <Ola.Liljedahl@arm.com>
> > CC: Varun Sethi <Varun.Sethi@freescale.com>
> > CC: Zeng Xin <xin.zeng@intel.com>
> > CC: Keating Brian <brian.a.keating@intel.com>
> > CC: Ma Liang J <liang.j.ma@intel.com>
> > CC: Griffin John <john.griffin@intel.com>
> > CC: Hanweidong <hanweidong@huawei.com>
> > ---
> >  content.tex       |   2 +
> >  virtio-crypto.tex | 793
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 795 insertions(+)
> >  create mode 100644 virtio-crypto.tex
> >
> > diff --git a/content.tex b/content.tex
> > index 4b45678..ab75f78 100644
> > --- a/content.tex
> > +++ b/content.tex
> > @@ -5750,6 +5750,8 @@ descriptor for the \field{sense_len},
> \field{residual},
> >  \field{status_qualifier}, \field{status}, \field{response} and
> >  \field{sense} fields.
> >
> > +\input{virtio-crypto.tex}
> > +
> >  \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> >
> >  Currently there are three device-independent feature bits defined:
> > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > new file mode 100644
> > index 0000000..9465de3
> > --- /dev/null
> > +++ b/virtio-crypto.tex
> > @@ -0,0 +1,793 @@
> > +\section{Crypto Device}\label{sec:Device Types / 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. The virtio crypto
> > +device can provide seven crypto services: CIPHER, MAC, HASH, AEAD,
> > +KDF, ASYM, PRIMITIVE.
> > +
> > +\subsection{Device ID}\label{sec:Device Types / Crypto Device / Device ID}
> > +
> > +20
> > +
> > +\subsection{Virtqueues}\label{sec:Device Types / Crypto Device /
> > Virtqueues}
> > +
> > +\begin{description}
> > +\item[0] dataq1
> > +\item[\ldots]
> > +\item[N-1] dataqN
> > +\item[N] controlq
> > +\end{description}
> > +
> > +N is set by \field{max_dataqueues} (\field{max_dataqueues} >= 1).
> > +
> > +\subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature
> > bits}
> > +  None currently defined
> > +
> > +\subsection{Device configuration layout}\label{sec:Device Types / Crypto
> > Device / Device configuration layout}
> > +
> > +Thirteen driver-read-only configuration fields are currently defined.
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_config {
> > +    le32  version;
> > +    le16  status;
> > +    le16  max_dataqueues;
> > +    le32  crypto_services;
> > +    /* 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;
> > +};
> > +\end{lstlisting}
> > +
> > +The first driver-read-only field, \field{version} specifies the virtio crypto's
> > +version, which is reserved for back-compatibility in future.It's currently
> > +defined for the version field:
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_CRYPTO_VERSION_1  (1)
> 
> Suggest to remove this macro,
> Do you think a version which is composed of major version and
> minor version is better?
> 

I think we should tell the developer how to set the value of version field,
but I have no idea about which value or form is better, so I used 1 as the
first version. What's your opinion?

> > +\end{lstlisting}
> > +
> > +One read-only bit (for the device) is currently defined for the \field{status}
> > +field: VIRTIO_CRYPTO_S_HW_READY.
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_CRYPTO_S_HW_READY  (1 << 0)
> > +\end{lstlisting}
> > +
> > +The following driver-read-only field, \field{max_dataqueuess} specifies the
> > +maximum number of data virtqueues (dataq1\ldots dataqN). The
> > crypto_services
> > +shows the crypto services the virtio crypto supports. The service currently
> > +defined are:
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_CRYPTO_NO_SERVICE (0) /* cipher services */
> 
> Why is this bit needed?
> 
As you said, If the device guys have no realize any service yet, but the whole
framework. The guys who develop the driver code also do their jobs well by this bit,
which makes sense I think.

> > +#define VIRTIO_CRYPTO_SERVICE_CIPHER (1) /* cipher services */
> > +#define VIRTIO_CRYPTO_SERVICE_HASH  (2) /* hash service */
> > +#define VIRTIO_CRYPTO_SERVICE_MAC   (3) /* MAC (Message
> > Authentication Codes) service */
> > +#define VIRTIO_CRYPTO_SERVICE_AEAD  (4) /* AEAD(Authenticated
> > Encryption with Associated Data) service */
> > +\end{lstlisting}
> > +
> > +The last driver-read-only fields specify detailed algorithms mask which
> > +the device offered for corresponding service. The below CIPHER algorithms
> > +are defined currently:
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_CRYPTO_NO_CIPHER                 0
> > +#define VIRTIO_CRYPTO_CIPHER_ARC4               1
> > +#define VIRTIO_CRYPTO_CIPHER_AES_ECB            2
> > +#define VIRTIO_CRYPTO_CIPHER_AES_CBC            3
> > +#define VIRTIO_CRYPTO_CIPHER_AES_CTR            4
> > +#define VIRTIO_CRYPTO_CIPHER_DES_ECB            5
> > +#define VIRTIO_CRYPTO_CIPHER_DES_CBC            6
> > +#define VIRTIO_CRYPTO_CIPHER_3DES_ECB           7
> > +#define VIRTIO_CRYPTO_CIPHER_3DES_CBC           8
> > +#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
> > +\end{lstlisting}
> > +
> > +The below HASH algorithms are defined currently:
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_CRYPTO_NO_HASH            0
> > +#define VIRTIO_CRYPTO_HASH_MD5           1
> > +#define VIRTIO_CRYPTO_HASH_SHA1          2
> > +#define VIRTIO_CRYPTO_HASH_SHA_224       3
> > +#define VIRTIO_CRYPTO_HASH_SHA_256       4
> > +#define VIRTIO_CRYPTO_HASH_SHA_384       5
> > +#define VIRTIO_CRYPTO_HASH_SHA_512       6
> > +#define VIRTIO_CRYPTO_HASH_SHA3_224      7
> > +#define VIRTIO_CRYPTO_HASH_SHA3_256      8
> > +#define VIRTIO_CRYPTO_HASH_SHA3_384      9
> > +#define VIRTIO_CRYPTO_HASH_SHA3_512      10
> > +#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE128      11
> > +#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE256      12
> > +\end{lstlisting}
> > +
> > +The below MAC algorithms are defined currently:
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_CRYPTO_NO_MAC                       0
> > +#define VIRTIO_CRYPTO_MAC_HMAC_MD5                 1
> > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA1                2
> > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_224             3
> > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_256             4
> > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_384             5
> > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_512             6
> > +#define VIRTIO_CRYPTO_MAC_CMAC_3DES                25
> > +#define VIRTIO_CRYPTO_MAC_CMAC_AES                 26
> > +#define VIRTIO_CRYPTO_MAC_CMAC_KASUMI_F9           27
> > +#define VIRTIO_CRYPTO_MAC_CMAC_SNOW3G_UIA2         28
> > +#define VIRTIO_CRYPTO_MAC_GMAC_AES                 41
> > +#define VIRTIO_CRYPTO_MAC_GMAC_TWOFISH             42
> > +#define VIRTIO_CRYPTO_MAC_CBCMAC_AES               49
> > +#define VIRTIO_CRYPTO_MAC_CBCMAC_KASUMI_F9         50
> > +#define VIRTIO_CRYPTO_MAC_XCBC_AES                 53
> > +#define VIRTIO_CRYPTO_MAC_ZUC_EIA3                 54
> > +\end{lstlisting}
> > +
> > +The below AEAD algorithms are defined currently:
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_CRYPTO_NO_AEAD     0
> > +#define VIRTIO_CRYPTO_AEAD_GCM    1
> > +#define VIRTIO_CRYPTO_AEAD_CCM    2
> > +#define VIRTIO_CRYPTO_AEAD_CHACHA20_POLY1305  3
> > +\end{lstlisting}
> > +
> > +\subsubsection{Device Requirements: Device configuration
> > layout}\label{sec:Device Types / Crypto Device / Device configuration layout
> > / Device Requirements: Device configuration layout}
> > +
> > +\begin{itemize*}
> > +\item The device MUST set \field{version} in version filed.
> > +\item The device MUST set \field{max_dataqueues} to between 1 and
> > 65535 inclusive.
> > +\item The device SHOULD set \field{status} according to the status of the
> > hardware-backed implementation.
> > +\item The device MUST set \field{crypto_services} according to the crypto
> > services which the device offered.
> > +\item The device MUST set detailed algorithms mask according to
> > crypto_services field.
> > +\end{itemize*}
> > +
> > +\subsubsection{Driver Requirements: Device configuration
> > layout}\label{sec:Device Types / Crypto Device / Device configuration layout
> > / Driver Requirements: Device configuration layout}
> > +
> > +\begin{itemize*}
> > +\item The driver MUST read the ready \field{status} from the bottom bit of
> > status to
> > +      check whether the hardware-backed implementation is ready or not.
> > +\item The driver MAY read \field{max_dataqueues} field to discover how
> > many data queues the device supports.
> > +\item The driver MUST read \field{crypto_services} field to discover which
> > services the device is able to offer.
> > +\item The driver MUST read detail algorithms field according to
> > crypto_services field.
> > +\end{itemize*}
> > +
> > +\subsection{Device Initialization}{Device Types / Crypto Device / Device
> > Initialization}
> > +
> > +The driver SHOULD perform a typical initialization routine like so:
> > +
> > +\begin{enumerate}
> > +\item Identify and initialize data virtqueue, up to \field{max_dataqueues}.
> > +\item Identify the control virtqueue.
> > +\item Identify the ready status of hardware-backend comes from the
> > \field{status}.
> > +\item Read the supported crypto services from bits of
> > \field{crypto_servies}.
> > +\item Read the supported algorithms according to \field{crypto_services}
> > field.
> > +\end{enumerate}
> > +
> > +\subsection{Device Operation}\label{sec:Device Types / Crypto Device /
> > Device Operation}
> > +
> > +Packets can be transmitted by placing them in both the controlq and dataq.
> > +The packet are consisted of general header and services 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 parameters are algorithm-specific parameters, input data is the
> > +data should be operated , output data is the "operation result + result
> > buffer".
> 
> According to following sections, input are device-writable.
> output are device-readable. So need to update the description
> according to following sections.
> 

Yes, good catch, thanks.

> > +The general header of controlq:
> > +
> > +\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;
> > +    /* control flag to control the request */
> > +    u8 flag;
> > +    /* data virtqeueu id */
> > +    le16 queue_id;
> > +};
> > +\end{lstlisting}
> > +
> > +The general header of dataq:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_op_header {
> > +#define VIRTIO_CRYPTO_CIPHER_ENCRYPT
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x00)
> > +#define VIRTIO_CRYPTO_CIPHER_DECRYPT
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x01)
> > +#define VIRTIO_CRYPTO_HASH
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x00)
> > +#define VIRTIO_CRYPTO_MAC
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x00)
> > +#define VIRTIO_CRYPTO_AEAD_ENCRYPT
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x00)
> > +#define VIRTIO_CRYPTO_AEAD_DECRYPT
> > VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x01)
> > +    le32 opcode;
> > +    /* algo should be service-specific algorithms */
> > +    le32 algo;
> > +    /* control flag to control the request */
> > +    u8 flag;
> 
> Here we could define flag u16 to make structure alignment.

It makes senses.

> 
> > +    /* data virtqeueu id */
> > +    le16 queue_id;
> > +    /* session_id should be service-specific algorithms */
> > +    le64 session_id;
> > +};
> > +\end{lstlisting}
> > +
> > +\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 which finish the non-data plane operations, such as session
> > +operations (see \ref{sec:Device Types / Crypto Device / Device Operation /
> > Session operation}).
> > +The packet of controlq:
> > +
> > +\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
> sym_destroy_session;
> > +    } u;
> > +};
> > +\end{lstlisting}
> > +
> > +The header is the general header, the union is algorithm-specific type,
> > +which is set by the driver. All the properties in the union will be shown as
> > follow.
> > +
> > +\subsubsection{Session operation}\label{sec:Device Types / Crypto Device
> > / Device Operation / 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:
> > +
> > +\begin{enumerate}
> > +\item The operation (cipher, hash/mac or both, and if both, the order in
> > +      which the algorithms should be applied).
> > +\item The cipher setup data, including the cipher algorithm and mode,
> > +      the key and its length, and the direction (encrypt or decrypt).
> > +\item The hash/mac setup data, including the hash algorithm or mac
> > algorithm,
> > +      and digest result length (to allow for truncation).
> > +\begin{itemize*}
> > +\item Authenticated mode can refer to MAC, which requires that the key
> > and
> > +      its length are also specified.
> > +\item For nested mode, the inner and outer prefix data and length are
> > specified,
> > +      as well as the outer hash algorithm.
> > +\end{itemize*}
> > +\end{enumerate}
> > +
> > +\paragraph{Session operation: HASH session}\label{sec:Device Types /
> > Crypto Device / Device Operation / Session operation / Session operation:
> > HASH session}
> > +
> > +The packet of HASH session is:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_sym_session_input {
> > +    u8     status;
> > +    le64    session_id;
> > +};
> > +
> > +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 {
> > +    struct virtio_crypto_hash_session_para parameter;
> > +    /*
> > +     * in header guest physical address, See struct
> > virtio_crypto_sym_session_input
> > +     * above
> > +     */
> > +    le64 inhdr_addr;
> > +};
> > +\end{lstlisting}
> > +
> 
> Does the inhdr_addr point to a virtio_crypto_sym_session_input structure?

Yes.

> Why not put virtio_crypto_sym_session_input into
> virtio_crypto_hash_session_para
> directly?

Yes. It's clearer.

> If inhdr_addr point to a physical address, if the address is not physically
> contiguous, how does the backend device handle?
> Same comments to MAC/SYM/AEAD session creation.
> 

It has no problems if we move struct virtio_crypto_sym_session_input into 
struct virtio_crypto_hash_session_para. I'll fix them in the next version.

> > +\paragraph{Session operation: MAC session}\label{sec:Device Types /
> > Crypto Device / Device
> > +Operation / Session operation / Session operation: MAC session}
> > +
> > +The packet of MAC session is:
> > +
> > +\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;
> > +};
> > +struct virtio_crypto_mac_create_session_req {
> > +    struct virtio_crypto_mac_session_para  parameter;
> > +    /*
> > +     * in header guest physical address, See struct
> > virtio_crypto_sym_session_input
> > +     * above
> > +     */
> > +    le64 inhdr_addr;
> > +};
> > +\end{lstlisting}
> > +
> > +\paragraph{Session operation: Symmetric algorithms
> > session}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Session operation / Session operation: Symmetric algorithms
> > session}
> > +
> > +The symmetric session include both ciphers (CIPHER service) and chain
> > +algorithms (chain cipher and hash/mac). The packet of symmetric session is:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_cipher_session_para {
> > +/* See VIRTIO_CRYPTO_CIPHER* above */
> > +    le32 algo;
> > +    /* length of key */
> > +    le32 keylen;
> > +    /* encrypt or decrypt */
> > +    u8 op;
> > +};
> > +struct virtio_crypto_sym_create_session_req {
> > +/* 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
> > +    u8 op_type;
> > +
> > +    union {
> > +        struct virtio_crypto_cipher_session_para cipher_param;
> > +        struct virtio_crypto_alg_chain_param {
> > +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER
> > 1
> > +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH
> > 2
> > +            u8 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
> > +            u8 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;
> > +        } chain;
> > +    } sym;
> > +
> > +    /*
> > +     * in header guest physical address, See struct
> > virtio_crypto_sym_session_input
> > +     * above
> > +     */
> > +    le64 inhdr_addr;
> > +};
> > +\end{lstlisting}
> > +
> > +\paragraph{Session operation: AEAD session}\label{sec:Device Types /
> > Crypto Device / Device
> > +Operation / Session operation / Session operation: AEAD session}
> > +
> > +The packet of AEAD session is:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_aead_session_para {
> > +    /* See VIRTIO_CRYPTO_AEAD_* above*/
> > +    u8 algo;
> > +    /* length of key */
> > +    le32 key_len;
> > +    /* digest result length */
> > +    le32 digest_result_len;
> > +    /* The length of the additional authenticated data (AAD) in bytes */
> > +    le32 aad_len;
> > +    /* encrypt or decrypt */
> > +    u8 op;
> > +};
> > +struct virtio_crypto_aead_create_session_req {
> > +    struct virtio_crypto_aead_session_para parameter;
> > +    /*
> > +     * in header guest physical address, See struct
> > virtio_crypto_sym_session_input
> > +     * above
> > +     */
> > +    le64 inhdr_addr;
> > +};
> > +\end{lstlisting}
> > +
> > +\paragraph{Session operation: create session}\label{sec:Device Types /
> > Crypto Device / Device
> > +Operation / Session operation / Session operation: create session}
> > +
> > +A request of creating a session including the following information:
> > +
> > +\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
> sym_destroy_session;
> > +    } u;
> > +};
> > +\end{lstlisting}
> > +
> > +\subparagraph{Driver Requirements: Session operation: create
> > session}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Session operation / Session operation: create session / Driver
> > Requirements: Session operation: create session}
> > +
> > +The driver MUST set the control general header and corresponding
> > property
> > +of union in structure virtio_crypto_op_ctrl_req. See \ref{sec:Device Types /
> > Crypto Device / Device Operation}.
> > +Take the example of MAC service, the driver MUST set
> > VIRTIO_CRYPTO_MAC_CREATE_SESSION
> > +for \field{opcode} field in struct virtio_crypto_op_ctrl_req, and set the
> > \field{queue_id}
> > +to show dataq used.
> > +
> > +\subparagraph{Device Requirements: Session operation: create
> > session}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Session operation / Session operation: create session / Device
> > Requirements: Session operation: create session}
> > +
> > +The device MUST return a session identifier to the driver when the device
> > +finishes the processing of session creation. The session creation request
> > +MUST end by a GPA of tailer: \field{inhdr_addr} field in struct
> > virtio_crypto_*_create_session_req
> > +of each crypto service. The \field{inhdr_addr} field point to the below
> > structure:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_sym_session_input {
> > +    u8     status;
> > +    le64    session_id;
> > +};
> > +\end{lstlisting}
> > +
> > +Both status and session_id are written by the device: either
> > VIRTIO_CRYPTO_OP_OK for success,
> > +VIRTIO_CRYPTO_OP_ERR for creation failed or device error,
> > VIRTIO_CRYPTO_OP_NOTSUPP for not support,
> > +VIRTIO_CRYPTO_OP_INVSESS for invalid session id when executing crypto
> > operations.
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_CRYPTO_OP_OK        0
> > +#define VIRTIO_CRYPTO_OP_ERR       1
> > +#define VIRTIO_CRYPTO_OP_BADMSG    2
> > +#define VIRTIO_CRYPTO_OP_NOTSUPP   3
> > +#define VIRTIO_CRYPTO_OP_INVSESS   4
> > +\end{lstlisting}
> > +
> > +\paragraph{Session operation: destroy session}\label{sec:Device Types /
> > Crypto Device / Device
> > +Operation / Session operation / Session operation: destroy session}
> > +
> > +A request which destroy a session including the following information:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_destroy_session_req {
> > +    le64    session_id; /* OUT parameter */
> > +    u8     status; /* IN parameter */
> > +};
> > +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
> sym_destroy_session;
> > +    } u;
> > +};
> > +\end{lstlisting}
> > +
> > +\subparagraph{Driver Requirements: Session operation: destroy
> > session}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Session operation / Session operation: destroy session / Driver
> > Requirements: Session operation: destroy session}
> > +
> > +The driver MUST set the control general header and corresponding
> > property
> > +of union in struct virtio_crypto_op_ctrl_req. See \ref{sec:Device Types /
> > Crypto Device / Device Operation}.
> > +Take the example of MAC service, the driver MUST set
> > VIRTIO_CRYPTO_MAC_DESTROY_SESSION
> > +for \field{opcode} field in struct virtio_crypto_op_ctrl_req, and set the
> > \field{queue_id} shows dataq used.
> > +The driver MUST set the \field{session_id} which MUST be a valid value
> > which assigned by the
> > +device when a session was created.
> > +
> > +\subparagraph{Device Requirements: Session operation: destroy
> > session}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Session operation / Session operation: destroy session / Device
> > Requirements: Session operation: destroy session}
> > +
> > +Status is written by the device: either VIRTIO_CRYPTO_OP_OK for success,
> > VIRTIO_CRYPTO_OP_ERR for failure or device error.
> > +
> > +\subsubsection{Crypto operation}\label{sec:Device Types / Crypto Device /
> > Device Operation / Crypto operation}
> > +
> > +The driver uses the dataq to finish the data plane operations (such as crypto
> > operation).
> > +The packet of dataq:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_op_data_req {
> > +    struct virtio_crypto_op_header header;
> > +
> > +    union {
> > +        struct virtio_crypto_sym_data_req   sym_req;
> > +        struct virtio_crypto_hash_data_req  hash_req;
> > +        struct virtio_crypto_mac_data_req   mac_req;
> > +        struct virtio_crypto_aead_data_req  aead_req;
> > +    } u;
> > +};
> > +\end{lstlisting}
> > +
> > +The header is the general header, the union is algorithm-specific type,
> > +which are set by the driver. All the properties in the union will be shown as
> > follow.
> > +
> > +\paragraph{Crypto operation: HASH operation}\label{sec:Device Types /
> > Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: HASH operation}
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_sym_crypt_op_inhdr {
> > +    u8    status;
> > +};
> > +
> > +struct virtio_crypto_hash_para {
> > +    /* length of source data */
> > +    le32 src_data_len;
> > +};
> > +
> 
> src_data_len can be put in virtio_crypto_hash_input.
> If a request doesn't need a parameter, we can just
> ignore it.
> 

OK, I agree.

> > +struct virtio_crypto_hash_input {
> > +    le64 digest_result_addr; /* digest result guest physical address */
> > +    /*
> > +     * in header guest physical address, See struct
> > virtio_crypto_sym_crypt_op_inhdr
> > +     * above
> > +     */
> > +    le64 inhdr_addr;
> > +};
> > +
> > +struct virtio_crypto_hash_output {
> > +    le64 src_data_addr; /* source data guest physical address */
> > +};
> > +
> > +struct virtio_crypto_hash_data_req {
> > +    struct virtio_crypto_hash_para parameter;
> > +    struct virtio_crypto_hash_input idata;
> > +    struct virtio_crypto_hash_output odata;
> > +};
> > +\end{lstlisting}
> > +
> > +Both input and output parameters store the guest physical address (GPA)
> > only so
> > +that each data operation request only occupies one entry of the Vring
> > descriptor
> > +table, which improves the throughput of data transferring for HASH crypto
> > service.
> > +The struct virtio_crypto_hash_para store the corresponding parameters,
> > such as
> > +\field{src_data_len}. The length and GPA can determine the specific
> > content in
> > +the guest memory.
> > +
> > +\subparagraph{Driver Requirements: Crypto operation: HASH
> > operation}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: HASH operation / Driver
> > Requirements: Crypto operation: HASH operation}
> > +
> > +The driver MUST set the \field{opcode}, \field{session_id} in struct
> > virtio_crypto_op_header.
> > +The \field{opcode} is set to VIRTIO_CRYPTO_HASH, \field{session_id} MUST
> > be a valid value
> > +which assigned by the device when a session was created.
> > +The driver SHOULD set the \field{queue_id} field to show dataq used in
> > struct virtio_crypto_op_header.
> > +The driver MUST fill out all fields in struct virtio_crypto_hash_data_req,
> > including \field{parameter},
> > +\field{idata} and \field{odata} sub structures.
> > +
> > +Note: \field{inhdr_addr} is a GPA pointed the struct
> > virtio_crypto_sym_crypt_op_inhdr
> > +which allocated by the driver.
> > +
> > +\subparagraph{Device Requirements: Crypto operation: HASH
> > operation}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: HASH operation / Device
> > Requirements: Crypto operation: HASH operation}
> > +
> > +The device MUST copy the result of hash operation recorded by
> > \field{digest_result_addr}
> > +filed in struct virtio_crypto_hash_input.
> > +The device MUST set the status recorded by \field{inhdr_addr field} in strut
> > virtio_crypto_hash_input
> > +which point struct virtio_crypto_sym_crypt_op_inhdr: either
> > VIRTIO_CRYPTO_OP_OK for success,
> > +VIRTIO_CRYPTO_OP_ERR for creation failed or device error,
> > VIRTIO_CRYPTO_OP_NOTSUPP for not support.
> > +
> > +\paragraph{Crypto operation: MAC operation}\label{sec:Device Types /
> > Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: MAC operation}
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_mac_para {
> > +    struct virtio_crypto_hash_para hash_para;
> > +};
> > +
> > +struct virtio_crypto_mac_input {
> > +    struct virtio_crypto_hash_input hash_input;
> > +};
> > +
> > +struct virtio_crypto_mac_output {
> > +    struct virtio_crypto_hash_output hash_output;
> > +};
> > +
> > +struct virtio_crypto_mac_data_req {
> > +    struct virtio_crypto_mac_para parameter;
> > +    struct virtio_crypto_mac_input idata;
> > +    struct virtio_crypto_mac_output odata;
> > +};
> > +\end{lstlisting}
> > +
> > +Both input and output parameters store the guest physical address (GPA)
> > only so
> > +that each data operation request only occupies one entry of the Vring
> > descriptor
> > +table, which improves the throughput of data transferring for MAC crypto
> > service.
> > +The struct virtio_crypto_mac_para store the corresponding parameters,
> > such as
> > +\field{src_data_len}. The length and GPA can determine the specific
> > content in the guest memory.
> > +
> > +\subparagraph{Driver Requirements: Crypto operation: MAC
> > operation}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: MAC operation / Driver
> > Requirements: Crypto operation: MAC operation}
> > +
> > +Refer to the hash operation.
> > +The driver MUST set the \field{opcode} field to VIRTIO_CRYPTO_MAC.
> > +
> > +\subparagraph{Device Requirements: Crypto operation: MAC
> > operation}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: MAC operation / Device
> > Requirements: Crypto operation: MAC operation}
> > +
> > +Refer to the hash operation.
> > +
> > +\paragraph{Crypto operation: Symmetric algorithms
> > operation}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: Symmetric algorithms
> > operation}
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_cipher_para {
> > +    le32 iv_len;
> > +    /* length of source data */
> > +    le32 src_data_len;
> > +    /* length of dst data */
> > +    le32 dst_data_len;
> > +};
> > +struct virtio_crypto_cipher_input {
> > +    le64 dst_data_addr; /* destination data guest physical address */
> > +    /*
> > +     * in header guest physical address, See struct
> > virtio_crypto_sym_crypt_op_inhdr
> > +     * above
> > +     */
> > +    le64 inhdr_addr;
> > +};
> > +
> > +struct virtio_crypto_cipher_output {
> > +    le64 iv_addr; /* iv guest physical address */
> > +    le64 src_data_addr; /* source data guest physical address */
> > +};
> > +struct virtio_crypto_cipher_data_req {
> > +    struct virtio_crypto_cipher_para parameter;
> > +    struct virtio_crypto_cipher_input idata;
> > +    struct virtio_crypto_cipher_output odata;
> > +};
> > +struct virtio_crypto_sym_data_req {
> > +    struct virtio_crypto_cipher_data_req cipher_req;
> > +    union {
> > +        struct virtio_crypto_hash_data_req hash_req;
> > +        struct virtio_crypto_mac_data_req mac_req;
> > +    } u;
> > +};
> > +\end{lstlisting}
> > +
> > +Both input and output parameters store the guest physical address (GPA)
> > only so that
> > +each data operation request only occupies one entry of the Vring descriptor
> > table,
> > +which improves the throughput of data transferring for symmetric
> > algorithms.
> > +The struct virtio_crypto_cipher_para store the corresponding parameters,
> > +such as \field{src_data_len}. The length and GPA can determine the specific
> > content in the guest memory.
> > +
> > +\subparagraph{Driver Requirements: Crypto operation: Symmetric
> > algorithms encryption}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: Symmetric algorithms
> > operation / Driver Requirements: Crypto operation: Symmetric algorithms
> > encryption}
> > +
> > +Refer to the hash operation.
> > +The driver MUST set the opcode field to VIRTIO_CRYPTO_CIPHER_ENCRYPT.
> > +The driver MUST fill out the fields in struct virtio_crypto_sym_data_req
> > according to
> > +the operation type of the session. For example, if the created session is
> > based
> > +on VIRTIO_CRYPTO_SYM_OP_CIPHER, that means the driver just SHOULD
> > fill out fields
> > +of struct virtio_crypto_cipher_data_req in struct
> > virtio_crypto_sym_data_req.
> > +If the created session is VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING
> > type and
> > +VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH mode, that means the driver
> > SHOULD fill out
> > +fields of both struct virtio_crypto_cipher_data_req and struct
> > +virtio_crypto_mac_data_req mac_req in struct
> virtio_crypto_sym_data_req.
> > +
> > +\subparagraph{Device Requirements: Crypto operation: Symmetric
> > algorithms encryption}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: Symmetric algorithms
> > operation / Device Requirements: Crypto operation: Symmetric algorithms
> > encryption}
> > +
> > +The device MUST parse the virtio_crypto_sym_data_req according to the
> > session_id in general header.
> > +For example, The device SHOULD only parsed 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.
> > +The driver SHOULD parse fields of both struct
> > virtio_crypto_cipher_data_req and struct
> > +virtio_crypto_mac_data_req mac_req in struct virtio_crypto_sym_data_req
> > if the created
> > +session is VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING operation
> > type and VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH mode.
> > +
> > +The device MUST copy the result of encryption operation recorded by
> > \filed{dst_data_addr} filed in struct virtio_crypto_cipher_input in plain cipher
> > mode.
> > +The device MUST copy the result of hash operation recorded by
> > \filed{digest_result_addr} filed in struct virtio_crypto_hash_input in chain
> > hash/mac mode.
> > +The device MUST set the status recorded by \filed{inhdr_addr} field in strut
> > virtio_crypto_cipher_input which point
> > +struct virtio_crypto_sym_crypt_op_inhdr: either VIRTIO_CRYPTO_OP_OK
> > for success, VIRTIO_CRYPTO_OP_ERR for creation
> > +failed or device error, VIRTIO_CRYPTO_OP_NOTSUPP for not support.
> > +
> > +\subparagraph{Device Requirements: Crypto operation: Steps of
> > encryption}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: Symmetric algorithms
> > operation / Device Requirements: Crypto operation: Steps of encryption}
> > +
> > +Step1: Create a session:
> > +\begin{enumerate}
> > +\item The driver fills out the context message, including algorithm name,
> > key, keylen etc;
> > +\item The driver sends a context message to the backend device by
> > controlq;
> > +\item The device creates a session using the message transmitted by
> > controlq;
> > +\item Return the session id to the driver.
> > +\end{enumerate}
> > +
> > +Step2: Execute the detail encryption operation:
> > +\begin{enumerate}
> > +\item The driver fills out the encrypt requests;
> > +\item Put the requests into dataq and kick the virtqueue;
> > +\item The device executes the encryption operation according the
> > requests' arguments;
> > +\item The device returns the encryption result to the driver by dataq;
> > +\item The driver callback handle the result and over.
> > +\end{enumerate}
> > +
> > +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.
> > +
> > +\paragraph{Crypto operation: AEAD operation}\label{sec:Device Types /
> > Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: AEAD operation}
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_aead_para {
> > +    le32 iv_len;
> > +    /* length of additional auth data */
> > +    le32 aad_len;
> > +    /* length of source data */
> > +    le32 src_data_len;
> > +    /* length of dst data */
> > +    le32 dst_data_len;
> > +};
> > +
> > +struct virtio_crypto_aead_input {
> > +    le64 dst_data_addr; /* destination data guest physical address */
> > +    le64 digest_result_addr; /* digest result guest physical address */
> > +    /*
> > +     * in header guest physical address, See struct
> > virtio_crypto_sym_crypt_op_inhdr
> > +     * above
> > +     */
> > +    le64 inhdr_addr;
> > +};
> > +
> > +struct virtio_crypto_aead_output {
> > +    le64 iv_addr; /* iv guest physical address */
> > +    le64 src_data_addr; /* source data guest physical address */
> > +    le64 add_data_addr; /* additional auth data guest physical address */
> > +};
> > +struct virtio_crypto_aead_data_req {
> > +    struct virtio_crypto_aead_para parameter;
> > +    struct virtio_crypto_aead_input idata;
> > +    struct virtio_crypto_aead_output odata;
> > +};
> > +\end{lstlisting}
> > +
> > +Both input and output parameters store the guest physical address (GPA)
> > only so that
> > +each data operation request only occupies one entry of the Vring descriptor
> > table,
> > +which improves the throughput of data transferring for AEAD crypto service.
> > The
> > +struct virtio_crypto_aead_para store the corresponding parameters, such
> > as \field{src_data_len}.
> > +The length and GPA can determine the specific content in the guest
> > memory.
> > +
> > +\subparagraph{Driver Requirements: Crypto operation: AEAD
> > encryption}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: AEAD operation / Driver
> > Requirements: Crypto operation: AEAD encryption}
> > +
> > +Refer to the symmetric algorithms encryption operation.
> > +The driver MUST set the \field{opcode} field to
> > VIRTIO_CRYPTO_AEAD_ENCRYPT.
> > +
> > +\subparagraph{Device Requirements: Crypto operation: AEAD
> > encryption}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: AEAD operation / Device
> > Requirements: Crypto operation: AEAD encryption}
> > +
> > +Refer to the symmetric algorithms encryption operation.
> > +
> > +\subparagraph{Driver Requirements: Crypto operation: AEAD
> > decryption}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: AEAD operation / Driver
> > Requirements: Crypto operation: AEAD decryption}
> > +
> > +Refer to the symmetric algorithms encryption operation.
> > +The driver MUST set the \field{opcode} field to
> > VIRTIO_CRYPTO_AEAD_DECRYPT.
> > +
> > +\subparagraph{Device Requirements: Crypto operation: AEAD
> > decryption}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Crypto operation / Crypto operation: AEAD operation / Device
> > Requirements: Crypto operation: AEAD decryption}
> > +
> > +The device MUST verify and return the verify result to the driver.
> > +If the verify result is not correct, VIRTIO_CRYPTO_OP_BADMSG (bad
> > message)
> > +MUST be returned the driver.
> > --
> > 1.7.12.4
> >

Re: [Qemu-devel] [PATCH v6 0/2] virtio-crypto: virtio crypto device specification

[Michael S. Tsirkin]

On Wed, Aug 03, 2016 at 07:23:36PM +0200, Cornelia Huck wrote:
> On Wed, 3 Aug 2016 16:53:36 +0200
> Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> 
> > On Wed, 3 Aug 2016 17:40:07 +0300
> > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > 
> > > On Wed, Aug 03, 2016 at 04:33:28PM +0200, Cornelia Huck wrote:
> > > > On Wed, 3 Aug 2016 17:30:22 +0300
> > > > "Michael S. Tsirkin" <mst@redhat.com> wrote:
> > > > 
> > > > > On Mon, Aug 01, 2016 at 05:20:19PM +0800, Gonglei wrote:
> > > > > > This is the specification (version 6) about a new virtio crypto device.
> > > > > > After a big reconstruction, the spec (symmetric algos) is near to stabilize.
> > > > > > This version fix some problems of formating and return value, etc.
> > > > > > 
> > > > > > If you have any comments, please let me know, thanks :)
> > > > > 
> > > > > You might want to open a jira tracker in oasis jira to add this.
> > > > 
> > > > It may make sense to open one/many jira trackers to reserve the device
> > > > ids (for the various device types that have accumulated) at least.
> > > 
> > > Yes! Can you do this please?
> > 
> > Sure, will do.
> 
> Done (issues 147-150).
> 
> I'm just unsure what to put as target version :/

So please use 1.1 - I will rename it if we decide
on a different version.

-- 
MST

Re: [Qemu-devel] [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification

[Michael S. Tsirkin]

On Thu, Aug 04, 2016 at 11:24:26AM +0000, Gonglei (Arei) wrote:
> > > +The first driver-read-only field, \field{version} specifies the virtio crypto's
> > > +version, which is reserved for back-compatibility in future.It's currently
> > > +defined for the version field:
> > > +
> > > +\begin{lstlisting}
> > > +#define VIRTIO_CRYPTO_VERSION_1  (1)
> > 
> > Suggest to remove this macro,
> > Do you think a version which is composed of major version and
> > minor version is better?
> > 
> 
> I think we should tell the developer how to set the value of version field,
> but I have no idea about which value or form is better, so I used 1 as the
> first version. What's your opinion?

My opinion is that you should drop this completely. We do feature bits,
not version numbers in virtio. We do not want each device doing its own
thing for compatibility.

-- 
MST

Re: [Qemu-devel] [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification

[Zeng, Xin]

On Thu, Friday, August 05, 2016 3:56 AM, Michael S. Tsirkin wrote:
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Friday, August 05, 2016 3:56 AM
> To: Gonglei (Arei)
> Cc: Zeng, Xin; qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org;
> Huangpeng (Peter); Luonengjun; cornelia.huck@de.ibm.com;
> stefanha@redhat.com; denglingli@chinamobile.com; Jani Kokkonen;
> Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Keating, Brian A; Ma,
> Liang J; Griffin, John; Hanweidong (Randy); Huangweidong (C)
> Subject: Re: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device
> specification
> 
> On Thu, Aug 04, 2016 at 11:24:26AM +0000, Gonglei (Arei) wrote:
> > > > +The first driver-read-only field, \field{version} specifies the
> > > > +virtio crypto's version, which is reserved for back-compatibility
> > > > +in future.It's currently defined for the version field:
> > > > +
> > > > +\begin{lstlisting}
> > > > +#define VIRTIO_CRYPTO_VERSION_1  (1)
> > >
> > > Suggest to remove this macro,
> > > Do you think a version which is composed of major version and minor
> > > version is better?
> > >
> >
> > I think we should tell the developer how to set the value of version
> > field, but I have no idea about which value or form is better, so I
> > used 1 as the first version. What's your opinion?
> 
> My opinion is that you should drop this completely. We do feature bits, not
> version numbers in virtio. We do not want each device doing its own thing for
> compatibility.
> 

But as I mentioned before,  considering the bug fix case, if each backend device 
release need a feature bit meaning "some bugs are fixed", are the feature bits
enough? 
Physical devices usually have a revision ID to mark its version, could we have a
revision Id field for each virtio device to distinguish the the virtio devices which 
have the same feature sets but have different version? 

> --
> MST

Re: [Qemu-devel] [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification

[Cornelia Huck]

On Mon, 8 Aug 2016 06:27:15 +0000
"Zeng, Xin" <xin.zeng@intel.com> wrote:

> On Thu, Friday, August 05, 2016 3:56 AM, Michael S. Tsirkin wrote:
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Friday, August 05, 2016 3:56 AM
> > To: Gonglei (Arei)
> > Cc: Zeng, Xin; qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org;
> > Huangpeng (Peter); Luonengjun; cornelia.huck@de.ibm.com;
> > stefanha@redhat.com; denglingli@chinamobile.com; Jani Kokkonen;
> > Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Keating, Brian A; Ma,
> > Liang J; Griffin, John; Hanweidong (Randy); Huangweidong (C)
> > Subject: Re: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device
> > specification
> > 
> > On Thu, Aug 04, 2016 at 11:24:26AM +0000, Gonglei (Arei) wrote:
> > > > > +The first driver-read-only field, \field{version} specifies the
> > > > > +virtio crypto's version, which is reserved for back-compatibility
> > > > > +in future.It's currently defined for the version field:
> > > > > +
> > > > > +\begin{lstlisting}
> > > > > +#define VIRTIO_CRYPTO_VERSION_1  (1)
> > > >
> > > > Suggest to remove this macro,
> > > > Do you think a version which is composed of major version and minor
> > > > version is better?
> > > >
> > >
> > > I think we should tell the developer how to set the value of version
> > > field, but I have no idea about which value or form is better, so I
> > > used 1 as the first version. What's your opinion?
> > 
> > My opinion is that you should drop this completely. We do feature bits, not
> > version numbers in virtio. We do not want each device doing its own thing for
> > compatibility.
> > 
> 
> But as I mentioned before,  considering the bug fix case, if each backend device 
> release need a feature bit meaning "some bugs are fixed", are the feature bits
> enough? 
> Physical devices usually have a revision ID to mark its version, could we have a
> revision Id field for each virtio device to distinguish the the virtio devices which 
> have the same feature sets but have different version? 

I think we really need to decouple device features from bugs in a
certain implementation.

Let's say we have a working virtio-crypto device in qemu 3.1, and a
working virtio-crypto device in another hypervisor 1.3. Both advertise
version 1.

Now we realize that we completely messed up the implementation of a
certain algorithm in qemu. We release qemu 3.2 with this fixed. The
other hypervisor 1.3 was not broken, and therefore they don't need to
do an update.

What should happen to 'version' now? If we keep it at 1, it does not
convey any further information. If we bump it to 2 in qemu 3.2, the
guest still does not know whether a version 1 device is broken or not:
It might be running in the other hypervisor 1.3, which is fine. We
would have basically forced the other hypervisor to release 1.4 for the
version bump to 2 (and deprecated a perfectly fine implementation).

For the above case, the only sane solutions I see are (a) have the host
admin deal with it by either updating qemu or disabling the broken
feature, or (b) have the guest discover the hypervisor and its version
and avoid using the feature if the version is known bad.

If the 'bugs' are rather design problems in the specification of an
algorithm, we should replace it with a v2 specification and guard that
via a feature bit. That is hopefully rare.

tl;dr - I think we should just drop version as it is not helpful.

Re: [Qemu-devel] [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification

[Michael S. Tsirkin]

On Mon, Aug 08, 2016 at 06:27:15AM +0000, Zeng, Xin wrote:
> On Thu, Friday, August 05, 2016 3:56 AM, Michael S. Tsirkin wrote:
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Friday, August 05, 2016 3:56 AM
> > To: Gonglei (Arei)
> > Cc: Zeng, Xin; qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org;
> > Huangpeng (Peter); Luonengjun; cornelia.huck@de.ibm.com;
> > stefanha@redhat.com; denglingli@chinamobile.com; Jani Kokkonen;
> > Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Keating, Brian A; Ma,
> > Liang J; Griffin, John; Hanweidong (Randy); Huangweidong (C)
> > Subject: Re: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device
> > specification
> > 
> > On Thu, Aug 04, 2016 at 11:24:26AM +0000, Gonglei (Arei) wrote:
> > > > > +The first driver-read-only field, \field{version} specifies the
> > > > > +virtio crypto's version, which is reserved for back-compatibility
> > > > > +in future.It's currently defined for the version field:
> > > > > +
> > > > > +\begin{lstlisting}
> > > > > +#define VIRTIO_CRYPTO_VERSION_1  (1)
> > > >
> > > > Suggest to remove this macro,
> > > > Do you think a version which is composed of major version and minor
> > > > version is better?
> > > >
> > >
> > > I think we should tell the developer how to set the value of version
> > > field, but I have no idea about which value or form is better, so I
> > > used 1 as the first version. What's your opinion?
> > 
> > My opinion is that you should drop this completely. We do feature bits, not
> > version numbers in virtio. We do not want each device doing its own thing for
> > compatibility.
> > 
> 
> But as I mentioned before,  considering the bug fix case, if each backend device 
> release need a feature bit meaning "some bugs are fixed", are the feature bits
> enough? 

It depends on whether the bug is very entrenched and important. In most
cases, for minor bugs, it's better to just fix the bug in the driver or
the hypervisor and be done with it.  For cases where
that's not feasible because many drivers relied on a specific bug,
and the bug is very important, we can always add more
if we run out of feature bits.

> Physical devices usually have a revision ID to mark its version,

Because compatibility is one way (new devices usually need
new drivers).

> could we have a
> revision Id field for each virtio device to distinguish the the virtio devices which 
> have the same feature sets but have different version? 

ccw has version negotiation. It was only changed once at the 0.9->1.0
transition so far, it's not used for bug fixes.  We could discuss adding
this to pci and mmio as well, but if yes this should be discussed
separately.

So far no argument made here was crypto specific, so
let's not put this in the crypto device.


> > --
> > MST

Re: [Qemu-devel] [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification

[Zeng, Xin]

On Tuesday, August 9, 2016 6:58 PM Michael S. Tsirkin Wrote:
> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Tuesday, August 9, 2016 6:58 PM
> To: Zeng, Xin <xin.zeng@intel.com>
> Cc: Gonglei (Arei) <arei.gonglei@huawei.com>; qemu-devel@nongnu.org;
> virtio-dev@lists.oasis-open.org; Huangpeng (Peter)
> <peter.huangpeng@huawei.com>; Luonengjun <luonengjun@huawei.com>;
> cornelia.huck@de.ibm.com; stefanha@redhat.com;
> denglingli@chinamobile.com; Jani Kokkonen <Jani.Kokkonen@huawei.com>;
> Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Keating, Brian A
> <brian.a.keating@intel.com>; Ma, Liang J <liang.j.ma@intel.com>; Griffin,
> John <john.griffin@intel.com>; Hanweidong (Randy)
> <hanweidong@huawei.com>; Huangweidong (C)
> <weidong.huang@huawei.com>
> Subject: Re: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device
> specification
> 
> On Mon, Aug 08, 2016 at 06:27:15AM +0000, Zeng, Xin wrote:
> > On Thu, Friday, August 05, 2016 3:56 AM, Michael S. Tsirkin wrote:
> > > -----Original Message-----
> > > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > > Sent: Friday, August 05, 2016 3:56 AM
> > > To: Gonglei (Arei)
> > > Cc: Zeng, Xin; qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org;
> > > Huangpeng (Peter); Luonengjun; cornelia.huck@de.ibm.com;
> > > stefanha@redhat.com; denglingli@chinamobile.com; Jani Kokkonen;
> > > Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Keating, Brian A;
> Ma,
> > > Liang J; Griffin, John; Hanweidong (Randy); Huangweidong (C)
> > > Subject: Re: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device
> > > specification
> > >
> > > On Thu, Aug 04, 2016 at 11:24:26AM +0000, Gonglei (Arei) wrote:
> > > > > > +The first driver-read-only field, \field{version} specifies the
> > > > > > +virtio crypto's version, which is reserved for back-compatibility
> > > > > > +in future.It's currently defined for the version field:
> > > > > > +
> > > > > > +\begin{lstlisting}
> > > > > > +#define VIRTIO_CRYPTO_VERSION_1  (1)
> > > > >
> > > > > Suggest to remove this macro,
> > > > > Do you think a version which is composed of major version and minor
> > > > > version is better?
> > > > >
> > > >
> > > > I think we should tell the developer how to set the value of version
> > > > field, but I have no idea about which value or form is better, so I
> > > > used 1 as the first version. What's your opinion?
> > >
> > > My opinion is that you should drop this completely. We do feature bits,
> not
> > > version numbers in virtio. We do not want each device doing its own
> thing for
> > > compatibility.
> > >
> >
> > But as I mentioned before,  considering the bug fix case, if each backend
> device
> > release need a feature bit meaning "some bugs are fixed", are the feature
> bits
> > enough?
> 
> It depends on whether the bug is very entrenched and important. In most
> cases, for minor bugs, it's better to just fix the bug in the driver or
> the hypervisor and be done with it.  For cases where
> that's not feasible because many drivers relied on a specific bug,
> and the bug is very important, we can always add more
> if we run out of feature bits.
> 
> > Physical devices usually have a revision ID to mark its version,
> 
> Because compatibility is one way (new devices usually need
> new drivers).
> 

Yes, that's also why I propose to put version(revision) field into device 
configuration read-only filed instead of feature bits field.

> > could we have a
> > revision Id field for each virtio device to distinguish the the virtio devices
> which
> > have the same feature sets but have different version?
> 
> ccw has version negotiation. It was only changed once at the 0.9->1.0
> transition so far, it's not used for bug fixes.  We could discuss adding
> this to pci and mmio as well, but if yes this should be discussed
> separately.
> 

Ok, that's good, I do think this is needed. I can initiate this discussion in 
another separated mail loop. 

> So far no argument made here was crypto specific, so
> let's not put this in the crypto device.
> 

Sure,  It is indeed not crypto device specific,  but probably all virtio  
devices needs. We can drop it from crypto device spec.
Thanks!

> 
> > > --
> > > MST

Re: [Qemu-devel] [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification

[Zeng, Xin]

On  Tuesday, August 9, 2016 6:44 PM  Cornelia Huck wrote:
> -----Original Message-----
> From: Cornelia Huck [mailto:cornelia.huck@de.ibm.com]
> Sent: Tuesday, August 9, 2016 6:44 PM
> To: Zeng, Xin <xin.zeng@intel.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>; Gonglei (Arei)
> <arei.gonglei@huawei.com>; qemu-devel@nongnu.org; virtio-
> dev@lists.oasis-open.org; Huangpeng (Peter)
> <peter.huangpeng@huawei.com>; Luonengjun <luonengjun@huawei.com>;
> stefanha@redhat.com; denglingli@chinamobile.com; Jani Kokkonen
> <Jani.Kokkonen@huawei.com>; Ola.Liljedahl@arm.com;
> Varun.Sethi@freescale.com; Keating, Brian A <brian.a.keating@intel.com>;
> Ma, Liang J <liang.j.ma@intel.com>; Griffin, John <john.griffin@intel.com>;
> Hanweidong (Randy) <hanweidong@huawei.com>; Huangweidong (C)
> <weidong.huang@huawei.com>
> Subject: Re: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device
> specification
> 
> On Mon, 8 Aug 2016 06:27:15 +0000
> "Zeng, Xin" <xin.zeng@intel.com> wrote:
> 
> > On Thu, Friday, August 05, 2016 3:56 AM, Michael S. Tsirkin wrote:
> > > -----Original Message-----
> > > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > > Sent: Friday, August 05, 2016 3:56 AM
> > > To: Gonglei (Arei)
> > > Cc: Zeng, Xin; qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org;
> > > Huangpeng (Peter); Luonengjun; cornelia.huck@de.ibm.com;
> > > stefanha@redhat.com; denglingli@chinamobile.com; Jani Kokkonen;
> > > Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Keating, Brian A;
> Ma,
> > > Liang J; Griffin, John; Hanweidong (Randy); Huangweidong (C)
> > > Subject: Re: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device
> > > specification
> > >
> > > On Thu, Aug 04, 2016 at 11:24:26AM +0000, Gonglei (Arei) wrote:
> > > > > > +The first driver-read-only field, \field{version} specifies the
> > > > > > +virtio crypto's version, which is reserved for back-compatibility
> > > > > > +in future.It's currently defined for the version field:
> > > > > > +
> > > > > > +\begin{lstlisting}
> > > > > > +#define VIRTIO_CRYPTO_VERSION_1  (1)
> > > > >
> > > > > Suggest to remove this macro,
> > > > > Do you think a version which is composed of major version and minor
> > > > > version is better?
> > > > >
> > > >
> > > > I think we should tell the developer how to set the value of version
> > > > field, but I have no idea about which value or form is better, so I
> > > > used 1 as the first version. What's your opinion?
> > >
> > > My opinion is that you should drop this completely. We do feature bits,
> not
> > > version numbers in virtio. We do not want each device doing its own
> thing for
> > > compatibility.
> > >
> >
> > But as I mentioned before,  considering the bug fix case, if each backend
> device
> > release need a feature bit meaning "some bugs are fixed", are the feature
> bits
> > enough?
> > Physical devices usually have a revision ID to mark its version, could we
> have a
> > revision Id field for each virtio device to distinguish the the virtio devices
> which
> > have the same feature sets but have different version?
> 
> I think we really need to decouple device features from bugs in a
> certain implementation.
> 
> Let's say we have a working virtio-crypto device in qemu 3.1, and a
> working virtio-crypto device in another hypervisor 1.3. Both advertise
> version 1.
> 
> Now we realize that we completely messed up the implementation of a
> certain algorithm in qemu. We release qemu 3.2 with this fixed. The
> other hypervisor 1.3 was not broken, and therefore they don't need to
> do an update.
> 
> What should happen to 'version' now? If we keep it at 1, it does not
> convey any further information. If we bump it to 2 in qemu 3.2, the
> guest still does not know whether a version 1 device is broken or not:
> It might be running in the other hypervisor 1.3, which is fine. We
> would have basically forced the other hypervisor to release 1.4 for the
> version bump to 2 (and deprecated a perfectly fine implementation).
> 
> For the above case, the only sane solutions I see are (a) have the host
> admin deal with it by either updating qemu or disabling the broken
> feature, or (b) have the guest discover the hypervisor and its version
> and avoid using the feature if the version is known bad.

The frontend driver and the backend device's compatibility is what I 
considered. We can't avoid the issue in the device's implementation, 
so it's better to let the newer frontend driver know whether the device 
it's currently using has some defects so that it could use workaround 
instead.
Thanks.

> 
> If the 'bugs' are rather design problems in the specification of an
> algorithm, we should replace it with a v2 specification and guard that
> via a feature bit. That is hopefully rare.
> 
> tl;dr - I think we should just drop version as it is not helpful.

Re: [Qemu-devel] [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification

[Gonglei (Arei)]

Hi guys,

Thanks for your good comments, we should come to an agreement on droping version filed
In virtio crypto specific. And I'll update a new version soon.

Regards,
-Gonglei


> -----Original Message-----
> From: Zeng, Xin [mailto:xin.zeng@intel.com]
> Sent: Tuesday, August 09, 2016 9:42 PM
> To: Michael S. Tsirkin
> Cc: Gonglei (Arei); qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org;
> Huangpeng (Peter); Luonengjun; cornelia.huck@de.ibm.com;
> stefanha@redhat.com; denglingli@chinamobile.com; Jani Kokkonen;
> Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Keating, Brian A; Ma,
> Liang J; Griffin, John; Hanweidong (Randy); Huangweidong (C)
> Subject: RE: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device specification
> 
> On Tuesday, August 9, 2016 6:58 PM Michael S. Tsirkin Wrote:
> > -----Original Message-----
> > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > Sent: Tuesday, August 9, 2016 6:58 PM
> > To: Zeng, Xin <xin.zeng@intel.com>
> > Cc: Gonglei (Arei) <arei.gonglei@huawei.com>; qemu-devel@nongnu.org;
> > virtio-dev@lists.oasis-open.org; Huangpeng (Peter)
> > <peter.huangpeng@huawei.com>; Luonengjun <luonengjun@huawei.com>;
> > cornelia.huck@de.ibm.com; stefanha@redhat.com;
> > denglingli@chinamobile.com; Jani Kokkonen <Jani.Kokkonen@huawei.com>;
> > Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Keating, Brian A
> > <brian.a.keating@intel.com>; Ma, Liang J <liang.j.ma@intel.com>; Griffin,
> > John <john.griffin@intel.com>; Hanweidong (Randy)
> > <hanweidong@huawei.com>; Huangweidong (C)
> > <weidong.huang@huawei.com>
> > Subject: Re: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device
> > specification
> >
> > On Mon, Aug 08, 2016 at 06:27:15AM +0000, Zeng, Xin wrote:
> > > On Thu, Friday, August 05, 2016 3:56 AM, Michael S. Tsirkin wrote:
> > > > -----Original Message-----
> > > > From: Michael S. Tsirkin [mailto:mst@redhat.com]
> > > > Sent: Friday, August 05, 2016 3:56 AM
> > > > To: Gonglei (Arei)
> > > > Cc: Zeng, Xin; qemu-devel@nongnu.org; virtio-dev@lists.oasis-open.org;
> > > > Huangpeng (Peter); Luonengjun; cornelia.huck@de.ibm.com;
> > > > stefanha@redhat.com; denglingli@chinamobile.com; Jani Kokkonen;
> > > > Ola.Liljedahl@arm.com; Varun.Sethi@freescale.com; Keating, Brian A;
> > Ma,
> > > > Liang J; Griffin, John; Hanweidong (Randy); Huangweidong (C)
> > > > Subject: Re: [PATCH v6 1/2] virtio-crypto: Add virtio crypto device
> > > > specification
> > > >
> > > > On Thu, Aug 04, 2016 at 11:24:26AM +0000, Gonglei (Arei) wrote:
> > > > > > > +The first driver-read-only field, \field{version} specifies the
> > > > > > > +virtio crypto's version, which is reserved for back-compatibility
> > > > > > > +in future.It's currently defined for the version field:
> > > > > > > +
> > > > > > > +\begin{lstlisting}
> > > > > > > +#define VIRTIO_CRYPTO_VERSION_1  (1)
> > > > > >
> > > > > > Suggest to remove this macro,
> > > > > > Do you think a version which is composed of major version and minor
> > > > > > version is better?
> > > > > >
> > > > >
> > > > > I think we should tell the developer how to set the value of version
> > > > > field, but I have no idea about which value or form is better, so I
> > > > > used 1 as the first version. What's your opinion?
> > > >
> > > > My opinion is that you should drop this completely. We do feature bits,
> > not
> > > > version numbers in virtio. We do not want each device doing its own
> > thing for
> > > > compatibility.
> > > >
> > >
> > > But as I mentioned before,  considering the bug fix case, if each backend
> > device
> > > release need a feature bit meaning "some bugs are fixed", are the feature
> > bits
> > > enough?
> >
> > It depends on whether the bug is very entrenched and important. In most
> > cases, for minor bugs, it's better to just fix the bug in the driver or
> > the hypervisor and be done with it.  For cases where
> > that's not feasible because many drivers relied on a specific bug,
> > and the bug is very important, we can always add more
> > if we run out of feature bits.
> >
> > > Physical devices usually have a revision ID to mark its version,
> >
> > Because compatibility is one way (new devices usually need
> > new drivers).
> >
> 
> Yes, that's also why I propose to put version(revision) field into device
> configuration read-only filed instead of feature bits field.
> 
> > > could we have a
> > > revision Id field for each virtio device to distinguish the the virtio devices
> > which
> > > have the same feature sets but have different version?
> >
> > ccw has version negotiation. It was only changed once at the 0.9->1.0
> > transition so far, it's not used for bug fixes.  We could discuss adding
> > this to pci and mmio as well, but if yes this should be discussed
> > separately.
> >
> 
> Ok, that's good, I do think this is needed. I can initiate this discussion in
> another separated mail loop.
> 
> > So far no argument made here was crypto specific, so
> > let's not put this in the crypto device.
> >
> 
> Sure,  It is indeed not crypto device specific,  but probably all virtio
> devices needs. We can drop it from crypto device spec.
> Thanks!
> 
> >
> > > > --
> > > > MST