Re: [PATCH 7/7] target/riscv: Fix write_misa vs aligned next_pc

[Alistair Francis <alistair23@gmail.com>]

On Sat, Apr 26, 2025 at 1:26 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Do not examine a random host return address, but
> properly compute the next pc for the guest cpu.
>
> Fixes: f18637cd611 ("RISC-V: Add misa runtime write support")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

Alistair

> ---
>  target/riscv/csr.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index a663f527a4..85f9b4c3d2 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -30,6 +30,8 @@
>  #include "exec/icount.h"
>  #include "qemu/guest-random.h"
>  #include "qapi/error.h"
> +#include "tcg/insn-start-words.h"
> +#include "internals.h"
>  #include <stdbool.h>
>
>  /* CSR function table public API */
> @@ -2099,6 +2101,19 @@ static RISCVException read_misa(CPURISCVState *env, int csrno,
>      return RISCV_EXCP_NONE;
>  }
>
> +static target_ulong get_next_pc(CPURISCVState *env, uintptr_t ra)
> +{
> +    uint64_t data[TARGET_INSN_START_WORDS];
> +
> +    /* Outside of a running cpu, env contains the next pc. */
> +    if (ra == 0 || !cpu_unwind_state_data(env_cpu(env), ra, data)) {
> +        return env->pc;
> +    }
> +
> +    /* Within unwind data, [0] is pc and [1] is the opcode. */
> +    return data[0] + insn_len(data[1]);
> +}
> +
>  static RISCVException write_misa(CPURISCVState *env, int csrno,
>                                   target_ulong val, uintptr_t ra)
>  {
> @@ -2114,11 +2129,8 @@ 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
> -     * TODO: this should check next_pc
> -     */
> -    if ((val & RVC) && (GETPC() & ~3) != 0) {
> +    /* Suppress 'C' if next instruction is not aligned. */
> +    if ((val & RVC) && (get_next_pc(env, ra) & ~3) != 0) {
>          val &= ~RVC;
>      }
>
> --
> 2.43.0
>
>