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

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

[Gonglei]

As a non-native English speaker, I sincerely hope you
give me more suggestions about English style and/or
grammar for the spec. Thanks!


v18 -> v17:
 - fix many English grammar problems suggested by Stefan, Thanks a lot!

v17 -> v16:
 - Some grammar fixes [Stefan, Halil, Michael]
 - add a section named "Supported crypto services" in order to explain bit
   numbers and valuse clearly. [Halil, Cornelia]
 - avoid word reptition [Halil]
 - rename non-session mode to stateless mode [Halil]
 - change descriptions for all elements in struct virtio_crypto_config [Halil]
 - add Halil as a reviewer in the ackonwledgement part, thanks for his work.
 - other fixes here and there.

Changes since v15:
 - use feature bits for non-session mode in order to keep compatibility with
   pre-existing code. [Halil & Michael]
 - introduce VIRTIO_CRYPTO_F_ NON_SESSION_MODE feature bit to control all other
   non-session mode feature bits.
 - fix some typos. [Stefan]
 - introduce struct virtio_crypto_op_data_req_mux to support both session
   and non-session based crypto operations and keep compatibility with
   pre-existing code.

Changes since v14:
 - drop VIRTIO_CRYPTO_S_STARTED status [Halil & Cornelia]
 - correct a sentence about dataqueue and controlq in the first paragraph. [Halil]
 - change a MAY to MUST about max_dataqueues. [Halil]
 - add non-session mode support
   a) add four features for different crypto services to identify wheather support session mode.
   b) rewrite some

For pervious versions of virtio crypto spec, Pls see:

[v14]:
https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg02212.html

[v13]:
https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg07348.html

For more information, please see:
 http://qemu-project.org/Features/VirtioCrypto


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

 acknowledgements.tex |    2 +
 conformance.tex      |   29 ++
 content.tex          |    2 +
 virtio-crypto.tex    | 1309 ++++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 1342 insertions(+)
 create mode 100644 virtio-crypto.tex

-- 
1.7.12.4

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

[Gonglei]

The virtio crypto device is a virtual crypto device (ie. hardware
crypto accelerator card). Currently, the virtio crypto device provides
the following crypto services: CIPHER, MAC, HASH, and AEAD.

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

VIRTIO-153

Signed-off-by: Gonglei <arei.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: Mihai Claudiu Caraman <mike.caraman@nxp.com>
CC: Halil Pasic <pasic@linux.vnet.ibm.com>
---
 acknowledgements.tex |    2 +
 content.tex          |    2 +
 virtio-crypto.tex    | 1309 ++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1313 insertions(+)
 create mode 100644 virtio-crypto.tex

diff --git a/acknowledgements.tex b/acknowledgements.tex
index 53942b0..43b8a9b 100644
--- a/acknowledgements.tex
+++ b/acknowledgements.tex
@@ -26,6 +26,7 @@ Sasha Levin,	Oracle	\newline
 Sergey Tverdyshev,	Thales e-Security	\newline
 Stefan Hajnoczi,	Red Hat	\newline
 Tom Lyon,	Samya Systems, Inc.	\newline
+Lei Gong,  Huawei   \newline
 \end{oasistitlesection}
 
 The following non-members have provided valuable feedback on this
@@ -44,4 +45,5 @@ Patrick Durusau,	Technical Advisory Board, OASIS	\newline
 Thomas Huth,	Red Hat	\newline
 Yan Vugenfirer, Red Hat / Daynix	\newline
 Kevin Lo,	MSI	\newline
+Halil Pasic, IBM  \newline
 \end{oasistitlesection}
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..2708023
--- /dev/null
+++ b/virtio-crypto.tex
@@ -0,0 +1,1309 @@
+\section{Crypto Device}\label{sec:Device Types / Crypto Device}
+
+The virtio crypto device is a virtual cryptography device as well as a kind of
+virtual hardware accelerator for virtual machines. The encryption and
+decryption requests are placed in any of the data queues and are ultimately handled by the
+backend crypto accelerators. The second kind of queue is the control queue used to create 
+or destroy sessions for symmetric algorithms and will control some advanced
+features in the future. The virtio crypto device provides the following crypto
+services: CIPHER, MAC, HASH, and AEAD.
+
+
+\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}.
+
+\subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature bits}
+
+VIRTIO_CRYPTO_F_STATELESS_MODE (0) stateless mode is available.
+VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE (1) stateless mode is available for CIPHER service.
+VIRTIO_CRYPTO_F_HASH_STATELESS_MODE (2) stateless mode is available for HASH service.
+VIRTIO_CRYPTO_F_MAC_STATELESS_MODE (3) stateless mode is available for MAC service.
+VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE (4) stateless mode is available for AEAD service.
+
+\subsubsection{Feature bit requirements}\label{sec:Device Types / Crypto Device / Feature bits}
+
+Some crypto feature bits require other crypto feature bits
+(see \ref{drivernormative:Basic Facilities of a Virtio Device / Feature Bits}):
+
+\begin{description}
+\item[VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
+\item[VIRTIO_CRYPTO_F_HASH_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
+\item[VIRTIO_CRYPTO_F_MAC_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
+\item[VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
+\end{description}
+
+\subsection{Supported crypto services}\label{sec:Device Types / Crypto Device / Supported crypto services}
+
+The virtio crypto device provides the following crypto services: CIPHER, MAC, HASH, and AEAD.
+
+\begin{lstlisting}
+/* CIPHER service */
+#define VIRTIO_CRYPTO_SERVICE_CIPHER 0
+/* HASH service */
+#define VIRTIO_CRYPTO_SERVICE_HASH   1
+/* MAC (Message Authentication Codes) service */
+#define VIRTIO_CRYPTO_SERVICE_MAC    2
+/* AEAD (Authenticated Encryption with Associated Data) service */
+#define VIRTIO_CRYPTO_SERVICE_AEAD   3
+\end{lstlisting}
+
+The above constants are bit numbers, which tell the driver which crypto services
+are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
+
+\subsubsection{CIPHER services}\label{sec:Device Types / Crypto Device / Supported crypto services / CIPHER services}
+
+The following CIPHER algorithms are defined:
+
+\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 above constants have two usages:
+\begin{enumerate}
+\item As bit numbers, used to tell the driver which CIPHER algorithms
+are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
+\item As values, used to tell the device which CIPHER algorithm
+a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
+\end{enumerate}
+
+\subsubsection{HASH services}\label{sec:Device Types / Crypto Device / Supported crypto services / HASH services}
+
+The following HASH algorithms are defined:
+
+\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 above constants have two usages:
+\begin{enumerate}
+\item As bit numbers, used to tell the driver which HASH algorithms
+are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
+\item As values, used to tell the device which HASH algorithm
+a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
+\end{enumerate}
+
+\subsubsection{MAC services}\label{sec:Device Types / Crypto Device / Supported crypto services / MAC services}
+
+The following MAC algorithms are defined:
+
+\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_KASUMI_F9                27
+#define VIRTIO_CRYPTO_MAC_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 above constants have two usages:
+\begin{enumerate}
+\item As bit numbers, used to tell the driver which MAC algorithms
+are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
+\item As values, used to tell the device which MAC algorithm
+a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
+\end{enumerate}
+
+\subsubsection{AEAD services}\label{sec:Device Types / Crypto Device / Supported crypto services / AEAD services}
+
+The following AEAD algorithms are defined:
+
+\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}
+
+The above constants have two usages:
+\begin{enumerate}
+\item As bit numbers, used to tell the driver which AEAD algorithms
+are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
+\item As values, used to tell the device what AEAD algorithm
+a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
+\end{enumerate}
+
+\subsection{Device configuration layout}\label{sec:Device Types / Crypto Device / Device configuration layout}
+
+\begin{lstlisting}
+struct virtio_crypto_config {
+    le32 status;
+    le32 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 aead_algo;
+    /* Maximum length of cipher key in bytes */
+    le32 max_cipher_key_len;
+    /* Maximum length of authenticated key in bytes */
+    le32 max_auth_key_len;
+    le32 reserved;
+    /* Maximum size of each crypto request's content in bytes */
+    le64 max_size;
+};
+\end{lstlisting}
+
+\begin{description}
+\item[\field{status}] is used to show whether the device is ready to work or not, it can be either zero or have one or more flags
+    Only one read-only bit (for the driver) 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}
+
+\item[\field{max_dataqueues}] is the maximum number of data virtqueues exposed by
+    the device. The driver MAY use only one data queue,
+    or it can use more to achieve better performance.
+
+\item[\field{crypto_services}] is a 32-bit mask which indicates the crypto services supported by
+    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services}.
+
+\item[\field{cipher_algo_l}] is the low 32-bit mask which indicates the CIPHER algorithms supported by
+    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / CIPHER services}.
+
+\item[\field{cipher_algo_h}] is the high 32-bit mask which indicates the CIPHER algorithms supported by
+    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / CIPHER services}.
+
+\item[\field{hash_algo}] is a 32-bit mask which indicates the HASH algorithms supported by
+    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / HASH services}.
+
+\item[\field{mac_algo_l}] is the low 32-bit mask which indicates the MAC algorithms supported by
+    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / MAC services}.
+
+\item[\field{mac_algo_h}] is the high 32-bit mask which indicates the MAC algorithms supported by
+    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / MAC services}.
+
+\item[\field{aead_algo}] is a 32-bit mask which indicates the AEAD algorithms supported by
+    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / AEAD services}.
+
+\item[\field{max_cipher_key_len}] is the maximum length of cipher key supported by the device.
+
+\item[\field{max_auth_key_len}] is the maximum length of authenticated key supported by the device.
+
+\item[\field{reserved}] is reserved for future use.
+
+\item[\field{max_size}] is the maximum size of each crypto request's content supported by the device
+\end{description}
+
+\begin{note}
+Unless explicitly stated otherwise all lengths and sizes are in bytes.
+\end{note}
+
+\devicenormative{\subsubsection}{Device configuration layout}{Device Types / Crypto Device / Device configuration layout}
+
+\begin{itemize*}
+\item The device MUST set \field{max_dataqueues} to between 1 and 65535 inclusive.
+\item The device MUST set \field{status} based on the status of the backend crypto accelerator. 
+\item The device MUST accept and handle requests after \field{status} is set to VIRTIO_CRYPTO_S_HW_READY.
+\item The device MUST set \field{crypto_services} based on the crypto services the device offers.
+\item The device MUST set detailed algorithms masks based on the \field{crypto_services} field.
+\item The device MUST set \field{max_size} to show the maximum size of crypto request the device supports.
+\item The device MUST set \field{max_cipher_key_len} to show the maximum length of cipher key if the device supports CIPHER service.
+\item The device MUST set \field{max_auth_key_len} to show the maximum length of authenticated key if the device supports MAC service.
+\end{itemize*}
+
+\drivernormative{\subsubsection}{Device configuration layout}{Device Types / Crypto Device / Device configuration layout}
+
+\begin{itemize*}
+\item The driver MUST read the ready \field{status} from the bottom bit of status to check whether the backend crypto accelerator
+      is ready or not, and the driver MUST reread it after device reset. 
+\item The driver MUST NOT transmit any requests to the device if the ready \field{status} is not set.
+\item The driver MUST read \field{max_dataqueues} field to discover the number of 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 the detailed algorithms fields based on \field{crypto_services} field.
+\item The driver SHOULD read \field{max_size} to discover the maximum size of crypto request the device supports.
+\item The driver SHOULD read \field{max_cipher_key_len} to discover the maximum length of cipher key the device supports.
+\item The driver SHOULD read \field{max_auth_key_len} to discover the maximum length of authenticated key the device supports.
+\end{itemize*}
+
+\subsection{Device Initialization}\label{sec:Device Types / Crypto Device / Device Initialization}
+
+\drivernormative{\subsubsection}{Device Initialization}{Device Types / Crypto Device / Device Initialization}
+
+\begin{itemize*}
+\item The driver MUST identify and initialize all virtqueues.
+\item The driver MUST read the supported crypto services from bits of \field{crypto_services}. 
+\item The driver MUST read the supported algorithms based on \field{crypto_services} field.
+\end{itemize*}
+
+\subsection{Device Operation}\label{sec:Device Types / Crypto Device / Device Operation}
+
+Requests can be transmitted by placing them in the controlq or dataq.
+Requests consist of a queue-type specific header specifying among
+others the operation, and an operation specific payload.
+The payload is generally composed of operation parameters, output data, and input data.
+Operation parameters are algorithm-specific parameters, output data is the
+data that should be utilized in operations, and input data is equal to
+"operation result + result data".
+
+The device can support both session mode (See \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}) and stateless mode.
+If VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE is negotiated, the driver can use stateless mode for CIPHER service, otherwise it can only use session mode.
+
+The header for controlq is as follows:
+
+\begin{lstlisting}
+#define VIRTIO_CRYPTO_OPCODE(service, op)   (((service) << 8) | (op))
+
+struct virtio_crypto_ctrl_header {
+#define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x02)
+#define VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x03)
+#define VIRTIO_CRYPTO_HASH_CREATE_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x02)
+#define VIRTIO_CRYPTO_HASH_DESTROY_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x03)
+#define VIRTIO_CRYPTO_MAC_CREATE_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x02)
+#define VIRTIO_CRYPTO_MAC_DESTROY_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x03)
+#define VIRTIO_CRYPTO_AEAD_CREATE_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02)
+#define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \
+       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03)
+    le32 opcode;
+    /* algo should be service-specific algorithms */
+    le32 algo;
+    le32 flag;
+    /* data virtqueue id */
+    le32 queue_id;
+};
+\end{lstlisting}
+
+The header for dataq is as follows:
+
+\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;
+    le64 session_id;
+#define VIRTIO_CRYPTO_FLAG_STATE_MODE 1
+#define VIRTIO_CRYPTO_FLAG_STATELESS_MODE 2
+    /* control flag to control the request */
+    le32 flag;
+    le32 padding;
+};
+\end{lstlisting}
+
+The device can set the operation status as follows: VIRTIO_CRYPTO_OK: success;
+VIRTIO_CRYPTO_ERR: failure or device error; VIRTIO_CRYPTO_NOTSUPP: not supported;
+VIRTIO_CRYPTO_INVSESS: invalid session ID when executing crypto operations.
+
+\begin{lstlisting}
+enum VIRTIO_CRYPTO_STATUS {
+    VIRTIO_CRYPTO_OK = 0,
+    VIRTIO_CRYPTO_ERR = 1,
+    VIRTIO_CRYPTO_BADMSG = 2,
+    VIRTIO_CRYPTO_NOTSUPP = 3,
+    VIRTIO_CRYPTO_INVSESS = 4,
+    VIRTIO_CRYPTO_MAX
+};
+\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, such as session operations (See \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}).
+
+Controlq requests are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_op_ctrl_req {
+    struct virtio_crypto_ctrl_header header;
+
+    union {
+        struct virtio_crypto_sym_create_session_req   sym_create_session;
+        struct virtio_crypto_hash_create_session_req  hash_create_session;
+        struct virtio_crypto_mac_create_session_req   mac_create_session;
+        struct virtio_crypto_aead_create_session_req  aead_create_session;
+        struct virtio_crypto_destroy_session_req      destroy_session;
+    } u;
+};
+\end{lstlisting}
+
+struct virtio_crypto_op_ctrl_req is the only allowed control request.
+The header is the general header, and the union is of the algorithm-specific type,
+which is set by the driver. All the properties in the union are shown as follows.
+
+\paragraph{Session operation}\label{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}
+
+The symmetric algorithms involve 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:
+
+\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 set data, including the CIPHER algorithm and mode,
+      the key and its length, and the direction (encryption or decryption).
+\item The HASH/MAC set data, including the HASH algorithm or MAC algorithm,
+      and hash 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}
+
+The following structure stores the result of session creation set by the device:
+
+\begin{lstlisting}
+struct virtio_crypto_session_input {
+    /* Device-writable part */
+    le64 session_id;
+    le32 status;
+    le32 padding;
+};
+\end{lstlisting}
+
+A request to destroy a session includes the following information:
+
+\begin{lstlisting}
+struct virtio_crypto_destroy_session_req {
+    /* Device-readable part */
+    le64  session_id;
+    /* Device-writable part */
+    le32  status;
+    le32  padding;
+};
+\end{lstlisting}
+
+\subparagraph{Session operation: HASH session}\label{sec:Device Types / Crypto Device / Device
+Operation / Control Virtqueue / Session operation / Session operation: HASH session}
+
+HASH session requests are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_hash_session_para {
+    /* See VIRTIO_CRYPTO_HASH_* above */
+    le32 algo;
+    /* hash result length */
+    le32 hash_result_len;
+};
+struct virtio_crypto_hash_create_session_req {
+    /* Device-readable part */
+    struct virtio_crypto_hash_session_para para;
+    /* Device-writable part */
+    struct virtio_crypto_session_input input;
+};
+\end{lstlisting}
+
+\subparagraph{Session operation: MAC session}\label{sec:Device Types / Crypto Device / Device
+Operation / Control Virtqueue / Session operation / Session operation: MAC session}
+
+MAC session requests are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_mac_session_para {
+    /* See VIRTIO_CRYPTO_MAC_* above */
+    le32 algo;
+    /* hash result length */
+    le32 hash_result_len;
+    /* length of authenticated key */
+    le32 auth_key_len;
+    le32 padding;
+};
+
+struct virtio_crypto_mac_create_session_req {
+    /* Device-readable part */
+    struct virtio_crypto_mac_session_para para;
+    /* The authenticated key */
+    u8 auth_key[auth_key_len];
+
+    /* Device-writable part */
+    struct virtio_crypto_session_input input;
+};
+\end{lstlisting}
+
+\subparagraph{Session operation: Symmetric algorithms session}\label{sec:Device Types / Crypto Device / Device
+Operation / Control Virtqueue / Session operation / Session operation: Symmetric algorithms session}
+
+The request of symmetric session includes two parts, CIPHER algorithms and chain
+algorithms (chaining CIPHER and HASH/MAC). 
+
+CIPHER session requests are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_cipher_session_para {
+    /* See VIRTIO_CRYPTO_CIPHER* above */
+    le32 algo;
+    /* length of key */
+    le32 keylen;
+#define VIRTIO_CRYPTO_OP_ENCRYPT  1
+#define VIRTIO_CRYPTO_OP_DECRYPT  2
+    /* encryption or decryption */
+    le32 op;
+    le32 padding;
+};
+
+struct virtio_crypto_cipher_session_req {
+    /* Device-readable part */
+    struct virtio_crypto_cipher_session_para para;
+    /* The cipher key */
+    u8 cipher_key[keylen];
+
+    /* Device-writable part */
+    struct virtio_crypto_session_input input;
+};
+\end{lstlisting}
+
+Algorithm chaining requests are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_alg_chain_session_para {
+#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER  1
+#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH  2
+    le32 alg_chain_order;
+/* Plain hash */
+#define VIRTIO_CRYPTO_SYM_HASH_MODE_PLAIN    1
+/* Authenticated hash (mac) */
+#define VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH     2
+/* Nested hash */
+#define VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED   3
+    le32 hash_mode;
+    struct virtio_crypto_cipher_session_para cipher_param;
+    union {
+        struct virtio_crypto_hash_session_para hash_param;
+        struct virtio_crypto_mac_session_para mac_param;
+    } u;
+    /* length of the additional authenticated data (AAD) in bytes */
+    le32 aad_len;
+    le32 padding;
+};
+
+struct virtio_crypto_alg_chain_session_req {
+    /* Device-readable part */
+    struct virtio_crypto_alg_chain_session_para para;
+    /* The cipher key */
+    u8 cipher_key[keylen];
+    /* The authenticated key */
+    u8 auth_key[auth_key_len];
+
+    /* Device-writable part */
+    struct virtio_crypto_session_input input;
+};
+\end{lstlisting}
+
+Symmetric algorithm requests are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_sym_create_session_req {
+    union {
+        struct virtio_crypto_cipher_session_req cipher;
+        struct virtio_crypto_alg_chain_session_req chain;
+    } u;
+
+    /* Device-readable part */
+
+/* No operation */
+#define VIRTIO_CRYPTO_SYM_OP_NONE  0
+/* Cipher only operation on the data */
+#define VIRTIO_CRYPTO_SYM_OP_CIPHER  1
+/* Chain any cipher with any hash or mac operation. The order
+   depends on the value of alg_chain_order param */
+#define VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING  2
+    le32 op_type;
+    le32 padding;
+};
+\end{lstlisting}
+
+The driver can set the \field{op_type} field in struct virtio_crypto_sym_create_session_req as follows: VIRTIO_CRYPTO_SYM_OP_NONE: no operation;
+VIRTIO_CRYPTO_SYM_OP_CIPHER: Cipher only operation on the data; VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING: Chain any cipher with any hash or mac operation.
+
+\subparagraph{Session operation: AEAD session}\label{sec:Device Types / Crypto Device / Device
+Operation / Control Virtqueue / Session operation / Session operation: AEAD session}
+
+AEAD session requests are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_aead_session_para {
+    /* See VIRTIO_CRYPTO_AEAD_* above */
+    le32 algo;
+    /* length of key */
+    le32 key_len;
+    /* Authentication tag length */
+    le32 tag_len;
+    /* The length of the additional authenticated data (AAD) in bytes */
+    le32 aad_len;
+    /* encryption or decryption, See above VIRTIO_CRYPTO_OP_* */
+    le32 op;
+    le32 padding;
+};
+
+struct virtio_crypto_aead_create_session_req {
+    /* Device-readable part */
+    struct virtio_crypto_aead_session_para para;
+    u8 key[key_len];
+
+    /* Device-writeable part */
+    struct virtio_crypto_session_input input;
+};
+\end{lstlisting}
+
+\drivernormative{\subparagraph}{Session operation: create session}{Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation / Session operation: create session}
+
+\begin{itemize*}
+\item The driver MUST set the control general header and corresponding properties of the union in structure virtio_crypto_op_ctrl_req. See \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue}.
+\item The driver MUST set the \field{opcode} field based on service type: CIPHER, HASH, MAC, or AEAD.
+\item The driver MUST set the \field{queue_id} field to show used dataq.
+\end{itemize*}
+
+\devicenormative{\subparagraph}{Session operation: create session}{Device Types / Crypto Device / Device
+Operation / Control Virtqueue / Session operation / Session operation: create session}
+
+\begin{itemize*}
+\item The device MUST set the \field{session_id} field to a unique session identifier when the device finishes processing session creation.
+\item The device MUST set the \field{status} field to one of the values of enum VIRTIO_CRYPTO_STATUS.
+\end{itemize*}
+
+\drivernormative{\subparagraph}{Session operation: destroy session}{Device Types / Crypto Device / Device
+Operation / Control Virtqueue / Session operation / Session operation: destroy session}
+
+\begin{itemize*}
+\item The driver MUST set the \field{opcode} field based on service type: CIPHER, HASH, MAC, or AEAD.
+\item The driver MUST set the \field{session_id} to a valid value assigned by the device when the session was created.
+\end{itemize*}
+
+\devicenormative{\subparagraph}{Session operation: destroy session}{Device Types / Crypto Device / Device
+Operation / Control Virtqueue / Session operation / Session operation: destroy session}
+
+\begin{itemize*}
+\item The device MUST set the \field{status} field to one of the values of enum VIRTIO_CRYPTO_STATUS.
+\end{itemize*}
+
+\subsubsection{Data Virtqueue}\label{sec:Device Types / Crypto Device / Device Operation / Data Virtqueue}
+
+The driver uses the data virtqueue to transmit crypto operation requests to the device,
+and completes the crypto operations.
+
+Session mode dataq requests are as follows:
+
+\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}
+
+Dataq requests for both session and stateless modes are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_op_data_req_mux {
+    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;
+        struct virtio_crypto_sym_data_req_stateless   sym_stateless_req;
+        struct virtio_crypto_hash_data_req_stateless  hash_stateless_req;
+        struct virtio_crypto_mac_data_req_stateless   mac_stateless_req;
+        struct virtio_crypto_aead_data_req_stateless  aead_stateless_req;
+    } u;
+};
+\end{lstlisting}
+
+The header is the general header and the union is of the algorithm-specific type,
+which is set by the driver. All properties in the union are shown as follows.
+
+There is a unified input header structure for all crypto services.
+
+The structure is defined as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_inhdr {
+    u8 status;
+};
+\end{lstlisting}
+
+\subsubsection{HASH Service Operation}\label{sec:Device Types / Crypto Device / Device Operation / HASH Service Operation}
+
+Session mode HASH service requests are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_hash_para {
+    /* length of source data */
+    le32 src_data_len;
+    /* hash result length */
+    le32 hash_result_len;
+};
+
+struct virtio_crypto_hash_data_req {
+    /* Device-readable part */
+    struct virtio_crypto_hash_para para;
+    /* Source data */
+    u8 src_data[src_data_len];
+
+    /* Device-writable part */
+    /* Hash result data */
+    u8 hash_result[hash_result_len];
+    struct virtio_crypto_inhdr inhdr;
+};
+\end{lstlisting}
+
+Each data request uses virtio_crypto_hash_data_req structure to store information
+used to run the HASH operations. 
+
+The information includes the hash parameters stored in \field{para}, output data and input data.
+The output data here includes the source data and the input data includes the hash result data used to save the results of the HASH operations.
+\field{inhdr} stores the status of executing the HASH operations.
+
+Stateless mode HASH service requests are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_hash_para_statelesss {
+    struct {
+        /* See VIRTIO_CRYPTO_HASH_* above */
+        le32 algo;
+    } sess_para;
+
+    /* length of source data */
+    le32 src_data_len;
+    /* hash result length */
+    le32 hash_result_len;
+    le32 reserved;
+};
+struct virtio_crypto_hash_data_req_stateless {
+    /* Device-readable part */
+    struct virtio_crypto_hash_para_stateless para;
+    /* Source data */
+    u8 src_data[src_data_len];
+
+    /* Device-writable part */
+    /* Hash result data */
+    u8 hash_result[hash_result_len];
+    struct virtio_crypto_inhdr inhdr;
+};
+\end{lstlisting}
+
+\drivernormative{\paragraph}{HASH Service Operation}{Device Types / Crypto Device / Device Operation / HASH Service Operation}
+
+\begin{itemize*}
+\item If the driver uses the session mode, then the driver MUST set \field{session_id} in struct virtio_crypto_op_header
+      to a valid value assigned by the device when the session was created.
+\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated, the driver MUST use struct virtio_crypto_op_data_req_mux to wrap crypto requests. Otherwise, the driver MUST use struct virtio_crypto_op_data_req.
+\item If the VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is negotiated, 1) if the driver uses the stateless mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header
+      to VIRTIO_CRYPTO_FLAG_STATELESS_MODE and MUST set the fields in struct virtio_crypto_hash_para_statelession.sess_para, 2) if the driver uses the session mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header to VIRTIO_CRYPTO_FLAG_STATE_MODE.
+\item The driver MUST set \field{opcode} in struct virtio_crypto_op_header to VIRTIO_CRYPTO_HASH.
+\end{itemize*}
+
+\devicenormative{\paragraph}{HASH Service Operation}{Device Types / Crypto Device / Device Operation / HASH Service Operation}
+
+\begin{itemize*}
+\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated, the device MUST parse the struct virtio_crypto_op_data_req_mux for crypto requests. Otherwise, the device MUST parse the struct virtio_crypto_op_data_req.
+\item If the VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is negotiated, the device MUST parse \field{flag} field in struct virtio_crypto_op_header in order to decide which mode the driver uses.
+\item The device MUST copy the results of HASH operations in the hash_result[] if HASH operations success.
+\item The device MUST set \field{status} in struct virtio_crypto_inhdr to one of the values of enum VIRTIO_CRYPTO_STATUS.
+\end{itemize*}
+
+\subsubsection{MAC Service Operation}\label{sec:Device Types / Crypto Device / Device Operation / MAC Service Operation}
+
+Session mode MAC service requests are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_mac_para {
+    struct virtio_crypto_hash_para hash;
+};
+
+struct virtio_crypto_mac_data_req {
+    /* Device-readable part */
+    struct virtio_crypto_mac_para para;
+    /* Source data */
+    u8 src_data[src_data_len];
+
+    /* Device-writable part */
+    /* Hash result data */
+    u8 hash_result[hash_result_len];
+    struct virtio_crypto_inhdr inhdr;
+};
+\end{lstlisting}
+
+Each data request uses virtio_crypto_mac_data_req structure to store information
+used to run the MAC operations. 
+
+The information includes the hash parameters stored in \field{para}, output data and input data.
+The output data here includes the source data and the input data includes the hash result data used to save the results of the MAC operations.
+\field{inhdr} stores the status of executing the MAC operations.
+
+Stateless mode MAC service requests are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_mac_para_stateless {
+    struct {
+        /* See VIRTIO_CRYPTO_MAC_* above */
+        le32 algo;
+        /* length of authenticated key */
+        le32 auth_key_len;
+    } sess_para;
+
+    /* length of source data */
+    le32 src_data_len;
+    /* hash result length */
+    le32 hash_result_len;
+};
+
+struct virtio_crypto_mac_data_req_stateless {
+    /* Device-readable part */
+    struct virtio_crypto_mac_para_stateless para;
+    /* The authenticated key */
+    u8 auth_key[auth_key_len];
+    /* Source data */
+    u8 src_data[src_data_len];
+
+    /* Device-writable part */
+    /* Hash result data */
+    u8 hash_result[hash_result_len];
+    struct virtio_crypto_inhdr inhdr;
+};
+\end{lstlisting}
+
+\drivernormative{\paragraph}{MAC Service Operation}{Device Types / Crypto Device / Device Operation / MAC Service Operation}
+
+\begin{itemize*}
+\item If the driver uses the session mode, then the driver MUST set \field{session_id} in struct virtio_crypto_op_header
+      to a valid value assigned by the device when the session was created.
+\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated, the driver MUST use struct virtio_crypto_op_data_req_mux to wrap crypto requests. Otherwise, the driver MUST use struct virtio_crypto_op_data_req.
+\item If the VIRTIO_CRYPTO_F_MAC_STATELESS_MODE feature bit is negotiated, 1) if the driver uses the stateless mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header
+      to VIRTIO_CRYPTO_FLAG_STATELESS_MODE and MUST set the fields in struct virtio_crypto_mac_para_statelession.sess_para, 2) if the driver uses the session mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header to VIRTIO_CRYPTO_FLAG_STATE_MODE.
+\item The driver MUST set \field{opcode} in struct virtio_crypto_op_header to VIRTIO_CRYPTO_MAC.
+\end{itemize*}
+
+\devicenormative{\paragraph}{MAC Service Operation}{Device Types / Crypto Device / Device Operation / MAC Service Operation}
+
+\begin{itemize*}
+\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated, the device MUST parse the struct virtio_crypto_op_data_req_mux for crypto requests. Otherwise, the device MUST parse the struct virtio_crypto_op_data_req.
+\item If the VIRTIO_CRYPTO_F_MAC_STATELESS_MODE feature bit is negotiated, the device MUST parse \field{flag} field in struct virtio_crypto_op_header in order to decide which mode the driver uses.
+\item The device MUST copy the results of MAC operations in the hash_result[] if HASH operations success.
+\item The device MUST set \field{status} in struct virtio_crypto_inhdr to one of the values of enum VIRTIO_CRYPTO_STATUS.
+\end{itemize*}
+
+\subsubsection{Symmetric algorithms Operation}\label{sec:Device Types / Crypto Device / Device Operation / Symmetric algorithms Operation}
+
+Session mode CIPHER service requests are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_cipher_para {
+    /*
+     * Byte Length of valid IV/Counter data pointed to by the below iv data.
+     *
+     * For block ciphers in CBC or F8 mode, or for Kasumi in F8 mode, or for
+     *   SNOW3G in UEA2 mode, this is the length of the IV (which
+     *   must be the same as the block length of the cipher).
+     * For block ciphers in CTR mode, this is the length of the counter
+     *   (which must be the same as the block length of the cipher).
+     */
+    le32 iv_len;
+    /* length of source data */
+    le32 src_data_len;
+    /* length of destination data */
+    le32 dst_data_len;
+    le32 padding;
+};
+
+struct virtio_crypto_cipher_data_req {
+    /* Device-readable part */
+    struct virtio_crypto_cipher_para para;
+    /*
+     * Initialization Vector or Counter data.
+     *
+     * For block ciphers in CBC or F8 mode, or for Kasumi in F8 mode, or for
+     *   SNOW3G in UEA2 mode, this is the Initialization Vector (IV)
+     *   value.
+     * For block ciphers in CTR mode, this is the counter.
+     * For AES-XTS, this is the 128bit tweak, i, from IEEE Std 1619-2007.
+     *
+     * The IV/Counter will be updated after every partial cryptographic
+     * operation.
+     */
+    u8 iv[iv_len];
+    /* Source data */
+    u8 src_data[src_data_len];
+
+    /* Device-writable part */
+    /* Destination data */
+    u8 dst_data[dst_data_len];
+    struct virtio_crypto_inhdr inhdr;
+};
+\end{lstlisting}
+
+Session mode requests of algorithm chaining are as follows: 
+
+\begin{lstlisting}
+struct virtio_crypto_alg_chain_data_para {
+    le32 iv_len;
+    /* Length of source data */
+    le32 src_data_len;
+    /* Length of destination data */
+    le32 dst_data_len;
+    /* Starting point for cipher processing in source data */
+    le32 cipher_start_src_offset;
+    /* Length of the source data that the cipher will be computed on */
+    le32 len_to_cipher;
+    /* Starting point for hash processing in source data */
+    le32 hash_start_src_offset;
+    /* Length of the source data that the hash will be computed on */
+    le32 len_to_hash;
+    /* Length of the additional auth data */
+    le32 aad_len;
+    /* Length of the hash result */
+    le32 hash_result_len;
+    le32 reserved;
+};
+
+struct virtio_crypto_alg_chain_data_req {
+    /* Device-readable part */
+    struct virtio_crypto_alg_chain_data_para para;
+    /* Initialization Vector or Counter data */
+    u8 iv[iv_len];
+    /* Source data */
+    u8 src_data[src_data_len];
+    /* Additional authenticated data if exists */
+    u8 aad[aad_len];
+
+    /* Device-writable part */
+    /* Destination data */
+    u8 dst_data[dst_data_len];
+    /* Hash result data */
+    u8 hash_result[hash_result_len];
+    struct virtio_crypto_inhdr inhdr;
+};
+\end{lstlisting}
+
+Session mode requests of symmetric algorithm are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_sym_data_req {
+    union {
+        struct virtio_crypto_cipher_data_req cipher;
+        struct virtio_crypto_alg_chain_data_req chain;
+    } u;
+
+    /* Device-readable part */
+
+    /* See above VIRTIO_CRYPTO_SYM_OP_* */
+    le32 op_type;
+    le32 padding;
+};
+\end{lstlisting}
+
+Each data request uses virtio_crypto_sym_data_req structure to store information
+used to run the CIPHER operations. 
+
+The information includes the cipher parameters stored in \field{para}, output data and input data.
+In the first virtio_crypto_cipher_para structure, \field{iv_len} specifies the length of the initialization vector or counter,
+\field{src_data_len} specifies the length of the source data, and \field{dst_data_len} specifies the
+length of the destination data. 
+For plain CIPHER operations, the output data here includes the IV/Counter data and source data, and the input data includes the destination data used to save the results of the CIPHER operations. 
+
+For algorithms chain, the output data here includes the IV/Counter data, source data and additional authenticated data if exists.
+The input data includes both destination data and hash result data used to store the results of the HASH/MAC operations.
+\field{inhdr} stores the status of executing the crypto operations.
+
+Stateless mode CIPHER service requests are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_cipher_para_stateless {
+    struct {
+        /* See VIRTIO_CRYPTO_CIPHER* above */
+        le32 algo;
+        /* length of key */
+        le32 keylen;
+
+        /* See VIRTIO_CRYPTO_OP_* above */
+        le32 op;
+    } sess_para;
+
+    /*
+     * Byte Length of valid IV/Counter data pointed to by the below iv data.
+     */
+    le32 iv_len;
+    /* length of source data */
+    le32 src_data_len;
+    /* length of destination data */
+    le32 dst_data_len;
+};
+
+struct virtio_crypto_cipher_data_req_stateless {
+    /* Device-readable part */
+    struct virtio_crypto_cipher_para_stateless para;
+    /* The cipher key */
+    u8 cipher_key[keylen];
+
+    /* Initialization Vector or Counter data. */
+    u8 iv[iv_len];
+    /* Source data */
+    u8 src_data[src_data_len];
+
+    /* Device-writable part */
+    /* Destination data */
+    u8 dst_data[dst_data_len];
+    struct virtio_crypto_inhdr inhdr;
+};
+\end{lstlisting}
+
+Stateless mode requests of algorithm chaining are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_alg_chain_data_para_stateless {
+    struct {
+        /* See VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_* above */
+        le32 alg_chain_order;
+        /* length of the additional authenticated data in bytes */
+        le32 aad_len;
+
+        struct {
+            /* See VIRTIO_CRYPTO_CIPHER* above */
+            le32 algo;
+            /* length of key */
+            le32 keylen;
+            /* See VIRTIO_CRYPTO_OP_* above */
+            le32 op;
+        } cipher;
+
+        struct {
+            /* See VIRTIO_CRYPTO_HASH_* or VIRTIO_CRYPTO_MAC_* above */
+            le32 algo;
+            /* length of authenticated key */
+            le32 auth_key_len;
+            /* See VIRTIO_CRYPTO_SYM_HASH_MODE_* above */
+            le32 hash_mode;
+        } hash;
+    } sess_para;
+
+    le32 iv_len;
+    /* Length of source data */
+    le32 src_data_len;
+    /* Length of destination data */
+    le32 dst_data_len;
+    /* Starting point for cipher processing in source data */
+    le32 cipher_start_src_offset;
+    /* Length of the source data that the cipher will be computed on */
+    le32 len_to_cipher;
+    /* Starting point for hash processing in source data */
+    le32 hash_start_src_offset;
+    /* Length of the source data that the hash will be computed on */
+    le32 len_to_hash;
+    /* Length of the additional auth data */
+    le32 aad_len;
+    /* Length of the hash result */
+    le32 hash_result_len;
+    le32 reserved;
+};
+
+struct virtio_crypto_alg_chain_data_req_stateless {
+    /* Device-readable part */
+    struct virtio_crypto_alg_chain_data_para_stateless para;
+    /* The cipher key */
+    u8 cipher_key[keylen];
+    /* The auth key */
+    u8 auth_key[auth_key_len];
+    /* Initialization Vector or Counter data */
+    u8 iv[iv_len];
+    /* Source data */
+    u8 src_data[src_data_len];
+    /* Additional authenticated data if exists */
+    u8 aad[aad_len];
+
+    /* Device-writable part */
+    /* Destination data */
+    u8 dst_data[dst_data_len];
+    /* Hash result data */
+    u8 hash_result[hash_result_len];
+    struct virtio_crypto_inhdr inhdr;
+};
+\end{lstlisting}
+
+Stateless mode requests of symmetric algorithm are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_sym_data_req_stateless {
+    union {
+        struct virtio_crypto_cipher_data_req_stateless cipher;
+        struct virtio_crypto_alg_chain_data_req_stateless chain;
+    } u;
+
+    /* Device-readable part */
+
+    /* See above VIRTIO_CRYPTO_SYM_OP_* */
+    le32 op_type;
+    le32 padding;
+};
+\end{lstlisting}
+
+\drivernormative{\paragraph}{Symmetric algorithms Operation}{Device Types / Crypto Device / Device Operation / Symmetric algorithms Operation}
+
+\begin{itemize*}
+\item If the driver uses the session mode, then the driver MUST set \field{session_id} in struct virtio_crypto_op_header
+      to a valid value assigned by the device when the session was created.
+\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated, the driver MUST use struct virtio_crypto_op_data_req_mux to wrap crypto requests. Otherwise, the driver MUST use struct virtio_crypto_op_data_req.
+\item If the VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE feature bit is negotiated, 1) if the driver uses the stateless mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header
+      to VIRTIO_CRYPTO_FLAG_STATELESS_MODE and MUST set the fields in struct virtio_crypto_cipher_para_statelession.sess_para or struct virtio_crypto_alg_chain_data_para_stateless.sess_para, 2) if the driver uses the session mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header to VIRTIO_CRYPTO_FLAG_STATE_MODE.
+\item The driver MUST set the \field{opcode} field in struct virtio_crypto_op_header to VIRTIO_CRYPTO_CIPHER_ENCRYPT or VIRTIO_CRYPTO_CIPHER_DECRYPT.
+\item The driver MUST specify the fields of struct virtio_crypto_cipher_data_req in struct virtio_crypto_sym_data_req if the request is based on VIRTIO_CRYPTO_SYM_OP_CIPHER.
+\item The driver MUST specify the fields of both struct virtio_crypto_cipher_data_req and struct virtio_crypto_mac_data_req in struct virtio_crypto_sym_data_req if the request
+      is of the VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING type and in the VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH mode.
+\end{itemize*}
+
+\devicenormative{\paragraph}{Symmetric algorithms Operation}{Device Types / Crypto Device / Device Operation / Symmetric algorithms Operation}
+
+\begin{itemize*}
+\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated, the device MUST parse the struct virtio_crypto_op_data_req_mux for crypto requests. Otherwise, the device MUST parse the struct virtio_crypto_op_data_req.
+\item If the VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE feature bit is negotiated, the device MUST parse \field{flag} field in struct virtio_crypto_op_header in order to decide which mode the driver uses.
+\item The device MUST parse the virtio_crypto_sym_data_req based on the \field{opcode} field in general header.
+\item The device SHOULD only parse fields of struct virtio_crypto_cipher_data_req in struct virtio_crypto_sym_data_req if the request is VIRTIO_CRYPTO_SYM_OP_CIPHER type.
+\item The device MUST parse fields of both struct virtio_crypto_cipher_data_req and struct virtio_crypto_mac_data_req in struct virtio_crypto_sym_data_req if the request
+      is of the VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING operation type and in the VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH mode.
+\item The device MUST copy the result of cryptographic operation in the dst_data[] in both plain CIPHER mode and algorithms chain mode.
+\item The device MUST check the \field{para}.\field{add_len} is bigger than 0 before parse the additional authenticated data in plain algorithms chain mode.
+\item The device MUST copy the result of HASH/MAC operation in the hash_result[] is of the VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING type.
+\item The device MUST set the \field{status} field in struct virtio_crypto_inhdr to one of the values of enum VIRTIO_CRYPTO_STATUS.
+\end{itemize*}
+
+\paragraph{Steps of Operation}\label{sec:Device Types / Crypto Device / Device Operation / Symmetric algorithms Operation / Steps of Operation}
+
+\subparagraph{Step1: Create session}\label{sec:Device Types / Crypto Device / Device Operation / Symmetric algorithms Operation / Steps of Operation / Step1: Create session on session mode}
+
+\begin{enumerate}
+\item The driver specifies information in struct virtio_crypto_op_ctrl_req, including the algorithm name, key, keylen etc;
+\item The driver adds the request of session creation into the controlq's Vring Descriptor Table;
+\item The driver kicks the device;
+\item The device receives the request from controlq;
+\item The device parses information about the request, and determines the information concerning the backend crypto accelerator;
+\item The device packs information based on the APIs of the backend crypto accelerator;
+\item The device invokes the session creation APIs of the backend crypto accelerator to create a session;
+\item The device returns the session id to the driver.
+\end{enumerate}
+
+\subparagraph{Step2: Execute cryptographic operation}\label{sec:Device Types / Crypto Device / Device Operation / Symmetric algorithms Operation / Steps of Operation / Step2: Execute cryptographic operation}
+
+\begin{enumerate}
+\item The driver specifies information in struct virtio_crypto_op_data_req, including struct virtio_crypto_op_header and struct virtio_crypto_sym_data_req, see \ref{sec:Device Types / Crypto Device / Device
+      Operation / Symmetric algorithms Operation};
+\item The driver adds the request for cryptographic operation into the dataq's Vring Descriptor Table;
+\item The driver kicks the device (Or the device actively polls the dataq's Vring Descriptor Table);
+\item The device receives the request from dataq;
+\item The device parses information about the request, and determines the identification information for the backend crypto accelerator. For example, converting guest physical addresses to host physical addresses;
+\item The device packs identification information based on the API of the backend crypto accelerator;
+\item The device invokes the cryptographic APIs of the backend crypto accelerator;
+\item The backend crypto accelerator executes the cryptographic operation implicitly;
+\item The device receives the cryptographic results from the backend crypto accelerator (synchronous or asynchronous);
+\item The device sets the \field{status} in struct virtio_crypto_inhdr;
+\item The device updates and flushes the Used Ring to return the cryptographic results to the driver;
+\item The device notifies the driver (Or the driver actively polls the dataq's Used Ring);
+\item The driver saves the cryptographic results.
+\end{enumerate}
+
+\begin{note}
+\begin{itemize*}
+\item For better performance, the device should by preference use vhost scheme (user space or kernel space)
+      as the backend crypto accelerator in the real production environment.
+\end{itemize*}
+\end{note}
+
+\subsubsection{AEAD Service Operation}\label{sec:Device Types / Crypto Device / Device Operation / AEAD Service Operation}
+
+Session mode requests of symmetric algorithm are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_aead_para {
+    /*
+     * Byte Length of valid IV data.
+     *
+     * For GCM mode, this is either 12 (for 96-bit IVs) or 16, in which
+     *   case iv points to J0.
+     * For CCM mode, this is the length of the nonce, which can be in the
+     *   range 7 to 13 inclusive.
+     */
+    le32 iv_len;
+    /* length of additional auth data */
+    le32 aad_len;
+    /* length of source data */
+    le32 src_data_len;
+    /* length of dst data, this should be at least src_data_len + tag_len */
+    le32 dst_data_len;
+    /* Authentication tag length */
+    le32 tag_len;
+    le32 reserved;
+};
+
+struct virtio_crypto_aead_data_req {
+    /* Device-readable part */
+    struct virtio_crypto_aead_para para;
+    /*
+     * Initialization Vector data.
+     *
+     * For GCM mode, this is either the IV (if the length is 96 bits) or J0
+     *   (for other sizes), where J0 is as defined by NIST SP800-38D.
+     *   Regardless of the IV length, a full 16 bytes needs to be allocated.
+     * For CCM mode, the first byte is reserved, and the nonce should be
+     *   written starting at &iv[1] (to allow space for the implementation
+     *   to write in the flags in the first byte).  Note that a full 16 bytes
+     *   should be allocated, even though the iv_len field will have
+     *   a value less than this.
+     *
+     * The IV will be updated after every partial cryptographic operation.
+     */
+    u8 iv[iv_len];
+    /* Source data */
+    u8 src_data[src_data_len];
+    /* Additional authenticated data if exists */
+    u8 aad[aad_len];
+
+    /* Device-writable part */
+    /* Pointer to output data */
+    u8 dst_data[dst_data_len];
+
+    struct virtio_crypto_inhdr inhdr;
+};
+\end{lstlisting}
+
+Each data request uses virtio_crypto_aead_data_req structure to store information
+used to run the AEAD operations. 
+
+The information includes the hash parameters stored in \field{para}, output data and input data.
+In the first virtio_crypto_aead_para structure, \field{iv_len} specifies the length of the initialization vector. \field{tag_len} specifies the length of the authentication tag;
+\field{aad_len} specifies the length of additional authentication data, \field{src_data_len} specifies the
+length of the source data; \field{dst_data_len} specifies the length of the destination data, which is at least \field{src_data_len} + \field{tag_len}.
+
+The output data here includes the IV/Counter data, source data and additional authenticated data if exists.
+The input data includes both destination data used to save the results of the AEAD operations.
+\field{inhdr} stores the status of executing the AEAD operations.
+
+Stateless mode AEAD service requests are as follows:
+
+\begin{lstlisting}
+struct virtio_crypto_aead_para_stateless {
+    struct {
+        /* See VIRTIO_CRYPTO_AEAD_* above */
+        le32 algo;
+        /* length of key */
+        le32 key_len;
+        /* encrypt or decrypt, See above VIRTIO_CRYPTO_OP_* */
+        le32 op;
+    } sess_para;
+
+    /* Byte Length of valid IV data. */
+    le32 iv_len;
+    /* Authentication tag length */
+    le32 tag_len;
+    /* length of additional auth data */
+    le32 aad_len;
+    /* length of source data */
+    le32 src_data_len;
+    /* length of dst data, this should be at least src_data_len + tag_len */
+    le32 dst_data_len;
+};
+
+struct virtio_crypto_aead_data_req_stateless {
+    /* Device-readable part */
+    struct virtio_crypto_aead_para_stateless para;
+    /* The cipher key */
+    u8 key[key_len];
+    /* Initialization Vector data. */
+    u8 iv[iv_len];
+    /* Source data */
+    u8 src_data[src_data_len];
+    /* Additional authenticated data if exists */
+    u8 aad[aad_len];
+
+    /* Device-writable part */
+    /* Pointer to output data */
+    u8 dst_data[dst_data_len];
+
+    struct virtio_crypto_inhdr inhdr;
+};
+\end{lstlisting}
+
+\drivernormative{\paragraph}{AEAD Service Operation}{Device Types / Crypto Device / Device Operation / AEAD Service Operation}
+
+\begin{itemize*}
+\item If the driver uses the session mode, then the driver MUST set \field{session_id} in struct virtio_crypto_op_header
+      to a valid value assigned by the device when the session was created.
+\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated, the driver MUST use struct virtio_crypto_op_data_req_mux to wrap crypto requests. Otherwise, the driver MUST use struct virtio_crypto_op_data_req.
+\item If the VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE feature bit is negotiated, 1) if the driver uses the stateless mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header
+      to VIRTIO_CRYPTO_FLAG_STATELESS_MODE and MUST set the fields in struct virtio_crypto_aead_para_statelession.sess_para, 2) if the driver uses the session mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header to VIRTIO_CRYPTO_FLAG_STATE_MODE.
+\item The driver MUST set the \field{opcode} field in struct virtio_crypto_op_header to VIRTIO_CRYPTO_AEAD_ENCRYPT or VIRTIO_CRYPTO_AEAD_DECRYPT.
+\end{itemize*}
+
+\devicenormative{\paragraph}{AEAD Service Operation}{Device Types / Crypto Device / Device Operation / AEAD Service Operation}
+
+\begin{itemize*}
+\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated, the device MUST parse the struct virtio_crypto_op_data_req_mux for crypto requests. Otherwise, the device MUST parse the struct virtio_crypto_op_data_req.
+\item If the VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE feature bit is negotiated, the device MUST parse the virtio_crypto_aead_data_req based on the \field{opcode} field in general header.
+\item The device MUST copy the result of cryptographic operation in the dst_data[].
+\item The device MUST copy the authentication tag in the dst_data[] offset the cipher result.
+\item The device MUST set the \field{status} field in struct virtio_crypto_inhdr to one of the values of enum VIRTIO_CRYPTO_STATUS.
+\item When the \field{opcode} field is VIRTIO_CRYPTO_AEAD_DECRYPT, the device MUST verify and return the verification result to the driver, and if the verification result is incorrect, VIRTIO_CRYPTO_BADMSG (bad message) MUST be returned to the driver.
+\end{itemize*}
\ No newline at end of file
-- 
1.7.12.4

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

[Halil Pasic]

On 04/22/2017 08:23 AM, Gonglei wrote:
> The virtio crypto device is a virtual crypto device (ie. hardware
> crypto accelerator card). Currently, the virtio crypto device provides
> the following crypto services: CIPHER, MAC, HASH, and AEAD.
> 
> In this patch, CIPHER, MAC, HASH, AEAD services are introduced.
> 
> VIRTIO-153
> 
> Signed-off-by: Gonglei <arei.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: Mihai Claudiu Caraman <mike.caraman@nxp.com>
> CC: Halil Pasic <pasic@linux.vnet.ibm.com>
> ---
>  acknowledgements.tex |    2 +
>  content.tex          |    2 +
>  virtio-crypto.tex    | 1309 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1313 insertions(+)
>  create mode 100644 virtio-crypto.tex
> 
> diff --git a/acknowledgements.tex b/acknowledgements.tex
> index 53942b0..43b8a9b 100644
> --- a/acknowledgements.tex
> +++ b/acknowledgements.tex
> @@ -26,6 +26,7 @@ Sasha Levin,	Oracle	\newline
>  Sergey Tverdyshev,	Thales e-Security	\newline
>  Stefan Hajnoczi,	Red Hat	\newline
>  Tom Lyon,	Samya Systems, Inc.	\newline
> +Lei Gong,  Huawei   \newline
>  \end{oasistitlesection}
> 
>  The following non-members have provided valuable feedback on this
> @@ -44,4 +45,5 @@ Patrick Durusau,	Technical Advisory Board, OASIS	\newline
>  Thomas Huth,	Red Hat	\newline
>  Yan Vugenfirer, Red Hat / Daynix	\newline
>  Kevin Lo,	MSI	\newline
> +Halil Pasic, IBM  \newline
>  \end{oasistitlesection}
> 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..2708023
> --- /dev/null
> +++ b/virtio-crypto.tex
> @@ -0,0 +1,1309 @@
> +\section{Crypto Device}\label{sec:Device Types / Crypto Device}
> +
> +The virtio crypto device is a virtual cryptography device as well as a kind of

If I google for "cryptography device" there ain't much turning up.
I wonder why? What is the difference between a cryptograpy device
and a cryptographic accelerator?

> +virtual hardware accelerator for virtual machines. The encryption and
> +decryption requests are placed in any of the data queues and are ultimately handled by the
> +backend crypto accelerators. The second kind of queue is the control queue used to create 

Could we leave out "backend" of the specification? What is
the benefit of talking about the backend in this spec?

> +or destroy sessions for symmetric algorithms and will control some advanced
> +features in the future. The virtio crypto device provides the following crypto
> +services: CIPHER, MAC, HASH, and AEAD.
> +

I would prefer:

The virtio crypto device is a virtual cryptography device as well as a
virtual cryptographic accelerator. The virtio crypto device provides the
following crypto services: CIPHER, MAC, HASH, and AEAD. Virtio crypto
devices have a single control queue and at least one data queue. Crypto
operation requests are placed into a data queue, and serviced by the
device. Some crypto operation requests are only valid in the context of a
session. The role of the control queue is facilitating control operation
requests. Sessions management is realized with control operation
requests.

> +
> +\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}.
> +
> +\subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature bits}
> +
> +VIRTIO_CRYPTO_F_STATELESS_MODE (0) stateless mode is available.
> +VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE (1) stateless mode is available for CIPHER service.
> +VIRTIO_CRYPTO_F_HASH_STATELESS_MODE (2) stateless mode is available for HASH service.
> +VIRTIO_CRYPTO_F_MAC_STATELESS_MODE (3) stateless mode is available for MAC service.
> +VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE (4) stateless mode is available for AEAD service.
> +
> +\subsubsection{Feature bit requirements}\label{sec:Device Types / Crypto Device / Feature bits}
> +
> +Some crypto feature bits require other crypto feature bits
> +(see \ref{drivernormative:Basic Facilities of a Virtio Device / Feature Bits}):
> +
> +\begin{description}
> +\item[VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
> +\item[VIRTIO_CRYPTO_F_HASH_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
> +\item[VIRTIO_CRYPTO_F_MAC_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
> +\item[VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
> +\end{description}


I find feature bit 0 redundant and bit confusing. We had a discussion
in v15 and v16.

Could you answer:
https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg03214.html
(Message-ID: <1fbe30cc-87ec-32bc-4c57-85f9b03b3034@linux.vnet.ibm.com>)



> +
> +\subsection{Supported crypto services}\label{sec:Device Types / Crypto Device / Supported crypto services}
> +
> +The virtio crypto device provides the following crypto services: CIPHER, MAC, HASH, and AEAD.

How about 
The following crypto services are defined:

> +
> +\begin{lstlisting}
> +/* CIPHER service */
> +#define VIRTIO_CRYPTO_SERVICE_CIPHER 0
> +/* HASH service */
> +#define VIRTIO_CRYPTO_SERVICE_HASH   1
> +/* MAC (Message Authentication Codes) service */
> +#define VIRTIO_CRYPTO_SERVICE_MAC    2
> +/* AEAD (Authenticated Encryption with Associated Data) service */
> +#define VIRTIO_CRYPTO_SERVICE_AEAD   3
> +\end{lstlisting}
> +
> +The above constants are bit numbers, which tell the driver which crypto services
> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.

The above constants designate bits used to indicate the which of crypto services are
offered by the device as described in . 


> +
> +\subsubsection{CIPHER services}\label{sec:Device Types / Crypto Device / Supported crypto services / CIPHER services}

> +
> +The following CIPHER algorithms are defined:

The naming is a bit inconsistent. In the title you say
services, here you say algorithms.

> +
> +\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 above constants have two usages:
> +\begin{enumerate}
> +\item As bit numbers, used to tell the driver which CIPHER algorithms
> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
> +\item As values, used to tell the device which CIPHER algorithm
> +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
> +\end{enumerate}
> +
> +\subsubsection{HASH services}\label{sec:Device Types / Crypto Device / Supported crypto services / HASH services}
> +
> +The following HASH algorithms are defined:

Same here.

> +
> +\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 above constants have two usages:
> +\begin{enumerate}
> +\item As bit numbers, used to tell the driver which HASH algorithms
> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
> +\item As values, used to tell the device which HASH algorithm
> +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
> +\end{enumerate}
> +
> +\subsubsection{MAC services}\label{sec:Device Types / Crypto Device / Supported crypto services / MAC services}
> +
> +The following MAC algorithms are defined:
> +
> +\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_KASUMI_F9                27
> +#define VIRTIO_CRYPTO_MAC_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 above constants have two usages:
> +\begin{enumerate}
> +\item As bit numbers, used to tell the driver which MAC algorithms
> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
> +\item As values, used to tell the device which MAC algorithm
> +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
> +\end{enumerate}
> +
> +\subsubsection{AEAD services}\label{sec:Device Types / Crypto Device / Supported crypto services / AEAD services}
> +
> +The following AEAD algorithms are defined:
> +
> +\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}
> +
> +The above constants have two usages:
> +\begin{enumerate}
> +\item As bit numbers, used to tell the driver which AEAD algorithms
> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
> +\item As values, used to tell the device what AEAD algorithm
> +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
> +\end{enumerate}
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / Crypto Device / Device configuration layout}
> +
> +\begin{lstlisting}
> +struct virtio_crypto_config {
> +    le32 status;
> +    le32 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 aead_algo;
> +    /* Maximum length of cipher key in bytes */
> +    le32 max_cipher_key_len;
> +    /* Maximum length of authenticated key in bytes */
> +    le32 max_auth_key_len;
> +    le32 reserved;
> +    /* Maximum size of each crypto request's content in bytes */
> +    le64 max_size;
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{status}] is used to show whether the device is ready to work or not, it can be either zero or have one or more flags
> +    Only one read-only bit (for the driver) 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}
> +
> +\item[\field{max_dataqueues}] is the maximum number of data virtqueues exposed by
> +    the device. The driver MAY use only one data queue,
> +    or it can use more to achieve better performance.
> +
> +\item[\field{crypto_services}] is a 32-bit mask which indicates the crypto services supported by
> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services}.

How about shortening these a little
\item[\field{crypto_services}] crypto service offered (see \ref{
sec:Device Types / Crypto Device / Supported crypto services})

\item[\field{cipher_algo_l}]  CIPHER algorithms bits 0-31

and so on

> +
> +\item[\field{cipher_algo_l}] is the low 32-bit mask which indicates the CIPHER algorithms supported by
> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / CIPHER services}.
> +
> +\item[\field{cipher_algo_h}] is the high 32-bit mask which indicates the CIPHER algorithms supported by
> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / CIPHER services}.
> +
> +\item[\field{hash_algo}] is a 32-bit mask which indicates the HASH algorithms supported by
> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / HASH services}.
> +
> +\item[\field{mac_algo_l}] is the low 32-bit mask which indicates the MAC algorithms supported by
> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / MAC services}.
> +
> +\item[\field{mac_algo_h}] is the high 32-bit mask which indicates the MAC algorithms supported by
> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / MAC services}.
> +
> +\item[\field{aead_algo}] is a 32-bit mask which indicates the AEAD algorithms supported by
> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / AEAD services}.
> +
> +\item[\field{max_cipher_key_len}] is the maximum length of cipher key supported by the device.
> +
> +\item[\field{max_auth_key_len}] is the maximum length of authenticated key supported by the device.
> +
> +\item[\field{reserved}] is reserved for future use.
> +
> +\item[\field{max_size}] is the maximum size of each crypto request's content supported by the device
> +\end{description}
> +
> +\begin{note}
> +Unless explicitly stated otherwise all lengths and sizes are in bytes.
> +\end{note}
> +
> +\devicenormative{\subsubsection}{Device configuration layout}{Device Types / Crypto Device / Device configuration layout}
> +
> +\begin{itemize*}
> +\item The device MUST set \field{max_dataqueues} to between 1 and 65535 inclusive.
> +\item The device MUST set \field{status} based on the status of the backend crypto accelerator. 
> +\item The device MUST accept and handle requests after \field{status} is set to VIRTIO_CRYPTO_S_HW_READY.
> +\item The device MUST set \field{crypto_services} based on the crypto services the device offers.
> +\item The device MUST set detailed algorithms masks based on the \field{crypto_services} field.

s/based on the \field{crypto_services} field/ for each service advertised by \field{crypto_services}

> +\item The device MUST set \field{max_size} to show the maximum size of crypto request the device supports.
> +\item The device MUST set \field{max_cipher_key_len} to show the maximum length of cipher key if the device supports CIPHER service.
> +\item The device MUST set \field{max_auth_key_len} to show the maximum length of authenticated key if the device supports MAC service.
> +\end{itemize*}
> +
> +\drivernormative{\subsubsection}{Device configuration layout}{Device Types / Crypto Device / Device configuration layout}
> +
> +\begin{itemize*}
> +\item The driver MUST read the ready \field{status} from the bottom bit of status to check whether the backend crypto accelerator
> +      is ready or not, and the driver MUST reread it after device reset. 
> +\item The driver MUST NOT transmit any requests to the device if the ready \field{status} is not set.
> +\item The driver MUST read \field{max_dataqueues} field to discover the number of 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 the detailed algorithms fields based on \field{crypto_services} field.
> +\item The driver SHOULD read \field{max_size} to discover the maximum size of crypto request the device supports.
> +\item The driver SHOULD read \field{max_cipher_key_len} to discover the maximum length of cipher key the device supports.
> +\item The driver SHOULD read \field{max_auth_key_len} to discover the maximum length of authenticated key the device supports.

Qouting a discussion from v15:
"""
>
>>> +The value of the \field{status} field is VIRTIO_CRYPTO_S_HW_READY or
> ~VIRTIO_CRYPTO_S_HW_READY.
>>
>> Not entirely happy with this. What you want to say is reserved
>> for future use, or? Would it make sense to have a general note
>> -- in a similar fashion like for 'sizes are in bytes' -- for
>> reserved for future use?
>>
>> One possible formulation would be:
>>
>> "In this specification, unless explicitly stated otherwise,
>> fields and bits reserved for future use shall be zeroed out.
>> Both the a device or a driver device and the driver should
>> detect violations of this rule, and deny the requested
>> operation in an appropriate way if possible."
>
> If we go with reserved-and-must-be-zero, we need to make rejecting
> non-zero for reserved value a MUST, or we may run into problems later.
>
> In this case, I'd opt for a specific formulation, though; like
>
> "The \field{status} field can be either zero or have one or more flags
> set. Valid flags are listed below."
>
> And then state that non-valid flags MUST NOT be set resp. MUST be
> rejected in a normative statement.
>
Sounds good.
"""
https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg01596.html

I can't find this. Did we agree on something else in the meanwhile?


> +\end{itemize*}
> +

What about extensibility regarding "detailed algorithms"? Is the driver required ignore algorithms
it does not "know about"? Should we reserve the not (yet) defined bits?

> +\subsection{Device Initialization}\label{sec:Device Types / Crypto Device / Device Initialization}
> +
> +\drivernormative{\subsubsection}{Device Initialization}{Device Types / Crypto Device / Device Initialization}
> +
> +\begin{itemize*}
> +\item The driver MUST identify and initialize all virtqueues.
> +\item The driver MUST read the supported crypto services from bits of \field{crypto_services}. 
> +\item The driver MUST read the supported algorithms based on \field{crypto_services} field.
> +\end{itemize*}
> +
> +\subsection{Device Operation}\label{sec:Device Types / Crypto Device / Device Operation}
> +
> +Requests can be transmitted by placing them in the controlq or dataq.
> +Requests consist of a queue-type specific header specifying among
> +others the operation, and an operation specific payload.
> +The payload is generally composed of operation parameters, output data, and input data.
> +Operation parameters are algorithm-specific parameters, output data is the
> +data that should be utilized in operations, and input data is equal to
> +"operation result + result data".
> +


> +The device can support both session mode (See \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}) and stateless mode.
> +If VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE is negotiated, the driver can use stateless mode for CIPHER service, otherwise it can only use session mode.

How about:
In stateless mode all operation parameters are supplied as a part
of each request, while in session mode, some or all operation parameters
are managed within the session. Stateless mode is guarded by
feature bits 0-4 on a service level. If stateless mode is negotiated
for some service, the service is available both in session and
stateless mode; otherwise it's only available in session mode.

> +
> +The header for controlq is as follows:
> +
> +\begin{lstlisting}
> +#define VIRTIO_CRYPTO_OPCODE(service, op)   (((service) << 8) | (op))
> +
> +struct virtio_crypto_ctrl_header {
> +#define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x02)
> +#define VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x03)
> +#define VIRTIO_CRYPTO_HASH_CREATE_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x02)
> +#define VIRTIO_CRYPTO_HASH_DESTROY_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x03)
> +#define VIRTIO_CRYPTO_MAC_CREATE_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x02)
> +#define VIRTIO_CRYPTO_MAC_DESTROY_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x03)
> +#define VIRTIO_CRYPTO_AEAD_CREATE_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02)
> +#define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03)
> +    le32 opcode;
> +    /* algo should be service-specific algorithms */
> +    le32 algo;
> +    le32 flag;
> +    /* data virtqueue id */
> +    le32 queue_id;
> +};
> +\end{lstlisting}
> +
> +The header for dataq is as follows:
> +
> +\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;
> +    le64 session_id;
> +#define VIRTIO_CRYPTO_FLAG_STATE_MODE 1

This name ain't consistent with the name session mode
used in the text. What's the purpose of this flag
anyway (a single bit should suffice)

> +#define VIRTIO_CRYPTO_FLAG_STATELESS_MODE 2
> +    /* control flag to control the request */
> +    le32 flag;
> +    le32 padding;
> +};
> +\end{lstlisting}

Will continue from here.

[..]

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

[Gonglei (Arei)]

Hi Halil,

> 
> 
> On 04/22/2017 08:23 AM, Gonglei wrote:
> > The virtio crypto device is a virtual crypto device (ie. hardware
> > crypto accelerator card). Currently, the virtio crypto device provides
> > the following crypto services: CIPHER, MAC, HASH, and AEAD.
> >
> > In this patch, CIPHER, MAC, HASH, AEAD services are introduced.
> >
> > VIRTIO-153
> >
> > Signed-off-by: Gonglei <arei.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: Mihai Claudiu Caraman <mike.caraman@nxp.com>
> > CC: Halil Pasic <pasic@linux.vnet.ibm.com>
> > ---
> >  acknowledgements.tex |    2 +
> >  content.tex          |    2 +
> >  virtio-crypto.tex    | 1309
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 1313 insertions(+)
> >  create mode 100644 virtio-crypto.tex
> >
> > diff --git a/acknowledgements.tex b/acknowledgements.tex
> > index 53942b0..43b8a9b 100644
> > --- a/acknowledgements.tex
> > +++ b/acknowledgements.tex
> > @@ -26,6 +26,7 @@ Sasha Levin,	Oracle	\newline
> >  Sergey Tverdyshev,	Thales e-Security	\newline
> >  Stefan Hajnoczi,	Red Hat	\newline
> >  Tom Lyon,	Samya Systems, Inc.	\newline
> > +Lei Gong,  Huawei   \newline
> >  \end{oasistitlesection}
> >
> >  The following non-members have provided valuable feedback on this
> > @@ -44,4 +45,5 @@ Patrick Durusau,	Technical Advisory Board, OASIS
> 	\newline
> >  Thomas Huth,	Red Hat	\newline
> >  Yan Vugenfirer, Red Hat / Daynix	\newline
> >  Kevin Lo,	MSI	\newline
> > +Halil Pasic, IBM  \newline
> >  \end{oasistitlesection}
> > 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..2708023
> > --- /dev/null
> > +++ b/virtio-crypto.tex
> > @@ -0,0 +1,1309 @@
> > +\section{Crypto Device}\label{sec:Device Types / Crypto Device}
> > +
> > +The virtio crypto device is a virtual cryptography device as well as a kind of
> 
> If I google for "cryptography device" there ain't much turning up.
> I wonder why? What is the difference between a cryptograpy device
> and a cryptographic accelerator?
> 
IMHO they have the same meaning.

> > +virtual hardware accelerator for virtual machines. The encryption and
> > +decryption requests are placed in any of the data queues and are ultimately
> handled by the
> > +backend crypto accelerators. The second kind of queue is the control queue
> used to create
> 
> Could we leave out "backend" of the specification? What is
> the benefit of talking about the backend in this spec?
> 
Ok, It's ok to me, let's drop the word.

> > +or destroy sessions for symmetric algorithms and will control some
> advanced
> > +features in the future. The virtio crypto device provides the following crypto
> > +services: CIPHER, MAC, HASH, and AEAD.
> > +
> 
> I would prefer:
> 
> The virtio crypto device is a virtual cryptography device as well as a
> virtual cryptographic accelerator. The virtio crypto device provides the
> following crypto services: CIPHER, MAC, HASH, and AEAD. Virtio crypto
> devices have a single control queue and at least one data queue. Crypto
> operation requests are placed into a data queue, and serviced by the
> device. Some crypto operation requests are only valid in the context of a
> session. The role of the control queue is facilitating control operation
> requests. Sessions management is realized with control operation
> requests.
> 
It's indeed better. :)

> > +
> > +\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}.
> > +
> > +\subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature
> bits}
> > +
> > +VIRTIO_CRYPTO_F_STATELESS_MODE (0) stateless mode is available.
> > +VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE (1) stateless mode is
> available for CIPHER service.
> > +VIRTIO_CRYPTO_F_HASH_STATELESS_MODE (2) stateless mode is available
> for HASH service.
> > +VIRTIO_CRYPTO_F_MAC_STATELESS_MODE (3) stateless mode is available
> for MAC service.
> > +VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE (4) stateless mode is available
> for AEAD service.
> > +
> > +\subsubsection{Feature bit requirements}\label{sec:Device Types / Crypto
> Device / Feature bits}
> > +
> > +Some crypto feature bits require other crypto feature bits
> > +(see \ref{drivernormative:Basic Facilities of a Virtio Device / Feature Bits}):
> > +
> > +\begin{description}
> > +\item[VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE] Requires
> VIRTIO_CRYPTO_F_STATELESS_MODE.
> > +\item[VIRTIO_CRYPTO_F_HASH_STATELESS_MODE] Requires
> VIRTIO_CRYPTO_F_STATELESS_MODE.
> > +\item[VIRTIO_CRYPTO_F_MAC_STATELESS_MODE] Requires
> VIRTIO_CRYPTO_F_STATELESS_MODE.
> > +\item[VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE] Requires
> VIRTIO_CRYPTO_F_STATELESS_MODE.
> > +\end{description}
> 
> 
> I find feature bit 0 redundant and bit confusing. We had a discussion
> in v15 and v16.
> 
> Could you answer:
> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg03214.html
> (Message-ID: <1fbe30cc-87ec-32bc-4c57-85f9b03b3034@linux.vnet.ibm.com>)
> 
> 
Please see my reply:
https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg03186.html

The main reason is we should keep compatibility to pre-existing driver and
support some function that different services have different modes.
We have only one unique crypto request named structure virtio_crypto_op_data_req_mux. 
Please continue to see the sepc, you'll find the truth.

> 
> > +
> > +\subsection{Supported crypto services}\label{sec:Device Types / Crypto
> Device / Supported crypto services}
> > +
> > +The virtio crypto device provides the following crypto services: CIPHER, MAC,
> HASH, and AEAD.
> 
> How about
> The following crypto services are defined:
> 
OK.

> > +
> > +\begin{lstlisting}
> > +/* CIPHER service */
> > +#define VIRTIO_CRYPTO_SERVICE_CIPHER 0
> > +/* HASH service */
> > +#define VIRTIO_CRYPTO_SERVICE_HASH   1
> > +/* MAC (Message Authentication Codes) service */
> > +#define VIRTIO_CRYPTO_SERVICE_MAC    2
> > +/* AEAD (Authenticated Encryption with Associated Data) service */
> > +#define VIRTIO_CRYPTO_SERVICE_AEAD   3
> > +\end{lstlisting}
> > +
> > +The above constants are bit numbers, which tell the driver which crypto
> services
> > +are supported by the device, see \ref{sec:Device Types / Crypto Device /
> Device configuration layout}.
> 
> The above constants designate bits used to indicate the which of crypto
> services are
> offered by the device as described in .
> 
Sorry, is "the which of " right usage?

> 
> > +
> > +\subsubsection{CIPHER services}\label{sec:Device Types / Crypto Device /
> Supported crypto services / CIPHER services}
> 
> > +
> > +The following CIPHER algorithms are defined:
> 
> The naming is a bit inconsistent. In the title you say
> services, here you say algorithms.
> 
Because the CIPHER service uses the specific algorithm to provide services.

> > +
> > +\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 above constants have two usages:
> > +\begin{enumerate}
> > +\item As bit numbers, used to tell the driver which CIPHER algorithms
> > +are supported by the device, see \ref{sec:Device Types / Crypto Device /
> Device configuration layout}.
> > +\item As values, used to tell the device which CIPHER algorithm
> > +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto
> Device / Device Operation / Control Virtqueue / Session operation}.
> > +\end{enumerate}
> > +
> > +\subsubsection{HASH services}\label{sec:Device Types / Crypto Device /
> Supported crypto services / HASH services}
> > +
> > +The following HASH algorithms are defined:
> 
> Same here.
> 
> > +
> > +\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 above constants have two usages:
> > +\begin{enumerate}
> > +\item As bit numbers, used to tell the driver which HASH algorithms
> > +are supported by the device, see \ref{sec:Device Types / Crypto Device /
> Device configuration layout}.
> > +\item As values, used to tell the device which HASH algorithm
> > +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto
> Device / Device Operation / Control Virtqueue / Session operation}.
> > +\end{enumerate}
> > +
> > +\subsubsection{MAC services}\label{sec:Device Types / Crypto Device /
> Supported crypto services / MAC services}
> > +
> > +The following MAC algorithms are defined:
> > +
> > +\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_KASUMI_F9                27
> > +#define VIRTIO_CRYPTO_MAC_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 above constants have two usages:
> > +\begin{enumerate}
> > +\item As bit numbers, used to tell the driver which MAC algorithms
> > +are supported by the device, see \ref{sec:Device Types / Crypto Device /
> Device configuration layout}.
> > +\item As values, used to tell the device which MAC algorithm
> > +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto
> Device / Device Operation / Control Virtqueue / Session operation}.
> > +\end{enumerate}
> > +
> > +\subsubsection{AEAD services}\label{sec:Device Types / Crypto Device /
> Supported crypto services / AEAD services}
> > +
> > +The following AEAD algorithms are defined:
> > +
> > +\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}
> > +
> > +The above constants have two usages:
> > +\begin{enumerate}
> > +\item As bit numbers, used to tell the driver which AEAD algorithms
> > +are supported by the device, see \ref{sec:Device Types / Crypto Device /
> Device configuration layout}.
> > +\item As values, used to tell the device what AEAD algorithm
> > +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto
> Device / Device Operation / Control Virtqueue / Session operation}.
> > +\end{enumerate}
> > +
> > +\subsection{Device configuration layout}\label{sec:Device Types / Crypto
> Device / Device configuration layout}
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_config {
> > +    le32 status;
> > +    le32 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 aead_algo;
> > +    /* Maximum length of cipher key in bytes */
> > +    le32 max_cipher_key_len;
> > +    /* Maximum length of authenticated key in bytes */
> > +    le32 max_auth_key_len;
> > +    le32 reserved;
> > +    /* Maximum size of each crypto request's content in bytes */
> > +    le64 max_size;
> > +};
> > +\end{lstlisting}
> > +
> > +\begin{description}
> > +\item[\field{status}] is used to show whether the device is ready to work or
> not, it can be either zero or have one or more flags
> > +    Only one read-only bit (for the driver) 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}
> > +
> > +\item[\field{max_dataqueues}] is the maximum number of data virtqueues
> exposed by
> > +    the device. The driver MAY use only one data queue,
> > +    or it can use more to achieve better performance.
> > +
> > +\item[\field{crypto_services}] is a 32-bit mask which indicates the crypto
> services supported by
> > +    the device, see \ref{sec:Device Types / Crypto Device / Supported
> crypto services}.
> 
> How about shortening these a little
> \item[\field{crypto_services}] crypto service offered (see \ref{
> sec:Device Types / Crypto Device / Supported crypto services})
> 
> \item[\field{cipher_algo_l}]  CIPHER algorithms bits 0-31
> 
> and so on
> 
OK.

> > +
> > +\item[\field{cipher_algo_l}] is the low 32-bit mask which indicates the
> CIPHER algorithms supported by
> > +    the device, see \ref{sec:Device Types / Crypto Device / Supported
> crypto services  / CIPHER services}.
> > +
> > +\item[\field{cipher_algo_h}] is the high 32-bit mask which indicates the
> CIPHER algorithms supported by
> > +    the device, see \ref{sec:Device Types / Crypto Device / Supported
> crypto services  / CIPHER services}.
> > +
> > +\item[\field{hash_algo}] is a 32-bit mask which indicates the HASH
> algorithms supported by
> > +    the device, see \ref{sec:Device Types / Crypto Device / Supported
> crypto services  / HASH services}.
> > +
> > +\item[\field{mac_algo_l}] is the low 32-bit mask which indicates the MAC
> algorithms supported by
> > +    the device, see \ref{sec:Device Types / Crypto Device / Supported
> crypto services  / MAC services}.
> > +
> > +\item[\field{mac_algo_h}] is the high 32-bit mask which indicates the MAC
> algorithms supported by
> > +    the device, see \ref{sec:Device Types / Crypto Device / Supported
> crypto services  / MAC services}.
> > +
> > +\item[\field{aead_algo}] is a 32-bit mask which indicates the AEAD
> algorithms supported by
> > +    the device, see \ref{sec:Device Types / Crypto Device / Supported
> crypto services  / AEAD services}.
> > +
> > +\item[\field{max_cipher_key_len}] is the maximum length of cipher key
> supported by the device.
> > +
> > +\item[\field{max_auth_key_len}] is the maximum length of authenticated
> key supported by the device.
> > +
> > +\item[\field{reserved}] is reserved for future use.
> > +
> > +\item[\field{max_size}] is the maximum size of each crypto request's
> content supported by the device
> > +\end{description}
> > +
> > +\begin{note}
> > +Unless explicitly stated otherwise all lengths and sizes are in bytes.
> > +\end{note}
> > +
> > +\devicenormative{\subsubsection}{Device configuration layout}{Device Types
> / Crypto Device / Device configuration layout}
> > +
> > +\begin{itemize*}
> > +\item The device MUST set \field{max_dataqueues} to between 1 and 65535
> inclusive.
> > +\item The device MUST set \field{status} based on the status of the backend
> crypto accelerator.
> > +\item The device MUST accept and handle requests after \field{status} is set
> to VIRTIO_CRYPTO_S_HW_READY.
> > +\item The device MUST set \field{crypto_services} based on the crypto
> services the device offers.
> > +\item The device MUST set detailed algorithms masks based on the
> \field{crypto_services} field.
> 
> s/based on the \field{crypto_services} field/ for each service advertised by
> \field{crypto_services}
> 
OK.

> > +\item The device MUST set \field{max_size} to show the maximum size of
> crypto request the device supports.
> > +\item The device MUST set \field{max_cipher_key_len} to show the
> maximum length of cipher key if the device supports CIPHER service.
> > +\item The device MUST set \field{max_auth_key_len} to show the maximum
> length of authenticated key if the device supports MAC service.
> > +\end{itemize*}
> > +
> > +\drivernormative{\subsubsection}{Device configuration layout}{Device Types
> / Crypto Device / Device configuration layout}
> > +
> > +\begin{itemize*}
> > +\item The driver MUST read the ready \field{status} from the bottom bit of
> status to check whether the backend crypto accelerator
> > +      is ready or not, and the driver MUST reread it after device reset.
> > +\item The driver MUST NOT transmit any requests to the device if the ready
> \field{status} is not set.
> > +\item The driver MUST read \field{max_dataqueues} field to discover the
> number of 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 the detailed algorithms fields based on
> \field{crypto_services} field.
> > +\item The driver SHOULD read \field{max_size} to discover the maximum
> size of crypto request the device supports.
> > +\item The driver SHOULD read \field{max_cipher_key_len} to discover the
> maximum length of cipher key the device supports.
> > +\item The driver SHOULD read \field{max_auth_key_len} to discover the
> maximum length of authenticated key the device supports.
> 
> Qouting a discussion from v15:
> """
> >
> >>> +The value of the \field{status} field is VIRTIO_CRYPTO_S_HW_READY or
> > ~VIRTIO_CRYPTO_S_HW_READY.
> >>
> >> Not entirely happy with this. What you want to say is reserved
> >> for future use, or? Would it make sense to have a general note
> >> -- in a similar fashion like for 'sizes are in bytes' -- for
> >> reserved for future use?
> >>
> >> One possible formulation would be:
> >>
> >> "In this specification, unless explicitly stated otherwise,
> >> fields and bits reserved for future use shall be zeroed out.
> >> Both the a device or a driver device and the driver should
> >> detect violations of this rule, and deny the requested
> >> operation in an appropriate way if possible."
> >
> > If we go with reserved-and-must-be-zero, we need to make rejecting
> > non-zero for reserved value a MUST, or we may run into problems later.
> >
> > In this case, I'd opt for a specific formulation, though; like
> >
> > "The \field{status} field can be either zero or have one or more flags
> > set. Valid flags are listed below."
> >
> > And then state that non-valid flags MUST NOT be set resp. MUST be
> > rejected in a normative statement.
> >
> Sounds good.
> """
> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg01596.html
> 
> I can't find this. Did we agree on something else in the meanwhile?
> 
Please see above description:

"""
\begin{description}
\item[\field{status}] is used to show whether the device is ready to work or not, it can be either zero or have one or more flags
    Only one read-only bit (for the driver) 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}
"""

> 
> > +\end{itemize*}
> > +
> 
> What about extensibility regarding "detailed algorithms"? Is the driver
> required ignore algorithms
> it does not "know about"? Should we reserve the not (yet) defined bits?
> 
> > +\subsection{Device Initialization}\label{sec:Device Types / Crypto Device /
> Device Initialization}
> > +
> > +\drivernormative{\subsubsection}{Device Initialization}{Device Types /
> Crypto Device / Device Initialization}
> > +
> > +\begin{itemize*}
> > +\item The driver MUST identify and initialize all virtqueues.
> > +\item The driver MUST read the supported crypto services from bits of
> \field{crypto_services}.
> > +\item The driver MUST read the supported algorithms based on
> \field{crypto_services} field.
> > +\end{itemize*}
> > +
> > +\subsection{Device Operation}\label{sec:Device Types / Crypto Device /
> Device Operation}
> > +
> > +Requests can be transmitted by placing them in the controlq or dataq.
> > +Requests consist of a queue-type specific header specifying among
> > +others the operation, and an operation specific payload.
> > +The payload is generally composed of operation parameters, output data,
> and input data.
> > +Operation parameters are algorithm-specific parameters, output data is the
> > +data that should be utilized in operations, and input data is equal to
> > +"operation result + result data".
> > +
> 
> 
> > +The device can support both session mode (See \ref{sec:Device Types /
> Crypto Device / Device Operation / Control Virtqueue / Session operation}) and
> stateless mode.
> > +If VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE is negotiated, the driver
> can use stateless mode for CIPHER service, otherwise it can only use session
> mode.
> 
> How about:
> In stateless mode all operation parameters are supplied as a part
> of each request, while in session mode, some or all operation parameters
> are managed within the session. Stateless mode is guarded by
> feature bits 0-4 on a service level. If stateless mode is negotiated
> for some service, the service is available both in session and
> stateless mode; otherwise it's only available in session mode.
> 
Good.

> > +
> > +The header for controlq is as follows:
> > +
> > +\begin{lstlisting}
> > +#define VIRTIO_CRYPTO_OPCODE(service, op)   (((service) << 8) | (op))
> > +
> > +struct virtio_crypto_ctrl_header {
> > +#define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION \
> > +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER,
> 0x02)
> > +#define VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION \
> > +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER,
> 0x03)
> > +#define VIRTIO_CRYPTO_HASH_CREATE_SESSION \
> > +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x02)
> > +#define VIRTIO_CRYPTO_HASH_DESTROY_SESSION \
> > +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x03)
> > +#define VIRTIO_CRYPTO_MAC_CREATE_SESSION \
> > +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x02)
> > +#define VIRTIO_CRYPTO_MAC_DESTROY_SESSION \
> > +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x03)
> > +#define VIRTIO_CRYPTO_AEAD_CREATE_SESSION \
> > +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02)
> > +#define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \
> > +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03)
> > +    le32 opcode;
> > +    /* algo should be service-specific algorithms */
> > +    le32 algo;
> > +    le32 flag;
> > +    /* data virtqueue id */
> > +    le32 queue_id;
> > +};
> > +\end{lstlisting}
> > +
> > +The header for dataq is as follows:
> > +
> > +\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;
> > +    le64 session_id;
> > +#define VIRTIO_CRYPTO_FLAG_STATE_MODE 1
> 
> This name ain't consistent with the name session mode
> used in the text. What's the purpose of this flag
> anyway (a single bit should suffice)
> 
Nice catch, we should name it VIRTIO_CRYPTO_FLAG_SESSION_MODE.
This flag is used to identify a the request is session mode or not. It's
true a single bit is enough.

> > +#define VIRTIO_CRYPTO_FLAG_STATELESS_MODE 2
> > +    /* control flag to control the request */
> > +    le32 flag;
> > +    le32 padding;
> > +};
> > +\end{lstlisting}
> 
> Will continue from here.
> 
Thanks!

> [..]

Thanks,
-Gonglei

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

[Halil Pasic]

On 05/04/2017 03:33 PM, Gonglei (Arei) wrote:
>>> +\begin{description}
>>> +\item[VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE] Requires
>> VIRTIO_CRYPTO_F_STATELESS_MODE.
>>> +\item[VIRTIO_CRYPTO_F_HASH_STATELESS_MODE] Requires
>> VIRTIO_CRYPTO_F_STATELESS_MODE.
>>> +\item[VIRTIO_CRYPTO_F_MAC_STATELESS_MODE] Requires
>> VIRTIO_CRYPTO_F_STATELESS_MODE.
>>> +\item[VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE] Requires
>> VIRTIO_CRYPTO_F_STATELESS_MODE.
>>> +\end{description}
>>
>> I find feature bit 0 redundant and bit confusing. We had a discussion
>> in v15 and v16.
>>
>> Could you answer:
>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg03214.html
>> (Message-ID: <1fbe30cc-87ec-32bc-4c57-85f9b03b3034@linux.vnet.ibm.com>)
>>
>>
> Please see my reply:
> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg03186.html
> 
> The main reason is we should keep compatibility to pre-existing driver and
> support some function that different services have different modes.
> We have only one unique crypto request named structure virtio_crypto_op_data_req_mux. 
> Please continue to see the sepc, you'll find the truth.
> 

Sorry, I still do not understand why can't we drop VIRTIO_CRYPTO_F_STATELESS_MODE
and just keep the four service specific modes.

Can you point me to the (published) code where  VIRTIO_CRYPTO_F_STATELESS_MODE is
used (that's what I'm missing -- preferably state the repository, the commit
a file and a line using VIRTIO_CRYPTO_F_STATELESS_MODE)?

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

[Gonglei (Arei)]

> 
> 
> On 05/04/2017 03:33 PM, Gonglei (Arei) wrote:
> >>> +\begin{description}
> >>> +\item[VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE] Requires
> >> VIRTIO_CRYPTO_F_STATELESS_MODE.
> >>> +\item[VIRTIO_CRYPTO_F_HASH_STATELESS_MODE] Requires
> >> VIRTIO_CRYPTO_F_STATELESS_MODE.
> >>> +\item[VIRTIO_CRYPTO_F_MAC_STATELESS_MODE] Requires
> >> VIRTIO_CRYPTO_F_STATELESS_MODE.
> >>> +\item[VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE] Requires
> >> VIRTIO_CRYPTO_F_STATELESS_MODE.
> >>> +\end{description}
> >>
> >> I find feature bit 0 redundant and bit confusing. We had a discussion
> >> in v15 and v16.
> >>
> >> Could you answer:
> >> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg03214.html
> >> (Message-ID:
> <1fbe30cc-87ec-32bc-4c57-85f9b03b3034@linux.vnet.ibm.com>)
> >>
> >>
> > Please see my reply:
> > https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg03186.html
> >
> > The main reason is we should keep compatibility to pre-existing driver and
> > support some function that different services have different modes.
> > We have only one unique crypto request named structure
> virtio_crypto_op_data_req_mux.
> > Please continue to see the sepc, you'll find the truth.
> >
> 
> Sorry, I still do not understand why can't we drop
> VIRTIO_CRYPTO_F_STATELESS_MODE
> and just keep the four service specific modes.
> 
> Can you point me to the (published) code where
> VIRTIO_CRYPTO_F_STATELESS_MODE is
> used (that's what I'm missing -- preferably state the repository, the commit
> a file and a line using VIRTIO_CRYPTO_F_STATELESS_MODE)?
> 
No no no, there isn't existing code use VIRTIO_CRYPTO_F_STATELESS_MODE yet,
It's just for future use.

Please the blow description:
"""
\drivernormative{\paragraph}{HASH Service Operation}{Device Types / Crypto Device / Device Operation / HASH Service Operation}

\begin{itemize*}
\item If the driver uses the session mode, then the driver MUST set \field{session_id} in struct virtio_crypto_op_header
      to a valid value assigned by the device when the session was created.
\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated, the driver MUST use struct virtio_crypto_op_data_req_mux to wrap crypto requests. Otherwise, the driver MUST use struct virtio_crypto_op_data_req.
\item If the VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is negotiated, 1) if the driver uses the stateless mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header
      to VIRTIO_CRYPTO_FLAG_STATELESS_MODE and MUST set the fields in struct virtio_crypto_hash_para_statelession.sess_para, 2) if the driver uses the session mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header to VIRTIO_CRYPTO_FLAG_STATE_MODE.
\item The driver MUST set \field{opcode} in struct virtio_crypto_op_header to VIRTIO_CRYPTO_HASH.
\end{itemize*}
"""

If we drop the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit, and the 
VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is not negotiated, 
then the driver doesn't to know use which structure to store the crypto request: 
is struct virtio_crypto_op_data_req_mux ? or struct virtio_crypto_op_data_req.

We assume the driver uses struct virtio_crypto_op_data_req, what about the device?
The device doesn't know how to parse the request in the virtio_crypto_handle_request()
of virito-crypto.c in Qemu.

static int
virtio_crypto_handle_request(VirtIOCryptoReq *request)
{
    VirtIOCrypto *vcrypto = request->vcrypto;
    VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
    VirtQueueElement *elem = &request->elem;
    int queue_index = virtio_crypto_vq2q(virtio_get_queue_index(request->vq));
    struct virtio_crypto_op_data_req req; ???

    ///or struct virtio_crypto_op_data_req_mux req; ???

Thanks,
-Gonglei

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

[Halil Pasic]

On 05/04/2017 04:13 PM, Gonglei (Arei) wrote:
>>
>>
>> On 05/04/2017 03:33 PM, Gonglei (Arei) wrote:
>>>>> +\begin{description}
>>>>> +\item[VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE] Requires
>>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
>>>>> +\item[VIRTIO_CRYPTO_F_HASH_STATELESS_MODE] Requires
>>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
>>>>> +\item[VIRTIO_CRYPTO_F_MAC_STATELESS_MODE] Requires
>>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
>>>>> +\item[VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE] Requires
>>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
>>>>> +\end{description}
>>>>
>>>> I find feature bit 0 redundant and bit confusing. We had a discussion
>>>> in v15 and v16.
>>>>
>>>> Could you answer:
>>>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg03214.html
>>>> (Message-ID:
>> <1fbe30cc-87ec-32bc-4c57-85f9b03b3034@linux.vnet.ibm.com>)
>>>>
>>>>
>>> Please see my reply:
>>> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg03186.html
>>>
>>> The main reason is we should keep compatibility to pre-existing driver and
>>> support some function that different services have different modes.
>>> We have only one unique crypto request named structure
>> virtio_crypto_op_data_req_mux.
>>> Please continue to see the sepc, you'll find the truth.
>>>
>>
>> Sorry, I still do not understand why can't we drop
>> VIRTIO_CRYPTO_F_STATELESS_MODE
>> and just keep the four service specific modes.
>>
>> Can you point me to the (published) code where
>> VIRTIO_CRYPTO_F_STATELESS_MODE is
>> used (that's what I'm missing -- preferably state the repository, the commit
>> a file and a line using VIRTIO_CRYPTO_F_STATELESS_MODE)?
>>
> No no no, there isn't existing code use VIRTIO_CRYPTO_F_STATELESS_MODE yet,
> It's just for future use.
> 

Thanks, that's a crucial piece of information. In the thread I referenced
this was not cleared (at least for me). It would be really nice to have
some working code before doing the spec, because I find it very easy to miss
a detail which renders the whole thing useless if one works solely on
theoretical level and does not try to create at least a working prototype.

> Please the blow description:
> """
> \drivernormative{\paragraph}{HASH Service Operation}{Device Types / Crypto Device / Device Operation / HASH Service Operation}
> 
> \begin{itemize*}
> \item If the driver uses the session mode, then the driver MUST set \field{session_id} in struct virtio_crypto_op_header
>       to a valid value assigned by the device when the session was created.
> \item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated, the driver MUST use struct virtio_crypto_op_data_req_mux to wrap crypto requests. Otherwise, the driver MUST use struct virtio_crypto_op_data_req.
> \item If the VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is negotiated, 1) if the driver uses the stateless mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header
>       to VIRTIO_CRYPTO_FLAG_STATELESS_MODE and MUST set the fields in struct virtio_crypto_hash_para_statelession.sess_para, 2) if the driver uses the session mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header to VIRTIO_CRYPTO_FLAG_STATE_MODE.
> \item The driver MUST set \field{opcode} in struct virtio_crypto_op_header to VIRTIO_CRYPTO_HASH.
> \end{itemize*}
> """
> 
> If we drop the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit, and the 
> VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is not negotiated, 
> then the driver doesn't to know use which structure to store the crypto request: 
> is struct virtio_crypto_op_data_req_mux ? or struct virtio_crypto_op_data_req.

Thanks for the detailed explanation.

Let's clarify some things first:
1) struct virtio_crypto_op_data_req_mux IS NOT a C language struct but
a somewhat informal desciption using C-like syntax. If you don't follow
try to reason about the size of struct virtio_crypto_op_data_req_mux.
For instance look at:
+struct virtio_crypto_cipher_data_req_stateless {
+    /* Device-readable part */
+    struct virtio_crypto_cipher_para_stateless para;
+    /* The cipher key */
+    u8 cipher_key[keylen];
+
+    /* Initialization Vector or Counter data. */
+    u8 iv[iv_len];
+    /* Source data */
+    u8 src_data[src_data_len];           <==

The  isrc_data_len is not a compile time constant!!

+
+    /* Device-writable part */
+    /* Destination data */
+    u8 dst_data[dst_data_len];
+    struct virtio_crypto_inhdr inhdr;
+};

Using something like BNF to describe the requests would be
less ambiguous but likely more difficult to read and reason about
for most of us (and also inconsistent with the rest of the spec).

2) Each struct virtio_crypto_op_data_req is also a struct virtio_crypto_op_data_req_mux
in a sense of 1).

3) The header struct virtio_crypto_op_header is a common prefix for 
struct virtio_crypto_op_data_req and a struct virtio_crypto_op_data_req_mux.


> 
> We assume the driver uses struct virtio_crypto_op_data_req, what about the device?
> The device doesn't know how to parse the request in the virtio_crypto_handle_request()
> of virito-crypto.c in Qemu.
> 
> static int
> virtio_crypto_handle_request(VirtIOCryptoReq *request)
> {
>     VirtIOCrypto *vcrypto = request->vcrypto;
>     VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
>     VirtQueueElement *elem = &request->elem;
>     int queue_index = virtio_crypto_vq2q(virtio_get_queue_index(request->vq));
>     struct virtio_crypto_op_data_req req; ???
> 
>     ///or struct virtio_crypto_op_data_req_mux req; ???
> 

Because of 3) and because the service associated with the request can be inferred
form virtio_crypto_op_header one can answer the question you as, and decide
if what comes after the header is a stateless or a session variant based
on virtio_crypto_op_header.flag -- maybe it does not even need opcode
to do that.

Thus (IMHO) dropping VIRTIO_CRYPTO_F_STATELESS_MODE is possible.

Or is there a flaw in my reasoning?

Halil

> Thanks,
> -Gonglei
> 

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

[Gonglei (Arei)]

> 
> 
> On 05/04/2017 04:13 PM, Gonglei (Arei) wrote:
> >>
> >>
> >> On 05/04/2017 03:33 PM, Gonglei (Arei) wrote:
> >>>>> +\begin{description}
> >>>>> +\item[VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE] Requires
> >>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
> >>>>> +\item[VIRTIO_CRYPTO_F_HASH_STATELESS_MODE] Requires
> >>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
> >>>>> +\item[VIRTIO_CRYPTO_F_MAC_STATELESS_MODE] Requires
> >>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
> >>>>> +\item[VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE] Requires
> >>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
> >>>>> +\end{description}
> >>>>
> >>>> I find feature bit 0 redundant and bit confusing. We had a discussion
> >>>> in v15 and v16.
> >>>>
> >>>> Could you answer:
> >>>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg03214.html
> >>>> (Message-ID:
> >> <1fbe30cc-87ec-32bc-4c57-85f9b03b3034@linux.vnet.ibm.com>)
> >>>>
> >>>>
> >>> Please see my reply:
> >>> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg03186.html
> >>>
> >>> The main reason is we should keep compatibility to pre-existing driver and
> >>> support some function that different services have different modes.
> >>> We have only one unique crypto request named structure
> >> virtio_crypto_op_data_req_mux.
> >>> Please continue to see the sepc, you'll find the truth.
> >>>
> >>
> >> Sorry, I still do not understand why can't we drop
> >> VIRTIO_CRYPTO_F_STATELESS_MODE
> >> and just keep the four service specific modes.
> >>
> >> Can you point me to the (published) code where
> >> VIRTIO_CRYPTO_F_STATELESS_MODE is
> >> used (that's what I'm missing -- preferably state the repository, the commit
> >> a file and a line using VIRTIO_CRYPTO_F_STATELESS_MODE)?
> >>
> > No no no, there isn't existing code use VIRTIO_CRYPTO_F_STATELESS_MODE
> yet,
> > It's just for future use.
> >
> 
> Thanks, that's a crucial piece of information. In the thread I referenced
> this was not cleared (at least for me). It would be really nice to have
> some working code before doing the spec, because I find it very easy to miss
> a detail which renders the whole thing useless if one works solely on
> theoretical level and does not try to create at least a working prototype.
> 
I see.

> > Please the blow description:
> > """
> > \drivernormative{\paragraph}{HASH Service Operation}{Device Types / Crypto
> Device / Device Operation / HASH Service Operation}
> >
> > \begin{itemize*}
> > \item If the driver uses the session mode, then the driver MUST set
> \field{session_id} in struct virtio_crypto_op_header
> >       to a valid value assigned by the device when the session was created.
> > \item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated,
> the driver MUST use struct virtio_crypto_op_data_req_mux to wrap crypto
> requests. Otherwise, the driver MUST use struct virtio_crypto_op_data_req.
> > \item If the VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is
> negotiated, 1) if the driver uses the stateless mode, then the driver MUST set
> the \field{flag} field in struct virtio_crypto_op_header
> >       to VIRTIO_CRYPTO_FLAG_STATELESS_MODE and MUST set the fields
> in struct virtio_crypto_hash_para_statelession.sess_para, 2) if the driver uses
> the session mode, then the driver MUST set the \field{flag} field in struct
> virtio_crypto_op_header to VIRTIO_CRYPTO_FLAG_STATE_MODE.
> > \item The driver MUST set \field{opcode} in struct virtio_crypto_op_header to
> VIRTIO_CRYPTO_HASH.
> > \end{itemize*}
> > """
> >
> > If we drop the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit, and the
> > VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is not negotiated,
> > then the driver doesn't to know use which structure to store the crypto
> request:
> > is struct virtio_crypto_op_data_req_mux ? or struct
> virtio_crypto_op_data_req.
> 
> Thanks for the detailed explanation.
> 
> Let's clarify some things first:
> 1) struct virtio_crypto_op_data_req_mux IS NOT a C language struct but
> a somewhat informal desciption using C-like syntax. If you don't follow
> try to reason about the size of struct virtio_crypto_op_data_req_mux.
> For instance look at:
> +struct virtio_crypto_cipher_data_req_stateless {
> +    /* Device-readable part */
> +    struct virtio_crypto_cipher_para_stateless para;
> +    /* The cipher key */
> +    u8 cipher_key[keylen];
> +
> +    /* Initialization Vector or Counter data. */
> +    u8 iv[iv_len];
> +    /* Source data */
> +    u8 src_data[src_data_len];           <==
> 
> The  isrc_data_len is not a compile time constant!!
> 
> +
> +    /* Device-writable part */
> +    /* Destination data */
> +    u8 dst_data[dst_data_len];
> +    struct virtio_crypto_inhdr inhdr;
> +};
> 
> Using something like BNF to describe the requests would be
> less ambiguous but likely more difficult to read and reason about
> for most of us (and also inconsistent with the rest of the spec).
> 
I used to use src_data_gpa/dst_data_gpa to express the data,
But you thought it's not clear, so I changed to use u8[len] like
virtio-scsi device in the virtio spec as your suggested.

struct virtio_scsi_req_cmd {
	// Device-readable part
	u8 lun[8];
	le64 id;
	u8 task_attr;
	u8 prio;
	u8 crn;
	u8 cdb[cdb_size];
	// The next two fields are only present if VIRTIO_SCSI_F_T10_PI
	// is negotiated.
	le32 pi_bytesout;
	le32 pi_bytesin;
	u8 pi_out[pi_bytesout];
	u8 dataout[];
	// Device-writable part
	le32 sense_len;
	le32 residual;
	le16 status_qualifier;
	u8 status;
	u8 response;
	u8 sense[sense_size];
	// The next two fields are only present if VIRTIO_SCSI_F_T10_PI
	// is negotiated
	u8 pi_in[pi_bytesin];
	u8 datain[];
};

> 2) Each struct virtio_crypto_op_data_req is also a struct
> virtio_crypto_op_data_req_mux
> in a sense of 1).
> 
> 3) The header struct virtio_crypto_op_header is a common prefix for
> struct virtio_crypto_op_data_req and a struct
> virtio_crypto_op_data_req_mux.
> 
Yes.

> 
> >
> > We assume the driver uses struct virtio_crypto_op_data_req, what about the
> device?
> > The device doesn't know how to parse the request in the
> virtio_crypto_handle_request()
> > of virito-crypto.c in Qemu.
> >
> > static int
> > virtio_crypto_handle_request(VirtIOCryptoReq *request)
> > {
> >     VirtIOCrypto *vcrypto = request->vcrypto;
> >     VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
> >     VirtQueueElement *elem = &request->elem;
> >     int queue_index =
> virtio_crypto_vq2q(virtio_get_queue_index(request->vq));
> >     struct virtio_crypto_op_data_req req; ???
> >
> >     ///or struct virtio_crypto_op_data_req_mux req; ???
> >
> 
> Because of 3) and because the service associated with the request can be
> inferred
> form virtio_crypto_op_header one can answer the question you as, and decide
> if what comes after the header is a stateless or a session variant based
> on virtio_crypto_op_header.flag -- maybe it does not even need opcode
> to do that.
> 
Yes, you are right. We can get the common header:

static int
virtio_crypto_handle_request(VirtIOCryptoReq *request)
{
...
struct virtio_crypto_op_header header;

iov_to_buf(out_iov, out_num, 0, &header, sizeof(header));

...

Then we check the header.flag: 

If (header.flag == VIRTIO_CRYPTO_FLAG_STATELESS_MOD) {
  //Use struct virtio_crypto_op_data_req_mux req;
} else if (header.flag == VIRTIO_CRYPTO_FLAG_SESSION_MOD) {
  //Use struct virtio_crypto_op_data_req_mux req;
} else {
  //Use struct virtio_crypto_op_data_req req;
}

But for existing code we don't use the header.flag to check the request, 
Will it break the pre-existing code? Because we don't assume header.flag is
zero before.


> Thus (IMHO) dropping VIRTIO_CRYPTO_F_STATELESS_MODE is possible.
> 
> Or is there a flaw in my reasoning?
> 

Thanks,
-Gonglei

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

[Halil Pasic]

On 05/05/2017 05:39 AM, Gonglei (Arei) wrote:
>>
>>
>> On 05/04/2017 04:13 PM, Gonglei (Arei) wrote:
>>>>
>>>>
>>>> On 05/04/2017 03:33 PM, Gonglei (Arei) wrote:
>>>>>>> +\begin{description}
>>>>>>> +\item[VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE] Requires
>>>>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
>>>>>>> +\item[VIRTIO_CRYPTO_F_HASH_STATELESS_MODE] Requires
>>>>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
>>>>>>> +\item[VIRTIO_CRYPTO_F_MAC_STATELESS_MODE] Requires
>>>>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
>>>>>>> +\item[VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE] Requires
>>>>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
>>>>>>> +\end{description}
>>>>>>
>>>>>> I find feature bit 0 redundant and bit confusing. We had a discussion
>>>>>> in v15 and v16.
>>>>>>
>>>>>> Could you answer:
>>>>>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg03214.html
>>>>>> (Message-ID:
>>>> <1fbe30cc-87ec-32bc-4c57-85f9b03b3034@linux.vnet.ibm.com>)
>>>>>>
>>>>>>
>>>>> Please see my reply:
>>>>> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg03186.html
>>>>>
>>>>> The main reason is we should keep compatibility to pre-existing driver and
>>>>> support some function that different services have different modes.
>>>>> We have only one unique crypto request named structure
>>>> virtio_crypto_op_data_req_mux.
>>>>> Please continue to see the sepc, you'll find the truth.
>>>>>
>>>>
>>>> Sorry, I still do not understand why can't we drop
>>>> VIRTIO_CRYPTO_F_STATELESS_MODE
>>>> and just keep the four service specific modes.
>>>>
>>>> Can you point me to the (published) code where
>>>> VIRTIO_CRYPTO_F_STATELESS_MODE is
>>>> used (that's what I'm missing -- preferably state the repository, the commit
>>>> a file and a line using VIRTIO_CRYPTO_F_STATELESS_MODE)?
>>>>
>>> No no no, there isn't existing code use VIRTIO_CRYPTO_F_STATELESS_MODE
>> yet,
>>> It's just for future use.
>>>
>>
>> Thanks, that's a crucial piece of information. In the thread I referenced
>> this was not cleared (at least for me). It would be really nice to have
>> some working code before doing the spec, because I find it very easy to miss
>> a detail which renders the whole thing useless if one works solely on
>> theoretical level and does not try to create at least a working prototype.
>>
> I see.
> 
>>> Please the blow description:
>>> """
>>> \drivernormative{\paragraph}{HASH Service Operation}{Device Types / Crypto
>> Device / Device Operation / HASH Service Operation}
>>>
>>> \begin{itemize*}
>>> \item If the driver uses the session mode, then the driver MUST set
>> \field{session_id} in struct virtio_crypto_op_header
>>>       to a valid value assigned by the device when the session was created.
>>> \item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated,
>> the driver MUST use struct virtio_crypto_op_data_req_mux to wrap crypto
>> requests. Otherwise, the driver MUST use struct virtio_crypto_op_data_req.
>>> \item If the VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is
>> negotiated, 1) if the driver uses the stateless mode, then the driver MUST set
>> the \field{flag} field in struct virtio_crypto_op_header
>>>       to VIRTIO_CRYPTO_FLAG_STATELESS_MODE and MUST set the fields
>> in struct virtio_crypto_hash_para_statelession.sess_para, 2) if the driver uses
>> the session mode, then the driver MUST set the \field{flag} field in struct
>> virtio_crypto_op_header to VIRTIO_CRYPTO_FLAG_STATE_MODE.
>>> \item The driver MUST set \field{opcode} in struct virtio_crypto_op_header to
>> VIRTIO_CRYPTO_HASH.
>>> \end{itemize*}
>>> """
>>>
>>> If we drop the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit, and the
>>> VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is not negotiated,
>>> then the driver doesn't to know use which structure to store the crypto
>> request:
>>> is struct virtio_crypto_op_data_req_mux ? or struct
>> virtio_crypto_op_data_req.
>>
>> Thanks for the detailed explanation.
>>
>> Let's clarify some things first:
>> 1) struct virtio_crypto_op_data_req_mux IS NOT a C language struct but
>> a somewhat informal desciption using C-like syntax. If you don't follow
>> try to reason about the size of struct virtio_crypto_op_data_req_mux.
>> For instance look at:
>> +struct virtio_crypto_cipher_data_req_stateless {
>> +    /* Device-readable part */
>> +    struct virtio_crypto_cipher_para_stateless para;
>> +    /* The cipher key */
>> +    u8 cipher_key[keylen];
>> +
>> +    /* Initialization Vector or Counter data. */
>> +    u8 iv[iv_len];
>> +    /* Source data */
>> +    u8 src_data[src_data_len];           <==
>>
>> The  isrc_data_len is not a compile time constant!!
>>
>> +
>> +    /* Device-writable part */
>> +    /* Destination data */
>> +    u8 dst_data[dst_data_len];
>> +    struct virtio_crypto_inhdr inhdr;
>> +};
>>
>> Using something like BNF to describe the requests would be
>> less ambiguous but likely more difficult to read and reason about
>> for most of us (and also inconsistent with the rest of the spec).
>>
> I used to use src_data_gpa/dst_data_gpa to express the data,
> But you thought it's not clear, so I changed to use u8[len] like
> virtio-scsi device in the virtio spec as your suggested.
> 
[..]

That's right, I suggested being consistent there, I'm a bit confused
by this *_gpa stuff though. I guess gpa stands for guest physical
address, so I guess that would mean transmitting only a 'pointer'
to the src_data via the virtq as a part of the request instead of
transmitting a buffer holding the src_data. So IFAIU that would
be a different protocol and not an alternative description of
the same protocol. AFAIK your prototype implementation was
transmitting the data buffer via the virtqueue and not only a pointer
to it. If that's not the case my proposal was utterly wrong!

>> 2) Each struct virtio_crypto_op_data_req is also a struct
>> virtio_crypto_op_data_req_mux
>> in a sense of 1).
>>
>> 3) The header struct virtio_crypto_op_header is a common prefix for
>> struct virtio_crypto_op_data_req and a struct
>> virtio_crypto_op_data_req_mux.
>>
> Yes.
> 
>>
>>>
>>> We assume the driver uses struct virtio_crypto_op_data_req, what about the
>> device?
>>> The device doesn't know how to parse the request in the
>> virtio_crypto_handle_request()
>>> of virito-crypto.c in Qemu.
>>>
>>> static int
>>> virtio_crypto_handle_request(VirtIOCryptoReq *request)
>>> {
>>>     VirtIOCrypto *vcrypto = request->vcrypto;
>>>     VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
>>>     VirtQueueElement *elem = &request->elem;
>>>     int queue_index =
>> virtio_crypto_vq2q(virtio_get_queue_index(request->vq));
>>>     struct virtio_crypto_op_data_req req; ???
>>>
>>>     ///or struct virtio_crypto_op_data_req_mux req; ???
>>>
>>
>> Because of 3) and because the service associated with the request can be
>> inferred
>> form virtio_crypto_op_header one can answer the question you as, and decide
>> if what comes after the header is a stateless or a session variant based
>> on virtio_crypto_op_header.flag -- maybe it does not even need opcode
>> to do that.
>>
> Yes, you are right. We can get the common header:
> 
> static int
> virtio_crypto_handle_request(VirtIOCryptoReq *request)
> {
> ...
> struct virtio_crypto_op_header header;
> 
> iov_to_buf(out_iov, out_num, 0, &header, sizeof(header));
> 
> ...
> 
> Then we check the header.flag: 
> 
> If (header.flag == VIRTIO_CRYPTO_FLAG_STATELESS_MOD) {
>   //Use struct virtio_crypto_op_data_req_mux req;
> } else if (header.flag == VIRTIO_CRYPTO_FLAG_SESSION_MOD) {
>   //Use struct virtio_crypto_op_data_req_mux req;
> } else {
>   //Use struct virtio_crypto_op_data_req req;
> }
> 
> But for existing code we don't use the header.flag to check the request, 
> Will it break the pre-existing code? Because we don't assume header.flag is
> zero before.
> 

That's a very good question, and it relates well to our discussion about
the algorithm bits. You say, the problem is that the published QEMU
implementation does not reject 'unknown' flags (that is requests having
unknown flags).

In pure theory, that ain't really a problem, because we can first look at
the op_code and only check the MOD flags if stateless feature bit was set
for that op_code (service). That way a (IMHO buggy) driver could continue
happily writing garbage into flag for session requests for any service if
it does not negotiate the stateless bit for that service.

Thus I think omitting VIRTIO_CRYPTO_F_HASH_STATELESS_MODE (using 4
feature bits for stateless) is theoretically possible. Do you agree?

But let's approach the problem from the header.flag perspective.

I see two ways for handling the problem you describe.
1) The device MUST ignore header.flag bit's not explicitly activated
by the presence of some condition involving feature bits, and MUST
honor activated ones.
2) We guard the header.flag (and possibly other stuff we forgot to
handle) must not have unknown bits with a feature bit. That is
the device MUST reject unknown bits. But even if we do this we still
need a feature bit based mechanism for adding new known bits. And
since we have a version out there with no flags defined we would
need to guard (that is 'activate' from 1)) at least the flags
introduced together.

Option 2) from above is probably an over-complication, so we should
probably go with 1).

I do not think 1) is correctly described in the spec because we miss MUST
ignore. This MUST ignore is analogous to the MUST ignore in 'The device
MUST ignore the used_event value' from 2.4.7.2 Device Requirements: 
Virtqueue Interrupt Suppression. Maybe replacing this MUST with two SHOULDs
is viable too (the driver SHOULD not send garbage and the device SHOULD
ignore garbage), but I think we need something. Do you agree?

Even if using just 4 feature bits for stateless is possible it does not
mean we that's the best way, but VIRTIO_CRYPTO_F_STATELESS_MODE is a
slightly confusing name given its semantic. If we examine how
VIRTIO_CRYPTO_F_STATELESS_MODE negotiated affects the protocol, we have
to conclude that it changes the request format on the data queues: if
negotiated you have to use _mux if not you must not use _mux. If you use
_mux you MUST set VIRTIO_CRYPTO_F_STATELESS_MODE for session requests, if
you do not use _mux you do not (here you == the driver). So it impacts
session requests too. My problem other problem with the name is that
VIRTIO_CRYPTO_F_STATELESS_MODE does not mean we can use stateless mode,
and that's counter-intuitive for me.

Honestly I have to think about the how formulations impact our ability to
extend the protocol some more myself. So sorry if I'm generating too much
noise.

Regards,
Halil

> 
>> Thus (IMHO) dropping VIRTIO_CRYPTO_F_STATELESS_MODE is possible.
>>
>> Or is there a flaw in my reasoning?
>>
> 
> Thanks,
> -Gonglei
> 
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> 

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

[Gonglei (Arei)]

>
> From: Halil Pasic [mailto:pasic@linux.vnet.ibm.com]
> Sent: Friday, May 05, 2017 9:52 PM
> 
> On 05/05/2017 05:39 AM, Gonglei (Arei) wrote:
> >>
> >>
> >> On 05/04/2017 04:13 PM, Gonglei (Arei) wrote:
> >>>>
> >>>>
> >>>> On 05/04/2017 03:33 PM, Gonglei (Arei) wrote:
> >>>>>>> +\begin{description}
> >>>>>>> +\item[VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE] Requires
> >>>>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
> >>>>>>> +\item[VIRTIO_CRYPTO_F_HASH_STATELESS_MODE] Requires
> >>>>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
> >>>>>>> +\item[VIRTIO_CRYPTO_F_MAC_STATELESS_MODE] Requires
> >>>>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
> >>>>>>> +\item[VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE] Requires
> >>>>>> VIRTIO_CRYPTO_F_STATELESS_MODE.
> >>>>>>> +\end{description}
> >>>>>>
> >>>>>> I find feature bit 0 redundant and bit confusing. We had a discussion
> >>>>>> in v15 and v16.
> >>>>>>
> >>>>>> Could you answer:
> >>>>>>
> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg03214.html
> >>>>>> (Message-ID:
> >>>> <1fbe30cc-87ec-32bc-4c57-85f9b03b3034@linux.vnet.ibm.com>)
> >>>>>>
> >>>>>>
> >>>>> Please see my reply:
> >>>>> https://lists.gnu.org/archive/html/qemu-devel/2017-01/msg03186.html
> >>>>>
> >>>>> The main reason is we should keep compatibility to pre-existing driver
> and
> >>>>> support some function that different services have different modes.
> >>>>> We have only one unique crypto request named structure
> >>>> virtio_crypto_op_data_req_mux.
> >>>>> Please continue to see the sepc, you'll find the truth.
> >>>>>
> >>>>
> >>>> Sorry, I still do not understand why can't we drop
> >>>> VIRTIO_CRYPTO_F_STATELESS_MODE
> >>>> and just keep the four service specific modes.
> >>>>
> >>>> Can you point me to the (published) code where
> >>>> VIRTIO_CRYPTO_F_STATELESS_MODE is
> >>>> used (that's what I'm missing -- preferably state the repository, the
> commit
> >>>> a file and a line using VIRTIO_CRYPTO_F_STATELESS_MODE)?
> >>>>
> >>> No no no, there isn't existing code use
> VIRTIO_CRYPTO_F_STATELESS_MODE
> >> yet,
> >>> It's just for future use.
> >>>
> >>
> >> Thanks, that's a crucial piece of information. In the thread I referenced
> >> this was not cleared (at least for me). It would be really nice to have
> >> some working code before doing the spec, because I find it very easy to miss
> >> a detail which renders the whole thing useless if one works solely on
> >> theoretical level and does not try to create at least a working prototype.
> >>
> > I see.
> >
> >>> Please the blow description:
> >>> """
> >>> \drivernormative{\paragraph}{HASH Service Operation}{Device Types /
> Crypto
> >> Device / Device Operation / HASH Service Operation}
> >>>
> >>> \begin{itemize*}
> >>> \item If the driver uses the session mode, then the driver MUST set
> >> \field{session_id} in struct virtio_crypto_op_header
> >>>       to a valid value assigned by the device when the session was
> created.
> >>> \item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is
> negotiated,
> >> the driver MUST use struct virtio_crypto_op_data_req_mux to wrap crypto
> >> requests. Otherwise, the driver MUST use struct
> virtio_crypto_op_data_req.
> >>> \item If the VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is
> >> negotiated, 1) if the driver uses the stateless mode, then the driver MUST
> set
> >> the \field{flag} field in struct virtio_crypto_op_header
> >>>       to VIRTIO_CRYPTO_FLAG_STATELESS_MODE and MUST set the
> fields
> >> in struct virtio_crypto_hash_para_statelession.sess_para, 2) if the driver
> uses
> >> the session mode, then the driver MUST set the \field{flag} field in struct
> >> virtio_crypto_op_header to VIRTIO_CRYPTO_FLAG_STATE_MODE.
> >>> \item The driver MUST set \field{opcode} in struct virtio_crypto_op_header
> to
> >> VIRTIO_CRYPTO_HASH.
> >>> \end{itemize*}
> >>> """
> >>>
> >>> If we drop the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit, and the
> >>> VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is not negotiated,
> >>> then the driver doesn't to know use which structure to store the crypto
> >> request:
> >>> is struct virtio_crypto_op_data_req_mux ? or struct
> >> virtio_crypto_op_data_req.
> >>
> >> Thanks for the detailed explanation.
> >>
> >> Let's clarify some things first:
> >> 1) struct virtio_crypto_op_data_req_mux IS NOT a C language struct but
> >> a somewhat informal desciption using C-like syntax. If you don't follow
> >> try to reason about the size of struct virtio_crypto_op_data_req_mux.
> >> For instance look at:
> >> +struct virtio_crypto_cipher_data_req_stateless {
> >> +    /* Device-readable part */
> >> +    struct virtio_crypto_cipher_para_stateless para;
> >> +    /* The cipher key */
> >> +    u8 cipher_key[keylen];
> >> +
> >> +    /* Initialization Vector or Counter data. */
> >> +    u8 iv[iv_len];
> >> +    /* Source data */
> >> +    u8 src_data[src_data_len];           <==
> >>
> >> The  isrc_data_len is not a compile time constant!!
> >>
> >> +
> >> +    /* Device-writable part */
> >> +    /* Destination data */
> >> +    u8 dst_data[dst_data_len];
> >> +    struct virtio_crypto_inhdr inhdr;
> >> +};
> >>
> >> Using something like BNF to describe the requests would be
> >> less ambiguous but likely more difficult to read and reason about
> >> for most of us (and also inconsistent with the rest of the spec).
> >>
> > I used to use src_data_gpa/dst_data_gpa to express the data,
> > But you thought it's not clear, so I changed to use u8[len] like
> > virtio-scsi device in the virtio spec as your suggested.
> >
> [..]
> 
> That's right, I suggested being consistent there, I'm a bit confused
> by this *_gpa stuff though. I guess gpa stands for guest physical
> address, so I guess that would mean transmitting only a 'pointer'
> to the src_data via the virtq as a part of the request instead of
> transmitting a buffer holding the src_data. So IFAIU that would
> be a different protocol and not an alternative description of
> the same protocol. AFAIK your prototype implementation was
> transmitting the data buffer via the virtqueue and not only a pointer
> to it. If that's not the case my proposal was utterly wrong!
> 
Yes, they express a data buffer.

> >> 2) Each struct virtio_crypto_op_data_req is also a struct
> >> virtio_crypto_op_data_req_mux
> >> in a sense of 1).
> >>
> >> 3) The header struct virtio_crypto_op_header is a common prefix for
> >> struct virtio_crypto_op_data_req and a struct
> >> virtio_crypto_op_data_req_mux.
> >>
> > Yes.
> >
> >>
> >>>
> >>> We assume the driver uses struct virtio_crypto_op_data_req, what about
> the
> >> device?
> >>> The device doesn't know how to parse the request in the
> >> virtio_crypto_handle_request()
> >>> of virito-crypto.c in Qemu.
> >>>
> >>> static int
> >>> virtio_crypto_handle_request(VirtIOCryptoReq *request)
> >>> {
> >>>     VirtIOCrypto *vcrypto = request->vcrypto;
> >>>     VirtIODevice *vdev = VIRTIO_DEVICE(vcrypto);
> >>>     VirtQueueElement *elem = &request->elem;
> >>>     int queue_index =
> >> virtio_crypto_vq2q(virtio_get_queue_index(request->vq));
> >>>     struct virtio_crypto_op_data_req req; ???
> >>>
> >>>     ///or struct virtio_crypto_op_data_req_mux req; ???
> >>>
> >>
> >> Because of 3) and because the service associated with the request can be
> >> inferred
> >> form virtio_crypto_op_header one can answer the question you as, and
> decide
> >> if what comes after the header is a stateless or a session variant based
> >> on virtio_crypto_op_header.flag -- maybe it does not even need opcode
> >> to do that.
> >>
> > Yes, you are right. We can get the common header:
> >
> > static int
> > virtio_crypto_handle_request(VirtIOCryptoReq *request)
> > {
> > ...
> > struct virtio_crypto_op_header header;
> >
> > iov_to_buf(out_iov, out_num, 0, &header, sizeof(header));
> >
> > ...
> >
> > Then we check the header.flag:
> >
> > If (header.flag == VIRTIO_CRYPTO_FLAG_STATELESS_MOD) {
> >   //Use struct virtio_crypto_op_data_req_mux req;
> > } else if (header.flag == VIRTIO_CRYPTO_FLAG_SESSION_MOD) {
> >   //Use struct virtio_crypto_op_data_req_mux req;
> > } else {
> >   //Use struct virtio_crypto_op_data_req req;
> > }
> >
> > But for existing code we don't use the header.flag to check the request,
> > Will it break the pre-existing code? Because we don't assume header.flag is
> > zero before.
> >
> 
> That's a very good question, and it relates well to our discussion about
> the algorithm bits. You say, the problem is that the published QEMU
> implementation does not reject 'unknown' flags (that is requests having
> unknown flags).
> 
> In pure theory, that ain't really a problem, because we can first look at
> the op_code and only check the MOD flags if stateless feature bit was set
> for that op_code (service). That way a (IMHO buggy) driver could continue
> happily writing garbage into flag for session requests for any service if
> it does not negotiate the stateless bit for that service.
> 
> Thus I think omitting VIRTIO_CRYPTO_F_HASH_STATELESS_MODE (using 4
> feature bits for stateless) is theoretically possible. Do you agree?
> 
Yes, I agree.

> But let's approach the problem from the header.flag perspective.
> 
> I see two ways for handling the problem you describe.
> 1) The device MUST ignore header.flag bit's not explicitly activated
> by the presence of some condition involving feature bits, and MUST
> honor activated ones.

It's true.

> 2) We guard the header.flag (and possibly other stuff we forgot to
> handle) must not have unknown bits with a feature bit. That is
> the device MUST reject unknown bits. But even if we do this we still
> need a feature bit based mechanism for adding new known bits. And
> since we have a version out there with no flags defined we would
> need to guard (that is 'activate' from 1)) at least the flags
> introduced together.
> 
> Option 2) from above is probably an over-complication, so we should
> probably go with 1).
> 
> I do not think 1) is correctly described in the spec because we miss MUST
> ignore. This MUST ignore is analogous to the MUST ignore in 'The device
> MUST ignore the used_event value' from 2.4.7.2 Device Requirements:
> Virtqueue Interrupt Suppression. Maybe replacing this MUST with two
> SHOULDs
> is viable too (the driver SHOULD not send garbage and the device SHOULD
> ignore garbage), but I think we need something. Do you agree?
> 
Yes, I agree.

> Even if using just 4 feature bits for stateless is possible it does not
> mean we that's the best way, but VIRTIO_CRYPTO_F_STATELESS_MODE is a
> slightly confusing name given its semantic. If we examine how
> VIRTIO_CRYPTO_F_STATELESS_MODE negotiated affects the protocol, we have
> to conclude that it changes the request format on the data queues: if
> negotiated you have to use _mux if not you must not use _mux. If you use
> _mux you MUST set VIRTIO_CRYPTO_F_STATELESS_MODE for session requests,
> if you do not use _mux you do not (here you == the driver). So it impacts
> session requests too. 

Sorry? A typo? 
s/ VIRTIO_CRYPTO_F_STATELESS_MODE/ VIRTIO_CRYPTO_FLAG_SESSION_MODE/ ?

> My problem other problem with the name is that
> VIRTIO_CRYPTO_F_STATELESS_MODE does not mean we can use stateless
> mode, and that's counter-intuitive for me.
> 
You're right, I agree with you too. So I think we should change the name
to VIRTIO_CRYPTO_F_MUX_MODE, what's your opinion?

> Honestly I have to think about the how formulations impact our ability to
> extend the protocol some more myself. So sorry if I'm generating too much
> noise.
> 
You comments are very useful, so I should say thank you sincerely. :) 

Thanks,
-Gonglei

> Regards,
> Halil
> 
> >
> >> Thus (IMHO) dropping VIRTIO_CRYPTO_F_STATELESS_MODE is possible.
> >>
> >> Or is there a flaw in my reasoning?
> >>
> >
> > Thanks,
> > -Gonglei
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> >

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

[Halil Pasic]

On 05/04/2017 03:33 PM, Gonglei (Arei) wrote:
>>> +\drivernormative{\subsubsection}{Device configuration layout}{Device Types
>> / Crypto Device / Device configuration layout}
>>> +
>>> +\begin{itemize*}
>>> +\item The driver MUST read the ready \field{status} from the bottom bit of
>> status to check whether the backend crypto accelerator
>>> +      is ready or not, and the driver MUST reread it after device reset.
>>> +\item The driver MUST NOT transmit any requests to the device if the ready
>> \field{status} is not set.
>>> +\item The driver MUST read \field{max_dataqueues} field to discover the
>> number of 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 the detailed algorithms fields based on
>> \field{crypto_services} field.
>>> +\item The driver SHOULD read \field{max_size} to discover the maximum
>> size of crypto request the device supports.
>>> +\item The driver SHOULD read \field{max_cipher_key_len} to discover the
>> maximum length of cipher key the device supports.
>>> +\item The driver SHOULD read \field{max_auth_key_len} to discover the
>> maximum length of authenticated key the device supports.
>>
>> Qouting a discussion from v15:
>> """
>>>>> +The value of the \field{status} field is VIRTIO_CRYPTO_S_HW_READY or
>>> ~VIRTIO_CRYPTO_S_HW_READY.
>>>> Not entirely happy with this. What you want to say is reserved
>>>> for future use, or? Would it make sense to have a general note
>>>> -- in a similar fashion like for 'sizes are in bytes' -- for
>>>> reserved for future use?
>>>>
>>>> One possible formulation would be:
>>>>
>>>> "In this specification, unless explicitly stated otherwise,
>>>> fields and bits reserved for future use shall be zeroed out.
>>>> Both the a device or a driver device and the driver should
>>>> detect violations of this rule, and deny the requested
>>>> operation in an appropriate way if possible."
>>> If we go with reserved-and-must-be-zero, we need to make rejecting
>>> non-zero for reserved value a MUST, or we may run into problems later.
>>>
>>> In this case, I'd opt for a specific formulation, though; like
>>>
>>> "The \field{status} field can be either zero or have one or more flags
>>> set. Valid flags are listed below."
>>>
>>> And then state that non-valid flags MUST NOT be set resp. MUST be
>>> rejected in a normative statement.
>>>
>> Sounds good.
>> """
>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg01596.html
>>
>> I can't find this. Did we agree on something else in the meanwhile?
>>
> Please see above description:
> 
> """
> \begin{description}
> \item[\field{status}] is used to show whether the device is ready to work or not, it can be either zero or have one or more flags
>     Only one read-only bit (for the driver) 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}
> """
> 

I miss 

>>> And then state that non-valid flags MUST NOT be set resp. MUST be
>>> rejected in a normative statement.
>>>
that is 'MUST BE REJECTED in a normative statement' in particular!

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

[Gonglei (Arei)]

Sorry, I missed one comment in the previous reply.

> 
> > +\end{itemize*}
> > +
> 
> What about extensibility regarding "detailed algorithms"? Is the driver
> required ignore algorithms
> it does not "know about"? Should we reserve the not (yet) defined bits?
> 
I mean the device MUST set the algorithms mask bits based on supported
algorithms by the device, and the driver read them to get the capacity.
I don't think we should care about the not defined bits.

Thanks,
-Gonglei

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

[Halil Pasic]

On 05/04/2017 03:53 PM, Gonglei (Arei) wrote:
> Sorry, I missed one comment in the previous reply.
> 
>>
>>> +\end{itemize*}
>>> +
>>
>> What about extensibility regarding "detailed algorithms"? Is the driver
>> required ignore algorithms
>> it does not "know about"? Should we reserve the not (yet) defined bits?
>>
> I mean the device MUST set the algorithms mask bits based on supported
> algorithms by the device, and the driver read them to get the capacity.
> I don't think we should care about the not defined bits.

Let us assume that the driver fails if it encounters an unknown bit
(i.e. bit 13 set in hash_algo). I do not think there is anything in
this document that prohibits the driver doing so -- if there is please
do tell. Now at some point we want to support a new hash algorithm.
If we can't be sure that existing drivers are going to play along with
defining new bits (which are 'not defined bits' using your words for
the existing drivers) we have a small problem.

Was I clear about my concern?

> 
> Thanks,
> -Gonglei
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
> 

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

[Gonglei (Arei)]

> 
> 
> On 05/04/2017 03:53 PM, Gonglei (Arei) wrote:
> > Sorry, I missed one comment in the previous reply.
> >
> >>
> >>> +\end{itemize*}
> >>> +
> >>
> >> What about extensibility regarding "detailed algorithms"? Is the driver
> >> required ignore algorithms
> >> it does not "know about"? Should we reserve the not (yet) defined bits?
> >>
> > I mean the device MUST set the algorithms mask bits based on supported
> > algorithms by the device, and the driver read them to get the capacity.
> > I don't think we should care about the not defined bits.
> 
> Let us assume that the driver fails if it encounters an unknown bit
> (i.e. bit 13 set in hash_algo). I do not think there is anything in
> this document that prohibits the driver doing so -- if there is please
> do tell. Now at some point we want to support a new hash algorithm.
> If we can't be sure that existing drivers are going to play along with
> defining new bits (which are 'not defined bits' using your words for
> the existing drivers) we have a small problem.
> 
> Was I clear about my concern?
> 
Sorry, I'm confused. For the device, it just set the bit mask based on
supported algorithms. Please see cryptodev_builtin_init() 
in cryptodev-builtin.c, the current device only support AES_CBC algorithm,
so we just need set:
 backend->conf.cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC;
 backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1;

Then the driver can only register AES CBC algorithm to the LKCF. Other
algorithms are not supported no matter the driver if register them or not.

Thanks,
-Gonglei

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

[Halil Pasic]

On 05/05/2017 07:56 AM, Gonglei (Arei) wrote:
>>
>>
>> On 05/04/2017 03:53 PM, Gonglei (Arei) wrote:
>>> Sorry, I missed one comment in the previous reply.
>>>
>>>>
>>>>> +\end{itemize*}
>>>>> +
>>>>
>>>> What about extensibility regarding "detailed algorithms"? Is the driver
>>>> required ignore algorithms
>>>> it does not "know about"? Should we reserve the not (yet) defined bits?
>>>>
>>> I mean the device MUST set the algorithms mask bits based on supported
>>> algorithms by the device, and the driver read them to get the capacity.
>>> I don't think we should care about the not defined bits.
>>
>> Let us assume that the driver fails if it encounters an unknown bit
>> (i.e. bit 13 set in hash_algo). I do not think there is anything in
>> this document that prohibits the driver doing so -- if there is please
>> do tell. Now at some point we want to support a new hash algorithm.
>> If we can't be sure that existing drivers are going to play along with
>> defining new bits (which are 'not defined bits' using your words for
>> the existing drivers) we have a small problem.
>>
>> Was I clear about my concern?
>>
> Sorry, I'm confused. For the device, it just set the bit mask based on
> supported algorithms. Please see cryptodev_builtin_init() 
> in cryptodev-builtin.c, the current device only support AES_CBC algorithm,
> so we just need set:
>  backend->conf.cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC;
>  backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1;
> 
> Then the driver can only register AES CBC algorithm to the LKCF. Other
> algorithms are not supported no matter the driver if register them or not.
> 
> Thanks,
> -Gonglei
> 

Now I'm confused too. So let's try to clear things up with two simple
question:

1) Is a device allowed to so set not defined bits ( garbage values or
even worse an implementation specific usage) way (e.g. bit 13 for
hash_algo)?

2) Is a driver allowed to rejects unknown algorithm bits (e.g. give up on
the device because it considers it broken)?

If the answer is 'no' please point me to the appropriate conformance
statements.

Question 1) is about why should we reserve the remaining
bits for future use.

Question 2) is about the interaction with feature bits -- if the answer
is 'yes' then we have to guard new algorithms with feature bits.

Halil

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

[Gonglei (Arei)]

>
> From: Halil Pasic [mailto:pasic@linux.vnet.ibm.com]
> Sent: Friday, May 05, 2017 10:33 PM
> 
> On 05/05/2017 07:56 AM, Gonglei (Arei) wrote:
> >>
> >>
> >> On 05/04/2017 03:53 PM, Gonglei (Arei) wrote:
> >>> Sorry, I missed one comment in the previous reply.
> >>>
> >>>>
> >>>>> +\end{itemize*}
> >>>>> +
> >>>>
> >>>> What about extensibility regarding "detailed algorithms"? Is the driver
> >>>> required ignore algorithms
> >>>> it does not "know about"? Should we reserve the not (yet) defined bits?
> >>>>
> >>> I mean the device MUST set the algorithms mask bits based on supported
> >>> algorithms by the device, and the driver read them to get the capacity.
> >>> I don't think we should care about the not defined bits.
> >>
> >> Let us assume that the driver fails if it encounters an unknown bit
> >> (i.e. bit 13 set in hash_algo). I do not think there is anything in
> >> this document that prohibits the driver doing so -- if there is please
> >> do tell. Now at some point we want to support a new hash algorithm.
> >> If we can't be sure that existing drivers are going to play along with
> >> defining new bits (which are 'not defined bits' using your words for
> >> the existing drivers) we have a small problem.
> >>
> >> Was I clear about my concern?
> >>
> > Sorry, I'm confused. For the device, it just set the bit mask based on
> > supported algorithms. Please see cryptodev_builtin_init()
> > in cryptodev-builtin.c, the current device only support AES_CBC algorithm,
> > so we just need set:
> >  backend->conf.cipher_algo_l = 1u << VIRTIO_CRYPTO_CIPHER_AES_CBC;
> >  backend->conf.hash_algo = 1u << VIRTIO_CRYPTO_HASH_SHA1;
> >
> > Then the driver can only register AES CBC algorithm to the LKCF. Other
> > algorithms are not supported no matter the driver if register them or not.
> >
> > Thanks,
> > -Gonglei
> >
> 
> Now I'm confused too. So let's try to clear things up with two simple
> question:
> 
> 1) Is a device allowed to so set not defined bits ( garbage values or
> even worse an implementation specific usage) way (e.g. bit 13 for
> hash_algo)?
> 
No.

> 2) Is a driver allowed to rejects unknown algorithm bits (e.g. give up on
> the device because it considers it broken)?
> 
No. The driver doesn't need to rejects, it just need care about the known algorithms.

> If the answer is 'no' please point me to the appropriate conformance
> statements.
> 
OK, let's add the conformance statements in the next version.

> Question 1) is about why should we reserve the remaining
> bits for future use.
> 
> Question 2) is about the interaction with feature bits -- if the answer
> is 'yes' then we have to guard new algorithms with feature bits.
> 
Thanks for explanation, I get your point until now :)
 
Thanks,
-Gonglei

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

[Halil Pasic]

On 04/22/2017 08:23 AM, Gonglei wrote:
> The virtio crypto device is a virtual crypto device (ie. hardware
> crypto accelerator card). Currently, the virtio crypto device provides
> the following crypto services: CIPHER, MAC, HASH, and AEAD.
> 
> In this patch, CIPHER, MAC, HASH, AEAD services are introduced.
> 
> VIRTIO-153
> 
> Signed-off-by: Gonglei <arei.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: Mihai Claudiu Caraman <mike.caraman@nxp.com>
> CC: Halil Pasic <pasic@linux.vnet.ibm.com>
> ---
>  acknowledgements.tex |    2 +
>  content.tex          |    2 +
>  virtio-crypto.tex    | 1309 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 1313 insertions(+)
>  create mode 100644 virtio-crypto.tex
> 
> diff --git a/acknowledgements.tex b/acknowledgements.tex
> index 53942b0..43b8a9b 100644
> --- a/acknowledgements.tex
> +++ b/acknowledgements.tex
> @@ -26,6 +26,7 @@ Sasha Levin,	Oracle	\newline
>  Sergey Tverdyshev,	Thales e-Security	\newline
>  Stefan Hajnoczi,	Red Hat	\newline
>  Tom Lyon,	Samya Systems, Inc.	\newline
> +Lei Gong,  Huawei   \newline
>  \end{oasistitlesection}
> 
>  The following non-members have provided valuable feedback on this
> @@ -44,4 +45,5 @@ Patrick Durusau,	Technical Advisory Board, OASIS	\newline
>  Thomas Huth,	Red Hat	\newline
>  Yan Vugenfirer, Red Hat / Daynix	\newline
>  Kevin Lo,	MSI	\newline
> +Halil Pasic, IBM  \newline
>  \end{oasistitlesection}
> 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..2708023
> --- /dev/null
> +++ b/virtio-crypto.tex
> @@ -0,0 +1,1309 @@
> +\section{Crypto Device}\label{sec:Device Types / Crypto Device}
> +
> +The virtio crypto device is a virtual cryptography device as well as a kind of
> +virtual hardware accelerator for virtual machines. The encryption and
> +decryption requests are placed in any of the data queues and are ultimately handled by the
> +backend crypto accelerators. The second kind of queue is the control queue used to create 
> +or destroy sessions for symmetric algorithms and will control some advanced
> +features in the future. The virtio crypto device provides the following crypto
> +services: CIPHER, MAC, HASH, and AEAD.
> +
> +
> +\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}.
> +
> +\subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature bits}
> +
> +VIRTIO_CRYPTO_F_STATELESS_MODE (0) stateless mode is available.
> +VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE (1) stateless mode is available for CIPHER service.
> +VIRTIO_CRYPTO_F_HASH_STATELESS_MODE (2) stateless mode is available for HASH service.
> +VIRTIO_CRYPTO_F_MAC_STATELESS_MODE (3) stateless mode is available for MAC service.
> +VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE (4) stateless mode is available for AEAD service.
> +
> +\subsubsection{Feature bit requirements}\label{sec:Device Types / Crypto Device / Feature bits}
> +
> +Some crypto feature bits require other crypto feature bits
> +(see \ref{drivernormative:Basic Facilities of a Virtio Device / Feature Bits}):
> +
> +\begin{description}
> +\item[VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
> +\item[VIRTIO_CRYPTO_F_HASH_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
> +\item[VIRTIO_CRYPTO_F_MAC_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
> +\item[VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
> +\end{description}
> +
> +\subsection{Supported crypto services}\label{sec:Device Types / Crypto Device / Supported crypto services}
> +
> +The virtio crypto device provides the following crypto services: CIPHER, MAC, HASH, and AEAD.
> +
> +\begin{lstlisting}
> +/* CIPHER service */
> +#define VIRTIO_CRYPTO_SERVICE_CIPHER 0
> +/* HASH service */
> +#define VIRTIO_CRYPTO_SERVICE_HASH   1
> +/* MAC (Message Authentication Codes) service */
> +#define VIRTIO_CRYPTO_SERVICE_MAC    2
> +/* AEAD (Authenticated Encryption with Associated Data) service */
> +#define VIRTIO_CRYPTO_SERVICE_AEAD   3
> +\end{lstlisting}
> +
> +The above constants are bit numbers, which tell the driver which crypto services
> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
> +
> +\subsubsection{CIPHER services}\label{sec:Device Types / Crypto Device / Supported crypto services / CIPHER services}
> +
> +The following CIPHER algorithms are defined:
> +
> +\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 above constants have two usages:
> +\begin{enumerate}
> +\item As bit numbers, used to tell the driver which CIPHER algorithms
> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
> +\item As values, used to tell the device which CIPHER algorithm
> +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
> +\end{enumerate}
> +
> +\subsubsection{HASH services}\label{sec:Device Types / Crypto Device / Supported crypto services / HASH services}
> +
> +The following HASH algorithms are defined:
> +
> +\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 above constants have two usages:
> +\begin{enumerate}
> +\item As bit numbers, used to tell the driver which HASH algorithms
> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
> +\item As values, used to tell the device which HASH algorithm
> +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
> +\end{enumerate}
> +
> +\subsubsection{MAC services}\label{sec:Device Types / Crypto Device / Supported crypto services / MAC services}
> +
> +The following MAC algorithms are defined:
> +
> +\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_KASUMI_F9                27
> +#define VIRTIO_CRYPTO_MAC_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 above constants have two usages:
> +\begin{enumerate}
> +\item As bit numbers, used to tell the driver which MAC algorithms
> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
> +\item As values, used to tell the device which MAC algorithm
> +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
> +\end{enumerate}
> +
> +\subsubsection{AEAD services}\label{sec:Device Types / Crypto Device / Supported crypto services / AEAD services}
> +
> +The following AEAD algorithms are defined:
> +
> +\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}
> +
> +The above constants have two usages:
> +\begin{enumerate}
> +\item As bit numbers, used to tell the driver which AEAD algorithms
> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
> +\item As values, used to tell the device what AEAD algorithm
> +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
> +\end{enumerate}
> +
> +\subsection{Device configuration layout}\label{sec:Device Types / Crypto Device / Device configuration layout}
> +
> +\begin{lstlisting}
> +struct virtio_crypto_config {
> +    le32 status;
> +    le32 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 aead_algo;
> +    /* Maximum length of cipher key in bytes */
> +    le32 max_cipher_key_len;
> +    /* Maximum length of authenticated key in bytes */
> +    le32 max_auth_key_len;
> +    le32 reserved;
> +    /* Maximum size of each crypto request's content in bytes */
> +    le64 max_size;
> +};
> +\end{lstlisting}
> +
> +\begin{description}
> +\item[\field{status}] is used to show whether the device is ready to work or not, it can be either zero or have one or more flags
> +    Only one read-only bit (for the driver) 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}
> +
> +\item[\field{max_dataqueues}] is the maximum number of data virtqueues exposed by
> +    the device. The driver MAY use only one data queue,
> +    or it can use more to achieve better performance.
> +
> +\item[\field{crypto_services}] is a 32-bit mask which indicates the crypto services supported by
> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services}.
> +
> +\item[\field{cipher_algo_l}] is the low 32-bit mask which indicates the CIPHER algorithms supported by
> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / CIPHER services}.
> +
> +\item[\field{cipher_algo_h}] is the high 32-bit mask which indicates the CIPHER algorithms supported by
> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / CIPHER services}.
> +
> +\item[\field{hash_algo}] is a 32-bit mask which indicates the HASH algorithms supported by
> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / HASH services}.
> +
> +\item[\field{mac_algo_l}] is the low 32-bit mask which indicates the MAC algorithms supported by
> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / MAC services}.
> +
> +\item[\field{mac_algo_h}] is the high 32-bit mask which indicates the MAC algorithms supported by
> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / MAC services}.
> +
> +\item[\field{aead_algo}] is a 32-bit mask which indicates the AEAD algorithms supported by
> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / AEAD services}.
> +
> +\item[\field{max_cipher_key_len}] is the maximum length of cipher key supported by the device.
> +
> +\item[\field{max_auth_key_len}] is the maximum length of authenticated key supported by the device.
> +
> +\item[\field{reserved}] is reserved for future use.
> +
> +\item[\field{max_size}] is the maximum size of each crypto request's content supported by the device
> +\end{description}
> +
> +\begin{note}
> +Unless explicitly stated otherwise all lengths and sizes are in bytes.
> +\end{note}
> +
> +\devicenormative{\subsubsection}{Device configuration layout}{Device Types / Crypto Device / Device configuration layout}
> +
> +\begin{itemize*}
> +\item The device MUST set \field{max_dataqueues} to between 1 and 65535 inclusive.
> +\item The device MUST set \field{status} based on the status of the backend crypto accelerator. 
> +\item The device MUST accept and handle requests after \field{status} is set to VIRTIO_CRYPTO_S_HW_READY.
> +\item The device MUST set \field{crypto_services} based on the crypto services the device offers.
> +\item The device MUST set detailed algorithms masks based on the \field{crypto_services} field.
> +\item The device MUST set \field{max_size} to show the maximum size of crypto request the device supports.
> +\item The device MUST set \field{max_cipher_key_len} to show the maximum length of cipher key if the device supports CIPHER service.
> +\item The device MUST set \field{max_auth_key_len} to show the maximum length of authenticated key if the device supports MAC service.
> +\end{itemize*}
> +
> +\drivernormative{\subsubsection}{Device configuration layout}{Device Types / Crypto Device / Device configuration layout}
> +
> +\begin{itemize*}
> +\item The driver MUST read the ready \field{status} from the bottom bit of status to check whether the backend crypto accelerator
> +      is ready or not, and the driver MUST reread it after device reset. 
> +\item The driver MUST NOT transmit any requests to the device if the ready \field{status} is not set.
> +\item The driver MUST read \field{max_dataqueues} field to discover the number of 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 the detailed algorithms fields based on \field{crypto_services} field.
> +\item The driver SHOULD read \field{max_size} to discover the maximum size of crypto request the device supports.
> +\item The driver SHOULD read \field{max_cipher_key_len} to discover the maximum length of cipher key the device supports.
> +\item The driver SHOULD read \field{max_auth_key_len} to discover the maximum length of authenticated key the device supports.
> +\end{itemize*}
> +
> +\subsection{Device Initialization}\label{sec:Device Types / Crypto Device / Device Initialization}
> +
> +\drivernormative{\subsubsection}{Device Initialization}{Device Types / Crypto Device / Device Initialization}
> +
> +\begin{itemize*}
> +\item The driver MUST identify and initialize all virtqueues.
> +\item The driver MUST read the supported crypto services from bits of \field{crypto_services}. 
> +\item The driver MUST read the supported algorithms based on \field{crypto_services} field.
> +\end{itemize*}
> +
> +\subsection{Device Operation}\label{sec:Device Types / Crypto Device / Device Operation}
> +
> +Requests can be transmitted by placing them in the controlq or dataq.
> +Requests consist of a queue-type specific header specifying among
> +others the operation, and an operation specific payload.
> +The payload is generally composed of operation parameters, output data, and input data.
> +Operation parameters are algorithm-specific parameters, output data is the
> +data that should be utilized in operations, and input data is equal to
> +"operation result + result data".
> +
> +The device can support both session mode (See \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}) and stateless mode.
> +If VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE is negotiated, the driver can use stateless mode for CIPHER service, otherwise it can only use session mode.
> +
> +The header for controlq is as follows:
> +
> +\begin{lstlisting}
> +#define VIRTIO_CRYPTO_OPCODE(service, op)   (((service) << 8) | (op))
> +
> +struct virtio_crypto_ctrl_header {
> +#define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x02)
> +#define VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x03)
> +#define VIRTIO_CRYPTO_HASH_CREATE_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x02)
> +#define VIRTIO_CRYPTO_HASH_DESTROY_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x03)
> +#define VIRTIO_CRYPTO_MAC_CREATE_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x02)
> +#define VIRTIO_CRYPTO_MAC_DESTROY_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x03)
> +#define VIRTIO_CRYPTO_AEAD_CREATE_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02)
> +#define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \
> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03)
> +    le32 opcode;
> +    /* algo should be service-specific algorithms */
> +    le32 algo;
> +    le32 flag;
> +    /* data virtqueue id */
> +    le32 queue_id;
> +};
> +\end{lstlisting}
> +
> +The header for dataq is as follows:
> +
> +\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;
> +    le64 session_id;
> +#define VIRTIO_CRYPTO_FLAG_STATE_MODE 1
> +#define VIRTIO_CRYPTO_FLAG_STATELESS_MODE 2
> +    /* control flag to control the request */
> +    le32 flag;
> +    le32 padding;
> +};
> +\end{lstlisting}
> +

START HERE

> +The device can set the operation status as follows: VIRTIO_CRYPTO_OK: success;
> +VIRTIO_CRYPTO_ERR: failure or device error; VIRTIO_CRYPTO_NOTSUPP: not supported;
> +VIRTIO_CRYPTO_INVSESS: invalid session ID when executing crypto operations.

You describe everything but BADMSG. Could you be a bit more specific
about the differences between _ERR _BADMSG and _NOTSUPP? Is for instance trying
to do something with a not-advertised service or algo a _NOTSUPP or a
_BADMSG or just a generic _ERR? Same qestion goes for different sorts of
out of resources. 

> +
> +\begin{lstlisting}
> +enum VIRTIO_CRYPTO_STATUS {
> +    VIRTIO_CRYPTO_OK = 0,
> +    VIRTIO_CRYPTO_ERR = 1,
> +    VIRTIO_CRYPTO_BADMSG = 2,
> +    VIRTIO_CRYPTO_NOTSUPP = 3,
> +    VIRTIO_CRYPTO_INVSESS = 4,
> +    VIRTIO_CRYPTO_MAX
> +};
> +\end{lstlisting}
> +

There are no more mentions of the values in the spec, so the description
above should be real good.

> +\subsubsection{Control Virtqueue}\label{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue}
> +
> +The driver uses the control virtqueue to send control commands to the
> +device, such as session operations (See \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}).
> +
> +Controlq requests are as follows:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_op_ctrl_req {
> +    struct virtio_crypto_ctrl_header header;
> +
> +    union {
> +        struct virtio_crypto_sym_create_session_req   sym_create_session;
> +        struct virtio_crypto_hash_create_session_req  hash_create_session;
> +        struct virtio_crypto_mac_create_session_req   mac_create_session;
> +        struct virtio_crypto_aead_create_session_req  aead_create_session;
> +        struct virtio_crypto_destroy_session_req      destroy_session;
> +    } u;
> +};
> +\end{lstlisting}
> +
> +struct virtio_crypto_op_ctrl_req is the only allowed control request.
> +The header is the general header, and the union is of the algorithm-specific type,
> +which is set by the driver. All the properties in the union are shown as follows.
> +
> +\paragraph{Session operation}\label{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}
> +
> +The symmetric algorithms involve the concept of sessions. A session is a
> +handle which describes the cryptographic parameters to be applied to
> +a number of buffers. 

8<
> The data within a session handle includes:
> +
> +\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 set data, including the CIPHER algorithm and mode,
> +      the key and its length, and the direction (encryption or decryption).
> +\item The HASH/MAC set data, including the HASH algorithm or MAC algorithm,
> +      and hash 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}
> +
>8

This part is slightly confusing for me. I guess you are trying to describe
what data can live in the session context (considering all the different
applications). I think we do not have to describe that here, because we have
to describe the individual session operations, and there we must describe
the precise impact of these operations (and their parameters).

> +The following structure stores the result of session creation set by the device:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_session_input {
> +    /* Device-writable part */
> +    le64 session_id;
> +    le32 status;
> +    le32 padding;
> +};
> +\end{lstlisting}
> +
> +A request to destroy a session includes the following information:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_destroy_session_req {
> +    /* Device-readable part */
> +    le64  session_id;
> +    /* Device-writable part */
> +    le32  status;
> +    le32  padding;
> +};
> +\end{lstlisting}
> +
> +\subparagraph{Session operation: HASH session}\label{sec:Device Types / Crypto Device / Device
> +Operation / Control Virtqueue / Session operation / Session operation: HASH session}
> +
> +HASH session requests are as follows:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_hash_session_para {
> +    /* See VIRTIO_CRYPTO_HASH_* above */
> +    le32 algo;
> +    /* hash result length */
> +    le32 hash_result_len;
> +};
> +struct virtio_crypto_hash_create_session_req {
> +    /* Device-readable part */
> +    struct virtio_crypto_hash_session_para para;
> +    /* Device-writable part */
> +    struct virtio_crypto_session_input input;
> +};
> +\end{lstlisting}
> +
> +\subparagraph{Session operation: MAC session}\label{sec:Device Types / Crypto Device / Device
> +Operation / Control Virtqueue / Session operation / Session operation: MAC session}
> +
> +MAC session requests are as follows:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_mac_session_para {
> +    /* See VIRTIO_CRYPTO_MAC_* above */
> +    le32 algo;
> +    /* hash result length */
> +    le32 hash_result_len;
> +    /* length of authenticated key */
> +    le32 auth_key_len;
> +    le32 padding;
> +};
> +
> +struct virtio_crypto_mac_create_session_req {
> +    /* Device-readable part */
> +    struct virtio_crypto_mac_session_para para;
> +    /* The authenticated key */
> +    u8 auth_key[auth_key_len];
> +
> +    /* Device-writable part */
> +    struct virtio_crypto_session_input input;
> +};
> +\end{lstlisting}
> +
> +\subparagraph{Session operation: Symmetric algorithms session}\label{sec:Device Types / Crypto Device / Device
> +Operation / Control Virtqueue / Session operation / Session operation: Symmetric algorithms session}
> +
> +The request of symmetric session includes two parts, CIPHER algorithms and chain
> +algorithms (chaining CIPHER and HASH/MAC). 
> +
> +CIPHER session requests are as follows:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_cipher_session_para {
> +    /* See VIRTIO_CRYPTO_CIPHER* above */
> +    le32 algo;
> +    /* length of key */
> +    le32 keylen;
> +#define VIRTIO_CRYPTO_OP_ENCRYPT  1
> +#define VIRTIO_CRYPTO_OP_DECRYPT  2
> +    /* encryption or decryption */
> +    le32 op;
> +    le32 padding;
> +};
> +
> +struct virtio_crypto_cipher_session_req {
> +    /* Device-readable part */
> +    struct virtio_crypto_cipher_session_para para;
> +    /* The cipher key */
> +    u8 cipher_key[keylen];
> +
> +    /* Device-writable part */
> +    struct virtio_crypto_session_input input;
> +};
> +\end{lstlisting}
> +
> +Algorithm chaining requests are as follows:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_alg_chain_session_para {
> +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER  1
> +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH  2
> +    le32 alg_chain_order;
> +/* Plain hash */
> +#define VIRTIO_CRYPTO_SYM_HASH_MODE_PLAIN    1
> +/* Authenticated hash (mac) */
> +#define VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH     2
> +/* Nested hash */
> +#define VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED   3
> +    le32 hash_mode;
> +    struct virtio_crypto_cipher_session_para cipher_param;
> +    union {
> +        struct virtio_crypto_hash_session_para hash_param;
> +        struct virtio_crypto_mac_session_para mac_param;
> +    } u;
> +    /* length of the additional authenticated data (AAD) in bytes */
> +    le32 aad_len;
> +    le32 padding;
> +};
> +
> +struct virtio_crypto_alg_chain_session_req {
> +    /* Device-readable part */
> +    struct virtio_crypto_alg_chain_session_para para;
> +    /* The cipher key */
> +    u8 cipher_key[keylen];
> +    /* The authenticated key */
> +    u8 auth_key[auth_key_len];
> +
> +    /* Device-writable part */
> +    struct virtio_crypto_session_input input;
> +};
> +\end{lstlisting}
> +
> +Symmetric algorithm requests are as follows:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_sym_create_session_req {
> +    union {
> +        struct virtio_crypto_cipher_session_req cipher;
> +        struct virtio_crypto_alg_chain_session_req chain;
> +    } u;
> +
> +    /* Device-readable part */
> +
> +/* No operation */
> +#define VIRTIO_CRYPTO_SYM_OP_NONE  0
> +/* Cipher only operation on the data */
> +#define VIRTIO_CRYPTO_SYM_OP_CIPHER  1
> +/* Chain any cipher with any hash or mac operation. The order
> +   depends on the value of alg_chain_order param */
> +#define VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING  2
> +    le32 op_type;
> +    le32 padding;
> +};
> +\end{lstlisting}
> +
> +The driver can set the \field{op_type} field in struct virtio_crypto_sym_create_session_req as follows: VIRTIO_CRYPTO_SYM_OP_NONE: no operation;
> +VIRTIO_CRYPTO_SYM_OP_CIPHER: Cipher only operation on the data; VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING: Chain any cipher with any hash or mac operation.
> +
> +\subparagraph{Session operation: AEAD session}\label{sec:Device Types / Crypto Device / Device
> +Operation / Control Virtqueue / Session operation / Session operation: AEAD session}
> +
> +AEAD session requests are as follows:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_aead_session_para {
> +    /* See VIRTIO_CRYPTO_AEAD_* above */
> +    le32 algo;
> +    /* length of key */
> +    le32 key_len;
> +    /* Authentication tag length */
> +    le32 tag_len;
> +    /* The length of the additional authenticated data (AAD) in bytes */
> +    le32 aad_len;
> +    /* encryption or decryption, See above VIRTIO_CRYPTO_OP_* */
> +    le32 op;
> +    le32 padding;
> +};
> +
> +struct virtio_crypto_aead_create_session_req {
> +    /* Device-readable part */
> +    struct virtio_crypto_aead_session_para para;
> +    u8 key[key_len];
> +
> +    /* Device-writeable part */
> +    struct virtio_crypto_session_input input;
> +};
> +\end{lstlisting}
> +
> +\drivernormative{\subparagraph}{Session operation: create session}{Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation / Session operation: create session}
> +
> +\begin{itemize*}
> +\item The driver MUST set the control general header and corresponding properties of the union in structure virtio_crypto_op_ctrl_req. See \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue}.
> +\item The driver MUST set the \field{opcode} field based on service type: CIPHER, HASH, MAC, or AEAD.

> +\item The driver MUST set the \field{queue_id} field to show used dataq.

Used for what? Is the driver obligued to use that dataq for the op reqs associated
with the given session. If yes where is the normative statement?

> +\end{itemize*}
> +
> +\devicenormative{\subparagraph}{Session operation: create session}{Device Types / Crypto Device / Device
> +Operation / Control Virtqueue / Session operation / Session operation: create session}
> +
> +\begin{itemize*}
> +\item The device MUST set the \field{session_id} field to a unique session identifier when the device finishes processing session creation.

I guess only if successfull (that is status == VIRTIO_CRYPTO_OK).

> +\item The device MUST set the \field{status} field to one of the values of enum VIRTIO_CRYPTO_STATUS.

Maybe put this one first. The formulation could be more fortunate in
a sense that it's required to set the right status (_OK if successs,
_NOTSUPP if ...).

What shall happen if the operation fails, e.g. becasue of out of resources?

Is there some sort of limit for the amount of live sessions (related to
the previous question)? Obviously the device needs storage for the
session data. Can the guest DOS attack the host by creating an absurd number
of sessions?


> +\end{itemize*}
> +
> +\drivernormative{\subparagraph}{Session operation: destroy session}{Device Types / Crypto Device / Device
> +Operation / Control Virtqueue / Session operation / Session operation: destroy session}
> +
> +\begin{itemize*}
> +\item The driver MUST set the \field{opcode} field based on service type: CIPHER, HASH, MAC, or AEAD.
> +\item The driver MUST set the \field{session_id} to a valid value assigned by the device when the session was created.
> +\end{itemize*}
> +
> +\devicenormative{\subparagraph}{Session operation: destroy session}{Device Types / Crypto Device / Device
> +Operation / Control Virtqueue / Session operation / Session operation: destroy session}
> +
> +\begin{itemize*}
> +\item The device MUST set the \field{status} field to one of the values of enum VIRTIO_CRYPTO_STATUS.

Same as above.

> +\end{itemize*}
> +
> +\subsubsection{Data Virtqueue}\label{sec:Device Types / Crypto Device / Device Operation / Data Virtqueue}
> +
> +The driver uses the data virtqueue to transmit crypto operation requests to the device,
> +and completes the crypto operations.
> +
> +Session mode dataq requests are as follows:
> +
> +\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}
> +
> +Dataq requests for both session and stateless modes are as follows:

This ain't very nice, I mean the 'both session and stateless modes' together
with the above 'session mode dataq requests are'. In the meanwhile I know what
you mean: if the SESSION_MODE feature bit was negotiated.

> +
> +\begin{lstlisting}
> +struct virtio_crypto_op_data_req_mux {
> +    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;
> +        struct virtio_crypto_sym_data_req_stateless   sym_stateless_req;
> +        struct virtio_crypto_hash_data_req_stateless  hash_stateless_req;
> +        struct virtio_crypto_mac_data_req_stateless   mac_stateless_req;
> +        struct virtio_crypto_aead_data_req_stateless  aead_stateless_req;
> +    } u;
> +};
> +\end{lstlisting}
> +
> +The header is the general header and the union is of the algorithm-specific type,
> +which is set by the driver. All properties in the union are shown as follows.
> +
> +There is a unified input header structure for all crypto services.
> +
> +The structure is defined as follows:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_inhdr {
> +    u8 status;
> +};
> +\end{lstlisting}
> +
> +\subsubsection{HASH Service Operation}\label{sec:Device Types / Crypto Device / Device Operation / HASH Service Operation}
> +
> +Session mode HASH service requests are as follows:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_hash_para {
> +    /* length of source data */
> +    le32 src_data_len;
> +    /* hash result length */
> +    le32 hash_result_len;
> +};
> +
> +struct virtio_crypto_hash_data_req {
> +    /* Device-readable part */
> +    struct virtio_crypto_hash_para para;
> +    /* Source data */
> +    u8 src_data[src_data_len];
> +
> +    /* Device-writable part */
> +    /* Hash result data */
> +    u8 hash_result[hash_result_len];
> +    struct virtio_crypto_inhdr inhdr;
> +};
> +\end{lstlisting}
> +
> +Each data request uses virtio_crypto_hash_data_req structure to store information
> +used to run the HASH operations. 
> +
> +The information includes the hash parameters stored in \field{para}, output data and input data.
> +The output data here includes the source data and the input data includes the hash result data used to save the results of the HASH operations.
> +\field{inhdr} stores the status of executing the HASH operations.
> +
> +Stateless mode HASH service requests are as follows:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_hash_para_statelesss {
> +    struct {
> +        /* See VIRTIO_CRYPTO_HASH_* above */
> +        le32 algo;
> +    } sess_para;
> +
> +    /* length of source data */
> +    le32 src_data_len;
> +    /* hash result length */
> +    le32 hash_result_len;
> +    le32 reserved;
> +};
> +struct virtio_crypto_hash_data_req_stateless {
> +    /* Device-readable part */
> +    struct virtio_crypto_hash_para_stateless para;
> +    /* Source data */
> +    u8 src_data[src_data_len];
> +
> +    /* Device-writable part */
> +    /* Hash result data */
> +    u8 hash_result[hash_result_len];
> +    struct virtio_crypto_inhdr inhdr;
> +};
> +\end{lstlisting}
> +
> +\drivernormative{\paragraph}{HASH Service Operation}{Device Types / Crypto Device / Device Operation / HASH Service Operation}
> +
> +\begin{itemize*}
> +\item If the driver uses the session mode, then the driver MUST set \field{session_id} in struct virtio_crypto_op_header
> +      to a valid value assigned by the device when the session was created.
> +\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated, the driver MUST use struct virtio_crypto_op_data_req_mux to wrap crypto requests. Otherwise, the driver MUST use struct virtio_crypto_op_data_req.
> +\item If the VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is negotiated, 1) if the driver uses the stateless mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header
> +      to VIRTIO_CRYPTO_FLAG_STATELESS_MODE and MUST set the fields in struct virtio_crypto_hash_para_statelession.sess_para, 2) if the driver uses the session mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header to VIRTIO_CRYPTO_FLAG_STATE_MODE.
> +\item The driver MUST set \field{opcode} in struct virtio_crypto_op_header to VIRTIO_CRYPTO_HASH.
> +\end{itemize*}
> +
> +\devicenormative{\paragraph}{HASH Service Operation}{Device Types / Crypto Device / Device Operation / HASH Service Operation}
> +
> +\begin{itemize*}
> +\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated, the device MUST parse the struct virtio_crypto_op_data_req_mux for crypto requests. Otherwise, the device MUST parse the struct virtio_crypto_op_data_req.
> +\item If the VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is negotiated, the device MUST parse \field{flag} field in struct virtio_crypto_op_header in order to decide which mode the driver uses.
> +\item The device MUST copy the results of HASH operations in the hash_result[] if HASH operations success.
> +\item The device MUST set \field{status} in struct virtio_crypto_inhdr to one of the values of enum VIRTIO_CRYPTO_STATUS.
> +\end{itemize*}
> +

OK, I have read this to the end and I don't think there is anything which requires
a comment at this stage. I would like to concentrate on your QEMU patches first,
and I think we have enough questions/issues already.

Regards,
Halil
[..]

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

[Halil Pasic]

On 05/10/2017 08:02 PM, Halil Pasic wrote:
> 
> 
> On 04/22/2017 08:23 AM, Gonglei wrote:
>> The virtio crypto device is a virtual crypto device (ie. hardware
>> crypto accelerator card). Currently, the virtio crypto device provides
>> the following crypto services: CIPHER, MAC, HASH, and AEAD.
>>
>> In this patch, CIPHER, MAC, HASH, AEAD services are introduced.
>>
>> VIRTIO-153
>>
>> Signed-off-by: Gonglei <arei.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: Mihai Claudiu Caraman <mike.caraman@nxp.com>
>> CC: Halil Pasic <pasic@linux.vnet.ibm.com>
>> ---
>>  acknowledgements.tex |    2 +
>>  content.tex          |    2 +
>>  virtio-crypto.tex    | 1309 ++++++++++++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 1313 insertions(+)
>>  create mode 100644 virtio-crypto.tex
>>
>> diff --git a/acknowledgements.tex b/acknowledgements.tex
>> index 53942b0..43b8a9b 100644
>> --- a/acknowledgements.tex
>> +++ b/acknowledgements.tex
>> @@ -26,6 +26,7 @@ Sasha Levin,	Oracle	\newline
>>  Sergey Tverdyshev,	Thales e-Security	\newline
>>  Stefan Hajnoczi,	Red Hat	\newline
>>  Tom Lyon,	Samya Systems, Inc.	\newline
>> +Lei Gong,  Huawei   \newline
>>  \end{oasistitlesection}
>>
>>  The following non-members have provided valuable feedback on this
>> @@ -44,4 +45,5 @@ Patrick Durusau,	Technical Advisory Board, OASIS	\newline
>>  Thomas Huth,	Red Hat	\newline
>>  Yan Vugenfirer, Red Hat / Daynix	\newline
>>  Kevin Lo,	MSI	\newline
>> +Halil Pasic, IBM  \newline
>>  \end{oasistitlesection}
>> 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..2708023
>> --- /dev/null
>> +++ b/virtio-crypto.tex
>> @@ -0,0 +1,1309 @@
>> +\section{Crypto Device}\label{sec:Device Types / Crypto Device}
>> +
>> +The virtio crypto device is a virtual cryptography device as well as a kind of
>> +virtual hardware accelerator for virtual machines. The encryption and
>> +decryption requests are placed in any of the data queues and are ultimately handled by the
>> +backend crypto accelerators. The second kind of queue is the control queue used to create 
>> +or destroy sessions for symmetric algorithms and will control some advanced
>> +features in the future. The virtio crypto device provides the following crypto
>> +services: CIPHER, MAC, HASH, and AEAD.
>> +
>> +
>> +\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}.
>> +
>> +\subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature bits}
>> +
>> +VIRTIO_CRYPTO_F_STATELESS_MODE (0) stateless mode is available.
>> +VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE (1) stateless mode is available for CIPHER service.
>> +VIRTIO_CRYPTO_F_HASH_STATELESS_MODE (2) stateless mode is available for HASH service.
>> +VIRTIO_CRYPTO_F_MAC_STATELESS_MODE (3) stateless mode is available for MAC service.
>> +VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE (4) stateless mode is available for AEAD service.
>> +
>> +\subsubsection{Feature bit requirements}\label{sec:Device Types / Crypto Device / Feature bits}
>> +
>> +Some crypto feature bits require other crypto feature bits
>> +(see \ref{drivernormative:Basic Facilities of a Virtio Device / Feature Bits}):
>> +
>> +\begin{description}
>> +\item[VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
>> +\item[VIRTIO_CRYPTO_F_HASH_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
>> +\item[VIRTIO_CRYPTO_F_MAC_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
>> +\item[VIRTIO_CRYPTO_F_AEAD_STATELESS_MODE] Requires VIRTIO_CRYPTO_F_STATELESS_MODE.
>> +\end{description}
>> +
>> +\subsection{Supported crypto services}\label{sec:Device Types / Crypto Device / Supported crypto services}
>> +
>> +The virtio crypto device provides the following crypto services: CIPHER, MAC, HASH, and AEAD.
>> +
>> +\begin{lstlisting}
>> +/* CIPHER service */
>> +#define VIRTIO_CRYPTO_SERVICE_CIPHER 0
>> +/* HASH service */
>> +#define VIRTIO_CRYPTO_SERVICE_HASH   1
>> +/* MAC (Message Authentication Codes) service */
>> +#define VIRTIO_CRYPTO_SERVICE_MAC    2
>> +/* AEAD (Authenticated Encryption with Associated Data) service */
>> +#define VIRTIO_CRYPTO_SERVICE_AEAD   3
>> +\end{lstlisting}
>> +
>> +The above constants are bit numbers, which tell the driver which crypto services
>> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
>> +
>> +\subsubsection{CIPHER services}\label{sec:Device Types / Crypto Device / Supported crypto services / CIPHER services}
>> +
>> +The following CIPHER algorithms are defined:
>> +
>> +\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 above constants have two usages:
>> +\begin{enumerate}
>> +\item As bit numbers, used to tell the driver which CIPHER algorithms
>> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
>> +\item As values, used to tell the device which CIPHER algorithm
>> +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
>> +\end{enumerate}
>> +
>> +\subsubsection{HASH services}\label{sec:Device Types / Crypto Device / Supported crypto services / HASH services}
>> +
>> +The following HASH algorithms are defined:
>> +
>> +\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 above constants have two usages:
>> +\begin{enumerate}
>> +\item As bit numbers, used to tell the driver which HASH algorithms
>> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
>> +\item As values, used to tell the device which HASH algorithm
>> +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
>> +\end{enumerate}
>> +
>> +\subsubsection{MAC services}\label{sec:Device Types / Crypto Device / Supported crypto services / MAC services}
>> +
>> +The following MAC algorithms are defined:
>> +
>> +\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_KASUMI_F9                27
>> +#define VIRTIO_CRYPTO_MAC_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 above constants have two usages:
>> +\begin{enumerate}
>> +\item As bit numbers, used to tell the driver which MAC algorithms
>> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
>> +\item As values, used to tell the device which MAC algorithm
>> +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
>> +\end{enumerate}
>> +
>> +\subsubsection{AEAD services}\label{sec:Device Types / Crypto Device / Supported crypto services / AEAD services}
>> +
>> +The following AEAD algorithms are defined:
>> +
>> +\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}
>> +
>> +The above constants have two usages:
>> +\begin{enumerate}
>> +\item As bit numbers, used to tell the driver which AEAD algorithms
>> +are supported by the device, see \ref{sec:Device Types / Crypto Device / Device configuration layout}.
>> +\item As values, used to tell the device what AEAD algorithm
>> +a crypto request from the driver requires, see \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}.
>> +\end{enumerate}
>> +
>> +\subsection{Device configuration layout}\label{sec:Device Types / Crypto Device / Device configuration layout}
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_config {
>> +    le32 status;
>> +    le32 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 aead_algo;
>> +    /* Maximum length of cipher key in bytes */
>> +    le32 max_cipher_key_len;
>> +    /* Maximum length of authenticated key in bytes */
>> +    le32 max_auth_key_len;
>> +    le32 reserved;
>> +    /* Maximum size of each crypto request's content in bytes */
>> +    le64 max_size;
>> +};
>> +\end{lstlisting}
>> +
>> +\begin{description}
>> +\item[\field{status}] is used to show whether the device is ready to work or not, it can be either zero or have one or more flags
>> +    Only one read-only bit (for the driver) 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}
>> +
>> +\item[\field{max_dataqueues}] is the maximum number of data virtqueues exposed by
>> +    the device. The driver MAY use only one data queue,
>> +    or it can use more to achieve better performance.
>> +
>> +\item[\field{crypto_services}] is a 32-bit mask which indicates the crypto services supported by
>> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services}.
>> +
>> +\item[\field{cipher_algo_l}] is the low 32-bit mask which indicates the CIPHER algorithms supported by
>> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / CIPHER services}.
>> +
>> +\item[\field{cipher_algo_h}] is the high 32-bit mask which indicates the CIPHER algorithms supported by
>> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / CIPHER services}.
>> +
>> +\item[\field{hash_algo}] is a 32-bit mask which indicates the HASH algorithms supported by
>> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / HASH services}.
>> +
>> +\item[\field{mac_algo_l}] is the low 32-bit mask which indicates the MAC algorithms supported by
>> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / MAC services}.
>> +
>> +\item[\field{mac_algo_h}] is the high 32-bit mask which indicates the MAC algorithms supported by
>> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / MAC services}.
>> +
>> +\item[\field{aead_algo}] is a 32-bit mask which indicates the AEAD algorithms supported by
>> +    the device, see \ref{sec:Device Types / Crypto Device / Supported crypto services  / AEAD services}.
>> +
>> +\item[\field{max_cipher_key_len}] is the maximum length of cipher key supported by the device.
>> +
>> +\item[\field{max_auth_key_len}] is the maximum length of authenticated key supported by the device.
>> +
>> +\item[\field{reserved}] is reserved for future use.
>> +
>> +\item[\field{max_size}] is the maximum size of each crypto request's content supported by the device
>> +\end{description}
>> +
>> +\begin{note}
>> +Unless explicitly stated otherwise all lengths and sizes are in bytes.
>> +\end{note}
>> +
>> +\devicenormative{\subsubsection}{Device configuration layout}{Device Types / Crypto Device / Device configuration layout}
>> +
>> +\begin{itemize*}
>> +\item The device MUST set \field{max_dataqueues} to between 1 and 65535 inclusive.
>> +\item The device MUST set \field{status} based on the status of the backend crypto accelerator. 
>> +\item The device MUST accept and handle requests after \field{status} is set to VIRTIO_CRYPTO_S_HW_READY.
>> +\item The device MUST set \field{crypto_services} based on the crypto services the device offers.
>> +\item The device MUST set detailed algorithms masks based on the \field{crypto_services} field.
>> +\item The device MUST set \field{max_size} to show the maximum size of crypto request the device supports.
>> +\item The device MUST set \field{max_cipher_key_len} to show the maximum length of cipher key if the device supports CIPHER service.
>> +\item The device MUST set \field{max_auth_key_len} to show the maximum length of authenticated key if the device supports MAC service.
>> +\end{itemize*}
>> +
>> +\drivernormative{\subsubsection}{Device configuration layout}{Device Types / Crypto Device / Device configuration layout}
>> +
>> +\begin{itemize*}
>> +\item The driver MUST read the ready \field{status} from the bottom bit of status to check whether the backend crypto accelerator
>> +      is ready or not, and the driver MUST reread it after device reset. 
>> +\item The driver MUST NOT transmit any requests to the device if the ready \field{status} is not set.
>> +\item The driver MUST read \field{max_dataqueues} field to discover the number of 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 the detailed algorithms fields based on \field{crypto_services} field.
>> +\item The driver SHOULD read \field{max_size} to discover the maximum size of crypto request the device supports.
>> +\item The driver SHOULD read \field{max_cipher_key_len} to discover the maximum length of cipher key the device supports.
>> +\item The driver SHOULD read \field{max_auth_key_len} to discover the maximum length of authenticated key the device supports.
>> +\end{itemize*}
>> +
>> +\subsection{Device Initialization}\label{sec:Device Types / Crypto Device / Device Initialization}
>> +
>> +\drivernormative{\subsubsection}{Device Initialization}{Device Types / Crypto Device / Device Initialization}
>> +
>> +\begin{itemize*}
>> +\item The driver MUST identify and initialize all virtqueues.
>> +\item The driver MUST read the supported crypto services from bits of \field{crypto_services}. 
>> +\item The driver MUST read the supported algorithms based on \field{crypto_services} field.
>> +\end{itemize*}
>> +
>> +\subsection{Device Operation}\label{sec:Device Types / Crypto Device / Device Operation}
>> +
>> +Requests can be transmitted by placing them in the controlq or dataq.
>> +Requests consist of a queue-type specific header specifying among
>> +others the operation, and an operation specific payload.
>> +The payload is generally composed of operation parameters, output data, and input data.
>> +Operation parameters are algorithm-specific parameters, output data is the
>> +data that should be utilized in operations, and input data is equal to
>> +"operation result + result data".
>> +
>> +The device can support both session mode (See \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}) and stateless mode.
>> +If VIRTIO_CRYPTO_F_CIPHER_STATELESS_MODE is negotiated, the driver can use stateless mode for CIPHER service, otherwise it can only use session mode.
>> +
>> +The header for controlq is as follows:
>> +
>> +\begin{lstlisting}
>> +#define VIRTIO_CRYPTO_OPCODE(service, op)   (((service) << 8) | (op))
>> +
>> +struct virtio_crypto_ctrl_header {
>> +#define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION \
>> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x02)
>> +#define VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION \
>> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x03)
>> +#define VIRTIO_CRYPTO_HASH_CREATE_SESSION \
>> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x02)
>> +#define VIRTIO_CRYPTO_HASH_DESTROY_SESSION \
>> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x03)
>> +#define VIRTIO_CRYPTO_MAC_CREATE_SESSION \
>> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x02)
>> +#define VIRTIO_CRYPTO_MAC_DESTROY_SESSION \
>> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x03)
>> +#define VIRTIO_CRYPTO_AEAD_CREATE_SESSION \
>> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02)
>> +#define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \
>> +       VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03)
>> +    le32 opcode;
>> +    /* algo should be service-specific algorithms */
>> +    le32 algo;
>> +    le32 flag;
>> +    /* data virtqueue id */
>> +    le32 queue_id;
>> +};
>> +\end{lstlisting}
>> +
>> +The header for dataq is as follows:
>> +
>> +\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;
>> +    le64 session_id;
>> +#define VIRTIO_CRYPTO_FLAG_STATE_MODE 1
>> +#define VIRTIO_CRYPTO_FLAG_STATELESS_MODE 2
>> +    /* control flag to control the request */
>> +    le32 flag;
>> +    le32 padding;
>> +};
>> +\end{lstlisting}
>> +
> 
> START HERE
> 
>> +The device can set the operation status as follows: VIRTIO_CRYPTO_OK: success;
>> +VIRTIO_CRYPTO_ERR: failure or device error; VIRTIO_CRYPTO_NOTSUPP: not supported;
>> +VIRTIO_CRYPTO_INVSESS: invalid session ID when executing crypto operations.
> 
> You describe everything but BADMSG. Could you be a bit more specific
> about the differences between _ERR _BADMSG and _NOTSUPP? Is for instance trying
> to do something with a not-advertised service or algo a _NOTSUPP or a
> _BADMSG or just a generic _ERR? Same qestion goes for different sorts of
> out of resources. 
> 
>> +
>> +\begin{lstlisting}
>> +enum VIRTIO_CRYPTO_STATUS {
>> +    VIRTIO_CRYPTO_OK = 0,
>> +    VIRTIO_CRYPTO_ERR = 1,
>> +    VIRTIO_CRYPTO_BADMSG = 2,
>> +    VIRTIO_CRYPTO_NOTSUPP = 3,
>> +    VIRTIO_CRYPTO_INVSESS = 4,
>> +    VIRTIO_CRYPTO_MAX
>> +};
>> +\end{lstlisting}
>> +
> 
> There are no more mentions of the values in the spec, so the description
> above should be real good.
> 
>> +\subsubsection{Control Virtqueue}\label{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue}
>> +
>> +The driver uses the control virtqueue to send control commands to the
>> +device, such as session operations (See \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}).
>> +
>> +Controlq requests are as follows:
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_op_ctrl_req {
>> +    struct virtio_crypto_ctrl_header header;
>> +
>> +    union {
>> +        struct virtio_crypto_sym_create_session_req   sym_create_session;
>> +        struct virtio_crypto_hash_create_session_req  hash_create_session;
>> +        struct virtio_crypto_mac_create_session_req   mac_create_session;
>> +        struct virtio_crypto_aead_create_session_req  aead_create_session;
>> +        struct virtio_crypto_destroy_session_req      destroy_session;
>> +    } u;
>> +};
>> +\end{lstlisting}
>> +
>> +struct virtio_crypto_op_ctrl_req is the only allowed control request.
>> +The header is the general header, and the union is of the algorithm-specific type,
>> +which is set by the driver. All the properties in the union are shown as follows.
>> +
>> +\paragraph{Session operation}\label{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation}
>> +
>> +The symmetric algorithms involve the concept of sessions. A session is a
>> +handle which describes the cryptographic parameters to be applied to
>> +a number of buffers. 
> 
> 8<
>> The data within a session handle includes:
>> +
>> +\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 set data, including the CIPHER algorithm and mode,
>> +      the key and its length, and the direction (encryption or decryption).
>> +\item The HASH/MAC set data, including the HASH algorithm or MAC algorithm,
>> +      and hash 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}
>> +
>> 8
> 
> This part is slightly confusing for me. I guess you are trying to describe
> what data can live in the session context (considering all the different
> applications). I think we do not have to describe that here, because we have
> to describe the individual session operations, and there we must describe
> the precise impact of these operations (and their parameters).
> 
>> +The following structure stores the result of session creation set by the device:
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_session_input {
>> +    /* Device-writable part */
>> +    le64 session_id;
>> +    le32 status;
>> +    le32 padding;
>> +};
>> +\end{lstlisting}
>> +
>> +A request to destroy a session includes the following information:
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_destroy_session_req {
>> +    /* Device-readable part */
>> +    le64  session_id;
>> +    /* Device-writable part */
>> +    le32  status;
>> +    le32  padding;
>> +};
>> +\end{lstlisting}
>> +
>> +\subparagraph{Session operation: HASH session}\label{sec:Device Types / Crypto Device / Device
>> +Operation / Control Virtqueue / Session operation / Session operation: HASH session}
>> +
>> +HASH session requests are as follows:
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_hash_session_para {
>> +    /* See VIRTIO_CRYPTO_HASH_* above */
>> +    le32 algo;
>> +    /* hash result length */
>> +    le32 hash_result_len;
>> +};
>> +struct virtio_crypto_hash_create_session_req {
>> +    /* Device-readable part */
>> +    struct virtio_crypto_hash_session_para para;
>> +    /* Device-writable part */
>> +    struct virtio_crypto_session_input input;
>> +};
>> +\end{lstlisting}
>> +
>> +\subparagraph{Session operation: MAC session}\label{sec:Device Types / Crypto Device / Device
>> +Operation / Control Virtqueue / Session operation / Session operation: MAC session}
>> +
>> +MAC session requests are as follows:
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_mac_session_para {
>> +    /* See VIRTIO_CRYPTO_MAC_* above */
>> +    le32 algo;
>> +    /* hash result length */
>> +    le32 hash_result_len;
>> +    /* length of authenticated key */
>> +    le32 auth_key_len;
>> +    le32 padding;
>> +};
>> +
>> +struct virtio_crypto_mac_create_session_req {
>> +    /* Device-readable part */
>> +    struct virtio_crypto_mac_session_para para;
>> +    /* The authenticated key */
>> +    u8 auth_key[auth_key_len];
>> +
>> +    /* Device-writable part */
>> +    struct virtio_crypto_session_input input;
>> +};
>> +\end{lstlisting}
>> +
>> +\subparagraph{Session operation: Symmetric algorithms session}\label{sec:Device Types / Crypto Device / Device
>> +Operation / Control Virtqueue / Session operation / Session operation: Symmetric algorithms session}
>> +
>> +The request of symmetric session includes two parts, CIPHER algorithms and chain
>> +algorithms (chaining CIPHER and HASH/MAC). 
>> +
>> +CIPHER session requests are as follows:
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_cipher_session_para {
>> +    /* See VIRTIO_CRYPTO_CIPHER* above */
>> +    le32 algo;
>> +    /* length of key */
>> +    le32 keylen;
>> +#define VIRTIO_CRYPTO_OP_ENCRYPT  1
>> +#define VIRTIO_CRYPTO_OP_DECRYPT  2
>> +    /* encryption or decryption */
>> +    le32 op;
>> +    le32 padding;
>> +};
>> +
>> +struct virtio_crypto_cipher_session_req {
>> +    /* Device-readable part */
>> +    struct virtio_crypto_cipher_session_para para;
>> +    /* The cipher key */
>> +    u8 cipher_key[keylen];
>> +
>> +    /* Device-writable part */
>> +    struct virtio_crypto_session_input input;
>> +};
>> +\end{lstlisting}
>> +
>> +Algorithm chaining requests are as follows:
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_alg_chain_session_para {
>> +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER  1
>> +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH  2
>> +    le32 alg_chain_order;
>> +/* Plain hash */
>> +#define VIRTIO_CRYPTO_SYM_HASH_MODE_PLAIN    1
>> +/* Authenticated hash (mac) */
>> +#define VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH     2
>> +/* Nested hash */
>> +#define VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED   3
>> +    le32 hash_mode;
>> +    struct virtio_crypto_cipher_session_para cipher_param;
>> +    union {
>> +        struct virtio_crypto_hash_session_para hash_param;
>> +        struct virtio_crypto_mac_session_para mac_param;
>> +    } u;
>> +    /* length of the additional authenticated data (AAD) in bytes */
>> +    le32 aad_len;
>> +    le32 padding;
>> +};
>> +
>> +struct virtio_crypto_alg_chain_session_req {
>> +    /* Device-readable part */
>> +    struct virtio_crypto_alg_chain_session_para para;
>> +    /* The cipher key */
>> +    u8 cipher_key[keylen];
>> +    /* The authenticated key */
>> +    u8 auth_key[auth_key_len];
>> +
>> +    /* Device-writable part */
>> +    struct virtio_crypto_session_input input;
>> +};
>> +\end{lstlisting}
>> +
>> +Symmetric algorithm requests are as follows:
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_sym_create_session_req {
>> +    union {
>> +        struct virtio_crypto_cipher_session_req cipher;
>> +        struct virtio_crypto_alg_chain_session_req chain;
>> +    } u;
>> +
>> +    /* Device-readable part */
>> +
>> +/* No operation */
>> +#define VIRTIO_CRYPTO_SYM_OP_NONE  0
>> +/* Cipher only operation on the data */
>> +#define VIRTIO_CRYPTO_SYM_OP_CIPHER  1
>> +/* Chain any cipher with any hash or mac operation. The order
>> +   depends on the value of alg_chain_order param */
>> +#define VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING  2
>> +    le32 op_type;
>> +    le32 padding;
>> +};
>> +\end{lstlisting}
>> +
>> +The driver can set the \field{op_type} field in struct virtio_crypto_sym_create_session_req as follows: VIRTIO_CRYPTO_SYM_OP_NONE: no operation;
>> +VIRTIO_CRYPTO_SYM_OP_CIPHER: Cipher only operation on the data; VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING: Chain any cipher with any hash or mac operation.
>> +
>> +\subparagraph{Session operation: AEAD session}\label{sec:Device Types / Crypto Device / Device
>> +Operation / Control Virtqueue / Session operation / Session operation: AEAD session}
>> +
>> +AEAD session requests are as follows:
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_aead_session_para {
>> +    /* See VIRTIO_CRYPTO_AEAD_* above */
>> +    le32 algo;
>> +    /* length of key */
>> +    le32 key_len;
>> +    /* Authentication tag length */
>> +    le32 tag_len;
>> +    /* The length of the additional authenticated data (AAD) in bytes */
>> +    le32 aad_len;
>> +    /* encryption or decryption, See above VIRTIO_CRYPTO_OP_* */
>> +    le32 op;
>> +    le32 padding;
>> +};
>> +
>> +struct virtio_crypto_aead_create_session_req {
>> +    /* Device-readable part */
>> +    struct virtio_crypto_aead_session_para para;
>> +    u8 key[key_len];
>> +
>> +    /* Device-writeable part */
>> +    struct virtio_crypto_session_input input;
>> +};
>> +\end{lstlisting}
>> +
>> +\drivernormative{\subparagraph}{Session operation: create session}{Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation / Session operation: create session}
>> +
>> +\begin{itemize*}
>> +\item The driver MUST set the control general header and corresponding properties of the union in structure virtio_crypto_op_ctrl_req. See \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue}.
>> +\item The driver MUST set the \field{opcode} field based on service type: CIPHER, HASH, MAC, or AEAD.
> 
>> +\item The driver MUST set the \field{queue_id} field to show used dataq.
> 
> Used for what? Is the driver obligued to use that dataq for the op reqs associated
> with the given session. If yes where is the normative statement?
> 
>> +\end{itemize*}
>> +
>> +\devicenormative{\subparagraph}{Session operation: create session}{Device Types / Crypto Device / Device
>> +Operation / Control Virtqueue / Session operation / Session operation: create session}
>> +
>> +\begin{itemize*}
>> +\item The device MUST set the \field{session_id} field to a unique session identifier when the device finishes processing session creation.
> 
> I guess only if successfull (that is status == VIRTIO_CRYPTO_OK).
> 
>> +\item The device MUST set the \field{status} field to one of the values of enum VIRTIO_CRYPTO_STATUS.
> 
> Maybe put this one first. The formulation could be more fortunate in
> a sense that it's required to set the right status (_OK if successs,
> _NOTSUPP if ...).
> 
> What shall happen if the operation fails, e.g. becasue of out of resources?
> 
> Is there some sort of limit for the amount of live sessions (related to
> the previous question)? Obviously the device needs storage for the
> session data. Can the guest DOS attack the host by creating an absurd number
> of sessions?
> 
> 
>> +\end{itemize*}
>> +
>> +\drivernormative{\subparagraph}{Session operation: destroy session}{Device Types / Crypto Device / Device
>> +Operation / Control Virtqueue / Session operation / Session operation: destroy session}
>> +
>> +\begin{itemize*}
>> +\item The driver MUST set the \field{opcode} field based on service type: CIPHER, HASH, MAC, or AEAD.
>> +\item The driver MUST set the \field{session_id} to a valid value assigned by the device when the session was created.
>> +\end{itemize*}
>> +
>> +\devicenormative{\subparagraph}{Session operation: destroy session}{Device Types / Crypto Device / Device
>> +Operation / Control Virtqueue / Session operation / Session operation: destroy session}
>> +
>> +\begin{itemize*}
>> +\item The device MUST set the \field{status} field to one of the values of enum VIRTIO_CRYPTO_STATUS.
> 
> Same as above.
> 
>> +\end{itemize*}
>> +
>> +\subsubsection{Data Virtqueue}\label{sec:Device Types / Crypto Device / Device Operation / Data Virtqueue}
>> +
>> +The driver uses the data virtqueue to transmit crypto operation requests to the device,
>> +and completes the crypto operations.
>> +
>> +Session mode dataq requests are as follows:
>> +
>> +\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}
>> +
>> +Dataq requests for both session and stateless modes are as follows:
> 
> This ain't very nice, I mean the 'both session and stateless modes' together
> with the above 'session mode dataq requests are'. In the meanwhile I know what
> you mean: if the SESSION_MODE feature bit was negotiated.
> 
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_op_data_req_mux {
>> +    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;
>> +        struct virtio_crypto_sym_data_req_stateless   sym_stateless_req;
>> +        struct virtio_crypto_hash_data_req_stateless  hash_stateless_req;
>> +        struct virtio_crypto_mac_data_req_stateless   mac_stateless_req;
>> +        struct virtio_crypto_aead_data_req_stateless  aead_stateless_req;
>> +    } u;
>> +};
>> +\end{lstlisting}
>> +
>> +The header is the general header and the union is of the algorithm-specific type,
>> +which is set by the driver. All properties in the union are shown as follows.
>> +
>> +There is a unified input header structure for all crypto services.
>> +
>> +The structure is defined as follows:
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_inhdr {
>> +    u8 status;
>> +};
>> +\end{lstlisting}
>> +
>> +\subsubsection{HASH Service Operation}\label{sec:Device Types / Crypto Device / Device Operation / HASH Service Operation}
>> +
>> +Session mode HASH service requests are as follows:
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_hash_para {
>> +    /* length of source data */
>> +    le32 src_data_len;
>> +    /* hash result length */
>> +    le32 hash_result_len;
>> +};
>> +
>> +struct virtio_crypto_hash_data_req {
>> +    /* Device-readable part */
>> +    struct virtio_crypto_hash_para para;
>> +    /* Source data */
>> +    u8 src_data[src_data_len];
>> +
>> +    /* Device-writable part */
>> +    /* Hash result data */
>> +    u8 hash_result[hash_result_len];
>> +    struct virtio_crypto_inhdr inhdr;
>> +};
>> +\end{lstlisting}
>> +
>> +Each data request uses virtio_crypto_hash_data_req structure to store information
>> +used to run the HASH operations. 
>> +
>> +The information includes the hash parameters stored in \field{para}, output data and input data.
>> +The output data here includes the source data and the input data includes the hash result data used to save the results of the HASH operations.
>> +\field{inhdr} stores the status of executing the HASH operations.
>> +
>> +Stateless mode HASH service requests are as follows:
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_hash_para_statelesss {
>> +    struct {
>> +        /* See VIRTIO_CRYPTO_HASH_* above */
>> +        le32 algo;
>> +    } sess_para;
>> +
>> +    /* length of source data */
>> +    le32 src_data_len;
>> +    /* hash result length */
>> +    le32 hash_result_len;
>> +    le32 reserved;
>> +};
>> +struct virtio_crypto_hash_data_req_stateless {
>> +    /* Device-readable part */
>> +    struct virtio_crypto_hash_para_stateless para;
>> +    /* Source data */
>> +    u8 src_data[src_data_len];
>> +
>> +    /* Device-writable part */
>> +    /* Hash result data */
>> +    u8 hash_result[hash_result_len];
>> +    struct virtio_crypto_inhdr inhdr;
>> +};
>> +\end{lstlisting}
>> +
>> +\drivernormative{\paragraph}{HASH Service Operation}{Device Types / Crypto Device / Device Operation / HASH Service Operation}
>> +
>> +\begin{itemize*}
>> +\item If the driver uses the session mode, then the driver MUST set \field{session_id} in struct virtio_crypto_op_header
>> +      to a valid value assigned by the device when the session was created.
>> +\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated, the driver MUST use struct virtio_crypto_op_data_req_mux to wrap crypto requests. Otherwise, the driver MUST use struct virtio_crypto_op_data_req.
>> +\item If the VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is negotiated, 1) if the driver uses the stateless mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header
>> +      to VIRTIO_CRYPTO_FLAG_STATELESS_MODE and MUST set the fields in struct virtio_crypto_hash_para_statelession.sess_para, 2) if the driver uses the session mode, then the driver MUST set the \field{flag} field in struct virtio_crypto_op_header to VIRTIO_CRYPTO_FLAG_STATE_MODE.
>> +\item The driver MUST set \field{opcode} in struct virtio_crypto_op_header to VIRTIO_CRYPTO_HASH.
>> +\end{itemize*}
>> +
>> +\devicenormative{\paragraph}{HASH Service Operation}{Device Types / Crypto Device / Device Operation / HASH Service Operation}
>> +
>> +\begin{itemize*}
>> +\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated, the device MUST parse the struct virtio_crypto_op_data_req_mux for crypto requests. Otherwise, the device MUST parse the struct virtio_crypto_op_data_req.
>> +\item If the VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is negotiated, the device MUST parse \field{flag} field in struct virtio_crypto_op_header in order to decide which mode the driver uses.
>> +\item The device MUST copy the results of HASH operations in the hash_result[] if HASH operations success.
>> +\item The device MUST set \field{status} in struct virtio_crypto_inhdr to one of the values of enum VIRTIO_CRYPTO_STATUS.
>> +\end{itemize*}
>> +
> 
> OK, I have read this to the end and I don't think there is anything which requires
> a comment at this stage. I would like to concentrate on your QEMU patches first,
> and I think we have enough questions/issues already.
> 
> Regards,
> Halil
> [..]
> 
> 

ping

If you have any need for further discussion on this, I would prefer
having it rather sooner than later. 

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

[Gonglei (Arei)]

Hi Halil,

Sorry for delay because I'm busy on inner production project recently.

> 
> 
> START HERE
> 
> > +The device can set the operation status as follows: VIRTIO_CRYPTO_OK:
> success;
> > +VIRTIO_CRYPTO_ERR: failure or device error; VIRTIO_CRYPTO_NOTSUPP:
> not supported;
> > +VIRTIO_CRYPTO_INVSESS: invalid session ID when executing crypto
> operations.
> 
> You describe everything but BADMSG. Could you be a bit more specific
> about the differences between _ERR _BADMSG and _NOTSUPP? Is for instance
> trying
> to do something with a not-advertised service or algo a _NOTSUPP or a
> _BADMSG or just a generic _ERR? Same qestion goes for different sorts of
> out of resources.
> 
Sure. Will do.

> > +
> > +\begin{lstlisting}
> > +enum VIRTIO_CRYPTO_STATUS {
> > +    VIRTIO_CRYPTO_OK = 0,
> > +    VIRTIO_CRYPTO_ERR = 1,
> > +    VIRTIO_CRYPTO_BADMSG = 2,
> > +    VIRTIO_CRYPTO_NOTSUPP = 3,
> > +    VIRTIO_CRYPTO_INVSESS = 4,
> > +    VIRTIO_CRYPTO_MAX
> > +};
> > +\end{lstlisting}
> > +
> 
> There are no more mentions of the values in the spec, so the description
> above should be real good.
> 
Agree.

> > +\subsubsection{Control Virtqueue}\label{sec:Device Types / Crypto Device /
> Device Operation / Control Virtqueue}
> > +
> > +The driver uses the control virtqueue to send control commands to the
> > +device, such as session operations (See \ref{sec:Device Types / Crypto
> Device / Device Operation / Control Virtqueue / Session operation}).
> > +
> > +Controlq requests are as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_op_ctrl_req {
> > +    struct virtio_crypto_ctrl_header header;
> > +
> > +    union {
> > +        struct virtio_crypto_sym_create_session_req
> sym_create_session;
> > +        struct virtio_crypto_hash_create_session_req
> hash_create_session;
> > +        struct virtio_crypto_mac_create_session_req
> mac_create_session;
> > +        struct virtio_crypto_aead_create_session_req
> aead_create_session;
> > +        struct virtio_crypto_destroy_session_req      destroy_session;
> > +    } u;
> > +};
> > +\end{lstlisting}
> > +
> > +struct virtio_crypto_op_ctrl_req is the only allowed control request.
> > +The header is the general header, and the union is of the algorithm-specific
> type,
> > +which is set by the driver. All the properties in the union are shown as
> follows.
> > +
> > +\paragraph{Session operation}\label{sec:Device Types / Crypto Device /
> Device Operation / Control Virtqueue / Session operation}
> > +
> > +The symmetric algorithms involve the concept of sessions. A session is a
> > +handle which describes the cryptographic parameters to be applied to
> > +a number of buffers.
> 
> 8<
> > The data within a session handle includes:
> > +
> > +\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 set data, including the CIPHER algorithm and mode,
> > +      the key and its length, and the direction (encryption or decryption).
> > +\item The HASH/MAC set data, including the HASH algorithm or MAC
> algorithm,
> > +      and hash 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}
> > +
> >8
> 
> This part is slightly confusing for me. I guess you are trying to describe
> what data can live in the session context (considering all the different
> applications). I think we do not have to describe that here, because we have
> to describe the individual session operations, and there we must describe
> the precise impact of these operations (and their parameters).
> 
Make sense to me.

> > +The following structure stores the result of session creation set by the
> device:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_session_input {
> > +    /* Device-writable part */
> > +    le64 session_id;
> > +    le32 status;
> > +    le32 padding;
> > +};
> > +\end{lstlisting}
> > +
> > +A request to destroy a session includes the following information:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_destroy_session_req {
> > +    /* Device-readable part */
> > +    le64  session_id;
> > +    /* Device-writable part */
> > +    le32  status;
> > +    le32  padding;
> > +};
> > +\end{lstlisting}
> > +
> > +\subparagraph{Session operation: HASH session}\label{sec:Device Types /
> Crypto Device / Device
> > +Operation / Control Virtqueue / Session operation / Session operation: HASH
> session}
> > +
> > +HASH session requests are as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_hash_session_para {
> > +    /* See VIRTIO_CRYPTO_HASH_* above */
> > +    le32 algo;
> > +    /* hash result length */
> > +    le32 hash_result_len;
> > +};
> > +struct virtio_crypto_hash_create_session_req {
> > +    /* Device-readable part */
> > +    struct virtio_crypto_hash_session_para para;
> > +    /* Device-writable part */
> > +    struct virtio_crypto_session_input input;
> > +};
> > +\end{lstlisting}
> > +
> > +\subparagraph{Session operation: MAC session}\label{sec:Device Types /
> Crypto Device / Device
> > +Operation / Control Virtqueue / Session operation / Session operation: MAC
> session}
> > +
> > +MAC session requests are as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_mac_session_para {
> > +    /* See VIRTIO_CRYPTO_MAC_* above */
> > +    le32 algo;
> > +    /* hash result length */
> > +    le32 hash_result_len;
> > +    /* length of authenticated key */
> > +    le32 auth_key_len;
> > +    le32 padding;
> > +};
> > +
> > +struct virtio_crypto_mac_create_session_req {
> > +    /* Device-readable part */
> > +    struct virtio_crypto_mac_session_para para;
> > +    /* The authenticated key */
> > +    u8 auth_key[auth_key_len];
> > +
> > +    /* Device-writable part */
> > +    struct virtio_crypto_session_input input;
> > +};
> > +\end{lstlisting}
> > +
> > +\subparagraph{Session operation: Symmetric algorithms
> session}\label{sec:Device Types / Crypto Device / Device
> > +Operation / Control Virtqueue / Session operation / Session operation:
> Symmetric algorithms session}
> > +
> > +The request of symmetric session includes two parts, CIPHER algorithms
> and chain
> > +algorithms (chaining CIPHER and HASH/MAC).
> > +
> > +CIPHER session requests are as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_cipher_session_para {
> > +    /* See VIRTIO_CRYPTO_CIPHER* above */
> > +    le32 algo;
> > +    /* length of key */
> > +    le32 keylen;
> > +#define VIRTIO_CRYPTO_OP_ENCRYPT  1
> > +#define VIRTIO_CRYPTO_OP_DECRYPT  2
> > +    /* encryption or decryption */
> > +    le32 op;
> > +    le32 padding;
> > +};
> > +
> > +struct virtio_crypto_cipher_session_req {
> > +    /* Device-readable part */
> > +    struct virtio_crypto_cipher_session_para para;
> > +    /* The cipher key */
> > +    u8 cipher_key[keylen];
> > +
> > +    /* Device-writable part */
> > +    struct virtio_crypto_session_input input;
> > +};
> > +\end{lstlisting}
> > +
> > +Algorithm chaining requests are as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_alg_chain_session_para {
> > +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER
> 1
> > +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH
> 2
> > +    le32 alg_chain_order;
> > +/* Plain hash */
> > +#define VIRTIO_CRYPTO_SYM_HASH_MODE_PLAIN    1
> > +/* Authenticated hash (mac) */
> > +#define VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH     2
> > +/* Nested hash */
> > +#define VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED   3
> > +    le32 hash_mode;
> > +    struct virtio_crypto_cipher_session_para cipher_param;
> > +    union {
> > +        struct virtio_crypto_hash_session_para hash_param;
> > +        struct virtio_crypto_mac_session_para mac_param;
> > +    } u;
> > +    /* length of the additional authenticated data (AAD) in bytes */
> > +    le32 aad_len;
> > +    le32 padding;
> > +};
> > +
> > +struct virtio_crypto_alg_chain_session_req {
> > +    /* Device-readable part */
> > +    struct virtio_crypto_alg_chain_session_para para;
> > +    /* The cipher key */
> > +    u8 cipher_key[keylen];
> > +    /* The authenticated key */
> > +    u8 auth_key[auth_key_len];
> > +
> > +    /* Device-writable part */
> > +    struct virtio_crypto_session_input input;
> > +};
> > +\end{lstlisting}
> > +
> > +Symmetric algorithm requests are as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_sym_create_session_req {
> > +    union {
> > +        struct virtio_crypto_cipher_session_req cipher;
> > +        struct virtio_crypto_alg_chain_session_req chain;
> > +    } u;
> > +
> > +    /* Device-readable part */
> > +
> > +/* No operation */
> > +#define VIRTIO_CRYPTO_SYM_OP_NONE  0
> > +/* Cipher only operation on the data */
> > +#define VIRTIO_CRYPTO_SYM_OP_CIPHER  1
> > +/* Chain any cipher with any hash or mac operation. The order
> > +   depends on the value of alg_chain_order param */
> > +#define VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING  2
> > +    le32 op_type;
> > +    le32 padding;
> > +};
> > +\end{lstlisting}
> > +
> > +The driver can set the \field{op_type} field in struct
> virtio_crypto_sym_create_session_req as follows:
> VIRTIO_CRYPTO_SYM_OP_NONE: no operation;
> > +VIRTIO_CRYPTO_SYM_OP_CIPHER: Cipher only operation on the data;
> VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING: Chain any cipher with any
> hash or mac operation.
> > +
> > +\subparagraph{Session operation: AEAD session}\label{sec:Device Types /
> Crypto Device / Device
> > +Operation / Control Virtqueue / Session operation / Session operation: AEAD
> session}
> > +
> > +AEAD session requests are as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_aead_session_para {
> > +    /* See VIRTIO_CRYPTO_AEAD_* above */
> > +    le32 algo;
> > +    /* length of key */
> > +    le32 key_len;
> > +    /* Authentication tag length */
> > +    le32 tag_len;
> > +    /* The length of the additional authenticated data (AAD) in bytes */
> > +    le32 aad_len;
> > +    /* encryption or decryption, See above VIRTIO_CRYPTO_OP_* */
> > +    le32 op;
> > +    le32 padding;
> > +};
> > +
> > +struct virtio_crypto_aead_create_session_req {
> > +    /* Device-readable part */
> > +    struct virtio_crypto_aead_session_para para;
> > +    u8 key[key_len];
> > +
> > +    /* Device-writeable part */
> > +    struct virtio_crypto_session_input input;
> > +};
> > +\end{lstlisting}
> > +
> > +\drivernormative{\subparagraph}{Session operation: create session}{Device
> Types / Crypto Device / Device Operation / Control Virtqueue / Session
> operation / Session operation: create session}
> > +
> > +\begin{itemize*}
> > +\item The driver MUST set the control general header and corresponding
> properties of the union in structure virtio_crypto_op_ctrl_req. See
> \ref{sec:Device Types / Crypto Device / Device Operation / Control Virtqueue}.
> > +\item The driver MUST set the \field{opcode} field based on service type:
> CIPHER, HASH, MAC, or AEAD.
> 
> > +\item The driver MUST set the \field{queue_id} field to show used dataq.
> 
> Used for what? Is the driver obligued to use that dataq for the op reqs
> associated
> with the given session. If yes where is the normative statement?
> 
Yes, I missed.


> > +\end{itemize*}
> > +
> > +\devicenormative{\subparagraph}{Session operation: create session}{Device
> Types / Crypto Device / Device
> > +Operation / Control Virtqueue / Session operation / Session operation:
> create session}
> > +
> > +\begin{itemize*}
> > +\item The device MUST set the \field{session_id} field to a unique session
> identifier when the device finishes processing session creation.
> 
> I guess only if successfull (that is status == VIRTIO_CRYPTO_OK).
> 
Sure. Let me add the condition.

> > +\item The device MUST set the \field{status} field to one of the values of
> enum VIRTIO_CRYPTO_STATUS.
> 
> Maybe put this one first. The formulation could be more fortunate in
> a sense that it's required to set the right status (_OK if successs,
> _NOTSUPP if ...).
> 
> What shall happen if the operation fails, e.g. becasue of out of resources?
> 
That means the driver can't create session, and the following crypto
operation are forbidden.

> Is there some sort of limit for the amount of live sessions (related to
> the previous question)? Obviously the device needs storage for the
> session data. Can the guest DOS attack the host by creating an absurd number
> of sessions?
> 
Good question. It's true that the guest can trigger DoS attack.
What's your opinion about the limit in the spec? Add a new feature?


> > +\end{itemize*}
> > +
> > +\drivernormative{\subparagraph}{Session operation: destroy
> session}{Device Types / Crypto Device / Device
> > +Operation / Control Virtqueue / Session operation / Session operation:
> destroy session}
> > +
> > +\begin{itemize*}
> > +\item The driver MUST set the \field{opcode} field based on service type:
> CIPHER, HASH, MAC, or AEAD.
> > +\item The driver MUST set the \field{session_id} to a valid value assigned by
> the device when the session was created.
> > +\end{itemize*}
> > +
> > +\devicenormative{\subparagraph}{Session operation: destroy
> session}{Device Types / Crypto Device / Device
> > +Operation / Control Virtqueue / Session operation / Session operation:
> destroy session}
> > +
> > +\begin{itemize*}
> > +\item The device MUST set the \field{status} field to one of the values of
> enum VIRTIO_CRYPTO_STATUS.
> 
> Same as above.
> 
OK.

> > +\end{itemize*}
> > +
> > +\subsubsection{Data Virtqueue}\label{sec:Device Types / Crypto Device /
> Device Operation / Data Virtqueue}
> > +
> > +The driver uses the data virtqueue to transmit crypto operation requests to
> the device,
> > +and completes the crypto operations.
> > +
> > +Session mode dataq requests are as follows:
> > +
> > +\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}
> > +
> > +Dataq requests for both session and stateless modes are as follows:
> 
> This ain't very nice, I mean the 'both session and stateless modes' together

So what's your idea?

> with the above 'session mode dataq requests are'. In the meanwhile I know
> what
> you mean: if the SESSION_MODE feature bit was negotiated.
> 
Yes, that's what I want. If the MUX_MODE feature bit is negotiated, the
driver MUST use struct virtio_crypto_op_data_req_mux for requests.


> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_op_data_req_mux {
> > +    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;
> > +        struct virtio_crypto_sym_data_req_stateless
> sym_stateless_req;
> > +        struct virtio_crypto_hash_data_req_stateless
> hash_stateless_req;
> > +        struct virtio_crypto_mac_data_req_stateless
> mac_stateless_req;
> > +        struct virtio_crypto_aead_data_req_stateless
> aead_stateless_req;
> > +    } u;
> > +};
> > +\end{lstlisting}
> > +
> > +The header is the general header and the union is of the algorithm-specific
> type,
> > +which is set by the driver. All properties in the union are shown as follows.
> > +
> > +There is a unified input header structure for all crypto services.
> > +
> > +The structure is defined as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_inhdr {
> > +    u8 status;
> > +};
> > +\end{lstlisting}
> > +
> > +\subsubsection{HASH Service Operation}\label{sec:Device Types / Crypto
> Device / Device Operation / HASH Service Operation}
> > +
> > +Session mode HASH service requests are as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_hash_para {
> > +    /* length of source data */
> > +    le32 src_data_len;
> > +    /* hash result length */
> > +    le32 hash_result_len;
> > +};
> > +
> > +struct virtio_crypto_hash_data_req {
> > +    /* Device-readable part */
> > +    struct virtio_crypto_hash_para para;
> > +    /* Source data */
> > +    u8 src_data[src_data_len];
> > +
> > +    /* Device-writable part */
> > +    /* Hash result data */
> > +    u8 hash_result[hash_result_len];
> > +    struct virtio_crypto_inhdr inhdr;
> > +};
> > +\end{lstlisting}
> > +
> > +Each data request uses virtio_crypto_hash_data_req structure to store
> information
> > +used to run the HASH operations.
> > +
> > +The information includes the hash parameters stored in \field{para}, output
> data and input data.
> > +The output data here includes the source data and the input data includes
> the hash result data used to save the results of the HASH operations.
> > +\field{inhdr} stores the status of executing the HASH operations.
> > +
> > +Stateless mode HASH service requests are as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_hash_para_statelesss {
> > +    struct {
> > +        /* See VIRTIO_CRYPTO_HASH_* above */
> > +        le32 algo;
> > +    } sess_para;
> > +
> > +    /* length of source data */
> > +    le32 src_data_len;
> > +    /* hash result length */
> > +    le32 hash_result_len;
> > +    le32 reserved;
> > +};
> > +struct virtio_crypto_hash_data_req_stateless {
> > +    /* Device-readable part */
> > +    struct virtio_crypto_hash_para_stateless para;
> > +    /* Source data */
> > +    u8 src_data[src_data_len];
> > +
> > +    /* Device-writable part */
> > +    /* Hash result data */
> > +    u8 hash_result[hash_result_len];
> > +    struct virtio_crypto_inhdr inhdr;
> > +};
> > +\end{lstlisting}
> > +
> > +\drivernormative{\paragraph}{HASH Service Operation}{Device Types /
> Crypto Device / Device Operation / HASH Service Operation}
> > +
> > +\begin{itemize*}
> > +\item If the driver uses the session mode, then the driver MUST set
> \field{session_id} in struct virtio_crypto_op_header
> > +      to a valid value assigned by the device when the session was
> created.
> > +\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated,
> the driver MUST use struct virtio_crypto_op_data_req_mux to wrap crypto
> requests. Otherwise, the driver MUST use struct virtio_crypto_op_data_req.
> > +\item If the VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is
> negotiated, 1) if the driver uses the stateless mode, then the driver MUST set
> the \field{flag} field in struct virtio_crypto_op_header
> > +      to VIRTIO_CRYPTO_FLAG_STATELESS_MODE and MUST set the fields
> in struct virtio_crypto_hash_para_statelession.sess_para, 2) if the driver uses
> the session mode, then the driver MUST set the \field{flag} field in struct
> virtio_crypto_op_header to VIRTIO_CRYPTO_FLAG_STATE_MODE.
> > +\item The driver MUST set \field{opcode} in struct virtio_crypto_op_header
> to VIRTIO_CRYPTO_HASH.
> > +\end{itemize*}
> > +
> > +\devicenormative{\paragraph}{HASH Service Operation}{Device Types /
> Crypto Device / Device Operation / HASH Service Operation}
> > +
> > +\begin{itemize*}
> > +\item If the VIRTIO_CRYPTO_F_STATELESS_MODE feature bit is negotiated,
> the device MUST parse the struct virtio_crypto_op_data_req_mux for crypto
> requests. Otherwise, the device MUST parse the struct
> virtio_crypto_op_data_req.
> > +\item If the VIRTIO_CRYPTO_F_HASH_STATELESS_MODE feature bit is
> negotiated, the device MUST parse \field{flag} field in struct
> virtio_crypto_op_header in order to decide which mode the driver uses.
> > +\item The device MUST copy the results of HASH operations in the
> hash_result[] if HASH operations success.
> > +\item The device MUST set \field{status} in struct virtio_crypto_inhdr to one
> of the values of enum VIRTIO_CRYPTO_STATUS.
> > +\end{itemize*}
> > +
> 
> OK, I have read this to the end and I don't think there is anything which requires
> a comment at this stage. I would like to concentrate on your QEMU patches
> first,
> and I think we have enough questions/issues already.
> 

Great thanks for your comments. Pls go ahead. :)

Thanks,
-Gonglei

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

[Halil Pasic]

On 05/16/2017 05:48 AM, Gonglei (Arei) wrote:

[..]
>>> +\begin{itemize*}
>>> +\item The device MUST set the \field{session_id} field to a unique session
>> identifier when the device finishes processing session creation.
>>
>> I guess only if successfull (that is status == VIRTIO_CRYPTO_OK).
>>
> Sure. Let me add the condition.
> 
>>> +\item The device MUST set the \field{status} field to one of the values of
>> enum VIRTIO_CRYPTO_STATUS.
>>
>> Maybe put this one first. The formulation could be more fortunate in
>> a sense that it's required to set the right status (_OK if successs,
>> _NOTSUPP if ...).
>>
>> What shall happen if the operation fails, e.g. becasue of out of resources?
>>
> That means the driver can't create session, and the following crypto
> operation are forbidden.
> 

I wanted to ask which status code has to be used in what case? Is it always
_ERR or _NOTSUPP?

>> Is there some sort of limit for the amount of live sessions (related to
>> the previous question)? Obviously the device needs storage for the
>> session data. Can the guest DOS attack the host by creating an absurd number
>> of sessions?
>>
> Good question. It's true that the guest can trigger DoS attack.
> What's your opinion about the limit in the spec? Add a new feature?
> 

Adding a new feature does not seem like a good idea to me, because
if the guest can turn of the safety mechanism (not negotiate the
corresponding feature), then we are almost back to where we started.

My idea would be to specify how is the device (and maybe also the driver,
I can't tell) supposed to fail if max_session is reached. The device should
just set some proper status for the request I guess. We could also
make the maximal number of sessions an implementation defined property
of the device, and expose it via configuration space.

Hard limit in the spec is also viable, but in that case we also need
to specify what happens if the limit is reached.

[..]

>>> +\subsubsection{Data Virtqueue}\label{sec:Device Types / Crypto Device /
>> Device Operation / Data Virtqueue}
>>> +
>>> +The driver uses the data virtqueue to transmit crypto operation requests to
s/virtqueue/virtqueues
We may have more than one dataq

>> the device,
>>> +and completes the crypto operations.
>>> +
>>> +Session mode dataq requests are as follows:
>>> +
>>> +\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}
>>> +
>>> +Dataq requests for both session and stateless modes are as follows:
>>
>> This ain't very nice, I mean the 'both session and stateless modes' together
> 
> So what's your idea?
> 

As I have pointed out while reviewing the QEMU implementation of the stateless
mode I'm not satisfied with the description of the request format(s).

This here is rather a  cosmetic issue, and my idea was to replace
'Session mode dataq requests' with 'If MUX_MODE feature bit is not
negotiated the dataq requests'. And also replace 'Dataq requests for both
session and stateless modes are' with 'If MUX_MODE feature bit is
negotiated the dataq requests are'.

In my eyes, how these requests are described is a much bigger problem.

>> with the above 'session mode dataq requests are'. In the meanwhile I know
>> what
>> you mean: if the SESSION_MODE feature bit was negotiated.
>>
> Yes, that's what I want. If the MUX_MODE feature bit is negotiated, the
> driver MUST use struct virtio_crypto_op_data_req_mux for requests.
> 
> 
[..]
>>
>> OK, I have read this to the end and I don't think there is anything which requires
>> a comment at this stage. I would like to concentrate on your QEMU patches
>> first,
>> and I think we have enough questions/issues already.
>>
> 
> Great thanks for your comments. Pls go ahead. :)

Your are welcome.

Regards,
Halil

> 
> Thanks,
> -Gonglei
> 
> 

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

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 2547 bytes --]

On Sat, Apr 22, 2017 at 02:23:50PM +0800, Gonglei wrote:
> +Dataq requests for both session and stateless modes are as follows:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_op_data_req_mux {
> +    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;
> +        struct virtio_crypto_sym_data_req_stateless   sym_stateless_req;
> +        struct virtio_crypto_hash_data_req_stateless  hash_stateless_req;
> +        struct virtio_crypto_mac_data_req_stateless   mac_stateless_req;
> +        struct virtio_crypto_aead_data_req_stateless  aead_stateless_req;
> +    } u;
> +};
> +\end{lstlisting}

Halil touched on this in the discussion: this spec uses a C-like struct
syntax but does not define whether unions really affect sizeof(mystruct)
like they would in C or whether you just mean that any of the union
fields can be used.  This distinction is important so device and driver
authors understand the exact memory layout of requests and responses.

Please include an explanation about the meaning of "union" in the text.

> +Session mode MAC service requests are as follows:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_mac_para {
> +    struct virtio_crypto_hash_para hash;
> +};
> +
> +struct virtio_crypto_mac_data_req {
> +    /* Device-readable part */
> +    struct virtio_crypto_mac_para para;
> +    /* Source data */
> +    u8 src_data[src_data_len];
> +
> +    /* Device-writable part */
> +    /* Hash result data */
> +    u8 hash_result[hash_result_len];
> +    struct virtio_crypto_inhdr inhdr;
> +};
> +\end{lstlisting}
> +
> +Each data request uses virtio_crypto_mac_data_req structure to store information

"Each request uses the virtio_crypto_mac_data_req structure to store
information"

> +Session mode requests of symmetric algorithm are as follows:
> +
> +\begin{lstlisting}
> +struct virtio_crypto_sym_data_req {
> +    union {
> +        struct virtio_crypto_cipher_data_req cipher;
> +        struct virtio_crypto_alg_chain_data_req chain;
> +    } u;
> +
> +    /* Device-readable part */
> +
> +    /* See above VIRTIO_CRYPTO_SYM_OP_* */
> +    le32 op_type;
> +    le32 padding;
> +};
> +\end{lstlisting}
> +
> +Each data request uses virtio_crypto_sym_data_req structure to store information

s/virtio_crypto_sym_data_req structure/the virtio_crypto_sym_data_req structure/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

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

[Halil Pasic]

[-- Attachment #1: Type: text/plain, Size: 1782 bytes --]



On 05/16/2017 05:33 PM, Stefan Hajnoczi wrote:
> On Sat, Apr 22, 2017 at 02:23:50PM +0800, Gonglei wrote:
>> +Dataq requests for both session and stateless modes are as follows:
>> +
>> +\begin{lstlisting}
>> +struct virtio_crypto_op_data_req_mux {
>> +    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;
>> +        struct virtio_crypto_sym_data_req_stateless   sym_stateless_req;
>> +        struct virtio_crypto_hash_data_req_stateless  hash_stateless_req;
>> +        struct virtio_crypto_mac_data_req_stateless   mac_stateless_req;
>> +        struct virtio_crypto_aead_data_req_stateless  aead_stateless_req;
>> +    } u;
>> +};
>> +\end{lstlisting}
> 
> Halil touched on this in the discussion: this spec uses a C-like struct
> syntax but does not define whether unions really affect sizeof(mystruct)
> like they would in C or whether you just mean that any of the union
> fields can be used.  This distinction is important so device and driver
> authors understand the exact memory layout of requests and responses.
> 
> Please include an explanation about the meaning of "union" in the text.
> 

I do not think simple explaining the union will do. I think this
description is bleeding from more wounds. I tried to explain this
while reviewing the implementation here:
https://lists.gnu.org/archive/html/qemu-devel/2017-05/msg03876.html

Unfortunately some technical issues precluded me from posting it
in a timely manner.

@Stefan: Thanks for joining the discussion.

Regards,
Halil


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

[Gonglei (Arei)]

>
> 
> On Sat, Apr 22, 2017 at 02:23:50PM +0800, Gonglei wrote:
> > +Dataq requests for both session and stateless modes are as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_op_data_req_mux {
> > +    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;
> > +        struct virtio_crypto_sym_data_req_stateless
> sym_stateless_req;
> > +        struct virtio_crypto_hash_data_req_stateless
> hash_stateless_req;
> > +        struct virtio_crypto_mac_data_req_stateless
> mac_stateless_req;
> > +        struct virtio_crypto_aead_data_req_stateless
> aead_stateless_req;
> > +    } u;
> > +};
> > +\end{lstlisting}
> 
> Halil touched on this in the discussion: this spec uses a C-like struct
> syntax but does not define whether unions really affect sizeof(mystruct)
> like they would in C or whether you just mean that any of the union
> fields can be used.  This distinction is important so device and driver
> authors understand the exact memory layout of requests and responses.
> 
> Please include an explanation about the meaning of "union" in the text.
> 
Halil has different opinion in other thread, I replied.

I'll wait for his new response about it. Thanks!

> > +Session mode MAC service requests are as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_mac_para {
> > +    struct virtio_crypto_hash_para hash;
> > +};
> > +
> > +struct virtio_crypto_mac_data_req {
> > +    /* Device-readable part */
> > +    struct virtio_crypto_mac_para para;
> > +    /* Source data */
> > +    u8 src_data[src_data_len];
> > +
> > +    /* Device-writable part */
> > +    /* Hash result data */
> > +    u8 hash_result[hash_result_len];
> > +    struct virtio_crypto_inhdr inhdr;
> > +};
> > +\end{lstlisting}
> > +
> > +Each data request uses virtio_crypto_mac_data_req structure to store
> information
> 
> "Each request uses the virtio_crypto_mac_data_req structure to store
> information"
> 
OK.

> > +Session mode requests of symmetric algorithm are as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_sym_data_req {
> > +    union {
> > +        struct virtio_crypto_cipher_data_req cipher;
> > +        struct virtio_crypto_alg_chain_data_req chain;
> > +    } u;
> > +
> > +    /* Device-readable part */
> > +
> > +    /* See above VIRTIO_CRYPTO_SYM_OP_* */
> > +    le32 op_type;
> > +    le32 padding;
> > +};
> > +\end{lstlisting}
> > +
> > +Each data request uses virtio_crypto_sym_data_req structure to store
> information
> 
> s/virtio_crypto_sym_data_req structure/the virtio_crypto_sym_data_req
> structure/

OK.

Thanks,
-Gonglei

[Qemu-devel] [PATCH v18 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 | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/conformance.tex b/conformance.tex
index f59e360..7c7a55c 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}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Initialization}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation / Session operation: create session}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation / Session operation: destroy session}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / HASH Service operation}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / MAC Service operation}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / Symmetric algorithms Operation}
+\item \ref{drivernormative:Device Types / Crypto Device / Device Operation / AEAD Service operation}
+\end{itemize}
+
 \section{Device Conformance}\label{sec:Conformance / Device Conformance}
 
 A device MUST conform to the following normative statements:
@@ -267,6 +282,20 @@ 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{devicenormative:Device Types / Crypto Device / Device configuration layout}
+\item \ref{devicenormative:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation / Session operation: create session}
+\item \ref{devicenormative:Device Types / Crypto Device / Device Operation / Control Virtqueue / Session operation / Session operation: destroy session}
+\item \ref{devicenormative:Device Types / Crypto Device / Device Operation / HASH Service operation}
+\item \ref{devicenormative:Device Types / Crypto Device / Device Operation / MAC Service operation}
+\item \ref{devicenormative:Device Types / Crypto Device / Device Operation / Symmetric algorithms Operation}
+\item \ref{devicenormative:Device Types / Crypto Device / Device Operation / AEAD Service operation}
+\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] [virtio-dev] [PATCH v18 0/2] virtio-crypto: virtio crypto device specification

[Halil Pasic]

Hi Gonglei!

I've started reviewing but it's going to take some time because
I've missed the previous version (vacation) and v16 was a long
time ago, so I have to re-check and re-learn things.

My first impression is, it reads much better that v16 but still
has some space for improvement.

One idea would be adding some references defining the
crypto services and algorithms. IMHO it would make the
spec a bit rounder, although from pragmatical perspective
I think we can safely assume, that the target audience is able
to figure out the stuff without.

Cheers,
Halil


On 04/22/2017 08:23 AM, Gonglei wrote:
> As a non-native English speaker, I sincerely hope you
> give me more suggestions about English style and/or
> grammar for the spec. Thanks!
> 
> 
> v18 -> v17:
>  - fix many English grammar problems suggested by Stefan, Thanks a lot!
> 
> v17 -> v16:
>  - Some grammar fixes [Stefan, Halil, Michael]
>  - add a section named "Supported crypto services" in order to explain bit
>    numbers and valuse clearly. [Halil, Cornelia]
>  - avoid word reptition [Halil]
>  - rename non-session mode to stateless mode [Halil]
>  - change descriptions for all elements in struct virtio_crypto_config [Halil]
>  - add Halil as a reviewer in the ackonwledgement part, thanks for his work.
>  - other fixes here and there.
> 
> Changes since v15:
>  - use feature bits for non-session mode in order to keep compatibility with
>    pre-existing code. [Halil & Michael]
>  - introduce VIRTIO_CRYPTO_F_ NON_SESSION_MODE feature bit to control all other
>    non-session mode feature bits.
>  - fix some typos. [Stefan]
>  - introduce struct virtio_crypto_op_data_req_mux to support both session
>    and non-session based crypto operations and keep compatibility with
>    pre-existing code.
> 
> Changes since v14:
>  - drop VIRTIO_CRYPTO_S_STARTED status [Halil & Cornelia]
>  - correct a sentence about dataqueue and controlq in the first paragraph. [Halil]
>  - change a MAY to MUST about max_dataqueues. [Halil]
>  - add non-session mode support
>    a) add four features for different crypto services to identify wheather support session mode.
>    b) rewrite some
> 
> For pervious versions of virtio crypto spec, Pls see:
> 
> [v14]:
> https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg02212.html
> 
> [v13]:
> https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg07348.html
> 
> For more information, please see:
>  http://qemu-project.org/Features/VirtioCrypto
> 
> 
> Gonglei (2):
>   virtio-crypto: Add virtio crypto device specification
>   virtio-crypto: Add conformance clauses
> 
>  acknowledgements.tex |    2 +
>  conformance.tex      |   29 ++
>  content.tex          |    2 +
>  virtio-crypto.tex    | 1309 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 1342 insertions(+)
>  create mode 100644 virtio-crypto.tex
> 

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

[Gonglei (Arei)]

Hi Halil,

Nice to see your feedback again :)


> -----Original Message-----
> From: Halil Pasic [mailto:pasic@linux.vnet.ibm.com]
> Sent: Wednesday, May 03, 2017 5:55 AM
> Subject: Re: [virtio-dev] [PATCH v18 0/2] virtio-crypto: virtio crypto device
> specification
> 
> 
> Hi Gonglei!
> 
> I've started reviewing but it's going to take some time because
> I've missed the previous version (vacation) and v16 was a long
> time ago, so I have to re-check and re-learn things.
> 
Yeah, it's a hard job.

> My first impression is, it reads much better that v16 but still
> has some space for improvement.
> 
> One idea would be adding some references defining the
> crypto services and algorithms. IMHO it would make the
> spec a bit rounder, although from pragmatical perspective
> I think we can safely assume, that the target audience is able
> to figure out the stuff without.
> 
I'm afraid there are too many references then because each algorithm
may have a rfc spec document.


Thanks,
-Gonglei


> Cheers,
> Halil
> 
> 
> On 04/22/2017 08:23 AM, Gonglei wrote:
> > As a non-native English speaker, I sincerely hope you
> > give me more suggestions about English style and/or
> > grammar for the spec. Thanks!
> >
> >
> > v18 -> v17:
> >  - fix many English grammar problems suggested by Stefan, Thanks a lot!
> >
> > v17 -> v16:
> >  - Some grammar fixes [Stefan, Halil, Michael]
> >  - add a section named "Supported crypto services" in order to explain bit
> >    numbers and valuse clearly. [Halil, Cornelia]
> >  - avoid word reptition [Halil]
> >  - rename non-session mode to stateless mode [Halil]
> >  - change descriptions for all elements in struct virtio_crypto_config [Halil]
> >  - add Halil as a reviewer in the ackonwledgement part, thanks for his work.
> >  - other fixes here and there.
> >
> > Changes since v15:
> >  - use feature bits for non-session mode in order to keep compatibility with
> >    pre-existing code. [Halil & Michael]
> >  - introduce VIRTIO_CRYPTO_F_ NON_SESSION_MODE feature bit to control
> all other
> >    non-session mode feature bits.
> >  - fix some typos. [Stefan]
> >  - introduce struct virtio_crypto_op_data_req_mux to support both session
> >    and non-session based crypto operations and keep compatibility with
> >    pre-existing code.
> >
> > Changes since v14:
> >  - drop VIRTIO_CRYPTO_S_STARTED status [Halil & Cornelia]
> >  - correct a sentence about dataqueue and controlq in the first paragraph.
> [Halil]
> >  - change a MAY to MUST about max_dataqueues. [Halil]
> >  - add non-session mode support
> >    a) add four features for different crypto services to identify wheather
> support session mode.
> >    b) rewrite some
> >
> > For pervious versions of virtio crypto spec, Pls see:
> >
> > [v14]:
> > https://lists.gnu.org/archive/html/qemu-devel/2016-11/msg02212.html
> >
> > [v13]:
> > https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg07348.html
> >
> > For more information, please see:
> >  http://qemu-project.org/Features/VirtioCrypto
> >
> >
> > Gonglei (2):
> >   virtio-crypto: Add virtio crypto device specification
> >   virtio-crypto: Add conformance clauses
> >
> >  acknowledgements.tex |    2 +
> >  conformance.tex      |   29 ++
> >  content.tex          |    2 +
> >  virtio-crypto.tex    | 1309
> ++++++++++++++++++++++++++++++++++++++++++++++++++
> >  4 files changed, 1342 insertions(+)
> >  create mode 100644 virtio-crypto.tex
> >