Received-SPF: pass (google.com: domain of linux-kernel+bounces-39059-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Thu, 25 Jan 2024 09:54:24 -0800
From: Deepak Gupta <debug@rivosinc.com>
To: Stefan O'Rear <sorear@fastmail.com>
Cc: rick.p.edgecombe@intel.com, broonie@kernel.org, Szabolcs.Nagy@arm.com,
	"kito.cheng@sifive.com" <kito.cheng@sifive.com>,
	Kees Cook <keescook@chromium.org>,
	Andrew Jones <ajones@ventanamicro.com>, paul.walmsley@sifive.com,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Conor Dooley <conor.dooley@microchip.com>, cleger@rivosinc.com,
	Atish Patra <atishp@atishpatra.org>,
	Alexandre Ghiti <alex@ghiti.fr>,
	=?iso-8859-1?Q?Bj=F6rn_T=F6pel?= <bjorn@rivosinc.com>,
	Alexandre Ghiti <alexghiti@rivosinc.com>,
	Jonathan Corbet <corbet@lwn.net>, Albert Ou <aou@eecs.berkeley.edu>,
	oleg@redhat.com, akpm@linux-foundation.org, arnd@arndb.de,
	"Eric W. Biederman" <ebiederm@xmission.com>, shuah@kernel.org,
	Christian Brauner <brauner@kernel.org>, guoren <guoren@kernel.org>,
	samitolvanen@google.com, Evan Green <evan@rivosinc.com>,
	xiao.w.wang@intel.com, Anup Patel <apatel@ventanamicro.com>,
	mchitale@ventanamicro.com, waylingii@gmail.com,
	greentime.hu@sifive.com, Heiko Stuebner <heiko@sntech.de>,
	Jisheng Zhang <jszhang@kernel.org>, shikemeng@huaweicloud.com,
	david@redhat.com, Charlie Jenkins <charlie@rivosinc.com>,
	panqinglin2020@iscas.ac.cn, willy@infradead.org,
	Vincent Chen <vincent.chen@sifive.com>,
	Andy Chiu <andy.chiu@sifive.com>, Greg Ungerer <gerg@kernel.org>,
	jeeheng.sia@starfivetech.com, mason.huo@starfivetech.com,
	ancientmodern4@gmail.com, mathis.salmen@matsal.de,
	cuiyunhui@bytedance.com, bhe@redhat.com, ruscur@russell.cc,
	bgray@linux.ibm.com, alx@kernel.org, baruch@tkos.co.il,
	zhangqing@loongson.cn, Catalin Marinas <catalin.marinas@arm.com>,
	revest@chromium.org, josh@joshtriplett.org, joey.gouly@arm.com,
	shr@devkernel.io, omosnace@redhat.com, ojeda@kernel.org,
	jhubbard@nvidia.com, linux-doc@vger.kernel.org,
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, linux-arch@vger.kernel.org,
	linux-kselftest@vger.kernel.org
Subject: Re: [RFC PATCH v1 02/28] riscv: envcfg save and restore on trap
 entry/exit
Message-ID: <ZbKgUNR+Em55Moaw@debug.ba.rivosinc.com>
References: <20240125062739.1339782-1-debug@rivosinc.com>
 <20240125062739.1339782-3-debug@rivosinc.com>
 <23d023c0-27cf-44fa-be0a-000d1534ef86@app.fastmail.com>
 <ZbKVutBWoelt33GM@debug.ba.rivosinc.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <ZbKVutBWoelt33GM@debug.ba.rivosinc.com>

On Thu, Jan 25, 2024 at 09:09:14AM -0800, Deepak Gupta wrote:
>On Thu, Jan 25, 2024 at 02:19:29AM -0500, Stefan O'Rear wrote:
>>On Thu, Jan 25, 2024, at 1:21 AM, debug@rivosinc.com wrote:
>>>From: Deepak Gupta <debug@rivosinc.com>
>>>
>>>envcfg CSR defines enabling bits for cache management instructions and soon
>>>will control enabling for control flow integrity and pointer masking features.
>>>
>>>Control flow integrity enabling for forward cfi and backward cfi is controlled
>>>via envcfg and thus need to be enabled on per thread basis.
>>>
>>>This patch creates a place holder for envcfg CSR in `thread_info` and adds
>>>logic to save and restore on trap entry and exits.
>>
>>Should only be "restore"?  I don't see saving.
>
>It's always saved in `thread_info` and user mode can't change it.
>So no point saving it.

Also I'll fix the commit message. I think that's what you were pointing out.

>
>>
>>>
>>>Signed-off-by: Deepak Gupta <debug@rivosinc.com>
>>>---
>>> arch/riscv/include/asm/thread_info.h | 1 +
>>> arch/riscv/kernel/asm-offsets.c      | 1 +
>>> arch/riscv/kernel/entry.S            | 4 ++++
>>> 3 files changed, 6 insertions(+)
>>>
>>>diff --git a/arch/riscv/include/asm/thread_info.h
>>>b/arch/riscv/include/asm/thread_info.h
>>>index 574779900bfb..320bc899a63b 100644
>>>--- a/arch/riscv/include/asm/thread_info.h
>>>+++ b/arch/riscv/include/asm/thread_info.h
>>>@@ -57,6 +57,7 @@ struct thread_info {
>>> 	long			user_sp;	/* User stack pointer */
>>> 	int			cpu;
>>> 	unsigned long		syscall_work;	/* SYSCALL_WORK_ flags */
>>>+	unsigned long envcfg;
>>> #ifdef CONFIG_SHADOW_CALL_STACK
>>> 	void			*scs_base;
>>> 	void			*scs_sp;
>>>diff --git a/arch/riscv/kernel/asm-offsets.c
>>>b/arch/riscv/kernel/asm-offsets.c
>>>index a03129f40c46..cdd8f095c30c 100644
>>>--- a/arch/riscv/kernel/asm-offsets.c
>>>+++ b/arch/riscv/kernel/asm-offsets.c
>>>@@ -39,6 +39,7 @@ void asm_offsets(void)
>>> 	OFFSET(TASK_TI_PREEMPT_COUNT, task_struct, thread_info.preempt_count);
>>> 	OFFSET(TASK_TI_KERNEL_SP, task_struct, thread_info.kernel_sp);
>>> 	OFFSET(TASK_TI_USER_SP, task_struct, thread_info.user_sp);
>>>+	OFFSET(TASK_TI_ENVCFG, task_struct, thread_info.envcfg);
>>> #ifdef CONFIG_SHADOW_CALL_STACK
>>> 	OFFSET(TASK_TI_SCS_SP, task_struct, thread_info.scs_sp);
>>> #endif
>>>diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
>>>index 54ca4564a926..63c3855ba80d 100644
>>>--- a/arch/riscv/kernel/entry.S
>>>+++ b/arch/riscv/kernel/entry.S
>>>@@ -129,6 +129,10 @@ SYM_CODE_START_NOALIGN(ret_from_exception)
>>> 	addi s0, sp, PT_SIZE_ON_STACK
>>> 	REG_S s0, TASK_TI_KERNEL_SP(tp)
>>>
>>>+	/* restore envcfg bits for current thread */
>>>+	REG_L s0, TASK_TI_ENVCFG(tp)
>>>+	csrw CSR_ENVCFG, s0
>>>+
>>
>>This is redundant if we're repeatedly processing interrupts or exceptions
>>within a single task.  We should only be writing envcfg when switching
>>between tasks or as part of the prctl.
>>
>>We need to use an ALTERNATIVE for this since the oldest supported hardware
>>does not have envcfg csrs.
>
>Yeah fixing that in next series. Thanks
>
>>
>>-s
>>
>>> 	/* Save the kernel shadow call stack pointer */
>>> 	scs_save_current
>>>
>>>--
>>>2.43.0
>>>
>>>
>>>_______________________________________________
>>>linux-riscv mailing list
>>>linux-riscv@lists.infradead.org
>>>http://lists.infradead.org/mailman/listinfo/linux-riscv