On Mon, Dec 23, 2024 at 7:09 AM James Clark <james.clark@xxxxxxxxxx> wrote: > > > > On 21/12/2024 7:26 pm, Ian Rogers wrote: > > At the RISC-V summit the topic of avoiding event data being in the > > RISC-V PMU kernel driver came up. There is a preference for sysfs/JSON > > events being the priority when no PMU is provided so that legacy > > events maybe supported via json. Originally Mark Rutland also > > expressed at LPC 2023 that doing this would resolve bugs on ARM Apple > > M? processors, but James Clark more recently tested this and believes > > the driver issues there may not have existed or have been resolved. In > > any case, it is inconsistent that with a PMU event names avoid legacy > > encodings, but when wildcarding PMUs (ie without a PMU with the event > > name) the legacy encodings have priority. > > > > The patch doing this work was reverted in a v6.10 release candidate > > as, even though the patch was posted for weeks and had been on > > linux-next for weeks without issue, Linus was in the habit of using > > explicit legacy events with unsupported precision options on his > > Neoverse-N1. This machine has SLC PMU events for bus and CPU cycles > > where ARM decided to call the events bus_cycles and cycles, the latter > > being also a legacy event name. ARM haven't renamed the cycles event > > to a more consistent cpu_cycles and avoided the problem. With these > > changes the problematic event will now be skipped, a large warning > > produced, and perf record will continue for the other PMU events. This > > solution was proposed by Arnaldo. > > > > Two minor changes have been added to help with the error message and > > to work around issues occurring with "perf stat metrics (shadow stat) > > test". > > > > The patches have only been tested on my x86 non-hybrid laptop. > > > > v3: Make no events opening for perf record a failure as suggested by > > James Clark and Aditya Bodkhe <Aditya.Bodkhe1@xxxxxxx>. Also, > > rebase. > > Looks like this could be interacting with the dummy event for itrace > events which I must have missed before. Now it "fails" but the exit code > is 0 which some of the tests rely on. I noticed "Miscellaneous Intel PT > testing" is failing because its skip mechanism is broken: > > $ sudo perf record -e intel_pt/aux-action=start-paused/u > Error: > Failure to open event 'intel_pt/aux-action=start-paused/u' on PMU > 'intel_pt' which will be removed. > > $ echo $? > 0 > > So the test thinks it has the aux-action feature but it doesn't. Thanks James, there is also discussion of this here: https://lore.kernel.org/lkml/CAP-5=fX8hWF-PaAE2VKHW3fk1W19xd0hyBVsP3653J9xw-U7VQ@xxxxxxxxxxxxxx/ I think in v3 I need to switch from detecting adding dummy events, to detecting parsing of dummy events. Then if there is nothing but dummy events and none were parsed we exit. Thanks, Ian > > v2: Rebase and add tested-by tags from James Clark, Leo Yan and Atish > > Patra who have tested on RISC-V and ARM CPUs, including the > > problem case from before. > > > > Ian Rogers (4): > > perf evsel: Add pmu_name helper > > perf stat: Fix find_stat for mixed legacy/non-legacy events > > perf record: Skip don't fail for events that don't open > > perf parse-events: Reapply "Prefer sysfs/JSON hardware events over > > legacy" > > > > tools/perf/builtin-record.c | 34 ++++++++++++--- > > tools/perf/util/evsel.c | 10 +++++ > > tools/perf/util/evsel.h | 1 + > > tools/perf/util/parse-events.c | 26 +++++++++--- > > tools/perf/util/parse-events.l | 76 +++++++++++++++++----------------- > > tools/perf/util/parse-events.y | 60 ++++++++++++++++++--------- > > tools/perf/util/pmus.c | 20 +++++++-- > > tools/perf/util/stat-shadow.c | 3 +- > > 8 files changed, 156 insertions(+), 74 deletions(-) > > >
