> -----Original Message----- > From: Stefan Hajnoczi [mailto:stefa...@redhat.com] > Sent: Tuesday, October 04, 2016 12:11 AM > Subject: Re: [PATCH v4 01/13] cryptodev: introduce cryptodev backend > interface > > On Wed, Sep 28, 2016 at 04:25:40PM +0800, Gonglei wrote: > > diff --git a/backends/cryptodev.c b/backends/cryptodev.c > > new file mode 100644 > > index 0000000..a15904b > > --- /dev/null > > +++ b/backends/cryptodev.c > > @@ -0,0 +1,175 @@ > > +/* > > + * QEMU Crypto Device Implement > > s/Implement/Implementation/ > > > diff --git a/include/sysemu/cryptodev.h b/include/sysemu/cryptodev.h > > new file mode 100644 > > index 0000000..cc3c3be > > --- /dev/null > > +++ b/include/sysemu/cryptodev.h > > @@ -0,0 +1,145 @@ > > +/* > > + * QEMU Crypto Device Implement > > s/Implement/Implementation/ > OK.
> > + * > > + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. > > + * > > + * Authors: > > + * Gonglei <arei.gong...@huawei.com> > > + * > > + * This library is free software; you can redistribute it and/or > > + * modify it under the terms of the GNU Lesser General Public > > + * License as published by the Free Software Foundation; either > > + * version 2 of the License, or (at your option) any later version. > > + * > > + * This library is distributed in the hope that it will be useful, > > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > GNU > > + * Lesser General Public License for more details. > > + * > > + * You should have received a copy of the GNU Lesser General Public > > + * License along with this library; if not, see > <http://www.gnu.org/licenses/>. > > + * > > + */ > > +#ifndef QCRYPTO_CRYPTODEV_H > > +#define QCRYPTO_CRYPTODEV_H > > + > > +#include "qom/object.h" > > +#include "qemu-common.h" > > + > > +/** > > + * QCryptoCryptoDevBackend: > > + * > > + * The QCryptoCryptoDevBackend object is an interface > > + * for different cryptodev backends, which provides crypto > > + * operation wrapper. > > I suggest calling it CryptoDevBackend since that's shorter and doesn't > repeat any information. I'm not sure why "QCrypto" is necessary. > Ok, and Daniel explained the reason in another reply. I'll rename those stuff. > > + * > > + */ > > + > > +#define TYPE_QCRYPTO_CRYPTODEV_BACKEND "cryptodev-backend" > > + > > +#define QCRYPTO_CRYPTODEV_BACKEND(obj) \ > > + OBJECT_CHECK(QCryptoCryptoDevBackend, \ > > + (obj), TYPE_QCRYPTO_CRYPTODEV_BACKEND) > > +#define QCRYPTO_CRYPTODEV_BACKEND_GET_CLASS(obj) \ > > + OBJECT_GET_CLASS(QCryptoCryptoDevBackendClass, \ > > + (obj), TYPE_QCRYPTO_CRYPTODEV_BACKEND) > > +#define QCRYPTO_CRYPTODEV_BACKEND_CLASS(klass) \ > > + OBJECT_CLASS_CHECK(QCryptoCryptoDevBackendClass, \ > > + (klass), TYPE_QCRYPTO_CRYPTODEV_BACKEND) > > + > > + > > +#define MAX_CRYPTO_QUEUE_NUM 64 > > + > > +typedef struct QCryptoCryptoDevBackendConf > QCryptoCryptoDevBackendConf; > > +typedef struct QCryptoCryptoDevBackendPeers > QCryptoCryptoDevBackendPeers; > > +typedef struct QCryptoCryptoDevBackendClientState > > + QCryptoCryptoDevBackendClientState; > > +typedef struct QCryptoCryptoDevBackend QCryptoCryptoDevBackend; > > + > > + > > +typedef struct QCryptoCryptoDevBackendClass { > > + ObjectClass parent_class; > > + > > + void (*init)(QCryptoCryptoDevBackend *backend, Error **errp); > > + void (*cleanup)(QCryptoCryptoDevBackend *backend, Error **errp); > > +} QCryptoCryptoDevBackendClass; > > + > > + > > +struct QCryptoCryptoDevBackendClientState { > > The naming could be simplified: CryptoDevClient. > > > + char *model; > > + char *name; > > + char info_str[128]; > > This should probably be char *info_str unless there is a specific reason > to use a fixed-size buffer. > OK. > > + unsigned int queue_index; > > + QTAILQ_ENTRY(QCryptoCryptoDevBackendClientState) next; > > +}; > > + > > +struct QCryptoCryptoDevBackendPeers { > > + QCryptoCryptoDevBackendClientState > *ccs[MAX_CRYPTO_QUEUE_NUM]; > > + uint32_t queues; > > +}; > > + > > +struct QCryptoCryptoDevBackendConf { > > + QCryptoCryptoDevBackendPeers peers; > > + > > + /* Supported service mask */ > > + uint32_t crypto_services; > > + > > + /* Detailed algorithms mask */ > > + uint32_t cipher_algo_l; > > + uint32_t cipher_algo_h; > > + uint32_t hash_algo; > > + uint32_t mac_algo_l; > > + uint32_t mac_algo_h; > > + uint32_t asym_algo; > > + uint32_t kdf_algo; > > + uint32_t aead_algo; > > + uint32_t primitive_algo; > > +}; > > + > > +struct QCryptoCryptoDevBackend { > > + Object parent_obj; > > + > > + int ready; > > Please use bool. That way it's clear the field only takes true and > false values. > OK. > > + QCryptoCryptoDevBackendConf conf; > > +}; > > + > > +/** > > + * qcrypto_cryptodev_backend_new_client: > > + * @model: the cryptodev backend model > > + * @name: the cryptodev backend name, can be NULL > > + * > > + * Creates a new cryptodev backend client object > > + * with the @name in the mode @mode. > > s/mode/model/ > Yes. Regards, -Gonglei
