Re: [PATCH v3 2/3] lib: sbi: ISA extension emulation.

Hi,

This is really useful, even if it's not very fast, for obvious reasons.

Fairly late feedback on this, but I found two bugs and a nit when working
to get Ubuntu 26.04 working on X280 (Tenstorrent Blackhole). It boots
with these fixes applied. In particular, without the cbo.zero fix Python
crashes on init.

For those curious, booting is quite slow due to the massive amount of emulation
needed. Implemented stats, boot to login prompt takes:

L2CPU 0 ISA-emulation counters (version 5, hart 0):
  ID  Extension      count
----  -------------  ---------------
   1  ANY                  128201838 (sum of all below)
   2  Zcb                  103287492
   5  Zbs                   12818741
   6  Zicond                12090563
   9  Zfa                       4517
  17  Zvbb                        69
  18  VS-off-bounce              455



-Olof

On Sat, Dec 27, 2025 at 01:18:01PM +0100, Benedikt Freisen wrote:
> Add trap-based emulation code or compatibility stubs for the following ISA extensions:
> Zicbom, Zicboz, Zicond, Zimop, Zawrs, Zfa, Zfhmin, Zcb, Zcmop, Zba, Zbb, Zbc, Zbs, Supm
> 
> Signed-off-by: Benedikt Freisen <[email protected]>
> ---

[...]

> diff --git a/lib/sbi/sbi_insn_emu.c b/lib/sbi/sbi_insn_emu.c
> new file mode 100644
> index 00000000..ffc3bade
> --- /dev/null
> +++ b/lib/sbi/sbi_insn_emu.c

[...]

> +#if defined(CONFIG_EMU_ZBA) || defined(CONFIG_EMU_ZBB) ||     \
> +	defined(CONFIG_EMU_ZBC) || defined(CONFIG_EMU_ZBS) || \
> +	defined(CONFIG_EMU_ZICOND)
> +int sbi_insn_emu_op(ulong insn, struct sbi_trap_regs *regs)
> +{

[...]

> +#ifdef CONFIG_EMU_ZBC
> +	/* Emulate Zbc instructions */
> +	case INSN_MATCH_CLMUL:
> +		rd_val = 0;
> +		for (int i = 0; i < __riscv_xlen; i++) {
> +			if ((rs2_val >> i) & 1)
> +				rd_val ^= rs1_val << i;
> +		}
> +		break;
> +	case INSN_MATCH_CLMULH:
> +		rd_val = 0;
> +		for (int i = 1; i <= __riscv_xlen; i++) {
> +			if ((rs2_val >> i) & 1)
> +				rd_val ^= rs1_val >> (__riscv_xlen - i);
> +		}
> +		break;

The above needs to be < __riscv_xlen. Per the spec:

        for (int i = 1; i < XLEN; i++)
            if ((rs2 >> i) & 1)
                x ^= rs1 >> (XLEN - i);

> +	case INSN_MATCH_CLMULR:
> +		rd_val = 0;
> +		for (int i = 0; i < __riscv_xlen; i++) {
> +			if ((rs2_val >> i) & 1)
> +				rd_val ^= rs1_val >> (__riscv_xlen - i - 1);
> +		}
> +		break;
> +#endif

[...]

> +int sbi_insn_emu_zicbom_zicboz(ulong insn, struct sbi_trap_regs *regs)
> +{
> +	/* NOTE: Errata workarounds for fence instructions are handled in
> +	 * misc_mem_opcode_insn. */
> +
> +	/* Emulate Zicbom and Zicboz */
> +	switch (insn & INSN_MASK_CBO) {
> +	case INSN_MATCH_CBO_ZERO: {
> +		/* Check whether the instruction was even allowed */
> +		ulong prev_mode = sbi_mstatus_prev_mode(regs->mstatus);
> +		if ((prev_mode == PRV_U &&
> +		     !(read_senvcfg_or_emu() & ENVCFG_CBZE)) ||
> +		    (prev_mode == PRV_S &&
> +		     !(read_menvcfg_or_emu() & ENVCFG_CBZE)))
> +			return truly_illegal_insn(insn, regs);
> +
> +		u32 *addr =
> +			(u32 *)(GET_RS1S(insn, regs) & 0xffffffffffffffc0ull);

This needs to be GET_RS1(), or you end up picking up the wrong register in some
cases (gpr[8]). 

[...]

> +int sbi_insn_emu_store_fp(ulong insn, struct sbi_trap_regs *regs)
> +{
> +	struct sbi_trap_context *tcntx =
> +		container_of(regs, struct sbi_trap_context, regs);
> +
> +	/* If floating point is available and insn is FSH,
> +	 * simply use the misaligned store handler */
> +	if ((regs->mstatus & MSTATUS_FS) != 0 &&
> +	    (sbi_mstatus_prev_mode(regs->mstatus) != PRV_U ||
> +	     (csr_read(CSR_SSTATUS) & SSTATUS_FS) != 0) &&
> +	    (insn & INSN_MASK_FSH) == INSN_MATCH_FSH) {
> +		tcntx->trap.cause = CAUSE_MISALIGNED_LOAD;

Nit: This should be CAUSE_MISALIGNED_STORE

> +		tcntx->trap.tval  = GET_RS1(insn, regs) + IMM_S(insn);
> +		return sbi_misaligned_store_handler(tcntx);
> +	}
> +
> +	return truly_illegal_insn(insn, regs);
> +}



-Olof

-- 
opensbi mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/opensbi