On Fri, Nov 29, 2024 at 12:15 AM Clément Léger <cle...@rivosinc.com> wrote:
>
> With the current implementation, if we had the following scenario:
> - Set bit x in menvcfg
> - Set bit x in henvcfg
> - Clear bit x in menvcfg
> then, the internal variable env->henvcfg would still contain bit x due
> to both a wrong menvcfg mask used in write_henvcfg() as well as a
> missing update of henvcfg upon menvcfg update.
> This can lead to some wrong interpretation of the context. In order to
> update henvcfg upon menvcfg writing, call write_henvcfg() after writing
> menvcfg. Clearing henvcfg upon writing the new value is also needed in
> write_henvcfg() as well as clearing henvcfg upper part when writing it
> with write_henvcfgh().
>
> Signed-off-by: Clément Léger <cle...@rivosinc.com>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
Alistair
> ---
> target/riscv/csr.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 5d8d0d7514..98c683df60 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -2383,6 +2383,8 @@ static RISCVException read_menvcfg(CPURISCVState *env,
> int csrno,
> return RISCV_EXCP_NONE;
> }
>
> +static RISCVException write_henvcfg(CPURISCVState *env, int csrno,
> + target_ulong val);
> static RISCVException write_menvcfg(CPURISCVState *env, int csrno,
> target_ulong val)
> {
> @@ -2403,6 +2405,7 @@ static RISCVException write_menvcfg(CPURISCVState *env,
> int csrno,
> }
> }
> env->menvcfg = (env->menvcfg & ~mask) | (val & mask);
> + write_henvcfg(env, CSR_HENVCFG, env->henvcfg);
>
> return RISCV_EXCP_NONE;
> }
> @@ -2414,6 +2417,8 @@ static RISCVException read_menvcfgh(CPURISCVState *env,
> int csrno,
> return RISCV_EXCP_NONE;
> }
>
> +static RISCVException write_henvcfgh(CPURISCVState *env, int csrno,
> + target_ulong val);
> static RISCVException write_menvcfgh(CPURISCVState *env, int csrno,
> target_ulong val)
> {
> @@ -2424,6 +2429,7 @@ static RISCVException write_menvcfgh(CPURISCVState
> *env, int csrno,
> uint64_t valh = (uint64_t)val << 32;
>
> env->menvcfg = (env->menvcfg & ~mask) | (valh & mask);
> + write_henvcfgh(env, CSR_HENVCFGH, env->henvcfg >> 32);
>
> return RISCV_EXCP_NONE;
> }
> @@ -2513,7 +2519,7 @@ static RISCVException write_henvcfg(CPURISCVState *env,
> int csrno,
> }
> }
>
> - env->henvcfg = (env->henvcfg & ~mask) | (val & mask);
> + env->henvcfg = val & mask;
>
> return RISCV_EXCP_NONE;
> }
> @@ -2546,7 +2552,7 @@ static RISCVException write_henvcfgh(CPURISCVState
> *env, int csrno,
> return ret;
> }
>
> - env->henvcfg = (env->henvcfg & ~mask) | (valh & mask);
> + env->henvcfg = (env->henvcfg & 0xFFFFFFFF) | (valh & mask);
> return RISCV_EXCP_NONE;
> }
>
> --
> 2.45.2
>
>
