Re: [PATCH V2 2/2] tools/perf/powerpc/util: Add support to handle compatible mode PVR for perf json events

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Hi,

Le 10/10/2024 à 16:51, Athira Rajeev a écrit :
> perf list picks the events supported for specific platform
> from pmu-events/arch/powerpc/<platform>. Example power10 events
> are in pmu-events/arch/powerpc/power10, power9 events are part
> of pmu-events/arch/powerpc/power9. The decision of which
> platform to pick is determined based on PVR value in powerpc.
> The PVR value is matched from pmu-events/arch/powerpc/mapfile.csv
> 
> Example:
> 
> Format:
> 	PVR,Version,JSON/file/pathname,Type
> 
> 0x004[bcd][[:xdigit:]]{4},1,power8,core
> 0x0066[[:xdigit:]]{4},1,power8,core
> 0x004e[[:xdigit:]]{4},1,power9,core
> 0x0080[[:xdigit:]]{4},1,power10,core
> 0x0082[[:xdigit:]]{4},1,power10,core
> 
> The code gets the PVR from system using get_cpuid_str function
> in arch/powerpc/util/headers.c ( from SPRN_PVR ) and compares
> with value from mapfile.csv
> In case of compat mode, say when partition is booted in a power9
> mode when the system is a power10, this picks incorrectly. Because
> PVR will point to power10 where as it should pick events from power9
> folder. To support generic events, add new folder
> pmu-events/arch/powerpc/compat to contain the ISA architected events
> which is supported in compat mode. Also return 0x00ffffff as pvr
> when booted in compat mode. Based on this pvr value, json will
> pick events from pmu-events/arch/powerpc/compat
> 
> Suggested-by: Madhavan Srinivasan <maddy@linux.ibm.com>
> Signed-off-by: Athira Rajeev <atrajeev@linux.vnet.ibm.com>

I see this patch was merged into mainline allthough it had CI failures 
and still has.

Could you please fix it ?

arch/powerpc/util/header.c: In function 'is_compat_mode':
Error: arch/powerpc/util/header.c:20:14: error: cast to pointer from 
integer of different size [-Werror=int-to-pointer-cast]
    20 |  if (!strcmp((char *)platform, (char *)base_platform))
       |              ^
Error: arch/powerpc/util/header.c:20:32: error: cast to pointer from 
integer of different size [-Werror=int-to-pointer-cast]
    20 |  if (!strcmp((char *)platform, (char *)base_platform))
       |                                ^
cc1: all warnings being treated as errors
make[6]: *** [/linux/tools/build/Makefile.build:86: 
/output/arch/powerpc/util/header.o] Error 1


The following fix but is maybe not the right one as in reality 
getauxval() seems to return a long not a u64.

diff --git a/tools/perf/arch/powerpc/util/header.c 
b/tools/perf/arch/powerpc/util/header.c
index c7df534dbf8f..1b045d410f31 100644
--- a/tools/perf/arch/powerpc/util/header.c
+++ b/tools/perf/arch/powerpc/util/header.c
@@ -17,7 +17,7 @@ static bool is_compat_mode(void)
  	u64 base_platform = getauxval(AT_BASE_PLATFORM);
  	u64 platform = getauxval(AT_PLATFORM);

-	if (!strcmp((char *)platform, (char *)base_platform))
+	if (!strcmp((char *)(long)platform, (char *)(long)base_platform))
  		return false;

  	return true;


Thanks
Christophe

> ---
> changelog:
> V1 -> V2:
> Corrected commit message and subject line
> 
>   tools/perf/arch/powerpc/util/header.c | 32 ++++++++++++++++++++++++++-
>   1 file changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/arch/powerpc/util/header.c b/tools/perf/arch/powerpc/util/header.c
> index 6b00efd53638..adc82c479443 100644
> --- a/tools/perf/arch/powerpc/util/header.c
> +++ b/tools/perf/arch/powerpc/util/header.c
> @@ -10,6 +10,18 @@
>   #include "utils_header.h"
>   #include "metricgroup.h"
>   #include <api/fs/fs.h>
> +#include <sys/auxv.h>
> +
> +static bool is_compat_mode(void)
> +{
> +	u64 base_platform = getauxval(AT_BASE_PLATFORM);
> +	u64 platform = getauxval(AT_PLATFORM);
> +
> +	if (!strcmp((char *)platform, (char *)base_platform))
> +		return false;
> +
> +	return true;
> +}
>   
>   int
>   get_cpuid(char *buffer, size_t sz)
> @@ -33,8 +45,26 @@ char *
>   get_cpuid_str(struct perf_pmu *pmu __maybe_unused)
>   {
>   	char *bufp;
> +	unsigned long pvr;
> +
> +	/*
> +	 * IBM Power System supports compatible mode. That is
> +	 * Nth generation platform can support previous generation
> +	 * OS in a mode called compatibile mode. For ex. LPAR can be
> +	 * booted in a Power9 mode when the system is a Power10.
> +	 *
> +	 * In the compatible mode, care must be taken when generating
> +	 * PVR value. When read, PVR will be of the AT_BASE_PLATFORM
> +	 * To support generic events, return 0x00ffffff as pvr when
> +	 * booted in compat mode. Based on this pvr value, json will
> +	 * pick events from pmu-events/arch/powerpc/compat
> +	 */
> +	if (!is_compat_mode())
> +		pvr = mfspr(SPRN_PVR);
> +	else
> +		pvr = 0x00ffffff;
>   
> -	if (asprintf(&bufp, "0x%.8lx", mfspr(SPRN_PVR)) < 0)
> +	if (asprintf(&bufp, "0x%.8lx", pvr) < 0)
>   		bufp = NULL;
>   
>   	return bufp;