Re: [PATCH v2 19/28] arm64/sve: ptrace and ELF coredump support

On Wed, Sep 06, 2017 at 04:21:50PM +0000, Okamoto, Takayuki wrote:
> Hi Dave,
> 
> I am an engineer of the postK computer from Fujitsu.
> 
> When I tried to read "max_vl" by ptrace with this patch on our local SVE 
> simulator, it was read as zero.
> I think the cause of this incident is that "max_vl" is set as "header->vl" 
> only on warning case in sve_init_header_from_task().
> "max_vl" should be set up also on normal case, like the following patch.
> 
> 
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -755,6 +755,8 @@ static void sve_init_header_from_task(struct user_sve_header *header,
> 
>         if (WARN_ON(!sve_vl_valid(sve_max_vl)))
>                 header->max_vl = header->vl;
> +       else
> +               header->max_vl = sve_max_vl;
> 
>         header->size = SVE_PT_SIZE(vq, header->flags);
>         header->max_size = SVE_PT_SIZE(sve_vq_from_vl(header->max_vl),

Hi, thanks for reporting this.

It looks like a refactoring mistake I made while removing BUG_ON()s,
which I missed in my testing.

Your fix looks correct and seems to work.  For stylistic reasons, I may
write it like this instead, but the effect should be the same:

	header->max_vl = sve_max_vl;
	if (WARN_ON(!sve_vl_valid(sve_max_vl))
		header->max_vl = header->vl;

Cheers
---Dave

> 
> 
> Best regards,
> Takayuki Okamoto
> 
> -----Original Message-----
> From: [email protected] [mailto:[email protected]] On Behalf Of Dave Martin
> Sent: Friday, September 1, 2017 2:01 AM
> To: [email protected]
> Cc: Catalin Marinas <[email protected]>; Will Deacon <[email protected]>; Ard Biesheuvel <[email protected]>; Alex Bennée <[email protected]>; Szabolcs Nagy <[email protected]>; Richard Sandiford <[email protected]>; [email protected]; [email protected]; [email protected]; [email protected]; Alan Hayward <[email protected]>; Yao Qi <[email protected]>; Oleg Nesterov <[email protected]>; Alexander Viro <[email protected]>
> Subject: [PATCH v2 19/28] arm64/sve: ptrace and ELF coredump support
> 

[...]

> @@ -702,6 +737,210 @@ 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;
> +	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);
> +
> +	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);
> +}

[...]