[PATCH v2] lparstat: print memory mode correctly

[Shrikanth Hegde]

Starting from power10, active memory sharing(AMS) is not supported.
So from power10 onwards the H_GET_MPP hcall fails and hence
corresponding fields in lparcfg are not populated, such as
entitled_memory_pool_number etc.

Use gcc builtins and print memory model as dedicated for power10
onwards.

Suggested-by: Peter Bergner <ber...@linux.ibm.com>
Signed-off-by: Shrikanth Hegde <ssh...@linux.ibm.com>
---
v1->v2:

v1: https://groups.google.com/g/powerpc-utils-devel/c/ASm9ihz_XPY
Use powerpc and built in support guards to fix build breaks in other
archs.

src/common/cpu_info_helpers.h | 6 ++++++
src/lparstat.c | 6 +++++-
2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/src/common/cpu_info_helpers.h b/src/common/cpu_info_helpers.h
index 77e6ad7..5cbe7de 100644
--- a/src/common/cpu_info_helpers.h
+++ b/src/common/cpu_info_helpers.h
@@ -47,4 +47,10 @@ extern int __get_one_smt_state(int core, int threads_per_cpu);
extern int __do_smt(bool numeric, int cpus_in_system, int threads_per_cpu,
bool print_smt_state);

+#if defined (__powerpc__) && defined (__BUILTIN_CPU_SUPPORTS__)
+# define BUILTIN_CPU_SUPPORTS(X) __builtin_cpu_supports(X)
+#else
+# define BUILTIN_CPU_SUPPORTS(X) 0
+#endif
+
#endif /* CPU_INFO_H */
diff --git a/src/lparstat.c b/src/lparstat.c
index db22316..7e1638c 100644
--- a/src/lparstat.c
+++ b/src/lparstat.c
@@ -794,7 +794,11 @@ void get_memory_mode(struct sysentry *se, char *buf)
struct sysentry *tmp;

tmp = get_sysentry("entitled_memory_pool_number");
- if (atoi(tmp->value) == 65535)
+ /*
+ * from power10 onwards Active Memory Sharing(AMS) is not
+ * supported. Hence always display it as dedicated for those
+ */
+ if (atoi(tmp->value) == 65535 || __builtin_cpu_supports("arch_3_1"))
sprintf(buf, "Dedicated");
else
sprintf(buf, "Shared");
--
2.39.3

[Tyrel Datwyler]

This needed to be updated to call our new internally defined wrapper
BUILTIN_CPU_SUPPORTS(). I'll go ahead and fix that up when I apply this to the
next branch.

-Tyrel

[Tyrel Datwyler]

On 3/2/25 8:06 PM, Shrikanth Hegde wrote:

> Starting from power10, active memory sharing(AMS) is not supported.
> So from power10 onwards the H_GET_MPP hcall fails and hence
> corresponding fields in lparcfg are not populated, such as
> entitled_memory_pool_number etc.
>
> Use gcc builtins and print memory model as dedicated for power10
> onwards.
>
> Suggested-by: Peter Bergner <ber...@linux.ibm.com>
> Signed-off-by: Shrikanth Hegde <ssh...@linux.ibm.com>
> ---

Patch appled to powerpc-utils/next branch.

https://github.com/ibm-power-utilities/powerpc-utils/commit/b6f50dc565eea17ce35389555489e9d8da3be9f3

Thanks,
-Tyrel

[Shrikanth Hegde]

Thanks Tyrel.

Sorry for the silly mistake from my end. Thanks for noticing it and
fixing it up.

> Thanks,
> -Tyrel

[Michal Suchánek]

Hello,

This does not work on distributions using older gcc versions such as
gcc 7.

On these distributions the 3.1 platform is as unknown as on x86 and the
code fails to build.

Very old versions of gcc (eg. 4.8) do not check the flag validity at
all. Then the code builds but is likely ineffective.

Not sure this is a good sultion in general.

Thanks

Michal

[Peter Bergner]

On 3/4/25 4:45 AM, Michal Suchánek wrote:
> This does not work on distributions using older gcc versions such as
> gcc 7.
>
> On these distributions the 3.1 platform is as unknown as on x86 and the
> code fails to build.
>
> Very old versions of gcc (eg. 4.8) do not check the flag validity at
> all. Then the code builds but is likely ineffective.

That should be fixable by adding a test for __GNUC__ and possibly
__GNUC_MINOR__ to guard the case of an old compiler that knows about
__builtin_cpu_supports, but not arch_3_1. Let me hunt through when
we added arch_3_1 and I'll find the right magic needed.

Peter

[Michal Suchánek]

That will fail the POWER10 detection, however. It will be compiled out,
and the bug that this aims to fix remain.

Thanks

Michal

[Peter Bergner]

How about the following then? PPC_FEATURE2_ARCH_3_1 is defined in the
kernel's asm/cpu_table.h and libc's bits/hwcap.h header files. If you don't
want to or can't include those, you can add the following to define it:

#define PPC_FEATURE2_ARCH_3_1 0x00040000


Peter


diff --git a/src/lparstat.c b/src/lparstat.c
index db22316..df5d0ab 100644
--- a/src/lparstat.c
+++ b/src/lparstat.c
@@ -794,7 +794,12 @@ void get_memory_mode(struct sysentry *se, char *buf)

struct sysentry *tmp;

tmp = get_sysentry("entitled_memory_pool_number");
- if (atoi(tmp->value) == 65535)
+ /*
+ * from power10 onwards Active Memory Sharing(AMS) is not
+ * supported. Hence always display it as dedicated for those
+ */
+ if (atoi(tmp->value) == 65535

+ || getauxval (AT_HWCAP2) & PPC_FEATURE2_ARCH_3_1)

[Shrikanth Hegde]

This works for me as well on power10. I included <sys/auxv.h>

[Michal Suchánek]

Hello,

Yes, this looks fine. Looking through distributions that are expected to
run on Power10 I see that the auxv definition is available when
supported natively, and not available when compat mode is expected.

There may be some autliers. This should be generally fine with some
ifdef to avoid build failure on old OS releases.

Thanks

Michal

[Tyrel Datwyler]

Just wanted to confirm that we have a consensus on moving forward with this
patch? If so I will go ahead an revert the previously applied patch from Shrikanth.

-Tyrel

[Shrikanth Hegde]

ok by me. Peter, would you be sending the patch?

> -Tyrel

[Peter Bergner]

On 3/6/25 9:44 PM, Shrikanth Hegde wrote:
>> Just wanted to confirm that we have a consensus on moving forward with this
>> patch? If so I will go ahead an revert the previously applied patch from Shrikanth.
>>
>
> ok by me. Peter, would you be sending the patch?

I think we did have consensus on the getauxval version of the patch.

Shrikanth, I was hoping you'd continue to sheppard the patch, as I'm swamped
with other work. Thanks.

Peter

[Shrikanth Hegde]

Ok sure, peter. Let me send the patch.

Tyrel, you can revert the previous patch. Let me send v3 of this.

>

[Shrikanth Hegde]

On 3/7/25 09:22, Peter Bergner wrote:

Hi Peter, Michal,

This is the patch i have at the moment. I didn't get the comment of some
odd old OS build might break. Is there anything more that needs to be
added to the below patch?

---

diff --git a/src/lparstat.c b/src/lparstat.c
index db22316..3300877 100644
--- a/src/lparstat.c
+++ b/src/lparstat.c
@@ -37,6 +37,7 @@
#include "pseries_platform.h"
#include "cpu_info_helpers.h"
#include <time.h>
+#include <sys/auxv.h>

#define LPARCFG_FILE "/proc/ppc64/lparcfg"
#define SE_NOT_FOUND "???"
@@ -794,7 +795,11 @@ void get_memory_mode(struct sysentry *se, char *buf)

struct sysentry *tmp;

tmp = get_sysentry("entitled_memory_pool_number");
- if (atoi(tmp->value) == 65535)
+ /*
+ * from power10 onwards Active Memory Sharing(AMS) is not
+ * supported. Hence always display it as dedicated for those
+ */

+ if (atoi(tmp->value) == 65535 || (getauxval(AT_HWCAP2) &
PPC_FEATURE2_ARCH_3_1))

[Michal Suchánek]

Hello,

^^^^ This may not be defined failing the build on older OS releases.

Thanks

Michal

[Shrikanth Hegde]

Ah ok.

Something like below will do for them?

+#ifndef PPC_FEATURE2_ARCH_3_1
+#define PPC_FEATURE2_ARCH_3_1 0x00040000
+#endif

[Michal Suchánek]

Yes, adding it is probably better than ignoring the problem on OS
versions that do not have up-to-date headers.

The other problem is that in x86 AT_HWCAP2 is probably not defined, and
to keep the x86 CI working that needs to be also ifdedef.

Thanks

Michal

[Shrikanth Hegde]

ok.

> The other problem is that in x86 AT_HWCAP2 is probably not defined, and
> to keep the x86 CI working that needs to be also ifdedef.
>

I am not familiar with glibc code much. Correct me if i am wrong.

I see AT_HWCAP2 is defined in elf/elf.h. That is arch independent right?
Even on x86 this definition should be available no?

https://elixir.bootlin.com/glibc/glibc-2.41.9000/source/elf/elf.h#L1237

> Thanks
>
> Michal

[Michal Suchánek]

If that is the case then it should be fine.

I suppose you can push the patch to guthub and run the CI to be sure.

Thanks

Michal

[Shrikanth Hegde]

Starting from power10, active memory sharing(AMS) is not supported.
So from power10 onwards the H_GET_MPP hcall fails and hence
corresponding fields in lparcfg are not populated, such as
entitled_memory_pool_number etc.

Use HWCAP mechanism and print memory model as dedicated for power10

onwards.

Suggested-by: Peter Bergner <ber...@linux.ibm.com>
Signed-off-by: Shrikanth Hegde <ssh...@linux.ibm.com>
---

v1->v2:

Use powerpc and built in support guards to fix build breaks in other
archs.

v2->v3:
Use HWCAP instead of gcc builtins to avoid build failure
and old OS build issues.
v2: https://groups.google.com/g/powerpc-utils-devel/c/eMDmWi-_BF4

src/lparstat.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/src/lparstat.c b/src/lparstat.c
index db22316..8eddd7c 100644
--- a/src/lparstat.c
+++ b/src/lparstat.c
@@ -37,6 +37,11 @@

#include "pseries_platform.h"
#include "cpu_info_helpers.h"
#include <time.h>
+#include <sys/auxv.h>

+

+#ifndef PPC_FEATURE2_ARCH_3_1
+#define PPC_FEATURE2_ARCH_3_1 0x00040000
+#endif

#define LPARCFG_FILE "/proc/ppc64/lparcfg"
#define SE_NOT_FOUND "???"

@@ -794,7 +799,11 @@ void get_memory_mode(struct sysentry *se, char *buf)

struct sysentry *tmp;

tmp = get_sysentry("entitled_memory_pool_number");
- if (atoi(tmp->value) == 65535)
+ /*
+ * from power10 onwards Active Memory Sharing(AMS) is not
+ * supported. Hence always display it as dedicated for those
+ */
+ if (atoi(tmp->value) == 65535 || (getauxval(AT_HWCAP2) & PPC_FEATURE2_ARCH_3_1))

sprintf(buf, "Dedicated");
else
sprintf(buf, "Shared");

--
2.39.3

[Shrikanth Hegde]

>>> Yes, adding it is probably better than ignoring the problem on OS
>>> versions that do not have up-to-date headers.
>>
>> ok.
>>
>>> The other problem is that in x86 AT_HWCAP2 is probably not defined, and
>>> to keep the x86 CI working that needs to be also ifdedef.
>>>
>>
>> I am not familiar with glibc code much. Correct me if i am wrong.
>>
>> I see AT_HWCAP2 is defined in elf/elf.h. That is arch independent right?
>> Even on x86 this definition should be available no?
>>
>> https://elixir.bootlin.com/glibc/glibc-2.41.9000/source/elf/elf.h#L1237
>
> If that is the case then it should be fine.
>
> I suppose you can push the patch to guthub and run the CI to be sure.
>
> Thanks

Hi Tyrel, I have sent v3 of the patch. Please revert v2 and consider
this patch instead.

Thanks

[Tyrel Datwyler]

The previous commit has been reverted on powerpc-utils/next branch.

https://github.com/ibm-power-utilities/powerpc-utils/commit/6c8bdb424654d91dfc6f00eed7892e962b7eee9a

-Tyrel

[Tyrel Datwyler]

On 3/10/25 9:59 AM, Shrikanth Hegde wrote:
> Starting from power10, active memory sharing(AMS) is not supported.
> So from power10 onwards the H_GET_MPP hcall fails and hence
> corresponding fields in lparcfg are not populated, such as
> entitled_memory_pool_number etc.
>
> Use HWCAP mechanism and print memory model as dedicated for power10
> onwards.
>
> Suggested-by: Peter Bergner <ber...@linux.ibm.com>
> Signed-off-by: Shrikanth Hegde <ssh...@linux.ibm.com>
> ---

Patch applied to powerpc-utils/next branch.

https://github.com/ibm-power-utilities/powerpc-utils/commit/4443a68d523043d69195e5a5a27fb9fc7ae8c50f

Thanks,
-Tyrel