[PATCH] tools/perf/arch/powerpc: Fix the CPU ID const char* value by adding 0x prefix

[PATCH] tools/perf/arch/powerpc: Fix the CPU ID const char* value by adding 0x prefix

[Athira Rajeev]

Simple expression parser test fails in powerpc as below:

    4: Simple expression parser
    test child forked, pid 170385
    Using CPUID 004e2102
    division by zero
    syntax error
    syntax error
    FAILED tests/expr.c:65 parse test failed
    test child finished with -1
    Simple expression parser: FAILED!

This is observed after commit:
'commit 9d5da30e4ae9 ("perf jevents: Add a new expression builtin strcmp_cpuid_str()")'

With this commit, a new expression builtin strcmp_cpuid_str
got added. This function takes an 'ID' type value, which is
a string. So expression parse for strcmp_cpuid_str expects
const char * as cpuid value type. In case of powerpc, CPU IDs
are numbers. Hence it doesn't get interpreted correctly by
bison parser. Example in case of power9, cpuid string returns
as: 004e2102

cpuid of string type is expected in two cases:
1. char *get_cpuid_str(struct perf_pmu *pmu __maybe_unused);

   Testcase "tests/expr.c" uses "perf_pmu__getcpuid" which calls
   get_cpuid_str to get the cpuid string.

2. cpuid field in  :struct pmu_events_map

   struct pmu_events_map {
           const char *arch;
	   const char *cpuid;

   Here cpuid field is used in "perf_pmu__find_events_table"
   function as "strcmp_cpuid_str(map->cpuid, cpuid)". The
   value for cpuid field is picked from mapfile.csv.

Fix the mapfile.csv and get_cpuid_str function to prefix
cpuid with 0x so that it gets correctly interpreted by
the bison parser

Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
---
 tools/perf/arch/powerpc/util/header.c          | 2 +-
 tools/perf/pmu-events/arch/powerpc/mapfile.csv | 6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/perf/arch/powerpc/util/header.c b/tools/perf/arch/powerpc/util/header.c
index c8d0dc775e5d..6b00efd53638 100644
--- a/tools/perf/arch/powerpc/util/header.c
+++ b/tools/perf/arch/powerpc/util/header.c
@@ -34,7 +34,7 @@ get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
 {
 	char *bufp;
 
-	if (asprintf(&bufp, "%.8lx", mfspr(SPRN_PVR)) < 0)
+	if (asprintf(&bufp, "0x%.8lx", mfspr(SPRN_PVR)) < 0)
 		bufp = NULL;
 
 	return bufp;
diff --git a/tools/perf/pmu-events/arch/powerpc/mapfile.csv b/tools/perf/pmu-events/arch/powerpc/mapfile.csv
index a534ff6db14b..f4908af7ad66 100644
--- a/tools/perf/pmu-events/arch/powerpc/mapfile.csv
+++ b/tools/perf/pmu-events/arch/powerpc/mapfile.csv
@@ -13,6 +13,6 @@
 #
 
 # Power8 entries
-004[bcd][[:xdigit:]]{4},1,power8,core
-004e[[:xdigit:]]{4},1,power9,core
-0080[[:xdigit:]]{4},1,power10,core
+0x004[bcd][[:xdigit:]]{4},1,power8,core
+0x004e[[:xdigit:]]{4},1,power9,core
+0x0080[[:xdigit:]]{4},1,power10,core
-- 
2.39.3

Re: [PATCH] tools/perf/arch/powerpc: Fix the CPU ID const char* value by adding 0x prefix

[Disha Goel]

[-- Attachment #1: Type: text/plain, Size: 3299 bytes --]

On 09/10/23 10:30 am, Athira Rajeev wrote:

> Simple expression parser test fails in powerpc as below:
>
>      4: Simple expression parser
>      test child forked, pid 170385
>      Using CPUID 004e2102
>      division by zero
>      syntax error
>      syntax error
>      FAILED tests/expr.c:65 parse test failed
>      test child finished with -1
>      Simple expression parser: FAILED!
>
> This is observed after commit:
> 'commit 9d5da30e4ae9 ("perf jevents: Add a new expression builtin strcmp_cpuid_str()")'
>
> With this commit, a new expression builtin strcmp_cpuid_str
> got added. This function takes an 'ID' type value, which is
> a string. So expression parse for strcmp_cpuid_str expects
> const char * as cpuid value type. In case of powerpc, CPU IDs
> are numbers. Hence it doesn't get interpreted correctly by
> bison parser. Example in case of power9, cpuid string returns
> as: 004e2102
>
> cpuid of string type is expected in two cases:
> 1. char *get_cpuid_str(struct perf_pmu *pmu __maybe_unused);
>
>     Testcase "tests/expr.c" uses "perf_pmu__getcpuid" which calls
>     get_cpuid_str to get the cpuid string.
>
> 2. cpuid field in  :struct pmu_events_map
>
>     struct pmu_events_map {
>             const char *arch;
> 	   const char *cpuid;
>
>     Here cpuid field is used in "perf_pmu__find_events_table"
>     function as "strcmp_cpuid_str(map->cpuid, cpuid)". The
>     value for cpuid field is picked from mapfile.csv.
>
> Fix the mapfile.csv and get_cpuid_str function to prefix
> cpuid with 0x so that it gets correctly interpreted by
> the bison parser
>
> Signed-off-by: Athira Rajeev<atrajeev@linux.vnet.ibm.com>

Tested the patch on Power10 machine, now its correctly able to interpret cpuid
value and perf Simple expression parser test passed.

# ./perf test -v 7
   7: Simple expression parser                                        :
--- start ---
test child forked, pid 87922
Using CPUID 0x00800200
division by zero
syntax error
test child finished with 0
---- end ----
Simple expression parser: Ok

Tested-by: Disha Goel<disgoel@linux.ibm.com>

> ---
>   tools/perf/arch/powerpc/util/header.c          | 2 +-
>   tools/perf/pmu-events/arch/powerpc/mapfile.csv | 6 +++---
>   2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/arch/powerpc/util/header.c b/tools/perf/arch/powerpc/util/header.c
> index c8d0dc775e5d..6b00efd53638 100644
> --- a/tools/perf/arch/powerpc/util/header.c
> +++ b/tools/perf/arch/powerpc/util/header.c
> @@ -34,7 +34,7 @@ get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
>   {
>   	char *bufp;
>
> -	if (asprintf(&bufp, "%.8lx", mfspr(SPRN_PVR)) < 0)
> +	if (asprintf(&bufp, "0x%.8lx", mfspr(SPRN_PVR)) < 0)
>   		bufp = NULL;
>
>   	return bufp;
> diff --git a/tools/perf/pmu-events/arch/powerpc/mapfile.csv b/tools/perf/pmu-events/arch/powerpc/mapfile.csv
> index a534ff6db14b..f4908af7ad66 100644
> --- a/tools/perf/pmu-events/arch/powerpc/mapfile.csv
> +++ b/tools/perf/pmu-events/arch/powerpc/mapfile.csv
> @@ -13,6 +13,6 @@
>   #
>
>   # Power8 entries
> -004[bcd][[:xdigit:]]{4},1,power8,core
> -004e[[:xdigit:]]{4},1,power9,core
> -0080[[:xdigit:]]{4},1,power10,core
> +0x004[bcd][[:xdigit:]]{4},1,power8,core
> +0x004e[[:xdigit:]]{4},1,power9,core
> +0x0080[[:xdigit:]]{4},1,power10,core

[-- Attachment #2: Type: text/html, Size: 3798 bytes --]

Re: [PATCH] tools/perf/arch/powerpc: Fix the CPU ID const char* value by adding 0x prefix

[Namhyung Kim]

On Mon, 9 Oct 2023 10:30:52 +0530, Athira Rajeev wrote:
> Simple expression parser test fails in powerpc as below:
> 
>     4: Simple expression parser
>     test child forked, pid 170385
>     Using CPUID 004e2102
>     division by zero
>     syntax error
>     syntax error
>     FAILED tests/expr.c:65 parse test failed
>     test child finished with -1
>     Simple expression parser: FAILED!
> 
> [...]

Applied to perf-tools-next, thanks!