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

[Gonglei (Arei)]

Hi,


> 
> On 01/04/2017 11:10 AM, Gonglei (Arei) wrote:
> > Hi all,
> >
> > I attach the diff files between v14 and v15 for better review.
> >
> Hi,
> 
> only had a quick look. Will try to come back to this later.
> 
That's cool.

> > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > index 9f7faf0..884ee95 100644
> > --- a/virtio-crypto.tex
> > +++ b/virtio-crypto.tex
> > @@ -2,8 +2,8 @@
> >
> >  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 the data queue and are ultimately handled
> by the
> > -backend crypto accelerators. The second queue is the control queue used to
> create
> > +decryption requests are placed in any of the data active queues and are
> ultimately handled by the
> s/data active/active data/
> > +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.
> 
> [..]
> 
> > ===============The below diff shows the changes of add non-session mode
> support:
> >
> > diff --git a/virtio-crypto.tex b/virtio-crypto.tex
> > index 884ee95..44819f9 100644
> > --- a/virtio-crypto.tex
> > +++ b/virtio-crypto.tex
> > @@ -26,7 +26,10 @@ N is set by \field{max_dataqueues}.
> >
> >  \subsection{Feature bits}\label{sec:Device Types / Crypto Device / Feature
> bits}
> >
> > -None currently defined.
> > +VIRTIO_CRYPTO_F_CIPHER_SESSION_MODE (1) Session mode is available
> for CIPHER service.
> > +VIRTIO_CRYPTO_F_HASH_SESSION_MODE (2) Session mode is available for
> HASH service.
> > +VIRTIO_CRYPTO_F_MAC_SESSION_MODE (3) Session mode is available for
> MAC service.
> > +VIRTIO_CRYPTO_F_AEAD_SESSION_MODE (4) Session mode is available for
> AEAD service.
> >
> >  \subsection{Device configuration layout}\label{sec:Device Types / Crypto
> Device / Device configuration layout}
> >
> > @@ -208,6 +211,9 @@ 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
> non-session mode, for example,
> > +As VIRTIO_CRYPTO_F_CIPHER_SESSION feature bit is negotiated, the driver
> can use session mode for CIPHER service, otherwise it can only use non-session
> mode.
> > +
> 
> As far as I understand you are adding non-session mode to the mix but
> providing feature bits for session mode. Would this render the the current
> implementation non-compliant?
> 
You are right, shall we use feature bits for non-session mode for compliancy?
Or because the spec is on the fly, and some structures in the virtio_crypto.h 
need to
be modified, can we keep the compliancy completely?

Thanks,
-Gonglei