Linux cryptographic layer development
 help / color / mirror / Atom feed
* Re: [PATCH 2/3] crypto: AF_ALG - disregard AAD buffer space for output
From: Herbert Xu @ 2016-11-28 11:39 UTC (permalink / raw)
  To: Stephan Mueller; +Cc: mathew.j.martineau, linux-crypto
In-Reply-To: <6354916.mphDsJscQs@positron.chronox.de>

On Sat, Nov 19, 2016 at 10:08:06PM +0100, Stephan Mueller wrote:
> 
> The way to go on this topic would be to use the same logic as the authenc 
> implementation by using a null cipher for the copy operation. Though, finding 
> out whether the src and dst buffers are the same is an interesting 
> proposition, because we need to traverse the src and dst SGLs to see whether 
> the same pages and same offsets are used. A simple check for src SGL == dst 
> SGL will not work for the AF_ALG implementation, because the src SGL will 
> always be different from the dst SGL because they are constructed in different 
> ways (tsgl will always be different from rsgl). What may be the same are the 
> pages and offsets that are pointed to by the SGL in case of zerocopy.

True, you won't be able to use the fast path and will end up doing
the copy.  However, it should still work because we also have an
aliasing check within the null copy code (skcipher_null_crypt).

So for the sake of simplicity, I think we should start with just
copying src to dst unconditionally in algif_aead, and then calling
the underlying crypto code with just dst to invoke the inplace code
path.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply

* Re: [PATCH v4] crypto: arm64/sha2: integrate OpenSSL implementations of SHA256/SHA512
From: Ard Biesheuvel @ 2016-11-28  9:50 UTC (permalink / raw)
  To: linux-crypto@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, Herbert Xu, Will Deacon
  Cc: Andy Polyakov, Ard Biesheuvel
In-Reply-To: <CAKv+Gu9SG5s=zddQaw+444191JGqmXfCh+AKyv3nZNqggmh=Kw@mail.gmail.com>

On 20 November 2016 at 11:43, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 20 November 2016 at 11:42, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> This integrates both the accelerated scalar and the NEON implementations
>> of SHA-224/256 as well as SHA-384/512 from the OpenSSL project.
>>
>> Relative performance compared to the respective generic C versions:
>>
>>                  |  SHA256-scalar  | SHA256-NEON* |  SHA512  |
>>      ------------+-----------------+--------------+----------+
>>      Cortex-A53  |      1.63x      |     1.63x    |   2.34x  |
>>      Cortex-A57  |      1.43x      |     1.59x    |   1.95x  |
>>      Cortex-A73  |      1.26x      |     1.56x    |     ?    |
>>
>> The core crypto code was authored by Andy Polyakov of the OpenSSL
>> project, in collaboration with whom the upstream code was adapted so
>> that this module can be built from the same version of sha512-armv8.pl.
>>
>> The version in this patch was taken from OpenSSL commit 32bbb62ea634
>> ("sha/asm/sha512-armv8.pl: fix big-endian support in __KERNEL__ case.")
>>
>> * The core SHA algorithm is fundamentally sequential, but there is a
>>   secondary transformation involved, called the schedule update, which
>>   can be performed independently. The NEON version of SHA-224/SHA-256
>>   only implements this part of the algorithm using NEON instructions,
>>   the sequential part is always done using scalar instructions.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>
> Missing changelog:
>
> v4: fixed the big-endian build; this required an upstream change (even
>     though upstream was not actually broken, since it explicitly defines
>     __ARMEB__ on AArch64 big-endian builds), so this patch is now based
>     on a more recent upstream OpenSSL commit (the __ILP32__ #ifdefs are
>     still present but never active)
>
> v3: at Will's request, the generated assembly files are now included
>     as .S_shipped files, for which generic build rules are defined
>     already.
>
> Note that sizeable patches like this one have caused issues in the past with
> patchwork, so for Herbert's convenience, the patch can be pulled from
> http://git.kernel.org/cgit/linux/kernel/git/ardb/linux.git, branch
> arm64-sha256 (based on today's cryptodev)

Herbert,

Assuming that everyone is happy now (Will?), could we get this one
queued for v4.10? The CRC stuff I sent over the past week can wait for
v4.11 (and I should probably do a v2 roundup with everything
combined), but this patch is good to go IMO

Thanks,
Ard.

^ permalink raw reply

* Re: [PATCH] crypto: vmx - rebuild generated asm when target changes
From: Nicholas Piggin @ 2016-11-28  8:48 UTC (permalink / raw)
  To: Naveen N. Rao
  Cc: Herbert Xu, Leonidas S . Barbosa, linux-crypto,
	Paulo Flabiano Smorigo, linuxppc-dev
In-Reply-To: <20161128082136.GD29256@naverao1-tp.localdomain>

On Mon, 28 Nov 2016 13:51:36 +0530
"Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com> wrote:

> On 2016/11/26 03:24PM, Nicholas Piggin wrote:
> > Switching from big endian to little endian can fail to regenerate
> > the crypto assembly properly. Switch to using standard form of
> > kbuild dependency checking (i.e., use FORCE and if_changed).
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>  
> 
> Hi Nick,
> A similar patch is already in:
> https://mid.mail-archive.com/linux-crypto@vger.kernel.org/msg21855.html
> 
> - Naveen
> 

Hi Naveen,

I didn't notice your patch, thanks for pointing it out.

Thanks,
Nick

^ permalink raw reply

* Re: [PATCH] crypto: vmx - rebuild generated asm when target changes
From: Naveen N. Rao @ 2016-11-28  8:21 UTC (permalink / raw)
  To: Nicholas Piggin
  Cc: Herbert Xu, Leonidas S . Barbosa, linux-crypto,
	Paulo Flabiano Smorigo, linuxppc-dev
In-Reply-To: <20161126042459.1964-1-npiggin@gmail.com>

On 2016/11/26 03:24PM, Nicholas Piggin wrote:
> Switching from big endian to little endian can fail to regenerate
> the crypto assembly properly. Switch to using standard form of
> kbuild dependency checking (i.e., use FORCE and if_changed).
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Hi Nick,
A similar patch is already in:
https://mid.mail-archive.com/linux-crypto@vger.kernel.org/msg21855.html

- Naveen

^ permalink raw reply

* RE: BALANCE PAYMENT
From: coral @ 2016-11-28  6:03 UTC (permalink / raw)


Dear Sir/s,

Please see attached.


Thanks and regards,

Accounts Department
Al Omraniya Trading Co. LLC
P.O. Box: 10757, Al Khabaisi Area,
Deira 2, Dubai, U.A.E.
Tel: +971 4 268 2730 / Fax: +971 4 268 4117

^ permalink raw reply

* RE: BALANCE PAYMENT
From: coral @ 2016-11-28  6:03 UTC (permalink / raw)


Dear Sir/s,

Please see attached.


Thanks and regards,

Accounts Department
Al Omraniya Trading Co. LLC
P.O. Box: 10757, Al Khabaisi Area,
Deira 2, Dubai, U.A.E.
Tel: +971 4 268 2730 / Fax: +971 4 268 4117

^ permalink raw reply

* RE: [virtio-dev] Re: [PATCH v2 2/2] crypto: add virtio-crypto driver
From: Gonglei (Arei) @ 2016-11-28  5:38 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
	virtio-dev@lists.oasis-open.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org, Luonengjun, stefanha@redhat.com,
	Huangweidong (C), Wubin (H), xin.zeng@intel.com, Claudio Fontana,
	herbert@gondor.apana.org.au, pasic@linux.vnet.ibm.com,
	davem@davemloft.net, "Zhoujian (jay, Euler)" <jian
In-Reply-To: <20161128070829-mutt-send-email-mst@kernel.org>


> 
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Monday, November 28, 2016 1:09 PM
> To: Gonglei (Arei)
> Subject: Re: [virtio-dev] Re: [PATCH v2 2/2] crypto: add virtio-crypto driver
> 
> On Mon, Nov 28, 2016 at 04:47:21AM +0000, Gonglei (Arei) wrote:
> > Michael, I'd like to add virtio-crypto stuff to your maintaining part likes
> > the virtio-net/blk parts so that the corresponding patches
> > can be CC'ed to you too because the virtio-crypto doesn't lay in
> > driver/virtio directory. Will you?
> >
> > Thanks!
> > -Gonglei
> 
> Sure, that's fine.
> 
Will do in the next version.  :)

Thanks,
-Gonglei

^ permalink raw reply

* Re: Re: [PATCH v2 2/2] crypto: add virtio-crypto driver
From: Michael S. Tsirkin @ 2016-11-28  5:09 UTC (permalink / raw)
  To: Gonglei (Arei)
  Cc: linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
	virtio-dev@lists.oasis-open.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org, Luonengjun, stefanha@redhat.com,
	Huangweidong (C), Wubin (H), xin.zeng@intel.com, Claudio Fontana,
	herbert@gondor.apana.org.au, pasic@linux.vnet.ibm.com,
	davem@davemloft.net,
	"Zhoujian (jay, Euler)" <jianjay.zhou@
In-Reply-To: <33183CC9F5247A488A2544077AF19020D97A10EC@SZXEMA503-MBS.china.huawei.com>

On Mon, Nov 28, 2016 at 04:47:21AM +0000, Gonglei (Arei) wrote:
> Michael, I'd like to add virtio-crypto stuff to your maintaining part likes
> the virtio-net/blk parts so that the corresponding patches
> can be CC'ed to you too because the virtio-crypto doesn't lay in 
> driver/virtio directory. Will you?
> 
> Thanks!
> -Gonglei

Sure, that's fine.

-- 
MST

^ permalink raw reply

* RE: [virtio-dev] Re: [PATCH v2 2/2] crypto: add virtio-crypto driver
From: Gonglei (Arei) @ 2016-11-28  4:47 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
	virtio-dev@lists.oasis-open.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org, Luonengjun, stefanha@redhat.com,
	Huangweidong (C), Wubin (H), xin.zeng@intel.com, Claudio Fontana,
	herbert@gondor.apana.org.au, pasic@linux.vnet.ibm.com,
	davem@davemloft.net, "Zhoujian (jay, Euler)" <jian
In-Reply-To: <20161127045532-mutt-send-email-mst@kernel.org>

Hi Michael,

>
> Subject: [virtio-dev] Re: [PATCH v2 2/2] crypto: add virtio-crypto driver
> 
> On Tue, Nov 22, 2016 at 04:10:23PM +0800, Gonglei wrote:
> > This patch introduces virtio-crypto driver for Linux Kernel.
> >
> > The virtio crypto device is a virtual cryptography device
> > as well as a kind of virtual hardware accelerator for
> > virtual machines. The encryption anddecryption requests
> > are placed in the data queue and are ultimately handled by
> > thebackend crypto accelerators. The second queue is the
> > control queue used to create or destroy sessions for
> > symmetric algorithms and will control some advanced features
> > in the future. The virtio crypto device provides the following
> > cryptoservices: CIPHER, MAC, HASH, and AEAD.
> >
> > For more information about virtio-crypto device, please see:
> >   http://qemu-project.org/Features/VirtioCrypto
> >
> > CC: Michael S. Tsirkin <mst@redhat.com>
> > CC: Cornelia Huck <cornelia.huck@de.ibm.com>
> > CC: Stefan Hajnoczi <stefanha@redhat.com>
> > CC: Herbert Xu <herbert@gondor.apana.org.au>
> > CC: Halil Pasic <pasic@linux.vnet.ibm.com>
> > CC: David S. Miller <davem@davemloft.net>
> > CC: Zeng Xin <xin.zeng@intel.com>
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> >  MAINTAINERS                                  |   8 +
> >  drivers/crypto/Kconfig                       |   2 +
> >  drivers/crypto/Makefile                      |   1 +
> >  drivers/crypto/virtio/Kconfig                |  10 +
> >  drivers/crypto/virtio/Makefile               |   5 +
> >  drivers/crypto/virtio/virtio_crypto.c        | 444
> +++++++++++++++++++++++
> >  drivers/crypto/virtio/virtio_crypto_algs.c   | 524
> +++++++++++++++++++++++++++
> >  drivers/crypto/virtio/virtio_crypto_common.h | 124 +++++++
> >  drivers/crypto/virtio/virtio_crypto_mgr.c    | 258 +++++++++++++
> >  include/uapi/linux/Kbuild                    |   1 +
> >  include/uapi/linux/virtio_crypto.h           | 435
> ++++++++++++++++++++++
> >  include/uapi/linux/virtio_ids.h              |   1 +
> >  12 files changed, 1813 insertions(+)
> >  create mode 100644 drivers/crypto/virtio/Kconfig
> >  create mode 100644 drivers/crypto/virtio/Makefile
> >  create mode 100644 drivers/crypto/virtio/virtio_crypto.c
> >  create mode 100644 drivers/crypto/virtio/virtio_crypto_algs.c
> >  create mode 100644 drivers/crypto/virtio/virtio_crypto_common.h
> >  create mode 100644 drivers/crypto/virtio/virtio_crypto_mgr.c
> >  create mode 100644 include/uapi/linux/virtio_crypto.h
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 411e3b8..d6b18bb 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -12844,6 +12844,14 @@ S:	Maintained
> >  F:	drivers/virtio/virtio_input.c
> >  F:	include/uapi/linux/virtio_input.h
> >
> > +VIRTIO CRYPTO DRIVER
> > +M:  Gonglei <arei.gonglei@huawei.com>
> > +L:  virtualization@lists.linux-foundation.org
> > +L:  linux-crypto@vger.kernel.org
> > +S:  Maintained
> > +F:  drivers/crypto/virtio/
> > +F:  include/uapi/linux/virtio_crypto.h
> > +
> >  VIA RHINE NETWORK DRIVER
> >  S:	Orphan
> >  F:	drivers/net/ethernet/via/via-rhine.c
> > diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> > index 4d2b81f..7956478 100644
> > --- a/drivers/crypto/Kconfig
> > +++ b/drivers/crypto/Kconfig
> > @@ -555,4 +555,6 @@ config CRYPTO_DEV_ROCKCHIP
> >
> >  source "drivers/crypto/chelsio/Kconfig"
> >
> > +source "drivers/crypto/virtio/Kconfig"
> > +
> >  endif # CRYPTO_HW
> > diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
> > index ad7250f..bc53cb8 100644
> > --- a/drivers/crypto/Makefile
> > +++ b/drivers/crypto/Makefile
> > @@ -32,3 +32,4 @@ obj-$(CONFIG_CRYPTO_DEV_VMX) += vmx/
> >  obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sunxi-ss/
> >  obj-$(CONFIG_CRYPTO_DEV_ROCKCHIP) += rockchip/
> >  obj-$(CONFIG_CRYPTO_DEV_CHELSIO) += chelsio/
> > +obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio/
> > diff --git a/drivers/crypto/virtio/Kconfig b/drivers/crypto/virtio/Kconfig
> > new file mode 100644
> > index 0000000..ceae88c
> > --- /dev/null
> > +++ b/drivers/crypto/virtio/Kconfig
> > @@ -0,0 +1,10 @@
> > +config CRYPTO_DEV_VIRTIO
> > +	tristate "VirtIO crypto driver"
> > +	depends on VIRTIO
> > +    select CRYPTO_AEAD
> > +    select CRYPTO_AUTHENC
> > +    select CRYPTO_BLKCIPHER
> > +	default m
> > +	help
> > +	  This driver provides support for virtio crypto device. If you
> > +	  choose 'M' here, this module will be called virtio-crypto.
> > diff --git a/drivers/crypto/virtio/Makefile b/drivers/crypto/virtio/Makefile
> > new file mode 100644
> > index 0000000..a316e92
> > --- /dev/null
> > +++ b/drivers/crypto/virtio/Makefile
> > @@ -0,0 +1,5 @@
> > +obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio-crypto.o
> > +virtio-crypto-objs := \
> > +	virtio_crypto_algs.o \
> > +	virtio_crypto_mgr.o \
> > +	virtio_crypto.o
> > diff --git a/drivers/crypto/virtio/virtio_crypto.c
> b/drivers/crypto/virtio/virtio_crypto.c
> > new file mode 100644
> > index 0000000..56fdfed
> > --- /dev/null
> > +++ b/drivers/crypto/virtio/virtio_crypto.c
> > @@ -0,0 +1,444 @@
> > + /* Driver for Virtio crypto device.
> > +  *
> > +  * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
> > +  *
> > +  * This program is free software; you can redistribute it and/or modify
> > +  * it under the terms of the GNU General Public License as published by
> > +  * the Free Software Foundation; either version 2 of the License, or
> > +  * (at your option) any later version.
> > +  *
> > +  * This program 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 General Public License for more details.
> > +  *
> > +  * You should have received a copy of the GNU General Public License
> > +  * along with this program; if not, see <http://www.gnu.org/licenses/>.
> > +  */
> > +
> > +#include <linux/err.h>
> > +#include <linux/module.h>
> > +#include <linux/virtio_config.h>
> > +#include <linux/cpu.h>
> > +
> > +#include <uapi/linux/virtio_crypto.h>
> > +#include "virtio_crypto_common.h"
> > +
> > +
> > +static void virtcrypto_dataq_callback(struct virtqueue *vq)
> > +{
> > +	struct virtio_crypto *vcrypto = vq->vdev->priv;
> > +	struct virtio_crypto_request *vc_req;
> > +	unsigned long flags;
> > +	unsigned int len;
> > +	struct ablkcipher_request *ablk_req;
> > +	int error;
> > +
> > +	spin_lock_irqsave(&vcrypto->lock, flags);
> > +	do {
> > +		virtqueue_disable_cb(vq);
> > +		while ((vc_req = virtqueue_get_buf(vq, &len)) != NULL) {
> > +			if (vc_req->type == VIRTIO_CRYPTO_SYM_OP_CIPHER) {
> > +				switch (vc_req->status) {
> > +				case VIRTIO_CRYPTO_OK:
> > +					error = 0;
> > +					break;
> > +				case VIRTIO_CRYPTO_INVSESS:
> > +				case VIRTIO_CRYPTO_ERR:
> > +					error = -EINVAL;
> > +					break;
> > +				case VIRTIO_CRYPTO_BADMSG:
> > +					error = -EBADMSG;
> > +					break;
> > +				default:
> > +					error = -EIO;
> > +					break;
> > +				}
> > +				ablk_req = vc_req->ablkcipher_req;
> > +				/* Finish the encrypt or decrypt process */
> > +				ablk_req->base.complete(&ablk_req->base, error);
> > +			}
> > +
> > +			kfree(vc_req->req_data);
> > +			kfree(vc_req->sgs);
> > +		}
> > +	} while (!virtqueue_enable_cb(vq));
> > +	spin_unlock_irqrestore(&vcrypto->lock, flags);
> > +}
> > +
> > +static int virtcrypto_find_vqs(struct virtio_crypto *vi)
> > +{
> > +	vq_callback_t **callbacks;
> > +	struct virtqueue **vqs;
> > +	int ret = -ENOMEM;
> > +	int i, total_vqs;
> > +	const char **names;
> > +
> > +	/* We expect 1 data virtqueue, followed by
> > +	 * possible N-1 data queues used in multiqueue mode, followed by
> > +	 * control vq.
> > +	 */
> > +	total_vqs = vi->max_data_queues + 1;
> > +
> > +	/* Allocate space for find_vqs parameters */
> > +	vqs = kcalloc(total_vqs, sizeof(*vqs), GFP_KERNEL);
> > +	if (!vqs)
> > +		goto err_vq;
> > +	callbacks = kcalloc(total_vqs, sizeof(*callbacks), GFP_KERNEL);
> > +	if (!callbacks)
> > +		goto err_callback;
> > +	names = kcalloc(total_vqs, sizeof(*names), GFP_KERNEL);
> > +	if (!names)
> > +		goto err_names;
> > +
> > +	/* Parameters for control virtqueue */
> > +	callbacks[total_vqs - 1] = NULL;
> > +	names[total_vqs - 1] = "controlq";
> > +
> > +	/* Allocate/initialize parameters for data virtqueues */
> > +	for (i = 0; i < vi->max_data_queues; i++) {
> > +		callbacks[i] = virtcrypto_dataq_callback;
> > +		snprintf(vi->data_vq[i].name, sizeof(vi->data_vq[i].name),
> > +				"dataq.%d", i);
> > +		names[i] = vi->data_vq[i].name;
> > +	}
> > +
> > +	ret = vi->vdev->config->find_vqs(vi->vdev, total_vqs, vqs, callbacks,
> > +					 names);
> > +	if (ret)
> > +		goto err_find;
> > +
> > +	vi->ctrl_vq = vqs[total_vqs - 1];
> > +
> > +	for (i = 0; i < vi->max_data_queues; i++)
> > +		vi->data_vq[i].vq = vqs[i];
> > +
> > +	kfree(names);
> > +	kfree(callbacks);
> > +	kfree(vqs);
> > +
> > +	return 0;
> > +
> > +err_find:
> > +	kfree(names);
> > +err_names:
> > +	kfree(callbacks);
> > +err_callback:
> > +	kfree(vqs);
> > +err_vq:
> > +	return ret;
> > +}
> > +
> > +static int virtcrypto_alloc_queues(struct virtio_crypto *vi)
> > +{
> > +	vi->data_vq = kcalloc(vi->max_data_queues, sizeof(*vi->data_vq),
> > +				GFP_KERNEL);
> > +	if (!vi->data_vq)
> > +		return -ENOMEM;
> > +
> > +	return 0;
> > +}
> > +
> > +static void virtcrypto_clean_affinity(struct virtio_crypto *vi, long hcpu)
> > +{
> > +	int i;
> > +
> > +	if (vi->affinity_hint_set) {
> > +		for (i = 0; i < vi->max_data_queues; i++)
> > +			virtqueue_set_affinity(vi->data_vq[i].vq, -1);
> > +
> > +		vi->affinity_hint_set = false;
> > +	}
> > +}
> > +
> > +static void virtcrypto_set_affinity(struct virtio_crypto *vi)
> > +{
> > +	int i;
> > +	int cpu;
> > +
> > +	/*
> > +	 * In multiqueue mode, when the number of cpu is equal to the number
> > +	 * of queue, we let the queue to be private to one cpu by
> > +	 * setting the affinity hint to eliminate the contention.
> > +	 */
> > +	if (vi->curr_queue == 1 ||
> > +	    vi->max_data_queues != num_online_cpus()) {
> > +		virtcrypto_clean_affinity(vi, -1);
> > +		return;
> > +	}
> > +
> 
> So how can you handle cpu hotplug then?
> I would imagine making max_data_queues > num_online_cpus
> and initializing just the required number would be
> a saner way.
> 

It makes sense.

> > +	i = 0;
> > +	for_each_online_cpu(cpu) {
> > +		virtqueue_set_affinity(vi->data_vq[i].vq, cpu);
> > +		i++;
> > +	}
> > +
> > +	vi->affinity_hint_set = true;
> > +}
> > +
> 
> Above is racy in prsence of cpu hotplug. Is there a userspace
> interface to fix up affinity after hotplug?

Not yet.

> If not you need to get notified after hotplug and
> fix it up yourself.
> 
I'll inspect it.

> > +static void virtcrypto_free_queues(struct virtio_crypto *vi)
> > +{
> > +	kfree(vi->data_vq);
> > +}
> > +
> > +static int virtcrypto_init_vqs(struct virtio_crypto *vi)
> > +{
> > +	int ret;
> > +
> > +	/* Allocate send & receive queues */
> > +	ret = virtcrypto_alloc_queues(vi);
> > +	if (ret)
> > +		goto err;
> > +
> > +	ret = virtcrypto_find_vqs(vi);
> > +	if (ret)
> > +		goto err_free;
> > +
> > +	get_online_cpus();
> > +	virtcrypto_set_affinity(vi);
> > +	put_online_cpus();
> > +
> > +	return 0;
> > +
> > +err_free:
> > +	virtcrypto_free_queues(vi);
> > +err:
> > +	return ret;
> > +}
> > +
> > +static int virtcrypto_update_status(struct virtio_crypto *vcrypto)
> > +{
> > +	__le32 status_le;
> > +	u32 status;
> > +	int err;
> > +
> > +	status_le = virtio_cread32_le(vcrypto->vdev,
> > +			offsetof(struct virtio_crypto_config, status));
> > +	status = le32_to_cpu(status_le);
> > +
> > +	/* Ignore unknown (future) status bits */
> > +	status &= VIRTIO_CRYPTO_S_HW_READY;
> > +
> > +	if (vcrypto->status == status)
> > +		return 0;
> > +
> > +	vcrypto->status = status;
> > +
> > +	if (vcrypto->status & VIRTIO_CRYPTO_S_HW_READY) {
> > +		err = virtcrypto_dev_start(vcrypto);
> > +		if (err) {
> > +			dev_err(&vcrypto->vdev->dev,
> > +				"Failed to start virtio crypto device.\n");
> > +			virtcrypto_dev_stop(vcrypto);
> > +			return -1;
> > +		}
> > +		dev_info(&vcrypto->vdev->dev, "Accelerator is ready\n");
> > +	} else {
> > +		virtcrypto_dev_stop(vcrypto);
> > +		dev_info(&vcrypto->vdev->dev, "Accelerator is not ready\n");
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static void virtcrypto_del_vqs(struct virtio_crypto *vcrypto)
> > +{
> > +	struct virtio_device *vdev = vcrypto->vdev;
> > +
> > +	virtcrypto_clean_affinity(vcrypto, -1);
> > +
> > +	vdev->config->del_vqs(vdev);
> > +
> > +	virtcrypto_free_queues(vcrypto);
> > +}
> > +
> > +static int virtcrypto_probe(struct virtio_device *vdev)
> > +{
> > +	int err = -EFAULT;
> > +	struct virtio_crypto *vcrypto;
> > +	__le32 max_data_queues_le = 0, max_cipher_key_len_le = 0;
> > +	__le32 max_auth_key_len_le = 0;
> > +	__le64 max_size_le = 0;
> > +
> 
> Pls validate that it's a modern device (has VERSION_1 set).
> We should find a way to do it in a central place,
> but we don't have that now.
> 
So what I will do is add a VERSION_1 feature bit checking at the
beginning of the probe function:

if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
	return -ENODEV;

> > +	if (!vdev->config->get) {
> > +		dev_err(&vdev->dev, "%s failure: config access disabled\n",
> > +			__func__);
> > +		return -EINVAL;
> > +	}
> > +
> > +	if (num_possible_nodes() > 1 && dev_to_node(&vdev->dev) < 0) {
> > +		/*
> > +		 * If the accelerator is connected to a node with no memory
> > +		 * there is no point in using the accelerator since the remote
> > +		 * memory transaction will be very slow.
> > +		 */
> > +		dev_err(&vdev->dev, "Invalid NUMA configuration.\n");
> > +		return -EINVAL;
> > +	}
> > +
> > +	vcrypto = kzalloc_node(sizeof(*vcrypto), GFP_KERNEL,
> > +					dev_to_node(&vdev->dev));
> > +	if (!vcrypto)
> > +		return -ENOMEM;
> > +
> > +	max_data_queues_le = virtio_cread32_le(vdev,
> > +			offsetof(struct virtio_crypto_config, max_dataqueues));
> > +	vcrypto->max_data_queues = le32_to_cpu(max_data_queues_le);
> > +	if (vcrypto->max_data_queues < 1)
> > +		vcrypto->max_data_queues = 1;
> > +
> > +	dev_info(&vdev->dev, "max_queues: %u\n", vcrypto->max_data_queues);
> > +
> > +	max_cipher_key_len_le = virtio_cread32_le(vdev,
> > +		offsetof(struct virtio_crypto_config, max_cipher_key_len));
> > +	max_auth_key_len_le = virtio_cread32_le(vdev,
> > +		offsetof(struct virtio_crypto_config, max_auth_key_len));
> > +	max_size_le = virtio_cread64_le(vdev,
> > +		offsetof(struct virtio_crypto_config, max_size));
> > +
> > +	/* Add virtio crypto device to global table */
> > +	err = virtcrypto_devmgr_add_dev(vcrypto);
> > +	if (err) {
> > +		dev_err(&vdev->dev, "Failed to add new virtio crypto device.\n");
> > +		goto free;
> > +	}
> > +	vcrypto->owner = THIS_MODULE;
> > +	vcrypto = vdev->priv = vcrypto;
> > +	vcrypto->vdev = vdev;
> > +	spin_lock_init(&vcrypto->lock);
> > +	spin_lock_init(&vcrypto->ctrl_lock);
> > +
> > +	/* Use sigle data queue as default */
> > +	vcrypto->curr_queue = 1;
> > +	vcrypto->max_cipher_key_len = le32_to_cpu(max_cipher_key_len_le);
> > +	vcrypto->max_auth_key_len = le32_to_cpu(max_auth_key_len_le);
> > +	vcrypto->max_size = le64_to_cpu(max_size_le);
> > +
> > +	err = virtcrypto_init_vqs(vcrypto);
> > +	if (err) {
> > +		dev_err(&vdev->dev, "Failed to initialize vqs.\n");
> > +		goto free_dev;
> > +	}
> > +	virtio_device_ready(vdev);
> > +
> > +	err = virtcrypto_update_status(vcrypto);
> > +	if (err) {
> > +		err = -EFAULT;
> 
> Why EFAULT?
> 
I wanted to keep the same with err's initial value.
Maybe I shouldn't change the err here but directly
assign value in virtcrypto_update_status().

EPERM is better.


> > +		goto free_vqs;
> > +	}
> > +
> > +	return 0;
> > +
> > +free_vqs:
> > +	virtcrypto_del_vqs(vcrypto);
> 
> as you called virtio_device_ready this is likely unsafe
> unless you first reset device or make sure there are
> no interrupts in some other way.
> 
Good catch. Will add reset operations.

> > +free_dev:
> > +	virtcrypto_devmgr_rm_dev(vcrypto);
> > +free:
> > +	kfree(vcrypto);
> > +	return err;
> > +}
> > +
> > +static void virtcrypto_free_unused_reqs(struct virtio_crypto *vcrypto)
> > +{
> > +	struct virtio_crypto_request *vc_req;
> > +	int i;
> > +	struct virtqueue *vq;
> > +
> > +	for (i = 0; i < vcrypto->max_data_queues; i++) {
> > +		vq = vcrypto->data_vq[i].vq;
> > +		while ((vc_req = virtqueue_detach_unused_buf(vq)) != NULL) {
> > +			kfree(vc_req->req_data);
> > +			kfree(vc_req->sgs);
> > +		}
> > +	}
> > +}
> > +
> > +static void virtcrypto_remove(struct virtio_device *vdev)
> > +{
> > +	struct virtio_crypto *vcrypto = vdev->priv;
> > +
> > +	dev_info(&vdev->dev, "Start virtcrypto_remove.\n");
> > +
> > +	if (virtcrypto_dev_started(vcrypto))
> > +		virtcrypto_dev_stop(vcrypto);
> > +	vdev->config->reset(vdev);
> > +	virtcrypto_free_unused_reqs(vcrypto);
> > +	virtcrypto_del_vqs(vcrypto);
> > +	virtcrypto_devmgr_rm_dev(vcrypto);
> > +	kfree(vcrypto);
> > +}
> > +
> > +static void virtcrypto_config_changed(struct virtio_device *vdev)
> > +{
> > +	struct virtio_crypto *vcrypto = vdev->priv;
> > +
> > +	virtcrypto_update_status(vcrypto);
> > +}
> > +
> > +#ifdef CONFIG_PM_SLEEP
> > +static int virtcrypto_freeze(struct virtio_device *vdev)
> > +{
> > +	struct virtio_crypto *vcrypto = vdev->priv;
> > +	unsigned long flags;
> > +
> > +	virtcrypto_free_unused_reqs(vcrypto);
> > +	spin_lock_irqsave(&vcrypto->lock, flags);
> > +	if (virtcrypto_dev_started(vcrypto))
> > +		virtcrypto_dev_stop(vcrypto);
> 
> This is taking mutex locks under a spinlock.
> Highly unlikely to work. Try this with lockdep
> enabled.
> 
Good catch, will do and inspect the necessity of 
the spin_lock here as well.  
> 
> > +	spin_unlock_irqrestore(&vcrypto->lock, flags);
> > +
> > +	virtcrypto_del_vqs(vcrypto);
> > +	return 0;
> > +}
> > +
> > +static int virtcrypto_restore(struct virtio_device *vdev)
> > +{
> > +	struct virtio_crypto *vcrypto = vdev->priv;
> > +	int err;
> > +
> > +	err = virtcrypto_init_vqs(vcrypto);
> > +	if (err)
> > +		return err;
> > +
> > +	virtio_device_ready(vdev);
> > +	err = virtcrypto_dev_start(vcrypto);
> > +	if (err) {
> > +		dev_err(&vdev->dev, "Failed to start virtio crypto device.\n");
> > +		return -EFAULT;
> > +	}
> > +
> > +	return 0;
> > +}
> > +#endif
> > +
> > +
> > +static unsigned int features[] = {
> > +	/* none */
> > +};
> > +
> > +static struct virtio_device_id id_table[] = {
> > +	{ VIRTIO_ID_CRYPTO, VIRTIO_DEV_ANY_ID },
> > +	{ 0 },
> > +};
> > +
> > +static struct virtio_driver virtio_crypto_driver = {
> > +	.driver.name         = KBUILD_MODNAME,
> > +	.driver.owner        = THIS_MODULE,
> > +	.feature_table       = features,
> > +	.feature_table_size  = ARRAY_SIZE(features),
> > +	.id_table            = id_table,
> > +	.probe               = virtcrypto_probe,
> > +	.remove              = virtcrypto_remove,
> > +	.config_changed = virtcrypto_config_changed,
> > +#ifdef CONFIG_PM_SLEEP
> > +	.freeze = virtcrypto_freeze,
> > +	.restore = virtcrypto_restore,
> > +#endif
> > +};
> > +
> > +module_virtio_driver(virtio_crypto_driver);
> > +
> > +MODULE_DEVICE_TABLE(virtio, id_table);
> > +MODULE_DESCRIPTION("virtio crypto device driver");
> > +MODULE_LICENSE("GPL");
> > +MODULE_AUTHOR("Gonglei <arei.gonglei@huawei.com>");
> > diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c
> b/drivers/crypto/virtio/virtio_crypto_algs.c
> > new file mode 100644
> > index 0000000..7b57584
> > --- /dev/null
> > +++ b/drivers/crypto/virtio/virtio_crypto_algs.c
> > @@ -0,0 +1,524 @@
> > + /* Algorithms supported by virtio crypto device
> > +  *
> > +  * Authors: Gonglei <arei.gonglei@huawei.com>
> > +  *
> > +  * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
> > +  *
> > +  * This program is free software; you can redistribute it and/or modify
> > +  * it under the terms of the GNU General Public License as published by
> > +  * the Free Software Foundation; either version 2 of the License, or
> > +  * (at your option) any later version.
> > +  *
> > +  * This program 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 General Public License for more details.
> > +  *
> > +  * You should have received a copy of the GNU General Public License
> > +  * along with this program; if not, see <http://www.gnu.org/licenses/>.
> > +  */
> > +
> > +#include <linux/scatterlist.h>
> > +#include <crypto/algapi.h>
> > +#include <linux/err.h>
> > +#include <crypto/scatterwalk.h>
> > +#include <linux/atomic.h>
> > +
> > +#include <uapi/linux/virtio_crypto.h>
> > +#include "virtio_crypto_common.h"
> > +
> > +static DEFINE_MUTEX(algs_lock);
> > +static unsigned int virtio_crypto_active_devs;
> > +
> > +static u64 virtio_crypto_alg_sg_nents_length(struct scatterlist *sg)
> > +{
> > +	u64 total = 0;
> > +
> > +	for (total = 0; sg; sg = sg_next(sg))
> > +		total += sg->length;
> > +
> > +	return total;
> > +}
> > +
> > +static int virtio_crypto_alg_validate_key(int key_len, int *alg)
> > +{
> > +	switch (key_len) {
> > +	case AES_KEYSIZE_128:
> > +	case AES_KEYSIZE_192:
> > +	case AES_KEYSIZE_256:
> > +		*alg = VIRTIO_CRYPTO_CIPHER_AES_CBC;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +	return 0;
> > +}
> > +
> > +static int virtio_crypto_alg_ablkcipher_init_session(
> > +		struct virtio_crypto_ablkcipher_ctx *ctx,
> > +		int alg, const uint8_t *key,
> > +		unsigned int keylen,
> > +		int encrypt)
> > +{
> > +	struct scatterlist outhdr, key_sg, inhdr, *sgs[3];
> > +	unsigned int tmp;
> > +	struct virtio_crypto *vcrypto = ctx->vcrypto;
> > +	int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT :
> VIRTIO_CRYPTO_OP_DECRYPT;
> > +	int err;
> > +	unsigned int num_out = 0, num_in = 0;
> > +
> > +	/*
> > +	 * Avoid to do DMA from the stack, switch to using
> > +	 * dynamically-allocated for the key
> > +	 */
> > +	uint8_t *cipher_key = kmalloc(keylen, GFP_ATOMIC);
> 
> Is all crypto using GFP_ATOMIC like this?
> Is there code that recovers if this fails?
> 
The crypto subsystem has a flag named CRYPTO_TFM_REQ_MAY_SLEEP to
decide to use GFP_KERENL or GFP_ATOMIC. We don't set the flag
for virtio-crypto device, so I use GFP_ATOMIC likes qat_alg.c

> 
> > +
> > +	if (!cipher_key)
> > +		return -ENOMEM;
> > +
> > +	memcpy(cipher_key, key, keylen);
> > +
> > +	spin_lock(&vcrypto->ctrl_lock);
> > +	/* Pad ctrl header */
> > +	vcrypto->ctrl.header.opcode =
> > +		cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION);
> > +	vcrypto->ctrl.header.algo = cpu_to_le32((uint32_t)alg);
> 
> why cast here?
> 
Will change the type of alg at the beginning of definition,
remove the cast as well.

> > +	/* Set the default dataqueue id to 0 */
> > +	vcrypto->ctrl.header.queue_id = 0;
> > +
> > +	vcrypto->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
> > +	/* Pad cipher's parameters */
> > +	vcrypto->ctrl.u.sym_create_session.op_type =
> > +		cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
> > +	vcrypto->ctrl.u.sym_create_session.u.cipher.para.algo =
> > +		vcrypto->ctrl.header.algo;
> > +	vcrypto->ctrl.u.sym_create_session.u.cipher.para.keylen =
> > +		cpu_to_le32(keylen);
> > +	vcrypto->ctrl.u.sym_create_session.u.cipher.para.op =
> > +		cpu_to_le32(op);
> > +
> > +	sg_init_one(&outhdr, &vcrypto->ctrl, sizeof(vcrypto->ctrl));
> > +	sgs[num_out++] = &outhdr;
> > +
> > +	/* Set key */
> > +	sg_init_one(&key_sg, cipher_key, keylen);
> > +	sgs[num_out++] = &key_sg;
> > +
> > +	/* Return status and session id back */
> > +	sg_init_one(&inhdr, &vcrypto->input, sizeof(vcrypto->input));
> > +	sgs[num_out + num_in++] = &inhdr;
> > +
> > +	err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out,
> > +				num_in, vcrypto, GFP_ATOMIC);
> > +	if (err < 0) {
> > +		spin_unlock(&vcrypto->ctrl_lock);
> > +		kfree(cipher_key);
> > +		return err;
> > +	}
> > +	virtqueue_kick(vcrypto->ctrl_vq);
> > +
> > +	/*
> > +	 * Spin for a response, the kick causes an ioport write, trapping
> > +	 * into the hypervisor, so the request should be handled immediately.
> > +	 */
> > +	while (!virtqueue_get_buf(vcrypto->ctrl_vq, &tmp) &&
> > +	       !virtqueue_is_broken(vcrypto->ctrl_vq))
> > +		cpu_relax();
> > +
> > +	if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) {
> > +		spin_unlock(&vcrypto->ctrl_lock);
> > +		pr_err("virtio_crypto: Create session failed status: %u\n",
> > +			le32_to_cpu(vcrypto->input.status));
> > +		kfree(cipher_key);
> > +		return -EINVAL;
> > +	}
> > +	spin_unlock(&vcrypto->ctrl_lock);
> > +
> > +	spin_lock(&ctx->lock);
> > +	if (encrypt)
> > +		ctx->enc_sess_info.session_id =
> > +			le64_to_cpu(vcrypto->input.session_id);
> > +	else
> > +		ctx->dec_sess_info.session_id =
> > +			le64_to_cpu(vcrypto->input.session_id);
> > +	spin_unlock(&ctx->lock);
> > +
> > +	kfree(cipher_key);
> > +	return 0;
> > +}
> > +
> > +static int virtio_crypto_alg_ablkcipher_close_session(
> > +		struct virtio_crypto_ablkcipher_ctx *ctx,
> > +		int encrypt)
> > +{
> > +	struct scatterlist outhdr, status_sg, *sgs[2];
> > +	unsigned int tmp;
> > +	struct virtio_crypto_destroy_session_req *destroy_session;
> > +	struct virtio_crypto *vcrypto = ctx->vcrypto;
> > +	int err;
> > +	unsigned int num_out = 0, num_in = 0;
> > +
> > +	spin_lock(&vcrypto->ctrl_lock);
> > +	vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR;
> > +	/* Pad ctrl header */
> > +	vcrypto->ctrl.header.opcode =
> > +		cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION);
> > +	/* Set the default virtqueue id to 0 */
> > +	vcrypto->ctrl.header.queue_id = 0;
> > +
> > +	destroy_session = &vcrypto->ctrl.u.destroy_session;
> > +
> > +	if (encrypt)
> > +		destroy_session->session_id =
> > +			cpu_to_le64(ctx->enc_sess_info.session_id);
> > +	else
> > +		destroy_session->session_id =
> > +			cpu_to_le64(ctx->dec_sess_info.session_id);
> > +
> > +	sg_init_one(&outhdr, &vcrypto->ctrl, sizeof(vcrypto->ctrl));
> > +	sgs[num_out++] = &outhdr;
> > +
> > +	/* Return status and session id back */
> > +	sg_init_one(&status_sg, &vcrypto->ctrl_status.status,
> > +		sizeof(vcrypto->ctrl_status.status));
> > +	sgs[num_out + num_in++] = &status_sg;
> > +
> > +	err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out,
> > +			num_in, vcrypto, GFP_ATOMIC);
> > +	if (err < 0) {
> > +		spin_unlock(&vcrypto->ctrl_lock);
> > +		return err;
> > +	}
> > +	virtqueue_kick(vcrypto->ctrl_vq);
> > +
> > +	while (!virtqueue_get_buf(vcrypto->ctrl_vq, &tmp) &&
> > +	       !virtqueue_is_broken(vcrypto->ctrl_vq))
> > +		cpu_relax();
> > +
> > +	if (vcrypto->ctrl_status.status != VIRTIO_CRYPTO_OK) {
> > +		spin_unlock(&vcrypto->ctrl_lock);
> > +		pr_err("virtio_crypto: Close session failed status: %u, session_id:
> 0x%llx\n",
> > +			vcrypto->ctrl_status.status,
> > +			destroy_session->session_id);
> > +
> > +		return -EINVAL;
> > +	}
> > +	spin_unlock(&vcrypto->ctrl_lock);
> > +
> > +	return 0;
> > +}
> > +
> > +static int virtio_crypto_alg_ablkcipher_init_sessions(
> > +		struct virtio_crypto_ablkcipher_ctx *ctx,
> > +		const uint8_t *key, unsigned int keylen)
> > +{
> > +	int alg;
> > +	int ret;
> > +	struct virtio_crypto *vcrypto = ctx->vcrypto;
> > +
> > +	if (keylen > vcrypto->max_cipher_key_len) {
> > +		pr_err("virtio_crypto: the key is too long\n");
> > +		goto bad_key;
> > +	}
> > +
> > +	if (virtio_crypto_alg_validate_key(keylen, &alg))
> > +		goto bad_key;
> > +
> > +	/* Create encryption session */
> > +	ret = virtio_crypto_alg_ablkcipher_init_session(ctx,
> > +			alg, key, keylen, 1);
> > +	if (ret)
> > +		return ret;
> > +	/* Create decryption session */
> > +	ret = virtio_crypto_alg_ablkcipher_init_session(ctx,
> > +			alg, key, keylen, 0);
> > +	if (ret) {
> > +		virtio_crypto_alg_ablkcipher_close_session(ctx, 1);
> > +		return ret;
> > +	}
> > +	return 0;
> > +
> > +bad_key:
> > +	crypto_tfm_set_flags(ctx->tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
> > +	return -EINVAL;
> > +}
> > +
> > +/* Note: kernel crypto API realization */
> > +static int virtio_crypto_ablkcipher_setkey(struct crypto_ablkcipher *tfm,
> > +					 const uint8_t *key,
> > +					 unsigned int keylen)
> > +{
> > +	struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(tfm);
> > +	int ret;
> > +
> > +	spin_lock(&ctx->lock);
> > +
> > +	if (!ctx->vcrypto) {
> > +		/* New key */
> > +		int node = virtio_crypto_get_current_node();
> > +		struct virtio_crypto *vcrypto =
> > +				      virtcrypto_get_dev_node(node);
> > +		if (!vcrypto) {
> > +			vcrypto = virtcrypto_devmgr_get_first();
> > +			if (!vcrypto) {
> > +				pr_err("virtio_crypto: Could not find a virtio device in the
> system");
> > +				spin_unlock(&ctx->lock);
> > +				return -ENODEV;
> > +			}
> > +		}
> > +
> > +		ctx->vcrypto = vcrypto;
> > +	}
> > +	spin_unlock(&ctx->lock);
> > +
> > +	ret = virtio_crypto_alg_ablkcipher_init_sessions(ctx, key, keylen);
> > +	if (ret) {
> > +		virtcrypto_dev_put(ctx->vcrypto);
> > +		ctx->vcrypto = NULL;
> > +
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int
> > +__virtio_crypto_ablkcipher_do_req(struct virtio_crypto_request *vc_req,
> > +		struct ablkcipher_request *req,
> > +		struct data_queue *data_vq,
> > +		__u8 op)
> > +{
> > +	struct virtio_crypto_ablkcipher_ctx *ctx = vc_req->ablkcipher_ctx;
> > +	struct virtio_crypto *vcrypto = ctx->vcrypto;
> > +	struct virtio_crypto_op_data_req *req_data;
> > +	int src_nents, dst_nents;
> > +	int err;
> > +	unsigned long flags;
> > +	struct scatterlist outhdr, iv_sg, status_sg, **sgs;
> > +	int i;
> > +	u64 dst_len;
> > +	unsigned int num_out = 0, num_in = 0;
> > +	int sg_total;
> > +
> > +	src_nents = sg_nents_for_len(req->src, req->nbytes);
> > +	dst_nents = sg_nents(req->dst);
> > +
> > +	pr_debug("virtio_crypto: Number of sgs (src_nents: %d,
> dst_nents: %d)\n",
> > +			src_nents, dst_nents);
> > +
> > +	/* Why 3?  outhdr + iv + inhdr */
> > +	sg_total = src_nents + dst_nents + 3;
> > +	sgs = kzalloc_node(sg_total * sizeof(*sgs), GFP_ATOMIC,
> > +				dev_to_node(&vcrypto->vdev->dev));
> > +	if (!sgs)
> > +		return -ENOMEM;
> > +
> > +	req_data = kzalloc_node(sizeof(*req_data), GFP_ATOMIC,
> > +				dev_to_node(&vcrypto->vdev->dev));
> > +	if (!req_data) {
> > +		kfree(sgs);
> > +		return -ENOMEM;
> > +	}
> > +
> > +	vc_req->req_data = req_data;
> > +	vc_req->type = VIRTIO_CRYPTO_SYM_OP_CIPHER;
> > +	/* Head of operation */
> > +	if (op) {
> > +		req_data->header.session_id =
> > +			cpu_to_le64(ctx->enc_sess_info.session_id);
> > +		req_data->header.opcode =
> > +			cpu_to_le32(VIRTIO_CRYPTO_CIPHER_ENCRYPT);
> > +	} else {
> > +		req_data->header.session_id =
> > +			cpu_to_le64(ctx->dec_sess_info.session_id);
> > +	    req_data->header.opcode =
> > +			cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DECRYPT);
> > +	}
> > +	req_data->u.sym_req.op_type =
> cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
> > +	req_data->u.sym_req.u.cipher.para.iv_len =
> cpu_to_le32(AES_BLOCK_SIZE);
> > +	req_data->u.sym_req.u.cipher.para.src_data_len =
> > +			cpu_to_le32(req->nbytes);
> > +
> > +	dst_len = virtio_crypto_alg_sg_nents_length(req->dst);
> > +	if (unlikely(dst_len > U32_MAX)) {
> > +		pr_err("virtio_crypto: The dst_len is beyond U32_MAX\n");
> > +		err = -EINVAL;
> > +		goto free;
> > +	}
> > +
> > +	pr_debug("virtio_crypto: src_len: %u, dst_len: %llu\n",
> > +			req->nbytes, dst_len);
> > +
> > +	if (unlikely(req->nbytes + dst_len + AES_BLOCK_SIZE +
> > +		sizeof(vc_req->status) > vcrypto->max_size)) {
> > +		pr_err("virtio_crypto: The length is too big\n");
> > +		err = -EINVAL;
> > +		goto free;
> > +	}
> > +
> > +	req_data->u.sym_req.u.cipher.para.dst_data_len =
> > +			cpu_to_le32((uint32_t)dst_len);
> > +
> > +	/* Outhdr */
> > +	sg_init_one(&outhdr, req_data, sizeof(*req_data));
> > +	sgs[num_out++] = &outhdr;
> > +
> > +	/* IV */
> > +	sg_init_one(&iv_sg, req->info, AES_BLOCK_SIZE);
> > +	sgs[num_out++] = &iv_sg;
> > +
> > +	/* Source data */
> > +	for (i = 0; i < src_nents; i++)
> > +		sgs[num_out++] = &req->src[i];
> > +
> > +	/* Destination data */
> > +	for (i = 0; i < dst_nents; i++)
> > +		sgs[num_out + num_in++] = &req->dst[i];
> > +
> > +	/* Status */
> > +	sg_init_one(&status_sg, &vc_req->status, sizeof(vc_req->status));
> > +	sgs[num_out + num_in++] = &status_sg;
> > +
> > +	vc_req->sgs = sgs;
> > +
> > +	spin_lock_irqsave(&vcrypto->lock, flags);
> > +	err = virtqueue_add_sgs(data_vq->vq, sgs, num_out,
> > +				num_in, vc_req, GFP_ATOMIC);
> > +	spin_unlock_irqrestore(&vcrypto->lock, flags);
> > +	if (unlikely(err < 0))
> > +		goto free;
> > +
> > +	return 0;
> > +
> > +free:
> > +	kfree(req_data);
> > +	kfree(sgs);
> > +	return err;
> > +}
> > +
> > +static int virtio_crypto_ablkcipher_encrypt(struct ablkcipher_request *req)
> > +{
> > +	struct crypto_ablkcipher *atfm = crypto_ablkcipher_reqtfm(req);
> > +	struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(atfm);
> > +	struct virtio_crypto_request *vc_req = ablkcipher_request_ctx(req);
> > +	struct virtio_crypto *vcrypto = ctx->vcrypto;
> > +	int ret;
> > +	/* Use the first data virtqueue as default */
> > +	struct data_queue *data_vq = &vcrypto->data_vq[0];
> > +
> > +	vc_req->ablkcipher_ctx = ctx;
> > +	vc_req->ablkcipher_req = req;
> > +	ret = __virtio_crypto_ablkcipher_do_req(vc_req, req, data_vq, 1);
> > +	if (ret < 0) {
> > +		pr_err("virtio_crypto: Encryption failed!\n");
> > +		return ret;
> > +	}
> > +	virtqueue_kick(data_vq->vq);
> > +
> > +	return -EINPROGRESS;
> > +}
> > +
> > +static int virtio_crypto_ablkcipher_decrypt(struct ablkcipher_request *req)
> > +{
> > +	struct crypto_ablkcipher *atfm = crypto_ablkcipher_reqtfm(req);
> > +	struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(atfm);
> > +	struct virtio_crypto_request *vc_req = ablkcipher_request_ctx(req);
> > +	struct virtio_crypto *vcrypto = ctx->vcrypto;
> > +	int ret;
> > +	/* Use the first data virtqueue as default */
> > +	struct data_queue *data_vq = &vcrypto->data_vq[0];
> > +
> > +	vc_req->ablkcipher_ctx = ctx;
> > +	vc_req->ablkcipher_req = req;
> > +
> > +	ret = __virtio_crypto_ablkcipher_do_req(vc_req, req, data_vq, 0);
> > +	if (ret < 0) {
> > +		pr_err("virtio_crypto: Decryption failed!\n");
> > +		return ret;
> > +	}
> > +	virtqueue_kick(data_vq->vq);
> > +
> > +	return -EINPROGRESS;
> > +}
> > +
> > +static int virtio_crypto_ablkcipher_init(struct crypto_tfm *tfm)
> > +{
> > +	struct virtio_crypto_ablkcipher_ctx *ctx = crypto_tfm_ctx(tfm);
> > +
> > +	spin_lock_init(&ctx->lock);
> > +	tfm->crt_ablkcipher.reqsize = sizeof(struct virtio_crypto_request);
> > +	ctx->tfm = tfm;
> > +
> > +	return 0;
> > +}
> > +
> > +static void virtio_crypto_ablkcipher_exit(struct crypto_tfm *tfm)
> > +{
> > +	struct virtio_crypto_ablkcipher_ctx *ctx = crypto_tfm_ctx(tfm);
> > +
> > +	if (!ctx->vcrypto)
> > +		return;
> > +
> > +	virtio_crypto_alg_ablkcipher_close_session(ctx, 1);
> > +	virtio_crypto_alg_ablkcipher_close_session(ctx, 0);
> > +	virtcrypto_dev_put(ctx->vcrypto);
> > +	ctx->vcrypto = NULL;
> > +}
> > +
> > +static struct crypto_alg virtio_crypto_algs[] = { {
> > +	.cra_name = "cbc(aes)",
> > +	.cra_driver_name = "virtio_crypto_aes_cbc",
> > +	.cra_priority = 4001,
> > +	.cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC,
> > +	.cra_blocksize = AES_BLOCK_SIZE,
> > +	.cra_ctxsize  = sizeof(struct virtio_crypto_ablkcipher_ctx),
> > +	.cra_alignmask = 0,
> > +	.cra_module = THIS_MODULE,
> > +	.cra_type = &crypto_ablkcipher_type,
> > +	.cra_init = virtio_crypto_ablkcipher_init,
> > +	.cra_exit = virtio_crypto_ablkcipher_exit,
> > +	.cra_u = {
> > +	   .ablkcipher = {
> > +			.setkey = virtio_crypto_ablkcipher_setkey,
> > +			.decrypt = virtio_crypto_ablkcipher_decrypt,
> > +			.encrypt = virtio_crypto_ablkcipher_encrypt,
> > +			.min_keysize = AES_MIN_KEY_SIZE,
> > +			.max_keysize = AES_MAX_KEY_SIZE,
> > +			.ivsize = AES_BLOCK_SIZE,
> > +		},
> > +	},
> > +} };
> > +
> > +int virtio_crypto_algs_register(void)
> > +{
> > +	int ret = 0, i;
> > +
> > +	mutex_lock(&algs_lock);
> > +	if (++virtio_crypto_active_devs != 1)
> > +		goto unlock;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(virtio_crypto_algs); i++) {
> > +		virtio_crypto_algs[i].cra_flags =
> > +			     CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC;
> > +	}
> > +
> > +	ret = crypto_register_algs(virtio_crypto_algs,
> > +			ARRAY_SIZE(virtio_crypto_algs));
> > +
> > +unlock:
> > +	mutex_unlock(&algs_lock);
> > +	return ret;
> > +}
> > +
> > +void virtio_crypto_algs_unregister(void)
> > +{
> > +	mutex_lock(&algs_lock);
> > +	if (--virtio_crypto_active_devs != 0)
> > +		goto unlock;
> > +
> > +	crypto_unregister_algs(virtio_crypto_algs,
> > +			ARRAY_SIZE(virtio_crypto_algs));
> > +
> > +unlock:
> > +	mutex_unlock(&algs_lock);
> > +}
> > diff --git a/drivers/crypto/virtio/virtio_crypto_common.h
> b/drivers/crypto/virtio/virtio_crypto_common.h
> > new file mode 100644
> > index 0000000..a599733
> > --- /dev/null
> > +++ b/drivers/crypto/virtio/virtio_crypto_common.h
> > @@ -0,0 +1,124 @@
> > +/* Common header for Virtio crypto device.
> > + *
> > + * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
> > + *
> > + * This program is free software; you can redistribute it and/or modify
> > + * it under the terms of the GNU General Public License as published by
> > + * the Free Software Foundation; either version 2 of the License, or
> > + * (at your option) any later version.
> > + *
> > + * This program 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 General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#ifndef _VIRITO_CRYPTO_COMMON_H
> > +#define _VIRITO_CRYPTO_COMMON_H
> > +
> > +#include <linux/virtio.h>
> > +#include <linux/crypto.h>
> > +#include <linux/spinlock.h>
> > +#include <crypto/aead.h>
> > +#include <crypto/aes.h>
> > +#include <crypto/authenc.h>
> > +
> > +
> > +/* Internal representation of a data virtqueue */
> > +struct data_queue {
> > +	/* Virtqueue associated with this send _queue */
> > +	struct virtqueue *vq;
> > +
> > +	/* Name of the tx queue: dataq.$index */
> > +	char name[32];
> > +};
> > +
> > +struct virtio_crypto {
> > +	struct virtio_device *vdev;
> > +	struct virtqueue *ctrl_vq;
> > +	struct data_queue *data_vq;
> > +
> > +	/* To protect the vq operations for the dataq */
> > +	spinlock_t lock;
> > +
> > +	/* To protect the vq operations for the controlq */
> > +	spinlock_t ctrl_lock;
> > +
> > +	/* Maximum of data queues supported by the device */
> > +	u32 max_data_queues;
> > +
> > +	/* Number of queue currently used by the driver */
> > +	u32 curr_queue;
> > +
> > +	/* Maximum length of cipher key */
> > +	u32 max_cipher_key_len;
> > +	/* Maximum length of authenticated key */
> > +	u32 max_auth_key_len;
> > +	/* Maximum size of per request */
> > +	u64 max_size;
> > +
> > +	/* Control VQ buffers: protected by the ctrl_lock */
> > +	struct virtio_crypto_op_ctrl_req ctrl;
> > +	struct virtio_crypto_session_input input;
> > +	struct virtio_crypto_inhdr ctrl_status;
> > +
> > +	unsigned long status;
> > +	atomic_t ref_count;
> > +	struct list_head list;
> > +	struct module *owner;
> > +	uint8_t dev_id;
> > +
> > +	/* Does the affinity hint is set for virtqueues? */
> > +	bool affinity_hint_set;
> > +};
> > +
> > +struct virtio_crypto_sym_session_info {
> > +	/* Backend session id, which come from the host side */
> > +	__u64 session_id;
> > +};
> > +
> > +struct virtio_crypto_ablkcipher_ctx {
> > +	struct virtio_crypto *vcrypto;
> > +	struct crypto_tfm *tfm;
> > +
> > +	struct virtio_crypto_sym_session_info enc_sess_info;
> > +	struct virtio_crypto_sym_session_info dec_sess_info;
> > +
> > +	/* Protects virtio_crypto_ablkcipher_ctx struct */
> > +	spinlock_t lock;
> > +};
> > +
> > +struct virtio_crypto_request {
> > +	/* Cipher or aead */
> > +	uint32_t type;
> > +	uint8_t status;
> > +	struct virtio_crypto_ablkcipher_ctx *ablkcipher_ctx;
> > +	struct ablkcipher_request *ablkcipher_req;
> > +	struct virtio_crypto_op_data_req *req_data;
> > +	struct scatterlist **sgs;
> > +};
> > +
> > +int virtcrypto_devmgr_add_dev(struct virtio_crypto *vcrypto_dev);
> > +struct list_head *virtcrypto_devmgr_get_head(void);
> > +void virtcrypto_devmgr_rm_dev(struct virtio_crypto *vcrypto_dev);
> > +struct virtio_crypto *virtcrypto_devmgr_get_first(void);
> > +int virtcrypto_dev_in_use(struct virtio_crypto *vcrypto_dev);
> > +int virtcrypto_dev_get(struct virtio_crypto *vcrypto_dev);
> > +void virtcrypto_dev_put(struct virtio_crypto *vcrypto_dev);
> > +int virtcrypto_dev_started(struct virtio_crypto *vcrypto_dev);
> > +struct virtio_crypto *virtcrypto_get_dev_node(int node);
> > +int virtcrypto_dev_start(struct virtio_crypto *vcrypto);
> > +void virtcrypto_dev_stop(struct virtio_crypto *vcrypto);
> > +
> > +static inline int virtio_crypto_get_current_node(void)
> > +{
> > +	return topology_physical_package_id(smp_processor_id());
> > +}
> > +
> > +int virtio_crypto_algs_register(void);
> > +void virtio_crypto_algs_unregister(void);
> > +
> > +#endif /* _VIRITO_CRYPTO_COMMON_H */
> > diff --git a/drivers/crypto/virtio/virtio_crypto_mgr.c
> b/drivers/crypto/virtio/virtio_crypto_mgr.c
> > new file mode 100644
> > index 0000000..5b7260c
> > --- /dev/null
> > +++ b/drivers/crypto/virtio/virtio_crypto_mgr.c
> > @@ -0,0 +1,258 @@
> > + /* Management for virtio crypto devices (refer to adf_dev_mgr.c)
> > +  *
> > +  * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
> > +  *
> > +  * This program is free software; you can redistribute it and/or modify
> > +  * it under the terms of the GNU General Public License as published by
> > +  * the Free Software Foundation; either version 2 of the License, or
> > +  * (at your option) any later version.
> > +  *
> > +  * This program 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 General Public License for more details.
> > +  *
> > +  * You should have received a copy of the GNU General Public License
> > +  * along with this program; if not, see <http://www.gnu.org/licenses/>.
> > +  */
> > +
> > +#include <linux/mutex.h>
> > +#include <linux/list.h>
> > +#include <linux/module.h>
> > +
> > +#include <uapi/linux/virtio_crypto.h>
> > +#include "virtio_crypto_common.h"
> > +
> > +static LIST_HEAD(virtio_crypto_table);
> > +static DEFINE_MUTEX(table_lock);
> > +static uint32_t num_devices;
> > +
> > +#define VIRTIO_CRYPTO_MAX_DEVICES 32
> > +
> > +
> > +/*
> > + * virtcrypto_devmgr_add_dev() - Add vcrypto_dev to the acceleration
> > + * framework.
> > + * @vcrypto_dev:  Pointer to virtio crypto device.
> > + *
> > + * Function adds virtio crypto device to the global list.
> > + * To be used by virtio crypto device specific drivers.
> > + *
> > + * Return: 0 on success, error code othewise.
> > + */
> > +int virtcrypto_devmgr_add_dev(struct virtio_crypto *vcrypto_dev)
> > +{
> > +	struct list_head *itr;
> > +
> > +	if (num_devices == VIRTIO_CRYPTO_MAX_DEVICES) {
> > +		pr_info("virtio_crypto: only support up to %d devices\n",
> > +			    VIRTIO_CRYPTO_MAX_DEVICES);
> > +		return -EFAULT;
> > +	}
> > +
> > +	mutex_lock(&table_lock);
> > +	list_for_each(itr, &virtio_crypto_table) {
> > +		struct virtio_crypto *ptr =
> > +				list_entry(itr, struct virtio_crypto, list);
> > +
> > +		if (ptr == vcrypto_dev) {
> > +			mutex_unlock(&table_lock);
> > +			return -EEXIST;
> > +		}
> > +	}
> > +	atomic_set(&vcrypto_dev->ref_count, 0);
> > +	list_add_tail(&vcrypto_dev->list, &virtio_crypto_table);
> > +	vcrypto_dev->dev_id = num_devices++;
> > +	mutex_unlock(&table_lock);
> > +	return 0;
> > +}
> > +
> > +struct list_head *virtcrypto_devmgr_get_head(void)
> > +{
> > +	return &virtio_crypto_table;
> > +}
> > +
> > +/*
> > + * virtcrypto_devmgr_rm_dev() - Remove vcrypto_dev from the acceleration
> > + * framework.
> > + * @vcrypto_dev:  Pointer to virtio crypto device.
> > + *
> > + * Function removes virtio crypto device from the acceleration framework.
> > + * To be used by virtio crypto device specific drivers.
> > + *
> > + * Return: void
> > + */
> > +void virtcrypto_devmgr_rm_dev(struct virtio_crypto *vcrypto_dev)
> > +{
> > +	mutex_lock(&table_lock);
> > +	list_del(&vcrypto_dev->list);
> > +	num_devices--;
> > +	mutex_unlock(&table_lock);
> > +}
> > +
> > +/*
> > + * virtcrypto_devmgr_get_first()
> > + *
> > + * Function returns the first virtio crypto device from the acceleration
> > + * framework.
> > + *
> > + * To be used by virtio crypto device specific drivers.
> > + *
> > + * Return: pointer to vcrypto_dev or NULL if not found.
> > + */
> > +struct virtio_crypto *virtcrypto_devmgr_get_first(void)
> > +{
> > +	struct virtio_crypto *dev = NULL;
> > +
> > +	if (!list_empty(&virtio_crypto_table))
> > +		dev = list_first_entry(&virtio_crypto_table,
> > +					struct virtio_crypto,
> > +				    list);
> > +	return dev;
> > +}
> > +
> > +/*
> > + * virtcrypto_dev_in_use() - Check whether vcrypto_dev is currently in use
> > + * @vcrypto_dev: Pointer to virtio crypto device.
> > + *
> > + * To be used by virtio crypto device specific drivers.
> > + *
> > + * Return: 1 when device is in use, 0 otherwise.
> > + */
> > +int virtcrypto_dev_in_use(struct virtio_crypto *vcrypto_dev)
> > +{
> > +	return atomic_read(&vcrypto_dev->ref_count) != 0;
> > +}
> > +
> > +/*
> > + * virtcrypto_dev_get() - Increment vcrypto_dev reference count
> > + * @vcrypto_dev: Pointer to virtio crypto device.
> > + *
> > + * Increment the vcrypto_dev refcount and if this is the first time
> > + * incrementing it during this period the vcrypto_dev is in use,
> > + * increment the module refcount too.
> > + * To be used by virtio crypto device specific drivers.
> > + *
> > + * Return: 0 when successful, EFAULT when fail to bump module refcount
> > + */
> > +int virtcrypto_dev_get(struct virtio_crypto *vcrypto_dev)
> > +{
> > +	if (atomic_add_return(1, &vcrypto_dev->ref_count) == 1)
> > +		if (!try_module_get(vcrypto_dev->owner))
> > +			return -EFAULT;
> > +	return 0;
> > +}
> > +
> > +/*
> > + * virtcrypto_dev_put() - Decrement vcrypto_dev reference count
> > + * @vcrypto_dev: Pointer to virtio crypto device.
> > + *
> > + * Decrement the vcrypto_dev refcount and if this is the last time
> > + * decrementing it during this period the vcrypto_dev is in use,
> > + * decrement the module refcount too.
> > + * To be used by virtio crypto device specific drivers.
> > + *
> > + * Return: void
> > + */
> > +void virtcrypto_dev_put(struct virtio_crypto *vcrypto_dev)
> > +{
> > +	if (atomic_sub_return(1, &vcrypto_dev->ref_count) == 0)
> > +		module_put(vcrypto_dev->owner);
> > +}
> > +
> > +/*
> > + * virtcrypto_dev_started() - Check whether device has started
> > + * @vcrypto_dev: Pointer to virtio crypto device.
> > + *
> > + * To be used by virtio crypto device specific drivers.
> > + *
> > + * Return: 1 when the device has started, 0 otherwise
> > + */
> > +int virtcrypto_dev_started(struct virtio_crypto *vcrypto_dev)
> > +{
> > +	return (vcrypto_dev->status & VIRTIO_CRYPTO_S_HW_READY);
> > +}
> > +
> > +/*
> > + * virtcrypto_get_dev_node() - Get vcrypto_dev on the node.
> > + * @node:  Node id the driver works.
> > + *
> > + * Function returns the virtio crypto device used fewest on the node.
> > + *
> > + * To be used by virtio crypto device specific drivers.
> > + *
> > + * Return: pointer to vcrypto_dev or NULL if not found.
> > + */
> > +struct virtio_crypto *virtcrypto_get_dev_node(int node)
> > +{
> > +	struct virtio_crypto *vcrypto_dev = NULL, *tmp_dev;
> > +	unsigned long best = ~0;
> > +	unsigned long ctr;
> > +
> > +	list_for_each_entry(tmp_dev, virtcrypto_devmgr_get_head(), list) {
> > +
> > +		if ((node == dev_to_node(&tmp_dev->vdev->dev) ||
> > +		     dev_to_node(&tmp_dev->vdev->dev) < 0) &&
> > +		    virtcrypto_dev_started(tmp_dev)) {
> > +			ctr = atomic_read(&tmp_dev->ref_count);
> > +			if (best > ctr) {
> > +				vcrypto_dev = tmp_dev;
> > +				best = ctr;
> > +			}
> > +		}
> > +	}
> > +
> > +	if (!vcrypto_dev) {
> > +		pr_info("virtio_crypto: Could not find a device on node %d\n",
> > +				node);
> > +		/* Get any started device */
> > +		list_for_each_entry(tmp_dev,
> > +				virtcrypto_devmgr_get_head(), list) {
> > +			if (virtcrypto_dev_started(tmp_dev)) {
> > +				vcrypto_dev = tmp_dev;
> > +				break;
> > +			}
> > +		}
> > +	}
> > +
> > +	if (!vcrypto_dev)
> > +		return NULL;
> > +
> > +	virtcrypto_dev_get(vcrypto_dev);
> > +	return vcrypto_dev;
> > +}
> > +
> > +/*
> > + * virtcrypto_dev_start() - Start virtio crypto device
> > + * @vcrypto:    Pointer to virtio crypto device.
> > + *
> > + * Function notifies all the registered services that the virtio crypto device
> > + * is ready to be used.
> > + * To be used by virtio crypto device specific drivers.
> > + *
> > + * Return: 0 on success, EFAULT when fail to register algorithms
> > + */
> > +int virtcrypto_dev_start(struct virtio_crypto *vcrypto)
> > +{
> > +	if (virtio_crypto_algs_register()) {
> > +		pr_err("virtio_crypto: Failed to register crypto algs\n");
> > +		return -EFAULT;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/*
> > + * virtcrypto_dev_stop() - Stop virtio crypto device
> > + * @vcrypto:    Pointer to virtio crypto device.
> > + *
> > + * Function notifies all the registered services that the virtio crypto device
> > + * is ready to be used.
> > + * To be used by virtio crypto device specific drivers.
> > + *
> > + * Return: void
> > + */
> > +void virtcrypto_dev_stop(struct virtio_crypto *vcrypto)
> > +{
> > +	virtio_crypto_algs_unregister();
> > +}
> > diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> > index cd2be1c..4bdb84c 100644
> > --- a/include/uapi/linux/Kbuild
> > +++ b/include/uapi/linux/Kbuild
> > @@ -460,6 +460,7 @@ header-y += virtio_rng.h
> >  header-y += virtio_scsi.h
> >  header-y += virtio_types.h
> >  header-y += virtio_vsock.h
> > +header-y += virtio_crypto.h
> >  header-y += vm_sockets.h
> >  header-y += vt.h
> >  header-y += vtpm_proxy.h
> > diff --git a/include/uapi/linux/virtio_crypto.h
> b/include/uapi/linux/virtio_crypto.h
> > new file mode 100644
> > index 0000000..abc2cf5
> > --- /dev/null
> > +++ b/include/uapi/linux/virtio_crypto.h
> > @@ -0,0 +1,435 @@
> > +#ifndef _VIRTIO_CRYPTO_H
> > +#define _VIRTIO_CRYPTO_H
> > +/* This header is BSD licensed so anyone can use the definitions to
> implement
> > + * compatible drivers/servers.
> > + *
> > + * Redistribution and use in source and binary forms, with or without
> > + * modification, are permitted provided that the following conditions
> > + * are met:
> > + * 1. Redistributions of source code must retain the above copyright
> > + *    notice, this list of conditions and the following disclaimer.
> > + * 2. Redistributions in binary form must reproduce the above copyright
> > + *    notice, this list of conditions and the following disclaimer in the
> > + *    documentation and/or other materials provided with the distribution.
> > + * 3. Neither the name of IBM nor the names of its contributors
> > + *    may be used to endorse or promote products derived from this
> software
> > + *    without specific prior written permission.
> > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> CONTRIBUTORS
> > + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT
> NOT
> > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND
> FITNESS
> > + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL
> IBM OR
> > + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT
> NOT
> > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
> OF
> > + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
> CAUSED AND
> > + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
> LIABILITY,
> > + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY
> WAY OUT
> > + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > + * SUCH DAMAGE.
> > + */
> > +#include <linux/types.h>
> > +#include <linux/virtio_types.h>
> > +#include <linux/virtio_ids.h>
> > +#include <linux/virtio_config.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
> > +
> > +#define VIRTIO_CRYPTO_OPCODE(service, op)   (((service) << 8) | (op))
> > +
> > +struct virtio_crypto_ctrl_header {
> > +#define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION \
> > +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x02)
> > +#define VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION \
> > +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x03)
> > +#define VIRTIO_CRYPTO_HASH_CREATE_SESSION \
> > +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x02)
> > +#define VIRTIO_CRYPTO_HASH_DESTROY_SESSION \
> > +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x03)
> > +#define VIRTIO_CRYPTO_MAC_CREATE_SESSION \
> > +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x02)
> > +#define VIRTIO_CRYPTO_MAC_DESTROY_SESSION \
> > +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x03)
> > +#define VIRTIO_CRYPTO_AEAD_CREATE_SESSION \
> > +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02)
> > +#define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \
> > +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03)
> > +	__le32 opcode;
> > +	__le32 algo;
> > +	__le32 flag;
> > +	/* data virtqueue id */
> > +	__le32 queue_id;
> > +};
> > +
> > +struct virtio_crypto_cipher_session_para {
> > +#define VIRTIO_CRYPTO_NO_CIPHER                 0
> > +#define VIRTIO_CRYPTO_CIPHER_ARC4               1
> > +#define VIRTIO_CRYPTO_CIPHER_AES_ECB            2
> > +#define VIRTIO_CRYPTO_CIPHER_AES_CBC            3
> > +#define VIRTIO_CRYPTO_CIPHER_AES_CTR            4
> > +#define VIRTIO_CRYPTO_CIPHER_DES_ECB            5
> > +#define VIRTIO_CRYPTO_CIPHER_DES_CBC            6
> > +#define VIRTIO_CRYPTO_CIPHER_3DES_ECB           7
> > +#define VIRTIO_CRYPTO_CIPHER_3DES_CBC           8
> > +#define VIRTIO_CRYPTO_CIPHER_3DES_CTR           9
> > +#define VIRTIO_CRYPTO_CIPHER_KASUMI_F8          10
> > +#define VIRTIO_CRYPTO_CIPHER_SNOW3G_UEA2        11
> > +#define VIRTIO_CRYPTO_CIPHER_AES_F8             12
> > +#define VIRTIO_CRYPTO_CIPHER_AES_XTS            13
> > +#define VIRTIO_CRYPTO_CIPHER_ZUC_EEA3           14
> > +	__le32 algo;
> > +	/* length of key */
> > +	__le32 keylen;
> > +
> > +#define VIRTIO_CRYPTO_OP_ENCRYPT  1
> > +#define VIRTIO_CRYPTO_OP_DECRYPT  2
> > +	/* encrypt or decrypt */
> > +	__le32 op;
> > +	__le32 padding;
> > +};
> > +
> > +struct virtio_crypto_session_input {
> > +	/* Device-writable part */
> > +	__le64 session_id;
> > +	__le32 status;
> > +	__le32 padding;
> > +};
> > +
> > +struct virtio_crypto_cipher_session_req {
> > +	struct virtio_crypto_cipher_session_para para;
> > +};
> > +
> > +struct virtio_crypto_hash_session_para {
> > +#define VIRTIO_CRYPTO_NO_HASH            0
> > +#define VIRTIO_CRYPTO_HASH_MD5           1
> > +#define VIRTIO_CRYPTO_HASH_SHA1          2
> > +#define VIRTIO_CRYPTO_HASH_SHA_224       3
> > +#define VIRTIO_CRYPTO_HASH_SHA_256       4
> > +#define VIRTIO_CRYPTO_HASH_SHA_384       5
> > +#define VIRTIO_CRYPTO_HASH_SHA_512       6
> > +#define VIRTIO_CRYPTO_HASH_SHA3_224      7
> > +#define VIRTIO_CRYPTO_HASH_SHA3_256      8
> > +#define VIRTIO_CRYPTO_HASH_SHA3_384      9
> > +#define VIRTIO_CRYPTO_HASH_SHA3_512      10
> > +#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE128      11
> > +#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE256      12
> > +	__le32 algo;
> > +	/* hash result length */
> > +	__le32 hash_result_len;
> > +};
> > +
> > +struct virtio_crypto_hash_create_session_req {
> > +	struct virtio_crypto_hash_session_para para;
> > +};
> > +
> > +struct virtio_crypto_mac_session_para {
> > +#define VIRTIO_CRYPTO_NO_MAC                       0
> > +#define VIRTIO_CRYPTO_MAC_HMAC_MD5                 1
> > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA1                2
> > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_224             3
> > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_256             4
> > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_384             5
> > +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_512             6
> > +#define VIRTIO_CRYPTO_MAC_CMAC_3DES                25
> > +#define VIRTIO_CRYPTO_MAC_CMAC_AES                 26
> > +#define VIRTIO_CRYPTO_MAC_KASUMI_F9                27
> > +#define VIRTIO_CRYPTO_MAC_SNOW3G_UIA2              28
> > +#define VIRTIO_CRYPTO_MAC_GMAC_AES                 41
> > +#define VIRTIO_CRYPTO_MAC_GMAC_TWOFISH             42
> > +#define VIRTIO_CRYPTO_MAC_CBCMAC_AES               49
> > +#define VIRTIO_CRYPTO_MAC_CBCMAC_KASUMI_F9         50
> > +#define VIRTIO_CRYPTO_MAC_XCBC_AES                 53
> > +	__le32 algo;
> > +	/* hash result length */
> > +	__le32 hash_result_len;
> > +	/* length of authenticated key */
> > +	__le32 auth_key_len;
> > +	__le32 padding;
> > +};
> > +
> > +struct virtio_crypto_mac_create_session_req {
> > +	struct virtio_crypto_mac_session_para para;
> > +};
> > +
> > +struct virtio_crypto_aead_session_para {
> > +#define VIRTIO_CRYPTO_NO_AEAD     0
> > +#define VIRTIO_CRYPTO_AEAD_GCM    1
> > +#define VIRTIO_CRYPTO_AEAD_CCM    2
> > +#define VIRTIO_CRYPTO_AEAD_CHACHA20_POLY1305  3
> > +	__le32 algo;
> > +	/* length of key */
> > +	__le32 key_len;
> > +	/* hash result length */
> > +	__le32 hash_result_len;
> > +	/* length of the additional authenticated data (AAD) in bytes */
> > +	__le32 aad_len;
> > +	/* encrypt or decrypt, See above VIRTIO_CRYPTO_OP_* */
> > +	__le32 op;
> > +	__le32 padding;
> > +};
> > +
> > +struct virtio_crypto_aead_create_session_req {
> > +	struct virtio_crypto_aead_session_para para;
> > +};
> > +
> > +struct virtio_crypto_alg_chain_session_para {
> > +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER
> 1
> > +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH
> 2
> > +	__le32 alg_chain_order;
> > +/* Plain hash */
> > +#define VIRTIO_CRYPTO_SYM_HASH_MODE_PLAIN    1
> > +/* Authenticated hash (mac) */
> > +#define VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH     2
> > +/* Nested hash */
> > +#define VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED   3
> > +	__le32 hash_mode;
> > +	struct virtio_crypto_cipher_session_para cipher_param;
> > +	union {
> > +		struct virtio_crypto_hash_session_para hash_param;
> > +		struct virtio_crypto_mac_session_para mac_param;
> > +	} u;
> 
> I personally dislike the use of unions of different size
> in these structures. For example, using hash_param,
> there's no way to initialize the trailing part of the
> structure. Also, the size of the structure is hard to
> figure out. I would like to see padding of all structures
> to be same size, and adding a union member that's just padding.
> 
> Same applies to other unions.
> 
Good suggestion :)  Let me rework them.

Michael, I'd like to add virtio-crypto stuff to your maintaining part likes
the virtio-net/blk parts so that the corresponding patches
can be CC'ed to you too because the virtio-crypto doesn't lay in 
driver/virtio directory. Will you?

Thanks!
-Gonglei

^ permalink raw reply

* RE: Re: [PATCH v2 1/2] virtio: introduce little edian functions for virtio_cread/write# family
From: Gonglei (Arei) @ 2016-11-28  2:57 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
	virtio-dev@lists.oasis-open.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org, Luonengjun, stefanha@redhat.com,
	Huangweidong (C), Wubin (H), xin.zeng@intel.com, Claudio Fontana,
	herbert@gondor.apana.org.au, pasic@linux.vnet.ibm.com,
	davem@davemloft.net, "Zhoujian (jay, Euler)" <jianjay
In-Reply-To: <20161128044845-mutt-send-email-mst@kernel.org>

>
> Subject: Re: [virtio-dev] Re: [PATCH v2 1/2] virtio: introduce little edian
> functions for virtio_cread/write# family
> 
> On Mon, Nov 28, 2016 at 04:34:56AM +0200, Michael S. Tsirkin wrote:
> > On Mon, Nov 28, 2016 at 01:56:04AM +0000, Gonglei (Arei) wrote:
> > > Hi Michael,
> > >
> > > Thanks for your feedback firstly!
> > >
> > > > -----Original Message-----
> > > > From: virtio-dev@lists.oasis-open.org
> [mailto:virtio-dev@lists.oasis-open.org]
> > > > On Behalf Of Michael S. Tsirkin
> > > > Sent: Sunday, November 27, 2016 11:33 AM
> > > > To: Gonglei (Arei)
> > > > Subject: [virtio-dev] Re: [PATCH v2 1/2] virtio: introduce little edian
> functions for
> > > > virtio_cread/write# family
> > > >
> > > > On Tue, Nov 22, 2016 at 04:10:22PM +0800, Gonglei wrote:
> > > > > Virtio modern devices are always little edian, let's introduce
> > > > > the LE functions for read/write configuration space for
> > > > > virtio modern devices, which avoid complaint by Sparse when
> > > > > we use the virtio_creaed/virtio_cwrite in VIRTIO_1 devices.
> > > > >
> > > > > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > > > > ---
> > > > >  include/linux/virtio_config.h | 45
> > > > +++++++++++++++++++++++++++++++++++++++++++
> > > > >  1 file changed, 45 insertions(+)
> > > > >
> > > > > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> > > > > index 26c155b..de05707 100644
> > > > > --- a/include/linux/virtio_config.h
> > > > > +++ b/include/linux/virtio_config.h
> > > > > @@ -414,4 +414,49 @@ static inline void virtio_cwrite64(struct
> virtio_device
> > > > *vdev,
> > > > >  		_r;							\
> > > > >  	})
> > > > >
> > > > > +static inline __le16 virtio_cread16_le(struct virtio_device *vdev,
> > > > > +				 unsigned int offset)
> > > > > +{
> > > > > +	__le16 ret;
> > > > > +
> > > > > +	vdev->config->get(vdev, offset, &ret, sizeof(ret));
> > > > > +	return ret;
> > > > > +}
> > > > > +
> > > > > +static inline void virtio_cwrite16_le(struct virtio_device *vdev,
> > > > > +				   unsigned int offset, __le16 val)
> > > > > +{
> > > > > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > > > > +}
> > > > > +
> > > > > +static inline __le32 virtio_cread32_le(struct virtio_device *vdev,
> > > > > +				 unsigned int offset)
> > > > > +{
> > > > > +	__le32 ret;
> > > > > +
> > > > > +	vdev->config->get(vdev, offset, &ret, sizeof(ret));
> > > > > +	return ret;
> > > > > +}
> > > > > +
> > > > > +static inline void virtio_cwrite32_le(struct virtio_device *vdev,
> > > > > +				   unsigned int offset, __le32 val)
> > > > > +{
> > > > > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > > > > +}
> > > > > +
> > > > > +static inline __le64 virtio_cread64_le(struct virtio_device *vdev,
> > > > > +				 unsigned int offset)
> > > > > +{
> > > > > +	__le64 ret;
> > > > > +
> > > > > +	__virtio_cread_many(vdev, offset, &ret, 1, sizeof(ret));
> > > > > +	return ret;
> > > > > +}
> > > > > +
> > > > > +static inline void virtio_cwrite64_le(struct virtio_device *vdev,
> > > > > +				   unsigned int offset, __le64 val)
> > > > > +{
> > > > > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > > > > +}
> > > > > +
> > > > >  #endif /* _LINUX_VIRTIO_CONFIG_H */
> > > >
> > > > Could you please better explain what is the issue you are facing?
> > > > virtio_cwrite/virtio_cread all accept and return native types.
> > > >
> > > virtio_cwrite/virtio_cread are used to write/read configuration
> > > space for virtio devices. For virtio-crypto device, I used __le32/64 directly
> > > in struct virtio_crypto_config. The sparse reports warnings if I use
> virtio_cread()
> > > for virtio-crypto device.
> >
> > I suspect that's because you are doing cread into an le32 variable.
> >
> >
> >
> > > Furthermore, it means the warnings exist for all VIRTIO_1 devices because
> > > they are definitely LE, which it's not necessary to use
> virtio_to_cpu/cpu_to_virtio.
> > >
> > >
> > > PS: I googled a discussion about this topic for virtio-input device, pls see:
> > >
> http://linux.kernel.narkive.com/3argfbWz/patch-1-1-add-virtio-input-driver
> > >
> > > Regards,
> > > -Gonglei
> >
> > Looks like we changed the macros since - at least ATM
> > virtio_console_config uses __u16 fields (which is probably a bug -
> > I'll look into fixing it up) and they
> > do not seem to trigger warnings.
> 
> 
> Sorry, I recall now. We use u32 and similar types in the config
> space, and endian-ness is implicit.
> This works fine if you are using virtio_cread interfaces which will swap
> to/from LE for you.
> 
> What doesn't work correctly is shadowing the whole config space
> using ->get since that does not byte-swap. so just don't do it.
> 
> I would say just do what everyone does in this version and
> we can revisit this later.
> 

OK, it's fair. :) 

Thanks!
-Gonglei

> >
> > > > If you want it in LE format, swap it!
> > > >
> > > >
> > > >
> > > > > --
> > > > > 1.8.3.1
> > > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

^ permalink raw reply

* Re: Re: [PATCH v2 1/2] virtio: introduce little edian functions for virtio_cread/write# family
From: Michael S. Tsirkin @ 2016-11-28  2:54 UTC (permalink / raw)
  To: Gonglei (Arei)
  Cc: linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
	virtio-dev@lists.oasis-open.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org, Luonengjun, stefanha@redhat.com,
	Huangweidong (C), Wubin (H), xin.zeng@intel.com, Claudio Fontana,
	herbert@gondor.apana.org.au, pasic@linux.vnet.ibm.com,
	davem@davemloft.net,
	"Zhoujian (jay, Euler)" <jianjay.zhou@
In-Reply-To: <20161128042142-mutt-send-email-mst@kernel.org>

On Mon, Nov 28, 2016 at 04:34:56AM +0200, Michael S. Tsirkin wrote:
> On Mon, Nov 28, 2016 at 01:56:04AM +0000, Gonglei (Arei) wrote:
> > Hi Michael,
> > 
> > Thanks for your feedback firstly!
> > 
> > > -----Original Message-----
> > > From: virtio-dev@lists.oasis-open.org [mailto:virtio-dev@lists.oasis-open.org]
> > > On Behalf Of Michael S. Tsirkin
> > > Sent: Sunday, November 27, 2016 11:33 AM
> > > To: Gonglei (Arei)
> > > Subject: [virtio-dev] Re: [PATCH v2 1/2] virtio: introduce little edian functions for
> > > virtio_cread/write# family
> > > 
> > > On Tue, Nov 22, 2016 at 04:10:22PM +0800, Gonglei wrote:
> > > > Virtio modern devices are always little edian, let's introduce
> > > > the LE functions for read/write configuration space for
> > > > virtio modern devices, which avoid complaint by Sparse when
> > > > we use the virtio_creaed/virtio_cwrite in VIRTIO_1 devices.
> > > >
> > > > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > > > ---
> > > >  include/linux/virtio_config.h | 45
> > > +++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 45 insertions(+)
> > > >
> > > > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> > > > index 26c155b..de05707 100644
> > > > --- a/include/linux/virtio_config.h
> > > > +++ b/include/linux/virtio_config.h
> > > > @@ -414,4 +414,49 @@ static inline void virtio_cwrite64(struct virtio_device
> > > *vdev,
> > > >  		_r;							\
> > > >  	})
> > > >
> > > > +static inline __le16 virtio_cread16_le(struct virtio_device *vdev,
> > > > +				 unsigned int offset)
> > > > +{
> > > > +	__le16 ret;
> > > > +
> > > > +	vdev->config->get(vdev, offset, &ret, sizeof(ret));
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static inline void virtio_cwrite16_le(struct virtio_device *vdev,
> > > > +				   unsigned int offset, __le16 val)
> > > > +{
> > > > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > > > +}
> > > > +
> > > > +static inline __le32 virtio_cread32_le(struct virtio_device *vdev,
> > > > +				 unsigned int offset)
> > > > +{
> > > > +	__le32 ret;
> > > > +
> > > > +	vdev->config->get(vdev, offset, &ret, sizeof(ret));
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static inline void virtio_cwrite32_le(struct virtio_device *vdev,
> > > > +				   unsigned int offset, __le32 val)
> > > > +{
> > > > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > > > +}
> > > > +
> > > > +static inline __le64 virtio_cread64_le(struct virtio_device *vdev,
> > > > +				 unsigned int offset)
> > > > +{
> > > > +	__le64 ret;
> > > > +
> > > > +	__virtio_cread_many(vdev, offset, &ret, 1, sizeof(ret));
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static inline void virtio_cwrite64_le(struct virtio_device *vdev,
> > > > +				   unsigned int offset, __le64 val)
> > > > +{
> > > > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > > > +}
> > > > +
> > > >  #endif /* _LINUX_VIRTIO_CONFIG_H */
> > > 
> > > Could you please better explain what is the issue you are facing?
> > > virtio_cwrite/virtio_cread all accept and return native types.
> > > 
> > virtio_cwrite/virtio_cread are used to write/read configuration
> > space for virtio devices. For virtio-crypto device, I used __le32/64 directly
> > in struct virtio_crypto_config. The sparse reports warnings if I use virtio_cread()
> > for virtio-crypto device.
> 
> I suspect that's because you are doing cread into an le32 variable.
> 
> 
> 
> > Furthermore, it means the warnings exist for all VIRTIO_1 devices because
> > they are definitely LE, which it's not necessary to use virtio_to_cpu/cpu_to_virtio.
> > 
> > 
> > PS: I googled a discussion about this topic for virtio-input device, pls see:
> >  http://linux.kernel.narkive.com/3argfbWz/patch-1-1-add-virtio-input-driver
> > 
> > Regards,
> > -Gonglei
> 
> Looks like we changed the macros since - at least ATM
> virtio_console_config uses __u16 fields (which is probably a bug -
> I'll look into fixing it up) and they
> do not seem to trigger warnings.


Sorry, I recall now. We use u32 and similar types in the config
space, and endian-ness is implicit.
This works fine if you are using virtio_cread interfaces which will swap
to/from LE for you.

What doesn't work correctly is shadowing the whole config space
using ->get since that does not byte-swap. so just don't do it.

I would say just do what everyone does in this version and
we can revisit this later.

> 
> > > If you want it in LE format, swap it!
> > > 
> > > 
> > > 
> > > > --
> > > > 1.8.3.1
> > > >
> > > 
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

^ permalink raw reply

* RE: Re: [PATCH v2 1/2] virtio: introduce little edian functions for virtio_cread/write# family
From: Gonglei (Arei) @ 2016-11-28  2:53 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
	virtio-dev@lists.oasis-open.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org, Luonengjun, stefanha@redhat.com,
	Huangweidong (C), Wubin (H), xin.zeng@intel.com, Claudio Fontana,
	herbert@gondor.apana.org.au, pasic@linux.vnet.ibm.com,
	davem@davemloft.net, "Zhoujian (jay, Euler)" <jianjay
In-Reply-To: <20161128042142-mutt-send-email-mst@kernel.org>

>
> Subject: Re: [virtio-dev] Re: [PATCH v2 1/2] virtio: introduce little edian
> functions for virtio_cread/write# family
> 
> On Mon, Nov 28, 2016 at 01:56:04AM +0000, Gonglei (Arei) wrote:
> > Hi Michael,
> >
> > Thanks for your feedback firstly!
> >
> > > -----Original Message-----
> > > From: virtio-dev@lists.oasis-open.org
> [mailto:virtio-dev@lists.oasis-open.org]
> > > On Behalf Of Michael S. Tsirkin
> > > Sent: Sunday, November 27, 2016 11:33 AM
> > > To: Gonglei (Arei)
> > > Subject: [virtio-dev] Re: [PATCH v2 1/2] virtio: introduce little edian functions
> for
> > > virtio_cread/write# family
> > >
> > > On Tue, Nov 22, 2016 at 04:10:22PM +0800, Gonglei wrote:
> > > > Virtio modern devices are always little edian, let's introduce
> > > > the LE functions for read/write configuration space for
> > > > virtio modern devices, which avoid complaint by Sparse when
> > > > we use the virtio_creaed/virtio_cwrite in VIRTIO_1 devices.
> > > >
> > > > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > > > ---
> > > >  include/linux/virtio_config.h | 45
> > > +++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 45 insertions(+)
> > > >
> > > > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> > > > index 26c155b..de05707 100644
> > > > --- a/include/linux/virtio_config.h
> > > > +++ b/include/linux/virtio_config.h
> > > > @@ -414,4 +414,49 @@ static inline void virtio_cwrite64(struct
> virtio_device
> > > *vdev,
> > > >  		_r;							\
> > > >  	})
> > > >
> > > > +static inline __le16 virtio_cread16_le(struct virtio_device *vdev,
> > > > +				 unsigned int offset)
> > > > +{
> > > > +	__le16 ret;
> > > > +
> > > > +	vdev->config->get(vdev, offset, &ret, sizeof(ret));
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static inline void virtio_cwrite16_le(struct virtio_device *vdev,
> > > > +				   unsigned int offset, __le16 val)
> > > > +{
> > > > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > > > +}
> > > > +
> > > > +static inline __le32 virtio_cread32_le(struct virtio_device *vdev,
> > > > +				 unsigned int offset)
> > > > +{
> > > > +	__le32 ret;
> > > > +
> > > > +	vdev->config->get(vdev, offset, &ret, sizeof(ret));
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static inline void virtio_cwrite32_le(struct virtio_device *vdev,
> > > > +				   unsigned int offset, __le32 val)
> > > > +{
> > > > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > > > +}
> > > > +
> > > > +static inline __le64 virtio_cread64_le(struct virtio_device *vdev,
> > > > +				 unsigned int offset)
> > > > +{
> > > > +	__le64 ret;
> > > > +
> > > > +	__virtio_cread_many(vdev, offset, &ret, 1, sizeof(ret));
> > > > +	return ret;
> > > > +}
> > > > +
> > > > +static inline void virtio_cwrite64_le(struct virtio_device *vdev,
> > > > +				   unsigned int offset, __le64 val)
> > > > +{
> > > > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > > > +}
> > > > +
> > > >  #endif /* _LINUX_VIRTIO_CONFIG_H */
> > >
> > > Could you please better explain what is the issue you are facing?
> > > virtio_cwrite/virtio_cread all accept and return native types.
> > >
> > virtio_cwrite/virtio_cread are used to write/read configuration
> > space for virtio devices. For virtio-crypto device, I used __le32/64 directly
> > in struct virtio_crypto_config. The sparse reports warnings if I use
> virtio_cread()
> > for virtio-crypto device.
> 
> I suspect that's because you are doing cread into an le32 variable.
> 
> 
Yes.

> 
> > Furthermore, it means the warnings exist for all VIRTIO_1 devices because
> > they are definitely LE, which it's not necessary to use
> virtio_to_cpu/cpu_to_virtio.
> >
> >
> > PS: I googled a discussion about this topic for virtio-input device, pls see:
> >  http://linux.kernel.narkive.com/3argfbWz/patch-1-1-add-virtio-input-driver
> >
> > Regards,
> > -Gonglei
> 
> Looks like we changed the macros since - at least ATM
> virtio_console_config uses __u16 fields (which is probably a bug -
> I'll look into fixing it up) and they
> do not seem to trigger warnings.
> 
Cool.

 __u16/32/64 are fine, but __le16/32/64 will trigger warnings.

Regards,
-Gonglei

> 
> > > If you want it in LE format, swap it!
> > >
> > >
> > >
> > > > --
> > > > 1.8.3.1
> > > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

^ permalink raw reply

* Re: Re: [PATCH v2 1/2] virtio: introduce little edian functions for virtio_cread/write# family
From: Michael S. Tsirkin @ 2016-11-28  2:34 UTC (permalink / raw)
  To: Gonglei (Arei)
  Cc: linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
	virtio-dev@lists.oasis-open.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org, Luonengjun, stefanha@redhat.com,
	Huangweidong (C), Wubin (H), xin.zeng@intel.com, Claudio Fontana,
	herbert@gondor.apana.org.au, pasic@linux.vnet.ibm.com,
	davem@davemloft.net,
	"Zhoujian (jay, Euler)" <jianjay.zhou@
In-Reply-To: <33183CC9F5247A488A2544077AF19020D97A103C@SZXEMA503-MBS.china.huawei.com>

On Mon, Nov 28, 2016 at 01:56:04AM +0000, Gonglei (Arei) wrote:
> Hi Michael,
> 
> Thanks for your feedback firstly!
> 
> > -----Original Message-----
> > From: virtio-dev@lists.oasis-open.org [mailto:virtio-dev@lists.oasis-open.org]
> > On Behalf Of Michael S. Tsirkin
> > Sent: Sunday, November 27, 2016 11:33 AM
> > To: Gonglei (Arei)
> > Subject: [virtio-dev] Re: [PATCH v2 1/2] virtio: introduce little edian functions for
> > virtio_cread/write# family
> > 
> > On Tue, Nov 22, 2016 at 04:10:22PM +0800, Gonglei wrote:
> > > Virtio modern devices are always little edian, let's introduce
> > > the LE functions for read/write configuration space for
> > > virtio modern devices, which avoid complaint by Sparse when
> > > we use the virtio_creaed/virtio_cwrite in VIRTIO_1 devices.
> > >
> > > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > > ---
> > >  include/linux/virtio_config.h | 45
> > +++++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 45 insertions(+)
> > >
> > > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> > > index 26c155b..de05707 100644
> > > --- a/include/linux/virtio_config.h
> > > +++ b/include/linux/virtio_config.h
> > > @@ -414,4 +414,49 @@ static inline void virtio_cwrite64(struct virtio_device
> > *vdev,
> > >  		_r;							\
> > >  	})
> > >
> > > +static inline __le16 virtio_cread16_le(struct virtio_device *vdev,
> > > +				 unsigned int offset)
> > > +{
> > > +	__le16 ret;
> > > +
> > > +	vdev->config->get(vdev, offset, &ret, sizeof(ret));
> > > +	return ret;
> > > +}
> > > +
> > > +static inline void virtio_cwrite16_le(struct virtio_device *vdev,
> > > +				   unsigned int offset, __le16 val)
> > > +{
> > > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > > +}
> > > +
> > > +static inline __le32 virtio_cread32_le(struct virtio_device *vdev,
> > > +				 unsigned int offset)
> > > +{
> > > +	__le32 ret;
> > > +
> > > +	vdev->config->get(vdev, offset, &ret, sizeof(ret));
> > > +	return ret;
> > > +}
> > > +
> > > +static inline void virtio_cwrite32_le(struct virtio_device *vdev,
> > > +				   unsigned int offset, __le32 val)
> > > +{
> > > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > > +}
> > > +
> > > +static inline __le64 virtio_cread64_le(struct virtio_device *vdev,
> > > +				 unsigned int offset)
> > > +{
> > > +	__le64 ret;
> > > +
> > > +	__virtio_cread_many(vdev, offset, &ret, 1, sizeof(ret));
> > > +	return ret;
> > > +}
> > > +
> > > +static inline void virtio_cwrite64_le(struct virtio_device *vdev,
> > > +				   unsigned int offset, __le64 val)
> > > +{
> > > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > > +}
> > > +
> > >  #endif /* _LINUX_VIRTIO_CONFIG_H */
> > 
> > Could you please better explain what is the issue you are facing?
> > virtio_cwrite/virtio_cread all accept and return native types.
> > 
> virtio_cwrite/virtio_cread are used to write/read configuration
> space for virtio devices. For virtio-crypto device, I used __le32/64 directly
> in struct virtio_crypto_config. The sparse reports warnings if I use virtio_cread()
> for virtio-crypto device.

I suspect that's because you are doing cread into an le32 variable.



> Furthermore, it means the warnings exist for all VIRTIO_1 devices because
> they are definitely LE, which it's not necessary to use virtio_to_cpu/cpu_to_virtio.
> 
> 
> PS: I googled a discussion about this topic for virtio-input device, pls see:
>  http://linux.kernel.narkive.com/3argfbWz/patch-1-1-add-virtio-input-driver
> 
> Regards,
> -Gonglei

Looks like we changed the macros since - at least ATM
virtio_console_config uses __u16 fields (which is probably a bug -
I'll look into fixing it up) and they
do not seem to trigger warnings.


> > If you want it in LE format, swap it!
> > 
> > 
> > 
> > > --
> > > 1.8.3.1
> > >
> > 
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> > For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

^ permalink raw reply

* RE: [virtio-dev] Re: [PATCH v2 1/2] virtio: introduce little edian functions for virtio_cread/write# family
From: Gonglei (Arei) @ 2016-11-28  1:56 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: linux-kernel@vger.kernel.org, qemu-devel@nongnu.org,
	virtio-dev@lists.oasis-open.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org, Luonengjun, stefanha@redhat.com,
	Huangweidong (C), Wubin (H), xin.zeng@intel.com, Claudio Fontana,
	herbert@gondor.apana.org.au, pasic@linux.vnet.ibm.com,
	davem@davemloft.net, "Zhoujian (jay, Euler)" <jian
In-Reply-To: <20161127052802-mutt-send-email-mst@kernel.org>

Hi Michael,

Thanks for your feedback firstly!

> -----Original Message-----
> From: virtio-dev@lists.oasis-open.org [mailto:virtio-dev@lists.oasis-open.org]
> On Behalf Of Michael S. Tsirkin
> Sent: Sunday, November 27, 2016 11:33 AM
> To: Gonglei (Arei)
> Subject: [virtio-dev] Re: [PATCH v2 1/2] virtio: introduce little edian functions for
> virtio_cread/write# family
> 
> On Tue, Nov 22, 2016 at 04:10:22PM +0800, Gonglei wrote:
> > Virtio modern devices are always little edian, let's introduce
> > the LE functions for read/write configuration space for
> > virtio modern devices, which avoid complaint by Sparse when
> > we use the virtio_creaed/virtio_cwrite in VIRTIO_1 devices.
> >
> > Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> > ---
> >  include/linux/virtio_config.h | 45
> +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 45 insertions(+)
> >
> > diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> > index 26c155b..de05707 100644
> > --- a/include/linux/virtio_config.h
> > +++ b/include/linux/virtio_config.h
> > @@ -414,4 +414,49 @@ static inline void virtio_cwrite64(struct virtio_device
> *vdev,
> >  		_r;							\
> >  	})
> >
> > +static inline __le16 virtio_cread16_le(struct virtio_device *vdev,
> > +				 unsigned int offset)
> > +{
> > +	__le16 ret;
> > +
> > +	vdev->config->get(vdev, offset, &ret, sizeof(ret));
> > +	return ret;
> > +}
> > +
> > +static inline void virtio_cwrite16_le(struct virtio_device *vdev,
> > +				   unsigned int offset, __le16 val)
> > +{
> > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > +}
> > +
> > +static inline __le32 virtio_cread32_le(struct virtio_device *vdev,
> > +				 unsigned int offset)
> > +{
> > +	__le32 ret;
> > +
> > +	vdev->config->get(vdev, offset, &ret, sizeof(ret));
> > +	return ret;
> > +}
> > +
> > +static inline void virtio_cwrite32_le(struct virtio_device *vdev,
> > +				   unsigned int offset, __le32 val)
> > +{
> > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > +}
> > +
> > +static inline __le64 virtio_cread64_le(struct virtio_device *vdev,
> > +				 unsigned int offset)
> > +{
> > +	__le64 ret;
> > +
> > +	__virtio_cread_many(vdev, offset, &ret, 1, sizeof(ret));
> > +	return ret;
> > +}
> > +
> > +static inline void virtio_cwrite64_le(struct virtio_device *vdev,
> > +				   unsigned int offset, __le64 val)
> > +{
> > +	vdev->config->set(vdev, offset, &val, sizeof(val));
> > +}
> > +
> >  #endif /* _LINUX_VIRTIO_CONFIG_H */
> 
> Could you please better explain what is the issue you are facing?
> virtio_cwrite/virtio_cread all accept and return native types.
> 
virtio_cwrite/virtio_cread are used to write/read configuration
space for virtio devices. For virtio-crypto device, I used __le32/64 directly
in struct virtio_crypto_config. The sparse reports warnings if I use virtio_cread()
for virtio-crypto device.

Furthermore, it means the warnings exist for all VIRTIO_1 devices because
they are definitely LE, which it's not necessary to use virtio_to_cpu/cpu_to_virtio.


PS: I googled a discussion about this topic for virtio-input device, pls see:
 http://linux.kernel.narkive.com/3argfbWz/patch-1-1-add-virtio-input-driver

Regards,
-Gonglei

> If you want it in LE format, swap it!
> 
> 
> 
> > --
> > 1.8.3.1
> >
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org

^ permalink raw reply

* Re: [PATCH v2 1/2] virtio: introduce little edian functions for virtio_cread/write# family
From: Michael S. Tsirkin @ 2016-11-27  3:32 UTC (permalink / raw)
  To: Gonglei
  Cc: virtio-dev, weidong.huang, claudio.fontana, hanweidong,
	qemu-devel, luonengjun, linux-kernel, virtualization,
	salvatore.benedetto, xuquan8, linux-crypto, stefanha,
	jianjay.zhou, longpeng2, arei.gonglei, davem, wu.wubin, herbert
In-Reply-To: <1479802223-121104-2-git-send-email-arei.gonglei@huawei.com>

On Tue, Nov 22, 2016 at 04:10:22PM +0800, Gonglei wrote:
> Virtio modern devices are always little edian, let's introduce
> the LE functions for read/write configuration space for
> virtio modern devices, which avoid complaint by Sparse when
> we use the virtio_creaed/virtio_cwrite in VIRTIO_1 devices.
> 
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  include/linux/virtio_config.h | 45 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
> 
> diff --git a/include/linux/virtio_config.h b/include/linux/virtio_config.h
> index 26c155b..de05707 100644
> --- a/include/linux/virtio_config.h
> +++ b/include/linux/virtio_config.h
> @@ -414,4 +414,49 @@ static inline void virtio_cwrite64(struct virtio_device *vdev,
>  		_r;							\
>  	})
>  
> +static inline __le16 virtio_cread16_le(struct virtio_device *vdev,
> +				 unsigned int offset)
> +{
> +	__le16 ret;
> +
> +	vdev->config->get(vdev, offset, &ret, sizeof(ret));
> +	return ret;
> +}
> +
> +static inline void virtio_cwrite16_le(struct virtio_device *vdev,
> +				   unsigned int offset, __le16 val)
> +{
> +	vdev->config->set(vdev, offset, &val, sizeof(val));
> +}
> +
> +static inline __le32 virtio_cread32_le(struct virtio_device *vdev,
> +				 unsigned int offset)
> +{
> +	__le32 ret;
> +
> +	vdev->config->get(vdev, offset, &ret, sizeof(ret));
> +	return ret;
> +}
> +
> +static inline void virtio_cwrite32_le(struct virtio_device *vdev,
> +				   unsigned int offset, __le32 val)
> +{
> +	vdev->config->set(vdev, offset, &val, sizeof(val));
> +}
> +
> +static inline __le64 virtio_cread64_le(struct virtio_device *vdev,
> +				 unsigned int offset)
> +{
> +	__le64 ret;
> +
> +	__virtio_cread_many(vdev, offset, &ret, 1, sizeof(ret));
> +	return ret;
> +}
> +
> +static inline void virtio_cwrite64_le(struct virtio_device *vdev,
> +				   unsigned int offset, __le64 val)
> +{
> +	vdev->config->set(vdev, offset, &val, sizeof(val));
> +}
> +
>  #endif /* _LINUX_VIRTIO_CONFIG_H */

Could you please better explain what is the issue you are facing?
virtio_cwrite/virtio_cread all accept and return native types.

If you want it in LE format, swap it!



> -- 
> 1.8.3.1
> 

^ permalink raw reply

* Re: [PATCH v2 2/2] crypto: add virtio-crypto driver
From: Michael S. Tsirkin @ 2016-11-27  3:27 UTC (permalink / raw)
  To: Gonglei
  Cc: linux-kernel, qemu-devel, virtio-dev, virtualization,
	linux-crypto, luonengjun, stefanha, weidong.huang, wu.wubin,
	xin.zeng, claudio.fontana, herbert, pasic, davem, jianjay.zhou,
	hanweidong, arei.gonglei, cornelia.huck, xuquan8, longpeng2,
	salvatore.benedetto
In-Reply-To: <1479802223-121104-3-git-send-email-arei.gonglei@huawei.com>

On Tue, Nov 22, 2016 at 04:10:23PM +0800, Gonglei wrote:
> This patch introduces virtio-crypto driver for Linux Kernel.
> 
> The virtio crypto device is a virtual cryptography device
> as well as a kind of virtual hardware accelerator for
> virtual machines. The encryption anddecryption requests
> are placed in the data queue and are ultimately handled by
> thebackend crypto accelerators. The second queue is the
> control queue used to create or destroy sessions for
> symmetric algorithms and will control some advanced features
> in the future. The virtio crypto device provides the following
> cryptoservices: CIPHER, MAC, HASH, and AEAD.
> 
> For more information about virtio-crypto device, please see:
>   http://qemu-project.org/Features/VirtioCrypto
> 
> CC: Michael S. Tsirkin <mst@redhat.com>
> CC: Cornelia Huck <cornelia.huck@de.ibm.com>
> CC: Stefan Hajnoczi <stefanha@redhat.com>
> CC: Herbert Xu <herbert@gondor.apana.org.au>
> CC: Halil Pasic <pasic@linux.vnet.ibm.com>
> CC: David S. Miller <davem@davemloft.net>
> CC: Zeng Xin <xin.zeng@intel.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> ---
>  MAINTAINERS                                  |   8 +
>  drivers/crypto/Kconfig                       |   2 +
>  drivers/crypto/Makefile                      |   1 +
>  drivers/crypto/virtio/Kconfig                |  10 +
>  drivers/crypto/virtio/Makefile               |   5 +
>  drivers/crypto/virtio/virtio_crypto.c        | 444 +++++++++++++++++++++++
>  drivers/crypto/virtio/virtio_crypto_algs.c   | 524 +++++++++++++++++++++++++++
>  drivers/crypto/virtio/virtio_crypto_common.h | 124 +++++++
>  drivers/crypto/virtio/virtio_crypto_mgr.c    | 258 +++++++++++++
>  include/uapi/linux/Kbuild                    |   1 +
>  include/uapi/linux/virtio_crypto.h           | 435 ++++++++++++++++++++++
>  include/uapi/linux/virtio_ids.h              |   1 +
>  12 files changed, 1813 insertions(+)
>  create mode 100644 drivers/crypto/virtio/Kconfig
>  create mode 100644 drivers/crypto/virtio/Makefile
>  create mode 100644 drivers/crypto/virtio/virtio_crypto.c
>  create mode 100644 drivers/crypto/virtio/virtio_crypto_algs.c
>  create mode 100644 drivers/crypto/virtio/virtio_crypto_common.h
>  create mode 100644 drivers/crypto/virtio/virtio_crypto_mgr.c
>  create mode 100644 include/uapi/linux/virtio_crypto.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 411e3b8..d6b18bb 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -12844,6 +12844,14 @@ S:	Maintained
>  F:	drivers/virtio/virtio_input.c
>  F:	include/uapi/linux/virtio_input.h
>  
> +VIRTIO CRYPTO DRIVER
> +M:  Gonglei <arei.gonglei@huawei.com>
> +L:  virtualization@lists.linux-foundation.org
> +L:  linux-crypto@vger.kernel.org
> +S:  Maintained
> +F:  drivers/crypto/virtio/
> +F:  include/uapi/linux/virtio_crypto.h
> +
>  VIA RHINE NETWORK DRIVER
>  S:	Orphan
>  F:	drivers/net/ethernet/via/via-rhine.c
> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index 4d2b81f..7956478 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -555,4 +555,6 @@ config CRYPTO_DEV_ROCKCHIP
>  
>  source "drivers/crypto/chelsio/Kconfig"
>  
> +source "drivers/crypto/virtio/Kconfig"
> +
>  endif # CRYPTO_HW
> diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
> index ad7250f..bc53cb8 100644
> --- a/drivers/crypto/Makefile
> +++ b/drivers/crypto/Makefile
> @@ -32,3 +32,4 @@ obj-$(CONFIG_CRYPTO_DEV_VMX) += vmx/
>  obj-$(CONFIG_CRYPTO_DEV_SUN4I_SS) += sunxi-ss/
>  obj-$(CONFIG_CRYPTO_DEV_ROCKCHIP) += rockchip/
>  obj-$(CONFIG_CRYPTO_DEV_CHELSIO) += chelsio/
> +obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio/
> diff --git a/drivers/crypto/virtio/Kconfig b/drivers/crypto/virtio/Kconfig
> new file mode 100644
> index 0000000..ceae88c
> --- /dev/null
> +++ b/drivers/crypto/virtio/Kconfig
> @@ -0,0 +1,10 @@
> +config CRYPTO_DEV_VIRTIO
> +	tristate "VirtIO crypto driver"
> +	depends on VIRTIO
> +    select CRYPTO_AEAD
> +    select CRYPTO_AUTHENC
> +    select CRYPTO_BLKCIPHER
> +	default m
> +	help
> +	  This driver provides support for virtio crypto device. If you
> +	  choose 'M' here, this module will be called virtio-crypto.
> diff --git a/drivers/crypto/virtio/Makefile b/drivers/crypto/virtio/Makefile
> new file mode 100644
> index 0000000..a316e92
> --- /dev/null
> +++ b/drivers/crypto/virtio/Makefile
> @@ -0,0 +1,5 @@
> +obj-$(CONFIG_CRYPTO_DEV_VIRTIO) += virtio-crypto.o
> +virtio-crypto-objs := \
> +	virtio_crypto_algs.o \
> +	virtio_crypto_mgr.o \
> +	virtio_crypto.o
> diff --git a/drivers/crypto/virtio/virtio_crypto.c b/drivers/crypto/virtio/virtio_crypto.c
> new file mode 100644
> index 0000000..56fdfed
> --- /dev/null
> +++ b/drivers/crypto/virtio/virtio_crypto.c
> @@ -0,0 +1,444 @@
> + /* Driver for Virtio crypto device.
> +  *
> +  * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
> +  *
> +  * This program is free software; you can redistribute it and/or modify
> +  * it under the terms of the GNU General Public License as published by
> +  * the Free Software Foundation; either version 2 of the License, or
> +  * (at your option) any later version.
> +  *
> +  * This program 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 General Public License for more details.
> +  *
> +  * You should have received a copy of the GNU General Public License
> +  * along with this program; if not, see <http://www.gnu.org/licenses/>.
> +  */
> +
> +#include <linux/err.h>
> +#include <linux/module.h>
> +#include <linux/virtio_config.h>
> +#include <linux/cpu.h>
> +
> +#include <uapi/linux/virtio_crypto.h>
> +#include "virtio_crypto_common.h"
> +
> +
> +static void virtcrypto_dataq_callback(struct virtqueue *vq)
> +{
> +	struct virtio_crypto *vcrypto = vq->vdev->priv;
> +	struct virtio_crypto_request *vc_req;
> +	unsigned long flags;
> +	unsigned int len;
> +	struct ablkcipher_request *ablk_req;
> +	int error;
> +
> +	spin_lock_irqsave(&vcrypto->lock, flags);
> +	do {
> +		virtqueue_disable_cb(vq);
> +		while ((vc_req = virtqueue_get_buf(vq, &len)) != NULL) {
> +			if (vc_req->type == VIRTIO_CRYPTO_SYM_OP_CIPHER) {
> +				switch (vc_req->status) {
> +				case VIRTIO_CRYPTO_OK:
> +					error = 0;
> +					break;
> +				case VIRTIO_CRYPTO_INVSESS:
> +				case VIRTIO_CRYPTO_ERR:
> +					error = -EINVAL;
> +					break;
> +				case VIRTIO_CRYPTO_BADMSG:
> +					error = -EBADMSG;
> +					break;
> +				default:
> +					error = -EIO;
> +					break;
> +				}
> +				ablk_req = vc_req->ablkcipher_req;
> +				/* Finish the encrypt or decrypt process */
> +				ablk_req->base.complete(&ablk_req->base, error);
> +			}
> +
> +			kfree(vc_req->req_data);
> +			kfree(vc_req->sgs);
> +		}
> +	} while (!virtqueue_enable_cb(vq));
> +	spin_unlock_irqrestore(&vcrypto->lock, flags);
> +}
> +
> +static int virtcrypto_find_vqs(struct virtio_crypto *vi)
> +{
> +	vq_callback_t **callbacks;
> +	struct virtqueue **vqs;
> +	int ret = -ENOMEM;
> +	int i, total_vqs;
> +	const char **names;
> +
> +	/* We expect 1 data virtqueue, followed by
> +	 * possible N-1 data queues used in multiqueue mode, followed by
> +	 * control vq.
> +	 */
> +	total_vqs = vi->max_data_queues + 1;
> +
> +	/* Allocate space for find_vqs parameters */
> +	vqs = kcalloc(total_vqs, sizeof(*vqs), GFP_KERNEL);
> +	if (!vqs)
> +		goto err_vq;
> +	callbacks = kcalloc(total_vqs, sizeof(*callbacks), GFP_KERNEL);
> +	if (!callbacks)
> +		goto err_callback;
> +	names = kcalloc(total_vqs, sizeof(*names), GFP_KERNEL);
> +	if (!names)
> +		goto err_names;
> +
> +	/* Parameters for control virtqueue */
> +	callbacks[total_vqs - 1] = NULL;
> +	names[total_vqs - 1] = "controlq";
> +
> +	/* Allocate/initialize parameters for data virtqueues */
> +	for (i = 0; i < vi->max_data_queues; i++) {
> +		callbacks[i] = virtcrypto_dataq_callback;
> +		snprintf(vi->data_vq[i].name, sizeof(vi->data_vq[i].name),
> +				"dataq.%d", i);
> +		names[i] = vi->data_vq[i].name;
> +	}
> +
> +	ret = vi->vdev->config->find_vqs(vi->vdev, total_vqs, vqs, callbacks,
> +					 names);
> +	if (ret)
> +		goto err_find;
> +
> +	vi->ctrl_vq = vqs[total_vqs - 1];
> +
> +	for (i = 0; i < vi->max_data_queues; i++)
> +		vi->data_vq[i].vq = vqs[i];
> +
> +	kfree(names);
> +	kfree(callbacks);
> +	kfree(vqs);
> +
> +	return 0;
> +
> +err_find:
> +	kfree(names);
> +err_names:
> +	kfree(callbacks);
> +err_callback:
> +	kfree(vqs);
> +err_vq:
> +	return ret;
> +}
> +
> +static int virtcrypto_alloc_queues(struct virtio_crypto *vi)
> +{
> +	vi->data_vq = kcalloc(vi->max_data_queues, sizeof(*vi->data_vq),
> +				GFP_KERNEL);
> +	if (!vi->data_vq)
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
> +static void virtcrypto_clean_affinity(struct virtio_crypto *vi, long hcpu)
> +{
> +	int i;
> +
> +	if (vi->affinity_hint_set) {
> +		for (i = 0; i < vi->max_data_queues; i++)
> +			virtqueue_set_affinity(vi->data_vq[i].vq, -1);
> +
> +		vi->affinity_hint_set = false;
> +	}
> +}
> +
> +static void virtcrypto_set_affinity(struct virtio_crypto *vi)
> +{
> +	int i;
> +	int cpu;
> +
> +	/*
> +	 * In multiqueue mode, when the number of cpu is equal to the number
> +	 * of queue, we let the queue to be private to one cpu by
> +	 * setting the affinity hint to eliminate the contention.
> +	 */
> +	if (vi->curr_queue == 1 ||
> +	    vi->max_data_queues != num_online_cpus()) {
> +		virtcrypto_clean_affinity(vi, -1);
> +		return;
> +	}
> +

So how can you handle cpu hotplug then?
I would imagine making max_data_queues > num_online_cpus
and initializing just the required number would be
a saner way.

> +	i = 0;
> +	for_each_online_cpu(cpu) {
> +		virtqueue_set_affinity(vi->data_vq[i].vq, cpu);
> +		i++;
> +	}
> +
> +	vi->affinity_hint_set = true;
> +}
> +

Above is racy in prsence of cpu hotplug. Is there a userspace
interface to fix up affinity after hotplug?
If not you need to get notified after hotplug and
fix it up yourself.

> +static void virtcrypto_free_queues(struct virtio_crypto *vi)
> +{
> +	kfree(vi->data_vq);
> +}
> +
> +static int virtcrypto_init_vqs(struct virtio_crypto *vi)
> +{
> +	int ret;
> +
> +	/* Allocate send & receive queues */
> +	ret = virtcrypto_alloc_queues(vi);
> +	if (ret)
> +		goto err;
> +
> +	ret = virtcrypto_find_vqs(vi);
> +	if (ret)
> +		goto err_free;
> +
> +	get_online_cpus();
> +	virtcrypto_set_affinity(vi);
> +	put_online_cpus();
> +
> +	return 0;
> +
> +err_free:
> +	virtcrypto_free_queues(vi);
> +err:
> +	return ret;
> +}
> +
> +static int virtcrypto_update_status(struct virtio_crypto *vcrypto)
> +{
> +	__le32 status_le;
> +	u32 status;
> +	int err;
> +
> +	status_le = virtio_cread32_le(vcrypto->vdev,
> +			offsetof(struct virtio_crypto_config, status));
> +	status = le32_to_cpu(status_le);
> +
> +	/* Ignore unknown (future) status bits */
> +	status &= VIRTIO_CRYPTO_S_HW_READY;
> +
> +	if (vcrypto->status == status)
> +		return 0;
> +
> +	vcrypto->status = status;
> +
> +	if (vcrypto->status & VIRTIO_CRYPTO_S_HW_READY) {
> +		err = virtcrypto_dev_start(vcrypto);
> +		if (err) {
> +			dev_err(&vcrypto->vdev->dev,
> +				"Failed to start virtio crypto device.\n");
> +			virtcrypto_dev_stop(vcrypto);
> +			return -1;
> +		}
> +		dev_info(&vcrypto->vdev->dev, "Accelerator is ready\n");
> +	} else {
> +		virtcrypto_dev_stop(vcrypto);
> +		dev_info(&vcrypto->vdev->dev, "Accelerator is not ready\n");
> +	}
> +
> +	return 0;
> +}
> +
> +static void virtcrypto_del_vqs(struct virtio_crypto *vcrypto)
> +{
> +	struct virtio_device *vdev = vcrypto->vdev;
> +
> +	virtcrypto_clean_affinity(vcrypto, -1);
> +
> +	vdev->config->del_vqs(vdev);
> +
> +	virtcrypto_free_queues(vcrypto);
> +}
> +
> +static int virtcrypto_probe(struct virtio_device *vdev)
> +{
> +	int err = -EFAULT;
> +	struct virtio_crypto *vcrypto;
> +	__le32 max_data_queues_le = 0, max_cipher_key_len_le = 0;
> +	__le32 max_auth_key_len_le = 0;
> +	__le64 max_size_le = 0;
> +

Pls validate that it's a modern device (has VERSION_1 set).
We should find a way to do it in a central place,
but we don't have that now.

> +	if (!vdev->config->get) {
> +		dev_err(&vdev->dev, "%s failure: config access disabled\n",
> +			__func__);
> +		return -EINVAL;
> +	}
> +
> +	if (num_possible_nodes() > 1 && dev_to_node(&vdev->dev) < 0) {
> +		/*
> +		 * If the accelerator is connected to a node with no memory
> +		 * there is no point in using the accelerator since the remote
> +		 * memory transaction will be very slow.
> +		 */
> +		dev_err(&vdev->dev, "Invalid NUMA configuration.\n");
> +		return -EINVAL;
> +	}
> +
> +	vcrypto = kzalloc_node(sizeof(*vcrypto), GFP_KERNEL,
> +					dev_to_node(&vdev->dev));
> +	if (!vcrypto)
> +		return -ENOMEM;
> +
> +	max_data_queues_le = virtio_cread32_le(vdev,
> +			offsetof(struct virtio_crypto_config, max_dataqueues));
> +	vcrypto->max_data_queues = le32_to_cpu(max_data_queues_le);
> +	if (vcrypto->max_data_queues < 1)
> +		vcrypto->max_data_queues = 1;
> +
> +	dev_info(&vdev->dev, "max_queues: %u\n", vcrypto->max_data_queues);
> +
> +	max_cipher_key_len_le = virtio_cread32_le(vdev,
> +		offsetof(struct virtio_crypto_config, max_cipher_key_len));
> +	max_auth_key_len_le = virtio_cread32_le(vdev,
> +		offsetof(struct virtio_crypto_config, max_auth_key_len));
> +	max_size_le = virtio_cread64_le(vdev,
> +		offsetof(struct virtio_crypto_config, max_size));
> +
> +	/* Add virtio crypto device to global table */
> +	err = virtcrypto_devmgr_add_dev(vcrypto);
> +	if (err) {
> +		dev_err(&vdev->dev, "Failed to add new virtio crypto device.\n");
> +		goto free;
> +	}
> +	vcrypto->owner = THIS_MODULE;
> +	vcrypto = vdev->priv = vcrypto;
> +	vcrypto->vdev = vdev;
> +	spin_lock_init(&vcrypto->lock);
> +	spin_lock_init(&vcrypto->ctrl_lock);
> +
> +	/* Use sigle data queue as default */
> +	vcrypto->curr_queue = 1;
> +	vcrypto->max_cipher_key_len = le32_to_cpu(max_cipher_key_len_le);
> +	vcrypto->max_auth_key_len = le32_to_cpu(max_auth_key_len_le);
> +	vcrypto->max_size = le64_to_cpu(max_size_le);
> +
> +	err = virtcrypto_init_vqs(vcrypto);
> +	if (err) {
> +		dev_err(&vdev->dev, "Failed to initialize vqs.\n");
> +		goto free_dev;
> +	}
> +	virtio_device_ready(vdev);
> +
> +	err = virtcrypto_update_status(vcrypto);
> +	if (err) {
> +		err = -EFAULT;

Why EFAULT?

> +		goto free_vqs;
> +	}
> +
> +	return 0;
> +
> +free_vqs:
> +	virtcrypto_del_vqs(vcrypto);

as you called virtio_device_ready this is likely unsafe
unless you first reset device or make sure there are
no interrupts in some other way.

> +free_dev:
> +	virtcrypto_devmgr_rm_dev(vcrypto);
> +free:
> +	kfree(vcrypto);
> +	return err;
> +}
> +
> +static void virtcrypto_free_unused_reqs(struct virtio_crypto *vcrypto)
> +{
> +	struct virtio_crypto_request *vc_req;
> +	int i;
> +	struct virtqueue *vq;
> +
> +	for (i = 0; i < vcrypto->max_data_queues; i++) {
> +		vq = vcrypto->data_vq[i].vq;
> +		while ((vc_req = virtqueue_detach_unused_buf(vq)) != NULL) {
> +			kfree(vc_req->req_data);
> +			kfree(vc_req->sgs);
> +		}
> +	}
> +}
> +
> +static void virtcrypto_remove(struct virtio_device *vdev)
> +{
> +	struct virtio_crypto *vcrypto = vdev->priv;
> +
> +	dev_info(&vdev->dev, "Start virtcrypto_remove.\n");
> +
> +	if (virtcrypto_dev_started(vcrypto))
> +		virtcrypto_dev_stop(vcrypto);
> +	vdev->config->reset(vdev);
> +	virtcrypto_free_unused_reqs(vcrypto);
> +	virtcrypto_del_vqs(vcrypto);
> +	virtcrypto_devmgr_rm_dev(vcrypto);
> +	kfree(vcrypto);
> +}
> +
> +static void virtcrypto_config_changed(struct virtio_device *vdev)
> +{
> +	struct virtio_crypto *vcrypto = vdev->priv;
> +
> +	virtcrypto_update_status(vcrypto);
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int virtcrypto_freeze(struct virtio_device *vdev)
> +{
> +	struct virtio_crypto *vcrypto = vdev->priv;
> +	unsigned long flags;
> +
> +	virtcrypto_free_unused_reqs(vcrypto);
> +	spin_lock_irqsave(&vcrypto->lock, flags);
> +	if (virtcrypto_dev_started(vcrypto))
> +		virtcrypto_dev_stop(vcrypto);

This is taking mutex locks under a spinlock.
Highly unlikely to work. Try this with lockdep
enabled.


> +	spin_unlock_irqrestore(&vcrypto->lock, flags);
> +
> +	virtcrypto_del_vqs(vcrypto);
> +	return 0;
> +}
> +
> +static int virtcrypto_restore(struct virtio_device *vdev)
> +{
> +	struct virtio_crypto *vcrypto = vdev->priv;
> +	int err;
> +
> +	err = virtcrypto_init_vqs(vcrypto);
> +	if (err)
> +		return err;
> +
> +	virtio_device_ready(vdev);
> +	err = virtcrypto_dev_start(vcrypto);
> +	if (err) {
> +		dev_err(&vdev->dev, "Failed to start virtio crypto device.\n");
> +		return -EFAULT;
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
> +
> +static unsigned int features[] = {
> +	/* none */
> +};
> +
> +static struct virtio_device_id id_table[] = {
> +	{ VIRTIO_ID_CRYPTO, VIRTIO_DEV_ANY_ID },
> +	{ 0 },
> +};
> +
> +static struct virtio_driver virtio_crypto_driver = {
> +	.driver.name         = KBUILD_MODNAME,
> +	.driver.owner        = THIS_MODULE,
> +	.feature_table       = features,
> +	.feature_table_size  = ARRAY_SIZE(features),
> +	.id_table            = id_table,
> +	.probe               = virtcrypto_probe,
> +	.remove              = virtcrypto_remove,
> +	.config_changed = virtcrypto_config_changed,
> +#ifdef CONFIG_PM_SLEEP
> +	.freeze = virtcrypto_freeze,
> +	.restore = virtcrypto_restore,
> +#endif
> +};
> +
> +module_virtio_driver(virtio_crypto_driver);
> +
> +MODULE_DEVICE_TABLE(virtio, id_table);
> +MODULE_DESCRIPTION("virtio crypto device driver");
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR("Gonglei <arei.gonglei@huawei.com>");
> diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c
> new file mode 100644
> index 0000000..7b57584
> --- /dev/null
> +++ b/drivers/crypto/virtio/virtio_crypto_algs.c
> @@ -0,0 +1,524 @@
> + /* Algorithms supported by virtio crypto device
> +  *
> +  * Authors: Gonglei <arei.gonglei@huawei.com>
> +  *
> +  * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
> +  *
> +  * This program is free software; you can redistribute it and/or modify
> +  * it under the terms of the GNU General Public License as published by
> +  * the Free Software Foundation; either version 2 of the License, or
> +  * (at your option) any later version.
> +  *
> +  * This program 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 General Public License for more details.
> +  *
> +  * You should have received a copy of the GNU General Public License
> +  * along with this program; if not, see <http://www.gnu.org/licenses/>.
> +  */
> +
> +#include <linux/scatterlist.h>
> +#include <crypto/algapi.h>
> +#include <linux/err.h>
> +#include <crypto/scatterwalk.h>
> +#include <linux/atomic.h>
> +
> +#include <uapi/linux/virtio_crypto.h>
> +#include "virtio_crypto_common.h"
> +
> +static DEFINE_MUTEX(algs_lock);
> +static unsigned int virtio_crypto_active_devs;
> +
> +static u64 virtio_crypto_alg_sg_nents_length(struct scatterlist *sg)
> +{
> +	u64 total = 0;
> +
> +	for (total = 0; sg; sg = sg_next(sg))
> +		total += sg->length;
> +
> +	return total;
> +}
> +
> +static int virtio_crypto_alg_validate_key(int key_len, int *alg)
> +{
> +	switch (key_len) {
> +	case AES_KEYSIZE_128:
> +	case AES_KEYSIZE_192:
> +	case AES_KEYSIZE_256:
> +		*alg = VIRTIO_CRYPTO_CIPHER_AES_CBC;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static int virtio_crypto_alg_ablkcipher_init_session(
> +		struct virtio_crypto_ablkcipher_ctx *ctx,
> +		int alg, const uint8_t *key,
> +		unsigned int keylen,
> +		int encrypt)
> +{
> +	struct scatterlist outhdr, key_sg, inhdr, *sgs[3];
> +	unsigned int tmp;
> +	struct virtio_crypto *vcrypto = ctx->vcrypto;
> +	int op = encrypt ? VIRTIO_CRYPTO_OP_ENCRYPT : VIRTIO_CRYPTO_OP_DECRYPT;
> +	int err;
> +	unsigned int num_out = 0, num_in = 0;
> +
> +	/*
> +	 * Avoid to do DMA from the stack, switch to using
> +	 * dynamically-allocated for the key
> +	 */
> +	uint8_t *cipher_key = kmalloc(keylen, GFP_ATOMIC);

Is all crypto using GFP_ATOMIC like this?
Is there code that recovers if this fails?


> +
> +	if (!cipher_key)
> +		return -ENOMEM;
> +
> +	memcpy(cipher_key, key, keylen);
> +
> +	spin_lock(&vcrypto->ctrl_lock);
> +	/* Pad ctrl header */
> +	vcrypto->ctrl.header.opcode =
> +		cpu_to_le32(VIRTIO_CRYPTO_CIPHER_CREATE_SESSION);
> +	vcrypto->ctrl.header.algo = cpu_to_le32((uint32_t)alg);

why cast here?

> +	/* Set the default dataqueue id to 0 */
> +	vcrypto->ctrl.header.queue_id = 0;
> +
> +	vcrypto->input.status = cpu_to_le32(VIRTIO_CRYPTO_ERR);
> +	/* Pad cipher's parameters */
> +	vcrypto->ctrl.u.sym_create_session.op_type =
> +		cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
> +	vcrypto->ctrl.u.sym_create_session.u.cipher.para.algo =
> +		vcrypto->ctrl.header.algo;
> +	vcrypto->ctrl.u.sym_create_session.u.cipher.para.keylen =
> +		cpu_to_le32(keylen);
> +	vcrypto->ctrl.u.sym_create_session.u.cipher.para.op =
> +		cpu_to_le32(op);
> +
> +	sg_init_one(&outhdr, &vcrypto->ctrl, sizeof(vcrypto->ctrl));
> +	sgs[num_out++] = &outhdr;
> +
> +	/* Set key */
> +	sg_init_one(&key_sg, cipher_key, keylen);
> +	sgs[num_out++] = &key_sg;
> +
> +	/* Return status and session id back */
> +	sg_init_one(&inhdr, &vcrypto->input, sizeof(vcrypto->input));
> +	sgs[num_out + num_in++] = &inhdr;
> +
> +	err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out,
> +				num_in, vcrypto, GFP_ATOMIC);
> +	if (err < 0) {
> +		spin_unlock(&vcrypto->ctrl_lock);
> +		kfree(cipher_key);
> +		return err;
> +	}
> +	virtqueue_kick(vcrypto->ctrl_vq);
> +
> +	/*
> +	 * Spin for a response, the kick causes an ioport write, trapping
> +	 * into the hypervisor, so the request should be handled immediately.
> +	 */
> +	while (!virtqueue_get_buf(vcrypto->ctrl_vq, &tmp) &&
> +	       !virtqueue_is_broken(vcrypto->ctrl_vq))
> +		cpu_relax();
> +
> +	if (le32_to_cpu(vcrypto->input.status) != VIRTIO_CRYPTO_OK) {
> +		spin_unlock(&vcrypto->ctrl_lock);
> +		pr_err("virtio_crypto: Create session failed status: %u\n",
> +			le32_to_cpu(vcrypto->input.status));
> +		kfree(cipher_key);
> +		return -EINVAL;
> +	}
> +	spin_unlock(&vcrypto->ctrl_lock);
> +
> +	spin_lock(&ctx->lock);
> +	if (encrypt)
> +		ctx->enc_sess_info.session_id =
> +			le64_to_cpu(vcrypto->input.session_id);
> +	else
> +		ctx->dec_sess_info.session_id =
> +			le64_to_cpu(vcrypto->input.session_id);
> +	spin_unlock(&ctx->lock);
> +
> +	kfree(cipher_key);
> +	return 0;
> +}
> +
> +static int virtio_crypto_alg_ablkcipher_close_session(
> +		struct virtio_crypto_ablkcipher_ctx *ctx,
> +		int encrypt)
> +{
> +	struct scatterlist outhdr, status_sg, *sgs[2];
> +	unsigned int tmp;
> +	struct virtio_crypto_destroy_session_req *destroy_session;
> +	struct virtio_crypto *vcrypto = ctx->vcrypto;
> +	int err;
> +	unsigned int num_out = 0, num_in = 0;
> +
> +	spin_lock(&vcrypto->ctrl_lock);
> +	vcrypto->ctrl_status.status = VIRTIO_CRYPTO_ERR;
> +	/* Pad ctrl header */
> +	vcrypto->ctrl.header.opcode =
> +		cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION);
> +	/* Set the default virtqueue id to 0 */
> +	vcrypto->ctrl.header.queue_id = 0;
> +
> +	destroy_session = &vcrypto->ctrl.u.destroy_session;
> +
> +	if (encrypt)
> +		destroy_session->session_id =
> +			cpu_to_le64(ctx->enc_sess_info.session_id);
> +	else
> +		destroy_session->session_id =
> +			cpu_to_le64(ctx->dec_sess_info.session_id);
> +
> +	sg_init_one(&outhdr, &vcrypto->ctrl, sizeof(vcrypto->ctrl));
> +	sgs[num_out++] = &outhdr;
> +
> +	/* Return status and session id back */
> +	sg_init_one(&status_sg, &vcrypto->ctrl_status.status,
> +		sizeof(vcrypto->ctrl_status.status));
> +	sgs[num_out + num_in++] = &status_sg;
> +
> +	err = virtqueue_add_sgs(vcrypto->ctrl_vq, sgs, num_out,
> +			num_in, vcrypto, GFP_ATOMIC);
> +	if (err < 0) {
> +		spin_unlock(&vcrypto->ctrl_lock);
> +		return err;
> +	}
> +	virtqueue_kick(vcrypto->ctrl_vq);
> +
> +	while (!virtqueue_get_buf(vcrypto->ctrl_vq, &tmp) &&
> +	       !virtqueue_is_broken(vcrypto->ctrl_vq))
> +		cpu_relax();
> +
> +	if (vcrypto->ctrl_status.status != VIRTIO_CRYPTO_OK) {
> +		spin_unlock(&vcrypto->ctrl_lock);
> +		pr_err("virtio_crypto: Close session failed status: %u, session_id: 0x%llx\n",
> +			vcrypto->ctrl_status.status,
> +			destroy_session->session_id);
> +
> +		return -EINVAL;
> +	}
> +	spin_unlock(&vcrypto->ctrl_lock);
> +
> +	return 0;
> +}
> +
> +static int virtio_crypto_alg_ablkcipher_init_sessions(
> +		struct virtio_crypto_ablkcipher_ctx *ctx,
> +		const uint8_t *key, unsigned int keylen)
> +{
> +	int alg;
> +	int ret;
> +	struct virtio_crypto *vcrypto = ctx->vcrypto;
> +
> +	if (keylen > vcrypto->max_cipher_key_len) {
> +		pr_err("virtio_crypto: the key is too long\n");
> +		goto bad_key;
> +	}
> +
> +	if (virtio_crypto_alg_validate_key(keylen, &alg))
> +		goto bad_key;
> +
> +	/* Create encryption session */
> +	ret = virtio_crypto_alg_ablkcipher_init_session(ctx,
> +			alg, key, keylen, 1);
> +	if (ret)
> +		return ret;
> +	/* Create decryption session */
> +	ret = virtio_crypto_alg_ablkcipher_init_session(ctx,
> +			alg, key, keylen, 0);
> +	if (ret) {
> +		virtio_crypto_alg_ablkcipher_close_session(ctx, 1);
> +		return ret;
> +	}
> +	return 0;
> +
> +bad_key:
> +	crypto_tfm_set_flags(ctx->tfm, CRYPTO_TFM_RES_BAD_KEY_LEN);
> +	return -EINVAL;
> +}
> +
> +/* Note: kernel crypto API realization */
> +static int virtio_crypto_ablkcipher_setkey(struct crypto_ablkcipher *tfm,
> +					 const uint8_t *key,
> +					 unsigned int keylen)
> +{
> +	struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(tfm);
> +	int ret;
> +
> +	spin_lock(&ctx->lock);
> +
> +	if (!ctx->vcrypto) {
> +		/* New key */
> +		int node = virtio_crypto_get_current_node();
> +		struct virtio_crypto *vcrypto =
> +				      virtcrypto_get_dev_node(node);
> +		if (!vcrypto) {
> +			vcrypto = virtcrypto_devmgr_get_first();
> +			if (!vcrypto) {
> +				pr_err("virtio_crypto: Could not find a virtio device in the system");
> +				spin_unlock(&ctx->lock);
> +				return -ENODEV;
> +			}
> +		}
> +
> +		ctx->vcrypto = vcrypto;
> +	}
> +	spin_unlock(&ctx->lock);
> +
> +	ret = virtio_crypto_alg_ablkcipher_init_sessions(ctx, key, keylen);
> +	if (ret) {
> +		virtcrypto_dev_put(ctx->vcrypto);
> +		ctx->vcrypto = NULL;
> +
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int
> +__virtio_crypto_ablkcipher_do_req(struct virtio_crypto_request *vc_req,
> +		struct ablkcipher_request *req,
> +		struct data_queue *data_vq,
> +		__u8 op)
> +{
> +	struct virtio_crypto_ablkcipher_ctx *ctx = vc_req->ablkcipher_ctx;
> +	struct virtio_crypto *vcrypto = ctx->vcrypto;
> +	struct virtio_crypto_op_data_req *req_data;
> +	int src_nents, dst_nents;
> +	int err;
> +	unsigned long flags;
> +	struct scatterlist outhdr, iv_sg, status_sg, **sgs;
> +	int i;
> +	u64 dst_len;
> +	unsigned int num_out = 0, num_in = 0;
> +	int sg_total;
> +
> +	src_nents = sg_nents_for_len(req->src, req->nbytes);
> +	dst_nents = sg_nents(req->dst);
> +
> +	pr_debug("virtio_crypto: Number of sgs (src_nents: %d, dst_nents: %d)\n",
> +			src_nents, dst_nents);
> +
> +	/* Why 3?  outhdr + iv + inhdr */
> +	sg_total = src_nents + dst_nents + 3;
> +	sgs = kzalloc_node(sg_total * sizeof(*sgs), GFP_ATOMIC,
> +				dev_to_node(&vcrypto->vdev->dev));
> +	if (!sgs)
> +		return -ENOMEM;
> +
> +	req_data = kzalloc_node(sizeof(*req_data), GFP_ATOMIC,
> +				dev_to_node(&vcrypto->vdev->dev));
> +	if (!req_data) {
> +		kfree(sgs);
> +		return -ENOMEM;
> +	}
> +
> +	vc_req->req_data = req_data;
> +	vc_req->type = VIRTIO_CRYPTO_SYM_OP_CIPHER;
> +	/* Head of operation */
> +	if (op) {
> +		req_data->header.session_id =
> +			cpu_to_le64(ctx->enc_sess_info.session_id);
> +		req_data->header.opcode =
> +			cpu_to_le32(VIRTIO_CRYPTO_CIPHER_ENCRYPT);
> +	} else {
> +		req_data->header.session_id =
> +			cpu_to_le64(ctx->dec_sess_info.session_id);
> +	    req_data->header.opcode =
> +			cpu_to_le32(VIRTIO_CRYPTO_CIPHER_DECRYPT);
> +	}
> +	req_data->u.sym_req.op_type = cpu_to_le32(VIRTIO_CRYPTO_SYM_OP_CIPHER);
> +	req_data->u.sym_req.u.cipher.para.iv_len = cpu_to_le32(AES_BLOCK_SIZE);
> +	req_data->u.sym_req.u.cipher.para.src_data_len =
> +			cpu_to_le32(req->nbytes);
> +
> +	dst_len = virtio_crypto_alg_sg_nents_length(req->dst);
> +	if (unlikely(dst_len > U32_MAX)) {
> +		pr_err("virtio_crypto: The dst_len is beyond U32_MAX\n");
> +		err = -EINVAL;
> +		goto free;
> +	}
> +
> +	pr_debug("virtio_crypto: src_len: %u, dst_len: %llu\n",
> +			req->nbytes, dst_len);
> +
> +	if (unlikely(req->nbytes + dst_len + AES_BLOCK_SIZE +
> +		sizeof(vc_req->status) > vcrypto->max_size)) {
> +		pr_err("virtio_crypto: The length is too big\n");
> +		err = -EINVAL;
> +		goto free;
> +	}
> +
> +	req_data->u.sym_req.u.cipher.para.dst_data_len =
> +			cpu_to_le32((uint32_t)dst_len);
> +
> +	/* Outhdr */
> +	sg_init_one(&outhdr, req_data, sizeof(*req_data));
> +	sgs[num_out++] = &outhdr;
> +
> +	/* IV */
> +	sg_init_one(&iv_sg, req->info, AES_BLOCK_SIZE);
> +	sgs[num_out++] = &iv_sg;
> +
> +	/* Source data */
> +	for (i = 0; i < src_nents; i++)
> +		sgs[num_out++] = &req->src[i];
> +
> +	/* Destination data */
> +	for (i = 0; i < dst_nents; i++)
> +		sgs[num_out + num_in++] = &req->dst[i];
> +
> +	/* Status */
> +	sg_init_one(&status_sg, &vc_req->status, sizeof(vc_req->status));
> +	sgs[num_out + num_in++] = &status_sg;
> +
> +	vc_req->sgs = sgs;
> +
> +	spin_lock_irqsave(&vcrypto->lock, flags);
> +	err = virtqueue_add_sgs(data_vq->vq, sgs, num_out,
> +				num_in, vc_req, GFP_ATOMIC);
> +	spin_unlock_irqrestore(&vcrypto->lock, flags);
> +	if (unlikely(err < 0))
> +		goto free;
> +
> +	return 0;
> +
> +free:
> +	kfree(req_data);
> +	kfree(sgs);
> +	return err;
> +}
> +
> +static int virtio_crypto_ablkcipher_encrypt(struct ablkcipher_request *req)
> +{
> +	struct crypto_ablkcipher *atfm = crypto_ablkcipher_reqtfm(req);
> +	struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(atfm);
> +	struct virtio_crypto_request *vc_req = ablkcipher_request_ctx(req);
> +	struct virtio_crypto *vcrypto = ctx->vcrypto;
> +	int ret;
> +	/* Use the first data virtqueue as default */
> +	struct data_queue *data_vq = &vcrypto->data_vq[0];
> +
> +	vc_req->ablkcipher_ctx = ctx;
> +	vc_req->ablkcipher_req = req;
> +	ret = __virtio_crypto_ablkcipher_do_req(vc_req, req, data_vq, 1);
> +	if (ret < 0) {
> +		pr_err("virtio_crypto: Encryption failed!\n");
> +		return ret;
> +	}
> +	virtqueue_kick(data_vq->vq);
> +
> +	return -EINPROGRESS;
> +}
> +
> +static int virtio_crypto_ablkcipher_decrypt(struct ablkcipher_request *req)
> +{
> +	struct crypto_ablkcipher *atfm = crypto_ablkcipher_reqtfm(req);
> +	struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(atfm);
> +	struct virtio_crypto_request *vc_req = ablkcipher_request_ctx(req);
> +	struct virtio_crypto *vcrypto = ctx->vcrypto;
> +	int ret;
> +	/* Use the first data virtqueue as default */
> +	struct data_queue *data_vq = &vcrypto->data_vq[0];
> +
> +	vc_req->ablkcipher_ctx = ctx;
> +	vc_req->ablkcipher_req = req;
> +
> +	ret = __virtio_crypto_ablkcipher_do_req(vc_req, req, data_vq, 0);
> +	if (ret < 0) {
> +		pr_err("virtio_crypto: Decryption failed!\n");
> +		return ret;
> +	}
> +	virtqueue_kick(data_vq->vq);
> +
> +	return -EINPROGRESS;
> +}
> +
> +static int virtio_crypto_ablkcipher_init(struct crypto_tfm *tfm)
> +{
> +	struct virtio_crypto_ablkcipher_ctx *ctx = crypto_tfm_ctx(tfm);
> +
> +	spin_lock_init(&ctx->lock);
> +	tfm->crt_ablkcipher.reqsize = sizeof(struct virtio_crypto_request);
> +	ctx->tfm = tfm;
> +
> +	return 0;
> +}
> +
> +static void virtio_crypto_ablkcipher_exit(struct crypto_tfm *tfm)
> +{
> +	struct virtio_crypto_ablkcipher_ctx *ctx = crypto_tfm_ctx(tfm);
> +
> +	if (!ctx->vcrypto)
> +		return;
> +
> +	virtio_crypto_alg_ablkcipher_close_session(ctx, 1);
> +	virtio_crypto_alg_ablkcipher_close_session(ctx, 0);
> +	virtcrypto_dev_put(ctx->vcrypto);
> +	ctx->vcrypto = NULL;
> +}
> +
> +static struct crypto_alg virtio_crypto_algs[] = { {
> +	.cra_name = "cbc(aes)",
> +	.cra_driver_name = "virtio_crypto_aes_cbc",
> +	.cra_priority = 4001,
> +	.cra_flags = CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC,
> +	.cra_blocksize = AES_BLOCK_SIZE,
> +	.cra_ctxsize  = sizeof(struct virtio_crypto_ablkcipher_ctx),
> +	.cra_alignmask = 0,
> +	.cra_module = THIS_MODULE,
> +	.cra_type = &crypto_ablkcipher_type,
> +	.cra_init = virtio_crypto_ablkcipher_init,
> +	.cra_exit = virtio_crypto_ablkcipher_exit,
> +	.cra_u = {
> +	   .ablkcipher = {
> +			.setkey = virtio_crypto_ablkcipher_setkey,
> +			.decrypt = virtio_crypto_ablkcipher_decrypt,
> +			.encrypt = virtio_crypto_ablkcipher_encrypt,
> +			.min_keysize = AES_MIN_KEY_SIZE,
> +			.max_keysize = AES_MAX_KEY_SIZE,
> +			.ivsize = AES_BLOCK_SIZE,
> +		},
> +	},
> +} };
> +
> +int virtio_crypto_algs_register(void)
> +{
> +	int ret = 0, i;
> +
> +	mutex_lock(&algs_lock);
> +	if (++virtio_crypto_active_devs != 1)
> +		goto unlock;
> +
> +	for (i = 0; i < ARRAY_SIZE(virtio_crypto_algs); i++) {
> +		virtio_crypto_algs[i].cra_flags =
> +			     CRYPTO_ALG_TYPE_ABLKCIPHER | CRYPTO_ALG_ASYNC;
> +	}
> +
> +	ret = crypto_register_algs(virtio_crypto_algs,
> +			ARRAY_SIZE(virtio_crypto_algs));
> +
> +unlock:
> +	mutex_unlock(&algs_lock);
> +	return ret;
> +}
> +
> +void virtio_crypto_algs_unregister(void)
> +{
> +	mutex_lock(&algs_lock);
> +	if (--virtio_crypto_active_devs != 0)
> +		goto unlock;
> +
> +	crypto_unregister_algs(virtio_crypto_algs,
> +			ARRAY_SIZE(virtio_crypto_algs));
> +
> +unlock:
> +	mutex_unlock(&algs_lock);
> +}
> diff --git a/drivers/crypto/virtio/virtio_crypto_common.h b/drivers/crypto/virtio/virtio_crypto_common.h
> new file mode 100644
> index 0000000..a599733
> --- /dev/null
> +++ b/drivers/crypto/virtio/virtio_crypto_common.h
> @@ -0,0 +1,124 @@
> +/* Common header for Virtio crypto device.
> + *
> + * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program 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 General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#ifndef _VIRITO_CRYPTO_COMMON_H
> +#define _VIRITO_CRYPTO_COMMON_H
> +
> +#include <linux/virtio.h>
> +#include <linux/crypto.h>
> +#include <linux/spinlock.h>
> +#include <crypto/aead.h>
> +#include <crypto/aes.h>
> +#include <crypto/authenc.h>
> +
> +
> +/* Internal representation of a data virtqueue */
> +struct data_queue {
> +	/* Virtqueue associated with this send _queue */
> +	struct virtqueue *vq;
> +
> +	/* Name of the tx queue: dataq.$index */
> +	char name[32];
> +};
> +
> +struct virtio_crypto {
> +	struct virtio_device *vdev;
> +	struct virtqueue *ctrl_vq;
> +	struct data_queue *data_vq;
> +
> +	/* To protect the vq operations for the dataq */
> +	spinlock_t lock;
> +
> +	/* To protect the vq operations for the controlq */
> +	spinlock_t ctrl_lock;
> +
> +	/* Maximum of data queues supported by the device */
> +	u32 max_data_queues;
> +
> +	/* Number of queue currently used by the driver */
> +	u32 curr_queue;
> +
> +	/* Maximum length of cipher key */
> +	u32 max_cipher_key_len;
> +	/* Maximum length of authenticated key */
> +	u32 max_auth_key_len;
> +	/* Maximum size of per request */
> +	u64 max_size;
> +
> +	/* Control VQ buffers: protected by the ctrl_lock */
> +	struct virtio_crypto_op_ctrl_req ctrl;
> +	struct virtio_crypto_session_input input;
> +	struct virtio_crypto_inhdr ctrl_status;
> +
> +	unsigned long status;
> +	atomic_t ref_count;
> +	struct list_head list;
> +	struct module *owner;
> +	uint8_t dev_id;
> +
> +	/* Does the affinity hint is set for virtqueues? */
> +	bool affinity_hint_set;
> +};
> +
> +struct virtio_crypto_sym_session_info {
> +	/* Backend session id, which come from the host side */
> +	__u64 session_id;
> +};
> +
> +struct virtio_crypto_ablkcipher_ctx {
> +	struct virtio_crypto *vcrypto;
> +	struct crypto_tfm *tfm;
> +
> +	struct virtio_crypto_sym_session_info enc_sess_info;
> +	struct virtio_crypto_sym_session_info dec_sess_info;
> +
> +	/* Protects virtio_crypto_ablkcipher_ctx struct */
> +	spinlock_t lock;
> +};
> +
> +struct virtio_crypto_request {
> +	/* Cipher or aead */
> +	uint32_t type;
> +	uint8_t status;
> +	struct virtio_crypto_ablkcipher_ctx *ablkcipher_ctx;
> +	struct ablkcipher_request *ablkcipher_req;
> +	struct virtio_crypto_op_data_req *req_data;
> +	struct scatterlist **sgs;
> +};
> +
> +int virtcrypto_devmgr_add_dev(struct virtio_crypto *vcrypto_dev);
> +struct list_head *virtcrypto_devmgr_get_head(void);
> +void virtcrypto_devmgr_rm_dev(struct virtio_crypto *vcrypto_dev);
> +struct virtio_crypto *virtcrypto_devmgr_get_first(void);
> +int virtcrypto_dev_in_use(struct virtio_crypto *vcrypto_dev);
> +int virtcrypto_dev_get(struct virtio_crypto *vcrypto_dev);
> +void virtcrypto_dev_put(struct virtio_crypto *vcrypto_dev);
> +int virtcrypto_dev_started(struct virtio_crypto *vcrypto_dev);
> +struct virtio_crypto *virtcrypto_get_dev_node(int node);
> +int virtcrypto_dev_start(struct virtio_crypto *vcrypto);
> +void virtcrypto_dev_stop(struct virtio_crypto *vcrypto);
> +
> +static inline int virtio_crypto_get_current_node(void)
> +{
> +	return topology_physical_package_id(smp_processor_id());
> +}
> +
> +int virtio_crypto_algs_register(void);
> +void virtio_crypto_algs_unregister(void);
> +
> +#endif /* _VIRITO_CRYPTO_COMMON_H */
> diff --git a/drivers/crypto/virtio/virtio_crypto_mgr.c b/drivers/crypto/virtio/virtio_crypto_mgr.c
> new file mode 100644
> index 0000000..5b7260c
> --- /dev/null
> +++ b/drivers/crypto/virtio/virtio_crypto_mgr.c
> @@ -0,0 +1,258 @@
> + /* Management for virtio crypto devices (refer to adf_dev_mgr.c)
> +  *
> +  * Copyright 2016 HUAWEI TECHNOLOGIES CO., LTD.
> +  *
> +  * This program is free software; you can redistribute it and/or modify
> +  * it under the terms of the GNU General Public License as published by
> +  * the Free Software Foundation; either version 2 of the License, or
> +  * (at your option) any later version.
> +  *
> +  * This program 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 General Public License for more details.
> +  *
> +  * You should have received a copy of the GNU General Public License
> +  * along with this program; if not, see <http://www.gnu.org/licenses/>.
> +  */
> +
> +#include <linux/mutex.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +
> +#include <uapi/linux/virtio_crypto.h>
> +#include "virtio_crypto_common.h"
> +
> +static LIST_HEAD(virtio_crypto_table);
> +static DEFINE_MUTEX(table_lock);
> +static uint32_t num_devices;
> +
> +#define VIRTIO_CRYPTO_MAX_DEVICES 32
> +
> +
> +/*
> + * virtcrypto_devmgr_add_dev() - Add vcrypto_dev to the acceleration
> + * framework.
> + * @vcrypto_dev:  Pointer to virtio crypto device.
> + *
> + * Function adds virtio crypto device to the global list.
> + * To be used by virtio crypto device specific drivers.
> + *
> + * Return: 0 on success, error code othewise.
> + */
> +int virtcrypto_devmgr_add_dev(struct virtio_crypto *vcrypto_dev)
> +{
> +	struct list_head *itr;
> +
> +	if (num_devices == VIRTIO_CRYPTO_MAX_DEVICES) {
> +		pr_info("virtio_crypto: only support up to %d devices\n",
> +			    VIRTIO_CRYPTO_MAX_DEVICES);
> +		return -EFAULT;
> +	}
> +
> +	mutex_lock(&table_lock);
> +	list_for_each(itr, &virtio_crypto_table) {
> +		struct virtio_crypto *ptr =
> +				list_entry(itr, struct virtio_crypto, list);
> +
> +		if (ptr == vcrypto_dev) {
> +			mutex_unlock(&table_lock);
> +			return -EEXIST;
> +		}
> +	}
> +	atomic_set(&vcrypto_dev->ref_count, 0);
> +	list_add_tail(&vcrypto_dev->list, &virtio_crypto_table);
> +	vcrypto_dev->dev_id = num_devices++;
> +	mutex_unlock(&table_lock);
> +	return 0;
> +}
> +
> +struct list_head *virtcrypto_devmgr_get_head(void)
> +{
> +	return &virtio_crypto_table;
> +}
> +
> +/*
> + * virtcrypto_devmgr_rm_dev() - Remove vcrypto_dev from the acceleration
> + * framework.
> + * @vcrypto_dev:  Pointer to virtio crypto device.
> + *
> + * Function removes virtio crypto device from the acceleration framework.
> + * To be used by virtio crypto device specific drivers.
> + *
> + * Return: void
> + */
> +void virtcrypto_devmgr_rm_dev(struct virtio_crypto *vcrypto_dev)
> +{
> +	mutex_lock(&table_lock);
> +	list_del(&vcrypto_dev->list);
> +	num_devices--;
> +	mutex_unlock(&table_lock);
> +}
> +
> +/*
> + * virtcrypto_devmgr_get_first()
> + *
> + * Function returns the first virtio crypto device from the acceleration
> + * framework.
> + *
> + * To be used by virtio crypto device specific drivers.
> + *
> + * Return: pointer to vcrypto_dev or NULL if not found.
> + */
> +struct virtio_crypto *virtcrypto_devmgr_get_first(void)
> +{
> +	struct virtio_crypto *dev = NULL;
> +
> +	if (!list_empty(&virtio_crypto_table))
> +		dev = list_first_entry(&virtio_crypto_table,
> +					struct virtio_crypto,
> +				    list);
> +	return dev;
> +}
> +
> +/*
> + * virtcrypto_dev_in_use() - Check whether vcrypto_dev is currently in use
> + * @vcrypto_dev: Pointer to virtio crypto device.
> + *
> + * To be used by virtio crypto device specific drivers.
> + *
> + * Return: 1 when device is in use, 0 otherwise.
> + */
> +int virtcrypto_dev_in_use(struct virtio_crypto *vcrypto_dev)
> +{
> +	return atomic_read(&vcrypto_dev->ref_count) != 0;
> +}
> +
> +/*
> + * virtcrypto_dev_get() - Increment vcrypto_dev reference count
> + * @vcrypto_dev: Pointer to virtio crypto device.
> + *
> + * Increment the vcrypto_dev refcount and if this is the first time
> + * incrementing it during this period the vcrypto_dev is in use,
> + * increment the module refcount too.
> + * To be used by virtio crypto device specific drivers.
> + *
> + * Return: 0 when successful, EFAULT when fail to bump module refcount
> + */
> +int virtcrypto_dev_get(struct virtio_crypto *vcrypto_dev)
> +{
> +	if (atomic_add_return(1, &vcrypto_dev->ref_count) == 1)
> +		if (!try_module_get(vcrypto_dev->owner))
> +			return -EFAULT;
> +	return 0;
> +}
> +
> +/*
> + * virtcrypto_dev_put() - Decrement vcrypto_dev reference count
> + * @vcrypto_dev: Pointer to virtio crypto device.
> + *
> + * Decrement the vcrypto_dev refcount and if this is the last time
> + * decrementing it during this period the vcrypto_dev is in use,
> + * decrement the module refcount too.
> + * To be used by virtio crypto device specific drivers.
> + *
> + * Return: void
> + */
> +void virtcrypto_dev_put(struct virtio_crypto *vcrypto_dev)
> +{
> +	if (atomic_sub_return(1, &vcrypto_dev->ref_count) == 0)
> +		module_put(vcrypto_dev->owner);
> +}
> +
> +/*
> + * virtcrypto_dev_started() - Check whether device has started
> + * @vcrypto_dev: Pointer to virtio crypto device.
> + *
> + * To be used by virtio crypto device specific drivers.
> + *
> + * Return: 1 when the device has started, 0 otherwise
> + */
> +int virtcrypto_dev_started(struct virtio_crypto *vcrypto_dev)
> +{
> +	return (vcrypto_dev->status & VIRTIO_CRYPTO_S_HW_READY);
> +}
> +
> +/*
> + * virtcrypto_get_dev_node() - Get vcrypto_dev on the node.
> + * @node:  Node id the driver works.
> + *
> + * Function returns the virtio crypto device used fewest on the node.
> + *
> + * To be used by virtio crypto device specific drivers.
> + *
> + * Return: pointer to vcrypto_dev or NULL if not found.
> + */
> +struct virtio_crypto *virtcrypto_get_dev_node(int node)
> +{
> +	struct virtio_crypto *vcrypto_dev = NULL, *tmp_dev;
> +	unsigned long best = ~0;
> +	unsigned long ctr;
> +
> +	list_for_each_entry(tmp_dev, virtcrypto_devmgr_get_head(), list) {
> +
> +		if ((node == dev_to_node(&tmp_dev->vdev->dev) ||
> +		     dev_to_node(&tmp_dev->vdev->dev) < 0) &&
> +		    virtcrypto_dev_started(tmp_dev)) {
> +			ctr = atomic_read(&tmp_dev->ref_count);
> +			if (best > ctr) {
> +				vcrypto_dev = tmp_dev;
> +				best = ctr;
> +			}
> +		}
> +	}
> +
> +	if (!vcrypto_dev) {
> +		pr_info("virtio_crypto: Could not find a device on node %d\n",
> +				node);
> +		/* Get any started device */
> +		list_for_each_entry(tmp_dev,
> +				virtcrypto_devmgr_get_head(), list) {
> +			if (virtcrypto_dev_started(tmp_dev)) {
> +				vcrypto_dev = tmp_dev;
> +				break;
> +			}
> +		}
> +	}
> +
> +	if (!vcrypto_dev)
> +		return NULL;
> +
> +	virtcrypto_dev_get(vcrypto_dev);
> +	return vcrypto_dev;
> +}
> +
> +/*
> + * virtcrypto_dev_start() - Start virtio crypto device
> + * @vcrypto:    Pointer to virtio crypto device.
> + *
> + * Function notifies all the registered services that the virtio crypto device
> + * is ready to be used.
> + * To be used by virtio crypto device specific drivers.
> + *
> + * Return: 0 on success, EFAULT when fail to register algorithms
> + */
> +int virtcrypto_dev_start(struct virtio_crypto *vcrypto)
> +{
> +	if (virtio_crypto_algs_register()) {
> +		pr_err("virtio_crypto: Failed to register crypto algs\n");
> +		return -EFAULT;
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * virtcrypto_dev_stop() - Stop virtio crypto device
> + * @vcrypto:    Pointer to virtio crypto device.
> + *
> + * Function notifies all the registered services that the virtio crypto device
> + * is ready to be used.
> + * To be used by virtio crypto device specific drivers.
> + *
> + * Return: void
> + */
> +void virtcrypto_dev_stop(struct virtio_crypto *vcrypto)
> +{
> +	virtio_crypto_algs_unregister();
> +}
> diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
> index cd2be1c..4bdb84c 100644
> --- a/include/uapi/linux/Kbuild
> +++ b/include/uapi/linux/Kbuild
> @@ -460,6 +460,7 @@ header-y += virtio_rng.h
>  header-y += virtio_scsi.h
>  header-y += virtio_types.h
>  header-y += virtio_vsock.h
> +header-y += virtio_crypto.h
>  header-y += vm_sockets.h
>  header-y += vt.h
>  header-y += vtpm_proxy.h
> diff --git a/include/uapi/linux/virtio_crypto.h b/include/uapi/linux/virtio_crypto.h
> new file mode 100644
> index 0000000..abc2cf5
> --- /dev/null
> +++ b/include/uapi/linux/virtio_crypto.h
> @@ -0,0 +1,435 @@
> +#ifndef _VIRTIO_CRYPTO_H
> +#define _VIRTIO_CRYPTO_H
> +/* This header is BSD licensed so anyone can use the definitions to implement
> + * compatible drivers/servers.
> + *
> + * Redistribution and use in source and binary forms, with or without
> + * modification, are permitted provided that the following conditions
> + * are met:
> + * 1. Redistributions of source code must retain the above copyright
> + *    notice, this list of conditions and the following disclaimer.
> + * 2. Redistributions in binary form must reproduce the above copyright
> + *    notice, this list of conditions and the following disclaimer in the
> + *    documentation and/or other materials provided with the distribution.
> + * 3. Neither the name of IBM nor the names of its contributors
> + *    may be used to endorse or promote products derived from this software
> + *    without specific prior written permission.
> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + * ``AS IS'' AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
> + * FOR A PARTICULAR PURPOSE ARE DISCLAIMED.  IN NO EVENT SHALL IBM OR
> + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
> + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> + * SUCH DAMAGE.
> + */
> +#include <linux/types.h>
> +#include <linux/virtio_types.h>
> +#include <linux/virtio_ids.h>
> +#include <linux/virtio_config.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
> +
> +#define VIRTIO_CRYPTO_OPCODE(service, op)   (((service) << 8) | (op))
> +
> +struct virtio_crypto_ctrl_header {
> +#define VIRTIO_CRYPTO_CIPHER_CREATE_SESSION \
> +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x02)
> +#define VIRTIO_CRYPTO_CIPHER_DESTROY_SESSION \
> +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x03)
> +#define VIRTIO_CRYPTO_HASH_CREATE_SESSION \
> +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x02)
> +#define VIRTIO_CRYPTO_HASH_DESTROY_SESSION \
> +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x03)
> +#define VIRTIO_CRYPTO_MAC_CREATE_SESSION \
> +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x02)
> +#define VIRTIO_CRYPTO_MAC_DESTROY_SESSION \
> +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x03)
> +#define VIRTIO_CRYPTO_AEAD_CREATE_SESSION \
> +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x02)
> +#define VIRTIO_CRYPTO_AEAD_DESTROY_SESSION \
> +	   VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x03)
> +	__le32 opcode;
> +	__le32 algo;
> +	__le32 flag;
> +	/* data virtqueue id */
> +	__le32 queue_id;
> +};
> +
> +struct virtio_crypto_cipher_session_para {
> +#define VIRTIO_CRYPTO_NO_CIPHER                 0
> +#define VIRTIO_CRYPTO_CIPHER_ARC4               1
> +#define VIRTIO_CRYPTO_CIPHER_AES_ECB            2
> +#define VIRTIO_CRYPTO_CIPHER_AES_CBC            3
> +#define VIRTIO_CRYPTO_CIPHER_AES_CTR            4
> +#define VIRTIO_CRYPTO_CIPHER_DES_ECB            5
> +#define VIRTIO_CRYPTO_CIPHER_DES_CBC            6
> +#define VIRTIO_CRYPTO_CIPHER_3DES_ECB           7
> +#define VIRTIO_CRYPTO_CIPHER_3DES_CBC           8
> +#define VIRTIO_CRYPTO_CIPHER_3DES_CTR           9
> +#define VIRTIO_CRYPTO_CIPHER_KASUMI_F8          10
> +#define VIRTIO_CRYPTO_CIPHER_SNOW3G_UEA2        11
> +#define VIRTIO_CRYPTO_CIPHER_AES_F8             12
> +#define VIRTIO_CRYPTO_CIPHER_AES_XTS            13
> +#define VIRTIO_CRYPTO_CIPHER_ZUC_EEA3           14
> +	__le32 algo;
> +	/* length of key */
> +	__le32 keylen;
> +
> +#define VIRTIO_CRYPTO_OP_ENCRYPT  1
> +#define VIRTIO_CRYPTO_OP_DECRYPT  2
> +	/* encrypt or decrypt */
> +	__le32 op;
> +	__le32 padding;
> +};
> +
> +struct virtio_crypto_session_input {
> +	/* Device-writable part */
> +	__le64 session_id;
> +	__le32 status;
> +	__le32 padding;
> +};
> +
> +struct virtio_crypto_cipher_session_req {
> +	struct virtio_crypto_cipher_session_para para;
> +};
> +
> +struct virtio_crypto_hash_session_para {
> +#define VIRTIO_CRYPTO_NO_HASH            0
> +#define VIRTIO_CRYPTO_HASH_MD5           1
> +#define VIRTIO_CRYPTO_HASH_SHA1          2
> +#define VIRTIO_CRYPTO_HASH_SHA_224       3
> +#define VIRTIO_CRYPTO_HASH_SHA_256       4
> +#define VIRTIO_CRYPTO_HASH_SHA_384       5
> +#define VIRTIO_CRYPTO_HASH_SHA_512       6
> +#define VIRTIO_CRYPTO_HASH_SHA3_224      7
> +#define VIRTIO_CRYPTO_HASH_SHA3_256      8
> +#define VIRTIO_CRYPTO_HASH_SHA3_384      9
> +#define VIRTIO_CRYPTO_HASH_SHA3_512      10
> +#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE128      11
> +#define VIRTIO_CRYPTO_HASH_SHA3_SHAKE256      12
> +	__le32 algo;
> +	/* hash result length */
> +	__le32 hash_result_len;
> +};
> +
> +struct virtio_crypto_hash_create_session_req {
> +	struct virtio_crypto_hash_session_para para;
> +};
> +
> +struct virtio_crypto_mac_session_para {
> +#define VIRTIO_CRYPTO_NO_MAC                       0
> +#define VIRTIO_CRYPTO_MAC_HMAC_MD5                 1
> +#define VIRTIO_CRYPTO_MAC_HMAC_SHA1                2
> +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_224             3
> +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_256             4
> +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_384             5
> +#define VIRTIO_CRYPTO_MAC_HMAC_SHA_512             6
> +#define VIRTIO_CRYPTO_MAC_CMAC_3DES                25
> +#define VIRTIO_CRYPTO_MAC_CMAC_AES                 26
> +#define VIRTIO_CRYPTO_MAC_KASUMI_F9                27
> +#define VIRTIO_CRYPTO_MAC_SNOW3G_UIA2              28
> +#define VIRTIO_CRYPTO_MAC_GMAC_AES                 41
> +#define VIRTIO_CRYPTO_MAC_GMAC_TWOFISH             42
> +#define VIRTIO_CRYPTO_MAC_CBCMAC_AES               49
> +#define VIRTIO_CRYPTO_MAC_CBCMAC_KASUMI_F9         50
> +#define VIRTIO_CRYPTO_MAC_XCBC_AES                 53
> +	__le32 algo;
> +	/* hash result length */
> +	__le32 hash_result_len;
> +	/* length of authenticated key */
> +	__le32 auth_key_len;
> +	__le32 padding;
> +};
> +
> +struct virtio_crypto_mac_create_session_req {
> +	struct virtio_crypto_mac_session_para para;
> +};
> +
> +struct virtio_crypto_aead_session_para {
> +#define VIRTIO_CRYPTO_NO_AEAD     0
> +#define VIRTIO_CRYPTO_AEAD_GCM    1
> +#define VIRTIO_CRYPTO_AEAD_CCM    2
> +#define VIRTIO_CRYPTO_AEAD_CHACHA20_POLY1305  3
> +	__le32 algo;
> +	/* length of key */
> +	__le32 key_len;
> +	/* hash result length */
> +	__le32 hash_result_len;
> +	/* length of the additional authenticated data (AAD) in bytes */
> +	__le32 aad_len;
> +	/* encrypt or decrypt, See above VIRTIO_CRYPTO_OP_* */
> +	__le32 op;
> +	__le32 padding;
> +};
> +
> +struct virtio_crypto_aead_create_session_req {
> +	struct virtio_crypto_aead_session_para para;
> +};
> +
> +struct virtio_crypto_alg_chain_session_para {
> +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_HASH_THEN_CIPHER  1
> +#define VIRTIO_CRYPTO_SYM_ALG_CHAIN_ORDER_CIPHER_THEN_HASH  2
> +	__le32 alg_chain_order;
> +/* Plain hash */
> +#define VIRTIO_CRYPTO_SYM_HASH_MODE_PLAIN    1
> +/* Authenticated hash (mac) */
> +#define VIRTIO_CRYPTO_SYM_HASH_MODE_AUTH     2
> +/* Nested hash */
> +#define VIRTIO_CRYPTO_SYM_HASH_MODE_NESTED   3
> +	__le32 hash_mode;
> +	struct virtio_crypto_cipher_session_para cipher_param;
> +	union {
> +		struct virtio_crypto_hash_session_para hash_param;
> +		struct virtio_crypto_mac_session_para mac_param;
> +	} u;

I personally dislike the use of unions of different size
in these structures. For example, using hash_param,
there's no way to initialize the trailing part of the
structure. Also, the size of the structure is hard to
figure out. I would like to see padding of all structures
to be same size, and adding a union member that's just padding.

Same applies to other unions.

> +	/* length of the additional authenticated data (AAD) in bytes */
> +	__le32 aad_len;
> +	__le32 padding;
> +};
> +
> +struct virtio_crypto_alg_chain_session_req {
> +	struct virtio_crypto_alg_chain_session_para para;
> +};
> +
> +struct virtio_crypto_sym_create_session_req {
> +	union {
> +		struct virtio_crypto_cipher_session_req cipher;
> +		struct virtio_crypto_alg_chain_session_req chain;
> +	} u;
> +
> +	/* Device-readable part */
> +
> +/* No operation */
> +#define VIRTIO_CRYPTO_SYM_OP_NONE  0
> +/* Cipher only operation on the data */
> +#define VIRTIO_CRYPTO_SYM_OP_CIPHER  1
> +/*
> + * Chain any cipher with any hash or mac operation. The order
> + * depends on the value of alg_chain_order param
> + */
> +#define VIRTIO_CRYPTO_SYM_OP_ALGORITHM_CHAINING  2
> +	__le32 op_type;
> +	__le32 padding;
> +};
> +
> +struct virtio_crypto_destroy_session_req {
> +	/* Device-readable part */
> +	__le64  session_id;
> +};
> +
> +/* The request of the control viritqueue's packet */
> +struct virtio_crypto_op_ctrl_req {
> +	struct virtio_crypto_ctrl_header header;
> +
> +	union {
> +		struct virtio_crypto_sym_create_session_req
> +			sym_create_session;
> +		struct virtio_crypto_hash_create_session_req
> +			hash_create_session;
> +		struct virtio_crypto_mac_create_session_req
> +			mac_create_session;
> +		struct virtio_crypto_aead_create_session_req
> +			aead_create_session;
> +		struct virtio_crypto_destroy_session_req
> +			destroy_session;
> +	} u;
> +};
> +
> +struct virtio_crypto_op_header {
> +#define VIRTIO_CRYPTO_CIPHER_ENCRYPT \
> +	VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x00)
> +#define VIRTIO_CRYPTO_CIPHER_DECRYPT \
> +	VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_CIPHER, 0x01)
> +#define VIRTIO_CRYPTO_HASH \
> +	VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_HASH, 0x00)
> +#define VIRTIO_CRYPTO_MAC \
> +	VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_MAC, 0x00)
> +#define VIRTIO_CRYPTO_AEAD_ENCRYPT \
> +	VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x00)
> +#define VIRTIO_CRYPTO_AEAD_DECRYPT \
> +	VIRTIO_CRYPTO_OPCODE(VIRTIO_CRYPTO_SERVICE_AEAD, 0x01)
> +	__le32 opcode;
> +	/* algo should be service-specific algorithms */
> +	__le32 algo;
> +	/* session_id should be service-specific algorithms */
> +	__le64 session_id;
> +	/* control flag to control the request */
> +	__le32 flag;
> +	__le32 padding;
> +};
> +
> +struct virtio_crypto_cipher_para {
> +	/*
> +	 * Byte Length of valid IV/Counter
> +	 *
> +	 * For block ciphers in CBC or F8 mode, or for Kasumi in F8 mode, or for
> +	 *   SNOW3G in UEA2 mode, this is the length of the IV (which
> +	 *   must be the same as the block length of the cipher).
> +	 * For block ciphers in CTR mode, this is the length of the counter
> +	 *   (which must be the same as the block length of the cipher).
> +	 * For AES-XTS, this is the 128bit tweak, i, from IEEE Std 1619-2007.
> +	 *
> +	 * The IV/Counter will be updated after every partial cryptographic
> +	 * operation.
> +	 */
> +	__le32 iv_len;
> +	/* length of source data */
> +	__le32 src_data_len;
> +	/* length of dst data */
> +	__le32 dst_data_len;
> +	__le32 padding;
> +};
> +
> +struct virtio_crypto_hash_para {
> +	/* length of source data */
> +	__le32 src_data_len;
> +	/* hash result length */
> +	__le32 hash_result_len;
> +};
> +
> +struct virtio_crypto_mac_para {
> +	struct virtio_crypto_hash_para hash;
> +};
> +
> +struct virtio_crypto_aead_para {
> +	/*
> +	 * Byte Length of valid IV data pointed to by the below iv_addr
> +	 * parameter.
> +	 *
> +	 * For GCM mode, this is either 12 (for 96-bit IVs) or 16, in which
> +	 *   case iv_addr points to J0.
> +	 * For CCM mode, this is the length of the nonce, which can be in the
> +	 *   range 7 to 13 inclusive.
> +	 */
> +	__le32 iv_len;
> +	/* length of additional auth data */
> +	__le32 aad_len;
> +	/* length of source data */
> +	__le32 src_data_len;
> +	/* length of dst data */
> +	__le32 dst_data_len;
> +};
> +
> +struct virtio_crypto_cipher_data_req {
> +	/* Device-readable part */
> +	struct virtio_crypto_cipher_para para;
> +};
> +
> +struct virtio_crypto_hash_data_req {
> +	/* Device-readable part */
> +	struct virtio_crypto_hash_para para;
> +};
> +
> +struct virtio_crypto_mac_data_req {
> +	/* Device-readable part */
> +	struct virtio_crypto_mac_para para;
> +};
> +
> +struct virtio_crypto_alg_chain_data_para {
> +	__le32 iv_len;
> +	/* Length of source data */
> +	__le32 src_data_len;
> +	/* Length of destination data */
> +	__le32 dst_data_len;
> +	/* Starting point for cipher processing in source data */
> +	__le32 cipher_start_src_offset;
> +	/* Length of the source data that the cipher will be computed on */
> +	__le32 len_to_cipher;
> +	/* Starting point for hash processing in source data */
> +	__le32 hash_start_src_offset;
> +	/* Length of the source data that the hash will be computed on */
> +	__le32 len_to_hash;
> +	/* Length of the additional auth data */
> +	__le32 aad_len;
> +	/* Length of the hash result */
> +	__le32 hash_result_len;
> +	__le32 reserved;
> +};
> +
> +struct virtio_crypto_alg_chain_data_req {
> +	/* Device-readable part */
> +	struct virtio_crypto_alg_chain_data_para para;
> +};
> +
> +struct virtio_crypto_sym_data_req {
> +	union {
> +		struct virtio_crypto_cipher_data_req cipher;
> +		struct virtio_crypto_alg_chain_data_req chain;
> +	} u;
> +
> +	/* See above VIRTIO_CRYPTO_SYM_OP_* */
> +	__le32 op_type;
> +	__le32 padding;
> +};
> +
> +struct virtio_crypto_aead_data_req {
> +	/* Device-readable part */
> +	struct virtio_crypto_aead_para para;
> +};
> +
> +/* The request of the data viritqueue's packet */
> +struct virtio_crypto_op_data_req {
> +	struct virtio_crypto_op_header header;
> +
> +	union {
> +		struct virtio_crypto_sym_data_req  sym_req;
> +		struct virtio_crypto_hash_data_req hash_req;
> +		struct virtio_crypto_mac_data_req mac_req;
> +		struct virtio_crypto_aead_data_req aead_req;
> +	} u;
> +};
> +
> +#define VIRTIO_CRYPTO_OK        0
> +#define VIRTIO_CRYPTO_ERR       1
> +#define VIRTIO_CRYPTO_BADMSG    2
> +#define VIRTIO_CRYPTO_NOTSUPP   3
> +#define VIRTIO_CRYPTO_INVSESS   4 /* Invaild session id */
> +
> +/* The accelerator hardware is ready */
> +#define VIRTIO_CRYPTO_S_HW_READY  (1 << 0)
> +
> +struct virtio_crypto_config {
> +	/* See VIRTIO_CRYPTO_OP_* above */
> +	__le32  status;
> +
> +	/*
> +	 * Maximum number of data queue legal values are between 1 and 0x8000
> +	 */
> +	__le32  max_dataqueues;
> +
> +	/*
> +	 * Specifies the services mask which the devcie support,
> +	 * see VIRTIO_CRYPTO_SERVICE_* above
> +	 */
> +	__le32 crypto_services;
> +
> +	/* Detailed algorithms mask */
> +	__le32 cipher_algo_l;
> +	__le32 cipher_algo_h;
> +	__le32 hash_algo;
> +	__le32 mac_algo_l;
> +	__le32 mac_algo_h;
> +	__le32 aead_algo;
> +	/* Maximum length of cipher key */
> +	__le32 max_cipher_key_len;
> +	/* Maximum length of authenticated key */
> +	__le32 max_auth_key_len;
> +	__le32 reserve;
> +	/* Maximum size of each crypto request's content */
> +	__le64 max_size;
> +};
> +
> +struct virtio_crypto_inhdr {
> +	/* See VIRTIO_CRYPTO_* above */
> +	__u8 status;
> +};
> +#endif
> diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
> index 3228d58..6d5c3b2 100644
> --- a/include/uapi/linux/virtio_ids.h
> +++ b/include/uapi/linux/virtio_ids.h
> @@ -42,5 +42,6 @@
>  #define VIRTIO_ID_GPU          16 /* virtio GPU */
>  #define VIRTIO_ID_INPUT        18 /* virtio input */
>  #define VIRTIO_ID_VSOCK        19 /* virtio vsock transport */
> +#define VIRTIO_ID_CRYPTO       20 /* virtio crypto */
>  
>  #endif /* _LINUX_VIRTIO_IDS_H */
> -- 
> 1.8.3.1
> 

^ permalink raw reply

* [PATCH 2/2] crypto: arm/crc32 - accelerated support based on x86 SSE implementation
From: Ard Biesheuvel @ 2016-11-26 20:15 UTC (permalink / raw)
  To: linux-crypto, herbert, linux-arm-kernel, catalin.marinas,
	will.deacon, linux
  Cc: steve.capper, Ard Biesheuvel
In-Reply-To: <1480191314-2331-1-git-send-email-ard.biesheuvel@linaro.org>

This is a combination of the the Intel algorithm implemented using SSE
and PCLMULQDQ instructions from arch/x86/crypto/crc32-pclmul_asm.S, and
the new CRC32 extensions introduced for both 32-bit and 64-bit ARM in
version 8 of the architecture.

The PMULL/NEON algorithm is faster, but operates on blocks of at least
64 bytes, and on multiples of 16 bytes only. For the remaining input,
or for all input on systems that lack the PMULL 64x64->128 instructions,
the CRC32 instructions will be used.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/crypto/Kconfig         |   5 +
 arch/arm/crypto/Makefile        |   2 +
 arch/arm/crypto/crc32-ce-core.S | 257 ++++++++++++++++++++
 arch/arm/crypto/crc32-ce-glue.c | 129 ++++++++++
 4 files changed, 393 insertions(+)

diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
index fce801fa52a1..be5cb5a7d3fa 100644
--- a/arch/arm/crypto/Kconfig
+++ b/arch/arm/crypto/Kconfig
@@ -125,4 +125,9 @@ config CRYPTO_CRCT10DIF_ARM_CE
 	depends on KERNEL_MODE_NEON && CRC_T10DIF
 	select CRYPTO_HASH
 
+config CRYPTO_CRC32_ARM_CE
+	tristate "CRC32 digest algorithm using CRC and/or PMULL instructions"
+	depends on KERNEL_MODE_NEON && CRC32
+	select CRYPTO_HASH
+
 endif
diff --git a/arch/arm/crypto/Makefile b/arch/arm/crypto/Makefile
index fc77265014b7..b578a1820ab1 100644
--- a/arch/arm/crypto/Makefile
+++ b/arch/arm/crypto/Makefile
@@ -14,6 +14,7 @@ ce-obj-$(CONFIG_CRYPTO_SHA1_ARM_CE) += sha1-arm-ce.o
 ce-obj-$(CONFIG_CRYPTO_SHA2_ARM_CE) += sha2-arm-ce.o
 ce-obj-$(CONFIG_CRYPTO_GHASH_ARM_CE) += ghash-arm-ce.o
 ce-obj-$(CONFIG_CRYPTO_CRCT10DIF_ARM_CE) += crct10dif-arm-ce.o
+ce-obj-$(CONFIG_CRYPTO_CRC32_ARM_CE) += crc32-arm-ce.o
 
 ifneq ($(ce-obj-y)$(ce-obj-m),)
 ifeq ($(call as-instr,.fpu crypto-neon-fp-armv8,y,n),y)
@@ -38,6 +39,7 @@ sha2-arm-ce-y	:= sha2-ce-core.o sha2-ce-glue.o
 aes-arm-ce-y	:= aes-ce-core.o aes-ce-glue.o
 ghash-arm-ce-y	:= ghash-ce-core.o ghash-ce-glue.o
 crct10dif-arm-ce-y	:= crct10dif-ce-core.o crct10dif-ce-glue.o
+crc32-arm-ce-y:= crc32-ce-core.o crc32-ce-glue.o
 
 quiet_cmd_perl = PERL    $@
       cmd_perl = $(PERL) $(<) > $(@)
diff --git a/arch/arm/crypto/crc32-ce-core.S b/arch/arm/crypto/crc32-ce-core.S
new file mode 100644
index 000000000000..ef671f040672
--- /dev/null
+++ b/arch/arm/crypto/crc32-ce-core.S
@@ -0,0 +1,257 @@
+/*
+ * Accelerated CRC32 using ARM CRC, NEON and Crypto Extensions instructions
+ *
+ * Copyright (C) 2016 Linaro Ltd <ard.biesheuvel@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/* GPL HEADER START
+ *
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 only,
+ * as published by the Free Software Foundation.
+ *
+ * This program 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
+ * General Public License version 2 for more details (a copy is included
+ * in the LICENSE file that accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 along with this program; If not, see http://www.gnu.org/licenses
+ *
+ * Please  visit http://www.xyratex.com/contact if you need additional
+ * information or have any questions.
+ *
+ * GPL HEADER END
+ */
+
+/*
+ * Copyright 2012 Xyratex Technology Limited
+ *
+ * Using hardware provided PCLMULQDQ instruction to accelerate the CRC32
+ * calculation.
+ * CRC32 polynomial:0x04c11db7(BE)/0xEDB88320(LE)
+ * PCLMULQDQ is a new instruction in Intel SSE4.2, the reference can be found
+ * at:
+ * http://www.intel.com/products/processor/manuals/
+ * Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+ * Volume 2B: Instruction Set Reference, N-Z
+ *
+ * Authors:   Gregory Prestas <Gregory_Prestas@us.xyratex.com>
+ *	      Alexander Boyko <Alexander_Boyko@xyratex.com>
+ */
+
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+
+	.text
+	.align		4
+	.arch		armv8-a
+	.arch_extension	crc
+	.fpu		crypto-neon-fp-armv8
+
+	/*
+	 * [x4*128+32 mod P(x) << 32)]'  << 1   = 0x154442bd4
+	 * #define CONSTANT_R1  0x154442bd4LL
+	 *
+	 * [(x4*128-32 mod P(x) << 32)]' << 1   = 0x1c6e41596
+	 * #define CONSTANT_R2  0x1c6e41596LL
+	 */
+.Lconstant_R2R1:
+	.quad		0x0000000154442bd4
+	.quad		0x00000001c6e41596
+
+	/*
+	 * [(x128+32 mod P(x) << 32)]'   << 1   = 0x1751997d0
+	 * #define CONSTANT_R3  0x1751997d0LL
+	 *
+	 * [(x128-32 mod P(x) << 32)]'   << 1   = 0x0ccaa009e
+	 * #define CONSTANT_R4  0x0ccaa009eLL
+	 */
+.Lconstant_R4R3:
+	.quad		0x00000001751997d0
+	.quad		0x00000000ccaa009e
+
+	/*
+	 * [(x64 mod P(x) << 32)]'       << 1   = 0x163cd6124
+	 * #define CONSTANT_R5  0x163cd6124LL
+	 */
+.Lconstant_R5:
+	.quad		0x0000000163cd6124
+
+.Lconstant_mask32:
+	.quad		0x00000000FFFFFFFF
+
+	/*
+	 * #define CRCPOLY_TRUE_LE_FULL 0x1DB710641LL
+	 *
+	 * Barrett Reduction constant (u64`) = u` = (x**64 / P(x))`
+	 *                                                      = 0x1F7011641LL
+	 * #define CONSTANT_RU  0x1F7011641LL
+	 */
+.Lconstant_RUpoly:
+	.quad		0x00000001DB710641
+	.quad		0x00000001F7011641
+
+	dCONSTANTl	.req	d0
+	dCONSTANTh	.req	d1
+	qCONSTANT	.req	q0
+
+	BUF		.req	r0
+	LEN		.req	r1
+	CRC		.req	r2
+
+	qzr		.req	q9
+
+	/**
+	 * Calculate crc32
+	 * BUF - buffer
+	 * LEN - sizeof buffer (multiple of 16 bytes), LEN should be > 63
+	 * CRC - initial crc32
+	 * return %eax crc32
+	 * uint crc32_pmull_le(unsigned char const *buffer,
+	 *                     size_t len, uint crc32)
+	 */
+ENTRY(crc32_pmull_le)
+	bic		LEN, LEN, #15
+	vld1.8		{q1-q2}, [BUF]!
+	vld1.8		{q3-q4}, [BUF]!
+	vmov.i8		qzr, #0
+	vmov.i8		qCONSTANT, #0
+	vmov		dCONSTANTl[0], CRC
+	veor.8		d2, d2, dCONSTANTl
+	sub		LEN, LEN, #0x40
+	cmp		LEN, #0x40
+	blt		less_64
+
+	vldr		dCONSTANTl, .Lconstant_R2R1
+	vldr		dCONSTANTh, .Lconstant_R2R1 + 8
+
+loop_64:		/* 64 bytes Full cache line folding */
+	sub		LEN, LEN, #0x40
+
+	vmull.p64	q5, d3, dCONSTANTh
+	vmull.p64	q6, d5, dCONSTANTh
+	vmull.p64	q7, d7, dCONSTANTh
+	vmull.p64	q8, d9, dCONSTANTh
+
+	vmull.p64	q1, d2, dCONSTANTl
+	vmull.p64	q2, d4, dCONSTANTl
+	vmull.p64	q3, d6, dCONSTANTl
+	vmull.p64	q4, d8, dCONSTANTl
+
+	veor.8		q1, q1, q5
+	vld1.8		{q5}, [BUF]!
+	veor.8		q2, q2, q6
+	vld1.8		{q6}, [BUF]!
+	veor.8		q3, q3, q7
+	vld1.8		{q7}, [BUF]!
+	veor.8		q4, q4, q8
+	vld1.8		{q8}, [BUF]!
+
+	veor.8		q1, q1, q5
+	veor.8		q2, q2, q6
+	veor.8		q3, q3, q7
+	veor.8		q4, q4, q8
+
+	cmp		LEN, #0x40
+	bge		loop_64
+
+less_64:		/* Folding cache line into 128bit */
+	vldr		dCONSTANTl, .Lconstant_R4R3
+	vldr		dCONSTANTh, .Lconstant_R4R3 + 8
+
+	vmull.p64	q5, d3, dCONSTANTh
+	vmull.p64	q1, d2, dCONSTANTl
+	veor.8		q1, q1, q5
+	veor.8		q1, q1, q2
+
+	vmull.p64	q5, d3, dCONSTANTh
+	vmull.p64	q1, d2, dCONSTANTl
+	veor.8		q1, q1, q5
+	veor.8		q1, q1, q3
+
+	vmull.p64	q5, d3, dCONSTANTh
+	vmull.p64	q1, d2, dCONSTANTl
+	veor.8		q1, q1, q5
+	veor.8		q1, q1, q4
+
+	teq		LEN, #0
+	beq		fold_64
+
+loop_16:		/* Folding rest buffer into 128bit */
+	subs		LEN, LEN, #0x10
+
+	vld1.8		{q2}, [BUF]!
+	vmull.p64	q5, d3, dCONSTANTh
+	vmull.p64	q1, d2, dCONSTANTl
+	veor.8		q1, q1, q5
+	veor.8		q1, q1, q2
+
+	bne		loop_16
+
+fold_64:
+	/* perform the last 64 bit fold, also adds 32 zeroes
+	 * to the input stream */
+	vmull.p64	q2, d2, dCONSTANTh
+	vext.8		q1, q1, qzr, #8
+	veor.8		q1, q1, q2
+
+	/* final 32-bit fold */
+	vldr		dCONSTANTl, .Lconstant_R5
+	vldr		d6, .Lconstant_mask32
+	vmov.i8		d7, #0
+
+	vext.8		q2, q1, qzr, #4
+	vand.8		d2, d2, d6
+	vmull.p64	q1, d2, dCONSTANTl
+	veor.8		q1, q1, q2
+
+	/* Finish up with the bit-reversed barrett reduction 64 ==> 32 bits */
+	vldr		dCONSTANTl, .Lconstant_RUpoly
+	vldr		dCONSTANTh, .Lconstant_RUpoly + 8
+
+	vand.8		q2, q1, q3
+	vext.8		q2, qzr, q2, #8
+	vmull.p64	q2, d5, dCONSTANTh
+	vand.8		q2, q2, q3
+	vmull.p64	q2, d4, dCONSTANTl
+	veor.8		q1, q1, q2
+	vmov		r0, s5
+
+	bx		lr
+ENDPROC(crc32_pmull_le)
+
+ENTRY(crc32_armv8_le)
+	mov		ip, r2
+0:	subs		ip, ip, #8
+	bmi		4f
+	ldrd		r2, r3, [r1], #8
+ARM_BE8(rev		r2, r2		)
+ARM_BE8(rev		r3, r3		)
+	crc32w		r0, r0, r2
+	crc32w		r0, r0, r3
+	b		0b
+
+4:	tst		ip, #4
+	beq		2f
+	ldr		r3, [r1], #4
+ARM_BE8(rev		r3, r3		)
+	crc32w		r0, r0, r3
+2:	tst		ip, #2
+	beq		1f
+	ldrh		r3, [r1], #2
+ARM_BE8(rev16		r3, r3		)
+	crc32h		r0, r0, r3
+1:	tst		ip, #1
+	bxeq		lr
+	ldrb		r3, [r1]
+	crc32b		r0, r0, r3
+	bx		lr
+ENDPROC(crc32_armv8_le)
diff --git a/arch/arm/crypto/crc32-ce-glue.c b/arch/arm/crypto/crc32-ce-glue.c
new file mode 100644
index 000000000000..61916b6b40e9
--- /dev/null
+++ b/arch/arm/crypto/crc32-ce-glue.c
@@ -0,0 +1,129 @@
+/*
+ * Accelerated CRC32 using ARM CRC, NEON and Crypto Extensions instructions
+ *
+ * Copyright (C) 2016 Linaro Ltd <ard.biesheuvel@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/crc32.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/string.h>
+
+#include <crypto/internal/hash.h>
+
+#include <asm/hwcap.h>
+#include <asm/neon.h>
+#include <asm/simd.h>
+#include <asm/unaligned.h>
+
+#define PMULL_MIN_LEN		64L	/* minimum size of buffer
+					 * for crc32_pmull_le_16 */
+#define SCALE_F			16L	/* size of NEON register */
+
+asmlinkage u32 crc32_pmull_le(const u8 buf[], u32 len, u32 init_crc);
+asmlinkage u32 crc32_armv8_le(u32 init_crc, const u8 buf[], u32 len);
+
+static int crc32_pmull_cra_init(struct crypto_tfm *tfm)
+{
+	u32 *key = crypto_tfm_ctx(tfm);
+
+	*key = 0;
+	return 0;
+}
+
+static int crc32_pmull_setkey(struct crypto_shash *hash, const u8 *key,
+			      unsigned int keylen)
+{
+	u32 *mctx = crypto_shash_ctx(hash);
+
+	if (keylen != sizeof(u32)) {
+		crypto_shash_set_flags(hash, CRYPTO_TFM_RES_BAD_KEY_LEN);
+		return -EINVAL;
+	}
+	*mctx = le32_to_cpup((__le32 *)key);
+	return 0;
+}
+
+static int crc32_pmull_init(struct shash_desc *desc)
+{
+	u32 *mctx = crypto_shash_ctx(desc->tfm);
+	u32 *crc = shash_desc_ctx(desc);
+
+	*crc = *mctx;
+	return 0;
+}
+
+static int crc32_pmull_update(struct shash_desc *desc, const u8 *data,
+			 unsigned int length)
+{
+	u32 *crc = shash_desc_ctx(desc);
+
+	if (length >= PMULL_MIN_LEN && may_use_simd() &&
+	    (elf_hwcap2 & HWCAP2_PMULL)) {
+		kernel_neon_begin();
+		*crc = crc32_pmull_le(data, round_down(length, SCALE_F), *crc);
+		kernel_neon_end();
+
+		data += round_down(length, SCALE_F);
+		length %= SCALE_F;
+	}
+
+	if (length > 0) {
+		if (elf_hwcap2 & HWCAP2_CRC32)
+			*crc = crc32_armv8_le(*crc, data, length);
+		else
+			*crc = crc32_le(*crc, data, length);
+	}
+
+	return 0;
+}
+
+static int crc32_pmull_final(struct shash_desc *desc, u8 *out)
+{
+	u32 *crc = shash_desc_ctx(desc);
+
+	put_unaligned_le32(*crc, out);
+	return 0;
+}
+
+static struct shash_alg crc32_pmull_alg = {
+	.setkey			= crc32_pmull_setkey,
+	.init			= crc32_pmull_init,
+	.update			= crc32_pmull_update,
+	.final			= crc32_pmull_final,
+	.descsize		= sizeof(u32),
+	.digestsize		= sizeof(u32),
+
+	.base.cra_ctxsize	= sizeof(u32),
+	.base.cra_init		= crc32_pmull_cra_init,
+	.base.cra_name		= "crc32",
+	.base.cra_driver_name	= "crc32-arm-ce",
+	.base.cra_priority	= 200,
+	.base.cra_blocksize	= 1,
+	.base.cra_module	= THIS_MODULE,
+};
+
+static int __init crc32_pmull_mod_init(void)
+{
+	if (!(elf_hwcap2 & (HWCAP2_PMULL|HWCAP2_CRC32)))
+		return -ENODEV;
+
+	return crypto_register_shash(&crc32_pmull_alg);
+}
+
+static void __exit crc32_pmull_mod_exit(void)
+{
+	crypto_unregister_shash(&crc32_pmull_alg);
+}
+
+module_init(crc32_pmull_mod_init);
+module_exit(crc32_pmull_mod_exit);
+
+MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS_CRYPTO("crc32");
-- 
2.7.4

^ permalink raw reply related

* [PATCH 1/2] crypto: arm64/crc32 - accelerated support based on x86 SSE implementation
From: Ard Biesheuvel @ 2016-11-26 20:15 UTC (permalink / raw)
  To: linux-crypto, herbert, linux-arm-kernel, catalin.marinas,
	will.deacon, linux
  Cc: steve.capper, Ard Biesheuvel
In-Reply-To: <1480191314-2331-1-git-send-email-ard.biesheuvel@linaro.org>

This is a combination of the the Intel algorithm implemented using SSE
and PCLMULQDQ instructions from arch/x86/crypto/crc32-pclmul_asm.S, and
the new CRC32 extensions introduced for both 32-bit and 64-bit ARM in
version 8 of the architecture.

The PMULL/NEON algorithm is faster, but operates on blocks of at least
64 bytes, and on multiples of 16 bytes only. For the remaining input,
or for all input on systems that lack the PMULL 64x64->128 instructions,
the CRC32 instructions will be used.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm64/crypto/Kconfig         |   6 +
 arch/arm64/crypto/Makefile        |   3 +
 arch/arm64/crypto/crc32-ce-core.S | 246 ++++++++++++++++++++
 arch/arm64/crypto/crc32-ce-glue.c | 124 ++++++++++
 4 files changed, 379 insertions(+)

diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
index 1b50671ffec3..11dc2ac1f2e5 100644
--- a/arch/arm64/crypto/Kconfig
+++ b/arch/arm64/crypto/Kconfig
@@ -58,4 +58,10 @@ config CRYPTO_CRC32_ARM64
 	tristate "CRC32 and CRC32C using optional ARMv8 instructions"
 	depends on ARM64
 	select CRYPTO_HASH
+
+config CRYPTO_CRC32_ARM64_CE
+	tristate "CRC32 digest algorithm using PMULL instructions"
+	depends on ARM64 && KERNEL_MODE_NEON
+	select CRYPTO_HASH
+
 endif
diff --git a/arch/arm64/crypto/Makefile b/arch/arm64/crypto/Makefile
index 36fd3eb4201b..144387805a46 100644
--- a/arch/arm64/crypto/Makefile
+++ b/arch/arm64/crypto/Makefile
@@ -20,6 +20,9 @@ ghash-ce-y := ghash-ce-glue.o ghash-ce-core.o
 obj-$(CONFIG_CRYPTO_CRCT10DIF_ARM64_CE) += crct10dif-ce.o
 crct10dif-ce-y := crct10dif-ce-core.o crct10dif-ce-glue.o
 
+obj-$(CONFIG_CRYPTO_CRC32_ARM64_CE) += crc32-ce.o
+crc32-ce-y:= crc32-ce-core.o crc32-ce-glue.o
+
 obj-$(CONFIG_CRYPTO_AES_ARM64_CE) += aes-ce-cipher.o
 CFLAGS_aes-ce-cipher.o += -march=armv8-a+crypto
 
diff --git a/arch/arm64/crypto/crc32-ce-core.S b/arch/arm64/crypto/crc32-ce-core.S
new file mode 100644
index 000000000000..eff7fe100dab
--- /dev/null
+++ b/arch/arm64/crypto/crc32-ce-core.S
@@ -0,0 +1,246 @@
+/*
+ * Accelerated CRC32 using arm64 CRC, NEON and Crypto Extensions instructions
+ *
+ * Copyright (C) 2016 Linaro Ltd <ard.biesheuvel@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+/* GPL HEADER START
+ *
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 only,
+ * as published by the Free Software Foundation.
+ *
+ * This program 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
+ * General Public License version 2 for more details (a copy is included
+ * in the LICENSE file that accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License
+ * version 2 along with this program; If not, see http://www.gnu.org/licenses
+ *
+ * Please  visit http://www.xyratex.com/contact if you need additional
+ * information or have any questions.
+ *
+ * GPL HEADER END
+ */
+
+/*
+ * Copyright 2012 Xyratex Technology Limited
+ *
+ * Using hardware provided PCLMULQDQ instruction to accelerate the CRC32
+ * calculation.
+ * CRC32 polynomial:0x04c11db7(BE)/0xEDB88320(LE)
+ * PCLMULQDQ is a new instruction in Intel SSE4.2, the reference can be found
+ * at:
+ * http://www.intel.com/products/processor/manuals/
+ * Intel(R) 64 and IA-32 Architectures Software Developer's Manual
+ * Volume 2B: Instruction Set Reference, N-Z
+ *
+ * Authors:   Gregory Prestas <Gregory_Prestas@us.xyratex.com>
+ *	      Alexander Boyko <Alexander_Boyko@xyratex.com>
+ */
+
+#include <linux/linkage.h>
+#include <asm/assembler.h>
+
+
+	.text
+	.align		4
+	.cpu		generic+crypto+crc
+
+	/*
+	 * [x4*128+32 mod P(x) << 32)]'  << 1   = 0x154442bd4
+	 * #define CONSTANT_R1  0x154442bd4LL
+	 *
+	 * [(x4*128-32 mod P(x) << 32)]' << 1   = 0x1c6e41596
+	 * #define CONSTANT_R2  0x1c6e41596LL
+	 */
+.Lconstant_R2R1:
+	.octa		0x00000001c6e415960000000154442bd4
+
+	/*
+	 * [(x128+32 mod P(x) << 32)]'   << 1   = 0x1751997d0
+	 * #define CONSTANT_R3  0x1751997d0LL
+	 *
+	 * [(x128-32 mod P(x) << 32)]'   << 1   = 0x0ccaa009e
+	 * #define CONSTANT_R4  0x0ccaa009eLL
+	 */
+.Lconstant_R4R3:
+	.octa		0x00000000ccaa009e00000001751997d0
+
+	/*
+	 * [(x64 mod P(x) << 32)]'       << 1   = 0x163cd6124
+	 * #define CONSTANT_R5  0x163cd6124LL
+	 */
+.Lconstant_R5:
+	.octa		0x00000000000000000000000163cd6124
+.Lconstant_mask32:
+	.octa		0x000000000000000000000000FFFFFFFF
+
+	/*
+	 * #define CRCPOLY_TRUE_LE_FULL 0x1DB710641LL
+	 *
+	 * Barrett Reduction constant (u64`) = u` = (x**64 / P(x))`
+	 *                                                      = 0x1F7011641LL
+	 * #define CONSTANT_RU  0x1F7011641LL
+	 */
+.Lconstant_RUpoly:
+	.octa		0x00000001F701164100000001DB710641
+
+	vCONSTANT	.req	v0
+	dCONSTANT	.req	d0
+	qCONSTANT	.req	q0
+
+	BUF		.req	x0
+	LEN		.req	x1
+	CRC		.req	x2
+
+	vzr		.req	v9
+
+	/**
+	 * Calculate crc32
+	 * BUF - buffer
+	 * LEN - sizeof buffer (multiple of 16 bytes), LEN should be > 63
+	 * CRC - initial crc32
+	 * return %eax crc32
+	 * uint crc32_pmull_le(unsigned char const *buffer,
+	 *                     size_t len, uint crc32)
+	 */
+ENTRY(crc32_pmull_le)
+	bic		LEN, LEN, #15
+	ld1		{v1.16b-v4.16b}, [BUF], #0x40
+	movi		vzr.16b, #0
+	fmov		dCONSTANT, CRC
+	eor		v1.16b, v1.16b, vCONSTANT.16b
+	sub		LEN, LEN, #0x40
+	cmp		LEN, #0x40
+	b.lt		less_64
+
+	ldr		qCONSTANT, .Lconstant_R2R1
+
+loop_64:		/* 64 bytes Full cache line folding */
+	sub		LEN, LEN, #0x40
+
+	pmull2		v5.1q, v1.2d, vCONSTANT.2d
+	pmull2		v6.1q, v2.2d, vCONSTANT.2d
+	pmull2		v7.1q, v3.2d, vCONSTANT.2d
+	pmull2		v8.1q, v4.2d, vCONSTANT.2d
+
+	pmull		v1.1q, v1.1d, vCONSTANT.1d
+	pmull		v2.1q, v2.1d, vCONSTANT.1d
+	pmull		v3.1q, v3.1d, vCONSTANT.1d
+	pmull		v4.1q, v4.1d, vCONSTANT.1d
+
+	eor		v1.16b, v1.16b, v5.16b
+	ld1		{v5.16b}, [BUF], #0x10
+	eor		v2.16b, v2.16b, v6.16b
+	ld1		{v6.16b}, [BUF], #0x10
+	eor		v3.16b, v3.16b, v7.16b
+	ld1		{v7.16b}, [BUF], #0x10
+	eor		v4.16b, v4.16b, v8.16b
+	ld1		{v8.16b}, [BUF], #0x10
+
+	eor		v1.16b, v1.16b, v5.16b
+	eor		v2.16b, v2.16b, v6.16b
+	eor		v3.16b, v3.16b, v7.16b
+	eor		v4.16b, v4.16b, v8.16b
+
+	cmp		LEN, #0x40
+	b.ge		loop_64
+
+less_64:		/* Folding cache line into 128bit */
+	ldr		qCONSTANT, .Lconstant_R4R3
+
+	pmull2		v5.1q, v1.2d, vCONSTANT.2d
+	pmull		v1.1q, v1.1d, vCONSTANT.1d
+	eor		v1.16b, v1.16b, v5.16b
+	eor		v1.16b, v1.16b, v2.16b
+
+	pmull2		v5.1q, v1.2d, vCONSTANT.2d
+	pmull		v1.1q, v1.1d, vCONSTANT.1d
+	eor		v1.16b, v1.16b, v5.16b
+	eor		v1.16b, v1.16b, v3.16b
+
+	pmull2		v5.1q, v1.2d, vCONSTANT.2d
+	pmull		v1.1q, v1.1d, vCONSTANT.1d
+	eor		v1.16b, v1.16b, v5.16b
+	eor		v1.16b, v1.16b, v4.16b
+
+	cbz		LEN, fold_64
+
+loop_16:		/* Folding rest buffer into 128bit */
+	subs		LEN, LEN, #0x10
+
+	ld1		{v2.16b}, [BUF], #0x10
+	pmull2		v5.1q, v1.2d, vCONSTANT.2d
+	pmull		v1.1q, v1.1d, vCONSTANT.1d
+	eor		v1.16b, v1.16b, v5.16b
+	eor		v1.16b, v1.16b, v2.16b
+
+	b.ne		loop_16
+
+fold_64:
+	/* perform the last 64 bit fold, also adds 32 zeroes
+	 * to the input stream */
+	ext		v2.16b, v1.16b, v1.16b, #8
+	pmull2		v2.1q, v2.2d, vCONSTANT.2d
+	ext		v1.16b, v1.16b, vzr.16b, #8
+	eor		v1.16b, v1.16b, v2.16b
+
+	/* final 32-bit fold */
+	ldr		qCONSTANT, .Lconstant_R5
+	ldr		q3, .Lconstant_mask32
+
+	ext		v2.16b, v1.16b, vzr.16b, #4
+	and		v1.16b, v1.16b, v3.16b
+	pmull		v1.1q, v1.1d, vCONSTANT.1d
+	eor		v1.16b, v1.16b, v2.16b
+
+	/* Finish up with the bit-reversed barrett reduction 64 ==> 32 bits */
+	ldr		qCONSTANT, .Lconstant_RUpoly
+
+	and		v2.16b, v1.16b, v3.16b
+	ext		v2.16b, vzr.16b, v2.16b, #8
+	pmull2		v2.1q, v2.2d, vCONSTANT.2d
+	and		v2.16b, v2.16b, v3.16b
+	pmull		v2.1q, v2.1d, vCONSTANT.1d
+	eor		v1.16b, v1.16b, v2.16b
+	mov		w0, v1.s[1]
+
+	ret
+ENDPROC(crc32_pmull_le)
+
+ENTRY(crc32_armv8_le)
+0:	subs		x2, x2, #16
+	b.mi		8f
+	ldp		x3, x4, [x1], #16
+CPU_BE(	rev		x3, x3		)
+CPU_BE(	rev		x4, x4		)
+	crc32x		w0, w0, x3
+	crc32x		w0, w0, x4
+	b		0b
+
+8:	tbz		x2, #3, 4f
+	ldr		x3, [x1], #8
+CPU_BE(	rev		x3, x3		)
+	crc32x		w0, w0, x3
+4:	tbz		x2, #2, 2f
+	ldr		w3, [x1], #4
+CPU_BE(	rev		w3, w3		)
+	crc32w		w0, w0, w3
+2:	tbz		x2, #1, 1f
+	ldrh		w3, [x1], #2
+CPU_BE(	rev16		w3, w3		)
+	crc32h		w0, w0, w3
+1:	tbz		x2, #0, 0f
+	ldrb		w3, [x1]
+	crc32b		w0, w0, w3
+0:	ret
+ENDPROC(crc32_armv8_le)
diff --git a/arch/arm64/crypto/crc32-ce-glue.c b/arch/arm64/crypto/crc32-ce-glue.c
new file mode 100644
index 000000000000..567203f29ac6
--- /dev/null
+++ b/arch/arm64/crypto/crc32-ce-glue.c
@@ -0,0 +1,124 @@
+/*
+ * Accelerated CRC32 using arm64 NEON and Crypto Extensions instructions
+ *
+ * Copyright (C) 2016 Linaro Ltd <ard.biesheuvel@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/cpufeature.h>
+#include <linux/crc32.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/string.h>
+
+#include <crypto/internal/hash.h>
+
+#include <asm/hwcap.h>
+#include <asm/neon.h>
+#include <asm/unaligned.h>
+
+#define PMULL_MIN_LEN		64L	/* minimum size of buffer
+					 * for crc32_pmull_le_16 */
+#define SCALE_F			16L	/* size of NEON register */
+
+asmlinkage u32 crc32_pmull_le(const u8 buf[], u64 len, u32 init_crc);
+asmlinkage u32 crc32_armv8_le(u32 init_crc, const u8 buf[], u64 len);
+
+static int crc32_pmull_cra_init(struct crypto_tfm *tfm)
+{
+	u32 *key = crypto_tfm_ctx(tfm);
+
+	*key = 0;
+	return 0;
+}
+
+static int crc32_pmull_setkey(struct crypto_shash *hash, const u8 *key,
+			      unsigned int keylen)
+{
+	u32 *mctx = crypto_shash_ctx(hash);
+
+	if (keylen != sizeof(u32)) {
+		crypto_shash_set_flags(hash, CRYPTO_TFM_RES_BAD_KEY_LEN);
+		return -EINVAL;
+	}
+	*mctx = le32_to_cpup((__le32 *)key);
+	return 0;
+}
+
+static int crc32_pmull_init(struct shash_desc *desc)
+{
+	u32 *mctx = crypto_shash_ctx(desc->tfm);
+	u32 *crc = shash_desc_ctx(desc);
+
+	*crc = *mctx;
+	return 0;
+}
+
+static int crc32_pmull_update(struct shash_desc *desc, const u8 *data,
+			 unsigned int length)
+{
+	u32 *crc = shash_desc_ctx(desc);
+
+	if (length >= PMULL_MIN_LEN) {
+		kernel_neon_begin_partial(10);
+		*crc = crc32_pmull_le(data, round_down(length, SCALE_F), *crc);
+		kernel_neon_end();
+
+		data += round_down(length, SCALE_F);
+		length %= SCALE_F;
+	}
+
+	if (length > 0) {
+		if (elf_hwcap & HWCAP_CRC32)
+			*crc = crc32_armv8_le(*crc, data, length);
+		else
+			*crc = crc32_le(*crc, data, length);
+	}
+
+	return 0;
+}
+
+static int crc32_pmull_final(struct shash_desc *desc, u8 *out)
+{
+	u32 *crc = shash_desc_ctx(desc);
+
+	put_unaligned_le32(*crc, out);
+	return 0;
+}
+
+static struct shash_alg crc32_pmull_alg = {
+	.setkey			= crc32_pmull_setkey,
+	.init			= crc32_pmull_init,
+	.update			= crc32_pmull_update,
+	.final			= crc32_pmull_final,
+	.descsize		= sizeof(u32),
+	.digestsize		= sizeof(u32),
+
+	.base.cra_ctxsize	= sizeof(u32),
+	.base.cra_init		= crc32_pmull_cra_init,
+	.base.cra_name		= "crc32",
+	.base.cra_driver_name	= "crc32-arm64-ce",
+	.base.cra_priority	= 200,
+	.base.cra_blocksize	= 1,
+	.base.cra_module	= THIS_MODULE,
+};
+
+static int __init crc32_pmull_mod_init(void)
+{
+	return crypto_register_shash(&crc32_pmull_alg);
+}
+
+static void __exit crc32_pmull_mod_exit(void)
+{
+	crypto_unregister_shash(&crc32_pmull_alg);
+}
+
+module_cpu_feature_match(PMULL, crc32_pmull_mod_init);
+module_exit(crc32_pmull_mod_exit);
+
+MODULE_AUTHOR("Ard Biesheuvel <ard.biesheuvel@linaro.org>");
+MODULE_LICENSE("GPL v2");
-- 
2.7.4

^ permalink raw reply related

* [PATCH 0/2] CRC32 for ARM and arm64 using PMULL and CRC instructions
From: Ard Biesheuvel @ 2016-11-26 20:15 UTC (permalink / raw)
  To: linux-crypto, herbert, linux-arm-kernel, catalin.marinas,
	will.deacon, linux
  Cc: steve.capper, Ard Biesheuvel

Version 8 of the ARM architecture introduces both a set of dedicated CRC32
instructions, and a 64x64 to 128 bit polynomial multiplication instruction,
both of which can be used to accelerate CRC32 calculations.

These patches contains ports of the existing polynomial multiplication based
CRC32 code that resides in arch/x86/crypto/crc32-pclmul_asm.S, but since that
algorithm operates on multiples of 16 bytes only, and requires at least 64
bytes of input, the remainders are calculated with the CRC32 instructions,
if available.

These patches apply on top of the CRC-T10DIF series I sent out last Thursday.

https://git.kernel.org/cgit/linux/kernel/git/ardb/linux.git/log/?h=crc32

Ard Biesheuvel (2):
  crypto: arm64/crc32 - accelerated support based on x86 SSE
    implementation
  crypto: arm/crc32 - accelerated support based on x86 SSE
    implementation

 arch/arm/crypto/Kconfig           |   5 +
 arch/arm/crypto/Makefile          |   2 +
 arch/arm/crypto/crc32-ce-core.S   | 257 ++++++++++++++++++++
 arch/arm/crypto/crc32-ce-glue.c   | 129 ++++++++++
 arch/arm64/crypto/Kconfig         |   6 +
 arch/arm64/crypto/Makefile        |   3 +
 arch/arm64/crypto/crc32-ce-core.S | 246 +++++++++++++++++++
 arch/arm64/crypto/crc32-ce-glue.c | 124 ++++++++++
 8 files changed, 772 insertions(+)
 create mode 100644 arch/arm/crypto/crc32-ce-core.S
 create mode 100644 arch/arm/crypto/crc32-ce-glue.c
 create mode 100644 arch/arm64/crypto/crc32-ce-core.S
 create mode 100644 arch/arm64/crypto/crc32-ce-glue.c

-- 
2.7.4

^ permalink raw reply

* [PATCH] crypto: CTR DRBG - prevent invalid SG mappings
From: Stephan Mueller @ 2016-11-26  8:54 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto
In-Reply-To: <3805767.QuT5G9UC4v@positron.chronox.de>

Hi Herbert,

as discussed in another thread, SGs must not be used with stack memory 
pointers. This issue was the culprit to the error I see with the CTR DRBG. The 
attached patch fixes the issue.

---8<---

When using SGs, only heap memory (memory that is valid as per
virt_addr_valid) is allowed to be referenced. The CTR DRBG used to
reference the caller-provided memory directly in an SG. In case the
caller provided stack memory pointers, the SG mapping is not considered
to be valid. In some cases, this would even cause a paging fault.

The change adds a new scratch buffer that is used in case the
caller-provided buffer is deemed not suitable for use in an SG. The
crypto operation of the CTR DRBG produces its output with that scratch
buffer.

The scratch buffer is allocated during allocation time of the CTR DRBG
as its access is protected with the DRBG mutex.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/drbg.c         | 35 +++++++++++++++++++++++++++++++----
 include/crypto/drbg.h |  2 ++
 2 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/crypto/drbg.c b/crypto/drbg.c
index 9a95b61..cbbd19f 100644
--- a/crypto/drbg.c
+++ b/crypto/drbg.c
@@ -262,6 +262,7 @@ static int drbg_kcapi_sym_ctr(struct drbg_state *drbg,
 			      u8 *inbuf, u32 inbuflen,
 			      u8 *outbuf, u32 outlen);
 #define DRBG_CTR_NULL_LEN 128
+#define DRBG_OUTSCRATCHLEN DRBG_CTR_NULL_LEN
 
 /* BCC function for CTR DRBG as defined in 10.4.3 */
 static int drbg_ctr_bcc(struct drbg_state *drbg,
@@ -1644,6 +1645,9 @@ static int drbg_fini_sym_kernel(struct drbg_state *drbg)
 	kfree(drbg->ctr_null_value_buf);
 	drbg->ctr_null_value = NULL;
 
+	kfree(drbg->outscratchpadbuf);
+	drbg->outscratchpadbuf = NULL;
+
 	return 0;
 }
 
@@ -1708,6 +1712,15 @@ static int drbg_init_sym_kernel(struct drbg_state 
*drbg)
 	drbg->ctr_null_value = (u8 *)PTR_ALIGN(drbg->ctr_null_value_buf,
 					       alignmask + 1);
 
+	drbg->outscratchpadbuf = kmalloc(DRBG_OUTSCRATCHLEN + alignmask,
+					 GFP_KERNEL);
+	if (!drbg->outscratchpadbuf) {
+		drbg_fini_sym_kernel(drbg);
+		return -ENOMEM;
+	}
+	drbg->outscratchpad = (u8 *)PTR_ALIGN(drbg->outscratchpadbuf,
+					      alignmask + 1);
+
 	return alignmask;
 }
 
@@ -1737,15 +1750,22 @@ static int drbg_kcapi_sym_ctr(struct drbg_state *drbg,
 			      u8 *outbuf, u32 outlen)
 {
 	struct scatterlist sg_in;
+	bool virt_addr_valid = virt_addr_valid(outbuf);
+	int ret = 0;
 
 	sg_init_one(&sg_in, inbuf, inlen);
 
 	while (outlen) {
 		u32 cryptlen = min_t(u32, inlen, outlen);
 		struct scatterlist sg_out;
-		int ret;
 
-		sg_init_one(&sg_out, outbuf, cryptlen);
+		/* If output buffer is not valid for SGL, use scratchpad */
+		if (virt_addr_valid)
+			sg_init_one(&sg_out, outbuf, cryptlen);
+		else {
+			cryptlen = min_t(u32, cryptlen, DRBG_OUTSCRATCHLEN);
+			sg_init_one(&sg_out, drbg->outscratchpad, cryptlen);
+		}
 		skcipher_request_set_crypt(drbg->ctr_req, &sg_in, &sg_out,
 					   cryptlen, drbg->V);
 		ret = crypto_skcipher_encrypt(drbg->ctr_req);
@@ -1761,15 +1781,22 @@ static int drbg_kcapi_sym_ctr(struct drbg_state *drbg,
 				break;
 			}
 		default:
-			return ret;
+			goto out;
 		}
 		init_completion(&drbg->ctr_completion);
 
+		if (!virt_addr_valid)
+			memcpy(outbuf, drbg->outscratchpad, cryptlen);
+
 		outlen -= cryptlen;
 		outbuf += cryptlen;
 	}
+	ret = 0;
 
-	return 0;
+out:
+	if (!virt_addr_valid)
+		memzero_explicit(drbg->outscratchpad, DRBG_OUTSCRATCHLEN);
+	return ret;
 }
 #endif /* CONFIG_CRYPTO_DRBG_CTR */
 
diff --git a/include/crypto/drbg.h b/include/crypto/drbg.h
index 61580b1..22f884c 100644
--- a/include/crypto/drbg.h
+++ b/include/crypto/drbg.h
@@ -124,6 +124,8 @@ struct drbg_state {
 	struct skcipher_request *ctr_req;	/* CTR mode request handle */
 	__u8 *ctr_null_value_buf;		/* CTR mode unaligned buffer */
 	__u8 *ctr_null_value;			/* CTR mode aligned zero buf */
+	__u8 *outscratchpadbuf;			/* CTR mode output scratchpad */
+        __u8 *outscratchpad;			/* CTR mode aligned outbuf */
 	struct completion ctr_completion;	/* CTR mode async handler */
 	int ctr_async_err;			/* CTR mode async error */
 
-- 
2.9.3

^ permalink raw reply related

* RE: [PATCH v2 0/2] virtio-crypto: add Linux driver
From: Gonglei (Arei) @ 2016-11-26  9:38 UTC (permalink / raw)
  To: Gonglei (Arei), linux-kernel@vger.kernel.org,
	qemu-devel@nongnu.org, virtio-dev@lists.oasis-open.org,
	virtualization@lists.linux-foundation.org,
	linux-crypto@vger.kernel.org
  Cc: Luonengjun, mst@redhat.com, stefanha@redhat.com, Huangweidong (C),
	Wubin (H), xin.zeng@intel.com, Claudio Fontana,
	herbert@gondor.apana.org.au, pasic@linux.vnet.ibm.com,
	davem@davemloft.net, Zhoujian (jay, Euler), Hanweidong (Randy),
	arei.gonglei@hotmail.com, cornelia.huck@de.ibm.com,
	Xuquan (Quan Xu), longpeng,
	"salvatore.benedetto@intel.com" <salvat
In-Reply-To: <1479802223-121104-1-git-send-email-arei.gonglei@huawei.com>

Hi,

> -----Original Message-----
> From: Gonglei (Arei)
> Sent: Tuesday, November 22, 2016 4:10 PM
> To: linux-kernel@vger.kernel.org; qemu-devel@nongnu.org;
> virtio-dev@lists.oasis-open.org; virtualization@lists.linux-foundation.org;
> linux-crypto@vger.kernel.org
> Subject: [PATCH v2 0/2] virtio-crypto: add Linux driver
> 
> The virtio crypto device is a virtual cryptography device
> as well as a kind of virtual hardware accelerator for
> virtual machines. The encryption anddecryption requests
> are placed in the data queue and are ultimately handled by
> thebackend crypto accelerators. The second queue is the
> control queue used to create or destroy sessions for
> symmetric algorithms and will control some advanced features
> in the future. The virtio crypto device provides the following
> cryptoservices: CIPHER, MAC, HASH, and AEAD.
> 
> For more information about virtio-crypto device, please see:
>   http://qemu-project.org/Features/VirtioCrypto
> 
> For better reviewing:
> 
> Patch 1 introduces the little edian functions for VIRTIO_1
> devices.
> 
> Patch 2 mainly includes five files:
>  1) virtio_crypto.h is the header file for virtio-crypto device,
> which is based on the virtio-crypto specification.
>  2) virtio_crypto.c is the entry of the driver module,
> which is similar with other virtio devices, such as virtio-net,
> virtio-input etc.
>  3) virtio_crypto_mgr.c is used to manage the virtio
> crypto devices in the system. We support up to 32 virtio-crypto
> devices currently. I use a global list to store the virtio crypto
> devices which refer to Intel QAT driver. Meanwhile, the file
> includs the functions of add/del/search/start/stop for virtio
> crypto devices.
>  4) virtio_crypto_common.h is a private header file for virtio
> crypto driver, includes structure definations, and function declarations.
>  5) virtio_crypto_algs.c is the realization of algs based on Linux Crypto
> Framwork,
> which can register different crypto algorithms. Currently it's only support
> AES-CBC.
> The Crypto guys can mainly focus to this file.
> 
> Actually I have no idea the virtio-crypto driver should be gone in whose
> tree, Michael's or Herbert's?
> 
> Would you give me a feedback? Thanks a lot!
> 
Ping?

Any ideas? Thanks.

Regards,
-Gonglei

> 
> v2:
>  - stop doing DMA from the stack, CONFIG_VMAP_STACK=y [Salvatore]
>  - convert __virtio32/64 to __le32/64 in virtio_crypto.h
>  - remove VIRTIO_CRYPTO_S_STARTED based on the lastest virtio crypto spec.
>  - introduces the little edian functions for VIRTIO_1 devices in patch 1.
> 
> Gonglei (2):
>   virtio: introduce little edian functions for virtio_cread/write#
>     family
>   crypto: add virtio-crypto driver
> 
>  MAINTAINERS                                  |   8 +
>  drivers/crypto/Kconfig                       |   2 +
>  drivers/crypto/Makefile                      |   1 +
>  drivers/crypto/virtio/Kconfig                |  10 +
>  drivers/crypto/virtio/Makefile               |   5 +
>  drivers/crypto/virtio/virtio_crypto.c        | 444
> +++++++++++++++++++++++
>  drivers/crypto/virtio/virtio_crypto_algs.c   | 524
> +++++++++++++++++++++++++++
>  drivers/crypto/virtio/virtio_crypto_common.h | 124 +++++++
>  drivers/crypto/virtio/virtio_crypto_mgr.c    | 258 +++++++++++++
>  include/linux/virtio_config.h                |  45 +++
>  include/uapi/linux/Kbuild                    |   1 +
>  include/uapi/linux/virtio_crypto.h           | 435
> ++++++++++++++++++++++
>  include/uapi/linux/virtio_ids.h              |   1 +
>  13 files changed, 1858 insertions(+)
>  create mode 100644 drivers/crypto/virtio/Kconfig
>  create mode 100644 drivers/crypto/virtio/Makefile
>  create mode 100644 drivers/crypto/virtio/virtio_crypto.c
>  create mode 100644 drivers/crypto/virtio/virtio_crypto_algs.c
>  create mode 100644 drivers/crypto/virtio/virtio_crypto_common.h
>  create mode 100644 drivers/crypto/virtio/virtio_crypto_mgr.c
>  create mode 100644 include/uapi/linux/virtio_crypto.h
> 
> --
> 1.8.3.1
> 

^ permalink raw reply

* [PATCH] crypto: vmx - rebuild generated asm when target changes
From: Nicholas Piggin @ 2016-11-26  4:24 UTC (permalink / raw)
  To: Herbert Xu
  Cc: Nicholas Piggin, Leonidas S . Barbosa, Paulo Flabiano Smorigo,
	linux-crypto, Michael Ellerman, linuxppc-dev

Switching from big endian to little endian can fail to regenerate
the crypto assembly properly. Switch to using standard form of
kbuild dependency checking (i.e., use FORCE and if_changed).

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 drivers/crypto/vmx/Makefile | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/drivers/crypto/vmx/Makefile b/drivers/crypto/vmx/Makefile
index b47da00..16ab2a0 100644
--- a/drivers/crypto/vmx/Makefile
+++ b/drivers/crypto/vmx/Makefile
@@ -1,6 +1,8 @@
 obj-$(CONFIG_CRYPTO_DEV_VMX_ENCRYPT) += vmx-crypto.o
 vmx-crypto-objs := vmx.o aesp8-ppc.o ghashp8-ppc.o aes.o aes_cbc.o aes_ctr.o aes_xts.o ghash.o
 
+targets += aesp8-ppc.S ghashp8-ppc.S
+
 ifdef CONFIG_CPU_LITTLE_ENDIAN
 TARGET := linux-ppc64le
 else
@@ -11,13 +13,13 @@ TARGET := linux-ppc64
 endif
 endif
 
-quiet_cmd_perl = PERL $@
+quiet_cmd_perl = PERL    $@
       cmd_perl = $(PERL) $(<) $(TARGET) > $(@)
 
-$(src)/aesp8-ppc.S: $(src)/aesp8-ppc.pl
-	$(call cmd,perl)
+$(src)/aesp8-ppc.S: $(src)/aesp8-ppc.pl FORCE
+	$(call if_changed,perl)
   
-$(src)/ghashp8-ppc.S: $(src)/ghashp8-ppc.pl
-	$(call cmd,perl)
+$(src)/ghashp8-ppc.S: $(src)/ghashp8-ppc.pl FORCE
+	$(call if_changed,perl)
 
 .PRECIOUS: $(obj)/aesp8-ppc.S $(obj)/ghashp8-ppc.S
-- 
2.10.2

^ permalink raw reply related

* Re: bug in blkcipher_walk code
From: Stephan Mueller @ 2016-11-25 12:43 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto
In-Reply-To: <1833318.YYJALW9cvC@positron.chronox.de>

Am Freitag, 18. November 2016, 12:31:10 CET schrieb Stephan Mueller:

Hi Herbert,

> Hi Herbert,
> 
> Once in a while I seem to trigger a bug in the blkcipher_walk code which I
> cannot track down. This bug happens sporadically where I assume that it has
> something to do with the memory management in the slow path of
> blkcipher_walk.
> 
> I am using the CTR DRBG code that in turn uses the ctr-aes-aesni
> implementation. The bug only appears when I want to obtain a random number
> that is less than the CTR AES block size. In my particular case, I want 4
> bytes from the DRBG.
> 
> The bug happens in arch/x86/crypto/aesni-intel_glue.c:ctr_crypt_final() at
> the line:
> 
> 	memcpy(dst, keystream, nbytes);
> 
> The bug looks like the following:
> 
> [   12.328676] BUG: unable to handle kernel paging request at
> ffffa17ae418b988 [   12.328680] IP: [<ffffffff82060eea>]
> ctr_crypt+0x19a/0x1c0
> [   12.328681] PGD 66fed067
> [   12.328681] PUD 0
> [   12.328681]
> [   12.328683] Oops: 0002 [#1] SMP
> [   12.328692] Modules linked in: bridge(+) stp llc ebtable_nat ip6table_raw
> ip6table_security ip6table_mangle iptable_raw iptable_security
> iptable_mangle ebtable_filter ebtables ip6table_filter ip6_tables
> crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcspkr i2c_piix4
> virtio_net virtio_balloon acpi_cpufreq sch_fq_codel virtio_console
> virtio_blk virtio_pci virtio_ring serio_raw crc32c_intel virtio
> [   12.328693] CPU: 0 PID: 521 Comm: modprobe Not tainted 4.9.0-rc1+ #253
> [   12.328694] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.9.1-1.fc24 04/01/2014
> [   12.328694] task: ffffa17ab8453fc0 task.stack: ffffbdafc0744000
> [   12.328696] RIP: 0010:[<ffffffff82060eea>]  [<ffffffff82060eea>]
> ctr_crypt +0x19a/0x1c0
> [   12.328696] RSP: 0018:ffffbdafc0747a60  EFLAGS: 00010002
> [   12.328697] RAX: 0000000032e455a6 RBX: 0000000000000004 RCX:
> 0000000000000002
> [   12.328697] RDX: 0000000000000001 RSI: 0000000000000086 RDI:
> 0000000000000086
> [   12.328698] RBP: ffffbdafc0747b28 R08: ffffa17abc16e900 R09:
> 0000000000000019
> [   12.328698] R10: ffffa17a764f68b0 R11: 000000000002e918 R12:
> ffffbdafc0747b38
> [   12.328698] R13: ffffa17a764f6840 R14: ffffa17ae418b988 R15:
> ffffbdafc0747a70
> [   12.328699] FS:  00007f55f57a6700(0000) GS:ffffa17abfc00000(0000) knlGS:
> 0000000000000000
> [   12.328700] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [   12.328700] CR2: ffffa17ae418b988 CR3: 0000000079b26000 CR4:
> 00000000003406f0
> [   12.328703] Stack:
> [   12.328705]  ffffa17abc16e900 ffffa17ab845fd80 2ae7e40732e455a6
> 3a224612a8f9841d
> [   12.328706]  fffffb4e81e117c0 ffffa17ab845fd80 fffffb4e829062c0
> ffffa17ae418b988
> [   12.328707]  ffffbdafc0747ba8 ffffffff00000d80 ffffffff00000004
> ffffbdafc0747bc8
> [   12.328708] Call Trace:
> [   12.328712]  [<ffffffff823e5fd3>] __ablk_encrypt+0x43/0x50
> [   12.328714]  [<ffffffff823e6012>] ablk_encrypt+0x32/0xc0
> [   12.328716]  [<ffffffff823c4f2e>] skcipher_encrypt_ablkcipher+0x5e/0x60
> [   12.328717]  [<ffffffff823dbb80>] drbg_kcapi_sym_ctr+0xb0/0x130
> [   12.328719]  [<ffffffff823de153>] drbg_ctr_generate+0x53/0x80
> 
> 
> Now, the interesting part is the following: the original memory pointer that
> shall be processed by the DRBG is in my example ffffffffc018b988 -- this
> pointer is used until the DRBG invokes crypto_skcipher_encrypt. However,
> when I print out the buffer pointer that is used as dst in the memcpy of
> ctr_crypt_final, I see ffffa17ae418b988 -- i.e. the buffer that causes
> paging failure.
> 
> During tracing the blkcipher_walk code I see that the slow code path is used
> when the request size is smaller than the block size. That slow code path
> allocates new memory that will be used for the dst pointer in
> ctr_crypt_final.
> 
> May I ask you for checking whether the allocation and the memory pointer
> logic has an issue that would cause a paging failure?

Following up this issue, I found the location where the wrong memory pointer 
is produced -- the following call tree is used:

1. set up of SGL with proper pointer

2. skcipher_encrypt_ablkcipher with SGL

3. invocation of ctr_crypt from arch/x86/crypto/aesni-intel_glue.c

4. blkcipher_walk_virt_block

5. blkcipher_walk_first

6. blkcipher_walk_next (this code does not use the code path to allocate a 
page)

7. blkcipher_next_fast

        walk->dst.virt.addr = walk->src.virt.addr;
			-> copy src virt address into dst address pointer

		Now, the diff path is used:
        if (diff) {
                walk->flags |= BLKCIPHER_WALK_DIFF;
                blkcipher_map_dst(walk);
        }

8. blkcipher_map_dst

        walk->dst.virt.addr = scatterwalk_map(&walk->out);

		==> this pointer is wrong

The interesting point is that step 8 gets the low and high bits right, but not 
the bits in the middle:

The real data pointer for the dst buffer is ffffffffc0332988. The data pointer 
used by the crypto API is ffff96a995332988 -- as often as I see the issue, 
this similarity in the pointer values is always there.


Please note that the caller uses a static variable that shall be used as dst 
buffer.

Thanks
Stephan

^ permalink raw reply

* 40147 linux-crypto
From: service @ 2016-11-24 19:05 UTC (permalink / raw)
  To: linux-crypto

[-- Attachment #1: INFO_628278483951873_linux-crypto.zip --]
[-- Type: application/zip, Size: 4860 bytes --]

^ permalink raw reply

* Re: [PATCH 4/4] crypto: arm/crct10dif - port x86 SSE implementation to ARM
From: Ard Biesheuvel @ 2016-11-24 17:32 UTC (permalink / raw)
  To: linux-crypto@vger.kernel.org, Herbert Xu,
	linux-arm-kernel@lists.infradead.org, Catalin Marinas,
	Will Deacon, Russell King - ARM Linux
  Cc: Steve Capper, dingtinahong, yangshengkai, YueHaibing, Hanjun Guo,
	Ard Biesheuvel
In-Reply-To: <1480002201-1427-5-git-send-email-ard.biesheuvel@linaro.org>

On 24 November 2016 at 15:43, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> This is a straight transliteration of the Intel algorithm implemented
> using SSE and PCLMULQDQ instructions that resides under in the file
> arch/x86/crypto/crct10dif-pcl-asm_64.S.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  arch/arm/crypto/Kconfig                        |   5 +
>  arch/arm/crypto/Makefile                       |   2 +
>  arch/{arm64 => arm}/crypto/crct10dif-ce-core.S | 457 +++++++++++---------
>  arch/{arm64 => arm}/crypto/crct10dif-ce-glue.c |  23 +-
>  4 files changed, 277 insertions(+), 210 deletions(-)
>

This patch needs the following hunk folded in to avoid breaking the
Thumb2 build:

"""
diff --git a/arch/arm/crypto/crct10dif-ce-core.S
b/arch/arm/crypto/crct10dif-ce-core.S
index 30168b0f8581..4fdbca94dd0c 100644
--- a/arch/arm/crypto/crct10dif-ce-core.S
+++ b/arch/arm/crypto/crct10dif-ce-core.S
@@ -152,7 +152,8 @@ CPU_LE(     vrev64.8        q7, q7                  )
        // XOR the initial_crc value
        veor.8          q0, q0, q10

-       adrl            ip, rk3
+ARM(   adrl            ip, rk3         )
+THUMB( adr             ip, rk3         )
        vld1.64         {q10}, [ip]     // xmm10 has rk3 and rk4
                                        // type of pmull instruction
                                        // will determine which constant to use
"""

Updated patch(es) can be found here
https://git.kernel.org/cgit/linux/kernel/git/ardb/linux.git/log/?h=arm-crct10dif

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox