On Mon, Jul 3, 2023 at 5:46 AM Alexandre Ghiti <alexghiti@xxxxxxxxxxxx> wrote: > > riscv used to allow direct access to cycle/time/instret counters, > bypassing the perf framework, this patchset intends to allow the user to > mmap any counter when accessed through perf. But we can't break the > existing behaviour so we introduce a sysctl perf_user_access like arm64 > does, which defaults to the legacy mode described above. > > This version needs openSBI v1.3 *and* a kernel fix that went upstream lately > (https://lore.kernel.org/lkml/20230616114831.3186980-1-maz@xxxxxxxxxx/T/). > > **Important**: In this version, the default mode is now user access, not > the legacy so some applications will break. > Maybe providing a little more context to the issue will help understanding why breaking those legacy applications is necessary due to security reasons. Here was the previous discussion around this[1]. Most of the legacy user applications using rdcycle should use rdtime instead as they just want to record the time elapsed. Allowing rdcycle/rdinstret to be read from user space can lead to very deterministic attacks[2]. [1] https://groups.google.com/a/groups.riscv.org/g/sw-dev/c/REWcwYnzsKE?pli=1 [2] https://www.youtube.com/watch?v=3-c4C_L2PRQ&ab_channel=IEEESymposiumonSecurityandPrivacy Any user application that really requires to read rdcycle directly can use this new perf interface to do that without any latency. > base-commit-tag: v6.4-rc6 > > Changes in v4: > - Fixed some nits in riscv_pmu_sbi.c thanks to Andrew > - Fixed the documentation thanks to Andrew > - Added RB from Andrew \o/ > > Changes in v3: > - patch 1 now contains the ref to the faulty commit (no Fixes tag as it is only a comment), as Andrew suggested > - Removed RISCV_PMU_LEGACY_TIME from patch 3, as Andrew suggested > - Rename RISCV_PMU_PDEV_NAME to "riscv-pmu-sbi", patch4 is just cosmetic now, as Andrew suggested > - Removed a few useless (and wrong) comments, as Andrew suggested > - Simplify arch_perf_update_userpage code, as Andrew suggested > - Documentation now mentions that time CSR is *always* accessible, whatever the mode, as suggested by Andrew > - Removed CYCLEH reference and add TODO for rv32 support, as suggested by Atish > - Do not rename the pmu instance as Atish suggested > - Set pmc_width only if rdpmc is enabled and CONFIG_RISCV_PMU is set and the event is a hw event > - Move arch_perf_update_userpage https://lore.kernel.org/lkml/20230616114831.3186980-1-maz@xxxxxxxxxx/T/ > - **Switch to user mode access by default** > > Changes in v2: > - Split into smaller patches, way better! > - Add RB from Conor > - Simplify the way we checked riscv architecture > - Fix race mmap and other thread running on other cpus > - Use hwc when available > - Set all userspace access flags in event_init, too cumbersome to handle sysctl changes > - Fix arch_perf_update_userpage for pmu other than riscv-pmu by renaming pmu driver > - Fixed kernel test robot build error > - Fixed documentation (Andrew and Bagas) > - perf testsuite passes mmap tests in all 3 modes > > Alexandre Ghiti (10): > perf: Fix wrong comment about default event_idx > include: riscv: Fix wrong include guard in riscv_pmu.h > riscv: Make legacy counter enum match the HW numbering > drivers: perf: Rename riscv pmu sbi driver > riscv: Prepare for user-space perf event mmap support > drivers: perf: Implement perf event mmap support in the legacy backend > drivers: perf: Implement perf event mmap support in the SBI backend > Documentation: admin-guide: Add riscv sysctl_perf_user_access > tools: lib: perf: Implement riscv mmap support > perf: tests: Adapt mmap-basic.c for riscv > > Documentation/admin-guide/sysctl/kernel.rst | 27 ++- > drivers/perf/riscv_pmu.c | 113 +++++++++++ > drivers/perf/riscv_pmu_legacy.c | 28 ++- > drivers/perf/riscv_pmu_sbi.c | 196 +++++++++++++++++++- > include/linux/perf/riscv_pmu.h | 12 +- > include/linux/perf_event.h | 3 +- > tools/lib/perf/mmap.c | 65 +++++++ > tools/perf/tests/mmap-basic.c | 4 +- > 8 files changed, 428 insertions(+), 20 deletions(-) > > -- > 2.39.2 > -- Regards, Atish
