On Thu, Jul 28, 2016 at 05:28:33AM +0000, Zeng, Xin wrote:
> On Thursday, July 28, 2016 10:51 AM Gonglei (Arei) Wrote:
> > > > Changes from v4:
> > > > - introduce crypto services into virtio crypto device. The services
> > > > currently defined are CIPHER, MAC, HASH, AEAD, KDF, ASYM,
> > > PRIMITIVE.
> > > > - define a unified crypto request format that is consisted of
> > > > general header + service specific request, Where 'general header'
> > > > is for
> > > all
> > > > crypto request, 'service specific request' is composed of
> > > > operation parameter + input data + output data in generally.
> > > > operation parameter is algorithm-specific parameters,
> > > > input data is the data should be operated ,
> > > > output data is the "operation result + result buffer".
> > > > - redefine the algorithms and structure based on above crypto services.
> > > > - rearrange the title and subtitle
> > > > - Only support CIPHER, MAC, HASH and AEAD crypto services, and Xin will
> > > > focus KDF, ASYM and PRIMITIVE services.
> > > > - Some other corresponding fixes.
> > > > - Make a formal patch using tex type.
> > > >
> > > > Changes from v3:
> > > > - Don't use enum is the spec but macros in specific structures.
> > > > [Michael &
> > > Stefan]
> > > > - Add two complete structures for session creation and closing, so that
> > > > the spec is clear on how to lay out the request. [Stefan]
> > > > - Definite the crypto operation request with assigned structure, in
> > > > this
> > way,
> > > > each data request only occupies *one entry* of the Vring descriptor
> > table,
> > > > which *improves* the *throughput* of data transferring.
> > > >
> > > > Changes from v2:
> > > > - Reserve virtio device ID 20 for crypto device. [Cornelia]
> > > > - Drop all feature bits, those capabilities are offered by the device
> > > > all the
> > > time. [Stefan & Cornelia]
> > > > - Add a new section 1.4.2 for driver requirements. [Stefan]
> > > > - Use definite type definition instead of enum type in some structure.
> > > [Stefan]
> > > > - Add virtio_crypto_cipher_alg definition. [Stefan]
> > > > - Add a "Device requirements" section as using MUST. [Stefan]
> > > > - Some grammar nits fixes and typo fixes. [Stefan & Cornelia]
> > > > - Add one VIRTIO_CRYPTO_S_STARTED status for the driver as the flag of
> > > virtio-crypto device started and can work now.
> > > >
> > > > Great thanks for Stefan and Cornelia!
> > > >
> > > > Changes from v1:
> > > > - Drop the feature bit definition for each algorithm, and using config
> > space
> > > instead [Cornelia]
> > > > - Add multiqueue support and add corresponding feature bit
> > > > - Update Encryption process and header definition
> > > > - Add session operation process and add corresponding header
> > description
> > > > - Other better description in order to fit for virtio spec [Michael]
> > > > - Some other trivial fixes.
> > >
> > > OK I will let people who understand crypto comment on this.
> >
> > Excellently, thanks!
> >
> > > Down the road before we release this we'll need to link confirmance
> > clauses
> > > from confirmance section. Can be a patch on top, no big deal.
> > >
> >
> > Sorry, where is the confirmance section and what's confirmance clauses?
I meant conformance :) The stuff in conformance.tex
> >
> > >
> > > > ---
> > > > content.tex | 2 +
> > > > virtio-crypto.tex | 792
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > 2 files changed, 794 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:
> > >
> > > Generally we keep it all in a single file. But ok, let's
> > > experiment with this split layout and see whether it
> > > makes things easier or harder. We can always squash it later.
> > >
> > > > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > > > new file mode 100644
> > > > index 0000000..0918f36
> > > > --- /dev/null
> > > > +++ b/virtio-crypto.tex
> > > > @@ -0,0 +1,792 @@
> > > > +\section{Crypto Device}\label{sec:Device Types / Crypto Device}
> > > > +
> > > > +The virtio crypto device is a virtual crypto device (ie. hardware
> > > > +crypto accelerator card). The encryption and decryption requests of
> > > > +are placed in the data queue, and handled by the real hardware crypto
> > > > +accelerators finally. The second queue is the control queue, which
> > > > +is used to create or destroy session for symmetric algorithms, and
> > > > +to control some advanced features in the future. The virtio crypto
> > > > +device can provide seven crypto services: CIPHER, MAC, HASH, AEAD,
> > > > +KDF, ASYM, PRIMITIVE.
> > > > +
> > > > +\subsection{Device ID}\label{sec:Device Types / Crypto Device / Device
> > ID}
> > > > +
> > > > +20
> > > > +
> > > > +\subsection{Virtqueues}\label{sec:Device Types / Crypto Device /
> > > Virtqueues}
> > > > +
> > > > +\begin{description}
> > > > +\item[0] dataq1
> > > > +\item[\ldots]
> > > > +\item[N-1] dataqN
> > > > +\item[N] controlq
> > > > +\end{description}
> > > > +
> > > > +N is set by \field{max_dataqueues} (\field{max_dataqueues} >= 1).
> > > > +
> > > > +\subsection{Feature bits}\label{sec:Device Types / Crypto Device /
> > Feature
> > > bits}
> > > > + None currently defined
> > > > +
> > > > +\subsection{Device configuration layout}\label{sec:Device Types /
> > Crypto
> > > Device / Device configuration layout}
> > > > +
> > > > +Thirteen driver-read-only configuration fields are currently defined.
> > > > +
> > > > +\begin{lstlisting}
> > > > +struct virtio_crypto_config {
> > > > + le32 version;
> > > > + le16 status;
> > > > + le16 max_dataqueues;
> > > > + le32 crypto_services;
> > > > + /* detailed algorithms mask */
> > > > + le32 cipher_algo_l;
> > > > + le32 cipher_algo_h;
> > > > + le32 hash_algo;
> > > > + le32 mac_algo_l;
> > > > + le32 mac_algo_h;
> > > > + le32 asym_algo;
> > > > + le32 kdf_algo;
> > > > + le32 aead_algo;
> > > > + le32 primitive_algo;
> > > > +};
> > > > +\end{lstlisting}
> > > > +
> > > > +The first driver-read-only field, \field{version} specifies the virtio
> > crypto??s
> > > > +version, which is reserved for back-compatibility in future.It??s
> > > > currently
> > > > +defined for the version field:
> > > > +
> > > > +\begin{lstlisting}
> > > > +#define VIRTIO_CRYPTO_VERSION_1 (1)
> > > > +\end{lstlisting}
> > > > +
> > >
> > > We have feature bits so I think this should not be necessary.
> > >
> > Yes, you are right.
> >
>
> I think version field is useful in some cases.
> Considering a case, if virtio crypto device implementation has defects,
> and a new version fixes this defect, how can the driver know whether the
> behavior of the device is correct or not?
> Feature bit is mostly like a negotiable field between device and driver.
You would add a feature bit meaning "bug is fixed". Allows fixing bugs
in driver as well - device looks at driver features and can work
around bugs or fail init.
--
MST
