Re: [PATCH v2 0/6] target/riscv: Add support for Control Transfer Records Ext.

[Alistair Francis]

On Thu, Jun 20, 2024 at 1:28 AM Rajnesh Kanwal <rkanwal@rivosinc.com> wrote:
>
> This series enables Control Transfer Records extension support on riscv
> platform. This extension is similar to Arch LBR in x86 and BRBE in ARM.
> The Extension has been stable and the latest release can be found here [0]

Can you be explicit about the exact version. RISC-V specs have a
tendency to change and the latest release today might not be the
latest tomorrow.

v1.0.0_rc2  at 
https://github.com/riscv/riscv-control-transfer-records/releases/tag/v1.0.0_rc2
for example would be really helpful

>
> CTR extension depends on couple of other extensions:
>
> 1. S[m|s]csrind : The indirect CSR extension [1] which defines additional
>    ([M|S|VS]IREG2-[M|S|VS]IREG6) register to address size limitation of
>    RISC-V CSR address space. CTR access ctrsource, ctrtartget and ctrdata
>    CSRs using sscsrind extension.
>
> 2. Smstateen: The mstateen bit[54] controls the access to the CTR ext to
>    S-mode.
>
> 3. Sscofpmf: Counter overflow and privilege mode filtering. [2]
>
> The series is based on Smcdeleg/Ssccfg counter delegation extension [3]
> patches. CTR itself doesn't depend on counter delegation support. This
> rebase is basically to include the Smcsrind patches.

Same comment here about specific versions

>
> Due to the dependency of these extensions, the following extensions must be
> enabled to use the control transfer records feature.
>
> "smstateen=true,sscofpmf=true,smcsrind=true,sscsrind=true,smctr=true,ssctr=true"

We shouldn't have this dependency if the actual spec doesn't have it.

Even if the spec does have the dependency we should the implication
rules to enable the required extensions if a user asks for CTR

Alistair

>
> Here is the link to a quick guide [5] to setup and run a basic perf demo on
> Linux to use CTR Ext.
>
> The Qemu patches can be found here:
> https://github.com/rajnesh-kanwal/qemu/tree/ctr_upstream_v2
>
> The opensbi patch can be found here:
> https://github.com/rajnesh-kanwal/opensbi/tree/ctr_upstream_v2
>
> The Linux kernel patches can be found here:
> https://github.com/rajnesh-kanwal/linux/tree/ctr_upstream_v2
>
> [0]: https://github.com/riscv/riscv-control-transfer-records/release
> [1]: https://github.com/riscv/riscv-indirect-csr-access
> [2]: https://github.com/riscvarchive/riscv-count-overflow/tree/main
> [3]: https://github.com/riscv/riscv-smcdeleg-ssccfg
> [4]: https://lore.kernel.org/all/20240217000134.3634191-1-atishp@rivosinc.com/
> [5]: 
> https://github.com/rajnesh-kanwal/linux/wiki/Running-CTR-basic-demo-on-QEMU-RISC%E2%80%90V-Virt-machine
>
> Changelog:
>
> v2: Lots of improvements based on Jason Chien's feedback including:
>   - Added CTR recording for cm.jalt, cm.jt, cm.popret, cm.popretz.
>   - Fixed and added more CTR extension enable checks.
>   - Fixed CTR CSR predicate functions.
>   - Fixed external trap xTE bit checks.
>   - One fix in freeze function for VS-mode.
>   - Lots of minor code improvements.
>   - Added checks in sctrclr instruction helper.
>
> v1:
>   - https://github.com/rajnesh-kanwal/qemu/tree/ctr_upstream
>
>
> Rajnesh Kanwal (6):
>   target/riscv: Remove obsolete sfence.vm instruction
>   target/riscv: Add Control Transfer Records CSR definitions.
>   target/riscv: Add support for Control Transfer Records extension CSRs.
>   target/riscv: Add support to record CTR entries.
>   target/riscv: Add CTR sctrclr instruction.
>   target/riscv: Add support to access ctrsource, ctrtarget, ctrdata
>     regs.
>
>  target/riscv/cpu.c                            |   4 +
>  target/riscv/cpu.h                            |  14 +
>  target/riscv/cpu_bits.h                       | 154 ++++++++++
>  target/riscv/cpu_cfg.h                        |   2 +
>  target/riscv/cpu_helper.c                     | 265 +++++++++++++++++
>  target/riscv/csr.c                            | 276 +++++++++++++++++-
>  target/riscv/helper.h                         |   9 +-
>  target/riscv/insn32.decode                    |   2 +-
>  .../riscv/insn_trans/trans_privileged.c.inc   |  21 +-
>  target/riscv/insn_trans/trans_rvi.c.inc       |  31 ++
>  target/riscv/insn_trans/trans_rvzce.c.inc     |  20 ++
>  target/riscv/op_helper.c                      | 159 +++++++++-
>  target/riscv/tcg/tcg-cpu.c                    |   6 +
>  target/riscv/translate.c                      |  10 +
>  14 files changed, 960 insertions(+), 13 deletions(-)
>
> --
> 2.34.1
>
>