| Message ID | 20221215224541.1423431-1-abrestic@rivosinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] target/riscv: Fix up masking of vsip/vsie accesses | expand |
On Fri, Dec 16, 2022 at 8:46 AM Andrew Bresticker <abrestic@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 <abrestic@rivosinc.com> Reviewed-by: Alistair Francis <alistair.francis@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 > >
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;
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 <abrestic@rivosinc.com> --- target/riscv/csr.c | 35 +++++++++++------------------------ 1 file changed, 11 insertions(+), 24 deletions(-)