[PATCH v3 0/4] riscv: zicntr/zihpm flags and disable support

[PATCH v3 0/4] riscv: zicntr/zihpm flags and disable support

[Daniel Henrique Barboza]

Hi,

In this v3 the patches that added the extensions flags were squashed
with the patches that handled the disablement of the extensions in TCG,
as suggested by Alistair in v2.

No other change made. Patches based on Alistair's riscv-to-apply.next.

Patches missing acks: patch 3

Changes from v2:
- patch 2: squashed with patch 1
- patch 5: squashed with patch 4
- v2 link: https://lore.kernel.org/qemu-riscv/20231017221226.136764-1-dbarboza@ventanamicro.com/

Daniel Henrique Barboza (4):
  target/riscv: add zicntr extension flag for TCG
  target/riscv/kvm: add zicntr reg
  target/riscv: add zihpm extension flag for TCG
  target/riscv/kvm: add zihpm reg

 target/riscv/cpu.c         | 15 +++++++++++++++
 target/riscv/cpu_cfg.h     |  2 ++
 target/riscv/csr.c         |  4 ++++
 target/riscv/kvm/kvm-cpu.c |  2 ++
 target/riscv/tcg/tcg-cpu.c | 21 +++++++++++++++++++++
 5 files changed, 44 insertions(+)

-- 
2.41.0

[PATCH v3 1/4] target/riscv: add zicntr extension flag for TCG

[Daniel Henrique Barboza]

zicntr is the Base Counters and Timers extension described in chapter 12
of the unprivileged spec. It describes support for RDCYCLE, RDTIME and
RDINSTRET.

QEMU already implements it in TCG way before it was a discrete
extension.  zicntr is part of the RVA22 profile, so let's add it to QEMU
to make the future profile implementation flag complete. Given than it
represents an already existing feature, default it to 'true' for all
CPUs.

For TCG, we need a way to disable zicntr 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>
---
 target/riscv/cpu.c         | 12 ++++++++++++
 target/riscv/cpu_cfg.h     |  1 +
 target/riscv/csr.c         |  4 ++++
 target/riscv/tcg/tcg-cpu.c |  8 ++++++++
 4 files changed, 25 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index a2881bfa38..69d64ec4ca 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -80,6 +80,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
     ISA_EXT_DATA_ENTRY(zicbom, PRIV_VERSION_1_12_0, ext_zicbom),
     ISA_EXT_DATA_ENTRY(zicboz, PRIV_VERSION_1_12_0, ext_zicboz),
     ISA_EXT_DATA_ENTRY(zicond, PRIV_VERSION_1_12_0, ext_zicond),
+    ISA_EXT_DATA_ENTRY(zicntr, PRIV_VERSION_1_12_0, ext_zicntr),
     ISA_EXT_DATA_ENTRY(zicsr, PRIV_VERSION_1_10_0, ext_zicsr),
     ISA_EXT_DATA_ENTRY(zifencei, PRIV_VERSION_1_10_0, ext_zifencei),
     ISA_EXT_DATA_ENTRY(zihintntl, PRIV_VERSION_1_10_0, ext_zihintntl),
@@ -1208,6 +1209,15 @@ static void riscv_cpu_init(Object *obj)
     qdev_init_gpio_in(DEVICE(obj), riscv_cpu_set_irq,
                       IRQ_LOCAL_MAX + IRQ_LOCAL_GUEST_MAX);
 #endif /* CONFIG_USER_ONLY */
+
+    /*
+     * The timer and performance counters extensions were supported
+     * in QEMU before they were added as discrete extensions in the
+     * ISA. To keep compatibility we'll always default them to 'true'
+     * for all CPUs. Each accelerator will decide what to do when
+     * users disable them.
+     */
+    RISCV_CPU(obj)->cfg.ext_zicntr = true;
 }
 
 typedef struct misa_ext_info {
@@ -1297,6 +1307,8 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
     MULTI_EXT_CFG_BOOL("svnapot", ext_svnapot, false),
     MULTI_EXT_CFG_BOOL("svpbmt", ext_svpbmt, false),
 
+    MULTI_EXT_CFG_BOOL("zicntr", ext_zicntr, true),
+
     MULTI_EXT_CFG_BOOL("zba", ext_zba, true),
     MULTI_EXT_CFG_BOOL("zbb", ext_zbb, true),
     MULTI_EXT_CFG_BOOL("zbc", ext_zbc, true),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index e7ce977189..73fd4b3231 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -62,6 +62,7 @@ struct RISCVCPUConfig {
     bool ext_zksh;
     bool ext_zkt;
     bool ext_zifencei;
+    bool ext_zicntr;
     bool ext_zicsr;
     bool ext_zicbom;
     bool ext_zicboz;
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index f4e0a3962f..4ca96ddd1d 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 c5ff03efce..a1e4ed2e24 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

[PATCH v3 2/4] target/riscv/kvm: add zicntr reg

[Daniel Henrique Barboza]

Add zicntr support in the KVM driver now that QEMU supports it.

This reg was added in Linux 6.6.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/kvm/kvm-cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index cf98071f92..0a8be40786 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -228,6 +228,7 @@ static void kvm_riscv_update_cpu_misa_ext(RISCVCPU *cpu, CPUState *cs)
 static KVMCPUConfig kvm_multi_ext_cfgs[] = {
     KVM_EXT_CFG("zicbom", ext_zicbom, KVM_RISCV_ISA_EXT_ZICBOM),
     KVM_EXT_CFG("zicboz", ext_zicboz, KVM_RISCV_ISA_EXT_ZICBOZ),
+    KVM_EXT_CFG("zicntr", ext_zicntr, KVM_RISCV_ISA_EXT_ZICNTR),
     KVM_EXT_CFG("zihintpause", ext_zihintpause, KVM_RISCV_ISA_EXT_ZIHINTPAUSE),
     KVM_EXT_CFG("zbb", ext_zbb, KVM_RISCV_ISA_EXT_ZBB),
     KVM_EXT_CFG("ssaia", ext_ssaia, KVM_RISCV_ISA_EXT_SSAIA),
-- 
2.41.0

[PATCH v3 3/4] target/riscv: add zihpm extension flag for TCG

[Daniel Henrique Barboza]

zihpm is the Hardware Performance Counters extension described in
chapter 12 of the unprivileged spec. It describes support for 29
unprivileged performance counters, hpmcounter3-hpmcounter31.

As with zicntr, QEMU already implements zihpm before it was even an
extension. zihpm is also part of the RVA22 profile, so add it to QEMU
to complement the future profile implementation. Default it to 'true'
for all existing CPUs since it was always present in the code.

As for disabling it, there is already code in place in
target/riscv/csr.c in all predicates for these counters (ctr() and
mctr()) that disables them if cpu->cfg.pmu_num is zero. Thus, setting
cpu->cfg.pmu_num to zero if 'zihpm=false' is enough to disable the
extension.

Set cpu->pmu_avail_ctrs mask to zero as well since this is also checked
to verify if the counters exist.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
---
 target/riscv/cpu.c         |  3 +++
 target/riscv/cpu_cfg.h     |  1 +
 target/riscv/tcg/tcg-cpu.c | 13 +++++++++++++
 3 files changed, 17 insertions(+)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 69d64ec4ca..f40da4c661 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -85,6 +85,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
     ISA_EXT_DATA_ENTRY(zifencei, PRIV_VERSION_1_10_0, ext_zifencei),
     ISA_EXT_DATA_ENTRY(zihintntl, PRIV_VERSION_1_10_0, ext_zihintntl),
     ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause),
+    ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_zihpm),
     ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul),
     ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs),
     ISA_EXT_DATA_ENTRY(zfa, PRIV_VERSION_1_12_0, ext_zfa),
@@ -1218,6 +1219,7 @@ static void riscv_cpu_init(Object *obj)
      * users disable them.
      */
     RISCV_CPU(obj)->cfg.ext_zicntr = true;
+    RISCV_CPU(obj)->cfg.ext_zihpm = true;
 }
 
 typedef struct misa_ext_info {
@@ -1308,6 +1310,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
     MULTI_EXT_CFG_BOOL("svpbmt", ext_svpbmt, false),
 
     MULTI_EXT_CFG_BOOL("zicntr", ext_zicntr, true),
+    MULTI_EXT_CFG_BOOL("zihpm", ext_zihpm, true),
 
     MULTI_EXT_CFG_BOOL("zba", ext_zba, true),
     MULTI_EXT_CFG_BOOL("zbb", ext_zbb, true),
diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
index 73fd4b3231..6eef4a51ea 100644
--- a/target/riscv/cpu_cfg.h
+++ b/target/riscv/cpu_cfg.h
@@ -69,6 +69,7 @@ struct RISCVCPUConfig {
     bool ext_zicond;
     bool ext_zihintntl;
     bool ext_zihintpause;
+    bool ext_zihpm;
     bool ext_smstateen;
     bool ext_sstc;
     bool ext_svadu;
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index a1e4ed2e24..093bda2e75 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -549,6 +549,19 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
         cpu->cfg.ext_zicntr = false;
     }
 
+    if (cpu->cfg.ext_zihpm && !cpu->cfg.ext_zicsr) {
+        if (cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zihpm))) {
+            error_setg(errp, "zihpm requires zicsr");
+            return;
+        }
+        cpu->cfg.ext_zihpm = false;
+    }
+
+    if (!cpu->cfg.ext_zihpm) {
+        cpu->cfg.pmu_num = 0;
+        cpu->pmu_avail_ctrs = 0;
+    }
+
     /*
      * Disable isa extensions based on priv spec after we
      * validated and set everything we need.
-- 
2.41.0

[PATCH v3 4/4] target/riscv/kvm: add zihpm reg

[Daniel Henrique Barboza]

Add zihpm support in the KVM driver now that QEMU supports it.

This reg was added in Linux 6.6.

Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
 target/riscv/kvm/kvm-cpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c
index 0a8be40786..6d1c0a7915 100644
--- a/target/riscv/kvm/kvm-cpu.c
+++ b/target/riscv/kvm/kvm-cpu.c
@@ -230,6 +230,7 @@ static KVMCPUConfig kvm_multi_ext_cfgs[] = {
     KVM_EXT_CFG("zicboz", ext_zicboz, KVM_RISCV_ISA_EXT_ZICBOZ),
     KVM_EXT_CFG("zicntr", ext_zicntr, KVM_RISCV_ISA_EXT_ZICNTR),
     KVM_EXT_CFG("zihintpause", ext_zihintpause, KVM_RISCV_ISA_EXT_ZIHINTPAUSE),
+    KVM_EXT_CFG("zihpm", ext_zihpm, KVM_RISCV_ISA_EXT_ZIHPM),
     KVM_EXT_CFG("zbb", ext_zbb, KVM_RISCV_ISA_EXT_ZBB),
     KVM_EXT_CFG("ssaia", ext_ssaia, KVM_RISCV_ISA_EXT_SSAIA),
     KVM_EXT_CFG("sstc", ext_sstc, KVM_RISCV_ISA_EXT_SSTC),
-- 
2.41.0

Re: [PATCH v3 3/4] target/riscv: add zihpm extension flag for TCG

[Alistair Francis]

On Tue, Oct 24, 2023 at 1:40 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> zihpm is the Hardware Performance Counters extension described in
> chapter 12 of the unprivileged spec. It describes support for 29
> unprivileged performance counters, hpmcounter3-hpmcounter31.
>
> As with zicntr, QEMU already implements zihpm before it was even an
> extension. zihpm is also part of the RVA22 profile, so add it to QEMU
> to complement the future profile implementation. Default it to 'true'
> for all existing CPUs since it was always present in the code.
>
> As for disabling it, there is already code in place in
> target/riscv/csr.c in all predicates for these counters (ctr() and
> mctr()) that disables them if cpu->cfg.pmu_num is zero. Thus, setting
> cpu->cfg.pmu_num to zero if 'zihpm=false' is enough to disable the
> extension.
>
> Set cpu->pmu_avail_ctrs mask to zero as well since this is also checked
> to verify if the counters exist.
>
> Signed-off-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com>

Reviewed-by: Alistair Francis <alistair.francis@wdc.com>

Alistair

> ---
>  target/riscv/cpu.c         |  3 +++
>  target/riscv/cpu_cfg.h     |  1 +
>  target/riscv/tcg/tcg-cpu.c | 13 +++++++++++++
>  3 files changed, 17 insertions(+)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 69d64ec4ca..f40da4c661 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -85,6 +85,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
>      ISA_EXT_DATA_ENTRY(zifencei, PRIV_VERSION_1_10_0, ext_zifencei),
>      ISA_EXT_DATA_ENTRY(zihintntl, PRIV_VERSION_1_10_0, ext_zihintntl),
>      ISA_EXT_DATA_ENTRY(zihintpause, PRIV_VERSION_1_10_0, ext_zihintpause),
> +    ISA_EXT_DATA_ENTRY(zihpm, PRIV_VERSION_1_12_0, ext_zihpm),
>      ISA_EXT_DATA_ENTRY(zmmul, PRIV_VERSION_1_12_0, ext_zmmul),
>      ISA_EXT_DATA_ENTRY(zawrs, PRIV_VERSION_1_12_0, ext_zawrs),
>      ISA_EXT_DATA_ENTRY(zfa, PRIV_VERSION_1_12_0, ext_zfa),
> @@ -1218,6 +1219,7 @@ static void riscv_cpu_init(Object *obj)
>       * users disable them.
>       */
>      RISCV_CPU(obj)->cfg.ext_zicntr = true;
> +    RISCV_CPU(obj)->cfg.ext_zihpm = true;
>  }
>
>  typedef struct misa_ext_info {
> @@ -1308,6 +1310,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
>      MULTI_EXT_CFG_BOOL("svpbmt", ext_svpbmt, false),
>
>      MULTI_EXT_CFG_BOOL("zicntr", ext_zicntr, true),
> +    MULTI_EXT_CFG_BOOL("zihpm", ext_zihpm, true),
>
>      MULTI_EXT_CFG_BOOL("zba", ext_zba, true),
>      MULTI_EXT_CFG_BOOL("zbb", ext_zbb, true),
> diff --git a/target/riscv/cpu_cfg.h b/target/riscv/cpu_cfg.h
> index 73fd4b3231..6eef4a51ea 100644
> --- a/target/riscv/cpu_cfg.h
> +++ b/target/riscv/cpu_cfg.h
> @@ -69,6 +69,7 @@ struct RISCVCPUConfig {
>      bool ext_zicond;
>      bool ext_zihintntl;
>      bool ext_zihintpause;
> +    bool ext_zihpm;
>      bool ext_smstateen;
>      bool ext_sstc;
>      bool ext_svadu;
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index a1e4ed2e24..093bda2e75 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -549,6 +549,19 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
>          cpu->cfg.ext_zicntr = false;
>      }
>
> +    if (cpu->cfg.ext_zihpm && !cpu->cfg.ext_zicsr) {
> +        if (cpu_cfg_ext_is_user_set(CPU_CFG_OFFSET(ext_zihpm))) {
> +            error_setg(errp, "zihpm requires zicsr");
> +            return;
> +        }
> +        cpu->cfg.ext_zihpm = false;
> +    }
> +
> +    if (!cpu->cfg.ext_zihpm) {
> +        cpu->cfg.pmu_num = 0;
> +        cpu->pmu_avail_ctrs = 0;
> +    }
> +
>      /*
>       * Disable isa extensions based on priv spec after we
>       * validated and set everything we need.
> --
> 2.41.0
>
>

Re: [PATCH v3 0/4] riscv: zicntr/zihpm flags and disable support

[Alistair Francis]

On Tue, Oct 24, 2023 at 1:40 AM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Hi,
>
> In this v3 the patches that added the extensions flags were squashed
> with the patches that handled the disablement of the extensions in TCG,
> as suggested by Alistair in v2.
>
> No other change made. Patches based on Alistair's riscv-to-apply.next.
>
> Patches missing acks: patch 3
>
> Changes from v2:
> - patch 2: squashed with patch 1
> - patch 5: squashed with patch 4
> - v2 link: https://lore.kernel.org/qemu-riscv/20231017221226.136764-1-dbarboza@ventanamicro.com/
>
> Daniel Henrique Barboza (4):
>   target/riscv: add zicntr extension flag for TCG
>   target/riscv/kvm: add zicntr reg
>   target/riscv: add zihpm extension flag for TCG
>   target/riscv/kvm: add zihpm reg

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  target/riscv/cpu.c         | 15 +++++++++++++++
>  target/riscv/cpu_cfg.h     |  2 ++
>  target/riscv/csr.c         |  4 ++++
>  target/riscv/kvm/kvm-cpu.c |  2 ++
>  target/riscv/tcg/tcg-cpu.c | 21 +++++++++++++++++++++
>  5 files changed, 44 insertions(+)
>
> --
> 2.41.0
>
>

Re: [PATCH v3 0/4] riscv: zicntr/zihpm flags and disable support

[Clément Chigot]

Hi Daniel,

This series is triggering warnings when instantiating a CPU having a
spec version older than 1.12.
  | $ qemu-system-riscv32 -M sifive_e
  | qemu-system-riscv32: warning: disabling zicntr extension for hart
0x00000000 because privilege spec version does not match
  | qemu-system-riscv32: warning: disabling zihpm extension for hart
0x00000000 because privilege spec version does not match

And IIUC cpu-tcg.c:riscv_cpu_disable_priv_spec_isa_exts(), they will
end up being disabled as a result of these warnings.

I think these two extensions should be skipped in the above function.
Though we can also disable them on purpose in those old CPUs. WDYT ?

Thanks,
Clément

On Mon, Oct 23, 2023 at 5:40 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
> Hi,
>
> In this v3 the patches that added the extensions flags were squashed
> with the patches that handled the disablement of the extensions in TCG,
> as suggested by Alistair in v2.
>
> No other change made. Patches based on Alistair's riscv-to-apply.next.
>
> Patches missing acks: patch 3
>
> Changes from v2:
> - patch 2: squashed with patch 1
> - patch 5: squashed with patch 4
> - v2 link: https://lore.kernel.org/qemu-riscv/20231017221226.136764-1-dbarboza@ventanamicro.com/
>
> Daniel Henrique Barboza (4):
>   target/riscv: add zicntr extension flag for TCG
>   target/riscv/kvm: add zicntr reg
>   target/riscv: add zihpm extension flag for TCG
>   target/riscv/kvm: add zihpm reg
>
>  target/riscv/cpu.c         | 15 +++++++++++++++
>  target/riscv/cpu_cfg.h     |  2 ++
>  target/riscv/csr.c         |  4 ++++
>  target/riscv/kvm/kvm-cpu.c |  2 ++
>  target/riscv/tcg/tcg-cpu.c | 21 +++++++++++++++++++++
>  5 files changed, 44 insertions(+)
>
> --
> 2.41.0
>
>

Re: [PATCH v3 0/4] riscv: zicntr/zihpm flags and disable support

[Daniel Henrique Barboza]

On 11/13/23 13:08, Clément Chigot wrote:
> Hi Daniel,
> 
> This series is triggering warnings when instantiating a CPU having a
> spec version older than 1.12.
>    | $ qemu-system-riscv32 -M sifive_e
>    | qemu-system-riscv32: warning: disabling zicntr extension for hart
> 0x00000000 because privilege spec version does not match
>    | qemu-system-riscv32: warning: disabling zihpm extension for hart
> 0x00000000 because privilege spec version does not match

ooops ...

> 
> And IIUC cpu-tcg.c:riscv_cpu_disable_priv_spec_isa_exts(), they will
> end up being disabled as a result of these warnings.
> 
> I think these two extensions should be skipped in the above function.
> Though we can also disable them on purpose in those old CPUs. WDYT ?

I'm not sure if we should disable zicntr/zihpm in these old CPUs. They were
added in a time where weren't no discrete extensions for these counters,
so existing implementations might be relying on these timers to work
properly.

Your idea of skipping them in riscv_cpu_disable_priv_spec_isa_exts() looks
good to me because it handles the problem at the core: these timers are old
features that turned out to be extensions only later on, and our old CPUs were
already implementing them before that. We have some exceptions for zicntr/zihpm
in the code (e.g. they're the only always enable extension in the parent CPU),
might as well add another one.

CC me in this patch (I'm assuming you're sending it, let me know if you want
me to do it instead) and let's get this fixed during this freeze window.



Thanks,


Daniel


> 
> Thanks,
> Clément
> 
> On Mon, Oct 23, 2023 at 5:40 PM Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>>
>> Hi,
>>
>> In this v3 the patches that added the extensions flags were squashed
>> with the patches that handled the disablement of the extensions in TCG,
>> as suggested by Alistair in v2.
>>
>> No other change made. Patches based on Alistair's riscv-to-apply.next.
>>
>> Patches missing acks: patch 3
>>
>> Changes from v2:
>> - patch 2: squashed with patch 1
>> - patch 5: squashed with patch 4
>> - v2 link: https://lore.kernel.org/qemu-riscv/20231017221226.136764-1-dbarboza@ventanamicro.com/
>>
>> Daniel Henrique Barboza (4):
>>    target/riscv: add zicntr extension flag for TCG
>>    target/riscv/kvm: add zicntr reg
>>    target/riscv: add zihpm extension flag for TCG
>>    target/riscv/kvm: add zihpm reg
>>
>>   target/riscv/cpu.c         | 15 +++++++++++++++
>>   target/riscv/cpu_cfg.h     |  2 ++
>>   target/riscv/csr.c         |  4 ++++
>>   target/riscv/kvm/kvm-cpu.c |  2 ++
>>   target/riscv/tcg/tcg-cpu.c | 21 +++++++++++++++++++++
>>   5 files changed, 44 insertions(+)
>>
>> --
>> 2.41.0
>>
>>