zicfilp and zicfiss support in opensbi

PATCH Superseded Deepak Gupta 2 participants 8 messages Aug 21, 2024 – Aug 22, 2024 RISC-V

Share & tools

Deepak Gupta

author

2024-08-21 23:55 UTC #0 lore raw

cover zicfilp and zicfiss support in opensbi

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(+)

Signature · 1 line

2.44.0

Deepak Gupta

author

2024-08-21 23:55 UTC #1 lore raw

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

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 <[email protected]>

---
 include/sbi/riscv_encoding.h | 6 ++++++
 include/sbi/sbi_hart.h       | 3 +++
 2 files changed, 9 insertions(+)

include/sbi/riscv_encoding.h +6 -0

index 2ed05f2..64cd9ab 100644
@@ -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

include/sbi/sbi_hart.h +3 -0

index 81ec061..2aa6867 100644
@@ -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,

Signature · 1 line

2.44.0

Samuel Holland

2024-08-22 00:03 UTC #2 lore raw

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

Hi Deepak,

Quoted 19 lines · 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 <[email protected]>
---
 include/sbi/riscv_encoding.h | 6 ++++++
 include/sbi/sbi_hart.h       | 3 +++
 2 files changed, 9 insertions(+)
include/sbi/riscv_encoding.h +0 -0
index 2ed05f2..64cd9ab 100644

include/sbi/riscv_encoding.h line 36

Quoted 5 lines

include/sbi/riscv_encoding.h +2 -0

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

include/sbi/riscv_encoding.h line 219

Quoted 9 lines

include/sbi/riscv_encoding.h +2 -0

 #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).

Quoted 24 lines

include/sbi/riscv_encoding.h +2 -0

 #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

include/sbi/sbi_hart.h +0 -0

index 81ec061..2aa6867 100644

include/sbi/sbi_hart.h enum sbi_hart_extensions { · line 72

Quoted 6 lines

include/sbi/sbi_hart.h +3 -0

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

Quoted 3 lines

include/sbi/sbi_hart.h +0 -0

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

Deepak Gupta

author

2024-08-21 23:55 UTC #3 lore raw

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

In reply to 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 <[email protected]>

---
 lib/sbi/sbi_hart.c | 28 ++++++++++++++++++++++++++++
 lib/sbi/sbi_trap.c | 16 ++++++++++++++++
 2 files changed, 44 insertions(+)

lib/sbi/sbi_hart.c +28 -0

index c366701..67a2e42 100644
@@ -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);

lib/sbi/sbi_trap.c +16 -0

index b4f3a17..2273b3a 100644
@@ -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;

Signature · 1 line

2.44.0

Samuel Holland

2024-08-22 00:32 UTC #4 lore raw

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

Hi Deepak,

Quoted 21 lines · 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 <[email protected]>
---
 lib/sbi/sbi_hart.c | 28 ++++++++++++++++++++++++++++
 lib/sbi/sbi_trap.c | 16 ++++++++++++++++
 2 files changed, 44 insertions(+)
lib/sbi/sbi_hart.c +0 -0
index c366701..67a2e42 100644
@@ -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;

lib/sbi/sbi_hart.c static void mstatus_init(struct sbi_scratch *scratch) · line 159

Quoted 10 lines

lib/sbi/sbi_hart.c +9 -0

 
+		/*
+		 * 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

Quoted 22 lines

lib/sbi/sbi_hart.c +4 -0

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

lib/sbi/sbi_hart.c __pmp_skip: · line 951

Quoted 6 lines

lib/sbi/sbi_hart.c +3 -0

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

Quoted 28 lines

lib/sbi/sbi_hart.c +12 -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);

lib/sbi/sbi_trap.c +1 -0

index b4f3a17..2273b3a 100644
@@ -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

lib/sbi/sbi_trap.c int sbi_trap_redirect(struct sbi_trap_regs *regs, · line 122

Quoted 6 lines

lib/sbi/sbi_trap.c +3 -0

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

Quoted 29 lines

lib/sbi/sbi_trap.c +12 -0

+		/* 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;

Deepak Gupta

author

2024-08-22 06:37 UTC #5 lore raw

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

Quoted 38 lines · Samuel Holland

Hi Deepak,
Quoted 21 lines · 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 <[email protected]>
---
 lib/sbi/sbi_hart.c | 28 ++++++++++++++++++++++++++++
 lib/sbi/sbi_trap.c | 16 ++++++++++++++++
 2 files changed, 44 insertions(+)
lib/sbi/sbi_hart.c +0 -0
index c366701..67a2e42 100644
@@ -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;
lib/sbi/sbi_hart.c static void mstatus_init(struct sbi_scratch *scratch) · line 159
Quoted 10 lines
lib/sbi/sbi_hart.c +9 -0
 
+		/*
+		 * 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].

Quoted 3 lines

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

Quoted 25 lines

lib/sbi/sbi_hart.c +4 -0

+
 		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);

Quoted 6 lines

lib/sbi/sbi_hart.c __pmp_skip: · line 951
Quoted 3 lines
lib/sbi/sbi_hart.c +3 -0
+	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.

Quoted 32 lines

lib/sbi/sbi_hart.c +12 -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);

lib/sbi/sbi_trap.c +1 -0

index b4f3a17..2273b3a 100644
@@ -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;

Quoted 5 lines

lib/sbi/sbi_trap.c int sbi_trap_redirect(struct sbi_trap_regs *regs, · line 122
Quoted 3 lines
lib/sbi/sbi_trap.c +3 -0
+	/* 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.

Quoted 34 lines

Regards,
Samuel

Quoted 29 lines

lib/sbi/sbi_trap.c +12 -0

+		/* 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;

Deepak Gupta

author

2024-08-21 23:55 UTC #6 lore raw

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

In reply to 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 <[email protected]>

---
 lib/sbi/sbi_hart.c | 3 +++
 1 file changed, 3 insertions(+)

lib/sbi/sbi_hart.c +3 -0

index 67a2e42..5bc02a1 100644
@@ -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);

Signature · 1 line

2.44.0

Samuel Holland

2024-08-22 00:36 UTC #7 lore raw

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

Hi Deepak,

Quoted 14 lines · 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 <[email protected]>
---
 lib/sbi/sbi_hart.c | 3 +++
 1 file changed, 3 insertions(+)
lib/sbi/sbi_hart.c +0 -0
index 67a2e42..5bc02a1 100644

lib/sbi/sbi_hart.c static int delegate_traps(struct sbi_scratch *scratch) · line 238

Quoted 5 lines

lib/sbi/sbi_hart.c +2 -0

@@ -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 <[email protected]>

Quoted 4 lines

lib/sbi/sbi_hart.c +1 -0

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