Date:   Wed, 19 Apr 2023 02:00:18 +0530
From:   Atish Patra <atishp@...shpatra.org>
To:     Ian Rogers <irogers@...gle.com>
Cc:     David Laight <David.Laight@...lab.com>,
        Alexandre Ghiti <alexghiti@...osinc.com>,
        Jonathan Corbet <corbet@....net>,
        Peter Zijlstra <peterz@...radead.org>,
        Ingo Molnar <mingo@...hat.com>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        Jiri Olsa <jolsa@...nel.org>,
        Namhyung Kim <namhyung@...nel.org>,
        Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        Anup Patel <anup@...infault.org>,
        Will Deacon <will@...nel.org>, Rob Herring <robh@...nel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-perf-users@...r.kernel.org" <linux-perf-users@...r.kernel.org>,
        "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        paranlee <p4ranlee@...il.com>
Subject: Re: [PATCH 0/4] riscv: Allow userspace to directly access perf counters

On Tue, Apr 18, 2023 at 11:46 PM Ian Rogers <irogers@...gle.com> wrote:
>
> On Tue, Apr 18, 2023 at 9:43 AM Atish Patra <atishp@...shpatra.org> wrote:
> >
> > On Fri, Apr 14, 2023 at 2:40 AM David Laight <David.Laight@...lab.com> wrote:
> > >
> > > From: Atish Patra
> > > > Sent: 13 April 2023 20:18
> > > >
> > > > On Thu, Apr 13, 2023 at 9:47 PM Alexandre Ghiti <alexghiti@...osinc.com> 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.
> > > > >
> > > >
> > > > It would be good provide additional direction for user space packages:
> > > >
> > > > The legacy behavior is supported for now in order to avoid breaking
> > > > existing software.
> > > > However, reading counters directly without perf interaction may
> > > > provide incorrect values which
> > > > the userspace software must avoid. We are hoping that the user space
> > > > packages which
> > > > read the cycle/instret directly, will move to the proper interface
> > > > eventually if they actually need it.
> > > > Most of the users are supposed to read "time" instead of "cycle" if
> > > > they intend to read timestamps.
> > >
> > > If you are trying to measure the performance of short code
> > > fragments then you need pretty much raw access directly to
> > > the cycle/clock count register.
> > >
> > > I've done this on x86 to compare the actual cycle times
> > > of different implementations of the IP checksum loop
> > > (and compare them to the theoretical limit).
> > > The perf framework just added far too much latency,
> > > only directly reading the cpu registers gave anything
> > > like reliable (and consistent) answers.
> > >
> >
> > This series allows direct access to the counters once configured
> > through the perf.
> > Earlier the cycle/instret counters are directly exposed to the
> > userspace without kernel/perf frameworking knowing
> > when/which user space application is reading it. That has security implications.
> >
> > With this series applied, the user space application just needs to
> > configure the event (cycle/instret) through perf syscall.
> > Once configured, the userspace application can find out the counter
> > information from the mmap & directly
> > read the counter. There is no latency while reading the counters.
> >
> > This mechanism allows stop/clear the counters when the requesting task
> > is not running. It also takes care of context switching
> > which may result in invalid values as you mentioned below. This is
> > nothing new and all other arch (x86, ARM64) allow user space
> > counter read through the same mechanism.
> >
> > Here is the relevant upstream discussion:
> > https://lore.kernel.org/lkml/Y7wLa7I2hlz3rKw%2F@hirez.programming.kicks-ass.net/T/
> >
> > ARM64:
> > https://docs.kernel.org/arm64/perf.html?highlight=perf_user_access#perf-userspace-pmu-hardware-counter-access
> >
> > example usage in x86:
> > https://github.com/andikleen/pmu-tools/blob/master/jevents/rdpmc.c
>
> The canonical implementation of this should be:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/lib/perf/mmap.c#n400

Thanks for sharing the libperf implementation.

> which is updated in these patches but the tests are not:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/perf/tests/mmap-basic.c#n287
> Which appears to be an oversight. The tests display some differences

Yes. It's an oversight. We should make sure that perf mmap tests pass
for RISC-V as well.


> between x86 and aarch64 that have assumed userspace hardware counter
> access, and everything else that it is assumed don't.
>
> Thanks,
> Ian
>
> > > Clearly process switches (especially cpu migrations) cause
> > > problems, but they are obviously invalid values and can
> > > be ignored.
> > >
> > > So while a lot of uses may be 'happy' with the values the
> > > perf framework gives, sometimes you do need to directly
> > > read the relevant registers.
> > >
> > >         David
> > >
> > > -
> > > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
> > > Registration No: 1397386 (Wales)
> >
> >
> >
> > --
> > Regards,
> > Atish



--
Regards,
Atish

Powered by blists - more mailing lists