OpenSBI Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 00/10] Improve trap handling for nested traps
@ 2024-03-17 13:02 Anup Patel
  2024-03-17 13:02 ` [PATCH v3 01/10] lib: sbi: Remove epc from struct sbi_trap_info Anup Patel
                   ` (10 more replies)
  0 siblings, 11 replies; 12+ messages in thread
From: Anup Patel @ 2024-03-17 13:02 UTC (permalink / raw)
  To: opensbi

Nested traps will be a common when dealing with RAS error traps so
this series improves trap handling for nested traps by introducing
a linked-list based trap context chain.

These patches can also be found the trap_handling_imp_v3 branch at
https://github.com/avpatel/opensbi.git

Changes since v2:
 - Addressed comments in PATCH4, PATCH6, PATCH7, PATCH8, and PATCH10.

Changes since v1:
 - Include samuel's patch in the series
 - Embed sbi_trap_regs and sbi_trap_info in the new
   struct sbi_trap_context
 - Drop the mcause parameter of sbi_trap_aia_irq()

Anup Patel (9):
  lib: sbi: Remove sbi_trap_exit() and related code
  include: sbi: Add trap_context pointer in struct sbi_scratch
  lib: sbi: Introduce trap context
  lib: sbi: Simplify parameters of misaligned and access fault handlers
  lib: sbi: Simplify parameters of sbi_illegal_insn_handler()
  lib: sbi: Remove regs paramter of sbi_irqchip_process()
  lib: sbi: Remove regs parameter from trap irq handling functions
  lib: sbi: Pass trap context pointer to sbi_ecall_handler()
  lib: sbi: Extend sbi_trap_error() to dump state in a nested trap

Samuel Holland (1):
  lib: sbi: Remove epc from struct sbi_trap_info

 firmware/fw_base.S             |  76 +++++++++----
 include/sbi/riscv_encoding.h   |   2 +
 include/sbi/sbi_ecall.h        |   4 +-
 include/sbi/sbi_illegal_insn.h |   4 +-
 include/sbi/sbi_irqchip.h      |   5 +-
 include/sbi/sbi_scratch.h      |  14 +--
 include/sbi/sbi_trap.h         |  45 ++++++--
 include/sbi/sbi_trap_ldst.h    |  12 +-
 lib/sbi/sbi_ecall.c            |   3 +-
 lib/sbi/sbi_ecall_legacy.c     |   4 -
 lib/sbi/sbi_expected_trap.S    |   4 -
 lib/sbi/sbi_illegal_insn.c     |   9 +-
 lib/sbi/sbi_irqchip.c          |  10 +-
 lib/sbi/sbi_trap.c             | 196 +++++++++++++++------------------
 lib/sbi/sbi_trap_ldst.c        |  71 ++++++------
 lib/utils/irqchip/imsic.c      |   2 +-
 16 files changed, 239 insertions(+), 222 deletions(-)

-- 
2.34.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3 01/10] lib: sbi: Remove epc from struct sbi_trap_info
  2024-03-17 13:02 [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel
@ 2024-03-17 13:02 ` Anup Patel
  2024-03-17 13:02 ` [PATCH v3 02/10] lib: sbi: Remove sbi_trap_exit() and related code Anup Patel
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Anup Patel @ 2024-03-17 13:02 UTC (permalink / raw)
  To: opensbi

From: Samuel Holland <samuel.holland@sifive.com>

In the only places this value is used, it duplicates mepc from
struct sbi_trap_regs.

Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
 include/sbi/sbi_trap.h      | 16 ++++++----------
 lib/sbi/sbi_ecall_legacy.c  |  4 ----
 lib/sbi/sbi_expected_trap.S |  4 ----
 lib/sbi/sbi_illegal_insn.c  |  2 --
 lib/sbi/sbi_trap.c          |  5 ++---
 lib/sbi/sbi_trap_ldst.c     |  4 ----
 6 files changed, 8 insertions(+), 27 deletions(-)

diff --git a/include/sbi/sbi_trap.h b/include/sbi/sbi_trap.h
index 2727bdb..3757694 100644
--- a/include/sbi/sbi_trap.h
+++ b/include/sbi/sbi_trap.h
@@ -87,20 +87,18 @@
 /** Last member index in sbi_trap_regs */
 #define SBI_TRAP_REGS_last			35
 
-/** Index of epc member in sbi_trap_info */
-#define SBI_TRAP_INFO_epc			0
 /** Index of cause member in sbi_trap_info */
-#define SBI_TRAP_INFO_cause			1
+#define SBI_TRAP_INFO_cause			0
 /** Index of tval member in sbi_trap_info */
-#define SBI_TRAP_INFO_tval			2
+#define SBI_TRAP_INFO_tval			1
 /** Index of tval2 member in sbi_trap_info */
-#define SBI_TRAP_INFO_tval2			3
+#define SBI_TRAP_INFO_tval2			2
 /** Index of tinst member in sbi_trap_info */
-#define SBI_TRAP_INFO_tinst			4
+#define SBI_TRAP_INFO_tinst			3
 /** Index of gva member in sbi_trap_info */
-#define SBI_TRAP_INFO_gva			5
+#define SBI_TRAP_INFO_gva			4
 /** Last member index in sbi_trap_info */
-#define SBI_TRAP_INFO_last			6
+#define SBI_TRAP_INFO_last			5
 
 /* clang-format on */
 
@@ -194,8 +192,6 @@ struct sbi_trap_regs {
 
 /** Representation of trap details */
 struct sbi_trap_info {
-	/** epc Trap program counter */
-	unsigned long epc;
 	/** cause Trap exception cause */
 	unsigned long cause;
 	/** tval Trap value */
diff --git a/lib/sbi/sbi_ecall_legacy.c b/lib/sbi/sbi_ecall_legacy.c
index 14913c9..9a1ae1e 100644
--- a/lib/sbi/sbi_ecall_legacy.c
+++ b/lib/sbi/sbi_ecall_legacy.c
@@ -74,7 +74,6 @@ static int sbi_ecall_legacy_handler(unsigned long extid, unsigned long funcid,
 						&hmask, &trap)) {
 			ret = sbi_ipi_send_smode(hmask, 0);
 		} else {
-			trap.epc = regs->mepc;
 			sbi_trap_redirect(regs, &trap);
 			out->skip_regs_update = true;
 		}
@@ -86,7 +85,6 @@ static int sbi_ecall_legacy_handler(unsigned long extid, unsigned long funcid,
 					  SBI_TLB_FENCE_I, source_hart);
 			ret = sbi_tlb_request(hmask, 0, &tlb_info);
 		} else {
-			trap.epc = regs->mepc;
 			sbi_trap_redirect(regs, &trap);
 			out->skip_regs_update = true;
 		}
@@ -98,7 +96,6 @@ static int sbi_ecall_legacy_handler(unsigned long extid, unsigned long funcid,
 					  SBI_TLB_SFENCE_VMA, source_hart);
 			ret = sbi_tlb_request(hmask, 0, &tlb_info);
 		} else {
-			trap.epc = regs->mepc;
 			sbi_trap_redirect(regs, &trap);
 			out->skip_regs_update = true;
 		}
@@ -112,7 +109,6 @@ static int sbi_ecall_legacy_handler(unsigned long extid, unsigned long funcid,
 					  source_hart);
 			ret = sbi_tlb_request(hmask, 0, &tlb_info);
 		} else {
-			trap.epc = regs->mepc;
 			sbi_trap_redirect(regs, &trap);
 			out->skip_regs_update = true;
 		}
diff --git a/lib/sbi/sbi_expected_trap.S b/lib/sbi/sbi_expected_trap.S
index 1f2d6b9..99dede5 100644
--- a/lib/sbi/sbi_expected_trap.S
+++ b/lib/sbi/sbi_expected_trap.S
@@ -23,8 +23,6 @@
 	.global __sbi_expected_trap
 __sbi_expected_trap:
 	/* Without H-extension so, MTVAL2 and MTINST CSRs and GVA not available */
-	csrr	a4, CSR_MEPC
-	REG_S	a4, SBI_TRAP_INFO_OFFSET(epc)(a3)
 	csrr	a4, CSR_MCAUSE
 	REG_S	a4, SBI_TRAP_INFO_OFFSET(cause)(a3)
 	csrr	a4, CSR_MTVAL
@@ -41,8 +39,6 @@ __sbi_expected_trap:
 	.global __sbi_expected_trap_hext
 __sbi_expected_trap_hext:
 	/* With H-extension so, MTVAL2 and MTINST CSRs and GVA available */
-	csrr	a4, CSR_MEPC
-	REG_S	a4, SBI_TRAP_INFO_OFFSET(epc)(a3)
 	csrr	a4, CSR_MCAUSE
 	REG_S	a4, SBI_TRAP_INFO_OFFSET(cause)(a3)
 	csrr	a4, CSR_MTVAL
diff --git a/lib/sbi/sbi_illegal_insn.c b/lib/sbi/sbi_illegal_insn.c
index 2be4757..dd0b3c1 100644
--- a/lib/sbi/sbi_illegal_insn.c
+++ b/lib/sbi/sbi_illegal_insn.c
@@ -25,7 +25,6 @@ static int truly_illegal_insn(ulong insn, struct sbi_trap_regs *regs)
 {
 	struct sbi_trap_info trap;
 
-	trap.epc = regs->mepc;
 	trap.cause = CAUSE_ILLEGAL_INSTRUCTION;
 	trap.tval = insn;
 	trap.tval2 = 0;
@@ -156,7 +155,6 @@ int sbi_illegal_insn_handler(ulong insn, struct sbi_trap_regs *regs)
 	if (unlikely((insn & 3) != 3)) {
 		insn = sbi_get_insn(regs->mepc, &uptrap);
 		if (uptrap.cause) {
-			uptrap.epc = regs->mepc;
 			return sbi_trap_redirect(regs, &uptrap);
 		}
 		if ((insn & 3) != 3)
diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
index c665013..c9db73f 100644
--- a/lib/sbi/sbi_trap.c
+++ b/lib/sbi/sbi_trap.c
@@ -140,7 +140,7 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
 	if (next_virt) {
 		/* Update VS-mode exception info */
 		csr_write(CSR_VSTVAL, trap->tval);
-		csr_write(CSR_VSEPC, trap->epc);
+		csr_write(CSR_VSEPC, regs->mepc);
 		csr_write(CSR_VSCAUSE, trap->cause);
 
 		/* Set MEPC to VS-mode exception vector base */
@@ -171,7 +171,7 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
 	} else {
 		/* Update S-mode exception info */
 		csr_write(CSR_STVAL, trap->tval);
-		csr_write(CSR_SEPC, trap->epc);
+		csr_write(CSR_SEPC, regs->mepc);
 		csr_write(CSR_SCAUSE, trap->cause);
 
 		/* Set MEPC to S-mode exception vector base */
@@ -286,7 +286,6 @@ struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs)
 		return regs;
 	}
 	/* Original trap_info */
-	trap.epc   = regs->mepc;
 	trap.cause = mcause;
 	trap.tval  = mtval;
 	trap.tval2 = mtval2;
diff --git a/lib/sbi/sbi_trap_ldst.c b/lib/sbi/sbi_trap_ldst.c
index d864ad1..5a0537b 100644
--- a/lib/sbi/sbi_trap_ldst.c
+++ b/lib/sbi/sbi_trap_ldst.c
@@ -70,7 +70,6 @@ static int sbi_trap_emulate_load(struct sbi_trap_regs *regs,
 		 */
 		insn = sbi_get_insn(regs->mepc, &uptrap);
 		if (uptrap.cause) {
-			uptrap.epc = regs->mepc;
 			return sbi_trap_redirect(regs, &uptrap);
 		}
 		insn_len = INSN_LEN(insn);
@@ -193,7 +192,6 @@ static int sbi_trap_emulate_store(struct sbi_trap_regs *regs,
 		 */
 		insn = sbi_get_insn(regs->mepc, &uptrap);
 		if (uptrap.cause) {
-			uptrap.epc = regs->mepc;
 			return sbi_trap_redirect(regs, &uptrap);
 		}
 		insn_len = INSN_LEN(insn);
@@ -277,7 +275,6 @@ static int sbi_misaligned_ld_emulator(int rlen, union sbi_ldst_data *out_val,
 		out_val->data_bytes[i] =
 			sbi_load_u8((void *)(orig_trap->tval + i), &uptrap);
 		if (uptrap.cause) {
-			uptrap.epc   = regs->mepc;
 			uptrap.tinst = sbi_misaligned_tinst_fixup(
 				orig_trap->tinst, uptrap.tinst, i);
 			return sbi_trap_redirect(regs, &uptrap);
@@ -304,7 +301,6 @@ static int sbi_misaligned_st_emulator(int wlen, union sbi_ldst_data in_val,
 		sbi_store_u8((void *)(orig_trap->tval + i),
 			     in_val.data_bytes[i], &uptrap);
 		if (uptrap.cause) {
-			uptrap.epc   = regs->mepc;
 			uptrap.tinst = sbi_misaligned_tinst_fixup(
 				orig_trap->tinst, uptrap.tinst, i);
 			return sbi_trap_redirect(regs, &uptrap);
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 02/10] lib: sbi: Remove sbi_trap_exit() and related code
  2024-03-17 13:02 [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel
  2024-03-17 13:02 ` [PATCH v3 01/10] lib: sbi: Remove epc from struct sbi_trap_info Anup Patel
@ 2024-03-17 13:02 ` Anup Patel
  2024-03-17 13:02 ` [PATCH v3 03/10] include: sbi: Add trap_context pointer in struct sbi_scratch Anup Patel
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Anup Patel @ 2024-03-17 13:02 UTC (permalink / raw)
  To: opensbi

Over the years, no uses of sbi_trap_exit() have been found so remove
it and also remove related code from fw_base.S and sbi_scratch.h.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
Tested-by: Samuel Holland <samuel.holland@sifive.com>
---
 firmware/fw_base.S        | 11 -----------
 include/sbi/sbi_scratch.h | 15 +++------------
 include/sbi/sbi_trap.h    |  2 --
 lib/sbi/sbi_trap.c        | 19 -------------------
 4 files changed, 3 insertions(+), 44 deletions(-)

diff --git a/firmware/fw_base.S b/firmware/fw_base.S
index 126b067..c404d8b 100644
--- a/firmware/fw_base.S
+++ b/firmware/fw_base.S
@@ -298,9 +298,6 @@ _scratch_init:
 	/* Store hartid-to-scratch function address in scratch space */
 	lla	a4, _hartid_to_scratch
 	REG_S	a4, SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET(tp)
-	/* Store trap-exit function address in scratch space */
-	lla	a4, _trap_exit
-	REG_S	a4, SBI_SCRATCH_TRAP_EXIT_OFFSET(tp)
 	/* Clear tmp0 in scratch space */
 	REG_S	zero, SBI_SCRATCH_TMP0_OFFSET(tp)
 	/* Store firmware options in scratch space */
@@ -453,10 +450,6 @@ _start_warm:
 	srli	a5, a5, ('H' - 'A')
 	andi	a5, a5, 0x1
 	beq	a5, zero, _skip_trap_handler_rv32_hyp
-	/* Override trap exit for H-extension */
-	csrr	a5, CSR_MSCRATCH
-	lla	a4, _trap_exit_rv32_hyp
-	REG_S	a4, SBI_SCRATCH_TRAP_EXIT_OFFSET(a5)
 	lla	a4, _trap_handler_rv32_hyp
 _skip_trap_handler_rv32_hyp:
 #endif
@@ -695,7 +688,6 @@ memcmp:
 	.section .entry, "ax", %progbits
 	.align 3
 	.globl _trap_handler
-	.globl _trap_exit
 _trap_handler:
 	TRAP_SAVE_AND_SETUP_SP_T0
 
@@ -705,7 +697,6 @@ _trap_handler:
 
 	TRAP_CALL_C_ROUTINE
 
-_trap_exit:
 	TRAP_RESTORE_GENERAL_REGS_EXCEPT_A0_T0
 
 	TRAP_RESTORE_MEPC_MSTATUS 0
@@ -718,7 +709,6 @@ _trap_exit:
 	.section .entry, "ax", %progbits
 	.align 3
 	.globl _trap_handler_rv32_hyp
-	.globl _trap_exit_rv32_hyp
 _trap_handler_rv32_hyp:
 	TRAP_SAVE_AND_SETUP_SP_T0
 
@@ -728,7 +718,6 @@ _trap_handler_rv32_hyp:
 
 	TRAP_CALL_C_ROUTINE
 
-_trap_exit_rv32_hyp:
 	TRAP_RESTORE_GENERAL_REGS_EXCEPT_A0_T0
 
 	TRAP_RESTORE_MEPC_MSTATUS 1
diff --git a/include/sbi/sbi_scratch.h b/include/sbi/sbi_scratch.h
index e6a33ba..55b937f 100644
--- a/include/sbi/sbi_scratch.h
+++ b/include/sbi/sbi_scratch.h
@@ -36,14 +36,12 @@
 #define SBI_SCRATCH_PLATFORM_ADDR_OFFSET	(9 * __SIZEOF_POINTER__)
 /** Offset of hartid_to_scratch member in sbi_scratch */
 #define SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET	(10 * __SIZEOF_POINTER__)
-/** Offset of trap_exit member in sbi_scratch */
-#define SBI_SCRATCH_TRAP_EXIT_OFFSET		(11 * __SIZEOF_POINTER__)
 /** Offset of tmp0 member in sbi_scratch */
-#define SBI_SCRATCH_TMP0_OFFSET			(12 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_TMP0_OFFSET			(11 * __SIZEOF_POINTER__)
 /** Offset of options member in sbi_scratch */
-#define SBI_SCRATCH_OPTIONS_OFFSET		(13 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_OPTIONS_OFFSET		(12 * __SIZEOF_POINTER__)
 /** Offset of extra space in sbi_scratch */
-#define SBI_SCRATCH_EXTRA_SPACE_OFFSET		(14 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_EXTRA_SPACE_OFFSET		(13 * __SIZEOF_POINTER__)
 /** Maximum size of sbi_scratch (4KB) */
 #define SBI_SCRATCH_SIZE			(0x1000)
 
@@ -77,8 +75,6 @@ struct sbi_scratch {
 	unsigned long platform_addr;
 	/** Address of HART ID to sbi_scratch conversion function */
 	unsigned long hartid_to_scratch;
-	/** Address of trap exit function */
-	unsigned long trap_exit;
 	/** Temporary storage */
 	unsigned long tmp0;
 	/** Options for OpenSBI library */
@@ -129,11 +125,6 @@ _Static_assert(
 		== SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET,
 	"struct sbi_scratch definition has changed, please redefine "
 	"SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET");
-_Static_assert(
-	offsetof(struct sbi_scratch, trap_exit)
-		== SBI_SCRATCH_TRAP_EXIT_OFFSET,
-	"struct sbi_scratch definition has changed, please redefine "
-	"SBI_SCRATCH_TRAP_EXIT_OFFSET");
 _Static_assert(
 	offsetof(struct sbi_scratch, tmp0)
 		== SBI_SCRATCH_TMP0_OFFSET,
diff --git a/include/sbi/sbi_trap.h b/include/sbi/sbi_trap.h
index 3757694..a6032ab 100644
--- a/include/sbi/sbi_trap.h
+++ b/include/sbi/sbi_trap.h
@@ -225,8 +225,6 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
 
 struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs);
 
-void __noreturn sbi_trap_exit(const struct sbi_trap_regs *regs);
-
 #endif
 
 #endif
diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
index c9db73f..b059c4b 100644
--- a/lib/sbi/sbi_trap.c
+++ b/lib/sbi/sbi_trap.c
@@ -334,22 +334,3 @@ trap_error:
 		sbi_trap_error(msg, rc, mcause, mtval, mtval2, mtinst, regs);
 	return regs;
 }
-
-typedef void (*trap_exit_t)(const struct sbi_trap_regs *regs);
-
-/**
- * Exit trap/interrupt handling
- *
- * This function is called by non-firmware code to abruptly exit
- * trap/interrupt handling and resume execution at context pointed
- * by given register state.
- *
- * @param regs pointer to register state
- */
-void __noreturn sbi_trap_exit(const struct sbi_trap_regs *regs)
-{
-	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
-
-	((trap_exit_t)scratch->trap_exit)(regs);
-	__builtin_unreachable();
-}
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 03/10] include: sbi: Add trap_context pointer in struct sbi_scratch
  2024-03-17 13:02 [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel
  2024-03-17 13:02 ` [PATCH v3 01/10] lib: sbi: Remove epc from struct sbi_trap_info Anup Patel
  2024-03-17 13:02 ` [PATCH v3 02/10] lib: sbi: Remove sbi_trap_exit() and related code Anup Patel
@ 2024-03-17 13:02 ` Anup Patel
  2024-03-17 13:02 ` [PATCH v3 04/10] lib: sbi: Introduce trap context Anup Patel
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Anup Patel @ 2024-03-17 13:02 UTC (permalink / raw)
  To: opensbi

To track nested traps, the struct sbi_scratch needs a pointer the
current trap context so add trap_context pointer in struct sbi_context.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
Tested-by: Samuel Holland <samuel.holland@sifive.com>
---
 firmware/fw_base.S        |  3 ++-
 include/sbi/sbi_scratch.h | 15 ++++++++++++---
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/firmware/fw_base.S b/firmware/fw_base.S
index c404d8b..539fd1d 100644
--- a/firmware/fw_base.S
+++ b/firmware/fw_base.S
@@ -298,7 +298,8 @@ _scratch_init:
 	/* Store hartid-to-scratch function address in scratch space */
 	lla	a4, _hartid_to_scratch
 	REG_S	a4, SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET(tp)
-	/* Clear tmp0 in scratch space */
+	/* Clear trap_context and tmp0 in scratch space */
+	REG_S	zero, SBI_SCRATCH_TRAP_CONTEXT_OFFSET(tp)
 	REG_S	zero, SBI_SCRATCH_TMP0_OFFSET(tp)
 	/* Store firmware options in scratch space */
 	MOV_3R	s0, a0, s1, a1, s2, a2
diff --git a/include/sbi/sbi_scratch.h b/include/sbi/sbi_scratch.h
index 55b937f..12e6a98 100644
--- a/include/sbi/sbi_scratch.h
+++ b/include/sbi/sbi_scratch.h
@@ -36,12 +36,14 @@
 #define SBI_SCRATCH_PLATFORM_ADDR_OFFSET	(9 * __SIZEOF_POINTER__)
 /** Offset of hartid_to_scratch member in sbi_scratch */
 #define SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET	(10 * __SIZEOF_POINTER__)
+/** Offset of trap_context member in sbi_scratch */
+#define SBI_SCRATCH_TRAP_CONTEXT_OFFSET		(11 * __SIZEOF_POINTER__)
 /** Offset of tmp0 member in sbi_scratch */
-#define SBI_SCRATCH_TMP0_OFFSET			(11 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_TMP0_OFFSET			(12 * __SIZEOF_POINTER__)
 /** Offset of options member in sbi_scratch */
-#define SBI_SCRATCH_OPTIONS_OFFSET		(12 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_OPTIONS_OFFSET		(13 * __SIZEOF_POINTER__)
 /** Offset of extra space in sbi_scratch */
-#define SBI_SCRATCH_EXTRA_SPACE_OFFSET		(13 * __SIZEOF_POINTER__)
+#define SBI_SCRATCH_EXTRA_SPACE_OFFSET		(14 * __SIZEOF_POINTER__)
 /** Maximum size of sbi_scratch (4KB) */
 #define SBI_SCRATCH_SIZE			(0x1000)
 
@@ -75,6 +77,8 @@ struct sbi_scratch {
 	unsigned long platform_addr;
 	/** Address of HART ID to sbi_scratch conversion function */
 	unsigned long hartid_to_scratch;
+	/** Address of current trap context */
+	unsigned long trap_context;
 	/** Temporary storage */
 	unsigned long tmp0;
 	/** Options for OpenSBI library */
@@ -125,6 +129,11 @@ _Static_assert(
 		== SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET,
 	"struct sbi_scratch definition has changed, please redefine "
 	"SBI_SCRATCH_HARTID_TO_SCRATCH_OFFSET");
+_Static_assert(
+	offsetof(struct sbi_scratch, trap_context)
+		== SBI_SCRATCH_TRAP_CONTEXT_OFFSET,
+	"struct sbi_scratch definition has changed, please redefine "
+	"SBI_SCRATCH_TRAP_CONTEXT_OFFSET");
 _Static_assert(
 	offsetof(struct sbi_scratch, tmp0)
 		== SBI_SCRATCH_TMP0_OFFSET,
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 04/10] lib: sbi: Introduce trap context
  2024-03-17 13:02 [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel
                   ` (2 preceding siblings ...)
  2024-03-17 13:02 ` [PATCH v3 03/10] include: sbi: Add trap_context pointer in struct sbi_scratch Anup Patel
@ 2024-03-17 13:02 ` Anup Patel
  2024-03-17 13:02 ` [PATCH v3 05/10] lib: sbi: Simplify parameters of misaligned and access fault handlers Anup Patel
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Anup Patel @ 2024-03-17 13:02 UTC (permalink / raw)
  To: opensbi

Club the struct sbi_trap_regs and struct sbi_trap_info a new
struct sbi_trap_context (aka trap context) which must be saved
by low-level trap handler before calling sbi_trap_handler().

To track nested traps, the struct sbi_scratch points to the current
trap context and the trap context has pointer to pervious context
of previous trap.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
Tested-by: Samuel Holland <samuel.holland@sifive.com>
---
 firmware/fw_base.S     | 62 ++++++++++++++++++++++++++++++++++--------
 include/sbi/sbi_trap.h | 29 +++++++++++++++++++-
 lib/sbi/sbi_trap.c     | 58 +++++++++++++++++----------------------
 3 files changed, 103 insertions(+), 46 deletions(-)

diff --git a/firmware/fw_base.S b/firmware/fw_base.S
index 539fd1d..6290322 100644
--- a/firmware/fw_base.S
+++ b/firmware/fw_base.S
@@ -446,14 +446,12 @@ _start_warm:
 
 	/* Setup trap handler */
 	lla	a4, _trap_handler
-#if __riscv_xlen == 32
 	csrr	a5, CSR_MISA
 	srli	a5, a5, ('H' - 'A')
 	andi	a5, a5, 0x1
-	beq	a5, zero, _skip_trap_handler_rv32_hyp
-	lla	a4, _trap_handler_rv32_hyp
-_skip_trap_handler_rv32_hyp:
-#endif
+	beq	a5, zero, _skip_trap_handler_hyp
+	lla	a4, _trap_handler_hyp
+_skip_trap_handler_hyp:
 	csrw	CSR_MTVEC, a4
 
 	/* Initialize SBI runtime */
@@ -564,10 +562,10 @@ memcmp:
 	xor	t0, tp, t0
 
 	/* Save original SP on exception stack */
-	REG_S	sp, (SBI_TRAP_REGS_OFFSET(sp) - SBI_TRAP_REGS_SIZE)(t0)
+	REG_S	sp, (SBI_TRAP_REGS_OFFSET(sp) - SBI_TRAP_CONTEXT_SIZE)(t0)
 
-	/* Set SP to exception stack and make room for trap registers */
-	add	sp, t0, -(SBI_TRAP_REGS_SIZE)
+	/* Set SP to exception stack and make room for trap context */
+	add	sp, t0, -(SBI_TRAP_CONTEXT_SIZE)
 
 	/* Restore T0 from scratch space */
 	REG_L	t0, SBI_SCRATCH_TMP0_OFFSET(tp)
@@ -627,6 +625,32 @@ memcmp:
 	REG_S	t6, SBI_TRAP_REGS_OFFSET(t6)(sp)
 .endm
 
+.macro	TRAP_SAVE_INFO have_mstatush have_h_extension
+	csrr	t0, CSR_MCAUSE
+	REG_S	t0, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(cause))(sp)
+	csrr	t0, CSR_MTVAL
+	REG_S	t0, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(tval))(sp)
+.if \have_h_extension
+	csrr	t0, CSR_MTVAL2
+	REG_S	t0, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(tval2))(sp)
+	csrr	t0, CSR_MTINST
+	REG_S	t0, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(tinst))(sp)
+	.if \have_mstatush
+	csrr	t0, CSR_MSTATUSH
+	srli	t0, t0, MSTATUSH_GVA_SHIFT
+	.else
+	csrr	t0, CSR_MSTATUS
+	srli	t0, t0, MSTATUS_GVA_SHIFT
+	.endif
+	and	t0, t0, 0x1
+.else
+	REG_S	zero, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(tval2))(sp)
+	REG_S	zero, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(tinst))(sp)
+	li	t0, 0
+.endif
+	REG_S	t0, (SBI_TRAP_REGS_SIZE + SBI_TRAP_INFO_OFFSET(gva))(sp)
+.endm
+
 .macro	TRAP_CALL_C_ROUTINE
 	/* Call C routine */
 	add	a0, sp, zero
@@ -696,6 +720,8 @@ _trap_handler:
 
 	TRAP_SAVE_GENERAL_REGS_EXCEPT_SP_T0
 
+	TRAP_SAVE_INFO 0 0
+
 	TRAP_CALL_C_ROUTINE
 
 	TRAP_RESTORE_GENERAL_REGS_EXCEPT_A0_T0
@@ -706,27 +732,39 @@ _trap_handler:
 
 	mret
 
-#if __riscv_xlen == 32
 	.section .entry, "ax", %progbits
 	.align 3
-	.globl _trap_handler_rv32_hyp
-_trap_handler_rv32_hyp:
+	.globl _trap_handler_hyp
+_trap_handler_hyp:
 	TRAP_SAVE_AND_SETUP_SP_T0
 
+#if __riscv_xlen == 32
 	TRAP_SAVE_MEPC_MSTATUS 1
+#else
+	TRAP_SAVE_MEPC_MSTATUS 0
+#endif
 
 	TRAP_SAVE_GENERAL_REGS_EXCEPT_SP_T0
 
+#if __riscv_xlen == 32
+	TRAP_SAVE_INFO 1 1
+#else
+	TRAP_SAVE_INFO 0 1
+#endif
+
 	TRAP_CALL_C_ROUTINE
 
 	TRAP_RESTORE_GENERAL_REGS_EXCEPT_A0_T0
 
+#if __riscv_xlen == 32
 	TRAP_RESTORE_MEPC_MSTATUS 1
+#else
+	TRAP_RESTORE_MEPC_MSTATUS 0
+#endif
 
 	TRAP_RESTORE_A0_T0
 
 	mret
-#endif
 
 	.section .entry, "ax", %progbits
 	.align 3
diff --git a/include/sbi/sbi_trap.h b/include/sbi/sbi_trap.h
index a6032ab..f7c170e 100644
--- a/include/sbi/sbi_trap.h
+++ b/include/sbi/sbi_trap.h
@@ -112,9 +112,15 @@
 /** Size (in bytes) of sbi_trap_info */
 #define SBI_TRAP_INFO_SIZE SBI_TRAP_INFO_OFFSET(last)
 
+/** Size (in bytes) of sbi_trap_context */
+#define SBI_TRAP_CONTEXT_SIZE (SBI_TRAP_REGS_SIZE + \
+			       SBI_TRAP_INFO_SIZE + \
+			       __SIZEOF_POINTER__)
+
 #ifndef __ASSEMBLER__
 
 #include <sbi/sbi_types.h>
+#include <sbi/sbi_scratch.h>
 
 /** Representation of register state at time of trap/interrupt */
 struct sbi_trap_regs {
@@ -204,6 +210,16 @@ struct sbi_trap_info {
 	unsigned long gva;
 };
 
+/** Representation of trap context saved on stack */
+struct sbi_trap_context {
+	/** Register state */
+	struct sbi_trap_regs regs;
+	/** Trap details */
+	struct sbi_trap_info trap;
+	/** Pointer to previous trap context */
+	struct sbi_trap_context *prev_context;
+};
+
 static inline unsigned long sbi_regs_gva(const struct sbi_trap_regs *regs)
 {
 	/*
@@ -223,7 +239,18 @@ static inline unsigned long sbi_regs_gva(const struct sbi_trap_regs *regs)
 int sbi_trap_redirect(struct sbi_trap_regs *regs,
 		      const struct sbi_trap_info *trap);
 
-struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs);
+static inline struct sbi_trap_context *sbi_trap_get_context(struct sbi_scratch *scratch)
+{
+	return (scratch) ? (void *)scratch->trap_context : NULL;
+}
+
+static inline void sbi_trap_set_context(struct sbi_scratch *scratch,
+					struct sbi_trap_context *tcntx)
+{
+	scratch->trap_context = (unsigned long)tcntx;
+}
+
+struct sbi_trap_context *sbi_trap_handler(struct sbi_trap_context *tcntx);
 
 #endif
 
diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
index b059c4b..4dbda06 100644
--- a/lib/sbi/sbi_trap.c
+++ b/lib/sbi/sbi_trap.c
@@ -24,18 +24,18 @@
 #include <sbi/sbi_trap.h>
 
 static void __noreturn sbi_trap_error(const char *msg, int rc,
-				      ulong mcause, ulong mtval, ulong mtval2,
-				      ulong mtinst, struct sbi_trap_regs *regs)
+				      const struct sbi_trap_info *trap,
+				      const struct sbi_trap_regs *regs)
 {
 	u32 hartid = current_hartid();
 
 	sbi_printf("%s: hart%d: %s (error %d)\n", __func__, hartid, msg, rc);
 	sbi_printf("%s: hart%d: mcause=0x%" PRILX " mtval=0x%" PRILX "\n",
-		   __func__, hartid, mcause, mtval);
+		   __func__, hartid, trap->cause, trap->tval);
 	if (misa_extension('H')) {
 		sbi_printf("%s: hart%d: mtval2=0x%" PRILX
 			   " mtinst=0x%" PRILX "\n",
-			   __func__, hartid, mtval2, mtinst);
+			   __func__, hartid, trap->tval2, trap->tinst);
 	}
 	sbi_printf("%s: hart%d: mepc=0x%" PRILX " mstatus=0x%" PRILX "\n",
 		   __func__, hartid, regs->mepc, regs->mstatus);
@@ -258,20 +258,20 @@ static int sbi_trap_aia_irq(struct sbi_trap_regs *regs, ulong mcause)
  * 6. Stack pointer (SP) is setup for current HART
  * 7. Interrupts are disabled in MSTATUS CSR
  *
- * @param regs pointer to register state
+ * @param tcntx pointer to trap context
  */
-struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs)
+struct sbi_trap_context *sbi_trap_handler(struct sbi_trap_context *tcntx)
 {
 	int rc = SBI_ENOTSUPP;
 	const char *msg = "trap handler failed";
-	ulong mcause = csr_read(CSR_MCAUSE);
-	ulong mtval = csr_read(CSR_MTVAL), mtval2 = 0, mtinst = 0;
-	struct sbi_trap_info trap;
+	struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
+	const struct sbi_trap_info *trap = &tcntx->trap;
+	struct sbi_trap_regs *regs = &tcntx->regs;
+	ulong mcause = tcntx->trap.cause;
 
-	if (misa_extension('H')) {
-		mtval2 = csr_read(CSR_MTVAL2);
-		mtinst = csr_read(CSR_MTINST);
-	}
+	/* Update trap context pointer */
+	tcntx->prev_context = sbi_trap_get_context(scratch);
+	sbi_trap_set_context(scratch, tcntx);
 
 	if (mcause & (1UL << (__riscv_xlen - 1))) {
 		if (sbi_hart_has_extension(sbi_scratch_thishart_ptr(),
@@ -279,32 +279,23 @@ struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs)
 			rc = sbi_trap_aia_irq(regs, mcause);
 		else
 			rc = sbi_trap_nonaia_irq(regs, mcause);
-		if (rc) {
-			msg = "unhandled local interrupt";
-			goto trap_error;
-		}
-		return regs;
+		msg = "unhandled local interrupt";
+		goto trap_done;
 	}
-	/* Original trap_info */
-	trap.cause = mcause;
-	trap.tval  = mtval;
-	trap.tval2 = mtval2;
-	trap.tinst = mtinst;
-	trap.gva   = sbi_regs_gva(regs);
 
 	switch (mcause) {
 	case CAUSE_ILLEGAL_INSTRUCTION:
-		rc  = sbi_illegal_insn_handler(mtval, regs);
+		rc  = sbi_illegal_insn_handler(tcntx->trap.tval, regs);
 		msg = "illegal instruction handler failed";
 		break;
 	case CAUSE_MISALIGNED_LOAD:
 		sbi_pmu_ctr_incr_fw(SBI_PMU_FW_MISALIGNED_LOAD);
-		rc  = sbi_misaligned_load_handler(regs, &trap);
+		rc  = sbi_misaligned_load_handler(regs, trap);
 		msg = "misaligned load handler failed";
 		break;
 	case CAUSE_MISALIGNED_STORE:
 		sbi_pmu_ctr_incr_fw(SBI_PMU_FW_MISALIGNED_STORE);
-		rc  = sbi_misaligned_store_handler(regs, &trap);
+		rc  = sbi_misaligned_store_handler(regs, trap);
 		msg = "misaligned store handler failed";
 		break;
 	case CAUSE_SUPERVISOR_ECALL:
@@ -314,23 +305,24 @@ struct sbi_trap_regs *sbi_trap_handler(struct sbi_trap_regs *regs)
 		break;
 	case CAUSE_LOAD_ACCESS:
 		sbi_pmu_ctr_incr_fw(SBI_PMU_FW_ACCESS_LOAD);
-		rc  = sbi_load_access_handler(regs, &trap);
+		rc  = sbi_load_access_handler(regs, trap);
 		msg = "load fault handler failed";
 		break;
 	case CAUSE_STORE_ACCESS:
 		sbi_pmu_ctr_incr_fw(SBI_PMU_FW_ACCESS_STORE);
-		rc  = sbi_store_access_handler(regs, &trap);
+		rc  = sbi_store_access_handler(regs, trap);
 		msg = "store fault handler failed";
 		break;
 	default:
 		/* If the trap came from S or U mode, redirect it there */
 		msg = "trap redirect failed";
-		rc  = sbi_trap_redirect(regs, &trap);
+		rc  = sbi_trap_redirect(regs, trap);
 		break;
 	}
 
-trap_error:
+trap_done:
 	if (rc)
-		sbi_trap_error(msg, rc, mcause, mtval, mtval2, mtinst, regs);
-	return regs;
+		sbi_trap_error(msg, rc, trap, regs);
+	sbi_trap_set_context(scratch, tcntx->prev_context);
+	return tcntx;
 }
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 05/10] lib: sbi: Simplify parameters of misaligned and access fault handlers
  2024-03-17 13:02 [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel
                   ` (3 preceding siblings ...)
  2024-03-17 13:02 ` [PATCH v3 04/10] lib: sbi: Introduce trap context Anup Patel
@ 2024-03-17 13:02 ` Anup Patel
  2024-03-17 13:02 ` [PATCH v3 06/10] lib: sbi: Simplify parameters of sbi_illegal_insn_handler() Anup Patel
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Anup Patel @ 2024-03-17 13:02 UTC (permalink / raw)
  To: opensbi

The struct sbi_trap_context already has the information needed by
misaligned load/store and access fault load/store handlers so directly
pass struct sbi_trap_context pointer to these functions.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
Tested-by: Samuel Holland <samuel.holland@sifive.com>
Reviewed-by: Cl?ment L?ger <cleger@rivosinc.com>
---
 include/sbi/sbi_trap_ldst.h | 12 +++----
 lib/sbi/sbi_trap.c          |  8 ++---
 lib/sbi/sbi_trap_ldst.c     | 67 ++++++++++++++++++-------------------
 3 files changed, 40 insertions(+), 47 deletions(-)

diff --git a/include/sbi/sbi_trap_ldst.h b/include/sbi/sbi_trap_ldst.h
index 9cab4e4..8aee316 100644
--- a/include/sbi/sbi_trap_ldst.h
+++ b/include/sbi/sbi_trap_ldst.h
@@ -20,16 +20,12 @@ union sbi_ldst_data {
 	ulong data_ulong;
 };
 
-int sbi_misaligned_load_handler(struct sbi_trap_regs *regs,
-				const struct sbi_trap_info *orig_trap);
+int sbi_misaligned_load_handler(struct sbi_trap_context *tcntx);
 
-int sbi_misaligned_store_handler(struct sbi_trap_regs *regs,
-				 const struct sbi_trap_info *orig_trap);
+int sbi_misaligned_store_handler(struct sbi_trap_context *tcntx);
 
-int sbi_load_access_handler(struct sbi_trap_regs *regs,
-			    const struct sbi_trap_info *orig_trap);
+int sbi_load_access_handler(struct sbi_trap_context *tcntx);
 
-int sbi_store_access_handler(struct sbi_trap_regs *regs,
-			     const struct sbi_trap_info *orig_trap);
+int sbi_store_access_handler(struct sbi_trap_context *tcntx);
 
 #endif
diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
index 4dbda06..a2afb0a 100644
--- a/lib/sbi/sbi_trap.c
+++ b/lib/sbi/sbi_trap.c
@@ -290,12 +290,12 @@ struct sbi_trap_context *sbi_trap_handler(struct sbi_trap_context *tcntx)
 		break;
 	case CAUSE_MISALIGNED_LOAD:
 		sbi_pmu_ctr_incr_fw(SBI_PMU_FW_MISALIGNED_LOAD);
-		rc  = sbi_misaligned_load_handler(regs, trap);
+		rc  = sbi_misaligned_load_handler(tcntx);
 		msg = "misaligned load handler failed";
 		break;
 	case CAUSE_MISALIGNED_STORE:
 		sbi_pmu_ctr_incr_fw(SBI_PMU_FW_MISALIGNED_STORE);
-		rc  = sbi_misaligned_store_handler(regs, trap);
+		rc  = sbi_misaligned_store_handler(tcntx);
 		msg = "misaligned store handler failed";
 		break;
 	case CAUSE_SUPERVISOR_ECALL:
@@ -305,12 +305,12 @@ struct sbi_trap_context *sbi_trap_handler(struct sbi_trap_context *tcntx)
 		break;
 	case CAUSE_LOAD_ACCESS:
 		sbi_pmu_ctr_incr_fw(SBI_PMU_FW_ACCESS_LOAD);
-		rc  = sbi_load_access_handler(regs, trap);
+		rc  = sbi_load_access_handler(tcntx);
 		msg = "load fault handler failed";
 		break;
 	case CAUSE_STORE_ACCESS:
 		sbi_pmu_ctr_incr_fw(SBI_PMU_FW_ACCESS_STORE);
-		rc  = sbi_store_access_handler(regs, trap);
+		rc  = sbi_store_access_handler(tcntx);
 		msg = "store fault handler failed";
 		break;
 	default:
diff --git a/lib/sbi/sbi_trap_ldst.c b/lib/sbi/sbi_trap_ldst.c
index 5a0537b..f9c0241 100644
--- a/lib/sbi/sbi_trap_ldst.c
+++ b/lib/sbi/sbi_trap_ldst.c
@@ -12,7 +12,6 @@
 #include <sbi/riscv_fp.h>
 #include <sbi/sbi_error.h>
 #include <sbi/sbi_trap_ldst.h>
-#include <sbi/sbi_pmu.h>
 #include <sbi/sbi_trap.h>
 #include <sbi/sbi_unpriv.h>
 #include <sbi/sbi_platform.h>
@@ -23,8 +22,7 @@
  * @return rlen=success, 0=success w/o regs modification, or negative error
  */
 typedef int (*sbi_trap_ld_emulator)(int rlen, union sbi_ldst_data *out_val,
-				    struct sbi_trap_regs *regs,
-				    const struct sbi_trap_info *orig_trap);
+				    struct sbi_trap_context *tcntx);
 
 /**
  * Store emulator callback:
@@ -32,8 +30,7 @@ typedef int (*sbi_trap_ld_emulator)(int rlen, union sbi_ldst_data *out_val,
  * @return wlen=success, 0=success w/o regs modification, or negative error
  */
 typedef int (*sbi_trap_st_emulator)(int wlen, union sbi_ldst_data in_val,
-				    struct sbi_trap_regs *regs,
-				    const struct sbi_trap_info *orig_trap);
+				    struct sbi_trap_context *tcntx);
 
 static ulong sbi_misaligned_tinst_fixup(ulong orig_tinst, ulong new_tinst,
 					ulong addr_offset)
@@ -47,10 +44,11 @@ static ulong sbi_misaligned_tinst_fixup(ulong orig_tinst, ulong new_tinst,
 		return orig_tinst | (addr_offset << SH_RS1);
 }
 
-static int sbi_trap_emulate_load(struct sbi_trap_regs *regs,
-				 const struct sbi_trap_info *orig_trap,
+static int sbi_trap_emulate_load(struct sbi_trap_context *tcntx,
 				 sbi_trap_ld_emulator emu)
 {
+	const struct sbi_trap_info *orig_trap = &tcntx->trap;
+	struct sbi_trap_regs *regs = &tcntx->regs;
 	ulong insn, insn_len;
 	union sbi_ldst_data val = { 0 };
 	struct sbi_trap_info uptrap;
@@ -150,8 +148,7 @@ static int sbi_trap_emulate_load(struct sbi_trap_regs *regs,
 		return sbi_trap_redirect(regs, orig_trap);
 	}
 
-	rc = emu(len, &val, regs, orig_trap);
-
+	rc = emu(len, &val, tcntx);
 	if (rc <= 0)
 		return rc;
 
@@ -169,10 +166,11 @@ static int sbi_trap_emulate_load(struct sbi_trap_regs *regs,
 	return 0;
 }
 
-static int sbi_trap_emulate_store(struct sbi_trap_regs *regs,
-				  const struct sbi_trap_info *orig_trap,
+static int sbi_trap_emulate_store(struct sbi_trap_context *tcntx,
 				  sbi_trap_st_emulator emu)
 {
+	const struct sbi_trap_info *orig_trap = &tcntx->trap;
+	struct sbi_trap_regs *regs = &tcntx->regs;
 	ulong insn, insn_len;
 	union sbi_ldst_data val;
 	struct sbi_trap_info uptrap;
@@ -254,8 +252,7 @@ static int sbi_trap_emulate_store(struct sbi_trap_regs *regs,
 		return sbi_trap_redirect(regs, orig_trap);
 	}
 
-	rc = emu(len, val, regs, orig_trap);
-
+	rc = emu(len, val, tcntx);
 	if (rc <= 0)
 		return rc;
 
@@ -265,9 +262,10 @@ static int sbi_trap_emulate_store(struct sbi_trap_regs *regs,
 }
 
 static int sbi_misaligned_ld_emulator(int rlen, union sbi_ldst_data *out_val,
-				      struct sbi_trap_regs *regs,
-				      const struct sbi_trap_info *orig_trap)
+				      struct sbi_trap_context *tcntx)
 {
+	const struct sbi_trap_info *orig_trap = &tcntx->trap;
+	struct sbi_trap_regs *regs = &tcntx->regs;
 	struct sbi_trap_info uptrap;
 	int i;
 
@@ -283,17 +281,16 @@ static int sbi_misaligned_ld_emulator(int rlen, union sbi_ldst_data *out_val,
 	return rlen;
 }
 
-int sbi_misaligned_load_handler(struct sbi_trap_regs *regs,
-				const struct sbi_trap_info *orig_trap)
+int sbi_misaligned_load_handler(struct sbi_trap_context *tcntx)
 {
-	return sbi_trap_emulate_load(regs, orig_trap,
-				     sbi_misaligned_ld_emulator);
+	return sbi_trap_emulate_load(tcntx, sbi_misaligned_ld_emulator);
 }
 
 static int sbi_misaligned_st_emulator(int wlen, union sbi_ldst_data in_val,
-				      struct sbi_trap_regs *regs,
-				      const struct sbi_trap_info *orig_trap)
+				      struct sbi_trap_context *tcntx)
 {
+	const struct sbi_trap_info *orig_trap = &tcntx->trap;
+	struct sbi_trap_regs *regs = &tcntx->regs;
 	struct sbi_trap_info uptrap;
 	int i;
 
@@ -309,17 +306,17 @@ static int sbi_misaligned_st_emulator(int wlen, union sbi_ldst_data in_val,
 	return wlen;
 }
 
-int sbi_misaligned_store_handler(struct sbi_trap_regs *regs,
-				 const struct sbi_trap_info *orig_trap)
+int sbi_misaligned_store_handler(struct sbi_trap_context *tcntx)
 {
-	return sbi_trap_emulate_store(regs, orig_trap,
-				      sbi_misaligned_st_emulator);
+	return sbi_trap_emulate_store(tcntx, sbi_misaligned_st_emulator);
 }
 
 static int sbi_ld_access_emulator(int rlen, union sbi_ldst_data *out_val,
-				  struct sbi_trap_regs *regs,
-				  const struct sbi_trap_info *orig_trap)
+				  struct sbi_trap_context *tcntx)
 {
+	const struct sbi_trap_info *orig_trap = &tcntx->trap;
+	struct sbi_trap_regs *regs = &tcntx->regs;
+
 	/* If fault came from M mode, just fail */
 	if (((regs->mstatus & MSTATUS_MPP) >> MSTATUS_MPP_SHIFT) == PRV_M)
 		return SBI_EINVAL;
@@ -332,16 +329,17 @@ static int sbi_ld_access_emulator(int rlen, union sbi_ldst_data *out_val,
 	return rlen;
 }
 
-int sbi_load_access_handler(struct sbi_trap_regs *regs,
-			    const struct sbi_trap_info *orig_trap)
+int sbi_load_access_handler(struct sbi_trap_context *tcntx)
 {
-	return sbi_trap_emulate_load(regs, orig_trap, sbi_ld_access_emulator);
+	return sbi_trap_emulate_load(tcntx, sbi_ld_access_emulator);
 }
 
 static int sbi_st_access_emulator(int wlen, union sbi_ldst_data in_val,
-				  struct sbi_trap_regs *regs,
-				  const struct sbi_trap_info *orig_trap)
+				  struct sbi_trap_context *tcntx)
 {
+	const struct sbi_trap_info *orig_trap = &tcntx->trap;
+	struct sbi_trap_regs *regs = &tcntx->regs;
+
 	/* If fault came from M mode, just fail */
 	if (((regs->mstatus & MSTATUS_MPP) >> MSTATUS_MPP_SHIFT) == PRV_M)
 		return SBI_EINVAL;
@@ -354,8 +352,7 @@ static int sbi_st_access_emulator(int wlen, union sbi_ldst_data in_val,
 	return wlen;
 }
 
-int sbi_store_access_handler(struct sbi_trap_regs *regs,
-			     const struct sbi_trap_info *orig_trap)
+int sbi_store_access_handler(struct sbi_trap_context *tcntx)
 {
-	return sbi_trap_emulate_store(regs, orig_trap, sbi_st_access_emulator);
+	return sbi_trap_emulate_store(tcntx, sbi_st_access_emulator);
 }
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 06/10] lib: sbi: Simplify parameters of sbi_illegal_insn_handler()
  2024-03-17 13:02 [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel
                   ` (4 preceding siblings ...)
  2024-03-17 13:02 ` [PATCH v3 05/10] lib: sbi: Simplify parameters of misaligned and access fault handlers Anup Patel
@ 2024-03-17 13:02 ` Anup Patel
  2024-03-17 13:02 ` [PATCH v3 07/10] lib: sbi: Remove regs paramter of sbi_irqchip_process() Anup Patel
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Anup Patel @ 2024-03-17 13:02 UTC (permalink / raw)
  To: opensbi

The struct sbi_trap_context already has the information needed by
sbi_illegal_insn_handler() so directly pass struct sbi_trap_context
pointer to this function.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
Tested-by: Samuel Holland <samuel.holland@sifive.com>
---
 include/sbi/sbi_illegal_insn.h | 4 ++--
 lib/sbi/sbi_illegal_insn.c     | 7 ++++---
 lib/sbi/sbi_trap.c             | 2 +-
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/include/sbi/sbi_illegal_insn.h b/include/sbi/sbi_illegal_insn.h
index 0397935..7be72ac 100644
--- a/include/sbi/sbi_illegal_insn.h
+++ b/include/sbi/sbi_illegal_insn.h
@@ -12,8 +12,8 @@
 
 #include <sbi/sbi_types.h>
 
-struct sbi_trap_regs;
+struct sbi_trap_context;
 
-int sbi_illegal_insn_handler(ulong insn, struct sbi_trap_regs *regs);
+int sbi_illegal_insn_handler(struct sbi_trap_context *tcntx);
 
 #endif
diff --git a/lib/sbi/sbi_illegal_insn.c b/lib/sbi/sbi_illegal_insn.c
index dd0b3c1..ed6f111 100644
--- a/lib/sbi/sbi_illegal_insn.c
+++ b/lib/sbi/sbi_illegal_insn.c
@@ -136,8 +136,10 @@ static const illegal_insn_func illegal_insn_table[32] = {
 	truly_illegal_insn  /* 31 */
 };
 
-int sbi_illegal_insn_handler(ulong insn, struct sbi_trap_regs *regs)
+int sbi_illegal_insn_handler(struct sbi_trap_context *tcntx)
 {
+	struct sbi_trap_regs *regs = &tcntx->regs;
+	ulong insn = tcntx->trap.tval;
 	struct sbi_trap_info uptrap;
 
 	/*
@@ -154,9 +156,8 @@ int sbi_illegal_insn_handler(ulong insn, struct sbi_trap_regs *regs)
 	sbi_pmu_ctr_incr_fw(SBI_PMU_FW_ILLEGAL_INSN);
 	if (unlikely((insn & 3) != 3)) {
 		insn = sbi_get_insn(regs->mepc, &uptrap);
-		if (uptrap.cause) {
+		if (uptrap.cause)
 			return sbi_trap_redirect(regs, &uptrap);
-		}
 		if ((insn & 3) != 3)
 			return truly_illegal_insn(insn, regs);
 	}
diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
index a2afb0a..4e691df 100644
--- a/lib/sbi/sbi_trap.c
+++ b/lib/sbi/sbi_trap.c
@@ -285,7 +285,7 @@ struct sbi_trap_context *sbi_trap_handler(struct sbi_trap_context *tcntx)
 
 	switch (mcause) {
 	case CAUSE_ILLEGAL_INSTRUCTION:
-		rc  = sbi_illegal_insn_handler(tcntx->trap.tval, regs);
+		rc  = sbi_illegal_insn_handler(tcntx);
 		msg = "illegal instruction handler failed";
 		break;
 	case CAUSE_MISALIGNED_LOAD:
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 07/10] lib: sbi: Remove regs paramter of sbi_irqchip_process()
  2024-03-17 13:02 [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel
                   ` (5 preceding siblings ...)
  2024-03-17 13:02 ` [PATCH v3 06/10] lib: sbi: Simplify parameters of sbi_illegal_insn_handler() Anup Patel
@ 2024-03-17 13:02 ` Anup Patel
  2024-03-17 13:02 ` [PATCH v3 08/10] lib: sbi: Remove regs parameter from trap irq handling functions Anup Patel
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Anup Patel @ 2024-03-17 13:02 UTC (permalink / raw)
  To: opensbi

The irqchip handlers will typically not need pointer to trap registers
so remove regs parameter of sbi_irqchip_process().

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
Tested-by: Samuel Holland <samuel.holland@sifive.com>
Reviewed-by: Cl?ment L?ger <cleger@rivosinc.com>
---
 include/sbi/sbi_irqchip.h |  5 ++---
 lib/sbi/sbi_irqchip.c     | 10 +++++-----
 lib/sbi/sbi_trap.c        |  4 ++--
 lib/utils/irqchip/imsic.c |  2 +-
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/include/sbi/sbi_irqchip.h b/include/sbi/sbi_irqchip.h
index 6acc6e3..0ed02eb 100644
--- a/include/sbi/sbi_irqchip.h
+++ b/include/sbi/sbi_irqchip.h
@@ -13,7 +13,6 @@
 #include <sbi/sbi_types.h>
 
 struct sbi_scratch;
-struct sbi_trap_regs;
 
 /**
  * Set external interrupt handling function
@@ -23,7 +22,7 @@ struct sbi_trap_regs;
  *
  * @param fn function pointer for handling external irqs
  */
-void sbi_irqchip_set_irqfn(int (*fn)(struct sbi_trap_regs *regs));
+void sbi_irqchip_set_irqfn(int (*fn)(void));
 
 /**
  * Process external interrupts
@@ -33,7 +32,7 @@ void sbi_irqchip_set_irqfn(int (*fn)(struct sbi_trap_regs *regs));
  *
  * @param regs pointer for trap registers
  */
-int sbi_irqchip_process(struct sbi_trap_regs *regs);
+int sbi_irqchip_process(void);
 
 /** Initialize interrupt controllers */
 int sbi_irqchip_init(struct sbi_scratch *scratch, bool cold_boot);
diff --git a/lib/sbi/sbi_irqchip.c b/lib/sbi/sbi_irqchip.c
index 24128be..0ae604a 100644
--- a/lib/sbi/sbi_irqchip.c
+++ b/lib/sbi/sbi_irqchip.c
@@ -10,22 +10,22 @@
 #include <sbi/sbi_irqchip.h>
 #include <sbi/sbi_platform.h>
 
-static int default_irqfn(struct sbi_trap_regs *regs)
+static int default_irqfn(void)
 {
 	return SBI_ENODEV;
 }
 
-static int (*ext_irqfn)(struct sbi_trap_regs *regs) = default_irqfn;
+static int (*ext_irqfn)(void) = default_irqfn;
 
-void sbi_irqchip_set_irqfn(int (*fn)(struct sbi_trap_regs *regs))
+void sbi_irqchip_set_irqfn(int (*fn)(void))
 {
 	if (fn)
 		ext_irqfn = fn;
 }
 
-int sbi_irqchip_process(struct sbi_trap_regs *regs)
+int sbi_irqchip_process(void)
 {
-	return ext_irqfn(regs);
+	return ext_irqfn();
 }
 
 int sbi_irqchip_init(struct sbi_scratch *scratch, bool cold_boot)
diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
index 4e691df..72b1788 100644
--- a/lib/sbi/sbi_trap.c
+++ b/lib/sbi/sbi_trap.c
@@ -209,7 +209,7 @@ static int sbi_trap_nonaia_irq(struct sbi_trap_regs *regs, ulong mcause)
 		sbi_ipi_process();
 		break;
 	case IRQ_M_EXT:
-		return sbi_irqchip_process(regs);
+		return sbi_irqchip_process();
 	default:
 		return SBI_ENOENT;
 	}
@@ -232,7 +232,7 @@ static int sbi_trap_aia_irq(struct sbi_trap_regs *regs, ulong mcause)
 			sbi_ipi_process();
 			break;
 		case IRQ_M_EXT:
-			rc = sbi_irqchip_process(regs);
+			rc = sbi_irqchip_process();
 			if (rc)
 				return rc;
 			break;
diff --git a/lib/utils/irqchip/imsic.c b/lib/utils/irqchip/imsic.c
index 36ef66c..f2a35c6 100644
--- a/lib/utils/irqchip/imsic.c
+++ b/lib/utils/irqchip/imsic.c
@@ -140,7 +140,7 @@ int imsic_get_target_file(u32 hartid)
 	return imsic_get_hart_file(scratch);
 }
 
-static int imsic_external_irqfn(struct sbi_trap_regs *regs)
+static int imsic_external_irqfn(void)
 {
 	ulong mirq;
 
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 08/10] lib: sbi: Remove regs parameter from trap irq handling functions
  2024-03-17 13:02 [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel
                   ` (6 preceding siblings ...)
  2024-03-17 13:02 ` [PATCH v3 07/10] lib: sbi: Remove regs paramter of sbi_irqchip_process() Anup Patel
@ 2024-03-17 13:02 ` Anup Patel
  2024-03-17 13:02 ` [PATCH v3 09/10] lib: sbi: Pass trap context pointer to sbi_ecall_handler() Anup Patel
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 12+ messages in thread
From: Anup Patel @ 2024-03-17 13:02 UTC (permalink / raw)
  To: opensbi

The trap irq handling functions no longer require regs parameter
so remove it.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
Tested-by: Samuel Holland <samuel.holland@sifive.com>
---
 include/sbi/riscv_encoding.h |  2 ++
 lib/sbi/sbi_trap.c           | 13 ++++++-------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/sbi/riscv_encoding.h b/include/sbi/riscv_encoding.h
index 46bbeed..d914828 100644
--- a/include/sbi/riscv_encoding.h
+++ b/include/sbi/riscv_encoding.h
@@ -80,6 +80,8 @@
 #define HSTATUS_GVA			_UL(0x00000040)
 #define HSTATUS_VSBE			_UL(0x00000020)
 
+#define MCAUSE_IRQ_MASK			(_UL(1) << (__riscv_xlen - 1))
+
 #define IRQ_S_SOFT			1
 #define IRQ_VS_SOFT			2
 #define IRQ_M_SOFT			3
diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
index 72b1788..2462763 100644
--- a/lib/sbi/sbi_trap.c
+++ b/lib/sbi/sbi_trap.c
@@ -198,10 +198,9 @@ int sbi_trap_redirect(struct sbi_trap_regs *regs,
 	return 0;
 }
 
-static int sbi_trap_nonaia_irq(struct sbi_trap_regs *regs, ulong mcause)
+static int sbi_trap_nonaia_irq(unsigned long irq)
 {
-	mcause &= ~(1UL << (__riscv_xlen - 1));
-	switch (mcause) {
+	switch (irq) {
 	case IRQ_M_TIMER:
 		sbi_timer_process();
 		break;
@@ -217,7 +216,7 @@ static int sbi_trap_nonaia_irq(struct sbi_trap_regs *regs, ulong mcause)
 	return 0;
 }
 
-static int sbi_trap_aia_irq(struct sbi_trap_regs *regs, ulong mcause)
+static int sbi_trap_aia_irq(void)
 {
 	int rc;
 	unsigned long mtopi;
@@ -273,12 +272,12 @@ struct sbi_trap_context *sbi_trap_handler(struct sbi_trap_context *tcntx)
 	tcntx->prev_context = sbi_trap_get_context(scratch);
 	sbi_trap_set_context(scratch, tcntx);
 
-	if (mcause & (1UL << (__riscv_xlen - 1))) {
+	if (mcause & MCAUSE_IRQ_MASK) {
 		if (sbi_hart_has_extension(sbi_scratch_thishart_ptr(),
 					   SBI_HART_EXT_SMAIA))
-			rc = sbi_trap_aia_irq(regs, mcause);
+			rc = sbi_trap_aia_irq();
 		else
-			rc = sbi_trap_nonaia_irq(regs, mcause);
+			rc = sbi_trap_nonaia_irq(mcause & ~MCAUSE_IRQ_MASK);
 		msg = "unhandled local interrupt";
 		goto trap_done;
 	}
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 09/10] lib: sbi: Pass trap context pointer to sbi_ecall_handler()
  2024-03-17 13:02 [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel
                   ` (7 preceding siblings ...)
  2024-03-17 13:02 ` [PATCH v3 08/10] lib: sbi: Remove regs parameter from trap irq handling functions Anup Patel
@ 2024-03-17 13:02 ` Anup Patel
  2024-03-17 13:02 ` [PATCH v3 10/10] lib: sbi: Extend sbi_trap_error() to dump state in a nested trap Anup Patel
  2024-03-19  6:06 ` [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel
  10 siblings, 0 replies; 12+ messages in thread
From: Anup Patel @ 2024-03-17 13:02 UTC (permalink / raw)
  To: opensbi

To be consistent with other trap handlers, pass trap context pointer
to sbi_ecall_handler().

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
Tested-by: Samuel Holland <samuel.holland@sifive.com>
Reviewed-by: Cl?ment L?ger <cleger@rivosinc.com>
---
 include/sbi/sbi_ecall.h | 4 ++--
 lib/sbi/sbi_ecall.c     | 3 ++-
 lib/sbi/sbi_trap.c      | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/sbi/sbi_ecall.h b/include/sbi/sbi_ecall.h
index 0bf42d1..0b35eff 100644
--- a/include/sbi/sbi_ecall.h
+++ b/include/sbi/sbi_ecall.h
@@ -18,7 +18,7 @@
 #define SBI_OPENSBI_IMPID		1
 
 struct sbi_trap_regs;
-struct sbi_trap_info;
+struct sbi_trap_context;
 
 struct sbi_ecall_return {
 	/* Return flag to skip register update */
@@ -87,7 +87,7 @@ int sbi_ecall_register_extension(struct sbi_ecall_extension *ext);
 
 void sbi_ecall_unregister_extension(struct sbi_ecall_extension *ext);
 
-int sbi_ecall_handler(struct sbi_trap_regs *regs);
+int sbi_ecall_handler(struct sbi_trap_context *tcntx);
 
 int sbi_ecall_init(void);
 
diff --git a/lib/sbi/sbi_ecall.c b/lib/sbi/sbi_ecall.c
index 631c5dd..d4fc58c 100644
--- a/lib/sbi/sbi_ecall.c
+++ b/lib/sbi/sbi_ecall.c
@@ -95,9 +95,10 @@ void sbi_ecall_unregister_extension(struct sbi_ecall_extension *ext)
 		sbi_list_del_init(&ext->head);
 }
 
-int sbi_ecall_handler(struct sbi_trap_regs *regs)
+int sbi_ecall_handler(struct sbi_trap_context *tcntx)
 {
 	int ret = 0;
+	struct sbi_trap_regs *regs = &tcntx->regs;
 	struct sbi_ecall_extension *ext;
 	unsigned long extension_id = regs->a7;
 	unsigned long func_id = regs->a6;
diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
index 2462763..060cec6 100644
--- a/lib/sbi/sbi_trap.c
+++ b/lib/sbi/sbi_trap.c
@@ -299,7 +299,7 @@ struct sbi_trap_context *sbi_trap_handler(struct sbi_trap_context *tcntx)
 		break;
 	case CAUSE_SUPERVISOR_ECALL:
 	case CAUSE_MACHINE_ECALL:
-		rc  = sbi_ecall_handler(regs);
+		rc  = sbi_ecall_handler(tcntx);
 		msg = "ecall handler failed";
 		break;
 	case CAUSE_LOAD_ACCESS:
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 10/10] lib: sbi: Extend sbi_trap_error() to dump state in a nested trap
  2024-03-17 13:02 [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel
                   ` (8 preceding siblings ...)
  2024-03-17 13:02 ` [PATCH v3 09/10] lib: sbi: Pass trap context pointer to sbi_ecall_handler() Anup Patel
@ 2024-03-17 13:02 ` Anup Patel
  2024-03-19  6:06 ` [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel
  10 siblings, 0 replies; 12+ messages in thread
From: Anup Patel @ 2024-03-17 13:02 UTC (permalink / raw)
  To: opensbi

The sbi_trap_error() should dump state of all in-flight traps upon
failure in a nested trap so extend it accordingly.

Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Tested-by: Samuel Holland <samuel.holland@sifive.com>
---
 lib/sbi/sbi_trap.c | 105 ++++++++++++++++++++++++++-------------------
 1 file changed, 60 insertions(+), 45 deletions(-)

diff --git a/lib/sbi/sbi_trap.c b/lib/sbi/sbi_trap.c
index 060cec6..175560f 100644
--- a/lib/sbi/sbi_trap.c
+++ b/lib/sbi/sbi_trap.c
@@ -23,54 +23,69 @@
 #include <sbi/sbi_timer.h>
 #include <sbi/sbi_trap.h>
 
-static void __noreturn sbi_trap_error(const char *msg, int rc,
-				      const struct sbi_trap_info *trap,
-				      const struct sbi_trap_regs *regs)
+static void sbi_trap_error_one(const struct sbi_trap_context *tcntx,
+			       const char *prefix, u32 hartid, u32 depth)
 {
-	u32 hartid = current_hartid();
+	const struct sbi_trap_info *trap = &tcntx->trap;
+	const struct sbi_trap_regs *regs = &tcntx->regs;
 
-	sbi_printf("%s: hart%d: %s (error %d)\n", __func__, hartid, msg, rc);
-	sbi_printf("%s: hart%d: mcause=0x%" PRILX " mtval=0x%" PRILX "\n",
-		   __func__, hartid, trap->cause, trap->tval);
+	sbi_printf("\n");
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "mcause", trap->cause, "mtval", trap->tval);
 	if (misa_extension('H')) {
-		sbi_printf("%s: hart%d: mtval2=0x%" PRILX
-			   " mtinst=0x%" PRILX "\n",
-			   __func__, hartid, trap->tval2, trap->tinst);
+		sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+			   hartid, depth, "mtval2", trap->tval2, "mtinst", trap->tinst);
 	}
-	sbi_printf("%s: hart%d: mepc=0x%" PRILX " mstatus=0x%" PRILX "\n",
-		   __func__, hartid, regs->mepc, regs->mstatus);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "ra", regs->ra, "sp", regs->sp);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "gp", regs->gp, "tp", regs->tp);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "s0", regs->s0, "s1", regs->s1);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "a0", regs->a0, "a1", regs->a1);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "a2", regs->a2, "a3", regs->a3);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "a4", regs->a4, "a5", regs->a5);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "a6", regs->a6, "a7", regs->a7);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "s2", regs->s2, "s3", regs->s3);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "s4", regs->s4, "s5", regs->s5);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "s6", regs->s6, "s7", regs->s7);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "s8", regs->s8, "s9", regs->s9);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "s10", regs->s10, "s11", regs->s11);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "t0", regs->t0, "t1", regs->t1);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "t2", regs->t2, "t3", regs->t3);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", __func__,
-		   hartid, "t4", regs->t4, "t5", regs->t5);
-	sbi_printf("%s: hart%d: %s=0x%" PRILX "\n", __func__, hartid, "t6",
-		   regs->t6);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "mepc", regs->mepc, "mstatus", regs->mstatus);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "ra", regs->ra, "sp", regs->sp);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "gp", regs->gp, "tp", regs->tp);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "s0", regs->s0, "s1", regs->s1);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "a0", regs->a0, "a1", regs->a1);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "a2", regs->a2, "a3", regs->a3);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "a4", regs->a4, "a5", regs->a5);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "a6", regs->a6, "a7", regs->a7);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "s2", regs->s2, "s3", regs->s3);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "s4", regs->s4, "s5", regs->s5);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "s6", regs->s6, "s7", regs->s7);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "s8", regs->s8, "s9", regs->s9);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "s10", regs->s10, "s11", regs->s11);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "t0", regs->t0, "t1", regs->t1);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "t2", regs->t2, "t3", regs->t3);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX " %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "t4", regs->t4, "t5", regs->t5);
+	sbi_printf("%s: hart%d: trap%d: %s=0x%" PRILX "\n", prefix,
+		   hartid, depth, "t6", regs->t6);
+}
+
+static void __noreturn sbi_trap_error(const char *msg, int rc,
+				      const struct sbi_trap_context *tcntx)
+{
+	u32 depth = 0, hartid = current_hartid();
+	const struct sbi_trap_context *tc;
+
+	for (tc = tcntx; tc; tc = tc->prev_context)
+		depth++;
+
+	sbi_printf("\n");
+	sbi_printf("%s: hart%d: trap%d: %s (error %d)\n", __func__,
+		   hartid, depth - 1, msg, rc);
+	for (tc = tcntx; tc; tc = tc->prev_context)
+		sbi_trap_error_one(tc, __func__, hartid, --depth);
 
 	sbi_hart_hang();
 }
@@ -321,7 +336,7 @@ struct sbi_trap_context *sbi_trap_handler(struct sbi_trap_context *tcntx)
 
 trap_done:
 	if (rc)
-		sbi_trap_error(msg, rc, trap, regs);
+		sbi_trap_error(msg, rc, tcntx);
 	sbi_trap_set_context(scratch, tcntx->prev_context);
 	return tcntx;
 }
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3 00/10] Improve trap handling for nested traps
  2024-03-17 13:02 [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel
                   ` (9 preceding siblings ...)
  2024-03-17 13:02 ` [PATCH v3 10/10] lib: sbi: Extend sbi_trap_error() to dump state in a nested trap Anup Patel
@ 2024-03-19  6:06 ` Anup Patel
  10 siblings, 0 replies; 12+ messages in thread
From: Anup Patel @ 2024-03-19  6:06 UTC (permalink / raw)
  To: opensbi

On Sun, Mar 17, 2024 at 6:32?PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> Nested traps will be a common when dealing with RAS error traps so
> this series improves trap handling for nested traps by introducing
> a linked-list based trap context chain.
>
> These patches can also be found the trap_handling_imp_v3 branch at
> https://github.com/avpatel/opensbi.git
>
> Changes since v2:
>  - Addressed comments in PATCH4, PATCH6, PATCH7, PATCH8, and PATCH10.
>
> Changes since v1:
>  - Include samuel's patch in the series
>  - Embed sbi_trap_regs and sbi_trap_info in the new
>    struct sbi_trap_context
>  - Drop the mcause parameter of sbi_trap_aia_irq()
>
> Anup Patel (9):
>   lib: sbi: Remove sbi_trap_exit() and related code
>   include: sbi: Add trap_context pointer in struct sbi_scratch
>   lib: sbi: Introduce trap context
>   lib: sbi: Simplify parameters of misaligned and access fault handlers
>   lib: sbi: Simplify parameters of sbi_illegal_insn_handler()
>   lib: sbi: Remove regs paramter of sbi_irqchip_process()
>   lib: sbi: Remove regs parameter from trap irq handling functions
>   lib: sbi: Pass trap context pointer to sbi_ecall_handler()
>   lib: sbi: Extend sbi_trap_error() to dump state in a nested trap
>
> Samuel Holland (1):
>   lib: sbi: Remove epc from struct sbi_trap_info

Applied this series to the riscv/opensbi repo.

Thanks,
Anup

>
>  firmware/fw_base.S             |  76 +++++++++----
>  include/sbi/riscv_encoding.h   |   2 +
>  include/sbi/sbi_ecall.h        |   4 +-
>  include/sbi/sbi_illegal_insn.h |   4 +-
>  include/sbi/sbi_irqchip.h      |   5 +-
>  include/sbi/sbi_scratch.h      |  14 +--
>  include/sbi/sbi_trap.h         |  45 ++++++--
>  include/sbi/sbi_trap_ldst.h    |  12 +-
>  lib/sbi/sbi_ecall.c            |   3 +-
>  lib/sbi/sbi_ecall_legacy.c     |   4 -
>  lib/sbi/sbi_expected_trap.S    |   4 -
>  lib/sbi/sbi_illegal_insn.c     |   9 +-
>  lib/sbi/sbi_irqchip.c          |  10 +-
>  lib/sbi/sbi_trap.c             | 196 +++++++++++++++------------------
>  lib/sbi/sbi_trap_ldst.c        |  71 ++++++------
>  lib/utils/irqchip/imsic.c      |   2 +-
>  16 files changed, 239 insertions(+), 222 deletions(-)
>
> --
> 2.34.1
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-03-19  6:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-17 13:02 [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel
2024-03-17 13:02 ` [PATCH v3 01/10] lib: sbi: Remove epc from struct sbi_trap_info Anup Patel
2024-03-17 13:02 ` [PATCH v3 02/10] lib: sbi: Remove sbi_trap_exit() and related code Anup Patel
2024-03-17 13:02 ` [PATCH v3 03/10] include: sbi: Add trap_context pointer in struct sbi_scratch Anup Patel
2024-03-17 13:02 ` [PATCH v3 04/10] lib: sbi: Introduce trap context Anup Patel
2024-03-17 13:02 ` [PATCH v3 05/10] lib: sbi: Simplify parameters of misaligned and access fault handlers Anup Patel
2024-03-17 13:02 ` [PATCH v3 06/10] lib: sbi: Simplify parameters of sbi_illegal_insn_handler() Anup Patel
2024-03-17 13:02 ` [PATCH v3 07/10] lib: sbi: Remove regs paramter of sbi_irqchip_process() Anup Patel
2024-03-17 13:02 ` [PATCH v3 08/10] lib: sbi: Remove regs parameter from trap irq handling functions Anup Patel
2024-03-17 13:02 ` [PATCH v3 09/10] lib: sbi: Pass trap context pointer to sbi_ecall_handler() Anup Patel
2024-03-17 13:02 ` [PATCH v3 10/10] lib: sbi: Extend sbi_trap_error() to dump state in a nested trap Anup Patel
2024-03-19  6:06 ` [PATCH v3 00/10] Improve trap handling for nested traps Anup Patel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox