arm64: fpsimd: avoid restoring fpcr if the contents haven't changed
PATCH v2 1/1 arm64: fpsimd: avoid restoring fpcr if the contents haven't changed
Writing to the FPCR is commonly implemented as a self-synchronising operation in the CPU, so avoid writing to the register when the saved value matches that in the hardware already.
---
v1 -> v2 : Move FPCR restoration out into a macro and update partial
restore code too.
arch/arm64/include/asm/fpsimdmacros.h | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
arch/arm64/include/asm/fpsimdmacros.h +15 -2
@@ -40,6 +40,19 @@ str w\tmpnr, [\state, #16 * 2 + 4] .endm +.macro fpsimd_restore_fpcr state, tmp +/* +* Writes to fpcr may be self-synchronising, so avoid restoring +* the register if it hasn't changed. +*/ + mrs \tmp, fpcr + cmp \tmp, \state + b.eq 9999f + msr fpcr, \state +9999: +.endm + +/* Clobbers \state */ .macro fpsimd_restore state, tmpnr ldp q0, q1, [\state, #16 * 0] ldp q2, q3, [\state, #16 * 2]
@@ -60,7 +73,7 @@ ldr w\tmpnr, [\state, #16 * 2] msr fpsr, x\tmpnr ldr w\tmpnr, [\state, #16 * 2 + 4] - msr fpcr, x\tmpnr + fpsimd_restore_fpcr x\tmpnr, \state .endm .altmacro
@@ -84,7 +97,7 @@ .macro fpsimd_restore_partial state, tmpnr1, tmpnr2 ldp w\tmpnr1, w\tmpnr2, [\state] msr fpsr, x\tmpnr1 - msr fpcr, x\tmpnr2 + fpsimd_restore_fpcr x\tmpnr1, x\tmpnr2 adr x\tmpnr1, 0f ldr w\tmpnr2, [\state, #8] add \state, \state, x\tmpnr2, lsl #4
Signature · 1 line
2.0.0
PATCH v2 1/1 arm64: fpsimd: avoid restoring fpcr if the contents haven't changed
Quoted 49 lines · Will Deacon
Writing to the FPCR is commonly implemented as a self-synchronising operation in the CPU, so avoid writing to the register when the saved value matches that in the hardware already.
Cc Ard Biesheuvel <[email protected]>Reviewed-by Lorenzo Pieralisi <[email protected]>Signed-off-by Will Deacon <[email protected]>---
v1 -> v2 : Move FPCR restoration out into a macro and update partial
restore code too.arch/arm64/include/asm/fpsimdmacros.h | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)arch/arm64/include/asm/fpsimdmacros.h +14
-1@@ -40,6 +40,19 @@ str w\tmpnr, [\state, #16 * 2 + 4] .endm +.macro fpsimd_restore_fpcr state, tmp +/* +* Writes to fpcr may be self-synchronising, so avoid restoring +* the register if it hasn't changed. +*/ + mrs \tmp, fpcr + cmp \tmp, \state + b.eq 9999f + msr fpcr, \state +9999: +.endm + +/* Clobbers \state */ .macro fpsimd_restore state, tmpnr ldp q0, q1, [\state, #16 * 0] ldp q2, q3, [\state, #16 * 2]@@ -60,7 +73,7 @@ ldr w\tmpnr, [\state, #16 * 2] msr fpsr, x\tmpnr ldr w\tmpnr, [\state, #16 * 2 + 4] - msr fpcr, x\tmpnr + fpsimd_restore_fpcr x\tmpnr, \state .endm .altmacro
arch/arm64/include/asm/fpsimdmacros.h line 100
Quoted 5 lines
arch/arm64/include/asm/fpsimdmacros.h +1
-1@@ -84,7 +97,7 @@ .macro fpsimd_restore_partial state, tmpnr1, tmpnr2 ldp w\tmpnr1, w\tmpnr2, [\state] msr fpsr, x\tmpnr1 - msr fpcr, x\tmpnr2 + fpsimd_restore_fpcr x\tmpnr1, x\tmpnr2
Ehm, isn't this the wrong way around?
Quoted 6 lines
arch/arm64/include/asm/fpsimdmacros.h +0
-1adr x\tmpnr1, 0f ldr w\tmpnr2, [\state, #8] add \state, \state, x\tmpnr2, lsl #4 --2.0.0
PATCH v2 1/1 arm64: fpsimd: avoid restoring fpcr if the contents haven't changed
Quoted 4 lines · Ard Biesheuvel
Quoted 3 lines · Will Deacon
Writing to the FPCR is commonly implemented as a self-synchronising operation in the CPU, so avoid writing to the register when the saved value matches that in the hardware already.
[...]
Quoted 7 lines
arch/arm64/include/asm/fpsimdmacros.h line 100
Quoted 5 lines
.macro
fpsimd_restore_partialstate, tmpnr1, tmpnr2
ldp w\tmpnr1, w\tmpnr2, [\state]
msr fpsr, x\tmpnr1arch/arm64/include/asm/fpsimdmacros.h +1
-1- msr fpcr, x\tmpnr2 + fpsimd_restore_fpcr x\tmpnr1, x\tmpnr2Ehm, isn't this the wrong way around?
Yup, well spotted. I'll give the crypto selftests a run to test this path with the fix (I was just running paranoia in userspace before).
Will
PATCH v2 1/1 arm64: fpsimd: avoid restoring fpcr if the contents haven't changed
Quoted 14 lines · Will Deacon
Quoted 4 lines · Ard Biesheuvel
Quoted 3 lines · Will Deacon
Writing to the FPCR is commonly implemented as a self-synchronising operation in the CPU, so avoid writing to the register when the saved value matches that in the hardware already.
[...]
Quoted 6 lines
Quoted 5 lines
.macro
fpsimd_restore_partialstate, tmpnr1, tmpnr2
ldp w\tmpnr1, w\tmpnr2, [\state]
msr fpsr, x\tmpnr1(quoted diff) +1
-1- msr fpcr, x\tmpnr2 + fpsimd_restore_fpcr x\tmpnr1, x\tmpnr2
Quoted 5 lines
Quoted 1 line
Ehm, isn't this the wrong way around?
Yup, well spotted. I'll give the crypto selftests a run to test this path with the fix (I was just running paranoia in userspace before).
That won't cut it, I'm afraid. What I used is
arch/arm64/kernel/fpsimd.c +2 -0
@@ -221,6 +221,8 @@ void fpsimd_flush_task_state(struct task_struct *t) static DEFINE_PER_CPU(struct fpsimd_partial_state, hardirq_fpsimdstate); static DEFINE_PER_CPU(struct fpsimd_partial_state, softirq_fpsimdstate); +#define in_interrupt () (1) +/* * Kernel-side NEON support functions */
and run my [lengthy] userland benchmark while doing 'modprobe tcrypt mode=x' a number of times in another terminal (with only 1 CPU up). [with x=2 for SHA1 and x=6 for SHA256, for instance]
The first iteration of the interrupt context NEON series had an ARM counterpart which I was able to test using iperf over a WPA2/CCMP link (with the bit-sliced AES in CTR mode). No such luck [yet] on arm64, unfortunately, so I had to improvise a bit.
Signature · 1 line
Ard.