diff --git a/sys/x86/cpufreq/hwpstate_amd.c b/sys/x86/cpufreq/hwpstate_amd.c --- a/sys/x86/cpufreq/hwpstate_amd.c +++ b/sys/x86/cpufreq/hwpstate_amd.c @@ -73,6 +73,7 @@ #include #include +#include #include @@ -152,6 +153,10 @@ #define HWPFL_USE_CPPC (1 << 0) #define HWPFL_CPPC_REQUEST_NOT_READ (1 << 1) +/* Bits for the hwpstate_amd_cppc_verify tunable. */ +#define HWPSTATE_AMD_CPPC_VERIFY_CPUID (1 << 0) +#define HWPSTATE_AMD_CPPC_VERIFY_ACPI (1 << 1) + struct hwpstate_cpufreq_methods { int (*get)(device_t dev, struct cf_setting *cf); int (*set)(device_t dev, const struct cf_setting *cf); @@ -177,6 +182,7 @@ }; struct { uint64_t request; + struct acpi_cppc_ctx *ctx; } cppc; }; u_int cpuid; @@ -211,6 +217,13 @@ &hwpstate_amd_cppc_enable, 0, "Set 1 (default) to enable AMD CPPC, 0 to disable"); +static int hwpstate_amd_cppc_verify = HWPSTATE_AMD_CPPC_VERIFY_CPUID | + HWPSTATE_AMD_CPPC_VERIFY_ACPI; +SYSCTL_INT(_machdep, OID_AUTO, hwpstate_amd_cppc_verify, CTLFLAG_RDTUN, + &hwpstate_amd_cppc_verify, 0, + "Bitmask gating CPPC use (default 0x3): bit 0 (0x1) requires the CPUID " + "CPPC bit; bit 1 (0x2) requires an ACPI _CPC object"); + static device_method_t hwpstate_methods[] = { /* Device interface */ DEVMETHOD(device_identify, hwpstate_identify), @@ -871,10 +884,20 @@ * does. */ SET_BITS_VALUE(data->request, AMD_CPPC_REQUEST_EPP_BITS, 0); - /* 0 in "Desired Performance" is autonomous mode. */ - MPASS(highest_perf != 0); - SET_BITS_VALUE(data->request, AMD_CPPC_REQUEST_DES_PERF_BITS, - highest_perf); + + /* + * Some CPUs advertise CPPC in CPUID but lack a _CPC object in the + * DSDT, and their autonomous mode is broken. When the _CPC object is + * present we use autonomous mode; otherwise we pin to the highest + * performance level to avoid performance regressions. + */ + if (sc->cppc.ctx != NULL) + /* 0 in "Desired Performance" is autonomous mode. */ + SET_BITS_VALUE(data->request, AMD_CPPC_REQUEST_DES_PERF_BITS, + 0); + else + SET_BITS_VALUE(data->request, AMD_CPPC_REQUEST_DES_PERF_BITS, + highest_perf); error = wrmsr_safe(MSR_AMD_CPPC_REQUEST, data->request); if (error != 0) @@ -1023,6 +1046,7 @@ hwpstate_probe(device_t dev) { struct hwpstate_softc *const sc = device_get_softc(dev); + struct acpi_cppc_ctx *ctx; if (cpu_get_pcpu(dev) == NULL) { device_printf(dev, @@ -1031,19 +1055,33 @@ } sc->cpuid = cpu_get_pcpuid(dev); - if (hwpstate_amd_cppc_enable && - (amd_extended_feature_extensions & AMDFEID_CPPC)) { + if (hwpstate_amd_cppc_enable) sc->flags |= HWPFL_USE_CPPC; - device_set_desc(dev, - "AMD Collaborative Processor Performance Control (CPPC)"); - } else - device_set_desc(dev, "Cool`n'Quiet 2.0"); + + if ((hwpstate_amd_cppc_verify & HWPSTATE_AMD_CPPC_VERIFY_CPUID) != 0 && + (amd_extended_feature_extensions & AMDFEID_CPPC) == 0) + sc->flags &= ~HWPFL_USE_CPPC; + /* + * Keep the _CPC context local until we commit to CPPC: sc->cppc shares + * a union with the P-state settings, so storing it before choosing the + * P-state path would leak the context and be clobbered by that path. + */ + ctx = acpi_cppc_ctx_init(dev); + if ((hwpstate_amd_cppc_verify & HWPSTATE_AMD_CPPC_VERIFY_ACPI) != 0 && + ctx == NULL) + sc->flags &= ~HWPFL_USE_CPPC; sc->dev = dev; if ((sc->flags & HWPFL_USE_CPPC) != 0) { + sc->cppc.ctx = ctx; sc->cpufreq_methods = &cppc_methods; + device_set_desc(dev, + "AMD Collaborative Processor Performance Control (CPPC)"); return (0); } + if (ctx != NULL) + acpi_cppc_ctx_free(dev, ctx); + device_set_desc(dev, "Cool`n'Quiet 2.0"); sc->cpufreq_methods = &pstate_methods; return (hwpstate_probe_pstate(dev)); } @@ -1283,6 +1321,8 @@ sc = device_get_softc(dev); if ((sc->flags & HWPFL_USE_CPPC) == 0) hwpstate_goto_pstate(dev, 0); + else if (sc->cppc.ctx != NULL) + acpi_cppc_ctx_free(dev, sc->cppc.ctx); return (cpufreq_unregister(dev)); }