Mark Kettenis <mark.kette...@xs4all.nl> writes:
> Sounds like Intel changed the way CPU frequency scaling is implemented
> on these new CPUs. Somebody will have to look into this, but many
> OpenBSD developers prefer to buy machines with AMD CPU which is
> probably why this hasn't happened yet.
For those joining us on tech@, original bug report:
https://marc.info/?l=openbsd-bugs&m=162424580212241&w=2
I managed to work out what's happening here. There's a feature called
Intel Hardware P-States (HWP), which is known by the marketing gimmick
name of "Intel SpeedShift". This is apparently something distinct from
SpeedStep.
This is nothing new - it's been around since Skylake apparently. But, my
research indicates it is usually disabled by default. For whatever
reason - whether it be the fact my chipset is very very new, or just by
pure luck - my BIOS *enables* it by default.
Despite the fact there's a separate setting for SpeedStep, if SpeedShift
is enabled, according to Intel's documentation, the chip will no longer
repond to the usual scaling mechanisms (i.e. SpeedStep). So one solution
to this is just to turn it off in the BIOS - this works for me, even
once I re-enabled SpeedStep and C-States, leaving SpeedShift disabled
was enough to get good performance.
The issue is while you can detect that SpeedShift is on at
run-time, you can't turn it off without a reset. The Intel documentation
however suggests a workaround, which is implemented in the patch below.
https://www.intel.com/content/dam/www/public/us/en/documents/manuals/64-ia-32-architectures-software-developer-vol-3b-part-2-manual.pdf
Seconds 14.4 has all the necessary info.
The workaround consists of this:
- Detect if HWP is enabled and set a flag indicating the package
supports it.
- If it is, don't enable SpeedStep (since it won't work anyway).
Then, for each core:
- Ask what the "maximum performance" is by querying the capabilities
register.
- Request that the minimum and maximum performance be set to this and
call it a day.
(there is an MSR defined in the manual for doing this step for the
*whole* package, but the manual indicates it's not always available - I
tried it on my hardware with no success)
This is kinda ugly as it basically runs the CPU flat-out as if no
scaling was enabled. Unfortunately, *not* doing the second step results
in very poor performance as I was experiencing before. After I found
this I wondered if just disabling the SpeedStep driver was sufficient -
apparently not.
Interestingly, my compile test went from 2min15sec with SpeedShift
disabled in the BIOS, to 1min30sec with this patch (compared to the
7mins in my initial report where scaling wasn't working properly).
I have no idea if anyone will be interested in committing this patch, I
just wanted to share what I learned. The ultimate solution is probably
to either write a driver for this or import the one that's in FreeBSD to
replace SpeedStep for CPUs where HWP is enabled. But I'm probably going
to just disable SpeedShift in the BIOS since that does the job. :-)
Thanks,
-ajf
Index: sys/arch/amd64//amd64/identcpu.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
retrieving revision 1.118
diff -u -p -u -p -r1.118 identcpu.c
--- sys/arch/amd64//amd64/identcpu.c 31 Dec 2020 06:22:33 -0000 1.118
+++ sys/arch/amd64//amd64/identcpu.c 23 Jun 2021 02:58:59 -0000
@@ -64,6 +64,7 @@ int amd64_has_aesni;
#endif
int has_rdrand;
int has_rdseed;
+int intel_has_hwp;
const struct {
u_int32_t bit;
@@ -214,6 +215,7 @@ const struct {
}, cpu_tpm_eaxfeatures[] = {
{ TPM_SENSOR, "SENSOR" },
{ TPM_ARAT, "ARAT" },
+ { TPM_HWP, "HWP" },
}, cpu_cpuid_perf_eax[] = {
{ CPUIDEAX_VERID, "PERF" },
}, cpu_cpuid_apmi_edx[] = {
@@ -699,7 +701,24 @@ identifycpu(struct cpu_info *ci)
setperf_setup = k1x_init;
}
- if (cpu_ecxfeature & CPUIDECX_EST)
+ /*
+ * Intel HWP/"SpeedShift" - if enabled, CPU scaling won't work right.
+ * This detects that the package has this capability, but needs to be
+ * configured per core.
+ */
+ intel_has_hwp = 0;
+ if (!strcmp(cpu_vendor, "GenuineIntel") && (ci->ci_feature_tpmflags & TPM_HWP))
+ {
+ uint64_t msr_hwp;
+
+ msr_hwp = rdmsr(IA32_PM_ENABLE);
+ if (msr_hwp & IA32_HWP_ENABLE) {
+ printf("SpeedShift detected. SpeedStep will not be enabled, and CPU scaling won't work.\n");
+ intel_has_hwp = 1;
+ }
+ }
+
+ if ((cpu_ecxfeature & CPUIDECX_EST) && !intel_has_hwp)
setperf_setup = est_init;
#endif
@@ -729,6 +748,25 @@ identifycpu(struct cpu_info *ci)
sensor_task_register(ci, intelcore_update_sensor, 5);
sensor_attach(&ci->ci_sensordev, &ci->ci_sensor);
sensordev_install(&ci->ci_sensordev);
+ }
+
+ if (intel_has_hwp)
+ {
+ uint64_t msr_cap, msr_req, max_perf;
+
+ msr_cap = rdmsr(IA32_HWP_CAPABILITIES);
+
+ /* Lowest byte is the "highest performance" capability */
+ max_perf = msr_cap & 0xFF;
+
+ /*
+ * Least-significant byte is "minimum performance",
+ * next byte is "maximum performance". Setting these
+ * the same bypasses HWP - basically, runs the chip
+ * at full tilt.
+ */
+ msr_req = (max_perf << 8) + max_perf;
+ wrmsr(IA32_HWP_REQUEST, msr_req);
}
#endif
Index: sys/arch/amd64//include/specialreg.h
===================================================================
RCS file: /cvs/src/sys/arch/amd64/include/specialreg.h,v
retrieving revision 1.90
diff -u -p -u -p -r1.90 specialreg.h
--- sys/arch/amd64//include/specialreg.h 14 May 2021 16:44:38 -0000 1.90
+++ sys/arch/amd64//include/specialreg.h 23 Jun 2021 02:58:59 -0000
@@ -235,6 +235,7 @@
*/
#define TPM_SENSOR 0x00000001 /* Digital temp sensor */
#define TPM_ARAT 0x00000004 /* APIC Timer Always Running */
+#define TPM_HWP 0x00000080 /* Hardware P-State (HWP) */
/*
* "Architectural Performance Monitoring" bits (CPUID function 0x0a):
@@ -931,6 +932,12 @@
#define IA32_DEBUG_INTERFACE_ENABLE 0x00000001
#define IA32_DEBUG_INTERFACE_LOCK 0x40000000
#define IA32_DEBUG_INTERFACE_MASK 0x80000000
+
+/* Intel HWP - Skylake and newer */
+#define IA32_PM_ENABLE 0x00000770 /* IA32_PM_ENABLE MSR */
+#define IA32_HWP_ENABLE (1ULL << 0) /* HWP status bit */
+#define IA32_HWP_CAPABILITIES 0x00000771 /* HWP capabilities MSR */
+#define IA32_HWP_REQUEST 0x00000774 /* IA32_HWP_REQUEST MSR */
/*
* VMX
