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