Re: [PATCH 3/8] cpupower: Add CPUPOWER_CAP_AMD_HW_PSTATE cpuid caps flag

[Shuah Khan]

On 1/22/21 10:38 AM, Nathan Fontenot wrote:

Add a check in get_cpu_info() for the ability to read frequencies
from hardware and set the CPUPOWER_CAP_AMD_HW_PSTATE cpuid flag.
The cpuid flag is set when CPUID_80000007_EDX[7] is set,
which is all families >= 10h. The check excludes family 14h
because HW pstate reporting was not implemented on family 14h.

This is intended to reduce family checks in the main code paths.

Signed-off-by: Nathan Fontenot <nathan.fonte...@amd.com>
---
  tools/power/cpupower/utils/helpers/amd.c     |    6 +-----
  tools/power/cpupower/utils/helpers/cpuid.c   |   12 +++++++++---
  tools/power/cpupower/utils/helpers/helpers.h |    1 +
  3 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/power/cpupower/utils/helpers/amd.c 
b/tools/power/cpupower/utils/helpers/amd.c
index 34368436bbd6..496844a20fe2 100644
--- a/tools/power/cpupower/utils/helpers/amd.c
+++ b/tools/power/cpupower/utils/helpers/amd.c
@@ -94,11 +94,7 @@ int decode_pstates(unsigned int cpu, unsigned int cpu_family,
        union core_pstate pstate;
        unsigned long long val;

- /* Only read out frequencies from HW when CPU might be boostable

-          to keep the code as short and clean as possible.
-          Otherwise frequencies are exported via ACPI tables.
-       */

Please update comment instead of deleting it.

-       if (cpu_family < 0x10 || cpu_family == 0x14)
+       if (!(cpupower_cpu_info.caps & CPUPOWER_CAP_AMD_HW_PSTATE))
                return -1;

if (read_msr(cpu, MSR_AMD_PSTATE_LIMIT, &val))

diff --git a/tools/power/cpupower/utils/helpers/cpuid.c 
b/tools/power/cpupower/utils/helpers/cpuid.c
index f9a66a430b72..d577220a193b 100644
--- a/tools/power/cpupower/utils/helpers/cpuid.c
+++ b/tools/power/cpupower/utils/helpers/cpuid.c
@@ -128,9 +128,15 @@ int get_cpu_info(struct cpupower_cpu_info *cpu_info)
        /* AMD or Hygon Boost state enable/disable register */
        if (cpu_info->vendor == X86_VENDOR_AMD ||
            cpu_info->vendor == X86_VENDOR_HYGON) {
-               if (ext_cpuid_level >= 0x80000007 &&
-                   (cpuid_edx(0x80000007) & (1 << 9)))
-                       cpu_info->caps |= CPUPOWER_CAP_AMD_CPB;
+               if (ext_cpuid_level >= 0x80000007) {
+                       if (cpuid_edx(0x80000007) & (1 << 9))
+                               cpu_info->caps |= CPUPOWER_CAP_AMD_CPB;
+
+                       if ((cpuid_edx(0x80000007) & (1 << 7)) &&
+                           cpu_info->family != 0x14)
+                               /* HW pstate was not implemented in family 0x14 
*/
+                               cpu_info->caps |= CPUPOWER_CAP_AMD_HW_PSTATE;
+               }

if (ext_cpuid_level >= 0x80000008 &&

                    cpuid_ebx(0x80000008) & (1 << 4))
diff --git a/tools/power/cpupower/utils/helpers/helpers.h 
b/tools/power/cpupower/utils/helpers/helpers.h
index a84f85a9dbd2..5f61eefff5b2 100644
--- a/tools/power/cpupower/utils/helpers/helpers.h
+++ b/tools/power/cpupower/utils/helpers/helpers.h
@@ -70,6 +70,7 @@ enum cpupower_cpu_vendor {X86_VENDOR_UNKNOWN = 0, 
X86_VENDOR_INTEL,
  #define CPUPOWER_CAP_IS_SNB           0x00000020
  #define CPUPOWER_CAP_INTEL_IDA                0x00000040
  #define CPUPOWER_CAP_AMD_RDPRU                0x00000080
+#define CPUPOWER_CAP_AMD_HW_PSTATE     0x00000100

#define CPUPOWER_AMD_CPBDIS 0x02000000

thanks,
-- Shuah