* [PATCH v2 1/3] arm64/sve: Split _sve_flush macro into separate Z and predicate flushes
2021-05-11 16:04 [PATCH v2 0/3] arm64/sve: Trivial optimisation for 128 bit SVE vectors Mark Brown
@ 2021-05-11 16:04 ` Mark Brown
2021-05-12 13:40 ` Dave Martin
2021-05-11 16:04 ` [PATCH v2 2/3] arm64/sve: Use the sve_flush macros in sve_load_from_fpsimd_state() Mark Brown
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Mark Brown @ 2021-05-11 16:04 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon; +Cc: Dave Martin, linux-arm-kernel, Mark Brown
Trivial refactoring to support further work, no change to generated code.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
arch/arm64/include/asm/fpsimdmacros.h | 4 +++-
arch/arm64/kernel/entry-fpsimd.S | 3 ++-
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/include/asm/fpsimdmacros.h b/arch/arm64/include/asm/fpsimdmacros.h
index a2563992d2dc..059204477ce6 100644
--- a/arch/arm64/include/asm/fpsimdmacros.h
+++ b/arch/arm64/include/asm/fpsimdmacros.h
@@ -213,8 +213,10 @@
mov v\nz\().16b, v\nz\().16b
.endm
-.macro sve_flush
+.macro sve_flush_z
_for n, 0, 31, _sve_flush_z \n
+.endm
+.macro sve_flush_p_ffr
_for n, 0, 15, _sve_pfalse \n
_sve_wrffr 0
.endm
diff --git a/arch/arm64/kernel/entry-fpsimd.S b/arch/arm64/kernel/entry-fpsimd.S
index 3ecec60d3295..7921d58427c2 100644
--- a/arch/arm64/kernel/entry-fpsimd.S
+++ b/arch/arm64/kernel/entry-fpsimd.S
@@ -72,7 +72,8 @@ SYM_FUNC_END(sve_load_from_fpsimd_state)
/* Zero all SVE registers but the first 128-bits of each vector */
SYM_FUNC_START(sve_flush_live)
- sve_flush
+ sve_flush_z
+ sve_flush_p_ffr
ret
SYM_FUNC_END(sve_flush_live)
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 1/3] arm64/sve: Split _sve_flush macro into separate Z and predicate flushes
2021-05-11 16:04 ` [PATCH v2 1/3] arm64/sve: Split _sve_flush macro into separate Z and predicate flushes Mark Brown
@ 2021-05-12 13:40 ` Dave Martin
0 siblings, 0 replies; 9+ messages in thread
From: Dave Martin @ 2021-05-12 13:40 UTC (permalink / raw)
To: Mark Brown; +Cc: Catalin Marinas, Will Deacon, linux-arm-kernel
On Tue, May 11, 2021 at 05:04:44PM +0100, Mark Brown wrote:
> Trivial refactoring to support further work, no change to generated code.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Dave Martin <Dave.Martin@arm.com>
> ---
> arch/arm64/include/asm/fpsimdmacros.h | 4 +++-
> arch/arm64/kernel/entry-fpsimd.S | 3 ++-
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/include/asm/fpsimdmacros.h b/arch/arm64/include/asm/fpsimdmacros.h
> index a2563992d2dc..059204477ce6 100644
> --- a/arch/arm64/include/asm/fpsimdmacros.h
> +++ b/arch/arm64/include/asm/fpsimdmacros.h
> @@ -213,8 +213,10 @@
> mov v\nz\().16b, v\nz\().16b
> .endm
>
> -.macro sve_flush
> +.macro sve_flush_z
> _for n, 0, 31, _sve_flush_z \n
> +.endm
> +.macro sve_flush_p_ffr
> _for n, 0, 15, _sve_pfalse \n
> _sve_wrffr 0
> .endm
> diff --git a/arch/arm64/kernel/entry-fpsimd.S b/arch/arm64/kernel/entry-fpsimd.S
> index 3ecec60d3295..7921d58427c2 100644
> --- a/arch/arm64/kernel/entry-fpsimd.S
> +++ b/arch/arm64/kernel/entry-fpsimd.S
> @@ -72,7 +72,8 @@ SYM_FUNC_END(sve_load_from_fpsimd_state)
>
> /* Zero all SVE registers but the first 128-bits of each vector */
> SYM_FUNC_START(sve_flush_live)
> - sve_flush
> + sve_flush_z
> + sve_flush_p_ffr
> ret
> SYM_FUNC_END(sve_flush_live)
>
> --
> 2.20.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 2/3] arm64/sve: Use the sve_flush macros in sve_load_from_fpsimd_state()
2021-05-11 16:04 [PATCH v2 0/3] arm64/sve: Trivial optimisation for 128 bit SVE vectors Mark Brown
2021-05-11 16:04 ` [PATCH v2 1/3] arm64/sve: Split _sve_flush macro into separate Z and predicate flushes Mark Brown
@ 2021-05-11 16:04 ` Mark Brown
2021-05-12 13:40 ` Dave Martin
2021-05-11 16:04 ` [PATCH v2 3/3] arm64/sve: Skip flushing Z registers with 128 bit vectors Mark Brown
2021-05-12 16:16 ` [PATCH v2 0/3] arm64/sve: Trivial optimisation for 128 bit SVE vectors Catalin Marinas
3 siblings, 1 reply; 9+ messages in thread
From: Mark Brown @ 2021-05-11 16:04 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon; +Cc: Dave Martin, linux-arm-kernel, Mark Brown
This makes the code a bit clearer and as a result we can also make the
indentation more normal, there is no change to the generated code.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
arch/arm64/kernel/entry-fpsimd.S | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/kernel/entry-fpsimd.S b/arch/arm64/kernel/entry-fpsimd.S
index 7921d58427c2..dd8382e5ce82 100644
--- a/arch/arm64/kernel/entry-fpsimd.S
+++ b/arch/arm64/kernel/entry-fpsimd.S
@@ -63,11 +63,10 @@ SYM_FUNC_END(sve_set_vq)
* and the rest zeroed. All the other SVE registers will be zeroed.
*/
SYM_FUNC_START(sve_load_from_fpsimd_state)
- sve_load_vq x1, x2, x3
- fpsimd_restore x0, 8
- _for n, 0, 15, _sve_pfalse \n
- _sve_wrffr 0
- ret
+ sve_load_vq x1, x2, x3
+ fpsimd_restore x0, 8
+ sve_flush_p_ffr
+ ret
SYM_FUNC_END(sve_load_from_fpsimd_state)
/* Zero all SVE registers but the first 128-bits of each vector */
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v2 2/3] arm64/sve: Use the sve_flush macros in sve_load_from_fpsimd_state()
2021-05-11 16:04 ` [PATCH v2 2/3] arm64/sve: Use the sve_flush macros in sve_load_from_fpsimd_state() Mark Brown
@ 2021-05-12 13:40 ` Dave Martin
0 siblings, 0 replies; 9+ messages in thread
From: Dave Martin @ 2021-05-12 13:40 UTC (permalink / raw)
To: Mark Brown; +Cc: Catalin Marinas, Will Deacon, linux-arm-kernel
On Tue, May 11, 2021 at 05:04:45PM +0100, Mark Brown wrote:
> This makes the code a bit clearer and as a result we can also make the
> indentation more normal, there is no change to the generated code.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
Reviewed-by: Dave Martin <Dave.Martin@arm.com>
> ---
> arch/arm64/kernel/entry-fpsimd.S | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/kernel/entry-fpsimd.S b/arch/arm64/kernel/entry-fpsimd.S
> index 7921d58427c2..dd8382e5ce82 100644
> --- a/arch/arm64/kernel/entry-fpsimd.S
> +++ b/arch/arm64/kernel/entry-fpsimd.S
> @@ -63,11 +63,10 @@ SYM_FUNC_END(sve_set_vq)
> * and the rest zeroed. All the other SVE registers will be zeroed.
> */
> SYM_FUNC_START(sve_load_from_fpsimd_state)
> - sve_load_vq x1, x2, x3
> - fpsimd_restore x0, 8
> - _for n, 0, 15, _sve_pfalse \n
> - _sve_wrffr 0
> - ret
> + sve_load_vq x1, x2, x3
> + fpsimd_restore x0, 8
> + sve_flush_p_ffr
> + ret
> SYM_FUNC_END(sve_load_from_fpsimd_state)
>
> /* Zero all SVE registers but the first 128-bits of each vector */
> --
> 2.20.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2 3/3] arm64/sve: Skip flushing Z registers with 128 bit vectors
2021-05-11 16:04 [PATCH v2 0/3] arm64/sve: Trivial optimisation for 128 bit SVE vectors Mark Brown
2021-05-11 16:04 ` [PATCH v2 1/3] arm64/sve: Split _sve_flush macro into separate Z and predicate flushes Mark Brown
2021-05-11 16:04 ` [PATCH v2 2/3] arm64/sve: Use the sve_flush macros in sve_load_from_fpsimd_state() Mark Brown
@ 2021-05-11 16:04 ` Mark Brown
2021-05-12 13:49 ` Dave Martin
2021-05-12 16:16 ` [PATCH v2 0/3] arm64/sve: Trivial optimisation for 128 bit SVE vectors Catalin Marinas
3 siblings, 1 reply; 9+ messages in thread
From: Mark Brown @ 2021-05-11 16:04 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon; +Cc: Dave Martin, linux-arm-kernel, Mark Brown
When the SVE vector length is 128 bits then there are no bits in the Z
registers which are not shared with the V registers so we can skip them
when zeroing state not shared with FPSIMD, this results in a minor
performance improvement.
Signed-off-by: Mark Brown <broonie@kernel.org>
---
arch/arm64/include/asm/fpsimd.h | 2 +-
arch/arm64/kernel/entry-fpsimd.S | 9 +++++++--
arch/arm64/kernel/fpsimd.c | 6 ++++--
3 files changed, 12 insertions(+), 5 deletions(-)
diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
index 2599504674b5..c072161d5c65 100644
--- a/arch/arm64/include/asm/fpsimd.h
+++ b/arch/arm64/include/asm/fpsimd.h
@@ -69,7 +69,7 @@ static inline void *sve_pffr(struct thread_struct *thread)
extern void sve_save_state(void *state, u32 *pfpsr);
extern void sve_load_state(void const *state, u32 const *pfpsr,
unsigned long vq_minus_1);
-extern void sve_flush_live(void);
+extern void sve_flush_live(unsigned long vq_minus_1);
extern void sve_load_from_fpsimd_state(struct user_fpsimd_state const *state,
unsigned long vq_minus_1);
extern unsigned int sve_get_vl(void);
diff --git a/arch/arm64/kernel/entry-fpsimd.S b/arch/arm64/kernel/entry-fpsimd.S
index dd8382e5ce82..87ef25836963 100644
--- a/arch/arm64/kernel/entry-fpsimd.S
+++ b/arch/arm64/kernel/entry-fpsimd.S
@@ -69,10 +69,15 @@ SYM_FUNC_START(sve_load_from_fpsimd_state)
ret
SYM_FUNC_END(sve_load_from_fpsimd_state)
-/* Zero all SVE registers but the first 128-bits of each vector */
+/*
+ * Zero all SVE registers but the first 128-bits of each vector
+ *
+ * x0 = VQ - 1
+ */
SYM_FUNC_START(sve_flush_live)
+ cbz x0, 1f // A VQ-1 of 0 is 128 bits so no extra Z state
sve_flush_z
- sve_flush_p_ffr
+1: sve_flush_p_ffr
ret
SYM_FUNC_END(sve_flush_live)
diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index ad3dd34a83cf..e57b23f95284 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -957,8 +957,10 @@ void do_sve_acc(unsigned int esr, struct pt_regs *regs)
* disabling the trap, otherwise update our in-memory copy.
*/
if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) {
- sve_set_vq(sve_vq_from_vl(current->thread.sve_vl) - 1);
- sve_flush_live();
+ unsigned long vq_minus_one =
+ sve_vq_from_vl(current->thread.sve_vl) - 1;
+ sve_set_vq(vq_minus_one);
+ sve_flush_live(vq_minus_one);
fpsimd_bind_task_to_cpu();
} else {
fpsimd_to_sve(current);
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 3/3] arm64/sve: Skip flushing Z registers with 128 bit vectors
2021-05-11 16:04 ` [PATCH v2 3/3] arm64/sve: Skip flushing Z registers with 128 bit vectors Mark Brown
@ 2021-05-12 13:49 ` Dave Martin
2021-05-12 14:22 ` Mark Brown
0 siblings, 1 reply; 9+ messages in thread
From: Dave Martin @ 2021-05-12 13:49 UTC (permalink / raw)
To: Mark Brown; +Cc: Catalin Marinas, Will Deacon, linux-arm-kernel
On Tue, May 11, 2021 at 05:04:46PM +0100, Mark Brown wrote:
> When the SVE vector length is 128 bits then there are no bits in the Z
> registers which are not shared with the V registers so we can skip them
> when zeroing state not shared with FPSIMD, this results in a minor
> performance improvement.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> arch/arm64/include/asm/fpsimd.h | 2 +-
> arch/arm64/kernel/entry-fpsimd.S | 9 +++++++--
> arch/arm64/kernel/fpsimd.c | 6 ++++--
> 3 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm64/include/asm/fpsimd.h b/arch/arm64/include/asm/fpsimd.h
> index 2599504674b5..c072161d5c65 100644
> --- a/arch/arm64/include/asm/fpsimd.h
> +++ b/arch/arm64/include/asm/fpsimd.h
> @@ -69,7 +69,7 @@ static inline void *sve_pffr(struct thread_struct *thread)
> extern void sve_save_state(void *state, u32 *pfpsr);
> extern void sve_load_state(void const *state, u32 const *pfpsr,
> unsigned long vq_minus_1);
> -extern void sve_flush_live(void);
> +extern void sve_flush_live(unsigned long vq_minus_1);
> extern void sve_load_from_fpsimd_state(struct user_fpsimd_state const *state,
> unsigned long vq_minus_1);
> extern unsigned int sve_get_vl(void);
> diff --git a/arch/arm64/kernel/entry-fpsimd.S b/arch/arm64/kernel/entry-fpsimd.S
> index dd8382e5ce82..87ef25836963 100644
> --- a/arch/arm64/kernel/entry-fpsimd.S
> +++ b/arch/arm64/kernel/entry-fpsimd.S
> @@ -69,10 +69,15 @@ SYM_FUNC_START(sve_load_from_fpsimd_state)
> ret
> SYM_FUNC_END(sve_load_from_fpsimd_state)
>
> -/* Zero all SVE registers but the first 128-bits of each vector */
> +/*
> + * Zero all SVE registers but the first 128-bits of each vector
> + *
> + * x0 = VQ - 1
This does require that ZCR_EL1.LEN has already been set to match x0, and
is not changed again before entering userspace.
It would be a good idea to at least describe this in a comment so that
this doesn't get forgotten later on, but there's a limit to how
foolproof this low-level backend code needs to be...
> + */
> SYM_FUNC_START(sve_flush_live)
> + cbz x0, 1f // A VQ-1 of 0 is 128 bits so no extra Z state
> sve_flush_z
> - sve_flush_p_ffr
> +1: sve_flush_p_ffr
> ret
> SYM_FUNC_END(sve_flush_live)
>
> diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
> index ad3dd34a83cf..e57b23f95284 100644
> --- a/arch/arm64/kernel/fpsimd.c
> +++ b/arch/arm64/kernel/fpsimd.c
> @@ -957,8 +957,10 @@ void do_sve_acc(unsigned int esr, struct pt_regs *regs)
> * disabling the trap, otherwise update our in-memory copy.
> */
> if (!test_thread_flag(TIF_FOREIGN_FPSTATE)) {
> - sve_set_vq(sve_vq_from_vl(current->thread.sve_vl) - 1);
> - sve_flush_live();
> + unsigned long vq_minus_one =
> + sve_vq_from_vl(current->thread.sve_vl) - 1;
> + sve_set_vq(vq_minus_one);
> + sve_flush_live(vq_minus_one);
> fpsimd_bind_task_to_cpu();
> } else {
> fpsimd_to_sve(current);
> --
> 2.20.1
With a comment added as outlined above,
Reviewed-by: Dave Martin <Dave.Martin@arm.com>
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 3/3] arm64/sve: Skip flushing Z registers with 128 bit vectors
2021-05-12 13:49 ` Dave Martin
@ 2021-05-12 14:22 ` Mark Brown
0 siblings, 0 replies; 9+ messages in thread
From: Mark Brown @ 2021-05-12 14:22 UTC (permalink / raw)
To: Dave Martin; +Cc: Catalin Marinas, Will Deacon, linux-arm-kernel
[-- Attachment #1.1: Type: text/plain, Size: 825 bytes --]
On Wed, May 12, 2021 at 02:49:09PM +0100, Dave Martin wrote:
> On Tue, May 11, 2021 at 05:04:46PM +0100, Mark Brown wrote:
> > +/*
> > + * Zero all SVE registers but the first 128-bits of each vector
> > + *
> > + * x0 = VQ - 1
> This does require that ZCR_EL1.LEN has already been set to match x0, and
> is not changed again before entering userspace.
> It would be a good idea to at least describe this in a comment so that
> this doesn't get forgotten later on, but there's a limit to how
> foolproof this low-level backend code needs to be...
Similar concerns exist for huge chunks of the existing SVE code (eg, the
no further changes on vector length constraint is pretty much universal
and is I'd say largely more of a "make sure you handle the register
contents" thing on anything that changes the vector length).
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2 0/3] arm64/sve: Trivial optimisation for 128 bit SVE vectors
2021-05-11 16:04 [PATCH v2 0/3] arm64/sve: Trivial optimisation for 128 bit SVE vectors Mark Brown
` (2 preceding siblings ...)
2021-05-11 16:04 ` [PATCH v2 3/3] arm64/sve: Skip flushing Z registers with 128 bit vectors Mark Brown
@ 2021-05-12 16:16 ` Catalin Marinas
3 siblings, 0 replies; 9+ messages in thread
From: Catalin Marinas @ 2021-05-12 16:16 UTC (permalink / raw)
To: Mark Brown; +Cc: Will Deacon, Dave Martin, linux-arm-kernel
On Tue, May 11, 2021 at 05:04:43PM +0100, Mark Brown wrote:
> This series is a combination of factoring out some duplicated code and a
> very minor optimisation to the performance of handling converting FPSIMD
> state to SVE in the live registers for 128 bit SVE vectors.
>
> v2:
> - Combine P and FFR flushing into a single macro.
>
> Mark Brown (3):
> arm64/sve: Split _sve_flush macro into separate Z and predicate
> flushes
> arm64/sve: Use the sve_flush macros in sve_load_from_fpsimd_state()
> arm64/sve: Skip flushing Z registers with 128 bit vectors
The series makes sense to me:
Acked-by: Catalin Marinas <catalin.marinas@arm.com>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 9+ messages in thread