Add CPU frequency detection from CPUID vendor string by MegaMaddin · Pull Request #121 · KernelTestFramework/ktf

MegaMaddin · 2020-10-22T08:33:45Z

Issue #, if available: #80

Description of changes:

This commit series is adding a cpu frequency detection by parsing the specific bits out of the CPUID vendor string.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Testing done:

running UNITTEST=1 make clean boot on the branch results in executing unit tests

Got CPU string 'Intel(R) Xeon(R) CPU E3-1270 V2 @ 3.50GHz' and frequency '3500000000'
Got CPU string 'Intel(R) Celeron(R) CPU J1900 @ 1.99GHz' and frequency '1990000000'
Got CPU string 'AMD G-T48L Processor' and frequency '0'
Got CPU string 'Intel(R) Atom(TM) CPU E3815 @ 1.46GHz' and frequency '1460000000'
Got CPU string 'Intel(R) Atom(TM) CPU E620 @ 600MHz' and frequency '600000000'
Got CPU string 'VIA C7 Processor 1000MHz' and frequency '1000000000'
Got CPU string 'Intel(R) Core(TM) i7 CPU 950 @ 3.07GHz' and frequency '3070000000'
Got CPU string 'AMD Ryzen Threadripper 1950X 16-Core Processor' and frequency '0'
Got CPU string 'Prototyp Amazing Foo One @ 1GHz' and frequency '1000000000'
Got CPU string 'Prototyp Amazing Foo Two @ 1.00GHz' and frequency '1000000000'

wipawel · 2020-10-22T08:45:00Z

+#define MHZ_MULTIPLIER 1000000
+#define GHZ_MULTIPLIER 1000000000
+
+static inline uint64_t get_cpu_freq(const char *cpu_str) {


I would make it regular function (no inline).

Did that, had to use it somewhere afterwards :)

wipawel · 2020-10-22T08:46:29Z

+#define MHZ_MULTIPLIER 1000000
+#define GHZ_MULTIPLIER 1000000000


it might make sense to add common defines for HZ() KHZ() MHZ() and GHZ(), similar to the ones for KB(), MB(), and GB()

wipawel · 2020-10-22T08:49:29Z

+
+    /* we need to reverse the vendor string for parsing the freq */
+    while (len--) {
+        if (isspace(*(cpu_str + len))) {


nit: cpu_str[len]

wipawel · 2020-10-22T08:50:11Z

+            *(reverse) = '\0';
+            break;
+        }
+        *(reverse++) = *(cpu_str + len);


nit: *reverse++ = cpu_str[len]

wipawel · 2020-10-22T08:51:00Z

+    uint64_t frequency = 0;
+    char buf[16];
+    char buf2[16];
+    char *reverse = &buf[0];


nit: reverse can be declared in the while() {} loop scope using it

wipawel · 2020-10-22T08:51:18Z

+    }
+
+    if (!strstr(buf, "zHM") && !strstr(buf, "zHG")) {
+        // if we have nothing to calculate, just return


nit: no C++ comments pls

also this if seems redundant. We call the same strstr() in the below if-else if-else. We can make return 0 the statement of the final else

wipawel · 2020-10-22T08:55:07Z

+    size_t len = strlen(cpu_str);
+    uint64_t multiplier = 0;
+    uint64_t frequency = 0;
+    char buf[16];


nit: I would call it reverse and the buf2 result

wipawel · 2020-10-22T09:00:06Z

+            if (strlen(buf2) == 2)
+                frequency += strtoul(buf2, NULL, 0) * 10 * multiplier;
+            else if (strlen(buf2) == 1)
+                frequency += strtoul(buf2, NULL, 0) * 100 * multiplier;


this could be: frequency += strtoul(buf2, NULL, 0) * (1000 / (10 * strlen(buf2))) * multiplier

wipawel · 2020-10-22T18:21:35Z

-    if (cpu_vendor_string(&cpu_identifier[0]))
+    if (cpu_vendor_string(&cpu_identifier[0])) {
        printk("CPU: %.48s\n", cpu_identifier);
+        printk("Frequency: %llu Hz\n", get_cpu_freq(&cpu_identifier[0]));


nit: I would prefer MHz and %lu formatter should be ok

wipawel · 2020-10-22T18:22:13Z

 #define CPUID_BRAND_INFO_MIN 0x80000002U
 #define CPUID_BRAND_INFO_MAX 0x80000004U

+#define KHZ(x) (_U64(x) * ipow(10, 3))


nit: this and below I would move to compiler.h

wipawel · 2020-10-22T18:26:12Z

+            }
+        }
+    }
+    else {


nit: since frequency is set to 0 by default, this is redundant

wipawel · 2020-10-22T18:28:05Z

 }

+static inline unsigned int ipow(int base, int exp) {
+    int result = 1;


This should be unsigned long

wipawel · 2020-10-22T18:28:20Z

    return n;
 }

+static inline unsigned int ipow(int base, int exp) {


The function should return unsigned long

wipawel · 2020-10-22T18:30:49Z


+static inline unsigned int ipow(int base, int exp) {
+    int result = 1;
+    for (;;) {


pow(nit, 10): while(true)

Signed-off-by: Martin Mazein <amazein@amazon.de>

wipawel · 2020-10-23T07:31:34Z


    /* Print cpu vendor info */
-    if (cpu_vendor_string(&cpu_identifier[0]))
+    if (cpu_vendor_string(&cpu_identifier[0])) {


nit: here and below: s/&cpu_identifier[0]/cpu_identifier/

wipawel · 2020-10-23T07:32:19Z

 #define GB(x) (_U64(x) << 30)

+#define KHZ(x) (_U64(x) * 1000)
+#define MHZ(x) (_U64(x) * 1000000)


nit: MHZ(x) (KHZ(x) * 1000)

wipawel · 2020-10-23T07:36:15Z

    return n;
 }

+static inline unsigned long ipow(int base, int exp) {


s/int exp/unsigned int exp/

Signed-off-by: Martin Mazein <amazein@amazon.de>

… string Signed-off-by: Martin Mazein <amazein@amazon.de>

Signed-off-by: Martin Mazein <amazein@amazon.de>

MegaMaddin requested a review from wipawel October 22, 2020 08:33

MegaMaddin linked an issue Oct 22, 2020 that may be closed by this pull request

[Feature] add CPUID processor brand string handling and parsing #80

Closed

wipawel reviewed Oct 22, 2020

View reviewed changes

wipawel added the Priority: 3 Regular feature label Oct 22, 2020

wipawel added this to the v0.3 milestone Oct 22, 2020

MegaMaddin force-pushed the cpufreq branch 5 times, most recently from 65b6dd4 to 695e61d Compare October 22, 2020 16:38

wipawel reviewed Oct 22, 2020

View reviewed changes

MegaMaddin force-pushed the cpufreq branch from 01fe400 to 068ace4 Compare October 23, 2020 06:29

string: add function for detecting point punctation

776ffed

Signed-off-by: Martin Mazein <amazein@amazon.de>

MegaMaddin force-pushed the cpufreq branch from 068ace4 to 131f422 Compare October 23, 2020 06:30

wipawel reviewed Oct 23, 2020

View reviewed changes

lib: add ipow() function to calculate n power of m with integers

a464bc6

Signed-off-by: Martin Mazein <amazein@amazon.de>

MegaMaddin force-pushed the cpufreq branch from 131f422 to 6816de8 Compare October 23, 2020 07:58

MegaMaddin added 2 commits October 23, 2020 10:34

cpuid: add function for detecting cpu frequency from the CPUID vendor…

ac22b73

… string Signed-off-by: Martin Mazein <amazein@amazon.de>

tests: add unittest for cpu frequency detection

ffb77e0

Signed-off-by: Martin Mazein <amazein@amazon.de>

MegaMaddin force-pushed the cpufreq branch from 6816de8 to ffb77e0 Compare October 23, 2020 08:35

wipawel approved these changes Oct 23, 2020

View reviewed changes

MegaMaddin merged commit 6cf4ea8 into KernelTestFramework:mainline Oct 23, 2020

MegaMaddin deleted the cpufreq branch December 1, 2020 08:06

		#define MHZ_MULTIPLIER 1000000
		#define GHZ_MULTIPLIER 1000000000

Conversation

MegaMaddin commented Oct 22, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants