Page MenuHomeFreeBSD

acpi: Tell SMM we will handle CPPC notifications
ClosedPublic

Authored by thj on Sep 25 2022, 3:27 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, May 6, 4:13 PM
Unknown Object (File)
Tue, Apr 30, 7:44 AM
Unknown Object (File)
Sat, Apr 27, 3:10 AM
Unknown Object (File)
Fri, Apr 26, 11:25 AM
Unknown Object (File)
Tue, Apr 23, 4:05 AM
Unknown Object (File)
Mon, Apr 22, 9:03 PM
Unknown Object (File)
Mon, Apr 22, 8:12 PM
Unknown Object (File)
Mon, Apr 22, 9:25 AM

Details

Summary

Buggy SMM implementations can hang while processing CPPC notifications.
This leads to some laptops (notably Thinkpads) hanging when the
hwpstate_intel driver is loaded.

Tell the SMM that we will handle CPPC notifications as described in:

  • Intel® Processor Vendor-Specific ACPI
  • Intel® 64 and IA-32 Architectures Software Developer’s Manual

CPPC events default to masked (disabled) so while we do not do any
handling right now this does not seem to lead to any issues.

This approach was found via this Linux Kernel patch:
https://lkml.org/lkml/2016/3/17/563

Test Plan

I have a Thinkpad x1 carbon with a 10th Generation i7 (Intel(R) Core(TM)
i7-10610U CPU @ 1.80GHz). Before this patch it would hard lock under load, on
some power events such as inserting or removing power cables or just after some
time.

Once patched I can buildworld/buildkernel and run openssl speed and `povray
--benchmark` without issue.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

thj requested review of this revision.Sep 25 2022, 3:27 PM
thj created this revision.

Hmmm, are we supposed to be doing something in response to interrupts for this? We may want to have some sort of tunable to opt-in to this if we don't provide full support yet.

I wonder if this "fixes" an issue on my X1 where if I pull the power while suspending it gets stuck in a weird state where it never resumes.

  • Add a hint to disable the workaround
sys/dev/acpica/acpi_cpu.c
407

I would maybe call the hint cppc_notify with the sense being that "1" enables it and "0" disables notifications (we try to avoid "negative" hints/tunables). You would then just want to set the default value to 1 and change the last test to cppcval != 0. I would also perhaps rename cppcval to cppc_notify as well.

Sorry, one more suggestion. It occurred to me that having this be a hint under hint.acpi.0 is probably not really the best approach. For driver-wide default settings like this, the normal practice is to use a tunable exported as a sysctl under 'hw.<driver_name>'. For ACPI things these all live under hw.acpi, e.g.:

hw.acpi.cpu.cx_lowest: C8
hw.acpi.reset_video: 0
hw.acpi.handle_reboot: 1
hw.acpi.disable_on_reboot: 0
hw.acpi.verbose: 0
hw.acpi.s4bios: 0
hw.acpi.sleep_delay: 1
hw.acpi.suspend_state: S3
hw.acpi.standby_state: NONE
hw.acpi.lid_switch_state: NONE
hw.acpi.sleep_button_state: S3
hw.acpi.power_button_state: S5
hw.acpi.supported_sleep_state: S3 S4 S5

Probably this should be a hw.cpu.cppc_notify CTLFLAG_RDTUN sysctl. That also means you will have a way to document it via the sysctl description string, and in the future you can determine if it is enabled on user's machines in case we have future bug reports due to this change.

In D36699#837740, @jhb wrote:

For driver-wide default settings like this, the normal practice is to use a tunable exported as a sysctl under 'hw.<driver_name>'. For ACPI things these all live under hw.acpi, e.g.:

I was going to make a similar suggestion. Otherwise I like this change.

  • use a RDTUN sysctl rather than a hint
  • Use a global control for cppc notifications
This revision is now accepted and ready to land.Oct 7 2022, 5:56 PM

I'm on stable/13 9168218160 and just had my Carbon X1 gen 7 wedge hard again after rebuilding world and kernel with 111554 patch applied and the hwpstate_intel.0.disabled hint still commented out.
I can't provide any debug info but I rebuild my system this morning and the uptime couldn't have been more than a few hours.
Before applying the 111554 patch I applied 110951 (also whilst on stable/13 9168218160) and my system had been running stable since 30 September ( https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=253288#c67 )
I always do a 'make cleanworld' before rebuilding world and a custom kernel I also was using with the 110951 patch applied.
After building world and kernel I always run beinstall.sh and then immediately reboot.
I use latest and all my packages had been updated before rebuilding world and kernel

  • pkg info -x drm-510-kmod

drm-510-kmod-5.10.113_7

cd /usr/src
sh tools/build/beinstall.sh KERNCONF=HARBINGER

/usr/src/sys/amd64/conf/HARBINGER:

include GENERIC

ident HARBINGER

makeoptions WITH_EXTRA_TCP_STACKS=1
options TCPHPTS

Should I have waited for the patch to officially land?

This revision was automatically updated to reflect the committed changes.