Page MenuHomeFreeBSD

hwpmc/amd: add PerfMonV2 global-control path
Needs ReviewPublic

Authored by afscoelho_gmail.com on Wed, Jul 15, 1:46 PM.

Details

Summary

Add support for AMD PerfMonV2 (Family 19h+) core counters, which need
both the per-counter EVSEL enable bit and the global GLOBAL_CTL bit set
to count. Detects PerfMonV2 at init and switches to v2-specific
start/stop/interrupt handlers; older CPUs and L3/DF counters keep using
the classic path unchanged. Adds a read-only sysctl,
kern.hwpmc.amd_perfmon_v2, to report which path is active.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 74843
Build 71726: arc lint + arc unit

Event Timeline

Maybe I'm missing something but it doesn't look like this feature is completely implemented.

sys/dev/hwpmc/hwpmc_amd.c
578

Shouldn't you maintain the mask and only enable core counters that are running otherwise you might get a spurious interrupt on the wrong counter.

643

This doesn't look right am I confused and you did it elsewhere. Shouldn't you use the global counter control to block the interrupts in a non-racey way. This is basically the same as the old code but you don't wait for the spurious interrupt.

1298

Are these printf's just for debugging I don't think we should keep them right?

This revision now requires changes to proceed.Wed, Jul 15, 4:07 PM

Maybe I'm missing something but it doesn't look like this feature is completely implemented.

I think it looks incomplete to you because this patch is just the foundation for the next one, LBR V2, so I intentionally didn't introduce changes to user visible behavior, pmcstat works exactly as before, and the only new sysctl reporting if the v2 path is active. These changes make counters to start/stop via EVSEL + GLOBAL_CTL pair, and the NMI handler finds overflows with one GLOBAL_STATUS read instead of polling every counter. But I may have missed something for this foundation, do you have someting in mind?

sys/dev/hwpmc/hwpmc_amd.c
578

Everything I did here was to mimic what Linux does, and in Linux amd_pmu_core_enable_all() writes the full counter mask unconditionally and so counter that is not running is already frozen by its own EVSEL Enable bit. Because of that, it can`t raise an NMI.

643

Hmm, you are right! I got confused because of GLOBAL_CTL. From what I got, clearing this counter's GLOBAL_CTL bit stops it immediately, the hardware makes an AND between it and the EVSEL bit, so dropping any of they freezes the counter immediately . This is why the race will not happen. But, what I did wrong was to not wait for the possible in flight overflow NMI. Thanks for noticing it, I will fix in the next patch

1298

yeah, its a left over : )

Seems sensible as a starting point.

sys/dev/hwpmc/hwpmc_amd.c
797–800

Slightly more idiomatic/efficient way to loop over a bitmap is with ffsll():

status = rdmsr(AMD_PMC_GLOBAL_STATUS);
status &= amd_global_cntr_mask;

int idx;
while ((idx = ffsll(status) - 1) != 0) {
    status &= ~(1ul << idx);

    /* handle overflow */
}

(Please double check the return of ffsll() to figure out exactly where the - 1 is needed)