> -----Original Message-----
> From: Michael S. Tsirkin [mailto:m...@redhat.com]
> Sent: Friday, September 09, 2016 11:43 AM
> Subject: Re: [PATCH v9 1/2] virtio-crypto: Add virtio crypto device
> specification
>
> On Fri, Sep 09, 2016 at 02:42:41AM +0000, Gonglei (Arei) wrote:
> > Hi Michael,
> >
> >
> > > -----Original Message-----
> > > From: Michael S. Tsirkin [mailto:m...@redhat.com]
> > > Sent: Friday, September 09, 2016 12:44 AM
> > > Subject: Re: [PATCH v9 1/2] virtio-crypto: Add virtio crypto device
> specification
> > >
> > > On Thu, Sep 08, 2016 at 06:05:14PM +0800, Gonglei wrote:
> > > > The virtio crypto device is a virtual crypto device (ie. hardware
> > > > crypto accelerator card). The virtio crypto device can provide
> > > > five crypto services: CIPHER, MAC, HASH, AEAD, KDF, ASYM, PRIMITIVE.
> > > >
> > > > In this patch, CIPHER, MAC, HASH, AEAD services are introduced.
> > > >
> > > > Signed-off-by: Gonglei <arei.gong...@huawei.com>
> > > > CC: Michael S. Tsirkin <m...@redhat.com>
> > > > CC: Cornelia Huck <cornelia.h...@de.ibm.com>
> > > > CC: Stefan Hajnoczi <stefa...@redhat.com>
> > > > CC: Lingli Deng <denglin...@chinamobile.com>
> > > > CC: Jani Kokkonen <jani.kokko...@huawei.com>
> > > > CC: Ola Liljedahl <ola.liljed...@arm.com>
> > > > CC: Varun Sethi <varun.se...@freescale.com>
> > > > CC: Zeng Xin <xin.z...@intel.com>
> > > > CC: Keating Brian <brian.a.keat...@intel.com>
> > > > CC: Ma Liang J <liang.j...@intel.com>
> > > > CC: Griffin John <john.grif...@intel.com>
> > > > CC: Hanweidong <hanweid...@huawei.com>
> > > > CC: Mihai Claudiu Caraman <mike.cara...@nxp.com>
> > >
> > > I mostly looked at the conformance clauses.
> > > Here are some comments worth addressing.
> > >
> > Good, Thanks !
> >
> > > Thanks!
> > >
> > > > ---
> > > > content.tex | 2 +
> > > > virtio-crypto.tex | 926
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > 2 files changed, 928 insertions(+)
> > > > create mode 100644 virtio-crypto.tex
> > > >
> > > > diff --git a/content.tex b/content.tex
> > > > index 4b45678..ab75f78 100644
> > > > --- a/content.tex
> > > > +++ b/content.tex
> > > > @@ -5750,6 +5750,8 @@ descriptor for the \field{sense_len},
> > > \field{residual},
> > > > \field{status_qualifier}, \field{status}, \field{response} and
> > > > \field{sense} fields.
> > > >
> > > > +\input{virtio-crypto.tex}
> > > > +
> > > > \chapter{Reserved Feature Bits}\label{sec:Reserved Feature Bits}
> > > >
> > > > Currently there are three device-independent feature bits defined:
> > > > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > > > new file mode 100644
> > > > index 0000000..eec4741
> > > > --- /dev/null
> > > > +++ b/virtio-crypto.tex
> > > > @@ -0,0 +1,926 @@
> > > > +\section{Crypto Device}\label{sec:Device Types / Crypto Device}
> > > > +
> > > > +The virtio crypto device is a virtual crypto device, and is a kind of
> > > > +virtual hardware accelerator for virtual machines. The encryption and
> > > > +decryption requests are placed in the data queue, and handled by the
> > > > +real crypto accelerators finally. The second queue is the control
> > > > queue,
> > > > +which is used to create or destroy sessions for symmetric algorithms,
> and
> > > > +control some advanced features in the future. The virtio crypto
> > > > +device can provide seven crypto services: CIPHER, MAC, HASH, AEAD,
> > > > +KDF, ASYM, PRIMITIVE.
> > > > +
> > > > +\subsection{Device ID}\label{sec:Device Types / Crypto Device / Device
> ID}
> > > > +
> > > > +20
> > > > +
> > > > +\subsection{Virtqueues}\label{sec:Device Types / Crypto Device /
> > > Virtqueues}
> > > > +
> > > > +\begin{description}
> > > > +\item[0] dataq1
> > > > +\item[\ldots]
> > > > +\item[N-1] dataqN
> > > > +\item[N] controlq
> > > > +\end{description}
> > > > +
> > > > +N is set by \field{max_dataqueues}.
> > > > +
> > > > +\subsection{Feature bits}\label{sec:Device Types / Crypto Device /
> Feature
> > > bits}
> > > > + None currently defined
> > > > +
> > > > +\subsection{Device configuration layout}\label{sec:Device Types /
> > > > Crypto
> > > Device / Device configuration layout}
> > > > +
> > > > +The following driver-read-only configuration fields are currently
> > > > defined.
> > > > +
> > > > +\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 asym_algo;
> > > > + le32 kdf_algo;
> > > > + le32 aead_algo;
> > > > + le32 primitive_algo;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +The first field, \field{status} is currently defined:
> > > VIRTIO_CRYPTO_S_HW_READY
> > > > +and VIRTIO_CRYPTO_S_STARTED.
> > > > +
> > > > +\begin{lstlisting}
> > > > +#define VIRTIO_CRYPTO_S_HW_READY (1 << 0)
> > > > +#define VIRTIO_CRYPTO_S_STARTED (1 << 1)
> > > > +\end{lstlisting}
> > > > +
> > > > +The following driver-read-only field, \field{max_dataqueuess} specifies
> the
> > > > +maximum number of data virtqueues (dataq1\ldots dataqN). The
> > > \field{crypto_services}
> > > > +shows the crypto service the virtio crypto supports. The service
> > > > currently
> > > > +defined are:
> > >
> > > I'm not a native english speaker myself but I can tell there are some
> > > mistakes in english in this text. Could you pls get a native speaker go
> > > over the text for you? We'll likely get it corrected during public
> > > review anyway, but it's better to fix them early.
> > >
> > Yes, you are right. I'll do this thing before next version's publication,
> > hope it's
> not too late. :)
> > >
> > > > +
> > > > +\begin{lstlisting}
> > > > +#define VIRTIO_CRYPTO_SERVICE_CIPHER (0) /* cipher service */
> > > > +#define VIRTIO_CRYPTO_SERVICE_HASH (1) /* hash service */
> > >
> > > You write cipher and hash here, but elsewhere in text you
> > > refer to them as CIPHER and HASH.
> > >
> > > > +#define VIRTIO_CRYPTO_SERVICE_MAC (2) /* MAC (Message
> > > Authentication Codes) service */
> > > > +#define VIRTIO_CRYPTO_SERVICE_AEAD (3) /* AEAD (Authenticated
> > > Encryption with Associated Data) service */
> > > > +\end{lstlisting}
> > > > +
> > > > +The last driver-read-only fields specify detailed algorithms masks
> > > > which
> > > > +the device offers for corresponding services. The below CIPHER
> algorithms
> > > > +are defined currently:
> > >
> > > ... are currently defined
> > > Similarly "finally" etc elsewhere.
> > > Or better just drop "currently", it adds no value, here and
> > > elsewhere.
> > >
> > OK, drop it.
> >
> > >
> > > > +
> > > > +\begin{lstlisting}
> > > > +#define VIRTIO_CRYPTO_NO_CIPHER 0
> > > > +#define VIRTIO_CRYPTO_CIPHER_ARC4 1
> > > > +#define VIRTIO_CRYPTO_CIPHER_AES_ECB 2
> > > > +#define VIRTIO_CRYPTO_CIPHER_AES_CBC 3
> > > > +#define VIRTIO_CRYPTO_CIPHER_AES_CTR 4
> > > > +#define VIRTIO_CRYPTO_CIPHER_DES_ECB 5
> > > > +#define VIRTIO_CRYPTO_CIPHER_DES_CBC 6
> > > > +#define VIRTIO_CRYPTO_CIPHER_3DES_ECB 7
> > > > +#define VIRTIO_CRYPTO_CIPHER_3DES_CBC 8
> > > > +#define VIRTIO_CRYPTO_CIPHER_3DES_CTR 9
> > > > +#define VIRTIO_CRYPTO_CIPHER_KASUMI_F8 10
> > > > +#define VIRTIO_CRYPTO_CIPHER_SNOW3G_UEA2 11
> > > > +#define VIRTIO_CRYPTO_CIPHER_AES_F8 12
> > > > +#define VIRTIO_CRYPTO_CIPHER_AES_XTS 13
> > > > +#define VIRTIO_CRYPTO_CIPHER_ZUC_EEA3 14
> > > > +\end{lstlisting}
> > > > +
> > > > +The below HASH algorithms are defined currently:
> > > > +
> > > > +\begin{lstlisting}
> > > > +#define VIRTIO_CRYPTO_NO_HASH 0
> > > > +#define VIRTIO_CRYPTO_HASH_MD5 1
> > > > +#define VIRTIO_CRYPTO_HASH_SHA1 2
> > > > +#define VIRTIO_CRYPTO_HASH_SHA_224 3
> > > > +#define VIRTIO_CRYPTO_HASH_SHA_256 4
> > > > +#define VIRTIO_CRYPTO_HASH_SHA_384 5
> > > > +#define VIRTIO_CRYPTO_HASH_SHA_512 6
> > > > +#define VIRTIO_CRYPTO_HASH_SHA3_224 7
> > > > +#define VIRTIO_CRYPTO_HASH_SHA3_256 8
> > > > +#define VIRTIO_CRYPTO_HASH_SHA3_384 9
> > > > +#define VIRTIO_CRYPTO_HASH_SHA3_512 10
> > > > +#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE128 11
> > > > +#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE256 12
> > > > +\end{lstlisting}
> > > > +
> > > > +The below MAC algorithms are defined currently:
> > > > +
> > > > +\begin{lstlisting}
> > > > +#define VIRTIO_CRYPTO_NO_MAC 0
> > > > +#define VIRTIO_CRYPTO_MAC_HMAC_MD5 1
> > > > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA1 2
> > > > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_224 3
> > > > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_256 4
> > > > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_384 5
> > > > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_512 6
> > > > +#define VIRTIO_CRYPTO_MAC_CMAC_3DES 25
> > > > +#define VIRTIO_CRYPTO_MAC_CMAC_AES 26
> > > > +#define VIRTIO_CRYPTO_MAC_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 below AEAD algorithms are defined currently:
> > > > +
> > > > +\begin{lstlisting}
> > > > +#define VIRTIO_CRYPTO_NO_AEAD 0
> > > > +#define VIRTIO_CRYPTO_AEAD_GCM 1
> > > > +#define VIRTIO_CRYPTO_AEAD_CCM 2
> > > > +#define VIRTIO_CRYPTO_AEAD_CHACHA20_POLY1305 3
> > > > +\end{lstlisting}
> > >
> > > Maybe clarify that more will be defined in the future,
> > OK.
> >
> > > and that drivers are expected to ignore the algorithms they
> > > do not know how to handle.
> > >
> > We can add this in the driver requirement section.
> >
> > > > +
> > > > +\devicenormative{\subsection}{Device Requirements: Device
> configuration
> > > layout}\label{sec:Device Types / Crypto Device / Device configuration
> > > layout
> /
> > > Device Requirements: Device configuration layout}
> > > > +
> > > > +\begin{itemize*}
> > > > +\item The device MUST set \field{max_dataqueues} to between 1 and
> 65535
> > > inclusive.
> > > > +\item The device SHOULD set \field{status} according to the status of
> > > > the
> > > hardware-backed implementation.
> > >
> > > Is the requirement here to be able to handle some activity
> > > once ready is set? which exactly?
> > >
> > I mean that the 'ready' status shows the device is active,
> > otherwise the driver assumes it's not active.
>
> Yes but what does active mean?
> What should it be able to do as opposed to when it is
> inactive? you should explain that.
>
OK, I get your point. :)
> > > > +\item The device MUST set \field{crypto_services} according to the
> crypto
> > > services which the device offered.
> > >
> > > Why offered in the past? Generally please try to use the simple tense as
> > > much as possible.
> > >
> > OK.
> >
> > > > +\item The device MUST set detailed algorithms mask according to
> > > \field{crypto_services} field.
> > >
> > > and what does this mean?
> > >
> > The device MUST set corresponding algorithms masks according to
> > \field{crypto_services} field.
> >
> > > > +\end{itemize*}
> > > > +
> > > > +\drivernormative{\subsection}{Driver Requirements: Device
> configuration
> > > layout}\label{sec:Device Types / Crypto Device / Device configuration
> > > layout
> /
> > > Driver Requirements: Device configuration layout}
> > > > +
> > > > +\begin{itemize*}
> > > > +\item The driver MUST read the ready \field{status} from the bottom bit
> of
> > > status to
> > > > + check whether the hardware-backed implementation is ready or
> not.
> > >
> > > And then what? What is and what is not legal to do before ready is set?
> > > Is it legal to access any other fields before ready is set?
> > >
> > I mean that the 'ready' status shows the device is active,
> > otherwise the driver assumes it's not active. But the driver can also
> > read any other fields though ready is not set.
>
> So it can read any field but must not submit any requests
> on data or control queue? Is that it?
>
Yes, it is. I'll add the explanation.
>
> > > Also, does ready get cleared on reset? Does driver have to
> > > re-read it after reset?
> > >
> > Yes, I think so.
>
> Spec should probably say this.
>
OK, will add.
> > > > +\item The driver MAY read \field{max_dataqueues} field to discover how
> > > many data queues the device supports.
> > >
> > > looks more like a device requirement to me
> > >
> > Yes. Drop it here.
> >
> > > > +\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 \field{algorithms} field
> according to
> > > \field{crypto_services} field.
> > >
> > > Is the requirement that drivers MUST ignore
> > > algorithms that they do not know how to handle?
> > > If yes say so.
> > >
> > Yes.
> >
> > >
> > > > +\end{itemize*}
> > > > +
> > > > +\subsection{Device Initialization}\label{sec:Device Types / Crypto
> > > > Device
> /
> > > Device Initialization}
> > > > +
> > > > +\subsubsection{Driver Requirements: Device
> Initialization}\label{sec:Device
> > > Types / Crypto Device / Device Initialization / Driver Requirements:
> > > Device
> > > Initialization}
> > > > +
> > > > +\begin{itemize*}
> > > > +\item The driver MUST identify and initialize up to
> \field{max_dataqueues}
> > > data virtqueues.
> > > > +\item The driver MUST identify the control virtqueue.
> > >
> > > identify and then what? is it required to initialize it?
> > >
> > Sure, the driver must initialize the control virtqueue. I'll add the
> > "initialize"
> word.
> >
> > > > +\item The driver MUST identify the ready status of hardware-backend
> from
> > > \field{status} field.
> > >
> > > How is this different from requirement above?
> > >
> > No, drop it here.
> >
> > > > +\item The driver MUST read the supported crypto services from bits of
> > > \field{crypto_servies}.
> > >
> > > Do we really need it to read them? When?
> > > Probably the real requirement is not to use any not listed in
> > > crypto_services?
> > >
> > Yes. My original thought is the driver need to read the crypto_services
> > field,
> then
> > it can know what the device supply, then it can do register the crypto
> services/algos to
> > kernel space or user space cryptographic framework.
> >
> > > > +\item The driver MUST read the supported algorithms according to
> > > \field{crypto_services} field.
> > >
> > > Same question.
> > >
> > > > +\end{itemize*}
> > > > +
> > > > +\subsubsection{Device Requirements: Device
> Initialization}\label{sec:Device
> > > Types / Crypto Device / Device Initialization / Device Requirements:
> > > Device
> > > Initialization}
> > > > +
> > > > +\begin{itemize*}
> > > > +\item The device MUST be configured at least one accelerator which
> > > executes real crypto operations.
> > > > +\item The device MUST write the \field{crypto_services} field according
> to
> > > the capacities of the backend accelerator.
> > > > +\end{itemize*}
> > > > +
> > > > +\subsection{Device Operation}\label{sec:Device Types / Crypto Device /
> > > Device Operation}
> > > > +
> > > > +Packets can be transmitted by placing them in both the controlq and
> dataq.
> > > > +Packets consist of a generic header and a service-specific request.
> > > > +Where 'general header' is for all crypto requests, 'service specific
> requests'
> > > > +are composed of operation parameter + output data + input data in
> general.
> > > > +Operation parameters are algorithm-specific parameters, output data is
> the
> > > > +data should be operated, input data is the "operation result + result
> buffer".
> > > > +The general header of controlq:
> > > > +
> > > > +\begin{lstlisting}
> > > > +#define VIRTIO_CRYPTO_OPCODE(service, op) ((service << 8) | (op))
> > > > +
> > > > +struct virtio_crypto_ctrl_header {
> > > > +#define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION \
> > > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER,
> > > 0x02)
> > > > +#define VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION \
> > > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER,
> > > 0x03)
> > > > +#define VIRTIO_CRYPTO_HASH_CREATE_SESSION \
> > > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH,
> 0x02)
> > > > +#define VIRTIO_CRYPTO_HASH_DESTROY_SESSION \
> > > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH,
> 0x03)
> > > > +#define VIRTIO_CRYPTO_MAC_CREATE_SESSION \
> > > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC,
> 0x02)
> > > > +#define VIRTIO_CRYPTO_MAC_DESTROY_SESSION \
> > > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC,
> 0x03)
> > > > +#define VIRTIO_CRYPTO_AEAD_CREATE_SESSION \
> > > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD,
> 0x02)
> > > > +#define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \
> > > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD,
> 0x03)
> > > > + __virtio32 opcode;
> > > > + __virtio32 algo;
> > > > + __virtio32 flag;
> > > > + /* data virtqueue id */
> > > > + __virtio32 queue_id;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +The general header of dataq:
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_crypto_op_header {
> > > > +#define VIRTIO_CRYPTO_CIPHER_ENCRYPT \
> > > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER,
> 0x00)
> > > > +#define VIRTIO_CRYPTO_CIPHER_DECRYPT \
> > > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER,
> 0x01)
> > > > +#define VIRTIO_CRYPTO_HASH \
> > > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x00)
> > > > +#define VIRTIO_CRYPTO_MAC \
> > > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x00)
> > > > +#define VIRTIO_CRYPTO_AEAD_ENCRYPT \
> > > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x00)
> > > > +#define VIRTIO_CRYPTO_AEAD_DECRYPT \
> > > > + VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x01)
> > > > + __virtio32 opcode;
> > > > + /* algo should be service-specific algorithms */
> > > > + __virtio32 algo;
> > > > + /* session_id should be service-specific algorithms */
> > > > + __virtio64 session_id;
> > > > + /* control flag to control the request */
> > > > + __virtio32 flag;
> > > > + __virtio32 padding;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +The device CAN set the status of operation as follows:
> > >
> > > Please do not capitalize CAN - I don't think CAN is an RFC 2119 keyword.
> > > Also FYI text using RFC 2119 keywords should go into conformance
> statements,
> > > other text should avoid them to avoid confusion.
> > >
> > OK, thanks for your information. :)
> >
> > >
> > > > VIRTIO_CRYPTO_OP_OK for success,
> > > > +VIRTIO_CRYPTO_OP_ERR for failure or device error,
> > > VIRTIO_CRYPTO_OP_NOTSUPP for not support,
> > > > +VIRTIO_CRYPTO_OP_INVSESS for invalid session id when executing
> crypto
> > > operations.
> > > > +
> > > > +\begin{lstlisting}
> > > > +#define VIRTIO_CRYPTO_OP_OK 0
> > > > +#define VIRTIO_CRYPTO_OP_ERR 1
> > > > +#define VIRTIO_CRYPTO_OP_BADMSG 2
> > > > +#define VIRTIO_CRYPTO_OP_NOTSUPP 3
> > > > +#define VIRTIO_CRYPTO_OP_INVSESS 4
> > > > +\end{lstlisting}
> > > > +
> > > > +\subsubsection{Control Virtqueue}\label{sec:Device Types / Crypto
> Device /
> > > Device Operation / Control Virtqueue}
> > > > +
> > > > +The driver uses the control virtqueue to send control commands to the
> > > > +device which handles the non-data plane operations, such as session
> > > > +operations (See \ref{sec:Device Types / Crypto Device / Device
> > > > Operation
> /
> > > Control Virtqueue / Session operation}).
> > > > +The packet of controlq:
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_crypto_op_ctrl_req {
> > > > + struct virtio_crypto_ctrl_header header;
> > > > +
> > > > + union {
> > > > + struct virtio_crypto_sym_create_session_req
> > > sym_create_session;
> > > > + struct virtio_crypto_hash_create_session_req
> > > hash_create_session;
> > > > + struct virtio_crypto_mac_create_session_req
> > > mac_create_session;
> > > > + struct virtio_crypto_aead_create_session_req
> > > aead_create_session;
> > > > + struct virtio_crypto_destroy_session_req
> destroy_session;
> > > > + } u;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +The header is the general header, the union is an algorithm-specific
> > > > type,
> > > > +which is set by the driver. All the properties in the union will be
> > > > shown as
> > > follows.
> > > > +
> > > > +\paragraph{Session operation}\label{sec:Device Types / Crypto Device /
> > > Device Operation / Control Virtqueue / Session operation}
> > > > +
> > > > +The symmetric algorithms have the concept of sessions. A session is a
> > > > +handle which describes the cryptographic parameters to be applied to
> > > > +a number of buffers. The data within a session handle includes the
> following:
> > > > +
> > > > +\begin{enumerate}
> > > > +\item The operation (cipher, hash/mac or both, and if both, the order
> > > > in
> > > > + which the algorithms should be applied).
> > > > +\item The cipher set data, including the cipher algorithm and mode,
> > > > + the key and its length, and the direction (encrypt or decrypt).
> > > > +\item The hash/mac setup data, including the hash algorithm or mac
> > > algorithm,
> > > > + and digest result length (to allow for truncation).
> > > > +\begin{itemize*}
> > > > +\item Authenticated mode can refer to MAC, which requires that the key
> > > and
> > > > + its length are also specified.
> > > > +\item For nested mode, the inner and outer prefix data and length are
> > > specified,
> > > > + as well as the outer hash algorithm.
> > > > +\end{itemize*}
> > > > +\end{enumerate}
> > > > +
> > > > +The below structure store the result of session creation which is set
> > > > by
> the
> > > device:
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_crypto_session_input {
> > > > + /* Device-writable part */
> > > > + __virtio64 session_id;
> > > > + __virtio32 status;
> > > > + __virtio32 padding;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +A request which destroy a session including the following information:
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_crypto_destroy_session_req {
> > > > + /* Device-readable part */
> > > > + __virtio64 session_id;
> > > > + /* Device-writable part */
> > > > + __virtio32 status;
> > > > + __virtio32 padding;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +\subparagraph{Session operation: HASH session}\label{sec:Device Types
> /
> > > Crypto Device / Device
> > > > +Operation / Control Virtqueue / Session operation / Session operation:
> HASH
> > > session}
> > > > +
> > > > +The packet of HASH session is:
> > > > +
> > > > +\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}
> > > > +
> > > > +The packet of MAC session is:
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_crypto_mac_session_para {
> > > > + /* See VIRTIO_CRYPTO_MAC_* above */
> > > > + le32 algo;
> > > > + /* hash result length */
> > > > + le32 hash_result_len;
> > > > + /* length of authenticated key */
> > > > + le32 auth_key_len;
> > > > + __virtio32 padding;
> > > > +};
> > > > +struct virtio_crypto_mac_session_output {
> > > > + __virtio64 auth_key_addr; /* guest key physical address */
> > > > +};
> > > > +
> > > > +struct virtio_crypto_mac_create_session_req {
> > > > + /* Device-readable part */
> > > > + struct virtio_crypto_mac_session_para para;
> > > > + struct virtio_crypto_mac_session_output out;
> > > > + /* 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). The packet of CIPHER
> session is:
> > > > +
> > > > +\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
> > > > + /* encrypt or decrypt */
> > > > + le32 op;
> > > > + le32 padding;
> > > > +};
> > > > +
> > > > +struct virtio_crypto_cipher_session_output {
> > > > + __virtio64 key_addr; /* guest key physical address */
> > > > +};
> > > > +
> > > > +struct virtio_crypto_cipher_session_req {
> > > > + struct virtio_crypto_cipher_session_para para;
> > > > + struct virtio_crypto_cipher_session_output out;
> > > > + struct virtio_crypto_session_input input;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +The packet of algorithm chaining is:
> > > > +
> > > > +\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
> > > > + __virtio32 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
> > > > + __virtio32 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 */
> > > > + __virtio32 aad_len;
> > > > + __virtio32 padding;
> > > > +};
> > > > +
> > > > +struct virtio_crypto_alg_chain_session_output {
> > > > + struct virtio_crypto_cipher_session_output cipher;
> > > > + struct virtio_crypto_mac_session_output mac;
> > > > +};
> > > > +
> > > > +struct virtio_crypto_alg_chain_session_req {
> > > > + struct virtio_crypto_alg_chain_session_para para;
> > > > + struct virtio_crypto_alg_chain_session_output out;
> > > > + struct virtio_crypto_session_input input;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +The packet of symmetric algorithm is:
> > > > +
> > > > +\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
> > > > + __virtio32 op_type;
> > > > + __virtio32 padding;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +\subparagraph{Session operation: AEAD session}\label{sec:Device Types
> /
> > > Crypto Device / Device
> > > > +Operation / Control Virtqueue / Session operation / Session operation:
> AEAD
> > > session}
> > > > +
> > > > +The packet of AEAD session is:
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_crypto_aead_session_para {
> > > > + /* See VIRTIO_CRYPTO_AEAD_* above*/
> > > > + le32 algo;
> > > > + /* length of key */
> > > > + le32 key_len;
> > > > + /* digest result length */
> > > > + le32 digest_result_len;
> > > > + /* The length of the additional authenticated data (AAD) in bytes
> > > > */
> > > > + le32 aad_len;
> > > > + /* encrypt or decrypt, See above VIRTIO_CRYPTO_OP_* */
> > > > + le32 op;
> > > > + le32 padding;
> > > > +};
> > > > +
> > > > +struct virtio_crypto_aead_session_output {
> > > > + __virtio64 key_addr; /* guest key phycial address */
> > > > +};
> > > > +
> > > > +struct virtio_crypto_aead_create_session_req {
> > > > + struct virtio_crypto_aead_session_para para;
> > > > + struct virtio_crypto_aead_session_output out;
> > > > + struct virtio_crypto_session_input input;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +\drivernormative{\subparagraph}{Session operation: create
> > > session}\label{sec:Device Types / Crypto Device / Device
> > > > +Operation / Control Virtqueue / Session operation / Session operation:
> > > create session / Driver Requirements: Session operation: create session}
> > > > +
> > > > +The driver MUST set the control general header and corresponding
> property
> > > > +of union in structure virtio_crypto_op_ctrl_req. See \ref{sec:Device
> Types /
> > > Crypto Device / Device Operation / Control Virtqueue}.
> > > > +Take the example of MAC service, the driver MUST set
> > > VIRTIO_CRYPTO_MAC_CREATE_SESSION
> > > > +for \field{opcode} field in struct virtio_crypto_op_ctrl_req, and set
> > > > the
> > > \field{queue_id}
> > > > +to show dataq used.
> > > > +
> > > > +\devicenormative{\subparagraph}{Session operation: create
> > > session}\label{sec:Device Types / Crypto Device / Device
> > > > +Operation / Control Virtqueue / Session operation / Session operation:
> > > create session / Device Requirements: Session operation: create session}
> > > > +
> > > > +The device MUST return a session identifier to the driver when the
> device
> > > > +finishes the processing of session creation. The session creation
> > > > request
> > > > +MUST end by a \field{status} field and a \field{session_id} field:
> > >
> > > Pls reword to say "device MUST ..."
> > >
> > OK.
> >
> > > > +
> > > > +Both
> > >
> > > should be lower case both after :
> > >
> > > >\field{status} and \field{session_id} are written by the device: either
> > > VIRTIO_CRYPTO_OP_OK for success,
> > > > +VIRTIO_CRYPTO_OP_ERR for creation failed or device error,
> > > VIRTIO_CRYPTO_OP_NOTSUPP for not support,
> > > > +VIRTIO_CRYPTO_OP_INVSESS for invalid session id when executing
> crypto
> > > operations.
> > > > +
> > > > +\drivernormative{\subparagraph}{Session operation: destroy
> > > session}\label{sec:Device Types / Crypto Device / Device
> > > > +Operation / Control Virtqueue / Session operation / Session operation:
> > > destroy session / Driver Requirements: Session operation: destroy session}
> > > > +
> > > > +The driver MUST set the control general header and corresponding
> property
> > > > +of union in struct virtio_crypto_op_ctrl_req. See \ref{sec:Device
> > > > Types /
> > > Crypto Device / Device Operation / Control Virtqueue}.
> > > > +Take the example of MAC service,
> > >
> > > This isn't the place for examples. Either list all requirements or drop
> > > this.
> > >
> > Can I use " \begin{note} ... \end{note}" for this example at the end of
> > section?
> > Just like virtio-net Device Initialization section.
>
> You can but I think it's better to add examples elsewhere, outside
> conformance sections. There might be a bit of text duplication.
>
OK.
> > > > the driver MUST set VIRTIO_CRYPTO_MAC_DESTROY_SESSION
> > > > +for \field{opcode} field in struct virtio_crypto_op_ctrl_req, and set
> > > > the
> > > \field{queue_id} shows dataq used.
> > > > +The driver MUST set the \field{session_id} which MUST be a valid value
> > > which assigned by the
> > > > +device when a session was created.
> > >
> > > Two MUST in a single statement is one too many.
> > > Something like
> > > The driver MUST set the \field{session_id} to a value assigned by the
> > > device at session creation.
> > > ?
> > >
> > Good!
> >
> > > > +
> > > > +\devicenormative{\subparagraph}{Session operation: destroy
> > > session}\label{sec:Device Types / Crypto Device / Device
> > > > +Operation / Control Virtqueue / Session operation / Session operation:
> > > destroy session / Device Requirements: Session operation: destroy session}
> > > > +
> > > > +\field{status} field is written by the device: either
> VIRTIO_CRYPTO_OP_OK
> > > for success, VIRTIO_CRYPTO_OP_ERR for failure or device error.
> > > > +
> > > > +\subsubsection{Data Virtqueue}\label{sec:Device Types / Crypto Device /
> > > Device Operation / Data Virtqueue}
> > > > +
> > > > +The driver uses the data virtqueue to transmit the requests of crypto
> > > operation to the device,
> > > > +and to finish the data plane operations (such as crypto operation).
> > > > +
> > > > +The packet of dataq:
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_crypto_op_data_req {
> > > > + struct virtio_crypto_op_header header;
> > > > +
> > > > + union {
> > > > + struct virtio_crypto_sym_data_req sym_req;
> > > > + struct virtio_crypto_hash_data_req hash_req;
> > > > + struct virtio_crypto_mac_data_req mac_req;
> > > > + struct virtio_crypto_aead_data_req aead_req;
> > > > + } u;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +The header is the general header, the union is algorithm-specific type,
> > > > +which are set by the driver. All properties in the union will be shown
> > > > as
> > > follow.
> > > > +
> > > > +There is a unify idata structure for all symmetric algorithms,
> > > > including
> > > CIPHER, HASH, MAC, AEAD.
> > > > +The structure is defined as:
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_crypto_sym_input {
> > > > + /* Destination data guest address, it's useless for plain HASH and
> MAC
> > > */
> > > > + __virtio64 dst_data_addr;
> > > > + /* Digest result guest address, it's useless for plain cipher
> > > > algos */
> > > > + __virtio64 digest_result_addr;
> > > > +
> > > > + __virtio32 status;
> > > > + __virtio32 padding;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +\paragraph{HASH Service Operation}\label{sec:Device Types / Crypto
> Device
> > > / Device Operation / Data Virtqueue / HASH Service Operation}
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_crypto_hash_input {
> > > > + struct virtio_crypto_sym_input input;
> > > > +};
> > > > +
> > > > +struct virtio_crypto_hash_output {
> > > > + /* source data guest address */
> > > > + __virtio64 src_data_addr;
> > > > + /* length of source data */
> > > > + __virtio32 src_data_len;
> > > > + __virtio32 padding;
> > > > +};
> > > > +
> > > > +struct virtio_crypto_hash_data_req {
> > > > + /* Device-readable part */
> > > > + struct virtio_crypto_hash_output odata;
> > > > + /* Device-writable part */
> > > > + struct virtio_crypto_hash_input idata;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +Each data request uses virtio_crypto_hash_data_req structure to store
> > > informations,
> > > > +which are used to execute one HASH operation. The request only
> occupies
> > > one entry
> > > > +of the Vring descriptor table in virtio crypto device's dataq, which
> improves
> > > > +the throughput of data transferring for HASH service, so that the
> > > > virtio
> > > crypto
> > > > +device CAN get the better result of acceleration.
> > > > +
> > > > +The informations include the source data guest physical address stored
> by
> > > \field{src_data_addr},
> > > > +length of source data stored by \field{src_data_len}, digest result
> > > > guest
> > > physical address
> > > > +stored by \field{digest_result_addr} which is used to save the result
> > > > of
> HASH
> > > operation.
> > > > +The address and length CAN determine the specific content in the guest
> > > memory.
> > > > +
> > > > +Note: The guest memory MUST be guaranteed to be allocated and
> > > physically-contiguous
> > > > +pointed by \field{digest_result_addr} in struct
> > > > virtio_crypto_hash_input
> and
> > > > +\field{src_data_addr} in struct virtio_crypto_hash_output.
> > > > +
> > > > +\drivernormative{\subparagraph}{HASH Service
> Operation}\label{sec:Device
> > > Types / Crypto Device / Device
> > > > +Operation / Data Virtqueue / HASH Service Operation / Driver
> Requirements:
> > > HASH Service Operation}
> > > > +
> > > > +The driver MUST set the \field{opcode}, \field{session_id} in struct
> > > virtio_crypto_op_header.
> > > > +The \field{opcode} is set to VIRTIO_CRYPTO_HASH, \field{session_id}
> MUST
> > > be a valid value
> > > > +which assigned by the device when a session was created.
> > >
> > > Why repeat this for each opcode/session_id?
> > >
> > Er.. It seems superfluous, drop it. :(
> >
> > > > +The driver SHOULD set the \field{queue_id} field to show dataq used in
> > > struct virtio_crypto_op_header.
> > >
> > > Do things still work if it doesn't?
> > >
> > No, it doesn't. Because the device will read this filed to decide which data
> virtqueue
> > (connected cryptodev client) will be used latter. Shall we need to change
> SHOULD to MUST?
>
> If it's required, make it MUST.
>
OK.
> > > > +The driver MUST fill out all fields in struct
> > > > virtio_crypto_hash_data_req,
> > > including \field{parameter},
> > > > +\field{odata} and \field{idata} sub structures.
> > > > +
> > > > +\devicenormative{\subparagraph}{HASH Service
> Operation}\label{sec:Device
> > > Types / Crypto Device / Device
> > > > +Operation / Data Virtqueue / HASH Service Operation / Device
> > > Requirements: HASH Service Operation}
> > > > +
> > > > +The device MUST copy the result of hash operation recorded by
> > > \field{digest_result_addr}
> > > > +filed in struct virtio_crypto_hash_input.
> > > > +The device MUST set the \field{status} in strut
> > > > virtio_crypto_hash_input:
> > > either VIRTIO_CRYPTO_OP_OK for success,
> > > > +VIRTIO_CRYPTO_OP_ERR for creation failed or device error,
> > > VIRTIO_CRYPTO_OP_NOTSUPP for not support.
> > > > +
> > > > +\paragraph{MAC Service Operation}\label{sec:Device Types / Crypto
> Device
> > > / Device Operation / Data Virtqueue / MAC Service Operation}
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_crypto_mac_input {
> > > > + struct virtio_crypto_sym_input input;
> > > > +};
> > > > +
> > > > +struct virtio_crypto_mac_output {
> > > > + struct virtio_crypto_hash_output hash_output;
> > > > +};
> > > > +
> > > > +struct virtio_crypto_mac_data_req {
> > > > + /* Device-readable part */
> > > > + struct virtio_crypto_mac_output odata;
> > > > + /* Device-writable part */
> > > > + struct virtio_crypto_mac_input idata;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +Each data request uses virtio_crypto_mac_data_req structure to store
> > > informations,
> > > > +which are used to execute one MAC operation. The request only
> occupies
> > > one entry
> > > > +of the Vring descriptor table in virtio crypto device's dataq, which
> improves
> > > > +the throughput of data transferring for MAC service, so that the virtio
> > > crypto
> > > > +device CAN get the better result of acceleration.
> > > > +
> > > > +The informations include the source data guest physical address stored
> by
> > > \field{src_data_addr},
> > > > +length of source data stored by \field{src_data_len}, digest result
> > > > guest
> > > physical address
> > > > +stored by \field{digest_result_addr} which is used to save the result
> > > > of
> MAC
> > > operation.
> > > > +The address and length CAN determine the specific content in the guest
> > > memory.
> > >
> > > don't upper-case please
> > >
> > OK.
> >
> > > > +
> > > > +Note: The guest memory MUST be guaranteed
> > >
> > > Pls put statemenets with RFC keywords, including MUST
> > > in a conformance statement section.
> > > Or reword to avoid this:
> > >
> > > The guest memory is always guaranteed to be allocated and
> > > physically-contiguous
> > >
> > OK, thanks.
> >
> > >
> > > >to be allocated and physically-contiguous
> > > > +pointed by \field{digest_result_addr} in struct
> > > > virtio_crypto_hash_input
> and
> > > > +\field{src_data_addr} in struct virtio_crypto_hash_output.
> > > > +
> > > > +\drivernormative{\subparagraph}{MAC Service
> Operation}\label{sec:Device
> > > Types / Crypto Device / Device
> > > > +Operation / Data Virtqueue / MAC Service Operation / Driver
> Requirements:
> > > MAC Service Operation}
> > > > +
> > > > +Refer to the hash operation.
> > >
> > > what does this mean? That same rules apply to all operations? Better just
> > > combine them in
> > > one section then, and list the minor differences.
> > >
> > OK, I'll combine them in the following version.
> >
> > Regards,
> > -Gonglei
