On Wed, Jul 27, 2022 at 1:35 AM Weiwei Li <liwei...@iscas.ac.cn> wrote:
> > 在 2022/7/27 下午2:49, Atish Patra 写道: > > 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. > > > > Reviewed-by: Bin Meng <bmeng...@gmail.com> > > Acked-by: Alistair Francis <alistair.fran...@wdc.com> > > Signed-off-by: Atish Patra <ati...@rivosinc.com> > > --- > > target/riscv/csr.c | 112 +++++---------------------------------------- > > 1 file changed, 11 insertions(+), 101 deletions(-) > > > > diff --git a/target/riscv/csr.c b/target/riscv/csr.c > > index 1233bfa0a726..57dbbf9b09a0 100644 > > --- a/target/riscv/csr.c > > +++ b/target/riscv/csr.c > > @@ -74,6 +74,7 @@ static RISCVException ctr(CPURISCVState *env, int > csrno) > > CPUState *cs = env_cpu(env); > > RISCVCPU *cpu = RISCV_CPU(cs); > > int ctr_index; > > + target_ulong ctr_mask; > > int base_csrno = CSR_CYCLE; > > bool rv32 = riscv_cpu_mxl(env) == MXL_RV32 ? true : false; > > > > @@ -82,122 +83,31 @@ 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->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))) || > > + ((env->priv == PRV_U) && (!get_field(env->scounteren, > 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 (!get_field(env->hcounteren, COUNTEREN_IR) && > > - get_field(env->mcounteren, COUNTEREN_IR)) { > > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > > - } > > - break; > > - case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H: > > - if (!get_field(env->hcounteren, 1 << ctr_index) && > > - get_field(env->mcounteren, 1 << ctr_index)) { > > - return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > > - } > > - break; > > - } > > + if (!get_field(env->mcounteren, ctr_mask)) { > > + /* The bit must be set in mcountern for HS mode access */ > > + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > > + } else if (!get_field(env->hcounteren, ctr_mask)) { > > + return RISCV_EXCP_VIRT_INSTRUCTION_FAULT; > > } > > The logic is changed here. In original logic, > RISCV_EXCP_VIRT_INSTRUCTION_FAULT is triggered when > > !get_field(env->hcounteren, 1 << ctr_index) && get_field(env->mcounteren, > 1 << ctr_index) > > The new logic is RISCV_EXCP_VIRT_INSTRUCTION_FAULT is triggered when > !get_field(env->mcounteren, ctr_mask) > > or !get_field(env->hcounteren, 1 << ctr_index) && > get_field(env->mcounteren, 1 << ctr_index) > > Yes. It's just an optimization where we can break early just by checking mcountern. Do you see any issue with it ? > Regards, > > Weiwei Li > > > } > > #endif > >