Re: [PATCH] power: Fix VSCR position on ucontext

[Rogerio Alves <rcardoso at linux dot ibm dot com>]

Hi everyone,

Here is patch v3 with the suggestions given. Sorry for take me that long 
to reply. Thanks for the review

Regards

Em 16-11-2018 11:18, Gabriel F. T. Gomes escreveu:
> On Wed, 14 Nov 2018, Florian Weimer wrote:
>
> > * Rogerio Alves:
> >
> > > +#ifdef _ARCH_PWR8
> >
> > Is it possible to perform run-time detection instead?

Now I am using auxval to get if the arch is supported for the test
> That's a fair point.  Even though we have bots that test powerpc64
> (big-endian) builds configured with `--with-cpu=power8', that's not the
> default.  Runtime detection could make this test work on default builds
> (provided they are run on a POWER8 machine).
>
> > +  /* Set SAT bit in VSCR register.  */
> > +  asm volatile ("vspltisb %0,0\n"
> > +                "vspltisb %1,-1\n"
> > +                "vpkudus %0,%0,%1\n"
>
> Also, if you replace `vpkudus' (only available since Power ISA 2.07) with
> `vpkuwus' (available since PowerISA 2.03) the test remains valid and it
> could be tested on older machines.

Done

>From f7d3468236f17f0d1c1ca5acf3865bc89ac27125 Mon Sep 17 00:00:00 2001
From: Rogerio Alves <rcardoso@linux.ibm.com>
Date: Mon, 5 Nov 2018 10:18:38 -0600
Subject: [PATCH v3] powerpc: Fix VSCR position in ucontext.

This patch fix VSCR position on ucontext. VSCR was read in the wrong
position on ucontext structure because it was ignoring the machine
endianess.

2018-11-05  Rogerio A. Cardoso  <rcardoso@linux.ibm.com>

	* sysdeps/unix/sysv/linux/powerpc/sys/ucontext.h
    (struct _libc_vscr, vscr_t): Added ifdef to fix read of VSCR.
	* sysdeps/powerpc/powerpc64/Makefile: Add tst-ucontext-ppc64-vscr.c
    to test list.
	* sysdeps/powerpc/powerpc64/tst-ucontext-ppc64-vscr.c: New test file.
---
 Patch v3: fixed as per Florian and Gabriel review. Using getauxval to get
 if (runtime) the test is supported instead using ifdef. Changed
 vpkudus por vpkuwus since it's supported by POWER 5 or higher.

 Patch v2: fixed as per Gabriel and Segher review. Fix formating and changelog
 Change the assembly inline to a better version. Changed ifdef to POWER8.

 sysdeps/powerpc/powerpc64/Makefile                 |  5 ++
 .../powerpc/powerpc64/tst-ucontext-ppc64-vscr.c    | 82 ++++++++++++++++++++++
 sysdeps/unix/sysv/linux/powerpc/sys/ucontext.h     |  5 ++
 3 files changed, 92 insertions(+)
 create mode 100644 sysdeps/powerpc/powerpc64/tst-ucontext-ppc64-vscr.c

diff --git a/sysdeps/powerpc/powerpc64/Makefile b/sysdeps/powerpc/powerpc64/Makefile
index a0bd0c9..6e88df1 100644
--- a/sysdeps/powerpc/powerpc64/Makefile
+++ b/sysdeps/powerpc/powerpc64/Makefile
@@ -59,3 +59,8 @@ $(objpfx)tst-setjmp-bug21895-static.out: $(objpfx)setjmp-bug21895.so
 tst-setjmp-bug21895-static-ENV = \
 	LD_LIBRARY_PATH=$(objpfx):$(common-objpfx):$(common-objpfx)setjmp:$(common-objpfx)elf
 endif
+
+ifeq ($(subdir),stdlib)
+CFLAGS-tst-ucontext-ppc64-vscr.c += -maltivec
+tests += tst-ucontext-ppc64-vscr
+endif
diff --git a/sysdeps/powerpc/powerpc64/tst-ucontext-ppc64-vscr.c b/sysdeps/powerpc/powerpc64/tst-ucontext-ppc64-vscr.c
new file mode 100644
index 0000000..be99108
--- /dev/null
+++ b/sysdeps/powerpc/powerpc64/tst-ucontext-ppc64-vscr.c
@@ -0,0 +1,82 @@
+/* Test if POWER vscr read by ucontext.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <support/check.h>
+#include <sys/auxv.h>
+#include <ucontext.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <altivec.h>
+
+#define SAT 0x1
+
+/* This test is supported only on POWER 5 or higher.  */
+#define PPC_CPU_SUPPORTED (PPC_FEATURE_POWER5 | PPC_FEATURE_POWER5_PLUS | \
+                           PPC_FEATURE_ARCH_2_05 | PPC_FEATURE_ARCH_2_06 | \
+                           PPC_FEATURE2_ARCH_2_07)
+static int
+do_test (void)
+{
+
+  if (!(getauxval(AT_HWCAP2) & PPC_CPU_SUPPORTED))
+    {
+      if (!(getauxval(AT_HWCAP) & PPC_CPU_SUPPORTED))
+      FAIL_UNSUPPORTED("This test is unsupported on POWER < 5\n");
+    }
+
+  uint32_t vscr[4] __attribute__ ((aligned (16)));
+  uint32_t* vscr_ptr = vscr;
+  uint32_t vscr_word;
+  ucontext_t ucp;
+  __vector __int128_t v0 = {0};
+  __vector __int128_t v1 = {0};
+
+  /* Set SAT bit in VSCR register.  */
+  asm volatile ("vspltisb %0,0\n"
+                "vspltisb %1,-1\n"
+                "vpkuwus %0,%0,%1\n"
+                "mfvscr %0\n"
+                "stvx %0,0,%2"
+       : "=v" (v0), "=v" (v1)
+       : "r" (vscr_ptr)
+       : "memory"
+       );
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+  vscr_word = vscr[0];
+#else
+  vscr_word = vscr[3];
+#endif
+
+  if ((vscr_word & SAT) != SAT)
+    {
+      FAIL_EXIT1("FAIL: SAT bit is not set.\n");
+    }
+
+  if (getcontext (&ucp))
+    {
+      FAIL_EXIT1("FAIL: getcontext error\n");
+    }
+  if (ucp.uc_mcontext.v_regs->vscr.vscr_word != vscr_word)
+    {
+      FAIL_EXIT1("FAIL: ucontext vscr does not match with vscr\n");
+    }
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/unix/sysv/linux/powerpc/sys/ucontext.h b/sysdeps/unix/sysv/linux/powerpc/sys/ucontext.h
index 1bb6e4c..49a92c5 100644
--- a/sysdeps/unix/sysv/linux/powerpc/sys/ucontext.h
+++ b/sysdeps/unix/sysv/linux/powerpc/sys/ucontext.h
@@ -98,8 +98,13 @@ typedef double fpregset_t[__NFPREG];
    a whole quadword speedup save/restore.  */
 typedef struct _libc_vscr
 {
+#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
 	unsigned int __pad[3];
 	unsigned int __ctx(vscr_word);
+#else
+	unsigned int __ctx(vscr_word);
+	unsigned int __pad[3];
+#endif
 } vscr_t;
 
 /* Container for Altivec/VMX registers and status.
-- 
2.7.4