Re: [PATCH v3 1/3] target/riscv: Fix IALIGN check in misa write

[Alistair Francis <alistair23@gmail.com>]

On Sun, Mar 22, 2026 at 12:47 AM Nicholas Piggin <npiggin@gmail.com> wrote:
>
> The instruction alignment check for the C extension was inverted.
> The new value should be checked for C bit clear (thus increasing
> IALIGN). If IALIGN is incompatible, then the write to misa should
> be suppressed, not just ignoring the update to the C bit.
>
> From the ISA:
>
>   Writing misa may increase IALIGN, e.g., by disabling the "C"
>   extension. If an instruction that would write misa increases IALIGN,
>   and the subsequent instruction’s address is not IALIGN-bit aligned,
>   the write to misa is suppressed, leaving misa unchanged.
>
> This was found with a verification test generator based on RiESCUE.
>
> Reported-by: Nicholas Joaquin <njoaquin@tenstorrent.com>
> Reported-by: Ganesh Valliappan <gvalliappan@tenstorrent.com>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/csr.c                        | 16 ++++-
>  tests/tcg/riscv64/Makefile.softmmu-target |  5 ++
>  tests/tcg/riscv64/misa-ialign.S           | 88 +++++++++++++++++++++++
>  3 files changed, 106 insertions(+), 3 deletions(-)
>  create mode 100644 tests/tcg/riscv64/misa-ialign.S
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 5064483917..91421a2dd8 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -2129,9 +2129,19 @@ static RISCVException write_misa(CPURISCVState *env, int csrno,
>      /* Mask extensions that are not supported by this hart */
>      val &= env->misa_ext_mask;
>
> -    /* Suppress 'C' if next instruction is not aligned. */
> -    if ((val & RVC) && (get_next_pc(env, ra) & 3) != 0) {
> -        val &= ~RVC;
> +    /*
> +     * misa writes that increase IALIGN beyond alignment of the next
> +     * instruction cause the write to misa to be suppressed. Clearing
> +     * "C" extension increases IALIGN.
> +     */
> +    if (!(val & RVC) && (get_next_pc(env, ra) & 3) != 0) {
> +        /*
> +         * If the next instruction is unaligned mod 4 then "C" must be
> +         * set or this instruction could not be executing, so we know
> +         * this is is clearing "C" (and not just keeping it clear).
> +         */
> +        g_assert(env->misa_ext & RVC);
> +        return RISCV_EXCP_NONE;
>      }
>
>      /* Disable RVG if any of its dependencies are disabled */
> diff --git a/tests/tcg/riscv64/Makefile.softmmu-target b/tests/tcg/riscv64/Makefile.softmmu-target
> index eb1ce6504a..f176f87ed0 100644
> --- a/tests/tcg/riscv64/Makefile.softmmu-target
> +++ b/tests/tcg/riscv64/Makefile.softmmu-target
> @@ -36,5 +36,10 @@ run-plugin-interruptedmemory: interruptedmemory
>           $(QEMU) -plugin ../plugins/libdiscons.so -d plugin -D $<.pout \
>           $(QEMU_OPTS)$<)
>
> +EXTRA_RUNS += run-misa-ialign
> +run-misa-ialign: QEMU_OPTS := -cpu rv64,c=true,v=true,x-misa-w=on $(QEMU_OPTS)
> +run-misa-ialign: misa-ialign
> +       $(call run-test, $<, $(QEMU) $(QEMU_OPTS)$<)
> +
>  # We don't currently support the multiarch system tests
>  undefine MULTIARCH_TESTS
> diff --git a/tests/tcg/riscv64/misa-ialign.S b/tests/tcg/riscv64/misa-ialign.S
> new file mode 100644
> index 0000000000..7f1eb30023
> --- /dev/null
> +++ b/tests/tcg/riscv64/misa-ialign.S
> @@ -0,0 +1,88 @@
> +/*
> + * Test for MISA changing C and related IALIGN alignment cases
> + *
> + * This test verifies that the "C" extension can be cleared and set in MISA,
> + * that a branch to 2-byte aligned instructions can be executed when "C" is
> + * enabled, and that a write to MISA which would increase IALIGN and cause
> + * the next instruction to be unaligned is ignored.
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#define RVC (1 << ('C'-'A'))
> +#define RVV (1 << ('V'-'A'))
> +
> +.option norvc
> +       .text
> +       .global _start
> +_start:
> +       lla     t0, trap
> +       csrw    mtvec, t0
> +
> +       csrr    t0, misa
> +       li      t1, RVC
> +       not     t1, t1
> +       and     t0, t0, t1
> +       csrw    misa, t0
> +       csrr    t1, misa
> +       li      a0, 2 # fail code
> +       bne     t0, t1, _exit # Could not clear RVC in MISA
> +
> +       li      t1, RVC
> +       or      t0, t0, t1
> +       csrw    misa, t0
> +       csrr    t1, misa
> +       li      a0, 3 # fail code
> +       bne     t0, t1, _exit # Could not set RVC in MISA
> +
> +       j       unalign
> +. = . + 2
> +unalign:
> +
> +       li      t1, RVC
> +       not     t1, t1
> +       and     t0, t0, t1
> +       csrw    misa, t0
> +       csrr    t1, misa
> +       li      a0, 4 # fail code
> +       beq     t0, t1, _exit # Was able to clear RVC in MISA
> +
> +       li      t0, (RVC|RVV)
> +       not     t0, t0
> +       and     t0, t0, t1
> +       csrw    misa, t0
> +       csrr    t0, misa
> +       li      a0, 5 # fail code
> +       bne     t0, t1, _exit # MISA write was not ignored (RVV was cleared)
> +
> +       j       realign
> +. = . + 2
> +realign:
> +
> +       # Success!
> +       li      a0, 0
> +       j       _exit
> +
> +trap:
> +       # Any trap is a fail code 1
> +       li      a0, 1
> +
> +# Exit code in a0
> +_exit:
> +       lla     a1, semiargs
> +       li      t0, 0x20026     # ADP_Stopped_ApplicationExit
> +       sd      t0, 0(a1)
> +       sd      a0, 8(a1)
> +       li      a0, 0x20        # TARGET_SYS_EXIT_EXTENDED
> +
> +       # Semihosting call sequence
> +       .balign 16
> +       slli    zero, zero, 0x1f
> +       ebreak
> +       srai    zero, zero, 0x7
> +       j       .
> +
> +       .data
> +       .balign 16
> +semiargs:
> +       .space  16
> --
> 2.51.0
>
>