Re: [PATCH v12 07/17] riscv: Add vector struct and assembler definitions

From: Vineet Gupta <vineetg@rivosinc.com>
To: Chris Stillson <stillson@rivosinc.com>
Cc: Greentime Hu <greentime.hu@sifive.com>,
	Vincent Chen <vincent.chen@sifive.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Eric Biederman <ebiederm@xmission.com>,
	Kees Cook <keescook@chromium.org>,
	Anup Patel <anup@brainfault.org>,
	Atish Patra <atishp@atishpatra.org>,
	Oleg Nesterov <oleg@redhat.com>, Guo Ren <guoren@kernel.org>,
	Heinrich Schuchardt <heinrich.schuchardt@canonical.com>,
	Mayuresh Chitale <mchitale@ventanamicro.com>,
	Conor Dooley <conor.dooley@microchip.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Qinglin Pan <panqinglin2020@iscas.ac.cn>,
	Alexandre Ghiti <alexandre.ghiti@canonical.com>,
	Arnd Bergmann <arnd@arndb.de>, Andy Chiu <andy.chiu@sifive.com>,
	linux-riscv <linux-riscv@lists.infradead.org>,
	lkml <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v12 07/17] riscv: Add vector struct and assembler definitions
Date: Thu, 3 Nov 2022 22:13:04 -0700	[thread overview]
Message-ID: <784babf1-ca2b-0a7e-7bdb-c767304c3dd5@rivosinc.com> (raw)
In-Reply-To: <20220921214439.1491510-7-stillson@rivosinc.com>

On 9/21/22 14:43, Chris Stillson wrote:
> From: Greentime Hu <greentime.hu@sifive.com>
> 
> Add vector state context struct in struct thread and asm-offsets.c
> definitions.
> 
> The vector registers will be saved in datap pointer of __riscv_v_state. It
> will be dynamically allocated in kernel space. It will be put right after
> the __riscv_v_state data structure in user space.

"Vector state includes vector reg file and additional dynamic 
configuration CSRs. To handle variable sized reg file context (due to 
implementation defined ref size) and to enable lazy-allocation of this, 
there's datap which points to appropriate location on user/kernel mode 
stack as relevant..."

Something like above.

> diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
> index 19eedd4af4cd..95917a2b24f9 100644
> --- a/arch/riscv/include/asm/processor.h
> +++ b/arch/riscv/include/asm/processor.h
> @@ -39,6 +39,7 @@ struct thread_struct {
>   	unsigned long s[12];	/* s[0]: frame pointer */
>   	struct __riscv_d_ext_state fstate;
>   	unsigned long bad_cause;
> +	struct __riscv_v_state vstate;

I think this patch should be preparatory, don't wire up the vstate in 
thread_struct now. Only do it when the save/restore calls are wired up 
in low level code.

> +struct __riscv_v_state {
> +	unsigned long vstart;
> +	unsigned long vl;
> +	unsigned long vtype;
> +	unsigned long vcsr;
> +	void *datap;
> +	/*
> +	 * In signal handler, datap will be set a correct user stack offset
> +	 * and vector registers will be copied to the address of datap
> +	 * pointer.
> +	 *
> +	 * In ptrace syscall, datap will be set to zero and the vector
> +	 * registers will be copied to the address right after this
> +	 * structure.
> +	 */

Nice.

>   
> +	OFFSET(RISCV_V_STATE_VSTART, __riscv_v_state, vstart);
> +	OFFSET(RISCV_V_STATE_VL, __riscv_v_state, vl);
> +	OFFSET(RISCV_V_STATE_VTYPE, __riscv_v_state, vtype);
> +	OFFSET(RISCV_V_STATE_VCSR, __riscv_v_state, vcsr);
> +	OFFSET(RISCV_V_STATE_DATAP, __riscv_v_state, datap);
> +

Ok.

Also move the __vstate_{save,restore} functions from patch 5/17 here.