[PATCH] powerpc: Add space for HWCAP3/HWCAP4 in the TCB for future Power.

[PATCH] powerpc: Add space for HWCAP3/HWCAP4 in the TCB for future Power.

[Manjunath Matti]

This patch reserves space for HWCAP3/HWCAP4 in the TCB of powerpc.
These hardware capabilities bits will be used by future Power
architectures.

This is an ABI change for GLIBC 2.39.

Suggested-by: Peter Bergner <bergner@linux.ibm.com>
---
 sysdeps/powerpc/hwcapinfo.c          |  1 +
 sysdeps/powerpc/nptl/tcb-offsets.sym |  1 +
 sysdeps/powerpc/nptl/tls.h           | 13 ++++++++++++-
 3 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/sysdeps/powerpc/hwcapinfo.c b/sysdeps/powerpc/hwcapinfo.c
index f2c473c556..bf46a4c84d 100644
--- a/sysdeps/powerpc/hwcapinfo.c
+++ b/sysdeps/powerpc/hwcapinfo.c
@@ -70,6 +70,7 @@ __tcb_parse_hwcap_and_convert_at_platform (void)
   /* Consolidate both HWCAP and HWCAP2 into a single doubleword so that
      we can read both in a single load later.  */
   __tcb.hwcap = (h1 << 32) | (h2 & 0xffffffff);
+  __tcb.hwcap_extn = 0x0;
 
 }
 #if IS_IN (rtld)
diff --git a/sysdeps/powerpc/nptl/tcb-offsets.sym b/sysdeps/powerpc/nptl/tcb-offsets.sym
index 4c01615ad0..9b29fe8d1a 100644
--- a/sysdeps/powerpc/nptl/tcb-offsets.sym
+++ b/sysdeps/powerpc/nptl/tcb-offsets.sym
@@ -26,3 +26,4 @@ TCB_AT_PLATFORM			(offsetof (tcbhead_t, at_platform) - TLS_TCB_OFFSET - sizeof(t
 PADDING				(offsetof (tcbhead_t, padding) - TLS_TCB_OFFSET - sizeof(tcbhead_t))
 #endif
 TCB_HWCAP			(offsetof (tcbhead_t, hwcap) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
+TCB_HWCAP_EXTN			(offsetof (tcbhead_t, hwcap_extn) - TLS_TCB_OFFSET - sizeof (tcbhead_t))
diff --git a/sysdeps/powerpc/nptl/tls.h b/sysdeps/powerpc/nptl/tls.h
index a668798181..3f8eca57b5 100644
--- a/sysdeps/powerpc/nptl/tls.h
+++ b/sysdeps/powerpc/nptl/tls.h
@@ -64,6 +64,9 @@
    are private.  */
 typedef struct
 {
+  /* Reservation for HWCAP3 and HWCAP4 data.  To be accessed by GCC in
+     __builtin_cpu_supports(), so it is a part of the public ABI.  */
+  uint64_t hwcap_extn;
   /* Reservation for HWCAP data.  To be accessed by GCC in
      __builtin_cpu_supports(), so it is a part of public ABI.  */
   uint64_t hwcap;
@@ -138,6 +141,7 @@ typedef struct
   ({ 									      \
     __thread_register = (void *) (tcbp) + TLS_TCB_OFFSET;		      \
     THREAD_SET_HWCAP (__tcb.hwcap);					      \
+    THREAD_SET_HWCAP_EXTN (__tcb.hwcap_extn);				      \
     THREAD_SET_AT_PLATFORM (__tcb.at_platform);				      \
     true;								      \
   })
@@ -147,6 +151,8 @@ typedef struct
     void *tp = (void *) (pd) + TLS_TCB_OFFSET + TLS_PRE_TCB_SIZE;	      \
     (((tcbhead_t *) ((char *) tp - TLS_TCB_OFFSET))[-1].hwcap) =	      \
       THREAD_GET_HWCAP ();						      \
+    (((tcbhead_t *) ((char *) tp - TLS_TCB_OFFSET))[-1].hwcap_extn) =	      \
+      THREAD_GET_HWCAP_EXTN ();						      \
     (((tcbhead_t *) ((char *) tp - TLS_TCB_OFFSET))[-1].at_platform) =	      \
       THREAD_GET_AT_PLATFORM ();
 
@@ -189,12 +195,17 @@ typedef struct
 		     + TLS_PRE_TCB_SIZE))[-1].pointer_guard		      \
      = THREAD_GET_POINTER_GUARD())
 
-/* hwcap field in TCB head.  */
+/* hwcap & hwcap_extn fields in TCB head.  */
 # define THREAD_GET_HWCAP() \
     (((tcbhead_t *) ((char *) __thread_register				      \
 		     - TLS_TCB_OFFSET))[-1].hwcap)
+# define THREAD_GET_HWCAP_EXTN() \
+    (((tcbhead_t *) ((char *) __thread_register				      \
+		     - TLS_TCB_OFFSET))[-1].hwcap_extn)
 # define THREAD_SET_HWCAP(value) \
     (THREAD_GET_HWCAP () = (value))
+# define THREAD_SET_HWCAP_EXTN(value) \
+    (THREAD_GET_HWCAP_EXTN () = (value))
 
 /* at_platform field in TCB head.  */
 # define THREAD_GET_AT_PLATFORM() \
-- 
2.39.3

Re: [PATCH] powerpc: Add space for HWCAP3/HWCAP4 in the TCB for future Power.

[Florian Weimer]

* Manjunath Matti:

> This patch reserves space for HWCAP3/HWCAP4 in the TCB of powerpc.
> These hardware capabilities bits will be used by future Power
> architectures.
>
> This is an ABI change for GLIBC 2.39.

Sorry, I don't see the ABI change.

Peter and I discussed this a while ago, and it's possible to avoid the
ABI change by indicating the presence of hwcap_extn with another flag in
hwcap.  Do you plan to follow this path?  The patch does not make this
clear.

Thanks,
Florian

Re: [PATCH] powerpc: Add space for HWCAP3/HWCAP4 in the TCB for future Power.

[Peter Bergner]

On 11/29/23 7:55 AM, Florian Weimer wrote:
>> This patch reserves space for HWCAP3/HWCAP4 in the TCB of powerpc.
>> These hardware capabilities bits will be used by future Power
>> architectures.
>>
>> This is an ABI change for GLIBC 2.39.
> 
> Sorry, I don't see the ABI change.

Hi Florian,

The ABI extension is the extra space added to tcbhead_t via the new
hwcap_extn field.  Currently, this patch just reserves the space and
initializes it to zero.  Manjunath has a follow-on patch which is
dependent on a linux kernel patch to define AT_HWCAP3 and AT_HWCAP4
which is still in the process of getting upstreamed.  There have been
no objections and the aarch64 team mentioned needing HWCAP3 soon-ish
too, it's just taking a bit to get pulled.

We were going to wait to post Manjunath's follow-on patch until the
kernel change went in, but I guess he could submit it now so you can
see what it does, but obviously, we have to wait for the kernel patch
to land before pushing it to glibc, so we can reply on the AT_HWCAP3
and AT_HWCAP4 values defined by the kernel.

Manjunath, can you please post your 2nd patch for review so everyone
can see have a look at it now, while we wait for the kernel changes
to land?  Thanks.



> Peter and I discussed this a while ago, and it's possible to avoid the
> ABI change by indicating the presence of hwcap_extn with another flag in
> hwcap.  Do you plan to follow this path?  The patch does not make this
> clear.

All true, but we decided against that solution.  Currently, a call to
__builtin_cpu_supports ("foo") for a HWCAP/HWCAP2 feature will expand
into a load-compare-branch sequence.  If we use your suggestion, we
would eliminate the ABI extension, but we'd end up having to execute
a load-compare-branch-load-compare-branch sequence for the HWCAP3/HWCAP4
features, which when they exist, will probably be the most commonly
tested for features, essentially doubling the cost of the common case.

Instead, we've decided to go with the extended ABI solution, but early
enough, so that when we finally have HWCAP3 features, most (all?) of the
shipping distro toolchains should already support the new HWCAP3/HWCAP4
fields in tcbhead_t.

Peter

Re: [PATCH] powerpc: Add space for HWCAP3/HWCAP4 in the TCB for future Power.

[Peter Bergner]

On 11/29/23 10:48 PM, Peter Bergner wrote:
> On 11/29/23 7:55 AM, Florian Weimer wrote:
>>> This patch reserves space for HWCAP3/HWCAP4 in the TCB of powerpc.
>>> These hardware capabilities bits will be used by future Power
>>> architectures.
>>>
>>> This is an ABI change for GLIBC 2.39.
>>
>> Sorry, I don't see the ABI change.
> 
> Hi Florian,
> 
> The ABI extension is the extra space added to tcbhead_t via the new
> hwcap_extn field.  Currently, this patch just reserves the space and
> initializes it to zero.
[snip]

Hi Florian,

Thinking about this more, I guess the exported symbols announcing the
availability of that stack space which is currently in Manjunath's
2nd patch should probably be added to this patch, since the exported
symbols are themselves also part of the ABI.  Is that part of what
caused this not to be clear?

Peter

Re: [PATCH] powerpc: Add space for HWCAP3/HWCAP4 in the TCB for future Power.

[Florian Weimer]

* Peter Bergner:

> On 11/29/23 10:48 PM, Peter Bergner wrote:
>> On 11/29/23 7:55 AM, Florian Weimer wrote:
>>>> This patch reserves space for HWCAP3/HWCAP4 in the TCB of powerpc.
>>>> These hardware capabilities bits will be used by future Power
>>>> architectures.
>>>>
>>>> This is an ABI change for GLIBC 2.39.
>>>
>>> Sorry, I don't see the ABI change.
>> 
>> Hi Florian,
>> 
>> The ABI extension is the extra space added to tcbhead_t via the new
>> hwcap_extn field.  Currently, this patch just reserves the space and
>> initializes it to zero.
> [snip]
>
> Hi Florian,
>
> Thinking about this more, I guess the exported symbols announcing the
> availability of that stack space which is currently in Manjunath's
> 2nd patch should probably be added to this patch, since the exported
> symbols are themselves also part of the ABI.  Is that part of what
> caused this not to be clear?

Yes, I was confused because there was no discernible ABI change.  If
there's the symbol and compilers will emit references to it, then yes,
this looks good to me.

Thanks,
Florian

Re: [PATCH] powerpc: Add space for HWCAP3/HWCAP4 in the TCB for future Power.

[Peter Bergner]

On 11/30/23 11:45 AM, Florian Weimer wrote:
>> On 11/29/23 10:48 PM, Peter Bergner wrote:
>> Thinking about this more, I guess the exported symbols announcing the
>> availability of that stack space which is currently in Manjunath's
>> 2nd patch should probably be added to this patch, since the exported
>> symbols are themselves also part of the ABI.  Is that part of what
>> caused this not to be clear?
> 
> Yes, I was confused because there was no discernible ABI change.  If
> there's the symbol and compilers will emit references to it, then yes,
> this looks good to me.

Ok, thanks.

Manjunath, can you please repost your two patches, except please move
the special exported symbol from patch 2 into patch 1, since that is
also part of the ABI extension.  Thanks.

Peter