* mv_cesa hash functions @ 2012-02-22 13:03 Frank 2012-02-22 20:10 ` Nikos Mavrogiannopoulos 2012-02-23 18:34 ` Phil Sutter 0 siblings, 2 replies; 17+ messages in thread From: Frank @ 2012-02-22 13:03 UTC (permalink / raw) To: 'Sebastian Andrzej Siewior', uri; +Cc: linux-crypto Hi, After doing some trials with hardware crypto offloading through usermode interfaces (af_alg and cryptodev) to Marvell CESA accelerated ciphers and hash functions with the 3.2.4 kernel's mv_cesa in Debian Wheezy on a Marvell Kirkwood system, I've noticed the following kernel output when I load the mv_cesa kernel module: [490889.448060] alg: hash: Test 1 failed for mv-sha1 [490889.452786] 00000000: c1 94 3f 2e a2 41 ce 88 d5 47 07 43 c4 a8 17 5d [490889.459368] 00000010: 77 e8 47 ca [490889.464321] alg: hash: Test 1 failed for mv-hmac-sha1 [490889.469493] 00000000: 06 71 4d 7c cc cc b5 cf 1b d6 c7 ab d0 25 c4 21 [490889.476068] 00000010: 66 0b 8e 70 Using SHA1 in a ssl/tls handshake fails in tests with mv_cesa loaded, which might be related to this. Regards, Frank ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: mv_cesa hash functions 2012-02-22 13:03 mv_cesa hash functions Frank @ 2012-02-22 20:10 ` Nikos Mavrogiannopoulos 2012-02-23 14:17 ` Sebastian Andrzej Siewior 2012-02-23 18:34 ` Phil Sutter 1 sibling, 1 reply; 17+ messages in thread From: Nikos Mavrogiannopoulos @ 2012-02-22 20:10 UTC (permalink / raw) To: Frank; +Cc: 'Sebastian Andrzej Siewior', uri, linux-crypto On 02/22/2012 02:03 PM, Frank wrote: > Hi, > > After doing some trials with hardware crypto offloading through usermode interfaces (af_alg and cryptodev) to Marvell CESA accelerated ciphers and hash functions with the 3.2.4 kernel's mv_cesa in Debian Wheezy on a Marvell Kirkwood system, I've noticed the following kernel output when I load the mv_cesa kernel module: > > [490889.448060] alg: hash: Test 1 failed for mv-sha1 > [490889.452786] 00000000: c1 94 3f 2e a2 41 ce 88 d5 47 07 43 c4 a8 17 5d > [490889.459368] 00000010: 77 e8 47 ca > [490889.464321] alg: hash: Test 1 failed for mv-hmac-sha1 > [490889.469493] 00000000: 06 71 4d 7c cc cc b5 cf 1b d6 c7 ab d0 25 c4 21 > [490889.476068] 00000010: 66 0b 8e 70 > Using SHA1 in a ssl/tls handshake fails in tests with mv_cesa loaded, which might be related to this. It might be related. I noticed the same issue in userspace with /dev/crypto. It can be solved by adding a 50 ms delay after the encryption, decryption and hashing operations. (a delay less than that didn't work). regards, Nikos ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: mv_cesa hash functions 2012-02-22 20:10 ` Nikos Mavrogiannopoulos @ 2012-02-23 14:17 ` Sebastian Andrzej Siewior 0 siblings, 0 replies; 17+ messages in thread From: Sebastian Andrzej Siewior @ 2012-02-23 14:17 UTC (permalink / raw) To: Nikos Mavrogiannopoulos; +Cc: Frank, uri, linux-crypto On Wed, Feb 22, 2012 at 09:10:46PM +0100, Nikos Mavrogiannopoulos wrote: > On 02/22/2012 02:03 PM, Frank wrote: > > > Hi, > > > > After doing some trials with hardware crypto offloading through usermode interfaces (af_alg and cryptodev) to Marvell CESA accelerated ciphers and hash functions with the 3.2.4 kernel's mv_cesa in Debian Wheezy on a Marvell Kirkwood system, I've noticed the following kernel output when I load the mv_cesa kernel module: > > > > [490889.448060] alg: hash: Test 1 failed for mv-sha1 > > [490889.452786] 00000000: c1 94 3f 2e a2 41 ce 88 d5 47 07 43 c4 a8 17 5d > > [490889.459368] 00000010: 77 e8 47 ca > > [490889.464321] alg: hash: Test 1 failed for mv-hmac-sha1 > > [490889.469493] 00000000: 06 71 4d 7c cc cc b5 cf 1b d6 c7 ab d0 25 c4 21 > > [490889.476068] 00000010: 66 0b 8e 70 > > Using SHA1 in a ssl/tls handshake fails in tests with mv_cesa loaded, which might be related to this. > > > It might be related. I noticed the same issue in userspace with > /dev/crypto. It can be solved by adding a 50 ms delay after the > encryption, decryption and hashing operations. (a delay less than that > didn't work). It might a missing cache flush before/after the opration or before passing the data to userland. I've seen that some hash operation fail which are not perfromed in one go but via the start, update, end path. However I had no time to get to the bottom of this. > > regards, > Nikos Sebastian ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: mv_cesa hash functions 2012-02-22 13:03 mv_cesa hash functions Frank @ 2012-02-23 18:34 ` Phil Sutter 2012-02-23 18:34 ` Phil Sutter 1 sibling, 0 replies; 17+ messages in thread From: Phil Sutter @ 2012-02-23 18:34 UTC (permalink / raw) To: Frank Cc: 'Sebastian Andrzej Siewior', uri, linux-crypto, Herbert Xu, Catalin Marinas, linux-arm-kernel, Simon Baatz Hello, On Wed, Feb 22, 2012 at 02:03:38PM +0100, Frank wrote: > After doing some trials with hardware crypto offloading through usermode interfaces (af_alg and cryptodev) to Marvell CESA accelerated ciphers and hash functions with the 3.2.4 kernel's mv_cesa in Debian Wheezy on a Marvell Kirkwood system, I've noticed the following kernel output when I load the mv_cesa kernel module: > > [490889.448060] alg: hash: Test 1 failed for mv-sha1 > [490889.452786] 00000000: c1 94 3f 2e a2 41 ce 88 d5 47 07 43 c4 a8 17 5d > [490889.459368] 00000010: 77 e8 47 ca > [490889.464321] alg: hash: Test 1 failed for mv-hmac-sha1 > [490889.469493] 00000000: 06 71 4d 7c cc cc b5 cf 1b d6 c7 ab d0 25 c4 21 > [490889.476068] 00000010: 66 0b 8e 70 I've tracked down the problem to commit 6ef8450, "crypto: mv_cesa - make count_sgs() null-pointer proof". So apparently it was me who broke it. The simpification introduced by that commit assumes a zero 'nbytes' field in the ahash_request struct passed to mv_hash_final, but crypto/testmgr.c calls crypto_ahash_update() and crypto_ahash_final() without altering the ahash_request in between. Herbert, please clarify: is it intended behaviour that ahash_alg's final callback ignores possibly present data in the request? If I wanted to finalise a hash operation with some final data, would I then use the finup callback instead? (Note the implied question of the actual difference between the two callbacks.) If my assumptions are correct, fixing this issue would be as easy as adding a call to ahash_request_set_crypt() to mv_hash_final(). Or manually zeroing req->nbytes, but that's probably unclean. Anyway, if the 'final' callback is always to ignore request data, why doesn't the API do this already? At least mv_cesa could use a single function for both callbacks then. > Using SHA1 in a ssl/tls handshake fails in tests with mv_cesa loaded, which might be related to this. You are using cryptodev-linux for that, right? If so, this should be unrelated since cryptodev-linux always calls crypto_ahash_final() with an empty request payload (from cryptodev_cipher.c): | ahash_request_set_crypt(hdata->async.request, NULL, output, 0); | | ret = crypto_ahash_final(hdata->async.request); But you might suffer from another problem, which is only present on ARM machines with VIVT cache and linux >= 2.6.37: due to commit f8b63c1, "ARM: 6382/1: Remove superfluous flush_kernel_dcache_page()" which prevents pages being flushed from inside the scatterlist iterator API. This patch seems to introduce problems in other places (namely NFS), too but I sadly did not have time to investigate this further. I will post a possible (cryptodev-internal) solution to cryptodev-linux-devel@gna.org, maybe this fixes the problem with openssl. Greetings, Phil ^ permalink raw reply [flat|nested] 17+ messages in thread
* mv_cesa hash functions @ 2012-02-23 18:34 ` Phil Sutter 0 siblings, 0 replies; 17+ messages in thread From: Phil Sutter @ 2012-02-23 18:34 UTC (permalink / raw) To: linux-arm-kernel Hello, On Wed, Feb 22, 2012 at 02:03:38PM +0100, Frank wrote: > After doing some trials with hardware crypto offloading through usermode interfaces (af_alg and cryptodev) to Marvell CESA accelerated ciphers and hash functions with the 3.2.4 kernel's mv_cesa in Debian Wheezy on a Marvell Kirkwood system, I've noticed the following kernel output when I load the mv_cesa kernel module: > > [490889.448060] alg: hash: Test 1 failed for mv-sha1 > [490889.452786] 00000000: c1 94 3f 2e a2 41 ce 88 d5 47 07 43 c4 a8 17 5d > [490889.459368] 00000010: 77 e8 47 ca > [490889.464321] alg: hash: Test 1 failed for mv-hmac-sha1 > [490889.469493] 00000000: 06 71 4d 7c cc cc b5 cf 1b d6 c7 ab d0 25 c4 21 > [490889.476068] 00000010: 66 0b 8e 70 I've tracked down the problem to commit 6ef8450, "crypto: mv_cesa - make count_sgs() null-pointer proof". So apparently it was me who broke it. The simpification introduced by that commit assumes a zero 'nbytes' field in the ahash_request struct passed to mv_hash_final, but crypto/testmgr.c calls crypto_ahash_update() and crypto_ahash_final() without altering the ahash_request in between. Herbert, please clarify: is it intended behaviour that ahash_alg's final callback ignores possibly present data in the request? If I wanted to finalise a hash operation with some final data, would I then use the finup callback instead? (Note the implied question of the actual difference between the two callbacks.) If my assumptions are correct, fixing this issue would be as easy as adding a call to ahash_request_set_crypt() to mv_hash_final(). Or manually zeroing req->nbytes, but that's probably unclean. Anyway, if the 'final' callback is always to ignore request data, why doesn't the API do this already? At least mv_cesa could use a single function for both callbacks then. > Using SHA1 in a ssl/tls handshake fails in tests with mv_cesa loaded, which might be related to this. You are using cryptodev-linux for that, right? If so, this should be unrelated since cryptodev-linux always calls crypto_ahash_final() with an empty request payload (from cryptodev_cipher.c): | ahash_request_set_crypt(hdata->async.request, NULL, output, 0); | | ret = crypto_ahash_final(hdata->async.request); But you might suffer from another problem, which is only present on ARM machines with VIVT cache and linux >= 2.6.37: due to commit f8b63c1, "ARM: 6382/1: Remove superfluous flush_kernel_dcache_page()" which prevents pages being flushed from inside the scatterlist iterator API. This patch seems to introduce problems in other places (namely NFS), too but I sadly did not have time to investigate this further. I will post a possible (cryptodev-internal) solution to cryptodev-linux-devel at gna.org, maybe this fixes the problem with openssl. Greetings, Phil ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: mv_cesa hash functions 2012-02-23 18:34 ` Phil Sutter (?) @ 2012-02-25 7:38 ` Herbert Xu 2012-02-27 11:17 ` [PATCH] crypto: mv_cesa - fix final callback not ignoring input data Phil Sutter -1 siblings, 1 reply; 17+ messages in thread From: Herbert Xu @ 2012-02-25 7:38 UTC (permalink / raw) To: Frank, 'Sebastian Andrzej Siewior', uri, linux-crypto, Catalin Marinas, linux-arm-kernel, Simon Baatz On Thu, Feb 23, 2012 at 07:34:39PM +0100, Phil Sutter wrote: > > Herbert, please clarify: is it intended behaviour that ahash_alg's final > callback ignores possibly present data in the request? If I wanted to > finalise a hash operation with some final data, would I then use the > finup callback instead? (Note the implied question of the actual > difference between the two callbacks.) final should ignore any data present in the request. So we should fix this up in mv_cesa. 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 [flat|nested] 17+ messages in thread
* [PATCH] crypto: mv_cesa - fix final callback not ignoring input data 2012-02-25 7:38 ` Herbert Xu @ 2012-02-27 11:17 ` Phil Sutter 2012-02-28 8:30 ` Herbert Xu 0 siblings, 1 reply; 17+ messages in thread From: Phil Sutter @ 2012-02-27 11:17 UTC (permalink / raw) To: linux-crypto; +Cc: Herbert Xu, stable, #, 3.0+ Broken by commit 6ef84509f3d439ed2d43ea40080643efec37f54f for users passing a request with non-zero 'nbytes' field, like e.g. testmgr. Cc: <stable@kernel.org> # 3.0+ Signed-off-by: Phil Sutter <phil.sutter@viprinet.com> --- drivers/crypto/mv_cesa.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/drivers/crypto/mv_cesa.c b/drivers/crypto/mv_cesa.c index 597235a..0d40cf6 100644 --- a/drivers/crypto/mv_cesa.c +++ b/drivers/crypto/mv_cesa.c @@ -714,6 +714,7 @@ static int mv_hash_final(struct ahash_request *req) { struct mv_req_hash_ctx *ctx = ahash_request_ctx(req); + ahash_request_set_crypt(req, NULL, req->result, 0); mv_update_hash_req_ctx(ctx, 1, 0); return mv_handle_req(&req->base); } -- 1.7.3.4 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH] crypto: mv_cesa - fix final callback not ignoring input data 2012-02-27 11:17 ` [PATCH] crypto: mv_cesa - fix final callback not ignoring input data Phil Sutter @ 2012-02-28 8:30 ` Herbert Xu 0 siblings, 0 replies; 17+ messages in thread From: Herbert Xu @ 2012-02-28 8:30 UTC (permalink / raw) To: Phil Sutter; +Cc: linux-crypto, stable, #, 3.0+ On Mon, Feb 27, 2012 at 12:17:04PM +0100, Phil Sutter wrote: > Broken by commit 6ef84509f3d439ed2d43ea40080643efec37f54f for users > passing a request with non-zero 'nbytes' field, like e.g. testmgr. > > Cc: <stable@kernel.org> # 3.0+ > Signed-off-by: Phil Sutter <phil.sutter@viprinet.com> Patch applied. Thanks a lot! -- 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 [flat|nested] 17+ messages in thread
* mv_cesa dcache problem since 2.6.37 was: Re: mv_cesa hash functions 2012-02-23 18:34 ` Phil Sutter @ 2012-05-05 22:49 ` Simon Baatz -1 siblings, 0 replies; 17+ messages in thread From: Simon Baatz @ 2012-05-05 22:49 UTC (permalink / raw) To: Frank, 'Sebastian Andrzej Siewior', uri, linux-crypto, Herbert Xu, Catalin Marinas, linux-arm-kernel Hi, Am 23.02.2012 19:34, schrieb Phil Sutter: > But you might suffer from another problem, which is only present on > ARM machines with VIVT cache and linux >= 2.6.37: due to commit > f8b63c1, "ARM: 6382/1: Remove superfluous flush_kernel_dcache_page()" > which prevents pages being flushed from inside the scatterlist > iterator API. This patch seems to introduce problems in other places > (namely NFS), too but I sadly did not have time to investigate this > further. I will post a possible (cryptodev-internal) solution to > cryptodev-linux-devel@gna.org, maybe this fixes the problem with > openssl. Greetings, Phil since there has been no reaction on this, I would like to bring this issue up again (I sadly don't have the expertise to investigate this further...). The issue is not limited to cryptodev, but seems to be either a problem with commit f8b63c1 or a problem in mv_cesa that was uncovered by this commit. In the past, I also had massive problems compiling on an NFS mounted file system when not reverting f8b63c1. I currently can't reproduce this anymore under 3.4-rc5. However, the following still happens on a 3.4-rc5 preempt kernel on arm kirkwood (VIVT cache): root@ww1:~# cryptsetup luksOpen /dev/sda2 c_sda2 Enter passphrase for /dev/sda2: root@ww1:~# vgchange -a y ww1_1 Volume group "ww1_1" not found root@ww1:~# vgchange -a y ww1_1 Segmentation fault Thus, the behavior of vgchange is unpredictable on a dm-crypt device when using mv_cesa (btw. the machine needs to have no load to show this behavior. I assume that the cache flushes caused by task switches make it work under load). Using the generic crypto modules, it works as expected: root@ww1:~# cryptsetup luksClose c_sda2 root@ww1:~# rmmod mv_cesa root@ww1:~# cryptsetup luksOpen /dev/sda2 c_sda2 Enter passphrase for /dev/sda2: root@ww1:~# vgchange -a y ww1_1 1 logical volume(s) in volume group "ww1_1" now active After reverting f8b63c1, it works as expected using mv_cesa as well. - Simon ^ permalink raw reply [flat|nested] 17+ messages in thread
* mv_cesa dcache problem since 2.6.37 was: Re: mv_cesa hash functions @ 2012-05-05 22:49 ` Simon Baatz 0 siblings, 0 replies; 17+ messages in thread From: Simon Baatz @ 2012-05-05 22:49 UTC (permalink / raw) To: linux-arm-kernel Hi, Am 23.02.2012 19:34, schrieb Phil Sutter: > But you might suffer from another problem, which is only present on > ARM machines with VIVT cache and linux >= 2.6.37: due to commit > f8b63c1, "ARM: 6382/1: Remove superfluous flush_kernel_dcache_page()" > which prevents pages being flushed from inside the scatterlist > iterator API. This patch seems to introduce problems in other places > (namely NFS), too but I sadly did not have time to investigate this > further. I will post a possible (cryptodev-internal) solution to > cryptodev-linux-devel at gna.org, maybe this fixes the problem with > openssl. Greetings, Phil since there has been no reaction on this, I would like to bring this issue up again (I sadly don't have the expertise to investigate this further...). The issue is not limited to cryptodev, but seems to be either a problem with commit f8b63c1 or a problem in mv_cesa that was uncovered by this commit. In the past, I also had massive problems compiling on an NFS mounted file system when not reverting f8b63c1. I currently can't reproduce this anymore under 3.4-rc5. However, the following still happens on a 3.4-rc5 preempt kernel on arm kirkwood (VIVT cache): root at ww1:~# cryptsetup luksOpen /dev/sda2 c_sda2 Enter passphrase for /dev/sda2: root at ww1:~# vgchange -a y ww1_1 Volume group "ww1_1" not found root at ww1:~# vgchange -a y ww1_1 Segmentation fault Thus, the behavior of vgchange is unpredictable on a dm-crypt device when using mv_cesa (btw. the machine needs to have no load to show this behavior. I assume that the cache flushes caused by task switches make it work under load). Using the generic crypto modules, it works as expected: root at ww1:~# cryptsetup luksClose c_sda2 root at ww1:~# rmmod mv_cesa root at ww1:~# cryptsetup luksOpen /dev/sda2 c_sda2 Enter passphrase for /dev/sda2: root at ww1:~# vgchange -a y ww1_1 1 logical volume(s) in volume group "ww1_1" now active After reverting f8b63c1, it works as expected using mv_cesa as well. - Simon ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: mv_cesa dcache problem since 2.6.37 was: Re: mv_cesa hash functions 2012-05-05 22:49 ` Simon Baatz @ 2012-05-06 12:25 ` Russell King - ARM Linux -1 siblings, 0 replies; 17+ messages in thread From: Russell King - ARM Linux @ 2012-05-06 12:25 UTC (permalink / raw) To: Simon Baatz Cc: Frank, 'Sebastian Andrzej Siewior', uri, linux-crypto, Herbert Xu, Catalin Marinas, linux-arm-kernel On Sun, May 06, 2012 at 12:49:30AM +0200, Simon Baatz wrote: > Am 23.02.2012 19:34, schrieb Phil Sutter: > > But you might suffer from another problem, which is only present on > > ARM machines with VIVT cache and linux >= 2.6.37: due to commit > > f8b63c1, "ARM: 6382/1: Remove superfluous flush_kernel_dcache_page()" > > which prevents pages being flushed from inside the scatterlist > > iterator API. This patch seems to introduce problems in other places > > (namely NFS), too but I sadly did not have time to investigate this > > further. I will post a possible (cryptodev-internal) solution to > > cryptodev-linux-devel@gna.org, maybe this fixes the problem with > > openssl. Greetings, Phil Well, the reason it has been removed is as follows: When pages are allocated, they have a special flag bit called PG_arch_1 cleared by the page allocator. We used to use PG_arch_1 to indicate that the page contained dirty cache lines - so by default, the page was marked clean. Normal kernel behaviour upon allocating a page into the page cache is to read data off the disk into the new page cache page, calling flush_kernel_dcache_page() in the process. This would ensure that data would be pushed out of the cache lines associated with the kernel mapping of the page into RAM. This means when we map the page into userspace, we can be sure that the new mapping will see the data written there. Moreover, when we place a page into a user process, we check this PG_arch_1 bit, and if it's set, we flush the kernel mapping cache lines at this point as well. Patch 6379/1 changed this behaviour: PG_arch_1 being _clear_ now indicates that the page contains dirty data. What this means is that when a page is placed into userspace, we check the PG_arch_1 bit. If it is clear, we flush the kernel mapping for the page to push dirty data out to RAM. This makes the flush in flush_kernel_dcache_page() entirely redundant. > since there has been no reaction on this, I would like to bring this > issue up again (I sadly don't have the expertise to investigate this > further...). The issue is not limited to cryptodev, but seems to be > either a problem with commit f8b63c1 or a problem in mv_cesa that was > uncovered by this commit. Can you reproduce it with anything else? It could be a problem with the way crypto stuff works - I've never had any dealings with that subsystem, so I really can't comment. If crypto uses scatterlists, and walks them with the standard API, and uses scatterlists with pages which are already mapped into userspace, then I can see how the above commit would make things go wrong. So, without knowledge of the crypto subsystem, I'm not sure I can help sort this out. If it's possible to reproduce with NFS, and it seems sorted in the latest kernel, that's probably because something changed with NFS - NFS did for a time have issues on ARM for various reasons (because of remapping kernel memory into vmalloc space and expecting it to be DMA coherent...) and that got fixed. Whether that's why you're not seeing problems with v3.4-rc5, I couldn't be sure unless you did a bisection between the bad and good kernel versions. That would at least allow us to confirm that that issue has been properly resolved. Thanks. ^ permalink raw reply [flat|nested] 17+ messages in thread
* mv_cesa dcache problem since 2.6.37 was: Re: mv_cesa hash functions @ 2012-05-06 12:25 ` Russell King - ARM Linux 0 siblings, 0 replies; 17+ messages in thread From: Russell King - ARM Linux @ 2012-05-06 12:25 UTC (permalink / raw) To: linux-arm-kernel On Sun, May 06, 2012 at 12:49:30AM +0200, Simon Baatz wrote: > Am 23.02.2012 19:34, schrieb Phil Sutter: > > But you might suffer from another problem, which is only present on > > ARM machines with VIVT cache and linux >= 2.6.37: due to commit > > f8b63c1, "ARM: 6382/1: Remove superfluous flush_kernel_dcache_page()" > > which prevents pages being flushed from inside the scatterlist > > iterator API. This patch seems to introduce problems in other places > > (namely NFS), too but I sadly did not have time to investigate this > > further. I will post a possible (cryptodev-internal) solution to > > cryptodev-linux-devel at gna.org, maybe this fixes the problem with > > openssl. Greetings, Phil Well, the reason it has been removed is as follows: When pages are allocated, they have a special flag bit called PG_arch_1 cleared by the page allocator. We used to use PG_arch_1 to indicate that the page contained dirty cache lines - so by default, the page was marked clean. Normal kernel behaviour upon allocating a page into the page cache is to read data off the disk into the new page cache page, calling flush_kernel_dcache_page() in the process. This would ensure that data would be pushed out of the cache lines associated with the kernel mapping of the page into RAM. This means when we map the page into userspace, we can be sure that the new mapping will see the data written there. Moreover, when we place a page into a user process, we check this PG_arch_1 bit, and if it's set, we flush the kernel mapping cache lines at this point as well. Patch 6379/1 changed this behaviour: PG_arch_1 being _clear_ now indicates that the page contains dirty data. What this means is that when a page is placed into userspace, we check the PG_arch_1 bit. If it is clear, we flush the kernel mapping for the page to push dirty data out to RAM. This makes the flush in flush_kernel_dcache_page() entirely redundant. > since there has been no reaction on this, I would like to bring this > issue up again (I sadly don't have the expertise to investigate this > further...). The issue is not limited to cryptodev, but seems to be > either a problem with commit f8b63c1 or a problem in mv_cesa that was > uncovered by this commit. Can you reproduce it with anything else? It could be a problem with the way crypto stuff works - I've never had any dealings with that subsystem, so I really can't comment. If crypto uses scatterlists, and walks them with the standard API, and uses scatterlists with pages which are already mapped into userspace, then I can see how the above commit would make things go wrong. So, without knowledge of the crypto subsystem, I'm not sure I can help sort this out. If it's possible to reproduce with NFS, and it seems sorted in the latest kernel, that's probably because something changed with NFS - NFS did for a time have issues on ARM for various reasons (because of remapping kernel memory into vmalloc space and expecting it to be DMA coherent...) and that got fixed. Whether that's why you're not seeing problems with v3.4-rc5, I couldn't be sure unless you did a bisection between the bad and good kernel versions. That would at least allow us to confirm that that issue has been properly resolved. Thanks. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: mv_cesa dcache problem since 2.6.37 was: Re: mv_cesa hash functions 2012-05-06 12:25 ` Russell King - ARM Linux @ 2012-05-07 16:50 ` Phil Sutter -1 siblings, 0 replies; 17+ messages in thread From: Phil Sutter @ 2012-05-07 16:50 UTC (permalink / raw) To: Russell King - ARM Linux Cc: linux-arm-kernel, Herbert Xu, Catalin Marinas, 'Sebastian Andrzej Siewior', linux-crypto, uri, Frank, Simon Baatz Hi, On Sun, May 06, 2012 at 01:25:06PM +0100, Russell King - ARM Linux wrote: > On Sun, May 06, 2012 at 12:49:30AM +0200, Simon Baatz wrote: > > since there has been no reaction on this, I would like to bring this > > issue up again (I sadly don't have the expertise to investigate this > > further...). The issue is not limited to cryptodev, but seems to be > > either a problem with commit f8b63c1 or a problem in mv_cesa that was > > uncovered by this commit. > > Can you reproduce it with anything else? > > It could be a problem with the way crypto stuff works - I've never had > any dealings with that subsystem, so I really can't comment. If crypto > uses scatterlists, and walks them with the standard API, and uses > scatterlists with pages which are already mapped into userspace, then > I can see how the above commit would make things go wrong. This is quite exactly what happens. Cryptodev uses get_user_pages to get the pages belonging to the passed userspace addresses, fills scatterlists with that information and passes them on to the crypto API. Mv_cesa.c (at least) then uses the scatterlist mapping iterator to feed the data chunk by chunk to the crypto engine. Upon completion, cryptodev simply releases the user pages again by using SetPageDirty() and page_cache_release(). This has proven unreliable, and the solution I found was to additionally call flush_dcache_page() before returning the pages to userspace. > So, without knowledge of the crypto subsystem, I'm not sure I can help > sort this out. What would you recommend for iterating over scatterlists with mapped pages instead? Since the above commit, neither the mapping iterator API nor the simple sg_next() seem to be suitable anymore. Greetings, Phil ^ permalink raw reply [flat|nested] 17+ messages in thread
* mv_cesa dcache problem since 2.6.37 was: Re: mv_cesa hash functions @ 2012-05-07 16:50 ` Phil Sutter 0 siblings, 0 replies; 17+ messages in thread From: Phil Sutter @ 2012-05-07 16:50 UTC (permalink / raw) To: linux-arm-kernel Hi, On Sun, May 06, 2012 at 01:25:06PM +0100, Russell King - ARM Linux wrote: > On Sun, May 06, 2012 at 12:49:30AM +0200, Simon Baatz wrote: > > since there has been no reaction on this, I would like to bring this > > issue up again (I sadly don't have the expertise to investigate this > > further...). The issue is not limited to cryptodev, but seems to be > > either a problem with commit f8b63c1 or a problem in mv_cesa that was > > uncovered by this commit. > > Can you reproduce it with anything else? > > It could be a problem with the way crypto stuff works - I've never had > any dealings with that subsystem, so I really can't comment. If crypto > uses scatterlists, and walks them with the standard API, and uses > scatterlists with pages which are already mapped into userspace, then > I can see how the above commit would make things go wrong. This is quite exactly what happens. Cryptodev uses get_user_pages to get the pages belonging to the passed userspace addresses, fills scatterlists with that information and passes them on to the crypto API. Mv_cesa.c (at least) then uses the scatterlist mapping iterator to feed the data chunk by chunk to the crypto engine. Upon completion, cryptodev simply releases the user pages again by using SetPageDirty() and page_cache_release(). This has proven unreliable, and the solution I found was to additionally call flush_dcache_page() before returning the pages to userspace. > So, without knowledge of the crypto subsystem, I'm not sure I can help > sort this out. What would you recommend for iterating over scatterlists with mapped pages instead? Since the above commit, neither the mapping iterator API nor the simple sg_next() seem to be suitable anymore. Greetings, Phil ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: mv_cesa dcache problem since 2.6.37 was: Re: mv_cesa hash functions 2012-05-06 12:25 ` Russell King - ARM Linux @ 2012-05-08 20:50 ` Simon Baatz -1 siblings, 0 replies; 17+ messages in thread From: Simon Baatz @ 2012-05-08 20:50 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Simon Baatz, Frank, 'Sebastian Andrzej Siewior', linux-crypto, Herbert Xu, Catalin Marinas, linux-arm-kernel On Sun, May 06, 2012 at 01:25:06PM +0100, Russell King - ARM Linux wrote: > On Sun, May 06, 2012 at 12:49:30AM +0200, Simon Baatz wrote: > > Am 23.02.2012 19:34, schrieb Phil Sutter: > > > But you might suffer from another problem, which is only present on > > > ARM machines with VIVT cache and linux >= 2.6.37: due to commit > > > f8b63c1, "ARM: 6382/1: Remove superfluous flush_kernel_dcache_page()" > > > which prevents pages being flushed from inside the scatterlist > > > iterator API. This patch seems to introduce problems in other places > > > (namely NFS), too but I sadly did not have time to investigate this > > > further. I will post a possible (cryptodev-internal) solution to > > > cryptodev-linux-devel@gna.org, maybe this fixes the problem with > > > openssl. Greetings, Phil > > Well, the reason it has been removed is as follows: > Thanks for the explanation. This helped a lot. > > since there has been no reaction on this, I would like to bring this > > issue up again (I sadly don't have the expertise to investigate this > > further...). The issue is not limited to cryptodev, but seems to be > > either a problem with commit f8b63c1 or a problem in mv_cesa that was > > uncovered by this commit. > > Can you reproduce it with anything else? No, I only have issues with mv_cesa (and NFS in the past, see below). > It could be a problem with the way crypto stuff works - I've never had > any dealings with that subsystem, so I really can't comment. If crypto > uses scatterlists, and walks them with the standard API, and uses > scatterlists with pages which are already mapped into userspace, then > I can see how the above commit would make things go wrong. As Phil explained in his mail, this seems to be what happens. I have another question on top of his. mv_cesa uses SG_MITER_TO_SG, whose function was described in the respective commit as: commit 6de7e356faf54aa75de5b624bbce28a5b776dfa8 Author: Sebastian Andrzej Siewior <sebastian@breakpoint.cc> Date: Thu Jun 18 10:19:12 2009 +0200 lib/scatterlist: add a flags to signalize mapping direction sg_miter_start() is currently unaware of the direction of the copy process (to or from the scatter list). It is important to know the direction because the page has to be flushed in case the data written is seen on a different mapping in user land on cache incoherent architectures. Thus, I would have expected that the scatterlist iterators do the necessary "magic" in the userspace mapped case. Apparently, this is not the case with the changes you describe above. Is each driver supposed to handle this case or can/should this be handled in a generic fashion by scatterlists? > If it's possible to reproduce with NFS, and it seems sorted in the latest > kernel, that's probably because something changed with NFS - NFS did for > a time have issues on ARM for various reasons (because of remapping kernel > memory into vmalloc space and expecting it to be DMA coherent...) and that > got fixed. Whether that's why you're not seeing problems with v3.4-rc5, > I couldn't be sure unless you did a bisection between the bad and good > kernel versions. That would at least allow us to confirm that that issue > has been properly resolved. I had no idea that I reverted f8b63c1 for so long, but you are right again. The merge b9d919a4ac6cf031b8e065f82ad8f1b0c9ed74b1 (in 2.6.38) containing: NFS: Don't use vm_map_ram() in readdir fixes my test case. - Simon ^ permalink raw reply [flat|nested] 17+ messages in thread
* mv_cesa dcache problem since 2.6.37 was: Re: mv_cesa hash functions @ 2012-05-08 20:50 ` Simon Baatz 0 siblings, 0 replies; 17+ messages in thread From: Simon Baatz @ 2012-05-08 20:50 UTC (permalink / raw) To: linux-arm-kernel On Sun, May 06, 2012 at 01:25:06PM +0100, Russell King - ARM Linux wrote: > On Sun, May 06, 2012 at 12:49:30AM +0200, Simon Baatz wrote: > > Am 23.02.2012 19:34, schrieb Phil Sutter: > > > But you might suffer from another problem, which is only present on > > > ARM machines with VIVT cache and linux >= 2.6.37: due to commit > > > f8b63c1, "ARM: 6382/1: Remove superfluous flush_kernel_dcache_page()" > > > which prevents pages being flushed from inside the scatterlist > > > iterator API. This patch seems to introduce problems in other places > > > (namely NFS), too but I sadly did not have time to investigate this > > > further. I will post a possible (cryptodev-internal) solution to > > > cryptodev-linux-devel at gna.org, maybe this fixes the problem with > > > openssl. Greetings, Phil > > Well, the reason it has been removed is as follows: > Thanks for the explanation. This helped a lot. > > since there has been no reaction on this, I would like to bring this > > issue up again (I sadly don't have the expertise to investigate this > > further...). The issue is not limited to cryptodev, but seems to be > > either a problem with commit f8b63c1 or a problem in mv_cesa that was > > uncovered by this commit. > > Can you reproduce it with anything else? No, I only have issues with mv_cesa (and NFS in the past, see below). > It could be a problem with the way crypto stuff works - I've never had > any dealings with that subsystem, so I really can't comment. If crypto > uses scatterlists, and walks them with the standard API, and uses > scatterlists with pages which are already mapped into userspace, then > I can see how the above commit would make things go wrong. As Phil explained in his mail, this seems to be what happens. I have another question on top of his. mv_cesa uses SG_MITER_TO_SG, whose function was described in the respective commit as: commit 6de7e356faf54aa75de5b624bbce28a5b776dfa8 Author: Sebastian Andrzej Siewior <sebastian@breakpoint.cc> Date: Thu Jun 18 10:19:12 2009 +0200 lib/scatterlist: add a flags to signalize mapping direction sg_miter_start() is currently unaware of the direction of the copy process (to or from the scatter list). It is important to know the direction because the page has to be flushed in case the data written is seen on a different mapping in user land on cache incoherent architectures. Thus, I would have expected that the scatterlist iterators do the necessary "magic" in the userspace mapped case. Apparently, this is not the case with the changes you describe above. Is each driver supposed to handle this case or can/should this be handled in a generic fashion by scatterlists? > If it's possible to reproduce with NFS, and it seems sorted in the latest > kernel, that's probably because something changed with NFS - NFS did for > a time have issues on ARM for various reasons (because of remapping kernel > memory into vmalloc space and expecting it to be DMA coherent...) and that > got fixed. Whether that's why you're not seeing problems with v3.4-rc5, > I couldn't be sure unless you did a bisection between the bad and good > kernel versions. That would at least allow us to confirm that that issue > has been properly resolved. I had no idea that I reverted f8b63c1 for so long, but you are right again. The merge b9d919a4ac6cf031b8e065f82ad8f1b0c9ed74b1 (in 2.6.38) containing: NFS: Don't use vm_map_ram() in readdir fixes my test case. - Simon ^ permalink raw reply [flat|nested] 17+ messages in thread
* RE: mv_cesa dcache problem since 2.6.37 was: Re: mv_cesa hash functions 2012-05-05 22:49 ` Simon Baatz (?) (?) @ 2012-05-07 13:01 ` Frank -1 siblings, 0 replies; 17+ messages in thread From: Frank @ 2012-05-07 13:01 UTC (permalink / raw) To: 'Simon Baatz', 'Sebastian Andrzej Siewior', uri, linux-crypto, 'Herbert Xu', 'Catalin Marinas', linux-arm-kernel Hi, > -----Original Message----- > From: Simon Baatz [mailto:gmbnomis@gmail.com] > Sent: Sunday, May 06, 2012 00:50 > To: Frank; 'Sebastian Andrzej Siewior'; uri@jdland.co.il; linux- > crypto@vger.kernel.org; Herbert Xu; Catalin Marinas; linux-arm- > kernel@lists.arm.linux.org.uk > Subject: mv_cesa dcache problem since 2.6.37 was: Re: mv_cesa hash > functions > > Hi, > > Am 23.02.2012 19:34, schrieb Phil Sutter: > > But you might suffer from another problem, which is only present on > > ARM machines with VIVT cache and linux >= 2.6.37: due to commit > > f8b63c1, "ARM: 6382/1: Remove superfluous flush_kernel_dcache_page()" > > which prevents pages being flushed from inside the scatterlist > > iterator API. This patch seems to introduce problems in other places > > (namely NFS), too but I sadly did not have time to investigate this > > further. I will post a possible (cryptodev-internal) solution to > > cryptodev-linux-devel@gna.org, maybe this fixes the problem with > > openssl. Greetings, Phil > > since there has been no reaction on this, I would like to bring this > issue up again (I sadly don't have the expertise to investigate this > further...). The issue is not limited to cryptodev, but seems to be > either a problem with commit f8b63c1 or a problem in mv_cesa that was > uncovered by this commit. In the past, I also had massive problems > compiling on an NFS mounted file system when not reverting f8b63c1. I > currently can't reproduce this anymore under 3.4-rc5. > > However, the following still happens on a 3.4-rc5 preempt kernel on arm > kirkwood (VIVT cache): > > root@ww1:~# cryptsetup luksOpen /dev/sda2 c_sda2 > Enter passphrase for /dev/sda2: > root@ww1:~# vgchange -a y ww1_1 > Volume group "ww1_1" not found > root@ww1:~# vgchange -a y ww1_1 > Segmentation fault > > > Thus, the behavior of vgchange is unpredictable on a dm-crypt device > when using mv_cesa (btw. the machine needs to have no load to show this > behavior. I assume that the cache flushes caused by task switches make > it work under load). > > Using the generic crypto modules, it works as expected: > > root@ww1:~# cryptsetup luksClose c_sda2 > root@ww1:~# rmmod mv_cesa > root@ww1:~# cryptsetup luksOpen /dev/sda2 c_sda2 > Enter passphrase for /dev/sda2: > root@ww1:~# vgchange -a y ww1_1 > 1 logical volume(s) in volume group "ww1_1" now active > > > After reverting f8b63c1, it works as expected using mv_cesa as well. > > > - Simon I'm not sure if this is related, but I do see a kernel oops (on kernel 3.2.15) when using mv_cesa in the situation where clients connect to a L2TP/IPSEC VPN and use eap-tls authentication for the ppp link: [ 495.526702] Unable to handle kernel paging request at virtual address 03205d58 [ 495.533964] pgd = c0004000 [ 495.536697] [03205d58] *pgd=00000000 [ 495.540298] Internal error: Oops: 5 [#1] [ 495.544236] Modules linked in: mv_cesa ppp_async crc_ccitt ppp_generic slhc authenc xfrm4_mode_transport tcm_loop tcm_fc libfc scsi_transport_fc scsi_tgt iscsi_target_mod target_core_pscsi target_core_file target_core_iblock target_core_mod configfs xfrm_user xfrm4_tunnel tunnel4 ipcomp xfrm_ipcomp esp4 ah4 ctr twofish_generic twofish_common camellia serpent blowfish_generic blowfish_common cast5 des_generic cbc aes_generic xcbc rmd160 sha512_generic sha256_generic sha1_generic hmac crypto_null af_key ipv6 ext3 jbd ext2 loop evdev snd_usb_audio snd_usbmidi_lib snd_hwdep snd_seq_midi snd_seq_midi_event snd_rawmidi snd_pcm snd_page_alloc snd_seq snd_seq_device snd_timer snd soundcore gpio_keys ext4 jbd2 mbcache sd_mod crc_t10dif usb_storage uas sata_mv libata ehci_hcd scsi_mod usbcore usb _common mv643xx_eth inet_lro libphy [last unloaded: l2tp_core] [ 495.620303] CPU: 0 Not tainted (3.2.0-2-kirkwood #1) [ 495.625645] PC is at memcpy+0x94/0x3a4 [ 495.629415] LR is at copy_src_to_buf+0x68/0x94 [mv_cesa] [ 495.634754] pc : [<c01929d4>] lr : [<bf530c3c>] psr: 20000013 [ 495.634759] sp : de989f24 ip : 00000008 fp : de988000 [ 495.646295] r10: 00000608 r9 : de5cd868 r8 : 00000000 [ 495.651545] r7 : e0e6e648 r6 : 00000040 r5 : 00000040 r4 : c01929cc [ 495.658104] r3 : 00000018 r2 : 00000008 r1 : 03205d58 r0 : e0e6e648 [ 495.664661] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment kernel [ 495.672003] Control: 0005397f Table: 1e564000 DAC: 00000017 [ 495.677775] Process mv_crypto (pid: 2484, stack limit = 0xde988270) [ 495.684071] Stack: (0xde989f24 to 0xde98a000) [ 495.688454] 9f20: 00000040 00000040 e0e6e648 00000000 e0e6e648 ded7e1ac bf530c3c [ 495.696668] 9f40: 00000080 ded7e180 00000608 ded7e180 ded7e100 bf530ca8 06080080 de5cd8e8 [ 495.704882] 9f60: 00000001 bf530e3c 00000070 00000000 00000000 00000000 00000000 06080080 [ 495.713097] 9f80: 00000000 00000000 00000004 de5cd8e8 bf531f38 ded7e180 ded7e1ac bf5312e0 [ 495.721312] 9fa0: ded7e180 ded7e1ac de989f30 ddccbdf0 ded7e180 bf530f70 00000013 00000000 [ 495.729533] 9fc0: 00000000 00000000 00000000 c003dc68 00000000 00000000 ded7e180 00000000 [ 495.737748] 9fe0: de989fe0 de989fe0 ddccbdf0 c003dbec c000edf8 c000edf8 00000000 00000000 [ 495.745964] Code: 108ff00c ea000012 e1a00000 e4913004 (e4914004) [ 495.752389] ---[ end trace 0054294d538730e7 ]--- Here's the patch for Debian Squeeze/Wheezy to allow ppp authentication against a RADIUS server in ppp: http://debian-nas.org/files/ppp-2.4.5-heiart-radius-eaptls-debian.patch Here are instructions on how to set it up: http://debian-nas.org/files/ppp-radius-eaptls-debian-howto.txt I haven't tested (yet) if reverting f8b63c1 fixes this issue for me. Regards, Frank ^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2012-05-08 20:50 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-02-22 13:03 mv_cesa hash functions Frank 2012-02-22 20:10 ` Nikos Mavrogiannopoulos 2012-02-23 14:17 ` Sebastian Andrzej Siewior 2012-02-23 18:34 ` Phil Sutter 2012-02-23 18:34 ` Phil Sutter 2012-02-25 7:38 ` Herbert Xu 2012-02-27 11:17 ` [PATCH] crypto: mv_cesa - fix final callback not ignoring input data Phil Sutter 2012-02-28 8:30 ` Herbert Xu 2012-05-05 22:49 ` mv_cesa dcache problem since 2.6.37 was: Re: mv_cesa hash functions Simon Baatz 2012-05-05 22:49 ` Simon Baatz 2012-05-06 12:25 ` Russell King - ARM Linux 2012-05-06 12:25 ` Russell King - ARM Linux 2012-05-07 16:50 ` Phil Sutter 2012-05-07 16:50 ` Phil Sutter 2012-05-08 20:50 ` Simon Baatz 2012-05-08 20:50 ` Simon Baatz 2012-05-07 13:01 ` Frank
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.