diff mbox series

[v2,5/7] lib: sbi: Workaround for FENCE(.I) errata on C906, C910.

Message ID 20251114203842.13396-6-b.freisen@gmx.net
State Changes Requested
Headers show
Series Add trap-based ISA extension emulation | expand

Commit Message

Benedikt Freisen Nov. 14, 2025, 8:38 p.m. UTC
According to the RISCVuzz paper by Thomas et al., the T-Head/XuanTie C906
and C910 cores fail to ignore reserved fields in the "fence" and "fence.i"
encodings and trigger illegal instruction traps if these fields are non-zero,
so address that in the illegal instruction trap handler.

Signed-off-by: Benedikt Freisen <b.freisen@gmx.net>
---
 include/sbi/riscv_encoding.h |  4 ++++
 lib/sbi/sbi_illegal_insn.c   | 17 +++++++++++++++++
 2 files changed, 21 insertions(+)

Comments

Anup Patel Dec. 15, 2025, 3:56 p.m. UTC | #1
On Sat, Nov 15, 2025 at 2:09 AM Benedikt Freisen <b.freisen@gmx.net> wrote:
>
> According to the RISCVuzz paper by Thomas et al., the T-Head/XuanTie C906
> and C910 cores fail to ignore reserved fields in the "fence" and "fence.i"
> encodings and trigger illegal instruction traps if these fields are non-zero,
> so address that in the illegal instruction trap handler.
>
> Signed-off-by: Benedikt Freisen <b.freisen@gmx.net>
> ---
>  include/sbi/riscv_encoding.h |  4 ++++
>  lib/sbi/sbi_illegal_insn.c   | 17 +++++++++++++++++
>  2 files changed, 21 insertions(+)
>
> diff --git a/include/sbi/riscv_encoding.h b/include/sbi/riscv_encoding.h
> index 2970ba0..d956ebb 100644
> --- a/include/sbi/riscv_encoding.h
> +++ b/include/sbi/riscv_encoding.h
> @@ -944,8 +944,12 @@
>  #define INSN_MASK_WFI                  0xffffff00
>  #define INSN_MATCH_WFI                 0x10500000
>
> +#define INSN_MASK_FENCE                        0x0000707f
> +#define INSN_MATCH_FENCE               0x0000000f
>  #define INSN_MASK_FENCE_TSO            0xfff0707f
>  #define INSN_MATCH_FENCE_TSO           0x8330000f
> +#define INSN_MASK_FENCE_I              0x0000707f
> +#define INSN_MATCH_FENCE_I             0x0000100f
>
>  #define INSN_MASK_VECTOR_UNIT_STRIDE           0xfdf0707f
>  #define INSN_MASK_VECTOR_FAULT_ONLY_FIRST      0xfdf0707f
> diff --git a/lib/sbi/sbi_illegal_insn.c b/lib/sbi/sbi_illegal_insn.c
> index fa82264..ed51f4d 100644
> --- a/lib/sbi/sbi_illegal_insn.c
> +++ b/lib/sbi/sbi_illegal_insn.c
> @@ -42,6 +42,23 @@ static int misc_mem_opcode_insn(ulong insn, struct sbi_trap_regs *regs)
>                 return 0;
>         }
>
> +#ifdef CONFIG_THEAD_C9XX_ERRATA
> +       /* Errata workaround: C906, C910 fail to ignore reserved fields
> +        * in the `fence` and `fence.i` encodings. [Thomas2024RISCVuzz] */
> +       if ((insn & INSN_MASK_FENCE) == INSN_MATCH_FENCE) {
> +               /* NOTE: Emulation should ideally preserve the `pred` and
> +                * `succ` fields, but that is not easily possible here. */
> +               mb();
> +               regs->mepc += 4;
> +               return 0;
> +       }
> +       if ((insn & INSN_MASK_FENCE_I) == INSN_MATCH_FENCE_I) {
> +               RISCV_FENCE_I;
> +               regs->mepc += 4;
> +               return 0;
> +       }
> +#endif
> +

Generic platform allows running the same firmware on multiple
platforms so CONFIG_THEAD_C9XX_ERRATA will be enabled
along with kconfig options of other platforms so I think there is
no point of having "#ifdef CONFIG_THEAD_C9XX_ERRATA"
check.

Regards,
Anup
diff mbox series

Patch

diff --git a/include/sbi/riscv_encoding.h b/include/sbi/riscv_encoding.h
index 2970ba0..d956ebb 100644
--- a/include/sbi/riscv_encoding.h
+++ b/include/sbi/riscv_encoding.h
@@ -944,8 +944,12 @@ 
 #define INSN_MASK_WFI			0xffffff00
 #define INSN_MATCH_WFI			0x10500000
 
+#define INSN_MASK_FENCE			0x0000707f
+#define INSN_MATCH_FENCE		0x0000000f
 #define INSN_MASK_FENCE_TSO		0xfff0707f
 #define INSN_MATCH_FENCE_TSO		0x8330000f
+#define INSN_MASK_FENCE_I		0x0000707f
+#define INSN_MATCH_FENCE_I		0x0000100f
 
 #define INSN_MASK_VECTOR_UNIT_STRIDE		0xfdf0707f
 #define INSN_MASK_VECTOR_FAULT_ONLY_FIRST	0xfdf0707f
diff --git a/lib/sbi/sbi_illegal_insn.c b/lib/sbi/sbi_illegal_insn.c
index fa82264..ed51f4d 100644
--- a/lib/sbi/sbi_illegal_insn.c
+++ b/lib/sbi/sbi_illegal_insn.c
@@ -42,6 +42,23 @@  static int misc_mem_opcode_insn(ulong insn, struct sbi_trap_regs *regs)
 		return 0;
 	}
 
+#ifdef CONFIG_THEAD_C9XX_ERRATA
+	/* Errata workaround: C906, C910 fail to ignore reserved fields
+	 * in the `fence` and `fence.i` encodings. [Thomas2024RISCVuzz] */
+	if ((insn & INSN_MASK_FENCE) == INSN_MATCH_FENCE) {
+		/* NOTE: Emulation should ideally preserve the `pred` and
+		 * `succ` fields, but that is not easily possible here. */
+		mb();
+		regs->mepc += 4;
+		return 0;
+	}
+	if ((insn & INSN_MASK_FENCE_I) == INSN_MATCH_FENCE_I) {
+		RISCV_FENCE_I;
+		regs->mepc += 4;
+		return 0;
+	}
+#endif
+
 	return truly_illegal_insn(insn, regs);
 }