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

["Gonglei (Arei)" <arei.gonglei@huawei.com>]

Hi,

> 
> [..]
> > +The header is the general header and the union is of the algorithm-specific
> type,
> > +which is set by the driver. All properties in the union are shown as follows.
> > +
> > +There is a unified idata structure for all symmetric algorithms, including
> CIPHER, HASH, MAC, and AEAD.
> > +
> > +The structure is defined as follows:
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_sym_input {
> > +    /* Destination data guest address, it's useless for plain HASH and MAC
> */
> > +    le64 dst_data_addr;
> > +    /* Digest result guest address, it's useless for plain cipher algos */
> > +    le64 digest_result_addr;
> > +
> > +    le32 status;
> > +    le32 padding;
> > +};
> > +
> 
> This seems to be out of sync regarding the code (e.g. can't find it in
> virtio-crypto.h of the
> series linked in the cover-letter). It seems to me this reflects v4 since the stuff
> is gone
> since v5 (qemu code).
> 
> > +\end{lstlisting}
> > +
> > +\subsubsection{HASH Service Operation}\label{sec:Device Types / Crypto
> Device / Device Operation / HASH Service Operation}
> > +
> > +\begin{lstlisting}
> > +struct virtio_crypto_hash_para {
> > +    /* length of source data */
> > +    le32 src_data_len;
> > +    /* hash result length */
> > +    le32 hash_result_len;
> > +};
> > +
> > +struct virtio_crypto_hash_input {
> > +    struct virtio_crypto_sym_input input;
> > +};
> > +
> > +struct virtio_crypto_hash_output {
> > +    /* source data guest address */
> > +    le64 src_data_addr;
> > +};
> > +
> > +struct virtio_crypto_hash_data_req {
> > +    /* Device-readable part */
> > +    struct virtio_crypto_hash_para para;
> > +    struct virtio_crypto_hash_output odata;
> > +    /* Device-writable part */
> > +    struct virtio_crypto_hash_input idata;
> > +};
> > +\end{lstlisting}
> > +
> > +Each data request uses virtio_crypto_hash_data_req structure to store
> information
> > +used to run the HASH operations. The request only occupies one entry
> > +in the Vring Descriptor Table in the virtio crypto device's dataq, which
> improves
> > +the throughput of data transmitted for the HASH service, so that the virtio
> crypto
> > +device can be better accelerated.
> > +
> > +The information includes the source data guest physical address stored by
> \field{odata}.\field{src_data_addr},
> > +length of source data stored by \field{para}.\field{src_data_len}, and the
> digest result guest physical address
> > +stored by \field{digest_result_addr} used to save the results of the HASH
> operations.
> > +The address and length can determine exclusive content in the guest
> memory.
> > +
> 
> Thus this does not make any sense to me. Furthermore the problem seems to
> persist
> across the specification. Thus in my opinion there is no point in reviewing
> this version. Or am I missing something here? In case I'm not missing anything
> and the spec describes something quite outdated when should we expect a
> new
> version of the spec?
> 
Nope, Actually I kept those description here is because I wanted to represent each packet
Intuitionally, otherwise I don't know how to explain them only occupy one entry in desc table
by indirect table method. So I changed the code completely as Stefan's suggestion and
revised the spec a little.

This just is a representative of the realization so that the people can easily understand what
the virtio crypto request's components. It isn't completely same with the code.
For virtio-scsi device, the struct virtio_scsi_req_cmd also used this way IIUR.

Thanks,
-Gonglei