Re: [PATCH v4 4/4] ARM: dts: imx7s: add CAAM device node

[Shawn Guo]

On Thu, Feb 22, 2018 at 02:22:50PM +, Rui Miguel Silva wrote:
> Add CAAM device node to the i.MX7s device tree.
> 
> Cc: Shawn Guo 
> Cc: Sascha Hauer 
> Cc: devicet...@vger.kernel.org
> Cc: "Horia Geantă" 
> Cc: Aymen Sghaier 
> Cc: Fabio Estevam 
> Cc: Peng Fan 
> Cc: "David S. Miller" 
> Cc: Lukas Auer 
> Signed-off-by: Rui Miguel Silva 

Applied this one and "clk: imx7d: add CAAM clock", thanks.

[PATCH 3/4] crypto: bcm: Constify *hash_alg_name[]

[Hernán Gonzalez]

Note: This is compile only tested.
No gain from this except some self-documenting.

Signed-off-by: Hernán Gonzalez 
---
 drivers/crypto/bcm/spu.c | 5 +++--
 drivers/crypto/bcm/spu.h | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/bcm/spu.c b/drivers/crypto/bcm/spu.c
index efaf3cf..c7bb79e 100644
--- a/drivers/crypto/bcm/spu.c
+++ b/drivers/crypto/bcm/spu.c
@@ -23,8 +23,9 @@
 #include "cipher.h"
 
 /* This array is based on the hash algo type supported in spu.h */
-char *hash_alg_name[] = { "None", "md5", "sha1", "sha224", "sha256", "aes",
-   "sha384", "sha512", "sha3_224", "sha3_256", "sha3_384", "sha3_512" };
+char const * const hash_alg_name[] = { "None", "md5", "sha1", "sha224",
+   "sha256", "aes", "sha384", "sha512", "sha3_224", "sha3_256", "sha3_384",
+   "sha3_512" };
 
 /* Assumes SPU-M messages are in big endian */
 void spum_dump_msg_hdr(u8 *buf, unsigned int buf_len)
diff --git a/drivers/crypto/bcm/spu.h b/drivers/crypto/bcm/spu.h
index f252367..71cf6b5 100644
--- a/drivers/crypto/bcm/spu.h
+++ b/drivers/crypto/bcm/spu.h
@@ -111,7 +111,7 @@ enum aead_type {
AEAD_TYPE_LAST
 };
 
-extern char *hash_alg_name[HASH_ALG_LAST];
+extern const char * const hash_alg_name[HASH_ALG_LAST];
 
 struct spu_request_opts {
bool is_inbound;
-- 
2.7.4

[PATCH 1/4] crypto: bcm: Remove unused variable (char *tag_to_hash_idx[])

[Hernán Gonzalez]

Note: this is compile only tested.
Variable was not used anywhere in the code, remove it and save 20 bytes.

add/remove: 0/1 grow/shrink: 0/0 up/down: 0/-20 (-20)
Function old new   delta
tag_to_hash_idx   20   - -20
Total: Before=9185256, After=9185236, chg -0.00%

Signed-off-by: Hernán Gonzalez 
---
 drivers/crypto/bcm/spu.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/crypto/bcm/spu.c b/drivers/crypto/bcm/spu.c
index dbb5c03..c3d177d 100644
--- a/drivers/crypto/bcm/spu.c
+++ b/drivers/crypto/bcm/spu.c
@@ -23,8 +23,6 @@
 #include "cipher.h"
 
 /* This array is based on the hash algo type supported in spu.h */
-char *tag_to_hash_idx[] = { "none", "md5", "sha1", "sha224", "sha256" };
-
 char *hash_alg_name[] = { "None", "md5", "sha1", "sha224", "sha256", "aes",
"sha384", "sha512", "sha3_224", "sha3_256", "sha3_384", "sha3_512" };
 
-- 
2.7.4

[PATCH 2/4] crypto: bcm: Move *aead_alg_name[] from spu.c to util.c. Constify too.

[Hernán Gonzalez]

Note: This is compile only tested.
Move variable to where it is actually used. No gain from this except for
some self-documenting.

Signed-off-by: Hernán Gonzalez 
---
 drivers/crypto/bcm/spu.c  | 2 --
 drivers/crypto/bcm/spu.h  | 1 -
 drivers/crypto/bcm/util.c | 2 ++
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/bcm/spu.c b/drivers/crypto/bcm/spu.c
index c3d177d..efaf3cf 100644
--- a/drivers/crypto/bcm/spu.c
+++ b/drivers/crypto/bcm/spu.c
@@ -26,8 +26,6 @@
 char *hash_alg_name[] = { "None", "md5", "sha1", "sha224", "sha256", "aes",
"sha384", "sha512", "sha3_224", "sha3_256", "sha3_384", "sha3_512" };
 
-char *aead_alg_name[] = { "ccm(aes)", "gcm(aes)", "authenc" };
-
 /* Assumes SPU-M messages are in big endian */
 void spum_dump_msg_hdr(u8 *buf, unsigned int buf_len)
 {
diff --git a/drivers/crypto/bcm/spu.h b/drivers/crypto/bcm/spu.h
index aa6fc38..f252367 100644
--- a/drivers/crypto/bcm/spu.h
+++ b/drivers/crypto/bcm/spu.h
@@ -112,7 +112,6 @@ enum aead_type {
 };
 
 extern char *hash_alg_name[HASH_ALG_LAST];
-extern char *aead_alg_name[AEAD_TYPE_LAST];
 
 struct spu_request_opts {
bool is_inbound;
diff --git a/drivers/crypto/bcm/util.c b/drivers/crypto/bcm/util.c
index d543c01..fa6161a 100644
--- a/drivers/crypto/bcm/util.c
+++ b/drivers/crypto/bcm/util.c
@@ -23,6 +23,8 @@
 #define SPU_OFIFO_CTRL  0x40
 #define SPU_FIFO_WATERMARK  0x1FF
 
+static char const * const aead_alg_name[] = { "ccm(aes)", "gcm(aes)", 
"authenc" };
+
 /**
  * spu_sg_at_offset() - Find the scatterlist entry at a given distance from the
  * start of a scatterlist.
-- 
2.7.4

[PATCH 4/4] crypto: bcm: Constify variables in spu2.c

[Hernán Gonzalez]

Note: This is compile only tested.
Variables constified:
*spu2_cipher_type_names
*spu2_hash_mode_names
*spu2_cipher_mode_names
*spu2_hash_type_names

Constifying and declaring as static saves 160 bytes.

add/remove: 0/4 grow/shrink: 0/0 up/down: 0/-160 (-160)
Function old new   delta
spu2_cipher_type_names24   - -24
spu2_hash_mode_names  32   - -32
spu2_cipher_mode_names32   - -32
spu2_hash_type_names  72   - -72
Total: Before=9185236, After=9185076, chg -0.00%

Signed-off-by: Hernán Gonzalez 
---
 drivers/crypto/bcm/spu2.c | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/bcm/spu2.c b/drivers/crypto/bcm/spu2.c
index bf7ac62..fcf3885 100644
--- a/drivers/crypto/bcm/spu2.c
+++ b/drivers/crypto/bcm/spu2.c
@@ -49,22 +49,22 @@ enum spu2_proto_sel {
SPU2_DTLS_AEAD = 10
 };
 
-char *spu2_cipher_type_names[] = { "None", "AES128", "AES192", "AES256",
-   "DES", "3DES"
+static char * const spu2_cipher_type_names[] = { "None", "AES128", "AES192",
+   "AES256", "DES", "3DES"
 };
 
-char *spu2_cipher_mode_names[] = { "ECB", "CBC", "CTR", "CFB", "OFB", "XTS",
-   "CCM", "GCM"
+static char * const spu2_cipher_mode_names[] = { "ECB", "CBC", "CTR", "CFB",
+   "OFB", "XTS", "CCM", "GCM"
 };
 
-char *spu2_hash_type_names[] = { "None", "AES128", "AES192", "AES256",
-   "Reserved", "Reserved", "MD5", "SHA1", "SHA224", "SHA256", "SHA384",
-   "SHA512", "SHA512/224", "SHA512/256", "SHA3-224", "SHA3-256",
+static char * const spu2_hash_type_names[] = { "None", "AES128", "AES192",
+   "AES256", "Reserved", "Reserved", "MD5", "SHA1", "SHA224", "SHA256",
+   "SHA384", "SHA512", "SHA512/224", "SHA512/256", "SHA3-224", "SHA3-256",
"SHA3-384", "SHA3-512"
 };
 
-char *spu2_hash_mode_names[] = { "CMAC", "CBC-MAC", "XCBC-MAC", "HMAC",
-   "Rabin", "CCM", "GCM", "Reserved"
+static char * const spu2_hash_mode_names[] = { "CMAC", "CBC-MAC", "XCBC-MAC",
+   "HMAC", "Rabin", "CCM", "GCM", "Reserved"
 };
 
 static char *spu2_ciph_type_name(enum spu2_cipher_type cipher_type)
-- 
2.7.4

Re: Can a driver->probe be called for two devices at the same time (WAS: Re: [PATCH] crypto/ccp: don't disable interrupts while setting up debugfs)

[Gary R Hook]

On 02/27/2018 01:36 PM, Sebastian Andrzej Siewior wrote:

On 2018-02-27 19:40:34 [+0100], Greg Kroah-Hartman wrote:

On Tue, Feb 27, 2018 at 06:33:14PM +0100, Sebastian Andrzej Siewior wrote:

On 2018-02-27 11:08:56 [-0600], Gary R Hook wrote:

That issue remains unclear to me: Are probes of PCI devices guaranteed to be
serialized? Observations on my CCPs says that they occur in order, but I
don't know for certain that serialization is guaranteed.

Is there a definitive statement on this somewhere that I just don't know
about?


The bus enforces this.


So the question if a driver can probe two devices simultaneously.


Depends on the bus type.


PCI


So the question is whether or not PCI enforces serial activity within a 
domain. The CCPs are all on different buses, so that doesn't matter.


I think we don't care in this situation, given that the CCP driver has 
minor requirements for locking. I just found it an interesting (albeit 
somewhat academic) question.


Thanks,
Gary

Re: [PATCH 2/2] hwrng: mxc-rnga - add driver support on boards with device tree

[Vladimir Zapolskiy]

On 02/27/2018 09:39 PM, Kim Phillips wrote:
> On Tue, 27 Feb 2018 18:53:08 +0200
> Vladimir Zapolskiy  wrote:
> 
>> On 02/27/2018 05:49 PM, Kim Phillips wrote:
>>> On Mon, 26 Feb 2018 20:38:49 +0200
>>> Vladimir Zapolskiy  wrote:
>>>
 +#ifdef CONFIG_OF
 +static const struct of_device_id mxc_rnga_of_match[] = {
 +  { .compatible = "fsl,imx31-rnga", },
 +  { /* sentinel */ },
 +};
 +MODULE_DEVICE_TABLE(of, mxc_rnga_of_match);
 +#endif
 +
  static struct platform_driver mxc_rnga_driver = {
.driver = {
 - .name = "mxc_rnga",
 - },
 +  .name = "mxc_rnga",
 +  .of_match_table = of_match_ptr(mxc_rnga_of_match),
>>>
>>> Does this build if CONFIG_OF is not set?
>>
>> Definitely it is expected to be built, you can verify it directly or
>> check of_match_ptr() macro definition from include/linux/of.h
> 
> Thanks, I verified it by removing the SOC_IMX31 dependency, and with
> netwinder_defconfig as a base.  I also verified that the #ifdef
> CONFIG_OF protecting the mxc_rnga_of_match definition is also not
> needed.

That's a commonplace observation, but I have serious doubts, if it
has become a common practice to remove CONFIG_OF and CONFIG_ACPI
macro guards around device id lists. Still I would prefer to save
compiled code size.

Arnd or Greg, your valued opinion is much appreciated.

--
With best wishes,
Vladimir

Re: [PATCH 2/2] hwrng: mxc-rnga - add driver support on boards with device tree

[Kim Phillips]

On Tue, 27 Feb 2018 18:53:08 +0200
Vladimir Zapolskiy  wrote:

> On 02/27/2018 05:49 PM, Kim Phillips wrote:
> > On Mon, 26 Feb 2018 20:38:49 +0200
> > Vladimir Zapolskiy  wrote:
> > 
> >> +#ifdef CONFIG_OF
> >> +static const struct of_device_id mxc_rnga_of_match[] = {
> >> +  { .compatible = "fsl,imx31-rnga", },
> >> +  { /* sentinel */ },
> >> +};
> >> +MODULE_DEVICE_TABLE(of, mxc_rnga_of_match);
> >> +#endif
> >> +
> >>  static struct platform_driver mxc_rnga_driver = {
> >>.driver = {
> >> - .name = "mxc_rnga",
> >> - },
> >> +  .name = "mxc_rnga",
> >> +  .of_match_table = of_match_ptr(mxc_rnga_of_match),
> > 
> > Does this build if CONFIG_OF is not set?
> 
> Definitely it is expected to be built, you can verify it directly or
> check of_match_ptr() macro definition from include/linux/of.h

Thanks, I verified it by removing the SOC_IMX31 dependency, and with
netwinder_defconfig as a base.  I also verified that the #ifdef
CONFIG_OF protecting the mxc_rnga_of_match definition is also not
needed.

Kim

Re: Can a driver->probe be called for two devices at the same time (WAS: Re: [PATCH] crypto/ccp: don't disable interrupts while setting up debugfs)

[Sebastian Andrzej Siewior]

On 2018-02-27 19:40:34 [+0100], Greg Kroah-Hartman wrote:
> On Tue, Feb 27, 2018 at 06:33:14PM +0100, Sebastian Andrzej Siewior wrote:
> > On 2018-02-27 11:08:56 [-0600], Gary R Hook wrote:
> > > That issue remains unclear to me: Are probes of PCI devices guaranteed to 
> > > be
> > > serialized? Observations on my CCPs says that they occur in order, but I
> > > don't know for certain that serialization is guaranteed.
> > > 
> > > Is there a definitive statement on this somewhere that I just don't know
> > > about?
> 
> The bus enforces this.
> 
> > So the question if a driver can probe two devices simultaneously.
> 
> Depends on the bus type.

PCI

> thanks,
> 
> greg k-h

Sebastian

Re: Can a driver->probe be called for two devices at the same time (WAS: Re: [PATCH] crypto/ccp: don't disable interrupts while setting up debugfs)

[Greg Kroah-Hartman]

On Tue, Feb 27, 2018 at 06:33:14PM +0100, Sebastian Andrzej Siewior wrote:
> On 2018-02-27 11:08:56 [-0600], Gary R Hook wrote:
> > That issue remains unclear to me: Are probes of PCI devices guaranteed to be
> > serialized? Observations on my CCPs says that they occur in order, but I
> > don't know for certain that serialization is guaranteed.
> > 
> > Is there a definitive statement on this somewhere that I just don't know
> > about?

The bus enforces this.

> So the question if a driver can probe two devices simultaneously.

Depends on the bus type.

thanks,

greg k-h

Re: Can a driver->probe be called for two devices at the same time (WAS: Re: [PATCH] crypto/ccp: don't disable interrupts while setting up debugfs)

[Gary R Hook]

On 02/27/2018 11:33 AM, Sebastian Andrzej Siewior wrote:

On 2018-02-27 11:08:56 [-0600], Gary R Hook wrote:

That issue remains unclear to me: Are probes of PCI devices guaranteed to be
serialized? Observations on my CCPs says that they occur in order, but I
don't know for certain that serialization is guaranteed.

Is there a definitive statement on this somewhere that I just don't know
about?


So the question if a driver can probe two devices simultaneously. I'm
not sure. We have PROBE_PREFER_ASYNCHRONOUS which defers the probe to
worker. However I have no idea if two of those worker can run at the
same time.


I think a mutex would be just fine; I got this wrong, clearly. Let me work
up a patch using a mutex.


I've sent one. Why not just ack it and be done with it?


Gary


Sebastian



Sorry, too much chaos right now. Of course.

Can a driver->probe be called for two devices at the same time (WAS: Re: [PATCH] crypto/ccp: don't disable interrupts while setting up debugfs)

[Sebastian Andrzej Siewior]

On 2018-02-27 11:08:56 [-0600], Gary R Hook wrote:
> That issue remains unclear to me: Are probes of PCI devices guaranteed to be
> serialized? Observations on my CCPs says that they occur in order, but I
> don't know for certain that serialization is guaranteed.
> 
> Is there a definitive statement on this somewhere that I just don't know
> about?

So the question if a driver can probe two devices simultaneously. I'm
not sure. We have PROBE_PREFER_ASYNCHRONOUS which defers the probe to
worker. However I have no idea if two of those worker can run at the
same time.

> I think a mutex would be just fine; I got this wrong, clearly. Let me work
> up a patch using a mutex.

I've sent one. Why not just ack it and be done with it?

> Gary

Sebastian

Re: [PATCH] crypto/ccp: don't disable interrupts while setting up debugfs

[Gary R Hook]

On 02/26/2018 02:35 AM, Sebastian Andrzej Siewior wrote:

On 2018-02-25 21:04:27 [-0500], Hook, Gary wrote:

On 2/23/2018 5:33 PM, Sebastian Andrzej Siewior wrote:

I don't why we need take a single write lock and disable interrupts
while setting up debugfs. This is what what happens when we try anyway:


There is more than one CCP on some processors. The lock is intended to
serialize attempts to initialize the new directory, but a R/W lock isn't
required.


And they are probed in parallel? Any you need disable interrupts while
creating the debugfs folder? A mutex isn't enough?


That issue remains unclear to me: Are probes of PCI devices guaranteed 
to be serialized? Observations on my CCPs says that they occur in order, 
but I don't know for certain that serialization is guaranteed.


Is there a definitive statement on this somewhere that I just don't know 
about?


I think a mutex would be just fine; I got this wrong, clearly. Let me 
work up a patch using a mutex.


Gary

Re: [PATCH 2/2] hwrng: mxc-rnga - add driver support on boards with device tree

[Vladimir Zapolskiy]

On 02/27/2018 05:49 PM, Kim Phillips wrote:
> On Mon, 26 Feb 2018 20:38:49 +0200
> Vladimir Zapolskiy  wrote:
> 
>> +#ifdef CONFIG_OF
>> +static const struct of_device_id mxc_rnga_of_match[] = {
>> +{ .compatible = "fsl,imx31-rnga", },
>> +{ /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, mxc_rnga_of_match);
>> +#endif
>> +
>>  static struct platform_driver mxc_rnga_driver = {
>>  .driver = {
>> -   .name = "mxc_rnga",
>> -   },
>> +.name = "mxc_rnga",
>> +.of_match_table = of_match_ptr(mxc_rnga_of_match),
> 
> Does this build if CONFIG_OF is not set?
> 

Definitely it is expected to be built, you can verify it directly or
check of_match_ptr() macro definition from include/linux/of.h

--
With best wishes,
Vladimir

Re: [PATCH 2/2] hwrng: mxc-rnga - add driver support on boards with device tree

[Kim Phillips]

On Mon, 26 Feb 2018 20:38:49 +0200
Vladimir Zapolskiy  wrote:

> +#ifdef CONFIG_OF
> +static const struct of_device_id mxc_rnga_of_match[] = {
> + { .compatible = "fsl,imx31-rnga", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, mxc_rnga_of_match);
> +#endif
> +
>  static struct platform_driver mxc_rnga_driver = {
>   .driver = {
> -.name = "mxc_rnga",
> -},
> + .name = "mxc_rnga",
> + .of_match_table = of_match_ptr(mxc_rnga_of_match),

Does this build if CONFIG_OF is not set?

Thanks,

Kim

Re: error in libkcapi 1.0.3 for aead aio

[Stephan Müller]

Am Sonntag, 25. Februar 2018, 06:25:06 CET schrieb Harsh Jain:

Hi Harsh,

> Hi Stephan,
> 
> 1 of the test mentioned in test.sh is failing for AEAD AIO operation even
> thought driver is returning EBADMSG(as expected) to af_alg with latest
> cryptodev tree.
> 
> Debug log and strace attached.
> 
> Command :
> 
> strace -o strace.log ../bin/kcapi   -x 10   -c "gcm(aes)" -i
> 7815d4b06ae50c9c56e87bd7 -k ea38ac0c9b9998c80e28fb496a2b88d9 -a
> "853f98a750098bec1aa7497e979e78098155c877879556bb51ddeb6374cbaefc" -t
> "c4ce58985b7203094be1d134c1b8ab0b" -q "b03692f86d1b8b39baf2abb255197c98"

I am not seeing that the driver reports -EBADMSG in the strace log. If you see 
the following line, you will spot the issue:

io_getevents(140715452821504, 1, 1, {{(nil), 0x24a41c0, 4294967222, 0}}, NULL) 
= 1

The 4th parameter is struct io_event that is returned from the kernel. It 
contains as 3rd value "4294967222". This value ought to be the return code of 
the crypto operation. If the value is positive, it returns the number of 
processed bytes. If it is negative, it holds the error code.

I am not sure you expect a value close to 2^32 bytes as return code -- from 
the io_submit syscall where you send 48 bytes and from the command above I 
assume that this return value is wrong.
> 
> Thanks & Regards
> 
> Harsh Jain


Ciao
Stephan

Re: [PATCH v3 0/4] crypto: AF_ALG AIO improvements

[Stephan Müller]

Am Freitag, 23. Februar 2018, 13:00:26 CET schrieb Herbert Xu:

Hi Herbert,

> On Fri, Feb 23, 2018 at 09:33:33AM +0100, Stephan Müller wrote:
> > A simple copy operation, however, will imply that in one AIO recvmsg
> > request, only *one* IOCB can be set and processed.
> 
> Sure, but the recvmsg will return as soon as the crypto API encrypt
> or decrypt function returns.  It's still fully async.  It's just
> that the setup part needs to be done with sendmsg/recvmsg.

Wouldn't a copy of the ctx->iv into a per-request buffer change the behavoir 
of the AF_ALG interface significantly?

Today, if multiple IOCBs are submitted, most cipher implementations would 
serialize the requests (e.g. all implementations that behave synchronous in 
nature like all software implementations).

Thus, when copying the ctx->iv into a separate per-request buffer, suddenly 
all block-chained cipher operations are not block chained any more.
> 
> Even if we wanted to do what you stated, just inlining the IV isn't
> enough.  You'd also need to inline the assoclen, and probably the
> optype in case you want to mix encrypt/decrypt too.

Maybe that is what we have to do.
> 
> However, I must say that I don't see the point of going all the way
> to support such a bulk submission interface (e.g., lio_listio).

IMHO, the point is that AF_ALG is the only interface to allow userspace to 
utilize hardware crypto implementations. For example, on a small chip with 
hardware crypto support, your user space code can offload crypto to that 
hardware to free CPU time.

How else would somebody access its crypto accelerators?
> 
> Remember, the algif interface due to its inherent overhead is meant
> for bulk data.  That is, the processing time for each request is
> dominated by the actual processing, not the submission process.

I see that. And for smaller chips with crypto support, this would be the case 
IMHO. Especially if we streamline the AF_ALG overhead such that we reduce the 
number of syscalls and user/kernel space roundtrips.
> 
> If you're instead processing lots of tiny requests, do NOT use
> algif because it's not designed for that.

The only issue in this case is that it makes the operation slower. 
> 
> Therefore spending too much time to optimise the submission overhead
> seems pointless to me.
> 
> Cheers,


Ciao
Stephan

[PATCH 2/6] crypto: omap-crypto: Verify page zone scatterlists before starting DMA

[Tero Kristo]

In certain platforms like DRA7xx having memory > 2GB with LPAE enabled
has a constraint that DMA can be done with the initial 2GB and marks it
as ZONE_DMA. But openssl when used with cryptodev does not make sure that
input buffer is DMA capable. So, adding a check to verify if the input
buffer is capable of DMA.

Signed-off-by: Tero Kristo 
---
 drivers/crypto/omap-crypto.c | 4 
 1 file changed, 4 insertions(+)

diff --git a/drivers/crypto/omap-crypto.c b/drivers/crypto/omap-crypto.c
index 23e3777..2c42e4b 100644
--- a/drivers/crypto/omap-crypto.c
+++ b/drivers/crypto/omap-crypto.c
@@ -104,6 +104,10 @@ static int omap_crypto_check_sg(struct scatterlist *sg, 
int total, int bs,
return OMAP_CRYPTO_NOT_ALIGNED;
if (!IS_ALIGNED(sg->length, bs))
return OMAP_CRYPTO_NOT_ALIGNED;
+#ifdef CONFIG_ZONE_DMA
+   if (page_zonenum(sg_page(sg)) != ZONE_DMA)
+   return OMAP_CRYPTO_NOT_ALIGNED;
+#endif
 
len += sg->length;
sg = sg_next(sg);
-- 
1.9.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH 3/6] crypto: omap-sham: make fallback size configurable

[Tero Kristo]

Crypto driver fallback size can now be configured from userspace. This
allows optimizing the DMA usage based on use case. Default fallback
size of 256 is still used.

Signed-off-by: Tero Kristo 
---
 drivers/crypto/omap-sham.c | 56 +++---
 1 file changed, 53 insertions(+), 3 deletions(-)

diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index ce398b7..7fb9eef 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -229,6 +229,7 @@ struct omap_sham_dev {
u8  xmit_buf[BUFLEN] OMAP_ALIGNED;
 
unsigned long   flags;
+   int fallback_sz;
struct crypto_queue queue;
struct ahash_request*req;
 
@@ -1009,7 +1010,7 @@ static int omap_sham_update_req(struct omap_sham_dev *dd)
 ctx->total, ctx->digcnt, (ctx->flags & BIT(FLAGS_FINUP)) != 0);
 
if (ctx->total < get_block_size(ctx) ||
-   ctx->total < OMAP_SHA_DMA_THRESHOLD)
+   ctx->total < dd->fallback_sz)
ctx->flags |= BIT(FLAGS_CPU);
 
if (ctx->flags & BIT(FLAGS_CPU))
@@ -1265,11 +1266,11 @@ static int omap_sham_final(struct ahash_request *req)
/*
 * OMAP HW accel works only with buffers >= 9.
 * HMAC is always >= 9 because ipad == block size.
-* If buffersize is less than DMA_THRESHOLD, we use fallback
+* If buffersize is less than fallback_sz, we use fallback
 * SW encoding, as using DMA + HW in this case doesn't provide
 * any benefit.
 */
-   if (!ctx->digcnt && ctx->bufcnt < OMAP_SHA_DMA_THRESHOLD)
+   if (!ctx->digcnt && ctx->bufcnt < ctx->dd->fallback_sz)
return omap_sham_final_shash(req);
else if (ctx->bufcnt)
return omap_sham_enqueue(req, OP_FINAL);
@@ -2020,6 +2021,47 @@ static int omap_sham_get_res_pdev(struct omap_sham_dev 
*dd,
return err;
 }
 
+static ssize_t fallback_show(struct device *dev, struct device_attribute *attr,
+char *buf)
+{
+   struct omap_sham_dev *dd = dev_get_drvdata(dev);
+
+   return sprintf(buf, "%d\n", dd->fallback_sz);
+}
+
+static ssize_t fallback_store(struct device *dev, struct device_attribute 
*attr,
+ const char *buf, size_t size)
+{
+   struct omap_sham_dev *dd = dev_get_drvdata(dev);
+   ssize_t status;
+   long value;
+
+   status = kstrtol(buf, 0, );
+   if (status)
+   return status;
+
+   /* HW accelerator only works with buffers > 9 */
+   if (value < 9) {
+   dev_err(dev, "minimum fallback size 9\n");
+   return -EINVAL;
+   }
+
+   dd->fallback_sz = value;
+
+   return size;
+}
+
+static DEVICE_ATTR_RW(fallback);
+
+static struct attribute *omap_sham_attrs[] = {
+   _attr_fallback.attr,
+   NULL,
+};
+
+static struct attribute_group omap_sham_attr_group = {
+   .attrs = omap_sham_attrs,
+};
+
 static int omap_sham_probe(struct platform_device *pdev)
 {
struct omap_sham_dev *dd;
@@ -2081,6 +2123,8 @@ static int omap_sham_probe(struct platform_device *pdev)
pm_runtime_use_autosuspend(dev);
pm_runtime_set_autosuspend_delay(dev, DEFAULT_AUTOSUSPEND_DELAY);
 
+   dd->fallback_sz = OMAP_SHA_DMA_THRESHOLD;
+
pm_runtime_enable(dev);
pm_runtime_irq_safe(dev);
 
@@ -2118,6 +2162,12 @@ static int omap_sham_probe(struct platform_device *pdev)
}
}
 
+   err = sysfs_create_group(>kobj, _sham_attr_group);
+   if (err) {
+   dev_err(dev, "could not create sysfs device attrs\n");
+   goto err_algs;
+   }
+
return 0;
 
 err_algs:
-- 
1.9.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH 5/6] crypto: omap-aes: make fallback size configurable

[Tero Kristo]

Crypto driver fallback size can now be configured from userspace. This
allows optimizing the DMA usage based on use case. Detault fallback
size of 200 is still used.

Signed-off-by: Tero Kristo 
---
 drivers/crypto/omap-aes.c | 48 ++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c
index fbec0a2..76bc94f 100644
--- a/drivers/crypto/omap-aes.c
+++ b/drivers/crypto/omap-aes.c
@@ -47,6 +47,8 @@
 static LIST_HEAD(dev_list);
 static DEFINE_SPINLOCK(list_lock);
 
+static int aes_fallback_sz = 200;
+
 #ifdef DEBUG
 #define omap_aes_read(dd, offset)  \
 ({ \
@@ -517,7 +519,7 @@ static int omap_aes_crypt(struct ablkcipher_request *req, 
unsigned long mode)
  !!(mode & FLAGS_ENCRYPT),
  !!(mode & FLAGS_CBC));
 
-   if (req->nbytes < 200) {
+   if (req->nbytes < aes_fallback_sz) {
SKCIPHER_REQUEST_ON_STACK(subreq, ctx->fallback);
 
skcipher_request_set_tfm(subreq, ctx->fallback);
@@ -1029,6 +1031,44 @@ static int omap_aes_get_res_pdev(struct omap_aes_dev *dd,
return err;
 }
 
+static ssize_t fallback_show(struct device *dev, struct device_attribute *attr,
+char *buf)
+{
+   return sprintf(buf, "%d\n", aes_fallback_sz);
+}
+
+static ssize_t fallback_store(struct device *dev, struct device_attribute 
*attr,
+ const char *buf, size_t size)
+{
+   ssize_t status;
+   long value;
+
+   status = kstrtol(buf, 0, );
+   if (status)
+   return status;
+
+   /* HW accelerator only works with buffers > 9 */
+   if (value < 9) {
+   dev_err(dev, "minimum fallback size 9\n");
+   return -EINVAL;
+   }
+
+   aes_fallback_sz = value;
+
+   return size;
+}
+
+static DEVICE_ATTR_RW(fallback);
+
+static struct attribute *omap_aes_attrs[] = {
+   _attr_fallback.attr,
+   NULL,
+};
+
+static struct attribute_group omap_aes_attr_group = {
+   .attrs = omap_aes_attrs,
+};
+
 static int omap_aes_probe(struct platform_device *pdev)
 {
struct device *dev = >dev;
@@ -1159,6 +1199,12 @@ static int omap_aes_probe(struct platform_device *pdev)
}
}
 
+   err = sysfs_create_group(>kobj, _aes_attr_group);
+   if (err) {
+   dev_err(dev, "could not create sysfs device attrs\n");
+   goto err_aead_algs;
+   }
+
return 0;
 err_aead_algs:
for (i = dd->pdata->aead_algs_info->registered - 1; i >= 0; i--) {
-- 
1.9.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH 1/6] crypto: omap-sham: Verify page zone of scatterlists before starting DMA

[Tero Kristo]

In certain platforms like DRA7xx having memory > 2GB with LPAE enabled
has a constraint that DMA can be done with the initial 2GB and marks it
as ZONE_DMA. But openssl when used with cryptodev does not make sure that
input buffer is DMA capable. So, adding a check to verify if the input
buffer is capable of DMA.

Signed-off-by: Tero Kristo 
Reported-by: Aparna Balasubramanian 
Reviewed-by: Lokesh Vutla 
---
 drivers/crypto/omap-sham.c | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index 86b89ac..ce398b7 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -759,6 +759,13 @@ static int omap_sham_align_sgs(struct scatterlist *sg,
while (nbytes > 0 && sg_tmp) {
n++;
 
+#ifdef CONFIG_ZONE_DMA
+   if (page_zonenum(sg_page(sg_tmp)) != ZONE_DMA) {
+   aligned = false;
+   break;
+   }
+#endif
+
if (offset < sg_tmp->length) {
if (!IS_ALIGNED(offset + sg_tmp->offset, 4)) {
aligned = false;
-- 
1.9.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH 4/6] crypto: omap-sham: make queue length configurable

[Tero Kristo]

Crypto driver queue size can now be configured from userspace. This
allows optimizing the queue usage based on use case. Default queue
size is still 10 entries.

Signed-off-by: Tero Kristo 
---
 drivers/crypto/omap-sham.c | 38 ++
 1 file changed, 38 insertions(+)

diff --git a/drivers/crypto/omap-sham.c b/drivers/crypto/omap-sham.c
index 7fb9eef..8e7e1582 100644
--- a/drivers/crypto/omap-sham.c
+++ b/drivers/crypto/omap-sham.c
@@ -2051,9 +2051,47 @@ static ssize_t fallback_store(struct device *dev, struct 
device_attribute *attr,
return size;
 }
 
+static ssize_t queue_len_show(struct device *dev, struct device_attribute 
*attr,
+ char *buf)
+{
+   struct omap_sham_dev *dd = dev_get_drvdata(dev);
+
+   return sprintf(buf, "%d\n", dd->queue.max_qlen);
+}
+
+static ssize_t queue_len_store(struct device *dev,
+  struct device_attribute *attr, const char *buf,
+  size_t size)
+{
+   struct omap_sham_dev *dd = dev_get_drvdata(dev);
+   ssize_t status;
+   long value;
+   unsigned long flags;
+
+   status = kstrtol(buf, 0, );
+   if (status)
+   return status;
+
+   if (value < 1)
+   return -EINVAL;
+
+   /*
+* Changing the queue size in fly is safe, if size becomes smaller
+* than current size, it will just not accept new entries until
+* it has shrank enough.
+*/
+   spin_lock_irqsave(>lock, flags);
+   dd->queue.max_qlen = value;
+   spin_unlock_irqrestore(>lock, flags);
+
+   return size;
+}
+
+static DEVICE_ATTR_RW(queue_len);
 static DEVICE_ATTR_RW(fallback);
 
 static struct attribute *omap_sham_attrs[] = {
+   _attr_queue_len.attr,
_attr_fallback.attr,
NULL,
 };
-- 
1.9.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH 0/6] crypto: omap: fixes + tweaks for omap crypto

[Tero Kristo]

Hi,

This series contains a couple of fixes for LPAE data handling (patch #1/#2),
and also a few configuration patches (#3..#6.) Without the fixes,
the driver will see crashes in certain data configs, most notably when
using cryptodev and passing data buffers from userspace. All these
patches can be targeted for 4.17 merge window.

The configuration patches make certain parameters for omap crypto drivers
user configurable, which helps optimizing performance on different HW
configurations, and also avoiding buffer overflows when using for example
UDP protocol for ipsec. The queue size patches are maybe more controversial,
but I could not find anybody modifying the queue sizes for now. Should there
be a separate API for this purpose or what would be preferable?

-Tero

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

[PATCH 6/6] crypto: omap-aes: make queue length configurable

[Tero Kristo]

Crypto driver queue size can now be configured from userspace. This
allows optimizing the queue usage based on use case. Default queue
size is still 10 entries.

Signed-off-by: Tero Kristo 
---
 drivers/crypto/omap-aes.c | 43 +++
 1 file changed, 43 insertions(+)

diff --git a/drivers/crypto/omap-aes.c b/drivers/crypto/omap-aes.c
index 76bc94f..250880a 100644
--- a/drivers/crypto/omap-aes.c
+++ b/drivers/crypto/omap-aes.c
@@ -1058,9 +1058,52 @@ static ssize_t fallback_store(struct device *dev, struct 
device_attribute *attr,
return size;
 }
 
+static ssize_t queue_len_show(struct device *dev, struct device_attribute 
*attr,
+ char *buf)
+{
+   struct omap_aes_dev *dd = dev_get_drvdata(dev);
+
+   return sprintf(buf, "%d\n", dd->engine->queue.max_qlen);
+}
+
+static ssize_t queue_len_store(struct device *dev,
+  struct device_attribute *attr, const char *buf,
+  size_t size)
+{
+   struct omap_aes_dev *dd;
+   ssize_t status;
+   long value;
+   unsigned long flags;
+
+   status = kstrtol(buf, 0, );
+   if (status)
+   return status;
+
+   if (value < 1)
+   return -EINVAL;
+
+   /*
+* Changing the queue size in fly is safe, if size becomes smaller
+* than current size, it will just not accept new entries until
+* it has shrank enough.
+*/
+   spin_lock_bh(_lock);
+   list_for_each_entry(dd, _list, list) {
+   spin_lock_irqsave(>lock, flags);
+   dd->engine->queue.max_qlen = value;
+   dd->aead_queue.base.max_qlen = value;
+   spin_unlock_irqrestore(>lock, flags);
+   }
+   spin_unlock_bh(_lock);
+
+   return size;
+}
+
+static DEVICE_ATTR_RW(queue_len);
 static DEVICE_ATTR_RW(fallback);
 
 static struct attribute *omap_aes_attrs[] = {
+   _attr_queue_len.attr,
_attr_fallback.attr,
NULL,
 };
-- 
1.9.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. 
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki