| Message ID | 1473738741-220600-2-git-send-email-arei.gonglei@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Sep 13, 2016 at 11:52:07AM +0800, Gonglei wrote: > cryptodev backend is used to realize the active work for > virtual crypto device. CryptoLegacyHW device is a cryptographic > hardware device seen by the virtual machine. > The relationship between cryptodev backend and legacy hadware > as follow: > crypto_legacy_hw device (1)--->(n) cryptodev client backend > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > diff --git a/crypto/crypto.c b/crypto/crypto.c > new file mode 100644 > index 0000000..fbc6497 > --- /dev/null > +++ b/crypto/crypto.c > @@ -0,0 +1,171 @@ > +/* > + * QEMU Crypto Device Implement > + * > + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. > + * > + * Authors: > + * Gonglei <arei.gonglei@huawei.com> > + * > + * Permission is hereby granted, free of charge, to any person obtaining a copy > + * of this software and associated documentation files (the "Software"), to deal > + * in the Software without restriction, including without limitation the rights > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > + * copies of the Software, and to permit persons to whom the Software is > + * furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice shall be included in > + * all copies or substantial portions of the Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > + * THE SOFTWARE. > + */ New files are expected to be submitted under the GPLv2+ license, unless they're header files imported from an external project, which this is not. The same license mistake is made across other files / patches in this series, so I won't repeat the comment - just fix all of them to be GPLv2+ please. > +#include "qemu/osdep.h" > +#include "sysemu/sysemu.h" > +#include "qapi/error.h" > +#include "qemu/iov.h" > +#include "qapi-visit.h" > +#include "qapi/opts-visitor.h" > + > +#include "crypto/crypto.h" > +#include "qemu/config-file.h" > +#include "monitor/monitor.h" > + > + > +static QTAILQ_HEAD(, CryptoClientState) crypto_clients; > + > +QemuOptsList qemu_cryptodev_opts = { > + .name = "cryptodev", > + .implied_opt_name = "type", > + .head = QTAILQ_HEAD_INITIALIZER(qemu_cryptodev_opts.head), > + .desc = { > + /* > + * no elements => accept any params > + * validation will happen later > + */ > + { /* end of list */ } > + }, > +}; No new code should be adding more command line options for device backends. Your backend impl should be using QOM to define a object, which can be created via the -object command line arg, or object_add monitor command. If you're not familiar with this, take a look at the crypto/secret.c file which is a pretty simple example of how to use QOM to define a new user creatable object. I'm going to skip reviewing any of the .c code in the crypto/ dir for now, since that will all change quite a bit when you switch over to QOM. > diff --git a/include/crypto/crypto.h b/include/crypto/crypto.h > new file mode 100644 > index 0000000..f93f6f9 > --- /dev/null > +++ b/include/crypto/crypto.h > +#ifndef QCRYPTO_CRYPTO_H__ > +#define QCRYPTO_CRYPTO_H__ > + > +#include "qemu/queue.h" > +#include "qapi-types.h" > + > +typedef void (CryptoPoll)(CryptoClientState *, bool); > +typedef void (CryptoCleanup) (CryptoClientState *); > +typedef void (CryptoClientDestructor)(CryptoClientState *); > +typedef void (CryptoHWStatusChanged)(CryptoClientState *); > + > +typedef struct CryptoClientInfo { > + CryptoClientOptionsKind type; > + size_t size; > + > + CryptoCleanup *cleanup; > + CryptoPoll *poll; > + CryptoHWStatusChanged *hw_status_changed; > +} CryptoClientInfo; > + > +struct CryptoClientState { > + CryptoClientInfo *info; > + int ready; > + QTAILQ_ENTRY(CryptoClientState) next; > + CryptoClientState *peer; > + char *model; > + char *name; > + char info_str[256]; > + CryptoClientDestructor *destructor; > +}; > + > +int crypto_client_init(QemuOpts *opts, Error **errp); > +int crypto_init_clients(void); > + > +CryptoClientState *new_crypto_client(CryptoClientInfo *info, > + CryptoClientState *peer, > + const char *model, > + const char *name); > + > +#endif /* QCRYPTO_CRYPTO_H__ */ For all files in the crypto sub-directory I've been trying to stick to the strict convention that all methods must follow the naming scheme qcrypto_[filename]_XX eg if you rename this file to cryptodev as requested, your methods should all follow the naming convention of 'qcrypto_cryptdev_XXX'. Similarly all structs would be QCryptoCryptoDevXXX Finally the .h file should contain full inline documentation. At start there should be a general description of the file and its purpose, along with example usages. Then there should be formal documentation for every single method in the .h file describing the method semantics, parameters and return values. See include/crypto/cipher.h for an example. > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h > index b113fcf..82843a9 100644 > --- a/include/qemu/typedefs.h > +++ b/include/qemu/typedefs.h > @@ -94,5 +94,6 @@ typedef struct SSIBus SSIBus; > typedef struct uWireSlave uWireSlave; > typedef struct VirtIODevice VirtIODevice; > typedef struct Visitor Visitor; > +typedef struct CryptoClientState CryptoClientState; Don't add to this file - anything that wants to see the CryptoClientState typedef should explicitly include the crypto/cryptodev.h file or whatever the master definition lives. > diff --git a/qapi-schema.json b/qapi-schema.json > index c4f3674..46f7993 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -4582,3 +4582,49 @@ > # Since: 2.7 > ## > { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] } > + > +## > +# @CryptoLegacyHWOptions > +# > +# Create a new cryptographic hardware device. > +# > +# @cryptodev: #optional id of -cryptodev to connect to > +# > +# @model: #optional device model (Only virtio at present) > +# > +# @vectors: #optional number of MSI-x vectors, 0 to disable MSI-X > +# > +# Since 2.8 > +## > +{ 'struct': 'CryptoLegacyHWOptions', > + 'data': { > + '*cryptodev': 'str', > + '*model': 'str', > + '*vectors': 'uint32' } } > + > +## > +# @CryptoClientOptions > +# > +# A discriminated record of crypto device traits. > +# > +# Since 2.8 > +# > +## > +{ 'union': 'CryptoClientOptions', > + 'data': {'legacy-hw': 'CryptoLegacyHWOptions'} } > + > +## > +# @Cryptodev > +# > +# Captures the configuration of a crypto device. > +# > +# @id: identifier for monitor commands. > +# > +# @opts: device type specific properties > +# > +# Since 2.8 > +## > +{ 'struct': 'Cryptodev', > + 'data': { > + 'id': 'str', > + 'opts': 'CryptoClientOptions' } } All crypto related QAPI additions should be in qapi/crypto.json Regards, Daniel
> > From: Daniel P. Berrange [mailto:berrange@redhat.com] > Sent: Tuesday, September 13, 2016 5:14 PM > Subject: Re: [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto > legacy hardware > > On Tue, Sep 13, 2016 at 11:52:07AM +0800, Gonglei wrote: > > cryptodev backend is used to realize the active work for > > virtual crypto device. CryptoLegacyHW device is a cryptographic > > hardware device seen by the virtual machine. > > The relationship between cryptodev backend and legacy hadware > > as follow: > > crypto_legacy_hw device (1)--->(n) cryptodev client backend > > > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > > > diff --git a/crypto/crypto.c b/crypto/crypto.c > > new file mode 100644 > > index 0000000..fbc6497 > > --- /dev/null > > +++ b/crypto/crypto.c > > @@ -0,0 +1,171 @@ > > +/* > > + * QEMU Crypto Device Implement > > + * > > + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. > > + * > > + * Authors: > > + * Gonglei <arei.gonglei@huawei.com> > > + * > > + * Permission is hereby granted, free of charge, to any person obtaining a > copy > > + * of this software and associated documentation files (the "Software"), to > deal > > + * in the Software without restriction, including without limitation the rights > > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > > + * copies of the Software, and to permit persons to whom the Software is > > + * furnished to do so, subject to the following conditions: > > + * > > + * The above copyright notice and this permission notice shall be included in > > + * all copies or substantial portions of the Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > EXPRESS OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO > EVENT SHALL > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, > DAMAGES OR OTHER > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > ARISING FROM, > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > OTHER DEALINGS IN > > + * THE SOFTWARE. > > + */ > > New files are expected to be submitted under the GPLv2+ license, > unless they're header files imported from an external project, > which this is not. > > The same license mistake is made across other files / patches > in this series, so I won't repeat the comment - just fix all > of them to be GPLv2+ please. > > > +#include "qemu/osdep.h" > > +#include "sysemu/sysemu.h" > > +#include "qapi/error.h" > > +#include "qemu/iov.h" > > +#include "qapi-visit.h" > > +#include "qapi/opts-visitor.h" > > + > > +#include "crypto/crypto.h" > > +#include "qemu/config-file.h" > > +#include "monitor/monitor.h" > > + > > + > > +static QTAILQ_HEAD(, CryptoClientState) crypto_clients; > > + > > +QemuOptsList qemu_cryptodev_opts = { > > + .name = "cryptodev", > > + .implied_opt_name = "type", > > + .head = QTAILQ_HEAD_INITIALIZER(qemu_cryptodev_opts.head), > > + .desc = { > > + /* > > + * no elements => accept any params > > + * validation will happen later > > + */ > > + { /* end of list */ } > > + }, > > +}; > > No new code should be adding more command line options for > device backends. > > Your backend impl should be using QOM to define a object, > which can be created via the -object command line arg, > or object_add monitor command. > Any backgrounds about this rule? Maybe I missed something. > If you're not familiar with this, take a look at the > crypto/secret.c file which is a pretty simple example of > how to use QOM to define a new user creatable object. > OK, thanks. > I'm going to skip reviewing any of the .c code in the > crypto/ dir for now, since that will all change quite a > bit when you switch over to QOM. > OK. > > diff --git a/include/crypto/crypto.h b/include/crypto/crypto.h > > new file mode 100644 > > index 0000000..f93f6f9 > > --- /dev/null > > +++ b/include/crypto/crypto.h > > > +#ifndef QCRYPTO_CRYPTO_H__ > > +#define QCRYPTO_CRYPTO_H__ > > + > > +#include "qemu/queue.h" > > +#include "qapi-types.h" > > + > > +typedef void (CryptoPoll)(CryptoClientState *, bool); > > +typedef void (CryptoCleanup) (CryptoClientState *); > > +typedef void (CryptoClientDestructor)(CryptoClientState *); > > +typedef void (CryptoHWStatusChanged)(CryptoClientState *); > > + > > +typedef struct CryptoClientInfo { > > + CryptoClientOptionsKind type; > > + size_t size; > > + > > + CryptoCleanup *cleanup; > > + CryptoPoll *poll; > > + CryptoHWStatusChanged *hw_status_changed; > > +} CryptoClientInfo; > > + > > +struct CryptoClientState { > > + CryptoClientInfo *info; > > + int ready; > > + QTAILQ_ENTRY(CryptoClientState) next; > > + CryptoClientState *peer; > > + char *model; > > + char *name; > > + char info_str[256]; > > + CryptoClientDestructor *destructor; > > +}; > > + > > +int crypto_client_init(QemuOpts *opts, Error **errp); > > +int crypto_init_clients(void); > > + > > +CryptoClientState *new_crypto_client(CryptoClientInfo *info, > > + CryptoClientState *peer, > > + const char *model, > > + const char *name); > > + > > +#endif /* QCRYPTO_CRYPTO_H__ */ > > For all files in the crypto sub-directory I've been trying > to stick to the strict convention that all methods must > follow the naming scheme qcrypto_[filename]_XX > eg if you rename this file to cryptodev as requested, > your methods should all follow the naming convention > of 'qcrypto_cryptdev_XXX'. > > Similarly all structs would be QCryptoCryptoDevXXX > OK. > Finally the .h file should contain full inline documentation. > At start there should be a general description of the file > and its purpose, along with example usages. Then there should > be formal documentation for every single method in the .h > file describing the method semantics, parameters and return > values. See include/crypto/cipher.h for an example. > OK, make senses to me. > > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h > > index b113fcf..82843a9 100644 > > --- a/include/qemu/typedefs.h > > +++ b/include/qemu/typedefs.h > > @@ -94,5 +94,6 @@ typedef struct SSIBus SSIBus; > > typedef struct uWireSlave uWireSlave; > > typedef struct VirtIODevice VirtIODevice; > > typedef struct Visitor Visitor; > > +typedef struct CryptoClientState CryptoClientState; > > Don't add to this file - anything that wants to see > the CryptoClientState typedef should explicitly include > the crypto/cryptodev.h file or whatever the master > definition lives. > OK. > > diff --git a/qapi-schema.json b/qapi-schema.json > > index c4f3674..46f7993 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -4582,3 +4582,49 @@ > > # Since: 2.7 > > ## > > { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] } > > + > > +## > > +# @CryptoLegacyHWOptions > > +# > > +# Create a new cryptographic hardware device. > > +# > > +# @cryptodev: #optional id of -cryptodev to connect to > > +# > > +# @model: #optional device model (Only virtio at present) > > +# > > +# @vectors: #optional number of MSI-x vectors, 0 to disable MSI-X > > +# > > +# Since 2.8 > > +## > > +{ 'struct': 'CryptoLegacyHWOptions', > > + 'data': { > > + '*cryptodev': 'str', > > + '*model': 'str', > > + '*vectors': 'uint32' } } > > + > > +## > > +# @CryptoClientOptions > > +# > > +# A discriminated record of crypto device traits. > > +# > > +# Since 2.8 > > +# > > +## > > +{ 'union': 'CryptoClientOptions', > > + 'data': {'legacy-hw': 'CryptoLegacyHWOptions'} } > > + > > +## > > +# @Cryptodev > > +# > > +# Captures the configuration of a crypto device. > > +# > > +# @id: identifier for monitor commands. > > +# > > +# @opts: device type specific properties > > +# > > +# Since 2.8 > > +## > > +{ 'struct': 'Cryptodev', > > + 'data': { > > + 'id': 'str', > > + 'opts': 'CryptoClientOptions' } } > > All crypto related QAPI additions should be in qapi/crypto.json > OK. > > Regards, > Daniel > -- > |: http://berrange.com -o- > http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- > http://virt-manager.org :| > |: http://autobuild.org -o- > http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- > http://live.gnome.org/gtk-vnc :|
On Tue, Sep 13, 2016 at 09:55:27AM +0000, Gonglei (Arei) wrote: > > > > > From: Daniel P. Berrange [mailto:berrange@redhat.com] > > Sent: Tuesday, September 13, 2016 5:14 PM > > Subject: Re: [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto > > legacy hardware > > > > On Tue, Sep 13, 2016 at 11:52:07AM +0800, Gonglei wrote: > > > cryptodev backend is used to realize the active work for > > > virtual crypto device. CryptoLegacyHW device is a cryptographic > > > hardware device seen by the virtual machine. > > > The relationship between cryptodev backend and legacy hadware > > > as follow: > > > crypto_legacy_hw device (1)--->(n) cryptodev client backend > > > > > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > > > > > diff --git a/crypto/crypto.c b/crypto/crypto.c > > > new file mode 100644 > > > index 0000000..fbc6497 > > > --- /dev/null > > > +++ b/crypto/crypto.c > > > @@ -0,0 +1,171 @@ > > > +/* > > > + * QEMU Crypto Device Implement > > > + * > > > + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. > > > + * > > > + * Authors: > > > + * Gonglei <arei.gonglei@huawei.com> > > > + * > > > + * Permission is hereby granted, free of charge, to any person obtaining a > > copy > > > + * of this software and associated documentation files (the "Software"), to > > deal > > > + * in the Software without restriction, including without limitation the rights > > > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > > > + * copies of the Software, and to permit persons to whom the Software is > > > + * furnished to do so, subject to the following conditions: > > > + * > > > + * The above copyright notice and this permission notice shall be included in > > > + * all copies or substantial portions of the Software. > > > + * > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, > > EXPRESS OR > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > > MERCHANTABILITY, > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO > > EVENT SHALL > > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, > > DAMAGES OR OTHER > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, > > ARISING FROM, > > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > > OTHER DEALINGS IN > > > + * THE SOFTWARE. > > > + */ > > > > New files are expected to be submitted under the GPLv2+ license, > > unless they're header files imported from an external project, > > which this is not. > > > > The same license mistake is made across other files / patches > > in this series, so I won't repeat the comment - just fix all > > of them to be GPLv2+ please. > > > > > +#include "qemu/osdep.h" > > > +#include "sysemu/sysemu.h" > > > +#include "qapi/error.h" > > > +#include "qemu/iov.h" > > > +#include "qapi-visit.h" > > > +#include "qapi/opts-visitor.h" > > > + > > > +#include "crypto/crypto.h" > > > +#include "qemu/config-file.h" > > > +#include "monitor/monitor.h" > > > + > > > + > > > +static QTAILQ_HEAD(, CryptoClientState) crypto_clients; > > > + > > > +QemuOptsList qemu_cryptodev_opts = { > > > + .name = "cryptodev", > > > + .implied_opt_name = "type", > > > + .head = QTAILQ_HEAD_INITIALIZER(qemu_cryptodev_opts.head), > > > + .desc = { > > > + /* > > > + * no elements => accept any params > > > + * validation will happen later > > > + */ > > > + { /* end of list */ } > > > + }, > > > +}; > > > > No new code should be adding more command line options for > > device backends. > > > > Your backend impl should be using QOM to define a object, > > which can be created via the -object command line arg, > > or object_add monitor command. > > > Any backgrounds about this rule? Maybe I missed something. I don't think it has been documented anywhere explicitly. The general idea of using QOM though is that it avoids the need for you to re-invent the wheel for command line parsing and gives you QMP/HMP based hotplug for free at the same time. It was first used for the RNG and hostmem backends, and more recently my tls-creds and secrets objects. A very very very long term goal would be for exinsting backends to be converted to be QOM based too, but for now we're just aiming for any new backends to use QOM from the start. Regards, Daniel
> -----Original Message----- > From: Daniel P. Berrange [mailto:berrange@redhat.com] > Sent: Tuesday, September 13, 2016 5:59 PM > Subject: Re: [PATCH v2 01/15] crypto: introduce cryptodev backend and crypto > legacy hardware > > On Tue, Sep 13, 2016 at 09:55:27AM +0000, Gonglei (Arei) wrote: > > > > > > > > From: Daniel P. Berrange [mailto:berrange@redhat.com] > > > Sent: Tuesday, September 13, 2016 5:14 PM > > > Subject: Re: [PATCH v2 01/15] crypto: introduce cryptodev backend and > crypto > > > legacy hardware > > > > > > On Tue, Sep 13, 2016 at 11:52:07AM +0800, Gonglei wrote: > > > > cryptodev backend is used to realize the active work for > > > > virtual crypto device. CryptoLegacyHW device is a cryptographic > > > > hardware device seen by the virtual machine. > > > > The relationship between cryptodev backend and legacy hadware > > > > as follow: > > > > crypto_legacy_hw device (1)--->(n) cryptodev client backend > > > > > > > > Signed-off-by: Gonglei <arei.gonglei@huawei.com> > > > > > > > diff --git a/crypto/crypto.c b/crypto/crypto.c > > > > new file mode 100644 > > > > index 0000000..fbc6497 > > > > --- /dev/null > > > > +++ b/crypto/crypto.c > > > > @@ -0,0 +1,171 @@ > > > > +/* > > > > + * QEMU Crypto Device Implement > > > > + * > > > > + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. > > > > + * > > > > + * Authors: > > > > + * Gonglei <arei.gonglei@huawei.com> > > > > + * > > > > + * Permission is hereby granted, free of charge, to any person obtaining > a > > > copy > > > > + * of this software and associated documentation files (the "Software"), > to > > > deal > > > > + * in the Software without restriction, including without limitation the > rights > > > > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > > > > + * copies of the Software, and to permit persons to whom the Software is > > > > + * furnished to do so, subject to the following conditions: > > > > + * > > > > + * The above copyright notice and this permission notice shall be included > in > > > > + * all copies or substantial portions of the Software. > > > > + * > > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY > KIND, > > > EXPRESS OR > > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF > > > MERCHANTABILITY, > > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN > NO > > > EVENT SHALL > > > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, > > > DAMAGES OR OTHER > > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR > OTHERWISE, > > > ARISING FROM, > > > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR > > > OTHER DEALINGS IN > > > > + * THE SOFTWARE. > > > > + */ > > > > > > New files are expected to be submitted under the GPLv2+ license, > > > unless they're header files imported from an external project, > > > which this is not. > > > > > > The same license mistake is made across other files / patches > > > in this series, so I won't repeat the comment - just fix all > > > of them to be GPLv2+ please. > > > > > > > +#include "qemu/osdep.h" > > > > +#include "sysemu/sysemu.h" > > > > +#include "qapi/error.h" > > > > +#include "qemu/iov.h" > > > > +#include "qapi-visit.h" > > > > +#include "qapi/opts-visitor.h" > > > > + > > > > +#include "crypto/crypto.h" > > > > +#include "qemu/config-file.h" > > > > +#include "monitor/monitor.h" > > > > + > > > > + > > > > +static QTAILQ_HEAD(, CryptoClientState) crypto_clients; > > > > + > > > > +QemuOptsList qemu_cryptodev_opts = { > > > > + .name = "cryptodev", > > > > + .implied_opt_name = "type", > > > > + .head = QTAILQ_HEAD_INITIALIZER(qemu_cryptodev_opts.head), > > > > + .desc = { > > > > + /* > > > > + * no elements => accept any params > > > > + * validation will happen later > > > > + */ > > > > + { /* end of list */ } > > > > + }, > > > > +}; > > > > > > No new code should be adding more command line options for > > > device backends. > > > > > > Your backend impl should be using QOM to define a object, > > > which can be created via the -object command line arg, > > > or object_add monitor command. > > > > > Any backgrounds about this rule? Maybe I missed something. > > I don't think it has been documented anywhere explicitly. > The general idea of using QOM though is that it avoids the > need for you to re-invent the wheel for command line parsing > and gives you QMP/HMP based hotplug for free at the same > time. It was first used for the RNG and hostmem backends, > and more recently my tls-creds and secrets objects. > > A very very very long term goal would be for exinsting backends > to be converted to be QOM based too, but for now we're just aiming > for any new backends to use QOM from the start. > OK, I see. Thanks! Regards, -Gonglei > Regards, > Daniel > -- > |: http://berrange.com -o- > http://www.flickr.com/photos/dberrange/ :| > |: http://libvirt.org -o- > http://virt-manager.org :| > |: http://autobuild.org -o- > http://search.cpan.org/~danberr/ :| > |: http://entangle-photo.org -o- > http://live.gnome.org/gtk-vnc :|
On 13/09/2016 11:13, Daniel P. Berrange wrote: > > + * Permission is hereby granted, free of charge, to any person obtaining a copy > > + * of this software and associated documentation files (the "Software"), to deal > > + * in the Software without restriction, including without limitation the rights > > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > > + * copies of the Software, and to permit persons to whom the Software is > > + * furnished to do so, subject to the following conditions: > > + * > > + * The above copyright notice and this permission notice shall be included in > > + * all copies or substantial portions of the Software. > > + * > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > > + * THE SOFTWARE. > > + */ > > New files are expected to be submitted under the GPLv2+ license, > unless they're header files imported from an external project, > which this is not. This is not true. New files are expected to be submitted under a GPLv2+-compatible license, which this one is. Paolo
On Tue, Sep 13, 2016 at 12:50:53PM +0200, Paolo Bonzini wrote: > > > On 13/09/2016 11:13, Daniel P. Berrange wrote: > > > + * Permission is hereby granted, free of charge, to any person obtaining a copy > > > + * of this software and associated documentation files (the "Software"), to deal > > > + * in the Software without restriction, including without limitation the rights > > > + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell > > > + * copies of the Software, and to permit persons to whom the Software is > > > + * furnished to do so, subject to the following conditions: > > > + * > > > + * The above copyright notice and this permission notice shall be included in > > > + * all copies or substantial portions of the Software. > > > + * > > > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR > > > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, > > > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL > > > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER > > > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, > > > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN > > > + * THE SOFTWARE. > > > + */ > > > > New files are expected to be submitted under the GPLv2+ license, > > unless they're header files imported from an external project, > > which this is not. > > This is not true. New files are expected to be submitted under a > GPLv2+-compatible license, which this one is. If you want to copy code snippets from existing files which are GPLv2+ licensed, into this new file, then that'd be a license violation as this new file is granting broader permissions which the original code did not grant. So I still believe this should be GPLv2+ unless there is a compelling reason why it cannot be. Regards, Daniel
diff --git a/crypto/Makefile.objs b/crypto/Makefile.objs index a36d2d9..2a63cb8 100644 --- a/crypto/Makefile.objs +++ b/crypto/Makefile.objs @@ -26,6 +26,7 @@ crypto-obj-y += xts.o crypto-obj-y += block.o crypto-obj-y += block-qcow.o crypto-obj-y += block-luks.o +crypto-obj-y += crypto.o # Let the userspace emulators avoid linking gnutls/etc crypto-aes-obj-y = aes.o diff --git a/crypto/crypto.c b/crypto/crypto.c new file mode 100644 index 0000000..fbc6497 --- /dev/null +++ b/crypto/crypto.c @@ -0,0 +1,171 @@ +/* + * QEMU Crypto Device Implement + * + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. + * + * Authors: + * Gonglei <arei.gonglei@huawei.com> + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#include "qemu/osdep.h" +#include "sysemu/sysemu.h" +#include "qapi/error.h" +#include "qemu/iov.h" +#include "qapi-visit.h" +#include "qapi/opts-visitor.h" + +#include "crypto/crypto.h" +#include "qemu/config-file.h" +#include "monitor/monitor.h" + + +static QTAILQ_HEAD(, CryptoClientState) crypto_clients; + +QemuOptsList qemu_cryptodev_opts = { + .name = "cryptodev", + .implied_opt_name = "type", + .head = QTAILQ_HEAD_INITIALIZER(qemu_cryptodev_opts.head), + .desc = { + /* + * no elements => accept any params + * validation will happen later + */ + { /* end of list */ } + }, +}; + +static int +crypto_init_cryptodev(void *dummy, QemuOpts *opts, Error **errp) +{ + Error *local_err = NULL; + int ret; + + ret = crypto_client_init(opts, &local_err); + if (local_err) { + error_report_err(local_err); + return -1; + } + + return ret; +} + +int crypto_init_clients(void) +{ + QTAILQ_INIT(&crypto_clients); + + if (qemu_opts_foreach(qemu_find_opts("cryptodev"), + crypto_init_cryptodev, NULL, NULL)) { + return -1; + } + + return 0; +} + +static int (* const crypto_client_init_fun[CRYPTO_CLIENT_OPTIONS_KIND__MAX])( + const CryptoClientOptions *opts, + const char *name, + CryptoClientState *peer, Error **errp); + +static int crypto_client_init1(const void *object, Error **errp) +{ + const CryptoClientOptions *opts; + const char *name; + + const Cryptodev *cryptodev = object; + opts = cryptodev->opts; + name = cryptodev->id; + + if (opts->type == CRYPTO_CLIENT_OPTIONS_KIND_LEGACY_HW || + !crypto_client_init_fun[opts->type]) { + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "type", + "a cryptodev backend type"); + return -1; + } + + if (crypto_client_init_fun[opts->type](opts, name, NULL, errp) < 0) { + if (errp && !*errp) { + error_setg(errp, QERR_DEVICE_INIT_FAILED, + CryptoClientOptionsKind_lookup[opts->type]); + } + return -1; + } + return 0; +} + +int crypto_client_init(QemuOpts *opts, Error **errp) +{ + void *object = NULL; + Error *err = NULL; + int ret = -1; + Visitor *v = opts_visitor_new(opts); + + visit_type_Cryptodev(v, NULL, (Cryptodev **)&object, &err); + if (!err) { + ret = crypto_client_init1(object, &err); + } + + qapi_free_Cryptodev(object); + error_propagate(errp, err); + return ret; +} + +static void crypto_client_destructor(CryptoClientState *cc) +{ + g_free(cc); +} + +static void crypto_client_setup(CryptoClientState *cc, + CryptoClientInfo *info, + CryptoClientState *peer, + const char *model, + const char *name, + CryptoClientDestructor *destructor) +{ + cc->info = info; + cc->model = g_strdup(model); + if (name) { + cc->name = g_strdup(name); + } + + if (peer) { + assert(!peer->peer); + cc->peer = peer; + peer->peer = cc; + } + QTAILQ_INSERT_TAIL(&crypto_clients, cc, next); + cc->destructor = destructor; +} + +CryptoClientState *new_crypto_client(CryptoClientInfo *info, + CryptoClientState *peer, + const char *model, + const char *name) +{ + CryptoClientState *cc; + + assert(info->size >= sizeof(CryptoClientState)); + + /* allocate the memroy of CryptoClientState's parent */ + cc = g_malloc0(info->size); + crypto_client_setup(cc, info, peer, model, name, + crypto_client_destructor); + + return cc; +} diff --git a/include/crypto/crypto.h b/include/crypto/crypto.h new file mode 100644 index 0000000..f93f6f9 --- /dev/null +++ b/include/crypto/crypto.h @@ -0,0 +1,66 @@ +/* + * QEMU Crypto Device Implement + * + * Copyright (c) 2016 HUAWEI TECHNOLOGIES CO., LTD. + * + * Authors: + * Gonglei <arei.gonglei@huawei.com> + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ +#ifndef QCRYPTO_CRYPTO_H__ +#define QCRYPTO_CRYPTO_H__ + +#include "qemu/queue.h" +#include "qapi-types.h" + +typedef void (CryptoPoll)(CryptoClientState *, bool); +typedef void (CryptoCleanup) (CryptoClientState *); +typedef void (CryptoClientDestructor)(CryptoClientState *); +typedef void (CryptoHWStatusChanged)(CryptoClientState *); + +typedef struct CryptoClientInfo { + CryptoClientOptionsKind type; + size_t size; + + CryptoCleanup *cleanup; + CryptoPoll *poll; + CryptoHWStatusChanged *hw_status_changed; +} CryptoClientInfo; + +struct CryptoClientState { + CryptoClientInfo *info; + int ready; + QTAILQ_ENTRY(CryptoClientState) next; + CryptoClientState *peer; + char *model; + char *name; + char info_str[256]; + CryptoClientDestructor *destructor; +}; + +int crypto_client_init(QemuOpts *opts, Error **errp); +int crypto_init_clients(void); + +CryptoClientState *new_crypto_client(CryptoClientInfo *info, + CryptoClientState *peer, + const char *model, + const char *name); + +#endif /* QCRYPTO_CRYPTO_H__ */ diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index b113fcf..82843a9 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -94,5 +94,6 @@ typedef struct SSIBus SSIBus; typedef struct uWireSlave uWireSlave; typedef struct VirtIODevice VirtIODevice; typedef struct Visitor Visitor; +typedef struct CryptoClientState CryptoClientState; #endif /* QEMU_TYPEDEFS_H */ diff --git a/include/sysemu/sysemu.h b/include/sysemu/sysemu.h index ee7c760..58e0d88 100644 --- a/include/sysemu/sysemu.h +++ b/include/sysemu/sysemu.h @@ -244,5 +244,6 @@ extern QemuOptsList qemu_netdev_opts; extern QemuOptsList qemu_net_opts; extern QemuOptsList qemu_global_opts; extern QemuOptsList qemu_mon_opts; +extern QemuOptsList qemu_cryptodev_opts; #endif diff --git a/qapi-schema.json b/qapi-schema.json index c4f3674..46f7993 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4582,3 +4582,49 @@ # Since: 2.7 ## { 'command': 'query-hotpluggable-cpus', 'returns': ['HotpluggableCPU'] } + +## +# @CryptoLegacyHWOptions +# +# Create a new cryptographic hardware device. +# +# @cryptodev: #optional id of -cryptodev to connect to +# +# @model: #optional device model (Only virtio at present) +# +# @vectors: #optional number of MSI-x vectors, 0 to disable MSI-X +# +# Since 2.8 +## +{ 'struct': 'CryptoLegacyHWOptions', + 'data': { + '*cryptodev': 'str', + '*model': 'str', + '*vectors': 'uint32' } } + +## +# @CryptoClientOptions +# +# A discriminated record of crypto device traits. +# +# Since 2.8 +# +## +{ 'union': 'CryptoClientOptions', + 'data': {'legacy-hw': 'CryptoLegacyHWOptions'} } + +## +# @Cryptodev +# +# Captures the configuration of a crypto device. +# +# @id: identifier for monitor commands. +# +# @opts: device type specific properties +# +# Since 2.8 +## +{ 'struct': 'Cryptodev', + 'data': { + 'id': 'str', + 'opts': 'CryptoClientOptions' } } diff --git a/qemu-options.hx b/qemu-options.hx index a71aaf8..a64ce25 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -3968,6 +3968,9 @@ contents of @code{iv.b64} to the second secret ETEXI +DEF("cryptodev", HAS_ARG, QEMU_OPTION_cryptodev, + "", + QEMU_ARCH_ALL) HXCOMM This is the last statement. Insert new options before this line! STEXI diff --git a/vl.c b/vl.c index ee557a1..24e86ea 100644 --- a/vl.c +++ b/vl.c @@ -119,6 +119,7 @@ int main(int argc, char **argv) #include "qapi-event.h" #include "exec/semihost.h" #include "crypto/init.h" +#include "crypto/crypto.h" #include "sysemu/replay.h" #include "qapi/qmp/qerror.h" @@ -3015,6 +3016,7 @@ int main(int argc, char **argv, char **envp) qemu_add_opts(&qemu_icount_opts); qemu_add_opts(&qemu_semihosting_config_opts); qemu_add_opts(&qemu_fw_cfg_opts); + qemu_add_opts(&qemu_cryptodev_opts); module_call_init(MODULE_INIT_OPTS); runstate_init(); @@ -3983,6 +3985,13 @@ int main(int argc, char **argv, char **envp) exit(1); } break; + case QEMU_OPTION_cryptodev: + opts = qemu_opts_parse_noisily(qemu_find_opts("cryptodev"), + optarg, false); + if (!opts) { + exit(1); + } + break; default: os_parse_cmd_args(popt->index, optarg); } @@ -4372,6 +4381,10 @@ int main(int argc, char **argv, char **envp) exit(1); } + if (crypto_init_clients() < 0) { + exit(1); + } + if (qemu_opts_foreach(qemu_find_opts("object"), user_creatable_add_opts_foreach, object_create_delayed, NULL)) {
cryptodev backend is used to realize the active work for virtual crypto device. CryptoLegacyHW device is a cryptographic hardware device seen by the virtual machine. The relationship between cryptodev backend and legacy hadware as follow: crypto_legacy_hw device (1)--->(n) cryptodev client backend Signed-off-by: Gonglei <arei.gonglei@huawei.com> --- crypto/Makefile.objs | 1 + crypto/crypto.c | 171 ++++++++++++++++++++++++++++++++++++++++++++++++ include/crypto/crypto.h | 66 +++++++++++++++++++ include/qemu/typedefs.h | 1 + include/sysemu/sysemu.h | 1 + qapi-schema.json | 46 +++++++++++++ qemu-options.hx | 3 + vl.c | 13 ++++ 8 files changed, 302 insertions(+) create mode 100644 crypto/crypto.c create mode 100644 include/crypto/crypto.h