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?
also please move this to arch/x86/util/pmu.c
so it matches the weak function object
> +
> + /*
> + * 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?
thanks,
jirka
