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

[Cornelia Huck <cornelia.huck@de.ibm.com>]

On Sun, 20 Nov 2016 08:45:57 +0000
gong lei <arei.gonglei@hotmail.com> wrote:

> On 2016/11/17 2:11, Halil Pasic wrote:
> > On 11/11/2016 10:23 AM, Gonglei wrote:

> >> +The value of the \field{status} field is VIRTIO_CRYPTO_S_HW_READY or VIRTIO_CRYPTO_S_STARTED.
> >> +
> >> +\begin{lstlisting}
> >> +#define VIRTIO_CRYPTO_S_HW_READY  (1 << 0)
> >> +#define VIRTIO_CRYPTO_S_STARTED  (1 << 1)
> >> +\end{lstlisting}
> >> +
> > Could not really figure out what this status actually does and how does
> > it relate to the device status field if at all.
> >
> > Furthermore I see no mention of VIRTIO_CRYPTO_S_STARTED except for this
> > one, so the only thing I can think of is that it's the initial value and
> > means hardware not ready (you state these are the only two values).
> Good catch. I set VIRTIO_CRYPTO_S_STARTED in the driver if the 
> virtio-crypto driver is ready to
> work in the guest (registing to the Linux Crypto Framework and the 
> device is ready), vice versa.

Can you use DRIVER_OK in the generic status field for that?

> 
> > This however does not seem consistent with what your QEMU reference
> > implementation does. Another thing is your implementations seem to
> > use VIRTIO_CRYPTO_S_HW_READY as flag but your specification would
> > (prohibit combining flags because you get another value).
> The QEMU side doesn't use VIRTIO_CRYPTO_S_STARTED, so maybe I can remove 
> it from
> the spec and define it in the driver. Does it make sense?

I think it makes most sense if you consider this status field to be
device-set only. The HW_READY flag makes sense as it gives the driver
additional information (much like the LINK_UP flag for virtio-net), but
I'm not sure what driver status you need beyond what you can already
provide via the generic status field.

In any case, the status field should use flags and not mutually
exclusive values.