Details

Reviewers

ali_mashtizadeh.com
mhorne
gnn

Commits

rG0aa4c25f3e83: hwpmc_ibs: Add Zen6 IBS ctl2 filters and alternate disable

Summary

Add kernel and userland support for Zen6 IBS extensions per AMD pub
69205 (rev 1.00, March 2026): alternate fetch/op disable via ctl2[0],
fetch latency filtering, virtual address bit 63 filtering, and
streaming-store filtering. Decode the new IbsOpData2 StrmSt and
RmtSocket bits. Update libpmc, pmcstat and manpage.

Pre-Zen6 systems work unchanged with ibs_ctl2 == 0.

Sponsored by: AMD
Signed-off-by: Andre Silva <andasilv@amd.com>

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

No Test Coverage

Build Status

Buildable 73083
Build 69966: arc lint + arc unit

Event Timeline

afscoelho_gmail.com created this revision.May 10 2026, 3:04 AM

afscoelho_gmail.com held this revision as a draft.

Herald added a reviewer: gnn. · View Herald TranscriptMay 10 2026, 3:04 AM

Herald added a reviewer: ali_mashtizadeh.com. · View Herald Transcript

Herald added subscribers: ziaee, imp. · View Herald Transcript

Harbormaster completed remote builds in B72960: Diff 177522.May 10 2026, 3:04 AM

Submitting diff

Harbormaster completed remote builds in B73083: Diff 177802.May 13 2026, 6:54 PM

afscoelho_gmail.com published this revision for review.May 13 2026, 6:58 PM

afscoelho_gmail.com edited reviewers, added: mhorne; removed: gnn.May 19 2026, 1:32 PM

Herald added a reviewer: gnn. · View Herald TranscriptMay 19 2026, 1:32 PM

@mashtizadeh_gmail.com

I did not look in detail at all of the code, but here is some initial comments.

lib/libpmc/libpmc.c
728–734	This seems unrelated to adding Zen6 feature support...
737–743	I think this type of validation is a little messy. In most cases, it can be removed; for example, it is harmless to allow the user to specify `l3miss` multiple times.
785–802	Here we can honor the final requested value for the flag. This is typical in unix utility option parsing. So if the user requests: `addr63=0,l3miss,add63=1`, then we would take "1" as the final choice. The logical advice is "don't do this, it's confusing", but we don't really need to guard against it.
867–870	Just checking, this behaviour is correct, and not reversed?
sys/dev/hwpmc/hwpmc_ibs.c
624–625	Oh I see. Something about this scheme seems quite fragile... Can't we just report `pl_mpdata[PMC_MPIDX_FETCH_CTL2]` as zero/empty if it is not supported?
sys/dev/hwpmc/hwpmc_ibs.h
135–136	You need to bump this, no? (And `PMC_MPIDX_OP_MAX` below.)

mhorne added a subscriber: pmc.May 19 2026, 4:02 PM

afscoelho_gmail.com added inline comments.May 19 2026, 7:37 PM

lib/libpmc/libpmc.c
728–734	Fair point, I'll restore the CPUID read and the userland gating for (l3miss/ldlat=/opcount/OPCNTEXT rate). Kernel-side EXTERROR validation can stay and libpmc as a cheap early reject. Will keep this patch focused on the Zen6 additions.
737–743	That's ok, I'll drop the seen* variables, adopt the "last value wins" convention
867–870	That's correct, it is not reversed. Per AMD pub 69205, IbsOpCtl2 bit 1 (ExclRip63Eq0) excludes RIP[63]==0 and bit 2 (ExclRip63Eq1) excludes RIP[63]==1. addr63=0 --> keep bit63=0 --> set EXCLRIP63EQ1 addr63=1 --> keep bit63=1 --> set EXCLRIP63EQ0
sys/dev/hwpmc/hwpmc_ibs.c
624–625	Thanks! I think this was a good catch. I will make pl_length = PMC_MPIDX_FETCH_MAX unconditionally and on pre-Zen6, pl_mpdata[PMC_MPIDX_FETCH_CTL2] stays zero
sys/dev/hwpmc/hwpmc_ibs.h
135–136	yes, thanks, that was an oversight. I'll handle this as part of the next fix.

Address mhorne review feedback

Harbormaster completed remote builds in B73233: Diff 178130.May 19 2026, 7:49 PM

Thanks for the review, mhorne. Addressed all six comments in the latest diff. PTAL when you get a chance.

The only major issue is that we shouldn't expose bit63 as a flag that's just CAP_USER and CAP_SYSTEM. The other changes are real minor. Mitchell didn't like the structure size changing but we have to support it if we want any resemblance of backwards compatibility.

lib/libpmc/libpmc.c
778	The whole point of addr63 is that it allows us to introduce kernel/user filtering without worrying about skid. I.e. the sample may have been captured in kernel mode but then is reported as a user sample because it takes potentially 10k cycles to delivery the NMI. Instead of exposing bit63 as a flag we should interpret the standard user/kernel flags that are already present as capabilities. This is why I left the CAP_USER/SYSTEM stuff a bit unfinished since we didn't really have a way to deal with it. So I'd rather the standard user/kernel flags trigger setting the capability flags that we 'or' in, in the kernel similar to other counters.
854	Same here this should just be user/system cap flags.
lib/libpmc/pmc.ibs.3
92	and here with a (zen 6)
101	Update this to say on zen6 we support user/kernel filtering.
sys/dev/hwpmc/hwpmc_ibs.c
83	I'd prefer you move this up above so fetch and op aren't interleaved.
192	Again I think your doing too much validation in the kernel we should land my safe msr change and not introduce complex logic. I really think it should just be a mask of valid bits and nothing else. That seems to be enough prevent GPs and I can't even reproduce a GP on zen5 seems they stopped doing it on a recent core.
449	Why define these functions when below you just manually or/and bits directly mixing and matching is a bit harder to read
563	Referring to here
624–625	Oh actually Mitchell I made that intentional as we expect this structures to grow. For backwards compatibility we will always have to look at the length field so I think its totally reasonable that we allow it change in size based on supported registers.
sys/dev/hwpmc/hwpmc_ibs.h
37	Can you add a line here where you pulled the Zen 6 specifications from?
116	Nit Zen6 -> Zen 6
196	Nit "Zen6" -> "Zen 6"

This revision now requires changes to proceed.May 19 2026, 9:25 PM

Address ali_mashtizadeh.com review feedback

Harbormaster completed remote builds in B73317: Diff 178334.May 21 2026, 5:35 PM

Address ali_mashtizadeh.com review feedback

Harbormaster completed remote builds in B73318: Diff 178335.May 21 2026, 5:36 PM

Thanks for the review, Ali.

Thanks Andre for all the hard work sorry to point out one more change.

I’m concerned with those wrmsr’s and I think it was a Zen2 errata I remember I was able to reproduce. I think you should just have avoided refactoring the start code in this change or just keep the middle bit that sets the main ctl values, the zero’ing.

If you’re going to work on RAPL, can I take a lock on hwpmc_ibs.[ch] so I can rebase and merge tools.

@mhorne @imp

I need to push the my tools its going to be a series of patches and fix two things:

Improve the man page based on my experience with the tools
Add the heuristics I got from userspace to cleanup the events reported to honor CAP_USER/SYSTEM this is needed even with Bit63 filtering.

sys/dev/hwpmc/hwpmc_ibs.c
641	I think I meant to use the middle of the function of what you were trying to do I have to look back. I’m not sure this is safe here, we have an enabled IBS counter and you’re zero’ing it out. I think it’s in Zen2 errata that says zero’ing the counter isn’t safe. It will lead to invalid NMIs that we won’t ack. That’s why I have a whole dance around clearing the counter, then disabling enable, and finally zeroing it out inside of stop.
679	Same here

This revision now requires changes to proceed.May 22 2026, 5:43 AM

In D56914#1310001, @ali_mashtizadeh.com wrote:

Thanks Andre for all the hard work sorry to point out one more change.

I’m concerned with those wrmsr’s and I think it was a Zen2 errata I remember I was able to reproduce. I think you should just have avoided refactoring the start code in this change or just keep the middle bit that sets the main ctl values, the zero’ing.

If you’re going to work on RAPL, can I take a lock on hwpmc_ibs.[ch] so I can rebase and merge tools.

@mhorne @imp

I need to push the my tools its going to be a series of patches and fix two things:

Improve the man page based on my experience with the tools

Add the heuristics I got from userspace to cleanup the events reported to honor CAP_USER/SYSTEM this is needed even with Bit63 filtering.

Correct! Thanks for remembering me about this errata. The zero-write was fine in ibs_start_pmc (counter off), but I was also calling the helper from the NMI handlers where IBS is still live, hitting the Family 10h #420 hazard that the stop dance avoids. Linux's perf_ibs_handle_irq doesn't touch ctl in the re-arm either.
I dropped both helpers, inlined the ctl2 init into ibs_start_pmc between the zero and enable writes, and reverted the NMI sites to the single wrmsr(ctl | ENABLE). Updated diff incoming.

On the lock : go ahead and take it. RAPL is next for me and it's a separate area, so I'll keep off hwpmc_ibs.[ch] until your tools land. Ping me when you're ready.

v2: inline start helpers, simplify NMI re-arm path

Harbormaster completed remote builds in B73350: Diff 178426.May 22 2026, 6:55 PM

afscoelho_gmail.com marked 2 inline comments as done.May 22 2026, 6:56 PM

@ali_mashtizadeh.com

ali_mashtizadeh.com accepted this revision.May 26 2026, 5:19 PM

ali_mashtizadeh.com added inline comments.

lib/libpmc/libpmc.c
857	I really feel better with consistency, I assume others will have comments and if they do just add new lines between the condition and caps line like you did below and above.

This revision is now accepted and ready to land.May 26 2026, 5:19 PM

@alfredo

@imp @mhorne Can one of you please look at this and try to land it? Mitchell your first pass review I think was taken care of and I did two passes after. This is blocking me from landing the tools.

LGTM. I will merge this today to unblock.

In D56914#1318494, @ali_mashtizadeh.com wrote:

@imp @mhorne Can one of you please look at this and try to land it? Mitchell your first pass review I think was taken care of and I did two passes after. This is blocking me from landing the tools.

What are the tools?

lib/libpmc/pmc.ibs.3
143–151	This does not seem to match the code in `ibs_allocate_pmc()`, which applies `IBS_OP_CTL_L3MISSONLY` unconditionally.
sys/dev/hwpmc/hwpmc_ibs.c
400–401	It's fine to proceed, but I maintain that the flag usage here is incorrect and should be fixed. The cases are: If only `PMC_CAP_SYSTEM` is set, measure only system/kernel events If only `PMC_CAP_USER` is set, measure only userspace events If both `PMC_CAP_SYSTEM` and `PMC_CAP_USER` are set, measure all events If neither is set, behave as 3. This is the precedent in e.g. `hwpmc_amd.c`, `hwpmc_arm64.c`, and `hwpmc_power8.c`. Where filtering features are not supported by the CPU, it makes sense to only permit cases 3 and 4. But for some reason `PMC_CAP_SYSTEM` is always required.

Closed by commit rG0aa4c25f3e83: hwpmc_ibs: Add Zen6 IBS ctl2 filters and alternate disable (authored by afscoelho_gmail.com, committed by mhorne). · Explain WhyThu, Jun 11, 3:12 PM

This revision was automatically updated to reflect the committed changes.

mhorne added a commit: rG0aa4c25f3e83: hwpmc_ibs: Add Zen6 IBS ctl2 filters and alternate disable.

hwpmc: Add Zen6 IBS ctl2 filters and alternate disable
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 177802

lib/libpmc/libpmc.c

lib/libpmc/pmc.ibs.3

sys/dev/hwpmc/hwpmc_ibs.h

sys/dev/hwpmc/hwpmc_ibs.c

usr.sbin/pmcstat/pmcstat_log.c

hwpmc: Add Zen6 IBS ctl2 filters and alternate disableClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 177802

lib/libpmc/libpmc.c

lib/libpmc/pmc.ibs.3

sys/dev/hwpmc/hwpmc_ibs.h

sys/dev/hwpmc/hwpmc_ibs.c

usr.sbin/pmcstat/pmcstat_log.c

hwpmc: Add Zen6 IBS ctl2 filters and alternate disable
ClosedPublic
Actions

Revision Contents
Changeset List