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

[Halil Pasic <pasic@linux.vnet.ibm.com>]

On 05/04/2017 03:33 PM, Gonglei (Arei) wrote:
>>> +\drivernormative{\subsubsection}{Device configuration layout}{Device Types
>> / Crypto Device / Device configuration layout}
>>> +
>>> +\begin{itemize*}
>>> +\item The driver MUST read the ready \field{status} from the bottom bit of
>> status to check whether the backend crypto accelerator
>>> +      is ready or not, and the driver MUST reread it after device reset.
>>> +\item The driver MUST NOT transmit any requests to the device if the ready
>> \field{status} 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 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.
>>
>> Qouting a discussion from v15:
>> """
>>>>> +The value of the \field{status} field is VIRTIO_CRYPTO_S_HW_READY or
>>> ~VIRTIO_CRYPTO_S_HW_READY.
>>>> Not entirely happy with this. What you want to say is reserved
>>>> for future use, or? Would it make sense to have a general note
>>>> -- in a similar fashion like for 'sizes are in bytes' -- for
>>>> reserved for future use?
>>>>
>>>> One possible formulation would be:
>>>>
>>>> "In this specification, unless explicitly stated otherwise,
>>>> fields and bits reserved for future use shall be zeroed out.
>>>> Both the a device or a driver device and the driver should
>>>> detect violations of this rule, and deny the requested
>>>> operation in an appropriate way if possible."
>>> If we go with reserved-and-must-be-zero, we need to make rejecting
>>> non-zero for reserved value a MUST, or we may run into problems later.
>>>
>>> In this case, I'd opt for a specific formulation, though; like
>>>
>>> "The \field{status} field can be either zero or have one or more flags
>>> set. Valid flags are listed below."
>>>
>>> And then state that non-valid flags MUST NOT be set resp. MUST be
>>> rejected in a normative statement.
>>>
>> Sounds good.
>> """
>> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg01596.html
>>
>> I can't find this. Did we agree on something else in the meanwhile?
>>
> Please see above description:
> 
> """
> \begin{description}
> \item[\field{status}] is used to show whether the device is ready to work or not, it can be either zero or have one or more flags
>     Only one read-only bit (for the driver) is currently defined for the \field{status} field: VIRTIO_CRYPTO_S_HW_READY:
> \begin{lstlisting}
> #define VIRTIO_CRYPTO_S_HW_READY  (1 << 0)
> \end{lstlisting}
> """
> 

I miss 

>>> And then state that non-valid flags MUST NOT be set resp. MUST be
>>> rejected in a normative statement.
>>>
that is 'MUST BE REJECTED in a normative statement' in particular!