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

Daniel Henrique Barboza posted 4 patches 1 year, 1 month ago
Failed in applying to current master (apply log)
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(+)
[PATCH v3 0/4] riscv: zicntr/zihpm flags and disable support
Posted by Daniel Henrique Barboza 1 year, 1 month ago
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
Posted by Clément Chigot 1 year ago
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
Posted by Daniel Henrique Barboza 1 year ago

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

Re: [PATCH v3 0/4] riscv: zicntr/zihpm flags and disable support
Posted by Alistair Francis 1 year ago
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
>
>