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@xxxxxxxxxx> 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@xxxxxxxxxx > > > 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@xxxxxxxxxxxx/ > > > > > > 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@xxxxxxxxxx/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.
