Re: [RFC PATCH v1 02/28] riscv: envcfg save and restore on trap entry/exit

Deepak Gupta
2024-01-25T17:09:20Z

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.

>
>>
>> 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