hwpmc: Support an older application running the old ABI
Needs ReviewPublic
Actions

Authored by gallatin on May 27 2025, 4:40 PM.

Details

Reviewers

mhorne
jhb
markj
gnn
jrtc27

Summary

The AMD uProf profiler does not run on 15-current for a variety of reasons:

It gets back our PMC_VERSION from modv and quits with "The driver failed to start profiling. (error code 0x80070005)"
It fails to parse the produced pmc log files if they are marked with the newer PMC_VERSION. I've also worked around a change in the format of the procexec log (introduced in 94426d21bf62f) by collecting the entryaddr as well, and logging that rather than base/dyndaddr for older applications
It leaves garbage in cl.pm_flags, so I zero them.
It fails to select which set of counters its using PMC_F_EV_PMU, so I force the selection

This change is very specific to uprof builds, but ideally leaves room to grow. This change is a no-op by default; the user needs to specify kern.hwpmc.compat_verson=0x09040000

Test Plan

Collect a profile using:
AMDuProf_FreeBSD_x64_4.0.332/bin/AMDuProfCLI collect --config assess -g -a -d 10 -o /d/amd

Create a report using:
/AMDuProf_FreeBSD_x64_4.0.332/bin/AMDuProfCLI report -i /d/amd/AMD*

View the report and verify "10 HOTTEST FUNCTIONS " makes sense and is not empty.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

gallatin created this revision.May 27 2025, 4:40 PM

Herald added a reviewer: gnn. · View Herald TranscriptMay 27 2025, 4:40 PM

Herald added a subscriber: imp. · View Herald Transcript

gallatin requested review of this revision.May 27 2025, 4:40 PM

Should we put the sysctl in the manual?

In D50546#1154136, @ziaee wrote:

Should we put the sysctl in the manual?

Yes, I'll add something. I'm waiting to get some feedback on current patch first though...

This generally seems fine to me, but I'm not familiar with the original change and wonder why the compat shims are so coupled to the one application.

FreeBSD/sys/dev/hwpmc/hwpmc_mod.c
2320	Shouldn't this be `p->p_osrel <= 1200500`? Why not just do this unconditionally, i.e., why bother with a sysctl at all?

markj added a reviewer: jrtc27.May 28 2025, 6:58 PM

In D50546#1154747, @markj wrote:

This generally seems fine to me, but I'm not familiar with the original change and wonder why the compat shims are so coupled to the one application.

It's so coupled to one application because, as far as I'm aware, that application (AMDuProf) is only closed source, binary application that uses hwpmc collected data. This patchset fell out of my quest to get AmduProf running on -current.. I tried to do things in a somewhat generic way, but I admit that this has only been tested on AMDuProf so far..

FreeBSD/sys/dev/hwpmc/hwpmc_mod.c
2320	Its the fact that I'm targeting AmduProf here. Every build I have has that p_osrel..

jrtc27 added inline comments.May 28 2025, 9:03 PM

FreeBSD/sys/dev/hwpmc/hwpmc_logging.c
722	The implementation is for a specific version, but you allow any arbitrary version here?

gallatin added inline comments.Jun 9 2025, 5:24 PM

FreeBSD/sys/dev/hwpmc/hwpmc_logging.c
722	Yes; just trying to make it as easy as possible for somebody dealing with the same or similar issue in the future to find a version that works for them.