[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH RFC 0/8] Add Counter delegation ISA extension support
From: |
Daniel Henrique Barboza |
Subject: |
Re: [PATCH RFC 0/8] Add Counter delegation ISA extension support |
Date: |
Wed, 21 Feb 2024 15:26:33 -0300 |
User-agent: |
Mozilla Thunderbird |
On 2/21/24 14:06, Atish Kumar Patra wrote:
On Wed, Feb 21, 2024 at 6:58 AM Daniel Henrique Barboza <dbarboza@ventanamicro.com
<mailto:dbarboza@ventanamicro.com>> wrote:
Hi Atish,
This series and its dependency, which I assume it's
"[PATCH v4 0/5] Add ISA extension smcntrpmf support"
Doesn't apply in neither master nor riscv-to-apply.next because of this
patch:
"target/riscv: Use RISCVException as return type for all csr ops"
That changed some functions from 'int' to "RISCVException" type. The
conflicts
from the v4 series are rather trivial but the conflicts for this RFC are
annoying
to deal with. It would be better if you could re-send both series rebased
with
the latest changes.
I was waiting for Alistair's ACK on the smcntrpmf series as he had some
comments. It looks like he is okay
with the series now (no further questions). Let me respin both the series.
One more thing:
On 2/16/24 21:01, Atish Patra wrote:
> This series adds the counter delegation extension support. The counter
> delegation ISA extension(Smcdeleg/Ssccfg) actually depends on multiple
ISA
> extensions.
>
> 1. S[m|s]csrind : The indirect CSR extension[1] which defines additional
> 5 ([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size limitation
of
> RISC-V CSR address space.
> 2. Smstateen: The stateen bit[60] controls the access to the registers
> indirectly via the above indirect registers.
> 3. Smcdeleg/Ssccfg: The counter delegation extensions[2]
>
> The counter delegation extension allows Supervisor mode to program the
> hpmevent and hpmcounters directly without needing the assistance from the
> M-mode via SBI calls. This results in a faster perf profiling and very
> few traps. This extension also introduces a scountinhibit CSR which
allows
> to stop/start any counter directly from the S-mode. As the counter
> delegation extension potentially can have more than 100 CSRs, the
specificaiton
> leverages the indirect CSR extension to save the precious CSR address
range.
>
> Due to the dependancy of these extensions, the following extensions must
be
> enabled to use the counter delegation feature in S-mode.
>
>
"smstateen=true,sscofpmf=true,ssccfg=true,smcdeleg=true,smcsrind=true,sscsrind=true"
>
> This makes the qemu command line quite tedious. In stead of that, I
think we
> can enable these features by default if there is no objection.
It wasn't need so far but, if needed, we can add specialized setters for
extensions
that has multiple dependencies. Instead of the usual setter we would do
something
like:
cpu_set_ssccfg() {
if (enabled) {
smstateen=true
sscofpmf=true
smcdeleg=true
smcsrind=true
sscsrind=true
}
}
The advantage is that this setter would also work for CPUs that doesn't
inherit defaults,
like bare-cps and profile CPUs.
Your suggested approach looks good to me. But I was asking about concerns about
enabling these extensions
by default rather than the actual mechanism to implement it. Few of the
extensions listed here such as smstateen,smcsrind
sscsrind are independent ISA extensions which are used for other ISA extensions
as well.
It looks like you are okay with the use case also ?
I don't mind setting new defaults in rv64.
That doesn't mean we can't add defaults for rv64, but for this particular
case I wonder if
the 'max' CPU wouldn't be better.
Not sure what you mean here. What does 'max' cpu have to do with pmu extensions
?
Save a few exceptions, all the extensions declared in riscv_cpu_extensions[]
will be enabled in the 'max' CPU, regardless of their default value for the
rv64 CPU (see riscv_init_max_cpu_extensions() in tcg-cpu.c).
If we count both the v4 and this RFC, the following extensions were added in
riscv_cpu_extensions[]:
+ MULTI_EXT_CFG_BOOL("smcntrpmf", ext_smcntrpmf, false),
+ MULTI_EXT_CFG_BOOL("smcsrind", ext_smcsrind, false),
+ MULTI_EXT_CFG_BOOL("sscsrind", ext_sscsrind, false),
+ MULTI_EXT_CFG_BOOL("smcdeleg", ext_smcdeleg, false),
+ MULTI_EXT_CFG_BOOL("ssccfg", ext_ssccfg, false),
All of them will be enabled by default in the 'max' CPU.
This is what I was referring to. We can just use the 'max' CPU and don't worry
about
enabling defaults in rv64.
Thanks,
Daniel
Thanks,
Daniel
>
> The first 2 patches decouple the indirect CSR usage from AIA
implementation
> while patch3 adds stateen bits validation for AIA.
> The PATCH4 implements indirect CSR extensions while remaining patches
> implement the counter delegation extensions.
>
> The Qemu patches can be found here:
> https://github.com/atishp04/qemu/tree/counter_delegation_rfc
<https://github.com/atishp04/qemu/tree/counter_delegation_rfc>
>
> The opensbi patch can be found here:
> https://github.com/atishp04/opensbi/tree/counter_delegation_v1
<https://github.com/atishp04/opensbi/tree/counter_delegation_v1>
>
> The Linux kernel patches can be found here:
> https://github.com/atishp04/linux/tree/counter_delegation_rfc
<https://github.com/atishp04/linux/tree/counter_delegation_rfc>
>
> [1] https://github.com/riscv/riscv-indirect-csr-access
<https://github.com/riscv/riscv-indirect-csr-access>
> [2] https://github.com/riscv/riscv-smcdeleg-ssccfg
<https://github.com/riscv/riscv-smcdeleg-ssccfg>
>
> Atish Patra (1):
> target/riscv: Enable S*stateen bits for AIA
>
> Kaiwen Xue (7):
> target/riscv: Add properties for Indirect CSR Access extension
> target/riscv: Decouple AIA processing from xiselect and xireg
> target/riscv: Support generic CSR indirect access
> target/riscv: Add smcdeleg/ssccfg properties
> target/riscv: Add counter delegation definitions
> target/riscv: Add select value range check for counter delegation
> target/riscv: Add counter delegation/configuration support
>
> target/riscv/cpu.c | 8 +
> target/riscv/cpu.h | 1 +
> target/riscv/cpu_bits.h | 34 +-
> target/riscv/cpu_cfg.h | 4 +
> target/riscv/csr.c | 713 +++++++++++++++++++++++++++++++++++++---
> target/riscv/machine.c | 1 +
> 6 files changed, 722 insertions(+), 39 deletions(-)
>
> --
> 2.34.1
>
- [PATCH RFC 1/8] target/riscv: Add properties for Indirect CSR Access extension, (continued)
- [PATCH RFC 1/8] target/riscv: Add properties for Indirect CSR Access extension, Atish Patra, 2024/02/16
- [PATCH RFC 2/8] target/riscv: Decouple AIA processing from xiselect and xireg, Atish Patra, 2024/02/16
- [PATCH RFC 3/8] target/riscv: Enable S*stateen bits for AIA, Atish Patra, 2024/02/16
- [PATCH RFC 5/8] target/riscv: Add smcdeleg/ssccfg properties, Atish Patra, 2024/02/16
- [PATCH RFC 6/8] target/riscv: Add counter delegation definitions, Atish Patra, 2024/02/16
- [PATCH RFC 7/8] target/riscv: Add select value range check for counter delegation, Atish Patra, 2024/02/16
- [PATCH RFC 4/8] target/riscv: Support generic CSR indirect access, Atish Patra, 2024/02/16
- [PATCH RFC 8/8] target/riscv: Add counter delegation/configuration support, Atish Patra, 2024/02/16
- Re: [PATCH RFC 0/8] Add Counter delegation ISA extension support, Daniel Henrique Barboza, 2024/02/21