diff mbox series

target/riscv: Implement WARL behaviour for mcountinhibit/mcounteren

Message ID 20230802124906.24197-1-rbradford@rivosinc.com (mailing list archive)
State New, archived
Headers show
Series target/riscv: Implement WARL behaviour for mcountinhibit/mcounteren | expand

Commit Message

Rob Bradford Aug. 2, 2023, 12:49 p.m. UTC
These are WARL fields - zero out the bits for unavailable counters and
special case the TM bit in mcountinhibit which is hardwired to zero.
This patch achieves this by modifying the value written so that any use
of the field will see the correctly masked bits.

Tested by modifying OpenSBI to write max value to these CSRs and upon
subsequent read the appropriate number of bits for number of PMUs is
enabled and the TM bit is zero in mcountinhibit.

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
---
 target/riscv/csr.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Atish Patra Aug. 7, 2023, 4:34 p.m. UTC | #1
On Wed, Aug 2, 2023 at 5:50 AM Rob Bradford <rbradford@rivosinc.com> wrote:
>
> These are WARL fields - zero out the bits for unavailable counters and
> special case the TM bit in mcountinhibit which is hardwired to zero.
> This patch achieves this by modifying the value written so that any use
> of the field will see the correctly masked bits.
>
> Tested by modifying OpenSBI to write max value to these CSRs and upon
> subsequent read the appropriate number of bits for number of PMUs is
> enabled and the TM bit is zero in mcountinhibit.
>
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> ---
>  target/riscv/csr.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index ea7585329e..495ff6a9c2 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1834,8 +1834,11 @@ static RISCVException write_mcountinhibit(CPURISCVState *env, int csrno,
>  {
>      int cidx;
>      PMUCTRState *counter;
> +    RISCVCPU *cpu = env_archcpu(env);
>
> -    env->mcountinhibit = val;
> +    /* WARL register - disable unavailable counters; TM bit is always 0 */
> +    env->mcountinhibit =
> +        val & (cpu->pmu_avail_ctrs | COUNTEREN_CY | COUNTEREN_IR);
>
>      /* Check if any other counter is also monitoring cycles/instructions */
>      for (cidx = 0; cidx < RV_MAX_MHPMCOUNTERS; cidx++) {
> @@ -1858,7 +1861,11 @@ static RISCVException read_mcounteren(CPURISCVState *env, int csrno,
>  static RISCVException write_mcounteren(CPURISCVState *env, int csrno,
>                                         target_ulong val)
>  {
> -    env->mcounteren = val;
> +    RISCVCPU *cpu = env_archcpu(env);
> +
> +    /* WARL register - disable unavailable counters */
> +    env->mcounteren = val & (cpu->pmu_avail_ctrs | COUNTEREN_CY | COUNTEREN_TM |
> +                             COUNTEREN_IR);
>      return RISCV_EXCP_NONE;
>  }
>
> --
> 2.41.0
>
>

LGTM.
Reviewed-by: Atish Patra <atishp@rivosinc.com>
Alistair Francis Aug. 10, 2023, 5:04 p.m. UTC | #2
On Wed, Aug 2, 2023 at 8:50 AM Rob Bradford <rbradford@rivosinc.com> wrote:
>
> These are WARL fields - zero out the bits for unavailable counters and
> special case the TM bit in mcountinhibit which is hardwired to zero.
> This patch achieves this by modifying the value written so that any use
> of the field will see the correctly masked bits.
>
> Tested by modifying OpenSBI to write max value to these CSRs and upon
> subsequent read the appropriate number of bits for number of PMUs is
> enabled and the TM bit is zero in mcountinhibit.
>
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>

Acked-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/csr.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index ea7585329e..495ff6a9c2 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1834,8 +1834,11 @@ static RISCVException write_mcountinhibit(CPURISCVState *env, int csrno,
>  {
>      int cidx;
>      PMUCTRState *counter;
> +    RISCVCPU *cpu = env_archcpu(env);
>
> -    env->mcountinhibit = val;
> +    /* WARL register - disable unavailable counters; TM bit is always 0 */
> +    env->mcountinhibit =
> +        val & (cpu->pmu_avail_ctrs | COUNTEREN_CY | COUNTEREN_IR);
>
>      /* Check if any other counter is also monitoring cycles/instructions */
>      for (cidx = 0; cidx < RV_MAX_MHPMCOUNTERS; cidx++) {
> @@ -1858,7 +1861,11 @@ static RISCVException read_mcounteren(CPURISCVState *env, int csrno,
>  static RISCVException write_mcounteren(CPURISCVState *env, int csrno,
>                                         target_ulong val)
>  {
> -    env->mcounteren = val;
> +    RISCVCPU *cpu = env_archcpu(env);
> +
> +    /* WARL register - disable unavailable counters */
> +    env->mcounteren = val & (cpu->pmu_avail_ctrs | COUNTEREN_CY | COUNTEREN_TM |
> +                             COUNTEREN_IR);
>      return RISCV_EXCP_NONE;
>  }
>
> --
> 2.41.0
>
>
Alistair Francis Aug. 10, 2023, 5:07 p.m. UTC | #3
On Wed, Aug 2, 2023 at 8:50 AM Rob Bradford <rbradford@rivosinc.com> wrote:
>
> These are WARL fields - zero out the bits for unavailable counters and
> special case the TM bit in mcountinhibit which is hardwired to zero.
> This patch achieves this by modifying the value written so that any use
> of the field will see the correctly masked bits.
>
> Tested by modifying OpenSBI to write max value to these CSRs and upon
> subsequent read the appropriate number of bits for number of PMUs is
> enabled and the TM bit is zero in mcountinhibit.
>
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/csr.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index ea7585329e..495ff6a9c2 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -1834,8 +1834,11 @@ static RISCVException write_mcountinhibit(CPURISCVState *env, int csrno,
>  {
>      int cidx;
>      PMUCTRState *counter;
> +    RISCVCPU *cpu = env_archcpu(env);
>
> -    env->mcountinhibit = val;
> +    /* WARL register - disable unavailable counters; TM bit is always 0 */
> +    env->mcountinhibit =
> +        val & (cpu->pmu_avail_ctrs | COUNTEREN_CY | COUNTEREN_IR);
>
>      /* Check if any other counter is also monitoring cycles/instructions */
>      for (cidx = 0; cidx < RV_MAX_MHPMCOUNTERS; cidx++) {
> @@ -1858,7 +1861,11 @@ static RISCVException read_mcounteren(CPURISCVState *env, int csrno,
>  static RISCVException write_mcounteren(CPURISCVState *env, int csrno,
>                                         target_ulong val)
>  {
> -    env->mcounteren = val;
> +    RISCVCPU *cpu = env_archcpu(env);
> +
> +    /* WARL register - disable unavailable counters */
> +    env->mcounteren = val & (cpu->pmu_avail_ctrs | COUNTEREN_CY | COUNTEREN_TM |
> +                             COUNTEREN_IR);
>      return RISCV_EXCP_NONE;
>  }
>
> --
> 2.41.0
>
>
diff mbox series

Patch

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index ea7585329e..495ff6a9c2 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -1834,8 +1834,11 @@  static RISCVException write_mcountinhibit(CPURISCVState *env, int csrno,
 {
     int cidx;
     PMUCTRState *counter;
+    RISCVCPU *cpu = env_archcpu(env);
 
-    env->mcountinhibit = val;
+    /* WARL register - disable unavailable counters; TM bit is always 0 */
+    env->mcountinhibit =
+        val & (cpu->pmu_avail_ctrs | COUNTEREN_CY | COUNTEREN_IR);
 
     /* Check if any other counter is also monitoring cycles/instructions */
     for (cidx = 0; cidx < RV_MAX_MHPMCOUNTERS; cidx++) {
@@ -1858,7 +1861,11 @@  static RISCVException read_mcounteren(CPURISCVState *env, int csrno,
 static RISCVException write_mcounteren(CPURISCVState *env, int csrno,
                                        target_ulong val)
 {
-    env->mcounteren = val;
+    RISCVCPU *cpu = env_archcpu(env);
+
+    /* WARL register - disable unavailable counters */
+    env->mcounteren = val & (cpu->pmu_avail_ctrs | COUNTEREN_CY | COUNTEREN_TM |
+                             COUNTEREN_IR);
     return RISCV_EXCP_NONE;
 }