The v2 path writes AMD_PMC_GLOBAL_CTL whenever the PerfMonV2 CPUID bit
is set. bhyve passes that bit through from the host but does not emulate
the register, so loading hwpmc in a guest on a Family 19h or later host
panics the kernel.

amd_v2_hwcheck_cpu() already probes EVSEL MSRs, so probe GLOBAL_CTL
there too, in order to reject the v2 path and fall back to the generic
CPU class.

Fixes: 9d3064010590 ("hwpmc_amd: add PerfMonV2 global-control path")
Sponsored by: The FreeBSD Foundation

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

christos created this revision.Mon, Nov 2, 1:52 PM

Herald added a reviewer: gnn. · View Herald TranscriptMon, Nov 2, 1:52 PM

Herald added a subscriber: imp. · View Herald Transcript

christos requested review of this revision.Mon, Nov 2, 1:52 PM

No, I do not think this is right. The problem is that VMM declare the capability while it does not implement it. So the fix must be in bhyve and not in consumers.

In D61133#1401454, @kib wrote:

No, I do not think this is right. The problem is that VMM declare the capability while it does not implement it. So the fix must be in bhyve and not in consumers.

D61141

christos mentioned this in D61141: amd64/vmm: Don't advertise AMD PerfMonV2 or performance counters.Tue, Nov 3, 9:15 AM

Reopening after discussion in D61141.

LGTM. Please allow @afscoelho_gmail.com to confirm.

This revision is now accepted and ready to land.Thu, Nov 5, 1:25 PM

I think this is worth tyhis having alongside D61141. I think a guest kernel shouldn't panic because a hypervisor gives inconsistent CPUID, and older bhyve hosts and other VMMs will be around for a while. Maybe mention that and D61141 in the summary.
Because every PerfMonV2 hardware also implements PerfCtrExtCore, so v2 without PCXC is an inconsistent CPUID. You could add to pmc_amd_initialize() :

today (pmc_amd_initialize(), hwpmc_amd.c):

/* EAX bit 0 holds the PerfMonV2 flag. */
if (EXTPERFMON_PERFMONV2(regs[0]) && family >= 0x19)
   amd_perfmon_v2 = true;

suggestion:

/*
 * EAX bit 0 holds the PerfMonV2 flag.  PerfMonV2 requires
 * PerfCtrExtCore; reject inconsistent CPUID (e.g. bhyve).
 */
if (EXTPERFMON_PERFMONV2(regs[0]) && family >= 0x19 &&
    (amd_feature2 & AMDID2_PCXC) != 0)
   amd_perfmon_v2 = true;

so we could also reject v2 when PCXC is clear, using CPUID only. I'd keep your GLOBAL_CTL probe also.

Tested on an AMD EPYC (Zen4) host: built a guest kernel+world with this patch, booted under bhyve+UEFI, ran kldload hwpmc. Loads cleanly (exit 0), no panic. This exercised the actual guard path - CPUID advertises PerfMonV2 but bhyve doesn't virtualize the MSRs, so hwpmc hits the read failure and falls back gracefully instead of crashing. (Note: this required the host to carry the bhyve/SVM ASID fix from D61226 - without it, no bhyve+UEFI guest boots at all on SVM/NPT hardware.) LGTM.