[PATCH v5 0/4] zicfilp and zicfiss support in opensbi

[PATCH v5 0/4] zicfilp and zicfiss support in opensbi

[Deepak Gupta]

v5 for zicfilp and zicfiss support in opensbi

---
v5:
 - OpenSBI parses ISA string which is recieved in FDT and if ISA
   string matches, it sets extension availablity in extension bitmap.
   Thus removed zicfilp and zicfiss trap based and mstatus detection
v4:
 - fixed up commit message in patch 2
v3:
 - added fwft implementation for shadow stack and landing pad interface
v2:
 - added mpelp for 32bit (mstatush) and its handling in trad redirection
 - removed default SSE enabling. it'll break shadow stack enabled kernel
 - put sw check delegation at correct place

Deepak Gupta (4):
  include: adding support for Zicfilp / Zicfiss encodings
  lib: sbi: add zicfilp/zicfiss and elp cfi state reflect back in status
  lib: sbi: sw check exception delegation
  lib: sbi: fwft: implement landing pad and shadow stack fwft interface

 include/sbi/riscv_encoding.h |  7 ++++
 include/sbi/sbi_hart.h       |  3 ++
 lib/sbi/sbi_fwft.c           | 74 ++++++++++++++++++++++++++++++++++++
 lib/sbi/sbi_hart.c           |  5 ++-
 lib/sbi/sbi_trap.c           | 20 ++++++++++
 5 files changed, 108 insertions(+), 1 deletion(-)

-- 
2.44.0

[PATCH v5 1/4] 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>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
---
 include/sbi/riscv_encoding.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/sbi/riscv_encoding.h b/include/sbi/riscv_encoding.h
index 2ed05f2..fa1d373 100644
--- a/include/sbi/riscv_encoding.h
+++ b/include/sbi/riscv_encoding.h
@@ -32,6 +32,7 @@
 #define MSTATUS_TVM			_UL(0x00100000)
 #define MSTATUS_TW			_UL(0x00200000)
 #define MSTATUS_TSR			_UL(0x00400000)
+#define MSTATUS_SPELP		_UL(0x00800000)
 #define MSTATUS32_SD			_UL(0x80000000)
 #if __riscv_xlen == 64
 #define MSTATUS_UXL			_ULL(0x0000000300000000)
@@ -41,12 +42,14 @@
 #define MSTATUS_GVA			_ULL(0x0000004000000000)
 #define MSTATUS_GVA_SHIFT		38
 #define MSTATUS_MPV			_ULL(0x0000008000000000)
+#define MSTATUS_MPELP		_ULL(0x0000020000000000)
 #else
 #define MSTATUSH_SBE			_UL(0x00000010)
 #define MSTATUSH_MBE			_UL(0x00000020)
 #define MSTATUSH_GVA			_UL(0x00000040)
 #define MSTATUSH_GVA_SHIFT		6
 #define MSTATUSH_MPV			_UL(0x00000080)
+#define MSTATUSH_MPELP			_UL(0x00000200)
 #endif
 #define MSTATUS32_SD			_UL(0x80000000)
 #define MSTATUS64_SD			_ULL(0x8000000000000000)
@@ -220,6 +223,8 @@
 #define ENVCFG_CBIE_ILL			_UL(0x0)
 #define ENVCFG_CBIE_FLUSH		_UL(0x1)
 #define ENVCFG_CBIE_INV			_UL(0x3)
+#define ENVCFG_SSE			(_UL(1) << 3)
+#define ENVCFG_LPE			(_UL(1) << 2)
 #define ENVCFG_FIOM			_UL(0x1)
 
 /* ===== User-level CSRs ===== */
@@ -228,6 +233,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 +769,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
-- 
2.44.0

[PATCH v5 2/4] lib: sbi: add zicfilp/zicfiss and elp cfi state reflect back in status

[Deepak Gupta]

This patch adds support to check for zicfilp / zicfiss extension.

zicfilp record 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>
---
 include/sbi/sbi_hart.h |  3 +++
 lib/sbi/sbi_hart.c     |  2 ++
 lib/sbi/sbi_trap.c     | 20 ++++++++++++++++++++
 3 files changed, 25 insertions(+)

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,
diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index c366701..e1bc346 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -680,6 +680,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),
diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
index b4f3a17..e2502f2 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,17 @@ 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)) {
+#if __riscv_xlen == 32
+		elp = regs->mstatusH & MSTATUSH_MPELP;
+		regs->mstatusH &= ~MSTATUSH_MPELP;
+#else
+		elp = regs->mstatus & MSTATUS_MPELP;
+		regs->mstatus &= ~MSTATUS_MPELP;
+#endif
+	}
+
 	/* If exceptions came from VS/VU-mode, redirect to VS-mode if
 	 * delegated in hedeleg
 	 */
@@ -169,6 +181,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 +225,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 v5 3/4] 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>
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
---
 lib/sbi/sbi_hart.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
index e1bc346..fa00f69 100644
--- a/lib/sbi/sbi_hart.c
+++ b/lib/sbi/sbi_hart.c
@@ -207,7 +207,8 @@ static int delegate_traps(struct sbi_scratch *scratch)
 	if (sbi_platform_has_mfaults_delegation(plat))
 		exceptions |= (1U << CAUSE_FETCH_PAGE_FAULT) |
 			      (1U << CAUSE_LOAD_PAGE_FAULT) |
-			      (1U << CAUSE_STORE_PAGE_FAULT);
+			      (1U << CAUSE_STORE_PAGE_FAULT)|
+			      (1U << CAUSE_SW_CHECK_EXCP);
 
 	/*
 	 * If hypervisor extension available then we only handle hypervisor
-- 
2.44.0

[PATCH v5 4/4] lib: sbi: fwft: implement landing pad and shadow stack fwft interface

[Deepak Gupta]

Supervisor software can enable control flow integrity features for itself
using fwft feature `SBI_FWFT_LANDING_PAD` and `SBI_FWFT_SHADOW_STACK`.
This patch implements the mechanism to enable both these fwft.

Signed-off-by: Deepak Gupta <debug@rivosinc.com>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
Reviewed-by: Cl?ment L?ger <cleger@rivosinc.com>
---
 lib/sbi/sbi_fwft.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 74 insertions(+)

diff --git a/lib/sbi/sbi_fwft.c b/lib/sbi/sbi_fwft.c
index ef881ef..747bc88 100644
--- a/lib/sbi/sbi_fwft.c
+++ b/lib/sbi/sbi_fwft.c
@@ -145,6 +145,68 @@ static int fwft_get_adue(struct fwft_config *conf, unsigned long *value)
 	return SBI_OK;
 }
 
+static int fwft_lpad_supported(struct fwft_config *conf)
+{
+	if (!sbi_hart_has_extension(sbi_scratch_thishart_ptr(),
+				    SBI_HART_EXT_ZICFILP))
+		return SBI_ENOTSUPP;
+
+	return SBI_OK;
+}
+
+static int fwft_enable_lpad(struct fwft_config *conf, unsigned long value)
+{
+	if (value == 1)
+		csr_set(CSR_MENVCFG, ENVCFG_LPE);
+	else if (value == 0)
+		csr_clear(CSR_MENVCFG, ENVCFG_LPE);
+	else
+		return SBI_EINVAL;
+
+	return SBI_OK;
+}
+
+static int fwft_get_lpad(struct fwft_config *conf, unsigned long *value)
+{
+	unsigned long cfg;
+
+	cfg = csr_read(CSR_MENVCFG) & ENVCFG_LPE;
+	*value = cfg != 0;
+
+	return SBI_OK;
+}
+
+static int fwft_sstack_supported(struct fwft_config *conf)
+{
+	if (!sbi_hart_has_extension(sbi_scratch_thishart_ptr(),
+				    SBI_HART_EXT_ZICFISS))
+		return SBI_ENOTSUPP;
+
+	return SBI_OK;
+}
+
+static int fwft_enable_sstack(struct fwft_config *conf, unsigned long value)
+{
+	if (value == 1)
+		csr_set(CSR_MENVCFG, ENVCFG_SSE);
+	else if (value == 0)
+		csr_clear(CSR_MENVCFG, ENVCFG_SSE);
+	else
+		return SBI_EINVAL;
+
+	return SBI_OK;
+}
+
+static int fwft_get_sstack(struct fwft_config *conf, unsigned long *value)
+{
+	unsigned long cfg;
+
+	cfg = csr_read(CSR_MENVCFG) & ENVCFG_SSE;
+	*value = cfg != 0;
+
+	return SBI_OK;
+}
+
 static struct fwft_config* get_feature_config(enum sbi_fwft_feature_t feature)
 {
 	int i;
@@ -236,6 +298,18 @@ static const struct fwft_feature features[] =
 		.set = fwft_set_adue,
 		.get = fwft_get_adue,
 	},
+	{
+		.id = SBI_FWFT_LANDING_PAD,
+		.supported = fwft_lpad_supported,
+		.set = fwft_enable_lpad,
+		.get = fwft_get_lpad,
+	},
+	{
+		.id = SBI_FWFT_SHADOW_STACK,
+		.supported = fwft_sstack_supported,
+		.set = fwft_enable_sstack,
+		.get = fwft_get_sstack,
+	},
 };
 
 int sbi_fwft_init(struct sbi_scratch *scratch, bool cold_boot)
-- 
2.44.0

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

[Samuel Holland]

On 2024-09-10 11:35 AM, 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>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
> ---
>  include/sbi/riscv_encoding.h | 7 +++++++
>  1 file changed, 7 insertions(+)

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

[PATCH v5 2/4] lib: sbi: add zicfilp/zicfiss and elp cfi state reflect back in status

[Samuel Holland]

On 2024-09-10 11:35 AM, Deepak Gupta wrote:
> This patch adds support to check for zicfilp / zicfiss extension.
> 
> zicfilp record 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>
> ---
>  include/sbi/sbi_hart.h |  3 +++
>  lib/sbi/sbi_hart.c     |  2 ++
>  lib/sbi/sbi_trap.c     | 20 ++++++++++++++++++++
>  3 files changed, 25 insertions(+)
> 
> 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,
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index c366701..e1bc346 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -680,6 +680,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),
> diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
> index b4f3a17..e2502f2 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,17 @@ 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 */

Did you mean "If hart has support for CFI" here?

> +	if (sbi_hart_has_extension(sbi_scratch_thishart_ptr(), SBI_HART_EXT_ZICFILP)) {
> +#if __riscv_xlen == 32
> +		elp = regs->mstatusH & MSTATUSH_MPELP;
> +		regs->mstatusH &= ~MSTATUSH_MPELP;
> +#else
> +		elp = regs->mstatus & MSTATUS_MPELP;
> +		regs->mstatus &= ~MSTATUS_MPELP;
> +#endif
> +	}
> +
>  	/* If exceptions came from VS/VU-mode, redirect to VS-mode if
>  	 * delegated in hedeleg
>  	 */
> @@ -169,6 +181,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 */

Missing space before "if". With this fixed:

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

> +		if (elp)
> +			vsstatus |= MSTATUS_SPELP;
> +
>  		/* Set SPP for VS-mode */
>  		vsstatus &= ~SSTATUS_SPP;
>  		if (prev_mode == PRV_S)
> @@ -209,6 +225,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 v5 3/4] lib: sbi: sw check exception delegation

[Samuel Holland]

On 2024-09-10 11:35 AM, 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>
> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
> ---
>  lib/sbi/sbi_hart.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> index e1bc346..fa00f69 100644
> --- a/lib/sbi/sbi_hart.c
> +++ b/lib/sbi/sbi_hart.c
> @@ -207,7 +207,8 @@ static int delegate_traps(struct sbi_scratch *scratch)
>  	if (sbi_platform_has_mfaults_delegation(plat))
>  		exceptions |= (1U << CAUSE_FETCH_PAGE_FAULT) |
>  			      (1U << CAUSE_LOAD_PAGE_FAULT) |
> -			      (1U << CAUSE_STORE_PAGE_FAULT);
> +			      (1U << CAUSE_STORE_PAGE_FAULT)|

nit: missing space before |

> +			      (1U << CAUSE_SW_CHECK_EXCP);
>  
>  	/*
>  	 * If hypervisor extension available then we only handle hypervisor

[PATCH v5 4/4] lib: sbi: fwft: implement landing pad and shadow stack fwft interface

[Samuel Holland]

Hi Deepak,

On 2024-09-10 11:35 AM, Deepak Gupta wrote:
> Supervisor software can enable control flow integrity features for itself
> using fwft feature `SBI_FWFT_LANDING_PAD` and `SBI_FWFT_SHADOW_STACK`.
> This patch implements the mechanism to enable both these fwft.
> 
> Signed-off-by: Deepak Gupta <debug@rivosinc.com>
> Reviewed-by: Atish Patra <atishp@rivosinc.com>
> Reviewed-by: Cl?ment L?ger <cleger@rivosinc.com>
> ---
>  lib/sbi/sbi_fwft.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 74 insertions(+)
> 
> diff --git a/lib/sbi/sbi_fwft.c b/lib/sbi/sbi_fwft.c
> index ef881ef..747bc88 100644
> --- a/lib/sbi/sbi_fwft.c
> +++ b/lib/sbi/sbi_fwft.c
> @@ -145,6 +145,68 @@ static int fwft_get_adue(struct fwft_config *conf, unsigned long *value)
>  	return SBI_OK;
>  }
>  
> +static int fwft_lpad_supported(struct fwft_config *conf)
> +{
> +	if (!sbi_hart_has_extension(sbi_scratch_thishart_ptr(),
> +				    SBI_HART_EXT_ZICFILP))
> +		return SBI_ENOTSUPP;
> +
> +	return SBI_OK;
> +}
> +
> +static int fwft_enable_lpad(struct fwft_config *conf, unsigned long value)
> +{
> +	if (value == 1)
> +		csr_set(CSR_MENVCFG, ENVCFG_LPE);
> +	else if (value == 0)
> +		csr_clear(CSR_MENVCFG, ENVCFG_LPE);
> +	else
> +		return SBI_EINVAL;
> +
> +	return SBI_OK;
> +}
> +
> +static int fwft_get_lpad(struct fwft_config *conf, unsigned long *value)
> +{
> +	unsigned long cfg;
> +
> +	cfg = csr_read(CSR_MENVCFG) & ENVCFG_LPE;
> +	*value = cfg != 0;
> +
> +	return SBI_OK;
> +}
> +
> +static int fwft_sstack_supported(struct fwft_config *conf)
> +{
> +	if (!sbi_hart_has_extension(sbi_scratch_thishart_ptr(),
> +				    SBI_HART_EXT_ZICFISS))
> +		return SBI_ENOTSUPP;
> +
> +	return SBI_OK;
> +}
> +
> +static int fwft_enable_sstack(struct fwft_config *conf, unsigned long value)
> +{
> +	if (value == 1)
> +		csr_set(CSR_MENVCFG, ENVCFG_SSE);
> +	else if (value == 0)
> +		csr_clear(CSR_MENVCFG, ENVCFG_SSE);
> +	else
> +		return SBI_EINVAL;
> +
> +	return SBI_OK;
> +}
> +
> +static int fwft_get_sstack(struct fwft_config *conf, unsigned long *value)
> +{
> +	unsigned long cfg;
> +
> +	cfg = csr_read(CSR_MENVCFG) & ENVCFG_SSE;
> +	*value = cfg != 0;
> +
> +	return SBI_OK;
> +}
> +
>  static struct fwft_config* get_feature_config(enum sbi_fwft_feature_t feature)
>  {
>  	int i;
> @@ -236,6 +298,18 @@ static const struct fwft_feature features[] =
>  		.set = fwft_set_adue,
>  		.get = fwft_get_adue,
>  	},
> +	{
> +		.id = SBI_FWFT_LANDING_PAD,
> +		.supported = fwft_lpad_supported,
> +		.set = fwft_enable_lpad,
> +		.get = fwft_get_lpad,
> +	},
> +	{
> +		.id = SBI_FWFT_SHADOW_STACK,
> +		.supported = fwft_sstack_supported,
> +		.set = fwft_enable_sstack,
> +		.get = fwft_get_sstack,
> +	},

Please keep these blocks (and the new functions) sorted by feature ID, so above
ADUE.

Regards,
Samuel

>  };
>  
>  int sbi_fwft_init(struct sbi_scratch *scratch, bool cold_boot)

[PATCH v5 2/4] lib: sbi: add zicfilp/zicfiss and elp cfi state reflect back in status

[Atish Kumar Patra]

On Sat, Sep 14, 2024 at 10:04?AM Samuel Holland
<samuel.holland@sifive.com> wrote:
>
> On 2024-09-10 11:35 AM, Deepak Gupta wrote:
> > This patch adds support to check for zicfilp / zicfiss extension.
> >
> > zicfilp record 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>
> > ---
> >  include/sbi/sbi_hart.h |  3 +++
> >  lib/sbi/sbi_hart.c     |  2 ++
> >  lib/sbi/sbi_trap.c     | 20 ++++++++++++++++++++
> >  3 files changed, 25 insertions(+)
> >
> > 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,
> >

nit: Just to follow the convention above, let's split the comment
into individual extensions.

However, I am not sure if those comments provide any additional info
as most of the comments above the extension have the exact same extension
name in lowercase which is not that useful.

> >       /** Maximum index of Hart extension */
> >       SBI_HART_EXT_MAX,
> > diff --git a/lib/sbi/sbi_hart.c b/lib/sbi/sbi_hart.c
> > index c366701..e1bc346 100644
> > --- a/lib/sbi/sbi_hart.c
> > +++ b/lib/sbi/sbi_hart.c
> > @@ -680,6 +680,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),
> > diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
> > index b4f3a17..e2502f2 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,17 @@ 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 */
>
> Did you mean "If hart has support for CFI" here?
>
> > +     if (sbi_hart_has_extension(sbi_scratch_thishart_ptr(), SBI_HART_EXT_ZICFILP)) {
> > +#if __riscv_xlen == 32
> > +             elp = regs->mstatusH & MSTATUSH_MPELP;
> > +             regs->mstatusH &= ~MSTATUSH_MPELP;
> > +#else
> > +             elp = regs->mstatus & MSTATUS_MPELP;
> > +             regs->mstatus &= ~MSTATUS_MPELP;
> > +#endif
> > +     }
> > +
> >       /* If exceptions came from VS/VU-mode, redirect to VS-mode if
> >        * delegated in hedeleg
> >        */
> > @@ -169,6 +181,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 */
>
> Missing space before "if". With this fixed:
>
> Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
>
> > +             if (elp)
> > +                     vsstatus |= MSTATUS_SPELP;
> > +
> >               /* Set SPP for VS-mode */
> >               vsstatus &= ~SSTATUS_SPP;
> >               if (prev_mode == PRV_S)
> > @@ -209,6 +225,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;
>

With Samuel's suggestion fixed,

Reviewed-by: Atish Patra <atishp@rivosinc.com>