| Message ID | 2340775.C3S44OUvaq@house (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
| Series | Why is guaranteed frequency exposed via intel_pstate driver and not via cppc_data? | expand |
On Fri, Dec 14, 2018 at 9:56 AM Thomas Renninger <trenn@suse.de> wrote: > > Hi, > > this is about: > Subject: ACPI / CPPC: Add support for guaranteed performance > Git-commit: 29523f095397637edca60c627bc3e5c25a02c40f > and > Subject: cpufreq: intel_pstate: Add base_frequency attribute > Git-commit: 86d333a8cc7f66c2314ab1e147834a1cd95ec2de > > both came in 4.20-rc1 > (so this sysfs API has not been released yet!) > > why has the guaranteed freq retrieved from ACPI CPPC data > not been exposed in: > /sys/devices/system/cpu/cpu1/acpi_cppc > /sys/devices/system/cpu/cpu1/acpi_cppc/reference_perf > /sys/devices/system/cpu/cpu1/acpi_cppc/highest_perf > /sys/devices/system/cpu/cpu1/acpi_cppc/nominal_perf > /sys/devices/system/cpu/cpu1/acpi_cppc/lowest_perf > /sys/devices/system/cpu/cpu1/acpi_cppc/lowest_nonlinear_perf > /sys/devices/system/cpu/cpu1/acpi_cppc/feedback_ctrs > /sys/devices/system/cpu/cpu1/acpi_cppc/wraparound_time > > like all the other cppc_perf_caps fields? > > Instead the above mentioned second patch (86d333a8cc7f66c2314ab1): > exports it through intel_pstate cpufreq driver sysfs subdirs. What is exposed by intel_pstate is the base frequency, which happens to related to the guaranteed performance parameter in CPPC on the system that provide it (which need not be the case as it is optional and that's why intel_pstate falls back to reading it from an MSR in case it is not present in the ACPI tables). > This data might be available and useful even if no > intel_pstate driver is running. So it might be exposed if anyone needs it *in* *addition* to what intel_pstate is exposing, but that would be a separate ABI change. Thanks, Rafael
On Friday, December 14, 2018 10:30:19 AM CET Rafael J. Wysocki wrote: > On Fri, Dec 14, 2018 at 9:56 AM Thomas Renninger <trenn@suse.de> wrote: > > Hi, > > > > this is about: > > Subject: ACPI / CPPC: Add support for guaranteed performance > > Git-commit: 29523f095397637edca60c627bc3e5c25a02c40f > > and > > Subject: cpufreq: intel_pstate: Add base_frequency attribute > > Git-commit: 86d333a8cc7f66c2314ab1e147834a1cd95ec2de > > > > both came in 4.20-rc1 > > (so this sysfs API has not been released yet!) ... > What is exposed by intel_pstate is the base frequency, which happens > to related to the guaranteed performance parameter in CPPC on the > system that provide it (which need not be the case as it is optional > and that's why intel_pstate falls back to reading it from an MSR in > case it is not present in the ACPI tables). ... > > This data might be available and useful even if no > > intel_pstate driver is running. > > So it might be exposed if anyone needs it *in* *addition* to what > intel_pstate is exposing, but that would be a separate ABI change. turbostat and x86_energy_perf_policy userspace tools already can read such MSR themselves: turbostat.c: if (get_msr(cpu, MSR_HWP_CAPABILITIES, &msr)) x86_energy_perf_policy.c: read_hwp_cap(cpu, &cap, MSR_HWP_CAPABILITIES); Only need I see is in secure boot or whenever msr userspace reads are not allowed. Also this: > What is exposed by intel_pstate is the base frequency sounds like a more general CPU data, not intel_pstate specific. I just wonder what happens in userspace when Intel comes up with another cpufreq driver... Other archs also have a base frequency? Thomas
On Fri, Dec 14, 2018 at 11:01 AM Thomas Renninger <trenn@suse.de> wrote: > > On Friday, December 14, 2018 10:30:19 AM CET Rafael J. Wysocki wrote: > > On Fri, Dec 14, 2018 at 9:56 AM Thomas Renninger <trenn@suse.de> wrote: > > > Hi, > > > > > > this is about: > > > Subject: ACPI / CPPC: Add support for guaranteed performance > > > Git-commit: 29523f095397637edca60c627bc3e5c25a02c40f > > > and > > > Subject: cpufreq: intel_pstate: Add base_frequency attribute > > > Git-commit: 86d333a8cc7f66c2314ab1e147834a1cd95ec2de > > > > > > both came in 4.20-rc1 > > > (so this sysfs API has not been released yet!) > > ... > > > What is exposed by intel_pstate is the base frequency, which happens > > to related to the guaranteed performance parameter in CPPC on the > > system that provide it (which need not be the case as it is optional > > and that's why intel_pstate falls back to reading it from an MSR in > > case it is not present in the ACPI tables). > > ... > > > > This data might be available and useful even if no > > > intel_pstate driver is running. > > > > So it might be exposed if anyone needs it *in* *addition* to what > > intel_pstate is exposing, but that would be a separate ABI change. > > turbostat and x86_energy_perf_policy userspace tools already can read > such MSR themselves: > > turbostat.c: if (get_msr(cpu, MSR_HWP_CAPABILITIES, &msr)) > x86_energy_perf_policy.c: read_hwp_cap(cpu, &cap, MSR_HWP_CAPABILITIES); > > Only need I see is in secure boot or whenever msr userspace reads are > not allowed. Which is in SUSE kernels (among others) IIRC, isn't it? Also you need to know how to combine the information from the MSR with the CPPC information. That's why it is there in the driver. > Also this: > > > What is exposed by intel_pstate is the base frequency > > sounds like a more general CPU data, not intel_pstate specific. You could argue this way, but that information is really available only when HWP is in use and that's the case handled by intel_pstate. > I just wonder what happens in userspace when Intel comes up with another > cpufreq driver... That is unlikely. > Other archs also have a base frequency? If they have something like turbo, they probably also have something like the base frequency, but how it is defined depends on the arch (or platform even).
On Friday, December 14, 2018 11:11:12 AM CET Rafael J. Wysocki wrote: > On Fri, Dec 14, 2018 at 11:01 AM Thomas Renninger <trenn@suse.de> wrote: > > On Friday, December 14, 2018 10:30:19 AM CET Rafael J. Wysocki wrote: > > > On Fri, Dec 14, 2018 at 9:56 AM Thomas Renninger <trenn@suse.de> wrote: ... > You could argue this way, but that information is really available > only when HWP is in use and that's the case handled by intel_pstate. That makes sense. > > I just wonder what happens in userspace when Intel comes up with another > > cpufreq driver... > > That is unlikely. I would risk a bet on this.. Thanks for clarification! Thomas
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 217a782c3e55..ed52ce259788 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -159,6 +159,7 @@ show_cppc_data(cppc_get_perf_caps, cppc_perf_caps, nominal_perf); show_cppc_data(cppc_get_perf_caps, cppc_perf_caps, lowest_nonlinear_perf); show_cppc_data(cppc_get_perf_caps, cppc_perf_caps, lowest_freq); show_cppc_data(cppc_get_perf_caps, cppc_perf_caps, nominal_freq); +show_cppc_data(cppc_get_perf_caps, cppc_perf_caps, guaranteed_freq); show_cppc_data(cppc_get_perf_ctrs, cppc_perf_fb_ctrs, reference_perf);
Hi, this is about: Subject: ACPI / CPPC: Add support for guaranteed performance Git-commit: 29523f095397637edca60c627bc3e5c25a02c40f and Subject: cpufreq: intel_pstate: Add base_frequency attribute Git-commit: 86d333a8cc7f66c2314ab1e147834a1cd95ec2de both came in 4.20-rc1 (so this sysfs API has not been released yet!) why has the guaranteed freq retrieved from ACPI CPPC data not been exposed in: /sys/devices/system/cpu/cpu1/acpi_cppc /sys/devices/system/cpu/cpu1/acpi_cppc/reference_perf /sys/devices/system/cpu/cpu1/acpi_cppc/highest_perf /sys/devices/system/cpu/cpu1/acpi_cppc/nominal_perf /sys/devices/system/cpu/cpu1/acpi_cppc/lowest_perf /sys/devices/system/cpu/cpu1/acpi_cppc/lowest_nonlinear_perf /sys/devices/system/cpu/cpu1/acpi_cppc/feedback_ctrs /sys/devices/system/cpu/cpu1/acpi_cppc/wraparound_time like all the other cppc_perf_caps fields? Instead the above mentioned second patch (86d333a8cc7f66c2314ab1): exports it through intel_pstate cpufreq driver sysfs subdirs. This data might be available and useful even if no intel_pstate driver is running. It should have been as straight forward as (untested like this): show_cppc_data(cppc_get_perf_ctrs, cppc_perf_fb_ctrs, wraparound_time); @@ -189,6 +190,7 @@ static struct attribute *cppc_attrs[] = { &nominal_perf.attr, &nominal_freq.attr, &lowest_freq.attr, + &guaranteed_freq.attr, NULL };