| Message ID | 20250425152311.804338-8-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | target/riscv: Fix write_misa vs aligned next_pc | expand |
On 25/4/25 17:23, Richard Henderson 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> > --- > target/riscv/csr.c | 22 +++++++++++++++++----- > 1 file changed, 17 insertions(+), 5 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
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 > >
On 4/25/25 08:23, Richard Henderson wrote: > - if ((val & RVC) && (GETPC() & ~3) != 0) { > + /* Suppress 'C' if next instruction is not aligned. */ > + if ((val & RVC) && (get_next_pc(env, ra) & ~3) != 0) { Bah. I preserved a second bug here: not "& ~3" but "& 3". r~
On Wed, Apr 30, 2025 at 12:34 AM Richard Henderson <richard.henderson@linaro.org> wrote: > > On 4/25/25 08:23, Richard Henderson wrote: > > - if ((val & RVC) && (GETPC() & ~3) != 0) { > > + /* Suppress 'C' if next instruction is not aligned. */ > > + if ((val & RVC) && (get_next_pc(env, ra) & ~3) != 0) { > > Bah. I preserved a second bug here: not "& ~3" but "& 3". Good catch I squashed this fix into the patch Alistair > > > r~ >
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; }
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> --- target/riscv/csr.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)