Re: [PATCH v4 09/11] target/riscv: Simplify counter predicate function
On Mon, Jan 10, 2022 at 12:27 AM Bin Meng wrote: > > On Fri, Jan 7, 2022 at 10:22 AM Atish Patra wrote: > > > > All the hpmcounters and the fixed counters (CY, IR, TM) can be represented > > as a unified counter. Thus, the predicate function doesn't need handle each > > case separately. > > > > Simplify the predicate function so that we just handle things differently > > between RV32/RV64 and S/HS mode. > > > > Signed-off-by: Atish Patra > > --- > > target/riscv/csr.c | 111 - > > 1 file changed, 10 insertions(+), 101 deletions(-) > > > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > > index d3a8bba6a518..feb053eb3f7b 100644 > > --- a/target/riscv/csr.c > > +++ b/target/riscv/csr.c > > @@ -109,6 +109,7 @@ static RISCVException ctr(CPURISCVState *env, int csrno) > > CPUState *cs = env_cpu(env); > > RISCVCPU *cpu = RISCV_CPU(cs); > > int ctr_index; > > +uint64_t ctr_mask; > > Use target_ulong should be enough? > Yeah. Will fix it in the next version. > > int base_csrno = CSR_CYCLE; > > bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false; > > > > @@ -117,122 +118,30 @@ static RISCVException ctr(CPURISCVState *env, int > > csrno) > > base_csrno += 0x80; > > } > > ctr_index = csrno - base_csrno; > > +ctr_mask = BIT(ctr_index); > > > > if ((csrno >= CSR_CYCLE && csrno <= CSR_INSTRET) || > > (csrno >= CSR_CYCLEH && csrno <= CSR_INSTRETH)) { > > goto skip_ext_pmu_check; > > } > > > > -if ((!cpu->cfg.pmu_num || !(cpu->pmu_avail_ctrs & BIT(ctr_index { > > +if ((!cpu->cfg.pmu_num || !(cpu->pmu_avail_ctrs & ctr_mask))) { > > /* No counter is enabled in PMU or the counter is out of range */ > > return RISCV_EXCP_ILLEGAL_INST; > > } > > > > skip_ext_pmu_check: > > > > -if (env->priv == PRV_S) { > > -switch (csrno) { > > -case CSR_CYCLE: > > -if (!get_field(env->mcounteren, COUNTEREN_CY)) { > > -return RISCV_EXCP_ILLEGAL_INST; > > -} > > -break; > > -case CSR_TIME: > > -if (!get_field(env->mcounteren, COUNTEREN_TM)) { > > -return RISCV_EXCP_ILLEGAL_INST; > > -} > > -break; > > -case CSR_INSTRET: > > -if (!get_field(env->mcounteren, COUNTEREN_IR)) { > > -return RISCV_EXCP_ILLEGAL_INST; > > -} > > -break; > > -case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31: > > -if (!get_field(env->mcounteren, 1 << ctr_index)) { > > -return RISCV_EXCP_ILLEGAL_INST; > > -} > > -break; > > -} > > -if (rv32) { > > -switch (csrno) { > > -case CSR_CYCLEH: > > -if (!get_field(env->mcounteren, COUNTEREN_CY)) { > > -return RISCV_EXCP_ILLEGAL_INST; > > -} > > -break; > > -case CSR_TIMEH: > > -if (!get_field(env->mcounteren, COUNTEREN_TM)) { > > -return RISCV_EXCP_ILLEGAL_INST; > > -} > > -break; > > -case CSR_INSTRETH: > > -if (!get_field(env->mcounteren, COUNTEREN_IR)) { > > -return RISCV_EXCP_ILLEGAL_INST; > > -} > > -break; > > -case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H: > > -if (!get_field(env->mcounteren, 1 << ctr_index)) { > > -return RISCV_EXCP_ILLEGAL_INST; > > -} > > -break; > > -} > > -} > > +if ((env->priv == PRV_S) && (!get_field(env->mcounteren, ctr_mask))) { > > +return RISCV_EXCP_ILLEGAL_INST; > > } > > > > if (riscv_cpu_virt_enabled(env)) { > > -switch (csrno) { > > -case CSR_CYCLE: > > -if (!get_field(env->hcounteren, COUNTEREN_CY) && > > -get_field(env->mcounteren, COUNTEREN_CY)) { > > -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > > -} > > -break; > > -case CSR_TIME: > > -if (!get_field(env->hcounteren, COUNTEREN_TM) && > > -get_field(env->mcounteren, COUNTEREN_TM)) { > > -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > > -} > > -break; > > -case CSR_INSTRET: > > -if (!get_field(env->hcounteren, COUNTEREN_IR) && > > -get_field(env->mcounteren, COUNTEREN_IR)) { > > -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > > -} > > -break; > > -case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31: > > -if (!get_field(env->hcounteren, 1 << ctr_index) && > > - get_field(env->mcounteren, 1 << ctr_index)) { > > -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > > -} > > -
Re: [PATCH v4 09/11] target/riscv: Simplify counter predicate function
On Fri, Jan 7, 2022 at 10:22 AM Atish Patra wrote: > > All the hpmcounters and the fixed counters (CY, IR, TM) can be represented > as a unified counter. Thus, the predicate function doesn't need handle each > case separately. > > Simplify the predicate function so that we just handle things differently > between RV32/RV64 and S/HS mode. > > Signed-off-by: Atish Patra > --- > target/riscv/csr.c | 111 - > 1 file changed, 10 insertions(+), 101 deletions(-) > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > index d3a8bba6a518..feb053eb3f7b 100644 > --- a/target/riscv/csr.c > +++ b/target/riscv/csr.c > @@ -109,6 +109,7 @@ static RISCVException ctr(CPURISCVState *env, int csrno) > CPUState *cs = env_cpu(env); > RISCVCPU *cpu = RISCV_CPU(cs); > int ctr_index; > +uint64_t ctr_mask; Use target_ulong should be enough? > int base_csrno = CSR_CYCLE; > bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false; > > @@ -117,122 +118,30 @@ static RISCVException ctr(CPURISCVState *env, int > csrno) > base_csrno += 0x80; > } > ctr_index = csrno - base_csrno; > +ctr_mask = BIT(ctr_index); > > if ((csrno >= CSR_CYCLE && csrno <= CSR_INSTRET) || > (csrno >= CSR_CYCLEH && csrno <= CSR_INSTRETH)) { > goto skip_ext_pmu_check; > } > > -if ((!cpu->cfg.pmu_num || !(cpu->pmu_avail_ctrs & BIT(ctr_index { > +if ((!cpu->cfg.pmu_num || !(cpu->pmu_avail_ctrs & ctr_mask))) { > /* No counter is enabled in PMU or the counter is out of range */ > return RISCV_EXCP_ILLEGAL_INST; > } > > skip_ext_pmu_check: > > -if (env->priv == PRV_S) { > -switch (csrno) { > -case CSR_CYCLE: > -if (!get_field(env->mcounteren, COUNTEREN_CY)) { > -return RISCV_EXCP_ILLEGAL_INST; > -} > -break; > -case CSR_TIME: > -if (!get_field(env->mcounteren, COUNTEREN_TM)) { > -return RISCV_EXCP_ILLEGAL_INST; > -} > -break; > -case CSR_INSTRET: > -if (!get_field(env->mcounteren, COUNTEREN_IR)) { > -return RISCV_EXCP_ILLEGAL_INST; > -} > -break; > -case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31: > -if (!get_field(env->mcounteren, 1 << ctr_index)) { > -return RISCV_EXCP_ILLEGAL_INST; > -} > -break; > -} > -if (rv32) { > -switch (csrno) { > -case CSR_CYCLEH: > -if (!get_field(env->mcounteren, COUNTEREN_CY)) { > -return RISCV_EXCP_ILLEGAL_INST; > -} > -break; > -case CSR_TIMEH: > -if (!get_field(env->mcounteren, COUNTEREN_TM)) { > -return RISCV_EXCP_ILLEGAL_INST; > -} > -break; > -case CSR_INSTRETH: > -if (!get_field(env->mcounteren, COUNTEREN_IR)) { > -return RISCV_EXCP_ILLEGAL_INST; > -} > -break; > -case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H: > -if (!get_field(env->mcounteren, 1 << ctr_index)) { > -return RISCV_EXCP_ILLEGAL_INST; > -} > -break; > -} > -} > +if ((env->priv == PRV_S) && (!get_field(env->mcounteren, ctr_mask))) { > +return RISCV_EXCP_ILLEGAL_INST; > } > > if (riscv_cpu_virt_enabled(env)) { > -switch (csrno) { > -case CSR_CYCLE: > -if (!get_field(env->hcounteren, COUNTEREN_CY) && > -get_field(env->mcounteren, COUNTEREN_CY)) { > -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > -} > -break; > -case CSR_TIME: > -if (!get_field(env->hcounteren, COUNTEREN_TM) && > -get_field(env->mcounteren, COUNTEREN_TM)) { > -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > -} > -break; > -case CSR_INSTRET: > -if (!get_field(env->hcounteren, COUNTEREN_IR) && > -get_field(env->mcounteren, COUNTEREN_IR)) { > -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > -} > -break; > -case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31: > -if (!get_field(env->hcounteren, 1 << ctr_index) && > - get_field(env->mcounteren, 1 << ctr_index)) { > -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > -} > -break; > -} > -if (rv32) { > -switch (csrno) { > -case CSR_CYCLEH: > -if (!get_field(env->hcounteren, COUNTEREN_CY) && > -get_field(env->mcounteren, COUNTEREN_CY)) { > -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > -} > -
[PATCH v4 09/11] target/riscv: Simplify counter predicate function
All the hpmcounters and the fixed counters (CY, IR, TM) can be represented as a unified counter. Thus, the predicate function doesn't need handle each case separately. Simplify the predicate function so that we just handle things differently between RV32/RV64 and S/HS mode. Signed-off-by: Atish Patra --- target/riscv/csr.c | 111 - 1 file changed, 10 insertions(+), 101 deletions(-) diff --git a/target/riscv/csr.c b/target/riscv/csr.c index d3a8bba6a518..feb053eb3f7b 100644 --- a/target/riscv/csr.c +++ b/target/riscv/csr.c @@ -109,6 +109,7 @@ static RISCVException ctr(CPURISCVState *env, int csrno) CPUState *cs = env_cpu(env); RISCVCPU *cpu = RISCV_CPU(cs); int ctr_index; +uint64_t ctr_mask; int base_csrno = CSR_CYCLE; bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false; @@ -117,122 +118,30 @@ static RISCVException ctr(CPURISCVState *env, int csrno) base_csrno += 0x80; } ctr_index = csrno - base_csrno; +ctr_mask = BIT(ctr_index); if ((csrno >= CSR_CYCLE && csrno <= CSR_INSTRET) || (csrno >= CSR_CYCLEH && csrno <= CSR_INSTRETH)) { goto skip_ext_pmu_check; } -if ((!cpu->cfg.pmu_num || !(cpu->pmu_avail_ctrs & BIT(ctr_index { +if ((!cpu->cfg.pmu_num || !(cpu->pmu_avail_ctrs & ctr_mask))) { /* No counter is enabled in PMU or the counter is out of range */ return RISCV_EXCP_ILLEGAL_INST; } skip_ext_pmu_check: -if (env->priv == PRV_S) { -switch (csrno) { -case CSR_CYCLE: -if (!get_field(env->mcounteren, COUNTEREN_CY)) { -return RISCV_EXCP_ILLEGAL_INST; -} -break; -case CSR_TIME: -if (!get_field(env->mcounteren, COUNTEREN_TM)) { -return RISCV_EXCP_ILLEGAL_INST; -} -break; -case CSR_INSTRET: -if (!get_field(env->mcounteren, COUNTEREN_IR)) { -return RISCV_EXCP_ILLEGAL_INST; -} -break; -case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31: -if (!get_field(env->mcounteren, 1 << ctr_index)) { -return RISCV_EXCP_ILLEGAL_INST; -} -break; -} -if (rv32) { -switch (csrno) { -case CSR_CYCLEH: -if (!get_field(env->mcounteren, COUNTEREN_CY)) { -return RISCV_EXCP_ILLEGAL_INST; -} -break; -case CSR_TIMEH: -if (!get_field(env->mcounteren, COUNTEREN_TM)) { -return RISCV_EXCP_ILLEGAL_INST; -} -break; -case CSR_INSTRETH: -if (!get_field(env->mcounteren, COUNTEREN_IR)) { -return RISCV_EXCP_ILLEGAL_INST; -} -break; -case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H: -if (!get_field(env->mcounteren, 1 << ctr_index)) { -return RISCV_EXCP_ILLEGAL_INST; -} -break; -} -} +if ((env->priv == PRV_S) && (!get_field(env->mcounteren, ctr_mask))) { +return RISCV_EXCP_ILLEGAL_INST; } if (riscv_cpu_virt_enabled(env)) { -switch (csrno) { -case CSR_CYCLE: -if (!get_field(env->hcounteren, COUNTEREN_CY) && -get_field(env->mcounteren, COUNTEREN_CY)) { -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; -} -break; -case CSR_TIME: -if (!get_field(env->hcounteren, COUNTEREN_TM) && -get_field(env->mcounteren, COUNTEREN_TM)) { -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; -} -break; -case CSR_INSTRET: -if (!get_field(env->hcounteren, COUNTEREN_IR) && -get_field(env->mcounteren, COUNTEREN_IR)) { -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; -} -break; -case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31: -if (!get_field(env->hcounteren, 1 << ctr_index) && - get_field(env->mcounteren, 1 << ctr_index)) { -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; -} -break; -} -if (rv32) { -switch (csrno) { -case CSR_CYCLEH: -if (!get_field(env->hcounteren, COUNTEREN_CY) && -get_field(env->mcounteren, COUNTEREN_CY)) { -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; -} -break; -case CSR_TIMEH: -if (!get_field(env->hcounteren, COUNTEREN_TM) && -get_field(env->mcounteren, COUNTEREN_TM)) { -return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; -} -break; -case CSR_INSTRETH: -if