RE: [PATCH v6 01/12] virtio-crypto: introduce virtio_crypto.h

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

> -----Original Message-----
> From: Eric Blake [mailto:eblake@redhat.com]
> Sent: Tuesday, October 11, 2016 3:58 AM
> Subject: Re: [PATCH v6 01/12] virtio-crypto: introduce virtio_crypto.h
> 
> On 10/10/2016 03:43 AM, Gonglei wrote:
> > Introduce the virtio_crypto.h which follows
> > virtio-crypto specification.
> >
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> >  include/standard-headers/linux/virtio_crypto.h | 381
> +++++++++++++++++++++++++
> >  1 file changed, 381 insertions(+)
> >  create mode 100644 include/standard-headers/linux/virtio_crypto.h
> >
> > diff --git a/include/standard-headers/linux/virtio_crypto.h
> b/include/standard-headers/linux/virtio_crypto.h
> > new file mode 100644
> > index 0000000..a62d192
> > --- /dev/null
> > +++ b/include/standard-headers/linux/virtio_crypto.h
> > @@ -0,0 +1,381 @@
> > +#ifndef _VIRTIO_CRYPTO_H
> > +#define _VIRTIO_CRYPTO_H
> 
> Missing a copyright and license notice.  Is this header copied from
> Linux, or is it new to qemu?  At any rate, being explicit about your
> license is important (in qemu, files without a license are GPLv2+;
> whereas the Linux kernel tends to favor GPLv2-only).
> 
It should be copied from Linux like other virtio devices do.
I'll add copyright for this in the next version.

> > +
> > +#include "standard-headers/linux/types.h"
> > +#include "standard-headers/linux/virtio_config.h"
> > +#include "standard-headers/linux/virtio_types.h"
> > +
> > +
> > +#define VIRTIO_CRYPTO_SERVICE_CIPHER (0)
> > +#define VIRTIO_CRYPTO_SERVICE_HASH (1)
> > +#define VIRTIO_CRYPTO_SERVICE_MAC  (2)
> > +#define VIRTIO_CRYPTO_SERVICE_AEAD (3)
> 
> Technically over-parenthesized, but doesn't hurt.
> 
> > +
> > +#define VIRTIO_CRYPTO_OPCODE(service, op)   ((service << 8) | (op))
> 
> Under-parenthesized.  You need more () around service.
> 
So, the best definition is:

#define VIRTIO_CRYPTO_SERVICE_CIPHER 0
#define VIRTIO_CRYPTO_SERVICE_HASH 1
#define VIRTIO_CRYPTO_SERVICE_MAC 2
#define VIRTIO_CRYPTO_SERVICE_AEAD 3

#define VIRTIO_CRYPTO_OPCODE(service, op)   (((service) << 8) | (op))

Will fix. Thanks~

Regards,
-Gonglei