* [PATCH RT] arm*: disable NEON in kernel mode
[not found] ` <20171201104331.GB1612@linutronix.de>
@ 2017-12-01 13:45 ` Sebastian Andrzej Siewior
2017-12-01 14:18 ` Mark Rutland
0 siblings, 1 reply; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2017-12-01 13:45 UTC (permalink / raw)
To: linux-arm-kernel
+arm folks, to let you know
On 2017-12-01 11:43:32 [+0100], To linux-rt-users at vger.kernel.org wrote:
> NEON in kernel mode is used by the crypto algorithms and raid6 code.
> While the raid6 code looks okay, the crypto algorithms do not: NEON
> is enabled on first invocation and may allocate/free/map memory before
> the NEON mode is disabled again.
> This needs to be changed until it can be enabled.
> On ARM NEON in kernel mode can be simply disabled. on ARM64 it needs to
> stay on due to possible EFI callbacks so here I disable each algorithm.
>
> Cc: stable-rt at vger.kernel.org
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> ---
> arch/arm/Kconfig | 2 +-
> arch/arm64/crypto/Kconfig | 20 ++++++++++----------
> arch/arm64/crypto/crc32-ce-glue.c | 3 ++-
> 3 files changed, 13 insertions(+), 12 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index d1346a160760..914fecb088a8 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -2164,7 +2164,7 @@ config NEON
>
> config KERNEL_MODE_NEON
> bool "Support for NEON in kernel mode"
> - depends on NEON && AEABI
> + depends on NEON && AEABI && !PREEMPT_RT_BASE
> help
> Say Y to include support for NEON in kernel mode.
>
> diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
> index 70c517aa4501..2a5f05b5a19a 100644
> --- a/arch/arm64/crypto/Kconfig
> +++ b/arch/arm64/crypto/Kconfig
> @@ -19,19 +19,19 @@ config CRYPTO_SHA512_ARM64
>
> config CRYPTO_SHA1_ARM64_CE
> tristate "SHA-1 digest algorithm (ARMv8 Crypto Extensions)"
> - depends on KERNEL_MODE_NEON
> + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> select CRYPTO_HASH
> select CRYPTO_SHA1
>
> config CRYPTO_SHA2_ARM64_CE
> tristate "SHA-224/SHA-256 digest algorithm (ARMv8 Crypto Extensions)"
> - depends on KERNEL_MODE_NEON
> + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> select CRYPTO_HASH
> select CRYPTO_SHA256_ARM64
>
> config CRYPTO_GHASH_ARM64_CE
> tristate "GHASH/AES-GCM using ARMv8 Crypto Extensions"
> - depends on KERNEL_MODE_NEON
> + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> select CRYPTO_HASH
> select CRYPTO_GF128MUL
> select CRYPTO_AES
> @@ -39,7 +39,7 @@ config CRYPTO_GHASH_ARM64_CE
>
> config CRYPTO_CRCT10DIF_ARM64_CE
> tristate "CRCT10DIF digest algorithm using PMULL instructions"
> - depends on KERNEL_MODE_NEON && CRC_T10DIF
> + depends on KERNEL_MODE_NEON && CRC_T10DIF && !PREEMPT_RT_BASE
> select CRYPTO_HASH
>
> config CRYPTO_CRC32_ARM64_CE
> @@ -53,13 +53,13 @@ config CRYPTO_AES_ARM64
>
> config CRYPTO_AES_ARM64_CE
> tristate "AES core cipher using ARMv8 Crypto Extensions"
> - depends on ARM64 && KERNEL_MODE_NEON
> + depends on ARM64 && KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> select CRYPTO_ALGAPI
> select CRYPTO_AES_ARM64
>
> config CRYPTO_AES_ARM64_CE_CCM
> tristate "AES in CCM mode using ARMv8 Crypto Extensions"
> - depends on ARM64 && KERNEL_MODE_NEON
> + depends on ARM64 && KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> select CRYPTO_ALGAPI
> select CRYPTO_AES_ARM64_CE
> select CRYPTO_AES_ARM64
> @@ -67,7 +67,7 @@ config CRYPTO_AES_ARM64_CE_CCM
>
> config CRYPTO_AES_ARM64_CE_BLK
> tristate "AES in ECB/CBC/CTR/XTS modes using ARMv8 Crypto Extensions"
> - depends on KERNEL_MODE_NEON
> + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> select CRYPTO_BLKCIPHER
> select CRYPTO_AES_ARM64_CE
> select CRYPTO_AES_ARM64
> @@ -75,7 +75,7 @@ config CRYPTO_AES_ARM64_CE_BLK
>
> config CRYPTO_AES_ARM64_NEON_BLK
> tristate "AES in ECB/CBC/CTR/XTS modes using NEON instructions"
> - depends on KERNEL_MODE_NEON
> + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> select CRYPTO_BLKCIPHER
> select CRYPTO_AES_ARM64
> select CRYPTO_AES
> @@ -83,13 +83,13 @@ config CRYPTO_AES_ARM64_NEON_BLK
>
> config CRYPTO_CHACHA20_NEON
> tristate "NEON accelerated ChaCha20 symmetric cipher"
> - depends on KERNEL_MODE_NEON
> + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> select CRYPTO_BLKCIPHER
> select CRYPTO_CHACHA20
>
> config CRYPTO_AES_ARM64_BS
> tristate "AES in ECB/CBC/CTR/XTS modes using bit-sliced NEON algorithm"
> - depends on KERNEL_MODE_NEON
> + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> select CRYPTO_BLKCIPHER
> select CRYPTO_AES_ARM64_NEON_BLK
> select CRYPTO_AES_ARM64
> diff --git a/arch/arm64/crypto/crc32-ce-glue.c b/arch/arm64/crypto/crc32-ce-glue.c
> index 624f4137918c..599de95cd86d 100644
> --- a/arch/arm64/crypto/crc32-ce-glue.c
> +++ b/arch/arm64/crypto/crc32-ce-glue.c
> @@ -206,7 +206,8 @@ static struct shash_alg crc32_pmull_algs[] = { {
>
> static int __init crc32_pmull_mod_init(void)
> {
> - if (IS_ENABLED(CONFIG_KERNEL_MODE_NEON) && (elf_hwcap & HWCAP_PMULL)) {
> + if (IS_ENABLED(CONFIG_KERNEL_MODE_NEON) &&
> + !IS_ENABLED(CONFIG_PREEMPT_RT_BASE) && (elf_hwcap & HWCAP_PMULL)) {
> crc32_pmull_algs[0].update = crc32_pmull_update;
> crc32_pmull_algs[1].update = crc32c_pmull_update;
>
> --
> 2.15.0
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RT] arm*: disable NEON in kernel mode
2017-12-01 13:45 ` [PATCH RT] arm*: disable NEON in kernel mode Sebastian Andrzej Siewior
@ 2017-12-01 14:18 ` Mark Rutland
2017-12-01 14:36 ` Sebastian Andrzej Siewior
0 siblings, 1 reply; 12+ messages in thread
From: Mark Rutland @ 2017-12-01 14:18 UTC (permalink / raw)
To: linux-arm-kernel
[Adding Ard, who wrote the NEON crypto code]
On Fri, Dec 01, 2017 at 02:45:06PM +0100, Sebastian Andrzej Siewior wrote:
> +arm folks, to let you know
>
> On 2017-12-01 11:43:32 [+0100], To linux-rt-users at vger.kernel.org wrote:
> > NEON in kernel mode is used by the crypto algorithms and raid6 code.
> > While the raid6 code looks okay, the crypto algorithms do not: NEON
> > is enabled on first invocation and may allocate/free/map memory before
> > the NEON mode is disabled again.
Could you elaborate on why this is a problem?
I guess this is because kernel_neon_{begin,end}() disable preemption?
... is this specific to RT?
Thanks,
Mark.
> > This needs to be changed until it can be enabled.
> > On ARM NEON in kernel mode can be simply disabled. on ARM64 it needs to
> > stay on due to possible EFI callbacks so here I disable each algorithm.
> >
> > Cc: stable-rt at vger.kernel.org
> > Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > ---
> > arch/arm/Kconfig | 2 +-
> > arch/arm64/crypto/Kconfig | 20 ++++++++++----------
> > arch/arm64/crypto/crc32-ce-glue.c | 3 ++-
> > 3 files changed, 13 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index d1346a160760..914fecb088a8 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -2164,7 +2164,7 @@ config NEON
> >
> > config KERNEL_MODE_NEON
> > bool "Support for NEON in kernel mode"
> > - depends on NEON && AEABI
> > + depends on NEON && AEABI && !PREEMPT_RT_BASE
> > help
> > Say Y to include support for NEON in kernel mode.
> >
> > diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
> > index 70c517aa4501..2a5f05b5a19a 100644
> > --- a/arch/arm64/crypto/Kconfig
> > +++ b/arch/arm64/crypto/Kconfig
> > @@ -19,19 +19,19 @@ config CRYPTO_SHA512_ARM64
> >
> > config CRYPTO_SHA1_ARM64_CE
> > tristate "SHA-1 digest algorithm (ARMv8 Crypto Extensions)"
> > - depends on KERNEL_MODE_NEON
> > + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> > select CRYPTO_HASH
> > select CRYPTO_SHA1
> >
> > config CRYPTO_SHA2_ARM64_CE
> > tristate "SHA-224/SHA-256 digest algorithm (ARMv8 Crypto Extensions)"
> > - depends on KERNEL_MODE_NEON
> > + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> > select CRYPTO_HASH
> > select CRYPTO_SHA256_ARM64
> >
> > config CRYPTO_GHASH_ARM64_CE
> > tristate "GHASH/AES-GCM using ARMv8 Crypto Extensions"
> > - depends on KERNEL_MODE_NEON
> > + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> > select CRYPTO_HASH
> > select CRYPTO_GF128MUL
> > select CRYPTO_AES
> > @@ -39,7 +39,7 @@ config CRYPTO_GHASH_ARM64_CE
> >
> > config CRYPTO_CRCT10DIF_ARM64_CE
> > tristate "CRCT10DIF digest algorithm using PMULL instructions"
> > - depends on KERNEL_MODE_NEON && CRC_T10DIF
> > + depends on KERNEL_MODE_NEON && CRC_T10DIF && !PREEMPT_RT_BASE
> > select CRYPTO_HASH
> >
> > config CRYPTO_CRC32_ARM64_CE
> > @@ -53,13 +53,13 @@ config CRYPTO_AES_ARM64
> >
> > config CRYPTO_AES_ARM64_CE
> > tristate "AES core cipher using ARMv8 Crypto Extensions"
> > - depends on ARM64 && KERNEL_MODE_NEON
> > + depends on ARM64 && KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> > select CRYPTO_ALGAPI
> > select CRYPTO_AES_ARM64
> >
> > config CRYPTO_AES_ARM64_CE_CCM
> > tristate "AES in CCM mode using ARMv8 Crypto Extensions"
> > - depends on ARM64 && KERNEL_MODE_NEON
> > + depends on ARM64 && KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> > select CRYPTO_ALGAPI
> > select CRYPTO_AES_ARM64_CE
> > select CRYPTO_AES_ARM64
> > @@ -67,7 +67,7 @@ config CRYPTO_AES_ARM64_CE_CCM
> >
> > config CRYPTO_AES_ARM64_CE_BLK
> > tristate "AES in ECB/CBC/CTR/XTS modes using ARMv8 Crypto Extensions"
> > - depends on KERNEL_MODE_NEON
> > + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> > select CRYPTO_BLKCIPHER
> > select CRYPTO_AES_ARM64_CE
> > select CRYPTO_AES_ARM64
> > @@ -75,7 +75,7 @@ config CRYPTO_AES_ARM64_CE_BLK
> >
> > config CRYPTO_AES_ARM64_NEON_BLK
> > tristate "AES in ECB/CBC/CTR/XTS modes using NEON instructions"
> > - depends on KERNEL_MODE_NEON
> > + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> > select CRYPTO_BLKCIPHER
> > select CRYPTO_AES_ARM64
> > select CRYPTO_AES
> > @@ -83,13 +83,13 @@ config CRYPTO_AES_ARM64_NEON_BLK
> >
> > config CRYPTO_CHACHA20_NEON
> > tristate "NEON accelerated ChaCha20 symmetric cipher"
> > - depends on KERNEL_MODE_NEON
> > + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> > select CRYPTO_BLKCIPHER
> > select CRYPTO_CHACHA20
> >
> > config CRYPTO_AES_ARM64_BS
> > tristate "AES in ECB/CBC/CTR/XTS modes using bit-sliced NEON algorithm"
> > - depends on KERNEL_MODE_NEON
> > + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> > select CRYPTO_BLKCIPHER
> > select CRYPTO_AES_ARM64_NEON_BLK
> > select CRYPTO_AES_ARM64
> > diff --git a/arch/arm64/crypto/crc32-ce-glue.c b/arch/arm64/crypto/crc32-ce-glue.c
> > index 624f4137918c..599de95cd86d 100644
> > --- a/arch/arm64/crypto/crc32-ce-glue.c
> > +++ b/arch/arm64/crypto/crc32-ce-glue.c
> > @@ -206,7 +206,8 @@ static struct shash_alg crc32_pmull_algs[] = { {
> >
> > static int __init crc32_pmull_mod_init(void)
> > {
> > - if (IS_ENABLED(CONFIG_KERNEL_MODE_NEON) && (elf_hwcap & HWCAP_PMULL)) {
> > + if (IS_ENABLED(CONFIG_KERNEL_MODE_NEON) &&
> > + !IS_ENABLED(CONFIG_PREEMPT_RT_BASE) && (elf_hwcap & HWCAP_PMULL)) {
> > crc32_pmull_algs[0].update = crc32_pmull_update;
> > crc32_pmull_algs[1].update = crc32c_pmull_update;
> >
> > --
> > 2.15.0
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RT] arm*: disable NEON in kernel mode
2017-12-01 14:18 ` Mark Rutland
@ 2017-12-01 14:36 ` Sebastian Andrzej Siewior
2017-12-01 15:03 ` Ard Biesheuvel
2017-12-01 17:14 ` Peter Zijlstra
0 siblings, 2 replies; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2017-12-01 14:36 UTC (permalink / raw)
To: linux-arm-kernel
On 2017-12-01 14:18:28 [+0000], Mark Rutland wrote:
> [Adding Ard, who wrote the NEON crypto code]
>
> On Fri, Dec 01, 2017 at 02:45:06PM +0100, Sebastian Andrzej Siewior wrote:
> > +arm folks, to let you know
> >
> > On 2017-12-01 11:43:32 [+0100], To linux-rt-users at vger.kernel.org wrote:
> > > NEON in kernel mode is used by the crypto algorithms and raid6 code.
> > > While the raid6 code looks okay, the crypto algorithms do not: NEON
> > > is enabled on first invocation and may allocate/free/map memory before
> > > the NEON mode is disabled again.
>
> Could you elaborate on why this is a problem?
>
> I guess this is because kernel_neon_{begin,end}() disable preemption?
>
> ... is this specific to RT?
It is RT specific, yes. One thing are the unbounded latencies since
everything in this preempt_disable section can take time depending on
the size of the request.
The other thing is code like in
arch/arm64/crypto/aes-ce-ccm-glue.c:ccm_encrypt()
where within this preempt_disable() section skcipher_walk_done() is
invoked. That function can allocate/free/map memory which is okay for
!RT but is not for RT. I tried to break those loops for x86 [0] and I
simply didn't had the time to do the same for ARM. I am aware that
store/restore of the NEON registers (as SSE and AVX) is expensive and
doing a lot of operations in one go is desired. So for x86 I would want
to do some benchmarks and come up with some numbers based on which I
can argue with people one way or another depending on how much it hurts
and how long preemption can be disabled.
[0] https://www.spinics.net/lists/kernel/msg2663115.html
> Thanks,
> Mark.
Sebastian
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RT] arm*: disable NEON in kernel mode
2017-12-01 14:36 ` Sebastian Andrzej Siewior
@ 2017-12-01 15:03 ` Ard Biesheuvel
2017-12-01 17:58 ` Dave Martin
2017-12-01 17:14 ` Peter Zijlstra
1 sibling, 1 reply; 12+ messages in thread
From: Ard Biesheuvel @ 2017-12-01 15:03 UTC (permalink / raw)
To: linux-arm-kernel
l
> On 1 Dec 2017, at 14:36, Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
>
>> On 2017-12-01 14:18:28 [+0000], Mark Rutland wrote:
>> [Adding Ard, who wrote the NEON crypto code]
>>
>>> On Fri, Dec 01, 2017 at 02:45:06PM +0100, Sebastian Andrzej Siewior wrote:
>>> +arm folks, to let you know
>>>
>>>> On 2017-12-01 11:43:32 [+0100], To linux-rt-users at vger.kernel.org wrote:
>>>> NEON in kernel mode is used by the crypto algorithms and raid6 code.
>>>> While the raid6 code looks okay, the crypto algorithms do not: NEON
>>>> is enabled on first invocation and may allocate/free/map memory before
>>>> the NEON mode is disabled again.
>>
>> Could you elaborate on why this is a problem?
>>
>> I guess this is because kernel_neon_{begin,end}() disable preemption?
>>
>> ... is this specific to RT?
>
> It is RT specific, yes. One thing are the unbounded latencies since
> everything in this preempt_disable section can take time depending on
> the size of the request.
> The other thing is code like in
> arch/arm64/crypto/aes-ce-ccm-glue.c:ccm_encrypt()
>
> where within this preempt_disable() section skcipher_walk_done() is
> invoked. That function can allocate/free/map memory which is okay for
> !RT but is not for RT. I tried to break those loops for x86 [0] and I
> simply didn't had the time to do the same for ARM. I am aware that
> store/restore of the NEON registers (as SSE and AVX) is expensive and
> doing a lot of operations in one go is desired.
I wouldn?t mind fixing the code instead. We never disable the neon, but only stack the contents of the registers the first time, and unstack them only before returning to userland (with the exception of nested neon use in softirq context). When this code was introduced, we always stacked/unstacked the whole register file eagerly every time.
> So for x86 I would want
> to do some benchmarks and come up with some numbers based on which I
> can argue with people one way or another depending on how much it hurts
> and how long preemption can be disabled.
>
> [0] https://www.spinics.net/lists/kernel/msg2663115.html
>
>> Thanks,
>> Mark.
>
> Sebastian
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RT] arm*: disable NEON in kernel mode
2017-12-01 14:36 ` Sebastian Andrzej Siewior
2017-12-01 15:03 ` Ard Biesheuvel
@ 2017-12-01 17:14 ` Peter Zijlstra
2017-12-01 17:31 ` Russell King - ARM Linux
1 sibling, 1 reply; 12+ messages in thread
From: Peter Zijlstra @ 2017-12-01 17:14 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Dec 01, 2017 at 03:36:48PM +0100, Sebastian Andrzej Siewior wrote:
> On 2017-12-01 14:18:28 [+0000], Mark Rutland wrote:
> > [Adding Ard, who wrote the NEON crypto code]
> >
> > On Fri, Dec 01, 2017 at 02:45:06PM +0100, Sebastian Andrzej Siewior wrote:
> > > +arm folks, to let you know
> > >
> > > On 2017-12-01 11:43:32 [+0100], To linux-rt-users at vger.kernel.org wrote:
> > > > NEON in kernel mode is used by the crypto algorithms and raid6 code.
> > > > While the raid6 code looks okay, the crypto algorithms do not: NEON
> > > > is enabled on first invocation and may allocate/free/map memory before
> > > > the NEON mode is disabled again.
> >
> > Could you elaborate on why this is a problem?
> >
> > I guess this is because kernel_neon_{begin,end}() disable preemption?
> >
> > ... is this specific to RT?
>
> It is RT specific, yes. One thing are the unbounded latencies since
> everything in this preempt_disable section can take time depending on
> the size of the request.
Well, PREEMPT cares about that too.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RT] arm*: disable NEON in kernel mode
2017-12-01 17:14 ` Peter Zijlstra
@ 2017-12-01 17:31 ` Russell King - ARM Linux
2017-12-01 17:39 ` Peter Zijlstra
0 siblings, 1 reply; 12+ messages in thread
From: Russell King - ARM Linux @ 2017-12-01 17:31 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Dec 01, 2017 at 06:14:58PM +0100, Peter Zijlstra wrote:
> On Fri, Dec 01, 2017 at 03:36:48PM +0100, Sebastian Andrzej Siewior wrote:
> > On 2017-12-01 14:18:28 [+0000], Mark Rutland wrote:
> > > [Adding Ard, who wrote the NEON crypto code]
> > >
> > > On Fri, Dec 01, 2017 at 02:45:06PM +0100, Sebastian Andrzej Siewior wrote:
> > > > +arm folks, to let you know
> > > >
> > > > On 2017-12-01 11:43:32 [+0100], To linux-rt-users at vger.kernel.org wrote:
> > > > > NEON in kernel mode is used by the crypto algorithms and raid6 code.
> > > > > While the raid6 code looks okay, the crypto algorithms do not: NEON
> > > > > is enabled on first invocation and may allocate/free/map memory before
> > > > > the NEON mode is disabled again.
> > >
> > > Could you elaborate on why this is a problem?
> > >
> > > I guess this is because kernel_neon_{begin,end}() disable preemption?
> > >
> > > ... is this specific to RT?
> >
> > It is RT specific, yes. One thing are the unbounded latencies since
> > everything in this preempt_disable section can take time depending on
> > the size of the request.
>
> Well, PREEMPT cares about that too.
Preempt may care, but it's the hit you take to use neon in the kernel.
The neon register set shares with the FPU, so preempting during that
path means that the normal FPU register saving would corrupt the
already saved user FPU context - and even worse would result in the
kernel's crypto function register contents being leaked to userspace.
If you care about preempt deeply, the only solution is to avoid using
kernel mode neon.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RT] arm*: disable NEON in kernel mode
2017-12-01 17:31 ` Russell King - ARM Linux
@ 2017-12-01 17:39 ` Peter Zijlstra
0 siblings, 0 replies; 12+ messages in thread
From: Peter Zijlstra @ 2017-12-01 17:39 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Dec 01, 2017 at 05:31:20PM +0000, Russell King - ARM Linux wrote:
> On Fri, Dec 01, 2017 at 06:14:58PM +0100, Peter Zijlstra wrote:
> > Well, PREEMPT cares about that too.
>
> Preempt may care, but it's the hit you take to use neon in the kernel.
> The neon register set shares with the FPU, so preempting during that
> path means that the normal FPU register saving would corrupt the
> already saved user FPU context - and even worse would result in the
> kernel's crypto function register contents being leaked to userspace.
Same thing on x86.
> If you care about preempt deeply, the only solution is to avoid using
> kernel mode neon.
Not quite, you can write the code such that it drops out of neon mode
regularly to allow preemption.
So setup a crypto block, enter neon, transform the block, drop out of
neon, rinse repeat.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RT] arm*: disable NEON in kernel mode
2017-12-01 15:03 ` Ard Biesheuvel
@ 2017-12-01 17:58 ` Dave Martin
2017-12-01 18:08 ` Russell King - ARM Linux
0 siblings, 1 reply; 12+ messages in thread
From: Dave Martin @ 2017-12-01 17:58 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Dec 01, 2017 at 03:03:35PM +0000, Ard Biesheuvel wrote:
>
> l
> > On 1 Dec 2017, at 14:36, Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote:
> >
> >> On 2017-12-01 14:18:28 [+0000], Mark Rutland wrote:
> >> [Adding Ard, who wrote the NEON crypto code]
> >>
> >>> On Fri, Dec 01, 2017 at 02:45:06PM +0100, Sebastian Andrzej Siewior wrote:
> >>> +arm folks, to let you know
> >>>
> >>>> On 2017-12-01 11:43:32 [+0100], To linux-rt-users at vger.kernel.org wrote:
> >>>> NEON in kernel mode is used by the crypto algorithms and raid6 code.
> >>>> While the raid6 code looks okay, the crypto algorithms do not: NEON
> >>>> is enabled on first invocation and may allocate/free/map memory before
> >>>> the NEON mode is disabled again.
> >>
> >> Could you elaborate on why this is a problem?
> >>
> >> I guess this is because kernel_neon_{begin,end}() disable preemption?
> >>
> >> ... is this specific to RT?
> >
> > It is RT specific, yes. One thing are the unbounded latencies since
> > everything in this preempt_disable section can take time depending on
> > the size of the request.
> > The other thing is code like in
> > arch/arm64/crypto/aes-ce-ccm-glue.c:ccm_encrypt()
> >
> > where within this preempt_disable() section skcipher_walk_done() is
> > invoked. That function can allocate/free/map memory which is okay for
> > !RT but is not for RT. I tried to break those loops for x86 [0] and I
> > simply didn't had the time to do the same for ARM. I am aware that
> > store/restore of the NEON registers (as SSE and AVX) is expensive and
> > doing a lot of operations in one go is desired.
>
> I wouldn?t mind fixing the code instead. We never disable the neon,
> but only stack the contents of the registers the first time, and
> unstack them only before returning to userland (with the exception of
> nested neon use in softirq context). When this code was introduced,
> we always stacked/unstacked the whole register file eagerly every
> time.
+1, at least for arm64. I don't see a really compelling reason for
holding kernel-mode NEON around memory management now that we have a
strict save-once-restore-lazily model.
This may not work so well for arm though -- I haven't looked at that
code for a while.
If there is memory manamement in any core loop, you already lost
the performance battle, and an extra
kernel_neon_end()+kernel_neon_begin() may not be that catastrophic.
[...]
Cheers
---Dave
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RT] arm*: disable NEON in kernel mode
2017-12-01 17:58 ` Dave Martin
@ 2017-12-01 18:08 ` Russell King - ARM Linux
2017-12-01 18:24 ` Dave Martin
0 siblings, 1 reply; 12+ messages in thread
From: Russell King - ARM Linux @ 2017-12-01 18:08 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Dec 01, 2017 at 05:58:45PM +0000, Dave Martin wrote:
> +1, at least for arm64. I don't see a really compelling reason for
> holding kernel-mode NEON around memory management now that we have a
> strict save-once-restore-lazily model.
>
> This may not work so well for arm though -- I haven't looked at that
> code for a while.
>
>
> If there is memory manamement in any core loop, you already lost
> the performance battle, and an extra
> kernel_neon_end()+kernel_neon_begin() may not be that catastrophic.
Remember that we don't permit context switches while kernel neon is
in use on ARM - if there's any possibility of scheduling to occur,
the get_cpu() in kernel_neon_begin() should trigger a schedule-while-
atomic warning.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RT] arm*: disable NEON in kernel mode
2017-12-01 18:08 ` Russell King - ARM Linux
@ 2017-12-01 18:24 ` Dave Martin
2017-12-01 19:20 ` Ard Biesheuvel
2017-12-04 9:21 ` Sebastian Andrzej Siewior
0 siblings, 2 replies; 12+ messages in thread
From: Dave Martin @ 2017-12-01 18:24 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Dec 01, 2017 at 06:08:28PM +0000, Russell King - ARM Linux wrote:
> On Fri, Dec 01, 2017 at 05:58:45PM +0000, Dave Martin wrote:
> > +1, at least for arm64. I don't see a really compelling reason for
> > holding kernel-mode NEON around memory management now that we have a
> > strict save-once-restore-lazily model.
> >
> > This may not work so well for arm though -- I haven't looked at that
> > code for a while.
> >
> >
> > If there is memory manamement in any core loop, you already lost
> > the performance battle, and an extra
> > kernel_neon_end()+kernel_neon_begin() may not be that catastrophic.
>
> Remember that we don't permit context switches while kernel neon is
> in use on ARM - if there's any possibility of scheduling to occur,
> the get_cpu() in kernel_neon_begin() should trigger a schedule-while-
> atomic warning.
Agreed, and an arm64 the same is true. (Actually softirq is disabled
too, for tortuous reasons involving SVE.)
On 2017-12-01 11:43:32 [+0100], To linux-rt-users at vger.kernel.org wrote:
> NEON in kernel mode is used by the crypto algorithms and raid6 code.
> While the raid6 code looks okay, the crypto algorithms do not: NEON
> is enabled on first invocation and may allocate/free/map memory before
> the NEON mode is disabled again.
> This needs to be changed until it can be enabled.
> On ARM NEON in kernel mode can be simply disabled. on ARM64 it needs to
> stay on due to possible EFI callbacks so here I disable each algorithm.
[...]
> diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
> index 70c517aa4501..2a5f05b5a19a 100644
> --- a/arch/arm64/crypto/Kconfig
> +++ b/arch/arm64/crypto/Kconfig
> @@ -19,19 +19,19 @@ config CRYPTO_SHA512_ARM64
>
> config CRYPTO_SHA1_ARM64_CE
> tristate "SHA-1 digest algorithm (ARMv8 Crypto Extensions)"
> - depends on KERNEL_MODE_NEON
> + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> select CRYPTO_HASH
> select CRYPTO_SHA1
Sebastian, can you piont to where sha1 (say) hits this issue?
I wonder whether this is really a sign that some refactoring is needed.
Cheers
---Dave
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RT] arm*: disable NEON in kernel mode
2017-12-01 18:24 ` Dave Martin
@ 2017-12-01 19:20 ` Ard Biesheuvel
2017-12-04 9:21 ` Sebastian Andrzej Siewior
1 sibling, 0 replies; 12+ messages in thread
From: Ard Biesheuvel @ 2017-12-01 19:20 UTC (permalink / raw)
To: linux-arm-kernel
On 1 December 2017 at 18:24, Dave Martin <Dave.Martin@arm.com> wrote:
> On Fri, Dec 01, 2017 at 06:08:28PM +0000, Russell King - ARM Linux wrote:
>> On Fri, Dec 01, 2017 at 05:58:45PM +0000, Dave Martin wrote:
>> > +1, at least for arm64. I don't see a really compelling reason for
>> > holding kernel-mode NEON around memory management now that we have a
>> > strict save-once-restore-lazily model.
>> >
>> > This may not work so well for arm though -- I haven't looked at that
>> > code for a while.
>> >
>> >
>> > If there is memory manamement in any core loop, you already lost
>> > the performance battle, and an extra
>> > kernel_neon_end()+kernel_neon_begin() may not be that catastrophic.
>>
>> Remember that we don't permit context switches while kernel neon is
>> in use on ARM - if there's any possibility of scheduling to occur,
>> the get_cpu() in kernel_neon_begin() should trigger a schedule-while-
>> atomic warning.
>
> Agreed, and an arm64 the same is true. (Actually softirq is disabled
> too, for tortuous reasons involving SVE.)
>
> On 2017-12-01 11:43:32 [+0100], To linux-rt-users at vger.kernel.org wrote:
>> NEON in kernel mode is used by the crypto algorithms and raid6 code.
>> While the raid6 code looks okay, the crypto algorithms do not: NEON
>> is enabled on first invocation and may allocate/free/map memory before
>> the NEON mode is disabled again.
>> This needs to be changed until it can be enabled.
>> On ARM NEON in kernel mode can be simply disabled. on ARM64 it needs to
>> stay on due to possible EFI callbacks so here I disable each algorithm.
>
> [...]
>
>> diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
>> index 70c517aa4501..2a5f05b5a19a 100644
>> --- a/arch/arm64/crypto/Kconfig
>> +++ b/arch/arm64/crypto/Kconfig
>> @@ -19,19 +19,19 @@ config CRYPTO_SHA512_ARM64
>>
>> config CRYPTO_SHA1_ARM64_CE
>> tristate "SHA-1 digest algorithm (ARMv8 Crypto Extensions)"
>> - depends on KERNEL_MODE_NEON
>> + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
>> select CRYPTO_HASH
>> select CRYPTO_SHA1
>
> Sebastian, can you piont to where sha1 (say) hits this issue?
> I wonder whether this is really a sign that some refactoring is needed.
>
I don't think the SHA code is affected. It is primarily the block
ciphers, where the scatterwalk API offers some guarantees to the API
client (the NEON crypto driver) that data is presented in suitable
blocks. This may involve copying data from the scatterlist to a temp
buffer if any of the chunks are not blocksize aligned.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH RT] arm*: disable NEON in kernel mode
2017-12-01 18:24 ` Dave Martin
2017-12-01 19:20 ` Ard Biesheuvel
@ 2017-12-04 9:21 ` Sebastian Andrzej Siewior
1 sibling, 0 replies; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2017-12-04 9:21 UTC (permalink / raw)
To: linux-arm-kernel
On 2017-12-01 18:24:10 [+0000], Dave Martin wrote:
> > diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
> > index 70c517aa4501..2a5f05b5a19a 100644
> > --- a/arch/arm64/crypto/Kconfig
> > +++ b/arch/arm64/crypto/Kconfig
> > @@ -19,19 +19,19 @@ config CRYPTO_SHA512_ARM64
> >
> > config CRYPTO_SHA1_ARM64_CE
> > tristate "SHA-1 digest algorithm (ARMv8 Crypto Extensions)"
> > - depends on KERNEL_MODE_NEON
> > + depends on KERNEL_MODE_NEON && !PREEMPT_RT_BASE
> > select CRYPTO_HASH
> > select CRYPTO_SHA1
>
> Sebastian, can you piont to where sha1 (say) hits this issue?
> I wonder whether this is really a sign that some refactoring is needed.
I disabled all NEON in one go. Looking at this, it shouldn't be a issue
with the block-walk. The only thing that might be a problem is that it
can do multiple blocks in one go.
> Cheers
> ---Dave
Sebastian
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-12-04 9:21 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20171130142216.GB12606@linutronix.de>
[not found] ` <20171130143028.GA1351@linutronix.de>
[not found] ` <20171201104331.GB1612@linutronix.de>
2017-12-01 13:45 ` [PATCH RT] arm*: disable NEON in kernel mode Sebastian Andrzej Siewior
2017-12-01 14:18 ` Mark Rutland
2017-12-01 14:36 ` Sebastian Andrzej Siewior
2017-12-01 15:03 ` Ard Biesheuvel
2017-12-01 17:58 ` Dave Martin
2017-12-01 18:08 ` Russell King - ARM Linux
2017-12-01 18:24 ` Dave Martin
2017-12-01 19:20 ` Ard Biesheuvel
2017-12-04 9:21 ` Sebastian Andrzej Siewior
2017-12-01 17:14 ` Peter Zijlstra
2017-12-01 17:31 ` Russell King - ARM Linux
2017-12-01 17:39 ` Peter Zijlstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).