2023-08-03 19:19:28

by Mark Brown

[permalink] [raw]
Subject: [PATCH 0/3] arm64/ptrace: Fixes for more SME only streaming SVE cases

When we added support for streaming mode SVE there were several missed
cases around ptrace, address them. Some could be seen on systems which
do physically have SVE, others would only impact SME only systems. The
Fixes: tag is a bit conservative for the SME only cases but it seems
like the safest and clearest choice.

Signed-off-by: Mark Brown <[email protected]>
---
Mark Brown (3):
arm64/ptrace: Don't enable SVE when setting streaming SVE
arm64/fpsimd: Sync FPSIMD state with SVE for SME only systems
arm64/fpsimd: Sync and zero pad FPSIMD state for streaming SVE

arch/arm64/kernel/fpsimd.c | 7 ++++---
arch/arm64/kernel/ptrace.c | 8 +++++---
2 files changed, 9 insertions(+), 6 deletions(-)
---
base-commit: 5d0c230f1de8c7515b6567d9afba1f196fb4e2f4
change-id: 20230802-arm64-fix-ptrace-ssve-no-sve-915863197925

Best regards,
--
Mark Brown <[email protected]>



2023-08-03 19:43:11

by Mark Brown

[permalink] [raw]
Subject: [PATCH 3/3] arm64/fpsimd: Sync and zero pad FPSIMD state for streaming SVE

We have a function sve_sync_from_fpsimd_zeropad() which is used by the
ptrace code to update the SVE state when the user writes to the the
FPSIMD register set. Currently this checks that the task has SVE
enabled but this will miss updates for tasks which have streaming SVE
enabled if SVE has not been enabled for the thread, also do the
conversion if the task has streaming SVE enabled.

Fixes: e12310a0d30 ("arm64/sme: Implement ptrace support for streaming mode SVE registers")
Signed-off-by: Mark Brown <[email protected]>
Cc: [email protected]
---
arch/arm64/kernel/fpsimd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index b4afa0d147cc..8e8b853da616 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -835,7 +835,8 @@ void sve_sync_from_fpsimd_zeropad(struct task_struct *task)
void *sst = task->thread.sve_state;
struct user_fpsimd_state const *fst = &task->thread.uw.fpsimd_state;

- if (!test_tsk_thread_flag(task, TIF_SVE))
+ if (!test_tsk_thread_flag(task, TIF_SVE) &&
+ !thread_sm_enabled(&task->thread))
return;

vq = sve_vq_from_vl(thread_get_cur_vl(&task->thread));

--
2.30.2


2023-08-03 19:44:26

by Mark Brown

[permalink] [raw]
Subject: [PATCH 2/3] arm64/fpsimd: Sync FPSIMD state with SVE for SME only systems

Currently we guard FPSIMD/SVE state conversions with a check for the system
supporting SVE but SME only systems may need to sync streaming mode SVE
state so add a check for SME support too. These functions are only used
by the ptrace code.

Fixes: e12310a0d30 ("arm64/sme: Implement ptrace support for streaming mode SVE registers")
Signed-off-by: Mark Brown <[email protected]>
Cc: [email protected]
---
arch/arm64/kernel/fpsimd.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kernel/fpsimd.c b/arch/arm64/kernel/fpsimd.c
index 520b681a07bb..b4afa0d147cc 100644
--- a/arch/arm64/kernel/fpsimd.c
+++ b/arch/arm64/kernel/fpsimd.c
@@ -679,7 +679,7 @@ static void fpsimd_to_sve(struct task_struct *task)
void *sst = task->thread.sve_state;
struct user_fpsimd_state const *fst = &task->thread.uw.fpsimd_state;

- if (!system_supports_sve())
+ if (!system_supports_sve() && !system_supports_sme())
return;

vq = sve_vq_from_vl(thread_get_cur_vl(&task->thread));
@@ -705,7 +705,7 @@ static void sve_to_fpsimd(struct task_struct *task)
unsigned int i;
__uint128_t const *p;

- if (!system_supports_sve())
+ if (!system_supports_sve() && !system_supports_sme())
return;

vl = thread_get_cur_vl(&task->thread);

--
2.30.2


2023-08-03 19:45:01

by Mark Brown

[permalink] [raw]
Subject: [PATCH 1/3] arm64/ptrace: Don't enable SVE when setting streaming SVE

Systems which implement SME without also implementing SVE are
architecturally valid but were not initially supported by the kernel,
unfortunately we missed one issue in the ptrace code.

The SVE register setting code is shared between SVE and streaming mode
SVE. When we set full SVE register state we currently enable TIF_SVE
unconditionally, in the case where streaming SVE is being configured on a
system that supports vanilla SVE this is not an issue since we always
initialise enough state for both vector lengths but on a system which only
support SME it will result in us attempting to restore the SVE vector
length after having set streaming SVE registers.

Fix this by making the enabling of SVE conditional on setting SVE vector
state. If we set streaming SVE state and SVE was not already enabled this
will result in a SVE access trap on next use of normal SVE, this will cause
us to flush our register state but this is fine since the only way to
trigger a SVE access trap would be to exit streaming mode which will cause
the in register state to be flushed anyway.

Fixes: e12310a0d30 ("arm64/sme: Implement ptrace support for streaming mode SVE registers")
Signed-off-by: Mark Brown <[email protected]>
Cc: [email protected]
---
arch/arm64/kernel/ptrace.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index d7f4f0d1ae12..8a85dcc66597 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -932,11 +932,13 @@ static int sve_set_common(struct task_struct *target,
/*
* Ensure target->thread.sve_state is up to date with target's
* FPSIMD regs, so that a short copyin leaves trailing
- * registers unmodified. Always enable SVE even if going into
- * streaming mode.
+ * registers unmodified. Only enable SVE if we are
+ * configuring normal SVE, a system with streaming SVE may not
+ * have normal SVE.
*/
fpsimd_sync_to_sve(target);
- set_tsk_thread_flag(target, TIF_SVE);
+ if (type == ARM64_VEC_SVE)
+ set_tsk_thread_flag(target, TIF_SVE);
target->thread.fp_type = FP_STATE_SVE;

BUILD_BUG_ON(SVE_PT_SVE_OFFSET != sizeof(header));

--
2.30.2


2023-08-04 19:15:49

by Catalin Marinas

[permalink] [raw]
Subject: Re: [PATCH 0/3] arm64/ptrace: Fixes for more SME only streaming SVE cases

On Thu, 03 Aug 2023 19:33:20 +0100, Mark Brown wrote:
> When we added support for streaming mode SVE there were several missed
> cases around ptrace, address them. Some could be seen on systems which
> do physically have SVE, others would only impact SME only systems. The
> Fixes: tag is a bit conservative for the SME only cases but it seems
> like the safest and clearest choice.
>
>
> [...]

Applied to arm64 (for-next/fixes), thanks!

[1/3] arm64/ptrace: Don't enable SVE when setting streaming SVE
https://git.kernel.org/arm64/c/045aecdfcb2e
[2/3] arm64/fpsimd: Sync FPSIMD state with SVE for SME only systems
https://git.kernel.org/arm64/c/507ea5dd92d2
[3/3] arm64/fpsimd: Sync and zero pad FPSIMD state for streaming SVE
https://git.kernel.org/arm64/c/69af56ae56a4

--
Catalin