* Re: [PATCH v5 1/1] crypto: add virtio-crypto driver
From: Herbert Xu @ 2016-12-07 9:48 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, mst@redhat.com,
stefanha@redhat.com, Huangweidong (C), Wubin (H),
xin.zeng@intel.com, Claudio Fontana, pasic@linux.vnet.ibm.com,
davem@davemloft.net, Zhoujian (jay, Euler)
In-Reply-To: <33183CC9F5247A488A2544077AF19020DA1546F2@DGGEMA505-MBX.china.huawei.com>
On Tue, Dec 06, 2016 at 09:01:49AM +0000, Gonglei (Arei) wrote:
>
> Would you please review and/or ack the virtio_crypto_algs.c?
> It is the realization of specified algs based on Linux Crypto Framework.
I have no issues with it. If the virtio folks are happy with
the interface with the host then I'll take this patch.
Thanks,
--
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: AF_ALG - fix AEAD tag memory handling
From: Herbert Xu @ 2016-12-07 11:42 UTC (permalink / raw)
To: Stephan Mueller; +Cc: linux-crypto
In-Reply-To: <2319539.qMqmu7oTgO@positron.chronox.de>
On Mon, Dec 05, 2016 at 03:26:19PM +0100, Stephan Mueller wrote:
> Hi Herbert,
>
> Changes v4: restore the old behavior -- if the caller does not provide sufficient
> output buffer size, return an error.
>
> ---8<---
>
> For encryption, the AEAD ciphers require AAD || PT as input and generate
> AAD || CT || Tag as output and vice versa for decryption. Prior to this
> patch, the AF_ALG interface for AEAD ciphers requires the buffer to be
> present as input for encryption. Similarly, the output buffer for
> decryption required the presence of the tag buffer too. This implies
> that the kernel reads / writes data buffers from/to kernel space
> even though this operation is not required.
>
> This patch changes the AF_ALG AEAD interface to be consistent with the
> in-kernel AEAD cipher requirements.
>
> Due to this handling, he changes are transparent to user space with one
> exception: the return code of recv indicates the mount of output buffer.
> That output buffer has a different size compared to before the patch
> which implies that the return code of recv will also be different.
> For example, a decryption operation uses 16 bytes AAD, 16 bytes CT and
> 16 bytes tag, the AF_ALG AEAD interface before showed a recv return
> code of 48 (bytes) whereas after this patch, the return code is 32
> since the tag is not returned any more.
>
> Reported-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
Hmm, I don't see the code that copies the AAD over. Did I miss it?
Thanks,
--
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: AF_ALG - fix AEAD tag memory handling
From: Stephan Müller @ 2016-12-07 11:49 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20161207114245.GA20486@gondor.apana.org.au>
Am Mittwoch, 7. Dezember 2016, 19:42:45 CET schrieb Herbert Xu:
Hi Herbert,
>
> Hmm, I don't see the code that copies the AAD over. Did I miss it?
You do not miss it. As I mentioned in a previous email, I would like:
- to include the current patch around the tag now as it has been on the
mailing list for a long time -- that current patch fixes the superfluous
copying of the tag value. That fix of the tag handling is unrelated to any AAD
logic. However, it changes the user-visible memory structure and therefore the
user interface (i.e. during decryption, the returned data is smaller than it
used to be and during encryption, less data is required as input).
- the AAD handling with the copying over of data shall come in the next
development cycle for 4.10. That change is not user visible as it does not
change the memory structure and thus the user interface. Also, the AAD change
is unrelated to the tag change provided with this patch.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v2] crypto/mcryptd: Check mcryptd algorithm compatibility
From: Herbert Xu @ 2016-12-07 11:49 UTC (permalink / raw)
To: Mikulas Patocka
Cc: Tim Chen, David S. Miller, megha.dey, linux-crypto, dm-devel,
Milan Broz, Eric Biggers, stable
In-Reply-To: <alpine.LRH.2.02.1612061604480.942@file01.intranet.prod.int.rdu2.redhat.com>
On Tue, Dec 06, 2016 at 04:07:46PM -0500, Mikulas Patocka wrote:
> I think a proper fix would be to find the reason why mcryptd crashes and
> fix that reason (i.e. make it fail in mcryptd_create_hash rather than
> crash).
>
> Testing flags could fix the bug for now but it could backfire later when
> more algorithms are added.
I agree that a better solution is needed for the next merge window.
The problem here is that mcryptd is only meant to be used on specific
algorithms, so it shouldn't have been a template in the first place.
It should instead switch over to the simd model of creating new
algorithms. So I'd like to see patch for the next merge window
that converted mcryptd and its users to something similar to what
we do in crypto/simd.c.
Thanks,
--
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: AF_ALG - fix AEAD tag memory handling
From: Herbert Xu @ 2016-12-07 11:50 UTC (permalink / raw)
To: Stephan Müller; +Cc: linux-crypto
In-Reply-To: <1650428.hiZ6moN7tX@tauon.atsec.com>
On Wed, Dec 07, 2016 at 12:49:25PM +0100, Stephan Müller wrote:
> Am Mittwoch, 7. Dezember 2016, 19:42:45 CET schrieb Herbert Xu:
>
> Hi Herbert,
> >
> > Hmm, I don't see the code that copies the AAD over. Did I miss it?
>
> You do not miss it. As I mentioned in a previous email, I would like:
>
> - to include the current patch around the tag now as it has been on the
> mailing list for a long time -- that current patch fixes the superfluous
> copying of the tag value. That fix of the tag handling is unrelated to any AAD
> logic. However, it changes the user-visible memory structure and therefore the
> user interface (i.e. during decryption, the returned data is smaller than it
> used to be and during encryption, less data is required as input).
>
> - the AAD handling with the copying over of data shall come in the next
> development cycle for 4.10. That change is not user visible as it does not
> change the memory structure and thus the user interface. Also, the AAD change
> is unrelated to the tag change provided with this patch.
OK, thanks for explaining this Stephan.
--
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] crypto: chcr - checking for IS_ERR() instead of NULL
From: Herbert Xu @ 2016-12-07 12:07 UTC (permalink / raw)
To: Dan Carpenter
Cc: David S. Miller, Harsh Jain, Jitendra Lulla, Atul Gupta,
Hariprasad Shenai, Wei Yongjun, linux-crypto, kernel-janitors
In-Reply-To: <20161201204937.GA10701@mwanda>
On Thu, Dec 01, 2016 at 11:49:37PM +0300, Dan Carpenter wrote:
> The create_hash_wr() function never returns error pointers. It returns
> NULL on error.
>
> Fixes: 358961d1cd1e ("crypto: chcr - Added new structure chcr_wr")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Patch applied. Thanks.
--
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] crypto: caam - fix pointer size for AArch64 boot loader, AArch32 kernel
From: Herbert Xu @ 2016-12-07 12:07 UTC (permalink / raw)
To: Horia Geantă
Cc: David S. Miller, linux-crypto, Dan Douglass, Alison Wang
In-Reply-To: <1480928818-8166-1-git-send-email-horia.geanta@nxp.com>
On Mon, Dec 05, 2016 at 11:06:58AM +0200, Horia Geantă wrote:
> Start with a clean slate before dealing with bit 16 (pointer size)
> of Master Configuration Register.
> This fixes the case of AArch64 boot loader + AArch32 kernel, when
> the boot loader might set MCFGR[PS] and kernel would fail to clear it.
>
> Cc: <stable@vger.kernel.org>
> Reported-by: Alison Wang <alison.wang@nxp.com>
> Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
Patch applied. Thanks.
--
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: AF_ALG - fix AEAD tag memory handling
From: Herbert Xu @ 2016-12-07 12:09 UTC (permalink / raw)
To: Stephan Mueller; +Cc: linux-crypto
In-Reply-To: <2319539.qMqmu7oTgO@positron.chronox.de>
On Mon, Dec 05, 2016 at 03:26:19PM +0100, Stephan Mueller wrote:
> Hi Herbert,
>
> Changes v4: restore the old behavior -- if the caller does not provide sufficient
> output buffer size, return an error.
Patch applied. Thanks.
--
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 v2] crypto: sun4i-ss: support the Security System PRNG
From: Herbert Xu @ 2016-12-07 12:09 UTC (permalink / raw)
To: Corentin Labbe
Cc: davem, maxime.ripard, wens, linux-kernel, linux-crypto,
linux-arm-kernel
In-Reply-To: <20161205125738.GA13525@Red>
On Mon, Dec 05, 2016 at 01:57:38PM +0100, Corentin Labbe wrote:
>
> So how to expose PRNG to user space ? or more generally how to "use" a PRNG ?
We do have the algif_rng interface.
> I found hisi-rng, crypto4xx_ and exynos-rng which seems to be PRNG used as hwrng.
Thanks for checking. Patches to remove these are welcome.
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 v3 0/6] crypto: ARM/arm64 CRC-T10DIF/CRC32/CRC32C roundup
From: Herbert Xu @ 2016-12-07 12:09 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-crypto, linux-arm-kernel
In-Reply-To: <1480963348-24203-1-git-send-email-ard.biesheuvel@linaro.org>
On Mon, Dec 05, 2016 at 06:42:22PM +0000, Ard Biesheuvel wrote:
> This v3 combines the CRC-T10DIF and CRC32 implementations for both ARM and
> arm64 that I sent out a couple of weeks ago, and adds support to the latter
> for CRC32C.
All applied. Thanks.
--
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 v2] crypto/mcryptd: Check mcryptd algorithm compatibility
From: Herbert Xu @ 2016-12-07 12:09 UTC (permalink / raw)
To: Tim Chen
Cc: Mikulas Patocka, David S. Miller, megha.dey, linux-crypto,
dm-devel, Milan Broz, Eric Biggers, stable
In-Reply-To: <6825659d1f6a96d4ab2327aa0dcf357f6252e915.1480967128.git.tim.c.chen@linux.intel.com>
On Mon, Dec 05, 2016 at 11:46:31AM -0800, Tim Chen wrote:
> Algorithms not compatible with mcryptd could be spawned by mcryptd
> with a direct crypto_alloc_tfm invocation using a "mcryptd(alg)" name
> construct. This causes mcryptd to crash the kernel if an arbitrary
> "alg" is incompatible and not intended to be used with mcryptd. It is
> an issue if AF_ALG tries to spawn mcryptd(alg) to expose it externally.
> But such algorithms must be used internally and not be exposed.
>
> We added a check to enforce that only internal algorithms are allowed
> with mcryptd at the time mcryptd is spawning an algorithm.
>
> Link: http://marc.info/?l=linux-crypto-vger&m=148063683310477&w=2
> Cc: stable@vger.kernel.org
> Reported-by: Mikulas Patocka <mpatocka@redhat.com>
> Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
Patch applied. Thanks.
--
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: AF_ALG - fix AEAD tag memory handling
From: Stephan Müller @ 2016-12-07 12:22 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20161207120914.GD20680@gondor.apana.org.au>
Am Mittwoch, 7. Dezember 2016, 20:09:14 CET schrieb Herbert Xu:
Hi Herbert,
> On Mon, Dec 05, 2016 at 03:26:19PM +0100, Stephan Mueller wrote:
> > Hi Herbert,
> >
> > Changes v4: restore the old behavior -- if the caller does not provide
> > sufficient output buffer size, return an error.
>
> Patch applied. Thanks.
Thank you. I will prepare the release of libkcapi 0.13.0 now which contains a
provision for this user interface change. This provision allows a calling
application to execute on older or newer kernels without changing its logic.
The library transparently takes care of that user space interface difference.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v2] crypto: AF_ALG - fix AEAD AIO handling of zero buffer
From: Stephan Müller @ 2016-12-07 12:29 UTC (permalink / raw)
To: Herbert Xu; +Cc: linux-crypto
In-Reply-To: <20161201132207.GF2249@gondor.apana.org.au>
Am Donnerstag, 1. Dezember 2016, 21:22:07 CET schrieb Herbert Xu:
Hi Herbert,
> On Thu, Dec 01, 2016 at 08:22:37AM +0100, Stephan Mueller wrote:
> > Hi Herbert,
> >
> > I split out the bug fix patch from the AD/tag formatting patch as they
> > most likely will come after the next merge window.
> >
> > ---8<---
> >
> > Handle the case when the caller provided a zero buffer to
> > sendmsg/sendpage. Such scenario is legal for AEAD ciphers when no
> > plaintext / ciphertext and no AAD is provided and the caller only
> > requests the generation of the tag value.
> >
> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
>
> Patch applied. Thanks.
May I suggest to forward that patch to stable as this patch fixes a kernel
crasher that can be triggered by an unprivileged user?
The bug was introduced in 4.7 with the AEAD AIO addition.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH v2] crypto: sun4i-ss: support the Security System PRNG
From: Corentin Labbe @ 2016-12-07 12:51 UTC (permalink / raw)
To: Herbert Xu
Cc: linux-kernel, wens, linux-crypto, maxime.ripard, davem,
linux-arm-kernel
In-Reply-To: <20161207120900.GC20680@gondor.apana.org.au>
On Wed, Dec 07, 2016 at 08:09:00PM +0800, Herbert Xu wrote:
> On Mon, Dec 05, 2016 at 01:57:38PM +0100, Corentin Labbe wrote:
> >
> > So how to expose PRNG to user space ? or more generally how to "use" a PRNG ?
>
> We do have the algif_rng interface.
>
So I must expose it as a crypto_rng ?
Could you explain why PRNG must not be used as hw_random ?
Regards
Corentin Labbe
^ permalink raw reply
* Re: [PATCH 10/10] virtio: enable endian checks for sparse builds
From: Michael S. Tsirkin @ 2016-12-07 13:22 UTC (permalink / raw)
To: Johannes Berg
Cc: kvm, Neil Armstrong, David Airlie, linux-remoteproc, dri-devel,
virtualization, linux-s390, James E.J. Bottomley, Herbert Xu,
linux-scsi, v9fs-developer, Asias He, Arnd Bergmann, linux-kbuild,
Jens Axboe, Michal Marek, Stefan Hajnoczi, Matt Mackall,
Greg Kroah-Hartman, linux-kernel, linux-crypto, netdev,
David S. Miller
In-Reply-To: <1481091951.4092.12.camel@sipsolutions.net>
On Wed, Dec 07, 2016 at 07:25:51AM +0100, Johannes Berg wrote:
> On Tue, 2016-12-06 at 17:41 +0200, Michael S. Tsirkin wrote:
>
> > It seems that there should be a better way to do it,
> > but this works too.
>
> In some cases there might be:
>
> > --- a/drivers/s390/virtio/Makefile
> > +++ b/drivers/s390/virtio/Makefile
> > @@ -6,6 +6,8 @@
> > # it under the terms of the GNU General Public License (version 2
> > only)
> > # as published by the Free Software Foundation.
> >
> > +CFLAGS_virtio_ccw.o += -D__CHECK_ENDIAN__
> > +CFLAGS_kvm_virtio.o += -D__CHECK_ENDIAN__
> > s390-virtio-objs := virtio_ccw.o
> > ifdef CONFIG_S390_GUEST_OLD_TRANSPORT
> > s390-virtio-objs += kvm_virtio.o
>
> Here you could use
>
> ccflags-y += -D__CHECK_ENDIAN__
>
> for example, or even
>
> subdir-ccflags-y += -D__CHECK_ENDIAN__
>
> (in case any subdirs ever get added here)
Oh right. I forgot this directory only has virtio stuff.
> > --- a/drivers/vhost/Makefile
> > +++ b/drivers/vhost/Makefile
> > @@ -1,3 +1,4 @@
> > +ccflags-y := -D__CHECK_ENDIAN__
>
> Looks like you did that here and in some other places though - so
> perhaps the s390 one was intentionally different?
>
> > --- a/net/packet/Makefile
> > +++ b/net/packet/Makefile
> > @@ -2,6 +2,7 @@
> > # Makefile for the packet AF.
> > #
> >
> > +ccflags-y := -D__CHECK_ENDIAN__
>
> Technically this is slightly more than advertised, but I guess that
> still makes sense if it's clean now.
>
> johannes
^ permalink raw reply
* Re: [PATCH 10/10] virtio: enable endian checks for sparse builds
From: Stefan Hajnoczi @ 2016-12-07 14:09 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: kvm, Neil Armstrong, David Airlie, linux-remoteproc, dri-devel,
virtualization, linux-s390, James E.J. Bottomley, Herbert Xu,
linux-scsi, v9fs-developer, Asias He, Arnd Bergmann, linux-kbuild,
Jens Axboe, Michal Marek, Matt Mackall, Greg Kroah-Hartman,
linux-kernel, linux-crypto, netdev, David S. Miller
In-Reply-To: <1481038106-24899-11-git-send-email-mst@redhat.com>
[-- Attachment #1.1: Type: text/plain, Size: 1143 bytes --]
On Tue, Dec 06, 2016 at 05:41:05PM +0200, Michael S. Tsirkin wrote:
> __CHECK_ENDIAN__ isn't on by default presumably because
> it triggers too many sparse warnings for correct code.
> But virtio is now clean of these warnings, and
> we want to keep it this way - enable this for
> sparse builds.
>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>
> It seems that there should be a better way to do it,
> but this works too.
>
> drivers/block/Makefile | 1 +
> drivers/char/Makefile | 1 +
> drivers/char/hw_random/Makefile | 2 ++
> drivers/gpu/drm/virtio/Makefile | 1 +
> drivers/net/Makefile | 3 +++
> drivers/net/caif/Makefile | 1 +
> drivers/rpmsg/Makefile | 1 +
> drivers/s390/virtio/Makefile | 2 ++
> drivers/scsi/Makefile | 1 +
> drivers/vhost/Makefile | 1 +
> drivers/virtio/Makefile | 3 +++
> net/9p/Makefile | 1 +
> net/packet/Makefile | 1 +
> net/vmw_vsock/Makefile | 2 ++
> 14 files changed, 21 insertions(+)
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
[-- Attachment #2: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH 1/3] crypto: brcm: DT documentation for Broadcom SPU driver
From: Rob (William) Rice @ 2016-12-07 15:46 UTC (permalink / raw)
To: Mark Rutland
Cc: Herbert Xu, David S. Miller, Rob Herring, linux-crypto,
devicetree, linux-kernel, Ray Jui, Scott Branden, Jon Mason,
bcm-kernel-feedback-list, Catalin Marinas, Will Deacon,
linux-arm-kernel, Steve Lin
In-Reply-To: <20161206140607.GB24177@leverpostej>
Mark,
Thanks for your comments. Replies below.
Rob
On 12/6/2016 9:06 AM, Mark Rutland wrote:
> On Wed, Nov 30, 2016 at 03:07:31PM -0500, Rob Rice wrote:
>> Device tree documentation for Broadcom Secure Processing Unit
>> (SPU) crypto driver.
>>
>> Signed-off-by: Steve Lin <steven.lin1@broadcom.com>
>> Signed-off-by: Rob Rice <rob.rice@broadcom.com>
>> ---
>> .../devicetree/bindings/crypto/brcm,spu-crypto.txt | 25 ++++++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
>>
>> diff --git a/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
>> new file mode 100644
>> index 0000000..e5fe942
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/crypto/brcm,spu-crypto.txt
>> @@ -0,0 +1,25 @@
>> +The Broadcom Secure Processing Unit (SPU) driver supports symmetric
>> +cryptographic offload for Broadcom SoCs with SPU hardware. A SoC may have
>> +multiple SPU hardware blocks.
> Bindings shound describe *hardware*, not *drivers*. Please drop mention
> of the driver, and just decribe the hardware.
Makes sense. I'll change it.
>
>> +Required properties:
>> +- compatible : Should be "brcm,spum-crypto" for devices with SPU-M hardware
>> + (e.g., Northstar2) or "brcm,spum-nsp-crypto" for the Northstar Plus variant
>> + of the SPU-M hardware.
>> +
>> +- reg: Should contain SPU registers location and length.
>> +- mboxes: A list of mailbox channels to be used by the kernel driver. Mailbox
>> +channels correspond to DMA rings on the device.
>> +
>> +Example:
>> + spu-crypto@612d0000 {
>> + compatible = "brcm,spum-crypto";
>> + reg = <0 0x612d0000 0 0x900>, /* SPU 0 control regs */
>> + <0 0x612f0000 0 0x900>, /* SPU 1 control regs */
>> + <0 0x61310000 0 0x900>, /* SPU 2 control regs */
>> + <0 0x61330000 0 0x900>; /* SPU 3 control regs */
> The above didn't mention there were several register sets, and the
> comment beside each makes them sound like they're separate SPU
> instances, so I don't think it makes sense to group them as one node.
>
> What's going on here?
That's right. For the SoC I used as the example, there are four SPU
hardware blocks. The driver round robins crypto requests to the hardware
blocks to handle requests in parallel and increase throughput. I want
one instance of the SPU driver that registers algos once with the crypto
API and manages all the mailbox channels. Maybe I can achieve that with
separate device tree entries for each SPU block, I'm not sure. I'll look
into it.
>
>> + mboxes = <&pdc0 0>,
>> + <&pdc1 0>,
>> + <&pdc2 0>,
>> + <&pdc3 0>;
> Does each mbox correspond to one of the SPUs above? Or is there a shared
> pool?
Yes, each of these mailbox channels corresponds to a different SPU. PDC
is a DMA ring manager for DMAs to the SPUs.
>
> Thanks,
> Mark.
^ permalink raw reply
* Re: [PATCH 2/3] crypto: brcm: Add Broadcom SPU driver
From: Rob (William) Rice @ 2016-12-07 15:49 UTC (permalink / raw)
To: Mark Rutland
Cc: Herbert Xu, David S. Miller, Rob Herring, linux-crypto,
devicetree, linux-kernel, Ray Jui, Scott Branden, Jon Mason,
bcm-kernel-feedback-list, Catalin Marinas, Will Deacon,
linux-arm-kernel, Steve Lin
In-Reply-To: <20161206141826.GC24177@leverpostej>
Mark,
Thanks for the comments. Replies below.
Rob
On 12/6/2016 9:18 AM, Mark Rutland wrote:
> On Wed, Nov 30, 2016 at 03:07:32PM -0500, Rob Rice wrote:
>> +static const struct of_device_id bcm_spu_dt_ids[] = {
>> + {
>> + .compatible = "brcm,spum-crypto",
>> + .data = &spum_ns2_types,
>> + },
>> + {
>> + .compatible = "brcm,spum-nsp-crypto",
>> + .data = &spum_nsp_types,
>> + },
>> + {
>> + .compatible = "brcm,spu2-crypto",
>> + .data = &spu2_types,
>> + },
>> + {
>> + .compatible = "brcm,spu2-v2-crypto",
>> + .data = &spu2_v2_types,
>> + },
> These last two weren't in the binding document.
yes, I'll add them.
>
>> + { /* sentinel */ }
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, bcm_spu_dt_ids);
>> +
>> +static int spu_dt_read(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct spu_hw *spu = &iproc_priv.spu;
>> + struct device_node *dn = pdev->dev.of_node;
>> + struct resource *spu_ctrl_regs;
>> + const struct of_device_id *match;
>> + struct spu_type_subtype *matched_spu_type;
>> + void __iomem *spu_reg_vbase[MAX_SPUS];
>> + int i;
>> + int err;
>> +
>> + if (!of_device_is_available(dn)) {
>> + dev_crit(dev, "SPU device not available");
>> + return -ENODEV;
>> + }
> How can this happen?
You are correct. This is unnecessary. I will remove.
>
>> + /* Count number of mailbox channels */
>> + spu->num_chan = of_count_phandle_with_args(dn, "mboxes", "#mbox-cells");
>> + dev_dbg(dev, "Device has %d SPU channels", spu->num_chan);
>> +
>> + match = of_match_device(of_match_ptr(bcm_spu_dt_ids), dev);
>> + matched_spu_type = (struct spu_type_subtype *)match->data;
> This cast usn't necessary.
Ok, will remove.
>
>> + spu->spu_type = matched_spu_type->type;
>> + spu->spu_subtype = matched_spu_type->subtype;
>> +
>> + /* Read registers and count number of SPUs */
>> + i = 0;
>> + while ((i < MAX_SPUS) && ((spu_ctrl_regs =
>> + platform_get_resource(pdev, IORESOURCE_MEM, i)) != NULL)) {
>> + dev_dbg(dev,
>> + "SPU %d control register region res.start = %#x, res.end = %#x",
>> + i,
>> + (unsigned int)spu_ctrl_regs->start,
>> + (unsigned int)spu_ctrl_regs->end);
>> +
>> + spu_reg_vbase[i] = devm_ioremap_resource(dev, spu_ctrl_regs);
>> + if (IS_ERR(spu_reg_vbase[i])) {
>> + err = PTR_ERR(spu_reg_vbase[i]);
>> + dev_err(&pdev->dev, "Failed to map registers: %d\n",
>> + err);
>> + spu_reg_vbase[i] = NULL;
>> + return err;
>> + }
>> + i++;
>> + }
> These *really* sound like independent devices. There are no shared
> registers, and each has its own mbox.
>
> Why do we group them like this?
As I said in the previous email, I want one instance of the driver to
register crypto algos once with the crypto API and to distribute crypto
requests among all available SPU hw blocks.
>
> Thanks,
> Mark.
^ permalink raw reply
* Re: [PATCH v3 1/6] crypto: testmgr - avoid overlap in chunked tests
From: Eric Biggers @ 2016-12-07 19:19 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-crypto, herbert, linux-arm-kernel
In-Reply-To: <1480963348-24203-2-git-send-email-ard.biesheuvel@linaro.org>
On Mon, Dec 05, 2016 at 06:42:23PM +0000, Ard Biesheuvel wrote:
> The IDXn offsets are chosen such that tap values (which may go up to
> 255) end up overlapping in the xbuf allocation. In particular, IDX1
> and IDX3 are too close together, so update IDX3 to avoid this issue.
>
Hi Ard,
This patch is causing the self-tests for "xts(ecb(aes-asm))" to fail.
This is on x86. Any idea why? Here's what I see in the log:
alg: skcipher: Chunk test 1 failed on encryption at page 0 for xts(ecb(aes-asm))
00000000: 1c 3b 3a 10 2f 77 03 86 e4 83 6c 99 e3 70 cf 9b
00000010: ea 00 80 3f 5e 48 23 57 a4 ae 12 d4 14 a3 e6 3b
00000020: 5d 31 e2 76 f8 fe 4a 8d 66 b3 17 f9 ac 68 3f 44
00000030: 68 0a 86 ac 35 ad fc 33 45 be fe cb 4b b1 88 fd
00000040: 57 76 92 6c 49 a3 09 5e b1 08 fd 10 98 ba ec 70
00000050: aa a6 69 99 a7 2a 82 f2 7d 84 8b 21 d4 a7 41 b0
00000060: c5 cd 4d 5f ff 9d ac 89 ae ba 12 29 61 d0 3a 75
00000070: 71 23 e9 87 0f 8a cf 10 00 02 08 87 89 14 29 ca
00000080: 2a 3e 7a 7d 7d f7 b1 03 55 16 5c 8b 9a 6d 0a 7d
00000090: e8 b0 62 c4 50 0d c4 cd 12 0c 0f 74 18 da e3 d0
000000a0: b5 78 1c 34 80 3f a7 54 21 c7 90 df e1 de 18 34
000000b0: f2 80 d7 66 7b 32 7f 6c 8c d7 55 7e 12 ac 3a 0f
000000c0: 93 ec 05 c5 2e 04 93 ef 31 a1 2d 3d 92 60 f7 9a
000000d0: 28 9d 6a 37 9b c7 0c 50 84 14 73 d1 a8 cc 81 ec
000000e0: 58 3e 96 45 e0 7b 8d 96 70 65 5b a5 bb cf ec c6
000000f0: dc 39 66 38 0a d8 fe cb 17 b6 ba 02 46 9a 02 0a
00000100: 84 e1 8e 8f 84 25 20 70 c1 3e 9f 1f 28 9b e5 4f
00000110: bc 48 14 57 77 8f 61 60 15 e1 32 7a 02 b1 40 f1
00000120: 50 5e b3 09 32 6d 68 37 8f 83 74 59 5c 84 9d 84
00000130: f4 c3 33 ec 44 23 88 51 43 cb 47 bd 71 c5 ed ae
00000140: 9b e6 9a 2f fe ce b1 be c9 de 24 4f be 15 99 2b
00000150: 11 b7 7c 04 0f 12 bd 8f 6a 97 5a 44 a0 f9 0c 29
00000160: a9 ab c3 d4 d8 93 92 72 84 c5 87 54 cc e2 94 52
00000170: 9f 86 14 dc d2 ab a9 91 92 5f ed c4 ae 74 ff ac
00000180: 6e 33 3b 93 eb 4a ff 04 79 da 9a 41 0e 44 50 e0
00000190: dd 7a e4 c6 e2 91 09 00 57 5d a4 01 fc 07 05 9f
000001a0: 64 5e 8b 7e 9b fd ef 33 94 30 54 ff 84 01 14 93
000001b0: c2 7b 34 29 ea ed b4 ed 53 76 44 1a 77 ed 43 85
000001c0: 1a d7 7f 16 f5 41 df d2 69 d5 0d 6a 5f 14 fb 0a
000001d0: 1e 2a 8f 42 61 9e 5e c2 59 bd 96 d0 e5 cc 23 1f
000001e0: fb 84 ed 15 a8 eb 66 07 31 6b f6 ef
Eric
^ permalink raw reply
* Re: [PATCH v3 1/6] crypto: testmgr - avoid overlap in chunked tests
From: Ard Biesheuvel @ 2016-12-07 19:53 UTC (permalink / raw)
To: Eric Biggers
Cc: linux-crypto@vger.kernel.org, Herbert Xu,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <20161207191920.GA139213@google.com>
On 7 December 2016 at 19:19, Eric Biggers <ebiggers@google.com> wrote:
> On Mon, Dec 05, 2016 at 06:42:23PM +0000, Ard Biesheuvel wrote:
>> The IDXn offsets are chosen such that tap values (which may go up to
>> 255) end up overlapping in the xbuf allocation. In particular, IDX1
>> and IDX3 are too close together, so update IDX3 to avoid this issue.
>>
>
> Hi Ard,
>
> This patch is causing the self-tests for "xts(ecb(aes-asm))" to fail.
> This is on x86. Any idea why?
Does this help at all?
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 670893bcf361..59e67f5b544b 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -63,7 +63,7 @@ int alg_test(const char *driver, const char *alg,
u32 type, u32 mask)
*/
#define IDX1 32
#define IDX2 32400
-#define IDX3 511
+#define IDX3 1511
#define IDX4 8193
#define IDX5 22222
#define IDX6 17101
> Here's what I see in the log:
>
> alg: skcipher: Chunk test 1 failed on encryption at page 0 for xts(ecb(aes-asm))
> 00000000: 1c 3b 3a 10 2f 77 03 86 e4 83 6c 99 e3 70 cf 9b
> 00000010: ea 00 80 3f 5e 48 23 57 a4 ae 12 d4 14 a3 e6 3b
> 00000020: 5d 31 e2 76 f8 fe 4a 8d 66 b3 17 f9 ac 68 3f 44
> 00000030: 68 0a 86 ac 35 ad fc 33 45 be fe cb 4b b1 88 fd
> 00000040: 57 76 92 6c 49 a3 09 5e b1 08 fd 10 98 ba ec 70
> 00000050: aa a6 69 99 a7 2a 82 f2 7d 84 8b 21 d4 a7 41 b0
> 00000060: c5 cd 4d 5f ff 9d ac 89 ae ba 12 29 61 d0 3a 75
> 00000070: 71 23 e9 87 0f 8a cf 10 00 02 08 87 89 14 29 ca
> 00000080: 2a 3e 7a 7d 7d f7 b1 03 55 16 5c 8b 9a 6d 0a 7d
> 00000090: e8 b0 62 c4 50 0d c4 cd 12 0c 0f 74 18 da e3 d0
> 000000a0: b5 78 1c 34 80 3f a7 54 21 c7 90 df e1 de 18 34
> 000000b0: f2 80 d7 66 7b 32 7f 6c 8c d7 55 7e 12 ac 3a 0f
> 000000c0: 93 ec 05 c5 2e 04 93 ef 31 a1 2d 3d 92 60 f7 9a
> 000000d0: 28 9d 6a 37 9b c7 0c 50 84 14 73 d1 a8 cc 81 ec
> 000000e0: 58 3e 96 45 e0 7b 8d 96 70 65 5b a5 bb cf ec c6
> 000000f0: dc 39 66 38 0a d8 fe cb 17 b6 ba 02 46 9a 02 0a
> 00000100: 84 e1 8e 8f 84 25 20 70 c1 3e 9f 1f 28 9b e5 4f
> 00000110: bc 48 14 57 77 8f 61 60 15 e1 32 7a 02 b1 40 f1
> 00000120: 50 5e b3 09 32 6d 68 37 8f 83 74 59 5c 84 9d 84
> 00000130: f4 c3 33 ec 44 23 88 51 43 cb 47 bd 71 c5 ed ae
> 00000140: 9b e6 9a 2f fe ce b1 be c9 de 24 4f be 15 99 2b
> 00000150: 11 b7 7c 04 0f 12 bd 8f 6a 97 5a 44 a0 f9 0c 29
> 00000160: a9 ab c3 d4 d8 93 92 72 84 c5 87 54 cc e2 94 52
> 00000170: 9f 86 14 dc d2 ab a9 91 92 5f ed c4 ae 74 ff ac
> 00000180: 6e 33 3b 93 eb 4a ff 04 79 da 9a 41 0e 44 50 e0
> 00000190: dd 7a e4 c6 e2 91 09 00 57 5d a4 01 fc 07 05 9f
> 000001a0: 64 5e 8b 7e 9b fd ef 33 94 30 54 ff 84 01 14 93
> 000001b0: c2 7b 34 29 ea ed b4 ed 53 76 44 1a 77 ed 43 85
> 000001c0: 1a d7 7f 16 f5 41 df d2 69 d5 0d 6a 5f 14 fb 0a
> 000001d0: 1e 2a 8f 42 61 9e 5e c2 59 bd 96 d0 e5 cc 23 1f
> 000001e0: fb 84 ed 15 a8 eb 66 07 31 6b f6 ef
>
> Eric
^ permalink raw reply related
* Re: algif for compression?
From: abed mohammad kamaluddin @ 2016-12-07 19:57 UTC (permalink / raw)
To: herbert; +Cc: linux-crypto
Hi,
>> I see the algif_hash and algif_blkcipher implementations to allow
>> userspace AF_ALG socket access to kernel blkcipher and hash
>> algorithms, but has anyone done a algif_compression to allow userspace
>> access to compression algs? I'm asking specifically wrt the 842
>> crypto module, which uses the hardware compression coprocessors on
>> newer powerpc systems.
>>
>> If not, is there any reason against adding a algif_compression to
>> provide the access?
>
> For a start nx-842 isn't even hooked into the crypto layer so
> there is no chance of exporting it through algif as is.
>
> There is no reason why you couldn't add it of course but that'd
> be a first step.
We are also looking for user-space access to the kernel crypto layer
compression algorithms. I have gone through AF_ALG but couldn’t find
support for compression ops through it. This is required for our
hardware zip engines whose driver hooks up to the crypto layer.
I was going through the lists and stumbled across this thread. Has
there been any updates or efforts put up in this direction since this
thread is a few years old. If not, are there any alternate
implementations that allow user-space access to compression? I have
gone through cryptodev and see the same limitation there.
Would appreciate any pointers in this regard.
Thanks,
Abed
(Cavium Inc)
^ permalink raw reply
* Re: [PATCH v5 1/1] crypto: add virtio-crypto driver
From: Sam Ravnborg @ 2016-12-07 20:20 UTC (permalink / raw)
To: Gonglei (Arei)
Cc: Xuquan (Quan Xu), Huangweidong (C), mst@redhat.com,
qemu-devel@nongnu.org, Wanzongshun (Vincent),
virtualization@lists.linux-foundation.org, Zhoujian (jay, Euler),
arei.gonglei@hotmail.com, virtio-dev@lists.oasis-open.org,
kbuild test robot, Hanweidong (Randy), longpeng,
linux-crypto@vger.kernel.org, Luonengjun, stefanha@redhat.com,
herbert@gondor.apana.org.au, Claudio Fontana,
"linux-kernel@vge
In-Reply-To: <33183CC9F5247A488A2544077AF19020DA1528DE@DGGEMA505-MBX.china.huawei.com>
On Mon, Dec 05, 2016 at 03:12:52AM +0000, Gonglei (Arei) wrote:
> I don't think the root cause of those warnings are introduced by virtio-crypto driver.
>
> What's your opinion? Sam and David?
Root cause here is that arch/sparc/include/asm/topology_64.h
references cpu_data without including arch/sparc/include/asm/cpudata.h
I think other architectures pull in the dependency from
either smp.h or they have it topology.h.
The easy fix would be to include cpudata.h in arch/sparc/include/asm/topology_64.h.
And that should also be a correct fix.
Could you include this in your patch-set and build test it?
Sam
^ permalink raw reply
* Re: [PATCH v3 1/6] crypto: testmgr - avoid overlap in chunked tests
From: Eric Biggers @ 2016-12-07 20:23 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-crypto@vger.kernel.org, Herbert Xu,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CAKv+Gu_HUjCparJnZXzZR=m3obt+aypibpHrP_ivA==1QNYksg@mail.gmail.com>
On Wed, Dec 07, 2016 at 07:53:51PM +0000, Ard Biesheuvel wrote:
> Does this help at all?
>
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index 670893bcf361..59e67f5b544b 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c
> @@ -63,7 +63,7 @@ int alg_test(const char *driver, const char *alg,
> u32 type, u32 mask)
> */
> #define IDX1 32
> #define IDX2 32400
> -#define IDX3 511
> +#define IDX3 1511
> #define IDX4 8193
> #define IDX5 22222
> #define IDX6 17101
>
Yes, with that change made the self-tests pass.
Eric
^ permalink raw reply
* RE: [PATCH v5 1/1] crypto: add virtio-crypto driver
From: Gonglei (Arei) @ 2016-12-08 2:00 UTC (permalink / raw)
To: Herbert Xu
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, mst@redhat.com,
stefanha@redhat.com, Huangweidong (C), Wubin (H),
xin.zeng@intel.com, Claudio Fontana, pasic@linux.vnet.ibm.com,
davem@davemloft.net, Zhoujian (jay, Euler)
In-Reply-To: <20161207094843.GA19970@gondor.apana.org.au>
>
> Subject: Re: [PATCH v5 1/1] crypto: add virtio-crypto driver
>
> On Tue, Dec 06, 2016 at 09:01:49AM +0000, Gonglei (Arei) wrote:
> >
> > Would you please review and/or ack the virtio_crypto_algs.c?
> > It is the realization of specified algs based on Linux Crypto Framework.
>
> I have no issues with it. If the virtio folks are happy with
> the interface with the host then I'll take this patch.
>
Cool, thanks!
Regards,
-Gonglei
^ permalink raw reply
* RE: [PATCH v5 1/1] crypto: add virtio-crypto driver
From: Gonglei (Arei) @ 2016-12-08 2:10 UTC (permalink / raw)
To: Sam Ravnborg
Cc: kbuild test robot, davem@davemloft.net, kbuild-all@01.org,
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, mst@redhat.com,
stefanha@redhat.com, Huangweidong (C), Wubin (H),
xin.zeng@intel.com, Claudio Fontana,
"herbert@gondor.apana.org.
In-Reply-To: <20161207202039.GA27878@ravnborg.org>
Hi Sam,
>
> Subject: Re: [PATCH v5 1/1] crypto: add virtio-crypto driver
>
> On Mon, Dec 05, 2016 at 03:12:52AM +0000, Gonglei (Arei) wrote:
> > I don't think the root cause of those warnings are introduced by virtio-crypto
> driver.
> >
> > What's your opinion? Sam and David?
>
> Root cause here is that arch/sparc/include/asm/topology_64.h
> references cpu_data without including arch/sparc/include/asm/cpudata.h
>
> I think other architectures pull in the dependency from
> either smp.h or they have it topology.h.
>
> The easy fix would be to include cpudata.h in
> arch/sparc/include/asm/topology_64.h.
> And that should also be a correct fix.
>
> Could you include this in your patch-set and build test it?
>
Sure, I can add this one at the head of my patch set, but because I
haven't sparc environment, so it depends on the kbuild test robot
to test.
Thanks,
-Gonglei
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox