As per the ISA definition, the upper 8 bits in hpmevent are defined
by Sscofpmf for privilege mode filtering and overflow bits while the
lower 56 bits are desginated for platform specific hpmevent values.
For the reset case, mhpmevent value should have zero in lower 56 bits.
Software may set the OF bit to indicate disable interrupt.

Ensure that correct value is checked after masking while clearing the
event encodings.

Signed-off-by: Atish Patra <atishp@rivosinc.com>
---
 target/riscv/pmu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
index cf713663ee56..0408f96e6af8 100644
--- a/target/riscv/pmu.c
+++ b/target/riscv/pmu.c
@@ -390,7 +390,7 @@ int riscv_pmu_update_event_map(CPURISCVState *env, uint64_t value,
      * Expected mhpmevent value is zero for reset case. Remove the current
      * mapping.
      */
-    if (!value) {
+    if (!(value & MHPMEVENT_IDX_MASK)) {
         g_hash_table_foreach_remove(cpu->pmu_event_ctr_map,
                                     pmu_remove_event_map,
                                     GUINT_TO_POINTER(ctr_idx));

-- 
2.34.1

On Thu, Jan 16, 2025 at 10:51 AM Atish Patra <atishp@rivosinc.com> wrote:
>
> As per the ISA definition, the upper 8 bits in hpmevent are defined
> by Sscofpmf for privilege mode filtering and overflow bits while the
> lower 56 bits are desginated for platform specific hpmevent values.
> For the reset case, mhpmevent value should have zero in lower 56 bits.
> Software may set the OF bit to indicate disable interrupt.
>
> Ensure that correct value is checked after masking while clearing the
> event encodings.
>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>

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

Alistair

> ---
>  target/riscv/pmu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
> index cf713663ee56..0408f96e6af8 100644
> --- a/target/riscv/pmu.c
> +++ b/target/riscv/pmu.c
> @@ -390,7 +390,7 @@ int riscv_pmu_update_event_map(CPURISCVState *env, uint64_t value,
>       * Expected mhpmevent value is zero for reset case. Remove the current
>       * mapping.
>       */
> -    if (!value) {
> +    if (!(value & MHPMEVENT_IDX_MASK)) {
>          g_hash_table_foreach_remove(cpu->pmu_event_ctr_map,
>                                      pmu_remove_event_map,
>                                      GUINT_TO_POINTER(ctr_idx));
>
> --
> 2.34.1
>
>

On 1/15/25 9:49 PM, Atish Patra wrote:
> As per the ISA definition, the upper 8 bits in hpmevent are defined
> by Sscofpmf for privilege mode filtering and overflow bits while the
> lower 56 bits are desginated for platform specific hpmevent values.

s/desginated/designated

> For the reset case, mhpmevent value should have zero in lower 56 bits.
> Software may set the OF bit to indicate disable interrupt.
> 
> Ensure that correct value is checked after masking while clearing the
> event encodings.
> 
> Signed-off-by: Atish Patra <atishp@rivosinc.com>
> ---


Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

>   target/riscv/pmu.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
> index cf713663ee56..0408f96e6af8 100644
> --- a/target/riscv/pmu.c
> +++ b/target/riscv/pmu.c
> @@ -390,7 +390,7 @@ int riscv_pmu_update_event_map(CPURISCVState *env, uint64_t value,
>        * Expected mhpmevent value is zero for reset case. Remove the current
>        * mapping.
>        */
> -    if (!value) {
> +    if (!(value & MHPMEVENT_IDX_MASK)) {
>           g_hash_table_foreach_remove(cpu->pmu_event_ctr_map,
>                                       pmu_remove_event_map,
>                                       GUINT_TO_POINTER(ctr_idx));
>