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

[Michael S. Tsirkin]

On Tue, Jan 09, 2018 at 06:05:41PM +0100, Halil Pasic wrote:
> > +\drivernormative{\subsubsection}{Device configuration layout}{Device Types 
> > / Crypto Device / Device configuration layout}
> > +
> > +\begin{itemize*}
> > +\item The driver MUST read the \field{status} from the bottom bit of 
> > status to check whether the
> > +    VIRTIO_CRYPTO_S_HW_READY is set, and the driver MUST reread it after 
> > device reset.
> > +\item The driver MUST NOT transmit any requests to the device if the 
> > VIRTIO_CRYPTO_S_HW_READY 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 SHOULD ignore the not defined algorithms bits.
> > +\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*}
> 
> Seems to like a common practice in the virtio spec to say SHOULD read some
> config space parameter in a driver normative but then having no driver 
> normative
> about the operational implications of the parameter.
> 
> While I do think that the reader is usually fully capable of inferring
> the operational requirement, I find it a bit strange: having the operational
> stuff explicit (and possibly needs to be read from the config space implicit)
> seems more straight forward to me.
> 
> Anyway, you are consistent with the rest of the spec, so prefer keeping it
> like this.

The reason for this in virtio generally is that driver is really
only required to be consistent with the device.
Something like reading the value once and storing in e.g.
registry, then using the stored value from registry on
following values seems to also be ok.

Restricting values below the limits allowed by the device
also seems to be ok.