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

Daniel Henrique Barboza posted 6 patches 1 year, 1 month ago
There is a newer version of this series
[PATCH v2 2/6] target/riscv/tcg: add ext_zicntr disable support
Posted by Daniel Henrique Barboza 1 year, 1 month ago
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>
---
 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
Re: [PATCH v2 2/6] target/riscv/tcg: add ext_zicntr disable support
Posted by Alistair Francis 1 year, 1 month ago
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

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
>
>
Re: [PATCH v2 2/6] target/riscv/tcg: add ext_zicntr disable support
Posted by Daniel Henrique Barboza 1 year, 1 month ago

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
>>
>>