On Fri, Sep 10, 2021 at 4:27 AM Atish Patra <atish.pa...@wdc.com> wrote:
>
> Currently, the predicate function for PMU related CSRs only works if
> virtualization is enabled. Ideally, they should check the mcountern
> bits before cycle/minstret/hpmcounterx access. The predicate function
> also calculates the counter index incorrectly for hpmcounterx.
>
> Signed-off-by: Atish Patra <atish.pa...@wdc.com>
> ---
> target/riscv/csr.c | 62 +++++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 58 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> index 9a4ed18ac597..0515d851b948 100644
> --- a/target/riscv/csr.c
> +++ b/target/riscv/csr.c
> @@ -62,12 +62,64 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
> #if !defined(CONFIG_USER_ONLY)
> CPUState *cs = env_cpu(env);
> RISCVCPU *cpu = RISCV_CPU(cs);
> + int ctr_index;
>
> if (!cpu->cfg.ext_counters) {
> /* The Counters extensions is not enabled */
> return RISCV_EXCP_ILLEGAL_INST;
> }
>
> + if (env->priv == PRV_S) {
> + switch (csrno) {
> + case CSR_CYCLE:
> + if (!get_field(env->mcounteren, HCOUNTEREN_CY)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + break;
> + case CSR_TIME:
> + if (!get_field(env->mcounteren, HCOUNTEREN_TM)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + break;
> + case CSR_INSTRET:
> + if (!get_field(env->mcounteren, HCOUNTEREN_IR)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + break;
> + case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31:
> + ctr_index = csrno - CSR_HPMCOUNTER3 + 3;
> + if (!get_field(env->mcounteren, 1 << ctr_index)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + break;
> + }
> + if (riscv_cpu_is_32bit(env)) {
> + switch (csrno) {
> + case CSR_CYCLEH:
> + if (!get_field(env->mcounteren, HCOUNTEREN_CY)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + break;
> + case CSR_TIMEH:
> + if (!get_field(env->mcounteren, HCOUNTEREN_TM)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + break;
> + case CSR_INSTRETH:
> + if (!get_field(env->mcounteren, HCOUNTEREN_IR)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + break;
> + case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H:
> + ctr_index = csrno - CSR_HPMCOUNTER3H + 3;
> + if (!get_field(env->mcounteren, 1 << ctr_index)) {
> + return RISCV_EXCP_ILLEGAL_INST;
> + }
> + break;
> + }
> + }
> + }
> +
> if (riscv_cpu_virt_enabled(env)) {
> switch (csrno) {
> case CSR_CYCLE:
> @@ -89,8 +141,9 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
> }
> break;
> case CSR_HPMCOUNTER3...CSR_HPMCOUNTER31:
> - if (!get_field(env->hcounteren, 1 << (csrno - CSR_HPMCOUNTER3))
> &&
> - get_field(env->mcounteren, 1 << (csrno - CSR_HPMCOUNTER3))) {
> + ctr_index = csrno - CSR_HPMCOUNTER3 + 3;
ctr_index = csrno - CSR_CYCLE;
> + if (!get_field(env->hcounteren, 1 << ctr_index) &&
> + get_field(env->mcounteren, 1 << ctr_index)) {
This fix (along with the H part below) should be put in a separate patch.
> return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> }
> break;
> @@ -116,8 +169,9 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
> }
> break;
> case CSR_HPMCOUNTER3H...CSR_HPMCOUNTER31H:
> - if (!get_field(env->hcounteren, 1 << (csrno -
> CSR_HPMCOUNTER3H)) &&
> - get_field(env->mcounteren, 1 << (csrno -
> CSR_HPMCOUNTER3H))) {
> + ctr_index = csrno - CSR_HPMCOUNTER3H + 3;
ctr_index = csrno - CSR_CYCLEH;
> + if (!get_field(env->hcounteren, 1 << ctr_index) &&
> + get_field(env->mcounteren, 1 << ctr_index)) {
> return RISCV_EXCP_VIRT_INSTRUCTION_FAULT;
> }
> break;
> --
You may need to rebase the patch on:
http://patchwork.ozlabs.org/project/qemu-devel/patch/20210915084601.24304-1-bmeng...@gmail.com/
Regards,
Bin
