On 11/15/2018 8:53 AM, Jiri Olsa wrote:
On Wed, Nov 14, 2018 at 01:24:15PM -0800, kan.li...@linux.intel.com wrote:

SNIP

diff --git a/tools/perf/arch/x86/util/header.c 
b/tools/perf/arch/x86/util/header.c
index fb0d71afee8b..b428a4b00bf7 100644
--- a/tools/perf/arch/x86/util/header.c
+++ b/tools/perf/arch/x86/util/header.c
@@ -4,6 +4,7 @@
  #include <stdio.h>
  #include <stdlib.h>
  #include <string.h>
+#include <regex.h>
#include "../../util/header.h" @@ -70,9 +71,73 @@ get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
  {
        char *buf = malloc(128);
- if (buf && __get_cpuid(buf, 128, "%s-%u-%X$") < 0) {
+       if (buf && __get_cpuid(buf, 128, "%s-%u-%X-%X$") < 0) {
                free(buf);
                return NULL;
        }
        return buf;
  }
+
+/* Full CPUID format for x86 is vendor-family-model-stepping */
+static bool is_full_cpuid(const char *cpuid)
+{
+       const char *tmp = cpuid;
+       int count = 0;
+
+       while ((tmp = strchr(tmp, '-')) != NULL) {
+               count++;
+               tmp++;
+       }
+
+       if (count == 3)
+               return true;
+
+       return false;
+}
+
+int strcmp_cpuid_str(const char *mapcpuid, const char *cpuid)
+{
+       regex_t re;
+       regmatch_t pmatch[1];
+       int match;
+       bool full_mapcpuid = is_full_cpuid(mapcpuid);
+       bool full_cpuid = is_full_cpuid(cpuid);

cpuid will be always full from now right? why do we need to check it?


User may set cpuid by environment string "PERF_CPUID", which may not be full format.

also please move this to arch/x86/util/pmu.c
so it matches the weak function object

Sure.


+
+       /*
+        * Full CPUID format is required to identify a platform.
+        * Error out if the cpuid string is incomplete.
+        */
+       if (full_mapcpuid && !full_cpuid) {
+               pr_info("Invalid CPUID %s. Full CPUID is required, "
+                       "vendor-family-model-stepping\n", cpuid);
+               return 1;
+       }
+
+       if (regcomp(&re, mapcpuid, REG_EXTENDED) != 0) {
+               /* Warn unable to generate match particular string. */
+               pr_info("Invalid regular expression %s\n", mapcpuid);
+               return 1;
+       }
+
+       match = !regexec(&re, cpuid, 1, pmatch, 0);
+       regfree(&re);
+       if (match) {
+               size_t match_len = (pmatch[0].rm_eo - pmatch[0].rm_so);
+               size_t cpuid_len;
+
+               /* If the full CPUID format isn't required,
+                * ignoring the stepping.
+                */
+               if (!full_mapcpuid && full_cpuid)
+                       cpuid_len = strrchr(cpuid, '-') - cpuid;
+               else
+                       cpuid_len = strlen(cpuid);
+
+
+               /* Verify the entire string matched. */
+               if (match_len == cpuid_len)
+                       return 0;

why is this necessary?


It's from previous common code. As my understanding, it just double check the matched strings. There is no harmful. So I keep it.

Thanks,
Kan

Reply via email to