Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:4 as permitted sender) client-ip=2620:137:e000::3:4;
MIME-Version: 1.0
References: <20231205024310.1593100-1-atishp@rivosinc.com> <20231207-affiliate-state-c4a20ea7e8de@wendy>
In-Reply-To: <20231207-affiliate-state-c4a20ea7e8de@wendy>
From:   Atish Kumar Patra <atishp@rivosinc.com>
Date:   Thu, 7 Dec 2023 14:28:22 -0800
Message-ID: <CAHBxVyGE48dprUk+QnQ0WDijy0fi3fu5abDGjBKjuZhXC8y1vw@mail.gmail.com>
Subject: Re: [RFC 0/9] RISC-V SBI v2.0 PMU improvements and Perf sampling in
 KVM guest
To:     Conor Dooley <conor.dooley@microchip.com>
Cc:     linux-kernel@vger.kernel.org,
        Alexandre Ghiti <alexghiti@rivosinc.com>,
        Andrew Jones <ajones@ventanamicro.com>,
        Anup Patel <anup@brainfault.org>,
        Atish Patra <atishp@atishpatra.org>,
        Guo Ren <guoren@kernel.org>, Icenowy Zheng <uwu@icenowy.me>,
        kvm-riscv@lists.infradead.org, kvm@vger.kernel.org,
        linux-riscv@lists.infradead.org,
        Mark Rutland <mark.rutland@arm.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Will Deacon <will@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Dec 7, 2023 at 4:03=E2=80=AFAM Conor Dooley <conor.dooley@microchip=
.com> wrote:
>
> Hey Atish,
>
> On Mon, Dec 04, 2023 at 06:43:01PM -0800, Atish Patra wrote:
> > This series implements SBI PMU improvements done in SBI v2.0[1] i.e. PM=
U snapshot
> > and fw_read_hi() functions.
>
> I don't see any commentary in this cover letter as to why the series is
> an RFC. v2.0 is a frozen spec per the Releases tab on GitHub, so that
> has ruled out the usual reason for spec related things being RFCs.
>
> What is it about the series that you are not yet willing to stand over?
>

Nothing. It's just my script where I tag any first version of a
feature series as RFC :).
I am planning to send the next one with a version tag this week as I
got some feedback.

Thanks for reviewing the patches :).

> Cheers,
> Conor.
>
> > SBI v2.0 introduced PMU snapshot feature which allows the SBI implement=
ation
> > to provide counter information (i.e. values/overlfow status) via a shar=
ed
> > memory between the SBI implementation and supervisor OS. This allows to=
 minimize
> > the number of traps in when perf being used inside a kvm guest as it re=
lies on
> > SBI PMU + trap/emulation of the counters.
> >
> > The current set of ratified RISC-V specification also doesn't allow sco=
untovf
> > to be trap/emulated by the hypervisor. The SBI PMU snapshot bridges the=
 gap
> > in ISA as well and enables perf sampling in the guest. However, LCOFI i=
n the
> > guest only works via IRQ filtering in AIA specification. That's why, AI=
A
> > has to be enabled in the hardware (at least the Ssaia extension) in ord=
er to
> > use the sampling support in the perf.
> >
> > Here are the patch wise implementation details.
> >
> > PATCH 1-2 : Generic cleanups/improvements.
> > PATCH 3,4,9 : FW_READ_HI function implementation
> > PATCH 5-6: Add PMU snapshot feature in sbi pmu driver
> > PATCH 7-8: KVM implementation for snapshot and sampling in kvm guests
> >
> > The series is based on v6.70-rc3 and is available at:
> >
> > https://github.com/atishp04/linux/tree/kvm_pmu_snapshot_v1
> >
> > The kvmtool patch is also available at:
> > https://github.com/atishp04/kvmtool/tree/sscofpmf
> >
> > It also requires Ssaia ISA extension to be present in the hardware in o=
rder to
> > get perf sampling support in the guest. In Qemu virt machine, it can be=
 done
> > by the following config.
> >
> > ```
> > -cpu rv64,sscofpmf=3Dtrue,x-ssaia=3Dtrue
> > ```
> >
> > There is no other dependancies on AIA apart from that. Thus, Ssaia must=
 be disabled
> > for the guest if AIA patches are not available. Here is the example com=
mand.
> >
> > ```
> > ./lkvm-static run -m 256 -c2 --console serial -p "console=3DttyS0 early=
con" --disable-ssaia -k ./Image --debug
> > ```
> >
> > The series has been tested only in Qemu.
> > Here is the snippet of the perf running inside a kvm guest.
> >
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
> > # perf record -e cycles -e instructions perf bench sched messaging -g 5
> > ...
> > # Running 'sched/messaging' benchmark:
> > ...
> > [   45.928723] perf_duration_warn: 2 callbacks suppressed
> > [   45.929000] perf: interrupt took too long (484426 > 483186), lowerin=
g kernel.perf_event_max_sample_rate to 250
> > # 20 sender and receiver processes per group
> > # 5 groups =3D=3D 200 processes run
> >
> >      Total time: 14.220 [sec]
> > [ perf record: Woken up 1 times to write data ]
> > [ perf record: Captured and wrote 0.117 MB perf.data (1942 samples) ]
> > # perf report --stdio
> > # To display the perf.data header info, please use --header/--header-on=
ly optio>
> > #
> > #
> > # Total Lost Samples: 0
> > #
> > # Samples: 943  of event 'cycles'
> > # Event count (approx.): 5128976844
> > #
> > # Overhead  Command          Shared Object                Symbol       =
        >
> > # ........  ...............  ...........................  .............=
........>
> > #
> >      7.59%  sched-messaging  [kernel.kallsyms]            [k] memcpy
> >      5.48%  sched-messaging  [kernel.kallsyms]            [k] percpu_co=
unter_ad>
> >      5.24%  sched-messaging  [kernel.kallsyms]            [k] __sbi_rfe=
nce_v02_>
> >      4.00%  sched-messaging  [kernel.kallsyms]            [k] _raw_spin=
_unlock_>
> >      3.79%  sched-messaging  [kernel.kallsyms]            [k] set_pte_r=
ange
> >      3.72%  sched-messaging  [kernel.kallsyms]            [k] next_upto=
date_fol>
> >      3.46%  sched-messaging  [kernel.kallsyms]            [k] filemap_m=
ap_pages
> >      3.31%  sched-messaging  [kernel.kallsyms]            [k] handle_mm=
_fault
> >      3.20%  sched-messaging  [kernel.kallsyms]            [k] finish_ta=
sk_switc>
> >      3.16%  sched-messaging  [kernel.kallsyms]            [k] clear_pag=
e
> >      3.03%  sched-messaging  [kernel.kallsyms]            [k] mtree_ran=
ge_walk
> >      2.42%  sched-messaging  [kernel.kallsyms]            [k] flush_ica=
che_pte
> >
> > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D
> >
> > [1] https://github.com/riscv-non-isa/riscv-sbi-doc
> >
> > Atish Patra (9):
> > RISC-V: Fix the typo in Scountovf CSR name
> > drivers/perf: riscv: Add a flag to indicate SBI v2.0 support
> > RISC-V: Add FIRMWARE_READ_HI definition
> > drivers/perf: riscv: Read upper bits of a firmware counter
> > RISC-V: Add SBI PMU snapshot definitions
> > drivers/perf: riscv: Implement SBI PMU snapshot function
> > RISC-V: KVM: Implement SBI PMU Snapshot feature
> > RISC-V: KVM: Add perf sampling support for guests
> > RISC-V: KVM: Support 64 bit firmware counters on RV32
> >
> > arch/riscv/include/asm/csr.h          |   5 +-
> > arch/riscv/include/asm/errata_list.h  |   2 +-
> > arch/riscv/include/asm/kvm_vcpu_pmu.h |  16 +-
> > arch/riscv/include/asm/sbi.h          |  11 ++
> > arch/riscv/include/uapi/asm/kvm.h     |   1 +
> > arch/riscv/kvm/main.c                 |   1 +
> > arch/riscv/kvm/vcpu.c                 |   8 +-
> > arch/riscv/kvm/vcpu_onereg.c          |   1 +
> > arch/riscv/kvm/vcpu_pmu.c             | 232 ++++++++++++++++++++++++--
> > arch/riscv/kvm/vcpu_sbi_pmu.c         |  10 ++
> > drivers/perf/riscv_pmu.c              |   1 +
> > drivers/perf/riscv_pmu_sbi.c          | 219 ++++++++++++++++++++++--
> > include/linux/perf/riscv_pmu.h        |   6 +
> > 13 files changed, 478 insertions(+), 35 deletions(-)
> >
> > --
> > 2.34.1
> >