Re: [PATCH v2 2/6] target/riscv/tcg: add ext_zicntr disable support

[Daniel Henrique Barboza]

On 10/22/23 23:54, Alistair Francis wrote:
> On Wed, Oct 18, 2023 at 8:13 AM Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
> > Support for the zicntr counters are already in place. We need a way to
> > disable them if the user wants to. This is done by restricting access to
> > the CYCLE, TIME, and INSTRET counters via the 'ctr()' predicate when
> > we're about to access them.
> >
> > Disabling zicntr happens via the command line or if its dependency,
> > zicsr, happens to be disabled. We'll check for zicsr during realize() and,
> > in case it's absent, disable zicntr. However, if the user was explicit
> > about having zicntr support, error out instead of disabling it.
> >
> > Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>
> This should come before we expose the property to users though

I'll merge this patch with patch 1. Same thing with patch 4 and 5.


Thanks,

Daniel

> Alistair
>
> > ---
> >   target/riscv/csr.c         | 4 ++++
> >   target/riscv/tcg/tcg-cpu.c | 8 ++++++++
> >   2 files changed, 12 insertions(+)
> >
> > diff --git a/target/riscv/csr.c b/target/riscv/csr.c
> > index a5be1c202c..05c6a69123 100644
> > --- a/target/riscv/csr.c
> > +++ b/target/riscv/csr.c
> > @@ -122,6 +122,10 @@ static RISCVException ctr(CPURISCVState *env, int csrno)
> >
> >       if ((csrno >= CSR_CYCLE && csrno <= CSR_INSTRET) ||
> >           (csrno >= CSR_CYCLEH && csrno <= CSR_INSTRETH)) {
> > +        if (!riscv_cpu_cfg(env)->ext_zicntr) {
> > +            return RISCV_EXCP_ILLEGAL_INST;
> > +        }
> > +
> >           goto skip_ext_pmu_check;
> >       }
> >
> > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> > index bbce254ee1..a01b876621 100644
> > --- a/target/riscv/tcg/tcg-cpu.c
> > +++ b/target/riscv/tcg/tcg-cpu.c
> > @@ -541,6 +541,14 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, 
> > Error **errp)
> >           cpu_cfg_ext_auto_update(cpu, CPU_CFG_OFFSET(ext_zksh), true);
> >       }
> >
> > +    if (cpu->cfg.ext_zicntr && !cpu->cfg.ext_zicsr) {
> > +        if (cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zicntr))) {
> > +            error_setg(errp, "zicntr requires zicsr");
> > +            return;
> > +        }
> > +        cpu->cfg.ext_zicntr = false;
> > +    }
> > +
> >       /*
> >        * Disable isa extensions based on priv spec after we
> >        * validated and set everything we need.
> > --
> > 2.41.0