On Fri, Dec 16, 2022 at 8:46 AM Andrew Bresticker <abres...@rivosinc.com> wrote:
>
> The current logic attempts to shift the VS-level bits into their correct
> position in mip while leaving the remaining bits in-tact. This is both
> pointless and likely incorrect since one would expect that any new, future
> VS-level interrupts will get their own position in mip rather than sharing
> with their (H)S-level equivalent. Fix this, and make the logic more
> readable, by just making off the VS-level bits and shifting them into
> position.
>
> This also fixes reads of vsip, which would only ever report vsip.VSSIP
> since the non-writable bits got masked off as well.
>
> Fixes: d028ac7512f1 ("arget/riscv: Implement AIA CSRs for 64 local interrupts
> on RV32")
> Signed-off-by: Andrew Bresticker <abres...@rivosinc.com>
Reviewed-by: Alistair Francis <alistair.fran...@wdc.com>
Alistair
> ---
> target/riscv/csr.c | 35 +++++++++++------------------------
> 1 file changed, 11 insertions(+), 24 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 5c9a7ee287..984548bf87 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1975,22 +1975,15 @@ static RISCVException rmw_vsie64(CPURISCVState *env,
> int csrno,
> uint64_t new_val, uint64_t wr_mask)
> {
> RISCVException ret;
> - uint64_t rval, vsbits, mask = env->hideleg & VS_MODE_INTERRUPTS;
> + uint64_t rval, mask = env->hideleg & VS_MODE_INTERRUPTS;
>
> /* Bring VS-level bits to correct position */
> - vsbits = new_val & (VS_MODE_INTERRUPTS >> 1);
> - new_val &= ~(VS_MODE_INTERRUPTS >> 1);
> - new_val |= vsbits << 1;
> - vsbits = wr_mask & (VS_MODE_INTERRUPTS >> 1);
> - wr_mask &= ~(VS_MODE_INTERRUPTS >> 1);
> - wr_mask |= vsbits << 1;
> + new_val = (new_val & (VS_MODE_INTERRUPTS >> 1)) << 1;
> + wr_mask = (wr_mask & (VS_MODE_INTERRUPTS >> 1)) << 1;
>
> ret = rmw_mie64(env, csrno, &rval, new_val, wr_mask & mask);
> if (ret_val) {
> - rval &= mask;
> - vsbits = rval & VS_MODE_INTERRUPTS;
> - rval &= ~VS_MODE_INTERRUPTS;
> - *ret_val = rval | (vsbits >> 1);
> + *ret_val = (rval & mask) >> 1;
> }
>
> return ret;
> @@ -2191,22 +2184,16 @@ static RISCVException rmw_vsip64(CPURISCVState *env,
> int csrno,
> uint64_t new_val, uint64_t wr_mask)
> {
> RISCVException ret;
> - uint64_t rval, vsbits, mask = env->hideleg & vsip_writable_mask;
> + uint64_t rval, mask = env->hideleg & VS_MODE_INTERRUPTS;
>
> /* Bring VS-level bits to correct position */
> - vsbits = new_val & (VS_MODE_INTERRUPTS >> 1);
> - new_val &= ~(VS_MODE_INTERRUPTS >> 1);
> - new_val |= vsbits << 1;
> - vsbits = wr_mask & (VS_MODE_INTERRUPTS >> 1);
> - wr_mask &= ~(VS_MODE_INTERRUPTS >> 1);
> - wr_mask |= vsbits << 1;
> -
> - ret = rmw_mip64(env, csrno, &rval, new_val, wr_mask & mask);
> + new_val = (new_val & (VS_MODE_INTERRUPTS >> 1)) << 1;
> + wr_mask = (wr_mask & (VS_MODE_INTERRUPTS >> 1)) << 1;
> +
> + ret = rmw_mip64(env, csrno, &rval, new_val,
> + wr_mask & mask & vsip_writable_mask);
> if (ret_val) {
> - rval &= mask;
> - vsbits = rval & VS_MODE_INTERRUPTS;
> - rval &= ~VS_MODE_INTERRUPTS;
> - *ret_val = rval | (vsbits >> 1);
> + *ret_val = (rval & mask) >> 1;
> }
>
> return ret;
> --
> 2.25.1
>
>
