On Tue, Oct 10, 2017 at 07:38:36PM +0100, Dave P Martin wrote:
> @@ -702,6 +737,211 @@ static int system_call_set(struct task_struct *target,
> return ret;
> }
>
> +#ifdef CONFIG_ARM64_SVE
> +
> +static void sve_init_header_from_task(struct user_sve_header *header,
> + struct task_struct *target)
> +{
> + unsigned int vq;
> +
> + memset(header, 0, sizeof(*header));
> +
> + header->flags = test_tsk_thread_flag(target, TIF_SVE) ?
> + SVE_PT_REGS_SVE : SVE_PT_REGS_FPSIMD;
For PTRACE_SYSCALL, we may or may not have TIF_SVE depending on what
happened with the target. Just a thought: shall we clear TIF_SVE (and
sync it to fpsimd) in syscall_trace_enter()?
> + if (test_tsk_thread_flag(target, TIF_SVE_VL_INHERIT))
> + header->flags |= SVE_PT_VL_INHERIT;
> +
> + header->vl = target->thread.sve_vl;
> + vq = sve_vq_from_vl(header->vl);
> +
> + header->max_vl = sve_max_vl;
> + if (WARN_ON(!sve_vl_valid(sve_max_vl)))
> + header->max_vl = header->vl;
> +
> + header->size = SVE_PT_SIZE(vq, header->flags);
> + header->max_size = SVE_PT_SIZE(sve_vq_from_vl(header->max_vl),
> + SVE_PT_REGS_SVE);
> +}
[...]
> +static int sve_set(struct task_struct *target,
> + const struct user_regset *regset,
> + unsigned int pos, unsigned int count,
> + const void *kbuf, const void __user *ubuf)
> +{
> + int ret;
> + struct user_sve_header header;
> + unsigned int vq;
> + unsigned long start, end;
> +
> + if (!system_supports_sve())
> + return -EINVAL;
> +
> + /* Header */
> + if (count < sizeof(header))
> + return -EINVAL;
> + ret = user_regset_copyin(&pos, &count, &kbuf, &ubuf, &header,
> + 0, sizeof(header));
> + if (ret)
> + goto out;
> +
> + /*
> + * Apart from PT_SVE_REGS_MASK, all PT_SVE_* flags are consumed by
> + * sve_set_vector_length(), which will also validate them for us:
> + */
> + ret = sve_set_vector_length(target, header.vl,
> + ((unsigned long)header.flags & ~SVE_PT_REGS_MASK) << 16);
> + if (ret)
> + goto out;
> +
> + /* Actual VL set may be less than the user asked for: */
> + vq = sve_vq_from_vl(target->thread.sve_vl);
> +
> + /* Registers: FPSIMD-only case */
> +
> + BUILD_BUG_ON(SVE_PT_FPSIMD_OFFSET != sizeof(header));
> + if ((header.flags & SVE_PT_REGS_MASK) == SVE_PT_REGS_FPSIMD) {
> + sve_sync_to_fpsimd(target);
> +
> + ret = __fpr_set(target, regset, pos, count, kbuf, ubuf,
> + SVE_PT_FPSIMD_OFFSET);
> + clear_tsk_thread_flag(target, TIF_SVE);
> + goto out;
> + }
__fpr_set() already calls sve_sync_to_fpsimd(). Anyway, do you actually
need this since we are going to override the FPSIMD state anyway here.
> +
> + /* Otherwise: full SVE case */
> +
> + /*
> + * If setting a different VL from the requested VL and there is
> + * register data, the data layout will be wrong: don't even
> + * try to set the registers in this case.
> + */
> + if (count && vq != sve_vq_from_vl(header.vl)) {
> + ret = -EIO;
> + goto out;
> + }
> +
> + sve_alloc(target);
> + fpsimd_sync_to_sve(target);
Similarly here, it's a full SVE case, so we are going to override it
anyway.
> + set_tsk_thread_flag(target, TIF_SVE);
This has the side-effect of enabling TIF_SVE even for PTRACE_SYSCALL
which may be cleared in some circumstances. It may not be an issue
though.
--
Catalin
