| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <31a3d76b-617b-4c97-b83d-2cd47fabbf02@linux.ibm.com>
Date: Fri, 21 Mar 2025 17:12:01 +0530
From: Madhavan Srinivasan <maddy@...ux.ibm.com>
To: Christophe Leroy <christophe.leroy@...roup.eu>,
Athira Rajeev <atrajeev@...ux.vnet.ibm.com>, namhyung@...nel.org
Cc: linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org, irogers@...gle.com,
akanksha@...ux.ibm.com, kjain@...ux.ibm.com,
disgoel@...ux.vnet.ibm.com, acme@...nel.org, hbathini@...ux.ibm.com,
adrian.hunter@...el.com, jolsa@...nel.org
Subject: Re: [PATCH V2 2/2] tools/perf/powerpc/util: Add support to handle
compatible mode PVR for perf json events
On 3/21/25 4:04 PM, Christophe Leroy wrote:
> 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@...ux.ibm.com>
>> Signed-off-by: Athira Rajeev <atrajeev@...ux.vnet.ibm.com>
>
> I see this patch was merged into mainline allthough it had CI failures and still has.
>
> Could you please fix it ?
We already have a fix patch posted for this issue
https://patchwork.ozlabs.org/project/linuxppc-dev/patch/20250321100726.699956-1-likhitha@linux.ibm.com/
Maddy
>
> 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;
>
Powered by blists - more mailing lists