* may_use_simd on aarch64, chacha20 @ 2017-05-21 17:02 ` Jason A. Donenfeld 0 siblings, 0 replies; 10+ messages in thread From: Jason A. Donenfeld @ 2017-05-21 17:02 UTC (permalink / raw) To: Linux Crypto Mailing List, linux-arm-kernel, Ard Biesheuvel, Steffen Klassert Hi folks, I noticed that the ARM implementation [1] of chacha20 makes a check to may_use_simd(), but the ARM64 implementation [2] does not. Question 1: is this a bug, in which case I'll submit a patch shortly, or is this intentional? In case of the latter, could somebody explain the reasoning? On a similar note, the only ARM64 glue code that uses may_use_simd() is sha256; everything else does not. Shall I submit a substantial patch series to fix this up everywhere? Secondly, I noticed that may_use_simd() is essentially aliased to !in_interrupt(), since it uses the asm-generic variety. Question 2: Isn't this overkill? Couldn't we make an arm/arm64 variant of this that only checks in_irq()? Lastly, APIs like pcrypts and padata execute with bottom halves disabled, even though their actual execution environment is process context, via a workqueue. Thus, here, in_interrupt() will always be true, even though this is likely a place where we want to use simd. Question 3: is there something better that could be done? Thanks, Jason [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/crypto/chacha20-neon-glue.c#n67 [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/crypto/chacha20-neon-glue.c#n66 ^ permalink raw reply [flat|nested] 10+ messages in thread
* may_use_simd on aarch64, chacha20 @ 2017-05-21 17:02 ` Jason A. Donenfeld 0 siblings, 0 replies; 10+ messages in thread From: Jason A. Donenfeld @ 2017-05-21 17:02 UTC (permalink / raw) To: linux-arm-kernel Hi folks, I noticed that the ARM implementation [1] of chacha20 makes a check to may_use_simd(), but the ARM64 implementation [2] does not. Question 1: is this a bug, in which case I'll submit a patch shortly, or is this intentional? In case of the latter, could somebody explain the reasoning? On a similar note, the only ARM64 glue code that uses may_use_simd() is sha256; everything else does not. Shall I submit a substantial patch series to fix this up everywhere? Secondly, I noticed that may_use_simd() is essentially aliased to !in_interrupt(), since it uses the asm-generic variety. Question 2: Isn't this overkill? Couldn't we make an arm/arm64 variant of this that only checks in_irq()? Lastly, APIs like pcrypts and padata execute with bottom halves disabled, even though their actual execution environment is process context, via a workqueue. Thus, here, in_interrupt() will always be true, even though this is likely a place where we want to use simd. Question 3: is there something better that could be done? Thanks, Jason [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/crypto/chacha20-neon-glue.c#n67 [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm64/crypto/chacha20-neon-glue.c#n66 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: may_use_simd on aarch64, chacha20 2017-05-21 17:02 ` Jason A. Donenfeld @ 2017-05-21 20:55 ` Ard Biesheuvel -1 siblings, 0 replies; 10+ messages in thread From: Ard Biesheuvel @ 2017-05-21 20:55 UTC (permalink / raw) To: Jason A. Donenfeld Cc: Linux Crypto Mailing List, linux-arm-kernel, Steffen Klassert, Dave Martin (+ Dave) > On 21 May 2017, at 19:02, Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > Hi folks, > > I noticed that the ARM implementation [1] of chacha20 makes a check to > may_use_simd(), but the ARM64 implementation [2] does not. Question 1: > is this a bug, in which case I'll submit a patch shortly, or is this > intentional? In case of the latter, could somebody explain the > reasoning? This is intentional. arm64 supports kernel mode NEON in any context, whereas ARM only supports it in process context. This is due to the way lazy FP restore is implemented on ARM. However, we are about to change this on arm64 to only allow non-nested kernel mode NEON, similar to x86. This is necessary to support SVE. > On a similar note, the only ARM64 glue code that uses > may_use_simd() is sha256; everything else does not. Shall I submit a > substantial patch series to fix this up everywhere? > Currently, may_use_simd() is only used as a hint on arm64 whether it makes sense to offload crypto to process context. In the sha256 code, whose arm64 neon implementation is only marginally faster than scalar on some micro-architectures, it is used to prefer the scalar code in interrupt context, because the NEON code preserves/restores the NEON state of the interrupted context eagerly, which is costly. > Secondly, I noticed that may_use_simd() is essentially aliased to > !in_interrupt(), since it uses the asm-generic variety. Question 2: > Isn't this overkill? Couldn't we make an arm/arm64 variant of this > that only checks in_irq()? > No. ARM does not support kernel mode NEON in softirq context, and arm64 will soon have its own override that only allows non-nested use in softirq context. > Lastly, APIs like pcrypts and padata execute with bottom halves > disabled, even though their actual execution environment is process > context, via a workqueue. Thus, here, in_interrupt() will always be > true, even though this is likely a place where we want to use simd. > Question 3: is there something better that could be done? I guess we should switch to in_serving_softirq() instead. Thanks, Ard. ^ permalink raw reply [flat|nested] 10+ messages in thread
* may_use_simd on aarch64, chacha20 @ 2017-05-21 20:55 ` Ard Biesheuvel 0 siblings, 0 replies; 10+ messages in thread From: Ard Biesheuvel @ 2017-05-21 20:55 UTC (permalink / raw) To: linux-arm-kernel (+ Dave) > On 21 May 2017, at 19:02, Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > Hi folks, > > I noticed that the ARM implementation [1] of chacha20 makes a check to > may_use_simd(), but the ARM64 implementation [2] does not. Question 1: > is this a bug, in which case I'll submit a patch shortly, or is this > intentional? In case of the latter, could somebody explain the > reasoning? This is intentional. arm64 supports kernel mode NEON in any context, whereas ARM only supports it in process context. This is due to the way lazy FP restore is implemented on ARM. However, we are about to change this on arm64 to only allow non-nested kernel mode NEON, similar to x86. This is necessary to support SVE. > On a similar note, the only ARM64 glue code that uses > may_use_simd() is sha256; everything else does not. Shall I submit a > substantial patch series to fix this up everywhere? > Currently, may_use_simd() is only used as a hint on arm64 whether it makes sense to offload crypto to process context. In the sha256 code, whose arm64 neon implementation is only marginally faster than scalar on some micro-architectures, it is used to prefer the scalar code in interrupt context, because the NEON code preserves/restores the NEON state of the interrupted context eagerly, which is costly. > Secondly, I noticed that may_use_simd() is essentially aliased to > !in_interrupt(), since it uses the asm-generic variety. Question 2: > Isn't this overkill? Couldn't we make an arm/arm64 variant of this > that only checks in_irq()? > No. ARM does not support kernel mode NEON in softirq context, and arm64 will soon have its own override that only allows non-nested use in softirq context. > Lastly, APIs like pcrypts and padata execute with bottom halves > disabled, even though their actual execution environment is process > context, via a workqueue. Thus, here, in_interrupt() will always be > true, even though this is likely a place where we want to use simd. > Question 3: is there something better that could be done? I guess we should switch to in_serving_softirq() instead. Thanks, Ard. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: may_use_simd on aarch64, chacha20 2017-05-21 20:55 ` Ard Biesheuvel @ 2017-05-26 13:28 ` Dave Martin -1 siblings, 0 replies; 10+ messages in thread From: Dave Martin @ 2017-05-26 13:28 UTC (permalink / raw) To: Ard Biesheuvel Cc: Jason A. Donenfeld, Steffen Klassert, Linux Crypto Mailing List, linux-arm-kernel On Sun, May 21, 2017 at 10:55:20PM +0200, Ard Biesheuvel wrote: > (+ Dave) Apologies for the slow reply -- hopefully this is still useful. > > On 21 May 2017, at 19:02, Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > Hi folks, > > > > I noticed that the ARM implementation [1] of chacha20 makes a check to > > may_use_simd(), but the ARM64 implementation [2] does not. Question 1: > > is this a bug, in which case I'll submit a patch shortly, or is this > > intentional? In case of the latter, could somebody explain the > > reasoning? > > This is intentional. arm64 supports kernel mode NEON in any context, > whereas ARM only supports it in process context. This is due to the way > lazy FP restore is implemented on ARM. However, we are about to change > this on arm64 to only allow non-nested kernel mode NEON, similar to > x86. This is necessary to support SVE. > > > On a similar note, the only ARM64 glue code that uses > > may_use_simd() is sha256; everything else does not. Shall I submit a > > substantial patch series to fix this up everywhere? > > > > Currently, may_use_simd() is only used as a hint on arm64 whether it > makes sense to offload crypto to process context. In the sha256 code, > whose arm64 neon implementation is only marginally faster than scalar > on some micro-architectures, it is used to prefer the scalar code in > interrupt context, because the NEON code preserves/restores the NEON > state of the interrupted context eagerly, which is costly. > > > Secondly, I noticed that may_use_simd() is essentially aliased to > > !in_interrupt(), since it uses the asm-generic variety. Question 2: > > Isn't this overkill? Couldn't we make an arm/arm64 variant of this > > that only checks in_irq()? > > > > No. ARM does not support kernel mode NEON in softirq context, and > arm64 will soon have its own override that only allows non-nested use > in softirq context. > > > Lastly, APIs like pcrypts and padata execute with bottom halves > > disabled, even though their actual execution environment is process > > context, via a workqueue. Thus, here, in_interrupt() will always be > > true, even though this is likely a place where we want to use simd. > > Question 3: is there something better that could be done? > > I guess we should switch to in_serving_softirq() instead. For context, the arm64 kernel-mode NEON refactoring I've been working on [1] defines may_use_simd() to indicate whether calling kernel_neon_begin() is safe or not, rather then being a hint about whether it is desirable to do so. This changes the definition of may_use_simd() to something that is probably appropriate for your scenario. From process context without any outer kernel_neon_begin()...kernel_neon_end(), it should return true. In general, callers in any context should do something like: if (may_use_simd()) { kernel_neon_begin(); /* SIMD implementation */ kernel_neon_end(); } else { /* fallback implementation, or defer to another context */ } Even from task context where may_use_simd() should always return true and thus where the fallback path can be omitted, it may be a good idea to do something like if (!may_use_simd()) { WARN_ON(1); return -EBUSY; } ... or similar. If the semantics or name don't feel right, I'm happy to change them. Cheers ---Dave [1] lists.infradead.org/pipermail/linux-arm-kernel/2017-May/508901.html (arm64: neon: Remove support for nested or hardirq kernel-mode NEON) ^ permalink raw reply [flat|nested] 10+ messages in thread
* may_use_simd on aarch64, chacha20 @ 2017-05-26 13:28 ` Dave Martin 0 siblings, 0 replies; 10+ messages in thread From: Dave Martin @ 2017-05-26 13:28 UTC (permalink / raw) To: linux-arm-kernel On Sun, May 21, 2017 at 10:55:20PM +0200, Ard Biesheuvel wrote: > (+ Dave) Apologies for the slow reply -- hopefully this is still useful. > > On 21 May 2017, at 19:02, Jason A. Donenfeld <Jason@zx2c4.com> wrote: > > > > Hi folks, > > > > I noticed that the ARM implementation [1] of chacha20 makes a check to > > may_use_simd(), but the ARM64 implementation [2] does not. Question 1: > > is this a bug, in which case I'll submit a patch shortly, or is this > > intentional? In case of the latter, could somebody explain the > > reasoning? > > This is intentional. arm64 supports kernel mode NEON in any context, > whereas ARM only supports it in process context. This is due to the way > lazy FP restore is implemented on ARM. However, we are about to change > this on arm64 to only allow non-nested kernel mode NEON, similar to > x86. This is necessary to support SVE. > > > On a similar note, the only ARM64 glue code that uses > > may_use_simd() is sha256; everything else does not. Shall I submit a > > substantial patch series to fix this up everywhere? > > > > Currently, may_use_simd() is only used as a hint on arm64 whether it > makes sense to offload crypto to process context. In the sha256 code, > whose arm64 neon implementation is only marginally faster than scalar > on some micro-architectures, it is used to prefer the scalar code in > interrupt context, because the NEON code preserves/restores the NEON > state of the interrupted context eagerly, which is costly. > > > Secondly, I noticed that may_use_simd() is essentially aliased to > > !in_interrupt(), since it uses the asm-generic variety. Question 2: > > Isn't this overkill? Couldn't we make an arm/arm64 variant of this > > that only checks in_irq()? > > > > No. ARM does not support kernel mode NEON in softirq context, and > arm64 will soon have its own override that only allows non-nested use > in softirq context. > > > Lastly, APIs like pcrypts and padata execute with bottom halves > > disabled, even though their actual execution environment is process > > context, via a workqueue. Thus, here, in_interrupt() will always be > > true, even though this is likely a place where we want to use simd. > > Question 3: is there something better that could be done? > > I guess we should switch to in_serving_softirq() instead. For context, the arm64 kernel-mode NEON refactoring I've been working on [1] defines may_use_simd() to indicate whether calling kernel_neon_begin() is safe or not, rather then being a hint about whether it is desirable to do so. This changes the definition of may_use_simd() to something that is probably appropriate for your scenario. From process context without any outer kernel_neon_begin()...kernel_neon_end(), it should return true. In general, callers in any context should do something like: if (may_use_simd()) { kernel_neon_begin(); /* SIMD implementation */ kernel_neon_end(); } else { /* fallback implementation, or defer to another context */ } Even from task context where may_use_simd() should always return true and thus where the fallback path can be omitted, it may be a good idea to do something like if (!may_use_simd()) { WARN_ON(1); return -EBUSY; } ... or similar. If the semantics or name don't feel right, I'm happy to change them. Cheers ---Dave [1] lists.infradead.org/pipermail/linux-arm-kernel/2017-May/508901.html (arm64: neon: Remove support for nested or hardirq kernel-mode NEON) ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: may_use_simd on aarch64, chacha20 2017-05-26 13:28 ` Dave Martin @ 2017-05-26 17:44 ` Ard Biesheuvel -1 siblings, 0 replies; 10+ messages in thread From: Ard Biesheuvel @ 2017-05-26 17:44 UTC (permalink / raw) To: Dave Martin Cc: Jason A. Donenfeld, Steffen Klassert, Linux Crypto Mailing List, linux-arm-kernel On 26 May 2017 at 15:28, Dave Martin <Dave.Martin@arm.com> wrote: > On Sun, May 21, 2017 at 10:55:20PM +0200, Ard Biesheuvel wrote: >> (+ Dave) > > Apologies for the slow reply -- hopefully this is still useful. > >> > On 21 May 2017, at 19:02, Jason A. Donenfeld <Jason@zx2c4.com> wrote: >> > >> > Hi folks, >> > >> > I noticed that the ARM implementation [1] of chacha20 makes a check to >> > may_use_simd(), but the ARM64 implementation [2] does not. Question 1: >> > is this a bug, in which case I'll submit a patch shortly, or is this >> > intentional? In case of the latter, could somebody explain the >> > reasoning? >> >> This is intentional. arm64 supports kernel mode NEON in any context, >> whereas ARM only supports it in process context. This is due to the way >> lazy FP restore is implemented on ARM. However, we are about to change >> this on arm64 to only allow non-nested kernel mode NEON, similar to >> x86. This is necessary to support SVE. >> >> > On a similar note, the only ARM64 glue code that uses >> > may_use_simd() is sha256; everything else does not. Shall I submit a >> > substantial patch series to fix this up everywhere? >> > >> >> Currently, may_use_simd() is only used as a hint on arm64 whether it >> makes sense to offload crypto to process context. In the sha256 code, >> whose arm64 neon implementation is only marginally faster than scalar >> on some micro-architectures, it is used to prefer the scalar code in >> interrupt context, because the NEON code preserves/restores the NEON >> state of the interrupted context eagerly, which is costly. >> >> > Secondly, I noticed that may_use_simd() is essentially aliased to >> > !in_interrupt(), since it uses the asm-generic variety. Question 2: >> > Isn't this overkill? Couldn't we make an arm/arm64 variant of this >> > that only checks in_irq()? >> > >> >> No. ARM does not support kernel mode NEON in softirq context, and >> arm64 will soon have its own override that only allows non-nested use >> in softirq context. >> >> > Lastly, APIs like pcrypts and padata execute with bottom halves >> > disabled, even though their actual execution environment is process >> > context, via a workqueue. Thus, here, in_interrupt() will always be >> > true, even though this is likely a place where we want to use simd. >> > Question 3: is there something better that could be done? >> >> I guess we should switch to in_serving_softirq() instead. > > For context, the arm64 kernel-mode NEON refactoring I've been working > on [1] defines may_use_simd() to indicate whether calling > kernel_neon_begin() is safe or not, rather then being a hint about > whether it is desirable to do so. > I should mention again, perhaps redundantly, that may_use_simd() was not intended as a hint. It only became this way after we removed the limitation on arm64 that SIMD may only be used in process context. > This changes the definition of may_use_simd() to something that is > probably appropriate for your scenario. From process context without > any outer kernel_neon_begin()...kernel_neon_end(), it should return > true. > > In general, callers in any context should do something like: > > if (may_use_simd()) { > kernel_neon_begin(); > /* SIMD implementation */ > kernel_neon_end(); > } else { > /* fallback implementation, or defer to another context */ > } > > Even from task context where may_use_simd() should always return true > and thus where the fallback path can be omitted, it may be a good idea > to do something like > > if (!may_use_simd()) { > WARN_ON(1); > return -EBUSY; > } > > ... or similar. > I don't think it is generally a good idea to make inferences about whether may_use_simd() is going to return true or false in code that is not tightly coupled to the FP/SIMD arch code itself, although I do understand that there may be cases where it is cumbersome to provide a fallback, especially if you know it is never going to be used. ^ permalink raw reply [flat|nested] 10+ messages in thread
* may_use_simd on aarch64, chacha20 @ 2017-05-26 17:44 ` Ard Biesheuvel 0 siblings, 0 replies; 10+ messages in thread From: Ard Biesheuvel @ 2017-05-26 17:44 UTC (permalink / raw) To: linux-arm-kernel On 26 May 2017 at 15:28, Dave Martin <Dave.Martin@arm.com> wrote: > On Sun, May 21, 2017 at 10:55:20PM +0200, Ard Biesheuvel wrote: >> (+ Dave) > > Apologies for the slow reply -- hopefully this is still useful. > >> > On 21 May 2017, at 19:02, Jason A. Donenfeld <Jason@zx2c4.com> wrote: >> > >> > Hi folks, >> > >> > I noticed that the ARM implementation [1] of chacha20 makes a check to >> > may_use_simd(), but the ARM64 implementation [2] does not. Question 1: >> > is this a bug, in which case I'll submit a patch shortly, or is this >> > intentional? In case of the latter, could somebody explain the >> > reasoning? >> >> This is intentional. arm64 supports kernel mode NEON in any context, >> whereas ARM only supports it in process context. This is due to the way >> lazy FP restore is implemented on ARM. However, we are about to change >> this on arm64 to only allow non-nested kernel mode NEON, similar to >> x86. This is necessary to support SVE. >> >> > On a similar note, the only ARM64 glue code that uses >> > may_use_simd() is sha256; everything else does not. Shall I submit a >> > substantial patch series to fix this up everywhere? >> > >> >> Currently, may_use_simd() is only used as a hint on arm64 whether it >> makes sense to offload crypto to process context. In the sha256 code, >> whose arm64 neon implementation is only marginally faster than scalar >> on some micro-architectures, it is used to prefer the scalar code in >> interrupt context, because the NEON code preserves/restores the NEON >> state of the interrupted context eagerly, which is costly. >> >> > Secondly, I noticed that may_use_simd() is essentially aliased to >> > !in_interrupt(), since it uses the asm-generic variety. Question 2: >> > Isn't this overkill? Couldn't we make an arm/arm64 variant of this >> > that only checks in_irq()? >> > >> >> No. ARM does not support kernel mode NEON in softirq context, and >> arm64 will soon have its own override that only allows non-nested use >> in softirq context. >> >> > Lastly, APIs like pcrypts and padata execute with bottom halves >> > disabled, even though their actual execution environment is process >> > context, via a workqueue. Thus, here, in_interrupt() will always be >> > true, even though this is likely a place where we want to use simd. >> > Question 3: is there something better that could be done? >> >> I guess we should switch to in_serving_softirq() instead. > > For context, the arm64 kernel-mode NEON refactoring I've been working > on [1] defines may_use_simd() to indicate whether calling > kernel_neon_begin() is safe or not, rather then being a hint about > whether it is desirable to do so. > I should mention again, perhaps redundantly, that may_use_simd() was not intended as a hint. It only became this way after we removed the limitation on arm64 that SIMD may only be used in process context. > This changes the definition of may_use_simd() to something that is > probably appropriate for your scenario. From process context without > any outer kernel_neon_begin()...kernel_neon_end(), it should return > true. > > In general, callers in any context should do something like: > > if (may_use_simd()) { > kernel_neon_begin(); > /* SIMD implementation */ > kernel_neon_end(); > } else { > /* fallback implementation, or defer to another context */ > } > > Even from task context where may_use_simd() should always return true > and thus where the fallback path can be omitted, it may be a good idea > to do something like > > if (!may_use_simd()) { > WARN_ON(1); > return -EBUSY; > } > > ... or similar. > I don't think it is generally a good idea to make inferences about whether may_use_simd() is going to return true or false in code that is not tightly coupled to the FP/SIMD arch code itself, although I do understand that there may be cases where it is cumbersome to provide a fallback, especially if you know it is never going to be used. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: may_use_simd on aarch64, chacha20 2017-05-26 17:44 ` Ard Biesheuvel @ 2017-05-26 17:59 ` Dave Martin -1 siblings, 0 replies; 10+ messages in thread From: Dave Martin @ 2017-05-26 17:59 UTC (permalink / raw) To: Ard Biesheuvel Cc: Steffen Klassert, Jason A. Donenfeld, Linux Crypto Mailing List, linux-arm-kernel On Fri, May 26, 2017 at 07:44:46PM +0200, Ard Biesheuvel wrote: > On 26 May 2017 at 15:28, Dave Martin <Dave.Martin@arm.com> wrote: > > On Sun, May 21, 2017 at 10:55:20PM +0200, Ard Biesheuvel wrote: > >> (+ Dave) [...] > >> > Lastly, APIs like pcrypts and padata execute with bottom halves > >> > disabled, even though their actual execution environment is process > >> > context, via a workqueue. Thus, here, in_interrupt() will always be > >> > true, even though this is likely a place where we want to use simd. > >> > Question 3: is there something better that could be done? > >> > >> I guess we should switch to in_serving_softirq() instead. > > > > For context, the arm64 kernel-mode NEON refactoring I've been working > > on [1] defines may_use_simd() to indicate whether calling > > kernel_neon_begin() is safe or not, rather then being a hint about > > whether it is desirable to do so. > > > > I should mention again, perhaps redundantly, that may_use_simd() was > not intended as a hint. It only became this way after we removed the > limitation on arm64 that SIMD may only be used in process context. My current may_use_simd() is intended to have those non-hint semantics -- however, it feels a bit odd to have to include an extra header just for that, when clients of kernel-mode NEON are necessarily not portable. I don't have a very strong feeling either way, though. > > This changes the definition of may_use_simd() to something that is > > probably appropriate for your scenario. From process context without > > any outer kernel_neon_begin()...kernel_neon_end(), it should return > > true. > > > > In general, callers in any context should do something like: > > > > if (may_use_simd()) { > > kernel_neon_begin(); > > /* SIMD implementation */ > > kernel_neon_end(); > > } else { > > /* fallback implementation, or defer to another context */ > > } > > > > Even from task context where may_use_simd() should always return true > > and thus where the fallback path can be omitted, it may be a good idea > > to do something like > > > > if (!may_use_simd()) { > > WARN_ON(1); > > return -EBUSY; > > } > > > > ... or similar. > > > > I don't think it is generally a good idea to make inferences about > whether may_use_simd() is going to return true or false in code that > is not tightly coupled to the FP/SIMD arch code itself, although I do > understand that there may be cases where it is cumbersome to provide a > fallback, especially if you know it is never going to be used. Agreed. I don't think we should encourage this, but I thought it worth illustrating. Hopefully there are few if any real situations where this would be justified. Cheers ---Dave ^ permalink raw reply [flat|nested] 10+ messages in thread
* may_use_simd on aarch64, chacha20 @ 2017-05-26 17:59 ` Dave Martin 0 siblings, 0 replies; 10+ messages in thread From: Dave Martin @ 2017-05-26 17:59 UTC (permalink / raw) To: linux-arm-kernel On Fri, May 26, 2017 at 07:44:46PM +0200, Ard Biesheuvel wrote: > On 26 May 2017 at 15:28, Dave Martin <Dave.Martin@arm.com> wrote: > > On Sun, May 21, 2017 at 10:55:20PM +0200, Ard Biesheuvel wrote: > >> (+ Dave) [...] > >> > Lastly, APIs like pcrypts and padata execute with bottom halves > >> > disabled, even though their actual execution environment is process > >> > context, via a workqueue. Thus, here, in_interrupt() will always be > >> > true, even though this is likely a place where we want to use simd. > >> > Question 3: is there something better that could be done? > >> > >> I guess we should switch to in_serving_softirq() instead. > > > > For context, the arm64 kernel-mode NEON refactoring I've been working > > on [1] defines may_use_simd() to indicate whether calling > > kernel_neon_begin() is safe or not, rather then being a hint about > > whether it is desirable to do so. > > > > I should mention again, perhaps redundantly, that may_use_simd() was > not intended as a hint. It only became this way after we removed the > limitation on arm64 that SIMD may only be used in process context. My current may_use_simd() is intended to have those non-hint semantics -- however, it feels a bit odd to have to include an extra header just for that, when clients of kernel-mode NEON are necessarily not portable. I don't have a very strong feeling either way, though. > > This changes the definition of may_use_simd() to something that is > > probably appropriate for your scenario. From process context without > > any outer kernel_neon_begin()...kernel_neon_end(), it should return > > true. > > > > In general, callers in any context should do something like: > > > > if (may_use_simd()) { > > kernel_neon_begin(); > > /* SIMD implementation */ > > kernel_neon_end(); > > } else { > > /* fallback implementation, or defer to another context */ > > } > > > > Even from task context where may_use_simd() should always return true > > and thus where the fallback path can be omitted, it may be a good idea > > to do something like > > > > if (!may_use_simd()) { > > WARN_ON(1); > > return -EBUSY; > > } > > > > ... or similar. > > > > I don't think it is generally a good idea to make inferences about > whether may_use_simd() is going to return true or false in code that > is not tightly coupled to the FP/SIMD arch code itself, although I do > understand that there may be cases where it is cumbersome to provide a > fallback, especially if you know it is never going to be used. Agreed. I don't think we should encourage this, but I thought it worth illustrating. Hopefully there are few if any real situations where this would be justified. Cheers ---Dave ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-05-26 17:59 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-05-21 17:02 may_use_simd on aarch64, chacha20 Jason A. Donenfeld 2017-05-21 17:02 ` Jason A. Donenfeld 2017-05-21 20:55 ` Ard Biesheuvel 2017-05-21 20:55 ` Ard Biesheuvel 2017-05-26 13:28 ` Dave Martin 2017-05-26 13:28 ` Dave Martin 2017-05-26 17:44 ` Ard Biesheuvel 2017-05-26 17:44 ` Ard Biesheuvel 2017-05-26 17:59 ` Dave Martin 2017-05-26 17:59 ` Dave Martin
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.