linux-kernel - Re: [PATCH v5 4/4] perf parse-events: Reapply "Prefer sysfs/JSON hardware events over legacy"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z4V99KbadR7ib4FA@google.com>
Date: Mon, 13 Jan 2025 12:56:20 -0800
From: Namhyung Kim <namhyung@...nel.org>
To: Atish Kumar Patra <atishp@...osinc.com>
Cc: Ian Rogers <irogers@...gle.com>, 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>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Kan Liang <kan.liang@...ux.intel.com>,
	James Clark <james.clark@...aro.org>, Ze Gao <zegao2021@...il.com>,
	Weilin Wang <weilin.wang@...el.com>,
	Dominique Martinet <asmadeus@...ewreck.org>,
	Jean-Philippe Romain <jean-philippe.romain@...s.st.com>,
	Junhao He <hejunhao3@...wei.com>, linux-perf-users@...r.kernel.org,
	linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
	Aditya Bodkhe <Aditya.Bodkhe1@....com>, Leo Yan <leo.yan@....com>,
	Beeman Strong <beeman@...osinc.com>,
	Arnaldo Carvalho de Melo <acme@...hat.com>
Subject: Re: [PATCH v5 4/4] perf parse-events: Reapply "Prefer sysfs/JSON
 hardware events over legacy"

Hello,

On Fri, Jan 10, 2025 at 11:52:47AM -0800, Atish Kumar Patra wrote:
> On Fri, Jan 10, 2025 at 11:40 AM Namhyung Kim <namhyung@...nel.org> wrote:
> >
> > On Thu, Jan 09, 2025 at 02:21:09PM -0800, Ian Rogers wrote:
> > > Originally posted and merged from:
> > > https://lore.kernel.org/r/20240416061533.921723-10-irogers@google.com
> > > This reverts commit 4f1b067359ac8364cdb7f9fda41085fa85789d0f although
> > > the patch is now smaller due to related fixes being applied in commit
> > > 22a4db3c3603 ("perf evsel: Add alternate_hw_config and use in
> > > evsel__match").
> > > The original commit message was:
> > >
> > > It was requested that RISC-V be able to add events to the perf tool so
> > > the PMU driver didn't need to map legacy events to config encodings:
> > > https://lore.kernel.org/lkml/20240217005738.3744121-1-atishp@rivosinc.com/
> > >
> > > This change makes the priority of events specified without a PMU the
> > > same as those specified with a PMU, namely sysfs and JSON events are
> > > checked first before using the legacy encoding.
> >
> > I'm still not convinced why we need this change despite of these
> > troubles.  If it's because RISC-V cannot define the lagacy hardware
> > events in the kernel driver, why not using a different name in JSON and
> 
> When the discussion happened a year back. we tried to avoid defining
> the legacy hardware events in
> the kernel driver. However, we agreed that we have to define it
> anyways for other reasons (legacy usage + virtualization)
> as described here[1]. I have improved the driver in such a way that it
> can handle both legacy events from the
> driver or json file (via this patch) if available. If this patch is
> available, a platform vendor can choose to encode the legacy events in
> json.
> Otherwise, it has to specify them in the driver. I will try to send
> the series today/tomorrow.

Ok, thanks for the update.

> 
> This patch will help avoid proliferation of usage of legacy events in
> the long run. But it is no longer absolutely necessary for RISC-V.
> If this patch is accepted, there is a hope that we can get rid of the
> specifying encodings in the driver in the distant future. However, we
> have
> to define them in the driver for reasons described in[1].
> 
> [1] https://lore.kernel.org/lkml/20241026121758.143259-1-irogers@google.com/T/#m653a6b98919a365a361a698032502bd26af9f6ba
> > ask users to use the name specifically?  Something like:
> >
> >   $ perf record -e riscv-cycles ...
> >
> 
> That was the first alternative I proposed back in 2022 plumbers :).

I see, sorry for missing the earlier discussion.

Thanks,
Namhyung


> But it was concluded that we don't want users to learn new ways
> of running perf in RISC-V which makes sense to me as well.