Skip to content

hwpmc: Avoid using AMD PMCs if in use by firmware#2277

Open
mashtizadeh wants to merge 1 commit into
freebsd:mainfrom
mashtizadeh:hwpmc-badbios
Open

hwpmc: Avoid using AMD PMCs if in use by firmware#2277
mashtizadeh wants to merge 1 commit into
freebsd:mainfrom
mashtizadeh:hwpmc-badbios

Conversation

@mashtizadeh

Copy link
Copy Markdown
Contributor

Some firmwares use the PMCs to monitor OS performance. We can't be certain that the BIOS would detect any change to the counters if we reprogram them. In cases where the firmware is using the PMCs to control power management this could have dangerous side effects or unexpected performance effects. We detect if any of the counters are enabled and disable the AMD PMCs.

Reported by: Sandipan Das
Sponsored by: Netflix

Some firmwares use the PMCs to monitor OS performance.  We can't be
certain that the BIOS would detect any change to the counters if we
reprogram them.  In cases where the firmware is using the PMCs to
control power management this could have dangerous side effects or
unexpected performance effects.  We detect if any of the counters are
enabled and disable the AMD PMCs.

Reported by: Sandipan Das
Sponsored by: Netflix
@github-actions

Copy link
Copy Markdown

Thank you for taking the time to contribute to FreeBSD!

There is an issue that needs to be resolved:

  • Missing Signed-off-by lines (ff2dd9b)

Note

Please review CONTRIBUTING.md, then update and push your branch again.

@mashtizadeh

mashtizadeh commented Jun 10, 2026

Copy link
Copy Markdown
Contributor Author

I need to write a similar patch for Intel.

I couldn't find an AMD machine with this problem, but I have an old Intel HP box that exhibits this behavior.

Sandipan is one of AMD's Linux committers that mentioned the problems in our last meeting. He cited this patch:
torvalds/linux@4407204

@mashtizadeh

Copy link
Copy Markdown
Contributor Author

@bsdimp @mhorne

@mhorne mhorne left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bad BIOS!

Generally, LGTM. You might split this logic into a helper function.

Comment thread sys/dev/hwpmc/hwpmc_amd.c
* performance monitoring or power management functions in the BIOS to
* safely make use of the counters.
*/
for (i = 0; i < AMD_PMC_CORE_DEFAULT; i++) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should these tests come after we have queried and set amd_core_npmcs? Or is it only important to check the first 6 counters?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine were it is, we should only see this on older machines. HWP and other mechanisms are providing alternative ways for them to continue to implement similar functionality without using the counters anymore. I can't repro it on any recent Intel or AMD machines I bought from those brands that I know have done this in the past.

Don't forget it needs to come above the VM check as that will touch the first PMC.

@mashtizadeh mashtizadeh Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As for the helper function, I'll make that change and update this diff tomorrow or Saturday.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also owe you a patch to clean up the SYSTEM/USER flag it's going to work as you suggested, but I want to implement a best effort filtering. IBS isn't like other counters we get accurate samples but still suffer from skid during delivery.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants