perf vendor events: Add stepping in CPUID string for x86

From: Kan Liang <[email protected]>

Add JSON metrics (based on event list v1) for Cascadelake server

Quoted 16 lines · Jiri Olsa

Quoted 3 lines · On Wed, Nov 14, 2018 at 01:24:16PM -0800, [email protected] wrote:

From: Kan Liang <[email protected]>

Add JSON metrics (based on event list v1) for Cascadelake server

hum mail server probably crippled the email so it won't apply:

patching file pmu-events/arch/x86/cascadelakex/cache.json
patching file pmu-events/arch/x86/cascadelakex/clx-metrics.json
patching file pmu-events/arch/x86/cascadelakex/floating-point.json
patching file pmu-events/arch/x86/cascadelakex/frontend.json
patching file pmu-events/arch/x86/cascadelakex/memory.json
patching file pmu-events/arch/x86/cascadelakex/other.json
patching file pmu-events/arch/x86/cascadelakex/pipeline.json
patch: **** malformed patch at line 29879: ow status bit and resets the counter to less than MAX. The reset value to the counter is not clocked immediately so the overflow status bit will flip 'high (1)' and generate another PMI (if enabled) after which the reset value gets clocked into the counter. Therefore, software will get the interrupt, read the overflow status bit '1 for bit 34 while the counter value is less than MAX. Software should ignore this case.",

Quoted 4 lines

I've similar error before and it was because of the 998 chars size limit for line.. I guess you could either post it as attachment or change the transfer encoding

The patch is attached. Can it be applied?

Quoted 19 lines · Jiri Olsa

Quoted 16 lines · On Wed, Nov 14, 2018 at 01:24:16PM -0800, [email protected] wrote:

Quoted 3 lines

From: Kan Liang <[email protected]>

Add JSON metrics (based on event list v1) for Cascadelake server

hum mail server probably crippled the email so it won't apply:

patching file pmu-events/arch/x86/cascadelakex/cache.json
patching file pmu-events/arch/x86/cascadelakex/clx-metrics.json
patching file pmu-events/arch/x86/cascadelakex/floating-point.json
patching file pmu-events/arch/x86/cascadelakex/frontend.json
patching file pmu-events/arch/x86/cascadelakex/memory.json
patching file pmu-events/arch/x86/cascadelakex/other.json
patching file pmu-events/arch/x86/cascadelakex/pipeline.json
patch: **** malformed patch at line 29879: ow status bit and resets the counter to less than MAX. The reset value to the counter is not clocked immediately so the overflow status bit will flip 'high (1)' and generate another PMI (if enabled) after which the reset value gets clocked into the counter. Therefore, software will get the interrupt, read the overflow status bit '1 for bit 34 while the counter value is less than MAX. Software should ignore this case.",

Quoted 6 lines

Quoted 4 lines

I've similar error before and it was because of the 998 chars size limit for line.. I guess you could either post it as attachment or change the transfer encoding

The patch is attached. Can it be applied?

yep, that one works

Em Thu, Nov 15, 2018 at 04:14:42PM +0100, Jiri Olsa escreveu:

Quoted 25 lines · Kan

Quoted 24 lines · Jiri Olsa

Quoted 20 lines · On Wed, Nov 14, 2018 at 01:24:16PM -0800, [email protected] wrote:

Quoted 3 lines

From: Kan Liang <[email protected]>

Add JSON metrics (based on event list v1) for Cascadelake server

hum mail server probably crippled the email so it won't apply:

patching file pmu-events/arch/x86/cascadelakex/cache.json
patching file pmu-events/arch/x86/cascadelakex/clx-metrics.json
patching file pmu-events/arch/x86/cascadelakex/floating-point.json
patching file pmu-events/arch/x86/cascadelakex/frontend.json
patching file pmu-events/arch/x86/cascadelakex/memory.json
patching file pmu-events/arch/x86/cascadelakex/other.json
patching file pmu-events/arch/x86/cascadelakex/pipeline.json
patch: **** malformed patch at line 29879: ow status bit and resets the counter to less than MAX. The reset value to the counter is not clocked immediately so the overflow status bit will flip 'high (1)' and generate another PMI (if enabled) after which the reset value gets clocked into the counter. Therefore, software will get the interrupt, read the overflow status bit '1 for bit 34 while the counter value is less than MAX. Software should ignore this case.",

I've similar error before and it was because of the 998 chars size limit for line.. I guess you could either post it as attachment or change the transfer encoding

Quoted 3 lines

Quoted 1 line

The patch is attached. Can it be applied?

yep, that one works

Can I have your reviewed-by for this one as well?

Em Wed, Nov 21, 2018 at 09:51:30AM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 26 lines

Em Thu, Nov 15, 2018 at 04:14:42PM +0100, Jiri Olsa escreveu:

Quoted 25 lines · Kan

Quoted 24 lines · Jiri Olsa

Quoted 20 lines · On Wed, Nov 14, 2018 at 01:24:16PM -0800, [email protected] wrote:

Quoted 3 lines

From: Kan Liang <[email protected]>

Add JSON metrics (based on event list v1) for Cascadelake server

hum mail server probably crippled the email so it won't apply:

patching file pmu-events/arch/x86/cascadelakex/cache.json
patching file pmu-events/arch/x86/cascadelakex/clx-metrics.json
patching file pmu-events/arch/x86/cascadelakex/floating-point.json
patching file pmu-events/arch/x86/cascadelakex/frontend.json
patching file pmu-events/arch/x86/cascadelakex/memory.json
patching file pmu-events/arch/x86/cascadelakex/other.json
patching file pmu-events/arch/x86/cascadelakex/pipeline.json
patch: **** malformed patch at line 29879: ow status bit and resets the counter to less than MAX. The reset value to the counter is not clocked immediately so the overflow status bit will flip 'high (1)' and generate another PMI (if enabled) after which the reset value gets clocked into the counter. Therefore, software will get the interrupt, read the overflow status bit '1 for bit 34 while the counter value is less than MAX. Software should ignore this case.",

I've similar error before and it was because of the 998 chars size limit for line.. I guess you could either post it as attachment or change the transfer encoding

Quoted 5 lines

Quoted 3 lines

Quoted 1 line

The patch is attached. Can it be applied?

yep, that one works

Can I have your reviewed-by for this one as well?

I turned the "yep, that one works" into an Acked-by, please let me know if that is enough.

Quoted 4 lines

Em Wed, Nov 21, 2018 at 09:51:30AM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 3 lines

Em Thu, Nov 15, 2018 at 04:14:42PM +0100, Jiri Olsa escreveu:

Quoted 2 lines · Kan

Quoted 1 line

The patch is attached. Can it be applied?

Quoted 6 lines

Quoted 3 lines

Quoted 1 line

yep, that one works

Can I have your reviewed-by for this one as well?

I turned the "yep, that one works" into an Acked-by, please let me know if that is enough.

yes, if it applies it's ok for me.. but can't judge the events inside ;-)

tools/perf/arch/x86/util/header.c +17 -1

index fb0d71afee8b..b428a4b00bf7 100644
@@ -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;
+

tools/perf/arch/x86/util/header.c +25 -0

+
+	/*
+	 * 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.

On Wed, Nov 14, 2018 at 01:24:15PM -0800, [email protected] wrote:

SNIP

Quoted 42 lines

tools/perf/arch/x86/util/header.c +21 -1

index fb0d71afee8b..b428a4b00bf7 100644
@@ -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)

tools/perf/arch/x86/util/header.c get_cpuid_str(struct perf_pmu *pmu __maybe_unused) · line 104

Quoted 6 lines

tools/perf/arch/x86/util/header.c +6 -0

+{
+	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?

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

Quoted 30 lines

tools/perf/arch/x86/util/header.c +29 -0

+
+	/*
+	 * 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

tools/perf/arch/x86/util/header.c get_cpuid_str(struct perf_pmu *pmu __maybe_unused) · line 139

Quoted 6 lines

tools/perf/arch/x86/util/header.c +6 -0

+			cpuid_len = strlen(cpuid);
+
+
+		/* Verify the entire string matched. */
+		if (match_len == cpuid_len)
+			return 0;

why is this necessary?

Quoted 43 lines · Jiri Olsa

On Wed, Nov 14, 2018 at 01:24:15PM -0800, [email protected] wrote:

SNIP

Quoted 39 lines

tools/perf/arch/x86/util/header.c +21 -1

index fb0d71afee8b..b428a4b00bf7 100644
@@ -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)

Quoted 9 lines

tools/perf/arch/x86/util/header.c get_cpuid_str(struct perf_pmu *pmu __maybe_unused) · line 104

Quoted 6 lines

tools/perf/arch/x86/util/header.c +6 -0

+{
+    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.

Quoted 2 lines

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

Sure.

Quoted 11 lines

Quoted 10 lines

tools/perf/arch/x86/util/header.c get_cpuid_str(struct perf_pmu *pmu __maybe_unused) · line 104

Quoted 7 lines

tools/perf/arch/x86/util/header.c +7 -0

+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?

Quoted 8 lines

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

Quoted 2 lines

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

Sure.

It looks like both pmu.c and header.c have cpuid related functions. For example,
get_cpuid_str() has weak function in pmu.c. It's declaration in header.h. It's x86 specific function in x86/util/header.c
get_cpuid() has weak function in header.c. It's declaration in header.h. It's x86 specific function in x86/util/header.c

It looks like most of the cpuid related functions are in header.c/h. I think it may be better to move all the cpuid related functions to header.c/h.

If it's OK for you, I will send a clean up patch later to move the weak functions strcmp_cpuid_str() and get_cpuid_str() to header.c

What do you think?

Quoted 43 lines · Jiri Olsa

On Wed, Nov 14, 2018 at 01:24:15PM -0800, [email protected] wrote:

SNIP

Quoted 39 lines

tools/perf/arch/x86/util/header.c +21 -1

index fb0d71afee8b..b428a4b00bf7 100644
@@ -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)

Quoted 9 lines

tools/perf/arch/x86/util/header.c get_cpuid_str(struct perf_pmu *pmu __maybe_unused) · line 104

Quoted 6 lines

tools/perf/arch/x86/util/header.c +6 -0

+{
+	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.

Quoted 3 lines

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

Sure.

Quoted 30 lines

Quoted 30 lines

tools/perf/arch/x86/util/header.c +29 -0

+
+	/*
+	 * 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

Quoted 9 lines

tools/perf/arch/x86/util/header.c get_cpuid_str(struct perf_pmu *pmu __maybe_unused) · line 139

Quoted 6 lines

tools/perf/arch/x86/util/header.c +6 -0

+			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.

Quoted 39 lines

Quoted 29 lines

Quoted 29 lines

tools/perf/arch/x86/util/header.c +28 -0

+	/*
+	 * 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

Quoted 9 lines

tools/perf/arch/x86/util/header.c get_cpuid_str(struct perf_pmu *pmu __maybe_unused) · line 139

Quoted 6 lines

tools/perf/arch/x86/util/header.c +6 -0

+			cpuid_len = strlen(cpuid);
+
+
+		/* Verify the entire string matched. */
+		if (match_len == cpuid_len)
+			return 0;

why is this necessary?

Quoted 2 lines

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

right.. did you consider using the wildcard in the map file so it'd cover the stepping, having entries like:

   GenuineIntel-6-1F-*,v2,nehalemep,core

I haven't thought this one through, but seems we could bypass those '-stepping' checks.. but probably other changes would be necessary for the wildcard

Quoted 42 lines

Quoted 39 lines

Quoted 29 lines

Quoted 29 lines

tools/perf/arch/x86/util/header.c +28 -0

+	/*
+	 * 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

Quoted 9 lines

tools/perf/arch/x86/util/header.c get_cpuid_str(struct perf_pmu *pmu __maybe_unused) · line 139

Quoted 6 lines

tools/perf/arch/x86/util/header.c +6 -0

+			cpuid_len = strlen(cpuid);
+
+
+		/* Verify the entire string matched. */
+		if (match_len == cpuid_len)
+			return 0;

why is this necessary?

Quoted 2 lines

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

Quoted 9 lines

right.. did you consider using the wildcard in the map file so it'd cover the stepping, having entries like:

   GenuineIntel-6-1F-*,v2,nehalemep,core

I haven't thought this one through, but seems we could bypass those '-stepping' checks.. but probably other changes would be necessary for the wildcard

If using wildcard, the full cpuid format is required for all platforms. The script which using environment string "PERF_CPUID" will definitely be broken on all platforms.
This patch (non-wildcard) only breaks the script on Skylake server and Cascadelake server.

I think that's the only drawback of using wildcard.

From: Kan Liang <[email protected]>

Perf tools cannot find the proper event list for Cascadelake server. Because Cascadelake server and Skylake server have the same CPU model number, which are used by perf tools to find the event list.

The stepping for Skylake server is up to 4. The stepping for Cascadelake server starts from 5. The stepping can be used to distinguish between them.

The stepping is added in get_cpuid_str().
The stepping information for Skylake server is updated in mapfile.csv. A x86 specific strcmp_cpuid_cmp() function is added to handle two CPUID formats in mapfile.csv, "vendor-family-model-stepping" and "vendor-family-model".
- If a cpuid-regular-expression from the mapfile.csv using the new stepping format, a cpuid-string generated on the machine must include stepping. Otherwise, it is a mismatch.
- If the cpuid-regular-expression using the old non-stepping format, the stepping in the cpuid-string will be ignored.

The script, which using environment string "PERF_CPUID" without stepping on Skylake server, will be broken. If so, users must fix their scripts.

Signed-off-by Kan Liang <[email protected]>

Quoted 21 lines · On Wed, Nov 14, 2018 at 01:24:15PM -0800, [email protected] wrote:

From: Kan Liang <[email protected]>

Perf tools cannot find the proper event list for Cascadelake server. Because Cascadelake server and Skylake server have the same CPU model number, which are used by perf tools to find the event list.

The stepping for Skylake server is up to 4. The stepping for Cascadelake server starts from 5. The stepping can be used to distinguish between them.

The stepping is added in get_cpuid_str().
The stepping information for Skylake server is updated in mapfile.csv. A x86 specific strcmp_cpuid_cmp() function is added to handle two CPUID formats in mapfile.csv, "vendor-family-model-stepping" and "vendor-family-model".
- If a cpuid-regular-expression from the mapfile.csv using the new stepping format, a cpuid-string generated on the machine must include stepping. Otherwise, it is a mismatch.
- If the cpuid-regular-expression using the old non-stepping format, the stepping in the cpuid-string will be ignored.

Quoted 4 lines

The script, which using environment string "PERF_CPUID" without stepping on Skylake server, will be broken. If so, users must fix their scripts.

Signed-off-by Kan Liang <[email protected]>

Reviewed-by Jiri Olsa <[email protected]>

Quoted 22 lines · On Wed, Nov 14, 2018 at 01:24:15PM -0800, [email protected] wrote:

Quoted 21 lines

From: Kan Liang <[email protected]>

Perf tools cannot find the proper event list for Cascadelake server. Because Cascadelake server and Skylake server have the same CPU model number, which are used by perf tools to find the event list.

The stepping for Skylake server is up to 4. The stepping for Cascadelake server starts from 5. The stepping can be used to distinguish between them.

The stepping is added in get_cpuid_str().
The stepping information for Skylake server is updated in mapfile.csv. A x86 specific strcmp_cpuid_cmp() function is added to handle two CPUID formats in mapfile.csv, "vendor-family-model-stepping" and "vendor-family-model".
- If a cpuid-regular-expression from the mapfile.csv using the new stepping format, a cpuid-string generated on the machine must include stepping. Otherwise, it is a mismatch.
- If the cpuid-regular-expression using the old non-stepping format, the stepping in the cpuid-string will be ignored.

Quoted 7 lines

Quoted 4 lines

The script, which using environment string "PERF_CPUID" without stepping on Skylake server, will be broken. If so, users must fix their scripts.

Signed-off-by Kan Liang <[email protected]>

Reviewed-by Jiri Olsa <[email protected]>

Thanks Jirka,

Hi Arnaldo,

Are you OK with the patch?
If yes, I will go ahead to cleanup the *_cpuid_str() by moving them to header.c as promised. https://lkml.org/lkml/2018/11/15/929
The new patch will be on top of this patch.

Em Thu, Nov 15, 2018 at 04:01:46PM -0500, Liang, Kan escreveu:

Quoted 37 lines · Jiri Olsa

Quoted 22 lines · On Wed, Nov 14, 2018 at 01:24:15PM -0800, [email protected] wrote:

Quoted 21 lines

From: Kan Liang <[email protected]>

Perf tools cannot find the proper event list for Cascadelake server. Because Cascadelake server and Skylake server have the same CPU model number, which are used by perf tools to find the event list.

The stepping for Skylake server is up to 4. The stepping for Cascadelake server starts from 5. The stepping can be used to distinguish between them.

The stepping is added in get_cpuid_str().
The stepping information for Skylake server is updated in mapfile.csv. A x86 specific strcmp_cpuid_cmp() function is added to handle two CPUID formats in mapfile.csv, "vendor-family-model-stepping" and "vendor-family-model".
- If a cpuid-regular-expression from the mapfile.csv using the new stepping format, a cpuid-string generated on the machine must include stepping. Otherwise, it is a mismatch.
- If the cpuid-regular-expression using the old non-stepping format, the stepping in the cpuid-string will be ignored.

Quoted 7 lines

Quoted 4 lines

The script, which using environment string "PERF_CPUID" without stepping on Skylake server, will be broken. If so, users must fix their scripts.

Signed-off-by Kan Liang <[email protected]>

Reviewed-by Jiri Olsa <[email protected]>

Thanks Jirka,

Hi Arnaldo,

Quoted 4 lines

Are you OK with the patch?
If yes, I will go ahead to cleanup the *_cpuid_str() by moving them to header.c as promised. https://lkml.org/lkml/2018/11/15/929
The new patch will be on top of this patch.

I'm travelling, will look at it soon, can't now, battery almost deead :-\

Quoted 22 lines · On Wed, Nov 14, 2018 at 01:24:15PM -0800, [email protected] wrote:

Quoted 21 lines

From: Kan Liang <[email protected]>

Perf tools cannot find the proper event list for Cascadelake server. Because Cascadelake server and Skylake server have the same CPU model number, which are used by perf tools to find the event list.

The stepping for Skylake server is up to 4. The stepping for Cascadelake server starts from 5. The stepping can be used to distinguish between them.

The stepping is added in get_cpuid_str().
The stepping information for Skylake server is updated in mapfile.csv. A x86 specific strcmp_cpuid_cmp() function is added to handle two CPUID formats in mapfile.csv, "vendor-family-model-stepping" and "vendor-family-model".
- If a cpuid-regular-expression from the mapfile.csv using the new stepping format, a cpuid-string generated on the machine must include stepping. Otherwise, it is a mismatch.
- If the cpuid-regular-expression using the old non-stepping format, the stepping in the cpuid-string will be ignored.

Quoted 6 lines

Quoted 4 lines

The script, which using environment string "PERF_CPUID" without stepping on Skylake server, will be broken. If so, users must fix their scripts.

Signed-off-by Kan Liang <[email protected]>

Reviewed-by Jiri Olsa <[email protected]>

Thanks Jirka,

Hi Arnaldo,

Are you OK with the patch?
If yes, I will go ahead to cleanup the *_cpuid_str() by moving them to header.c as promised. https://lkml.org/lkml/2018/11/15/929
The new patch will be on top of this patch.