* Re: [PATCH] target/riscv: Implement WARL behaviour for mcountinhibit/mcounteren
2023-08-02 12:49 [PATCH] target/riscv: Implement WARL behaviour for mcountinhibit/mcounteren Rob Bradford
@ 2023-08-07 16:34 ` Atish Patra
2023-08-10 17:04 ` Alistair Francis
2023-08-10 17:07 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Atish Patra @ 2023-08-07 16:34 UTC (permalink / raw)
To: Rob Bradford
Cc: qemu-devel, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, open list:RISC-V TCG CPUs
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>
--
Regards,
Atish
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] target/riscv: Implement WARL behaviour for mcountinhibit/mcounteren
2023-08-02 12:49 [PATCH] target/riscv: Implement WARL behaviour for mcountinhibit/mcounteren Rob Bradford
2023-08-07 16:34 ` Atish Patra
@ 2023-08-10 17:04 ` Alistair Francis
2023-08-10 17:07 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2023-08-10 17:04 UTC (permalink / raw)
To: Rob Bradford
Cc: qemu-devel, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, open list:RISC-V TCG CPUs
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
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] target/riscv: Implement WARL behaviour for mcountinhibit/mcounteren
2023-08-02 12:49 [PATCH] target/riscv: Implement WARL behaviour for mcountinhibit/mcounteren Rob Bradford
2023-08-07 16:34 ` Atish Patra
2023-08-10 17:04 ` Alistair Francis
@ 2023-08-10 17:07 ` Alistair Francis
2 siblings, 0 replies; 4+ messages in thread
From: Alistair Francis @ 2023-08-10 17:07 UTC (permalink / raw)
To: Rob Bradford
Cc: qemu-devel, Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, open list:RISC-V TCG CPUs
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
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread