On Mon, Jun 24, 2019 at 2:31 AM Palmer Dabbelt <pal...@sifive.com> wrote: > > On Mon, 17 Jun 2019 18:31:08 PDT (-0700), Alistair Francis wrote: > > Add a comment for the new mcountinhibit which conflicts with the > > CSR_MUCOUNTEREN from version 1.09.1. This can be updated when we remove > > 1.09.1. > > > > Signed-off-by: Alistair Francis <alistair.fran...@wdc.com> > > --- > > target/riscv/cpu_bits.h | 1 + > > target/riscv/csr.c | 6 ++++-- > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h > > index 47450a3cdb..11f971ad5d 100644 > > --- a/target/riscv/cpu_bits.h > > +++ b/target/riscv/cpu_bits.h > > @@ -136,6 +136,7 @@ > > #define CSR_MCOUNTEREN 0x306 > > > > /* Legacy Counter Setup (priv v1.9.1) */ > > +/* Update to #define CSR_MCOUNTINHIBIT 0x320 for 1.11.0 */ > > #define CSR_MUCOUNTEREN 0x320 > > #define CSR_MSCOUNTEREN 0x321 > > #define CSR_MHCOUNTEREN 0x322 > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > > index c67d29e206..437387fd28 100644 > > --- a/target/riscv/csr.c > > +++ b/target/riscv/csr.c > > @@ -461,18 +461,20 @@ static int write_mcounteren(CPURISCVState *env, int > > csrno, target_ulong val) > > return 0; > > } > > > > +/* This regiser is replaced with CSR_MCOUNTINHIBIT in 1.11.0 */ > > static int read_mscounteren(CPURISCVState *env, int csrno, target_ulong > > *val) > > { > > - if (env->priv_ver > PRIV_VERSION_1_09_1) { > > + if (env->priv_ver > PRIV_VERSION_1_09_1 && env->priv_ver < > > PRIV_VERSION_1_11_0) { > > return -1; > > } > > *val = env->mcounteren; > > return 0; > > } > > > > +/* This regiser is replaced with CSR_MCOUNTINHIBIT in 1.11.0 */ > > static int write_mscounteren(CPURISCVState *env, int csrno, target_ulong > > val) > > { > > - if (env->priv_ver > PRIV_VERSION_1_09_1) { > > + if (env->priv_ver > PRIV_VERSION_1_09_1 && env->priv_ver < > > PRIV_VERSION_1_11_0) { > > return -1; > > } > > env->mcounteren = val; > > I don't think this one is right: this should be unsupported on 1.11, as the > semantics of this bit are slightly different. It shouldn't be that hard to > just emulate it fully for both 1.09.1 and 1.11: for 1.09 this disables access > to the counters (which still tick), while for 1.11 it disables ticking the > counters (which can still be accessed). Since we don't do anything with the > counters in QEMU, I think this should do it > > LMK if you're OK with me replacing the patch with this > > commit e9169ccd5ca97a036de41dad23f37f6724712b90 > Author: Alistair Francis <alistair.fran...@wdc.com> > Date: Mon Jun 17 18:31:08 2019 -0700 > > target/riscv: Add the mcountinhibit CSR > > 1.11 defines mcountinhibit, which has the same numeric CSR value as > mucounteren from 1.09.1 but has different semantics. This patch enables > the CSR for 1.11-based targets, which is trivial to implement because > the counters in QEMU never tick (legal according to the spec). > > Signed-off-by: Alistair Francis <alistair.fran...@wdc.com> > [Palmer: Fix counter access semantics, change commit message to indicate > the behavior is fully emulated.] > Reviewed-by: Palmer Dabbelt <pal...@sifive.com> > Signed-off-by: Palmer Dabbelt <pal...@sifive.com>
Yep, looks good. Alistair > > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h > index 47450a3cdb75..11f971ad5df0 100644 > --- a/target/riscv/cpu_bits.h > +++ b/target/riscv/cpu_bits.h > @@ -136,6 +136,7 @@ > #define CSR_MCOUNTEREN 0x306 > > /* Legacy Counter Setup (priv v1.9.1) */ > +/* Update to #define CSR_MCOUNTINHIBIT 0x320 for 1.11.0 */ > #define CSR_MUCOUNTEREN 0x320 > #define CSR_MSCOUNTEREN 0x321 > #define CSR_MHCOUNTEREN 0x322 > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index c67d29e20618..2622b2e05474 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -56,6 +56,14 @@ static int fs(CPURISCVState *env, int csrno) > static int ctr(CPURISCVState *env, int csrno) > { > #if !defined(CONFIG_USER_ONLY) > + /* > + * The counters are always enabled on newer priv specs, as the CSR has > + * changed from controlling that the counters can be read to controlling > + * that the counters increment. > + */ > + if (env->priv_ver > PRIV_VERSION_1_09_1) > + return 0; > + > uint32_t ctr_en = ~0u; > > if (env->priv < PRV_M) { > @@ -461,18 +469,20 @@ static int write_mcounteren(CPURISCVState *env, int > csrno, target_ulong val) > return 0; > } > > +/* This regiser is replaced with CSR_MCOUNTINHIBIT in 1.11.0 */ > static int read_mscounteren(CPURISCVState *env, int csrno, target_ulong *val) > { > - if (env->priv_ver > PRIV_VERSION_1_09_1) { > + if (env->priv_ver > PRIV_VERSION_1_09_1 && env->priv_ver < > PRIV_VERSION_1_11_0) { > return -1; > } > *val = env->mcounteren; > return 0; > } > > +/* This regiser is replaced with CSR_MCOUNTINHIBIT in 1.11.0 */ > static int write_mscounteren(CPURISCVState *env, int csrno, target_ulong val) > { > - if (env->priv_ver > PRIV_VERSION_1_09_1) { > + if (env->priv_ver > PRIV_VERSION_1_09_1 && env->priv_ver < > PRIV_VERSION_1_11_0) { > return -1; > } > env->mcounteren = val;