[PATCH v1 1/2] riscv: abstract envcfg CSR

[PATCH v1 1/2] riscv: abstract envcfg CSR

[Deepak Gupta]

This patch abstracts envcfg CSR in kernel (as is done for other homonyn
CSRs). CSR_ENVCFG is used as alias for CSR_SENVCFG or CSR_MENVCFG depending
on how kernel is compiled.

Additionally it changes CBZE enabling to start using CSR_ENVCFG instead of
CSR_SENVCFG.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
---
 arch/riscv/include/asm/csr.h   | 2 ++
 arch/riscv/kernel/cpufeature.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index 306a19a5509c..b3400517b0a9 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -415,6 +415,7 @@
 # define CSR_STATUS	CSR_MSTATUS
 # define CSR_IE		CSR_MIE
 # define CSR_TVEC	CSR_MTVEC
+# define CSR_ENVCFG CSR_MENVCFG
 # define CSR_SCRATCH	CSR_MSCRATCH
 # define CSR_EPC	CSR_MEPC
 # define CSR_CAUSE	CSR_MCAUSE
@@ -439,6 +440,7 @@
 # define CSR_STATUS	CSR_SSTATUS
 # define CSR_IE		CSR_SIE
 # define CSR_TVEC	CSR_STVEC
+# define CSR_ENVCFG CSR_SENVCFG
 # define CSR_SCRATCH	CSR_SSCRATCH
 # define CSR_EPC	CSR_SEPC
 # define CSR_CAUSE	CSR_SCAUSE
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index b3785ffc1570..98623393fd1f 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -725,7 +725,7 @@ arch_initcall(check_unaligned_access_all_cpus);
 void riscv_user_isa_enable(void)
 {
 	if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ))
-		csr_set(CSR_SENVCFG, ENVCFG_CBZE);
+		csr_set(CSR_ENVCFG, ENVCFG_CBZE);
 }
 
 #ifdef CONFIG_RISCV_ALTERNATIVE
-- 
2.43.0


From 2ad5b91bda97f6dbc9f48a2e0e8c6eae5b8452ca Mon Sep 17 00:00:00 2001
From: Deepak Gupta <debug@rivosinc.com>
Date: Tue, 12 Dec 2023 14:28:59 -0800
Subject: [PATCH v1 2/2] riscv: envcfg save and restore on trap entry/exit

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 and pointer masking features need to be enabled on per
thread basis. Additionally, I believe cache management instructions need to be
enabled on per thread basis. As an example a seccomped task on riscv may be
restricted to not use cache management instructions

This patch creates a place holder for envcfg CSR in `thread_info` and adds
logic to save and restore on trap entry and exits. This allows such isa feature
to be enabled on per thread basis.

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            | 6 ++++++
 3 files changed, 8 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..a1d87013f15a 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -64,12 +64,14 @@ SYM_CODE_START(handle_exception)
 	csrr s3, CSR_TVAL
 	csrr s4, CSR_CAUSE
 	csrr s5, CSR_SCRATCH
+	csrr s6, CSR_ENVCFG
 	REG_S s0, PT_SP(sp)
 	REG_S s1, PT_STATUS(sp)
 	REG_S s2, PT_EPC(sp)
 	REG_S s3, PT_BADADDR(sp)
 	REG_S s4, PT_CAUSE(sp)
 	REG_S s5, PT_TP(sp)
+	REG_S s6, TASK_TI_ENVCFG(tp)
 
 	/*
 	 * Set the scratch register to 0, so that if a recursive exception
@@ -129,6 +131,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
+
 	/* Save the kernel shadow call stack pointer */
 	scs_save_current
 
-- 
2.43.0

Re: [PATCH v1 1/2] riscv: abstract envcfg CSR

[Deepak Gupta]

Some issues in workflow. Patches didn't come out separately.

Ignore this.

On Tue, Dec 12, 2023 at 3:43 PM Deepak Gupta <debug@rivosinc.com> wrote:
>
> This patch abstracts envcfg CSR in kernel (as is done for other homonyn
> CSRs). CSR_ENVCFG is used as alias for CSR_SENVCFG or CSR_MENVCFG depending
> on how kernel is compiled.
>
> Additionally it changes CBZE enabling to start using CSR_ENVCFG instead of
> CSR_SENVCFG.
>
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> ---
>  arch/riscv/include/asm/csr.h   | 2 ++
>  arch/riscv/kernel/cpufeature.c | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
> index 306a19a5509c..b3400517b0a9 100644
> --- a/arch/riscv/include/asm/csr.h
> +++ b/arch/riscv/include/asm/csr.h
> @@ -415,6 +415,7 @@
>  # define CSR_STATUS    CSR_MSTATUS
>  # define CSR_IE                CSR_MIE
>  # define CSR_TVEC      CSR_MTVEC
> +# define CSR_ENVCFG CSR_MENVCFG
>  # define CSR_SCRATCH   CSR_MSCRATCH
>  # define CSR_EPC       CSR_MEPC
>  # define CSR_CAUSE     CSR_MCAUSE
> @@ -439,6 +440,7 @@
>  # define CSR_STATUS    CSR_SSTATUS
>  # define CSR_IE                CSR_SIE
>  # define CSR_TVEC      CSR_STVEC
> +# define CSR_ENVCFG CSR_SENVCFG
>  # define CSR_SCRATCH   CSR_SSCRATCH
>  # define CSR_EPC       CSR_SEPC
>  # define CSR_CAUSE     CSR_SCAUSE
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index b3785ffc1570..98623393fd1f 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -725,7 +725,7 @@ arch_initcall(check_unaligned_access_all_cpus);
>  void riscv_user_isa_enable(void)
>  {
>         if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ))
> -               csr_set(CSR_SENVCFG, ENVCFG_CBZE);
> +               csr_set(CSR_ENVCFG, ENVCFG_CBZE);
>  }
>
>  #ifdef CONFIG_RISCV_ALTERNATIVE
> --
> 2.43.0
>
>
> From 2ad5b91bda97f6dbc9f48a2e0e8c6eae5b8452ca Mon Sep 17 00:00:00 2001
> From: Deepak Gupta <debug@rivosinc.com>
> Date: Tue, 12 Dec 2023 14:28:59 -0800
> Subject: [PATCH v1 2/2] riscv: envcfg save and restore on trap entry/exit
>
> 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 and pointer masking features need to be enabled on per
> thread basis. Additionally, I believe cache management instructions need to be
> enabled on per thread basis. As an example a seccomped task on riscv may be
> restricted to not use cache management instructions
>
> This patch creates a place holder for envcfg CSR in `thread_info` and adds
> logic to save and restore on trap entry and exits. This allows such isa feature
> to be enabled on per thread basis.
>
> 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            | 6 ++++++
>  3 files changed, 8 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..a1d87013f15a 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -64,12 +64,14 @@ SYM_CODE_START(handle_exception)
>         csrr s3, CSR_TVAL
>         csrr s4, CSR_CAUSE
>         csrr s5, CSR_SCRATCH
> +       csrr s6, CSR_ENVCFG
>         REG_S s0, PT_SP(sp)
>         REG_S s1, PT_STATUS(sp)
>         REG_S s2, PT_EPC(sp)
>         REG_S s3, PT_BADADDR(sp)
>         REG_S s4, PT_CAUSE(sp)
>         REG_S s5, PT_TP(sp)
> +       REG_S s6, TASK_TI_ENVCFG(tp)
>
>         /*
>          * Set the scratch register to 0, so that if a recursive exception
> @@ -129,6 +131,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
> +
>         /* Save the kernel shadow call stack pointer */
>         scs_save_current
>
> --
> 2.43.0
>

[PATCH v1 2/2] riscv: envcfg save and restore on trap entry/exit

[Deepak Gupta]

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 and pointer masking features need to be enabled on per
thread basis. Additionally, I believe cache management instructions need to be
enabled on per thread basis. As an example a seccomped task on riscv may be
restricted to not use cache management instructions

This patch creates a place holder for envcfg CSR in `thread_info` and adds
logic to save and restore on trap entry and exits. This allows such isa feature
to be enabled on per thread basis.

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            | 6 ++++++
 3 files changed, 8 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..a1d87013f15a 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -64,12 +64,14 @@ SYM_CODE_START(handle_exception)
 	csrr s3, CSR_TVAL
 	csrr s4, CSR_CAUSE
 	csrr s5, CSR_SCRATCH
+	csrr s6, CSR_ENVCFG
 	REG_S s0, PT_SP(sp)
 	REG_S s1, PT_STATUS(sp)
 	REG_S s2, PT_EPC(sp)
 	REG_S s3, PT_BADADDR(sp)
 	REG_S s4, PT_CAUSE(sp)
 	REG_S s5, PT_TP(sp)
+	REG_S s6, TASK_TI_ENVCFG(tp)
 
 	/*
 	 * Set the scratch register to 0, so that if a recursive exception
@@ -129,6 +131,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
+
 	/* Save the kernel shadow call stack pointer */
 	scs_save_current
 
-- 
2.43.0

[PATCH v1 1/2] riscv: abstract envcfg CSR

[Deepak Gupta]

This patch abstracts envcfg CSR in kernel (as is done for other homonyn
CSRs). CSR_ENVCFG is used as alias for CSR_SENVCFG or CSR_MENVCFG depending
on how kernel is compiled.

Additionally it changes CBZE enabling to start using CSR_ENVCFG instead of
CSR_SENVCFG.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
---
 arch/riscv/include/asm/csr.h   | 2 ++
 arch/riscv/kernel/cpufeature.c | 2 +-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index 306a19a5509c..b3400517b0a9 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -415,6 +415,7 @@
 # define CSR_STATUS	CSR_MSTATUS
 # define CSR_IE		CSR_MIE
 # define CSR_TVEC	CSR_MTVEC
+# define CSR_ENVCFG CSR_MENVCFG
 # define CSR_SCRATCH	CSR_MSCRATCH
 # define CSR_EPC	CSR_MEPC
 # define CSR_CAUSE	CSR_MCAUSE
@@ -439,6 +440,7 @@
 # define CSR_STATUS	CSR_SSTATUS
 # define CSR_IE		CSR_SIE
 # define CSR_TVEC	CSR_STVEC
+# define CSR_ENVCFG CSR_SENVCFG
 # define CSR_SCRATCH	CSR_SSCRATCH
 # define CSR_EPC	CSR_SEPC
 # define CSR_CAUSE	CSR_SCAUSE
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index b3785ffc1570..98623393fd1f 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -725,7 +725,7 @@ arch_initcall(check_unaligned_access_all_cpus);
 void riscv_user_isa_enable(void)
 {
 	if (riscv_cpu_has_extension_unlikely(smp_processor_id(), RISCV_ISA_EXT_ZICBOZ))
-		csr_set(CSR_SENVCFG, ENVCFG_CBZE);
+		csr_set(CSR_ENVCFG, ENVCFG_CBZE);
 }
 
 #ifdef CONFIG_RISCV_ALTERNATIVE
-- 
2.43.0

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

[Palmer Dabbelt]

On Tue, 12 Dec 2023 15:49:25 PST (-0800), debug@rivosinc.com wrote:
> 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 and pointer masking features need to be enabled on per
> thread basis. Additionally, I believe cache management instructions need to be
> enabled on per thread basis. As an example a seccomped task on riscv may be
> restricted to not use cache management instructions

Do we have anything in the kernel that actually does that?  Generally we 
need some use, I couldn't find any user-mode writable envcfg bits in any 
extesions I looked at (admittidly just CFI and pointer masking), and 
unless I'm missing something there's no per-thread state in the kernel.

> This patch creates a place holder for envcfg CSR in `thread_info` and adds
> logic to save and restore on trap entry and exits. This allows such isa feature
> to be enabled on per thread basis.
>
> 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            | 6 ++++++
>  3 files changed, 8 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..a1d87013f15a 100644
> --- a/arch/riscv/kernel/entry.S
> +++ b/arch/riscv/kernel/entry.S
> @@ -64,12 +64,14 @@ SYM_CODE_START(handle_exception)
>  	csrr s3, CSR_TVAL
>  	csrr s4, CSR_CAUSE
>  	csrr s5, CSR_SCRATCH
> +	csrr s6, CSR_ENVCFG
>  	REG_S s0, PT_SP(sp)
>  	REG_S s1, PT_STATUS(sp)
>  	REG_S s2, PT_EPC(sp)
>  	REG_S s3, PT_BADADDR(sp)
>  	REG_S s4, PT_CAUSE(sp)
>  	REG_S s5, PT_TP(sp)
> +	REG_S s6, TASK_TI_ENVCFG(tp)
>
>  	/*
>  	 * Set the scratch register to 0, so that if a recursive exception
> @@ -129,6 +131,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
> +
>  	/* Save the kernel shadow call stack pointer */
>  	scs_save_current

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

[Deepak Gupta]

On Tue, Dec 12, 2023 at 04:53:48PM -0800, Palmer Dabbelt wrote:
>On Tue, 12 Dec 2023 15:49:25 PST (-0800), debug@rivosinc.com wrote:
>>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 and pointer masking features need to be enabled on per
>>thread basis. Additionally, I believe cache management instructions need to be
>>enabled on per thread basis. As an example a seccomped task on riscv may be
>>restricted to not use cache management instructions
>
>Do we have anything in the kernel that actually does that?  Generally 
>we need some use, I couldn't find any user-mode writable envcfg bits 
>in any extesions I looked at (admittidly just CFI and pointer 
>masking), and unless I'm missing something there's no per-thread state 
>in the kernel.
>

Cache management operations?
As of now kernel blindly enables that for all the user mode. It will be good if
that is enabled on per-thread basis. Sure, all threads can have it enabled by
default. But if strict seccomp is enabled, I would argue that cache management
operations for that thread to be disabled as is done on other arches. As an
example x86 disable rdtsc on strict seccomp. 
RISCV allows this CMO extension and I expect CMO to leverage this (currently it
doesn't).

I was being opportunistic here so that I can reduce number of patches on CFI
enabling patchset.

Will it be okay if I revise this patch to include with a usecase to restrict CMO
(say for case of strict seccomp on risc-v)?

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

[Andrew Jones]

On Tue, Dec 12, 2023 at 05:02:43PM -0800, Deepak Gupta wrote:
> On Tue, Dec 12, 2023 at 04:53:48PM -0800, Palmer Dabbelt wrote:
> > On Tue, 12 Dec 2023 15:49:25 PST (-0800), debug@rivosinc.com wrote:
> > > 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 and pointer masking features need to be enabled on per
> > > thread basis. Additionally, I believe cache management instructions need to be
> > > enabled on per thread basis. As an example a seccomped task on riscv may be
> > > restricted to not use cache management instructions
> > 
> > Do we have anything in the kernel that actually does that?  Generally we
> > need some use, I couldn't find any user-mode writable envcfg bits in any
> > extesions I looked at (admittidly just CFI and pointer masking), and
> > unless I'm missing something there's no per-thread state in the kernel.
> > 
> 
> Cache management operations?
> As of now kernel blindly enables that for all the user mode. It will be good if
> that is enabled on per-thread basis. Sure, all threads can have it enabled by
> default. But if strict seccomp is enabled, I would argue that cache management
> operations for that thread to be disabled as is done on other arches. As an
> example x86 disable rdtsc on strict seccomp. RISCV allows this CMO extension
> and I expect CMO to leverage this (currently it
> doesn't).
> 
> I was being opportunistic here so that I can reduce number of patches on CFI
> enabling patchset.
> 
> Will it be okay if I revise this patch to include with a usecase to restrict CMO
> (say for case of strict seccomp on risc-v)?

I opted to only expose cache block zero since giving userspace the
ability to invalidate cache blocks seems risky from a side-channel attack
perspective.

I'm no security expert, so feedback welcome, but I don't see a risk with
userspace being granted cbo.zero, even for strict seccomp processes.

Thanks,
drew

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

[Deepak Gupta]

On Wed, Dec 13, 2023 at 4:24 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> On Tue, Dec 12, 2023 at 05:02:43PM -0800, Deepak Gupta wrote:
> > On Tue, Dec 12, 2023 at 04:53:48PM -0800, Palmer Dabbelt wrote:
> > > On Tue, 12 Dec 2023 15:49:25 PST (-0800), debug@rivosinc.com wrote:
> > > > 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 and pointer masking features need to be enabled on per
> > > > thread basis. Additionally, I believe cache management instructions need to be
> > > > enabled on per thread basis. As an example a seccomped task on riscv may be
> > > > restricted to not use cache management instructions
> > >
> > > Do we have anything in the kernel that actually does that?  Generally we
> > > need some use, I couldn't find any user-mode writable envcfg bits in any
> > > extesions I looked at (admittidly just CFI and pointer masking), and
> > > unless I'm missing something there's no per-thread state in the kernel.
> > >
> >
> > Cache management operations?
> > As of now kernel blindly enables that for all the user mode. It will be good if
> > that is enabled on per-thread basis. Sure, all threads can have it enabled by
> > default. But if strict seccomp is enabled, I would argue that cache management
> > operations for that thread to be disabled as is done on other arches. As an
> > example x86 disable rdtsc on strict seccomp. RISCV allows this CMO extension
> > and I expect CMO to leverage this (currently it
> > doesn't).
> >
> > I was being opportunistic here so that I can reduce number of patches on CFI
> > enabling patchset.
> >
> > Will it be okay if I revise this patch to include with a usecase to restrict CMO
> > (say for case of strict seccomp on risc-v)?
>
> I opted to only expose cache block zero since giving userspace the
> ability to invalidate cache blocks seems risky from a side-channel attack
> perspective.

I didn't pay attention. You're right, it's only cbo.zero that's exposed.
I will roll up my patch with cfi set then.

>
> I'm no security expert, so feedback welcome, but I don't see a risk with
> userspace being granted cbo.zero, even for strict seccomp processes.

Yeah I don't see a risk with cbo.zero to strict seccomp thread either.

>
> Thanks,
> drew