[PATCH 0/3] zicfilp and zicfiss support in opensbi

[PATCH 0/3] zicfilp and zicfiss support in opensbi

[Deepak Gupta]

I had sent opensbi patches for zicfilp and zicfiss support a while ago.
Since it was long time ago and specifications are ratified now, so
rebooting the patch series to version 1.

These patches to below things

- Add newly defined bits in existing CSRs
	* enabling bit in menvcfg
	* expected landing pad bit in mstatus for M and S
- Add new exception cause code and its delegation to S
	* software check exception is a new exception and is raised
	when control flow violation is detected by CPU
- Detection and enabling of zicfilp and zicfiss
	* zicfilp is detected by presence of MSTATUS_MPELP bit in mstatus
	* zicfiss is detected if CSR_SSP exists
	* zicfilp for S is not enabled by default. It is expected the S
	mode software will request a SBI call to enable it (via FWFT extn)
	* zicfiss is enabled by default for S mode because S mode can
	leverage it only if S mode is compiled with zicfiss support

Deepak Gupta (3):
  include: adding support for Zicfilp / Zicfiss encodings
  lib: sbi: Zicfilp/Zicfiss detection and elp cfi state reflect back in
    status
  lib: sbi: sw check exception delegation

 include/sbi/riscv_encoding.h |  6 ++++++
 include/sbi/sbi_hart.h       |  3 +++
 lib/sbi/sbi_hart.c           | 31 +++++++++++++++++++++++++++++++
 lib/sbi/sbi_trap.c           | 16 ++++++++++++++++
 4 files changed, 56 insertions(+)

-- 
2.44.0

[PATCH 1/3] include: adding support for Zicfilp / Zicfiss encodings

[Deepak Gupta]

Zicfilp / Zicfiss extension (see link) introduces b2 (LPE) in menvcfg CSR to
enable landing pads and b3 (SSE) in menvcfg CSR to enable shadow stack and
landing pad for privilege less than M. Additionally extension introduces new
bits in *status for recording landing pad state and a new exception type
`software check exception` with cause=0x12.

Link: https://github.com/riscv/riscv-cfi

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
---
 include/sbi/riscv_encoding.h | 6 ++++++
 include/sbi/sbi_hart.h       | 3 +++
 2 files changed, 9 insertions(+)

diff --git a/include/sbi/riscv_encoding.h b/include/sbi/riscv_encoding.h
index 2ed05f2..64cd9ab 100644
--- a/include/sbi/riscv_encoding.h
+++ b/include/sbi/riscv_encoding.h
@@ -32,6 +32,8 @@
 #define MSTATUS_TVM			_UL(0x00100000)
 #define MSTATUS_TW			_UL(0x00200000)
 #define MSTATUS_TSR			_UL(0x00400000)
+#define MSTATUS_SPELP		_UL(0x00800000)
+#define MSTATUS_MPELP		_UL(0x020000000000)
 #define MSTATUS32_SD			_UL(0x80000000)
 #if __riscv_xlen == 64
 #define MSTATUS_UXL			_ULL(0x0000000300000000)
@@ -213,6 +215,8 @@
 #define ENVCFG_PBMTE			(_ULL(1) << 62)
 #define ENVCFG_ADUE			(_ULL(1) << 61)
 #define ENVCFG_CDE			(_ULL(1) << 60)
+#define ENVCFG_SSE			(_ULL(1) << 3)
+#define ENVCFG_LPE			(_ULL(1) << 2)
 #define ENVCFG_CBZE			(_UL(1) << 7)
 #define ENVCFG_CBCFE			(_UL(1) << 6)
 #define ENVCFG_CBIE_SHIFT		4
@@ -228,6 +232,7 @@
 #define CSR_USTATUS			0x000
 #define CSR_UIE				0x004
 #define CSR_UTVEC			0x005
+#define CSR_SSP				0x011
 
 /* User Trap Handling (N-extension) */
 #define CSR_USCRATCH			0x040
@@ -763,6 +768,7 @@
 #define CAUSE_FETCH_PAGE_FAULT		0xc
 #define CAUSE_LOAD_PAGE_FAULT		0xd
 #define CAUSE_STORE_PAGE_FAULT		0xf
+#define CAUSE_SW_CHECK_EXCP			0x12
 #define CAUSE_FETCH_GUEST_PAGE_FAULT	0x14
 #define CAUSE_LOAD_GUEST_PAGE_FAULT	0x15
 #define CAUSE_VIRTUAL_INST_FAULT	0x16
diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
index 81ec061..2aa6867 100644
--- a/include/sbi/sbi_hart.h
+++ b/include/sbi/sbi_hart.h
@@ -67,6 +67,9 @@ enum sbi_hart_extensions {
 	SBI_HART_EXT_SVADE,
 	/** Hart has Svadu extension */
 	SBI_HART_EXT_SVADU,
+	/** HART has zicfiss & zicfilp extension */
+	SBI_HART_EXT_ZICFILP,
+	SBI_HART_EXT_ZICFISS,
 
 	/** Maximum index of Hart extension */
 	SBI_HART_EXT_MAX,
-- 
2.44.0

[PATCH 1/3] include: adding support for Zicfilp / Zicfiss encodings

[Samuel Holland]

Hi Deepak,

On 2024-08-21 6:55 PM, Deepak Gupta wrote:
> Zicfilp / Zicfiss extension (see link) introduces b2 (LPE) in menvcfg CSR to
> enable landing pads and b3 (SSE) in menvcfg CSR to enable shadow stack and
> landing pad for privilege less than M. Additionally extension introduces new
> bits in *status for recording landing pad state and a new exception type
> `software check exception` with cause=0x12.
> 
> Link: https://github.com/riscv/riscv-cfi
> 
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> ---
>  include/sbi/riscv_encoding.h | 6 ++++++
>  include/sbi/sbi_hart.h       | 3 +++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/include/sbi/riscv_encoding.h b/include/sbi/riscv_encoding.h
> index 2ed05f2..64cd9ab 100644
> --- a/include/sbi/riscv_encoding.h
> +++ b/include/sbi/riscv_encoding.h
> @@ -32,6 +32,8 @@
>  #define MSTATUS_TVM			_UL(0x00100000)
>  #define MSTATUS_TW			_UL(0x00200000)
>  #define MSTATUS_TSR			_UL(0x00400000)
> +#define MSTATUS_SPELP		_UL(0x00800000)
> +#define MSTATUS_MPELP		_UL(0x020000000000)

This won't compile on riscv32 because the value is out of range. The definition
needs to follow the pattern of those in the block below.

>  #define MSTATUS32_SD			_UL(0x80000000)
>  #if __riscv_xlen == 64
>  #define MSTATUS_UXL			_ULL(0x0000000300000000)
> @@ -213,6 +215,8 @@
>  #define ENVCFG_PBMTE			(_ULL(1) << 62)
>  #define ENVCFG_ADUE			(_ULL(1) << 61)
>  #define ENVCFG_CDE			(_ULL(1) << 60)
> +#define ENVCFG_SSE			(_ULL(1) << 3)
> +#define ENVCFG_LPE			(_ULL(1) << 2)

Please keep these ordered (should go just above ENVCFG_FIOM).

>  #define ENVCFG_CBZE			(_UL(1) << 7)
>  #define ENVCFG_CBCFE			(_UL(1) << 6)
>  #define ENVCFG_CBIE_SHIFT		4
> @@ -228,6 +232,7 @@
>  #define CSR_USTATUS			0x000
>  #define CSR_UIE				0x004
>  #define CSR_UTVEC			0x005
> +#define CSR_SSP				0x011
>  
>  /* User Trap Handling (N-extension) */
>  #define CSR_USCRATCH			0x040
> @@ -763,6 +768,7 @@
>  #define CAUSE_FETCH_PAGE_FAULT		0xc
>  #define CAUSE_LOAD_PAGE_FAULT		0xd
>  #define CAUSE_STORE_PAGE_FAULT		0xf
> +#define CAUSE_SW_CHECK_EXCP			0x12
>  #define CAUSE_FETCH_GUEST_PAGE_FAULT	0x14
>  #define CAUSE_LOAD_GUEST_PAGE_FAULT	0x15
>  #define CAUSE_VIRTUAL_INST_FAULT	0x16
> diff --git a/include/sbi/sbi_hart.h b/include/sbi/sbi_hart.h
> index 81ec061..2aa6867 100644
> --- a/include/sbi/sbi_hart.h
> +++ b/include/sbi/sbi_hart.h
> @@ -67,6 +67,9 @@ enum sbi_hart_extensions {
>  	SBI_HART_EXT_SVADE,
>  	/** Hart has Svadu extension */
>  	SBI_HART_EXT_SVADU,
> +	/** HART has zicfiss & zicfilp extension */
> +	SBI_HART_EXT_ZICFILP,
> +	SBI_HART_EXT_ZICFISS,

This needs to be part of the next patch, or it will trip the
_Static_assert(SBI_HART_EXT_MAX == array_size(sbi_hart_ext)) in sbi_hart.c.

Regards,
Samuel

>  
>  	/** Maximum index of Hart extension */
>  	SBI_HART_EXT_MAX,

[PATCH 2/3] lib: sbi: Zicfilp/Zicfiss detection and elp cfi state reflect back in status

[Deepak Gupta]

This patch adds support for zicfilp / zicfiss detection in sbi_hart.c
If zicfilp and zicfiss are detected, this patch turns on menvcfg.LPE and
menvcfg.SSE

Zicfilp records status of hart's ELP state in *status csr. Missing landing pad
sets MPELP in mstatus. When SBI is redirecting back to S/VS/HS, SPELP is
set in sstatus/vsstatus.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
---
 lib/sbi/sbi_hart.c | 28 ++++++++++++++++++++++++++++
 lib/sbi/sbi_trap.c | 16 ++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index c366701..67a2e42 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -148,6 +148,16 @@ static void mstatus_init(struct sbi_scratch *scratch)
 		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SVADE))
 			menvcfg_val &= ~ENVCFG_ADUE;
 
+		/*
+		 * By default allow shadow stack opreations in S/HS mode
+		 * don't enable landing pad because supervisor may keep faulting
+		 * due to missing landing pad. Open up a SBI interface to enable
+		 * landing pad
+		 */
+		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICFISS)) {
+			menvcfg_val |= ENVCFG_SSE;
+		}
+
 		csr_write(CSR_MENVCFG, menvcfg_val);
 #if __riscv_xlen == 32
 		csr_write(CSR_MENVCFGH, menvcfg_val >> 32);
@@ -680,6 +690,8 @@ const struct sbi_hart_ext_data sbi_hart_ext[] = {
 	__SBI_HART_EXT_DATA(ssccfg, SBI_HART_EXT_SSCCFG),
 	__SBI_HART_EXT_DATA(svade, SBI_HART_EXT_SVADE),
 	__SBI_HART_EXT_DATA(svadu, SBI_HART_EXT_SVADU),
+	__SBI_HART_EXT_DATA(zicfilp, SBI_HART_EXT_ZICFILP),
+	__SBI_HART_EXT_DATA(zicfiss, SBI_HART_EXT_ZICFISS),
 };
 
 _Static_assert(SBI_HART_EXT_MAX == array_size(sbi_hart_ext),
@@ -776,6 +788,7 @@ static int hart_detect_features(struct sbi_scratch *scratch)
 	unsigned long val, oldval;
 	bool has_zicntr = false;
 	int rc;
+	bool ssp_exist, elp_exist;
 
 	/* If hart features already detected then do nothing */
 	if (hfeatures->detected)
@@ -933,6 +946,21 @@ __pmp_skip:
 	/* Save trap based detection of Zicntr */
 	has_zicntr = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR);
 
+	if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
+		val = csr_read_allowed(CSR_SSP, (unsigned long)&trap);
+		ssp_exist = trap.cause == 0;
+		if (ssp_exist)
+			__sbi_hart_update_extension(hfeatures,
+					SBI_HART_EXT_ZICFISS, true);
+
+		csr_set(CSR_MSTATUS, MSTATUS_MPELP);
+		val = csr_read_clear(CSR_MSTATUS, MSTATUS_MPELP);
+		elp_exist = val & MSTATUS_MPELP;
+		if (elp_exist)
+			__sbi_hart_update_extension(hfeatures,
+					SBI_HART_EXT_ZICFILP, true);
+	}
+
 	/* Let platform populate extensions */
 	rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr(),
 					  hfeatures);
diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
index b4f3a17..2273b3a 100644
--- a/lib/sbi/sbi_trap.c
+++ b/lib/sbi/sbi_trap.c
@@ -103,6 +103,7 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
 		      const struct sbi_trap_info *trap)
 {
 	ulong hstatus, vsstatus, prev_mode;
+	bool elp = false;
 #if __riscv_xlen == 32
 	bool prev_virt = (regs->mstatusH & MSTATUSH_MPV) ? true : false;
 #else
@@ -116,6 +117,13 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
 	if (prev_mode != PRV_S && prev_mode != PRV_U)
 		return SBI_ENOTSUPP;
 
+	/* If extension has support for CFI, clear MPELP because redirecting to VS or (H)S */
+	if (sbi_hart_has_extension(sbi_scratch_thishart_ptr(), SBI_HART_EXT_ZICFILP)) {
+		elp = (regs->mstatus & MSTATUS_MPELP)? true: false;
+		/* Since redirecting, clear mpelp unconditionally */
+		regs->mstatus &= ~MSTATUS_MPELP;
+	}
+
 	/* If exceptions came from VS/VU-mode, redirect to VS-mode if
 	 * delegated in hedeleg
 	 */
@@ -169,6 +177,10 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
 		/* Get VS-mode SSTATUS CSR */
 		vsstatus = csr_read(CSR_VSSTATUS);
 
+		/*if elp was set, set it back in vsstatus */
+		if (elp)
+			vsstatus |= MSTATUS_SPELP;
+
 		/* Set SPP for VS-mode */
 		vsstatus &= ~SSTATUS_SPP;
 		if (prev_mode == PRV_S)
@@ -209,6 +221,10 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
 
 		/* Clear SIE for S-mode */
 		regs->mstatus &= ~MSTATUS_SIE;
+
+		/* if elp was set, set it back in mstatus */
+		if (elp)
+			regs->mstatus |= MSTATUS_SPELP;
 	}
 
 	return 0;
-- 
2.44.0

[PATCH 2/3] lib: sbi: Zicfilp/Zicfiss detection and elp cfi state reflect back in status

[Samuel Holland]

Hi Deepak,

On 2024-08-21 6:55 PM, Deepak Gupta wrote:
> This patch adds support for zicfilp / zicfiss detection in sbi_hart.c
> If zicfilp and zicfiss are detected, this patch turns on menvcfg.LPE and
> menvcfg.SSE
> 
> Zicfilp records status of hart's ELP state in *status csr. Missing landing pad
> sets MPELP in mstatus. When SBI is redirecting back to S/VS/HS, SPELP is
> set in sstatus/vsstatus.
> 
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> ---
>  lib/sbi/sbi_hart.c | 28 ++++++++++++++++++++++++++++
>  lib/sbi/sbi_trap.c | 16 ++++++++++++++++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index c366701..67a2e42 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -148,6 +148,16 @@ static void mstatus_init(struct sbi_scratch *scratch)
>  		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SVADE))
>  			menvcfg_val &= ~ENVCFG_ADUE;
>  
> +		/*
> +		 * By default allow shadow stack opreations in S/HS mode
> +		 * don't enable landing pad because supervisor may keep faulting
> +		 * due to missing landing pad. Open up a SBI interface to enable
> +		 * landing pad
> +		 */
> +		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICFISS)) {
> +			menvcfg_val |= ENVCFG_SSE;
> +		}

This violates the default value provided for the SHADOW_STACK feature in the SBI
FWFT extension specification[1].

[1]:
https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/src/ext-firmware-features.adoc#user-content-table_fw_features_attribute_values

> +
>  		csr_write(CSR_MENVCFG, menvcfg_val);
>  #if __riscv_xlen == 32
>  		csr_write(CSR_MENVCFGH, menvcfg_val >> 32);
> @@ -680,6 +690,8 @@ const struct sbi_hart_ext_data sbi_hart_ext[] = {
>  	__SBI_HART_EXT_DATA(ssccfg, SBI_HART_EXT_SSCCFG),
>  	__SBI_HART_EXT_DATA(svade, SBI_HART_EXT_SVADE),
>  	__SBI_HART_EXT_DATA(svadu, SBI_HART_EXT_SVADU),
> +	__SBI_HART_EXT_DATA(zicfilp, SBI_HART_EXT_ZICFILP),
> +	__SBI_HART_EXT_DATA(zicfiss, SBI_HART_EXT_ZICFISS),
>  };
>  
>  _Static_assert(SBI_HART_EXT_MAX == array_size(sbi_hart_ext),
> @@ -776,6 +788,7 @@ static int hart_detect_features(struct sbi_scratch *scratch)
>  	unsigned long val, oldval;
>  	bool has_zicntr = false;
>  	int rc;
> +	bool ssp_exist, elp_exist;
>  
>  	/* If hart features already detected then do nothing */
>  	if (hfeatures->detected)
> @@ -933,6 +946,21 @@ __pmp_skip:
>  	/* Save trap based detection of Zicntr */
>  	has_zicntr = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR);
>  
> +	if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
> +		val = csr_read_allowed(CSR_SSP, (unsigned long)&trap);
> +		ssp_exist = trap.cause == 0;

Would it be simpler to try setting menvcfg.SSE here, since that would never
cause a trap?

> +		if (ssp_exist)
> +			__sbi_hart_update_extension(hfeatures,
> +					SBI_HART_EXT_ZICFISS, true);
> +
> +		csr_set(CSR_MSTATUS, MSTATUS_MPELP);
> +		val = csr_read_clear(CSR_MSTATUS, MSTATUS_MPELP);
> +		elp_exist = val & MSTATUS_MPELP;
> +		if (elp_exist)
> +			__sbi_hart_update_extension(hfeatures,
> +					SBI_HART_EXT_ZICFILP, true);
> +	}
> +
>  	/* Let platform populate extensions */
>  	rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr(),
>  					  hfeatures);
> diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
> index b4f3a17..2273b3a 100644
> --- a/lib/sbi/sbi_trap.c
> +++ b/lib/sbi/sbi_trap.c
> @@ -103,6 +103,7 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
>  		      const struct sbi_trap_info *trap)
>  {
>  	ulong hstatus, vsstatus, prev_mode;
> +	bool elp = false;
>  #if __riscv_xlen == 32
>  	bool prev_virt = (regs->mstatusH & MSTATUSH_MPV) ? true : false;
>  #else
> @@ -116,6 +117,13 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
>  	if (prev_mode != PRV_S && prev_mode != PRV_U)
>  		return SBI_ENOTSUPP;
>  
> +	/* If extension has support for CFI, clear MPELP because redirecting to VS or (H)S */
> +	if (sbi_hart_has_extension(sbi_scratch_thishart_ptr(), SBI_HART_EXT_ZICFILP)) {
> +		elp = (regs->mstatus & MSTATUS_MPELP)? true: false;

The ternary expression here is unnecessary. The left side is already truthy/falsy.

Regards,
Samuel

> +		/* Since redirecting, clear mpelp unconditionally */
> +		regs->mstatus &= ~MSTATUS_MPELP;
> +	}
> +
>  	/* If exceptions came from VS/VU-mode, redirect to VS-mode if
>  	 * delegated in hedeleg
>  	 */
> @@ -169,6 +177,10 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
>  		/* Get VS-mode SSTATUS CSR */
>  		vsstatus = csr_read(CSR_VSSTATUS);
>  
> +		/*if elp was set, set it back in vsstatus */
> +		if (elp)
> +			vsstatus |= MSTATUS_SPELP;
> +
>  		/* Set SPP for VS-mode */
>  		vsstatus &= ~SSTATUS_SPP;
>  		if (prev_mode == PRV_S)
> @@ -209,6 +221,10 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
>  
>  		/* Clear SIE for S-mode */
>  		regs->mstatus &= ~MSTATUS_SIE;
> +
> +		/* if elp was set, set it back in mstatus */
> +		if (elp)
> +			regs->mstatus |= MSTATUS_SPELP;
>  	}
>  
>  	return 0;

[PATCH 2/3] lib: sbi: Zicfilp/Zicfiss detection and elp cfi state reflect back in status

[Deepak Gupta]

On Wed, Aug 21, 2024 at 07:32:35PM -0500, Samuel Holland wrote:
>Hi Deepak,
>
>On 2024-08-21 6:55 PM, Deepak Gupta wrote:
>> This patch adds support for zicfilp / zicfiss detection in sbi_hart.c
>> If zicfilp and zicfiss are detected, this patch turns on menvcfg.LPE and
>> menvcfg.SSE
>>
>> Zicfilp records status of hart's ELP state in *status csr. Missing landing pad
>> sets MPELP in mstatus. When SBI is redirecting back to S/VS/HS, SPELP is
>> set in sstatus/vsstatus.
>>
>> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
>> ---
>>  lib/sbi/sbi_hart.c | 28 ++++++++++++++++++++++++++++
>>  lib/sbi/sbi_trap.c | 16 ++++++++++++++++
>>  2 files changed, 44 insertions(+)
>>
>> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
>> index c366701..67a2e42 100644
>> --- a/lib/sbi/sbi_hart.c
>> +++ b/lib/sbi/sbi_hart.c
>> @@ -148,6 +148,16 @@ static void mstatus_init(struct sbi_scratch *scratch)
>>  		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_SVADE))
>>  			menvcfg_val &= ~ENVCFG_ADUE;
>>
>> +		/*
>> +		 * By default allow shadow stack opreations in S/HS mode
>> +		 * don't enable landing pad because supervisor may keep faulting
>> +		 * due to missing landing pad. Open up a SBI interface to enable
>> +		 * landing pad
>> +		 */
>> +		if (sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICFISS)) {
>> +			menvcfg_val |= ENVCFG_SSE;
>> +		}
>
>This violates the default value provided for the SHADOW_STACK feature in the SBI
>FWFT extension specification[1].
>
>[1]:
>https://github.com/riscv-non-isa/riscv-sbi-doc/blob/master/src/ext-firmware-features.adoc#user-content-table_fw_features_attribute_values
>

aah right. I remember now.
Kernel can't have shadow stack enabled from the get go.
I mean technically it can but someone (loader, grub, etc) will have to
allocate shadow for kernel. Although that's not how linux kernel is today.

So we can't have it enabled by default else shadow stack enabled kernel will
start tripping.

Will fix it.

>> +
>>  		csr_write(CSR_MENVCFG, menvcfg_val);
>>  #if __riscv_xlen == 32
>>  		csr_write(CSR_MENVCFGH, menvcfg_val >> 32);
>> @@ -680,6 +690,8 @@ const struct sbi_hart_ext_data sbi_hart_ext[] = {
>>  	__SBI_HART_EXT_DATA(ssccfg, SBI_HART_EXT_SSCCFG),
>>  	__SBI_HART_EXT_DATA(svade, SBI_HART_EXT_SVADE),
>>  	__SBI_HART_EXT_DATA(svadu, SBI_HART_EXT_SVADU),
>> +	__SBI_HART_EXT_DATA(zicfilp, SBI_HART_EXT_ZICFILP),
>> +	__SBI_HART_EXT_DATA(zicfiss, SBI_HART_EXT_ZICFISS),
>>  };
>>
>>  _Static_assert(SBI_HART_EXT_MAX == array_size(sbi_hart_ext),
>> @@ -776,6 +788,7 @@ static int hart_detect_features(struct sbi_scratch *scratch)
>>  	unsigned long val, oldval;
>>  	bool has_zicntr = false;
>>  	int rc;
>> +	bool ssp_exist, elp_exist;
>>
>>  	/* If hart features already detected then do nothing */
>>  	if (hfeatures->detected)
>> @@ -933,6 +946,21 @@ __pmp_skip:
>>  	/* Save trap based detection of Zicntr */
>>  	has_zicntr = sbi_hart_has_extension(scratch, SBI_HART_EXT_ZICNTR);
>>
>> +	if (hfeatures->priv_version >= SBI_HART_PRIV_VER_1_12) {
>> +		val = csr_read_allowed(CSR_SSP, (unsigned long)&trap);
>> +		ssp_exist = trap.cause == 0;
>
>Would it be simpler to try setting menvcfg.SSE here, since that would never
>cause a trap?

Moot point now.
>
>> +		if (ssp_exist)
>> +			__sbi_hart_update_extension(hfeatures,
>> +					SBI_HART_EXT_ZICFISS, true);
>> +
>> +		csr_set(CSR_MSTATUS, MSTATUS_MPELP);
>> +		val = csr_read_clear(CSR_MSTATUS, MSTATUS_MPELP);
>> +		elp_exist = val & MSTATUS_MPELP;
>> +		if (elp_exist)
>> +			__sbi_hart_update_extension(hfeatures,
>> +					SBI_HART_EXT_ZICFILP, true);
>> +	}
>> +
>>  	/* Let platform populate extensions */
>>  	rc = sbi_platform_extensions_init(sbi_platform_thishart_ptr(),
>>  					  hfeatures);
>> diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
>> index b4f3a17..2273b3a 100644
>> --- a/lib/sbi/sbi_trap.c
>> +++ b/lib/sbi/sbi_trap.c
>> @@ -103,6 +103,7 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
>>  		      const struct sbi_trap_info *trap)
>>  {
>>  	ulong hstatus, vsstatus, prev_mode;
>> +	bool elp = false;
>>  #if __riscv_xlen == 32
>>  	bool prev_virt = (regs->mstatusH & MSTATUSH_MPV) ? true : false;
>>  #else
>> @@ -116,6 +117,13 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
>>  	if (prev_mode != PRV_S && prev_mode != PRV_U)
>>  		return SBI_ENOTSUPP;
>>
>> +	/* If extension has support for CFI, clear MPELP because redirecting to VS or (H)S */
>> +	if (sbi_hart_has_extension(sbi_scratch_thishart_ptr(), SBI_HART_EXT_ZICFILP)) {
>> +		elp = (regs->mstatus & MSTATUS_MPELP)? true: false;
>
>The ternary expression here is unnecessary. The left side is already truthy/falsy.

Yeah I just followed style for `prev_virt` earlier in this function.
I can change this one.

>
>Regards,
>Samuel
>
>> +		/* Since redirecting, clear mpelp unconditionally */
>> +		regs->mstatus &= ~MSTATUS_MPELP;
>> +	}
>> +
>>  	/* If exceptions came from VS/VU-mode, redirect to VS-mode if
>>  	 * delegated in hedeleg
>>  	 */
>> @@ -169,6 +177,10 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
>>  		/* Get VS-mode SSTATUS CSR */
>>  		vsstatus = csr_read(CSR_VSSTATUS);
>>
>> +		/*if elp was set, set it back in vsstatus */
>> +		if (elp)
>> +			vsstatus |= MSTATUS_SPELP;
>> +
>>  		/* Set SPP for VS-mode */
>>  		vsstatus &= ~SSTATUS_SPP;
>>  		if (prev_mode == PRV_S)
>> @@ -209,6 +221,10 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
>>
>>  		/* Clear SIE for S-mode */
>>  		regs->mstatus &= ~MSTATUS_SIE;
>> +
>> +		/* if elp was set, set it back in mstatus */
>> +		if (elp)
>> +			regs->mstatus |= MSTATUS_SPELP;
>>  	}
>>
>>  	return 0;
>

[PATCH 3/3] lib: sbi: sw check exception delegation

[Deepak Gupta]

zicfiss and zicfilp introduces new exception (cause=18). Delegate this
exception to S mode because cfi violations in U / S will be reported
via this exception.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
---
 lib/sbi/sbi_hart.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index 67a2e42..5bc02a1 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -234,6 +234,9 @@ static int delegate_traps(struct sbi_scratch *scratch)
 		exceptions |= (1U << CAUSE_STORE_GUEST_PAGE_FAULT);
 	}
 
+	/* delegate sw check exception to S */
+	exceptions |= (1U << CAUSE_SW_CHECK_EXCP);
+
 	csr_write(CSR_MIDELEG, interrupts);
 	csr_write(CSR_MEDELEG, exceptions);
 
-- 
2.44.0

[PATCH 3/3] lib: sbi: sw check exception delegation

[Samuel Holland]

Hi Deepak,

On 2024-08-21 6:55 PM, Deepak Gupta wrote:
> zicfiss and zicfilp introduces new exception (cause=18). Delegate this
> exception to S mode because cfi violations in U / S will be reported
> via this exception.
> 
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> ---
>  lib/sbi/sbi_hart.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index 67a2e42..5bc02a1 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -234,6 +234,9 @@ static int delegate_traps(struct sbi_scratch *scratch)
>  		exceptions |= (1U << CAUSE_STORE_GUEST_PAGE_FAULT);
>  	}
>  
> +	/* delegate sw check exception to S */
> +	exceptions |= (1U << CAUSE_SW_CHECK_EXCP);

I would suggest to include this with the other unconditionally-delegated
exceptions earlier in the function. Either way:

Reviewed-by: Samuel Holland <samuel.holland@sifive.com>

> +
>  	csr_write(CSR_MIDELEG, interrupts);
>  	csr_write(CSR_MEDELEG, exceptions);
>