[PATCH 12/11] s390x: Add Add glibc-hwcaps support

Stefan Liebler stli@linux.ibm.com
Tue Dec 8 15:47:22 GMT 2020 

On 12/7/20 9:16 AM, Florian Weimer via Libc-alpha wrote:
> * Florian Weimer via Libc-alpha:
> 
>> diff --git a/elf/tst-glibc-hwcaps-cache.script b/elf/tst-glibc-hwcaps-cache.script
>> index 6a4675f9bd..19b06d0adc 100644
>> --- a/elf/tst-glibc-hwcaps-cache.script
>> +++ b/elf/tst-glibc-hwcaps-cache.script
>> @@ -11,6 +11,16 @@ mkdirp 0770 $L/glibc-hwcaps/power10
>>  cp $B/elf/libmarkermod3-2.so $L/glibc-hwcaps/power9/libmarkermod3.so
>>  cp $B/elf/libmarkermod3-3.so $L/glibc-hwcaps/power10/libmarkermod3.so
>>  
>> +mkdirp 0770 $L/glibc-hwcaps/z13
>> +cp $B/elf/libmarkermod2-2.so $L/glibc-hwcaps/z13/libmarkermod2.so
>> +mkdirp 0770 $L/glibc-hwcaps/z14
>> +cp $B/elf/libmarkermod3-2.so $L/glibc-hwcaps/z14/libmarkermod3.so
>> +cp $B/elf/libmarkermod3-3.so $L/glibc-hwcaps/z14/libmarkermod3.so
>> +mkdirp 0770 $L/glibc-hwcaps/z15
>> +cp $B/elf/libmarkermod4-2.so $L/glibc-hwcaps/z15/libmarkermod4.so
>> +cp $B/elf/libmarkermod4-3.so $L/glibc-hwcaps/z15/libmarkermod4.so
>> +cp $B/elf/libmarkermod4-4.so $L/glibc-hwcaps/z15/libmarkermod4.so
>> +
>>  mkdirp 0770 $L/glibc-hwcaps/x86-64-v2
>>  cp $B/elf/libmarkermod2-2.so $L/glibc-hwcaps/x86-64-v2/libmarkermod2.so
>>  mkdirp 0770 $L/glibc-hwcaps/x86-64-v3
> 
> Stefan pointed out that this should be:
> 
> mkdirp 0770 $L/glibc-hwcaps/z13
> cp $B/elf/libmarkermod2-2.so $L/glibc-hwcaps/z13/libmarkermod2.so
> mkdirp 0770 $L/glibc-hwcaps/z14
> cp $B/elf/libmarkermod3-2.so $L/glibc-hwcaps/z13/libmarkermod3.so
> cp $B/elf/libmarkermod3-3.so $L/glibc-hwcaps/z14/libmarkermod3.so
> mkdirp 0770 $L/glibc-hwcaps/z15
> cp $B/elf/libmarkermod4-2.so $L/glibc-hwcaps/z13/libmarkermod4.so
> cp $B/elf/libmarkermod4-3.so $L/glibc-hwcaps/z14/libmarkermod4.so
> cp $B/elf/libmarkermod4-4.so $L/glibc-hwcaps/z15/libmarkermod4.so
> 
> I have fixed it on the fw/glibc-hwcaps branch.
> 
> Thanks,
> Florian
> 

Hi Florian,

I've had a look to your patches. Can you please adjust some lines. Then
this patch is okay for s390x:
- The commit subject-line contains "Add Add"


- If e.g. a machine newer-than-z15 does not have HWCAP_S390_SORT, then
it would fall back to z14:

diff --git a/sysdeps/s390/s390-64/dl-hwcaps-subdirs.c
b/sysdeps/s390/s390-64/dl-hwcaps-subdirs.c
index fa8d2ce1f1..3673808a45 100644
--- a/sysdeps/s390/s390-64/dl-hwcaps-subdirs.c
+++ b/sysdeps/s390/s390-64/dl-hwcaps-subdirs.c
@@ -41,11 +41,12 @@ _dl_hwcaps_subdirs_active (void)
     return _dl_hwcaps_subdirs_build_bitmask (subdirs_count, active);
   ++active;

-  /* z15.  */
+  /* z15.
+     Note: We do not list HWCAP_S390_SORT and HWCAP_S390_DFLT here as,
+     according to the Principles of Operation, those may be replaced or
removed
+     in future.  */
   if (!((GLRO (dl_hwcap) & HWCAP_S390_VXRS_EXT2)
-        && (GLRO (dl_hwcap) & HWCAP_S390_VXRS_PDE)
-        && (GLRO (dl_hwcap) & HWCAP_S390_SORT)
-        && (GLRO (dl_hwcap) & HWCAP_S390_DFLT)))
+        && (GLRO (dl_hwcap) & HWCAP_S390_VXRS_PDE)))
     return _dl_hwcaps_subdirs_build_bitmask (subdirs_count, active);
   ++active;




- I've asked the kernel-guys regarding AT_PLATFORM: The list is
complete, but it never contains archXYZ. This is only available for
binutils/gcc -march=archXYZ.
- If running e.g. on z196 or older, tst-glibc-hwcaps will always fail,
as level would be <= 9 which leads to fails:
TEST_COMPARE (marker2 (), MIN (level - 9, 2));
Therefore compute_level should always return the baseline for older
platforms.
- If we are running on z13 or newer and the kernel was booted with novx,
then AT_PLATFORM is z13 or newer, but _dl_hwcaps_subdirs_active will
return zero and the _dl_hwcaps_subdirs are not searched as HWCAP_S390_VX
and all the other VX.. flags are not set. This leads to a test fail.

diff --git a/sysdeps/s390/s390-64/tst-glibc-hwcaps.c
b/sysdeps/s390/s390-64/tst-glibc-hwcaps.c
index 39f56d0c81..690f0d5fab 100644
--- a/sysdeps/s390/s390-64/tst-glibc-hwcaps.c
+++ b/sysdeps/s390/s390-64/tst-glibc-hwcaps.c
@@ -26,33 +26,37 @@ extern int marker2 (void);
 extern int marker3 (void);
 extern int marker4 (void);

-/* Return the POWER level, 8 for the baseline.  */
+/* Return the arch level, 10 for the baseline libmarkermod*.so's.  */
 static int
 compute_level (void)
 {
   const char *platform = (const char *) getauxval (AT_PLATFORM);

-  int result;
-  if (sscanf (platform, "arch%d", &result) == 1)
-     return result;
-
   /* The arch* versions refer to the edition of the Principles of
      Operation, and they are off by two when compared with the recent
      product names.  (The code below should not be considered an
      accurate mapping to Principles of Operation editions for earlier
      AT_PLATFORM strings).  */
   if (strcmp (platform, "z900") == 0)
-    return 5;
+    return 10;
   if (strcmp (platform, "z990") == 0)
-    return 6;
+    return 10;
   if (strcmp (platform, "z9-109") == 0)
-    return 7;
+    return 10;
   if (strcmp (platform, "z10") == 0)
-    return 8;
+    return 10;
   if (strcmp (platform, "z196") == 0)
-    return 9;
+    return 10;
   if (strcmp (platform, "zEC12") == 0)
     return 10;
+
+  /* If we are running on z13 or newer and the kernel was booted with novx,
+     then AT_PLATFORM is z13 or newer, but _dl_hwcaps_subdirs_active will
+     return zero and the _dl_hwcaps_subdirs are not searched.  */
+  const unsigned long int hwcap = getauxval (AT_HWCAP);
+  if ((hwcap & HWCAP_S390_VX) == 0)
+    return 10;
+
   if (strcmp (platform, "z13") == 0)
     return 11;
   if (strcmp (platform, "z14") == 0)


I've also recognized that if build with gcc 6.5.0, I'll get test-fails:
elf/tst-glibc-hwcaps-cache
elf/tst-glibc-hwcaps-prepend-cache
elf/tst-ldconfig-X
elf/tst-ldconfig-bad-aux-cache
elf/tst-ldconfig-ld_so_conf-update
elf/tst-stringtable

It seems as it always fails with "String table is too large".
I've debugged elf/tst-stringtable into elf/stringtable:185:
else if (__builtin_add_overflow (previous->offset,
                                 previous->length + 1,
                                 &current->offset))
                error (EXIT_FAILURE, 0, _("String table is too large"));

It seems as gcc 6.5.0 __builtin_add_overflow is buggy:
al  %r12,12(%r9)
st  %r12,12(%r7)
jhe 0x1002762 <stringtable_finalize+274>
larl        %r4,0x10050d6
lghi        %r3,0
lghi        %r2,1
brasl       %r14,0x1001bf8 <error@plt>
=> The jump should jump away in case of no overflow.

(gdb) p	*previous (== r9)
$2 = {next = 0x0, length = 3, offset = 7, string = 0x3fffdc0b800 "999"}
(gdb) p	*current (== r7)
$3 = {next = 0x0, length = 10, offset = 11, string = 0x3fffdfbf5c0
"prefix/899"}
(gdb) p	*(uint32_t*) (12 + $r9)
$1 = 7
(gdb) i r r12 cc
r12            0x3ff0000000b	   4393751543819
cc             0x1                 1
=> cc is 1, but "jhe" jumps away for cc==0 or cc==2.
Resulting Condition Code (of al instruction):
0 Result zero; no carry
1 Result not zero; no carry
2 Result zero; carry
3 Result not zero; carry

If build with other GCC versions, I don't see this bug.
Have you recognized such a bug on other archs?


While viewing your previous patches, I've found the following "if" in
commit "elf: Add glibc-hwcaps subdirectory support to ld.so cache
processing":
elf/dl-hwcaps.c:
static void
+sort_priorities_by_name (void)
+{
...
+	int cmp = memcmp (current->name, previous->name, to_compare);
+	if (cmp >= 0
+	    || (cmp == 0 && current->name_length >= previous->name_length))
+	  break;
Is this condition intended? The second part "cmp == 0" will never be
evaluated as in this case, the first part "cmp >= 0" is already true.

Thanks,
Stefan

More information about the Libc-alpha mailing list