You are correct, thank you for the observation. pmcstat does not exercise amd_get_msr() directly. The correct test path is via pmc_x86_get_msr() from libpmc, which issues a PMC_OP_GETMSR ioctl into the kernel amd_get_msr() code path.
Hardware validation was performed on AMD Ryzen 5 5600X (Family 19h, Zen 3), FreeBSD 16.0-CURRENT (hwpmc-amd-work-n284229-d18be873e2c2):

The change seems correct to me, but the test plan does not; pmcstat does not make any use of the pmc_get_msr() code path. Perhaps you have some other use-case or test program that you used?

In D32316#741213, @emaste wrote:

@ray I assume you can go ahead with committing this one?

@ray I assume you can go ahead with committing this one?

In D32316#737883, @ray wrote:

In D32316#737876, @mhorne wrote:

It seems to me that only the changes to pmc_arm64_initialize() should be necessary, because it handles optional classes in the same way that pmc_intel_initialize() does, by passing the correct nclasses value to pmc_mdep_alloc().

Problem here is in static machdep class numbers. If classes will be initialized in incorrect order, adjusted ri will be incorrect. So that modification may save some time on debugging such issue for new optional classes with just little time in hwpmc(4) init.

In D32316#737876, @mhorne wrote:

It seems to me that only the changes to pmc_arm64_initialize() should be necessary, because it handles optional classes in the same way that pmc_intel_initialize() does, by passing the correct nclasses value to pmc_mdep_alloc().

It seems to me that only the changes to pmc_arm64_initialize() should be necessary, because it handles optional classes in the same way that pmc_intel_initialize() does, by passing the correct nclasses value to pmc_mdep_alloc().

Update classes base ri to handle optional classes out of order.

Comment update LGTM

Squash commits.

Fix comment.

In D30047#676281, @mhorne wrote:

This looks fine to me, thanks for fixing this.

It seems that pmc_id_t should be an opaque type for consumers of libpmc? I.e. the only userspace consumer of these bit macros should be libpmc itself.

If that is the case, then this change should be okay without a PMC_VERSION bump. There are no uses of PMC_ID_TO_MODE or PMC_ID_MAKE_ID in libpmc. However, I did not check if this was true for older releases.

This looks fine to me, thanks for fixing this.

This was submitted as r282866.

In D2518#46456, @jhb wrote:

Ok. I think the longer term fix is to update the MAP_IN message to include the relative offset of the mapping to the backing object, then it wouldn't have to parse the header to see what offset of the file is mapped.

Ok. I think the longer term fix is to update the MAP_IN message to include the relative offset of the mapping to the backing object, then it wouldn't have to parse the header to see what offset of the file is mapped.

Example running unixbench:

With this change pmcstat(8) still does not resolve symbols for some applications, but at least they show up in pmcstat -T and in the callgraph (before that pmcstat would mark their frames as "dubious").