diff mbox series

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

Message ID 20231129083055.3627888-1-mmatti@linux.ibm.com
State Superseded
Headers
Series powerpc: Add space for HWCAP3/HWCAP4 in the TCB for future Power. |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
redhat-pt-bot/TryBot-32bit success Build for i686
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Testing passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Testing passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Testing passed

Commit Message

Manjunath Matti Nov. 29, 2023, 8:30 a.m. UTC 
  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(-)

Comments

Florian Weimer Nov. 29, 2023, 1:55 p.m. UTC | #1 
* 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
Peter Bergner Nov. 30, 2023, 5:38 p.m. UTC | #2 
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
Florian Weimer Nov. 30, 2023, 5:45 p.m. UTC | #3 
* 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
Peter Bergner Dec. 1, 2023, 4:05 a.m. UTC | #4 
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
diff mbox series

Patch

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() \